Skip to content

Commit 59afc11

Browse files
author
abluchet
committed
Fix route name resolving with subresources
Also resolves conflicts with existing subresources on a property having the same name
1 parent da77ac0 commit 59afc11

File tree

14 files changed

+438
-20
lines changed

14 files changed

+438
-20
lines changed

‎features/bootstrap/FeatureContext.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
1717
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation;
1818
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Container;
1919
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
20+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyAggregateOffer;
2021
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar;
2122
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCarColor;
2223
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyFriend;
24+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyOffer;
25+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyProduct;
2326
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy;
2427
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Node;
2528
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Question;
@@ -474,4 +477,23 @@ public function thePasswordForUserShouldBeHashed($password, $user)
474477
throw new \Exception('User password mismatch');
475478
}
476479
}
480+
481+
/**
482+
* @Given I have a product with offers
483+
*/
484+
public function createProductWithOffers()
485+
{
486+
$offer = new DummyOffer();
487+
$offer->setValue(2);
488+
$aggregate = new DummyAggregateOffer();
489+
$aggregate->setValue(1);
490+
$aggregate->addOffer($offer);
491+
492+
$product = new DummyProduct();
493+
$product->setName('Dummy product');
494+
$product->addOffer($aggregate);
495+
496+
$this->manager->persist($product);
497+
$this->manager->flush();
498+
}
477499
}

‎features/main/subresource.feature

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,6 @@ Feature: Subresource support
204204
}
205205
"""
206206

207-
@dropSchema
208207
Scenario: Get the embedded relation collection
209208
When I send a "GET" request to "/dummies/1/related_dummies/1/third_level"
210209
And the response status code should be 200
@@ -222,3 +221,50 @@ Feature: Subresource support
222221
}
223222
"""
224223

