Skip to content

Commit c1e4adf

Browse files
rvanlaaknicolas-grekas
rvanlaak
authored and
nicolas-grekas
committed
[ObjectMapper] do not require mapping a target's required promoted property when not on source (#2)
1 parent 50e177d commit c1e4adf

File tree

4 files changed

+77
-8
lines changed

4 files changed

+77
-8
lines changed

‎src/Symfony/Component/ObjectMapper/ObjectMapper.php

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function map(object $source, object|string|null $target = null): object
7373

7474
$mappedTarget = $mappingToObject ? $target : $targetRefl->newInstanceWithoutConstructor();
7575

76-
if (!$metadata && ($targetMetadata = $this->metadataFactory->create($mappedTarget))) {
76+
if (!$metadata && $targetMetadata = $this->metadataFactory->create($mappedTarget)) {
7777
$metadata = $targetMetadata;
7878
$map = $this->getMapTarget($metadata, null, $source, null);
7979
}
@@ -110,7 +110,7 @@ public function map(object $source, object|string|null $target = null): object
110110
}
111111

112112
$readMetadataFrom = $source;
113-
$refl = $this->getSourceReflectionClass($source, $targetRefl);
113+
$refl = $this->getSourceReflectionClass($source) ?? $targetRefl;
114114

115115
// When source contains no metadata, we read metadata on the target instead
116116
if ($refl === $targetRefl) {
@@ -170,7 +170,7 @@ public function map(object $source, object|string|null $target = null): object
170170

171171
if ($mappingToObject && $ctorArguments) {
172172
foreach ($ctorArguments as $property => $value) {
173-
if ($targetRefl->hasProperty($property) && $targetRefl->getProperty($property)->isPublic()) {
173+
if ($this->propertyIsMappable($refl, $property) && $this->propertyIsMappable($targetRefl, $property)) {
174174
$mapToProperties[$property] = $value;
175175
}
176176
}
@@ -314,11 +314,9 @@ private function getCallable(string|callable $fn, ?ContainerInterface $locator =
314314
}
315315

316316
/**
317-
* @param \ReflectionClass<object> $targetRefl
318-
*
319-
* @return \ReflectionClass<object|T>
317+
* @return ?\ReflectionClass<object|T>
320318
*/
321-
private function getSourceReflectionClass(object $source, \ReflectionClass $targetRefl): \ReflectionClass
319+
private function getSourceReflectionClass(object $source): ?\ReflectionClass
322320
{
323321
$metadata = $this->metadataFactory->create($source);
324322
try {
@@ -343,6 +341,11 @@ private function getSourceReflectionClass(object $source, \ReflectionClass $targ
343341
}
344342
}
345343

346-
return $targetRefl;
344+
return null;
345+
}
346+
347+
private function propertyIsMappable(\ReflectionClass $targetRefl, int|string $property): bool
348+
{
349+
return $targetRefl->hasProperty($property) && $targetRefl->getProperty($property)->isPublic();
347350
}
348351
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata;
13+
14+
use Symfony\Component\ObjectMapper\Tests\Fixtures\MapStruct\Map;
15+
16+
#[Map(target: Target::class)]
17+
class Source
18+
{
19+
public function __construct(
20+
public int $number,
21+
public string $name,
22+
) {
23+
}
24+
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata;
13+
14+
class Target
15+
{
16+
public function __construct(
17+
/**
18+
* This promoted property is required but should not lead to an exception on the object mapping as instantiation
19+
* happened earlier already.
20+
*/
21+
public string $notOnSourceButRequired,
22+
public int $number,
23+
public string $name,
24+
) {
25+
}
26+
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@
5858
use Symfony\Component\ObjectMapper\Tests\Fixtures\PartialInput\PartialInput;
5959
use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructor\Source as PromotedConstructorSource;
6060
use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructor\Target as PromotedConstructorTarget;
61+
use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata\Source as PromotedConstructorWithMetadataSource;
62+
use Symfony\Component\ObjectMapper\Tests\Fixtures\PromotedConstructorWithMetadata\Target as PromotedConstructorWithMetadataTarget;
6163
use Symfony\Component\ObjectMapper\Tests\Fixtures\Recursion\AB;
6264
use Symfony\Component\ObjectMapper\Tests\Fixtures\Recursion\Dto;
6365
use Symfony\Component\ObjectMapper\Tests\Fixtures\ServiceLocator\A as ServiceLocatorA;
@@ -366,6 +368,20 @@ public function testUpdateObjectWithConstructorPromotedProperties(ObjectMapperIn
366368
$this->assertSame($v->name, 'foo');
367369
}
368370

371+
/**
372+
* @dataProvider objectMapperProvider
373+
*/
374+
public function testUpdateMappedObjectWithAdditionalConstructorPromotedProperties(ObjectMapperInterface $mapper)
375+
{
376+
$a = new PromotedConstructorWithMetadataSource(3, 'foo-will-get-updated');
377+
$b = new PromotedConstructorWithMetadataTarget('notOnSourceButRequired', 1, 'bar');
378+
379+
$v = $mapper->map($a, $b);
380+
381+
$this->assertSame($v->name, $a->name);
382+
$this->assertSame($v->number, $a->number);
383+
}
384+
369385
/**
370386
* @return iterable<array{0: ObjectMapperInterface}>
371387
*/

0 commit comments

Comments
 (0)