From d88517c7c82307152ea03d3546c8ae0623bbb241 Mon Sep 17 00:00:00 2001 From: Magnus Nordlander Date: Sat, 10 Feb 2018 13:57:50 +0100 Subject: [PATCH] Added priorities to RouteCollection --- .../Component/Routing/RouteCollection.php | 93 +++++++++++++------ .../Routing/Tests/RouteCollectionTest.php | 29 +++++- 2 files changed, 92 insertions(+), 30 deletions(-) diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index ebe92da714c39..efc70d5a60bb1 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -26,9 +26,14 @@ class RouteCollection implements \IteratorAggregate, \Countable { /** - * @var Route[] + * @var Route[][] */ - private $routes = array(); + private $prioritizedRoutes = array(); + + /** + * @var int[] + */ + private $priorities = array(); /** * @var array @@ -37,8 +42,10 @@ class RouteCollection implements \IteratorAggregate, \Countable public function __clone() { - foreach ($this->routes as $name => $route) { - $this->routes[$name] = clone $route; + foreach ($this->prioritizedRoutes as $priority => $routes) { + foreach ($routes as $name => $route) { + $this->prioritizedRoutes[$priority][$name] = clone $route; + } } } @@ -48,12 +55,13 @@ public function __clone() * It implements \IteratorAggregate. * * @see all() + * @todo Change return type to Iterator|Route[] in 5.0 * * @return \ArrayIterator|Route[] An \ArrayIterator object for iterating over routes */ public function getIterator() { - return new \ArrayIterator($this->routes); + return new \ArrayIterator($this->all()); } /** @@ -63,7 +71,7 @@ public function getIterator() */ public function count() { - return count($this->routes); + return count($this->priorities); } /** @@ -74,11 +82,24 @@ public function count() */ public function add($name, Route $route) { - unset($this->routes[$name]); + $this->addWithPriority($name, $route, 0); + } - $this->routes[$name] = $route; + /** + * Adds a route. + * + * @param string $name The route name + * @param Route $route A Route instance + */ + public function addWithPriority($name, Route $route, int $priority = 0) + { + $this->remove($name); + + $this->priorities[$name] = $priority; + $this->prioritizedRoutes[$priority][$name] = $route; } + /** * Returns all routes in this collection. * @@ -86,7 +107,15 @@ public function add($name, Route $route) */ public function all() { - return $this->routes; + // We can't use array_merge here, because that doesn't preserve numeric keys + krsort($this->prioritizedRoutes); + + $all = []; + foreach ($this->prioritizedRoutes as $priority => $routes) { + $all = $all + $routes; + } + + return $all; } /** @@ -98,7 +127,7 @@ public function all() */ public function get($name) { - return isset($this->routes[$name]) ? $this->routes[$name] : null; + return isset($this->priorities[$name]) ? $this->prioritizedRoutes[$this->priorities[$name]][$name] : null; } /** @@ -109,7 +138,11 @@ public function get($name) public function remove($name) { foreach ((array) $name as $n) { - unset($this->routes[$n]); + if (!isset($this->priorities[$n])) { + continue; + }; + unset($this->prioritizedRoutes[$this->priorities[$n]][$n]); + unset($this->priorities[$n]); } } @@ -119,11 +152,10 @@ public function remove($name) */ public function addCollection(RouteCollection $collection) { - // we need to remove all routes with the same names first because just replacing them - // would not place the new route at the end of the merged array - foreach ($collection->all() as $name => $route) { - unset($this->routes[$name]); - $this->routes[$name] = $route; + foreach ($collection->prioritizedRoutes as $priority => $routes) { + foreach ($routes as $name => $route) { + $this->addWithPriority($name, $route, $priority); + } } foreach ($collection->getResources() as $resource) { @@ -146,7 +178,7 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement return; } - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->setPath('/'.$prefix.$route->getPath()); $route->addDefaults($defaults); $route->addRequirements($requirements); @@ -158,13 +190,18 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement */ public function addNamePrefix(string $prefix) { - $prefixedRoutes = array(); + $prefixedRoutes = []; + $prefixedPriorites = []; - foreach ($this->routes as $name => $route) { - $prefixedRoutes[$prefix.$name] = $route; + foreach ($this->prioritizedRoutes as $priority => $routes) { + foreach ($routes as $name => $route) { + $prefixedPriorites[$prefix.$name] = $priority; + $prefixedRoutes[$priority][$prefix.$name] = $route; + } } - $this->routes = $prefixedRoutes; + $this->priorities = $prefixedPriorites; + $this->prioritizedRoutes = $prefixedRoutes; } /** @@ -176,7 +213,7 @@ public function addNamePrefix(string $prefix) */ public function setHost($pattern, array $defaults = array(), array $requirements = array()) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->setHost($pattern); $route->addDefaults($defaults); $route->addRequirements($requirements); @@ -192,7 +229,7 @@ public function setHost($pattern, array $defaults = array(), array $requirements */ public function setCondition($condition) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->setCondition($condition); } } @@ -207,7 +244,7 @@ public function setCondition($condition) public function addDefaults(array $defaults) { if ($defaults) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->addDefaults($defaults); } } @@ -223,7 +260,7 @@ public function addDefaults(array $defaults) public function addRequirements(array $requirements) { if ($requirements) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->addRequirements($requirements); } } @@ -239,7 +276,7 @@ public function addRequirements(array $requirements) public function addOptions(array $options) { if ($options) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->addOptions($options); } } @@ -252,7 +289,7 @@ public function addOptions(array $options) */ public function setSchemes($schemes) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->setSchemes($schemes); } } @@ -264,7 +301,7 @@ public function setSchemes($schemes) */ public function setMethods($methods) { - foreach ($this->routes as $route) { + foreach ($this->all() as $route) { $route->setMethods($methods); } } diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 3527e12895683..7e8f129933709 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -86,20 +86,23 @@ public function testAddCollection() { $collection = new RouteCollection(); $collection->add('foo', new Route('/foo')); + $collection->add('prio', new Route('/prio')); $collection1 = new RouteCollection(); $collection1->add('bar', $bar = new Route('/bar')); $collection1->add('foo', $foo = new Route('/foo-new')); + $collection1->addWithPriority('prio', $prio = new Route('/prio-new'), 100); $collection2 = new RouteCollection(); $collection2->add('grandchild', $grandchild = new Route('/grandchild')); + $collection2->addWithPriority('first', $first = new Route('/grandchild'), 200); $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'); + $this->assertSame(array('first' => $first, 'prio' => $prio, '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 of their priority'); } public function testAddCollectionWithResources() @@ -317,4 +320,26 @@ public function testAddNamePrefix() $this->assertNull($collection->get('foo')); $this->assertNull($collection->get('bar')); } + + public function testAddWithNumericName() + { + $collection = new RouteCollection(); + $collection->add(100, new Route('/foo')); + + $this->assertSame([100], array_keys($collection->all())); + } + + public function testAddWithPriority() + { + $collection = new RouteCollection(); + $collection->addWithPriority('foo', $foo = new Route('/foo'), 0); + $collection->addWithPriority('bar', $bar = new Route('/bar'), 1); + + $expected = [ + 'bar' => $bar, + 'foo' => $foo, + ]; + + $this->assertSame($expected, $collection->all()); + } }