feature #35608 [Routing] add priority option to annotated routes (nic…

…olas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Routing] add priority option to annotated routes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | yes
| Tickets       | Fix  #26132
| License       | MIT
| Doc PR        | -

This PR allows defining the priority of routes using annotations:
`@Route(name="foo", priority=10)`

As requested [in this comment](#26132 (review)), priority is reserved to using annotations. For other formats, the order works fine.

Commits
-------

8522a83 [Routing] add priority option to annotated routes

UPGRADE-5.1.md

@@ -57,6 +57,7 @@ Routing
 -------
 
  * Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
+ * Added argument `$priority` to `RouteCollection::add()`
 
 Yaml
 ----

UPGRADE-6.0.md

@@ -47,3 +47,4 @@ Routing
 -------
 
  * Removed `RouteCollectionBuilder`.
+ * Added argument `$priority` to `RouteCollection::add()`

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 @@ public function getCondition()
     {
         return $this->condition;
     }
+
+    public function setPriority(int $priority): void
+    {
+        $this->priority = $priority;
+    }
+
+    public function getPriority(): ?int
+    {
+        return $this->priority;
+    }
 }

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
 -----

src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php

@@ -157,10 +157,8 @@ protected function addRoute(RouteCollection $collection, $annot, array $globals,
             $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 @@ protected function addRoute(RouteCollection $collection, $annot, array $globals,
             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 @@ protected function getGlobals(\ReflectionClass $class)
                 $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 @@ private function resetGlobals(): array
             'host' => '',
             'condition' => '',
             'name' => '',
+            'priority' => 0,
         ];
     }
 

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 @@ public function __clone()
      */
     public function getIterator()
     {
-        return new \ArrayIterator($this->routes);
+        return new \ArrayIterator($this->all());
     }
 
     /**
@@ -66,11 +71,22 @@ public function count()
         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 @@ public function add(string $name, Route $route)
      */
     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 @@ public function get(string $name)
     public function remove($name)
     {
         foreach ((array) $name as $n) {
-            unset($this->routes[$n]);
+            unset($this->routes[$n], $this->priorities[$n]);
         }
     }
 
@@ -114,8 +138,12 @@ public function addCollection(self $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]);
+            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 @@ public function addPrefix(string $prefix, array $defaults = [], array $requireme
     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;
     }
 
     /**

src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/MethodActionControllers.php

@@ -17,7 +17,7 @@ public function post()
     }
 
     /**
-     * @Route(name="put", methods={"PUT"})
+     * @Route(name="put", methods={"PUT"}, priority=10)
      */
     public function put()
     {

src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php

@@ -144,7 +144,7 @@ public function testDefaultValuesForMethods()
     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 @@ public function testGlobalDefaultsRoutesLoadWithAnnotation()
     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');
     }

src/Symfony/Component/Routing/Tests/RouteCollectionTest.php

@@ -330,4 +330,37 @@ public function testAddNamePrefixCanonicalRouteName()
         $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());
+    }
 }