Commits
-------
15910a0 fixed coding standards
24a3cd3 Finder - allow sorting when searching in multiple directories
Discussion
----------
[Finder] not searching in multiple dirs with sorting
I hit on a problem with **Finder, when using array of directories passed to ->in() together with sorting** (e.g. ->sortByName()):
*Catchable Fatal Error: Argument 1 passed to AppendIterator::append() must implement interface Iterator, instance of Symfony\Component\Finder\Iterator\SortableIterator given in ......\vendor\symfony\src\Symfony\Component\Finder\Finder.php line 421*
The problem is in Finder.php, line 419. When more than 1 directory is used, \AppendIterator is used to merge iterators for each directory. AppendIterator->append() accepts only objects implementing Iterator interface. But this is broken for SortableIterator, which implements IteratorAggregate and NOT Iterator.
My proposed solution retrieves an Iterator from IteratorAggregate, which is later valid as an input to AppendIterator->append()
(This solved the exception mentioned aboved in my testing project, not tested more.)
Commits
-------
e67f8d4 Properly skip memcached tests when no memcached server is present
005d86f Broaden timer tests limits
001c4fd Fix windows fs tests
Discussion
----------
Fix tests
- Some windows fixes
- Skip memcached for real when it's not there - by the way I think those tests are insane since they seem to run a purge() on the memcached server. If you run this by accident somewhere where it matters it could hurt. I think they should be put in a group disabled by default, I'll be happy to add that change if it's deemed useful.
- Fixed the timer tests for good on windows (which seems to be quite bad at `usleep`). I also documented them so you actually know how it failed when it does, the false instead of true wasn't super useful.
Commits
-------
6e75fd1 Resolves issue with spl_autoload_register creating new copies of the container and passing that into the closure.
Discussion
----------
[DoctrineBundle] fixed proxy loader memory leak
[](http://travis-ci.org/kriswallsmith/symfony)
The hack for loading Doctrine proxy classes has an obscure memory leak, fixed here by @jjbohn.
## The Proof
Run this test case before and after this patch:
```php
<?php
namespace Kris\JunkBundle\Tests\Controller;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
class DefaultControllerTest extends WebTestCase
{
/**
* @dataProvider asdf
*/
public function testIndex()
{
$client = static::createClient();
$crawler = $client->request('GET', '/hello/Fabien');
$this->assertTrue($crawler->filter('html:contains("Hello Fabien")')->count() > 0);
}
public function asdf()
{
return array_fill(0, 500, array());
}
}
```
### Before
```
~/Sites/symfony/standard (2.0) $ phpunit -c app/
PHPUnit 3.6.10 by Sebastian Bergmann.
Configuration read from /Users/kriswallsmith/Sites/symfony/standard/app/phpunit.xml.dist
............................................................... 63 / 500 ( 12%)
............................................................... 126 / 500 ( 25%)
............................................................... 189 / 500 ( 37%)
............................................................... 252 / 500 ( 50%)
............................................................... 315 / 500 ( 63%)
............................................................... 378 / 500 ( 75%)
............................................................... 441 / 500 ( 88%)
...........................................................
Time: 31 seconds, Memory: 289.50Mb
OK (500 tests, 500 assertions)
```
### After
```
~/Sites/symfony/standard (2.0) $ phpunit -c app/
PHPUnit 3.6.10 by Sebastian Bergmann.
Configuration read from /Users/kriswallsmith/Sites/symfony/standard/app/phpunit.xml.dist
............................................................... 63 / 500 ( 12%)
............................................................... 126 / 500 ( 25%)
............................................................... 189 / 500 ( 37%)
............................................................... 252 / 500 ( 50%)
............................................................... 315 / 500 ( 63%)
............................................................... 378 / 500 ( 75%)
............................................................... 441 / 500 ( 88%)
...........................................................
Time: 40 seconds, Memory: 51.25Mb
OK (500 tests, 500 assertions)
```
## tl;dr
Your test suite will use much less memory — 82% in this case.
```
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```
---------------------------------------------------------------------------
by mvrhov at 2012-02-23T06:25:57Z
IMHO this change warrants a comment inside a source code as somebody might actually try to remove the first by reference assign like stof said.
---------------------------------------------------------------------------
by lsmith77 at 2012-02-23T07:55:48Z
this autoloader sounds like something we also need in the ODM's?
---------------------------------------------------------------------------
by stof at 2012-02-23T08:23:17Z
@lsmith77 if you want to allow unserializing proxies without forcing to generate them before (which would be an issue in debug mode), yeah. But take care that each Doctrine bundle should use a different proxy namespace to allow doing the check (there was some issues for people using both the ORM and the mongo ODM because of this)
---------------------------------------------------------------------------
by lsmith77 at 2012-02-23T08:24:33Z
then maybe this could should be a static method inside the bridge?
---------------------------------------------------------------------------
by beberlei at 2012-02-23T11:50:08Z
I think another side of this problem is that ->boot() ALWAYS adds this method on the autoloading stack. So with N tests you have N more autoloaders on the stack.
---------------------------------------------------------------------------
by pminnieur at 2012-02-23T12:07:00Z
This could be an issue if you use Symfony with Leach as an application server, too. After a while, memory is exhausted in face of `gc_collect_cycles` and `$kernel->boot()` and `$kernel->shutdown()` calls in between each request - which ultimately leads to a segfault after some time. I tried to track down what causes increasing memory usage and I think this could be the error.
---------------------------------------------------------------------------
by beberlei at 2012-02-23T12:28:06Z
its definately the problem, we need to remove the autoloader in shutdown, or move it elsewhere.
---------------------------------------------------------------------------
by lsmith77 at 2012-02-23T14:58:37Z
why isnt this just a setup task for the autoloader just like the annotation registry?
---------------------------------------------------------------------------
by stof at 2012-02-23T16:52:42Z
@lsmith77 because the proxy namespace and the proxy dir are not known in the autoload.php file. They are configured in the config files
---------------------------------------------------------------------------
by fabpot at 2012-02-23T18:05:51Z
The `shutdown()` method is where the autoloader should be removed. Can we include this in this PR as well so that we fix everything once and for all?
---------------------------------------------------------------------------
by kriswallsmith at 2012-02-23T19:12:05Z
The once and for all solution is for the Doctrine O*M projects to provide a ProxyLoader class with register and unregister methods that we call in boot and shutdown. We're not solving anything specific to Symfony here.
This was imported from DoctrineBundle (see: doctrine/DoctrineBundle#23), since it can be used by other Doctrine bundles, too. It utilizes the ContainerAwareEventManager from f15dde6c59.
Commits
-------
15c6ba9 [HttpKernel] Fix call to Memcached::set() once again
Discussion
----------
[HttpKernel] Fix call to Memcached::set() once again
I originally fixed this in #3358, but it appears #3363 (which touched the same line) was merged soon after.
Commits
-------
957bbcb [WebProfiler] Add default route to access the profiler more easily
Discussion
----------
[WebProfiler] Add default route to access the profiler more easily
When you have the toolbar disabled, it's pretty annoying to reach the _profiler, I never remember what to type to get something except `/_profiler`. This shows the last ten runs which is quite useful.
Commits
-------
e6e9b5a [Routing] Return the _route parameter from ApacheUrlMatcher
Discussion
----------
[Routing] Return the _route parameter from ApacheUrlMatcher
---------------------------------------------------------------------------
by fabpot at 2012-02-22T23:13:49Z
Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by arnaud-lb at 2012-02-23T09:12:45Z
sure, done
Commits
-------
d02ca25 [MonologBundle] Fixed a bug when adding a processor on a service handler
Discussion
----------
Fix monolog processor
This is the proper bugfix replacing #3417, and adding a test for it
Commits
-------
b269e27 [Config] Improve handling of PrototypedArrayNode defaults
4feba09 [Config] implements feedback
bc122bd [Config] Fix nested prototyped array nodes
675e5eb [Config] Take advantage of the new PrototypedArrayNode API in the core bundles
cba2c33 [Config] Improve error messages & extensibility
bca2b0e [Config] Improve PrototypedArrayNode default value management
Discussion
----------
[Config] Improve prototype nodes usability, error messages, extensibility
### First commit
*Before* (you should set multiple defalutValues)
```php
<?php
$root
->arrayNode('node')
->prototype('array')
// when the node is not set
->defaultValue(array('foo' => 'bar')
->children()
// when the key is not set
->scalarNode('foo')->defaultValue('bar')->end()
$root
->arrayNode('node')
->prototype('array')
// when the node is not set
->defaultValue(array('defaults' => array('foo1' => 'bar1', 'foo2' => 'bar2')
->children()
->arrayNode('bar')
// when the node is not set
->addDefautsIfNotSet()
// when some values are not set (node being set)
->scalarNode('foo1')->defaultValue('bar1')->end()
->scalarNode('foo2')->defaultValue('bar2')->end()
```
*after*
```php
<?php
$root
->arrayNode('node')
->addDefaultChildrenWhenNoneSet()
->prototype('array')
->children()
->scalarNode('foo')->defaultValue('bar')->end()
$root
->arrayNode('node')
->addDefaultChildrenWhenNoneSet()
->prototype('array')
->children()
->arrayNode('bar')
->scalarNode('foo1')->defaultValue('bar1')->end()
->scalarNode('foo2')->defaultValue('bar2')->end()
```
*more* (exclusive configs)
```php
<?php
$root
->arrayNode('node')
// Add a default node named 'defaults'
->addDefaultChildrenWhenNoneSet()
// Add a default node named 'foo'
->addDefaultChildrenWhenNoneSet('foo')
// Add two default nodes named 'foo', 'bar'
->addDefaultChildrenWhenNoneSet(array('foo', 'bar'))
// Add two default nodes
->addDefaultChildrenWhenNoneSet(2)
```
### Second commit
Improves error messages (print the path to the error) & extensibility.
@schmittjoh I would appreciate you feedback on both the commits. Do you think a boolean $throw switch on `getNode` would make sense (i.e. to prevent throwing excs in prod ?).
---------------------------------------------------------------------------
by schmittjoh at 2012-02-20T15:43:18Z
The error improvements seem uncontroversial.
I'm not so convinced by the other changes though. What if the prototype is a map and not a simple list?
---------------------------------------------------------------------------
by vicb at 2012-02-20T16:07:51Z
I think there's one caveat left in the code as it is now that I will fix (nested prototypes).
Could you please give me more details on the use case you are referring to ?
You do not have to use the new feature but It can be really helpful [here](https://github.com/symfony/symfony/pull/3225/files#L4R38) for example.
---------------------------------------------------------------------------
by schmittjoh at 2012-02-20T17:20:02Z
What I mean is something like this:
```php
->arrayNode("foo")
->useAttributeAsKey("name")
->prototype(/* ...
```
---------------------------------------------------------------------------
by vicb at 2012-02-20T17:28:01Z
What would be wrong then ? (that's the use case I link in my previous msg)
---------------------------------------------------------------------------
by schmittjoh at 2012-02-20T17:28:55Z
How would adding defaults look like?
---------------------------------------------------------------------------
by vicb at 2012-02-20T17:36:35Z
Check the "more" part of the PR message.
In the linked use case, it would add a "defaults" server using the default host / port / weight. In this case I do not care about the name but the values are important to help alias the equivalent configs. You can override the "defaults" name by using a parameter.
---------------------------------------------------------------------------
by vicb at 2012-02-20T17:47:27Z
```php
<?php
// [...]
->arrayNode('servers')
->addDefaultChildrenWhenNodeSet()
->useAttributeAsKey('name')
->prototype('array')
->children()
```
---------------------------------------------------------------------------
by schmittjoh at 2012-02-20T17:47:54Z
What I was thinking about is having two nodes with different default values. Right now, both nodes while having different keys would still have the same default values which does not make much sense to me. However, we can address this in another PR.
One thing that we should fix though is that we should require keys in case of a map, and forbid them in case of a list. It might make sense to split it into different methods. Like the following examples make no sense (but are possible atm):
```php
->arrayNode("foo")
->useAttributeAsKey("name")
->addDefaultChildrenIfNotSet(5)
->arrayNode("foo")
->addDefaultChildrenIfNotSet("foo")
->prototype("scalar")->end()
```
Another minor nitpick, please rename "when" to "if".
---------------------------------------------------------------------------
by vicb at 2012-02-20T18:03:19Z
@schmittjoh thank you for your feedback.
message-2:
* I think the first case is fine (children "1" to "5"). Sometimes you just don't care about the names so it should not be forbidden.
* I also think the second case is fine as you would write `foo: value` in your config file anyway.
Let me know your thoughts about the previous statements.
Agree to change when to if.
message-1:
Will change
---------------------------------------------------------------------------
by vicb at 2012-02-20T18:06:33Z
I think "IfNoneSet" is more accurate than "IfNotSet" ?
---------------------------------------------------------------------------
by schmittjoh at 2012-02-20T18:09:59Z
If you call "useAttributeAsKey" it automatically means that the keys are meaningful to you (otherwise there is no point in calling it). In such a case, keys should be explicitly given.
On the other hand, if you do not call it, then the keys are ignored/dropped by the Config component. So if you give a key, it is an obvious error that we should catch. The second case I linked would look like ``foo: [value]`` in contrast to ``foo: { foo: value }``.
---------------------------------------------------------------------------
by schmittjoh at 2012-02-20T18:14:44Z
I'm not feeling strongly about this, but "IfNotSet" is more consistent with
"addDefaultsIfNotSet" and basically reads as "if array node is not set, do
...". Your example would refer to the children and read as "if none
(children) have been defined, do ...".
On Mon, Feb 20, 2012 at 12:06 PM, Victor Berchet <
reply@reply.github.com
> wrote:
> I think "IfNoneSet" is more accurate than "IfNotSet" ?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/3403#issuecomment-4058579
>
---------------------------------------------------------------------------
by vicb at 2012-02-20T18:30:21Z
message-2:
* Agree on first point, will change
* You could specify the keys in your config file if the prototype is an array (you used a scalar). Should we implement a switch in the validation (i.e. array / not array) or just go with numeric / null arg as you suggest ?
message-1:
> Your example would refer to the children and read as "if none (children) have been defined, do ..."
QED
---------------------------------------------------------------------------
by vicb at 2012-02-20T22:11:05Z
@schmittjoh I have implemented your suggestions (other than the "NoneSet"). Let me know if you think this is ok. Thanks.
---------------------------------------------------------------------------
by schmittjoh at 2012-02-21T03:24:19Z
Looks good to me.
As an additional improvement we might consider to allow to prepopulate an prototyped with values. For example, in the FOSRestBundle there is a case where this could be used.
```php
->arrayNode('formats')
->prepopulateValues(array('application/json' => 'json', 'application/xhtml+xml' => 'xml'))
->useAttributeAsKey('name')
->prototype('scalar')->canBeUnset()->end()
```
This could be done in a separate PR however and is not strictly related to these improvements.
---------------------------------------------------------------------------
by vicb at 2012-02-21T07:51:59Z
@schmittjoh that would be a great addition but I think need some thinking (i.e. the name, `initialValues` ?, should we handle duplicates, how - in case we are not using attribue as key, ...) so let's make an other PR, I'd like this one to be merged asap as I need this for the Cache Bundle.
@fabpot ready
Commits
-------
1953280 [MonologBridge] updated the class name from Monolog
96da7c8 [MonologBridge] Added the user agent check for the ChromePhpHandler
f7aa6c0 [MonologBridge] Added the Response-aware ChromePhpHandler
Discussion
----------
[MonologBridge] Added the Response-aware ChromePhpHandler
This adds an extended ChromePhpHandler based on the Response class to set the headers, similar to the extended FirePHPHandler.
This PR depends on Seldaek/monolog#58
---------------------------------------------------------------------------
by stof at 2012-02-20T16:36:47Z
@fabpot The monolog PR is merged now so this one is ready
---------------------------------------------------------------------------
by stloyd at 2012-02-20T17:11:14Z
@stof You need to rename file and class name to: [`ChromePHPHandler`](8d4ac5c0f7)
---------------------------------------------------------------------------
by fabpot at 2012-02-22T09:16:46Z
@stloyd is right. As per Symfony standard, you should use `ChromePhpHandler` for the Symfony class and `SymfonyPHPHandler` for the Monolog one.
---------------------------------------------------------------------------
by stof at 2012-02-22T09:22:27Z
@fabpot updated
Commits
-------
2871ea0 Update composer for HttpFoundation's PHP 5.4 forward compatibility.
ff8d740 [Locale] Update documentation for autoloader.
dd2c4aa [HttpFoundation] Documentation.
e585ca7 [HttpFoundation] Added forward compatibility for \SessionHandlerInterface
d339e74 [ClassLoader] Add ability to incrementally register fallbacks.
Discussion
----------
[HttpFoundation] PHP 5.4 forward compatibility for \Sessionhandlerinterface
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Utilising the same forward method as in Locale (as requested by @fabpot)
---------------------------------------------------------------------------
by stof at 2012-02-16T20:59:27Z
forward compatibility sent to the master branch, really ?
---------------------------------------------------------------------------
by stof at 2012-02-16T21:00:12Z
Ok, looking at the patch, it is not the same than places where we used the naming "forward compatibility". Sorry
---------------------------------------------------------------------------
by stof at 2012-02-16T21:01:56Z
As this fallback is mandatory to be able to use the component on 5.3, which is our target, please register the fallback in the composer.json file of the component and of the main repo so that it works out of the box when using Composer.
---------------------------------------------------------------------------
by drak at 2012-02-17T02:55:17Z
@stof as a native English speaker I find the terminology used in Symfony2 upside down. "Backward compatible" means compatible with past versions, i.e. legacy. "Forward compatibility" logically means compatible with future versions - which in this case we are making an interface available in PHP 5.4, available in PHP 5.3.
I will try to adapt my in the context of Sf2's terminology :-)
---------------------------------------------------------------------------
by drak at 2012-02-17T02:56:43Z
Ah I see what you are saying, you mean normally things are made forward compatible from Symfony 2.0 to 2.1, and in this case we're making a native PHP interface forward compatible: so actually we are on the same page :)
---------------------------------------------------------------------------
by drak at 2012-02-17T02:59:00Z
@stof - I took a look at the `composer.json` and I also don't see and handling for the `Locale` component fallback (as written in `autoloader.php.dist`. Is it even possible in Composer? /cc @Seldaek
---------------------------------------------------------------------------
by stof at 2012-02-17T07:03:44Z
@drak for the Locale, we don't register it through composer as we cannot do it fully: the stub functions are not autoloadable so it will always require some user work to do this. But for the HttpFoundation stub, it is possible as it is autoloadable:
```json
{
autoload: {
psr-0: {
'Symfony\\Component\\HttpFoundation': '',
'': 'Symfony/Component/HttpFoundation/Resources/stub'
}
}
}
```
---------------------------------------------------------------------------
by drak at 2012-02-17T08:11:56Z
Awesome @stof, thanks for the tip.
---------------------------------------------------------------------------
by drak at 2012-02-17T08:18:44Z
@stof do I also need to update the main repo's composer.json file too?
```
"autoload": {
"psr-0": {
"Symfony": "src/",
"": "src/Symfony/Component/HttpFoundation/Resources/stub"
}
}
```
---------------------------------------------------------------------------
by Seldaek at 2012-02-17T08:54:46Z
I would use that in the autoload just to avoid file_exists calls for nothing:
```json
"SessionHandlerInterface": "src/Symfony/Component/HttpFoundation/Resources/stub"
```
The class is part of the "prefix" too.
---------------------------------------------------------------------------
by drak at 2012-02-21T12:32:23Z
This is ready for merge @fabpot
---------------------------------------------------------------------------
by drak at 2012-02-21T16:20:43Z
@fabpot - done.
