made parsing controllers more robust

src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php

@@ -64,10 +64,18 @@ class ControllerResolver implements ControllerResolverInterface
             return false;
         }
 
-        if (is_array($controller) || (is_object($controller) && method_exists($controller, '__invoke'))) {
+        if (is_array($controller)) {
             return $controller;
         }
 
+        if (is_object($controller)) {
+            if (method_exists($controller, '__invoke')) {
+                return $controller;
+            }
+
+            throw new \InvalidArgumentException(sprintf('Controller "%s" for URI "%s" is not callable.', get_class($controller), $request->getPathInfo()));
+        }
+
         if (false === strpos($controller, ':')) {
             if (method_exists($controller, '__invoke')) {
                 return new $controller();
@@ -79,7 +87,7 @@ class ControllerResolver implements ControllerResolverInterface
         $callable = $this->createController($controller);
 
         if (!is_callable($callable)) {
-            throw new \InvalidArgumentException(sprintf('The controller for URI "%s" is not callable.', $request->getPathInfo()));
+            throw new \InvalidArgumentException(sprintf('Controller "%s" for URI "%s" is not callable.', $controller, $request->getPathInfo()));
         }
 
         return $callable;

src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php

@@ -24,66 +24,119 @@ class ControllerResolverTest extends \PHPUnit_Framework_TestCase
         }
     }
 
-    public function testGetController()
+    public function testGetControllerWithoutControllerParameter()
     {
-        $logger = new Logger();
+        $logger = $this->getMock('Psr\Log\LoggerInterface');
+        $logger->expects($this->once())->method('warning')->with('Unable to look for the controller as the "_controller" parameter is missing');
         $resolver = new ControllerResolver($logger);
 
         $request = Request::create('/');
         $this->assertFalse($resolver->getController($request), '->getController() returns false when the request has no _controller attribute');
-        $this->assertEquals(array('Unable to look for the controller as the "_controller" parameter is missing'), $logger->getLogs('warning'));
+    }
 
-        $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::testGetController');
+    public function testGetControllerWithLambda()
-        $controller = $resolver->getController($request);
+    {
-        $this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller[0], '->getController() returns a PHP callable');
+        $resolver = new ControllerResolver();
 
+        $request = Request::create('/');
         $request->attributes->set('_controller', $lambda = function () {});
         $controller = $resolver->getController($request);
         $this->assertSame($lambda, $controller);
+    }
 
+    public function testGetControllerWithObjectAndInvokeMethod()
+    {
+        $resolver = new ControllerResolver();
+
+        $request = Request::create('/');
         $request->attributes->set('_controller', $this);
         $controller = $resolver->getController($request);
         $this->assertSame($this, $controller);
+    }
 
-        $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest');
+    public function testGetControllerWithObjectAndMethod()
-        $controller = $resolver->getController($request);
+    {
-        $this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller);
+        $resolver = new ControllerResolver();
 
+        $request = Request::create('/');
         $request->attributes->set('_controller', array($this, 'controllerMethod1'));
         $controller = $resolver->getController($request);
         $this->assertSame(array($this, 'controllerMethod1'), $controller);
+    }
 
+    public function testGetControllerWithClassAndMethod()
+    {
+        $resolver = new ControllerResolver();
+
+        $request = Request::create('/');
         $request->attributes->set('_controller', array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', 'controllerMethod4'));
         $controller = $resolver->getController($request);
         $this->assertSame(array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', 'controllerMethod4'), $controller);
+    }
 
+    public function testGetControllerWithObjectAndMethodAsString()
+    {
+        $resolver = new ControllerResolver();
+
+        $request = Request::create('/');
+        $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::controllerMethod1');
+        $controller = $resolver->getController($request);
+        $this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller[0], '->getController() returns a PHP callable');
+    }
+
+    public function testGetControllerWithClassAndInvokeMethod()
+    {
+        $resolver = new ControllerResolver();
+
+        $request = Request::create('/');
+        $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest');
+        $controller = $resolver->getController($request);
+        $this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller);
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testGetControllerOnObjectWithoutInvokeMethod()
+    {
+        $resolver = new ControllerResolver();
+
+        $request = Request::create('/');
+        $request->attributes->set('_controller', new \stdClass());
+        $resolver->getController($request);
+    }
+
+    public function testGetControllerWithFunction()
+    {
+        $resolver = new ControllerResolver();
+
+        $request = Request::create('/');
         $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\some_controller_function');
         $controller = $resolver->getController($request);
         $this->assertSame('Symfony\Component\HttpKernel\Tests\Controller\some_controller_function', $controller);
+    }
 
-        $request->attributes->set('_controller', 'foo');
+    /**
-        try {
+     * @dataProvider      getUndefinedControllers
-            $resolver->getController($request);
+     * @expectedException \InvalidArgumentException
-            $this->fail('->getController() throws an \InvalidArgumentException if the _controller attribute is not well-formatted');
+     */
-        } catch (\Exception $e) {
+    public function testGetControllerOnNonUndefinedFunction($controller)
-            $this->assertInstanceOf('\InvalidArgumentException', $e, '->getController() throws an \InvalidArgumentException if the _controller attribute is not well-formatted');
+    {
-        }
+        $resolver = new ControllerResolver();
 
-        $request->attributes->set('_controller', 'foo::bar');
+        $request = Request::create('/');
-        try {
+        $request->attributes->set('_controller', $controller);
-            $resolver->getController($request);
+        $resolver->getController($request);
-            $this->fail('->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent class');
+    }
-        } catch (\Exception $e) {
-            $this->assertInstanceOf('\InvalidArgumentException', $e, '->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent class');
-        }
 
-        $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar');
+    public function getUndefinedControllers()
-        try {
+    {
-            $resolver->getController($request);
+        return array(
-            $this->fail('->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent method');
+            array('foo'),
-        } catch (\Exception $e) {
+            array('foo::bar'),
-            $this->assertInstanceOf('\InvalidArgumentException', $e, '->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent method');
+            array('stdClass'),
-        }
+            array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar'),
+        );
     }
 
     public function testGetArguments()
@@ -165,7 +218,7 @@ class ControllerResolverTest extends \PHPUnit_Framework_TestCase
     {
     }
 
-    protected function controllerMethod1($foo)
+    public function controllerMethod1($foo)
     {
     }