8000 bug #52913 [Routing] Fixed priority getting lost when setting localiz… · Luc45/symfony@0566c39 · GitHub
[go: up one dir, main page]
Skip to content
< 8000 script crossorigin="anonymous" defer="defer" type="application/javascript" src="https://github.githubassets.com/assets/sessions-1e75b15ae60a.js">

Commit 0566c39

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#52913 [Routing] Fixed priority getting lost when setting localized prefix (pritasil)
This PR was merged into the 5.4 branch. Discussion ---------- [Routing] Fixed priority getting lost when setting localized prefix | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#52912 | License | MIT When a route prefix is an array, the original route is removed from the collection and then re-added with a new name and path. During this process, the route's priority is lost as it's not preserved during the removal and re-addition. Current state of `PrefixTrait.php` ```php $routes->remove($name); ... $routes->add($name.'.'.$locale, $localizedRoute); ``` To resolve this issue, I implemented a change where the priority of the route is stored before removal. This stored priority is then reassigned to the route when it's added back to the collection. Commits ------- ba41175 [Routing] Fixed priority getting lost when defining prefix array
2 parents 27346bc + ba41175 commit 0566c39

File tree

5 files changed

+62
-2
lines changed

5 files changed

+62
-2
lines changed

‎src/Symfony/Component/Routing/Loader/Configurator/Traits/PrefixTrait.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,21 @@ final protected function addPrefix(RouteCollection $routes, $prefix, bool $trail
2929
}
3030
foreach ($routes->all() as $name => $route) {
3131
if (null === $locale = $route->getDefault('_locale')) {
32+
$priority = $routes->getPriority($name) ?? 0;
3233
$routes->remove($name);
3334
foreach ($prefix as $locale => $localePrefix) {
3435
$localizedRoute = clone $route;
3536
$localizedRoute->setDefault('_locale', $locale);
3637
$localizedRoute->setRequirement('_locale', preg_quote($locale));
3738
$localizedRoute->setDefault('_canonical_route', $name);
3839
$localizedRoute->setPath($localePrefix.(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
39-
$routes->add($name.'.'.$locale, $localizedRoute);
40+
$routes->add($name.'.'.$locale, $localizedRoute, $priority);
4041
}
4142
} elseif (!isset($prefix[$locale])) {
4243
throw new \InvalidArgumentException(sprintf('Route "%s" with locale "%s" is missing a corresponding prefix in its parent collection.', $name, $locale));
4344
} else {
4445
$route->setPath($prefix[$locale].(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
45-
$routes->add($name, $route);
46+
$routes->add($name, $route, $routes->getPriority($name) ?? 0);
4647
}
4748
}
4849

‎src/Symfony/Component/Routing/RouteCollection.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,4 +395,9 @@ public function getAlias(string $name): ?Alias
395395
{
396396
return $this->aliases[$name] ?? null;
397397
}
398+
399+
public function getPriority(string $name): ?int
400+
{
401+
return $this->priorities[$name] ?? null;
402+
}
398403
}

‎src/Symfony/Component/Routing/Tests/Fixtures/AttributeFixtures/RouteWithPriorityController.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Symfony\Component\Routing\Tests\Fixtures\AttributeFixtures;
4+
5+
use Symfony\Component\Routing\Annotation\Route;
6+
7+
class RouteWithPriorityController
8+
{
9+
/**
10+
* @Route("/important", name="important", priority=2)
11+
*/
12+
public function important()
13+
{
14+
}
15+
16+
/**
17+
* @Route("/also-important", name="also_important", priority=1, defaults={"_locale": "cs"})
18+
*/
19+
public function alsoImportant()
20+
{
21+
}
22+
}

‎src/Symfony/Component/Routing/Tests/Fixtures/localized/localized-prefix.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
important_controllers:
2+
resource: Symfony\Component\Routing\Tests\Fixtures\AttributeFixtures\RouteWithPriorityController
3+
type: annotation
4+
prefix:
5+
cs: /cs
6+
en: /en

‎src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111

1212
namespace Symfony\Component\Routing\Tests\Loader;
1313

14+
use Doctrine\Common\Annotations\AnnotationReader;
1415
use PHPUnit\Framework\TestCase;
1516
use Symfony\Component\Config\FileLocator;
17+
use Symfony\Component\Config\Loader\LoaderResolver;
1618
use Symfony\Component\Config\Resource\FileResource;
19+
use Symfony\Component\Routing\Loader\AnnotationClassLoader;
1720
use Symfony\Component\Routing\Loader\YamlFileLoader;
1821
use Symfony\Component\Routing\Route;
1922
use Symfony\Component\Routing\RouteCollection;
@@ -458,4 +461,27 @@ public function testImportingAliases()
458461

459462
$this->assertEquals($expectedRoutes('yaml'), $routes);
460463
}
464+
465+
public function testPriorityWithPrefix()
466+
{
467+
new LoaderResolver([
468+
$loader = new YamlFileLoader(new FileLocator(\dirname(__DIR__).'/Fixtures/localized')),
469+
new class(new AnnotationReader(), null) extends AnnotationClassLoader {
470+
protected function configureRoute(
471+
Route $route,
472+
\ReflectionClass $class,
473+
\ReflectionMethod $method,
474+
object $annot
475+
): void {
476+
$route->setDefault('_controller', $class->getName().'::'.$method->getName());
477+
}
478+
},
479+
]);
480+
481+
$routes = $loader->load('localized-prefix.yml');
482+
483+
$this->assertSame(2, $routes->getPriority('important.cs'));
484+
$this->assertSame(2, $routes->getPriority('important.en'));
485+
$this->assertSame(1, $routes->getPriority('also_important'));
486+
}
461487
}

0 commit comments

Comments
 (0)
0