Skip to content

Commit 682b437

Browse files
author
abluchet
committed
Fix collection subresource after toOne relation
1 parent 7b9c5de commit 682b437

File tree

12 files changed

+144
-70
lines changed

12 files changed

+144
-70
lines changed

‎features/bootstrap/FeatureContext.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,9 +609,11 @@ public function thereIsAnAnswerToTheQuestion(string $a, string $q)
609609
$question = new Question();
610610
$question->setContent($q);
611611
$question->setAnswer($answer);
612+
$answer->addRelatedQuestion($question);
612613

613614
$this->manager->persist($answer);
614615
$this->manager->persist($question);
616+
615617
$this->manager->flush();
616618
}
617619

‎features/main/subresource.feature

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,39 @@ Feature: Subresource support
1212
And the JSON should be equal to:
1313
"""
1414
{
15-
"@context": "\/contexts\/Answer",
16-
"@id": "\/answers\/1",
17-
"@type": "Answer",
18-
"id": 1,
19-
"content": "42",
20-
"question": "\/questions\/1"
15+
"@context": "/contexts/Answer",
16+
"@id": "/answers/1",
17+
"@type": "Answer",
18+
"id": 1,
19+
"content": "42",
20+
"question": "/questions/1",
21+
"relatedQuestions": [
22+
"/questions/1"
23+
]
24+
}
25+
"""
26+
27+
Scenario: Get subresource one to one relation
28+
When I send a "GET" request to "/questions/1/answer/related_questions"
29+
And print last JSON response
30+
And the response status code should be 200
31+
And the response should be in JSON
32+
And the JSON should be equal to:
33+
"""
34+
{
35+
"@context": "/contexts/Question",
36+
"@id": "/questions/1/answer/related_questions",
37+
"@type": "hydra:Collection",
38+
"hydra:member": [
39+
{
40+
"@id": "/questions/1",
41+
"@type": "Question",
42+
"content": "What's the answer to the Ultimate Question of Life, the Universe and Everything?",
43+
"id": 1,
44+
"answer": "/answers/1"
45+
}
46+
],
47+
"hydra:totalItems": 1
2148
}
2249
"""
2350

