From acff7356ce5a49e0346960428778c5246f106ede Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Tue, 5 Mar 2013 12:07:44 +0100 Subject: [PATCH 01/10] [Routing] trigger deprecation warning for deprecated features that will be removed in 2.3 --- .../Component/Routing/RouteCollection.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index e53606da5f..ffdf57f478 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -65,6 +65,9 @@ class RouteCollection implements \IteratorAggregate, \Countable */ public function getParent() { + trigger_error('getParent() is deprecated since version 2.2 and will be removed in 2.3. There is no substitution ' . + 'because RouteCollection is not tree structure anymore.', E_USER_DEPRECATED); + return $this->parent; } @@ -77,6 +80,9 @@ class RouteCollection implements \IteratorAggregate, \Countable */ public function getRoot() { + trigger_error('getRoot() is deprecated since version 2.2 and will be removed in 2.3. There is no substitution ' . + 'because RouteCollection is not tree structure anymore.', E_USER_DEPRECATED); + $parent = $this; while ($parent->getParent()) { $parent = $parent->getParent(); @@ -184,6 +190,8 @@ class RouteCollection implements \IteratorAggregate, \Countable // this is to keep BC $numargs = func_num_args(); if ($numargs > 1) { + trigger_error('addCollection() should only be used with a single parameter. The params $prefix, $defaults, $requirements and $options ' . + 'are deprecated since version 2.2 and will be removed in 2.3. Use addPrefix() and addOptions() instead.', E_USER_DEPRECATED); $collection->addPrefix($this->prefix . func_get_arg(1)); if ($numargs > 2) { $collection->addDefaults(func_get_arg(2)); @@ -232,7 +240,13 @@ class RouteCollection implements \IteratorAggregate, \Countable $this->prefix = '/' . $prefix . $this->prefix; // this is to keep BC - $options = func_num_args() > 3 ? func_get_arg(3) : array(); + if (func_num_args() > 3) { + trigger_error('The fourth parameter ($options) of addPrefix() is deprecated since version 2.2 and will be removed in 2.3. ' . + 'Use addOptions() instead.', E_USER_DEPRECATED); + $options = func_get_arg(3); + } else { + $options = array(); + } foreach ($this->routes as $route) { $route->setPath('/' . $prefix . $route->getPath()); @@ -251,6 +265,9 @@ class RouteCollection implements \IteratorAggregate, \Countable */ public function getPrefix() { + trigger_error('getPrefix() is deprecated since version 2.2 and will be removed in 2.3. The method suggests that ' . + 'all routes in the collection would have this prefix, which is not necessarily true.', E_USER_DEPRECATED); + return $this->prefix; } From 2b8bf6beec4514cfd59f161f77ec813b2eedd5b6 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 11:09:21 +0100 Subject: [PATCH 02/10] adjusted tests to not use RouteCollection::getPrefix --- .../Component/Routing/Tests/RouteCollectionTest.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 65f9967e6a..13a50de87f 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -133,9 +133,8 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $collection2 = new RouteCollection(); $collection2->add('bar', $bar = new Route('/bar')); $collection->addCollection($collection2); - $this->assertSame('', $collection->getPrefix(), '->getPrefix() is initialized with ""'); $collection->addPrefix(' / '); - $this->assertSame('', $collection->getPrefix(), '->addPrefix() trims the prefix and a single slash has no effect'); + $this->assertSame('/foo', $collection->get('foo')->getPattern(), '->addPrefix() trims the prefix and a single slash has no effect'); $collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'), array('foo' => 'bar')); $this->assertEquals('/{admin}/foo', $collection->get('foo')->getPath(), '->addPrefix() adds a prefix to all routes'); $this->assertEquals('/{admin}/bar', $collection->get('bar')->getPath(), '->addPrefix() adds a prefix to all routes'); @@ -152,11 +151,10 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $collection->get('bar')->getOptions(), '->addPrefix() adds an option to all routes' ); $collection->addPrefix('0'); - $this->assertEquals('/0/{admin}', $collection->getPrefix(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash'); + $this->assertEquals('/0/{admin}/foo', $collection->get('foo')->getPattern(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash'); $collection->addPrefix('/ /'); - $this->assertSame('/ /0/{admin}', $collection->getPrefix(), '->addPrefix() can handle spaces if desired'); - $this->assertSame('/ /0/{admin}/foo', $collection->get('foo')->getPath(), 'the route path is in synch with the collection prefix'); - $this->assertSame('/ /0/{admin}/bar', $collection->get('bar')->getPath(), 'the route path in a sub-collection is in synch with the collection prefix'); + $this->assertSame('/ /0/{admin}/foo', $collection->get('foo')->getPath(), '->addPrefix() can handle spaces if desired'); + $this->assertSame('/ /0/{admin}/bar', $collection->get('bar')->getPath(), 'the route pattern of an added collection is in synch with the added prefix'); } public function testAddPrefixOverridesDefaultsAndRequirements() From ceb9ab44a4796888af79014c65c72c9534e096f9 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 11:24:42 +0100 Subject: [PATCH 03/10] adjust tests to no use addPrefix with options --- .../Component/Routing/Tests/RouteCollectionTest.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 13a50de87f..62d58ca0d2 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -135,21 +135,13 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $collection->addCollection($collection2); $collection->addPrefix(' / '); $this->assertSame('/foo', $collection->get('foo')->getPattern(), '->addPrefix() trims the prefix and a single slash has no effect'); - $collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'), array('foo' => 'bar')); + $collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+')); $this->assertEquals('/{admin}/foo', $collection->get('foo')->getPath(), '->addPrefix() adds a prefix to all routes'); $this->assertEquals('/{admin}/bar', $collection->get('bar')->getPath(), '->addPrefix() adds a prefix to all routes'); $this->assertEquals(array('admin' => 'admin'), $collection->get('foo')->getDefaults(), '->addPrefix() adds defaults to all routes'); $this->assertEquals(array('admin' => 'admin'), $collection->get('bar')->getDefaults(), '->addPrefix() adds defaults to all routes'); $this->assertEquals(array('admin' => '\d+'), $collection->get('foo')->getRequirements(), '->addPrefix() adds requirements to all routes'); $this->assertEquals(array('admin' => '\d+'), $collection->get('bar')->getRequirements(), '->addPrefix() adds requirements to all routes'); - $this->assertEquals( - array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'), - $collection->get('foo')->getOptions(), '->addPrefix() adds an option to all routes' - ); - $this->assertEquals( - array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'), - $collection->get('bar')->getOptions(), '->addPrefix() adds an option to all routes' - ); $collection->addPrefix('0'); $this->assertEquals('/0/{admin}/foo', $collection->get('foo')->getPattern(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash'); $collection->addPrefix('/ /'); From 9e2bcb5d9e7475a18f71f1ca1936c781543750c7 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 11:44:47 +0100 Subject: [PATCH 04/10] refactor interator test that was still assuming a tree --- .../Routing/Tests/RouteCollectionTest.php | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 62d58ca0d2..9cccd4c34a 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -54,16 +54,19 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $this->assertEquals('/foo2', $collection->get('foo')->getPath()); } - public function testIteratorWithOverriddenRoutes() + public function testIterator() { $collection = new RouteCollection(); $collection->add('foo', new Route('/foo')); $collection1 = new RouteCollection(); - $collection1->add('foo', new Route('/foo1')); + $collection1->add('bar', $bar = new Route('/bar')); + $collection1->add('foo', $foo = new Route('/foo-new')); $collection->addCollection($collection1); + $collection->add('last', $last = new Route('/last')); - $this->assertEquals('/foo1', $this->getFirstNamedRoute($collection, 'foo')->getPath()); + $this->assertInstanceOf('\ArrayIterator', $collection->getIterator()); + $this->assertSame(array('bar' => $bar, 'foo' => $foo, 'last' => $last), $collection->getIterator()->getArrayCopy()); } public function testCount() @@ -72,25 +75,12 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $collection->add('foo', new Route('/foo')); $collection1 = new RouteCollection(); - $collection1->add('foo1', new Route('/foo1')); + $collection1->add('bar', new Route('/bar')); $collection->addCollection($collection1); $this->assertCount(2, $collection); } - protected function getFirstNamedRoute(RouteCollection $routeCollection, $name) - { - foreach ($routeCollection as $key => $route) { - if ($route instanceof RouteCollection) { - return $this->getFirstNamedRoute($route, $name); - } - - if ($name === $key) { - return $route; - } - } - } - public function testAddCollection() { if (!class_exists('Symfony\Component\Config\Resource\FileResource')) { From ea694e445d50b1abe9c83352593015f1b1169aaa Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 11:59:02 +0100 Subject: [PATCH 05/10] added tests for remove() that wasnt covered yet and special route name --- .../Routing/Tests/RouteCollectionTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 9cccd4c34a..487a537a87 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -197,13 +197,31 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $collection2 = new RouteCollection(); $collection2->add('b', $b = new Route('/b')); $collection1->addCollection($collection2); + $collection1->add('$péß^a|', $c = new Route('/special')); $this->assertSame($b, $collection1->get('b'), '->get() returns correct route in child collection'); + $this->assertSame($c, $collection1->get('$péß^a|'), '->get() can handle special characters'); $this->assertNull($collection2->get('a'), '->get() does not return the route defined in parent collection'); $this->assertNull($collection1->get('non-existent'), '->get() returns null when route does not exist'); $this->assertNull($collection1->get(0), '->get() does not disclose internal child RouteCollection'); } + public function testRemove() + { + $collection = new RouteCollection(); + $collection->add('foo', $foo = new Route('/foo')); + + $collection1 = new RouteCollection(); + $collection1->add('bar', $bar = new Route('/bar')); + $collection->addCollection($collection1); + $collection->add('last', $last = new Route('/last')); + + $collection->remove('foo'); + $this->assertSame(array('bar' => $bar, 'last' => $last), $collection->all(), '->remove() can remove a single route'); + $collection->remove(array('bar', 'last')); + $this->assertSame(array(), $collection->all(), '->remove() accepts an array and can remove multiple routes at once'); + } + public function testSetHost() { $collection = new RouteCollection(); From 0a1cfcdb5f0f75ad8c69506879eb5b8c38436d35 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 13:21:35 +0100 Subject: [PATCH 06/10] adjust RouteCollectionTest for the addCollection change and refactor the tests to only skip the part that really needs the config component --- .../Routing/Tests/RouteCollectionTest.php | 54 +++++++------------ 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 487a537a87..09748de0f3 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -82,32 +82,31 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase } public function testAddCollection() + { + $collection = new RouteCollection(); + $collection->add('foo', new Route('/foo')); + + $collection1 = new RouteCollection(); + $collection1->add('bar', $bar = new Route('/bar')); + $collection1->add('foo', $foo = new Route('/foo-new')); + + $collection2 = new RouteCollection(); + $collection2->add('grandchild', $grandchild = new Route('/grandchild')); + + $collection1->addCollection($collection2); + $collection->addCollection($collection1); + $collection->add('last', $last = new Route('/last')); + + $this->assertSame(array('bar' => $bar, 'foo' => $foo, 'grandchild' => $grandchild, 'last' => $last), $collection->all(), + '->addCollection() imports routes of another collection, overrides if necessary and adds them at the end'); + } + + public function testAddCollectionWithResources() { if (!class_exists('Symfony\Component\Config\Resource\FileResource')) { $this->markTestSkipped('The "Config" component is not available'); } - $collection = new RouteCollection(); - $collection->add('foo', $foo = new Route('/foo')); - $collection1 = new RouteCollection(); - $collection1->add('foo', $foo1 = new Route('/foo1')); - $collection1->add('bar', $bar1 = new Route('/bar1')); - $collection->addCollection($collection1); - $this->assertEquals(array('foo' => $foo1, 'bar' => $bar1), $collection->all(), '->addCollection() adds routes from another collection'); - - $collection = new RouteCollection(); - $collection->add('foo', $foo = new Route('/foo')); - $collection1 = new RouteCollection(); - $collection1->add('foo', $foo1 = new Route('/foo1')); - $collection->addCollection($collection1, '/{foo}', array('foo' => 'foo'), array('foo' => '\d+'), array('foo' => 'bar')); - $this->assertEquals('/{foo}/foo1', $collection->get('foo')->getPath(), '->addCollection() can add a prefix to all merged routes'); - $this->assertEquals(array('foo' => 'foo'), $collection->get('foo')->getDefaults(), '->addCollection() can add a prefix to all merged routes'); - $this->assertEquals(array('foo' => '\d+'), $collection->get('foo')->getRequirements(), '->addCollection() can add a prefix to all merged routes'); - $this->assertEquals( - array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'), - $collection->get('foo')->getOptions(), '->addCollection() can add an option to all merged routes' - ); - $collection = new RouteCollection(); $collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml')); $collection1 = new RouteCollection(); @@ -150,19 +149,6 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $this->assertEquals('https', $collection->get('bar')->getRequirement('_scheme'), '->addPrefix() overrides existing requirements'); } - public function testAddCollectionOverridesDefaultsAndRequirements() - { - $imported = new RouteCollection(); - $imported->add('foo', $foo = new Route('/foo')); - $imported->add('bar', $bar = new Route('/bar', array(), array('_scheme' => 'http'))); - - $collection = new RouteCollection(); - $collection->addCollection($imported, null, array(), array('_scheme' => 'https')); - - $this->assertEquals('https', $collection->get('foo')->getRequirement('_scheme'), '->addCollection() overrides existing requirements'); - $this->assertEquals('https', $collection->get('bar')->getRequirement('_scheme'), '->addCollection() overrides existing requirements'); - } - public function testResource() { if (!class_exists('Symfony\Component\Config\Resource\FileResource')) { From c0de07b9603602af3f33f82883683e7ff957952e Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 13:22:39 +0100 Subject: [PATCH 07/10] added tests for addDefaults, addRequirements, addOptions --- .../Routing/Tests/RouteCollectionTest.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 09748de0f3..d60d1c68af 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -115,6 +115,33 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array($foo, $foo1), $collection->getResources(), '->addCollection() merges resources'); } + public function testAddDefaultsAndRequirementsAndOptions() + { + $collection = new RouteCollection(); + $collection->add('foo', new Route('/{placeholder}')); + $collection1 = new RouteCollection(); + $collection1->add('bar', new Route('/{placeholder}', + array('_controller' => 'fixed', 'placeholder' => 'default'), array('placeholder' => '.+'), array('option' => 'value')) + ); + $collection->addCollection($collection1); + + $collection->addDefaults(array('placeholder' => 'new-default')); + $this->assertEquals(array('placeholder' => 'new-default'), $collection->get('foo')->getDefaults(), '->addDefaults() adds defaults to all routes'); + $this->assertEquals(array('_controller' => 'fixed', 'placeholder' => 'new-default'), $collection->get('bar')->getDefaults(), + '->addDefaults() adds defaults to all routes and overwrites existing ones'); + + $collection->addRequirements(array('placeholder' => '\d+')); + $this->assertEquals(array('placeholder' => '\d+'), $collection->get('foo')->getRequirements(), '->addRequirements() adds requirements to all routes'); + $this->assertEquals(array('placeholder' => '\d+'), $collection->get('bar')->getRequirements(), + '->addRequirements() adds requirements to all routes and overwrites existing ones'); + + $collection->addOptions(array('option' => 'new-value')); + $this->assertEquals( + array('option' => 'new-value', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'), + $collection->get('bar')->getOptions(), '->addOptions() adds options to all routes and overwrites existing ones' + ); + } + public function testAddPrefix() { $collection = new RouteCollection(); From 6180c5b031d0d0117786f514a3f8a46efa938a9b Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 13:28:59 +0100 Subject: [PATCH 08/10] add test for uniqueness of resources --- src/Symfony/Component/Routing/Tests/RouteCollectionTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index d60d1c68af..901317663f 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -184,7 +184,11 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase $collection = new RouteCollection(); $collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml')); - $this->assertEquals(array($foo), $collection->getResources(), '->addResources() adds a resource'); + $collection->addResource($bar = new FileResource(__DIR__.'/Fixtures/bar.xml')); + $collection->addResource(new FileResource(__DIR__.'/Fixtures/foo.xml')); + + $this->assertEquals(array($foo, $bar), $collection->getResources(), + '->addResource() adds a resource and getResources() only returns unique ones by comparing the string representation'); } public function testUniqueRouteWithGivenName() From 708c0d3ff9275b105719dba305163d5ceeb88236 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 7 Dec 2012 14:29:35 +0100 Subject: [PATCH 09/10] adjust routing tests to not use prefix in addCollection --- .../Dumper/ApacheMatcherDumperTest.php | 3 +- .../Matcher/Dumper/PhpMatcherDumperTest.php | 31 ++++++++++++------- .../Routing/Tests/Matcher/UrlMatcherTest.php | 20 ++++-------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php index da72d6b3a9..72bee71002 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php @@ -150,7 +150,8 @@ class ApacheMatcherDumperTest extends \PHPUnit_Framework_TestCase $route3 = new Route('/route3', array(), array(), array(), 'b.example.com'); $collection2->add('route3', $route3); - $collection1->addCollection($collection2, '/c2'); + $collection2->addPrefix('/c2'); + $collection1->addCollection($collection2); $route4 = new Route('/route4', array(), array(), array(), 'a.example.com'); $collection1->add('route4', $route4); diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php index 4764df617d..542ede85c0 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php @@ -118,14 +118,17 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase $collection1->add('overridden', new Route('/overridden1')); $collection1->add('foo1', new Route('/{foo}')); $collection1->add('bar1', new Route('/{bar}')); + $collection1->addPrefix('/b\'b'); $collection2 = new RouteCollection(); - $collection2->addCollection($collection1, '/b\'b'); + $collection2->addCollection($collection1); $collection2->add('overridden', new Route('/{var}', array(), array('var' => '.*'))); $collection1 = new RouteCollection(); $collection1->add('foo2', new Route('/{foo1}')); $collection1->add('bar2', new Route('/{bar1}')); - $collection2->addCollection($collection1, '/b\'b'); - $collection->addCollection($collection2, '/a'); + $collection1->addPrefix('/b\'b'); + $collection2->addCollection($collection1); + $collection2->addPrefix('/a'); + $collection->addCollection($collection2); // overridden through addCollection() and multiple sub-collections with no own prefix $collection1 = new RouteCollection(); @@ -137,15 +140,16 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase $collection3->add('hey', new Route('/hey/')); $collection2->addCollection($collection3); $collection1->addCollection($collection2); - $collection->addCollection($collection1, '/multi'); + $collection1->addPrefix('/multi'); + $collection->addCollection($collection1); // "dynamic" prefix $collection1 = new RouteCollection(); $collection1->add('foo3', new Route('/{foo}')); $collection1->add('bar3', new Route('/{bar}')); - $collection2 = new RouteCollection(); - $collection2->addCollection($collection1, '/b'); - $collection->addCollection($collection2, '/{_locale}'); + $collection1->addPrefix('/b'); + $collection1->addPrefix('{_locale}'); + $collection->addCollection($collection1); // route between collections $collection->add('ababa', new Route('/ababa')); @@ -153,7 +157,8 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase // collection with static prefix but only one route $collection1 = new RouteCollection(); $collection1->add('foo4', new Route('/{foo}')); - $collection->addCollection($collection1, '/aba'); + $collection1->addPrefix('/aba'); + $collection->addCollection($collection1); // prefix and host @@ -215,10 +220,12 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase $collection2->add('b', new Route('/{var}')); $collection3 = new RouteCollection(); $collection3->add('c', new Route('/{var}')); - - $collection2->addCollection($collection3, '/c'); - $collection1->addCollection($collection2, '/b'); - $collection->addCollection($collection1, '/a'); + $collection3->addPrefix('/c'); + $collection2->addCollection($collection3); + $collection2->addPrefix('/b'); + $collection1->addCollection($collection2); + $collection1->addPrefix('/a'); + $collection->addCollection($collection1); /* test case 2 */ diff --git a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php index fee6fe2b6f..8a1428f170 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php @@ -130,14 +130,10 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase public function testMatchWithPrefixes() { - $collection1 = new RouteCollection(); - $collection1->add('foo', new Route('/{foo}')); - - $collection2 = new RouteCollection(); - $collection2->addCollection($collection1, '/b'); - $collection = new RouteCollection(); - $collection->addCollection($collection2, '/a'); + $collection->add('foo', new Route('/{foo}')); + $collection->addPrefix('/b'); + $collection->addPrefix('/a'); $matcher = new UrlMatcher($collection, new RequestContext()); $this->assertEquals(array('_route' => 'foo', 'foo' => 'foo'), $matcher->match('/a/b/foo')); @@ -145,14 +141,10 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase public function testMatchWithDynamicPrefix() { - $collection1 = new RouteCollection(); - $collection1->add('foo', new Route('/{foo}')); - - $collection2 = new RouteCollection(); - $collection2->addCollection($collection1, '/b'); - $collection = new RouteCollection(); - $collection->addCollection($collection2, '/{_locale}'); + $collection->add('foo', new Route('/{foo}')); + $collection->addPrefix('/b'); + $collection->addPrefix('/{_locale}'); $matcher = new UrlMatcher($collection, new RequestContext()); $this->assertEquals(array('_locale' => 'fr', '_route' => 'foo', 'foo' => 'foo'), $matcher->match('/fr/b/foo')); From c0687cd5fa118d807f1a7593abc5abbdfbe764f4 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Tue, 5 Mar 2013 12:20:45 +0100 Subject: [PATCH 10/10] remove() should not use deprecated getParent() so it does not trigger deprecation internally --- src/Symfony/Component/Routing/RouteCollection.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index ffdf57f478..b60b7f3d2c 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -163,7 +163,10 @@ class RouteCollection implements \IteratorAggregate, \Countable public function remove($name) { // just for BC - $root = $this->getRoot(); + $root = $this; + while ($root->parent) { + $root = $root->parent; + } foreach ((array) $name as $n) { unset($root->routes[$n]);