[Routing] allow no-slash root on imported routes

nicolas-grekas · nicolas-grekas · commit e57e94eb81fd · 2018-03-21T16:28:08.000+01:00
diff --git a/Loader/Configurator/ImportConfigurator.php b/Loader/Configurator/ImportConfigurator.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Routing\Loader\Configurator;
 
+use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;
 
 /**
@@ -40,16 +41,18 @@ public function __destruct()
      *
      * @return $this
      */
-    final public function prefix($prefix, string $namePrefix = '')
+    final public function prefix($prefix, bool $trailingSlashOnRoot = true)
     {
-        if ('' !== $namePrefix) {
-            $this->route->addNamePrefix($namePrefix);
-        }
-        if (!$prefix) {
-            return $this;
-        }
         if (!\is_array($prefix)) {
             $this->route->addPrefix($prefix);
+            if (!$trailingSlashOnRoot) {
+                $rootPath = (new Route(trim(trim($prefix), '/').'/'))->getPath();
+                foreach ($this->route->all() as $route) {
+                    if ($route->getPath() === $rootPath) {
+                        $route->setPath(rtrim($rootPath, '/'));
+                    }
+                }
+            }
         } else {
             foreach ($prefix as $locale => $localePrefix) {
                 $prefix[$locale] = trim(trim($localePrefix), '/');
@@ -61,18 +64,30 @@ final public function prefix($prefix, string $namePrefix = '')
                         $localizedRoute = clone $route;
                         $localizedRoute->setDefault('_locale', $locale);
                         $localizedRoute->setDefault('_canonical_route', $name);
-                        $localizedRoute->setPath($localePrefix.$route->getPath());
+                        $localizedRoute->setPath($localePrefix.(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
                         $this->route->add($name.'.'.$locale, $localizedRoute);
                     }
                 } elseif (!isset($prefix[$locale])) {
                     throw new \InvalidArgumentException(sprintf('Route "%s" with locale "%s" is missing a corresponding prefix in its parent collection.', $name, $locale));
                 } else {
-                    $route->setPath($prefix[$locale].$route->getPath());
+                    $route->setPath($prefix[$locale].(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
                     $this->route->add($name, $route);
                 }
             }
         }
 
         return $this;
     }
+
+    /**
+     * Sets the prefix to add to the name of all child routes.
+     *
+     * @return $this
+     */
+    final public function namePrefix(string $namePrefix)
+    {
+        $this->route->addNamePrefix($namePrefix);
+
+        return $this;
+    }
 }
diff --git a/Loader/XmlFileLoader.php b/Loader/XmlFileLoader.php
@@ -158,6 +158,7 @@ protected function parseImport(RouteCollection $collection, \DOMElement $node, $
         $host = $node->hasAttribute('host') ? $node->getAttribute('host') : null;
         $schemes = $node->hasAttribute('schemes') ? preg_split('/[\s,\|]++/', $node->getAttribute('schemes'), -1, PREG_SPLIT_NO_EMPTY) : null;
         $methods = $node->hasAttribute('methods') ? preg_split('/[\s,\|]++/', $node->getAttribute('methods'), -1, PREG_SPLIT_NO_EMPTY) : null;
+        $trailingSlashOnRoot = $node->hasAttribute('trailing-slash-on-root') ? XmlUtils::phpize($node->getAttribute('trailing-slash-on-root')) : true;
 
         list($defaults, $requirements, $options, $condition, /* $paths */, $prefixes) = $this->parseConfigs($node, $path);
 
@@ -172,6 +173,14 @@ protected function parseImport(RouteCollection $collection, \DOMElement $node, $
 
         if ('' !== $prefix || !$prefixes) {
             $subCollection->addPrefix($prefix);
+            if (!$trailingSlashOnRoot) {
+                $rootPath = (new Route(trim(trim($prefix), '/').'/'))->getPath();
+                foreach ($subCollection->all() as $route) {
+                    if ($route->getPath() === $rootPath) {
+                        $route->setPath(rtrim($rootPath, '/'));
+                    }
+                }
+            }
         } else {
             foreach ($prefixes as $locale => $localePrefix) {
                 $prefixes[$locale] = trim(trim($localePrefix), '/');
@@ -181,15 +190,15 @@ protected function parseImport(RouteCollection $collection, \DOMElement $node, $
                     $subCollection->remove($name);
                     foreach ($prefixes as $locale => $localePrefix) {
                         $localizedRoute = clone $route;
-                        $localizedRoute->setPath($localePrefix.$route->getPath());
+                        $localizedRoute->setPath($localePrefix.(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
                         $localizedRoute->setDefault('_locale', $locale);
                         $localizedRoute->setDefault('_canonical_route', $name);
                         $subCollection->add($name.'.'.$locale, $localizedRoute);
                     }
                 } elseif (!isset($prefixes[$locale])) {
                     throw new \InvalidArgumentException(sprintf('Route "%s" with locale "%s" is missing a corresponding prefix when imported in "%s".', $name, $locale, $path));
                 } else {
-                    $route->setPath($prefixes[$locale].$route->getPath());
+                    $route->setPath($prefixes[$locale].(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
                     $subCollection->add($name, $route);
                 }
             }
diff --git a/Loader/YamlFileLoader.php b/Loader/YamlFileLoader.php
@@ -28,7 +28,7 @@
 class YamlFileLoader extends FileLoader
 {
     private static $availableKeys = array(
-        'resource', 'type', 'prefix', 'path', 'host', 'schemes', 'methods', 'defaults', 'requirements', 'options', 'condition', 'controller', 'name_prefix',
+        'resource', 'type', 'prefix', 'path', 'host', 'schemes', 'methods', 'defaults', 'requirements', 'options', 'condition', 'controller', 'name_prefix', 'trailing_slash_on_root',
     );
     private $yamlParser;
 
@@ -155,6 +155,7 @@ protected function parseImport(RouteCollection $collection, array $config, $path
         $condition = isset($config['condition']) ? $config['condition'] : null;
         $schemes = isset($config['schemes']) ? $config['schemes'] : null;
         $methods = isset($config['methods']) ? $config['methods'] : null;
+        $trailingSlashOnRoot = $config['trailing_slash_on_root'] ?? true;
 
         if (isset($config['controller'])) {
             $defaults['_controller'] = $config['controller'];
@@ -167,6 +168,14 @@ protected function parseImport(RouteCollection $collection, array $config, $path
 
         if (!\is_array($prefix)) {
             $subCollection->addPrefix($prefix);
+            if (!$trailingSlashOnRoot) {
+                $rootPath = (new Route(trim(trim($prefix), '/').'/'))->getPath();
+                foreach ($subCollection->all() as $route) {
+                    if ($route->getPath() === $rootPath) {
+                        $route->setPath(rtrim($rootPath, '/'));
+                    }
+                }
+            }
         } else {
             foreach ($prefix as $locale => $localePrefix) {
                 $prefix[$locale] = trim(trim($localePrefix), '/');
@@ -178,13 +187,13 @@ protected function parseImport(RouteCollection $collection, array $config, $path
                         $localizedRoute = clone $route;
                         $localizedRoute->setDefault('_locale', $locale);
                         $localizedRoute->setDefault('_canonical_route', $name);
-                        $localizedRoute->setPath($localePrefix.$route->getPath());
+                        $localizedRoute->setPath($localePrefix.(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
                         $subCollection->add($name.'.'.$locale, $localizedRoute);
                     }
                 } elseif (!isset($prefix[$locale])) {
                     throw new \InvalidArgumentException(sprintf('Route "%s" with locale "%s" is missing a corresponding prefix when imported in "%s".', $name, $locale, $file));
                 } else {
-                    $route->setPath($prefix[$locale].$route->getPath());
+                    $route->setPath($prefix[$locale].(!$trailingSlashOnRoot && '/' === $route->getPath() ? '' : $route->getPath()));
                     $subCollection->add($name, $route);
                 }
             }
diff --git a/Loader/schema/routing/routing-1.0.xsd b/Loader/schema/routing/routing-1.0.xsd
@@ -67,6 +67,7 @@
     <xsd:attribute name="schemes" type="xsd:string" />
     <xsd:attribute name="methods" type="xsd:string" />
     <xsd:attribute name="controller" type="xsd:string" />
+    <xsd:attribute name="trailing-slash-on-root" type="xsd:boolean" />
   </xsd:complexType>
 
   <xsd:complexType name="default" mixed="true">
diff --git a/Tests/Fixtures/import_with_no_trailing_slash/routing.xml b/Tests/Fixtures/import_with_no_trailing_slash/routing.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<routes xmlns="http://symfony.com/schema/routing"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://symfony.com/schema/routing
+        http://symfony.com/schema/routing/routing-1.0.xsd">
+
+    <import resource="../controller/routing.xml" prefix="/slash" name-prefix="a_" />
+    <import resource="../controller/routing.xml" prefix="/no-slash" name-prefix="b_" trailing-slash-on-root="false" />
+
+</routes>
diff --git a/Tests/Fixtures/import_with_no_trailing_slash/routing.yml b/Tests/Fixtures/import_with_no_trailing_slash/routing.yml
@@ -0,0 +1,10 @@
+app:
+    resource: ../controller/routing.yml
+    name_prefix: a_
+    prefix: /slash
+
+api:
+    resource: ../controller/routing.yml
+    name_prefix: b_
+    prefix: /no-slash
+    trailing_slash_on_root: false
diff --git a/Tests/Fixtures/php_dsl.php b/Tests/Fixtures/php_dsl.php
@@ -16,7 +16,11 @@
         ->requirements(array('id' => '\d+'));
 
     $routes->import('php_dsl_sub.php')
-        ->prefix('/zub', 'z_');
+        ->namePrefix('z_')
+        ->prefix('/zub');
+
+    $routes->import('php_dsl_sub_root.php')
+        ->prefix('/bus', false);
 
     $routes->add('ouf', '/ouf')
         ->schemes(array('https'))
diff --git a/Tests/Fixtures/php_dsl_sub.php b/Tests/Fixtures/php_dsl_sub.php
@@ -6,6 +6,7 @@
     $add = $routes->collection('c_')
         ->prefix('pub');
 
+    $add('root', '/');
     $add('bar', '/bar');
 
     $add->collection('pub_')
diff --git a/Tests/Fixtures/php_dsl_sub_root.php b/Tests/Fixtures/php_dsl_sub_root.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace Symfony\Component\Routing\Loader\Configurator;
+
+return function (RoutingConfigurator $routes) {
+    $add = $routes->collection('r_');
+
+    $add('root', '/');
+    $add('bar', '/bar/');
+};
diff --git a/Tests/Loader/PhpFileLoaderTest.php b/Tests/Loader/PhpFileLoaderTest.php
@@ -99,22 +99,29 @@ public function testRoutingConfigurator()
         $expectedCollection->add('buz', (new Route('/zub'))
             ->setDefaults(array('_controller' => 'foo:act'))
         );
+        $expectedCollection->add('c_root', (new Route('/sub/pub/'))
+            ->setRequirements(array('id' => '\d+'))
+        );
         $expectedCollection->add('c_bar', (new Route('/sub/pub/bar'))
             ->setRequirements(array('id' => '\d+'))
         );
         $expectedCollection->add('c_pub_buz', (new Route('/sub/pub/buz'))
             ->setHost('host')
             ->setRequirements(array('id' => '\d+'))
         );
+        $expectedCollection->add('z_c_root', new Route('/zub/pub/'));
         $expectedCollection->add('z_c_bar', new Route('/zub/pub/bar'));
         $expectedCollection->add('z_c_pub_buz', (new Route('/zub/pub/buz'))->setHost('host'));
+        $expectedCollection->add('r_root', new Route('/bus'));
+        $expectedCollection->add('r_bar', new Route('/bus/bar/'));
         $expectedCollection->add('ouf', (new Route('/ouf'))
             ->setSchemes(array('https'))
             ->setMethods(array('GET'))
             ->setDefaults(array('id' => 0))
         );
 
         $expectedCollection->addResource(new FileResource(realpath(__DIR__.'/../Fixtures/php_dsl_sub.php')));
+        $expectedCollection->addResource(new FileResource(realpath(__DIR__.'/../Fixtures/php_dsl_sub_root.php')));
         $expectedCollection->addResource(new FileResource(realpath(__DIR__.'/../Fixtures/php_dsl.php')));
 
         $this->assertEquals($expectedCollection, $routeCollection);
diff --git a/Tests/Loader/XmlFileLoaderTest.php b/Tests/Loader/XmlFileLoaderTest.php
@@ -411,4 +411,13 @@ public function testImportRouteWithNamePrefix()
         $this->assertNotNull($routeCollection->get('api_app_blog'));
         $this->assertEquals('/api/blog', $routeCollection->get('api_app_blog')->getPath());
     }
+
+    public function testImportRouteWithNoTrailingSlash()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/import_with_no_trailing_slash')));
+        $routeCollection = $loader->load('routing.xml');
+
+        $this->assertEquals('/slash/', $routeCollection->get('a_app_homepage')->getPath());
+        $this->assertEquals('/no-slash', $routeCollection->get('b_app_homepage')->getPath());
+    }
 }
diff --git a/Tests/Loader/YamlFileLoaderTest.php b/Tests/Loader/YamlFileLoaderTest.php
@@ -209,7 +209,6 @@ public function testLoadingLocalizedRoute()
         $this->assertCount(3, $routes);
     }
 
-
     public function testImportingRoutesFromDefinition()
     {
         $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/localized')));
@@ -275,4 +274,13 @@ public function testImportingWithControllerDefault()
         $this->assertEquals('DefaultController::defaultAction', $routes->get('home.nl')->getDefault('_controller'));
         $this->assertEquals('DefaultController::defaultAction', $routes->get('not_localized')->getDefault('_controller'));
     }
+
+    public function testImportRouteWithNoTrailingSlash()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/import_with_no_trailing_slash')));
+        $routeCollection = $loader->load('routing.yml');
+
+        $this->assertEquals('/slash/', $routeCollection->get('a_app_homepage')->getPath());
+        $this->assertEquals('/no-slash', $routeCollection->get('b_app_homepage')->getPath());
+    }
 }
