From 4a1ad4a5d6ec890c8fe76d91906271abf47f0ccb Mon Sep 17 00:00:00 2001 From: Antonio Pauletich Date: Mon, 8 Apr 2019 20:40:53 +0200 Subject: [PATCH] [Routing] Fix route URL generation in CLI context --- .../Resources/config/routing.xml | 1 + .../Bundle/FrameworkBundle/Routing/Router.php | 4 +- .../Tests/Routing/RouterTest.php | 26 +++++ .../Bundle/FrameworkBundle/composer.json | 2 +- src/Symfony/Component/Routing/Router.php | 12 +- .../Tests/Generator/UrlGeneratorTest.php | 103 +++++++++++++++++- 6 files changed, 141 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml index 9cad97d984..ea12c693d7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml @@ -71,6 +71,7 @@ + %kernel.default_locale% diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php index 6bea82e1ef..93e6c3ad85 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php +++ b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php @@ -43,7 +43,7 @@ class Router extends BaseRouter implements WarmableInterface, ServiceSubscriberI * @param ContainerInterface|null $parameters A ContainerInterface instance allowing to fetch parameters * @param LoggerInterface|null $logger */ - public function __construct(ContainerInterface $container, $resource, array $options = [], RequestContext $context = null, ContainerInterface $parameters = null, LoggerInterface $logger = null) + public function __construct(ContainerInterface $container, $resource, array $options = [], RequestContext $context = null, ContainerInterface $parameters = null, LoggerInterface $logger = null, string $defaultLocale = null) { $this->container = $container; $this->resource = $resource; @@ -58,6 +58,8 @@ class Router extends BaseRouter implements WarmableInterface, ServiceSubscriberI } else { throw new \LogicException(sprintf('You should either pass a "%s" instance or provide the $parameters argument of the "%s" method.', SymfonyContainerInterface::class, __METHOD__)); } + + $this->defaultLocale = $defaultLocale; } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php index 329f1e7cba..3f04e46dd5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php @@ -82,6 +82,32 @@ class RouterTest extends TestCase $this->assertSame('"bar" == "bar"', $router->getRouteCollection()->get('foo')->getCondition()); } + public function testGenerateWithDefaultLocale() + { + $routes = new RouteCollection(); + + $route = new Route(''); + + $name = 'testFoo'; + + foreach (['hr' => '/test-hr', 'en' => '/test-en'] as $locale => $path) { + $localizedRoute = clone $route; + $localizedRoute->setDefault('_locale', $locale); + $localizedRoute->setDefault('_canonical_route', $name); + $localizedRoute->setPath($path); + $routes->add($name.'.'.$locale, $localizedRoute); + } + + $sc = $this->getServiceContainer($routes); + + $router = new Router($sc, '', [], null, null, null, 'hr'); + + $this->assertSame('/test-hr', $router->generate($name)); + + $this->assertSame('/test-en', $router->generate($name, ['_locale' => 'en'])); + $this->assertSame('/test-hr', $router->generate($name, ['_locale' => 'hr'])); + } + public function testDefaultsPlaceholders() { $routes = new RouteCollection(); diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index 44bb6b0396..9921bf6a7b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -28,7 +28,7 @@ "symfony/polyfill-mbstring": "~1.0", "symfony/filesystem": "~3.4|~4.0", "symfony/finder": "~3.4|~4.0", - "symfony/routing": "^4.1" + "symfony/routing": "^4.2.6" }, "require-dev": { "doctrine/cache": "~1.0", diff --git a/src/Symfony/Component/Routing/Router.php b/src/Symfony/Component/Routing/Router.php index 27c32e14ae..7a40c363e0 100644 --- a/src/Symfony/Component/Routing/Router.php +++ b/src/Symfony/Component/Routing/Router.php @@ -73,6 +73,11 @@ class Router implements RouterInterface, RequestMatcherInterface */ protected $logger; + /** + * @var string|null + */ + protected $defaultLocale; + /** * @var ConfigCacheFactoryInterface|null */ @@ -90,13 +95,14 @@ class Router implements RouterInterface, RequestMatcherInterface * @param RequestContext $context The context * @param LoggerInterface $logger A logger instance */ - public function __construct(LoaderInterface $loader, $resource, array $options = [], RequestContext $context = null, LoggerInterface $logger = null) + public function __construct(LoaderInterface $loader, $resource, array $options = [], RequestContext $context = null, LoggerInterface $logger = null, string $defaultLocale = null) { $this->loader = $loader; $this->resource = $resource; $this->logger = $logger; $this->context = $context ?: new RequestContext(); $this->setOptions($options); + $this->defaultLocale = $defaultLocale; } /** @@ -321,7 +327,7 @@ class Router implements RouterInterface, RequestMatcherInterface } if (null === $this->options['cache_dir'] || null === $this->options['generator_cache_class']) { - $this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger); + $this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger, $this->defaultLocale); } else { $cache = $this->getConfigCacheFactory()->cache($this->options['cache_dir'].'/'.$this->options['generator_cache_class'].'.php', function (ConfigCacheInterface $cache) { @@ -340,7 +346,7 @@ class Router implements RouterInterface, RequestMatcherInterface require_once $cache->getPath(); } - $this->generator = new $this->options['generator_cache_class']($this->context, $this->logger); + $this->generator = new $this->options['generator_cache_class']($this->context, $this->logger, $this->defaultLocale); } if ($this->generator instanceof ConfigurableRequirementsInterface) { diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 7f64a1f378..c7cfb2852e 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -162,6 +162,82 @@ class UrlGeneratorTest extends TestCase $this->assertSame('/app.php/de', $url); } + public function testGenerateWithDefaultLocale() + { + $routes = new RouteCollection(); + + $route = new Route(''); + + $name = 'test'; + + foreach (['hr' => '/foo', 'en' => '/bar'] as $locale => $path) { + $localizedRoute = clone $route; + $localizedRoute->setDefault('_locale', $locale); + $localizedRoute->setDefault('_canonical_route', $name); + $localizedRoute->setPath($path); + $routes->add($name.'.'.$locale, $localizedRoute); + } + + $generator = $this->getGenerator($routes, [], null, 'hr'); + + $this->assertSame( + 'http://localhost/app.php/foo', + $generator->generate($name, [], UrlGeneratorInterface::ABSOLUTE_URL) + ); + } + + public function testGenerateWithOverriddenParameterLocale() + { + $routes = new RouteCollection(); + + $route = new Route(''); + + $name = 'test'; + + foreach (['hr' => '/foo', 'en' => '/bar'] as $locale => $path) { + $localizedRoute = clone $route; + $localizedRoute->setDefault('_locale', $locale); + $localizedRoute->setDefault('_canonical_route', $name); + $localizedRoute->setPath($path); + $routes->add($name.'.'.$locale, $localizedRoute); + } + + $generator = $this->getGenerator($routes, [], null, 'hr'); + + $this->assertSame( + 'http://localhost/app.php/bar', + $generator->generate($name, ['_locale' => 'en'], UrlGeneratorInterface::ABSOLUTE_URL) + ); + } + + public function testGenerateWithOverriddenParameterLocaleFromRequestContext() + { + $routes = new RouteCollection(); + + $route = new Route(''); + + $name = 'test'; + + foreach (['hr' => '/foo', 'en' => '/bar'] as $locale => $path) { + $localizedRoute = clone $route; + $localizedRoute->setDefault('_locale', $locale); + $localizedRoute->setDefault('_canonical_route', $name); + $localizedRoute->setPath($path); + $routes->add($name.'.'.$locale, $localizedRoute); + } + + $generator = $this->getGenerator($routes, [], null, 'hr'); + + $context = new RequestContext('/app.php'); + $context->setParameter('_locale', 'en'); + $generator->setContext($context); + + $this->assertSame( + 'http://localhost/app.php/bar', + $generator->generate($name, [], UrlGeneratorInterface::ABSOLUTE_URL) + ); + } + /** * @expectedException \Symfony\Component\Routing\Exception\RouteNotFoundException */ @@ -171,6 +247,29 @@ class UrlGeneratorTest extends TestCase $this->getGenerator($routes)->generate('test', [], UrlGeneratorInterface::ABSOLUTE_URL); } + /** + * @expectedException \Symfony\Component\Routing\Exception\RouteNotFoundException + */ + public function testGenerateWithInvalidLocale() + { + $routes = new RouteCollection(); + + $route = new Route(''); + + $name = 'test'; + + foreach (['hr' => '/foo', 'en' => '/bar'] as $locale => $path) { + $localizedRoute = clone $route; + $localizedRoute->setDefault('_locale', $locale); + $localizedRoute->setDefault('_canonical_route', $name); + $localizedRoute->setPath($path); + $routes->add($name.'.'.$locale, $localizedRoute); + } + + $generator = $this->getGenerator($routes, [], null, 'fr'); + $generator->generate($name); + } + /** * @expectedException \Symfony\Component\Routing\Exception\MissingMandatoryParametersException */ @@ -720,7 +819,7 @@ class UrlGeneratorTest extends TestCase yield ['/app.php/bar/a/b/bam/c/d/e', '/bar/{foo}/bam/{baz}', '(? $value) { @@ -728,7 +827,7 @@ class UrlGeneratorTest extends TestCase $context->$method($value); } - return new UrlGenerator($routes, $context, $logger); + return new UrlGenerator($routes, $context, $logger, $defaultLocale); } protected function getRoutes($name, Route $route)