Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(validation): validate query parameters with multiple values #5890

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions src/ParameterValidator/ParameterValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use ApiPlatform\ParameterValidator\Validator\MultipleOf;
use ApiPlatform\ParameterValidator\Validator\Pattern;
use ApiPlatform\ParameterValidator\Validator\Required;
use ApiPlatform\ParameterValidator\Validator\ValidatorInterface;
use Psr\Container\ContainerInterface;

/**
Expand All @@ -32,6 +33,7 @@
{
use FilterLocatorTrait;

/** @var list<ValidatorInterface> */
private array $validators;

public function __construct(ContainerInterface $filterLocator)
Expand Down Expand Up @@ -59,16 +61,40 @@
}

foreach ($filter->getDescription($resourceClass) as $name => $data) {
foreach ($this->validators as $validator) {
if ($errors = $validator->validate($name, $data, $queryParameters)) {
$errorList[] = $errors;
$collectionFormat = ParameterValueExtractor::getCollectionFormat($data);
$validatorErrors = [];

// validate simple values
foreach ($this->validate($name, $data, $queryParameters) as $error) {
$validatorErrors[] = $error;

Check warning on line 69 in src/ParameterValidator/ParameterValidator.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValidator.php#L69

Added line #L69 was not covered by tests
}

// manipulate query data to validate each value
foreach (ParameterValueExtractor::iterateValue($name, $queryParameters, $collectionFormat) as $scalarQueryParameters) {
foreach ($this->validate($name, $data, $scalarQueryParameters) as $error) {
$validatorErrors[] = $error;

Check warning on line 75 in src/ParameterValidator/ParameterValidator.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValidator.php#L74-L75

Added lines #L74 - L75 were not covered by tests
}
}

if ($validatorErrors) {
// Remove duplicate messages
$errorList[] = array_unique($validatorErrors);

Check warning on line 81 in src/ParameterValidator/ParameterValidator.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValidator.php#L81

Added line #L81 was not covered by tests
}
}
}

if ($errorList) {
throw new ValidationException(array_merge(...$errorList));
}
}

/** @return iterable<string> validation errors that occured */
private function validate(string $name, array $data, array $queryParameters): iterable
{
foreach ($this->validators as $validator) {
foreach ($validator->validate($name, $data, $queryParameters) as $error) {
yield $error;

Check warning on line 96 in src/ParameterValidator/ParameterValidator.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValidator.php#L96

Added line #L96 was not covered by tests
}
}
}
}
79 changes: 79 additions & 0 deletions src/ParameterValidator/ParameterValueExtractor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* 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\ParameterValidator;

/**
* Extract values from parameters.
*
* @internal
*
* @author Nicolas LAURENT <[email protected]>
*/
class ParameterValueExtractor
{
/**
* @param int|int[]|string|string[] $value
*
* @return int[]|string[]
*/
public static function getValue(int|string|array $value, string $collectionFormat = 'csv'): array

Check warning on line 30 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L30

Added line #L30 was not covered by tests
{
if (\is_array($value)) {
return $value;

Check warning on line 33 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L32-L33

Added lines #L32 - L33 were not covered by tests
}

if (\is_string($value)) {
return explode(self::getSeparator($collectionFormat), $value);

Check warning on line 37 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L36-L37

Added lines #L36 - L37 were not covered by tests
}

return [$value];

Check warning on line 40 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L40

Added line #L40 was not covered by tests
}

/** @return non-empty-string */
public static function getSeparator(string $collectionFormat): string

Check warning on line 44 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L44

Added line #L44 was not covered by tests
{
return match ($collectionFormat) {
'csv' => ',',
'ssv' => ' ',
'tsv' => '\t',
'pipes' => '|',
default => throw new \InvalidArgumentException(sprintf('Unknown collection format %s', $collectionFormat)),
};

Check warning on line 52 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L46-L52

Added lines #L46 - L52 were not covered by tests
}

/**
* @param array<string, array<string, mixed>> $filterDescription
*/
public static function getCollectionFormat(array $filterDescription): string
{
return $filterDescription['openapi']['collectionFormat'] ?? $filterDescription['swagger']['collectionFormat'] ?? 'csv';
}

/**
* @param array<string, mixed> $queryParameters
*
* @throws \InvalidArgumentException
*/
public static function iterateValue(string $name, array $queryParameters, string $collectionFormat = 'csv'): \Generator
{
foreach ($queryParameters as $key => $value) {
if ($key === $name || "{$key}[]" === $name) {
$values = self::getValue($value, $collectionFormat);
foreach ($values as $v) {
yield [$key => $v];

Check warning on line 74 in src/ParameterValidator/ParameterValueExtractor.php

View check run for this annotation

Codecov / codecov/patch

src/ParameterValidator/ParameterValueExtractor.php#L71-L74

Added lines #L71 - L74 were not covered by tests
}
}
}
}
}
107 changes: 107 additions & 0 deletions src/ParameterValidator/Tests/ParameterValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,111 @@ public function testOnKernelRequestWithRequiredFilter(): void

$this->testedInstance->validateFilters(Dummy::class, ['some_filter'], $request);
}

/**
* @dataProvider provideValidateNonScalarsCases
*/
public function testValidateNonScalars(array $request, array $description, string|null $exceptionMessage): void
{
$this->filterLocatorProphecy
->has('some_filter')
->shouldBeCalled()
->willReturn(true);

$filterProphecy = $this->prophesize(FilterInterface::class);
$filterProphecy
->getDescription(Dummy::class)
->shouldBeCalled()
->willReturn($description);

$this->filterLocatorProphecy
->get('some_filter')
->shouldBeCalled()
->willReturn($filterProphecy->reveal());

if (null !== $exceptionMessage) {
$this->expectException(ValidationException::class);
$this->expectExceptionMessageMatches('#^'.preg_quote($exceptionMessage).'$#');
}

$this->testedInstance->validateFilters(Dummy::class, ['some_filter'], $request);
}

