feature #804 added a way to delay the attachment of a controller to a route (fabpot)

This PR was merged into the master branch.

Discussion
----------

added a way to delay the attachment of a controller to a route

There are several ideas behind this PR. It's mostly about consistency by being able to decouple the setting of a controller from the match call. The old way still works of course.

It is more consistent as everything can now be done via method calls and the fact that the controller is the second argument makes things uglier, especially when the code in the controller is large.

As an added bonus, it helps when indenting the code (currently, I always wonder how to indent my code as it's not obvious). Last, but not the least, being able to put the code last also makes things more readable.

Current:

```
$app->get('/foo', function ($id) {
    // ...
})
->bind('home')
->assert('id', '\d+')
;
```

Now:

```
$app
    ->get('/foo')
    ->bind('home')
    ->assert('id', '\d+')
    ->run(function ($id) {
        // ...
    })
;
```

Commits
-------

8fbb9369 added a way to delay the attachment of a controller to a route

doc/changelog.rst

@@ -4,6 +4,7 @@ Changelog
 1.2.0 (2013-XX-XX)
 ------------------
 
+* Added run() on Route to be able to define the controller code
 * Deprecated TwigCoreExtension (register the new HttpFragmentServiceProvider instead)
 * Added HttpFragmentServiceProvider
 

src/Silex/Application.php

@@ -200,7 +200,7 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      *
      * @return Controller
      */
-    public function match($pattern, $to)
+    public function match($pattern, $to = null)
     {
         return $this['controllers']->match($pattern, $to);
     }
@@ -213,7 +213,7 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      *
      * @return Controller
      */
-    public function get($pattern, $to)
+    public function get($pattern, $to = null)
     {
         return $this['controllers']->get($pattern, $to);
     }
@@ -226,7 +226,7 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      *
      * @return Controller
      */
-    public function post($pattern, $to)
+    public function post($pattern, $to = null)
     {
         return $this['controllers']->post($pattern, $to);
     }
@@ -239,7 +239,7 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      *
      * @return Controller
      */
-    public function put($pattern, $to)
+    public function put($pattern, $to = null)
     {
         return $this['controllers']->put($pattern, $to);
     }
@@ -252,7 +252,7 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      *
      * @return Controller
      */
-    public function delete($pattern, $to)
+    public function delete($pattern, $to = null)
     {
         return $this['controllers']->delete($pattern, $to);
     }

src/Silex/ControllerCollection.php

@@ -47,14 +47,19 @@ class ControllerCollection
      *
      * @return Controller
      */
-    public function match($pattern, $to)
+    public function match($pattern, $to = null)
     {
         $route = clone $this->defaultRoute;
         $route->setPath($pattern);
-        $route->setDefault('_controller', $to);
-
         $this->controllers[] = $controller = new Controller($route);
 
+        if (null === $to) {
+            $to = function () use ($controller) {
+                throw new \LogicException(sprintf('The "%s" route must have code to run when it matches.', $controller->getRouteName()));
+            };
+        }
+        $route->setDefault('_controller', $to);
+
         return $controller;
     }
 
@@ -66,7 +71,7 @@ class ControllerCollection
      *
      * @return Controller
      */
-    public function get($pattern, $to)
+    public function get($pattern, $to = null)
     {
         return $this->match($pattern, $to)->method('GET');
     }
@@ -79,7 +84,7 @@ class ControllerCollection
      *
      * @return Controller
      */
-    public function post($pattern, $to)
+    public function post($pattern, $to = null)
     {
         return $this->match($pattern, $to)->method('POST');
     }
@@ -92,7 +97,7 @@ class ControllerCollection
      *
      * @return Controller
      */
-    public function put($pattern, $to)
+    public function put($pattern, $to = null)
     {
         return $this->match($pattern, $to)->method('PUT');
     }
@@ -105,7 +110,7 @@ class ControllerCollection
      *
      * @return Controller
      */
-    public function delete($pattern, $to)
+    public function delete($pattern, $to = null)
     {
         return $this->match($pattern, $to)->method('DELETE');
     }

src/Silex/Provider/SecurityServiceProvider.php

@@ -543,7 +543,7 @@ class SecurityServiceProvider implements ServiceProviderInterface
         foreach ($this->fakeRoutes as $route) {
             list($method, $pattern, $name) = $route;
 
-            $app->$method($pattern, null)->bind($name);
+            $app->$method($pattern)->run(null)->bind($name);
         }
     }
 

src/Silex/Route.php

@@ -41,6 +41,20 @@ class Route extends BaseRoute
         parent::__construct($path, $defaults, $requirements, $options, $host, $schemes, $methods);
     }
 
+    /**
+     * Sets the route code that should be executed when matched.
+     *
+     * @param callable $to PHP callback that returns the response when matched
+     *
+     * @return Route $this The current Route instance
+     */
+    public function run($to)
+    {
+        $this->setDefault('_controller', $to);
+
+        return $this;
+    }
+
     /**
      * Sets the requirement for a route variable.
      *

tests/Silex/Tests/ApplicationTest.php

@@ -93,7 +93,7 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
             return 'foo';
         });
 
-        $app->get('/bar', function () {
+        $app->get('/bar')->run(function () {
             return 'bar';
         });
 

tests/Silex/Tests/ControllerCollectionTest.php

@@ -30,6 +30,19 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(0, count($routes->all()));
     }
 
+    /**
+     * @expectedException \LogicException
+     * @expectedExceptionMessage The "foo" route must have code to run when it matches.
+     */
+    public function testGetRouteCollectionWithRouteWithoutController()
+    {
+        $controllers = new ControllerCollection(new Route());
+        $controllers->match('/foo')->bind('foo');
+        $routes = $controllers->flush();
+
+        call_user_func($routes->get('foo')->getDefault('_controller'));
+    }
+
     public function testGetRouteCollectionWithRoutes()
     {
         $controllers = new ControllerCollection(new Route());

tests/Silex/Tests/ControllerTest.php

@@ -68,6 +68,15 @@ class ControllerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('bar' => $func), $controller->getRoute()->getOption('_converters'));
     }
 
+    public function testRun()
+    {
+        $controller = new Controller(new Route('/foo/{bar}'));
+        $ret = $controller->run($cb = function () { return 'foo'; });
+
+        $this->assertSame($ret, $controller);
+        $this->assertEquals($cb, $controller->getRoute()->getDefault('_controller'));
+    }
+
     /**
      * @dataProvider provideRouteAndExpectedRouteName
      */

tests/Silex/Tests/Provider/SecurityServiceProviderTest.php

@@ -160,6 +160,24 @@ class SecurityServiceProviderTest extends WebTestCase
         $client->getRequest()->getSession()->save();
     }
 
+    public function testFakeRoutesAreSerializable()
+    {
+        $app = new Application();
+
+        $app->register(new SecurityServiceProvider(), array(
+            'security.firewalls' => array(
+                'admin' => array(
+                    'logout' => true,
+                ),
+            ),
+        ));
+
+        $app->boot();
+        $app->flush();
+
+        $this->assertCount(1, unserialize(serialize($app['routes'])));
+    }
+
     public function createApplication($authenticationMethod = 'form')
     {
         $app = new Application();