feature #864 allowed any callback to be a method call on a service (a:b notation) (fabpot)

This PR was merged into the master branch.

Discussion
----------

allowed any callback to be a method call on a service (a:b notation)

#800 added the possibility to use a service method (with the `a:b` notation) for converters. This PR goes one step further by allowing almost all methods accepting a callback to use a service method.

The following is now possible:

```php
$app->before('service:beforeApp');
$app->after('service:afterApp');
$app->finish('service:finishApp');
$app->error('service:error');
$app->on('kernel.request', 'service:onRequest');

$app
    ->match('/', 'service:controller')
    ->convert('foo', 'service:convert')
    ->before('service:before')
    ->after('service:after')
;
```

Commits
-------

87f5e203 allowed any callback to be a method call on a service (a:b notation)

doc/changelog.rst

@@ -9,7 +9,7 @@ Changelog
 * Added run() on Route to be able to define the controller code
 * Deprecated TwigCoreExtension (register the new HttpFragmentServiceProvider instead)
 * Added HttpFragmentServiceProvider
-* Add support for using a service method as a converter.
+* Allowed a callback to be a method call on a service (before, after, finish, error, on Application; convert, before, after on Controller)
 
 1.1.3 (2013-XX-XX)
 ------------------

src/Silex/Application.php

