cover all possible controller definition cases

FriendsOfSymfony · Feb 7, 2012 · 937e273 · 937e273
1 parent b03414b
commit 937e273
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 21 deletions.
diff --git a/Request/QueryFetcher.php b/Request/QueryFetcher.php
@@ -13,6 +13,7 @@
 
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser;
 
 /**
  * Helper to validate query parameters from the active request.
@@ -26,6 +27,11 @@ class QueryFetcher
      */
     private $container;
 
+    /**
+     * @var ControllerNameParser
+     */
+    private $parser;
+
     /**
      * @var QueryParamReader
      */
@@ -45,35 +51,43 @@ class QueryFetcher
     /**
      * Initializes fetcher.
      *
-     * @param ContainerInterface $container       Container
-     * @param Request           $request          Active request
-     * @param QueryParamReader  $queryParamReader Query param reader
+     * @param ContainerInterface    $container        Container
+     * @param ControllerNameParser  $parser           A ControllerNameParser instance
+     * @param Request               $request          Active request
+     * @param QueryParamReader      $queryParamReader Query param reader
      */
-    public function __construct(ContainerInterface $container, QueryParamReader $queryParamReader, Request $request)
+    public function __construct(ContainerInterface $container, ControllerNameParser $parser, QueryParamReader $queryParamReader, Request $request)
     {
         $this->container = $container;
+        $this->parser = $parser;
         $this->queryParamReader = $queryParamReader;
         $this->request = $request;
     }
 
     private function initParams()
     {
-        $_controller = $this->request->attributes->get('_controller');
-
-        if (null === $_controller) {
-            throw new \InvalidArgumentException('No _controller for request.');
+        $controller = $this->request->attributes->get('_controller');
+
+        if (false === strpos($controller, '::')) {
+            $count = substr_count($controller, ':');
+            if (2 == $count) {
+                // controller in the a:b:c notation then
+                $controller = $this->parser->parse($controller);
+            } elseif (1 == $count) {
+                // controller in the service:method notation
+                list($service, $method) = explode(':', $controller, 2);
+                $class = get_class($this->container->get($service));
+            } else {
+                throw new \LogicException(sprintf('Unable to parse the controller name "%s".', $controller));
+            }
         }
 
-        if (false !== strpos($_controller, '::')) {
-            list($class, $method) = explode('::', $_controller);
-        } else {
-            list($controller, $method) = explode(':', $_controller);
-            if (!$this->container->has($controller)) {
-                throw new \InvalidArgumentException('Controller service not available: '.$controller);
-            }
+        if (empty($class)) {
+            list($class, $method) = explode('::', $controller, 2);
+        }
 
-            $controller = $this->container->get($controller);
-            $class = get_class($controller);
+        if (!class_exists($class)) {
+            throw new \InvalidArgumentException(sprintf('Class "%s" does not exist.', $class));
         }
 
         $this->params = $this->queryParamReader->read(new \ReflectionClass($class), $method);

diff --git a/Resources/config/request.xml b/Resources/config/request.xml
@@ -15,6 +15,7 @@
 
         <service id="fos_rest.request.query_fetcher" class="%fos_rest.request.query_fetcher.class%" scope="request">
             <argument type="service" id="service_container"/>
+            <argument type="service" id="controller_name_converter"/>
             <argument type="service" id="fos_rest.request.query_fetcher.reader.query_param" />
             <argument type="service" id="request"/>
         </service>

diff --git a/Tests/Request/QueryFetcherTest.php b/Tests/Request/QueryFetcherTest.php
@@ -36,6 +36,10 @@ public function setup()
             ->disableOriginalConstructor()
             ->getMock();
 
+        $this->parser = $this->getMockBuilder('\Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser')
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $this->queryParamReader = $this->getMockBuilder('\FOS\RestBundle\Request\QueryParamReader')
             ->disableOriginalConstructor()
             ->getMock();
@@ -74,7 +78,7 @@ public function getQueryFetcher($query = array(), $attributes = null)
 
         $request = new Request($query, array(), $attributes);
 
-        return new QueryFetcher($this->container, $this->queryParamReader, $request);
+        return new QueryFetcher($this->container, $this->parser, $this->queryParamReader, $request);
     }
 
     /**
@@ -105,12 +109,12 @@ public static function validatesConfiguredQueryParamDataProvider()
     }
 
     /**
-     * @expectedException        InvalidArgumentException
-     * @expectedExceptionMessage No _controller for request.
+     * @expectedException        LogicException
+     * @expectedExceptionMessage Unable to parse the controller name ""
      */
     public function testExceptionOnRequestWithoutControllerAttribute()
     {
-        $queryFetcher = new QueryFetcher($this->container, $this->queryParamReader, new Request());
+        $queryFetcher = new QueryFetcher($this->container, $this->parser, $this->queryParamReader, new Request());
         $queryFetcher->getParameter('qux', '42');
     }