fix(serializer): fix denormalizing to non-cloneable objects (#5569)

* test(serializer): regression test for deserializing non-cloneable objects

* fix(serializer): fix error denormalizing to non-cloneable objects

Author: colinodell

src/Serializer/AbstractItemNormalizer.php

@@ -33,6 +33,7 @@
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
 use ApiPlatform\Symfony\Security\ResourceAccessCheckerInterface;
 use ApiPlatform\Util\ClassInfoTrait;
+use ApiPlatform\Util\CloneTrait;
 use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
@@ -59,6 +60,7 @@
 abstract class AbstractItemNormalizer extends AbstractObjectNormalizer
 {
     use ClassInfoTrait;
+    use CloneTrait;
     use ContextTrait;
     use InputOutputMetadataTrait;
 
@@ -360,13 +362,19 @@ public function denormalize($data, $class, $format = null, array $context = [])
             return $item;
         }
 
-        $previousObject = null !== $objectToPopulate ? clone $objectToPopulate : null;
+        $previousObject = $this->clone($objectToPopulate);
         $object = parent::denormalize($data, $resourceClass, $format, $context);
 
         if (!$this->resourceClassResolver->isResourceClass($context['resource_class'])) {
             return $object;
         }
 
+        // Bypass the post-denormalize attribute revert logic if the object could not be
+        // cloned since we cannot possibly revert any changes made to it.
+        if (null !== $objectToPopulate && null === $previousObject) {
+            return $object;
+        }
+
         // Revert attributes that aren't allowed to be changed after a post-denormalize check
         foreach (array_keys($data) as $attribute) {
             if (!$this->canAccessAttributePostDenormalize($object, $previousObject, $attribute, $context)) {

tests/Fixtures/TestBundle/Entity/NonCloneableDummy.php

@@ -0,0 +1,77 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
+
+use ApiPlatform\Core\Annotation\ApiProperty;
+use ApiPlatform\Core\Annotation\ApiResource;
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Validator\Constraints as Assert;
+
+/**
+ * Dummy class that cannot be cloned.
+ *
+ * @author Colin O'Dell <[email protected]>
+ *
+ * @ApiResource
+ *
+ * @ORM\Entity
+ */
+class NonCloneableDummy
+{
+    /**
+     * @var int|null The id
+     *
+     * @ORM\Column(type="integer", nullable=true)
+     *
+     * @ORM\Id
+     *
+     * @ORM\GeneratedValue(strategy="AUTO")
+     */
+    private $id;
+
+    /**
+     * @var string The dummy name
+     *
+     * @ORM\Column
+     *
+     * @Assert\NotBlank
+     *
+     * @ApiProperty(iri="http://schema.org/name")
+     */
+    private $name;
+
+    public function getId()
+    {
+        return $this->id;
+    }
+
+    public function setId($id)
+    {
+        $this->id = $id;
+    }
+
+    public function setName(string $name)
+    {
+        $this->name = $name;
+    }
+
+    public function getName(): string
+    {
+        return $this->name;
+    }
+
+    private function __clone()
+    {
+    }
+}

tests/Serializer/AbstractItemNormalizerTest.php

@@ -41,6 +41,7 @@
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyForAdditionalFieldsInput;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTableInheritance;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceChild;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\NonCloneableDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\SecuredDummy;
 use Doctrine\Common\Collections\ArrayCollection;
@@ -1702,6 +1703,56 @@ public function testDenormalizeCollectionDecodedFromXmlWithOneChild()
 
         $normalizer->denormalize($data, Dummy::class, 'xml');
     }
+
+    public function testDenormalizePopulatingNonCloneableObject()
+    {
+        $dummy = new NonCloneableDummy();
+        $dummy->setName('foo');
+
+        $data = [
+            'name' => 'bar',
+        ];
+
+        $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
+        $propertyNameCollectionFactoryProphecy->create(NonCloneableDummy::class, [])->willReturn(new PropertyNameCollection(['name']));
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(NonCloneableDummy::class, 'name', [])->willReturn((new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withDescription('')->withReadable(false)->withWritable(true));
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class);
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->getResourceClass(null, NonCloneableDummy::class)->willReturn(NonCloneableDummy::class);
+        $resourceClassResolverProphecy->getResourceClass($dummy, NonCloneableDummy::class)->willReturn(NonCloneableDummy::class);
+        $resourceClassResolverProphecy->isResourceClass(NonCloneableDummy::class)->willReturn(true);
+
+        $serializerProphecy = $this->prophesize(SerializerInterface::class);
+        $serializerProphecy->willImplement(NormalizerInterface::class);
+
+        $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [
+            $propertyNameCollectionFactoryProphecy->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $iriConverterProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            $propertyAccessorProphecy->reveal(),
+            null,
+            null,
+            null,
+            false,
+            [],
+            [],
+            null,
+            null,
+        ]);
+        $normalizer->setSerializer($serializerProphecy->reveal());
+
+        $context = [AbstractItemNormalizer::OBJECT_TO_POPULATE => $dummy];
+        $actual = $normalizer->denormalize($data, NonCloneableDummy::class, null, $context);
+
+        $this->assertInstanceOf(NonCloneableDummy::class, $actual);
+        $this->assertSame($dummy, $actual);
+        $propertyAccessorProphecy->setValue($actual, 'name', 'bar')->shouldHaveBeenCalled();
+    }
 }
 
 class ObjectWithBasicProperties