From cd40bb8604ca7842f0ffbaa4ba0782865a976401 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Tue, 24 Dec 2019 16:21:48 +0100 Subject: [PATCH] [Routing] Fix i18n routing when the url contains the locale --- .../Generator/CompiledUrlGenerator.php | 9 ++++- .../Generator/Dumper/PhpGeneratorDumper.php | 9 ++++- .../Routing/Generator/UrlGenerator.php | 14 ++++++-- .../Dumper/CompiledUrlGeneratorDumperTest.php | 31 +++++++++++++++-- .../Dumper/PhpGeneratorDumperTest.php | 34 +++++++++++++++++-- .../Tests/Generator/UrlGeneratorTest.php | 23 +++++++++++++ 6 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/CompiledUrlGenerator.php b/src/Symfony/Component/Routing/Generator/CompiledUrlGenerator.php index 41cd5893ae..adcc99e310 100644 --- a/src/Symfony/Component/Routing/Generator/CompiledUrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/CompiledUrlGenerator.php @@ -40,7 +40,6 @@ class CompiledUrlGenerator extends UrlGenerator if (null !== $locale) { do { if (($this->compiledRoutes[$name.'.'.$locale][1]['_canonical_route'] ?? null) === $name) { - unset($parameters['_locale']); $name .= '.'.$locale; break; } @@ -53,6 +52,14 @@ class CompiledUrlGenerator extends UrlGenerator list($variables, $defaults, $requirements, $tokens, $hostTokens, $requiredSchemes) = $this->compiledRoutes[$name]; + if (isset($defaults['_canonical_route']) && isset($defaults['_locale'])) { + if (!\in_array('_locale', $variables, true)) { + unset($parameters['_locale']); + } elseif (!isset($parameters['_locale'])) { + $parameters['_locale'] = $defaults['_locale']; + } + } + return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, $requiredSchemes); } } diff --git a/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php b/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php index 3869ffda4e..8526d81bc0 100644 --- a/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php +++ b/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php @@ -120,7 +120,6 @@ EOF; if (null !== $locale && null !== $name) { do { if ((self::$declaredRoutes[$name.'.'.$locale][1]['_canonical_route'] ?? null) === $name) { - unset($parameters['_locale']); $name .= '.'.$locale; break; } @@ -133,6 +132,14 @@ EOF; list($variables, $defaults, $requirements, $tokens, $hostTokens, $requiredSchemes) = self::$declaredRoutes[$name]; + if (isset($defaults['_canonical_route']) && isset($defaults['_locale'])) { + if (!\in_array('_locale', $variables, true)) { + unset($parameters['_locale']); + } elseif (!isset($parameters['_locale'])) { + $parameters['_locale'] = $defaults['_locale']; + } + } + return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, $requiredSchemes); } EOF; diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 8be593bf98..7287066996 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -134,7 +134,6 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt if (null !== $locale) { do { if (null !== ($route = $this->routes->get($name.'.'.$locale)) && $route->getDefault('_canonical_route') === $name) { - unset($parameters['_locale']); break; } } while (false !== $locale = strstr($locale, '_', true)); @@ -147,7 +146,18 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt // the Route has a cache of its own and is not recompiled as long as it does not get modified $compiledRoute = $route->compile(); - return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens(), $route->getSchemes()); + $defaults = $route->getDefaults(); + $variables = $compiledRoute->getVariables(); + + if (isset($defaults['_canonical_route']) && isset($defaults['_locale'])) { + if (!\in_array('_locale', $variables, true)) { + unset($parameters['_locale']); + } elseif (!isset($parameters['_locale'])) { + $parameters['_locale'] = $defaults['_locale']; + } + } + + return $this->doGenerate($variables, $defaults, $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens(), $route->getSchemes()); } /** diff --git a/src/Symfony/Component/Routing/Tests/Generator/Dumper/CompiledUrlGeneratorDumperTest.php b/src/Symfony/Component/Routing/Tests/Generator/Dumper/CompiledUrlGeneratorDumperTest.php index 521f0f126c..de4776ff78 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/Dumper/CompiledUrlGeneratorDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/Dumper/CompiledUrlGeneratorDumperTest.php @@ -131,9 +131,9 @@ class CompiledUrlGeneratorDumperTest extends TestCase public function testDumpWithFallbackLocaleLocalizedRoutes() { - $this->routeCollection->add('test.en', (new Route('/testing/is/fun'))->setDefault('_canonical_route', 'test')); - $this->routeCollection->add('test.nl', (new Route('/testen/is/leuk'))->setDefault('_canonical_route', 'test')); - $this->routeCollection->add('test.fr', (new Route('/tester/est/amusant'))->setDefault('_canonical_route', 'test')); + $this->routeCollection->add('test.en', (new Route('/testing/is/fun'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'test')); + $this->routeCollection->add('test.nl', (new Route('/testen/is/leuk'))->setDefault('_locale', 'nl')->setDefault('_canonical_route', 'test')); + $this->routeCollection->add('test.fr', (new Route('/tester/est/amusant'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'test')); $code = $this->generatorDumper->dump(); file_put_contents($this->testTmpFilepath, $code); @@ -231,4 +231,29 @@ class CompiledUrlGeneratorDumperTest extends TestCase $this->assertEquals('https://localhost/app.php/testing', $absoluteUrl); $this->assertEquals('/app.php/testing', $relativeUrl); } + + public function testDumpWithLocalizedRoutesPreserveTheGoodLocaleInTheUrl() + { + $this->routeCollection->add('foo.en', (new Route('/{_locale}/foo'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'foo')); + $this->routeCollection->add('foo.fr', (new Route('/{_locale}/foo'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'foo')); + $this->routeCollection->add('fun.en', (new Route('/fun'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'fun')); + $this->routeCollection->add('fun.fr', (new Route('/amusant'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'fun')); + + file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump()); + + $requestContext = new RequestContext(); + $requestContext->setParameter('_locale', 'fr'); + + $compiledUrlGenerator = new CompiledUrlGenerator(require $this->testTmpFilepath, $requestContext, null, null); + + $this->assertSame('/fr/foo', $compiledUrlGenerator->generate('foo')); + $this->assertSame('/en/foo', $compiledUrlGenerator->generate('foo.en')); + $this->assertSame('/en/foo', $compiledUrlGenerator->generate('foo', ['_locale' => 'en'])); + $this->assertSame('/en/foo', $compiledUrlGenerator->generate('foo.fr', ['_locale' => 'en'])); + + $this->assertSame('/amusant', $compiledUrlGenerator->generate('fun')); + $this->assertSame('/fun', $compiledUrlGenerator->generate('fun.en')); + $this->assertSame('/fun', $compiledUrlGenerator->generate('fun', ['_locale' => 'en'])); + $this->assertSame('/amusant', $compiledUrlGenerator->generate('fun.fr', ['_locale' => 'en'])); + } } diff --git a/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php b/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php index 5e81b8f5d5..1787144780 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php @@ -140,9 +140,9 @@ class PhpGeneratorDumperTest extends TestCase public function testDumpWithFallbackLocaleLocalizedRoutes() { - $this->routeCollection->add('test.en', (new Route('/testing/is/fun'))->setDefault('_canonical_route', 'test')); - $this->routeCollection->add('test.nl', (new Route('/testen/is/leuk'))->setDefault('_canonical_route', 'test')); - $this->routeCollection->add('test.fr', (new Route('/tester/est/amusant'))->setDefault('_canonical_route', 'test')); + $this->routeCollection->add('test.en', (new Route('/testing/is/fun'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'test')); + $this->routeCollection->add('test.nl', (new Route('/testen/is/leuk'))->setDefault('_locale', 'nl')->setDefault('_canonical_route', 'test')); + $this->routeCollection->add('test.fr', (new Route('/tester/est/amusant'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'test')); $code = $this->generatorDumper->dump([ 'class' => 'FallbackLocaleLocalizedProjectUrlGenerator', @@ -250,4 +250,32 @@ class PhpGeneratorDumperTest extends TestCase $this->assertEquals('https://localhost/app.php/testing', $absoluteUrl); $this->assertEquals('/app.php/testing', $relativeUrl); } + + public function testDumpWithLocalizedRoutesPreserveTheGoodLocaleInTheUrl() + { + $this->routeCollection->add('foo.en', (new Route('/{_locale}/foo'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'foo')); + $this->routeCollection->add('foo.fr', (new Route('/{_locale}/foo'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'foo')); + $this->routeCollection->add('fun.en', (new Route('/fun'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'fun')); + $this->routeCollection->add('fun.fr', (new Route('/amusant'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'fun')); + + file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump([ + 'class' => 'PreserveTheGoodLocaleInTheUrlGenerator', + ])); + include $this->testTmpFilepath; + + $requestContext = new RequestContext(); + $requestContext->setParameter('_locale', 'fr'); + + $phpGenerator = new \PreserveTheGoodLocaleInTheUrlGenerator($requestContext); + + $this->assertSame('/fr/foo', $phpGenerator->generate('foo')); + $this->assertSame('/en/foo', $phpGenerator->generate('foo.en')); + $this->assertSame('/en/foo', $phpGenerator->generate('foo', ['_locale' => 'en'])); + $this->assertSame('/en/foo', $phpGenerator->generate('foo.fr', ['_locale' => 'en'])); + + $this->assertSame('/amusant', $phpGenerator->generate('fun')); + $this->assertSame('/fun', $phpGenerator->generate('fun.en')); + $this->assertSame('/fun', $phpGenerator->generate('fun', ['_locale' => 'en'])); + $this->assertSame('/amusant', $phpGenerator->generate('fun.fr', ['_locale' => 'en'])); + } } diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index a768384747..01215da2c0 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -236,6 +236,29 @@ class UrlGeneratorTest extends TestCase ); } + public function testDumpWithLocalizedRoutesPreserveTheGoodLocaleInTheUrl() + { + $routeCollection = new RouteCollection(); + + $routeCollection->add('foo.en', (new Route('/{_locale}/foo'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'foo')); + $routeCollection->add('foo.fr', (new Route('/{_locale}/foo'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'foo')); + $routeCollection->add('fun.en', (new Route('/fun'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'fun')); + $routeCollection->add('fun.fr', (new Route('/amusant'))->setDefault('_locale', 'fr')->setDefault('_canonical_route', 'fun')); + + $urlGenerator = $this->getGenerator($routeCollection); + $urlGenerator->getContext()->setParameter('_locale', 'fr'); + + $this->assertSame('/app.php/fr/foo', $urlGenerator->generate('foo')); + $this->assertSame('/app.php/en/foo', $urlGenerator->generate('foo.en')); + $this->assertSame('/app.php/en/foo', $urlGenerator->generate('foo', ['_locale' => 'en'])); + $this->assertSame('/app.php/en/foo', $urlGenerator->generate('foo.fr', ['_locale' => 'en'])); + + $this->assertSame('/app.php/amusant', $urlGenerator->generate('fun')); + $this->assertSame('/app.php/fun', $urlGenerator->generate('fun.en')); + $this->assertSame('/app.php/fun', $urlGenerator->generate('fun', ['_locale' => 'en'])); + $this->assertSame('/app.php/amusant', $urlGenerator->generate('fun.fr', ['_locale' => 'en'])); + } + public function testGenerateWithoutRoutes() { $this->expectException('Symfony\Component\Routing\Exception\RouteNotFoundException');