@@ -272,13 +272,13 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
     public function on($eventName, $callback, $priority = 0)
     {
         if ($this->booted) {
-            $this['dispatcher']->addListener($eventName, $callback, $priority);
+            $this['dispatcher']->addListener($eventName, $this['callback_resolver']->resolveCallback($callback), $priority);
 
             return;
         }
 
         $this['dispatcher'] = $this->share($this->extend('dispatcher', function ($dispatcher, $app) use ($callback, $priority, $eventName) {
-            $dispatcher->addListener($eventName, $callback, $priority);
+            $dispatcher->addListener($eventName, $app['callback_resolver']->resolveCallback($callback), $priority);
 
             return $dispatcher;
         }));
@@ -295,12 +295,14 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      */
     public function before($callback, $priority = 0)
     {
-        $this->on(KernelEvents::REQUEST, function (GetResponseEvent $event) use ($callback) {
+        $app = $this;
+
+        $this->on(KernelEvents::REQUEST, function (GetResponseEvent $event) use ($callback, $app) {
             if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
                 return;
             }
 
-            $ret = call_user_func($callback, $event->getRequest());
+            $ret = call_user_func($app['callback_resolver']->resolveCallback($callback), $event->getRequest());
 
             if ($ret instanceof Response) {
                 $event->setResponse($ret);
@@ -319,12 +321,14 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      */
     public function after($callback, $priority = 0)
     {
-        $this->on(KernelEvents::RESPONSE, function (FilterResponseEvent $event) use ($callback) {
+        $app = $this;
+
+        $this->on(KernelEvents::RESPONSE, function (FilterResponseEvent $event) use ($callback, $app) {
             if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
                 return;
             }
 
-            call_user_func($callback, $event->getRequest(), $event->getResponse());
+            call_user_func($app['callback_resolver']->resolveCallback($callback), $event->getRequest(), $event->getResponse());
         }, $priority);
     }
 
@@ -339,8 +343,10 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
      */
     public function finish($callback, $priority = 0)
     {
-        $this->on(KernelEvents::TERMINATE, function (PostResponseEvent $event) use ($callback) {
-            call_user_func($callback, $event->getRequest(), $event->getResponse());
+        $app = $this;
+
+        $this->on(KernelEvents::TERMINATE, function (PostResponseEvent $event) use ($callback, $app) {
+            call_user_func($app['callback_resolver']->resolveCallback($callback), $event->getRequest(), $event->getResponse());
         }, $priority);
     }
 

src/Silex/CallbackResolver.php

@@ -43,7 +43,7 @@ class CallbackResolver
      *
      * @throws \InvalidArgumentException In case the method does not exist.
      */
-    public function getCallback($name)
+    public function convertCallback($name)
     {
         list($service, $method) = explode(':', $name, 2);
 
@@ -53,4 +53,18 @@ class CallbackResolver
 
         return array($this->app[$service], $method);
     }
+
+    /**
+     * Returns a callable given its string representation if it is a valid service method.
+     *
+     * @param string $name
+     *
+     * @return array A callable array
+     *
+     * @throws \InvalidArgumentException In case the method does not exist.
+     */
+    public function resolveCallback($name)
+    {
+        return $this->isValid($name) ? $this->convertCallback($name) : $name;
+    }
 }

src/Silex/EventListener/ConverterListener.php

@@ -50,7 +50,7 @@ class ConverterListener implements EventSubscriberInterface
         $route = $this->routes->get($request->attributes->get('_route'));
         if ($route && $converters = $route->getOption('_converters')) {
             foreach ($converters as $name => $callback) {
-                $callback = $this->callbackResolver->isValid($callback) ? $this->callbackResolver->getCallback($callback) : $callback;
+                $callback = $this->callbackResolver->resolveCallback($callback);
 
                 $request->attributes->set($name, call_user_func($callback, $request->attributes->get($name), $request));
             }

src/Silex/EventListener/MiddlewareListener.php

@@ -51,7 +51,7 @@ class MiddlewareListener implements EventSubscriberInterface
         }
 
         foreach ((array) $route->getOption('_before_middlewares') as $callback) {
-            $ret = call_user_func($callback, $request, $this->app);
+            $ret = call_user_func($this->app['callback_resolver']->resolveCallback($callback), $request, $this->app);
             if ($ret instanceof Response) {
                 $event->setResponse($ret);
 
@@ -76,7 +76,7 @@ class MiddlewareListener implements EventSubscriberInterface
         }
 
         foreach ((array) $route->getOption('_after_middlewares') as $callback) {
-            $response = call_user_func($callback, $request, $event->getResponse(), $this->app);
+            $response = call_user_func($this->app['callback_resolver']->resolveCallback($callback), $request, $event->getResponse(), $this->app);
             if ($response instanceof Response) {
                 $event->setResponse($response);
             } elseif (null !== $response) {

src/Silex/ExceptionListenerWrapper.php

@@ -42,6 +42,7 @@ class ExceptionListenerWrapper
     public function __invoke(GetResponseForExceptionEvent $event)
     {
         $exception = $event->getException();
+        $this->callback = $this->app['callback_resolver']->resolveCallback($this->callback);
 
         if (!$this->shouldRun($exception)) {
             return;

src/Silex/ServiceControllerResolver.php

@@ -47,7 +47,7 @@ class ServiceControllerResolver implements ControllerResolverInterface
             return $this->controllerResolver->getController($request);
         }
 
-        return $this->callbackResolver->getCallback($controller);
+        return $this->callbackResolver->convertCallback($controller);
     }
 
     /**

tests/Silex/Tests/CallbackResolverTest.php

@@ -28,7 +28,7 @@ class CallbackResolverTest extends \PHPUnit_Framework_Testcase
         $this->assertTrue($this->resolver->isValid('some_service:methodName'));
         $this->assertEquals(
             array($this->app['some_service'], 'methodName'),
-            $this->resolver->getCallback('some_service:methodName')
+            $this->resolver->convertCallback('some_service:methodName')
         );
     }
 
@@ -44,6 +44,6 @@ class CallbackResolverTest extends \PHPUnit_Framework_Testcase
      */
     public function testShouldThrowAnExceptionIfServiceIsMissing()
     {
-        $this->resolver->getCallback('some_service:methodName');
+        $this->resolver->convertCallback('some_service:methodName');
     }
 }

tests/Silex/Tests/CallbackServicesTests.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 Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Callback as services test cases.
+ *
+ * @author Fabien Potencier <fabien@symfony.com>
+ */
+class CallbackServicesTest extends \PHPUnit_Framework_TestCase
+{
+    public $called = array();
+
+    public function testCallbacksAsServices()
+    {
+        $app = new Application();
+
+        $app['service'] = $app->share(function () {
+            return new self();
+        });
+
+        $app->before('service:beforeApp');
+        $app->after('service:afterApp');
+        $app->finish('service:finishApp');
+        $app->error('service:error');
+        $app->on('kernel.request', 'service:onRequest');
+
+        $app
+            ->match('/', 'service:controller')
+            ->convert('foo', 'service:convert')
+            ->before('service:before')
+            ->after('service:after')
+        ;
+
+        $request = Request::create('/');
+        $response = $app->handle($request);
+        $app->terminate($request, $response);
+
+        $this->assertEquals(array(
+            'CONVERT',
+            'BEFORE APP',
+            'ON REQUEST',
+            'BEFORE',
+            'ERROR',
+            'AFTER',
+            'AFTER APP',
+            'FINISH APP',
+        ), $app['service']->called);
+    }
+
+    public function controller(Application $app)
+    {
+        return $app->abort(404);
+    }
+
+    public function before()
+    {
+        $this->called[] = 'BEFORE';
+    }
+
+    public function after()
+    {
+        $this->called[] = 'AFTER';
+    }
+
+    public function beforeApp()
+    {
+        $this->called[] = 'BEFORE APP';
+    }
+
+    public function afterApp()
+    {
+        $this->called[] = 'AFTER APP';
+    }
+
+    public function finishApp()
+    {
+        $this->called[] = 'FINISH APP';
+    }
+
+    public function error()
+    {
+        $this->called[] = 'ERROR';
+    }
+
+    public function convert()
+    {
+        $this->called[] = 'CONVERT';
+    }
+
+    public function onRequest()
+    {
+        $this->called[] = 'ON REQUEST';
+    }
+}

tests/Silex/Tests/ServiceControllerResolverTest.php

@@ -41,7 +41,7 @@ class ServiceControllerResolverTest extends \PHPUnit_Framework_Testcase
             ->will($this->returnValue(true));
 
         $this->mockCallbackResolver->expects($this->once())
-            ->method('getCallback')
+            ->method('convertCallback')
             ->with('some_service:methodName')
             ->will($this->returnValue(array('callback')));