Skip to content

Commit

Permalink
fix route generator not ignoring body param converters (#1934)
Browse files Browse the repository at this point in the history
* 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

* fix some small things after review

* simplify FQN

* fix cs

* revert global namespace FQN simplification
  • Loading branch information
tonivdv authored and GuilhemN committed Aug 21, 2018
1 parent 5e38509 commit 3725279
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 4 deletions.
27 changes: 23 additions & 4 deletions Routing/Loader/Reader/RestActionReader.php
Expand Up @@ -14,10 +14,14 @@
use Doctrine\Common\Annotations\Reader;
use FOS\RestBundle\Controller\Annotations\Route as RouteAnnotation;
use FOS\RestBundle\Inflector\InflectorInterface;
use FOS\RestBundle\Request\ParamFetcherInterface;
use FOS\RestBundle\Request\ParamReaderInterface;
use FOS\RestBundle\Routing\RestRouteCollection;
use Psr\Http\Message\MessageInterface;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Route;
use Symfony\Component\Validator\ConstraintViolationListInterface;

/**
* REST controller actions reader.
Expand Down Expand Up @@ -475,12 +479,23 @@ 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(ParamConverter::class)) {
$ignoreParameters = array_map(function ($annotation) {
return
$annotation instanceof 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,
\FOS\RestBundle\Request\ParamFetcherInterface::class,
\Symfony\Component\Validator\ConstraintViolationListInterface::class,
\Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter::class,
Request::class,
ParamFetcherInterface::class,
ConstraintViolationListInterface::class,
ParamConverter::class,
MessageInterface::class,
];

Expand All @@ -500,6 +515,10 @@ private function getMethodArguments(\ReflectionMethod $method)
}
}

if (in_array($argument->getName(), $ignoreParameters, true)) {
continue;
}

$arguments[] = $argument;
}

Expand Down
25 changes: 25 additions & 0 deletions 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;
}
11 changes: 11 additions & 0 deletions Tests/Routing/Loader/RestRouteLoaderTest.php
Expand Up @@ -370,6 +370,17 @@ 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->assertSame('/somethings.{_format}', $collection->get('post_something')->getPath());
}

/**
* Load routes collection from fixture class under Tests\Fixtures directory.
*
Expand Down

0 comments on commit 3725279

Please sign in to comment.