Skip to content

Commit f1bcbe4

Browse files
Nek-Nek-
committed
Rework how resource class resolver use inheritance
This commits comes with many changes: 1. The resource class resolver now works well with inheritance and is able to resolve resources using the ApiPlatform resource list (more clever) 2. This change adds a new feature: inherited items from api resources now are working as an api, a new behat test proves it 3. some parts of api platform were not compatible because it does not uses the resource resolver but "get class" to get the resource, theses parts are also fixed Notice: the cache from the resource class resolver is removed here. It was introduced in 2015 and was probably a great improvement, but since 2016 resources are not re-genarated on each call to the factory but we call instead a cached factory. This cache was just adding complexity so I removed it.
1 parent a5c1919 commit f1bcbe4

14 files changed

+286
-41
lines changed

‎features/bootstrap/DoctrineContext.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyOffer as DummyOfferDocument;
3232
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyProduct as DummyProductDocument;
3333
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyProperty as DummyPropertyDocument;
34+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyTableInheritanceNotApiResourceChild as DummyTableInheritanceNotApiResourceChildDocument;
3435
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\EmbeddableDummy as EmbeddableDummyDocument;
3536
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\EmbeddedDummy as EmbeddedDummyDocument;
3637
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\FileConfigDummy as FileConfigDummyDocument;
@@ -74,6 +75,7 @@
7475
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyOffer;
7576
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyProduct;
7677
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyProperty;
78+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceNotApiResourceChild;
7779
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy;
7880
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy;
7981
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy;
@@ -165,6 +167,17 @@ public function thereAreDummyObjects(int $nb)
165167
$this->manager->flush();
166168
}
167169

170+
/**
171+
* @When some dummy table inheritance data but not api resource child are created
172+
*/
173+
public function someDummyTableInheritanceDataButNotApiResourceChildAreCreated()
174+
{
175+
$dummy = $this->buildDummyTableInheritanceNotApiResourceChild();
176+
$dummy->setName('Foobarbaz inheritance');
177+
$this->manager->persist($dummy);
178+
$this->manager->flush();
179+
}
180+
168181
/**
169182
* @Given there are :nb foo objects with fake names
170183
*/
@@ -1272,6 +1285,14 @@ private function buildDummy()
12721285
return $this->isOrm() ? new Dummy() : new DummyDocument();
12731286
}
12741287

1288+
/**
1289+
* @return DummyTableInheritanceNotApiResourceChild|DummyTableInheritanceNotApiResourceChildDocument
1290+
*/
1291+
private function buildDummyTableInheritanceNotApiResourceChild()
1292+
{
1293+
return $this->isOrm() ? new DummyTableInheritanceNotApiResourceChild() : new DummyTableInheritanceNotApiResourceChildDocument();
1294+
}
1295+
12751296
/**
12761297
* @return DummyAggregateOffer|DummyAggregateOfferDocument
12771298
*/

‎features/main/table_inheritance.feature

