Merge pull request #8 from jdreesen/issue7-silex1

Refactor ControllerResolver

composer.json

@@ -16,6 +16,7 @@
     "require": {
         "php": ">=5.4",
         "php-di/php-di": "~5.0",
+        "php-di/invoker": "~1.2",
         "silex/silex" : "~1.3",
         "pimple/pimple" : "~1.1"
     },

src/Application.php

@@ -7,6 +7,10 @@
 use DI\Container;
 use DI\ContainerBuilder;
 use Interop\Container\ContainerInterface;
+use Invoker\CallableResolver;
+use Invoker\ParameterResolver\AssociativeArrayResolver;
+use Invoker\ParameterResolver\Container\TypeHintContainerResolver;
+use Invoker\ParameterResolver\ResolverChain;
 
 /**
  * Replacement for the Silex Application class to use PHP-DI instead of Pimple.
@@ -36,6 +40,8 @@ public function __construct(ContainerBuilder $containerBuilder = null, array $va
         $containerBuilder = $containerBuilder ?: new ContainerBuilder();
         $containerBuilder->addDefinitions([
             'Interop\Container\ContainerInterface' => $this->containerInteropProxy,
+            'Silex\Application' => $this,
+            get_class($this) => $this,
         ]);
         $containerBuilder->wrapContainer($this->containerInteropProxy);
         $this->phpdi = $containerBuilder->build();
@@ -44,7 +50,13 @@ public function __construct(ContainerBuilder $containerBuilder = null, array $va
 
         // Override the controller resolver with ours
         $this['resolver'] = $this->share(function () {
-            return new ControllerResolver($this->phpdi);
+            return new ControllerResolver(
+                new CallableResolver($this->containerInteropProxy),
+                new ResolverChain([
+                    new AssociativeArrayResolver,
+                    new TypeHintContainerResolver($this->containerInteropProxy),
+                ])
+            );
         });
     }
 

src/Controller/ControllerResolver.php

@@ -2,7 +2,10 @@
 
 namespace DI\Bridge\Silex\Controller;
 
-use DI\InvokerInterface;
+use Invoker\CallableResolver;
+use Invoker\Exception\NotCallableException;
+use Invoker\ParameterResolver\ParameterResolver;
+use Invoker\Reflection\CallableReflection;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
 
@@ -12,41 +15,87 @@
 class ControllerResolver implements ControllerResolverInterface
 {
     /**
-     * @var InvokerInterface
+     * @var CallableResolver
      */
-    private $invoker;
+    private $callableResolver;
 
-    public function __construct(InvokerInterface $invoker)
+    /**
+     * @var ParameterResolver
+     */
+    private $parameterResolver;
+
+    /**
+     * Constructor.
+     *
+     * @param CallableResolver  $callableResolver
+     * @param ParameterResolver $parameterResolver
+     */
+    public function __construct(CallableResolver $callableResolver, ParameterResolver $parameterResolver)
     {
-        $this->invoker = $invoker;
+        $this->callableResolver = $callableResolver;
+        $this->parameterResolver = $parameterResolver;
     }
 
     /**
      * {@inheritdoc}
      */
     public function getController(Request $request)
     {
-        $controller = $request->attributes->get('_controller');
-
-        if (! $controller) {
-            throw new \LogicException('No controller can be found for this request');
+        if (! $controller = $request->attributes->get('_controller')) {
+            throw new \LogicException(sprintf(
+                'Controller for URI "%s" could not be found because the "_controller" parameter is missing.',
+                $request->getPathInfo()
+            ));
         }
 
-        return function () use ($request, $controller) {
-            $parameters = [
-                'request' => $request,
-            ];
-            $parameters += $request->attributes->all();
-
-            return $this->invoker->call($controller, $parameters);
-        };
+        try {
+            return $this->callableResolver->resolve($controller);
+        } catch (NotCallableException $e) {
+            throw new \InvalidArgumentException(sprintf(
+                'Controller for URI "%s" is not callable: %s',
+                $request->getPathInfo(),
+                $e->getMessage()
+            ));
+        }
     }
 
     /**
      * {@inheritdoc}
      */
     public function getArguments(Request $request, $controller)
     {
-        return array();
+        $controllerReflection = CallableReflection::create($controller);
+        $controllerParameters = $controllerReflection->getParameters();
+        $resolvedArguments = [];
+
+        foreach ($controllerParameters as $index => $parameter) {
+            if ('request' === $parameter->getName() || ($parameter->getClass() && $parameter->getClass()->isInstance($request))) {
+                $resolvedArguments[$index] = $request;
+
+                break;
+            }
+        }
+
+        $arguments = $this->parameterResolver->getParameters(
+            $controllerReflection,
+            $request->attributes->all(),
+            $resolvedArguments
+        );
+
+        ksort($arguments);
+
+        // Check if all parameters are resolved
+        $diff = array_diff_key($controllerParameters, $arguments);
+        if (0 < count($diff)) {
+            /** @var \ReflectionParameter $parameter */
+            $parameter = reset($diff);
+            throw new \RuntimeException(sprintf(
+                'Controller "%s" requires that you provide a value for the "$%s" argument.',
+                $controllerReflection->getName(),
+                $parameter->getName()
+            ));
+        }
+
+        return $arguments;
     }
 }

