Skip to content

Commit 9182688

Browse files
silverbackdansoyuka
silverbackdan
and
soyuka
authored
fix(metadata): defaults should not override values (#4702)
* Fix AttributesResourceMetadataCollectionFactory::getOperationWithDefaults - do not overwrite operation properties with defaults * Test for attributes defining maximum items per page not being over-ridden * CS fix * Remove @createSchema from behat scenario, no longer needs to be run independently. * fix(metadata): defaults should not override values Co-authored-by: soyuka <[email protected]>
1 parent 733e17a commit 9182688

File tree

11 files changed

+224
-78
lines changed

11 files changed

+224
-78
lines changed

‎features/hydra/collection.feature

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,45 @@ Feature: Collections support
273273
}
274274
"""
275275

276+
@!mongodb
277+
@php8
278+
Scenario: Change the number of element by page client side with v3, attributes and PHP8. Defaults (max 40) should not override resource attribute (max 30)
279+
Given there are 80 pagination entities
280+
When I send a "GET" request to "/pagination_entities?page=2&itemsPerPage=40"
281+
Then the response status code should be 200
282+
And the response should be in JSON
283+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
284+
And the JSON should be valid according to this schema:
285+
"""
286+
{
287+
"type": "object",
288+
"properties": {
289+
"@context": {"pattern": "^/contexts/PaginationEntity"},
290+
"@id": {"pattern": "^/pagination_entities"},
291+
"@type": {"pattern": "^hydra:Collection$"},
292+
"hydra:totalItems": {"type":"number", "maximum": 80},
293+
"hydra:member": {
294+
"type": "array",
295+
"minItems": 30,
296+
"maxItems": 30
297+
},
298+
"hydra:view": {
299+
"type": "object",
300+
"properties": {
301+
"@id": {"pattern": "^/pagination_entities\\?itemsPerPage=40&page=2$"},
302+
"@type": {"pattern": "^hydra:PartialCollectionView$"},
303+
"hydra:first": {"pattern": "^/pagination_entities\\?itemsPerPage=40&page=1$"},
304+
"hydra:last": {"pattern": "^/pagination_entities\\?itemsPerPage=40&page=3$"},
305+
"hydra:previous": {"pattern": "^/pagination_entities\\?itemsPerPage=40&page=1$"},
306+
"hydra:next": {"pattern": "^/pagination_entities\\?itemsPerPage=40&page=3$"}
307+
}
308+
},
309+
"hydra:search": {}
310+
},
311+
"additionalProperties": false
312+
}
313+
"""
314+
276315
Scenario: Test presence of next
277316
When I send a "GET" request to "/dummies?page=3"
278317
Then the response status code should be 200

‎src/Metadata/GraphQl/Mutation.php

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ class Mutation extends Operation
2020
* {@inheritdoc}
2121
*/
2222
public function __construct(
23-
bool $delete = false,
23+
?bool $delete = null,
2424
?string $resolver = null,
25-
bool $collection = false,
25+
?bool $collection = null,
2626
?array $args = null,
2727
?string $shortName = null,
2828
?string $class = null,
@@ -36,34 +36,34 @@ public function __construct(
3636
?bool $paginationClientPartial = null,
3737
?bool $paginationFetchJoinCollection = null,
3838
?bool $paginationUseOutputWalkers = null,
39-
array $order = [],
39+
?array $order = null,
4040
?string $description = null,
41-
array $normalizationContext = [],
42-
array $denormalizationContext = [],
41+
?array $normalizationContext = null,
42+
?array $denormalizationContext = null,
4343
?string $security = null,
4444
?string $securityMessage = null,
4545
?string $securityPostDenormalize = null,
4646
?string $securityPostDenormalizeMessage = null,
4747
?string $securityPostValidation = null,
4848
?string $securityPostValidationMessage = null,
4949
?string $deprecationReason = null,
50-
array $filters = [],
51-
array $validationContext = [],
50+
?array $filters = null,
51+
?array $validationContext = null,
5252
$input = null,
5353
$output = null,
5454
$mercure = null,
5555
$messenger = null,
5656
?bool $elasticsearch = null,
5757
?int $urlGenerationStrategy = null,
58-
bool $read = true,
59-
bool $deserialize = true,
60-
bool $validate = true,
61-
bool $write = true,
62-
bool $serialize = true,
58+
?bool $read = null,
59+
?bool $deserialize = null,
60+
?bool $validate = null,
61+
?bool $write = null,
62+
?bool $serialize = null,
6363
?bool $fetchPartial = null,
6464
?bool $forceEager = null,
65-
int $priority = 0,
66-
string $name = '',
65+
?int $priority = null,
66+
?string $name = null,
6767
array $extraProperties = []
6868
) {
6969
parent::__construct(...\func_get_args());

‎src/Metadata/GraphQl/Operation.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class Operation
119119
public function __construct(
120120
?bool $delete = null,
121121
?string $resolver = null,
122-
bool $collection = false,
122+
?bool $collection = null,
123123
?array $args = null,
124124
?string $shortName = null,
125125
?string $class = null,
@@ -228,7 +228,7 @@ public function withResolver(?string $resolver = null): self
228228
return $self;
229229
}
230230

231-
public function isCollection(): bool
231+
public function isCollection(): ?bool
232232
{
233233
return $this->collection;
234234
}

‎src/Metadata/GraphQl/Query.php

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class Query extends Operation
2121
*/
2222
public function __construct(
2323
?string $resolver = null,
24-
bool $collection = false,
24+
?bool $collection = null,
2525
?array $args = null,
2626
?string $shortName = null,
2727
?string $class = null,
@@ -35,34 +35,34 @@ public function __construct(
3535
?bool $paginationClientPartial = null,
3636
?bool $paginationFetchJoinCollection = null,
3737
?bool $paginationUseOutputWalkers = null,
38-
array $order = [],
38+
?array $order = null,
3939
?string $description = null,
40-
array $normalizationContext = [],
41-
array $denormalizationContext = [],
40+
?array $normalizationContext = null,
41+
?array $denormalizationContext = null,
4242
?string $security = null,
4343
?string $securityMessage = null,
4444
?string $securityPostDenormalize = null,
4545
?string $securityPostDenormalizeMessage = null,
4646
?string $securityPostValidation = null,
4747
?string $securityPostValidationMessage = null,
4848
?string $deprecationReason = null,
49-
array $filters = [],
50-
array $validationContext = [],
49+
?array $filters = null,
50+
?array $validationContext = null,
5151
$input = null,
5252
$output = null,
5353
$mercure = null,
5454
$messenger = null,
5555
?bool $elasticsearch = null,
5656
?int $urlGenerationStrategy = null,
57-
bool $read = true,
58-
bool $deserialize = true,
59-
bool $validate = true,
60-
bool $write = true,
61-
bool $serialize = true,
57+
?bool $read = null,
58+
?bool $deserialize = null,
59+
?bool $validate = null,
60+
?bool $write = null,
61+
?bool $serialize = null,
6262
?bool $fetchPartial = null,
6363
?bool $forceEager = null,
64-
int $priority = 0,
65-
string $name = '',
64+
?int $priority = null,
65+
?string $name = null,
6666
array $extraProperties = []
6767
) {
6868
parent::__construct(false, ...\func_get_args());

‎src/Metadata/GraphQl/QueryCollection.php

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ final class QueryCollection extends Query
2121
*/
2222
public function __construct(
2323
?string $resolver = null,
24-
bool $collection = false,
24+
?bool $collection = null,
2525
?array $args = null,
2626
?string $shortName = null,
2727
?string $class = null,
@@ -35,34 +35,34 @@ public function __construct(
3535
?bool $paginationClientPartial = null,
3636
?bool $paginationFetchJoinCollection = null,
3737
?bool $paginationUseOutputWalkers = null,
38-
array $order = [],
38+
?array $order = null,
3939
?string $description = null,
40-
array $normalizationContext = [],
41-
array $denormalizationContext = [],
40+
?array $normalizationContext = null,
41+
?array $denormalizationContext = null,
4242
?string $security = null,
4343
?string $securityMessage = null,
4444
?string $securityPostDenormalize = null,
4545
?string $securityPostDenormalizeMessage = null,
4646
?string $securityPostValidation = null,
4747
?string $securityPostValidationMessage = null,
4848
?string $deprecationReason = null,
49-
array $filters = [],
50-
array $validationContext = [],
49+
?array $filters = null,
50+
?array $validationContext = null,
5151
$input = null,
5252
$output = null,
5353
$mercure = null,
5454
$messenger = null,
5555
?bool $elasticsearch = null,
5656
?int $urlGenerationStrategy = null,
57-
bool $read = true,
58-
bool $deserialize = true,
59-
bool $validate = true,
60-
bool $write = true,
61-
bool $serialize = true,
57+
?bool $read = null,
58+
?bool $deserialize = null,
59+
?bool $validate = null,
60+
?bool $write = null,
61+
?bool $serialize = null,
6262
?bool $fetchPartial = null,
6363
?bool $forceEager = null,
64-
int $priority = 0,
65-
string $name = '',
64+
?int $priority = null,
65+
?string $name = null,
6666
array $extraProperties = []
6767
) {
6868
parent::__construct(...\func_get_args());

‎src/Metadata/GraphQl/Subscription.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ final class Subscription extends Operation
2121
*/
2222
public function __construct(
2323
?string $resolver = null,
24-
bool $collection = false,
24+
?bool $collection = null,
2525
?array $args = null,
2626
?string $shortName = null,
2727
?string $class = null,
@@ -35,7 +35,7 @@ public function __construct(
3535
?bool $paginationClientPartial = null,
3636
?bool $paginationFetchJoinCollection = null,
3737
?bool $paginationUseOutputWalkers = null,
38-
array $order = [],
38+
?array $order = null,
3939
?string $description = null,
4040
array $normalizationContext = [],
4141
array $denormalizationContext = [],
@@ -46,23 +46,23 @@ public function __construct(
4646
?string $securityPostValidation = null,
4747
?string $securityPostValidationMessage = null,
4848
?string $deprecationReason = null,
49-
array $filters = [],
50-
array $validationContext = [],
49+
?array $filters = null,
50+
?array $validationContext = null,
5151
$input = null,
5252
$output = null,
5353
$mercure = null,
5454
$messenger = null,
5555
?bool $elasticsearch = null,
5656
?int $urlGenerationStrategy = null,
57-
bool $read = true,
58-
bool $deserialize = true,
59-
bool $validate = true,
60-
bool $write = true,
61-
bool $serialize = true,
57+
?bool $read = null,
58+
?bool $deserialize = null,
59+
?bool $validate = null,
60+
?bool $write = null,
61+
?bool $serialize = null,
6262
?bool $fetchPartial = null,
6363
?bool $forceEager = null,
64-
int $priority = 0,
65-
string $name = '',
64+
?int $priority = null,
65+
?string $name = null,
6666
array $extraProperties = []
6767
) {
6868
parent::__construct(false, ...\func_get_args());

‎src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ private function buildResourceOperations(array $attributes, string $resourceClas
147147
}
148148

149149
if (null === $graphQlOperations) {
150-
$resources[$index] = $this->addDefaultGraphQlOperations($resources[$index]);
150+
// Add default graphql operations on the first resource
151+
if (0 === $index) {
152+
$resources[$index] = $this->addDefaultGraphQlOperations($resources[$index]);
153+
}
151154
continue;
152155
}
153156

@@ -168,13 +171,7 @@ private function buildResourceOperations(array $attributes, string $resourceClas
168171
*/
169172
private function getOperationWithDefaults(ApiResource $resource, $operation): array
170173
{
171-
foreach ($this->defaults['attributes'] as $key => $value) {
172-
[$key, $value] = $this->getKeyValue($key, $value);
173-
if (method_exists($operation, 'get'.ucfirst($key)) && null !== $operation->{'get'.ucfirst($key)}()) {
174-
$operation = $operation->{'with'.ucfirst($key)}($value);
175-
}
176-
}
177-
174+
// Inherit from resource defaults
178175
foreach (get_class_methods($resource) as $methodName) {
179176
if (0 !== strpos($methodName, 'get')) {
180177
continue;
@@ -197,6 +194,9 @@ private function getOperationWithDefaults(ApiResource $resource, $operation): ar
197194
$operation = $operation->{'with'.substr($methodName, 3)}($value);
198195
}
199196

197+
// Add global defaults attributes to the operation
198+
$operation = $this->addGlobalDefaults($operation);
199+
200200
if ($operation instanceof GraphQlOperation) {
201201
if (!$operation->getName()) {
202202
throw new RuntimeException('No GraphQL operation name.');
@@ -216,6 +216,7 @@ private function getOperationWithDefaults(ApiResource $resource, $operation): ar
216216
if ($operation->getRouteName()) {
217217
$operation = $operation->withName($operation->getRouteName());
218218
}
219+
219220
// Check for name conflict
220221
if ($operation->getName()) {
221222
if (null !== $resource->getOperations() && !$resource->getOperations()->has($operation->getName())) {
@@ -232,20 +233,30 @@ private function getOperationWithDefaults(ApiResource $resource, $operation): ar
232233
];
233234
}
234235

235-
private function getResourceWithDefaults(string $resourceClass, string $shortName, ApiResource $resource): ApiResource
236+
/**
237+
* @param ApiResource|Operation|GraphQlOperation $operation
238+
*/
239+
private function addGlobalDefaults($operation)
236240
{
237-
$resource = $resource
238-
->withShortName($resource->getShortName() ?? $shortName)
239-
->withClass($resourceClass);
240-
241241
foreach ($this->defaults['attributes'] as $key => $value) {
242242
[$key, $value] = $this->getKeyValue($key, $value);
243-
if (method_exists($resource, 'get'.ucfirst($key)) && !$resource->{'get'.ucfirst($key)}()) {
244-
$resource = $resource->{'with'.ucfirst($key)}($value);
243+
$upperKey = ucfirst($key);
244+
$getter = 'get'.$upperKey;
245+
if (method_exists($operation, $getter) && null === $operation->{$getter}()) {
246+
$operation = $operation->{'with'.$upperKey}($value);
245247
}
246248
}
247249

248-
return $resource;
250+
return $operation;
251+
}
252+
253+
private function getResourceWithDefaults(string $resourceClass, string $shortName, ApiResource $resource): ApiResource
254+
{
255+
$resource = $resource
256+
->withShortName($resource->getShortName() ?? $shortName)
257+
->withClass($resourceClass);
258+
259+
return $this->addGlobalDefaults($resource);
249260
}
250261

251262
private function hasResourceAttributes(\ReflectionClass $reflectionClass): bool

‎tests/Core/Behat/DoctrineContext.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@
145145
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\NetworkPathDummy;
146146
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\NetworkPathRelationDummy;
147147
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Order;
148+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\PaginationEntity;
148149
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\PatchDummyRelation;
149150
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Payment;
150151
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Person;
@@ -278,6 +279,19 @@ public function thereAreDummyObjects(int $nb)
278279
$this->manager->flush();
279280
}
280281

282+
/**
283+
* @Given there are :nb pagination entities
284+
*/
285+
public function thereArePaginationEntities(int $nb)
286+
{
287+
for ($i = 1; $i <= $nb; ++$i) {
288+
$paginationEntity = new PaginationEntity();
289+
$this->manager->persist($paginationEntity);
290+
}
291+
292+
$this->manager->flush();
293+
}
294+
281295
/**
282296
* @Given there are :nb of these so many objects
283297
*/

0 commit comments

Comments
 (0)