Skip to content

Commit

Permalink
[Routing] add priority option to annotated routes
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Feb 5, 2020
1 parent 8657314 commit 8522a83
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 15 deletions.
1 change: 1 addition & 0 deletions UPGRADE-5.1.md
Expand Up @@ -57,6 +57,7 @@ Routing
------- -------


* Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`. * Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
* Added argument `$priority` to `RouteCollection::add()`


Yaml Yaml
---- ----
Expand Down
1 change: 1 addition & 0 deletions UPGRADE-6.0.md
Expand Up @@ -47,3 +47,4 @@ Routing
------- -------


* Removed `RouteCollectionBuilder`. * Removed `RouteCollectionBuilder`.
* Added argument `$priority` to `RouteCollection::add()`
11 changes: 11 additions & 0 deletions src/Symfony/Component/Routing/Annotation/Route.php
Expand Up @@ -34,6 +34,7 @@ class Route
private $locale; private $locale;
private $format; private $format;
private $utf8; private $utf8;
private $priority;


/** /**
* @param array $data An array of key/value parameters * @param array $data An array of key/value parameters
Expand Down Expand Up @@ -179,4 +180,14 @@ public function getCondition()
{ {
return $this->condition; return $this->condition;
} }

public function setPriority(int $priority): void
{
$this->priority = $priority;
}

public function getPriority(): ?int
{
return $this->priority;
}
} }
4 changes: 3 additions & 1 deletion src/Symfony/Component/Routing/CHANGELOG.md
Expand Up @@ -4,7 +4,9 @@ CHANGELOG
5.1.0 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 5.0.0
----- -----
Expand Down
13 changes: 7 additions & 6 deletions src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php
Expand Up @@ -157,10 +157,8 @@ protected function addRoute(RouteCollection $collection, $annot, array $globals,
$host = $globals['host']; $host = $globals['host'];
} }


$condition = $annot->getCondition(); $condition = $annot->getCondition() ?? $globals['condition'];
if (null === $condition) { $priority = $annot->getPriority() ?? $globals['priority'];
$condition = $globals['condition'];
}


$path = $annot->getLocalizedPaths() ?: $annot->getPath(); $path = $annot->getLocalizedPaths() ?: $annot->getPath();
$prefix = $globals['localized_paths'] ?: $globals['path']; $prefix = $globals['localized_paths'] ?: $globals['path'];
Expand Down Expand Up @@ -208,9 +206,9 @@ protected function addRoute(RouteCollection $collection, $annot, array $globals,
if (0 !== $locale) { if (0 !== $locale) {
$route->setDefault('_locale', $locale); $route->setDefault('_locale', $locale);
$route->setDefault('_canonical_route', $name); $route->setDefault('_canonical_route', $name);
$collection->add($name.'.'.$locale, $route); $collection->add($name.'.'.$locale, $route, $priority);
} else { } else {
$collection->add($name, $route); $collection->add($name, $route, $priority);
} }
} }
} }
Expand Down Expand Up @@ -297,6 +295,8 @@ protected function getGlobals(\ReflectionClass $class)
$globals['condition'] = $annot->getCondition(); $globals['condition'] = $annot->getCondition();
} }


$globals['priority'] = $annot->getPriority() ?? 0;

foreach ($globals['requirements'] as $placeholder => $requirement) { foreach ($globals['requirements'] as $placeholder => $requirement) {
if (\is_int($placeholder)) { 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())); 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()));
Expand All @@ -320,6 +320,7 @@ private function resetGlobals(): array
'host' => '', 'host' => '',
'condition' => '', 'condition' => '',
'name' => '', 'name' => '',
'priority' => 0,
]; ];
} }


Expand Down
43 changes: 38 additions & 5 deletions src/Symfony/Component/Routing/RouteCollection.php
Expand Up @@ -35,6 +35,11 @@ class RouteCollection implements \IteratorAggregate, \Countable
*/ */
private $resources = []; private $resources = [];


/**
* @var int[]
*/
private $priorities = [];

public function __clone() public function __clone()
{ {
foreach ($this->routes as $name => $route) { foreach ($this->routes as $name => $route) {
Expand All @@ -53,7 +58,7 @@ public function __clone()
*/ */
public function getIterator() public function getIterator()
{ {
return new \ArrayIterator($this->routes); return new \ArrayIterator($this->all());
} }


/** /**
Expand All @@ -66,11 +71,22 @@ public function count()
return \count($this->routes); 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; $this->routes[$name] = $route;

if ($priority = 3 <= \func_num_args() ? func_get_arg(2) : 0) {
$this->priorities[$name] = $priority;
}
} }


/** /**
Expand All @@ -80,6 +96,14 @@ public function add(string $name, Route $route)
*/ */
public function all() 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; return $this->routes;
} }


Expand All @@ -101,7 +125,7 @@ public function get(string $name)
public function remove($name) public function remove($name)
{ {
foreach ((array) $name as $n) { foreach ((array) $name as $n) {
unset($this->routes[$n]); unset($this->routes[$n], $this->priorities[$n]);
} }
} }


Expand All @@ -114,8 +138,12 @@ public function addCollection(self $collection)
// we need to remove all routes with the same names first because just replacing them // 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 // would not place the new route at the end of the merged array
foreach ($collection->all() as $name => $route) { foreach ($collection->all() as $name => $route) {
unset($this->routes[$name]); unset($this->routes[$name], $this->priorities[$name]);
$this->routes[$name] = $route; $this->routes[$name] = $route;

if (isset($collection->priorities[$name])) {
$this->priorities[$name] = $collection->priorities[$name];
}
} }


foreach ($collection->getResources() as $resource) { foreach ($collection->getResources() as $resource) {
Expand Down Expand Up @@ -147,15 +175,20 @@ public function addPrefix(string $prefix, array $defaults = [], array $requireme
public function addNamePrefix(string $prefix) public function addNamePrefix(string $prefix)
{ {
$prefixedRoutes = []; $prefixedRoutes = [];
$prefixedPriorities = [];


foreach ($this->routes as $name => $route) { foreach ($this->routes as $name => $route) {
$prefixedRoutes[$prefix.$name] = $route; $prefixedRoutes[$prefix.$name] = $route;
if (null !== $name = $route->getDefault('_canonical_route')) { if (null !== $name = $route->getDefault('_canonical_route')) {
$route->setDefault('_canonical_route', $prefix.$name); $route->setDefault('_canonical_route', $prefix.$name);
} }
if (isset($this->priorities[$name])) {
$prefixedPriorities[$prefix.$name] = $this->priorities[$name];
}
} }


$this->routes = $prefixedRoutes; $this->routes = $prefixedRoutes;
$this->priorities = $prefixedPriorities;
} }


/** /**
Expand Down
Expand Up @@ -17,7 +17,7 @@ public function post()
} }


/** /**
* @Route(name="put", methods={"PUT"}) * @Route(name="put", methods={"PUT"}, priority=10)
*/ */
public function put() public function put()
{ {
Expand Down
Expand Up @@ -144,7 +144,7 @@ public function testDefaultValuesForMethods()
public function testMethodActionControllers() public function testMethodActionControllers()
{ {
$routes = $this->loader->load(MethodActionControllers::class); $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('put')->getPath());
$this->assertEquals('/the/path', $routes->get('post')->getPath()); $this->assertEquals('/the/path', $routes->get('post')->getPath());
} }
Expand Down Expand Up @@ -178,7 +178,7 @@ public function testGlobalDefaultsRoutesLoadWithAnnotation()
public function testUtf8RoutesLoadWithAnnotation() public function testUtf8RoutesLoadWithAnnotation()
{ {
$routes = $this->loader->load(Utf8ActionControllers::class); $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->assertTrue($routes->get('one')->getOption('utf8'), 'The route must accept utf8');
$this->assertFalse($routes->get('two')->getOption('utf8'), 'The route must not accept utf8'); $this->assertFalse($routes->get('two')->getOption('utf8'), 'The route must not accept utf8');
} }
Expand Down
33 changes: 33 additions & 0 deletions src/Symfony/Component/Routing/Tests/RouteCollectionTest.php
Expand Up @@ -330,4 +330,37 @@ public function testAddNamePrefixCanonicalRouteName()
$this->assertEquals('api_bar', $collection->get('api_bar')->getDefault('_canonical_route')); $this->assertEquals('api_bar', $collection->get('api_bar')->getDefault('_canonical_route'));
$this->assertEquals('api_api_foo', $collection->get('api_api_foo')->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());
}
} }

0 comments on commit 8522a83

Please sign in to comment.