merged branch fabpot/global_route_settings (PR #326)

Commits
-------

eaf176d3 added a Route class
e3ddad54 added the possibility for settings set on a controller collection to influence controllers register later on
0356873a changed Controller::generateRouteName() visibility
31652440 added a way to define settings for all routes of a collection (closes #186)

Discussion
----------

added a way to define settings for all routes of a collection (closes #186)

see the ticket and docs from the patch.

---------------------------------------------------------------------------

by fabpot at 2012-05-21T13:00:13Z

ok, I've changed the strategy to what I've described above. ping @igorw

---------------------------------------------------------------------------

by igorw at 2012-05-21T16:09:56Z

`ControllerCollection extends Controller` is weird. I see how some code can be re-used, but a controller cannot be frozen or named, so those parts don't really make sense to me.

Apart from that it's a nice feature. If you can come up with a cleaner way to do it that doesn't bloat the code too much, that'd be nice.

---------------------------------------------------------------------------

by fabpot at 2012-05-21T16:30:16Z

What about creating a `Route` class that extends the Symfony `Route` class? It would contain the `assert`, `value`, `requireHttp`, `requireHttps`, `convert`, and `middleware` methods. Then, `Controller` and `ControllerCollection` extend this new `Route` class.

---------------------------------------------------------------------------

by fabpot at 2012-05-21T16:33:23Z

Extending the `Route` is not necessarily a good idea either. What about creating this `Route` class and compose it into both `Controller` and `ControllerCollection`?

---------------------------------------------------------------------------

by fabpot at 2012-05-21T16:33:50Z

That's what my last commit does.

---------------------------------------------------------------------------

by fabpot at 2012-05-21T16:36:34Z

By the way, a Trait would probably have been the best solution.

doc/changelog.rst

@@ -3,6 +3,8 @@ Changelog
 
 This changelog references all backward incompatibilities as we introduce them:
 
+* **2012-05-20**: Added a way to define settings on a controller collection.
+
 * **2012-05-20**: The Request instance is not available anymore from the
   Application after it has been handled.
 

doc/usage.rst

@@ -43,7 +43,7 @@ Routing
 -------
 
 In Silex you define a route and the controller that is called when that
-route is matched
+route is matched.
 
 A route pattern consists of:
 
@@ -433,6 +433,25 @@ redirect response, which you can create by calling the Application
 If a route middleware does not return a Symfony HTTP Response or ``null``, a
 ``RuntimeException`` is thrown.
 
+Global Configuration
+--------------------
+
+If a controller setting must be applied to all controllers (a converter, a
+middleware, a requirement, or a default value), you can configure it on
+``$app['controllers']``, which holds all application controllers::
+
+    $app['controllers']
+        ->value('id', '1')
+        ->assert('id', '\d+')
+        ->requireHttps()
+        ->method('get')
+        ->convert('id', function () { // ... })
+        ->middleware(function () { // ... })
+    ;
+
+These settings are applied to already registered controllers and they become
+the defaults for new controllers.
+
 Error handlers
 --------------
 
@@ -575,7 +594,16 @@ blog home page, and ``/forum/`` to the forum home page.
 
     When calling ``get()``, ``match()``, or any other HTTP methods on the
     Application, you are in fact calling them on a default instance of
-    ``ControllerCollection``.
+    ``ControllerCollection`` (stored in ``$app['controllers']``).
+
+Another benefit is the ability to apply settings on a set of controllers very
+easily. Building on the example from the middleware section, here is how you
+would secure all controllers for the backend collection::
+
+    $backend = new ControllerCollection();
+
+    // ensure that all controllers require logged-in users
+    $backend->middleware($mustBeLogged);
 
 .. tip::
 

src/Silex/Controller.php

@@ -12,7 +12,6 @@
 namespace Silex;
 
 use Silex\Exception\ControllerFrozenException;
-use Symfony\Component\Routing\Route;
 
 /**
  * A wrapper for a controller, mapped to a route.
@@ -83,7 +82,7 @@ class Controller
      */
     public function assert($variable, $regexp)
     {
-        $this->route->setRequirement($variable, $regexp);
+        $this->route->assert($variable, $regexp);
 
         return $this;
     }
@@ -98,7 +97,7 @@ class Controller
      */
     public function value($variable, $default)
     {
-        $this->route->setDefault($variable, $default);
+        $this->route->value($variable, $default);
 
         return $this;
     }
@@ -113,9 +112,7 @@ class Controller
      */
     public function convert($variable, $callback)
     {
-        $converters = $this->route->getOption('_converters');
-        $converters[$variable] = $callback;
-        $this->route->setOption('_converters', $converters);
+        $this->route->convert($variable, $callback);
 
         return $this;
     }
@@ -129,7 +126,7 @@ class Controller
      */
     public function method($method)
     {
-        $this->route->setRequirement('_method', $method);
+        $this->route->method($method);
 
         return $this;
     }
@@ -141,7 +138,7 @@ class Controller
      */
     public function requireHttp()
     {
-        $this->route->setRequirement('_scheme', 'http');
+        $this->route->requireHttp();
 
         return $this;
     }
@@ -153,7 +150,7 @@ class Controller
      */
     public function requireHttps()
     {
-        $this->route->setRequirement('_scheme', 'https');
+        $this->route->requireHttps();
 
         return $this;
     }
@@ -168,9 +165,7 @@ class Controller
      */
     public function middleware($callback)
     {
-        $middlewareCallbacks = $this->route->getOption('_middlewares');
-        $middlewareCallbacks[] = $callback;
-        $this->route->setOption('_middlewares', $middlewareCallbacks);
+        $this->route->middleware($callback);
 
         return $this;
     }

src/Silex/ControllerCollection.php

@@ -12,7 +12,6 @@
 namespace Silex;
 
 use Symfony\Component\Routing\RouteCollection;
-use Symfony\Component\Routing\Route;
 use Silex\Controller;
 
 /**
@@ -28,6 +27,17 @@ use Silex\Controller;
 class ControllerCollection
 {
     protected $controllers = array();
+    protected $defaultRoute;
+
+    /**
+     * Constructor.
+     *
+     * @param Route $route
+     */
+    public function __construct()
+    {
+        $this->defaultRoute = new Route('');
+    }
 
     /**
      * Maps a pattern to a callable.
@@ -41,9 +51,11 @@ class ControllerCollection
      */
     public function match($pattern, $to)
     {
-        $route = new Route($pattern, array('_controller' => $to));
-        $controller = new Controller($route);
-        $this->add($controller);
+        $route = clone $this->defaultRoute;
+        $route->setPattern($pattern);
+        $route->setDefault('_controller', $to);
+
+        $this->controllers[] = $controller = new Controller($route);
 
         return $controller;
     }
@@ -101,13 +113,129 @@ class ControllerCollection
     }
 
     /**
-     * Adds a controller to the staging area.
+     * Sets the requirement for a route variable.
+     *
+     * @param string $variable The variable name
+     * @param string $regexp   The regexp to apply
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function assert($variable, $regexp)
+    {
+        $this->defaultRoute->assert($variable, $regexp);
+
+        foreach ($this->controllers as $controller) {
+            $controller->assert($variable, $regexp);
+        }
+
+        return $this;
+    }
+
+    /**
+     * Sets the default value for a route variable.
+     *
+     * @param string $variable The variable name
+     * @param mixed  $default  The default value
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function value($variable, $default)
+    {
+        $this->defaultRoute->value($variable, $default);
+
+        foreach ($this->controllers as $controller) {
+            $controller->value($variable, $default);
+        }
+
+        return $this;
+    }
+
+    /**
+     * Sets a converter for a route variable.
+     *
+     * @param string $variable The variable name
+     * @param mixed  $callback A PHP callback that converts the original value
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function convert($variable, $callback)
+    {
+        $this->defaultRoute->convert($variable, $callback);
+
+        foreach ($this->controllers as $controller) {
+            $controller->convert($variable, $callback);
+        }
+
+        return $this;
+    }
+
+    /**
+     * Sets the requirement for the HTTP method.
+     *
+     * @param string $method The HTTP method name. Multiple methods can be supplied, delimited by a pipe character '|', eg. 'GET|POST'
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function method($method)
+    {
+        $this->defaultRoute->method($method);
+
+        foreach ($this->controllers as $controller) {
+            $controller->method($method);
+        }
+
+        return $this;
+    }
+
+    /**
+     * Sets the requirement of HTTP (no HTTPS) on this controller.
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function requireHttp()
+    {
+        $this->defaultRoute->requireHttp();
+
+        foreach ($this->controllers as $controller) {
+            $controller->requireHttp();
+        }
+
+        return $this;
+    }
+
+    /**
+     * Sets the requirement of HTTPS on this controller.
      *
-     * @param Controller $controller
+     * @return Controller $this The current Controller instance
      */
-    public function add(Controller $controller)
+    public function requireHttps()
     {
-        $this->controllers[] = $controller;
+        $this->defaultRoute->requireHttps();
+
+        foreach ($this->controllers as $controller) {
+            $controller->requireHttps();
+        }
+
+        return $this;
+    }
+
+    /**
+     * Sets a callback to handle before triggering the route callback.
+     * (a.k.a. "Route Middleware")
+     *
+     * @param mixed $callback A PHP callback to be triggered when the Route is matched, just before the route callback
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function middleware($callback)
+    {
+        $this->defaultRoute->middleware($callback);
+
+        foreach ($this->controllers as $controller) {
+            $controller->middleware($callback);
+        }
+
+        return $this;
     }
 
     /**

src/Silex/Route.php

+<?php
+
+/*
+ * This file is part of the Silex framework.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Silex;
+
+use Symfony\Component\Routing\Route as BaseRoute;
+
+/**
+ * A wrapper for a controller, mapped to a route.
+ *
+ * @author Fabien Potencier <fabien@symfony.com>
+ */
+class Route extends BaseRoute
+{
+    /**
+     * Sets the requirement for a route variable.
+     *
+     * @param string $variable The variable name
+     * @param string $regexp   The regexp to apply
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function assert($variable, $regexp)
+    {
+        $this->setRequirement($variable, $regexp);
+
+        return $this;
+    }
+
+    /**
+     * Sets the default value for a route variable.
+     *
+     * @param string $variable The variable name
+     * @param mixed  $default  The default value
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function value($variable, $default)
+    {
+        $this->setDefault($variable, $default);
+
+        return $this;
+    }
+
+    /**
+     * Sets a converter for a route variable.
+     *
+     * @param string $variable The variable name
+     * @param mixed  $callback A PHP callback that converts the original value
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function convert($variable, $callback)
+    {
+        $converters = $this->getOption('_converters');
+        $converters[$variable] = $callback;
+        $this->setOption('_converters', $converters);
+
+        return $this;
+    }
+
+    /**
+     * Sets the requirement for the HTTP method.
+     *
+     * @param string $method The HTTP method name. Multiple methods can be supplied, delimited by a pipe character '|', eg. 'GET|POST'
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function method($method)
+    {
+        $this->setRequirement('_method', $method);
+
+        return $this;
+    }
+
+    /**
+     * Sets the requirement of HTTP (no HTTPS) on this controller.
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function requireHttp()
+    {
+        $this->setRequirement('_scheme', 'http');
+
+        return $this;
+    }
+
+    /**
+     * Sets the requirement of HTTPS on this controller.
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function requireHttps()
+    {
+        $this->setRequirement('_scheme', 'https');
+
+        return $this;
+    }
+
+    /**
+     * Sets a callback to handle before triggering the route callback.
+     * (a.k.a. "Route Middleware")
+     *
+     * @param mixed $callback A PHP callback to be triggered when the Route is matched, just before the route callback
+     *
+     * @return Controller $this The current Controller instance
+     */
+    public function middleware($callback)
+    {
+        $middlewareCallbacks = $this->getOption('_middlewares');
+        $middlewareCallbacks[] = $callback;
+        $this->setOption('_middlewares', $middlewareCallbacks);
+
+        return $this;
+    }
+}

tests/Silex/Tests/ControllerCollectionTest.php

@@ -14,8 +14,7 @@ namespace Silex\Tests;
 use Silex\Controller;
 use Silex\ControllerCollection;
 use Silex\Exception\ControllerFrozenException;
-
-use Symfony\Component\Routing\Route;
+use Silex\Route;
 
 /**
  * ControllerCollection test cases.
@@ -34,8 +33,8 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
     public function testGetRouteCollectionWithRoutes()
     {
         $controllers = new ControllerCollection();
-        $controllers->add(new Controller(new Route('/foo')));
-        $controllers->add(new Controller(new Route('/bar')));
+        $controllers->match('/foo', function () {});
+        $controllers->match('/bar', function () {});
 
         $routes = $controllers->flush();
         $this->assertEquals(2, count($routes->all()));
@@ -45,13 +44,8 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
     {
         $controllers = new ControllerCollection();
 
-        $fooController = new Controller(new Route('/foo'));
-        $fooController->bind('foo');
-        $controllers->add($fooController);
-
-        $barController = new Controller(new Route('/bar'));
-        $barController->bind('bar');
-        $controllers->add($barController);
+        $fooController = $controllers->match('/foo', function () {})->bind('foo');
+        $barController = $controllers->match('/bar', function () {})->bind('bar');
 
         $controllers->flush();
 
@@ -72,8 +66,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
     {
         $controllers = new ControllerCollection();
 
-        $mountedRootController = new Controller(new Route('/'));
-        $controllers->add($mountedRootController);
+        $mountedRootController = $controllers->match('/', function () {});
 
         $mainRootController = new Controller(new Route('/'));
         $mainRootController->bind($mainRootController->generateRouteName('main_'));
@@ -87,11 +80,69 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
     {
         $controllers = new ControllerCollection();
 
-        $controllers->add(new Controller(new Route('/a-a')));
-        $controllers->add(new Controller(new Route('/a_a')));
+        $controllers->match('/a-a', function () {});
+        $controllers->match('/a_a', function () {});
+
         $routes = $controllers->flush();
 
         $this->assertCount(2, $routes->all());
         $this->assertEquals(array('_a_a', '_a_a_'), array_keys($routes->all()));
     }
+
+    public function testAssert()
+    {
+        $controllers = new ControllerCollection();
+        $controllers->assert('id', '\d+');
+        $controller = $controllers->match('/{id}/{name}/{extra}', function () {})->assert('name', '\w+')->assert('extra', '.*');
+        $controllers->assert('extra', '\w+');
+
+        $this->assertEquals('\d+', $controller->getRoute()->getRequirement('id'));
+        $this->assertEquals('\w+', $controller->getRoute()->getRequirement('name'));
+        $this->assertEquals('\w+', $controller->getRoute()->getRequirement('extra'));
+    }
+
+    public function testValue()
+    {
+        $controllers = new ControllerCollection();
+        $controllers->value('id', '1');
+        $controller = $controllers->match('/{id}/{name}/{extra}', function () {})->value('name', 'Fabien')->value('extra', 'Symfony');
+        $controllers->value('extra', 'Twig');
+
+        $this->assertEquals('1', $controller->getRoute()->getDefault('id'));
+        $this->assertEquals('Fabien', $controller->getRoute()->getDefault('name'));
+        $this->assertEquals('Twig', $controller->getRoute()->getDefault('extra'));
+    }
+
+    public function testConvert()
+    {
+        $controllers = new ControllerCollection();
+        $controllers->convert('id', '1');
+        $controller = $controllers->match('/{id}/{name}/{extra}', function () {})->convert('name', 'Fabien')->convert('extra', 'Symfony');
+        $controllers->convert('extra', 'Twig');
+
+        $this->assertEquals(array('id' => '1', 'name' => 'Fabien', 'extra' => 'Twig'), $controller->getRoute()->getOption('_converters'));
+    }
+
+    public function testRequireHttp()
+    {
+        $controllers = new ControllerCollection();
+        $controllers->requireHttp();
+        $controller = $controllers->match('/{id}/{name}/{extra}', function () {})->requireHttps();
+
+        $this->assertEquals('https', $controller->getRoute()->getRequirement('_scheme'));
+
+        $controllers->requireHttp();
+
+        $this->assertEquals('http', $controller->getRoute()->getRequirement('_scheme'));
+    }
+
+    public function testMiddleware()
+    {
+        $controllers = new ControllerCollection();
+        $controllers->middleware('mid1');
+        $controller = $controllers->match('/{id}/{name}/{extra}', function () {})->middleware('mid2');
+        $controllers->middleware('mid3');
+
+        $this->assertEquals(array('mid1', 'mid2', 'mid3'), $controller->getRoute()->getOption('_middlewares'));
+    }
 }

tests/Silex/Tests/ControllerTest.php

@@ -12,8 +12,7 @@
 namespace Silex\Tests;
 
 use Silex\Controller;
-
-use Symfony\Component\Routing\Route;
+use Silex\Route;
 
 /**
  * Controller test cases.