Skip to content

Commit 183b0a5

Browse files
dunglasdunglas
authored
Merge pull request #1110 from soyuka/fix/1107
Fix route name resolving with subresources
2 parents 99f5950 + e66ed60 commit 183b0a5

14 files changed

+454
-20
lines changed

‎features/bootstrap/FeatureContext.php

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

+47-1
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

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

+9-2
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,14 @@ public function getRouteName(string $resourceClass, $operationType): string
5151
// do nothing
5252
}
5353

54-
$routeName = $this->decorated->getRouteName($resourceClass, $operationType);
54+
if (func_num_args() > 2) {
55+
$context = func_get_arg(2);
56+
} else {
57+
$context = [];
58+
@trigger_error(sprintf('Method %s() will have a third `$context = []` argument in version 3.0. Not defining it is deprecated since 2.1.', __METHOD__), E_USER_DEPRECATED);
59+
}
60+
61+
$routeName = $this->decorated->getRouteName($resourceClass, $operationType, $context);
5562

5663
if (!isset($cacheItem)) {
5764
return $routeName;

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

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

+29-1
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,15 @@ 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+
if (func_num_args() > 2) {
41+
$context = func_get_arg(2);
42+
} else {
43+
$context = [];
44+
@trigger_error(sprintf('Method %s() will have a third `$context = []` argument in version 3.0. Not defining it is deprecated since 2.1.', __METHOD__), E_USER_DEPRECATED);
45+
}
46+
3947
$operationType = OperationTypeDeprecationHelper::getOperationType($operationType);
4048

4149
foreach ($this->router->getRouteCollection()->all() as $routeName => $route) {
@@ -44,10 +52,30 @@ public function getRouteName(string $resourceClass, $operationType): string
4452
$methods = $route->getMethods();
4553

4654
if ($resourceClass === $currentResourceClass && null !== $operation && (empty($methods) || in_array('GET', $methods, true))) {
55+
if ($operationType === OperationType::SUBRESOURCE && false === $this->isSameSubresource($context, $route->getDefault('_api_subresource_context'))) {
56+
continue;
57+
}
58+
4759
return $routeName;
4860
}
4961
}
5062

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

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

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

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

+12-4
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public function testConstruct()
3939
/**
4040
* @expectedException \ApiPlatform\Core\Exception\InvalidArgumentException
4141
* @expectedExceptionMessage No item route associated with the type "AppBundle\Entity\User".
42+
* @group legacy
4243
*/
4344
public function testGetRouteNameForItemRouteWithNoMatchingRoute()
4445
{
@@ -50,14 +51,17 @@ public function testGetRouteNameForItemRouteWithNoMatchingRoute()
5051
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldNotBeCalled();
5152

5253
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
53-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false)
54+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false, [])
5455
->willThrow(new InvalidArgumentException('No item route associated with the type "AppBundle\Entity\User".'))
5556
->shouldBeCalled();
5657

5758
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
5859
$cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', false);
5960
}
6061

62+
/**
63+
* @group legacy
64+
*/
6165
public function testGetRouteNameForItemRouteOnCacheMiss()
6266
{
6367
$cacheItemProphecy = $this->prophesize(CacheItemInterface::class);
@@ -69,7 +73,7 @@ public function testGetRouteNameForItemRouteOnCacheMiss()
6973
$cacheItemPoolProphecy->save($cacheItemProphecy)->willReturn(true)->shouldBeCalled();
7074

7175
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
72-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false)->willReturn('some_item_route')->shouldBeCalled();
76+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false, [])->willReturn('some_item_route')->shouldBeCalled();
7377

7478
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
7579
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', false);
@@ -99,6 +103,7 @@ public function testGetRouteNameForItemRouteOnCacheHit()
99103
/**
100104
* @expectedException \ApiPlatform\Core\Exception\InvalidArgumentException
101105
* @expectedExceptionMessage No collection route associated with the type "AppBundle\Entity\User".
106+
* @group legacy
102107
*/
103108
public function testGetRouteNameForCollectionRouteWithNoMatchingRoute()
104109
{
@@ -110,14 +115,17 @@ public function testGetRouteNameForCollectionRouteWithNoMatchingRoute()
110115
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldNotBeCalled();
111116

112117
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
113-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true)
118+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true, [])
114119
->willThrow(new InvalidArgumentException('No collection route associated with the type "AppBundle\Entity\User".'))
115120
->shouldBeCalled();
116121

117122
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
118123
$cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', true);
119124
}
120125

126+
/**
127+
* @group legacy
128+
*/
121129
public function testGetRouteNameForCollectionRouteOnCacheMiss()
122130
{
123131
$cacheItemProphecy = $this->prophesize(CacheItemInterface::class);
@@ -129,7 +137,7 @@ public function testGetRouteNameForCollectionRouteOnCacheMiss()
129137
$cacheItemPoolProphecy->save($cacheItemProphecy)->willReturn(true)->shouldBeCalled();
130138

131139
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
132-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true)->willReturn('some_collection_route')->shouldBeCalled();
140+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true, [])->willReturn('some_collection_route')->shouldBeCalled();
133141

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

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

+6-4
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

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