224+
Scenario: Get offers subresource from aggregate offers subresource
225+
Given I have a product with offers
226+
When I send a "GET" request to "/dummy_products/1/offers/1/offers"
227+
And the response status code should be 200
228+
And the response should be in JSON
229+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
230+
And the JSON should be equal to:
231+
"""
232+
{
233+
"@context": "/contexts/DummyOffer",
234+
"@id": "/dummy_products/1/offers/1/offers",
235+
"@type": "hydra:Collection",
236+
"hydra:member": [
237+
{
238+
"@id": "/dummy_offers/1",
239+
"@type": "DummyOffer",
240+
"id": 1,
241+
"value": 2
242+
}
243+
],
244+
"hydra:totalItems": 1
245+
}
246+
"""
247+
248+
@dropSchema
249+
Scenario: Get offers subresource from aggregate offers subresource
250+
When I send a "GET" request to "/dummy_aggregate_offers/1/offers"
251+
And the response status code should be 200
252+
And the response should be in JSON
253+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
254+
And the JSON should be equal to:
255+
"""
256+
{
257+
"@context": "/contexts/DummyOffer",
258+
"@id": "/dummy_aggregate_offers/1/offers",
259+
"@type": "hydra:Collection",
260+
"hydra:member": [
261+
{
262+
"@id": "/dummy_offers/1",
263+
"@type": "DummyOffer",
264+
"id": 1,
265+
"value": 2
266+
}
267+
],
268+
"hydra:totalItems": 1
269+
}
270+
"""

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ private function computeSubresourceOperations(RouteCollection $routeCollection,
173173
'_controller' => self::DEFAULT_ACTION_PATTERN.'get_subresource',
174174
'_format' => null,
175175
'_api_resource_class' => $subresource,
176-
'_api_subresource_operation_name' => 'get_subresource_'.$operation['property'],
176+
'_api_subresource_operation_name' => $operation['route_name'],
177177
'_api_subresource_context' => [
178178
'property' => $operation['property'],
179179
'identifiers' => $operation['identifiers'],

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function __construct(CacheItemPoolInterface $cacheItemPool, RouteNameReso
3737
/**
3838
* {@inheritdoc}
3939
*/
40-
public function getRouteName(string $resourceClass, $operationType): string
40+
public function getRouteName(string $resourceClass, $operationType /**, array $context = []**/): string
4141
{
4242
$cacheKey = self::CACHE_KEY_PREFIX.md5(serialize([$resourceClass, $operationType]));
4343

@@ -51,7 +51,7 @@ public function getRouteName(string $resourceClass, $operationType): string
5151
// do nothing
5252
}
5353

54-
$routeName = $this->decorated->getRouteName($resourceClass, $operationType);
54+
$routeName = $this->decorated->getRouteName($resourceClass, $operationType, @func_get_arg(2));
5555

5656
if (!isset($cacheItem)) {
5757
return $routeName;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,10 @@ public function getItemIriFromResourceClass(string $resourceClass, array $identi
125125
/**
126126
* {@inheritdoc}
127127
*/
128-
public function getSubresourceIriFromResourceClass(string $resourceClass, array $identifiers, int $referenceType = UrlGeneratorInterface::ABS_PATH): string
128+
public function getSubresourceIriFromResourceClass(string $resourceClass, array $context, int $referenceType = UrlGeneratorInterface::ABS_PATH): string
129129
{
130130
try {
131-
return $this->router->generate($this->routeNameResolver->getRouteName($resourceClass, OperationType::SUBRESOURCE), $identifiers, $referenceType);
131+
return $this->router->generate($this->routeNameResolver->getRouteName($resourceClass, OperationType::SUBRESOURCE, $context), $context['subresource_identifiers'], $referenceType);
132132
} catch (RoutingExceptionInterface $e) {
133133
throw new InvalidArgumentException(sprintf('Unable to generate an IRI for "%s".', $resourceClass), $e->getCode(), $e);
134134
}

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Bridge\Symfony\Routing;
1515

16+
use ApiPlatform\Core\Api\OperationType;
1617
use ApiPlatform\Core\Api\OperationTypeDeprecationHelper;
1718
use ApiPlatform\Core\Exception\InvalidArgumentException;
1819
use Symfony\Component\Routing\RouterInterface;
@@ -34,8 +35,14 @@ public function __construct(RouterInterface $router)
3435
/**
3536
* {@inheritdoc}
3637
*/
37-
public function getRouteName(string $resourceClass, $operationType): string
38+
public function getRouteName(string $resourceClass, $operationType /**, array $context = [] **/): string
3839
{
40+
$context = @func_get_arg(2);
41+
42+
if (!$context) {
43+
$context = [];
44+
}
45+
3946
$operationType = OperationTypeDeprecationHelper::getOperationType($operationType);
4047

4148
foreach ($this->router->getRouteCollection()->all() as $routeName => $route) {
@@ -44,10 +51,30 @@ public function getRouteName(string $resourceClass, $operationType): string
4451
$methods = $route->getMethods();
4552

4653
if ($resourceClass === $currentResourceClass && null !== $operation && (empty($methods) || in_array('GET', $methods, true))) {
54+
if ($operationType === OperationType::SUBRESOURCE && false === $this->isSameSubresource($context, $route->getDefault('_api_subresource_context'))) {
55+
continue;
56+
}
57+
4758
return $routeName;
4859
}
4960
}
5061

5162
throw new InvalidArgumentException(sprintf('No %s route associated with the type "%s".', $operationType, $resourceClass));
5263
}
64+
65+
private function isSameSubresource(array $context, array $currentContext): bool
66+
{
67+
$subresources = array_keys($context['subresource_resources']);
68+
$currentSubresources = [];
69+
70+
foreach ($currentContext['identifiers'] as $identiferContext) {
71+
$currentSubresources[] = $identiferContext[1];
72+
}
73+
74+
if ($currentSubresources === $subresources) {
75+
return true;
76+
}
77+
78+
return false;
79+
}
5380
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,5 @@ interface RouteNameResolverInterface
3232
*
3333
* @return string
3434
*/
35-
public function getRouteName(string $resourceClass, $operationType): string;
35+
public function getRouteName(string $resourceClass, $operationType /**, array $context = [] **/): string;
3636
}

‎src/Hydra/Serializer/CollectionNormalizer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public function normalize($object, $format = null, array $context = [])
7676
$context = $this->initContext($resourceClass, $context);
7777

7878
if (isset($context['operation_type']) && $context['operation_type'] === OperationType::SUBRESOURCE) {
79-
$data['@id'] = $this->iriConverter->getSubresourceIriFromResourceClass($resourceClass, $context['subresource_identifiers']);
79+
$data['@id'] = $this->iriConverter->getSubresourceIriFromResourceClass($resourceClass, $context);
8080
} else {
8181
$data['@id'] = $this->iriConverter->getIriFromResourceClass($resourceClass);
8282
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function testGetRouteNameForItemRouteWithNoMatchingRoute()
5050
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldNotBeCalled();
5151

5252
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
53-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false)
53+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false, false)
5454
->willThrow(new InvalidArgumentException('No item route associated with the type "AppBundle\Entity\User".'))
5555
->shouldBeCalled();
5656

@@ -69,7 +69,7 @@ public function testGetRouteNameForItemRouteOnCacheMiss()
6969
$cacheItemPoolProphecy->save($cacheItemProphecy)->willReturn(true)->shouldBeCalled();
7070

7171
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
72-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false)->willReturn('some_item_route')->shouldBeCalled();
72+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false, false)->willReturn('some_item_route')->shouldBeCalled();
7373

7474
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
7575
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', false);
@@ -110,7 +110,7 @@ public function testGetRouteNameForCollectionRouteWithNoMatchingRoute()
110110
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldNotBeCalled();
111111

112112
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
113-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true)
113+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true, false)
114114
->willThrow(new InvalidArgumentException('No collection route associated with the type "AppBundle\Entity\User".'))
115115
->shouldBeCalled();
116116

@@ -129,7 +129,7 @@ public function testGetRouteNameForCollectionRouteOnCacheMiss()
129129
$cacheItemPoolProphecy->save($cacheItemProphecy)->willReturn(true)->shouldBeCalled();
130130

