From 31e937aa797ce90f07d10a33c314700c64b951b1 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Thu, 8 Feb 2018 16:14:00 +0100 Subject: [PATCH 1/8] throw an exception if required filter is not set --- features/filter/filter_validation.feature | 15 ++++ .../Bundle/Resources/config/validator.xml | 8 ++ src/Filter/QueryParameterValidateListener.php | 77 +++++++++++++++++++ .../ApiPlatformExtensionTest.php | 1 + .../TestBundle/Entity/FilterValidator.php | 73 ++++++++++++++++++ .../Filter/NotRequiredBarFilter.php | 37 +++++++++ .../TestBundle/Filter/RequiredFooFilter.php | 37 +++++++++ tests/Fixtures/app/config/config_test.yml | 8 ++ 8 files changed, 256 insertions(+) create mode 100644 features/filter/filter_validation.feature create mode 100644 src/Filter/QueryParameterValidateListener.php create mode 100644 tests/Fixtures/TestBundle/Entity/FilterValidator.php create mode 100644 tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php create mode 100644 tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature new file mode 100644 index 00000000000..fc1c3a598d5 --- /dev/null +++ b/features/filter/filter_validation.feature @@ -0,0 +1,15 @@ +Feature: Validate filters based upon filter description + + @createSchema + Scenario: Required filter should not throw an error if set + When I am on "/filter_validators?foo=bar" + Then the response status code should be 200 + + When I am on "/filter_validators?foo=" + Then the response status code should be 200 + + @dropSchema + Scenario: Required filter should throw an error if not set + When I am on "/filter_validators" + Then the response status code should be 400 + And the JSON node "detail" should be equal to "query parameter `foo` is required" diff --git a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml index 8fcace8dc56..de68dad13d2 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml @@ -22,6 +22,14 @@ + + + + + + + + diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php new file mode 100644 index 00000000000..64c1147b290 --- /dev/null +++ b/src/Filter/QueryParameterValidateListener.php @@ -0,0 +1,77 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Filter; + +use ApiPlatform\Core\Api\FilterLocatorTrait; +use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; +use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Util\RequestAttributesExtractor; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\Validator\Constraints as Assert; +use Symfony\Component\Validator\Validator\ValidatorInterface as SymfonyValidatorInterface; + +/** + * Validate query parameters depending on filter description. + * + * @author Julien Deniau + */ +class QueryParameterValidateListener +{ + use FilterLocatorTrait; + + private $resourceMetadataFactory; + + private $validator; + + public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, SymfonyValidatorInterface $validator, $filterLocator) + { + $this->resourceMetadataFactory = $resourceMetadataFactory; + $this->validator = $validator; + $this->setFilterLocator($filterLocator); + } + + public function onKernelRequest(GetResponseEvent $event) + { + $request = $event->getRequest(); + if ( + !$request->isMethodSafe(false) + || !($attributes = RequestAttributesExtractor::extractAttributes($request)) + || !isset($attributes['collection_operation_name']) + || 'get' !== ($operationName = $attributes['collection_operation_name']) + ) { + return; + } + + $resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']); + $resourceFilters = $resourceMetadata->getCollectionOperationAttribute($operationName, 'filters', [], true); + + foreach ($resourceFilters as $filterId) { + if (!$filter = $this->getFilter($filterId)) { + continue; + } + + foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { + if ($data['required'] ?? false) { + $requiredConstraint = new Assert\NotNull(); + $requiredConstraint->message = sprintf('query parameter `%s` is required', $name); + $errorList = $this->validator->validate($request->query->get($name), $requiredConstraint); + + if (count($errorList) > 0) { + throw new ValidationException($errorList); + } + } + } + } + } +} diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index e57c7a07604..9af83478822 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -516,6 +516,7 @@ private function getPartialContainerBuilderProphecy($test = false) 'api_platform.listener.view.respond', 'api_platform.listener.view.serialize', 'api_platform.listener.view.validate', + 'ApiPlatform\Core\Filter\QueryParameterValidateListener', 'api_platform.listener.view.write', 'api_platform.metadata.extractor.xml', 'api_platform.metadata.property.metadata_factory.cached', diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php new file mode 100644 index 00000000000..629b39d6e0b --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -0,0 +1,73 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiProperty; +use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\NotRequiredBarFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFooFilter; +use Doctrine\ORM\Mapping as ORM; + +/** + * Filter Validator entity. + * + * @author Julien Deniau + * + * @ApiResource(attributes={ + * "filters"={ + * NotRequiredBarFilter::class, + * RequiredFooFilter::class + * } + * }) + * @ORM\Entity + */ +class FilterValidator +{ + /** + * @var int The id + * + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string A name + * + * @ORM\Column + * @ApiProperty(iri="http://schema.org/name") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function setId($id) + { + $this->id = $id; + } + + public function setName($name) + { + $this->name = $name; + } + + public function getName() + { + return $this->name; + } +} diff --git a/tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php b/tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php new file mode 100644 index 00000000000..3012dc2c247 --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; + +use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; +use Doctrine\ORM\QueryBuilder; + +class NotRequiredBarFilter extends AbstractFilter +{ + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) + { + } + + // This function is only used to hook in documentation generators (supported by Swagger and Hydra) + public function getDescription(string $resourceClass): array + { + return [ + 'bar' => [ + 'property' => 'bar', + 'type' => 'string', + 'required' => false, + ], + ]; + } +} diff --git a/tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php b/tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php new file mode 100644 index 00000000000..01e0ae5f20f --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; + +use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; +use Doctrine\ORM\QueryBuilder; + +class RequiredFooFilter extends AbstractFilter +{ + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) + { + } + + // This function is only used to hook in documentation generators (supported by Swagger and Hydra) + public function getDescription(string $resourceClass): array + { + return [ + 'foo' => [ + 'property' => 'foo', + 'type' => 'string', + 'required' => true, + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_test.yml b/tests/Fixtures/app/config/config_test.yml index 3762fa4c694..fc5975fb663 100644 --- a/tests/Fixtures/app/config/config_test.yml +++ b/tests/Fixtures/app/config/config_test.yml @@ -164,6 +164,14 @@ services: parent: 'api_platform.serializer.property_filter' tags: [ { name: 'api_platform.filter', id: 'my_dummy.property' } ] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFooFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] + + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\NotRequiredBarFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] + app.config_dummy_resource.action: class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Action\ConfigCustom' arguments: ['@api_platform.item_data_provider'] From 2367a47edb76a39b18287267b8508d81c897f4b6 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Thu, 15 Feb 2018 09:11:29 +0100 Subject: [PATCH 2/8] remove dependency on validator --- features/filter/filter_validation.feature | 6 +-- .../DependencyInjection/Configuration.php | 2 + .../Bundle/Resources/config/validator.xml | 1 - src/Exception/FilterValidationException.php | 36 ++++++++++++++++++ src/Filter/QueryParameterValidateListener.php | 26 ++++++------- .../ApiPlatformExtensionTest.php | 7 +++- .../DependencyInjection/ConfigurationTest.php | 2 + .../TestBundle/Entity/FilterValidator.php | 6 +-- ...quiredBarFilter.php => RequiredFilter.php} | 11 ++++-- .../TestBundle/Filter/RequiredFooFilter.php | 37 ------------------- tests/Fixtures/app/config/config_test.yml | 7 +--- 11 files changed, 72 insertions(+), 69 deletions(-) create mode 100644 src/Exception/FilterValidationException.php rename tests/Fixtures/TestBundle/Filter/{NotRequiredBarFilter.php => RequiredFilter.php} (78%) delete mode 100644 tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index fc1c3a598d5..e960a8abb36 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -2,14 +2,14 @@ Feature: Validate filters based upon filter description @createSchema Scenario: Required filter should not throw an error if set - When I am on "/filter_validators?foo=bar" + When I am on "/filter_validators?required=foo" Then the response status code should be 200 - When I am on "/filter_validators?foo=" + When I am on "/filter_validators?required=" Then the response status code should be 200 @dropSchema Scenario: Required filter should throw an error if not set When I am on "/filter_validators" Then the response status code should be 400 - And the JSON node "detail" should be equal to "query parameter `foo` is required" + And the JSON node "detail" should be equal to 'Query parameter "required" is required' diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index e426648b13a..b1b349e9bf9 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection; +use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Exception\InvalidArgumentException; use FOS\UserBundle\FOSUserBundle; use GraphQL\GraphQL; @@ -248,6 +249,7 @@ private function addExceptionToStatusSection(ArrayNodeDefinition $rootNode) ->defaultValue([ ExceptionInterface::class => Response::HTTP_BAD_REQUEST, InvalidArgumentException::class => Response::HTTP_BAD_REQUEST, + FilterValidationException::class => Response::HTTP_BAD_REQUEST, ]) ->info('The list of exceptions mapped to their HTTP status code.') ->normalizeKeys(false) diff --git a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml index de68dad13d2..d8ef9ab4845 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml @@ -25,7 +25,6 @@ - diff --git a/src/Exception/FilterValidationException.php b/src/Exception/FilterValidationException.php new file mode 100644 index 00000000000..aebe5e5e21e --- /dev/null +++ b/src/Exception/FilterValidationException.php @@ -0,0 +1,36 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Exception; + +/** + * Filter validation exception. + * + * @author Julien DENIAU + */ +class FilterValidationException extends \Exception implements ExceptionInterface +{ + private $constraintViolationList; + + public function __construct(array $constraintViolationList, string $message = '', int $code = 0, \Exception $previous = null) + { + $this->constraintViolationList = $constraintViolationList; + + parent::__construct($message ?: $this->__toString(), $code, $previous); + } + + public function __toString(): string + { + return implode("\n", $this->constraintViolationList); + } +} diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index 64c1147b290..d4e77dbbbd2 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -14,15 +14,14 @@ namespace ApiPlatform\Core\Filter; use ApiPlatform\Core\Api\FilterLocatorTrait; -use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; +use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Util\RequestAttributesExtractor; +use Psr\Container\ContainerInterface; use Symfony\Component\HttpKernel\Event\GetResponseEvent; -use Symfony\Component\Validator\Constraints as Assert; -use Symfony\Component\Validator\Validator\ValidatorInterface as SymfonyValidatorInterface; /** - * Validate query parameters depending on filter description. + * Validates query parameters depending on filter description. * * @author Julien Deniau */ @@ -32,12 +31,9 @@ class QueryParameterValidateListener private $resourceMetadataFactory; - private $validator; - - public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, SymfonyValidatorInterface $validator, $filterLocator) + public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, ContainerInterface $filterLocator) { $this->resourceMetadataFactory = $resourceMetadataFactory; - $this->validator = $validator; $this->setFilterLocator($filterLocator); } @@ -62,14 +58,14 @@ public function onKernelRequest(GetResponseEvent $event) } foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { - if ($data['required'] ?? false) { - $requiredConstraint = new Assert\NotNull(); - $requiredConstraint->message = sprintf('query parameter `%s` is required', $name); - $errorList = $this->validator->validate($request->query->get($name), $requiredConstraint); + $errorList = []; + + if (($data['required'] ?? false) && null === $request->query->get($name)) { + $errorList[] = sprintf('Query parameter "%s" is required', $name); + } - if (count($errorList) > 0) { - throw new ValidationException($errorList); - } + if ($errorList) { + throw new FilterValidationException($errorList); } } } diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 9af83478822..22ad96d1c9a 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -28,6 +28,7 @@ use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface; use ApiPlatform\Core\DataProvider\ItemDataProviderInterface; use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface; +use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; @@ -451,7 +452,11 @@ private function getPartialContainerBuilderProphecy($test = false) 'api_platform.description' => 'description', 'api_platform.error_formats' => ['jsonproblem' => ['application/problem+json'], 'jsonld' => ['application/ld+json']], 'api_platform.formats' => ['jsonld' => ['application/ld+json'], 'jsonhal' => ['application/hal+json']], - 'api_platform.exception_to_status' => [ExceptionInterface::class => Response::HTTP_BAD_REQUEST, InvalidArgumentException::class => Response::HTTP_BAD_REQUEST], + 'api_platform.exception_to_status' => [ + ExceptionInterface::class => Response::HTTP_BAD_REQUEST, + InvalidArgumentException::class => Response::HTTP_BAD_REQUEST, + FilterValidationException::class => Response::HTTP_BAD_REQUEST, + ], 'api_platform.title' => 'title', 'api_platform.version' => 'version', 'api_platform.allow_plain_identifiers' => false, diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ConfigurationTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ConfigurationTest.php index cde0a614204..eae598820a2 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ConfigurationTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ConfigurationTest.php @@ -14,6 +14,7 @@ namespace ApiPlatform\Core\Tests\Bridge\Symfony\Bundle\DependencyInjection; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Configuration; +use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Exception\InvalidArgumentException; use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Builder\TreeBuilder; @@ -67,6 +68,7 @@ public function testDefaultConfig() 'exception_to_status' => [ ExceptionInterface::class => Response::HTTP_BAD_REQUEST, InvalidArgumentException::class => Response::HTTP_BAD_REQUEST, + FilterValidationException::class => Response::HTTP_BAD_REQUEST, ], 'default_operation_path_resolver' => 'api_platform.operation_path_resolver.underscore', 'path_segment_name_generator' => 'api_platform.path_segment_name_generator.underscore', diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index 629b39d6e0b..094c768d36a 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -15,8 +15,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; -use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\NotRequiredBarFilter; -use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFooFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter; use Doctrine\ORM\Mapping as ORM; /** @@ -26,8 +25,7 @@ * * @ApiResource(attributes={ * "filters"={ - * NotRequiredBarFilter::class, - * RequiredFooFilter::class + * RequiredFilter::class * } * }) * @ORM\Entity diff --git a/tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php b/tests/Fixtures/TestBundle/Filter/RequiredFilter.php similarity index 78% rename from tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php rename to tests/Fixtures/TestBundle/Filter/RequiredFilter.php index 3012dc2c247..86954779f00 100644 --- a/tests/Fixtures/TestBundle/Filter/NotRequiredBarFilter.php +++ b/tests/Fixtures/TestBundle/Filter/RequiredFilter.php @@ -17,7 +17,7 @@ use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; -class NotRequiredBarFilter extends AbstractFilter +class RequiredFilter extends AbstractFilter { protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { @@ -27,8 +27,13 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB public function getDescription(string $resourceClass): array { return [ - 'bar' => [ - 'property' => 'bar', + 'required' => [ + 'property' => 'required', + 'type' => 'string', + 'required' => true, + ], + 'not-required' => [ + 'property' => 'not-required', 'type' => 'string', 'required' => false, ], diff --git a/tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php b/tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php deleted file mode 100644 index 01e0ae5f20f..00000000000 --- a/tests/Fixtures/TestBundle/Filter/RequiredFooFilter.php +++ /dev/null @@ -1,37 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -declare(strict_types=1); - -namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; - -use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; -use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; -use Doctrine\ORM\QueryBuilder; - -class RequiredFooFilter extends AbstractFilter -{ - protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) - { - } - - // This function is only used to hook in documentation generators (supported by Swagger and Hydra) - public function getDescription(string $resourceClass): array - { - return [ - 'foo' => [ - 'property' => 'foo', - 'type' => 'string', - 'required' => true, - ], - ]; - } -} diff --git a/tests/Fixtures/app/config/config_test.yml b/tests/Fixtures/app/config/config_test.yml index fc5975fb663..0c8ac67d36f 100644 --- a/tests/Fixtures/app/config/config_test.yml +++ b/tests/Fixtures/app/config/config_test.yml @@ -55,6 +55,7 @@ api_platform: exception_to_status: Symfony\Component\Serializer\Exception\ExceptionInterface: 400 ApiPlatform\Core\Exception\InvalidArgumentException: 400 + ApiPlatform\Core\Exception\FilterValidationException: 400 # Use this syntax with Symfony YAML 3.4+: #ApiPlatform\Core\Exception\InvalidArgumentException: !php/const Symfony\Component\HttpFoundation\Response::HTTP_BAD_REQUEST http_cache: @@ -164,11 +165,7 @@ services: parent: 'api_platform.serializer.property_filter' tags: [ { name: 'api_platform.filter', id: 'my_dummy.property' } ] - ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFooFilter: - arguments: [ '@doctrine' ] - tags: [ 'api_platform.filter' ] - - ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\NotRequiredBarFilter: + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter: arguments: [ '@doctrine' ] tags: [ 'api_platform.filter' ] From da476c1d0cd02f96a059a7186ec36710f86d5ee5 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 9 Mar 2018 00:02:01 +0100 Subject: [PATCH 3/8] fix issue with array notation filter name --- features/filter/filter_validation.feature | 26 ++++++- src/Filter/QueryParameterValidateListener.php | 21 +++++- .../Entity/ArrayFilterValidator.php | 71 +++++++++++++++++++ .../TestBundle/Filter/ArrayRequiredFilter.php | 42 +++++++++++ tests/Fixtures/app/config/config_test.yml | 4 ++ 5 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php create mode 100644 tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index e960a8abb36..2a47576d29d 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -8,8 +8,32 @@ Feature: Validate filters based upon filter description When I am on "/filter_validators?required=" Then the response status code should be 200 - @dropSchema Scenario: Required filter should throw an error if not set When I am on "/filter_validators" Then the response status code should be 400 And the JSON node "detail" should be equal to 'Query parameter "required" is required' + + Scenario: Required filter should not throw an error if set + When I am on "/array_filter_validators?arrayRequired[]=foo&indexedArrayRequired[foo]=foo" + Then the response status code should be 200 + + Scenario: Required filter should throw an error if not set + When I am on "/array_filter_validators" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "arrayRequired[]" is required' + + When I am on "/array_filter_validators?arrayRequired=foo&indexedArrayRequired[foo]=foo" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "arrayRequired[]" is required' + + When I am on "/array_filter_validators?arrayRequired[foo]=foo" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "arrayRequired[]" is required' + + When I am on "/array_filter_validators?arrayRequired[]=foo" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "indexedArrayRequired[foo]" is required' + + When I am on "/array_filter_validators?arrayRequired[]=foo&indexedArrayRequired[bar]=bar" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "indexedArrayRequired[foo]" is required' diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index d4e77dbbbd2..d3f937da43b 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -60,7 +60,15 @@ public function onKernelRequest(GetResponseEvent $event) foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { $errorList = []; - if (($data['required'] ?? false) && null === $request->query->get($name)) { + if (!($data['required'] ?? false)) { // property is not required + continue; + } + + if (false !== strpos($name, '[')) { // array notation of filter + if (!$this->isArrayNotationFilterValid($name, $request)) { + $errorList[] = sprintf('Query parameter "%s" is required', $name); + } + } elseif (null === $request->query->get($name)) { $errorList[] = sprintf('Query parameter "%s" is required', $name); } @@ -70,4 +78,15 @@ public function onKernelRequest(GetResponseEvent $event) } } } + + private function isArrayNotationFilterValid($name, $request): bool + { + $matches = []; + preg_match('/([^[]+)\[(.*)\]/', $name, $matches); + list(, $rootName, $keyName) = $matches; + $keyName = $keyName ?: 0; // array without index should test the first key + $queryParameter = $request->query->get($rootName); + + return is_array($queryParameter) && isset($queryParameter[$keyName]); + } } diff --git a/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php b/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php new file mode 100644 index 00000000000..a47788649f3 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php @@ -0,0 +1,71 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiProperty; +use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\ArrayRequiredFilter; +use Doctrine\ORM\Mapping as ORM; + +/** + * Filter Validator entity. + * + * @author Julien Deniau + * + * @ApiResource(attributes={ + * "filters"={ + * ArrayRequiredFilter::class + * } + * }) + * @ORM\Entity + */ +class ArrayFilterValidator +{ + /** + * @var int The id + * + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string A name + * + * @ORM\Column + * @ApiProperty(iri="http://schema.org/name") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function setId($id) + { + $this->id = $id; + } + + public function setName($name) + { + $this->name = $name; + } + + public function getName() + { + return $this->name; + } +} diff --git a/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php b/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php new file mode 100644 index 00000000000..025e0a6c205 --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; + +use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; +use Doctrine\ORM\QueryBuilder; + +class ArrayRequiredFilter extends AbstractFilter +{ + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) + { + } + + // This function is only used to hook in documentation generators (supported by Swagger and Hydra) + public function getDescription(string $resourceClass): array + { + return [ + 'arrayRequired[]' => [ + 'property' => 'arrayRequired', + 'type' => 'string', + 'required' => true, + ], + 'indexedArrayRequired[foo]' => [ + 'property' => 'indexedArrayRequired', + 'type' => 'string', + 'required' => true, + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_test.yml b/tests/Fixtures/app/config/config_test.yml index 0c8ac67d36f..37f6b0349c2 100644 --- a/tests/Fixtures/app/config/config_test.yml +++ b/tests/Fixtures/app/config/config_test.yml @@ -169,6 +169,10 @@ services: arguments: [ '@doctrine' ] tags: [ 'api_platform.filter' ] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\ArrayRequiredFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] + app.config_dummy_resource.action: class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Action\ConfigCustom' arguments: ['@api_platform.item_data_provider'] From 261e5226ef9487bf6b2eeb69aa6ca2a44a64ef9c Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 9 Mar 2018 09:57:25 +0100 Subject: [PATCH 4/8] add unit test on QueryParameterValidateListener --- .../QueryParameterValidateListenerTest.php | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 tests/Filter/QueryParameterValidateListenerTest.php diff --git a/tests/Filter/QueryParameterValidateListenerTest.php b/tests/Filter/QueryParameterValidateListenerTest.php new file mode 100644 index 00000000000..258cb67137a --- /dev/null +++ b/tests/Filter/QueryParameterValidateListenerTest.php @@ -0,0 +1,174 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Filter; + +use ApiPlatform\Core\Api\FilterInterface; +use ApiPlatform\Core\Exception\FilterValidationException; +use ApiPlatform\Core\Filter\QueryParameterValidateListener; +use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; + +class QueryParameterValidateListenerTest extends TestCase +{ + private $testedInstance; + + private $filterLocatorProphecy; + + /** + * unsafe method should not use filter validations. + */ + public function testOnKernelRequestWithUnsafeMethod() + { + $this->setUpWithFilters(); + + $request = new Request(); + $request->setMethod('POST'); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + + $this->assertNull( + $this->testedInstance->onKernelRequest($eventProphecy->reveal()) + ); + } + + /** + * If the tested filter is non-existant, then nothing should append. + */ + public function testOnKernelRequestWithWrongFilter() + { + $this->setUpWithFilters(['some_inexistent_filter']); + + $request = new Request([], [], ['_api_resource_class' => Dummy::class, '_api_collection_operation_name' => 'get']); + $request->setMethod('GET'); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + + $this->filterLocatorProphecy->has('some_inexistent_filter')->shouldBeCalled(); + $this->filterLocatorProphecy->get('some_inexistent_filter')->shouldNotBeCalled(); + + $this->assertNull( + $this->testedInstance->onKernelRequest($eventProphecy->reveal()) + ); + } + + /** + * if the required parameter is not set, throw an FilterValidationException. + */ + public function testOnKernelRequestWithRequiredFilterNotSet() + { + $this->setUpWithFilters(['some_filter']); + + $request = new Request([], [], ['_api_resource_class' => Dummy::class, '_api_collection_operation_name' => 'get']); + $request->setMethod('GET'); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + + $this->filterLocatorProphecy + ->has('some_filter') + ->shouldBeCalled() + ->willReturn(true) + ; + $filterProphecy = $this->prophesize(FilterInterface::class); + $filterProphecy + ->getDescription(Dummy::class) + ->shouldBeCalled() + ->willReturn([ + 'required' => [ + 'required' => true, + ], + ]) + ; + $this->filterLocatorProphecy + ->get('some_filter') + ->shouldBeCalled() + ->willReturn($filterProphecy->reveal()) + ; + + $this->expectException(FilterValidationException::class); + $this->expectExceptionMessage('Query parameter "required" is required'); + $this->testedInstance->onKernelRequest($eventProphecy->reveal()); + } + + /** + * if the required parameter is set, no exception should be throwned. + */ + public function testOnKernelRequestWithRequiredFilter() + { + $this->setUpWithFilters(['some_filter']); + + $request = new Request( + ['required' => 'foo'], + [], + ['_api_resource_class' => Dummy::class, '_api_collection_operation_name' => 'get'] + ); + $request->setMethod('GET'); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + + $this->filterLocatorProphecy + ->has('some_filter') + ->shouldBeCalled() + ->willReturn(true) + ; + $filterProphecy = $this->prophesize(FilterInterface::class); + $filterProphecy + ->getDescription(Dummy::class) + ->shouldBeCalled() + ->willReturn([ + 'required' => [ + 'required' => true, + ], + ]) + ; + $this->filterLocatorProphecy + ->get('some_filter') + ->shouldBeCalled() + ->willReturn($filterProphecy->reveal()) + ; + + $this->assertNull( + $this->testedInstance->onKernelRequest($eventProphecy->reveal()) + ); + } + + private function setUpWithFilters(array $filters = []) + { + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy + ->create(Dummy::class) + ->willReturn( + (new ResourceMetadata('dummy')) + ->withAttributes([ + 'filters' => $filters, + ]) + ) + ; + + $this->filterLocatorProphecy = $this->prophesize(ContainerInterface::class); + + $this->testedInstance = new QueryParameterValidateListener( + $resourceMetadataFactoryProphecy->reveal(), + $this->filterLocatorProphecy->reveal() + ); + } +} From d62466577b9e336aa654dcfb2a88117bc6604715 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 23 Mar 2018 15:18:35 +0100 Subject: [PATCH 5/8] use parse_str instead of preg_match --- src/Filter/QueryParameterValidateListener.php | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index d3f937da43b..bd821b955b3 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -64,11 +64,7 @@ public function onKernelRequest(GetResponseEvent $event) continue; } - if (false !== strpos($name, '[')) { // array notation of filter - if (!$this->isArrayNotationFilterValid($name, $request)) { - $errorList[] = sprintf('Query parameter "%s" is required', $name); - } - } elseif (null === $request->query->get($name)) { + if (!$this->isRequiredFilterValid($name, $request)) { $errorList[] = sprintf('Query parameter "%s" is required', $name); } @@ -79,14 +75,30 @@ public function onKernelRequest(GetResponseEvent $event) } } - private function isArrayNotationFilterValid($name, $request): bool + /** + * Test if required filter is valid. It validates array notation too like "required[bar]". + */ + private function isRequiredFilterValid($name, $request): bool { $matches = []; - preg_match('/([^[]+)\[(.*)\]/', $name, $matches); - list(, $rootName, $keyName) = $matches; - $keyName = $keyName ?: 0; // array without index should test the first key - $queryParameter = $request->query->get($rootName); + parse_str($name, $matches); + if (empty($matches)) { + return false; + } + + $rootName = array_keys($matches)[0] ?? ''; + if (!$rootName) { + return false; + } + + if (is_array($matches[$rootName])) { + $keyName = array_keys($matches[$rootName])[0]; + + $queryParameter = $request->query->get($rootName); + + return is_array($queryParameter) && isset($queryParameter[$keyName]); + } - return is_array($queryParameter) && isset($queryParameter[$keyName]); + return null !== $request->query->get($rootName); } } From 0186719584cc23dae7281adba9b0fd81a9ea1bc2 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Sat, 7 Apr 2018 15:06:00 +0200 Subject: [PATCH 6/8] is_array -> \is_array --- src/Filter/QueryParameterValidateListener.php | 6 +++--- tests/Filter/QueryParameterValidateListenerTest.php | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index bd821b955b3..1e7eea7b0c8 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -82,7 +82,7 @@ private function isRequiredFilterValid($name, $request): bool { $matches = []; parse_str($name, $matches); - if (empty($matches)) { + if (!$matches) { return false; } @@ -91,12 +91,12 @@ private function isRequiredFilterValid($name, $request): bool return false; } - if (is_array($matches[$rootName])) { + if (\is_array($matches[$rootName])) { $keyName = array_keys($matches[$rootName])[0]; $queryParameter = $request->query->get($rootName); - return is_array($queryParameter) && isset($queryParameter[$keyName]); + return \is_array($queryParameter) && isset($queryParameter[$keyName]); } return null !== $request->query->get($rootName); diff --git a/tests/Filter/QueryParameterValidateListenerTest.php b/tests/Filter/QueryParameterValidateListenerTest.php index 258cb67137a..573e8d1ff72 100644 --- a/tests/Filter/QueryParameterValidateListenerTest.php +++ b/tests/Filter/QueryParameterValidateListenerTest.php @@ -27,7 +27,6 @@ class QueryParameterValidateListenerTest extends TestCase { private $testedInstance; - private $filterLocatorProphecy; /** From fe516ffd22f6ab127b9e6e2ee1098d824af7c6f5 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Sat, 7 Apr 2018 15:26:56 +0200 Subject: [PATCH 7/8] error list throw all erreors (not only the first one) --- features/filter/filter_validation.feature | 4 ++-- src/Filter/QueryParameterValidateListener.php | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index 2a47576d29d..ab85b45dd5d 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -20,7 +20,7 @@ Feature: Validate filters based upon filter description Scenario: Required filter should throw an error if not set When I am on "/array_filter_validators" Then the response status code should be 400 - And the JSON node "detail" should be equal to 'Query parameter "arrayRequired[]" is required' + And the JSON node "detail" should match '/^Query parameter "arrayRequired\[\]" is required\nQuery parameter "indexedArrayRequired\[foo\]" is required$/' When I am on "/array_filter_validators?arrayRequired=foo&indexedArrayRequired[foo]=foo" Then the response status code should be 400 @@ -28,7 +28,7 @@ Feature: Validate filters based upon filter description When I am on "/array_filter_validators?arrayRequired[foo]=foo" Then the response status code should be 400 - And the JSON node "detail" should be equal to 'Query parameter "arrayRequired[]" is required' + And the JSON node "detail" should match '/^Query parameter "arrayRequired\[\]" is required\nQuery parameter "indexedArrayRequired\[foo\]" is required$/' When I am on "/array_filter_validators?arrayRequired[]=foo" Then the response status code should be 400 diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index 1e7eea7b0c8..d31efd6971d 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -52,14 +52,13 @@ public function onKernelRequest(GetResponseEvent $event) $resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']); $resourceFilters = $resourceMetadata->getCollectionOperationAttribute($operationName, 'filters', [], true); + $errorList = []; foreach ($resourceFilters as $filterId) { if (!$filter = $this->getFilter($filterId)) { continue; } foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { - $errorList = []; - if (!($data['required'] ?? false)) { // property is not required continue; } @@ -67,12 +66,12 @@ public function onKernelRequest(GetResponseEvent $event) if (!$this->isRequiredFilterValid($name, $request)) { $errorList[] = sprintf('Query parameter "%s" is required', $name); } - - if ($errorList) { - throw new FilterValidationException($errorList); - } } } + + if ($errorList) { + throw new FilterValidationException($errorList); + } } /** From 1e353afe562cda4a1cae025d3dd014c86927b64d Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 25 May 2018 16:27:56 +0200 Subject: [PATCH 8/8] make classes final + consistency in service name --- src/Bridge/Symfony/Bundle/Resources/config/validator.xml | 2 +- src/Exception/FilterValidationException.php | 2 +- src/Filter/QueryParameterValidateListener.php | 2 +- .../DependencyInjection/ApiPlatformExtensionTest.php | 2 +- tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php | 5 ----- tests/Fixtures/TestBundle/Entity/FilterValidator.php | 5 ----- tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php | 2 +- tests/Fixtures/TestBundle/Filter/RequiredFilter.php | 2 +- tests/Fixtures/app/config/config_test.yml | 8 ++++---- 9 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml index d8ef9ab4845..d34dcd8cfe6 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml @@ -23,7 +23,7 @@ - + diff --git a/src/Exception/FilterValidationException.php b/src/Exception/FilterValidationException.php index aebe5e5e21e..567f8ec32e9 100644 --- a/src/Exception/FilterValidationException.php +++ b/src/Exception/FilterValidationException.php @@ -18,7 +18,7 @@ * * @author Julien DENIAU */ -class FilterValidationException extends \Exception implements ExceptionInterface +final class FilterValidationException extends \Exception implements ExceptionInterface { private $constraintViolationList; diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index d31efd6971d..c0cb227be46 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -25,7 +25,7 @@ * * @author Julien Deniau */ -class QueryParameterValidateListener +final class QueryParameterValidateListener { use FilterLocatorTrait; diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 22ad96d1c9a..bc77ba5dcd5 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -521,7 +521,7 @@ private function getPartialContainerBuilderProphecy($test = false) 'api_platform.listener.view.respond', 'api_platform.listener.view.serialize', 'api_platform.listener.view.validate', - 'ApiPlatform\Core\Filter\QueryParameterValidateListener', + 'api_platform.listener.view.validate_query_parameters', 'api_platform.listener.view.write', 'api_platform.metadata.extractor.xml', 'api_platform.metadata.property.metadata_factory.cached', diff --git a/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php b/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php index a47788649f3..f0f705c28e3 100644 --- a/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/ArrayFilterValidator.php @@ -54,11 +54,6 @@ public function getId() return $this->id; } - public function setId($id) - { - $this->id = $id; - } - public function setName($name) { $this->name = $name; diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index 094c768d36a..118050a9b8f 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -54,11 +54,6 @@ public function getId() return $this->id; } - public function setId($id) - { - $this->id = $id; - } - public function setName($name) { $this->name = $name; diff --git a/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php b/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php index 025e0a6c205..887d6bdbdaa 100644 --- a/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php +++ b/tests/Fixtures/TestBundle/Filter/ArrayRequiredFilter.php @@ -17,7 +17,7 @@ use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; -class ArrayRequiredFilter extends AbstractFilter +final class ArrayRequiredFilter extends AbstractFilter { protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { diff --git a/tests/Fixtures/TestBundle/Filter/RequiredFilter.php b/tests/Fixtures/TestBundle/Filter/RequiredFilter.php index 86954779f00..83644a711e2 100644 --- a/tests/Fixtures/TestBundle/Filter/RequiredFilter.php +++ b/tests/Fixtures/TestBundle/Filter/RequiredFilter.php @@ -17,7 +17,7 @@ use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; -class RequiredFilter extends AbstractFilter +final class RequiredFilter extends AbstractFilter { protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { diff --git a/tests/Fixtures/app/config/config_test.yml b/tests/Fixtures/app/config/config_test.yml index 37f6b0349c2..0f772a12cea 100644 --- a/tests/Fixtures/app/config/config_test.yml +++ b/tests/Fixtures/app/config/config_test.yml @@ -166,12 +166,12 @@ services: tags: [ { name: 'api_platform.filter', id: 'my_dummy.property' } ] ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter: - arguments: [ '@doctrine' ] - tags: [ 'api_platform.filter' ] + arguments: ['@doctrine'] + tags: ['api_platform.filter'] ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\ArrayRequiredFilter: - arguments: [ '@doctrine' ] - tags: [ 'api_platform.filter' ] + arguments: ['@doctrine'] + tags: ['api_platform.filter'] app.config_dummy_resource.action: class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Action\ConfigCustom'