added the possibility for settings set on a controller collection to influence...

added the possibility for settings set on a controller collection to influence controllers register later on

doc/changelog.rst

@@ -3,7 +3,7 @@ Changelog
 
 This changelog references all backward incompatibilities as we introduce them:
 
-* **2012-05-20**: Added a way to define settings for all routes of a collection.
+* **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

@@ -436,9 +436,9 @@ If a route middleware does not return a Symfony HTTP Response or ``null``, a
 Global Configuration
 --------------------
 
-If a route setting must be applied to all routes (a converter, a middleware, a
-requirement, or a default value), you can configure it on
-``$app['controllers']``, which holds all controllers::
+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')
@@ -449,8 +449,8 @@ requirement, or a default value), you can configure it on
         ->middleware(function () { // ... })
     ;
 
-These settings can be set before or after controller definitions and settings
-defined on a controller always override the globally configured one.
+These settings are applied to already registered controllers and they become
+the defaults for new controllers.
 
 Error handlers
 --------------
@@ -596,12 +596,13 @@ blog home page, and ``/forum/`` to the forum home page.
     Application, you are in fact calling them on a default instance of
     ``ControllerCollection`` (stored in ``$app['controllers']``).
 
-Another benefit is the ability to apply default 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 all backend routes::
+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

@@ -21,11 +21,11 @@ use Symfony\Component\Routing\Route;
  */
 class Controller
 {
+    protected $route;
+
     private $routeName;
     private $isFrozen = false;
 
-    protected $route;
-
     /**
      * Constructor.
      *

src/Silex/ControllerCollection.php

@@ -51,9 +51,11 @@ class ControllerCollection extends Controller
      */
     public function match($pattern, $to)
     {
-        $route = new Route($pattern, array('_controller' => $to));
-        $controller = new Controller($route);
-        $this->add($controller);
+        $route = clone $this->route;
+        $route->setPattern($pattern);
+        $route->setDefault('_controller', $to);
+
+        $this->controllers[] = $controller = new Controller($route);
 
         return $controller;
     }
@@ -111,13 +113,101 @@ class ControllerCollection extends Controller
     }
 
     /**
-     * Adds a controller to the staging area.
-     *
-     * @param Controller $controller
+     * {@inheritDoc}
      */
-    public function add(Controller $controller)
+    public function assert($variable, $regexp)
     {
-        $this->controllers[] = $controller;
+        parent::assert($variable, $regexp);
+
+        foreach ($this->controllers as $controller) {
+            $controller->assert($variable, $regexp);
+        }
+
+        return $this;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function value($variable, $default)
+    {
+        parent::value($variable, $default);
+
+        foreach ($this->controllers as $controller) {
+            $controller->value($variable, $default);
+        }
+
+        return $this;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function convert($variable, $callback)
+    {
+        parent::convert($variable, $callback);
+
+        foreach ($this->controllers as $controller) {
+            $controller->convert($variable, $callback);
+        }
+
+        return $this;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function method($method)
+    {
+        parent::method($method);
+
+        foreach ($this->controllers as $controller) {
+            $controller->method($method);
+        }
+
+        return $this;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function requireHttp()
+    {
+        parent::requireHttp();
+
+        foreach ($this->controllers as $controller) {
+            $controller->requireHttp();
+        }
+
+        return $this;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function requireHttps()
+    {
+        parent::requireHttps();
+
+        foreach ($this->controllers as $controller) {
+            $controller->requireHttps();
+        }
+
+        return $this;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function middleware($callback)
+    {
+        parent::middleware($callback);
+
+        foreach ($this->controllers as $controller) {
+            $controller->middleware($callback);
+        }
+
+        return $this;
     }
 
     /**
@@ -130,8 +220,6 @@ class ControllerCollection extends Controller
         $routes = new RouteCollection();
 
         foreach ($this->controllers as $controller) {
-            $route = $controller->getRoute();
-
             if (!$name = $controller->getRouteName()) {
                 $name = $controller->generateRouteName($prefix);
                 while ($routes->get($name)) {
@@ -139,11 +227,7 @@ class ControllerCollection extends Controller
                 }
                 $controller->bind($name);
             }
-
-            $this->mergeGlobalSettings($route);
-
-            $routes->add($name, $route);
-
+            $routes->add($name, $controller->getRoute());
             $controller->freeze();
         }
 
@@ -151,22 +235,4 @@ class ControllerCollection extends Controller
 
         return $routes;
     }
-
-    protected function mergeGlobalSettings(Route $route)
-    {
-        $route->setOption('_middlewares', array_merge((array) $this->route->getOption('_middlewares'), (array) $route->getOption('_middlewares')));
-        $route->setOption('_converters', array_replace((array) $this->route->getOption('_converters'), (array) $route->getOption('_converters')));
-
-        foreach ($this->route->getDefaults() as $name => $value) {
-            if (!$route->hasDefault($name)) {
-                $route->setDefault($name, $value);
-            }
-        }
-
-        foreach ($this->route->getRequirements() as $name => $value) {
-            if (!$route->getRequirement($name)) {
-                $route->setRequirement($name, $value);
-            }
-        }
-    }
 }

tests/Silex/Tests/ControllerCollectionTest.php

@@ -34,8 +34,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 +45,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 +67,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('/'));
         $r = new \ReflectionObject($mainRootController);
@@ -90,74 +84,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 testRouteSettingsFromCollection()
+    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()
     {
-        $controller = new Controller(new Route('/'));
-        $controller
-            ->value('bar', 'bar')
-            ->value('baz', 'baz')
-            ->assert('bar', 'bar')
-            ->assert('baz', 'baz')
-            ->convert('bar', $converterBar = function () {})
-            ->middleware($middleware1 = function () {})
-            ->bind('home')
-        ;
-        $controller1 = new Controller(new Route('/'));
-        $controller1
-            ->requireHttp()
-            ->method('post')
-            ->convert('foo', $converterFoo1 = function () {})
-            ->bind('home1')
-        ;
+        $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->add($controller);
-        $controllers->add($controller1);
-
-        $controllers
-            ->value('foo', 'foo')
-            ->value('baz', 'not_used')
-            ->assert('foo', 'foo')
-            ->assert('baz', 'not_used')
-            ->requireHttps()
-            ->method('get')
-            ->convert('foo', $converterFoo = function () {})
-            ->middleware($middleware2 = function () {})
-        ;
+        $controllers->convert('id', '1');
+        $controller = $controllers->match('/{id}/{name}/{extra}', function () {})->convert('name', 'Fabien')->convert('extra', 'Symfony');
+        $controllers->convert('extra', 'Twig');
 
-        $routes = $controllers->flush();
+        $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(
-            'foo' => 'foo',
-            'bar' => 'bar',
-            'baz' => 'baz',
-        ), $routes->get('home')->getDefaults());
-
-        $this->assertEquals(array(
-            'foo'     => 'foo',
-            'bar'     => 'bar',
-            'baz'     => 'baz',
-            '_scheme' => 'https',
-            '_method' => 'get',
-        ), $routes->get('home')->getRequirements());
-
-        $this->assertEquals(array(
-            'foo' => $converterFoo,
-            'bar' => $converterBar,
-        ), $routes->get('home')->getOption('_converters'));
-
-        $this->assertEquals(array($middleware1, $middleware2), $routes->get('home')->getOption('_middlewares'));
-
-        $this->assertEquals('http', $routes->get('home1')->getRequirement('_scheme'));
-        $this->assertEquals('post', $routes->get('home1')->getRequirement('_method'));
-        $this->assertEquals(array('foo' => $converterFoo1), $routes->get('home1')->getOption('_converters'));
+        $this->assertEquals(array('mid1', 'mid2', 'mid3'), $controller->getRoute()->getOption('_middlewares'));
     }
 }