feature symfony#23227 Add support for "controller" keyword for configuring routes controllers (voronkovich)

nicolas-grekas · nicolas-grekas · commit 8c4a1e75bee9 · 2017-08-16T18:33:30.000+02:00
This PR was merged into the 3.4 branch. Discussion ---------- Add support for "controller" keyword for configuring routes controllers | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT This PR adds a syntax sugar for configuring routes controllers in more user-friendly way by using a `controller` keyword. ```yaml # Current syntax # It looks strange and exposes Symfony's internals blog_show: path: /blog/{slug} defaults: { _controller: AppBundle:Blog:show } # Suggested syntax blog_show: path: /blog/{slug} controller: AppBundle:Blog:show ``` ```xml <?xml version="1.0" encoding="UTF-8" ?> <routes xmlns="http://symfony.com/schema/routing" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">  <route id="blog_list" path="/blog"> <default key="_controller">AppBundle:Blog:list</default> </route>  <route id="blog_list" path="/blog" controller="AppBundle:Blog:list" /> </routes> ``` Commits ------- 06bbee8 [Routing] Use "controller" keyword for configuring routes controllers
diff --git a/src/Symfony/Component/Routing/CHANGELOG.md b/src/Symfony/Component/Routing/CHANGELOG.md
@@ -6,6 +6,7 @@ CHANGELOG
 
  * Added support for prioritized routing loaders.
  * Add matched and default parameters to redirect responses
+ * Added support for a `controller` keyword for configuring route controllers in YAML and XML configurations.
 
 3.3.0
 -----
diff --git a/src/Symfony/Component/Routing/Loader/XmlFileLoader.php b/src/Symfony/Component/Routing/Loader/XmlFileLoader.php
@@ -229,6 +229,16 @@ private function parseConfigs(\DOMElement $node, $path)
             }
         }
 
+        if ($controller = $node->getAttribute('controller')) {
+            if (isset($defaults['_controller'])) {
+                $name = $node->hasAttribute('id') ? sprintf('"%s"', $node->getAttribute('id')) : sprintf('the "%s" tag', $node->tagName);
+
+                throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both the "controller" attribute and the defaults key "_controller" for %s.', $path, $name));
+            }
+
+            $defaults['_controller'] = $controller;
+        }
+
         return array($defaults, $requirements, $options, $condition);
     }
 
diff --git a/src/Symfony/Component/Routing/Loader/YamlFileLoader.php b/src/Symfony/Component/Routing/Loader/YamlFileLoader.php
@@ -27,7 +27,7 @@
 class YamlFileLoader extends FileLoader
 {
     private static $availableKeys = array(
-        'resource', 'type', 'prefix', 'path', 'host', 'schemes', 'methods', 'defaults', 'requirements', 'options', 'condition',
+        'resource', 'type', 'prefix', 'path', 'host', 'schemes', 'methods', 'defaults', 'requirements', 'options', 'condition', 'controller',
     );
     private $yamlParser;
 
@@ -115,6 +115,10 @@ protected function parseRoute(RouteCollection $collection, $name, array $config,
         $methods = isset($config['methods']) ? $config['methods'] : array();
         $condition = isset($config['condition']) ? $config['condition'] : null;
 
+        if (isset($config['controller'])) {
+            $defaults['_controller'] = $config['controller'];
+        }
+
         $route = new Route($config['path'], $defaults, $requirements, $options, $host, $schemes, $methods, $condition);
 
         $collection->add($name, $route);
@@ -140,6 +144,10 @@ protected function parseImport(RouteCollection $collection, array $config, $path
         $schemes = isset($config['schemes']) ? $config['schemes'] : null;
         $methods = isset($config['methods']) ? $config['methods'] : null;
 
+        if (isset($config['controller'])) {
+            $defaults['_controller'] = $config['controller'];
+        }
+
         $this->setCurrentDir(dirname($path));
 
         $subCollection = $this->import($config['resource'], $type, false, $file);
@@ -203,5 +211,8 @@ protected function validate($config, $name, $path)
                 $name, $path
             ));
         }
+        if (isset($config['controller']) && isset($config['defaults']['_controller'])) {
+            throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both the "controller" key and the defaults key "_controller" for "%s".', $path, $name));
+        }
     }
 }