public function provideValidateNonScalarsCases(): iterable
{
$enum = ['parameter' => [
'openapi' => [
'enum' => ['foo', 'bar'],
],
]];

yield 'valid values should not throw' => [
['parameter' => 'bar'], $enum, null,
];

yield 'invalid single scalar should still throw' => [
['parameter' => 'baz'], $enum, 'Query parameter "parameter" must be one of "foo, bar"',
];

yield 'invalid single value in a non scalar should throw' => [
['parameter' => ['baz']], $enum, 'Query parameter "parameter" must be one of "foo, bar"',
];

yield 'multiple invalid values in a non scalar should throw' => [
['parameter' => ['baz', 'boo']], $enum, 'Query parameter "parameter" must be one of "foo, bar"',
];

yield 'combination of valid and invalid values should throw' => [
['parameter' => ['foo', 'boo']], $enum, 'Query parameter "parameter" must be one of "foo, bar"',
];

yield 'duplicate valid values should throw' => [
['parameter' => ['foo', 'foo']],
['parameter' => [
'openapi' => [
'enum' => ['foo', 'bar'],
'uniqueItems' => true,
],
]],
'Query parameter "parameter" must contain unique values',
];

yield 'if less values than allowed is provided it should throw' => [
['parameter' => ['foo']],
['parameter' => [
'openapi' => [
'enum' => ['foo', 'bar'],
'minItems' => 2,
],
]],
'Query parameter "parameter" must contain more than 2 values', // todo: this message does seem accurate
];

yield 'if more values than allowed is provided it should throw' => [
['parameter' => ['foo', 'bar', 'baz']],
['parameter' => [
'openapi' => [
'enum' => ['foo', 'bar', 'baz'],
'maxItems' => 2,
],
]],
'Query parameter "parameter" must contain less than 2 values', // todo: this message does seem accurate
];

yield 'for array constraints all violation should be reported' => [
['parameter' => ['foo', 'foo', 'bar']],
['parameter' => [
'openapi' => [
'enum' => ['foo', 'bar'],
'uniqueItems' => true,
'minItems' => 1,
'maxItems' => 2,
],
]],
implode(\PHP_EOL, [
'Query parameter "parameter" must contain less than 2 values',
'Query parameter "parameter" must contain unique values',
]),
];
}
}
131 changes: 131 additions & 0 deletions src/ParameterValidator/Tests/ParameterValueExtractorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* 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\ParameterValidator\Tests;

use ApiPlatform\ParameterValidator\ParameterValueExtractor;
use PHPUnit\Framework\TestCase;

/**
* @author Nicolas LAURENT <[email protected]>
*/
class ParameterValueExtractorTest extends TestCase
{
private const SUPPORTED_SEPARATORS = [
'csv' => ',',
'ssv' => ' ',
'tsv' => '\t',
'pipes' => '|',
];

/**
* {@inheritdoc}
*/
protected function setUp(): void
{
}

/**
* @dataProvider provideGetCollectionFormatCases
*/
public function testGetCollectionFormat(array $filterDescription, string $expectedResult): void
{
$this->assertSame($expectedResult, ParameterValueExtractor::getCollectionFormat($filterDescription));
}

/**
* @return iterable<array{array, string}>
*/
public function provideGetCollectionFormatCases(): iterable
{
yield 'empty description' => [
[], 'csv',
];

yield 'swagger description' => [
['swagger' => ['collectionFormat' => 'foo']], 'foo',
];

yield 'openapi description' => [
['openapi' => ['collectionFormat' => 'bar']], 'bar',
];
}

/**
* @dataProvider provideGetSeparatorCases
*/
public function testGetSeparator(string $separatorName, string $expectedSeparator, string|null $expectedException): void
{
if ($expectedException) {
$this->expectException($expectedException);
}
self::assertSame($expectedSeparator, ParameterValueExtractor::getSeparator($separatorName));
}

/**
* @return iterable<array{string, string, string|null}>
*/
public function provideGetSeparatorCases(): iterable
{
yield 'empty separator' => [
'', '', \InvalidArgumentException::class,
];

foreach (self::SUPPORTED_SEPARATORS as $separatorName => $expectedSeparator) {
yield "using '{$separatorName}'" => [
$separatorName, $expectedSeparator, null,
];
}
}

/**
* @dataProvider provideGetValueCases
*
* @param int[]|string[] $expectedValue
* @param int|int[]|string|string[] $value
*/
public function testGetValue(array $expectedValue, int|string|array $value, string $collectionFormat): void
{
self::assertSame($expectedValue, ParameterValueExtractor::getValue($value, $collectionFormat));
}

/**
* @return iterable<array{int[]|string[], int|string|int[]|string[], string}>
*/
public function provideGetValueCases(): iterable
{
yield 'empty input' => [
[], [], 'csv',
];

yield 'comma separated value' => [
['foo', 'bar'], 'foo,bar', 'csv',
];

yield 'space separated value' => [
['foo', 'bar'], 'foo bar', 'ssv',
];

yield 'tab separated value' => [
['foo', 'bar'], 'foo\tbar', 'tsv',
];

yield 'pipe separated value' => [
['foo', 'bar'], 'foo|bar', 'pipes',
];

yield 'array values' => [
['foo', 'bar'], ['foo', 'bar'], 'csv',
];
}
}
Loading
Loading