Skip to content

[ObjectMapper] Add component #20347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 7.3
Choose a base branch
from
Open

[ObjectMapper] Add component #20347

wants to merge 2 commits into from

Conversation

soyuka
Copy link
Contributor

@soyuka soyuka commented Oct 24, 2024

Documents the object mapper component from

@carsonbot carsonbot added this to the 7.2 milestone Oct 24, 2024
@OskarStark OskarStark changed the title [Object Mapper] component introduction [Object Mapper] Add component Oct 24, 2024
@OskarStark OskarStark added the Waiting Code Merge Docs for features pending to be merged label Oct 24, 2024
@carsonbot carsonbot modified the milestones: 7.2, next Oct 24, 2024
Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antoine, thanks for contributing the docs for this new component.

I left some minor comments.

#[Map(target: C::class, if: [Source::class, 'shouldMapToC'])]
class Source
{
public static function shouldMapToB(mixed $value, object $object): bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which are the mixed $value, object $object arguments passed to this method?

use Symfony\Component\ObjectMapper\Attributes\Map;

class Source {
#[Map(target: 'fullName', transform: TransformNameCallable::class)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I use a transform for the entire object instead of for each of its properties?

@javiereguiluz javiereguiluz modified the milestones: next, 7.3 Mar 24, 2025
@OskarStark OskarStark changed the title [Object Mapper] Add component [ObjectMapper] Add component Mar 24, 2025
@OskarStark OskarStark removed the Waiting Code Merge Docs for features pending to be merged label Mar 24, 2025
@OskarStark
Copy link
Contributor

CI should be green after a rebase on 7.3

@xabbuh xabbuh linked an issue Mar 24, 2025 that may be closed by this pull request
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A rebase on 7.3 branch would be helpful @soyuka, thanks

@soyuka soyuka requested a review from xabbuh as a code owner March 27, 2025 11:41
@soyuka
Copy link
Contributor Author

soyuka commented Mar 27, 2025

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?

@soyuka soyuka changed the base branch from 7.2 to 7.3 March 27, 2025 11:43
@soyuka soyuka force-pushed the object-mapper branch 5 times, most recently from de11d09 to f4c8610 Compare April 14, 2025 15:21
@soyuka
Copy link
Contributor Author

soyuka commented Apr 14, 2025

The new version needs symfony/symfony#60028

Copy link
Contributor

@Spomky Spomky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 👍🏼.


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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add "This component is especially useful when decoupling API input/output from internal models."

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if both the source and the target classes define #[Map] attributes? Which one takes precedence?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ObjectMapper] Object to Object mapper component
10 participants