[ObjectMapper] Add support for mapping nested objects #62377
Conversation
|
Hey! To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done? Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review. Cheers! Carsonbot |
ad52d38 to
3b71652
Compare
|
I don't understand, we support nested object mapping from the beginning https://github.com/symfony/symfony/blob/7.4/src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php#L105-L113 |
| } | ||
|
|
||
| $nestedValue = $this->getSourceValue($value, $mappedTarget, $nestedValue, $objectMap, $nestedMapping); | ||
| $this->storeValue($nestedTargetPropertyName, $mapToProperties, $ctorArguments, $nestedValue); |
There was a problem hiding this comment.
we should not do this, it's already happening at https://github.com/symfony/symfony/blob/7.4/src/Symfony/Component/ObjectMapper/ObjectMapper.php#L231-L261 and in a recursive manner.
3b71652 to
3794a99
Compare
|
Wait but I think I misunderstood here's how I'd do this: https://github.com/soyuka/symfony/pull/new/feat/object-mapper-nested-objects |
Much more elegant. We‘ll test this tomorrow and provide feedback. |
|
@soyuka Your solution works as expected. Only complain I'd have is, that it is a bit cryptic. A note in the class-level docblock could be helpful to understand the purpose of the class. |
|
@daFish not sure that we should contribute this, maybe an entry in the docs would be sufficient? |
|
@soyuka This use case should work out of the box IHMO making it a valuable addition. |
|
I'm also of the opinion that this use case should work out-of-the-box. Having nested objects isn't such an uncommon thing. |
|
Okay I've a lot of work these days so I won't be able to do this soon, do you think we should propose the transform or instead we should discover automatically that the property must be read on the embeded object? (btw feel free to take my suggestion and update your PR :)) |
|
I'd vote for the latter, discover automatically. But transform is fine, as well, as long as it works without implementing your own workarounds... |
|
@soyuka I am developing a new project at work and ran across some similar issues. However, in our case, we had nested object maps working before commit 1e22b511d1a219099c89d39c2c4d6ce340566cf7. Using the <?php
declare(strict_types=1);
use Symfony\Component\ObjectMapper\Attribute\Map;
use Symfony\Component\ObjectMapper\ObjectMapper;
require __DIR__ . '/vendor/autoload.php';
final readonly class BMapped {
public function __construct(
public string $var2,
) {}
}
final readonly class AMapped {
public function __construct(
public BMapped $b,
public string $var1,
) {}
}
#[Map(target: BMapped::class)]
final readonly class B {
public function __construct(
public string $var2,
) {}
}
#[Map(target: AMapped::class)]
final readonly class A {
public function __construct(
public B $b,
public string $var1,
) {}
}
/** @var AMapped $out */
$out = new ObjectMapper()->map(
source: new A(
b: new B(
var2: 'bar',
),
var1: 'foo',
),
);
var_dump($out);
echo $out->b->var2;Output before commit Output after commit Output after commit |
|
Hey guys. Maybe I'm blind or this request here is just for mapping nested objects of the source to flat properties of a target. But I have the reversed case and I can't find a way how to do that. I hoped to be able to do something like a dot notation. And I also can't figure out how to make it work with a custom transformer. I don't want to implement a custom mapping strategy as this will overcomplicate things in our code. Any idea? Example: |
|
@meinemitternacht can you please open a new issue? @siggidiel could you open a new issue ? |
Done: #62439 |
|
This feature adds support to map nested objects as reported in #62357. As reported by @temp, this does not work with the current component.