diff --git a/src/Symfony/Component/Routing/Loader/schema/routing/routing-1.0.xsd b/src/Symfony/Component/Routing/Loader/schema/routing/routing-1.0.xsd
@@ -41,6 +41,7 @@
     <xsd:attribute name="host" type="xsd:string" />
     <xsd:attribute name="schemes" type="xsd:string" />
     <xsd:attribute name="methods" type="xsd:string" />
+    <xsd:attribute name="controller" type="xsd:string" />
   </xsd:complexType>
 
   <xsd:complexType name="import">
@@ -52,6 +53,7 @@
     <xsd:attribute name="host" type="xsd:string" />
     <xsd:attribute name="schemes" type="xsd:string" />
     <xsd:attribute name="methods" type="xsd:string" />
+    <xsd:attribute name="controller" type="xsd:string" />
   </xsd:complexType>
 
   <xsd:complexType name="default" mixed="true">
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/import__controller.xml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/import__controller.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="routing.xml">
+        <default key="_controller">FrameworkBundle:Template:template</default>
+    </import>
+</routes>
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/import__controller.yml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/import__controller.yml
@@ -0,0 +1,4 @@
+_static:
+    resource: routing.yml
+    defaults:
+        _controller: FrameworkBundle:Template:template
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_controller.xml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_controller.xml
@@ -0,0 +1,8 @@
+<?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="routing.xml" controller="FrameworkBundle:Template:template" />
+</routes>
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_controller.yml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_controller.yml
@@ -0,0 +1,3 @@
+_static:
+    resource: routing.yml
+    controller: FrameworkBundle:Template:template
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_override_defaults.xml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_override_defaults.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="routing.xml" controller="FrameworkBundle:Template:template">
+        <default key="_controller">AppBundle:Blog:index</default>
+    </import>
+</routes>
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_override_defaults.yml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/import_override_defaults.yml
@@ -0,0 +1,5 @@
+_static:
+    resource: routing.yml
+    controller: FrameworkBundle:Template:template
+    defaults:
+        _controller: AppBundle:Homepage:show
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/override_defaults.xml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/override_defaults.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">
+
+    <route id="app_blog" path="/blog" controller="AppBundle:Homepage:show">
+        <default key="_controller">AppBundle:Blog:index</default>
+    </route>
+</routes>
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/override_defaults.yml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/override_defaults.yml
@@ -0,0 +1,5 @@
+app_blog:
+    path: /blog
+    controller: AppBundle:Homepage:show
+    defaults:
+        _controller: AppBundle:Blog:index
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/routing.xml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/routing.xml
@@ -0,0 +1,14 @@
+<?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">
+
+    <route id="app_homepage" path="/" controller="AppBundle:Homepage:show" />
+
+    <route id="app_blog" path="/blog">
+        <default key="_controller">AppBundle:Blog:list</default>
+    </route>
+
+    <route id="app_logout" path="/logout" />
+</routes>
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/controller/routing.yml b/src/Symfony/Component/Routing/Tests/Fixtures/controller/routing.yml
@@ -0,0 +1,11 @@
+app_homepage:
+    path: /
+    controller: AppBundle:Homepage:show
+
+app_blog:
+    path: /blog
+    defaults:
+        _controller: AppBundle:Blog:list
+
+app_logout:
+    path: /logout
diff --git a/src/Symfony/Component/Routing/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/XmlFileLoaderTest.php
@@ -287,4 +287,78 @@ public function testNullValuesInMap()
             $route->getDefault('map')
         );
     }
