feature #35608 [Routing] add priority option to annotated routes (nicolas-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](https://github.com/symfony/symfony/pull/26132#pullrequestreview-352321332), priority is reserved to using annotations. For other formats, the order works fine.
Commits
-------
8522a83217
[Routing] add priority option to annotated routes
This commit is contained in:
commit
829fc725c5
@ -57,6 +57,7 @@ Routing
|
||||
-------
|
||||
|
||||
* Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
|
||||
* Added argument `$priority` to `RouteCollection::add()`
|
||||
|
||||
Yaml
|
||||
----
|
||||
|
@ -47,3 +47,4 @@ Routing
|
||||
-------
|
||||
|
||||
* Removed `RouteCollectionBuilder`.
|
||||
* Added argument `$priority` to `RouteCollection::add()`
|
||||
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
@ -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
|
||||
-----
|
||||
|
@ -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,
|
||||
];
|
||||
}
|
||||
|
||||
|
@ -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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -17,7 +17,7 @@ class MethodActionControllers
|
||||
}
|
||||
|
||||
/**
|
||||
* @Route(name="put", methods={"PUT"})
|
||||
* @Route(name="put", methods={"PUT"}, priority=10)
|
||||
*/
|
||||
public function put()
|
||||
{
|
||||
|
@ -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');
|
||||
}
|
||||
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user