Merge pull request #3273 from teohhanhui/fix/remove-child-inherited-m…

…etadata

Remove nonsensical "child inherited" metadata

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

@@ -49,11 +49,6 @@
             <argument type="service" id="api_platform.property_info" />
         </service>
 
-        <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">
-            <argument type="service" id="api_platform.metadata.resource.name_collection_factory" />
-            <argument type="service" id="api_platform.metadata.property.name_collection_factory.inherited.inner" />
-        </service>
-
         <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">
             <argument type="service" id="api_platform.cache.metadata.property" />
             <argument type="service" id="api_platform.metadata.property.name_collection_factory.cached.inner" />
@@ -67,11 +62,6 @@
             <argument type="service" id="api_platform.metadata.property.metadata_factory.property_info.inner" />
         </service>
 
-        <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">
-            <argument type="service" id="api_platform.metadata.resource.name_collection_factory" />
-            <argument type="service" id="api_platform.metadata.property.metadata_factory.inherited.inner" />
-        </service>
-
         <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">
             <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
             <argument type="service" id="serializer.mapping.class_metadata_factory" />

src/Metadata/Property/Factory/AnnotationPropertyNameCollectionFactory.php

@@ -48,7 +48,7 @@ public function create(string $resourceClass, array $options = []): PropertyName
             try {
                 $propertyNameCollection = $this->decorated->create($resourceClass, $options);
             } catch (ResourceClassNotFoundException $resourceClassNotFoundException) {
-                // Ignore not found exceptions from parent
+                // Ignore not found exceptions from decorated factory
             }
         }
 
@@ -87,7 +87,7 @@ public function create(string $resourceClass, array $options = []): PropertyName
             }
         }
 
