Skip to content

Commit 22cbd01

Browse files
authored
fix(metadata): various parameter improvements (#6867)
- `Parameter::getValue()` now takes a default value as argument `getValue(mixed $default = new ParameterNotFound()): mixed` - `Parametes::get(string $key, string $parameterClass = QueryParameter::class)` (but also `has` and `remove`) now has a default value as second argument to `QueryParameter::class` - Constraint violation had the wrong message when using `property`, fixed by using the `key` instead:
1 parent 5ce7596 commit 22cbd01

File tree

8 files changed

+119
-14
lines changed

8 files changed

+119
-14
lines changed

features/filter/filter_validation.feature

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ Feature: Validate filters based upon filter description
2525
Scenario: Required filter should throw an error if not set
2626
When I am on "/array_filter_validators"
2727
Then the response status code should be 422
28-
And the JSON node "detail" should be equal to 'arrayRequired: This value should not be blank.\nindexedArrayRequired: This value should not be blank.'
28+
And the JSON node "detail" should be equal to 'arrayRequired[]: This value should not be blank.\nindexedArrayRequired[foo]: This value should not be blank.'
2929

3030
When I am on "/array_filter_validators?arrayRequired[foo]=foo"
3131
Then the response status code should be 422
32-
And the JSON node "detail" should be equal to 'indexedArrayRequired: This value should not be blank.'
32+
And the JSON node "detail" should be equal to 'indexedArrayRequired[foo]: This value should not be blank.'
3333

3434
When I am on "/array_filter_validators?arrayRequired[]=foo"
3535
Then the response status code should be 422
36-
And the JSON node "detail" should be equal to 'indexedArrayRequired: This value should not be blank.'
36+
And the JSON node "detail" should be equal to 'indexedArrayRequired[foo]: This value should not be blank.'
3737

3838
Scenario: Test filter bounds: maximum
3939
When I am on "/filter_validators?required=foo&required-allow-empty&maximum=10"
@@ -49,7 +49,7 @@ Feature: Validate filters based upon filter description
4949

5050
When I am on "/filter_validators?required=foo&required-allow-empty&exclusiveMaximum=10"
5151
Then the response status code should be 422
52-
And the JSON node "detail" should be equal to 'maximum: This value should be less than 10.'
52+
And the JSON node "detail" should be equal to 'exclusiveMaximum: This value should be less than 10.'
5353

5454
Scenario: Test filter bounds: minimum
5555
When I am on "/filter_validators?required=foo&required-allow-empty&minimum=5"

src/Metadata/Parameter.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ public function getSecurityMessage(): ?string
124124
*
125125
* @readonly
126126
*/
127-
public function getValue(): mixed
127+
public function getValue(mixed $default = new ParameterNotFound()): mixed
128128
{
129-
return $this->extraProperties['_api_values'] ?? new ParameterNotFound();
129+
return $this->extraProperties['_api_values'] ?? $default;
130130
}
131131

132132
/**

src/Metadata/Parameters.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function add(string $key, Parameter $value): self
7070
/**
7171
* @param class-string $parameterClass
7272
*/
73-
public function remove(string $key, string $parameterClass): self
73+
public function remove(string $key, string $parameterClass = QueryParameter::class): self
7474
{
7575
foreach ($this->parameters as $i => [$parameterName, $parameter]) {
7676
if ($parameterName === $key && $parameterClass === $parameter::class) {
@@ -86,7 +86,7 @@ public function remove(string $key, string $parameterClass): self
8686
/**
8787
* @param class-string $parameterClass
8888
*/
89-
public function get(string $key, string $parameterClass): ?Parameter
89+
public function get(string $key, string $parameterClass = QueryParameter::class): ?Parameter
9090
{
9191
foreach ($this->parameters as [$parameterName, $parameter]) {
9292
if ($parameterName === $key && $parameterClass === $parameter::class) {
@@ -100,7 +100,7 @@ public function get(string $key, string $parameterClass): ?Parameter
100100
/**
101101
* @param class-string $parameterClass
102102
*/
103-
public function has(string $key, string $parameterClass): bool
103+
public function has(string $key, string $parameterClass = QueryParameter::class): bool
104104
{
105105
foreach ($this->parameters as [$parameterName, $parameter]) {
106106
if ($parameterName === $key && $parameterClass === $parameter::class) {

src/Metadata/Tests/ParameterTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Metadata\Tests;
15+
16+
use ApiPlatform\Metadata\QueryParameter;
17+
use ApiPlatform\State\ParameterNotFound;
18+
use PHPUnit\Framework\TestCase;
19+
20+
class ParameterTest extends TestCase
21+
{
22+
public function testDefaultValue(): void
23+
{
24+
$parameter = new QueryParameter();
25+
$this->assertSame('test', $parameter->getValue('test'));
26+
$this->assertInstanceOf(ParameterNotFound::class, $parameter->getValue());
27+
}
28+
}

src/Metadata/Tests/ParametersTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Metadata\Tests;
15+
16+
use ApiPlatform\Metadata\Parameters;
17+
use ApiPlatform\Metadata\QueryParameter;
18+
use PHPUnit\Framework\TestCase;
19+
20+
class ParametersTest extends TestCase
21+
{
22+
public function testDefaultValue(): void
23+
{
24+
$r = new QueryParameter();
25+
$parameters = new Parameters(['a' => $r]);
26+
$this->assertSame($r, $parameters->get('a'));
27+
}
28+
}

src/Symfony/Validator/State/ParameterValidatorProvider.php

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@
1414
namespace ApiPlatform\Symfony\Validator\State;
1515

1616
use ApiPlatform\Metadata\Operation;
17+
use ApiPlatform\Metadata\Parameter;
1718
use ApiPlatform\State\ParameterNotFound;
1819
use ApiPlatform\State\ProviderInterface;
1920
use ApiPlatform\State\Util\ParameterParserTrait;
2021
use ApiPlatform\Validator\Exception\ValidationException;
2122
use Symfony\Component\HttpFoundation\Request;
2223
use Symfony\Component\Validator\ConstraintViolation;
24+
use Symfony\Component\Validator\ConstraintViolationInterface;
2325
use Symfony\Component\Validator\ConstraintViolationList;
2426
use Symfony\Component\Validator\Validator\ValidatorInterface;
2527

@@ -55,7 +57,6 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
5557
continue;
5658
}
5759

58-
$key = $parameter->getKey();
5960
$value = $parameter->getValue();
6061
if ($value instanceof ParameterNotFound) {
6162
$value = null;
@@ -68,9 +69,7 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
6869
$violation->getMessageTemplate(),
6970
$violation->getParameters(),
7071
$violation->getRoot(),
71-
$parameter->getProperty() ?? (
72-
str_contains($key, ':property') ? str_replace('[:property]', $violation->getPropertyPath(), $key) : $key.$violation->getPropertyPath()
73-
),
72+
$this->getProperty($parameter, $violation),
7473
$violation->getInvalidValue(),
7574
$violation->getPlural(),
7675
$violation->getCode(),
@@ -86,4 +85,24 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
8685

8786
return $this->decorated->provide($operation, $uriVariables, $context);
8887
}
88+
89+
// There's a `property` inside Parameter but it's used for hydra:search only as here we want the parameter name instead
90+
private function getProperty(Parameter $parameter, ConstraintViolationInterface $violation): string
91+
{
92+
$key = $parameter->getKey();
93+
94+
if (str_contains($key, '[:property]')) {
95+
return str_replace('[:property]', $violation->getPropertyPath(), $key);
96+
}
97+
98+
if (str_contains($key, ':property')) {
99+
return str_replace(':property', $violation->getPropertyPath(), $key);
100+
}
101+
102+
if ($p = $violation->getPropertyPath()) {
103+
return $p;
104+
}
105+
106+
return $key;
107+
}
89108
}

tests/Fixtures/TestBundle/ApiResource/WithParameter.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Symfony\Component\HttpFoundation\JsonResponse;
2828
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
2929
use Symfony\Component\Serializer\Attribute\Groups;
30+
use Symfony\Component\Validator\Constraints as Assert;
3031

3132
#[Get(
3233
uriTemplate: 'with_parameters/{id}{._format}',
@@ -63,6 +64,7 @@
6364
'array' => new QueryParameter(schema: ['minItems' => 2, 'maxItems' => 3]),
6465
'multipleOf' => new QueryParameter(schema: ['multipleOf' => 2]),
6566
'pattern' => new QueryParameter(schema: ['pattern' => '/\d/']),
67+
'int' => new QueryParameter(property: 'a', constraints: [new Assert\Type('integer')], provider: [self::class, 'toInt']),
6668
],
6769
provider: [self::class, 'collectionProvider']
6870
)]
@@ -132,4 +134,24 @@ public static function headerAndQueryProvider(Operation $operation, array $uriVa
132134

133135
return new JsonResponse($values);
134136
}
137+
138+
public static function toInt(Parameter $parameter, array $parameters = [], array $context = []): ?Operation
139+
{
140+
if (null === ($operation = $context['operation'] ?? null)) {
141+
return null;
142+
}
143+
144+
$value = $parameter->getValue();
145+
146+
if (is_numeric($value)) {
147+
$value = (int) $value;
148+
}
149+
150+
$parameters = $operation->getParameters();
151+
$parameters->add($parameter->getKey(), $parameter = $parameter->withExtraProperties(
152+
$parameter->getExtraProperties() + ['_api_values' => $value]
153+
));
154+
155+
return $operation->withParameters($parameters);
156+
}
135157
}

tests/Functional/Parameters/ValidationTest.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ final class ValidationTest extends ApiTestCase
2020
public function testWithGroupFilter(): void
2121
{
2222
$response = self::createClient()->request('GET', 'with_parameters_collection');
23-
$this->assertArraySubset(['violations' => [['propertyPath' => 'a', 'message' => 'The parameter "hydra" is required.']]], $response->toArray(false));
23+
$this->assertArraySubset(['violations' => [['message' => 'The parameter "hydra" is required.']]], $response->toArray(false));
2424
$response = self::createClient()->request('GET', 'with_parameters_collection?hydra');
2525
$this->assertResponseIsSuccessful();
2626
}
@@ -134,4 +134,12 @@ public function testValidatePropertyPlaceholder(): void
134134
],
135135
], $response->toArray(false));
136136
}
137+
138+
public function testValidateMessage(): void
139+
{
140+
$response = self::createClient()->request('GET', 'validate_parameters?int=test');
141+
$this->assertArraySubset([
142+
'detail' => 'int: This value should be of type integer.',
143+
], $response->toArray(false));
144+
}
137145
}

0 commit comments

Comments
 (0)