‎src/Bridge/Doctrine/Orm/SubresourceDataProvider.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public function getSubresource(string $resourceClass, array $identifiers, array
105105
$qb = $manager->createQueryBuilder();
106106
$alias = $queryNameGenerator->generateJoinAlias($identifier);
107107
$relationType = $classMetadata->getAssociationMapping($previousAssociationProperty)['type'];
108-
$normalizedIdentifiers = $this->normalizeIdentifiers($identifiers[$identifier], $manager, $identifierResourceClass);
108+
$normalizedIdentifiers = isset($identifiers[$identifier]) ? $this->normalizeIdentifiers($identifiers[$identifier], $manager, $identifierResourceClass) : [];
109109

110110
switch ($relationType) {
111111
//MANY_TO_MANY relations need an explicit join so that the identifier part can be retrieved
@@ -169,6 +169,11 @@ public function getSubresource(string $resourceClass, array $identifiers, array
169169

170170
if (true === $context['collection']) {
171171
foreach ($this->collectionExtensions as $extension) {
172+
// We don't need this anymore because we already made sub queries to ensure correct results
173+
if ($extension instanceof FilterEagerLoadingExtension) {
174+
continue;
175+
}
176+
172177
$extension->applyToCollection($queryBuilder, $queryNameGenerator, $resourceClass, $operationName);
173178

174179
if ($extension instanceof QueryResultCollectionExtensionInterface && $extension->supportsResult($resourceClass, $operationName)) {

‎src/Bridge/Symfony/Bundle/Action/SwaggerUiAction.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ private function getContext(Request $request, Documentation $documentation): arr
117117
$swaggerData['operationId'] = sprintf('%s%sCollection', $collectionOperationName, $swaggerData['shortName']);
118118
} elseif (null !== $itemOperationName = $request->attributes->get('_api_item_operation_name')) {
119119
$swaggerData['operationId'] = sprintf('%s%sItem', $itemOperationName, $swaggerData['shortName']);
120+
} elseif (null !== $subresourceOperationContext = $request->attributes->get('_api_subresource_context')) {
121+
$swaggerData['operationId'] = $subresourceOperationContext['operationId'];
120122
}
121123

122124
list($swaggerData['path'], $swaggerData['method']) = $this->getPathAndMethod($swaggerData);

‎src/Bridge/Symfony/Routing/ApiLoader.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public function load($data, $type = null): RouteCollection
101101
continue;
102102
}
103103

104-
foreach ($this->subresourceOperationFactory->create($resourceClass) as $operation) {
104+
foreach ($this->subresourceOperationFactory->create($resourceClass) as $operationId => $operation) {
105105
$routeCollection->add($operation['route_name'], new Route(
106106
$operation['path'],
107107
[
@@ -113,6 +113,8 @@ public function load($data, $type = null): RouteCollection
113113
'property' => $operation['property'],
114114
'identifiers' => $operation['identifiers'],
115115
'collection' => $operation['collection'],
116+
'operationId' => $operationId,
117+
116118
],
117119
],
118120
[],

‎src/EventListener/ReadListener.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,10 @@ private function getSubresourceData(Request $request, array $attributes)
133133
}
134134

135135
$identifiers = [];
136-
foreach ($attributes['subresource_context']['identifiers'] as $key => list($id)) {
137-
$identifiers[$id] = $request->attributes->get($id);
136+
foreach ($attributes['subresource_context']['identifiers'] as $key => list($id, $class, $hasIdentifier)) {
137+
if (true === $hasIdentifier) {
138+
$identifiers[$id] = $request->attributes->get($id);
139+
}
138140
}
139141

140142
$data = $this->subresourceDataProvider->getSubresource($attributes['resource_class'], $identifiers, $attributes['subresource_context'], $attributes['subresource_operation_name']);

‎src/Operation/Factory/SubresourceOperationFactory.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private function computeSubresourceOperations(string $resourceClass, array &$tre
104104
if (null === $parentOperation) {
105105
$rootResourceMetadata = $this->resourceMetadataFactory->create($rootResourceClass);
106106
$rootShortname = $rootResourceMetadata->getShortName();
107-
$operation['identifiers'] = [['id', $rootResourceClass]];
107+
$operation['identifiers'] = [['id', $rootResourceClass, true]];
108108
$operation['route_name'] = sprintf(
109109
'%s%s_%s_%s%s',
110110
RouteNameGenerator::ROUTE_NAME_PREFIX,
@@ -125,17 +125,21 @@ private function computeSubresourceOperations(string $resourceClass, array &$tre
125125
} else {
126126
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
127127
$operation['identifiers'] = $parentOperation['identifiers'];
128-
$operation['identifiers'][] = [$parentOperation['property'], $resourceClass];
128+
$operation['identifiers'][] = [$parentOperation['property'], $resourceClass, $parentOperation['collection']];
129129
$operation['route_name'] = str_replace('get'.self::SUBRESOURCE_SUFFIX, RouteNameGenerator::inflector($property, $operation['collection']).'_get'.self::SUBRESOURCE_SUFFIX, $parentOperation['route_name']);
130130
$operation['shortNames'][] = $resourceMetadata->getShortName();
131131

132132
$operation['path'] = str_replace(self::FORMAT_SUFFIX, '', $parentOperation['path']);
133-
list($key) = end($operation['identifiers']);
134-
$operation['path'] .= sprintf('/{%s}/%s%s', $key, $this->pathSegmentNameGenerator->getSegmentName($property, $operation['collection']), self::FORMAT_SUFFIX);
133+
if ($parentOperation['collection']) {
134+
list($key) = end($operation['identifiers']);
135+
$operation['path'] .= sprintf('/{%s}', $key);
136+
}
137+
$operation['path'] .= sprintf('/%s%s', $this->pathSegmentNameGenerator->getSegmentName($property, $operation['collection']), self::FORMAT_SUFFIX);
135138
}
136139

137140
$tree[$visiting] = $operation;
138141
$this->computeSubresourceOperations($subresourceClass, $tree, $rootResourceClass, $operation);
139142
}
140143
}
141144
}
145+

‎src/Swagger/Serializer/DocumentationNormalizer.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,10 @@ public function normalize($object, $format = null, array $context = [])
134134
$pathOperation['parameters'] = $parameters;
135135
}
136136

137-
foreach ($subresourceOperation['identifiers'] as $identifier) {
138-
$pathOperation['parameters'][] = ['name' => $identifier[0], 'in' => 'path', 'required' => true, 'type' => 'string'];
137+
foreach ($subresourceOperation['identifiers'] as list($identifier, $class, $hasIdentifier)) {
138+
if (true === $hasIdentifier) {
139+
$pathOperation['parameters'][] = ['name' => $identifier, 'in' => 'path', 'required' => true, 'type' => 'string'];
140+
}
139141
}
140142

141143
$paths[str_replace(SubresourceOperationFactory::FORMAT_SUFFIX, '', $subresourceOperation['path'])] = new \ArrayObject(['get' => $pathOperation]);

‎tests/Bridge/Symfony/Routing/ApiLoaderTest.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public function testApiLoader()
9191
);
9292

9393
$this->assertEquals(
94-
$this->getSubresourceRoute('/dummies/{id}/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_dummies_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', DummyEntity::class]], 'collection' => true]),
94+
$this->getSubresourceRoute('/dummies/{id}/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_dummies_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', DummyEntity::class, true]], 'collection' => true, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\DummyEntity-subresource-ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity']),
9595
$routeCollection->get('api_dummies_subresources_get_subresource')
9696
);
9797
}
@@ -160,32 +160,32 @@ public function testRecursiveSubresource()
160160
$routeCollection = $this->getApiLoaderWithResourceMetadata($resourceMetadata, true)->load(null);
161161

162162
$this->assertEquals(
163-
$this->getSubresourceRoute('/dummies/{id}/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_dummies_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', DummyEntity::class]], 'collection' => true]),
163+
$this->getSubresourceRoute('/dummies/{id}/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_dummies_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', DummyEntity::class, true]], 'collection' => true, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\DummyEntity-subresource-ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity']),
164164
$routeCollection->get('api_dummies_subresources_get_subresource')
165165
);
166166