+
+    public function testLoadRouteWithControllerAttribute()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load('routing.xml');
+
+        $route = $routeCollection->get('app_homepage');
+
+        $this->assertSame('AppBundle:Homepage:show', $route->getDefault('_controller'));
+    }
+
+    public function testLoadRouteWithoutControllerAttribute()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load('routing.xml');
+
+        $route = $routeCollection->get('app_logout');
+
+        $this->assertNull($route->getDefault('_controller'));
+    }
+
+    public function testLoadRouteWithControllerSetInDefaults()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load('routing.xml');
+
+        $route = $routeCollection->get('app_blog');
+
+        $this->assertSame('AppBundle:Blog:list', $route->getDefault('_controller'));
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The routing file "[^"]*" must not specify both the "controller" attribute and the defaults key "_controller" for "app_blog"/
+     */
+    public function testOverrideControllerInDefaults()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $loader->load('override_defaults.xml');
+    }
+
+    /**
+     * @dataProvider provideFilesImportingRoutesWithControllers
+     */
+    public function testImportRouteWithController($file)
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load($file);
+
+        $route = $routeCollection->get('app_homepage');
+        $this->assertSame('FrameworkBundle:Template:template', $route->getDefault('_controller'));
+
+        $route = $routeCollection->get('app_blog');
+        $this->assertSame('FrameworkBundle:Template:template', $route->getDefault('_controller'));
+
+        $route = $routeCollection->get('app_logout');
+        $this->assertSame('FrameworkBundle:Template:template', $route->getDefault('_controller'));
+    }
+
+    public function provideFilesImportingRoutesWithControllers()
+    {
+        yield array('import_controller.xml');
+        yield array('import__controller.xml');
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The routing file "[^"]*" must not specify both the "controller" attribute and the defaults key "_controller" for the "import" tag/
+     */
+    public function testImportWithOverriddenController()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $loader->load('import_override_defaults.xml');
+    }
 }
diff --git a/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php
@@ -108,4 +108,78 @@ public function testLoadWithResource()
             $this->assertSame('context.getMethod() == "POST"', $route->getCondition());
         }
     }
+
+    public function testLoadRouteWithControllerAttribute()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load('routing.yml');
+
+        $route = $routeCollection->get('app_homepage');
+
+        $this->assertSame('AppBundle:Homepage:show', $route->getDefault('_controller'));
+    }
+
+    public function testLoadRouteWithoutControllerAttribute()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load('routing.yml');
+
+        $route = $routeCollection->get('app_logout');
+
+        $this->assertNull($route->getDefault('_controller'));
+    }
+
+    public function testLoadRouteWithControllerSetInDefaults()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load('routing.yml');
+
+        $route = $routeCollection->get('app_blog');
+
+        $this->assertSame('AppBundle:Blog:list', $route->getDefault('_controller'));
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The routing file "[^"]*" must not specify both the "controller" key and the defaults key "_controller" for "app_blog"/
+     */
+    public function testOverrideControllerInDefaults()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $loader->load('override_defaults.yml');
+    }
+
+    /**
+     * @dataProvider provideFilesImportingRoutesWithControllers
+     */
+    public function testImportRouteWithController($file)
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $routeCollection = $loader->load($file);
+
+        $route = $routeCollection->get('app_homepage');
+        $this->assertSame('FrameworkBundle:Template:template', $route->getDefault('_controller'));
+
+        $route = $routeCollection->get('app_blog');
+        $this->assertSame('FrameworkBundle:Template:template', $route->getDefault('_controller'));
+
+        $route = $routeCollection->get('app_logout');
+        $this->assertSame('FrameworkBundle:Template:template', $route->getDefault('_controller'));
+    }
+
+    public function provideFilesImportingRoutesWithControllers()
+    {
+        yield array('import_controller.yml');
+        yield array('import__controller.yml');
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The routing file "[^"]*" must not specify both the "controller" key and the defaults key "_controller" for "_static"/
+     */
+    public function testImportWithOverriddenController()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/controller')));
+        $loader->load('import_override_defaults.yml');
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -229,6 +229,16 @@ private function parseConfigs(\DOMElement $node, $path)`
`229`	`229`	`}`
`230`	`230`	`}`
`231`	`231`
	`232`	`+ if ($controller = $node->getAttribute('controller')) {`
	`233`	`+ if (isset($defaults['_controller'])) {`
	`234`	`+ $name = $node->hasAttribute('id') ? sprintf('"%s"', $node->getAttribute('id')) : sprintf('the "%s" tag', $node->tagName);`
	`235`	`+`
	`236`	`+ throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both the "controller" attribute and the defaults key "_controller" for %s.', $path, $name));`
	`237`	`+ }`
	`238`	`+`
	`239`	`+ $defaults['_controller'] = $controller;`
	`240`	`+ }`
	`241`	`+`
`232`	`242`	`return array($defaults, $requirements, $options, $condition);`
`233`	`243`	`}`
`234`	`244`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+_static:`
	`2`	`+ resource: routing.yml`
	`3`	`+ controller: FrameworkBundle:Template:template`