fix route generator not ignoring body param converters

When fetching a list of function parameters from a controller action for use as URL parameters, check for the @ParamConverter annotation. If a parameter has the annotation with the "fos_rest.request_body" converter, then ignore it and do not include it in the list of parameters (thus removing it from the URL). This allows automatic route generation and ParamConverter to co-exist, as otherwise a parameter that is expected to be derived from the body would also be included as a URL parameter, requiring the developer to manually specify the route. fixes #1198
FriendsOfSymfony · Aug 21, 2018 · cde9656 · cde9656
1 parent 5e38509
commit cde9656
Show file tree

Hide file tree

Showing 5 changed files with 161 additions and 0 deletions.
diff --git a/Routing/Loader/Reader/RestActionReader.php b/Routing/Loader/Reader/RestActionReader.php
@@ -475,6 +475,17 @@ private function getMethodArguments(\ReflectionMethod $method)
         // ignore all query params
         $params = $this->paramReader->getParamsFromMethod($method);
 
+        // check if a parameter is coming from the request body
+        $ignoreParameters = [];
+        if (class_exists('Sensio\\Bundle\\FrameworkExtraBundle\\Configuration\\ParamConverter')) {
+            $ignoreParameters = array_map(function ($annotation) {
+                return
+                    $annotation instanceof \Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter &&
+                    'fos_rest.request_body' === $annotation->getConverter()
+                        ? $annotation->getName() : null;
+            }, $this->annotationReader->getMethodAnnotations($method));
+        }
+
         // ignore several type hinted arguments
         $ignoreClasses = [
             \Symfony\Component\HttpFoundation\Request::class,
@@ -500,6 +511,10 @@ private function getMethodArguments(\ReflectionMethod $method)
                 }
             }
 
+            if (in_array($argument->getName(), $ignoreParameters)) {
+                continue;
+            }
+
             $arguments[] = $argument;
         }
 

diff --git a/Tests/Fixtures/Controller/ParamConverterController.php b/Tests/Fixtures/Controller/ParamConverterController.php
@@ -0,0 +1,25 @@
+<?php
+
+namespace FOS\RestBundle\Tests\Fixtures\Controller;
+
+use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
+
+/**
+ * @author Toni Van de Voorde <toni.vdv@gmail.com>
+ */
+class ParamConverterController
+{
+    /**
+     * @ParamConverter("something", converter="fos_rest.request_body")
+     *
+     * @param Something $something
+     */
+    public function postSomethingAction(Something $something)
+    {
+    }
+}
+
+final class Something
+{
+    public $id;
+}
diff --git a/Tests/Routing/Loader/Reader/RestActionReaderTest.php b/Tests/Routing/Loader/Reader/RestActionReaderTest.php
@@ -0,0 +1,39 @@
+<?php
+/*
+ * This file is part of the Adlogix package.
+ *
+ * (c) Allan Segebarth <allan@adlogix.eu>
+ * (c) Jean-Jacques Courtens <jjc@adlogix.eu>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\RestBundle\Tests\Routing\Loader\Reader;
+
+use Doctrine\Common\Annotations\Reader;
+use FOS\RestBundle\Inflector\InflectorInterface;
+use FOS\RestBundle\Request\ParamReaderInterface;
+use FOS\RestBundle\Routing\Loader\Reader\RestActionReader;
+use PHPUnit\Framework\TestCase;
+use PHPUnit_Framework_MockObject_MockObject;
+
+/**
+ * @author Toni Van de Voorde <toni@adlogix.eu>
+ */
+final class RestActionReaderTest extends TestCase
+{
+    public function testReadSupportParamConverter()
+    {
+        /** @var Reader|PHPUnit_Framework_MockObject_MockObject $annotationReader */
+        $annotationReader = $this->getMockBuilder(Reader::class)->getMock();
+
+        /** @var ParamReaderInterface|PHPUnit_Framework_MockObject_MockObject $paramReader */
+        $paramReader = $this->getMockBuilder(ParamReaderInterface::class)->getMock();
+
+        /** @var InflectorInterface|PHPUnit_Framework_MockObject_MockObject $inflector */
+        $inflector = $this->getMockBuilder(InflectorInterface::class)->getMock();
+
+        $restActionReader = new RestActionReader($annotationReader, $paramReader, $inflector, true);
+    }
+}
diff --git a/Tests/Routing/Loader/RestRouteLoaderTest.php b/Tests/Routing/Loader/RestRouteLoaderTest.php
@@ -370,6 +370,19 @@ public function testRequestTypeHintsIgnoredCorrectly()
         $this->assertEquals('/articles/{id}.{_format}', $collection->get('post_article')->getPath());
     }
 
