Skip to content

Commit b72a90b

Browse files
teohhanhuiteohhanhui
authored
Merge pull request #3273 from teohhanhui/fix/remove-child-inherited-metadata
Remove nonsensical "child inherited" metadata
2 parents 2430ab5 + be5752f commit b72a90b

15 files changed

Lines changed: 138 additions & 63 deletions

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,6 @@
4949
<argument type="service" id="api_platform.property_info" />
5050
</service>
5151

52-
<service id="api_platform.metadata.property.name_collection_factory.inherited" class="ApiPlatform\Core\Metadata\Property\Factory\InheritedPropertyNameCollectionFactory" decorates="api_platform.metadata.property.name_collection_factory" decoration-priority="10" public="false">
53-
<argument type="service" id="api_platform.metadata.resource.name_collection_factory" />
54-
<argument type="service" id="api_platform.metadata.property.name_collection_factory.inherited.inner" />
55-
</service>
56-
5752
<service id="api_platform.metadata.property.name_collection_factory.cached" class="ApiPlatform\Core\Metadata\Property\Factory\CachedPropertyNameCollectionFactory" decorates="api_platform.metadata.property.name_collection_factory" decoration-priority="-10" public="false">
5853
<argument type="service" id="api_platform.cache.metadata.property" />
5954
<argument type="service" id="api_platform.metadata.property.name_collection_factory.cached.inner" />
@@ -67,11 +62,6 @@
6762
<argument type="service" id="api_platform.metadata.property.metadata_factory.property_info.inner" />
6863
</service>
6964

70-
<service id="api_platform.metadata.property.metadata_factory.inherited" class="ApiPlatform\Core\Metadata\Property\Factory\InheritedPropertyMetadataFactory" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="40" public="false">
71-
<argument type="service" id="api_platform.metadata.resource.name_collection_factory" />
72-
<argument type="service" id="api_platform.metadata.property.metadata_factory.inherited.inner" />
73-
</service>
74-
7565
<service id="api_platform.metadata.property.metadata_factory.serializer" class="ApiPlatform\Core\Metadata\Property\Factory\SerializerPropertyMetadataFactory" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="30" public="false">
7666
<argument type="service" id="api_platform.metadata.resource.metadata_factory" />
7767
<argument type="service" id="serializer.mapping.class_metadata_factory" />

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function create(string $resourceClass, array $options = []): PropertyName
4848
try {
4949
$propertyNameCollection = $this->decorated->create($resourceClass, $options);
5050
} catch (ResourceClassNotFoundException $resourceClassNotFoundException) {
51-
// Ignore not found exceptions from parent
51+
// Ignore not found exceptions from decorated factory
5252
}
5353
}
5454

@@ -87,7 +87,7 @@ public function create(string $resourceClass, array $options = []): PropertyName
8787
}
8888
}
8989

