From 8522a83217c9444ab17d21f73694e2bb3cfd6759 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 5 Feb 2020 18:53:43 +0100 Subject: [PATCH] [Routing] add priority option to annotated routes --- UPGRADE-5.1.md | 1 + UPGRADE-6.0.md | 1 + .../Component/Routing/Annotation/Route.php | 11 +++++ src/Symfony/Component/Routing/CHANGELOG.md | 4 +- .../Routing/Loader/AnnotationClassLoader.php | 13 +++--- .../Component/Routing/RouteCollection.php | 43 ++++++++++++++++--- .../MethodActionControllers.php | 2 +- .../Loader/AnnotationClassLoaderTest.php | 4 +- .../Routing/Tests/RouteCollectionTest.php | 33 ++++++++++++++ 9 files changed, 97 insertions(+), 15 deletions(-) diff --git a/UPGRADE-5.1.md b/UPGRADE-5.1.md index 026334ba53..1fa3c6cf45 100644 --- a/UPGRADE-5.1.md +++ b/UPGRADE-5.1.md @@ -57,6 +57,7 @@ Routing ------- * Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`. + * Added argument `$priority` to `RouteCollection::add()` Yaml ---- diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 08243073d4..1dfdf1bfda 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -47,3 +47,4 @@ Routing ------- * Removed `RouteCollectionBuilder`. + * Added argument `$priority` to `RouteCollection::add()` diff --git a/src/Symfony/Component/Routing/Annotation/Route.php b/src/Symfony/Component/Routing/Annotation/Route.php index 8183b6fc55..0cecfeaec1 100644 --- a/src/Symfony/Component/Routing/Annotation/Route.php +++ b/src/Symfony/Component/Routing/Annotation/Route.php @@ -34,6 +34,7 @@ class Route private $locale; private $format; private $utf8; + private $priority; /** * @param array $data An array of key/value parameters @@ -179,4 +180,14 @@ class Route { return $this->condition; } + + public function setPriority(int $priority): void + { + $this->priority = $priority; + } + + public function getPriority(): ?int + { + return $this->priority; + } } diff --git a/src/Symfony/Component/Routing/CHANGELOG.md b/src/Symfony/Component/Routing/CHANGELOG.md index 0ed447d6fe..11c285c60d 100644 --- a/src/Symfony/Component/Routing/CHANGELOG.md +++ b/src/Symfony/Component/Routing/CHANGELOG.md @@ -4,7 +4,9 @@ CHANGELOG 5.1.0 ----- - * Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`. + * deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`. + * added "priority" option to annotated routes + * added argument `$priority` to `RouteCollection::add()` 5.0.0 ----- diff --git a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php index 0c52b121f8..cc440d1d3a 100644 --- a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php +++ b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php @@ -157,10 +157,8 @@ abstract class AnnotationClassLoader implements LoaderInterface $host = $globals['host']; } - $condition = $annot->getCondition(); - if (null === $condition) { - $condition = $globals['condition']; - } + $condition = $annot->getCondition() ?? $globals['condition']; + $priority = $annot->getPriority() ?? $globals['priority']; $path = $annot->getLocalizedPaths() ?: $annot->getPath(); $prefix = $globals['localized_paths'] ?: $globals['path']; @@ -208,9 +206,9 @@ abstract class AnnotationClassLoader implements LoaderInterface if (0 !== $locale) { $route->setDefault('_locale', $locale); $route->setDefault('_canonical_route', $name); - $collection->add($name.'.'.$locale, $route); + $collection->add($name.'.'.$locale, $route, $priority); } else { - $collection->add($name, $route); + $collection->add($name, $route, $priority); } } } @@ -297,6 +295,8 @@ abstract class AnnotationClassLoader implements LoaderInterface $globals['condition'] = $annot->getCondition(); } + $globals['priority'] = $annot->getPriority() ?? 0; + foreach ($globals['requirements'] as $placeholder => $requirement) { if (\is_int($placeholder)) { throw new \InvalidArgumentException(sprintf('A placeholder name must be a string (%d given). Did you forget to specify the placeholder key for the requirement "%s" in "%s"?', $placeholder, $requirement, $class->getName())); @@ -320,6 +320,7 @@ abstract class AnnotationClassLoader implements LoaderInterface 'host' => '', 'condition' => '', 'name' => '', + 'priority' => 0, ]; } diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index 6c7bcf1989..630045c875 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -35,6 +35,11 @@ class RouteCollection implements \IteratorAggregate, \Countable */ private $resources = []; + /** + * @var int[] + */ + private $priorities = []; + public function __clone() { foreach ($this->routes as $name => $route) { @@ -53,7 +58,7 @@ class RouteCollection implements \IteratorAggregate, \Countable */ public function getIterator() { - return new \ArrayIterator($this->routes); + return new \ArrayIterator($this->all()); } /** @@ -66,11 +71,22 @@ class RouteCollection implements \IteratorAggregate, \Countable return \count($this->routes); } - public function add(string $name, Route $route) + /** + * @param int $priority + */ + public function add(string $name, Route $route/*, int $priority = 0*/) { - unset($this->routes[$name]); + if (\func_num_args() < 3 && __CLASS__ !== static::class && __CLASS__ !== (new \ReflectionMethod($this, __FUNCTION__))->getDeclaringClass()->getName() && !$this instanceof \PHPUnit\Framework\MockObject\MockObject && !$this instanceof \Prophecy\Prophecy\ProphecySubjectInterface) { + @trigger_error(sprintf('The "%s()" method will have a new "int $priority = 0" argument in version 6.0, not defining it is deprecated since Symfony 5.1.', __METHOD__), E_USER_DEPRECATED); + } + + unset($this->routes[$name], $this->priorities[$name]); $this->routes[$name] = $route; + + if ($priority = 3 <= \func_num_args() ? func_get_arg(2) : 0) { + $this->priorities[$name] = $priority; + } } /** @@ -80,6 +96,14 @@ class RouteCollection implements \IteratorAggregate, \Countable */ public function all() { + if ($this->priorities) { + $priorities = $this->priorities; + $keysOrder = array_flip(array_keys($this->routes)); + uksort($this->routes, static function ($n1, $n2) use ($priorities, $keysOrder) { + return (($priorities[$n2] ?? 0) <=> ($priorities[$n1] ?? 0)) ?: ($keysOrder[$n1] <=> $keysOrder[$n2]); + }); + } + return $this->routes; } @@ -101,7 +125,7 @@ class RouteCollection implements \IteratorAggregate, \Countable public function remove($name) { foreach ((array) $name as $n) { - unset($this->routes[$n]); + unset($this->routes[$n], $this->priorities[$n]); } } @@ -114,8 +138,12 @@ class RouteCollection implements \IteratorAggregate, \Countable // 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]); + unset($this->routes[$name], $this->priorities[$name]); $this->routes[$name] = $route; + + if (isset($collection->priorities[$name])) { + $this->priorities[$name] = $collection->priorities[$name]; + } } foreach ($collection->getResources() as $resource) { @@ -147,15 +175,20 @@ class RouteCollection implements \IteratorAggregate, \Countable public function addNamePrefix(string $prefix) { $prefixedRoutes = []; + $prefixedPriorities = []; foreach ($this->routes as $name => $route) { $prefixedRoutes[$prefix.$name] = $route; if (null !== $name = $route->getDefault('_canonical_route')) { $route->setDefault('_canonical_route', $prefix.$name); } + if (isset($this->priorities[$name])) { + $prefixedPriorities[$prefix.$name] = $this->priorities[$name]; + } } $this->routes = $prefixedRoutes; + $this->priorities = $prefixedPriorities; } /** diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/MethodActionControllers.php b/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/MethodActionControllers.php index 9350f3a914..b4c2f25391 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/MethodActionControllers.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/MethodActionControllers.php @@ -17,7 +17,7 @@ class MethodActionControllers } /** - * @Route(name="put", methods={"PUT"}) + * @Route(name="put", methods={"PUT"}, priority=10) */ public function put() { diff --git a/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php index 9dc9e2c50c..5f5b4f8fa7 100644 --- a/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php +++ b/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php @@ -144,7 +144,7 @@ class AnnotationClassLoaderTest extends AbstractAnnotationLoaderTest public function testMethodActionControllers() { $routes = $this->loader->load(MethodActionControllers::class); - $this->assertCount(2, $routes); + $this->assertSame(['put', 'post'], array_keys($routes->all())); $this->assertEquals('/the/path', $routes->get('put')->getPath()); $this->assertEquals('/the/path', $routes->get('post')->getPath()); } @@ -178,7 +178,7 @@ class AnnotationClassLoaderTest extends AbstractAnnotationLoaderTest public function testUtf8RoutesLoadWithAnnotation() { $routes = $this->loader->load(Utf8ActionControllers::class); - $this->assertCount(2, $routes); + $this->assertSame(['one', 'two'], array_keys($routes->all())); $this->assertTrue($routes->get('one')->getOption('utf8'), 'The route must accept utf8'); $this->assertFalse($routes->get('two')->getOption('utf8'), 'The route must not accept utf8'); } diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index f310d4e526..05d3d0162a 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -330,4 +330,37 @@ class RouteCollectionTest extends TestCase $this->assertEquals('api_bar', $collection->get('api_bar')->getDefault('_canonical_route')); $this->assertEquals('api_api_foo', $collection->get('api_api_foo')->getDefault('_canonical_route')); } + + public function testAddWithPriority() + { + $collection = new RouteCollection(); + $collection->add('foo', $foo = new Route('/foo'), 0); + $collection->add('bar', $bar = new Route('/bar'), 1); + $collection->add('baz', $baz = new Route('/baz')); + + $expected = [ + 'bar' => $bar, + 'foo' => $foo, + 'baz' => $baz, + ]; + + $this->assertSame($expected, $collection->all()); + + $collection2 = new RouteCollection(); + $collection2->add('foo2', $foo2 = new Route('/foo'), 0); + $collection2->add('bar2', $bar2 = new Route('/bar'), 1); + $collection2->add('baz2', $baz2 = new Route('/baz')); + $collection2->addCollection($collection); + + $expected = [ + 'bar2' => $bar2, + 'bar' => $bar, + 'foo2' => $foo2, + 'baz2' => $baz2, + 'foo' => $foo, + 'baz' => $baz, + ]; + + $this->assertSame($expected, $collection2->all()); + } }