tests/Fixture/Application.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace DI\Bridge\Silex\Test\Fixture;
+
+class Application extends \DI\Bridge\Silex\Application
+{
+}

tests/FunctionalTest.php

@@ -66,11 +66,11 @@ public function should_pass_url_placeholders()
     /**
      * @test
      */
-    public function should_pass_request_object()
+    public function should_pass_request_object_by_parameter_name()
     {
         $app = $this->createApplication();
 
-        $app->get('/', function (Request $request) {
+        $app->get('/', function ($request) {
             return 'Hello ' . $request->get('name');
         });
 
@@ -140,4 +140,52 @@ public function should_pass_the_container_based_on_type_hint()
         $response = $app->handle(Request::create('/'));
         $this->assertEquals('bar', $response->getContent());
     }
+
+    /**
+     * @test
+     */
+    public function should_pass_the_silex_application_based_on_type_hint()
+    {
+        $app = $this->createApplication();
+        $app['foo'] = 'bar';
+
+        $app->get('/', function (\Silex\Application $a) {
+            return $a['foo'];
+        });
+
+        $response = $app->handle(Request::create('/'));
+        $this->assertEquals('bar', $response->getContent());
+    }
+
+    /**
+     * @test
+     */
+    public function should_pass_the_own_application_based_on_type_hint()
+    {
+        $app = new \DI\Bridge\Silex\Test\Fixture\Application(null, [
+            'foo' => 'bar',
+        ]);
+
+        $app->get('/', function (\DI\Bridge\Silex\Test\Fixture\Application $a) {
+            return $a['foo'];
+        });
+
+        $response = $app->handle(Request::create('/'));
+        $this->assertEquals('bar', $response->getContent());
+    }
+
+    /**
+     * @test
+     */
+    public function should_pass_request_object_based_on_type_hint()
+    {
+        $app = $this->createApplication();
+
+        $app->get('/', function (Request $r) {
+            return 'Hello ' . $r->get('name');
+        });
+
+        $response = $app->handle(Request::create('/?name=john'));
+        $this->assertEquals('Hello john', $response->getContent());
+    }
 }

tests/ProvidersTest.php

@@ -2,7 +2,9 @@
 
 namespace DI\Bridge\Silex\Test;
 
+use DI\Bridge\Silex\Test\Fixture\Controller;
 use DI\ContainerBuilder;
+use Silex\Provider\ServiceControllerServiceProvider;
 use Silex\Provider\SwiftmailerServiceProvider;
 use Silex\Provider\TwigServiceProvider;
 use Silex\Provider\UrlGeneratorServiceProvider;
@@ -89,4 +91,22 @@ public function test_mailer()
         $response = $app->handle(Request::create('/'));
         $this->assertEquals('OK', $response->getContent());
     }
+
+    /**
+     * @see https://github.com/PHP-DI/Silex-Bridge/issues/7
+     * @test
+     */
+    public function test_service_controller_service_provider()
+    {
+        $app = $this->createApplication();
+
+        $app->register(new ServiceControllerServiceProvider, [
+            'service.controller' => new Controller,
+        ]);
+
+        $app->get('/{name}', 'service.controller:hello');
+
+        $response = $app->handle(Request::create('/john'));
+        $this->assertEquals('Hello john', $response->getContent());
+    }
 }