Skip to content

Commit 731b45f

Browse files
teohhanhuiteohhanhui
committed
Fix SerializerPropertyMetadataFactory readable/writable link status for resource with inheritance
1 parent 57d6788 commit 731b45f

File tree

5 files changed

+70
-46
lines changed

5 files changed

+70
-46
lines changed

‎src/Api/ResourceClassResolver.php‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,32 +50,33 @@ public function getResourceClass($value, string $resourceClass = null, bool $str
5050
throw new InvalidArgumentException('Resource type could not be determined. Resource class must be specified.');
5151
}
5252

53-
if (null !== $resourceClass && !$this->isResourceClass($resourceClass)) {
54-
throw new InvalidArgumentException(sprintf('Specified class "%s" is not a resource class.', $resourceClass));
53+
if (null !== $actualClass && !$this->isResourceClass($actualClass)) {
54+
throw new InvalidArgumentException(sprintf('No resource class found for object of type "%s".', $actualClass));
5555
}
5656

57-
if (null === $actualClass) {
58-
return $resourceClass;
57+
if (null !== $resourceClass && !$this->isResourceClass($resourceClass)) {
58+
throw new InvalidArgumentException(sprintf('Specified class "%s" is not a resource class.', $resourceClass));
5959
}
6060

61-
if ($strict && !is_a($actualClass, $resourceClass, true)) {
61+
if ($strict && null !== $actualClass && !is_a($actualClass, $resourceClass, true)) {
6262
throw new InvalidArgumentException(sprintf('Object of type "%s" does not match "%s" resource class.', $actualClass, $resourceClass));
6363
}
6464

65+
$targetClass = $actualClass ?? $resourceClass;
6566
$mostSpecificResourceClass = null;
6667

6768
foreach ($this->resourceNameCollectionFactory->create() as $resourceClassName) {
68-
if (!is_a($actualClass, $resourceClassName, true)) {
69+
if (!is_a($targetClass, $resourceClassName, true)) {
6970
continue;
7071
}
7172

72-
if (null === $mostSpecificResourceClass || is_subclass_of($resourceClassName, $mostSpecificResourceClass, true)) {
73+
if (null === $mostSpecificResourceClass || is_subclass_of($resourceClassName, $mostSpecificResourceClass)) {
7374
$mostSpecificResourceClass = $resourceClassName;
7475
}
7576
}
7677

7778
if (null === $mostSpecificResourceClass) {
78-
throw new InvalidArgumentException(sprintf('No resource class found for object of type "%s".', $actualClass));
79+
throw new \LogicException('Unexpected execution flow.');
7980
}
8081

8182
return $mostSpecificResourceClass;

‎src/Bridge/Symfony/Bundle/Resources/config/metadata/metadata.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
<argument type="service" id="api_platform.metadata.resource.metadata_factory" />
7171
<argument type="service" id="serializer.mapping.class_metadata_factory" />
7272
<argument type="service" id="api_platform.metadata.property.metadata_factory.serializer.inner" />
73+
<argument type="service" id="api_platform.resource_class_resolver" />
7374
</service>
7475

7576
<service id="api_platform.metadata.property.metadata_factory.cached" class="ApiPlatform\Core\Metadata\Property\Factory\CachedPropertyMetadataFactory" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="-10" public="false">

‎src/Hydra/Serializer/DocumentationNormalizer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ private function getProperty(PropertyMetadata $propertyMetadata, string $propert
459459
{
460460
$propertyData = [
461461
'@id' => $propertyMetadata->getIri() ?? "#$shortName/$propertyName",
462-
'@type' => $propertyMetadata->isReadableLink() ? 'rdf:Property' : 'hydra:Link',
462+
'@type' => false === $propertyMetadata->isReadableLink() ? 'hydra:Link' : 'rdf:Property',
463463
'rdfs:label' => $propertyName,
464464
'domain' => $prefixedShortName,
465465
];

‎src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Metadata\Property\Factory;
1515

16+
use ApiPlatform\Core\Api\ResourceClassResolverInterface;
1617
use ApiPlatform\Core\Exception\ResourceClassNotFoundException;
1718
use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
1819
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
@@ -29,12 +30,14 @@ final class SerializerPropertyMetadataFactory implements PropertyMetadataFactory
2930
private $resourceMetadataFactory;
3031
private $serializerClassMetadataFactory;
3132
private $decorated;
33+
private $resourceClassResolver;
3234

33-
public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, SerializerClassMetadataFactoryInterface $serializerClassMetadataFactory, PropertyMetadataFactoryInterface $decorated)
35+
public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, SerializerClassMetadataFactoryInterface $serializerClassMetadataFactory, PropertyMetadataFactoryInterface $decorated, ResourceClassResolverInterface $resourceClassResolver = null)
3436
{
3537
$this->resourceMetadataFactory = $resourceMetadataFactory;
3638
$this->serializerClassMetadataFactory = $serializerClassMetadataFactory;
3739
$this->decorated = $decorated;
40+
$this->resourceClassResolver = $resourceClassResolver;
3841
}
3942

4043
/**
@@ -52,14 +55,14 @@ public function create(string $resourceClass, string $property, array $options =
5255

5356
try {
5457
[$normalizationGroups, $denormalizationGroups] = $this->getEffectiveSerializerGroups($options, $resourceClass);
55-
56-
$propertyMetadata = $this->transformReadWrite($propertyMetadata, $resourceClass, $property, $normalizationGroups, $denormalizationGroups);
57-
$propertyMetadata = $this->transformLinkStatus($propertyMetadata, $normalizationGroups, $denormalizationGroups);
5858
} catch (ResourceClassNotFoundException $e) {
59-
// No need to check link status if related class is not a resource
59+
// TODO: for input/output classes, the serializer groups must be read from the actual resource class
60+
return $propertyMetadata;
6061
}
6162

62-
return $propertyMetadata;
63+
$propertyMetadata = $this->transformReadWrite($propertyMetadata, $resourceClass, $property, $normalizationGroups, $denormalizationGroups);
64+
65+
return $this->transformLinkStatus($propertyMetadata, $normalizationGroups, $denormalizationGroups);
6366
}
6467

6568
/**
@@ -94,8 +97,6 @@ private function transformReadWrite(PropertyMetadata $propertyMetadata, string $
9497
*
9598
* @param string[]|null $normalizationGroups
9699
* @param string[]|null $denormalizationGroups
97-
*
98-
* @throws ResourceClassNotFoundException
99100
*/
100101
private function transformLinkStatus(PropertyMetadata $propertyMetadata, array $normalizationGroups = null, array $denormalizationGroups = null): PropertyMetadata
101102
{
@@ -111,12 +112,18 @@ private function transformLinkStatus(PropertyMetadata $propertyMetadata, array $
111112

112113
$relatedClass = $type->isCollection() && ($collectionValueType = $type->getCollectionValueType()) ? $collectionValueType->getClassName() : $type->getClassName();
113114

114-
if (null === $relatedClass) {
115-
return $propertyMetadata->withReadableLink(true)->withWritableLink(true);
115+
// if property is not a resource relation, don't set link status (as it would have no meaning)
116+
if (null === $relatedClass || !$this->isResourceClass($relatedClass)) {
117+
return $propertyMetadata;
116118
}
117119

118-
$this->resourceMetadataFactory->create($relatedClass);
119-
$relatedGroups = $this->getResourceSerializerGroups($relatedClass);
120+
// find the resource class
121+
// this prevents serializer groups on non-resource child class from incorrectly influencing the decision
122+
if (null !== $this->resourceClassResolver) {
123+
$relatedClass = $this->resourceClassResolver->getResourceClass(null, $relatedClass);
124+
}
125+
126+
$relatedGroups = $this->getClassSerializerGroups($relatedClass);
120127

121128
if (null === $propertyMetadata->isReadableLink()) {
122129
$propertyMetadata = $propertyMetadata->withReadableLink(null !== $normalizationGroups && !empty(array_intersect($normalizationGroups, $relatedGroups)));
@@ -138,6 +145,8 @@ private function transformLinkStatus(PropertyMetadata $propertyMetadata, array $
138145
* - From metadata of the given operation ("collection_operation_name" and "item_operation_name" keys).
139146
* - From metadata of the current resource.
140147
*
148+
* @throws ResourceClassNotFoundException
149+
*
141150
* @return (string[]|null)[]
142151
*/
143152
private function getEffectiveSerializerGroups(array $options, string $resourceClass): array
@@ -174,9 +183,9 @@ private function getEffectiveSerializerGroups(array $options, string $resourceCl
174183
*
175184
* @return string[]
176185
*/
177-
private function getPropertySerializerGroups(string $resourceClass, string $property): array
186+
private function getPropertySerializerGroups(string $class, string $property): array
178187
{
179-
$serializerClassMetadata = $this->serializerClassMetadataFactory->getMetadataFor($resourceClass);
188+
$serializerClassMetadata = $this->serializerClassMetadataFactory->getMetadataFor($class);
180189

181190
foreach ($serializerClassMetadata->getAttributesMetadata() as $serializerAttributeMetadata) {
182191
if ($property === $serializerAttributeMetadata->getName()) {
@@ -188,19 +197,34 @@ private function getPropertySerializerGroups(string $resourceClass, string $prop
188197
}
189198

190199
/**
191-
* Gets the serializer groups defined in a resource.
200+
* Gets all serializer groups used in a class.
192201
*
193202
* @return string[]
194203
*/
195-
private function getResourceSerializerGroups(string $resourceClass): array
204+
private function getClassSerializerGroups(string $class): array
196205
{
197-
$serializerClassMetadata = $this->serializerClassMetadataFactory->getMetadataFor($resourceClass);
206+
$serializerClassMetadata = $this->serializerClassMetadataFactory->getMetadataFor($class);
198207

199208
$groups = [];
200209
foreach ($serializerClassMetadata->getAttributesMetadata() as $serializerAttributeMetadata) {
201-
$groups += array_flip($serializerAttributeMetadata->getGroups());
210+
$groups = array_merge($groups, $serializerAttributeMetadata->getGroups());
211+
}
212+
213+
return array_unique($groups);
214+
}
215+
216+
private function isResourceClass(string $class): bool
217+
{
218+
if (null !== $this->resourceClassResolver) {
219+
return $this->resourceClassResolver->isResourceClass($class);
202220
}
203221

204-
return array_keys($groups);
222+
try {
223+
$this->resourceMetadataFactory->create($class);
224+
225+
return true;
226+
} catch (ResourceClassNotFoundException $e) {
227+
return false;
228+
}
205229
}
206230
}

‎tests/Metadata/Property/Factory/SerializerPropertyMetadataFactoryTest.php‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Tests\Metadata\Property\Factory;
1515

16+
use ApiPlatform\Core\Api\ResourceClassResolverInterface;
1617
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
1718
use ApiPlatform\Core\Metadata\Property\Factory\SerializerPropertyMetadataFactory;
1819
use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
@@ -65,9 +66,7 @@ public function testCreate($readGroups, $writeGroups)
6566
AbstractNormalizer::GROUPS => $writeGroups,
6667
],
6768
]);
68-
$resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn($dummyResourceMetadata)->shouldBeCalled();
69-
$resourceMetadataFactoryProphecy->create(RelatedDummy::class)->willReturn(new ResourceMetadata())->shouldBeCalled();
70-
$resourceMetadataFactory = $resourceMetadataFactoryProphecy->reveal();
69+
$resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn($dummyResourceMetadata);
7170

7271
$serializerClassMetadataFactoryProphecy = $this->prophesize(SerializerClassMetadataFactoryInterface::class);
7372
$dummySerializerClassMetadata = new SerializerClassMetadata(Dummy::class);
@@ -81,32 +80,34 @@ public function testCreate($readGroups, $writeGroups)
8180
$dummySerializerClassMetadata->addAttributeMetadata($relatedDummySerializerAttributeMetadata);
8281
$nameConvertedSerializerAttributeMetadata = new SerializerAttributeMetadata('nameConverted');
8382
$dummySerializerClassMetadata->addAttributeMetadata($nameConvertedSerializerAttributeMetadata);
84-
$serializerClassMetadataFactoryProphecy->getMetadataFor(Dummy::class)->willReturn($dummySerializerClassMetadata)->shouldBeCalled();
83+
$serializerClassMetadataFactoryProphecy->getMetadataFor(Dummy::class)->willReturn($dummySerializerClassMetadata);
8584
$relatedDummySerializerClassMetadata = new SerializerClassMetadata(RelatedDummy::class);
8685
$idSerializerAttributeMetadata = new SerializerAttributeMetadata('id');
8786
$idSerializerAttributeMetadata->addGroup('dummy_read');
8887
$relatedDummySerializerClassMetadata->addAttributeMetadata($idSerializerAttributeMetadata);
8988
$nameSerializerAttributeMetadata = new SerializerAttributeMetadata('name');
9089
$nameSerializerAttributeMetadata->addGroup('dummy_read');
9190
$relatedDummySerializerClassMetadata->addAttributeMetadata($nameSerializerAttributeMetadata);
92-
$serializerClassMetadataFactoryProphecy->getMetadataFor(RelatedDummy::class)->willReturn($relatedDummySerializerClassMetadata)->shouldBeCalled();
93-
$serializerClassMetadataFactory = $serializerClassMetadataFactoryProphecy->reveal();
91+
$serializerClassMetadataFactoryProphecy->getMetadataFor(RelatedDummy::class)->willReturn($relatedDummySerializerClassMetadata);
9492

9593
$decoratedProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
9694
$fooPropertyMetadata = (new PropertyMetadata())
9795
->withType(new Type(Type::BUILTIN_TYPE_ARRAY, true))
9896
->withReadable(false)
9997
->withWritable(true);
100-
$decoratedProphecy->create(Dummy::class, 'foo', [])->willReturn($fooPropertyMetadata)->shouldBeCalled();
98+
$decoratedProphecy->create(Dummy::class, 'foo', [])->willReturn($fooPropertyMetadata);
10199
$relatedDummyPropertyMetadata = (new PropertyMetadata())
102100
->withType(new Type(Type::BUILTIN_TYPE_OBJECT, true, RelatedDummy::class));
103-
$decoratedProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn($relatedDummyPropertyMetadata)->shouldBeCalled();
101+
$decoratedProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn($relatedDummyPropertyMetadata);
104102
$nameConvertedPropertyMetadata = (new PropertyMetadata())
105103
->withType(new Type(Type::BUILTIN_TYPE_STRING, true));
106-
$decoratedProphecy->create(Dummy::class, 'nameConverted', [])->willReturn($nameConvertedPropertyMetadata)->shouldBeCalled();
107-
$decorated = $decoratedProphecy->reveal();
104+
$decoratedProphecy->create(Dummy::class, 'nameConverted', [])->willReturn($nameConvertedPropertyMetadata);
108105

109-
$serializerPropertyMetadataFactory = new SerializerPropertyMetadataFactory($resourceMetadataFactory, $serializerClassMetadataFactory, $decorated);
106+
$resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
107+
$resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(true);
108+
$resourceClassResolverProphecy->getResourceClass(null, RelatedDummy::class)->willReturn(RelatedDummy::class);
109+
110+
$serializerPropertyMetadataFactory = new SerializerPropertyMetadataFactory($resourceMetadataFactoryProphecy->reveal(), $serializerClassMetadataFactoryProphecy->reveal(), $decoratedProphecy->reveal(), $resourceClassResolverProphecy->reveal());
110111

111112
$actual = [];
112113
$actual[] = $serializerPropertyMetadataFactory->create(Dummy::class, 'foo');
@@ -139,22 +140,19 @@ public function groupsProvider(): array
139140
public function testCreateInherited()
140141
{
141142
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
142-
$resourceMetadataFactoryProphecy->create(DummyTableInheritanceChild::class)->willReturn(new ResourceMetadata())->shouldBeCalled();
143-
$resourceMetadataFactory = $resourceMetadataFactoryProphecy->reveal();
143+
$resourceMetadataFactoryProphecy->create(DummyTableInheritanceChild::class)->willReturn(new ResourceMetadata());
144144

145145
$serializerClassMetadataFactoryProphecy = $this->prophesize(SerializerClassMetadataFactoryInterface::class);
146146
$dummySerializerClassMetadata = new SerializerClassMetadata(DummyTableInheritanceChild::class);
147-
$serializerClassMetadataFactoryProphecy->getMetadataFor(DummyTableInheritanceChild::class)->willReturn($dummySerializerClassMetadata)->shouldBeCalled();
148-
$serializerClassMetadataFactory = $serializerClassMetadataFactoryProphecy->reveal();
147+
$serializerClassMetadataFactoryProphecy->getMetadataFor(DummyTableInheritanceChild::class)->willReturn($dummySerializerClassMetadata);
149148

150149
$decoratedProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
151150
$fooPropertyMetadata = (new PropertyMetadata())
152151
->withType(new Type(Type::BUILTIN_TYPE_ARRAY, true))
153152
->withChildInherited(DummyTableInheritanceChild::class);
154-
$decoratedProphecy->create(DummyTableInheritance::class, 'nickname', [])->willReturn($fooPropertyMetadata)->shouldBeCalled();
155-
$decorated = $decoratedProphecy->reveal();
153+
$decoratedProphecy->create(DummyTableInheritance::class, 'nickname', [])->willReturn($fooPropertyMetadata);
156154

157-
$serializerPropertyMetadataFactory = new SerializerPropertyMetadataFactory($resourceMetadataFactory, $serializerClassMetadataFactory, $decorated);
155+
$serializerPropertyMetadataFactory = new SerializerPropertyMetadataFactory($resourceMetadataFactoryProphecy->reveal(), $serializerClassMetadataFactoryProphecy->reveal(), $decoratedProphecy->reveal());
158156

159157
$actual = $serializerPropertyMetadataFactory->create(DummyTableInheritance::class, 'nickname');
160158

0 commit comments

Comments
 (0)