Lines changed: 95 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,18 @@ Feature: Table inheritance
2020
"properties": {
2121
"@type": {
2222
"type": "string",
23-
"pattern": "^DummyTableInheritanceChild$"
23+
"pattern": "^DummyTableInheritanceChild$",
24+
"required": "true"
2425
},
2526
"@context": {
2627
"type": "string",
27-
"pattern": "^/contexts/DummyTableInheritanceChild$"
28+
"pattern": "^/contexts/DummyTableInheritanceChild$",
29+
"required": "true"
2830
},
2931
"@id": {
3032
"type": "string",
31-
"pattern": "^/dummy_table_inheritance_children/1$"
33+
"pattern": "^/dummy_table_inheritance_children/1$",
34+
"required": "true"
3235
},
3336
"name": {
3437
"type": "string",
@@ -61,7 +64,8 @@ Feature: Table inheritance
6164
"properties": {
6265
"@type": {
6366
"type": "string",
64-
"pattern": "^DummyTableInheritanceChild$"
67+
"pattern": "^DummyTableInheritanceChild$",
68+
"required": "true"
6569
},
6670
"name": {
6771
"type": "string",
@@ -80,7 +84,40 @@ Feature: Table inheritance
8084
}
8185
"""
8286

83-
@createSchema
87+
Scenario: Some children not api resources are created in the app
88+
When some dummy table inheritance data but not api resource child are created
89+
And I send a "GET" request to "/dummy_table_inheritances"
90+
Then the response status code should be 200
91+
And the response should be in JSON
92+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
93+
And the JSON should be valid according to this schema:
94+
"""
95+
{
96+
"type": "object",
97+
"properties": {
98+
"hydra:member": {
99+
"type": "array",
100+
"items": {
101+
"type": "object",
102+
"properties": {
103+
"@type": {
104+
"type": "string",
105+
"pattern": "^DummyTableInheritance(Child)?$",
106+
"required": "true"
107+
},
108+
"name": {
109+
"type": "string",
110+
"required": "true"
111+
}
112+
}
113+
},
114+
"minItems": 1
115+
}
116+
},
117+
"required": ["hydra:member"]
118+
}
119+
"""
120+
84121
Scenario: Create a table inherited resource
85122
When I add "Content-Type" header equal to "application/ld+json"
86123
And I send a "POST" request to "/dummy_table_inheritance_children" with body:
@@ -97,15 +134,18 @@ Feature: Table inheritance
97134
"properties": {
98135
"@type": {
99136
"type": "string",
100-
"pattern": "^DummyTableInheritanceChild$"
137+
"pattern": "^DummyTableInheritanceChild$",
138+
"required": "true"
101139
},
102140
"@context": {
103141
"type": "string",
104-
"pattern": "^/contexts/DummyTableInheritanceChild$"
142+
"pattern": "^/contexts/DummyTableInheritanceChild$",
143+
"required": "true"
105144
},
106145
"@id": {
107146
"type": "string",
108-
"pattern": "^/dummy_table_inheritance_children/1$"
147+
"pattern": "^/dummy_table_inheritance_children/3$",
148+
"required": "true"
109149
},
110150
"name": {
111151
"type": "string",
@@ -136,15 +176,18 @@ Feature: Table inheritance
136176
"properties": {
137177
"@type": {
138178
"type": "string",
139-
"pattern": "^DummyTableInheritanceDifferentChild$"
179+
"pattern": "^DummyTableInheritanceDifferentChild$",
180+
"required": "true"
140181
},
141182
"@context": {
142183
"type": "string",
143-
"pattern": "^/contexts/DummyTableInheritanceDifferentChild$"
184+
"pattern": "^/contexts/DummyTableInheritanceDifferentChild$",
185+
"required": "true"
144186
},
145187
"@id": {
146188
"type": "string",
147-
"pattern": "^/dummy_table_inheritance_different_children/2$"
189+
"pattern": "^/dummy_table_inheritance_different_children/4$",
190+
"required": "true"
148191
},
149192
"name": {
150193
"type": "string",
@@ -167,7 +210,7 @@ Feature: Table inheritance
167210
{
168211
"children": [
169212
"/dummy_table_inheritance_children/1",
170-
"/dummy_table_inheritance_different_children/2"
213+
"/dummy_table_inheritance_different_children/4"
171214
]
172215
}
173216
"""
@@ -181,15 +224,18 @@ Feature: Table inheritance
181224
"properties": {
182225
"@type": {
183226
"type": "string",
184-
"pattern": "^DummyTableInheritanceRelated$"
227+
"pattern": "^DummyTableInheritanceRelated$",
228+
"required": "true"
185229
},
186230
"@context": {
187231
"type": "string",
188-
"pattern": "^/contexts/DummyTableInheritanceRelated$"
232+
"pattern": "^/contexts/DummyTableInheritanceRelated$",
233+
"required": "true"
189234
},
190235
"@id": {
191236
"type": "string",
192-
"pattern": "^/dummy_table_inheritance_relateds/1$"
237+
"pattern": "^/dummy_table_inheritance_relateds/1$",
238+
"required": "true"
193239
},
194240
"children": {
195241
"items": {
@@ -199,7 +245,8 @@ Feature: Table inheritance
199245
"properties": {
200246
"@type": {
201247
"type": "string",
202-
"pattern": "^DummyTableInheritanceChild$"
248+
"pattern": "^DummyTableInheritanceChild$",
249+
"required": "true"
203250
},
204251
"name": {
205252
"type": "string",
@@ -215,7 +262,21 @@ Feature: Table inheritance
215262
"properties": {
216263
"@type": {
217264
"type": "string",
218-
"pattern": "^DummyTableInheritanceDifferentChild$"
265+
"pattern": "^DummyTableInheritance$",
266+
"required": "true"
267+
},
268+
"name": {
269+
"type": "string",
270+
"required": "true"
271+
}
272+
}
273+
},
274+
{
275+
"properties": {
276+
"@type": {
277+
"type": "string",
278+
"pattern": "^DummyTableInheritanceDifferentChild$",
279+
"required": "true"
219280
},
220281
"name": {
221282
"type": "string",
@@ -255,7 +316,8 @@ Feature: Table inheritance
255316
"properties": {
256317
"@type": {
257318
"type": "string",
258-
"pattern": "^DummyTableInheritanceChild$"
319+
"pattern": "^DummyTableInheritanceChild$",
320+
"required": "true"
259321
},
260322
"name": {
261323
"type": "string",
@@ -271,7 +333,21 @@ Feature: Table inheritance
271333
"properties": {
272334
"@type": {
273335
"type": "string",
274-
"pattern": "^DummyTableInheritanceDifferentChild$"
336+
"pattern": "^DummyTableInheritance$",
337+
"required": "true"
338+
},
339+
"name": {
340+
"type": "string",
341+
"required": "true"
342+
}
343+
}
344+
},
345+
{
346+
"properties": {
347+
"@type": {
348+
"type": "string",
349+
"pattern": "^DummyTableInheritanceDifferentChild$",
350+
"required": "true"
275351
},
276352
"name": {
277353
"type": "string",

‎src/Api/IdentifiersExtractor.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ public function getIdentifiersFromResourceClass(string $resourceClass): array
6767
public function getIdentifiersFromItem($item): array
6868
{
6969
$identifiers = [];
70-
$type = $this->getObjectClass($item);
71-
$resourceClass = $this->resourceClassResolver->isResourceClass($type) ? $this->resourceClassResolver->getResourceClass($item) : $type;
70+
$resourceClass = $this->getObjectClass($item);
71+
if (null !== $this->resourceClassResolver) { // BC Layer
72+
$resourceClass = $this->resourceClassResolver->isResourceClass($resourceClass) ? $this->resourceClassResolver->getResourceClass($item) : $resourceClass;
73+
}
7274

7375
foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) {
7476
$propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName);

‎src/Api/ResourceClassResolver.php

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ final class ResourceClassResolver implements ResourceClassResolverInterface
2828
use ClassInfoTrait;
2929

3030
private $resourceNameCollectionFactory;
31-
private $localIsResourceClassCache = [];
3231

3332
public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory)
3433
{
@@ -56,20 +55,27 @@ public function getResourceClass($value, string $resourceClass = null, bool $str
5655
$resolvedResourceClass = $currentResourceClass;
5756
}
5857
}
58+
5959
if (null !== $resolvedResourceClass) {
6060
$resourceClass = $resolvedResourceClass;
6161
}
6262

63+
$typeIsStrictResourceClass = $this->isStrictResourceClass($type);
64+
if ($strict && $typeIsStrictResourceClass) {
65+
return $type;
66+
}
67+
68+
$typeIsResourceClass = $this->isResourceClass($type);
6369
if (
6470
null === $type
65-
|| ((!$strict || $resourceClass === $type) && $isResourceClass = $this->isResourceClass($type))
66-
|| null !== $resolvedResourceClass && interface_exists($resourceClass)
71+
|| (((!$strict || $resourceClass === $type)) && $typeIsResourceClass)
72+
|| null !== $resolvedResourceClass && (interface_exists($resourceClass) || !$typeIsStrictResourceClass)
6773
) {
6874
return $resourceClass;
6975
}
7076

7177
if (
72-
($isResourceClass ?? $this->isResourceClass($type))
78+
$typeIsResourceClass
7379
|| (is_subclass_of($type, $resourceClass) && $this->isResourceClass($resourceClass))
7480
) {
7581
return $type;
@@ -83,16 +89,28 @@ public function getResourceClass($value, string $resourceClass = null, bool $str
8389
*/
8490
public function isResourceClass(string $type): bool
8591
{
86-
if (isset($this->localIsResourceClassCache[$type])) {
87-
return $this->localIsResourceClassCache[$type];
92+
foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {
93+
if ($type === $resourceClass || is_subclass_of($type, $resourceClass)) {
94+
return true;
95+
}
8896
}
8997

98+
return false;
99+
}
100+
101+
/**
102+
* Same of isResourceClass but stricter: it ignores inheritance.
103+
*
104+
* @param string $type FQCN of an object
105+
*/
106+
private function isStrictResourceClass(string $type): bool
107+
{
90108
foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {
91-
if ($type === $resourceClass || is_subclass_of($type, $resourceClass)) {
92-
return $this->localIsResourceClassCache[$type] = true;
109+
if ($type === $resourceClass) {
110+
return true;
93111
}
94112
}
95113

96-
return $this->localIsResourceClassCache[$type] = false;
114+
return false;
97115
}
98116
}

‎src/Api/ResourceClassResolverInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ interface ResourceClassResolverInterface
2525
/**
2626
* Guesses the associated resource.
2727
*
28-
* @param object $value Object you're playing with
28+
* @param mixed $value Object you're playing with
2929
* @param string $resourceClass Resource class it is supposed to be (could be parent class for instance)
3030
* @param bool $strict value must be type of resource class given or it will return type
3131
*

‎src/Bridge/Doctrine/EventListener/PublishMercureUpdatesListener.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,11 @@ private function reset(): void
121121
private function storeEntityToPublish($entity, string $property): void
122122
{
123123
$resourceClass = $this->getObjectClass($entity);
124+
124125
if (!$this->resourceClassResolver->isResourceClass($resourceClass)) {
125126
return;
126127
}
128+
$resourceClass = $this->resourceClassResolver->getResourceClass($entity);
127129

128130
$value = $this->resourceMetadataFactory->create($resourceClass)->getAttribute('mercure', false);
129131
if (false === $value) {

‎tests/Api/IdentifiersExtractorTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ private function getResourceClassResolver()
179179
$resourceClassResolver->isResourceClass(Argument::type('string'))->will(function ($args) {
180180
return !(Uuid::class === $args[0]);
181181
});
182+
$resourceClassResolver->getResourceClass(Argument::any())->will(function ($args) {return \get_class($args[0]); });
182183

183184
return $resourceClassResolver->reveal();
184185
}

0 commit comments

Comments
 (0)