Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 0566c39

Browse filesBrowse files
bug #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 #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
Copy full SHA for 0566c39

File tree

5 files changed

+62
-2
lines changed
Filter options

5 files changed

+62
-2
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Loader/Configurator/Traits/PrefixTrait.php
+3-2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/RouteCollection.php
+5Lines 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
}
+22Lines 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+
}
+6Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php
+26Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.