131131
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
132-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true)->willReturn('some_collection_route')->shouldBeCalled();
132+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true, false)->willReturn('some_collection_route')->shouldBeCalled();
133133

134134
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
135135
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', true);

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2222
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
2323
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
24+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
25+
use Prophecy\Argument;
2426
use Symfony\Component\Routing\Exception\RouteNotFoundException;
2527
use Symfony\Component\Routing\RouterInterface;
2628

@@ -206,7 +208,7 @@ public function testGetSubresourceIriFromResourceClass()
206208
$itemDataProviderProphecy = $this->prophesize(ItemDataProviderInterface::class);
207209

208210
$routeNameResolverProphecy = $this->prophesize(RouteNameResolverInterface::class);
209-
$routeNameResolverProphecy->getRouteName(Dummy::class, OperationType::SUBRESOURCE)->willReturn('api_dummies_related_dummies_get_subresource');
211+
$routeNameResolverProphecy->getRouteName(Dummy::class, OperationType::SUBRESOURCE, Argument::type('array'))->willReturn('api_dummies_related_dummies_get_subresource');
210212

211213
$routerProphecy = $this->prophesize(RouterInterface::class);
212214
$routerProphecy->generate('api_dummies_related_dummies_get_subresource', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->willReturn('/dummies/1/related_dummies');
@@ -219,7 +221,7 @@ public function testGetSubresourceIriFromResourceClass()
219221
$routerProphecy->reveal()
220222
);
221223

222-
$this->assertEquals($converter->getSubresourceIriFromResourceClass(Dummy::class, ['id' => 1]), '/dummies/1/related_dummies');
224+
$this->assertEquals($converter->getSubresourceIriFromResourceClass(Dummy::class, ['subresource_identifiers' => ['id' => 1], 'subresource_resources' => [RelatedDummy::class => 1]]), '/dummies/1/related_dummies');
223225
}
224226

225227
/**
@@ -235,7 +237,7 @@ public function testNotAbleToGenerateGetSubresourceIriFromResourceClass()
235237
$itemDataProviderProphecy = $this->prophesize(ItemDataProviderInterface::class);
236238

237239
$routeNameResolverProphecy = $this->prophesize(RouteNameResolverInterface::class);
238-
$routeNameResolverProphecy->getRouteName(Dummy::class, OperationType::SUBRESOURCE)->willReturn('dummies');
240+
$routeNameResolverProphecy->getRouteName(Dummy::class, OperationType::SUBRESOURCE, Argument::type('array'))->willReturn('dummies');
239241

240242
$routerProphecy = $this->prophesize(RouterInterface::class);
241243
$routerProphecy->generate('dummies', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->willThrow(new RouteNotFoundException());
@@ -248,7 +250,7 @@ public function testNotAbleToGenerateGetSubresourceIriFromResourceClass()
248250
$routerProphecy->reveal()
249251
);
250252

251-
$converter->getSubresourceIriFromResourceClass(Dummy::class, ['id' => 1]);
253+
$converter->getSubresourceIriFromResourceClass(Dummy::class, ['subresource_identifiers' => ['id' => 1], 'subresource_resources' => [RelatedDummy::class => 1]]);
252254
}
253255

254256
public function testGetItemIriFromResourceClass()

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,15 @@ public function testGetRouteNameForCollectionRoute()
118118
public function testGetRouteNameForSubresourceRoute()
119119
{
120120
$routeCollection = new RouteCollection();
121-
$routeCollection->add('some_subresource_route', new Route('/some/item/path/{id}', [
121+
$routeCollection->add('a_some_subresource_route', new Route('/a/some/item/path/{id}', [
122+
'_api_resource_class' => 'AppBundle\Entity\User',
123+
'_api_subresource_operation_name' => 'some_other_item_op',
124+
'_api_subresource_context' => ['identifiers' => [[1, 'bar']]],
125+
]));
126+
$routeCollection->add('b_some_subresource_route', new Route('/b/some/item/path/{id}', [
122127
'_api_resource_class' => 'AppBundle\Entity\User',
123128
'_api_subresource_operation_name' => 'some_item_op',
129+
'_api_subresource_context' => ['identifiers' => [[1, 'foo']]],
124130
]));
125131
$routeCollection->add('some_collection_route', new Route('/some/collection/path', [
126132
'_api_resource_class' => 'AppBundle\Entity\User',
@@ -131,8 +137,8 @@ public function testGetRouteNameForSubresourceRoute()
131137
$routerProphecy->getRouteCollection()->willReturn($routeCollection);
132138

133139
$routeNameResolver = new RouteNameResolver($routerProphecy->reveal());
134-
$actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::SUBRESOURCE);
140+
$actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::SUBRESOURCE, ['subresource_resources' => ['foo' => 1]]);
135141

136-
$this->assertSame('some_subresource_route', $actual);
142+
$this->assertSame('b_some_subresource_route', $actual);
137143
}
138144
}

0 commit comments

Comments
 (0)