From e16d296550ba7eccd90cd9a4ff16044762940646 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Tue, 20 Feb 2018 00:28:39 +0100 Subject: [PATCH 01/16] test `allowEmptyValue` in filter validation --- features/filter/filter_validation.feature | 10 +-- src/Filter/QueryParameterValidateListener.php | 67 ++++++++++++++++--- .../TestBundle/Entity/FilterValidator.php | 4 +- .../Filter/RequiredAllowEmptyFilter.php | 40 +++++++++++ tests/Fixtures/app/config/config_common.yml | 4 ++ 5 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index ab85b45dd5d..258022fdfe0 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -2,16 +2,18 @@ Feature: Validate filters based upon filter description @createSchema Scenario: Required filter should not throw an error if set - When I am on "/filter_validators?required=foo" + When I am on "/filter_validators?required=foo&required-allow-empty=&arrayRequired[foo]=" Then the response status code should be 200 - When I am on "/filter_validators?required=" - Then the response status code should be 200 + Scenario: Required filter that does not allow empty value should throw an error if empty + When I am on "/filter_validators?required=&required-allow-empty=&arrayRequired[foo]=" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "required" does not allow empty value' 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' + Then the JSON node "detail" should match '/^Query parameter "required" is required\nQuery parameter "required-allow-empty" is required$/' Scenario: Required filter should not throw an error if set When I am on "/array_filter_validators?arrayRequired[]=foo&indexedArrayRequired[foo]=foo" diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index c6583a02c44..7d6e39e77c0 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -60,13 +60,7 @@ public function onKernelRequest(RequestEvent $event): void } foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { - if (!($data['required'] ?? false)) { // property is not required - continue; - } - - if (!$this->isRequiredFilterValid($name, $request)) { - $errorList[] = sprintf('Query parameter "%s" is required', $name); - } + $errorList = $this->checkRequired($errorList, $name, $data, $request); } } @@ -75,10 +69,32 @@ public function onKernelRequest(RequestEvent $event): void } } + private function checkRequired(array $errorList, string $name, array $data, Request $request): array + { + // filter is not required, the `checkRequired` method can not break + if (!($data['required'] ?? false)) { + return $errorList; + } + + // if query param is not given, then break + if (!$this->requestHasQueryParameter($request, $name)) { + $errorList[] = sprintf('Query parameter "%s" is required', $name); + + return $errorList; + } + + // if query param is empty and the configuration does not allow it + if (!($data['swagger']['allowEmptyValue'] ?? false) && empty($this->requestGetQueryParameter($request, $name))) { + $errorList[] = sprintf('Query parameter "%s" does not allow empty value', $name); + } + + return $errorList; + } + /** - * Test if required filter is valid. It validates array notation too like "required[bar]". + * Test if request has required parameter. */ - private function isRequiredFilterValid(string $name, Request $request): bool + private function requestHasQueryParameter(Request $request, string $name): bool { $matches = []; parse_str($name, $matches); @@ -99,6 +115,37 @@ private function isRequiredFilterValid(string $name, Request $request): bool return \is_array($queryParameter) && isset($queryParameter[$keyName]); } - return null !== $request->query->get($rootName); + return $request->query->has($rootName); + } + + /** + * Test if required filter is valid. It validates array notation too like "required[bar]". + */ + private function requestGetQueryParameter(Request $request, string $name) + { + $matches = []; + parse_str($name, $matches); + if (!$matches) { + return null; + } + + $rootName = array_keys($matches)[0] ?? ''; + if (!$rootName) { + return null; + } + + if (\is_array($matches[$rootName])) { + $keyName = array_keys($matches[$rootName])[0]; + + $queryParameter = $request->query->get($rootName); + + if (\is_array($queryParameter) && isset($queryParameter[$keyName])) { + return $queryParameter[$keyName]; + } + + return null; + } + + return $request->query->get($rootName); } } diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index 118050a9b8f..ae1ca5a1110 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter; use Doctrine\ORM\Mapping as ORM; @@ -25,7 +26,8 @@ * * @ApiResource(attributes={ * "filters"={ - * RequiredFilter::class + * RequiredFilter::class, + * RequiredAllowEmptyFilter::class * } * }) * @ORM\Entity diff --git a/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php b/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php new file mode 100644 index 00000000000..d0dc25882dd --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php @@ -0,0 +1,40 @@ + + * + * 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 RequiredAllowEmptyFilter 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 [ + 'required-allow-empty' => [ + 'property' => 'required-allow-empty', + 'type' => 'string', + 'required' => true, + 'swagger' => [ + 'allowEmptyValue' => true, + ], + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index e4c861cbb4a..92a009625f9 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -141,6 +141,10 @@ services: arguments: ['@doctrine'] tags: ['api_platform.filter'] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\: resource: '../../TestBundle/Controller' tags: ['controller.service_arguments'] From 44009bb27ae1b6631faaa563a1b720e14008c1c2 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Tue, 20 Feb 2018 01:28:46 +0100 Subject: [PATCH 02/16] add bounds filter validator --- features/filter/filter_validation.feature | 33 ++++++++++ src/Filter/QueryParameterValidateListener.php | 30 +++++++++ .../TestBundle/Entity/FilterValidator.php | 2 + .../TestBundle/Filter/BoundsFilter.php | 66 +++++++++++++++++++ tests/Fixtures/app/config/config_common.yml | 4 ++ 5 files changed, 135 insertions(+) create mode 100644 tests/Fixtures/TestBundle/Filter/BoundsFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index 258022fdfe0..93fa4379b95 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -39,3 +39,36 @@ Feature: Validate filters based upon filter description 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' + + Scenario: Test filter bounds: maximum + When I am on "/filter_validators?required=foo&required-allow-empty&maximum=10" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&maximum=11" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "maximum" must be less than or equal to 10' + + Scenario: Test filter bounds: exclusiveMaximum + When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMaximum=9" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMaximum=10" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "exclusiveMaximum" must be less than 10' + + Scenario: Test filter bounds: minimum + When I am on "/filter_validators?required=foo&required-allow-empty&minimum=5" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&minimum=0" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "minimum" must be greater than or equal to 5' + + @dropSchema + Scenario: Test filter bounds: exclusiveMinimum + When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMinimum=6" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMinimum=5" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "exclusiveMinimum" must be greater than 5' diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index 7d6e39e77c0..6647ce90dbd 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -61,6 +61,7 @@ public function onKernelRequest(RequestEvent $event): void foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { $errorList = $this->checkRequired($errorList, $name, $data, $request); + $errorList = $this->checkBounds($errorList, $name, $data, $request); } } @@ -148,4 +149,33 @@ private function requestGetQueryParameter(Request $request, string $name) return $request->query->get($rootName); } + + private function checkBounds(array $errorList, string $name, array $data, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value) { + return $errorList; + } + + $maximum = $data['swagger']['maximum'] ?? null; + $minimum = $data['swagger']['minimum'] ?? null; + + if (null !== $maximum) { + if (($data['swagger']['exclusiveMaximum'] ?? false) && $value >= $maximum) { + $errorList[] = sprintf('Query parameter "%s" must be less than %s', $name, $maximum); + } elseif ($value > $maximum) { + $errorList[] = sprintf('Query parameter "%s" must be less than or equal to %s', $name, $maximum); + } + } + + if (null !== $minimum) { + if (($data['swagger']['exclusiveMinimum'] ?? false) && $value <= $minimum) { + $errorList[] = sprintf('Query parameter "%s" must be greater than %s', $name, $minimum); + } elseif ($value < $minimum) { + $errorList[] = sprintf('Query parameter "%s" must be greater than or equal to %s', $name, $minimum); + } + } + + return $errorList; + } } diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index ae1ca5a1110..613d42531bf 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter; use Doctrine\ORM\Mapping as ORM; @@ -26,6 +27,7 @@ * * @ApiResource(attributes={ * "filters"={ + * BoundsFilter::class, * RequiredFilter::class, * RequiredAllowEmptyFilter::class * } diff --git a/tests/Fixtures/TestBundle/Filter/BoundsFilter.php b/tests/Fixtures/TestBundle/Filter/BoundsFilter.php new file mode 100644 index 00000000000..4327e8377ea --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/BoundsFilter.php @@ -0,0 +1,66 @@ + + * + * 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 BoundsFilter 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 [ + 'maximum' => [ + 'property' => 'maximum', + 'type' => 'number', + 'required' => false, + 'swagger' => [ + 'maximum' => 10, + ], + ], + 'exclusiveMaximum' => [ + 'property' => 'maximum', + 'type' => 'number', + 'required' => false, + 'swagger' => [ + 'maximum' => 10, + 'exclusiveMaximum' => true, + ], + ], + 'minimum' => [ + 'property' => 'minimum', + 'type' => 'number', + 'required' => false, + 'swagger' => [ + 'minimum' => 5, + ], + ], + 'exclusiveMinimum' => [ + 'property' => 'exclusiveMinimum', + 'type' => 'number', + 'required' => false, + 'swagger' => [ + 'minimum' => 5, + 'exclusiveMinimum' => true, + ], + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index 92a009625f9..16b072e686a 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -144,6 +144,10 @@ services: ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter: arguments: [ '@doctrine' ] tags: [ 'api_platform.filter' ] + + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\: resource: '../../TestBundle/Controller' From 7f4b1c8ddade048189f71db5144a7077f771c4cf Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Tue, 20 Feb 2018 17:39:00 +0100 Subject: [PATCH 03/16] check filter `maxLength` and `minLength` --- features/filter/filter_validation.feature | 22 ++++++++- src/Filter/QueryParameterValidateListener.php | 36 ++++++++++++++ .../TestBundle/Entity/FilterValidator.php | 2 + .../TestBundle/Filter/LengthFilter.php | 48 +++++++++++++++++++ tests/Fixtures/app/config/config_common.yml | 4 ++ 5 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 tests/Fixtures/TestBundle/Filter/LengthFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index 93fa4379b95..086179dc424 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -64,7 +64,6 @@ Feature: Validate filters based upon filter description Then the response status code should be 400 And the JSON node "detail" should be equal to 'Query parameter "minimum" must be greater than or equal to 5' - @dropSchema Scenario: Test filter bounds: exclusiveMinimum When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMinimum=6" Then the response status code should be 200 @@ -72,3 +71,24 @@ Feature: Validate filters based upon filter description When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMinimum=5" Then the response status code should be 400 And the JSON node "detail" should be equal to 'Query parameter "exclusiveMinimum" must be greater than 5' + + Scenario: Test filter bounds: max length + When I am on "/filter_validators?required=foo&required-allow-empty&max-length-3=123" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&max-length-3=1234" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "max-length-3" length must be lower than or equal to 3' + + Scenario: Do not throw an error if value is not an array + When I am on "/filter_validators?required=foo&required-allow-empty&max-length-3[]=12345" + Then the response status code should be 200 + + @dropSchema + Scenario: Test filter bounds: min length + When I am on "/filter_validators?required=foo&required-allow-empty&min-length-3=123" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&min-length-3=12" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "min-length-3" length must be greater than or equal to 3' diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index 6647ce90dbd..7b0cae4c108 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -62,6 +62,7 @@ public function onKernelRequest(RequestEvent $event): void foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { $errorList = $this->checkRequired($errorList, $name, $data, $request); $errorList = $this->checkBounds($errorList, $name, $data, $request); + $errorList = $this->checkLength($errorList, $name, $data, $request); } } @@ -178,4 +179,39 @@ private function checkBounds(array $errorList, string $name, array $data, Reques return $errorList; } + + private function checkLength(array $errorList, string $name, array $data, Request $request): array + { + $maxLength = $data['swagger']['maxLength'] ?? null; + $minLength = $data['swagger']['minLength'] ?? null; + + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return $errorList; + } + + // if (!is_string($value)) { + // $errorList[] = sprintf('Query parameter "%s" must be less than or equal to %s', $name, $maximum); + // return $errorList; + // } + + if (null !== $maxLength && mb_strlen($value) > $maxLength) { + $errorList[] = sprintf('Query parameter "%s" length must be lower than or equal to %s', $name, $maxLength); + } + + if (null !== $minLength && mb_strlen($value) < $minLength) { + $errorList[] = sprintf('Query parameter "%s" length must be greater than or equal to %s', $name, $minLength); + } + + return $errorList; + } + + // TODO grouper les filtres required dans une classe + // avoir deux entités, une required, une pour le reste + // pattern string See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.2.3. + // maxItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.2. + // minItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.3. + // uniqueItems boolean See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.4. + // enum [*] See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.1. + // multipleOf } diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index 613d42531bf..e1a76feb51d 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -16,6 +16,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\LengthFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter; use Doctrine\ORM\Mapping as ORM; @@ -28,6 +29,7 @@ * @ApiResource(attributes={ * "filters"={ * BoundsFilter::class, + * LengthFilter::class, * RequiredFilter::class, * RequiredAllowEmptyFilter::class * } diff --git a/tests/Fixtures/TestBundle/Filter/LengthFilter.php b/tests/Fixtures/TestBundle/Filter/LengthFilter.php new file mode 100644 index 00000000000..be9c30af0b2 --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/LengthFilter.php @@ -0,0 +1,48 @@ + + * + * 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 LengthFilter 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 [ + 'max-length-3' => [ + 'property' => 'max-length-3', + 'type' => 'string', + 'required' => false, + 'swagger' => [ + 'maxLength' => 3, + ], + ], + 'min-length-3' => [ + 'property' => 'min-length-3', + 'type' => 'string', + 'required' => false, + 'swagger' => [ + 'minLength' => 3, + ], + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index 16b072e686a..ee301765248 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -148,6 +148,10 @@ services: ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter: arguments: [ '@doctrine' ] tags: [ 'api_platform.filter' ] + + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\LengthFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\: resource: '../../TestBundle/Controller' From e5136062490067c4a69785a2e1c353d0ec6cdff1 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Wed, 21 Feb 2018 16:26:15 +0100 Subject: [PATCH 04/16] add lots of validations --- features/filter/filter_validation.feature | 27 +++++++++- src/Filter/QueryParameterValidateListener.php | 54 +++++++++++++++++-- .../TestBundle/Entity/FilterValidator.php | 6 +++ .../Fixtures/TestBundle/Filter/EnumFilter.php | 40 ++++++++++++++ .../TestBundle/Filter/MultipleOfFilter.php | 40 ++++++++++++++ .../TestBundle/Filter/PatternFilter.php | 40 ++++++++++++++ tests/Fixtures/app/config/config_common.yml | 12 +++++ 7 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Filter/EnumFilter.php create mode 100644 tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php create mode 100644 tests/Fixtures/TestBundle/Filter/PatternFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index 086179dc424..77f1d7ff87d 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -84,7 +84,6 @@ Feature: Validate filters based upon filter description When I am on "/filter_validators?required=foo&required-allow-empty&max-length-3[]=12345" Then the response status code should be 200 - @dropSchema Scenario: Test filter bounds: min length When I am on "/filter_validators?required=foo&required-allow-empty&min-length-3=123" Then the response status code should be 200 @@ -92,3 +91,29 @@ Feature: Validate filters based upon filter description When I am on "/filter_validators?required=foo&required-allow-empty&min-length-3=12" Then the response status code should be 400 And the JSON node "detail" should be equal to 'Query parameter "min-length-3" length must be greater than or equal to 3' + + Scenario: Test filter pattern + When I am on "/filter_validators?required=foo&required-allow-empty&pattern=pattern" + When I am on "/filter_validators?required=foo&required-allow-empty&pattern=nrettap" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&pattern=not-pattern" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "pattern" must match pattern /^(pattern|nrettap)$/' + + Scenario: Test filter enum + When I am on "/filter_validators?required=foo&required-allow-empty&enum=in-enum" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&enum=not-in-enum" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "enum" must be one of "in-enum, mune-ni"' + + @dropSchema + Scenario: Test filter multipleOf + When I am on "/filter_validators?required=foo&required-allow-empty&multiple-of=4" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&multiple-of=3" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "multiple-of" must multiple of 2' diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index 7b0cae4c108..c38e520ae16 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -63,6 +63,9 @@ public function onKernelRequest(RequestEvent $event): void $errorList = $this->checkRequired($errorList, $name, $data, $request); $errorList = $this->checkBounds($errorList, $name, $data, $request); $errorList = $this->checkLength($errorList, $name, $data, $request); + $errorList = $this->checkPattern($errorList, $name, $data, $request); + $errorList = $this->checkEnum($errorList, $name, $data, $request); + $errorList = $this->checkMultipleOf($errorList, $name, $data, $request); } } @@ -206,12 +209,57 @@ private function checkLength(array $errorList, string $name, array $data, Reques return $errorList; } + public function checkPattern(array $errorList, string $name, array $data, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return $errorList; + } + + $pattern = $data['swagger']['pattern'] ?? null; + + if (null !== $pattern && !preg_match($pattern, $value)) { + $errorList[] = sprintf('Query parameter "%s" must match pattern %s', $name, $pattern); + } + + return $errorList; + } + + public function checkEnum(array $errorList, string $name, array $data, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return $errorList; + } + + $enum = $data['swagger']['enum'] ?? null; + + if (null !== $enum && !\in_array($value, $enum, true)) { + $errorList[] = sprintf('Query parameter "%s" must be one of "%s"', $name, implode(', ', $enum)); + } + + return $errorList; + } + + public function checkMultipleOf(array $errorList, string $name, array $data, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return $errorList; + } + + $multipleOf = $data['swagger']['multipleOf'] ?? null; + + if (null !== $multipleOf && 0 !== ($value % $multipleOf)) { + $errorList[] = sprintf('Query parameter "%s" must multiple of %s', $name, $multipleOf); + } + + return $errorList; + } + // TODO grouper les filtres required dans une classe // avoir deux entités, une required, une pour le reste - // pattern string See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.2.3. // maxItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.2. // minItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.3. // uniqueItems boolean See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.4. - // enum [*] See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.1. - // multipleOf } diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index e1a76feb51d..359a3042911 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -16,7 +16,10 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\EnumFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\LengthFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\MultipleOfFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\PatternFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter; use Doctrine\ORM\Mapping as ORM; @@ -29,7 +32,10 @@ * @ApiResource(attributes={ * "filters"={ * BoundsFilter::class, + * EnumFilter::class, * LengthFilter::class, + * MultipleOfFilter::class, + * PatternFilter::class, * RequiredFilter::class, * RequiredAllowEmptyFilter::class * } diff --git a/tests/Fixtures/TestBundle/Filter/EnumFilter.php b/tests/Fixtures/TestBundle/Filter/EnumFilter.php new file mode 100644 index 00000000000..a2fe49b2598 --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/EnumFilter.php @@ -0,0 +1,40 @@ + + * + * 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 EnumFilter 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 [ + 'enum' => [ + 'property' => 'enum', + 'type' => 'string', + 'required' => false, + 'swagger' => [ + 'enum' => ['in-enum', 'mune-ni'], + ], + ], + ]; + } +} diff --git a/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php b/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php new file mode 100644 index 00000000000..6f0703bec8c --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php @@ -0,0 +1,40 @@ + + * + * 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 MultipleOfFilter 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 [ + 'multiple-of' => [ + 'property' => 'multiple-of', + 'type' => 'number', + 'required' => false, + 'swagger' => [ + 'multipleOf' => 2, + ], + ], + ]; + } +} diff --git a/tests/Fixtures/TestBundle/Filter/PatternFilter.php b/tests/Fixtures/TestBundle/Filter/PatternFilter.php new file mode 100644 index 00000000000..ccb9f56e731 --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/PatternFilter.php @@ -0,0 +1,40 @@ + + * + * 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 PatternFilter 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 [ + 'pattern' => [ + 'property' => 'pattern', + 'type' => 'string', + 'required' => false, + 'swagger' => [ + 'pattern' => '/^(pattern|nrettap)$/', + ], + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index ee301765248..f6556d6662f 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -153,10 +153,22 @@ services: arguments: [ '@doctrine' ] tags: [ 'api_platform.filter' ] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\PatternFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\: resource: '../../TestBundle/Controller' tags: ['controller.service_arguments'] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\EnumFilter: + arguments: [ '@doctrine' ] + tags: [ 'api_platform.filter' ] + + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\MultipleOfFilter: + 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 950c172d701c629af0a63d41d0653897aad90b1f Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Wed, 21 Feb 2018 17:12:43 +0100 Subject: [PATCH 05/16] move validators in separate files --- src/Filter/QueryParameterValidateListener.php | 204 ++---------------- src/Filter/Validator/Bounds.php | 50 +++++ src/Filter/Validator/Enum.php | 37 ++++ src/Filter/Validator/Length.php | 42 ++++ src/Filter/Validator/MultipleOf.php | 37 ++++ src/Filter/Validator/Pattern.php | 37 ++++ src/Filter/Validator/Required.php | 101 +++++++++ src/Filter/Validator/ValidatorInterface.php | 21 ++ 8 files changed, 339 insertions(+), 190 deletions(-) create mode 100644 src/Filter/Validator/Bounds.php create mode 100644 src/Filter/Validator/Enum.php create mode 100644 src/Filter/Validator/Length.php create mode 100644 src/Filter/Validator/MultipleOf.php create mode 100644 src/Filter/Validator/Pattern.php create mode 100644 src/Filter/Validator/Required.php create mode 100644 src/Filter/Validator/ValidatorInterface.php diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index c38e520ae16..2315ab60123 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -18,7 +18,6 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Util\RequestAttributesExtractor; use Psr\Container\ContainerInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\RequestEvent; /** @@ -32,10 +31,21 @@ final class QueryParameterValidateListener private $resourceMetadataFactory; + private $validators; + public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, ContainerInterface $filterLocator) { $this->resourceMetadataFactory = $resourceMetadataFactory; $this->setFilterLocator($filterLocator); + + $this->validators = [ + new Validator\Required(), + new Validator\Bounds(), + new Validator\Length(), + new Validator\Pattern(), + new Validator\Enum(), + new Validator\MultipleOf(), + ]; } public function onKernelRequest(RequestEvent $event): void @@ -60,12 +70,9 @@ public function onKernelRequest(RequestEvent $event): void } foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { - $errorList = $this->checkRequired($errorList, $name, $data, $request); - $errorList = $this->checkBounds($errorList, $name, $data, $request); - $errorList = $this->checkLength($errorList, $name, $data, $request); - $errorList = $this->checkPattern($errorList, $name, $data, $request); - $errorList = $this->checkEnum($errorList, $name, $data, $request); - $errorList = $this->checkMultipleOf($errorList, $name, $data, $request); + foreach ($this->validators as $validator) { + $errorList = array_merge($errorList, $validator->validate($name, $data, $request)); + } } } @@ -74,189 +81,6 @@ public function onKernelRequest(RequestEvent $event): void } } - private function checkRequired(array $errorList, string $name, array $data, Request $request): array - { - // filter is not required, the `checkRequired` method can not break - if (!($data['required'] ?? false)) { - return $errorList; - } - - // if query param is not given, then break - if (!$this->requestHasQueryParameter($request, $name)) { - $errorList[] = sprintf('Query parameter "%s" is required', $name); - - return $errorList; - } - - // if query param is empty and the configuration does not allow it - if (!($data['swagger']['allowEmptyValue'] ?? false) && empty($this->requestGetQueryParameter($request, $name))) { - $errorList[] = sprintf('Query parameter "%s" does not allow empty value', $name); - } - - return $errorList; - } - - /** - * Test if request has required parameter. - */ - private function requestHasQueryParameter(Request $request, string $name): bool - { - $matches = []; - parse_str($name, $matches); - if (!$matches) { - return false; - } - - $rootName = (string) (array_keys($matches)[0] ?? null); - 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 $request->query->has($rootName); - } - - /** - * Test if required filter is valid. It validates array notation too like "required[bar]". - */ - private function requestGetQueryParameter(Request $request, string $name) - { - $matches = []; - parse_str($name, $matches); - if (!$matches) { - return null; - } - - $rootName = array_keys($matches)[0] ?? ''; - if (!$rootName) { - return null; - } - - if (\is_array($matches[$rootName])) { - $keyName = array_keys($matches[$rootName])[0]; - - $queryParameter = $request->query->get($rootName); - - if (\is_array($queryParameter) && isset($queryParameter[$keyName])) { - return $queryParameter[$keyName]; - } - - return null; - } - - return $request->query->get($rootName); - } - - private function checkBounds(array $errorList, string $name, array $data, Request $request): array - { - $value = $request->query->get($name); - if (empty($value) && '0' !== $value) { - return $errorList; - } - - $maximum = $data['swagger']['maximum'] ?? null; - $minimum = $data['swagger']['minimum'] ?? null; - - if (null !== $maximum) { - if (($data['swagger']['exclusiveMaximum'] ?? false) && $value >= $maximum) { - $errorList[] = sprintf('Query parameter "%s" must be less than %s', $name, $maximum); - } elseif ($value > $maximum) { - $errorList[] = sprintf('Query parameter "%s" must be less than or equal to %s', $name, $maximum); - } - } - - if (null !== $minimum) { - if (($data['swagger']['exclusiveMinimum'] ?? false) && $value <= $minimum) { - $errorList[] = sprintf('Query parameter "%s" must be greater than %s', $name, $minimum); - } elseif ($value < $minimum) { - $errorList[] = sprintf('Query parameter "%s" must be greater than or equal to %s', $name, $minimum); - } - } - - return $errorList; - } - - private function checkLength(array $errorList, string $name, array $data, Request $request): array - { - $maxLength = $data['swagger']['maxLength'] ?? null; - $minLength = $data['swagger']['minLength'] ?? null; - - $value = $request->query->get($name); - if (empty($value) && '0' !== $value || !\is_string($value)) { - return $errorList; - } - - // if (!is_string($value)) { - // $errorList[] = sprintf('Query parameter "%s" must be less than or equal to %s', $name, $maximum); - // return $errorList; - // } - - if (null !== $maxLength && mb_strlen($value) > $maxLength) { - $errorList[] = sprintf('Query parameter "%s" length must be lower than or equal to %s', $name, $maxLength); - } - - if (null !== $minLength && mb_strlen($value) < $minLength) { - $errorList[] = sprintf('Query parameter "%s" length must be greater than or equal to %s', $name, $minLength); - } - - return $errorList; - } - - public function checkPattern(array $errorList, string $name, array $data, Request $request): array - { - $value = $request->query->get($name); - if (empty($value) && '0' !== $value || !\is_string($value)) { - return $errorList; - } - - $pattern = $data['swagger']['pattern'] ?? null; - - if (null !== $pattern && !preg_match($pattern, $value)) { - $errorList[] = sprintf('Query parameter "%s" must match pattern %s', $name, $pattern); - } - - return $errorList; - } - - public function checkEnum(array $errorList, string $name, array $data, Request $request): array - { - $value = $request->query->get($name); - if (empty($value) && '0' !== $value || !\is_string($value)) { - return $errorList; - } - - $enum = $data['swagger']['enum'] ?? null; - - if (null !== $enum && !\in_array($value, $enum, true)) { - $errorList[] = sprintf('Query parameter "%s" must be one of "%s"', $name, implode(', ', $enum)); - } - - return $errorList; - } - - public function checkMultipleOf(array $errorList, string $name, array $data, Request $request): array - { - $value = $request->query->get($name); - if (empty($value) && '0' !== $value || !\is_string($value)) { - return $errorList; - } - - $multipleOf = $data['swagger']['multipleOf'] ?? null; - - if (null !== $multipleOf && 0 !== ($value % $multipleOf)) { - $errorList[] = sprintf('Query parameter "%s" must multiple of %s', $name, $multipleOf); - } - - return $errorList; - } - // TODO grouper les filtres required dans une classe // avoir deux entités, une required, une pour le reste // maxItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.2. diff --git a/src/Filter/Validator/Bounds.php b/src/Filter/Validator/Bounds.php new file mode 100644 index 00000000000..119d4c9dba6 --- /dev/null +++ b/src/Filter/Validator/Bounds.php @@ -0,0 +1,50 @@ + + * + * 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\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class Bounds implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value) { + return []; + } + + $maximum = $filterDescription['swagger']['maximum'] ?? null; + $minimum = $filterDescription['swagger']['minimum'] ?? null; + + $errorList = []; + + if (null !== $maximum) { + if (($filterDescription['swagger']['exclusiveMaximum'] ?? false) && $value >= $maximum) { + $errorList[] = sprintf('Query parameter "%s" must be less than %s', $name, $maximum); + } elseif ($value > $maximum) { + $errorList[] = sprintf('Query parameter "%s" must be less than or equal to %s', $name, $maximum); + } + } + + if (null !== $minimum) { + if (($filterDescription['swagger']['exclusiveMinimum'] ?? false) && $value <= $minimum) { + $errorList[] = sprintf('Query parameter "%s" must be greater than %s', $name, $minimum); + } elseif ($value < $minimum) { + $errorList[] = sprintf('Query parameter "%s" must be greater than or equal to %s', $name, $minimum); + } + } + + return $errorList; + } +} diff --git a/src/Filter/Validator/Enum.php b/src/Filter/Validator/Enum.php new file mode 100644 index 00000000000..abc4b8b89df --- /dev/null +++ b/src/Filter/Validator/Enum.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\Filter\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class Enum implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return []; + } + + $enum = $filterDescription['swagger']['enum'] ?? null; + + if (null !== $enum && !\in_array($value, $enum, true)) { + return [ + sprintf('Query parameter "%s" must be one of "%s"', $name, implode(', ', $enum)), + ]; + } + + return []; + } +} diff --git a/src/Filter/Validator/Length.php b/src/Filter/Validator/Length.php new file mode 100644 index 00000000000..a64d370cec1 --- /dev/null +++ b/src/Filter/Validator/Length.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\Filter\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class Length implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + $maxLength = $filterDescription['swagger']['maxLength'] ?? null; + $minLength = $filterDescription['swagger']['minLength'] ?? null; + + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return []; + } + + $errorList = []; + + if (null !== $maxLength && mb_strlen($value) > $maxLength) { + $errorList[] = sprintf('Query parameter "%s" length must be lower than or equal to %s', $name, $maxLength); + } + + if (null !== $minLength && mb_strlen($value) < $minLength) { + $errorList[] = sprintf('Query parameter "%s" length must be greater than or equal to %s', $name, $minLength); + } + + return $errorList; + } +} diff --git a/src/Filter/Validator/MultipleOf.php b/src/Filter/Validator/MultipleOf.php new file mode 100644 index 00000000000..a4f7ee90fa1 --- /dev/null +++ b/src/Filter/Validator/MultipleOf.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\Filter\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class MultipleOf implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return []; + } + + $multipleOf = $filterDescription['swagger']['multipleOf'] ?? null; + + if (null !== $multipleOf && 0 !== ($value % $multipleOf)) { + return [ + sprintf('Query parameter "%s" must multiple of %s', $name, $multipleOf), + ]; + } + + return []; + } +} diff --git a/src/Filter/Validator/Pattern.php b/src/Filter/Validator/Pattern.php new file mode 100644 index 00000000000..c6c46667416 --- /dev/null +++ b/src/Filter/Validator/Pattern.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\Filter\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class Pattern implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + $value = $request->query->get($name); + if (empty($value) && '0' !== $value || !\is_string($value)) { + return []; + } + + $pattern = $filterDescription['swagger']['pattern'] ?? null; + + if (null !== $pattern && !preg_match($pattern, $value)) { + return [ + sprintf('Query parameter "%s" must match pattern %s', $name, $pattern), + ]; + } + + return []; + } +} diff --git a/src/Filter/Validator/Required.php b/src/Filter/Validator/Required.php new file mode 100644 index 00000000000..ff6332bb590 --- /dev/null +++ b/src/Filter/Validator/Required.php @@ -0,0 +1,101 @@ + + * + * 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\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class Required implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + // filter is not required, the `checkRequired` method can not break + if (!($filterDescription['required'] ?? false)) { + return []; + } + + // if query param is not given, then break + if (!$this->requestHasQueryParameter($request, $name)) { + return [ + sprintf('Query parameter "%s" is required', $name), + ]; + } + + // if query param is empty and the configuration does not allow it + if (!($filterDescription['swagger']['allowEmptyValue'] ?? false) && empty($this->requestGetQueryParameter($request, $name))) { + return [ + sprintf('Query parameter "%s" does not allow empty value', $name), + ]; + } + + return []; + } + + /** + * Test if request has required parameter. + */ + private function requestHasQueryParameter(Request $request, string $name): bool + { + $matches = []; + parse_str($name, $matches); + if (!$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 $request->query->has($rootName); + } + + /** + * Test if required filter is valid. It validates array notation too like "required[bar]". + */ + private function requestGetQueryParameter(Request $request, string $name) + { + $matches = []; + parse_str($name, $matches); + if (empty($matches)) { + return null; + } + + $rootName = array_keys($matches)[0] ?? ''; + if (!$rootName) { + return null; + } + + if (\is_array($matches[$rootName])) { + $keyName = array_keys($matches[$rootName])[0]; + + $queryParameter = $request->query->get($rootName); + + if (\is_array($queryParameter) && isset($queryParameter[$keyName])) { + return $queryParameter[$keyName]; + } + + return null; + } + + return $request->query->get($rootName); + } +} diff --git a/src/Filter/Validator/ValidatorInterface.php b/src/Filter/Validator/ValidatorInterface.php new file mode 100644 index 00000000000..33740725059 --- /dev/null +++ b/src/Filter/Validator/ValidatorInterface.php @@ -0,0 +1,21 @@ + + * + * 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\Validator; + +use Symfony\Component\HttpFoundation\Request; + +interface ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array; +} From 0da5c75c22b3b9c1b9482e33f24dae85de6a2be9 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Thu, 22 Feb 2018 08:20:41 +0100 Subject: [PATCH 06/16] validate maxItems, minItems & uniqueItems filter --- features/filter/filter_validation.feature | 50 ++++++++++- src/Filter/QueryParameterValidateListener.php | 13 +-- src/Filter/Validator/ArrayItems.php | 82 ++++++++++++++++++ .../TestBundle/Entity/FilterValidator.php | 2 + .../TestBundle/Filter/ArrayItemsFilter.php | 83 +++++++++++++++++++ tests/Fixtures/app/config/config_common.yml | 4 + 6 files changed, 224 insertions(+), 10 deletions(-) create mode 100644 src/Filter/Validator/ArrayItems.php create mode 100644 tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php diff --git a/features/filter/filter_validation.feature b/features/filter/filter_validation.feature index 77f1d7ff87d..23b63bcff09 100644 --- a/features/filter/filter_validation.feature +++ b/features/filter/filter_validation.feature @@ -109,7 +109,6 @@ Feature: Validate filters based upon filter description Then the response status code should be 400 And the JSON node "detail" should be equal to 'Query parameter "enum" must be one of "in-enum, mune-ni"' - @dropSchema Scenario: Test filter multipleOf When I am on "/filter_validators?required=foo&required-allow-empty&multiple-of=4" Then the response status code should be 200 @@ -117,3 +116,52 @@ Feature: Validate filters based upon filter description When I am on "/filter_validators?required=foo&required-allow-empty&multiple-of=3" Then the response status code should be 400 And the JSON node "detail" should be equal to 'Query parameter "multiple-of" must multiple of 2' + + Scenario: Test filter array items csv format minItems + When I am on "/filter_validators?required=foo&required-allow-empty&csv-min-2=a,b" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&csv-min-2=a" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "csv-min-2" must contain more than 2 values' + + Scenario: Test filter array items csv format maxItems + When I am on "/filter_validators?required=foo&required-allow-empty&csv-max-3=a,b,c" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&csv-max-3=a,b,c,d" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "csv-max-3" must contain less than 3 values' + + Scenario: Test filter array items tsv format minItems + When I am on "/filter_validators?required=foo&required-allow-empty&tsv-min-2=a\tb" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&tsv-min-2=a,b" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "tsv-min-2" must contain more than 2 values' + + Scenario: Test filter array items pipes format minItems + When I am on "/filter_validators?required=foo&required-allow-empty&pipes-min-2=a|b" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&pipes-min-2=a,b" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "pipes-min-2" must contain more than 2 values' + + Scenario: Test filter array items ssv format minItems + When I am on "/filter_validators?required=foo&required-allow-empty&ssv-min-2=a b" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&ssv-min-2=a,b" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "ssv-min-2" must contain more than 2 values' + + @dropSchema + Scenario: Test filter array items unique items + When I am on "/filter_validators?required=foo&required-allow-empty&csv-uniques=a,b" + Then the response status code should be 200 + + When I am on "/filter_validators?required=foo&required-allow-empty&csv-uniques=a,a" + Then the response status code should be 400 + And the JSON node "detail" should be equal to 'Query parameter "csv-uniques" must contain unique values' diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidateListener.php index 2315ab60123..1707c1bd362 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidateListener.php @@ -39,12 +39,13 @@ public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFa $this->setFilterLocator($filterLocator); $this->validators = [ - new Validator\Required(), + new Validator\ArrayItems(), new Validator\Bounds(), - new Validator\Length(), - new Validator\Pattern(), new Validator\Enum(), + new Validator\Length(), new Validator\MultipleOf(), + new Validator\Pattern(), + new Validator\Required(), ]; } @@ -80,10 +81,4 @@ public function onKernelRequest(RequestEvent $event): void throw new FilterValidationException($errorList); } } - - // TODO grouper les filtres required dans une classe - // avoir deux entités, une required, une pour le reste - // maxItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.2. - // minItems integer See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.3. - // uniqueItems boolean See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.3.4. } diff --git a/src/Filter/Validator/ArrayItems.php b/src/Filter/Validator/ArrayItems.php new file mode 100644 index 00000000000..fd903925cb6 --- /dev/null +++ b/src/Filter/Validator/ArrayItems.php @@ -0,0 +1,82 @@ + + * + * 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\Validator; + +use Symfony\Component\HttpFoundation\Request; + +class ArrayItems implements ValidatorInterface +{ + public function validate(string $name, array $filterDescription, Request $request): array + { + if (!$request->query->has($name)) { + return []; + } + + $maxItems = $filterDescription['swagger']['maxItems'] ?? null; + $minItems = $filterDescription['swagger']['minItems'] ?? null; + $uniqueItems = $filterDescription['swagger']['uniqueItems'] ?? false; + + $errorList = []; + + $value = $this->getValue($name, $filterDescription, $request); + $nbItems = \count($value); + + if (null !== $maxItems && $nbItems > $maxItems) { + $errorList[] = sprintf('Query parameter "%s" must contain less than %d values', $name, $maxItems); + } + + if (null !== $minItems && $nbItems < $minItems) { + $errorList[] = sprintf('Query parameter "%s" must contain more than %d values', $name, $minItems); + } + + if (true === $uniqueItems && $nbItems > \count(array_unique($value))) { + $errorList[] = sprintf('Query parameter "%s" must contain unique values', $name); + } + + return $errorList; + } + + private function getValue(string $name, array $filterDescription, Request $request): array + { + $value = $request->query->get($name); + + if (empty($value) && '0' !== $value) { + return []; + } + + if (\is_array($value)) { + return $value; + } + + $collectionFormat = $filterDescription['swagger']['collectionFormat'] ?? 'csv'; + + return explode(self::getSeparator($collectionFormat), $value); + } + + private static function getSeparator(string $collectionFormat): string + { + switch ($collectionFormat) { + case 'csv': + return ','; + case 'ssv': + return ' '; + case 'tsv': + return '\t'; + case 'pipes': + return '|'; + default: + throw new \InvalidArgumentException(sprintf('Unkwown collection format %s', $collectionFormat)); + } + } +} diff --git a/tests/Fixtures/TestBundle/Entity/FilterValidator.php b/tests/Fixtures/TestBundle/Entity/FilterValidator.php index 359a3042911..eaa62b26345 100644 --- a/tests/Fixtures/TestBundle/Entity/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Entity/FilterValidator.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\ArrayItemsFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\EnumFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\LengthFilter; @@ -31,6 +32,7 @@ * * @ApiResource(attributes={ * "filters"={ + * ArrayItemsFilter::class, * BoundsFilter::class, * EnumFilter::class, * LengthFilter::class, diff --git a/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php b/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php new file mode 100644 index 00000000000..7feb9b3409f --- /dev/null +++ b/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php @@ -0,0 +1,83 @@ + + * + * 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 ArrayItemsFilter 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 [ + 'csv-min-2' => [ + 'property' => 'csv-min-2', + 'type' => 'array', + 'required' => false, + 'swagger' => [ + 'minItems' => 2, + ], + ], + 'csv-max-3' => [ + 'property' => 'csv-max-3', + 'type' => 'array', + 'required' => false, + 'swagger' => [ + 'maxItems' => 3, + ], + ], + 'ssv-min-2' => [ + 'property' => 'ssv-min-2', + 'type' => 'array', + 'required' => false, + 'swagger' => [ + 'collectionFormat' => 'ssv', + 'minItems' => 2, + ], + ], + 'tsv-min-2' => [ + 'property' => 'tsv-min-2', + 'type' => 'array', + 'required' => false, + 'swagger' => [ + 'collectionFormat' => 'tsv', + 'minItems' => 2, + ], + ], + 'pipes-min-2' => [ + 'property' => 'pipes-min-2', + 'type' => 'array', + 'required' => false, + 'swagger' => [ + 'collectionFormat' => 'pipes', + 'minItems' => 2, + ], + ], + 'csv-uniques' => [ + 'property' => 'csv-uniques', + 'type' => 'array', + 'required' => false, + 'swagger' => [ + 'uniqueItems' => true, + ], + ], + ]; + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index f6556d6662f..0fe02d4b37d 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -169,6 +169,10 @@ services: arguments: [ '@doctrine' ] tags: [ 'api_platform.filter' ] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\ArrayItemsFilter: + 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 3a20868abcb68b0d075dfb1ae8eb13aae2c55b97 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Sat, 16 Jun 2018 16:11:17 +0200 Subject: [PATCH 07/16] split query parameter listener into a validator + a listener (need to split tests as well) --- .../Bundle/Resources/config/validator.xml | 8 ++- .../QueryParameterValidateListener.php | 55 +++++++++++++++++++ ...stener.php => QueryParameterValidator.php} | 28 ++-------- .../ApiPlatformExtensionTest.php | 1 + .../QueryParameterValidateListenerTest.php | 2 +- 5 files changed, 68 insertions(+), 26 deletions(-) create mode 100644 src/EventListener/QueryParameterValidateListener.php rename src/Filter/{QueryParameterValidateListener.php => QueryParameterValidator.php} (55%) rename tests/{Filter => EventListener}/QueryParameterValidateListenerTest.php (98%) diff --git a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml index d34dcd8cfe6..2526c93cfc6 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/validator.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/validator.xml @@ -23,9 +23,13 @@ - - + + + + + + diff --git a/src/EventListener/QueryParameterValidateListener.php b/src/EventListener/QueryParameterValidateListener.php new file mode 100644 index 00000000000..14990387485 --- /dev/null +++ b/src/EventListener/QueryParameterValidateListener.php @@ -0,0 +1,55 @@ + + * + * 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\EventListener; + +use ApiPlatform\Core\Filter\QueryParameterValidator; +use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Util\RequestAttributesExtractor; +use Symfony\Component\HttpKernel\Event\RequestEvent; + +/** + * Validates query parameters depending on filter description. + * + * @author Julien Deniau + */ +final class QueryParameterValidateListener +{ + private $resourceMetadataFactory; + + private $queryParameterValidator; + + public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, QueryParameterValidator $queryParameterValidator) + { + $this->resourceMetadataFactory = $resourceMetadataFactory; + $this->queryParameterValidator = $queryParameterValidator; + } + + public function onKernelRequest(RequestEvent $event) + { + $request = $event->getRequest(); + if ( + !$request->isMethodSafe() + || !($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); + + $this->queryParameterValidator->validateFilters($attributes['resource_class'], $resourceFilters, $request); + } +} diff --git a/src/Filter/QueryParameterValidateListener.php b/src/Filter/QueryParameterValidator.php similarity index 55% rename from src/Filter/QueryParameterValidateListener.php rename to src/Filter/QueryParameterValidator.php index 1707c1bd362..62f6f5d0e4a 100644 --- a/src/Filter/QueryParameterValidateListener.php +++ b/src/Filter/QueryParameterValidator.php @@ -15,27 +15,22 @@ use ApiPlatform\Core\Api\FilterLocatorTrait; 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\RequestEvent; +use Symfony\Component\HttpFoundation\Request; /** * Validates query parameters depending on filter description. * * @author Julien Deniau */ -final class QueryParameterValidateListener +class QueryParameterValidator { use FilterLocatorTrait; - private $resourceMetadataFactory; - private $validators; - public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, ContainerInterface $filterLocator) + public function __construct(ContainerInterface $filterLocator) { - $this->resourceMetadataFactory = $resourceMetadataFactory; $this->setFilterLocator($filterLocator); $this->validators = [ @@ -49,28 +44,15 @@ public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFa ]; } - public function onKernelRequest(RequestEvent $event): void + public function validateFilters(string $resourceClass, array $resourceFilters, Request $request) { - $request = $event->getRequest(); - if ( - !$request->isMethodSafe() - || !($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); - $errorList = []; foreach ($resourceFilters as $filterId) { if (!$filter = $this->getFilter($filterId)) { continue; } - foreach ($filter->getDescription($attributes['resource_class']) as $name => $data) { + foreach ($filter->getDescription($resourceClass) as $name => $data) { foreach ($this->validators as $validator) { $errorList = array_merge($errorList, $validator->validate($name, $data, $request)); } diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index e9034147400..93c83f0429d 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -1236,6 +1236,7 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo 'api_platform.swagger.action.ui', 'api_platform.swagger.listener.ui', 'api_platform.validator', + 'api_platform.validator.query_parameter_validator', 'test.api_platform.client', ]; diff --git a/tests/Filter/QueryParameterValidateListenerTest.php b/tests/EventListener/QueryParameterValidateListenerTest.php similarity index 98% rename from tests/Filter/QueryParameterValidateListenerTest.php rename to tests/EventListener/QueryParameterValidateListenerTest.php index 47e6efacfb2..96fb106191e 100644 --- a/tests/Filter/QueryParameterValidateListenerTest.php +++ b/tests/EventListener/QueryParameterValidateListenerTest.php @@ -14,8 +14,8 @@ namespace ApiPlatform\Core\Tests\Filter; use ApiPlatform\Core\Api\FilterInterface; +use ApiPlatform\Core\EventListener\QueryParameterValidateListener; 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; From 70083744326fcb316b38c8c57b409782b3588087 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Thu, 21 Jun 2018 14:12:50 +0200 Subject: [PATCH 08/16] split tests in two --- .../QueryParameterValidateListenerTest.php | 52 ++----- tests/Filter/QueryParameterValidatorTest.php | 130 ++++++++++++++++++ 2 files changed, 141 insertions(+), 41 deletions(-) create mode 100644 tests/Filter/QueryParameterValidatorTest.php diff --git a/tests/EventListener/QueryParameterValidateListenerTest.php b/tests/EventListener/QueryParameterValidateListenerTest.php index 96fb106191e..0c1cc92f258 100644 --- a/tests/EventListener/QueryParameterValidateListenerTest.php +++ b/tests/EventListener/QueryParameterValidateListenerTest.php @@ -13,21 +13,20 @@ namespace ApiPlatform\Core\Tests\Filter; -use ApiPlatform\Core\Api\FilterInterface; use ApiPlatform\Core\EventListener\QueryParameterValidateListener; use ApiPlatform\Core\Exception\FilterValidationException; +use ApiPlatform\Core\Filter\QueryParameterValidator; 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\RequestEvent; class QueryParameterValidateListenerTest extends TestCase { private $testedInstance; - private $filterLocatorProphecy; + private $queryParameterValidor; /** * unsafe method should not use filter validations. @@ -60,8 +59,7 @@ public function testOnKernelRequestWithWrongFilter() $eventProphecy = $this->prophesize(RequestEvent::class); $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); - $this->filterLocatorProphecy->has('some_inexistent_filter')->shouldBeCalled(); - $this->filterLocatorProphecy->get('some_inexistent_filter')->shouldNotBeCalled(); + $this->queryParameterValidor->validateFilters(Dummy::class, ['some_inexistent_filter'], $request)->shouldBeCalled(); $this->assertNull( $this->testedInstance->onKernelRequest($eventProphecy->reveal()) @@ -81,24 +79,10 @@ public function testOnKernelRequestWithRequiredFilterNotSet() $eventProphecy = $this->prophesize(RequestEvent::class); $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); - $this->filterLocatorProphecy - ->has('some_filter') + $this->queryParameterValidor + ->validateFilters(Dummy::class, ['some_filter'], $request) ->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()); - + ->willThrow(new FilterValidationException(['Query parameter "required" is required'])); $this->expectException(FilterValidationException::class); $this->expectExceptionMessage('Query parameter "required" is required'); $this->testedInstance->onKernelRequest($eventProphecy->reveal()); @@ -121,23 +105,9 @@ public function testOnKernelRequestWithRequiredFilter() $eventProphecy = $this->prophesize(RequestEvent::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->queryParameterValidor + ->validateFilters(Dummy::class, ['some_filter'], $request) + ->shouldBeCalled(); $this->assertNull( $this->testedInstance->onKernelRequest($eventProphecy->reveal()) @@ -156,11 +126,11 @@ private function setUpWithFilters(array $filters = []) ]) ); - $this->filterLocatorProphecy = $this->prophesize(ContainerInterface::class); + $this->queryParameterValidor = $this->prophesize(QueryParameterValidator::class); $this->testedInstance = new QueryParameterValidateListener( $resourceMetadataFactoryProphecy->reveal(), - $this->filterLocatorProphecy->reveal() + $this->queryParameterValidor->reveal() ); } } diff --git a/tests/Filter/QueryParameterValidatorTest.php b/tests/Filter/QueryParameterValidatorTest.php new file mode 100644 index 00000000000..f6c4169f267 --- /dev/null +++ b/tests/Filter/QueryParameterValidatorTest.php @@ -0,0 +1,130 @@ + + * + * 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\Test\Filter; + +use ApiPlatform\Core\Api\FilterInterface; +use ApiPlatform\Core\Exception\FilterValidationException; +use ApiPlatform\Core\Filter\QueryParameterValidator; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Symfony\Component\HttpFoundation\Request; + +/** + * Class QueryParameterValidatorTest. + * + * @author Julien Deniau + */ +class QueryParameterValidatorTest extends TestCase +{ + private $testedInstance; + private $filterLocatorProphecy; + + /** + * {@inheritdoc} + */ + protected function setUp(): void + { + $this->filterLocatorProphecy = $this->prophesize(ContainerInterface::class); + + $this->testedInstance = new QueryParameterValidator( + $this->filterLocatorProphecy->reveal() + ); + } + + /** + * unsafe method should not use filter validations. + */ + public function testOnKernelRequestWithUnsafeMethod() + { + $request = new Request(); + + $this->assertNull( + $this->testedInstance->validateFilters(Dummy::class, [], $request) + ); + } + + /** + * If the tested filter is non-existant, then nothing should append. + */ + public function testOnKernelRequestWithWrongFilter() + { + $request = new Request(); + + $this->assertNull( + $this->testedInstance->validateFilters(Dummy::class, ['some_inexistent_filter'], $request) + ); + } + + /** + * if the required parameter is not set, throw an FilterValidationException. + */ + public function testOnKernelRequestWithRequiredFilterNotSet() + { + $request = new Request(); + + $filterProphecy = $this->prophesize(FilterInterface::class); + $filterProphecy + ->getDescription(Dummy::class) + ->shouldBeCalled() + ->willReturn([ + 'required' => [ + 'required' => true, + ], + ]); + $this->filterLocatorProphecy + ->has('some_filter') + ->shouldBeCalled() + ->willReturn(true); + $this->filterLocatorProphecy + ->get('some_filter') + ->shouldBeCalled() + ->willReturn($filterProphecy->reveal()); + + $this->expectException(FilterValidationException::class); + $this->expectExceptionMessage('Query parameter "required" is required'); + $this->testedInstance->validateFilters(Dummy::class, ['some_filter'], $request); + } + + /** + * if the required parameter is set, no exception should be throwned. + */ + public function testOnKernelRequestWithRequiredFilter() + { + $request = new Request( + ['required' => 'foo'] + ); + + $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->validateFilters(Dummy::class, ['some_filter'], $request) + ); + } +} From 437d87e6562bb91b974f91a463221bec4b96c52e Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Thu, 21 Jun 2018 15:23:37 +0200 Subject: [PATCH 09/16] add unit tests for each Validators --- src/Filter/Validator/ArrayItems.php | 2 +- .../QueryParameterValidateListenerTest.php | 2 +- tests/Filter/QueryParameterValidatorTest.php | 3 +- tests/Filter/Validator/ArrayItemsTest.php | 199 ++++++++++++++++++ tests/Filter/Validator/BoundsTest.php | 180 ++++++++++++++++ tests/Filter/Validator/EnumTest.php | 77 +++++++ tests/Filter/Validator/LengthTest.php | 142 +++++++++++++ tests/Filter/Validator/MultipleOfTest.php | 77 +++++++ tests/Filter/Validator/PatternTest.php | 102 +++++++++ tests/Filter/Validator/RequiredTest.php | 105 +++++++++ 10 files changed, 886 insertions(+), 3 deletions(-) create mode 100644 tests/Filter/Validator/ArrayItemsTest.php create mode 100644 tests/Filter/Validator/BoundsTest.php create mode 100644 tests/Filter/Validator/EnumTest.php create mode 100644 tests/Filter/Validator/LengthTest.php create mode 100644 tests/Filter/Validator/MultipleOfTest.php create mode 100644 tests/Filter/Validator/PatternTest.php create mode 100644 tests/Filter/Validator/RequiredTest.php diff --git a/src/Filter/Validator/ArrayItems.php b/src/Filter/Validator/ArrayItems.php index fd903925cb6..42dfa298eed 100644 --- a/src/Filter/Validator/ArrayItems.php +++ b/src/Filter/Validator/ArrayItems.php @@ -76,7 +76,7 @@ private static function getSeparator(string $collectionFormat): string case 'pipes': return '|'; default: - throw new \InvalidArgumentException(sprintf('Unkwown collection format %s', $collectionFormat)); + throw new \InvalidArgumentException(sprintf('Unknown collection format %s', $collectionFormat)); } } } diff --git a/tests/EventListener/QueryParameterValidateListenerTest.php b/tests/EventListener/QueryParameterValidateListenerTest.php index 0c1cc92f258..2c52067f715 100644 --- a/tests/EventListener/QueryParameterValidateListenerTest.php +++ b/tests/EventListener/QueryParameterValidateListenerTest.php @@ -11,7 +11,7 @@ declare(strict_types=1); -namespace ApiPlatform\Core\Tests\Filter; +namespace ApiPlatform\Core\Tests\EventListener; use ApiPlatform\Core\EventListener\QueryParameterValidateListener; use ApiPlatform\Core\Exception\FilterValidationException; diff --git a/tests/Filter/QueryParameterValidatorTest.php b/tests/Filter/QueryParameterValidatorTest.php index f6c4169f267..c0f781407a2 100644 --- a/tests/Filter/QueryParameterValidatorTest.php +++ b/tests/Filter/QueryParameterValidatorTest.php @@ -11,11 +11,12 @@ declare(strict_types=1); -namespace ApiPlatform\Core\Test\Filter; +namespace ApiPlatform\Core\Tests\Filter; use ApiPlatform\Core\Api\FilterInterface; use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Filter\QueryParameterValidator; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Symfony\Component\HttpFoundation\Request; diff --git a/tests/Filter/Validator/ArrayItemsTest.php b/tests/Filter/Validator/ArrayItemsTest.php new file mode 100644 index 00000000000..6e4d2a3e9a4 --- /dev/null +++ b/tests/Filter/Validator/ArrayItemsTest.php @@ -0,0 +1,199 @@ + + * + * 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\Validator; + +use ApiPlatform\Core\Filter\Validator\ArrayItems; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Julien Deniau + */ +class ArrayItemsTest extends TestCase +{ + public function testNonDefinedFilter() + { + $request = new Request(); + $filter = new ArrayItems(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testEmptyQueryParameter() + { + $request = new Request(['some_filter' => '']); + $filter = new ArrayItems(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testNonMatchingParameter() + { + $filter = new ArrayItems(); + + $filterDefinition = [ + 'swagger' => [ + 'maxItems' => 3, + 'minItems' => 2, + ], + ]; + + $request = new Request(['some_filter' => ['foo', 'bar', 'bar', 'foo']]); + $this->assertEquals( + ['Query parameter "some_filter" must contain less than 3 values'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $request = new Request(['some_filter' => ['foo']]); + $this->assertEquals( + ['Query parameter "some_filter" must contain more than 2 values'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testMatchingParameter() + { + $filter = new ArrayItems(); + + $filterDefinition = [ + 'swagger' => [ + 'maxItems' => 3, + 'minItems' => 2, + ], + ]; + + $request = new Request(['some_filter' => ['foo', 'bar']]); + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $request = new Request(['some_filter' => ['foo', 'bar', 'baz']]); + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testNonMatchingUniqueItems() + { + $filter = new ArrayItems(); + + $filterDefinition = [ + 'swagger' => [ + 'uniqueItems' => true, + ], + ]; + + $request = new Request(['some_filter' => ['foo', 'bar', 'bar', 'foo']]); + $this->assertEquals( + ['Query parameter "some_filter" must contain unique values'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testMatchingUniqueItems() + { + $filter = new ArrayItems(); + + $filterDefinition = [ + 'swagger' => [ + 'uniqueItems' => true, + ], + ]; + + $request = new Request(['some_filter' => ['foo', 'bar', 'baz']]); + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testSeparators() + { + $filter = new ArrayItems(); + + $filterDefinition = [ + 'swagger' => [ + 'maxItems' => 2, + 'uniqueItems' => true, + 'collectionFormat' => 'csv', + ], + ]; + + $request = new Request(['some_filter' => 'foo,bar,bar']); + $this->assertEquals( + [ + 'Query parameter "some_filter" must contain less than 2 values', + 'Query parameter "some_filter" must contain unique values', + ], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition['swagger']['collectionFormat'] = 'ssv'; + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition['swagger']['collectionFormat'] = 'ssv'; + $request = new Request(['some_filter' => 'foo bar bar']); + $this->assertEquals( + [ + 'Query parameter "some_filter" must contain less than 2 values', + 'Query parameter "some_filter" must contain unique values', + ], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition['swagger']['collectionFormat'] = 'tsv'; + $request = new Request(['some_filter' => 'foo\tbar\tbar']); + $this->assertEquals( + [ + 'Query parameter "some_filter" must contain less than 2 values', + 'Query parameter "some_filter" must contain unique values', + ], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition['swagger']['collectionFormat'] = 'pipes'; + $request = new Request(['some_filter' => 'foo|bar|bar']); + $this->assertEquals( + [ + 'Query parameter "some_filter" must contain less than 2 values', + 'Query parameter "some_filter" must contain unique values', + ], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testSeparatorsUnknownSeparator() + { + $filter = new ArrayItems(); + + $filterDefinition = [ + 'swagger' => [ + 'maxItems' => 2, + 'uniqueItems' => true, + 'collectionFormat' => 'unknownFormat', + ], + ]; + $request = new Request(['some_filter' => 'foo,bar,bar']); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Unknown collection format unknownFormat'); + + $filter->validate('some_filter', $filterDefinition, $request); + } +} diff --git a/tests/Filter/Validator/BoundsTest.php b/tests/Filter/Validator/BoundsTest.php new file mode 100644 index 00000000000..d993777a911 --- /dev/null +++ b/tests/Filter/Validator/BoundsTest.php @@ -0,0 +1,180 @@ + + * + * 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\Validator; + +use ApiPlatform\Core\Filter\Validator\Bounds; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Julien Deniau + */ +class BoundsTest extends TestCase +{ + public function testNonDefinedFilter() + { + $request = new Request(); + $filter = new Bounds(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testEmptyQueryParameter() + { + $request = new Request(['some_filter' => '']); + $filter = new Bounds(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testNonMatchingMinimum() + { + $request = new Request(['some_filter' => '9']); + $filter = new Bounds(); + + $filterDefinition = [ + 'swagger' => [ + 'minimum' => 10, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be greater than or equal to 10'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition = [ + 'swagger' => [ + 'minimum' => 10, + 'exclusiveMinimum' => false, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be greater than or equal to 10'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition = [ + 'swagger' => [ + 'minimum' => 9, + 'exclusiveMinimum' => true, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be greater than 9'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testMatchingMinimum() + { + $request = new Request(['some_filter' => '10']); + $filter = new Bounds(); + + $filterDefinition = [ + 'swagger' => [ + 'minimum' => 10, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition = [ + 'swagger' => [ + 'minimum' => 9, + 'exclusiveMinimum' => false, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testNonMatchingMaximum() + { + $request = new Request(['some_filter' => '11']); + $filter = new Bounds(); + + $filterDefinition = [ + 'swagger' => [ + 'maximum' => 10, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be less than or equal to 10'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition = [ + 'swagger' => [ + 'maximum' => 10, + 'exclusiveMaximum' => false, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be less than or equal to 10'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition = [ + 'swagger' => [ + 'maximum' => 9, + 'exclusiveMaximum' => true, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be less than 9'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testMatchingMaximum() + { + $request = new Request(['some_filter' => '10']); + $filter = new Bounds(); + + $filterDefinition = [ + 'swagger' => [ + 'maximum' => 10, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + + $filterDefinition = [ + 'swagger' => [ + 'maximum' => 10, + 'exclusiveMaximum' => false, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + } +} diff --git a/tests/Filter/Validator/EnumTest.php b/tests/Filter/Validator/EnumTest.php new file mode 100644 index 00000000000..e55697e4376 --- /dev/null +++ b/tests/Filter/Validator/EnumTest.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\Tests\Filter\Validator; + +use ApiPlatform\Core\Filter\Validator\Enum; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Julien Deniau + */ +class EnumTest extends TestCase +{ + public function testNonDefinedFilter() + { + $request = new Request(); + $filter = new Enum(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testEmptyQueryParameter() + { + $request = new Request(['some_filter' => '']); + $filter = new Enum(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testNonMatchingParameter() + { + $request = new Request(['some_filter' => 'foobar']); + $filter = new Enum(); + + $filterDefinition = [ + 'swagger' => [ + 'enum' => ['foo', 'bar'], + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must be one of "foo, bar"'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testMatchingParameter() + { + $request = new Request(['some_filter' => 'foo']); + $filter = new Enum(); + + $filterDefinition = [ + 'swagger' => [ + 'enum' => ['foo', 'bar'], + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + } +} diff --git a/tests/Filter/Validator/LengthTest.php b/tests/Filter/Validator/LengthTest.php new file mode 100644 index 00000000000..76e60645af2 --- /dev/null +++ b/tests/Filter/Validator/LengthTest.php @@ -0,0 +1,142 @@ + + * + * 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\Validator; + +use ApiPlatform\Core\Filter\Validator\Length; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Julien Deniau + */ +class LengthTest extends TestCase +{ + public function testNonDefinedFilter() + { + $request = new Request(); + $filter = new Length(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testEmptyQueryParameter() + { + $request = new Request(['some_filter' => '']); + $filter = new Length(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testNonMatchingParameter() + { + $filter = new Length(); + + $filterDefinition = [ + 'swagger' => [ + 'minLength' => 3, + 'maxLength' => 5, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" length must be greater than or equal to 3'], + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'ab'])) + ); + + $this->assertEquals( + ['Query parameter "some_filter" length must be lower than or equal to 5'], + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcdef'])) + ); + } + + public function testNonMatchingParameterWithOnlyOneDefinition() + { + $filter = new Length(); + + $filterDefinition = [ + 'swagger' => [ + 'minLength' => 3, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" length must be greater than or equal to 3'], + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'ab'])) + ); + + $filterDefinition = [ + 'swagger' => [ + 'maxLength' => 5, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" length must be lower than or equal to 5'], + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcdef'])) + ); + } + + public function testMatchingParameter() + { + $filter = new Length(); + + $filterDefinition = [ + 'swagger' => [ + 'minLength' => 3, + 'maxLength' => 5, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abc'])) + ); + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcd'])) + ); + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcde'])) + ); + } + + public function testMatchingParameterWithOneDefinition() + { + $filter = new Length(); + + $filterDefinition = [ + 'swagger' => [ + 'minLength' => 3, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abc'])) + ); + + $filterDefinition = [ + 'swagger' => [ + 'maxLength' => 5, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcde'])) + ); + } +} diff --git a/tests/Filter/Validator/MultipleOfTest.php b/tests/Filter/Validator/MultipleOfTest.php new file mode 100644 index 00000000000..a90386db765 --- /dev/null +++ b/tests/Filter/Validator/MultipleOfTest.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\Tests\Filter\Validator; + +use ApiPlatform\Core\Filter\Validator\MultipleOf; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Julien Deniau + */ +class MultipleOfTest extends TestCase +{ + public function testNonDefinedFilter() + { + $request = new Request(); + $filter = new MultipleOf(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testEmptyQueryParameter() + { + $request = new Request(['some_filter' => '']); + $filter = new MultipleOf(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testNonMatchingParameter() + { + $request = new Request(['some_filter' => '8']); + $filter = new MultipleOf(); + + $filterDefinition = [ + 'swagger' => [ + 'multipleOf' => 3, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must multiple of 3'], + $filter->validate('some_filter', $filterDefinition, $request) + ); + } + + public function testMatchingParameter() + { + $request = new Request(['some_filter' => '8']); + $filter = new MultipleOf(); + + $filterDefinition = [ + 'swagger' => [ + 'multipleOf' => 4, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $filterDefinition, $request) + ); + } +} diff --git a/tests/Filter/Validator/PatternTest.php b/tests/Filter/Validator/PatternTest.php new file mode 100644 index 00000000000..02a924a8f5b --- /dev/null +++ b/tests/Filter/Validator/PatternTest.php @@ -0,0 +1,102 @@ + + * + * 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\Validator; + +use ApiPlatform\Core\Filter\Validator\Pattern; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Julien Deniau + */ +class PatternTest extends TestCase +{ + public function testNonDefinedFilter() + { + $request = new Request(); + $filter = new Pattern(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + } + + public function testFilterWithEmptyValue() + { + $filter = new Pattern(); + + $explicitFilterDefinition = [ + 'swagger' => [ + 'pattern' => '/foo/', + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => ''])) + ); + + $weirdParameter = new \stdClass(); + $weirdParameter->foo = 'non string value should not exists'; + $this->assertEmpty( + $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => $weirdParameter])) + ); + } + + public function testFilterWithZeroAsParameter() + { + $filter = new Pattern(); + + $explicitFilterDefinition = [ + 'swagger' => [ + 'pattern' => '/foo/', + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must match pattern /foo/'], + $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => '0'])) + ); + } + + public function testFilterWithNonMatchingValue() + { + $filter = new Pattern(); + + $explicitFilterDefinition = [ + 'swagger' => [ + 'pattern' => '/foo/', + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" must match pattern /foo/'], + $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => 'bar'])) + ); + } + + public function testFilterWithNonchingValue() + { + $filter = new Pattern(); + + $explicitFilterDefinition = [ + 'swagger' => [ + 'pattern' => '/foo \d+/', + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => 'this is a foo '.random_int(0, 10).' and it should match'])) + ); + } +} diff --git a/tests/Filter/Validator/RequiredTest.php b/tests/Filter/Validator/RequiredTest.php new file mode 100644 index 00000000000..b2c1596c144 --- /dev/null +++ b/tests/Filter/Validator/RequiredTest.php @@ -0,0 +1,105 @@ + + * + * 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\Validator; + +use ApiPlatform\Core\Filter\Validator\Required; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * Class RequiredTest. + * + * @author Julien Deniau + */ +class RequiredTest extends TestCase +{ + public function testNonRequiredFilter() + { + $request = new Request(); + $filter = new Required(); + + $this->assertEmpty( + $filter->validate('some_filter', [], $request) + ); + + $this->assertEmpty( + $filter->validate('some_filter', ['required' => false], $request) + ); + } + + public function testRequiredFilterNotInQuery() + { + $request = new Request(); + $filter = new Required(); + + $this->assertEquals( + ['Query parameter "some_filter" is required'], + $filter->validate('some_filter', ['required' => true], $request) + ); + } + + public function testRequiredFilterIsPresent() + { + $request = new Request(['some_filter' => 'some_value']); + $filter = new Required(); + + $this->assertEmpty( + $filter->validate('some_filter', ['required' => true], $request) + ); + } + + public function testEmptyValueNotAllowed() + { + $request = new Request(['some_filter' => '']); + $filter = new Required(); + + $explicitFilterDefinition = [ + 'required' => true, + 'swagger' => [ + 'allowEmptyValue' => false, + ], + ]; + + $this->assertEquals( + ['Query parameter "some_filter" does not allow empty value'], + $filter->validate('some_filter', $explicitFilterDefinition, $request) + ); + + $implicitFilterDefinition = [ + 'required' => true, + ]; + + $this->assertEquals( + ['Query parameter "some_filter" does not allow empty value'], + $filter->validate('some_filter', $implicitFilterDefinition, $request) + ); + } + + public function testEmptyValueAllowed() + { + $request = new Request(['some_filter' => '']); + $filter = new Required(); + + $explicitFilterDefinition = [ + 'required' => true, + 'swagger' => [ + 'allowEmptyValue' => true, + ], + ]; + + $this->assertEmpty( + $filter->validate('some_filter', $explicitFilterDefinition, $request) + ); + } +} From dbde28a038edea0a2b8e353834aec00cf359c7fd Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Wed, 26 Dec 2018 15:52:35 +0100 Subject: [PATCH 10/16] use float instead of number in Filters::getDescription --- tests/Fixtures/TestBundle/Filter/BoundsFilter.php | 8 ++++---- tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Fixtures/TestBundle/Filter/BoundsFilter.php b/tests/Fixtures/TestBundle/Filter/BoundsFilter.php index 4327e8377ea..ef46fc29f36 100644 --- a/tests/Fixtures/TestBundle/Filter/BoundsFilter.php +++ b/tests/Fixtures/TestBundle/Filter/BoundsFilter.php @@ -29,7 +29,7 @@ public function getDescription(string $resourceClass): array return [ 'maximum' => [ 'property' => 'maximum', - 'type' => 'number', + 'type' => 'float', 'required' => false, 'swagger' => [ 'maximum' => 10, @@ -37,7 +37,7 @@ public function getDescription(string $resourceClass): array ], 'exclusiveMaximum' => [ 'property' => 'maximum', - 'type' => 'number', + 'type' => 'float', 'required' => false, 'swagger' => [ 'maximum' => 10, @@ -46,7 +46,7 @@ public function getDescription(string $resourceClass): array ], 'minimum' => [ 'property' => 'minimum', - 'type' => 'number', + 'type' => 'float', 'required' => false, 'swagger' => [ 'minimum' => 5, @@ -54,7 +54,7 @@ public function getDescription(string $resourceClass): array ], 'exclusiveMinimum' => [ 'property' => 'exclusiveMinimum', - 'type' => 'number', + 'type' => 'float', 'required' => false, 'swagger' => [ 'minimum' => 5, diff --git a/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php b/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php index 6f0703bec8c..f806a99950d 100644 --- a/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php +++ b/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php @@ -29,7 +29,7 @@ public function getDescription(string $resourceClass): array return [ 'multiple-of' => [ 'property' => 'multiple-of', - 'type' => 'number', + 'type' => 'float', 'required' => false, 'swagger' => [ 'multipleOf' => 2, From 7f4f7e6df67ee639e1ac88befece657e4b3db1a4 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Wed, 26 Dec 2018 15:57:11 +0100 Subject: [PATCH 11/16] fix phpstan / cs-fixer returns --- src/Filter/Validator/ArrayItems.php | 2 +- src/Filter/Validator/Required.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Filter/Validator/ArrayItems.php b/src/Filter/Validator/ArrayItems.php index 42dfa298eed..960fa39f45e 100644 --- a/src/Filter/Validator/ArrayItems.php +++ b/src/Filter/Validator/ArrayItems.php @@ -61,7 +61,7 @@ private function getValue(string $name, array $filterDescription, Request $reque $collectionFormat = $filterDescription['swagger']['collectionFormat'] ?? 'csv'; - return explode(self::getSeparator($collectionFormat), $value); + return explode(self::getSeparator($collectionFormat), $value) ?: []; } private static function getSeparator(string $collectionFormat): string diff --git a/src/Filter/Validator/Required.php b/src/Filter/Validator/Required.php index ff6332bb590..d290881b3ea 100644 --- a/src/Filter/Validator/Required.php +++ b/src/Filter/Validator/Required.php @@ -60,12 +60,12 @@ private function requestHasQueryParameter(Request $request, string $name): bool if (\is_array($matches[$rootName])) { $keyName = array_keys($matches[$rootName])[0]; - $queryParameter = $request->query->get($rootName); + $queryParameter = $request->query->get((string) $rootName); return \is_array($queryParameter) && isset($queryParameter[$keyName]); } - return $request->query->has($rootName); + return $request->query->has((string) $rootName); } /** @@ -87,7 +87,7 @@ private function requestGetQueryParameter(Request $request, string $name) if (\is_array($matches[$rootName])) { $keyName = array_keys($matches[$rootName])[0]; - $queryParameter = $request->query->get($rootName); + $queryParameter = $request->query->get((string) $rootName); if (\is_array($queryParameter) && isset($queryParameter[$keyName])) { return $queryParameter[$keyName]; @@ -96,6 +96,6 @@ private function requestGetQueryParameter(Request $request, string $name) return null; } - return $request->query->get($rootName); + return $request->query->get((string) $rootName); } } From 7e4f145952eda910faee627e9be1ee12a32cecf8 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 25 Jan 2019 11:36:31 +0100 Subject: [PATCH 12/16] add filter tests for mongodb too --- .../TestBundle/Document/FilterValidator.php | 16 +++++++++++++++- .../TestBundle/Filter/ArrayItemsFilter.php | 3 +++ .../Fixtures/TestBundle/Filter/BoundsFilter.php | 3 +++ tests/Fixtures/TestBundle/Filter/EnumFilter.php | 3 +++ .../Fixtures/TestBundle/Filter/LengthFilter.php | 3 +++ .../TestBundle/Filter/MultipleOfFilter.php | 3 +++ .../Fixtures/TestBundle/Filter/PatternFilter.php | 3 +++ .../Filter/RequiredAllowEmptyFilter.php | 3 +++ 8 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/Fixtures/TestBundle/Document/FilterValidator.php b/tests/Fixtures/TestBundle/Document/FilterValidator.php index 722ac9f849f..1756965196a 100644 --- a/tests/Fixtures/TestBundle/Document/FilterValidator.php +++ b/tests/Fixtures/TestBundle/Document/FilterValidator.php @@ -15,6 +15,13 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\ArrayItemsFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\BoundsFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\EnumFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\LengthFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\MultipleOfFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\PatternFilter; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredAllowEmptyFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter\RequiredFilter; use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; @@ -26,7 +33,14 @@ * * @ApiResource(attributes={ * "filters"={ - * RequiredFilter::class + * ArrayItemsFilter::class, + * BoundsFilter::class, + * EnumFilter::class, + * LengthFilter::class, + * MultipleOfFilter::class, + * PatternFilter::class, + * RequiredFilter::class, + * RequiredAllowEmptyFilter::class * } * }) * @ODM\Document diff --git a/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php b/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php index 7feb9b3409f..848eaa34104 100644 --- a/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php +++ b/tests/Fixtures/TestBundle/Filter/ArrayItemsFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class ArrayItemsFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } diff --git a/tests/Fixtures/TestBundle/Filter/BoundsFilter.php b/tests/Fixtures/TestBundle/Filter/BoundsFilter.php index ef46fc29f36..9c3cca1984a 100644 --- a/tests/Fixtures/TestBundle/Filter/BoundsFilter.php +++ b/tests/Fixtures/TestBundle/Filter/BoundsFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class BoundsFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } diff --git a/tests/Fixtures/TestBundle/Filter/EnumFilter.php b/tests/Fixtures/TestBundle/Filter/EnumFilter.php index a2fe49b2598..1c4bd33fa27 100644 --- a/tests/Fixtures/TestBundle/Filter/EnumFilter.php +++ b/tests/Fixtures/TestBundle/Filter/EnumFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class EnumFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } diff --git a/tests/Fixtures/TestBundle/Filter/LengthFilter.php b/tests/Fixtures/TestBundle/Filter/LengthFilter.php index be9c30af0b2..6d44799156f 100644 --- a/tests/Fixtures/TestBundle/Filter/LengthFilter.php +++ b/tests/Fixtures/TestBundle/Filter/LengthFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class LengthFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } diff --git a/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php b/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php index f806a99950d..4918188526e 100644 --- a/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php +++ b/tests/Fixtures/TestBundle/Filter/MultipleOfFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class MultipleOfFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } diff --git a/tests/Fixtures/TestBundle/Filter/PatternFilter.php b/tests/Fixtures/TestBundle/Filter/PatternFilter.php index ccb9f56e731..1cb136520d6 100644 --- a/tests/Fixtures/TestBundle/Filter/PatternFilter.php +++ b/tests/Fixtures/TestBundle/Filter/PatternFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class PatternFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } diff --git a/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php b/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php index d0dc25882dd..8727b754d89 100644 --- a/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php +++ b/tests/Fixtures/TestBundle/Filter/RequiredAllowEmptyFilter.php @@ -13,12 +13,15 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Filter; +use ApiPlatform\Core\Bridge\Doctrine\Common\PropertyHelperTrait; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractFilter; use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use Doctrine\ORM\QueryBuilder; class RequiredAllowEmptyFilter extends AbstractFilter { + use PropertyHelperTrait; + protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) { } From 9cc5965bbe4b4223d78995411c17d0c8d1521269 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Mon, 28 Jan 2019 12:39:19 +0100 Subject: [PATCH 13/16] make validator classes final --- src/Bridge/Symfony/Validator/Validator.php | 2 ++ src/Filter/QueryParameterValidator.php | 2 +- src/Filter/Validator/ArrayItems.php | 2 +- src/Filter/Validator/Bounds.php | 2 +- src/Filter/Validator/Enum.php | 2 +- src/Filter/Validator/Length.php | 2 +- src/Filter/Validator/MultipleOf.php | 2 +- src/Filter/Validator/Pattern.php | 2 +- src/Filter/Validator/Required.php | 2 +- 9 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Bridge/Symfony/Validator/Validator.php b/src/Bridge/Symfony/Validator/Validator.php index 84a7e3a36dc..b804999e732 100644 --- a/src/Bridge/Symfony/Validator/Validator.php +++ b/src/Bridge/Symfony/Validator/Validator.php @@ -23,6 +23,8 @@ * Validates an item using the Symfony validator component. * * @author Kévin Dunglas + * + * @final */ class Validator implements ValidatorInterface { diff --git a/src/Filter/QueryParameterValidator.php b/src/Filter/QueryParameterValidator.php index 62f6f5d0e4a..7eb34c7791e 100644 --- a/src/Filter/QueryParameterValidator.php +++ b/src/Filter/QueryParameterValidator.php @@ -44,7 +44,7 @@ public function __construct(ContainerInterface $filterLocator) ]; } - public function validateFilters(string $resourceClass, array $resourceFilters, Request $request) + public function validateFilters(string $resourceClass, array $resourceFilters, Request $request): void { $errorList = []; foreach ($resourceFilters as $filterId) { diff --git a/src/Filter/Validator/ArrayItems.php b/src/Filter/Validator/ArrayItems.php index 960fa39f45e..517be613f84 100644 --- a/src/Filter/Validator/ArrayItems.php +++ b/src/Filter/Validator/ArrayItems.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class ArrayItems implements ValidatorInterface +final class ArrayItems implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { diff --git a/src/Filter/Validator/Bounds.php b/src/Filter/Validator/Bounds.php index 119d4c9dba6..a77481a4506 100644 --- a/src/Filter/Validator/Bounds.php +++ b/src/Filter/Validator/Bounds.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class Bounds implements ValidatorInterface +final class Bounds implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { diff --git a/src/Filter/Validator/Enum.php b/src/Filter/Validator/Enum.php index abc4b8b89df..8c7bd6d26bc 100644 --- a/src/Filter/Validator/Enum.php +++ b/src/Filter/Validator/Enum.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class Enum implements ValidatorInterface +final class Enum implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { diff --git a/src/Filter/Validator/Length.php b/src/Filter/Validator/Length.php index a64d370cec1..d129ef1dbb1 100644 --- a/src/Filter/Validator/Length.php +++ b/src/Filter/Validator/Length.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class Length implements ValidatorInterface +final class Length implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { diff --git a/src/Filter/Validator/MultipleOf.php b/src/Filter/Validator/MultipleOf.php index a4f7ee90fa1..4d9e397b03d 100644 --- a/src/Filter/Validator/MultipleOf.php +++ b/src/Filter/Validator/MultipleOf.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class MultipleOf implements ValidatorInterface +final class MultipleOf implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { diff --git a/src/Filter/Validator/Pattern.php b/src/Filter/Validator/Pattern.php index c6c46667416..d5dc6d3d736 100644 --- a/src/Filter/Validator/Pattern.php +++ b/src/Filter/Validator/Pattern.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class Pattern implements ValidatorInterface +final class Pattern implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { diff --git a/src/Filter/Validator/Required.php b/src/Filter/Validator/Required.php index d290881b3ea..466fe8fac1f 100644 --- a/src/Filter/Validator/Required.php +++ b/src/Filter/Validator/Required.php @@ -15,7 +15,7 @@ use Symfony\Component\HttpFoundation\Request; -class Required implements ValidatorInterface +final class Required implements ValidatorInterface { public function validate(string $name, array $filterDescription, Request $request): array { From 5d8cf4937f02ef82ac329777fe3f015007f6324c Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Mon, 28 Jan 2019 14:44:59 +0100 Subject: [PATCH 14/16] Use query parameters array instead of Symfony Request --- src/Filter/QueryParameterValidator.php | 2 +- src/Filter/Validator/ArrayItems.php | 15 ++++++------ src/Filter/Validator/Bounds.php | 9 ++++--- src/Filter/Validator/Enum.php | 9 ++++--- src/Filter/Validator/Length.php | 15 ++++++------ src/Filter/Validator/MultipleOf.php | 9 ++++--- src/Filter/Validator/Pattern.php | 9 ++++--- src/Filter/Validator/Required.php | 25 ++++++++++--------- src/Filter/Validator/ValidatorInterface.php | 9 ++++--- tests/Filter/Validator/ArrayItemsTest.php | 27 ++++++++++----------- tests/Filter/Validator/BoundsTest.php | 15 +++++------- tests/Filter/Validator/EnumTest.php | 13 +++------- tests/Filter/Validator/LengthTest.php | 25 +++++++++---------- tests/Filter/Validator/MultipleOfTest.php | 10 +++----- tests/Filter/Validator/PatternTest.php | 14 +++++------ tests/Filter/Validator/RequiredTest.php | 13 +++++----- 16 files changed, 107 insertions(+), 112 deletions(-) diff --git a/src/Filter/QueryParameterValidator.php b/src/Filter/QueryParameterValidator.php index 7eb34c7791e..2aac20e1e2b 100644 --- a/src/Filter/QueryParameterValidator.php +++ b/src/Filter/QueryParameterValidator.php @@ -54,7 +54,7 @@ public function validateFilters(string $resourceClass, array $resourceFilters, R foreach ($filter->getDescription($resourceClass) as $name => $data) { foreach ($this->validators as $validator) { - $errorList = array_merge($errorList, $validator->validate($name, $data, $request)); + $errorList = array_merge($errorList, $validator->validate($name, $data, $request->query->all())); } } } diff --git a/src/Filter/Validator/ArrayItems.php b/src/Filter/Validator/ArrayItems.php index 517be613f84..e29e7200f84 100644 --- a/src/Filter/Validator/ArrayItems.php +++ b/src/Filter/Validator/ArrayItems.php @@ -13,13 +13,14 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class ArrayItems implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { - if (!$request->query->has($name)) { + if (!\array_key_exists($name, $queryParameters)) { return []; } @@ -29,7 +30,7 @@ public function validate(string $name, array $filterDescription, Request $reques $errorList = []; - $value = $this->getValue($name, $filterDescription, $request); + $value = $this->getValue($name, $filterDescription, $queryParameters); $nbItems = \count($value); if (null !== $maxItems && $nbItems > $maxItems) { @@ -47,9 +48,9 @@ public function validate(string $name, array $filterDescription, Request $reques return $errorList; } - private function getValue(string $name, array $filterDescription, Request $request): array + private function getValue(string $name, array $filterDescription, array $queryParameters): array { - $value = $request->query->get($name); + $value = $queryParameters[$name] ?? null; if (empty($value) && '0' !== $value) { return []; diff --git a/src/Filter/Validator/Bounds.php b/src/Filter/Validator/Bounds.php index a77481a4506..bb7c974b2c1 100644 --- a/src/Filter/Validator/Bounds.php +++ b/src/Filter/Validator/Bounds.php @@ -13,13 +13,14 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class Bounds implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { - $value = $request->query->get($name); + $value = $queryParameters[$name] ?? null; if (empty($value) && '0' !== $value) { return []; } diff --git a/src/Filter/Validator/Enum.php b/src/Filter/Validator/Enum.php index 8c7bd6d26bc..5393de43ad0 100644 --- a/src/Filter/Validator/Enum.php +++ b/src/Filter/Validator/Enum.php @@ -13,13 +13,14 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class Enum implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { - $value = $request->query->get($name); + $value = $queryParameters[$name] ?? null; if (empty($value) && '0' !== $value || !\is_string($value)) { return []; } diff --git a/src/Filter/Validator/Length.php b/src/Filter/Validator/Length.php index d129ef1dbb1..6897ef57e02 100644 --- a/src/Filter/Validator/Length.php +++ b/src/Filter/Validator/Length.php @@ -13,20 +13,21 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class Length implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { - $maxLength = $filterDescription['swagger']['maxLength'] ?? null; - $minLength = $filterDescription['swagger']['minLength'] ?? null; - - $value = $request->query->get($name); + $value = $queryParameters[$name] ?? null; if (empty($value) && '0' !== $value || !\is_string($value)) { return []; } + $maxLength = $filterDescription['swagger']['maxLength'] ?? null; + $minLength = $filterDescription['swagger']['minLength'] ?? null; + $errorList = []; if (null !== $maxLength && mb_strlen($value) > $maxLength) { diff --git a/src/Filter/Validator/MultipleOf.php b/src/Filter/Validator/MultipleOf.php index 4d9e397b03d..75235007bdd 100644 --- a/src/Filter/Validator/MultipleOf.php +++ b/src/Filter/Validator/MultipleOf.php @@ -13,13 +13,14 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class MultipleOf implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { - $value = $request->query->get($name); + $value = $queryParameters[$name] ?? null; if (empty($value) && '0' !== $value || !\is_string($value)) { return []; } diff --git a/src/Filter/Validator/Pattern.php b/src/Filter/Validator/Pattern.php index d5dc6d3d736..5346feac04b 100644 --- a/src/Filter/Validator/Pattern.php +++ b/src/Filter/Validator/Pattern.php @@ -13,13 +13,14 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class Pattern implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { - $value = $request->query->get($name); + $value = $queryParameters[$name] ?? null; if (empty($value) && '0' !== $value || !\is_string($value)) { return []; } diff --git a/src/Filter/Validator/Required.php b/src/Filter/Validator/Required.php index 466fe8fac1f..b018f59e4b9 100644 --- a/src/Filter/Validator/Required.php +++ b/src/Filter/Validator/Required.php @@ -13,11 +13,12 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - final class Required implements ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array + /** + * {@inheritdoc} + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array { // filter is not required, the `checkRequired` method can not break if (!($filterDescription['required'] ?? false)) { @@ -25,14 +26,14 @@ public function validate(string $name, array $filterDescription, Request $reques } // if query param is not given, then break - if (!$this->requestHasQueryParameter($request, $name)) { + if (!$this->requestHasQueryParameter($queryParameters, $name)) { return [ sprintf('Query parameter "%s" is required', $name), ]; } // if query param is empty and the configuration does not allow it - if (!($filterDescription['swagger']['allowEmptyValue'] ?? false) && empty($this->requestGetQueryParameter($request, $name))) { + if (!($filterDescription['swagger']['allowEmptyValue'] ?? false) && empty($this->requestGetQueryParameter($queryParameters, $name))) { return [ sprintf('Query parameter "%s" does not allow empty value', $name), ]; @@ -44,7 +45,7 @@ public function validate(string $name, array $filterDescription, Request $reques /** * Test if request has required parameter. */ - private function requestHasQueryParameter(Request $request, string $name): bool + private function requestHasQueryParameter(array $queryParameters, string $name): bool { $matches = []; parse_str($name, $matches); @@ -60,18 +61,20 @@ private function requestHasQueryParameter(Request $request, string $name): bool if (\is_array($matches[$rootName])) { $keyName = array_keys($matches[$rootName])[0]; - $queryParameter = $request->query->get((string) $rootName); + $queryParameter = $queryParameters[(string) $rootName]; return \is_array($queryParameter) && isset($queryParameter[$keyName]); } - return $request->query->has((string) $rootName); + return \array_key_exists((string) $rootName, $queryParameters); } /** * Test if required filter is valid. It validates array notation too like "required[bar]". + * + * @return ?mixed */ - private function requestGetQueryParameter(Request $request, string $name) + private function requestGetQueryParameter(array $queryParameters, string $name) { $matches = []; parse_str($name, $matches); @@ -87,7 +90,7 @@ private function requestGetQueryParameter(Request $request, string $name) if (\is_array($matches[$rootName])) { $keyName = array_keys($matches[$rootName])[0]; - $queryParameter = $request->query->get((string) $rootName); + $queryParameter = $queryParameters[(string) $rootName]; if (\is_array($queryParameter) && isset($queryParameter[$keyName])) { return $queryParameter[$keyName]; @@ -96,6 +99,6 @@ private function requestGetQueryParameter(Request $request, string $name) return null; } - return $request->query->get((string) $rootName); + return $queryParameters[(string) $rootName]; } } diff --git a/src/Filter/Validator/ValidatorInterface.php b/src/Filter/Validator/ValidatorInterface.php index 33740725059..2a2113362b0 100644 --- a/src/Filter/Validator/ValidatorInterface.php +++ b/src/Filter/Validator/ValidatorInterface.php @@ -13,9 +13,12 @@ namespace ApiPlatform\Core\Filter\Validator; -use Symfony\Component\HttpFoundation\Request; - interface ValidatorInterface { - public function validate(string $name, array $filterDescription, Request $request): array; + /** + * @var string the parameter name to validate + * @var array $filterDescription the filter descriptions as returned by `ApiPlatform\Core\Api\FilterInterface::getDescription()` + * @var array $queryParameters the list of query parameter + */ + public function validate(string $name, array $filterDescription, array $queryParameters): array; } diff --git a/tests/Filter/Validator/ArrayItemsTest.php b/tests/Filter/Validator/ArrayItemsTest.php index 6e4d2a3e9a4..66aee6ee7e5 100644 --- a/tests/Filter/Validator/ArrayItemsTest.php +++ b/tests/Filter/Validator/ArrayItemsTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\ArrayItems; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * @author Julien Deniau @@ -24,7 +23,7 @@ class ArrayItemsTest extends TestCase { public function testNonDefinedFilter() { - $request = new Request(); + $request = []; $filter = new ArrayItems(); $this->assertEmpty( @@ -34,7 +33,7 @@ public function testNonDefinedFilter() public function testEmptyQueryParameter() { - $request = new Request(['some_filter' => '']); + $request = ['some_filter' => '']; $filter = new ArrayItems(); $this->assertEmpty( @@ -53,13 +52,13 @@ public function testNonMatchingParameter() ], ]; - $request = new Request(['some_filter' => ['foo', 'bar', 'bar', 'foo']]); + $request = ['some_filter' => ['foo', 'bar', 'bar', 'foo']]; $this->assertEquals( ['Query parameter "some_filter" must contain less than 3 values'], $filter->validate('some_filter', $filterDefinition, $request) ); - $request = new Request(['some_filter' => ['foo']]); + $request = ['some_filter' => ['foo']]; $this->assertEquals( ['Query parameter "some_filter" must contain more than 2 values'], $filter->validate('some_filter', $filterDefinition, $request) @@ -77,12 +76,12 @@ public function testMatchingParameter() ], ]; - $request = new Request(['some_filter' => ['foo', 'bar']]); + $request = ['some_filter' => ['foo', 'bar']]; $this->assertEmpty( $filter->validate('some_filter', $filterDefinition, $request) ); - $request = new Request(['some_filter' => ['foo', 'bar', 'baz']]); + $request = ['some_filter' => ['foo', 'bar', 'baz']]; $this->assertEmpty( $filter->validate('some_filter', $filterDefinition, $request) ); @@ -98,7 +97,7 @@ public function testNonMatchingUniqueItems() ], ]; - $request = new Request(['some_filter' => ['foo', 'bar', 'bar', 'foo']]); + $request = ['some_filter' => ['foo', 'bar', 'bar', 'foo']]; $this->assertEquals( ['Query parameter "some_filter" must contain unique values'], $filter->validate('some_filter', $filterDefinition, $request) @@ -115,7 +114,7 @@ public function testMatchingUniqueItems() ], ]; - $request = new Request(['some_filter' => ['foo', 'bar', 'baz']]); + $request = ['some_filter' => ['foo', 'bar', 'baz']]; $this->assertEmpty( $filter->validate('some_filter', $filterDefinition, $request) ); @@ -133,7 +132,7 @@ public function testSeparators() ], ]; - $request = new Request(['some_filter' => 'foo,bar,bar']); + $request = ['some_filter' => 'foo,bar,bar']; $this->assertEquals( [ 'Query parameter "some_filter" must contain less than 2 values', @@ -148,7 +147,7 @@ public function testSeparators() ); $filterDefinition['swagger']['collectionFormat'] = 'ssv'; - $request = new Request(['some_filter' => 'foo bar bar']); + $request = ['some_filter' => 'foo bar bar']; $this->assertEquals( [ 'Query parameter "some_filter" must contain less than 2 values', @@ -158,7 +157,7 @@ public function testSeparators() ); $filterDefinition['swagger']['collectionFormat'] = 'tsv'; - $request = new Request(['some_filter' => 'foo\tbar\tbar']); + $request = ['some_filter' => 'foo\tbar\tbar']; $this->assertEquals( [ 'Query parameter "some_filter" must contain less than 2 values', @@ -168,7 +167,7 @@ public function testSeparators() ); $filterDefinition['swagger']['collectionFormat'] = 'pipes'; - $request = new Request(['some_filter' => 'foo|bar|bar']); + $request = ['some_filter' => 'foo|bar|bar']; $this->assertEquals( [ 'Query parameter "some_filter" must contain less than 2 values', @@ -189,7 +188,7 @@ public function testSeparatorsUnknownSeparator() 'collectionFormat' => 'unknownFormat', ], ]; - $request = new Request(['some_filter' => 'foo,bar,bar']); + $request = ['some_filter' => 'foo,bar,bar']; $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Unknown collection format unknownFormat'); diff --git a/tests/Filter/Validator/BoundsTest.php b/tests/Filter/Validator/BoundsTest.php index d993777a911..50f2958ec43 100644 --- a/tests/Filter/Validator/BoundsTest.php +++ b/tests/Filter/Validator/BoundsTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\Bounds; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * @author Julien Deniau @@ -24,27 +23,25 @@ class BoundsTest extends TestCase { public function testNonDefinedFilter() { - $request = new Request(); $filter = new Bounds(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], []) ); } public function testEmptyQueryParameter() { - $request = new Request(['some_filter' => '']); $filter = new Bounds(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], ['some_filter' => '']) ); } public function testNonMatchingMinimum() { - $request = new Request(['some_filter' => '9']); + $request = ['some_filter' => '9']; $filter = new Bounds(); $filterDefinition = [ @@ -85,7 +82,7 @@ public function testNonMatchingMinimum() public function testMatchingMinimum() { - $request = new Request(['some_filter' => '10']); + $request = ['some_filter' => '10']; $filter = new Bounds(); $filterDefinition = [ @@ -112,7 +109,7 @@ public function testMatchingMinimum() public function testNonMatchingMaximum() { - $request = new Request(['some_filter' => '11']); + $request = ['some_filter' => '11']; $filter = new Bounds(); $filterDefinition = [ @@ -153,7 +150,7 @@ public function testNonMatchingMaximum() public function testMatchingMaximum() { - $request = new Request(['some_filter' => '10']); + $request = ['some_filter' => '10']; $filter = new Bounds(); $filterDefinition = [ diff --git a/tests/Filter/Validator/EnumTest.php b/tests/Filter/Validator/EnumTest.php index e55697e4376..bd55f076a65 100644 --- a/tests/Filter/Validator/EnumTest.php +++ b/tests/Filter/Validator/EnumTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\Enum; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * @author Julien Deniau @@ -24,27 +23,24 @@ class EnumTest extends TestCase { public function testNonDefinedFilter() { - $request = new Request(); $filter = new Enum(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], []) ); } public function testEmptyQueryParameter() { - $request = new Request(['some_filter' => '']); $filter = new Enum(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], ['some_filter' => '']) ); } public function testNonMatchingParameter() { - $request = new Request(['some_filter' => 'foobar']); $filter = new Enum(); $filterDefinition = [ @@ -55,13 +51,12 @@ public function testNonMatchingParameter() $this->assertEquals( ['Query parameter "some_filter" must be one of "foo, bar"'], - $filter->validate('some_filter', $filterDefinition, $request) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'foobar']) ); } public function testMatchingParameter() { - $request = new Request(['some_filter' => 'foo']); $filter = new Enum(); $filterDefinition = [ @@ -71,7 +66,7 @@ public function testMatchingParameter() ]; $this->assertEmpty( - $filter->validate('some_filter', $filterDefinition, $request) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'foo']) ); } } diff --git a/tests/Filter/Validator/LengthTest.php b/tests/Filter/Validator/LengthTest.php index 76e60645af2..d9f36b3500c 100644 --- a/tests/Filter/Validator/LengthTest.php +++ b/tests/Filter/Validator/LengthTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\Length; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * @author Julien Deniau @@ -24,21 +23,19 @@ class LengthTest extends TestCase { public function testNonDefinedFilter() { - $request = new Request(); $filter = new Length(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], []) ); } public function testEmptyQueryParameter() { - $request = new Request(['some_filter' => '']); $filter = new Length(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], ['some_filter' => '']) ); } @@ -55,12 +52,12 @@ public function testNonMatchingParameter() $this->assertEquals( ['Query parameter "some_filter" length must be greater than or equal to 3'], - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'ab'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'ab']) ); $this->assertEquals( ['Query parameter "some_filter" length must be lower than or equal to 5'], - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcdef'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abcdef']) ); } @@ -76,7 +73,7 @@ public function testNonMatchingParameterWithOnlyOneDefinition() $this->assertEquals( ['Query parameter "some_filter" length must be greater than or equal to 3'], - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'ab'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'ab']) ); $filterDefinition = [ @@ -87,7 +84,7 @@ public function testNonMatchingParameterWithOnlyOneDefinition() $this->assertEquals( ['Query parameter "some_filter" length must be lower than or equal to 5'], - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcdef'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abcdef']) ); } @@ -103,15 +100,15 @@ public function testMatchingParameter() ]; $this->assertEmpty( - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abc'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abc']) ); $this->assertEmpty( - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcd'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abcd']) ); $this->assertEmpty( - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcde'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abcde']) ); } @@ -126,7 +123,7 @@ public function testMatchingParameterWithOneDefinition() ]; $this->assertEmpty( - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abc'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abc']) ); $filterDefinition = [ @@ -136,7 +133,7 @@ public function testMatchingParameterWithOneDefinition() ]; $this->assertEmpty( - $filter->validate('some_filter', $filterDefinition, new Request(['some_filter' => 'abcde'])) + $filter->validate('some_filter', $filterDefinition, ['some_filter' => 'abcde']) ); } } diff --git a/tests/Filter/Validator/MultipleOfTest.php b/tests/Filter/Validator/MultipleOfTest.php index a90386db765..f313cd3cba2 100644 --- a/tests/Filter/Validator/MultipleOfTest.php +++ b/tests/Filter/Validator/MultipleOfTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\MultipleOf; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * @author Julien Deniau @@ -24,17 +23,16 @@ class MultipleOfTest extends TestCase { public function testNonDefinedFilter() { - $request = new Request(); $filter = new MultipleOf(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], []) ); } public function testEmptyQueryParameter() { - $request = new Request(['some_filter' => '']); + $request = ['some_filter' => '']; $filter = new MultipleOf(); $this->assertEmpty( @@ -44,7 +42,7 @@ public function testEmptyQueryParameter() public function testNonMatchingParameter() { - $request = new Request(['some_filter' => '8']); + $request = ['some_filter' => '8']; $filter = new MultipleOf(); $filterDefinition = [ @@ -61,7 +59,7 @@ public function testNonMatchingParameter() public function testMatchingParameter() { - $request = new Request(['some_filter' => '8']); + $request = ['some_filter' => '8']; $filter = new MultipleOf(); $filterDefinition = [ diff --git a/tests/Filter/Validator/PatternTest.php b/tests/Filter/Validator/PatternTest.php index 02a924a8f5b..18f58aff86c 100644 --- a/tests/Filter/Validator/PatternTest.php +++ b/tests/Filter/Validator/PatternTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\Pattern; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * @author Julien Deniau @@ -24,11 +23,10 @@ class PatternTest extends TestCase { public function testNonDefinedFilter() { - $request = new Request(); $filter = new Pattern(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], []) ); } @@ -43,13 +41,13 @@ public function testFilterWithEmptyValue() ]; $this->assertEmpty( - $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => ''])) + $filter->validate('some_filter', $explicitFilterDefinition, ['some_filter' => '']) ); $weirdParameter = new \stdClass(); $weirdParameter->foo = 'non string value should not exists'; $this->assertEmpty( - $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => $weirdParameter])) + $filter->validate('some_filter', $explicitFilterDefinition, ['some_filter' => $weirdParameter]) ); } @@ -65,7 +63,7 @@ public function testFilterWithZeroAsParameter() $this->assertEquals( ['Query parameter "some_filter" must match pattern /foo/'], - $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => '0'])) + $filter->validate('some_filter', $explicitFilterDefinition, ['some_filter' => '0']) ); } @@ -81,7 +79,7 @@ public function testFilterWithNonMatchingValue() $this->assertEquals( ['Query parameter "some_filter" must match pattern /foo/'], - $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => 'bar'])) + $filter->validate('some_filter', $explicitFilterDefinition, ['some_filter' => 'bar']) ); } @@ -96,7 +94,7 @@ public function testFilterWithNonchingValue() ]; $this->assertEmpty( - $filter->validate('some_filter', $explicitFilterDefinition, new Request(['some_filter' => 'this is a foo '.random_int(0, 10).' and it should match'])) + $filter->validate('some_filter', $explicitFilterDefinition, ['some_filter' => 'this is a foo '.random_int(0, 10).' and it should match']) ); } } diff --git a/tests/Filter/Validator/RequiredTest.php b/tests/Filter/Validator/RequiredTest.php index b2c1596c144..fa7cd89ed97 100644 --- a/tests/Filter/Validator/RequiredTest.php +++ b/tests/Filter/Validator/RequiredTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Core\Filter\Validator\Required; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; /** * Class RequiredTest. @@ -26,11 +25,11 @@ class RequiredTest extends TestCase { public function testNonRequiredFilter() { - $request = new Request(); + $request = []; $filter = new Required(); $this->assertEmpty( - $filter->validate('some_filter', [], $request) + $filter->validate('some_filter', [], []) ); $this->assertEmpty( @@ -40,7 +39,7 @@ public function testNonRequiredFilter() public function testRequiredFilterNotInQuery() { - $request = new Request(); + $request = []; $filter = new Required(); $this->assertEquals( @@ -51,7 +50,7 @@ public function testRequiredFilterNotInQuery() public function testRequiredFilterIsPresent() { - $request = new Request(['some_filter' => 'some_value']); + $request = ['some_filter' => 'some_value']; $filter = new Required(); $this->assertEmpty( @@ -61,7 +60,7 @@ public function testRequiredFilterIsPresent() public function testEmptyValueNotAllowed() { - $request = new Request(['some_filter' => '']); + $request = ['some_filter' => '']; $filter = new Required(); $explicitFilterDefinition = [ @@ -88,7 +87,7 @@ public function testEmptyValueNotAllowed() public function testEmptyValueAllowed() { - $request = new Request(['some_filter' => '']); + $request = ['some_filter' => '']; $filter = new Required(); $explicitFilterDefinition = [ From 167f8061cd99f678e1d8450bbd55c39a919308b1 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Mon, 28 Jan 2019 15:05:55 +0100 Subject: [PATCH 15/16] use internal RequestParser instead of $request->query->all() --- .../QueryParameterValidateListener.php | 5 ++++- src/Filter/QueryParameterValidator.php | 6 +++--- src/Filter/Validator/Required.php | 4 ++-- .../QueryParameterValidateListenerTest.php | 13 ++++++++----- tests/Filter/QueryParameterValidatorTest.php | 11 ++++------- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/EventListener/QueryParameterValidateListener.php b/src/EventListener/QueryParameterValidateListener.php index 14990387485..2d2ca127cc2 100644 --- a/src/EventListener/QueryParameterValidateListener.php +++ b/src/EventListener/QueryParameterValidateListener.php @@ -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\RequestEvent; /** @@ -46,10 +47,12 @@ public function onKernelRequest(RequestEvent $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); } } diff --git a/src/Filter/QueryParameterValidator.php b/src/Filter/QueryParameterValidator.php index 2aac20e1e2b..5a85a22333a 100644 --- a/src/Filter/QueryParameterValidator.php +++ b/src/Filter/QueryParameterValidator.php @@ -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. @@ -44,9 +43,10 @@ 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; @@ -54,7 +54,7 @@ public function validateFilters(string $resourceClass, array $resourceFilters, R 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)); } } } diff --git a/src/Filter/Validator/Required.php b/src/Filter/Validator/Required.php index b018f59e4b9..4c5fb2d92a8 100644 --- a/src/Filter/Validator/Required.php +++ b/src/Filter/Validator/Required.php @@ -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]); } @@ -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]; diff --git a/tests/EventListener/QueryParameterValidateListenerTest.php b/tests/EventListener/QueryParameterValidateListenerTest.php index 2c52067f715..8a0b53f8361 100644 --- a/tests/EventListener/QueryParameterValidateListenerTest.php +++ b/tests/EventListener/QueryParameterValidateListenerTest.php @@ -59,7 +59,7 @@ public function testOnKernelRequestWithWrongFilter() $eventProphecy = $this->prophesize(RequestEvent::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()) @@ -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'])); $this->expectException(FilterValidationException::class); @@ -96,9 +96,12 @@ 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'); @@ -106,7 +109,7 @@ public function testOnKernelRequestWithRequiredFilter() $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); $this->queryParameterValidor - ->validateFilters(Dummy::class, ['some_filter'], $request) + ->validateFilters(Dummy::class, ['some_filter'], ['required' => 'foo']) ->shouldBeCalled(); $this->assertNull( diff --git a/tests/Filter/QueryParameterValidatorTest.php b/tests/Filter/QueryParameterValidatorTest.php index c0f781407a2..1c9e60f06f6 100644 --- a/tests/Filter/QueryParameterValidatorTest.php +++ b/tests/Filter/QueryParameterValidatorTest.php @@ -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. @@ -48,7 +47,7 @@ protected function setUp(): void */ public function testOnKernelRequestWithUnsafeMethod() { - $request = new Request(); + $request = []; $this->assertNull( $this->testedInstance->validateFilters(Dummy::class, [], $request) @@ -60,7 +59,7 @@ public function testOnKernelRequestWithUnsafeMethod() */ public function testOnKernelRequestWithWrongFilter() { - $request = new Request(); + $request = []; $this->assertNull( $this->testedInstance->validateFilters(Dummy::class, ['some_inexistent_filter'], $request) @@ -72,7 +71,7 @@ public function testOnKernelRequestWithWrongFilter() */ public function testOnKernelRequestWithRequiredFilterNotSet() { - $request = new Request(); + $request = []; $filterProphecy = $this->prophesize(FilterInterface::class); $filterProphecy @@ -102,9 +101,7 @@ public function testOnKernelRequestWithRequiredFilterNotSet() */ public function testOnKernelRequestWithRequiredFilter() { - $request = new Request( - ['required' => 'foo'] - ); + $request = ['required' => 'foo']; $this->filterLocatorProphecy ->has('some_filter') From 3ec427e85ccaeff23aca620335bdd0b7c5da3ded Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Wed, 4 Dec 2019 23:13:15 +0100 Subject: [PATCH 16/16] Add support of collection get operations with different names ( thanks @TracKer ) --- src/EventListener/QueryParameterValidateListener.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EventListener/QueryParameterValidateListener.php b/src/EventListener/QueryParameterValidateListener.php index 2d2ca127cc2..e9334c1583c 100644 --- a/src/EventListener/QueryParameterValidateListener.php +++ b/src/EventListener/QueryParameterValidateListener.php @@ -43,7 +43,8 @@ public function onKernelRequest(RequestEvent $event) !$request->isMethodSafe() || !($attributes = RequestAttributesExtractor::extractAttributes($request)) || !isset($attributes['collection_operation_name']) - || 'get' !== ($operationName = $attributes['collection_operation_name']) + || !($operationName = $attributes['collection_operation_name']) + || 'GET' !== $request->getMethod() ) { return; }