diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index e53606da5f19..b60b7f3d2c49 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -65,6 +65,9 @@ public function __clone() */ 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 @@ public function getParent() */ 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(); @@ -157,7 +163,10 @@ public function get($name) 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]); @@ -184,6 +193,8 @@ public function addCollection(RouteCollection $collection) // 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 +243,13 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement $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 +268,9 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement */ 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; } diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php index da72d6b3a91f..72bee7100228 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 @@ private function getRouteCollection() $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 4764df617da4..542ede85c001 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 @@ public function getRouteCollections() $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 @@ public function getRouteCollections() $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 @@ public function getRouteCollections() // 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 @@ public function getRouteCollections() $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 fee6fe2b6f8a..8a1428f17085 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php @@ -130,14 +130,10 @@ public function testMatch() 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 @@ public function testMatchWithPrefixes() 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')); diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 65f9967e6a1a..901317663f69 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -54,16 +54,19 @@ public function testDeepOverriddenRoute() $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,58 +75,71 @@ public function testCount() $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) + public function testAddCollection() { - foreach ($routeCollection as $key => $route) { - if ($route instanceof RouteCollection) { - return $this->getFirstNamedRoute($route, $name); - } - - if ($name === $key) { - return $route; - } - } + $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 testAddCollection() + 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')); + $collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml')); $collection1 = new RouteCollection(); - $collection1->add('foo', $foo1 = new Route('/foo1')); - $collection1->add('bar', $bar1 = new Route('/bar1')); + $collection1->addResource($foo1 = new FileResource(__DIR__.'/Fixtures/foo1.xml')); $collection->addCollection($collection1); - $this->assertEquals(array('foo' => $foo1, 'bar' => $bar1), $collection->all(), '->addCollection() adds routes from another collection'); + $this->assertEquals(array($foo, $foo1), $collection->getResources(), '->addCollection() merges resources'); + } + public function testAddDefaultsAndRequirementsAndOptions() + { $collection = new RouteCollection(); - $collection->add('foo', $foo = new Route('/foo')); + $collection->add('foo', new Route('/{placeholder}')); $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' + $collection1->add('bar', new Route('/{placeholder}', + array('_controller' => 'fixed', 'placeholder' => 'default'), array('placeholder' => '.+'), array('option' => 'value')) ); - - $collection = new RouteCollection(); - $collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml')); - $collection1 = new RouteCollection(); - $collection1->addResource($foo1 = new FileResource(__DIR__.'/Fixtures/foo1.xml')); $collection->addCollection($collection1); - $this->assertEquals(array($foo, $foo1), $collection->getResources(), '->addCollection() merges resources'); + + $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() @@ -133,30 +149,20 @@ public function testAddPrefix() $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'); - $collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'), array('foo' => 'bar')); + $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+')); $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}', $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() @@ -170,19 +176,6 @@ public function testAddPrefixOverridesDefaultsAndRequirements() $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')) { @@ -191,7 +184,11 @@ public function testResource() $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() @@ -217,13 +214,31 @@ public function testGet() $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();