Skip to content

Commit f6f9d8c

Browse files
soyukavinceAmstoutzmladencosamichielkalle
authored
Merge 3.4 (#6869)
* fix(jsonschema): hashmaps produces invalid openapi schema (#6830) * fix(jsonschema): hashmaps produces invalid openapi schema * fix --------- Co-authored-by: soyuka <[email protected]> * fix: add missing error normalizer trait and remove deprecated interface (#6853) * fix: test empty parameter against null (#6852) fixes #6844 * Fix deprecation symfony/dependency-injection * cs: run php-cs-fixer * ci: use problem error normalizer trait * 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: --------- Co-authored-by: Vincent Amstoutz <[email protected]> Co-authored-by: mladencosa <[email protected]> Co-authored-by: Michiel Kalle <[email protected]>
2 parents 4db72f5 + f87d7f2 commit f6f9d8c

File tree

17 files changed

+297
-25
lines changed

17 files changed

+297
-25
lines changed

features/filter/filter_validation.feature

+4-4
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/Doctrine/Odm/Extension/ParameterExtension.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function __construct(private readonly ContainerInterface $filterLocator)
3636
private function applyFilter(Builder $aggregationBuilder, ?string $resourceClass = null, ?Operation $operation = null, array &$context = []): void
3737
{
3838
foreach ($operation->getParameters() ?? [] as $parameter) {
39-
if (!($v = $parameter->getValue()) || $v instanceof ParameterNotFound) {
39+
if (null === ($v = $parameter->getValue()) || $v instanceof ParameterNotFound) {
4040
continue;
4141
}
4242

src/Doctrine/Orm/Extension/ParameterExtension.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function __construct(private readonly ContainerInterface $filterLocator)
4141
private function applyFilter(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, ?Operation $operation = null, array $context = []): void
4242
{
4343
foreach ($operation?->getParameters() ?? [] as $parameter) {
44-
if (!($v = $parameter->getValue()) || $v instanceof ParameterNotFound) {
44+
if (null === ($v = $parameter->getValue()) || $v instanceof ParameterNotFound) {
4545
continue;
4646
}
4747

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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\JsonApi\Serializer;
15+
16+
use ApiPlatform\Exception\ErrorCodeSerializableInterface;
17+
use Symfony\Component\ErrorHandler\Exception\FlattenException;
18+
use Symfony\Component\HttpFoundation\Response;
19+
20+
/**
21+
* @deprecated
22+
*/
23+
trait ErrorNormalizerTrait
24+
{
25+
private function getErrorMessage($object, array $context, bool $debug = false): string
26+
{
27+
$message = $object->getMessage();
28+
29+
if ($debug) {
30+
return $message;
31+
}
32+
33+
if ($object instanceof FlattenException) {
34+
$statusCode = $context['statusCode'] ?? $object->getStatusCode();
35+
if ($statusCode >= 500 && $statusCode < 600) {
36+
$message = Response::$statusTexts[$statusCode] ?? Response::$statusTexts[Response::HTTP_INTERNAL_SERVER_ERROR];
37+
}
38+
}
39+
40+
return $message;
41+
}
42+
43+
private function getErrorCode(object $object): ?string
44+
{
45+
if ($object instanceof FlattenException) {
46+
$exceptionClass = $object->getClass();
47+
} else {
48+
$exceptionClass = $object::class;
49+
}
50+
51+
if (is_a($exceptionClass, ErrorCodeSerializableInterface::class, true)) {
52+
return $exceptionClass::getErrorCode();
53+
}
54+
55+
return null;
56+
}
57+
}

src/JsonSchema/Metadata/Property/Factory/SchemaPropertyMetadataFactory.php

+9-5
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ final class SchemaPropertyMetadataFactory implements PropertyMetadataFactoryInte
3434

3535
public const JSON_SCHEMA_USER_DEFINED = 'user_defined_schema';
3636

37-
public function __construct(ResourceClassResolverInterface $resourceClassResolver, private readonly ?PropertyMetadataFactoryInterface $decorated = null)
38-
{
37+
public function __construct(
38+
ResourceClassResolverInterface $resourceClassResolver,
39+
private readonly ?PropertyMetadataFactoryInterface $decorated = null,
40+
) {
3941
$this->resourceClassResolver = $resourceClassResolver;
4042
}
4143

@@ -198,6 +200,8 @@ private function typeToArray(Type $type, ?bool $readableLink = null): array
198200
* Gets the JSON Schema document which specifies the data type corresponding to the given PHP class, and recursively adds needed new schema to the current schema if provided.
199201
*
200202
* Note: if the class is not part of exceptions listed above, any class is considered as a resource.
203+
*
204+
* @throws PropertyNotFoundException
201205
*/
202206
private function getClassType(?string $className, bool $nullable, ?bool $readableLink): array
203207
{
@@ -240,7 +244,8 @@ private function getClassType(?string $className, bool $nullable, ?bool $readabl
240244
];
241245
}
242246

243-
if (!$this->isResourceClass($className) && is_a($className, \BackedEnum::class, true)) {
247+
$isResourceClass = $this->isResourceClass($className);
248+
if (!$isResourceClass && is_a($className, \BackedEnum::class, true)) {
244249
$enumCases = array_map(static fn (\BackedEnum $enum): string|int => $enum->value, $className::cases());
245250

246251
$type = \is_string($enumCases[0] ?? '') ? 'string' : 'integer';
@@ -255,15 +260,14 @@ private function getClassType(?string $className, bool $nullable, ?bool $readabl
255260
];
256261
}
257262

258-
if (true !== $readableLink && $this->isResourceClass($className)) {
263+
if (true !== $readableLink && $isResourceClass) {
259264
return [
260265
'type' => 'string',
261266
'format' => 'iri-reference',
262267
'example' => 'https://example.com/',
263268
];
264269
}
265270

266-
// TODO: add propertyNameCollectionFactory and recurse to find the underlying schema? Right now SchemaFactory does the job so we don't compute anything here.
267271
return ['type' => Schema::UNKNOWN_TYPE];
268272
}
269273

src/JsonSchema/SchemaFactory.php

+5-3
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ private function buildPropertySchema(Schema $schema, string $definitionName, str
179179
$propertySchemaType = $propertySchema['type'] ?? false;
180180

181181
$isUnknown = Schema::UNKNOWN_TYPE === $propertySchemaType
182-
|| ('array' === $propertySchemaType && Schema::UNKNOWN_TYPE === ($propertySchema['items']['type'] ?? null));
182+
|| ('array' === $propertySchemaType && Schema::UNKNOWN_TYPE === ($propertySchema['items']['type'] ?? null))
183+
|| ('object' === $propertySchemaType && Schema::UNKNOWN_TYPE === ($propertySchema['additionalProperties']['type'] ?? null));
183184

184185
if (
185186
!$isUnknown && (
@@ -231,8 +232,9 @@ private function buildPropertySchema(Schema $schema, string $definitionName, str
231232
}
232233

233234
if ($isCollection) {
234-
$propertySchema['items']['$ref'] = $subSchema['$ref'];
235-
unset($propertySchema['items']['type']);
235+
$key = ($propertySchema['type'] ?? null) === 'object' ? 'additionalProperties' : 'items';
236+
$propertySchema[$key]['$ref'] = $subSchema['$ref'];
237+
unset($propertySchema[$key]['type']);
236238
break;
237239
}
238240

src/Metadata/Parameter.php

+2-2
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

+3-3
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

+28
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

+28
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/Bundle/Resources/config/elasticsearch.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
<argument type="service" id="api_platform.elasticsearch.client" />
8787
<argument type="service" id="serializer" />
8888
<argument type="service" id="api_platform.pagination" />
89-
<argument type="tagged" tag="api_platform.elasticsearch.request_body_search_extension.collection" />
89+
<argument type="tagged_iterator" tag="api_platform.elasticsearch.request_body_search_extension.collection" />
9090
<argument type="service" id="api_platform.inflector" on-invalid="null" />
9191

9292
<tag name="api_platform.state_provider" priority="-100" key="ApiPlatform\Elasticsearch\State\CollectionProvider"/>

src/Symfony/Validator/State/ParameterValidatorProvider.php

+23-4
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
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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\Tests\Fixtures\TestBundle\ApiResource\Issue6800;
15+
16+
class Foo
17+
{
18+
public string $bar;
19+
public int $baz;
20+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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\Tests\Fixtures\TestBundle\ApiResource\Issue6800;
15+
16+
use ApiPlatform\Metadata\Get;
17+
18+
#[Get]
19+
class TestApiDocHashmapArrayObjectIssue
20+
{
21+
/** @var array<Foo> */
22+
public array $foos;
23+
24+
/** @var Foo[] */
25+
public array $fooOtherSyntax;
26+
27+
/** @var array<string, Foo> */
28+
public array $fooHashmaps;
29+
}

0 commit comments

Comments
 (0)