Commit Graph

7876 Commits

Author SHA1 Message Date
Sasa Stamenkovic
74548943e2 Added missing dot to translation messages. 2012-04-13 21:43:24 +02:00
Sasa Stamenkovic
7e21dd1c57 Added missing dot to translation messages. 2012-04-13 21:43:23 +02:00
Fabien Potencier
22177fa029 merged branch vicb/profiler/listener (PR #3920)
Commits
-------

b611db8 [Profiler] Sub requests are not Main requests
2551270 [Profiler] Minimize the number of Profile writes

Discussion
----------

[HttpKernel] Profiler Listener tweaks

* `setParent()` is called in [`Profile::addChild()`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Profiler/Profile.php#L180) in 2.1
* The profiles are now only saved once only in the listener (either at the end of the main request or on an exception)
* The profiles are now only saved once only in the TraceableEventDispatcher (twice for the root profile when there is a kernel.terminate' event

[![Build Status](https://secure.travis-ci.org/vicb/symfony.png?branch=profiler/listener)](http://travis-ci.org/vicb/symfony)

---------------------------------------------------------------------------

by vicb at 2012-04-13T11:15:25Z

Not so sure for the save part... I'll double check
2012-04-13 18:56:35 +02:00
Victor Berchet
b611db8e41 [Profiler] Sub requests are not Main requests 2012-04-13 18:42:55 +02:00
Victor Berchet
255127081a [Profiler] Minimize the number of Profile writes
squash

squash
2012-04-13 18:30:43 +02:00
Bernhard Schussek
6df7a7223e [Form] Deprecated FormValidatorInterface and moved implementations to event listeners 2012-04-13 16:42:01 +02:00
Fabien Potencier
463114134b merged branch asm89/configurable-session-save-path (PR #3912)
Commits
-------

c0e7ee9 [FrameworkBundle] Make session save path configurable

Discussion
----------

[FrameworkBundle] Make session save path configurable

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=configurable-session-save-path)](http://travis-ci.org/asm89/symfony)

Makes it possible to configure the session save path. It still defaults to saving sessions in the kernel cache dir. This might not be appropriate if you want to keep your user sessions after deploying your application.

As promised to @fabpot I will also do a PR on the docs explaining why this might be useful.

---------------------------------------------------------------------------

by drak at 2012-04-13T10:16:17Z

It might be good to default this value to `%kernel.cache_dir%/sessions`.

---------------------------------------------------------------------------

by stloyd at 2012-04-13T10:30:58Z

@drak https://github.com/symfony/symfony/pull/3912/files#L0R184

---------------------------------------------------------------------------

by drak at 2012-04-13T10:31:57Z

@stloyd need my eyes checked :-P
2012-04-13 14:51:40 +02:00
Fabien Potencier
1547a82eae merged branch Tobion/httpkernel-test (PR #3906)
Commits
-------

c331f4a HttpKernel test fix on windows

Discussion
----------

HttpKernel test fix on windows

The changes in `StopwatchEventTest` are only for consistency with the other tests in this file.

---------------------------------------------------------------------------

by drak at 2012-04-13T04:16:19Z

@fabpot - This seems to be an eternal problem with the these particular tests. I wonder if there is a better way to do this.  How about a simple greater than condition to show time has elapsed?

---------------------------------------------------------------------------

by Tobion at 2012-04-13T04:33:04Z

The tests are fine. I didn't change them. Just made it more consistent.

---------------------------------------------------------------------------

by drak at 2012-04-13T04:49:20Z

Yes, but if you look at the history of these tests files, they are constantly being tweaked for whatever reason (and they often fail on windows builds randomly). This is a clear indication the tests are not robust and a different approach is probably warranted if it can be found.

---------------------------------------------------------------------------

by drak at 2012-04-13T04:52:53Z

@Tobion - regarding the commit message, what does "fix" refer to if the tests are fine?

---------------------------------------------------------------------------

by Tobion at 2012-04-13T04:56:39Z

The test in `KernelTest` did not pass for me. That's fixed.
2012-04-13 14:45:21 +02:00
Alexander
c0e7ee9a6c [FrameworkBundle] Make session save path configurable 2012-04-13 10:48:56 +02:00
Fabien Potencier
114bc14c21 [FrameworkBundle] fixed name collision 2012-04-13 09:04:04 +02:00
Fabien Potencier
d87a197d80 merged branch drak/kernel_dicb (PR #3909)
Commits
-------

82bbf3b [HttpKernel] Allow override of ContainerBuilder instance used to build container

Discussion
----------

[HttpKernel] Allow override of ContainerBuilder class in Kernel

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

The ContainerBuilder class is hard coded into the `buildContainer()` method and is therefor not extensible.

---------------------------------------------------------------------------

by fabpot at 2012-04-13T04:54:28Z

I would definitely prefer a `getContainerBuilder()` that returns a container builder. But why would you want to do that?

---------------------------------------------------------------------------

by drak at 2012-04-13T05:12:13Z

@fabpot - The use case is to override the behaviour of the compilation that is performed by the kernel on the container.  There is no way to override the compile() method called on the builder because the class is created hard in `Kernel::buildContainer()`. This was the simplest way to change it without other cascading changes.

If we had a method which returned a container builder instance, would it just be something that creates a container builder, or acts as a getter on a property? The reason I ask is there is no real need to have a container builder persisting in a property, so a method that just returns a `new ContainerBuilder()` would also work but then we have to deal with the constructor as there is no way to set a ParameterBag on a container and `Kernel::buildContainer()` needs that.

Basically, my reasoning is that since this particular container is just a throwaway, it seems ok to control the class name that was used to create the throwaway builder.

So are you suggesting doing this instead?

    protected function getContainerBuilder()
    {
        return new ContainerBuilder(new ParameterBag($this->getKernelParameters()));
    }

---------------------------------------------------------------------------

by fabpot at 2012-04-13T05:22:20Z

Yes this was my suggestion. But are you sure you cannot solve your problems with compiler passes?

---------------------------------------------------------------------------

by drak at 2012-04-13T05:38:29Z

@fabpot, yes, this particular issue can't be solved by compiler passes.  I'll adapt the PR.

---------------------------------------------------------------------------

by drak at 2012-04-13T05:46:03Z

@fabpot, amended as per your suggestion.
2012-04-13 07:50:57 +02:00
Drak
82bbf3b8b1 [HttpKernel] Allow override of ContainerBuilder instance used to build container 2012-04-13 11:27:54 +05:45
Fabien Potencier
70df8d3892 [FrameworkBundle] made the Esi and Store instances configurable in HttpCache base class 2012-04-13 07:23:33 +02:00
Fabien Potencier
0956be908c merged branch Tobion/route-compiler (PR #3891)
Commits
-------

cb47b03 [Routing] small refactoring + language fixes

Discussion
----------

[Routing] small refactoring + language fixes

tests pass, no bc break

---------------------------------------------------------------------------

by vicb at 2012-04-12T06:42:19Z

@Tobion could you squash your commits ?

---------------------------------------------------------------------------

by Tobion at 2012-04-12T19:25:03Z

done
2012-04-13 06:59:08 +02:00
Fabien Potencier
cec8fed31c merged branch willdurand/add-all-method-to-form-builder (PR #3886)
Commits
-------

be2456b [Form] [Tests] Used assertCount()
4120f13 [Form] Added all() method to the FormBuilder class

Discussion
----------

[Form] Added all() method to the FormBuilder class

In order to perform some introspection on a FormBuilder instance,
we need to be able to get its children. Almost everything is accessible
in this class, but the children are not.

This PR adds a all() method to respect the current API (get(), remove(),
...).

---------------------------------------------------------------------------

by bschussek at 2012-04-12T09:54:04Z

👍
2012-04-13 06:56:27 +02:00
Tobias Schultze
c331f4a306 HttpKernel test fix on windows 2012-04-13 00:48:19 +02:00
Tobias Schultze
cb47b03209 [Routing] small refactoring + language fixes 2012-04-12 21:23:30 +02:00
Drak
e199049581 [EventDispatcher] Fixed edge case not covered by tests that generated E_NOTICES 2012-04-12 16:21:33 +05:45
Fabien Potencier
5a320ca7bd merged 2.0 2012-04-12 12:30:32 +02:00
Fabien Potencier
5ab006aca2 merged branch Tobion/generator-dumper (PR #3894)
Commits
-------

382b083 [Routing] improved generated class by PhpGeneratorDumper
27a05f4 [Routing] small optimization of PhpGeneratorDumper

Discussion
----------

[Routing] improved PhpGeneratorDumper

Test pass: yes
BC break: no

The first commit only replaces arrays with strings and makes some cosmetic changes, so that it's more readable. This makes the `PhpGeneratorDumper` consistent in style with `PhpMatcherDumper` that I fixed recently and should slightly improve performance of the generation of the class.

The second commit changes the output of the `PhpGeneratorDumper->dump` and tries to optimize the resulting class that is used to generate URLs. It's best explained with an example.

Before my changes:

```php
class ProjectUrlGenerator extends Symfony\Component\Routing\Generator\UrlGenerator
{
    static private $declaredRouteNames = array(
       'Test' => true,
       'Test2' => true,
    );

    /**
     * Constructor.
     */
    public function __construct(RequestContext $context)
    {
        $this->context = $context;
    }

    public function generate($name, $parameters = array(), $absolute = false)
    {
        if (!isset(self::$declaredRouteNames[$name])) {
            throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
        }

        $escapedName = str_replace('.', '__', $name);

        list($variables, $defaults, $requirements, $tokens) = $this->{'get'.$escapedName.'RouteInfo'}();

        return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute);
    }

    private function getTestRouteInfo()
    {
        return array(array (  0 => 'foo',), array (), array (), array (  0 =>   array (    0 => 'variable',    1 => '/',    2 => '[^/]+?',    3 => 'foo',  ),  1 =>   array (    0 => 'text',    1 => '/testing',  ),));
    }

    private function getTest2RouteInfo()
    {
        return array(array (), array (), array (), array (  0 =>   array (    0 => 'text',    1 => '/testing2',  ),));
    }
}
```

After my changes in second commit:

```php
class ProjectUrlGenerator extends Symfony\Component\Routing\Generator\UrlGenerator
{
    static private $declaredRoutes = array(
        'Test' => array (  0 =>   array (    0 => 'foo',  ),  1 =>   array (  ),  2 =>   array (  ),  3 =>   array (    0 =>     array (      0 => 'variable',      1 => '/',      2 => '[^/]+?',      3 => 'foo',    ),    1 =>     array (      0 => 'text',      1 => '/testing',    ),  ),),
        'Test2' => array (  0 =>   array (  ),  1 =>   array (  ),  2 =>   array (  ),  3 =>   array (    0 =>     array (      0 => 'text',      1 => '/testing2',    ),  ),),
    );

    /**
     * Constructor.
     */
    public function __construct(RequestContext $context)
    {
        $this->context = $context;
    }

    public function generate($name, $parameters = array(), $absolute = false)
    {
        if (!isset(self::$declaredRoutes[$name])) {
            throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
        }

        list($variables, $defaults, $requirements, $tokens) = self::$declaredRoutes[$name];

        return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute);
    }
}
```

As you can see, there is no need to escape the route name and invoke a special method anymore. Instead the route properties are included in the static route array directly, that existed anyway. Is also easier to read as defined routes and their properties are in the same place.
2012-04-12 12:27:45 +02:00
Drak
57dd9147d9 [EventDispatcher] Fixed E_NOTICES with multiple eventnames per subscriber with mixed priorities 2012-04-12 15:56:02 +05:45
Tobias Schultze
382b08361b [Routing] improved generated class by PhpGeneratorDumper 2012-04-12 07:35:08 +02:00
Fabien Potencier
bf1131cc50 merged branch Tobion/generator-cache (PR #3892)
Commits
-------

03d30fd [Routing] remove duplicated cache of compiled routes

Discussion
----------

[Routing] remove duplicated cache of compiled routes

The UrlGenerator caches compiled routes for generating URLs. But the Route class caches it's compiled route itself as long as it does not get modified. So compiled routes are cached twice which makes no sense in my opinion.

Test pass: yes
BC break: no
2012-04-12 06:57:29 +02:00
Fabien Potencier
3923ade0aa merged branch kimhemsoe/process_deadlock (PR #3888)
Commits
-------

89a5c1a [process] Added destructor to process to make sure handles are always closed in the right order.

Discussion
----------

[process] Added destructor to process to make sure handles are always cl...

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-04-12 06:56:15 +02:00
Tobias Schultze
27a05f4c24 [Routing] small optimization of PhpGeneratorDumper 2012-04-12 06:16:26 +02:00
Tobias Schultze
03d30fdadb [Routing] remove duplicated cache of compiled routes 2012-04-12 04:08:51 +02:00
Jordan Alliot
6483d88f69 [Security][ACL] Fixed ObjectIdentity::fromDomainObject and UserSecurityIdentity::from(Account|Token) when working with proxies
Backported ClassUtils class from Doctrine Common 2.2
Fixes #2611, #2056, #2048, #2035
2012-04-12 00:40:59 +02:00
Kim Hemsø Rasmussen
89a5c1a845 [process] Added destructor to process to make sure handles are always closed in the right order. 2012-04-11 23:08:57 +02:00
William DURAND
be2456b19e [Form] [Tests] Used assertCount() 2012-04-11 20:45:41 +02:00
Josip Kruslin
771ab45f08 Updated croatian validator translation and deleted validator xliff file 2012-04-11 20:41:00 +02:00
Fabien Potencier
570bb6ab67 merged branch willdurand/fix-form-type-doc (PR #3880)
Commits
-------

6f56dfc [Form] Fixed DateType default options
779d3bb [Form] Fixed documentation, and the DateType (default options)

Discussion
----------

[Form] Fixed documentation, and the DateType (default options)

---------------------------------------------------------------------------

by fabpot at 2012-04-11T16:48:04Z

That breaks the tests.

---------------------------------------------------------------------------

by willdurand at 2012-04-11T16:50:35Z

I got an error with the Form test suite before to write this patch..

---------------------------------------------------------------------------

by willdurand at 2012-04-11T16:53:30Z

Nevermind, I can see broken tests.. I'm on, sorry

---------------------------------------------------------------------------

by willdurand at 2012-04-11T16:57:52Z

@fabpot fixed.

```
OK, but incomplete or skipped tests!
Tests: 945, Assertions: 1439, Incomplete: 11.
```
2012-04-11 19:18:10 +02:00
William DURAND
4120f1392f [Form] Added all() method to the FormBuilder class
In order to perform some introspection on a FormBuilder instance,
we need to be able to get its children. Almost everything is accessible
in this class, but the children are not.

This PR adds a all() method to respect the current API (get(), remove(),
...).
2012-04-11 19:12:00 +02:00
William DURAND
6f56dfc0d6 [Form] Fixed DateType default options 2012-04-11 18:56:33 +02:00
Fabien Potencier
61bec64003 [HttpFoundation] added missing variable declaration 2012-04-11 18:56:05 +02:00
Tobias Schultze
f666836900 [Routing] simplified regex with named variables 2012-04-11 18:27:19 +02:00
William DURAND
779d3bbb8e [Form] Fixed documentation, and the DateType (default options) 2012-04-11 17:26:46 +02:00
Fabien Potencier
05842c54b8 merged branch bschussek/issue3354 (PR #3789)
Commits
-------

8329087 [Form] Moved calculation of ChoiceType options to closures
5adec19 [Form] Fixed typos
cb87ccb [Form] Failing test for empty_data option BC break
b733045 [Form] Fixed option support in Form component

Discussion
----------

[Form] Fixed option support in Form component

Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #3354, #3512, #3685, #3694
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3354)

This PR also introduces a new helper `DefaultOptions` for solving option graphs. It accepts default options to be defined on various layers of your class hierarchy. These options can then be merged with the options passed by the user. This is called *resolving*.

The important feature of this utility is that it lets you define *lazy options*. Lazy options are specified using closures that are evaluated when resolving and thus have access to the resolved values of other (potentially lazy) options. The class detects cyclic option dependencies and fails with an exception in this case.

For more information, check the inline documentation of the `DefaultOptions` class and the UPGRADE file.

@fabpot: Might this be worth a separate component? (in total the utility consists of five classes with two associated tests)

---------------------------------------------------------------------------

by beberlei at 2012-04-05T08:54:10Z

"The important feature of this utility is that it lets you define lazy options. Lazy options are specified using closures"

What about options that are closures? are those differentiated?

---------------------------------------------------------------------------

by bschussek at 2012-04-05T08:57:35Z

@beberlei Yes. Closures for lazy options receive a Symfony\Component\Form\Options instance as first argument. All other closures are interpreted as normal values.

---------------------------------------------------------------------------

by stof at 2012-04-05T11:09:49Z

I'm wondering if these classes should go in the Config component. My issue with it is that it would add a required dependency to the Config component and that the Config component mixes many different things in it already (the loader part, the resource part, the definition part...)

---------------------------------------------------------------------------

by sstok at 2012-04-06T13:36:36Z

Sharing the Options class would be great, and its more then one class so why not give it its own Component folder?
Filesystem is just one class, and that has its own folder.

Great job on the class bschussek 👏

---------------------------------------------------------------------------

by bschussek at 2012-04-10T12:32:34Z

@fabpot Any input?

---------------------------------------------------------------------------

by bschussek at 2012-04-10T13:54:13Z

@fabpot Apart from the decision about the final location of DefaultOptions et al., could you merge this soon? This would make my work a bit easier since this one is a blocker.

---------------------------------------------------------------------------

by fabpot at 2012-04-10T18:08:18Z

@bschussek: Can you rebase on master? I will merge afterwards. Thanks.
2012-04-11 17:03:28 +02:00
Bernhard Schussek
8329087a20 [Form] Moved calculation of ChoiceType options to closures 2012-04-11 16:50:57 +02:00
Bernhard Schussek
5adec19f56 [Form] Fixed typos 2012-04-11 16:37:42 +02:00
Jeremy Mikola
cb87ccb284 [Form] Failing test for empty_data option BC break
This demonstrates the issue described in symfony/symfony#3354. FieldType no longer has access to the child type's data_class option, which makes it unable to create the default closure for empty_data.
2012-04-11 16:37:42 +02:00
Bernhard Schussek
b7330456b6 [Form] Fixed option support in Form component 2012-04-11 16:37:42 +02:00
Fabien Potencier
cc833b13b6 merged branch hason/validator (PR #552)
Commits
-------

f9a486e [Validator] Added support for pluralization of the SizeLengthValidator
c0715f1 [FrameworkBundle], [TwigBundle] added support for form error message pluralization
7a6376e [Form] added support for error message pluralization
345981f [Validator] added support for plural messages

Discussion
----------

[Validator] Added support for plural error messages

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Todo: create translations for en and update others (FrameworkBundle)

[![Build Status](https://secure.travis-ci.org/hason/symfony.png?branch=validator)](http://travis-ci.org/hason/symfony)

---------------------------------------------------------------------------

by fabpot at 2011-05-14T20:41:01Z

@bschussek: What's your opinion?

---------------------------------------------------------------------------

by stof at 2011-09-04T13:14:29Z

@hason could you rebase your branch on top of master and update the PR ?

You also need to change the messages in the constraint that uses the pluralization to a pluralized format.

---------------------------------------------------------------------------

by stof at 2011-10-16T18:06:22Z

@hason ping

---------------------------------------------------------------------------

by stof at 2011-11-11T14:58:19Z

@hason ping again

---------------------------------------------------------------------------

by stof at 2011-12-12T20:39:10Z

@hason ping again. Can you update your PR ?

---------------------------------------------------------------------------

by hason at 2011-12-12T21:29:14Z

@stof I hope that I will update PR this week.

---------------------------------------------------------------------------

by bschussek at 2012-01-15T19:07:32Z

Looks good to me.

---------------------------------------------------------------------------

by canni at 2012-02-02T17:28:54Z

@hason can you update this PR and squash commits, it conflicts with current master

---------------------------------------------------------------------------

by hason at 2012-02-09T07:21:41Z

@stof, @canni Rebased.

What is the best solution for the translation of messages?

1. Change messages in the classes and all xliff files?
2. Keep messages in the classes and change all xliff files?

---------------------------------------------------------------------------

by stof at 2012-02-09T08:19:41Z

The constraints contain the en message so you will need to modify them to update the message

---------------------------------------------------------------------------

by hason at 2012-02-09T08:55:55Z

I prefer second option. The Validator component should be decoupled from the Translation component. The constraints contain the en message which is also the key for Translation component. We should create validators.en.xlf in the FrameworkBundle for en message. I think that this is better solution. What do you think?

---------------------------------------------------------------------------

by stof at 2012-04-04T02:22:02Z

@hason Please rebase your branch. It conflicts with master because of the move of the tests

@fabpot ping
2012-04-11 16:12:39 +02:00
Fabien Potencier
70d49c3c2c merged branch jakzal/FilesystemMirrorCleanup (PR #3844)
Commits
-------

efad5d5 [Filesystem] Prevented infiite loop on windows while calling mirror on symlink. Added test for mirroring symlinks.

Discussion
----------

[Filesystem] Prevented infinite loop on windows while mirrorring symlinks

First check for filetype in *mirror()* method is:

    if (is_link($file)) {
        $this->symlink($file, $target);

later we see:

    } elseif (is_file($file) || ($copyOnWindows && is_link($file))) {
        $this->copy($file, $target, isset($options['override']) ? $options['override'] : false);

The later check for links on windows (*$copyOnWindows && is_link($file)*) won't ever get called. Calling *symlink()* in *mirror()* on windows would lead to calling *mirror()* again.

Note that I didn't actually try running it on windows platform. I added a test for mirroring symlinks (non-windows test). I think it'd be good if someone added some windows specific tests to this class.

I also modified the target path:

    $target = $targetDir.'/'.str_replace($originDir.DIRECTORY_SEPARATOR, '', $file->getPathname());

It didn't use DIRECTORY_SEPARATOR and is equivalent to:

    $target = str_replace($originDir, $targetDir, $file->getPathname());

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
2012-04-11 16:06:53 +02:00
Fabien Potencier
0c57db1771 merged branch vicb/routing_improvements (PR #3876)
Commits
-------

9307f5b [Routing] Implement bug fixes and enhancements

Discussion
----------

[Routing] Implement bug fixes and enhancements (from @Tobion)

This is mainly #3754 with some minor formatting changes.

Original PR message from @Tobion:

Here a list of what is fixed. Tests pass.

1. The `RouteCollection` states

    > it overrides existing routes with the same name defined in the instance or its children and parents.

    But this is not true for `->addCollection()` but only for `->add()`. addCollection does not remove routes with the same name in its parents (only in its children). I don't think this is on purpose.
    So I fixed it by making sure there can only be one route per name in all connected collections. This way we can also simplify `->get()` and `->remove()` and improve performance since we can stop iterating recursively when we found the first and only route with a given name.
    See `testUniqueRouteWithGivenName` that fails in the old code but works now.

2. There was an bug with `$collection->addPrefix('0');` that didn't apply the starting slash. Fixed and test case added.

3. There is an issue with `->get()` that I also think is not intended. Currently it allows to access a sub-RouteCollection by specifing $name as array integer index. But according to the PHPdoc you should only be allowed to receive a Route instance and not a RouteCollection.
    See `testGet` that has a failing test case. I fixed this behavior.

4. Then I recognized that `->addCollection` depended on the order of applying them. So

        $collection1->addCollection($collection2, '/b');
        $collection2->addCollection($collection3, '/c');
        $rootCollection->addCollection($collection1, '/a');

    had a different pattern result from

        $collection2->addCollection($collection3, '/c');
        $collection1->addCollection($collection2, '/b');
        $rootCollection->addCollection($collection1, '/a');

    Fixed and test case added. See `testPatternDoesNotChangeWhenDefinitionOrderChanges`.

5. PHP could have ended in an infinite loop when one tried to add an existing RouteCollection to the tree. Fixed by throwing an exception when this situation is detected. See tests `testCannotSelfJoinCollection` and `testCannotAddExistingCollectionToTree`.

6. I made `setParent()` private because its not useful outside the class itself. And `remove()` also removes the route from its parents. Added public `getRoot()` method.

7. The `Route` class throwed a PHP warning when trying to set an empty requirement.

8. Fixed issue #3777. See discussion there for more info. I fixed it by removing the over-optimization that was introduced in 91f4097a09 but didn't work properly. One cannot reorder the route definitions, as is was done, because then the wrong route might me matched before the correct one. If one really wanted to do that, it would require to calculate the intersection of two regular expressions to determine if they can be grouped together (a tool that would also be useful to check whether a route is unreachable, see discussion in #3678) We can only safely optimize routes with a static prefix within a RouteCollection, not across multiple RouteCollections with different parents.  This is especially true when using variables and regular expressions requirements.
I could however apply an optimization that was missing yet: Collections with a single route were missing the static prefix optimization with `0 === strpos()`.

9. Fixed an issue where the `PhpMatcherDumper` would not apply the optimization if the root collection to be dumped has a prefix itself. For this I had to rewrite `compileRoutes`. It is also much easier to understand now. Addionally I added many comments and PHPdoc because complex recursive methods like this are still hard to grasp.
I added a test case for this (`url_matcher3.php`).

10. Fix that `Route::compile` needs to recompile a route once it is modified. Otherwise we have a wrong result. Test case added.
2012-04-11 15:53:10 +02:00
Tobias Schultze
9307f5b33b [Routing] Implement bug fixes and enhancements 2012-04-11 15:45:27 +02:00
Fabien Potencier
3469713d90 merged branch kmohrf/ticket_2827_validator_mail_A_RR_DNS_hostcheck (PR #3799)
Commits
-------

f617e02 [Validator] added less-strict email host verification

Discussion
----------

[Validator] added less-strict email host verification

uhhhhh, my first pull request :>. uhm... tell me if i did something wrong :)

#### Request info
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes... well,not really (i guess master branch is not passing - at least i didnt broke the email test)
Fixes the following tickets: #2827

#### Description
New checkHost attribute in email constraint will make the validator check for only one of MX, A or AAAA DNS resource records to verify it as a valid email address.
2012-04-11 14:41:31 +02:00
Fabien Potencier
3c3ec5c677 merged branch tvlooy/GetSetMethodNormalizer (PR #3582)
Commits
-------

039ff6f allow more control on GetSetMethodNormalizer by using callback functions and an ignoreAttributes list

Discussion
----------

allow more control on GetSetMethodNormalizer

Here is an other attempt. You would use this as follows:

    $serializer = new \Symfony\Component\Serializer\Serializer(
        array(new \Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer()),
        array('json' => new \Symfony\Component\Serializer\Encoder\JsonEncoder())
    );

    $callbacks = array('books' => function ($books) { return NULL; });

    return new Response(
        $serializer->serialize($paginator->getRows(), 'json', $callbacks),
        200,
        array('Content-Type' => 'application/json')
    );

Besides of returning NULL, you could also do things like:

    $callbacks = array(
        'books' => function ($books) {
            $ids = array();
            foreach ($books as $book) {
                $ids[] = $book->getId();
            }
            return $ids;
        },
        'author' => function ($author) {
            return $author->getId();
        },
        'creationDate' => function ($creationDate) {
            return $creationDate->format('d/m/Y');
        },
    );

The commit is not complete yet. But at this point I am interested in your opinions.

---------------------------------------------------------------------------

by lsmith77 at 2012-03-12T22:53:18Z

in general i agree that using a callback is a good solution to provide more power without complicating the API or implementation in this case.

please add a test case, this should also help illustrate how this can be used in practice.

---------------------------------------------------------------------------

by schmittjoh at 2012-03-13T04:54:33Z

Note that your change breaks the API defined by the interface, i.e. someone using this method needs to type-hint the serializer implementation, not the interface.

It also adds a parameter to the public API of the serializer which will only work with one specific normalizer. What if another normalizer needs additional information, should another parameter be added to the serialize method? What about deserialization?

Bottom line is, the serializer component was simply not designed for this kind of thing. I've tried to make it more flexible before creating the bundle, but some things simply cannot be fixed in a sane way.

---------------------------------------------------------------------------

by tvlooy at 2012-03-13T06:07:45Z

Would just adding a setCallbacks() to the GetSetMethodNormalizer be a better solution? That doesn't touch the API. I will try to write some tests this evening.

---------------------------------------------------------------------------

by schmittjoh at 2012-03-13T16:22:50Z

That would definitely be better.

You would then need to retrieve the normalizer instance before calling ``serialize`` on the serializer which also leaves a stale taste, but I have no other solution for now.

---------------------------------------------------------------------------

by tvlooy at 2012-03-13T21:32:26Z

So, this should be it then. Yet an other usage example:

    $normalizer = new \Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer();
    $normalizer->setCallbacks(
        array(
            'books' => function ($books) {
                $ids = array();
                foreach ($books as $book) {
                    $ids[] = $book->getId();
                }
                return $ids;
            },
        )
    );
    $serializer = new \Symfony\Component\Serializer\Serializer(
        array($normalizer),
        array('json' => new \Symfony\Component\Serializer\Encoder\JsonEncoder())
    );

    return new Response(
        $serializer->serialize($paginator->getRows(), 'json'),
        200,
        array('Content-Type' => 'application/json')
    );

---------------------------------------------------------------------------

by tvlooy at 2012-03-18T21:16:48Z

Anything else needed for this to get pulled in?

---------------------------------------------------------------------------

by tvlooy at 2012-03-19T18:33:58Z

Hm, I like to keep it that way because I like the fact that not passing a callable will result in a warning instead of silently skipping it. You don't get that behaviour by treating it as null.

---------------------------------------------------------------------------

by vicb at 2012-03-19T23:15:37Z

I was unclear: the code should throw an exception when an element is not callable, this is why `null` will not be supported any more (it is not a callback as the `setCallbacks` indicate).

They are several way to support the former behavior:

* the cb can return a defined interface,
* the cb can throw a defines exc,
* by adding a `setIgnoredAttributes` method

Please also squash your commits.

---------------------------------------------------------------------------

by tvlooy at 2012-03-20T21:02:06Z

Yes, I like the setIgnoredAttributes solution. I changed it and squashed the commits.

---------------------------------------------------------------------------

by tvlooy at 2012-03-26T20:07:36Z

some improvements and squashed the commits

---------------------------------------------------------------------------

by stof at 2012-04-03T22:36:15Z

@tvlooy Please rebase your branch. It conflicts with master because of the move of tests.

---------------------------------------------------------------------------

by tvlooy at 2012-04-04T07:43:47Z

@stof I will do it on saturday, if that is ok with you.

---------------------------------------------------------------------------

by fabpot at 2012-04-10T18:29:30Z

Is it mergeable now? ping @Seldaek, @schmittjoh.

---------------------------------------------------------------------------

by tvlooy at 2012-04-10T18:55:04Z

yes, it should be
2012-04-11 12:08:10 +02:00
Fabien Potencier
22d8a436fa merged branch kimhemsoe/xcache_classloader (PR #3809)
Commits
-------

c36651b Fixed spelling error
f123684 Removed leftover from c/p
b74a5d4 Updated to new cache loader pattern.
7e66908 Added XCache class loader

Discussion
----------

[ClassLoader] Added XCache class loader

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

There is no tests, as it seems there is no way to use xcache storage functions from CLI.

---------------------------------------------------------------------------

by stof at 2012-04-06T20:12:09Z

Please implement a XcacheClassLoader following the same pattern than the new ApcClassLoader instead

---------------------------------------------------------------------------

by cordoval at 2012-04-07T14:20:47Z

- should include tests
- should include documentation (will you also update the component documentation for this new class?)

---------------------------------------------------------------------------

by stof at 2012-04-07T14:25:00Z

@cordoval the PR explains why there is no tests: xcache canot be used in the CLI.

---------------------------------------------------------------------------

by cordoval at 2012-04-07T14:26:43Z

ok @stof sorry it said it seemed not to be possible, i thought it was possible but I am wrong.

---------------------------------------------------------------------------

by kimhemsoe at 2012-04-07T15:01:24Z

@cordoval My english is really horrible. I would not mind if someone else could do that task for me. We also need to add doc for the new ApcClassLoader.

---------------------------------------------------------------------------

by cordoval at 2012-04-07T15:03:57Z

I wish you can explain me more then about this class and how to use it in code so then I can write easily the documentation :D deal?

---------------------------------------------------------------------------

by kimhemsoe at 2012-04-07T15:21:25Z

Deal :P

The XcacheClassLoader and ApcClassLoader replaces the old ApcUniversalClassLoader.
They giving us support for using another loader then UniversalClassLoader, without duplicating the cache layer.
Aslong it have a public function findFile($class) method.

 $loader = new ClassLoader();

// register classes with namespaces
$loader->add('Symfony\Component', __DIR__.'/component');
$loader->add('Symfony', __DIR__.'/framework');

$cachedLoader = new XcacheClassLoader('my_prefix', $loader);

// activate the cached autoloader
$cachedLoader->register();

Think that is more or less the essence of this.

---------------------------------------------------------------------------

by cordoval at 2012-04-09T08:28:53Z

it is not add but registerNamespace right?

so the main idea is to get rid of the restriction to use Apc with Universal loader

what is the comparative advantage between APC and Xcache?

---------------------------------------------------------------------------

by kimhemsoe at 2012-04-09T08:55:23Z

Yes if the $loader (class finder) were to be a instance UniversalClassLoader.

Yes the main idea is to be able to reuse the cache layer with any class loader there obey to the one restriction.

Difference between apc and xcache and why to use what is coming down to taste and your setup. We use xcache as APC have some issues in fastcgi setups. when we upgrade to php54 at somepoint we get to chance to move to php-fpm wich solves these issues. Short story: Slightly out of scope for any documentation in here :-P
2012-04-11 11:50:49 +02:00
Fabien Potencier
191aaa3f0c merged branch bschussek/issue3635 (PR #3820)
Commits
-------

65aa387 [Form] Fixed index generation in EntityChoiceList if ID is not an integer

Discussion
----------

[Form] Fixed index generation in EntityChoiceList if ID is not an integer

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3635
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3635)
2012-04-11 11:36:22 +02:00
Fabien Potencier
e9539d8a6d merged branch hhamon/command_description_fixes (PR #3871)
Commits
-------

b4f0a04 [TwigBundle] fixed twig:lint command description.
809933f [FrameworkBundle] fixed translation:update command description.

Discussion
----------

Command description fixes

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-04-11 09:36:53 +02:00