167167
$this->assertEquals(
168-
$this->getSubresourceRoute('/related_dummies/{id}/recursivesubresource/{recursivesubresource}/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_related_dummies_recursivesubresource_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', RelatedDummyEntity::class], ['recursivesubresource', DummyEntity::class]], 'collection' => true]),
168+
$this->getSubresourceRoute('/related_dummies/{id}/recursivesubresource/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_related_dummies_recursivesubresource_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', RelatedDummyEntity::class, true], ['recursivesubresource', DummyEntity::class, false]], 'collection' => true, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity-recursivesubresource-ApiPlatform\Core\Tests\Fixtures\DummyEntity-subresource-ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity']),
169169
$routeCollection->get('api_related_dummies_recursivesubresource_subresources_get_subresource')
170170
);
171171

172172
$this->assertEquals(
173-
$this->getSubresourceRoute('/related_dummies/{id}/recursivesubresource.{_format}', 'api_platform.action.get_subresource', DummyEntity::class, 'api_related_dummies_recursivesubresource_get_subresource', ['property' => 'recursivesubresource', 'identifiers' => [['id', RelatedDummyEntity::class]], 'collection' => false]),
173+
$this->getSubresourceRoute('/related_dummies/{id}/recursivesubresource.{_format}', 'api_platform.action.get_subresource', DummyEntity::class, 'api_related_dummies_recursivesubresource_get_subresource', ['property' => 'recursivesubresource', 'identifiers' => [['id', RelatedDummyEntity::class, true]], 'collection' => false, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity-recursivesubresource-ApiPlatform\Core\Tests\Fixtures\DummyEntity']),
174174
$routeCollection->get('api_related_dummies_recursivesubresource_get_subresource')
175175
);
176176

177177
$this->assertEquals(
178-
$this->getSubresourceRoute('/dummies/{id}/subresources/{subresource}/recursivesubresource.{_format}', 'api_platform.action.get_subresource', DummyEntity::class, 'api_dummies_subresources_recursivesubresource_get_subresource', ['property' => 'recursivesubresource', 'identifiers' => [['id', DummyEntity::class], ['subresource', RelatedDummyEntity::class]], 'collection' => false]),
178+
$this->getSubresourceRoute('/dummies/{id}/subresources/{subresource}/recursivesubresource.{_format}', 'api_platform.action.get_subresource', DummyEntity::class, 'api_dummies_subresources_recursivesubresource_get_subresource', ['property' => 'recursivesubresource', 'identifiers' => [['id', DummyEntity::class, true], ['subresource', RelatedDummyEntity::class, true]], 'collection' => false, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\DummyEntity-subresource-ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity-recursivesubresource-ApiPlatform\Core\Tests\Fixtures\DummyEntity']),
179179
$routeCollection->get('api_dummies_subresources_recursivesubresource_get_subresource')
180180
);
181181

182182
$this->assertEquals(
183-
$this->getSubresourceRoute('/related_dummies/{id}/secondrecursivesubresource/{secondrecursivesubresource}/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_related_dummies_secondrecursivesubresource_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', RelatedDummyEntity::class], ['secondrecursivesubresource', DummyEntity::class]], 'collection' => true]),
183+
$this->getSubresourceRoute('/related_dummies/{id}/secondrecursivesubresource/subresources.{_format}', 'api_platform.action.get_subresource', RelatedDummyEntity::class, 'api_related_dummies_secondrecursivesubresource_subresources_get_subresource', ['property' => 'subresource', 'identifiers' => [['id', RelatedDummyEntity::class, true], ['secondrecursivesubresource', DummyEntity::class, false]], 'collection' => true, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity-secondrecursivesubresource-ApiPlatform\Core\Tests\Fixtures\DummyEntity-subresource-ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity']),
184184
$routeCollection->get('api_related_dummies_secondrecursivesubresource_subresources_get_subresource')
185185
);
186186

187187
$this->assertEquals(
188-
$this->getSubresourceRoute('/related_dummies/{id}/secondrecursivesubresource.{_format}', 'api_platform.action.get_subresource', DummyEntity::class, 'api_related_dummies_secondrecursivesubresource_get_subresource', ['property' => 'secondrecursivesubresource', 'identifiers' => [['id', RelatedDummyEntity::class]], 'collection' => false]),
188+
$this->getSubresourceRoute('/related_dummies/{id}/secondrecursivesubresource.{_format}', 'api_platform.action.get_subresource', DummyEntity::class, 'api_related_dummies_secondrecursivesubresource_get_subresource', ['property' => 'secondrecursivesubresource', 'identifiers' => [['id', RelatedDummyEntity::class, true]], 'collection' => false, 'operationId' => 'ApiPlatform\Core\Tests\Fixtures\RelatedDummyEntity-secondrecursivesubresource-ApiPlatform\Core\Tests\Fixtures\DummyEntity']),
189189
$routeCollection->get('api_related_dummies_secondrecursivesubresource_get_subresource')
190190
);
191191
}

‎tests/EventListener/ReadListenerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ public function testRetrieveSubresource()
145145

146146
$data = [new \stdClass()];
147147
$subresourceDataProvider = $this->prophesize(SubresourceDataProviderInterface::class);
148-
$subresourceDataProvider->getSubresource('Foo', ['id' => 1], ['identifiers' => [['id', 'Bar']], 'property' => 'bar'], 'get')->willReturn($data)->shouldBeCalled();
148+
$subresourceDataProvider->getSubresource('Foo', ['id' => 1], ['identifiers' => [['id', 'Bar', true]], 'property' => 'bar'], 'get')->willReturn($data)->shouldBeCalled();
149149

150-
$request = new Request([], [], ['id' => 1, '_api_resource_class' => 'Foo', '_api_subresource_operation_name' => 'get', '_api_format' => 'json', '_api_mime_type' => 'application/json', '_api_subresource_context' => ['identifiers' => [['id', 'Bar']], 'property' => 'bar']]);
150+
$request = new Request([], [], ['id' => 1, '_api_resource_class' => 'Foo', '_api_subresource_operation_name' => 'get', '_api_format' => 'json', '_api_mime_type' => 'application/json', '_api_subresource_context' => ['identifiers' => [['id', 'Bar', true]], 'property' => 'bar']]);
151151
$request->setMethod(Request::METHOD_GET);
152152

153153
$event = $this->prophesize(GetResponseEvent::class);

‎tests/Fixtures/TestBundle/Entity/Answer.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity;
1515

1616
use ApiPlatform\Core\Annotation\ApiResource;
17+
use ApiPlatform\Core\Annotation\ApiSubresource;
1718
use Doctrine\ORM\Mapping as ORM;
19+
use Doctrine\Common\Collections\ArrayCollection;
1820
use Symfony\Component\Serializer\Annotation as Serializer;
1921

2022
/**
@@ -47,6 +49,18 @@ class Answer
4749
*/
4850
private $question;
4951

52+
/**
53+
* @ORM\OneToMany(targetEntity="Question", mappedBy="answer")
54+
* @Serializer\Groups({"foobar"})
55+
* @ApiSubresource
56+
*/
57+
private $relatedQuestions;
58+
59+
public function __construct()
60+
{
61+
$this->relatedQuestions = new ArrayCollection();
62+
}
63+
5064
/**
5165
* Get id.
5266
*
@@ -104,4 +118,18 @@ public function getQuestion()
104118
{
105119
return $this->question;
106120
}
121+
122+
/**
123+
* Get related question.
124+
*
125+
* @return ArrayCollection
126+
*/
127+
public function getRelatedQuestions()
128+
{
129+
return $this->relatedQuestions;
130+
}
131+
132+
public function addRelatedQuestion(Question $question) {
133+
$this->relatedQuestions->add($question);
134+
}
107135
}

0 commit comments

Comments
 (0)