Skip to content

Commit

Permalink
use internal RequestParser instead of $request->query->all()
Browse files Browse the repository at this point in the history
  • Loading branch information
jdeniau committed Jan 28, 2019
1 parent 5ab40f2 commit 6412b76
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 18 deletions.
5 changes: 4 additions & 1 deletion src/EventListener/QueryParameterValidateListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use ApiPlatform\Core\Filter\QueryParameterValidator;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use ApiPlatform\Core\Util\RequestAttributesExtractor;
use ApiPlatform\Core\Util\RequestParser;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;

/**
Expand Down Expand Up @@ -46,10 +47,12 @@ public function onKernelRequest(GetResponseEvent $event)
) {
return;
}
$queryString = RequestParser::getQueryString($request);
$queryParameters = $queryString ? RequestParser::parseRequestParams($queryString) : [];

$resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']);
$resourceFilters = $resourceMetadata->getCollectionOperationAttribute($operationName, 'filters', [], true);

$this->queryParameterValidator->validateFilters($attributes['resource_class'], $resourceFilters, $request);
$this->queryParameterValidator->validateFilters($attributes['resource_class'], $resourceFilters, $queryParameters);
}
}
6 changes: 3 additions & 3 deletions src/Filter/QueryParameterValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use ApiPlatform\Core\Api\FilterLocatorTrait;
use ApiPlatform\Core\Exception\FilterValidationException;
use Psr\Container\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;

/**
* Validates query parameters depending on filter description.
Expand Down Expand Up @@ -44,17 +43,18 @@ public function __construct(ContainerInterface $filterLocator)
];
}

public function validateFilters(string $resourceClass, array $resourceFilters, Request $request): void
public function validateFilters(string $resourceClass, array $resourceFilters, array $queryParameters): void
{
$errorList = [];

foreach ($resourceFilters as $filterId) {
if (!$filter = $this->getFilter($filterId)) {
continue;
}

foreach ($filter->getDescription($resourceClass) as $name => $data) {
foreach ($this->validators as $validator) {
$errorList = array_merge($errorList, $validator->validate($name, $data, $request->query->all()));
$errorList = array_merge($errorList, $validator->validate($name, $data, $queryParameters));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Filter/Validator/Required.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private function requestHasQueryParameter(array $queryParameters, string $name):
if (\is_array($matches[$rootName])) {
$keyName = array_keys($matches[$rootName])[0];

$queryParameter = $queryParameters[(string) $rootName];
$queryParameter = $queryParameters[(string) $rootName] ?? null;

return \is_array($queryParameter) && isset($queryParameter[$keyName]);
}
Expand Down Expand Up @@ -90,7 +90,7 @@ private function requestGetQueryParameter(array $queryParameters, string $name)
if (\is_array($matches[$rootName])) {
$keyName = array_keys($matches[$rootName])[0];

$queryParameter = $queryParameters[(string) $rootName];
$queryParameter = $queryParameters[(string) $rootName] ?? null;

if (\is_array($queryParameter) && isset($queryParameter[$keyName])) {
return $queryParameter[$keyName];
Expand Down
13 changes: 8 additions & 5 deletions tests/EventListener/QueryParameterValidateListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function testOnKernelRequestWithWrongFilter()
$eventProphecy = $this->prophesize(GetResponseEvent::class);
$eventProphecy->getRequest()->willReturn($request)->shouldBeCalled();

$this->queryParameterValidor->validateFilters(Dummy::class, ['some_inexistent_filter'], $request)->shouldBeCalled();
$this->queryParameterValidor->validateFilters(Dummy::class, ['some_inexistent_filter'], [])->shouldBeCalled();

$this->assertNull(
$this->testedInstance->onKernelRequest($eventProphecy->reveal())
Expand All @@ -80,7 +80,7 @@ public function testOnKernelRequestWithRequiredFilterNotSet()
$eventProphecy->getRequest()->willReturn($request)->shouldBeCalled();

$this->queryParameterValidor
->validateFilters(Dummy::class, ['some_filter'], $request)
->validateFilters(Dummy::class, ['some_filter'], [])
->shouldBeCalled()
->willThrow(new FilterValidationException(['Query parameter "required" is required']))
;
Expand All @@ -97,17 +97,20 @@ public function testOnKernelRequestWithRequiredFilter()
$this->setUpWithFilters(['some_filter']);

$request = new Request(
['required' => 'foo'],
[],
['_api_resource_class' => Dummy::class, '_api_collection_operation_name' => 'get']
[],
['_api_resource_class' => Dummy::class, '_api_collection_operation_name' => 'get'],
[],
[],
['QUERY_STRING' => 'required=foo']
);
$request->setMethod('GET');

$eventProphecy = $this->prophesize(GetResponseEvent::class);
$eventProphecy->getRequest()->willReturn($request)->shouldBeCalled();

$this->queryParameterValidor
->validateFilters(Dummy::class, ['some_filter'], $request)
->validateFilters(Dummy::class, ['some_filter'], ['required' => 'foo'])
->shouldBeCalled()
;

Expand Down
11 changes: 4 additions & 7 deletions tests/Filter/QueryParameterValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;

/**
* Class QueryParameterValidatorTest.
Expand All @@ -45,7 +44,7 @@ public function setUp()
*/
public function testOnKernelRequestWithUnsafeMethod()
{
$request = new Request();
$request = [];

$this->assertNull(
$this->testedInstance->validateFilters(Dummy::class, [], $request)
Expand All @@ -57,7 +56,7 @@ public function testOnKernelRequestWithUnsafeMethod()
*/
public function testOnKernelRequestWithWrongFilter()
{
$request = new Request();
$request = [];

$this->assertNull(
$this->testedInstance->validateFilters(Dummy::class, ['some_inexistent_filter'], $request)
Expand All @@ -69,7 +68,7 @@ public function testOnKernelRequestWithWrongFilter()
*/
public function testOnKernelRequestWithRequiredFilterNotSet()
{
$request = new Request();
$request = [];

$filterProphecy = $this->prophesize(FilterInterface::class);
$filterProphecy
Expand Down Expand Up @@ -102,9 +101,7 @@ public function testOnKernelRequestWithRequiredFilterNotSet()
*/
public function testOnKernelRequestWithRequiredFilter()
{
$request = new Request(
['required' => 'foo']
);
$request = ['required' => 'foo'];

$this->filterLocatorProphecy
->has('some_filter')
Expand Down

0 comments on commit 6412b76

Please sign in to comment.