Skip to content

Commit 7ddc064

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 7dfdcb1 commit 7ddc064

File tree

11 files changed

+413
-9
lines changed

11 files changed

+413
-9
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\DummyTour;
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 tour with offers
483+
*/
484+
public function createTourWithOffers()
485+
{
486+
$offer = new DummyOffer();
487+
$offer->setValue(2);
488+
$aggregate = new DummyAggregateOffer();
489+
$aggregate->setValue(1);
490+
$aggregate->addOffer($offer);
491+
492+
$tour = new DummyTour();
493+
$tour->setName('Dummy tour');
494+
$tour->addOffer($aggregate);
495+
496+
$this->manager->persist($tour);
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 tour with offers
226+
When I send a "GET" request to "/dummy_tours/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_tours/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, $context);
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: 22 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,7 +35,7 @@ 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
{
3940
$operationType = OperationTypeDeprecationHelper::getOperationType($operationType);
4041

@@ -44,10 +45,30 @@ public function getRouteName(string $resourceClass, $operationType): string
4445
$methods = $route->getMethods();
4546

4647
if ($resourceClass === $currentResourceClass && null !== $operation && (empty($methods) || in_array('GET', $methods, true))) {
48+
if ($operationType === OperationType::SUBRESOURCE && false === $this->isSameSubresource($context, $route->getDefault('_api_subresource_context'))) {
49+
continue;
50+
}
51+
4752
return $routeName;
4853
}
4954
}
5055

5156
throw new InvalidArgumentException(sprintf('No %s route associated with the type "%s".', $operationType, $resourceClass));
5257
}
58+
59+
private function isSameSubresource(array $context, array $currentContext): bool
60+
{
61+
$subresources = array_keys($context['subresource_resources']);
62+
$currentSubresources = [];
63+
64+
foreach ($currentContext['identifiers'] as $identiferContext) {
65+
$currentSubresources[] = $identiferContext[1];
66+
}
67+
68+
if ($currentSubresources === $subresources) {
69+
return true;
70+
}
71+
72+
return false;
73+
}
5374
}

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
}
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
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\Core\Tests\Fixtures\TestBundle\Entity;
15+
16+
use ApiPlatform\Core\Annotation\ApiProperty;
17+
use ApiPlatform\Core\Annotation\ApiResource;
18+
use Doctrine\Common\Collections\ArrayCollection;
19+
use Doctrine\ORM\Mapping as ORM;
20+
21+
/**
22+
* Dummy Aggregate Offer.
23+
* https://github.com/api-platform/core/issues/1107.
24+
*
25+
* @author Antoine Bluchet <[email protected]>
26+
*
27+
* @ApiResource
28+
* @ORM\Entity
29+
*/
30+
class DummyAggregateOffer
31+
{
32+
/**
33+
* @var int The id
34+
*
35+
* @ORM\Column(type="integer")
36+
* @ORM\Id
37+
* @ORM\GeneratedValue(strategy="AUTO")
38+
*/
39+
private $id;
40+
41+
/**
42+
* @var ArrayCollection
43+
*
44+
* @ApiProperty(subresource=true)
45+
* @ORM\OneToMany(targetEntity="DummyOffer", mappedBy="id", cascade={"persist"})
46+
*/
47+
private $offers;
48+
49+
/**
50+
* @var int The dummy aggregate offer value
51+
*
52+
* @ORM\Column(type="integer")
53+
*/
54+
private $value;
55+
56+
public function __construct()
57+
{
58+
$this->offers = new ArrayCollection();
59+
}
60+
61+
/**
62+
* Get offers.
63+
*
64+
* @return offers
65+
*/
66+
public function getOffers(): ArrayCollection
67+
{
68+
return $this->offers;
69+
}
70+
71+
/**
72+
* Set offers.
73+
*
74+
* @param offers the value to set
75+
*/
76+
public function setOffers($offers)
77+
{
78+
$this->offers = $offers;
79+
}
80+
81+
/**
82+
* Add offer.
83+
*
84+
* @param offer the value to add
85+
*/
86+
public function addOffer(DummyOffer $offer)
87+
{
88+
$this->offers->add($offer);
89+
}
90+
91+
/**
92+
* Get id.
93+
*
94+
* @return id
95+
*/
96+
public function getId(): int
97+
{
98+
return $this->id;
99+
}
100+
101+
/**
102+
* Get value.
103+
*
104+
* @return value
105+
*/
106+
public function getValue(): int
107+
{
108+
return $this->value;
109+
}
110+
111+
/**
112+
* Set value.
113+
*
114+
* @param value the value to set
115+
*/
116+
public function setValue(int $value)
117+
{
118+
$this->value = $value;
119+
}
120+
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
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\Core\Tests\Fixtures\TestBundle\Entity;
15+
16+
use ApiPlatform\Core\Annotation\ApiResource;
17+
use Doctrine\ORM\Mapping as ORM;
18+
19+
/**
20+
* Dummy Offer.
21+
* https://github.com/api-platform/core/issues/1107.
22+
*
23+
* @author Antoine Bluchet <[email protected]>
24+
*
25+
* @ApiResource
26+
* @ORM\Entity
27+
*/
28+
class DummyOffer
29+
{
30+
/**
31+
* @var int The id
32+
*
33+
* @ORM\Column(type="integer")
34+
* @ORM\Id
35+
* @ORM\GeneratedValue(strategy="AUTO")
36+
*/
37+
private $id;
38+
39+
/**
40+
* @var int The dummy aggregate offer value
41+
*
42+
* @ORM\Column(type="integer")
43+
*/
44+
private $value;
45+
46+
/**
47+
* Get id.
48+
*
49+
* @return id
50+
*/
51+
public function getId(): int
52+
{
53+
return $this->id;
54+
}
55+
56+
/**
57+
* Get value.
58+
*
59+
* @return value
60+
*/
61+
public function getValue(): int
62+
{
63+
return $this->value;
64+
}
65+
66+
/**
67+
* Set value.
68+
*
69+
* @param value the value to set
70+
*/
71+
public function setValue(int $value)
72+
{
73+
$this->value = $value;
74+
}
75+
}

0 commit comments

Comments
 (0)