90-
// Inherited from parent
90+
// add property names from decorated factory
9191
if (null !== $propertyNameCollection) {
9292
foreach ($propertyNameCollection as $propertyName) {
9393
$propertyNames[$propertyName] = $propertyName;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function create(string $resourceClass, array $options = []): PropertyName
4949
try {
5050
$propertyNameCollection = $this->decorated->create($resourceClass, $options);
5151
} catch (ResourceClassNotFoundException $resourceClassNotFoundException) {
52-
// Ignore not found exceptions from parent
52+
// Ignore not found exceptions from decorated factory
5353
}
5454

5555
foreach ($propertyNameCollection as $propertyName) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
1818

1919
/**
20-
* Get property metadata from eventual child inherited properties.
21-
*
22-
* @author Antoine Bluchet <soyuka@gmail.com>
20+
* @deprecated since 2.6, to be removed in 3.0
2321
*/
2422
final class InheritedPropertyMetadataFactory implements PropertyMetadataFactoryInterface
2523
{
@@ -28,6 +26,8 @@ final class InheritedPropertyMetadataFactory implements PropertyMetadataFactoryI
2826

2927
public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, PropertyMetadataFactoryInterface $decorated = null)
3028
{
29+
@trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
30+
3131
$this->resourceNameCollectionFactory = $resourceNameCollectionFactory;
3232
$this->decorated = $decorated;
3333
}
@@ -37,6 +37,8 @@ public function __construct(ResourceNameCollectionFactoryInterface $resourceName
3737
*/
3838
public function create(string $resourceClass, string $property, array $options = []): PropertyMetadata
3939
{
40+
@trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
41+
4042
$propertyMetadata = $this->decorated ? $this->decorated->create($resourceClass, $property, $options) : new PropertyMetadata();
4143

4244
foreach ($this->resourceNameCollectionFactory->create() as $knownResourceClass) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
1818

1919
/**
20-
* Creates a property name collection from eventual child inherited properties.
21-
*
22-
* @author Antoine Bluchet <soyuka@gmail.com>
20+
* @deprecated since 2.6, to be removed in 3.0
2321
*/
2422
final class InheritedPropertyNameCollectionFactory implements PropertyNameCollectionFactoryInterface
2523
{
@@ -28,6 +26,8 @@ final class InheritedPropertyNameCollectionFactory implements PropertyNameCollec
2826

2927
public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, PropertyNameCollectionFactoryInterface $decorated = null)
3028
{
29+
@trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
30+
3131
$this->resourceNameCollectionFactory = $resourceNameCollectionFactory;
3232
$this->decorated = $decorated;
3333
}
@@ -37,6 +37,8 @@ public function __construct(ResourceNameCollectionFactoryInterface $resourceName
3737
*/
3838
public function create(string $resourceClass, array $options = []): PropertyNameCollection
3939
{
40+
@trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
41+
4042
$propertyNames = [];
4143

4244
// Inherited from parent

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ public function create(string $resourceClass, string $property, array $options =
4848
{
4949
$propertyMetadata = $this->decorated->create($resourceClass, $property, $options);
5050

51-
// in case of a property inherited (in a child class), we need it's properties
52-
// to be mapped against serialization groups instead of the parent ones.
51+
// BC to be removed in 3.0
5352
if (null !== ($childResourceClass = $propertyMetadata->getChildInherited())) {
5453
$resourceClass = $childResourceClass;
5554
}

‎src/Metadata/Property/PropertyMetadata.php‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ final class PropertyMetadata
3131
private $required;
3232
private $iri;
3333
private $identifier;
34+
/**
35+
* @deprecated since 2.6, to be removed in 3.0
36+
*/
3437
private $childInherited;
3538
private $attributes;
3639
private $subresource;
@@ -47,6 +50,9 @@ public function __construct(Type $type = null, string $description = null, bool
4750
$this->required = $required;
4851
$this->identifier = $identifier;
4952
$this->iri = $iri;
53+
if (null !== $childInherited) {
54+
@trigger_error(sprintf('Providing a non-null value for the 10th argument ($childInherited) of the "%s" constructor is deprecated since 2.6 and will not be supported in 3.0.', __CLASS__), E_USER_DEPRECATED);
55+
}
5056
$this->childInherited = $childInherited;
5157
$this->attributes = $attributes;
5258
$this->subresource = $subresource;
@@ -258,38 +264,38 @@ public function withAttributes(array $attributes): self
258264
}
259265

260266
/**
261-
* Gets child inherited.
267+
* @deprecated since 2.6, to be removed in 3.0
262268
*/
263269
public function getChildInherited(): ?string
264270
{
265271
return $this->childInherited;
266272
}
267273

268274
/**
269-
* Is the property inherited from a child class?
275+
* @deprecated since 2.6, to be removed in 3.0
270276
*/
271277
public function hasChildInherited(): bool
272278
{
273279
return null !== $this->childInherited;
274280
}
275281

276282
/**
277-
* Is the property inherited from a child class?
278-
*
279-
* @deprecated since version 2.4, to be removed in 3.0.
283+
* @deprecated since 2.4, to be removed in 3.0
280284
*/
281285
public function isChildInherited(): ?string
282286
{
283-
@trigger_error(sprintf('The use of "%1$s::isChildInherited()" is deprecated since 2.4 and will be removed in 3.0. Use "%1$s::getChildInherited()" or "%1$s::hasChildInherited()" directly instead.', __CLASS__), E_USER_DEPRECATED);
287+
@trigger_error(sprintf('"%s::%s" is deprecated since 2.4 and will be removed in 3.0.', __CLASS__, __METHOD__), E_USER_DEPRECATED);
284288

285289
return $this->getChildInherited();
286290
}
287291

288292
/**
289-
* Returns a new instance with the given child inherited class.
293+
* @deprecated since 2.6, to be removed in 3.0
290294
*/
291295
public function withChildInherited(string $childInherited): self
292296
{
297+
@trigger_error(sprintf('"%s::%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__, __METHOD__), E_USER_DEPRECATED);
298+
293299
$metadata = clone $this;
294300
$metadata->childInherited = $childInherited;
295301

‎src/Serializer/AbstractItemNormalizer.php‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,11 @@ public function supportsDenormalization($data, $type, $format = null)
169169
*/
170170
public function denormalize($data, $class, $format = null, array $context = [])
171171
{
172-
$resourceClass = $this->resourceClassResolver->getResourceClass(null, $class);
172+
if (null === $objectToPopulate = $this->extractObjectToPopulate($class, $context, static::OBJECT_TO_POPULATE)) {
173+
$normalizedData = $this->prepareForDenormalization($data);
174+
$class = $this->getClassDiscriminatorResolvedClass($normalizedData, $class);
175+
}
176+
$resourceClass = $this->resourceClassResolver->getResourceClass($objectToPopulate, $class);
173177
$context['api_denormalize'] = true;
174178
$context['resource_class'] = $resourceClass;
175179

@@ -223,8 +227,7 @@ public function denormalize($data, $class, $format = null, array $context = [])
223227
}
224228

225229
/**
226-
* Method copy-pasted from symfony/serializer.
227-
* Remove it after symfony/serializer version update @link https://github.com/symfony/symfony/pull/28263.
230+
* Originally from {@see https://github.com/symfony/symfony/pull/28263}. Refactor after it is merged.
228231
*
229232
* {@inheritdoc}
230233
*
@@ -238,19 +241,8 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
238241
return $object;
239242
}
240243

241-
if ($this->classDiscriminatorResolver && $mapping = $this->classDiscriminatorResolver->getMappingForClass($class)) {
242-
if (!isset($data[$mapping->getTypeProperty()])) {
243-
throw new RuntimeException(sprintf('Type property "%s" not found for the abstract object "%s"', $mapping->getTypeProperty(), $class));
244-
}
245-
246-
$type = $data[$mapping->getTypeProperty()];
247-
if (null === ($mappedClass = $mapping->getClassForType($type))) {
248-
throw new RuntimeException(sprintf('The type "%s" has no mapped class for the abstract object "%s"', $type, $class));
249-
}
250-
251-
$class = $mappedClass;
252-
$reflectionClass = new \ReflectionClass($class);
253-
}
244+
$class = $this->getClassDiscriminatorResolvedClass($data, $class);
245+
$reflectionClass = new \ReflectionClass($class);
254246

255247
$constructor = $this->getConstructor($data, $class, $context, $reflectionClass, $allowedAttributes);
256248
if ($constructor) {
@@ -295,6 +287,24 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
295287
return new $class();
296288
}
297289

290+
protected function getClassDiscriminatorResolvedClass(array &$data, string $class): string
291+
{
292+
if (null === $this->classDiscriminatorResolver || (null === $mapping = $this->classDiscriminatorResolver->getMappingForClass($class))) {
293+
return $class;
294+
}
295+
296+
if (!isset($data[$mapping->getTypeProperty()])) {
297+
throw new RuntimeException(sprintf('Type property "%s" not found for the abstract object "%s"', $mapping->getTypeProperty(), $class));
298+
}
299+
300+
$type = $data[$mapping->getTypeProperty()];
301+
if (null === ($mappedClass = $mapping->getClassForType($type))) {
302+
throw new RuntimeException(sprintf('The type "%s" has no mapped class for the abstract object "%s"', $type, $class));
303+
}
304+
305+
return $mappedClass;
306+
}
307+
298308
/**
299309
* {@inheritdoc}
300310
*/
@@ -494,7 +504,6 @@ protected function createRelationSerializationContext(string $resourceClass, arr
494504
/**
495505
* {@inheritdoc}
496506
*
497-
* @throws NoSuchPropertyException
498507
* @throws UnexpectedValueException
499508
* @throws LogicException
500509
*/
@@ -503,6 +512,7 @@ protected function getAttributeValue($object, $attribute, $format = null, array
503512
$context['api_attribute'] = $attribute;
504513
$propertyMetadata = $this->propertyMetadataFactory->create($context['resource_class'], $attribute, $this->getFactoryOptions($context));
505514

515+
// BC to be removed in 3.0
506516
try {
507517
$attributeValue = $this->propertyAccessor->getValue($object, $attribute);
508518
} catch (NoSuchPropertyException $e) {

‎tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -922,12 +922,10 @@ private function getPartialContainerBuilderProphecy($configuration = null)
922922
'api_platform.listener.view.write',
923923
'api_platform.metadata.extractor.xml',
924924
'api_platform.metadata.property.metadata_factory.cached',
925-
'api_platform.metadata.property.metadata_factory.inherited',
926925
'api_platform.metadata.property.metadata_factory.property_info',
927926
'api_platform.metadata.property.metadata_factory.serializer',
928927
'api_platform.metadata.property.metadata_factory.xml',
929928
'api_platform.metadata.property.name_collection_factory.cached',
930-
'api_platform.metadata.property.name_collection_factory.inherited',
931929
'api_platform.metadata.property.name_collection_factory.property_info',
932930
'api_platform.metadata.property.name_collection_factory.xml',
933931
'api_platform.metadata.resource.metadata_factory.cached',

‎tests/Fixtures/TestBundle/Resources/config/serialization/abstract_dummy.yml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\AbstractDummy:
2-
discriminator_map:
3-
type_property: discr
4-
mapping:
5-
concrete: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ConcreteDummy'
2+
discriminator_map:
3+
type_property: discr
4+
mapping:
5+
concrete: ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ConcreteDummy
66

77
ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\AbstractDummy:
8-
discriminator_map:
9-
type_property: discr
10-
mapping:
11-
concrete: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ConcreteDummy'
8+
discriminator_map:
9+
type_property: discr
10+
mapping:
11+
concrete: ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ConcreteDummy

0 commit comments

Comments
 (0)