[ObjectMapper] do not require mapping a target's required promoted property when not on source (symfony#2)

Author: rvanlaak

src/Symfony/Component/ObjectMapper/ObjectMapper.php

@@ -73,7 +73,7 @@ public function map(object $source, object|string|null $target = null): object
 
         $mappedTarget = $mappingToObject ? $target : $targetRefl->newInstanceWithoutConstructor();
 
-        if (!$metadata && ($targetMetadata = $this->metadataFactory->create($mappedTarget))) {
+        if (!$metadata && $targetMetadata = $this->metadataFactory->create($mappedTarget)) {
             $metadata = $targetMetadata;
             $map = $this->getMapTarget($metadata, null, $source, null);
         }
@@ -110,7 +110,7 @@ public function map(object $source, object|string|null $target = null): object
         }
 
         $readMetadataFrom = $source;
-        $refl = $this->getSourceReflectionClass($source, $targetRefl);
+        $refl = $this->getSourceReflectionClass($source) ?? $targetRefl;
 
         // When source contains no metadata, we read metadata on the target instead
         if ($refl === $targetRefl) {
@@ -170,7 +170,7 @@ public function map(object $source, object|string|null $target = null): object
 
         if ($mappingToObject && $ctorArguments) {
             foreach ($ctorArguments as $property => $value) {
-                if ($targetRefl->hasProperty($property) && $targetRefl->getProperty($property)->isPublic()) {
+                if ($this->propertyIsMappable($refl, $property) && $this->propertyIsMappable($targetRefl, $property)) {
                     $mapToProperties[$property] = $value;
                 }
             }
@@ -314,11 +314,9 @@ private function getCallable(string|callable $fn, ?ContainerInterface $locator =
     }
 
     /**
-     * @param \ReflectionClass<object> $targetRefl
-     *
-     * @return \ReflectionClass<object|T>
+     * @return ?\ReflectionClass<object|T>
      */
-    private function getSourceReflectionClass(object $source, \ReflectionClass $targetRefl): \ReflectionClass
+    private function getSourceReflectionClass(object $source): ?\ReflectionClass
     {
         $metadata = $this->metadataFactory->create($source);
         try {
@@ -343,6 +341,11 @@ private function getSourceReflectionClass(object $source, \ReflectionClass $targ
             }
         }
 
-        return $targetRefl;
+        return null;
+    }
+
+    private function propertyIsMappable(\ReflectionClass $targetRefl, int|string $property): bool
+    {
+        return $targetRefl->hasProperty($property) && $targetRefl->getProperty($property)->isPublic();
     }
 }

src/Symfony/Component/ObjectMapper/Tests/Fixtures/PromotedConstructorWithMetadata/Source.php

@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata;
+
+use Symfony\Component\ObjectMapper\Tests\Fixtures\MapStruct\Map;
+
+#[Map(target: Target::class)]
+class Source
+{
+    public function __construct(
+        public int    $number,
+        public string $name,
+    ) {
+    }
+}

src/Symfony/Component/ObjectMapper/Tests/Fixtures/PromotedConstructorWithMetadata/Target.php

@@ -0,0 +1,26 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata;
+
+class Target
+{
+    public function __construct(
+        /**
+         * This promoted property is required but should not lead to an exception on the object mapping as instantiation
+         * happened earlier already.
+         */
+        public string $notOnSourceButRequired,
+        public int    $number,
+        public string $name,
+    ) {
+    }
+}

src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php

@@ -58,6 +58,8 @@
 use Symfony\Component\ObjectMapper\Tests\Fixtures\PartialInput\PartialInput;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructor\Source as PromotedConstructorSource;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructor\Target as PromotedConstructorTarget;
+use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata\Source as PromotedConstructorWithMetadataSource;
+use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata\Target as PromotedConstructorWithMetadataTarget;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\Recursion\AB;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\Recursion\Dto;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\ServiceLocator\A as ServiceLocatorA;
@@ -366,6 +368,20 @@ public function testUpdateObjectWithConstructorPromotedProperties(ObjectMapperIn
         $this->assertSame($v->name, 'foo');
     }
 
+    /**
+     * @dataProvider objectMapperProvider
+     */
+    public function testUpdateMappedObjectWithAdditionalConstructorPromotedProperties(ObjectMapperInterface $mapper)
+    {
+        $a = new PromotedConstructorWithMetadataSource(3, 'foo-will-get-updated');
+        $b = new PromotedConstructorWithMetadataTarget('notOnSourceButRequired', 1, 'bar');
+
+        $v = $mapper->map($a, $b);
+
+        $this->assertSame($v->name, $a->name);
+        $this->assertSame($v->number, $a->number);
+    }
+
     /**
      * @return iterable<array{0: ObjectMapperInterface}>
      */