bug symfony#61310 [ObjectMapper] read source metadata before transform (soyuka, rvanlaak)

This PR was merged into the 7.3 branch.

Discussion
----------

[ObjectMapper] read source metadata before transform

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | fixes symfony#61027
| License       | MIT

Commits
-------

c1e4adf [ObjectMapper] do not require mapping a target's required promoted property when not on source (symfony#2)
50e177d [ObjectMapper] read source metadata before transform

Author: nicolas-grekas

src/Symfony/Component/ObjectMapper/ObjectMapper.php

@@ -72,6 +72,12 @@ public function map(object $source, object|string|null $target = null): object
         }
 
         $mappedTarget = $mappingToObject ? $target : $targetRefl->newInstanceWithoutConstructor();
+
+        if (!$metadata && $targetMetadata = $this->metadataFactory->create($mappedTarget)) {
+            $metadata = $targetMetadata;
+            $map = $this->getMapTarget($metadata, null, $source, null);
+        }
+
         if ($map && $map->transform) {
             $mappedTarget = $this->applyTransforms($map, $mappedTarget, $source, null);
 
@@ -104,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) {
@@ -164,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;
                 }
             }
@@ -308,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 {
@@ -337,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/Fixtures/TargetTransform/SourceEntity.php

@@ -0,0 +1,17 @@
+<?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\TargetTransform;
+
+class SourceEntity
+{
+    public string $name;
+}

src/Symfony/Component/ObjectMapper/Tests/Fixtures/TargetTransform/TargetDto.php

@@ -0,0 +1,29 @@
+<?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\TargetTransform;
+
+use Symfony\Component\ObjectMapper\Attribute\Map;
+
+#[Map(source: SourceEntity::class, transform: [self::class, 't'])]
+class TargetDto
+{
+    #[Map(if: false)]
+    public bool $transformed;
+    public string $name;
+
+    public static function t(mixed $value, object $source, ?object $target)
+    {
+        $value->transformed = true;
+
+        return $value;
+    }
+}

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

@@ -58,12 +58,16 @@
 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;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\ServiceLocator\B as ServiceLocatorB;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\ServiceLocator\ConditionCallable;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\ServiceLocator\TransformCallable;
+use Symfony\Component\ObjectMapper\Tests\Fixtures\TargetTransform\SourceEntity;
+use Symfony\Component\ObjectMapper\Tests\Fixtures\TargetTransform\TargetDto as TargetTransformTargetDto;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 
 final class ObjectMapperTest extends TestCase
@@ -364,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}>
      */
@@ -447,4 +465,17 @@ public static function validPartialInputProvider(): iterable
 
         yield [$p, $f];
     }
+
+    public function testMapWithSourceTransform()
+    {
+        $source = new SourceEntity();
+        $source->name = 'test';
+
+        $mapper = new ObjectMapper();
+        $target = $mapper->map($source, TargetTransformTargetDto::class);
+
+        $this->assertInstanceOf(TargetTransformTargetDto::class, $target);
+        $this->assertTrue($target->transformed);
+        $this->assertSame('test', $target->name);
+    }
 }