-        // Inherited from parent
+        // add property names from decorated factory
         if (null !== $propertyNameCollection) {
             foreach ($propertyNameCollection as $propertyName) {
                 $propertyNames[$propertyName] = $propertyName;

src/Metadata/Property/Factory/ExtractorPropertyNameCollectionFactory.php

@@ -49,7 +49,7 @@ public function create(string $resourceClass, array $options = []): PropertyName
             try {
                 $propertyNameCollection = $this->decorated->create($resourceClass, $options);
             } catch (ResourceClassNotFoundException $resourceClassNotFoundException) {
-                // Ignore not found exceptions from parent
+                // Ignore not found exceptions from decorated factory
             }
 
             foreach ($propertyNameCollection as $propertyName) {

src/Metadata/Property/Factory/InheritedPropertyMetadataFactory.php

@@ -17,9 +17,7 @@
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
 
 /**
- * Get property metadata from eventual child inherited properties.
- *
- * @author Antoine Bluchet <soyuka@gmail.com>
+ * @deprecated since 2.6, to be removed in 3.0
  */
 final class InheritedPropertyMetadataFactory implements PropertyMetadataFactoryInterface
 {
@@ -28,6 +26,8 @@ final class InheritedPropertyMetadataFactory implements PropertyMetadataFactoryI
 
     public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, PropertyMetadataFactoryInterface $decorated = null)
     {
+        @trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
+
         $this->resourceNameCollectionFactory = $resourceNameCollectionFactory;
         $this->decorated = $decorated;
     }
@@ -37,6 +37,8 @@ public function __construct(ResourceNameCollectionFactoryInterface $resourceName
      */
     public function create(string $resourceClass, string $property, array $options = []): PropertyMetadata
     {
+        @trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
+
         $propertyMetadata = $this->decorated ? $this->decorated->create($resourceClass, $property, $options) : new PropertyMetadata();
 
         foreach ($this->resourceNameCollectionFactory->create() as $knownResourceClass) {

src/Metadata/Property/Factory/InheritedPropertyNameCollectionFactory.php

@@ -17,9 +17,7 @@
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
 
 /**
- * Creates a property name collection from eventual child inherited properties.
- *
- * @author Antoine Bluchet <soyuka@gmail.com>
+ * @deprecated since 2.6, to be removed in 3.0
  */
 final class InheritedPropertyNameCollectionFactory implements PropertyNameCollectionFactoryInterface
 {
@@ -28,6 +26,8 @@ final class InheritedPropertyNameCollectionFactory implements PropertyNameCollec
 
     public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, PropertyNameCollectionFactoryInterface $decorated = null)
     {
+        @trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
+
         $this->resourceNameCollectionFactory = $resourceNameCollectionFactory;
         $this->decorated = $decorated;
     }
@@ -37,6 +37,8 @@ public function __construct(ResourceNameCollectionFactoryInterface $resourceName
      */
     public function create(string $resourceClass, array $options = []): PropertyNameCollection
     {
+        @trigger_error(sprintf('"%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__), E_USER_DEPRECATED);
+
         $propertyNames = [];
 
         // Inherited from parent

src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php

@@ -48,8 +48,7 @@ public function create(string $resourceClass, string $property, array $options =
     {
         $propertyMetadata = $this->decorated->create($resourceClass, $property, $options);
 
-        // in case of a property inherited (in a child class), we need it's properties
-        // to be mapped against serialization groups instead of the parent ones.
+        // BC to be removed in 3.0
         if (null !== ($childResourceClass = $propertyMetadata->getChildInherited())) {
             $resourceClass = $childResourceClass;
         }

src/Metadata/Property/PropertyMetadata.php

@@ -31,6 +31,9 @@ final class PropertyMetadata
     private $required;
     private $iri;
     private $identifier;
+    /**
+     * @deprecated since 2.6, to be removed in 3.0
+     */
     private $childInherited;
     private $attributes;
     private $subresource;
@@ -47,6 +50,9 @@ public function __construct(Type $type = null, string $description = null, bool
         $this->required = $required;
         $this->identifier = $identifier;
         $this->iri = $iri;
+        if (null !== $childInherited) {
+            @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);
+        }
         $this->childInherited = $childInherited;
         $this->attributes = $attributes;
         $this->subresource = $subresource;
@@ -258,38 +264,38 @@ public function withAttributes(array $attributes): self
     }
 
     /**
-     * Gets child inherited.
+     * @deprecated since 2.6, to be removed in 3.0
      */
     public function getChildInherited(): ?string
     {
         return $this->childInherited;
     }
 
     /**
-     * Is the property inherited from a child class?
+     * @deprecated since 2.6, to be removed in 3.0
      */
     public function hasChildInherited(): bool
     {
         return null !== $this->childInherited;
     }
 
     /**
-     * Is the property inherited from a child class?
-     *
-     * @deprecated since version 2.4, to be removed in 3.0.
+     * @deprecated since 2.4, to be removed in 3.0
      */
     public function isChildInherited(): ?string
     {
-        @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);
+        @trigger_error(sprintf('"%s::%s" is deprecated since 2.4 and will be removed in 3.0.', __CLASS__, __METHOD__), E_USER_DEPRECATED);
 
         return $this->getChildInherited();
     }
 
     /**
-     * Returns a new instance with the given child inherited class.
+     * @deprecated since 2.6, to be removed in 3.0
      */
     public function withChildInherited(string $childInherited): self
     {
+        @trigger_error(sprintf('"%s::%s" is deprecated since 2.6 and will be removed in 3.0.', __CLASS__, __METHOD__), E_USER_DEPRECATED);
+
         $metadata = clone $this;
         $metadata->childInherited = $childInherited;
 

src/Serializer/AbstractItemNormalizer.php

@@ -169,7 +169,11 @@ public function supportsDenormalization($data, $type, $format = null)
      */
     public function denormalize($data, $class, $format = null, array $context = [])
     {
-        $resourceClass = $this->resourceClassResolver->getResourceClass(null, $class);
+        if (null === $objectToPopulate = $this->extractObjectToPopulate($class, $context, static::OBJECT_TO_POPULATE)) {
+            $normalizedData = $this->prepareForDenormalization($data);
+            $class = $this->getClassDiscriminatorResolvedClass($normalizedData, $class);
+        }
+        $resourceClass = $this->resourceClassResolver->getResourceClass($objectToPopulate, $class);
         $context['api_denormalize'] = true;
         $context['resource_class'] = $resourceClass;
 
@@ -223,8 +227,7 @@ public function denormalize($data, $class, $format = null, array $context = [])
     }
 
     /**
-     * Method copy-pasted from symfony/serializer.
-     * Remove it after symfony/serializer version update @link https://github.com/symfony/symfony/pull/28263.
+     * Originally from {@see https://github.com/symfony/symfony/pull/28263}. Refactor after it is merged.
      *
      * {@inheritdoc}
      *
@@ -238,19 +241,8 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
             return $object;
         }
 
-        if ($this->classDiscriminatorResolver && $mapping = $this->classDiscriminatorResolver->getMappingForClass($class)) {
-            if (!isset($data[$mapping->getTypeProperty()])) {
-                throw new RuntimeException(sprintf('Type property "%s" not found for the abstract object "%s"', $mapping->getTypeProperty(), $class));
-            }
-
-            $type = $data[$mapping->getTypeProperty()];
-            if (null === ($mappedClass = $mapping->getClassForType($type))) {
-                throw new RuntimeException(sprintf('The type "%s" has no mapped class for the abstract object "%s"', $type, $class));
-            }
-
-            $class = $mappedClass;
-            $reflectionClass = new \ReflectionClass($class);
-        }
+        $class = $this->getClassDiscriminatorResolvedClass($data, $class);
+        $reflectionClass = new \ReflectionClass($class);
 
         $constructor = $this->getConstructor($data, $class, $context, $reflectionClass, $allowedAttributes);
         if ($constructor) {
@@ -295,6 +287,24 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
         return new $class();
     }
 
+    protected function getClassDiscriminatorResolvedClass(array &$data, string $class): string
+    {
+        if (null === $this->classDiscriminatorResolver || (null === $mapping = $this->classDiscriminatorResolver->getMappingForClass($class))) {
+            return $class;
+        }
+
+        if (!isset($data[$mapping->getTypeProperty()])) {
+            throw new RuntimeException(sprintf('Type property "%s" not found for the abstract object "%s"', $mapping->getTypeProperty(), $class));
+        }
+
+        $type = $data[$mapping->getTypeProperty()];
+        if (null === ($mappedClass = $mapping->getClassForType($type))) {
+            throw new RuntimeException(sprintf('The type "%s" has no mapped class for the abstract object "%s"', $type, $class));
+        }
+
+        return $mappedClass;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -494,7 +504,6 @@ protected function createRelationSerializationContext(string $resourceClass, arr
     /**
      * {@inheritdoc}
      *
-     * @throws NoSuchPropertyException
      * @throws UnexpectedValueException
      * @throws LogicException
      */
@@ -503,6 +512,7 @@ protected function getAttributeValue($object, $attribute, $format = null, array
         $context['api_attribute'] = $attribute;
         $propertyMetadata = $this->propertyMetadataFactory->create($context['resource_class'], $attribute, $this->getFactoryOptions($context));
 
+        // BC to be removed in 3.0
         try {
             $attributeValue = $this->propertyAccessor->getValue($object, $attribute);
         } catch (NoSuchPropertyException $e) {

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

@@ -922,12 +922,10 @@ private function getPartialContainerBuilderProphecy($configuration = null)
             'api_platform.listener.view.write',
             'api_platform.metadata.extractor.xml',
             'api_platform.metadata.property.metadata_factory.cached',
-            'api_platform.metadata.property.metadata_factory.inherited',
             'api_platform.metadata.property.metadata_factory.property_info',
             'api_platform.metadata.property.metadata_factory.serializer',
             'api_platform.metadata.property.metadata_factory.xml',
             'api_platform.metadata.property.name_collection_factory.cached',
-            'api_platform.metadata.property.name_collection_factory.inherited',
             'api_platform.metadata.property.name_collection_factory.property_info',
             'api_platform.metadata.property.name_collection_factory.xml',
             'api_platform.metadata.resource.metadata_factory.cached',

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

@@ -1,11 +1,11 @@
 ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\AbstractDummy:
-  discriminator_map:
-    type_property: discr
-    mapping:
-      concrete: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ConcreteDummy'
+    discriminator_map:
+        type_property: discr
+        mapping:
+            concrete: ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ConcreteDummy
 
 ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\AbstractDummy:
-  discriminator_map:
-    type_property: discr
-    mapping:
-      concrete: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ConcreteDummy'
+    discriminator_map:
+        type_property: discr
+        mapping:
+            concrete: ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ConcreteDummy

tests/JsonApi/Serializer/ItemNormalizerTest.php

@@ -95,7 +95,72 @@ public function testSupportNormalization()
         $this->assertFalse($normalizer->supportsNormalization($std, ItemNormalizer::FORMAT));
     }
 
-    public function testNormalize()
+    public function testNormalize(): void
+    {
+        $dummy = new Dummy();
+        $dummy->setId(10);
+        $dummy->setName('hello');
+
+        $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
+        $propertyNameCollectionFactoryProphecy->create(Dummy::class, [])->willReturn(new PropertyNameCollection(['id', 'name', '\bad_property']));
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn(new PropertyMetadata(null, null, true));
+        $propertyMetadataFactoryProphecy->create(Dummy::class, 'id', [])->willReturn(new PropertyMetadata(null, null, true, null, null, null, null, true));
+        $propertyMetadataFactoryProphecy->create(Dummy::class, '\bad_property', [])->willReturn(new PropertyMetadata(null, null, true));
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/10');
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->getResourceClass($dummy, null)->willReturn(Dummy::class);
+        $resourceClassResolverProphecy->getResourceClass($dummy, Dummy::class)->willReturn(Dummy::class);
+
+        $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class);
+        $propertyAccessorProphecy->getValue($dummy, 'id')->willReturn(10);
+        $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('hello');
+
+        $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
+        $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata('Dummy', 'A dummy', '/dummy', null, null, ['id', 'name']));
+
+        $serializerProphecy = $this->prophesize(SerializerInterface::class);
+        $serializerProphecy->willImplement(NormalizerInterface::class);
+        $serializerProphecy->normalize('hello', ItemNormalizer::FORMAT, Argument::type('array'))->willReturn('hello');
+        $serializerProphecy->normalize(10, ItemNormalizer::FORMAT, Argument::type('array'))->willReturn(10);
+        $serializerProphecy->normalize(null, ItemNormalizer::FORMAT, Argument::type('array'))->willReturn(null);
+
+        $normalizer = new ItemNormalizer(
+            $propertyNameCollectionFactoryProphecy->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $iriConverterProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            $propertyAccessorProphecy->reveal(),
+            new ReservedAttributeNameConverter(),
+            $resourceMetadataFactoryProphecy->reveal(),
+            [],
+            []
+        );
+
+        $normalizer->setSerializer($serializerProphecy->reveal());
+
+        $expected = [
+            'data' => [
+                'type' => 'Dummy',
+                'id' => '/dummies/10',
+                'attributes' => [
+                    '_id' => 10,
+                    'name' => 'hello',
+                ],
+            ],
+        ];
+
+        $this->assertEquals($expected, $normalizer->normalize($dummy, ItemNormalizer::FORMAT));
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testNormalizeChildInheritedProperty(): void
     {
         $dummy = new Dummy();
         $dummy->setId(10);

tests/Metadata/Property/Factory/InheritedPropertyMetadataFactoryTest.php

@@ -24,7 +24,7 @@
 use Symfony\Component\PropertyInfo\Type;
 
 /**
- * @author Antoine Bluchet <soyuka@gmail.com>
+ * @group legacy
  */
 class InheritedPropertyMetadataFactoryTest extends TestCase
 {

tests/Metadata/Property/Factory/InheritedPropertyNameCollectionFactoryTest.php

@@ -23,7 +23,7 @@
 use PHPUnit\Framework\TestCase;
 
 /**
- * @author Antoine Bluchet <soyuka@gmail.com>
+ * @group legacy
  */
 class InheritedPropertyNameCollectionFactoryTest extends TestCase
 {