+    /**
+     * @see https://github.com/FriendsOfSymfony/FOSRestBundle/issues/1198
+     */
+    public function testParamConverterIsIgnoredInRouteGenerationCorrectly()
+    {
+        $collection = $this->loadFromControllerFixture('ParamConverterController');
+
+        $this->assertNotNull($collection->get('post_something'), 'route for "post_something" does not exist');
+        $this->assertEquals('/somethings.{_format}', $collection->get('post_something')->getPath());
+
+        $this->assertTrue(true);
+    }
+
     /**
      * Load routes collection from fixture class under Tests\Fixtures directory.
      *

diff --git a/qx30lB.diff b/qx30lB.diff
@@ -0,0 +1,69 @@
+diff --git a/Routing/Loader/Reader/RestActionReader.php b/Routing/Loader/Reader/RestActionReader.php
+index 89156d79ce8e82a6d2569bcea614639abf02f7fa..b168576fe12fb0a126d5a928477185984ba110b2 100644
+--- a/Routing/Loader/Reader/RestActionReader.php
++++ b/Routing/Loader/Reader/RestActionReader.php
+@@ -481,7 +481,7 @@ class RestActionReader
+             $ignoreParameters = array_map(function ($annotation) {
+                 return
+                     $annotation instanceof \Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter &&
+-                    $annotation->getConverter() === 'fos_rest.request_body'
++                    'fos_rest.request_body' === $annotation->getConverter()
+                         ? $annotation->getName() : null;
+             }, $this->annotationReader->getMethodAnnotations($method));
+         }
+diff --git a/Tests/Fixtures/Controller/ParamConverterController.php b/Tests/Fixtures/Controller/ParamConverterController.php
+index 9e54e410b81acfe5dab708f8a9f110086ced9ac9..4e4adfe297a36ce685a197ce03b4ecb973a32aeb 100644
+--- a/Tests/Fixtures/Controller/ParamConverterController.php
++++ b/Tests/Fixtures/Controller/ParamConverterController.php
+@@ -2,7 +2,6 @@
+
+ namespace FOS\RestBundle\Tests\Fixtures\Controller;
+
+-
+ use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
+
+ /**
+@@ -12,14 +11,15 @@ class ParamConverterController
+ {
+     /**
+      * @ParamConverter("something", converter="fos_rest.request_body")
++     *
+      * @param Something $something
+      */
+     public function postSomethingAction(Something $something)
+     {
+-
+     }
+ }
+
+-final class Something {
++final class Something
++{
+     public $id;
+-}
+\ No newline at end of file
++}
+diff --git a/Tests/Routing/Loader/Reader/RestActionReaderTest.php b/Tests/Routing/Loader/Reader/RestActionReaderTest.php
+index cd8aa1256f0b2eaa289774cb187971d59538ee8a..9bda49e109114ec9325ad261a0bf1c13d0f3bbd4 100644
+--- a/Tests/Routing/Loader/Reader/RestActionReaderTest.php
++++ b/Tests/Routing/Loader/Reader/RestActionReaderTest.php
+@@ -15,7 +15,6 @@ use Doctrine\Common\Annotations\Reader;
+ use FOS\RestBundle\Inflector\InflectorInterface;
+ use FOS\RestBundle\Request\ParamReaderInterface;
+ use FOS\RestBundle\Routing\Loader\Reader\RestActionReader;
+-use FOS\RestBundle\Routing\RestRouteCollection;
+ use PHPUnit\Framework\TestCase;
+ use PHPUnit_Framework_MockObject_MockObject;
+
+@@ -34,8 +33,7 @@ final class RestActionReaderTest extends TestCase
+
+         /** @var InflectorInterface|PHPUnit_Framework_MockObject_MockObject $inflector */
+         $inflector = $this->getMockBuilder(InflectorInterface::class)->getMock();
+-        
+-        $restActionReader = new RestActionReader($annotationReader, $paramReader, $inflector, true);
+
++        $restActionReader = new RestActionReader($annotationReader, $paramReader, $inflector, true);
+     }
+-}
+\ No newline at end of file
++}