bug #31023 [Routing] Fix route URL generation in CLI context (X-Coder…

…264) This PR was squashed before being merged into the 4.2 branch (closes #31023). Discussion ---------- [Routing] Fix route URL generation in CLI context | Q | A | ------------- | --- | Branch? | 4.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30996 | License | MIT | Doc PR | - This fixes #30996 and makes URL generation in the CLI context behave the same as it does in the web context where the `LocaleListener` sets the default locale (to the router context). The Travis CI failure is related to the fact that the constraint for `symfony/routing` should be bumped to `^4.2.6` in the composer.json of the FrameworkBundle (when it gets tagged). Commits ------- 4a1ad4a [Routing] Fix route URL generation in CLI context
symfony · Apr 22, 2019 · f50ffa9 · f50ffa9
2 parents 097c229 + 4a1ad4a
commit f50ffa9
Show file tree

Hide file tree

Showing 6 changed files with 141 additions and 7 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml
@@ -71,6 +71,7 @@
             <argument type="service" id="router.request_context" on-invalid="ignore" />
             <argument type="service" id="parameter_bag" on-invalid="ignore" />
             <argument type="service" id="logger" on-invalid="ignore" />
+            <argument>%kernel.default_locale%</argument>
             <call method="setConfigCacheFactory">
                 <argument type="service" id="config_cache_factory" />
             </call>

diff --git 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 @@ public function __construct(ContainerInterface $container, $resource, array $opt
         } 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
@@ -82,6 +82,32 @@ public function testGenerateWithServiceParamWithSfContainer()
         $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
@@ -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
@@ -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 @@ public function getGenerator()
         }
 
         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 @@ function (ConfigCacheInterface $cache) {
                 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
@@ -162,6 +162,82 @@ public function testGlobalParameterHasHigherPriorityThanDefault()
         $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 @@ public function testGenerateWithoutRoutes()
         $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,15 +819,15 @@ public function provideLookAroundRequirementsInPath()
         yield ['/app.php/bar/a/b/bam/c/d/e', '/bar/{foo}/bam/{baz}', '(?<!^).+'];
     }
 
-    protected function getGenerator(RouteCollection $routes, array $parameters = [], $logger = null)
+    protected function getGenerator(RouteCollection $routes, array $parameters = [], $logger = null, string $defaultLocale = null)
     {
         $context = new RequestContext('/app.php');
         foreach ($parameters as $key => $value) {
             $method = 'set'.$key;
             $context->$method($value);
         }
 
-        return new UrlGenerator($routes, $context, $logger);
+        return new UrlGenerator($routes, $context, $logger, $defaultLocale);
     }
 
     protected function getRoutes($name, Route $route)