[ObjectMapper] Add component#20347
Conversation
javiereguiluz
left a comment
There was a problem hiding this comment.
Antoine, thanks for contributing the docs for this new component.
I left some minor comments.
object-mapper.rst
Outdated
| #[Map(target: C::class, if: [Source::class, 'shouldMapToC'])] | ||
| class Source | ||
| { | ||
| public static function shouldMapToB(mixed $value, object $object): bool |
There was a problem hiding this comment.
Which are the mixed $value, object $object arguments passed to this method?
object-mapper.rst
Outdated
| use Symfony\Component\ObjectMapper\Attributes\Map; | ||
|
|
||
| class Source { | ||
| #[Map(target: 'fullName', transform: TransformNameCallable::class)] |
There was a problem hiding this comment.
Can I use a transform for the entire object instead of for each of its properties?
|
CI should be green after a rebase on |
OskarStark
left a comment
There was a problem hiding this comment.
A rebase on 7.3 branch would be helpful @soyuka, thanks
|
I've rebased, this also documents symfony/symfony#60028 as it's quite mandatory for many real world use cases. Interfaces are now matching the merged code. At the beginning I've used real word examples @94noni could you let me know your thoughts and if you think I should apply the same logic accross the whole documentation? |
de11d09 to
f4c8610
Compare
|
The new version needs symfony/symfony#60028 |
Spomky
left a comment
There was a problem hiding this comment.
The doc is already very clear.
Just a few small questions and remarks here and there.
Also, you could maybe add a short section on how to debug mappings.
But overall it looks good to me 👍🏼.
object-mapper.rst
Outdated
|
|
||
| Symfony provides a mapper component to transform a given object into another one, | ||
| simplifying tasks like converting DTOs (Data Transfer Objects) to entities or | ||
| vice-versa. |
There was a problem hiding this comment.
We could add "This component is especially useful when decoupling API input/output from internal models."
object-mapper.rst
Outdated
| class name we use ``newInstanceWithoutConstructor``) but *before* properties | ||
| are mapped. Its responsibility is often to return a correctly initialized | ||
| instance, potentially replacing the one created by the mapper. | ||
| The returned value *must* be an object of the target type. |
There was a problem hiding this comment.
What If it is not an object? An exception will be thrown?
| Sometimes, it's more convenient to define the mapping on the *target* class, | ||
| specifying where its properties should come from in the *source* class. This is | ||
| done using the ``source`` parameter within the ``#[Map]`` attribute on the target's | ||
| properties. |
There was a problem hiding this comment.
What if both the source and the target classes define #[Map] attributes? Which one takes precedence?
|
Thanks for the review @Spomky ! I think this is a good first shot to the documentation that can be improved later on. |
| vice-versa. It may also be useful when decoupling API input/output from internal | ||
| models helping with legacy code or hexagonal architectures implementations. | ||
|
|
||
| .. versionadded:: 7.2 |
There was a problem hiding this comment.
| .. versionadded:: 7.2 | |
| .. versionadded:: 7.3 |
| controller where you need it: | ||
|
|
||
| :: |
There was a problem hiding this comment.
| controller where you need it: | |
| :: | |
| controller where you need it:: |
Same for the other
| } | ||
| } | ||
|
|
||
| :: |
There was a problem hiding this comment.
| :: | |
| .. code-block:: php |
| public float $price_amount = 0.0; | ||
| } | ||
|
|
||
| :: |
| public ?User $manager = null; | ||
| } | ||
|
|
||
| :: |
7dea096 to
9ea1585
Compare
|
This is finally merged 🥳 Antoine, thanks so much for this colossal contribution. The docs are fantastic, comprehensive, well-structured, etc. The examples are very nice and easy to follow. A truly superb contribution 🙇 Thanks to all reviewers too. I did all the suggested fixes/changes while merging. |
Documents the object mapper component from