Introduce Controller and ControllerCollection with flushing

The challenge is to allow the RouteCollection to be mutable while making
it possible to set Route names. We do not want to set the route name
when creating the Controller (it must be optional, adding it after the
closure is ugly as hell), and RouteCollection does not allow changing
route names after they have been added.

The way to solve this is to add a staging area for these routes. This
staging area is the ControllerCollection. All defined controllers are
added to this area. Once the flush() method is called on the
ControllerContainer, the controllers are frozen (no name change
possible) and added as routes to the RouteContainer.

If you want to make use of the RouteCollection (for example: dumping out
routes to the console), you must explicitly call flush() on the
ControllerCollection. There was no good way to do this implicitly. The
application will also call flush() if you use handle() or run().

TLDR:
a) We can now set route names
b) Call getControllerCollection()->flush() before getRouteCollection()
c) We must document flushing
d) Bulat is awesome

src/Silex/Application.php

@@ -22,8 +22,8 @@ use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\EventDispatcher\Event;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\Routing\RouteCollection;
 use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
 use Symfony\Component\Routing\Matcher\UrlMatcher;
 use Symfony\Component\Routing\Matcher\Exception\MethodNotAllowedException;
 use Symfony\Component\Routing\Matcher\Exception\NotFoundException;
@@ -36,7 +36,8 @@ use Symfony\Component\Routing\Matcher\Exception\NotFoundException;
 class Application extends HttpKernel implements EventSubscriberInterface
 {
     private $dispatcher;
-    private $routes;
+    private $routeCollection;
+    private $controllerCollection;
     private $request;
 
     /**
@@ -44,7 +45,8 @@ class Application extends HttpKernel implements EventSubscriberInterface
      */
     public function __construct()
     {
-        $this->routes = new RouteCollection();
+        $this->routeCollection = new RouteCollection();
+        $this->controllerCollection = new ControllerCollection($this->routeCollection);
 
         $this->dispatcher = new EventDispatcher();
         $this->dispatcher->addSubscriber($this);
@@ -65,19 +67,37 @@ class Application extends HttpKernel implements EventSubscriberInterface
         return $this->request;
     }
 
+    /**
+     * Get the collection of routes.
+     *
+     * @return Symfony\Component\Routing\RouteCollection
+     */
+    public function getRouteCollection()
+    {
+        return $this->routeCollection;
+    }
+
+    /**
+     * Get the collection of controllers.
+     *
+     * @return Silex\ControllerCollection
+     */
+    public function getControllerCollection()
+    {
+        return $this->controllerCollection;
+    }
+
     /**
      * Map a pattern to a callable.
      *
      * You can optionally specify HTTP methods that should be matched.
      *
-     * This method is chainable.
-     *
      * @param string $pattern Matched route pattern
      * @param mixed $to Callback that returns the response when matched
      * @param string $method Matched HTTP methods, multiple can be supplied,
      *                       delimited by a pipe character '|', eg. 'GET|POST'.
      *
-     * @return $this
+     * @return Silex\Controller
      */
     public function match($pattern, $to, $method = null)
     {
@@ -87,81 +107,63 @@ class Application extends HttpKernel implements EventSubscriberInterface
             $requirements['_method'] = $method;
         }
 
-        $routeName = (string) $method.$pattern;
-        $routeName = str_replace(array('{', '}'), '', $routeName);
-        $routeName = str_replace(array('/', ':', '|'), '_', $routeName);
         $route = new Route($pattern, array('_controller' => $to), $requirements);
-        $this->routes->add($routeName, $route);
+        $controller = new Controller($route);
+        $this->controllerCollection->add($controller);
 
-        return $this;
+        return $controller;
     }
 
     /**
      * Map a GET request to a callable.
      *
-     * This method is chainable.
-     *
      * @param string $pattern Matched route pattern
      * @param mixed $to Callback that returns the response when matched
      *
-     * @return $this
+     * @return Silex\Controller
      */
     public function get($pattern, $to)
     {
-        $this->match($pattern, $to, 'GET');
-
-        return $this;
+        return $this->match($pattern, $to, 'GET');
     }
 
     /**
      * Map a POST request to a callable.
      *
-     * This method is chainable.
-     *
      * @param string $pattern Matched route pattern
      * @param mixed $to Callback that returns the response when matched
      *
-     * @return $this
+     * @return Silex\Controller
      */
     public function post($pattern, $to)
     {
-        $this->match($pattern, $to, 'POST');
-
-        return $this;
+        return $this->match($pattern, $to, 'POST');
     }
 
     /**
      * Map a PUT request to a callable.
      *
-     * This method is chainable.
-     *
      * @param string $pattern Matched route pattern
      * @param mixed $to Callback that returns the response when matched
      *
-     * @return $this
+     * @return Silex\Controller
      */
     public function put($pattern, $to)
     {
-        $this->match($pattern, $to, 'PUT');
-
-        return $this;
+        return $this->match($pattern, $to, 'PUT');
     }
 
     /**
      * Map a DELETE request to a callable.
      *
-     * This method is chainable.
-     *
      * @param string $pattern Matched route pattern
      * @param mixed $to Callback that returns the response when matched
      *
-     * @return $this
+     * @return Silex\Controller
      */
     public function delete($pattern, $to)
     {
-        $this->match($pattern, $to, 'DELETE');
-
-        return $this;
+        return $this->match($pattern, $to, 'DELETE');
     }
 
     /**
@@ -237,8 +239,6 @@ class Application extends HttpKernel implements EventSubscriberInterface
      * Handle the request and deliver the response.
      *
      * @param Request $request Request to process
-     *
-     * @return $this
      */
     public function run(Request $request = null)
     {
@@ -256,7 +256,9 @@ class Application extends HttpKernel implements EventSubscriberInterface
     {
         $this->request = $event->getRequest();
 
-        $matcher = new UrlMatcher($this->routes, array(
+        $this->controllerCollection->flush();
+
+        $matcher = new UrlMatcher($this->routeCollection, array(
             'base_url'  => $this->request->getBaseUrl(),
             'method'    => $this->request->getMethod(),
             'host'      => $this->request->getHost(),

src/Silex/Controller.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 Silex\Exception\ControllerFrozenException;
+use Symfony\Component\Routing\Route;
+
+/**
+ * A wrapper for a controller, mapped to a route.
+ *
+ * @author Igor Wiedler igor@wiedler.ch
+ */
+class Controller
+{
+    private $route;
+    private $routeName;
+    private $isFrozen = false;
+
+    /**
+     * Constructor.
+     *
+     * @param Route $route
+     */
+    public function __construct(Route $route)
+    {
+        $this->route = $route;
+        $this->setRouteName($this->defaultRouteName());
+    }
+
+    /**
+     * Get the controller's route.
+     */
+    public function getRoute()
+    {
+        return $this->route;
+    }
+
+    /**
+     * Get the controller's route name.
+     */
+    public function getRouteName()
+    {
+        return $this->routeName;
+    }
+
+    /**
+     * Set the controller's route.
+     *
+     * @param string $routeName
+     */
+    public function setRouteName($routeName)
+    {
+        if ($this->isFrozen) {
+            throw new ControllerFrozenException(sprintf('Calling %s on frozen %s instance.', __METHOD__, __CLASS__));
+        }
+
+        $this->routeName = $routeName;
+    }
+
+    /**
+     * Freeze the controller.
+     *
+     * Once the controller is frozen, you can no longer change the route name
+     */
+    public function freeze()
+    {
+        $this->isFrozen = true;
+    }
+
+    private function defaultRouteName()
+    {
+        $requirements = $this->route->getRequirements();
+        $method = isset($requirements['_method']) ? $requirements['_method'] : '';
+
+        $routeName = $method.$this->route->getPattern();
+        $routeName = str_replace(array('{', '}'), '', $routeName);
+        $routeName = str_replace(array('/', ':', '|'), '_', $routeName);
+
+        return $routeName;
+    }
+}

src/Silex/ControllerCollection.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\RouteCollection;
+
+/**
+ * A collection of Silex controllers.
+ *
+ * It acts as a staging area for routes. You are able to set the route name
+ * until flush() is called, at which point all controllers are frozen and
+ * added to the RouteCollection.
+ *
+ * @author Igor Wiedler igor@wiedler.ch
+ */
+class ControllerCollection
+{
+    private $controllers = array();
+    private $routeCollection;
+
+    public function __construct(RouteCollection $routeCollection)
+    {
+        $this->routeCollection = $routeCollection;
+    }
+
+    /**
+     * Add a controller to the staging area.
+     *
+     * @param Controller $controller
+     */
+    public function add(Controller $controller)
+    {
+        $this->controllers[] = $controller;
+    }
+
+    /**
+     * Persist and freeze staged controllers.
+     */
+    public function flush()
+    {
+        foreach ($this->controllers as $controller) {
+            $this->routeCollection->add($controller->getRouteName(), $controller->getRoute());
+            $controller->freeze();
+        }
+
+        $this->controllers = array();
+    }
+}

src/Silex/Exception/ControllerFrozenException.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\Exception;
+
+/**
+ * Exception, is thrown when a frozen controller is modified
+ *
+ * @author Igor Wiedler igor@wiedler.ch
+ */
+class ControllerFrozenException extends \RuntimeException
+{
+}

tests/Silex/Tests/ApplicationTest.php

@@ -21,24 +21,29 @@ use Symfony\Component\HttpFoundation\Request;
  */
 class ApplicationTest extends \PHPUnit_Framework_TestCase
 {
-    public function testFluidInterface()
+    public function testMatchReturnValue()
     {
         $application = new Application();
 
         $returnValue = $application->match('/foo', function() {});
-        $this->assertSame($application, $returnValue, '->match() should return $this');
+        $this->assertInstanceOf('Silex\Controller', $returnValue);
 
         $returnValue = $application->get('/foo', function() {});
-        $this->assertSame($application, $returnValue, '->get() should return $this');
+        $this->assertInstanceOf('Silex\Controller', $returnValue);
 
         $returnValue = $application->post('/foo', function() {});
-        $this->assertSame($application, $returnValue, '->post() should return $this');
+        $this->assertInstanceOf('Silex\Controller', $returnValue);
 
         $returnValue = $application->put('/foo', function() {});
-        $this->assertSame($application, $returnValue, '->put() should return $this');
+        $this->assertInstanceOf('Silex\Controller', $returnValue);
 
         $returnValue = $application->delete('/foo', function() {});
-        $this->assertSame($application, $returnValue, '->delete() should return $this');
+        $this->assertInstanceOf('Silex\Controller', $returnValue);
+    }
+
+    public function testFluidInterface()
+    {
+        $application = new Application();
 
         $returnValue = $application->before(function() {});
         $this->assertSame($application, $returnValue, '->before() should return $this');
@@ -64,4 +69,32 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
 
         $this->assertEquals($request, $application->getRequest());
     }
+
+    public function testGetRouteCollectionWithNoRoutes()
+    {
+        $application = new Application();
+
+        $routeCollection = $application->getRouteCollection();
+        $this->assertInstanceOf('Symfony\Component\Routing\RouteCollection', $routeCollection);
+        $this->assertEquals(0, count($routeCollection->all()));
+    }
+
+    public function testGetRouteCollectionWithRoutes()
+    {
+        $application = new Application();
+
+        $application->get('/foo', function() {
+            return 'foo';
+        });
+
+        $application->get('/bar', function() {
+            return 'bar';
+        });
+
+        $routeCollection = $application->getRouteCollection();
+        $this->assertInstanceOf('Symfony\Component\Routing\RouteCollection', $routeCollection);
+        $this->assertEquals(0, count($routeCollection->all()));
+        $application->getControllerCollection()->flush();
+        $this->assertEquals(2, count($routeCollection->all()));
+    }
 }

tests/Silex/Tests/ControllerCollectionTest.php

+<?php
+
+/*
+ * This file is part of the Silex framework.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Silex\Tests;
+
+use Silex\Application;
+use Silex\Controller;
+use Silex\ControllerCollection;
+use Silex\Exception\ControllerFrozenException;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
+
+/**
+ * ControllerCollection test cases.
+ *
+ * @author Igor Wiedler <igor@wiedler.ch>
+ */
+class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
+{
+    public function testGetRouteCollectionWithNoRoutes()
+    {
+        $routeCollection = new RouteCollection();
+        $controllerCollection = new ControllerCollection($routeCollection);
+
+        $this->assertEquals(0, count($routeCollection->all()));
+        $controllerCollection->flush();
+        $this->assertEquals(0, count($routeCollection->all()));
+    }
+
+    public function testGetRouteCollectionWithRoutes()
+    {
+        $routeCollection = new RouteCollection();
+        $controllerCollection = new ControllerCollection($routeCollection);
+        $controllerCollection->add(new Controller(new Route('/foo')));
+        $controllerCollection->add(new Controller(new Route('/bar')));
+
+        $this->assertEquals(0, count($routeCollection->all()));
+        $controllerCollection->flush();
+        $this->assertEquals(2, count($routeCollection->all()));
+    }
+
+    public function testControllerFreezing()
+    {
+        $routeCollection = new RouteCollection();
+        $controllerCollection = new ControllerCollection($routeCollection);
+
+        $fooController = new Controller(new Route('/foo'));
+        $fooController->setRouteName('foo');
+        $controllerCollection->add($fooController);
+
+        $barController = new Controller(new Route('/bar'));
+        $barController->setRouteName('bar');
+        $controllerCollection->add($barController);
+
+        $controllerCollection->flush();
+
+        try {
+            $fooController->setRouteName('foo2');
+            $this->fail();
+        } catch (ControllerFrozenException $e) {
+        }
+
+        try {
+            $barController->setRouteName('bar2');
+            $this->fail();
+        } catch (ControllerFrozenException $e) {
+        }
+    }
+}

tests/Silex/Tests/ControllerTest.php

+<?php
+
+/*
+ * This file is part of the Silex framework.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Silex\Tests;
+
+use Silex\Controller;
+use Symfony\Component\Routing\Route;
+
+/**
+ * Controller test cases.
+ *
+ * @author Igor Wiedler <igor@wiedler.ch>
+ */
+class ControllerTest extends \PHPUnit_Framework_TestCase
+{
+    public function testSetRouteName()
+    {
+        $controller = new Controller(new Route('/foo'));
+        $controller->setRouteName('foo');
+        $this->assertEquals('foo', $controller->getRouteName());
+    }
+
+    /**
+    * @expectedException Silex\Exception\ControllerFrozenException
+    */
+    public function testFrozenControllerShouldThrowException()
+    {
+        $controller = new Controller(new Route('/foo'));
+        $controller->setRouteName('foo');
+        $controller->freeze();
+        $controller->setRouteName('bar');
+    }
+}