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

Sorry guys, noob question here, but I was not at the conf so I didn't get the rationale.
Il love the syntax, but I fail to see the point of thisin the general Symfony components.

I see the point for APIP or such solutions, but I can't see why it should be a Symfony component, as I don't see the value it can provide compared to a basic custom mapper. And I don't see why we would want to get rid of custom mappers in generic projects if there's no gain except "one less class insrc, 5 more invendor". Especially since performance was not the primary issue in mind here. Ok, coding mappers is boring, but in that case, shouldn't the replacement be as simple (or even simpler) AND as fast AND as secure?

Other than that, once again, the DX is awesome, maybe except one or two small details.

I see the point for APIP or such solutions, but I can't see why it should be a Symfony component, as I don't see the value it can provide compared to a basic custom mapper. And I don't see why we would want to get rid of custom mappers in generic projects if there's no gain

That's a fair question. In practice, in a rich DTO system, when I have objects with relationships, things often get more complex than one class for the mapping. For example, I'm mappingProduct ->ProductDto and there is aProduct.category that should be mapped to aProductDto... and so from my mapper code fromProduct ->ProductDto, I also want to call the mapping code fromCategory ->CategoryDto. Or are other people not running into this?

Also, even if the solution is simple, there is some advantage to having a core interface & system for mapping. For API Platform, it opens up the possibility of mapping a DTO with#[ApiResource] and saying "get fetch the data from theProduct" entity. Because there is a central mapping system, it can use that to convert between theProduct andProductDto. If we don't have a central system (even if the problem is not that complex), we can't introduce features like that into API Platform's core. We could also document using DTO's in other places (like the form system) more readily due to there being a central way of handling that mapping part.

@weaverryan I also really liked your examples with the User DTOs (mapping password etc.) which are quite out of the scope of API Platform but are really useful within Symfony or with custom controllers altogether. Maybe we can share these?

Actually it's been quite some time that many Symfony users think that an AutoMapper component inside Symfony would be quite useful. I started a prototype after discussing with Ryan and therefore wanted to propose this as a component to help our community members.

@weaverryan I also really liked your examples with the User DTOs (mapping password etc.) which are quite out of the scope of API Platform but are really useful within Symfony or with custom controllers altogether. Maybe we can share these?

Sure :). Whether it's API Platform or using DTOs with forms, here are some requirements I had for a mapper:

A)User.plainPassword -> maps to ->User.password but we need to go through the password hasher
B) Same as above, but only map ifUser.plainPassword is notnull (don’t try to hash an empty string and override the existing hashed password)
C) Mapping A -> B, but B has some extra property on it that depends on, let’s say… 3 different properties on A and needs a service to calculate the final value.
D) Mapping fromUserDto ->User and because this is a PATCH operation / edit page,UserDto.id=5 and so we should actually query the database forUser whereid=5 instead of creating a new instance.
E) Same as above, but with a relation:ProductDto.category is aCategoryDto andProductDto.category.id=9. So when mappingProductDto.category ->Product.category, theCategory entity will be loaded from somewhere else (e..g database) but not instantiated.

First, thanks for your time!

That's a fair question. In practice, in a rich DTO system, when I have objects with relationships, things often get more complex than one class for the mapping. For example, I'm mappingProduct ->ProductDto and there is aProduct.category that should be mapped to aProductDto... and so from my mapper code fromProduct ->ProductDto, I also want to call the mapping code fromCategory ->CategoryDto. Or are other people not running into this?

I do have the same "problem" on a regular basis. In fact, pretty much every time I give a Symfony training (like twice a month at least), since it's pretty much part of the application we build as an exercice for the trainings at SL.

And again, I don't deny the DX gain on this, maybe it needs a bit more work (like, does it work when mapping collections, or nested collections?). But maybe I'm wrong, but I can't see something relying this much on Reflection being as effective as the boring solution, even with Mappers for nested properties. If you tell me the accepted tradeof is perf vs DX, why not, but I'm not sure it'll be used that much then, and so I'm back to my first question.

Also, even if the solution is simple, there is some advantage to having a core interface & system for mapping. For API Platform, it opens up the possibility of mapping a DTO with#[ApiResource] and saying "get fetch the data from theProduct" entity. Because there is a central mapping system, it can use that to convert between theProduct andProductDto. If we don't have a central system (even if the problem is not that complex), we can't introduce features like that into API Platform's core. We could also document using DTO's in other places (like the form system) more readily due to there being a central way of handling that mapping part.

Again, no question how it could be useful for APIP. But in that case IMHO it should stay in APIP.
Your examples seem typical to me of the common problems we have indeed by the way, but maybe I'm just not on point, but I still don't see the benefit of centralized Mapper for these either in its current design, except for DX (which is already great, but not enough IMHO).

Basically, if the perf question is resolved, and the gain is substantial not only for DX, I'm all for it. Otherwise, I'm not sure.

AutoMapper is a subject we hold close to heart here at JoliCode ❤️ (remember#30248? 4 years old already) and I would like to highlight another approach to the one exposed here.

I worked on bringingAutoMapper, previously in the JanePHP organization, now in JoliCode's in Symfony as a standalone component:Korbeil#1

It has not been opened on the Symfony repository yet as an excess of precaution and a will to bring something pre-approved (#30248 was a ton of work and never got merged so we were careful not to make the same mistake). It has been discussed on Symfony's Slack with some core maintainers but that didn't bring real feedback nor reviews.

If we step back for a sec, we all know that the Serializer revamp will not be merged before 7.1 at best. Beside, the current AutoMapper implementation looks quick and simple as it allows to map only DTO together. Meanwhile, we could take more time to consider my PR and deliver a complete and performant AutoMapper with the new Serializer implementation where the normalization part could leverage this AutoMapper and bring even more speed to the new Serializer#51718.

Performance should always be taken into account as a primary feature - we use Symfony on huge and heavy workload applications and it's not ok to have some features doing runtime Reflection with no cache. Code using such components would need to be hardly challenged from code reviews. And that’s why we do think AST is a very powerful way to get this performance.

Current implementations (automapper-plus, janephp/automapper) are good but feel to complicated when you want something accessible.

Could you elaborate on this statement? While your implementation is really nice and easy to read / maintain, from an user perspective, jolicode/automapper is as simple and accessible:

$automapper->map(source, target);

We all must keep in mind the benefit for Symfony's users, no matter what is our company, or the code ownership. Currently, we have 2 different suggestions for a new AutoMapper component, let's work together to leverage the best of both and bring something useful to the users.

Current implementations (automapper-plus, janephp/automapper) are good but feel to complicated when you want something accessible.
Could you elaborate on this statement? While your implementation is really nice and easy to read / maintain, from an user perspective, jolicode/automapper is as simple and accessible:

I tried using the janephp/automapper (maybe the jolicode one has improved?) but I had a very hard time with A, B, D & E from this list -#51741 (comment)

So yes, the interface is simple. I found customizing my mapping to be really hard / impossible in some cases. But these are things that can be improved :). I am more interested that there IS a solution that exists, not which one... though I'd love to collaborate on the DX for whatever is decided.

Probably a minor point, but i'd suggest to drop the "Auto" part of the component, and maybe keep it for the implementation.

The AutoMapper component allows you to map an object to another object,

For me if Symfony provide such component, i expect it to have a basic interface, hiding all the complexity / configuration behind.... including the possibility to use plain manual PHP / service code in implementations.

So maybe just name itMapper ? WDTY ?

namespace Symfony/Component/Mapper;class Automapperimplements MapperInterface

(ObectMapper does not provide much more, and impede potential evolutions)

After some discussions I:

• renamed the component toMapper
• introduced recursivity support (no need to control maxDepth here this is not a serializer)
• I want to change attributes to simplify even more the developer experience and try to cover a few cases people brought to me (thanks !!)

I've kept my old implementation here in case we want to track evolution. The description and title will be updated shortly.

// This calls strtoupper on A::transform#[MapWith('ucfirst')]public string $name;// This doesn't map A::bar if it's value is falsy.#[MapIf('boolval')]public bool $bar = false;

</details>

Hi, I have some suggestions. I think that in the attributes, the use of "Map" is redundant (MapTo, MapWith, MapIf, etc..). I think you can drop the "Map" prefix

Also, I suggest to drop the MapWith, and change the "To" attribute to be able to "transform" a value before applying it :
#[To(property: '...', classes: {B::class, C::class}, transform: {'ucfirst', ...})]
In this case, all parameters are not required :

• if you don't specify 'property', Automapper will find property with similar name
• if you don't specify 'classes', mapping will be valid for all #[Map()] attributes in the class
• if you don't specify 'transform', value will be ported as is

cool idea!
some thoughts:

• this does have some overlap with (de) serializer. could we see the serializer as a special case of the Mapper? serializer maps object to array, this mapper maps object to object...
• it could be interesting to have an interface for the metadata loader. annotations in the code is one way to define this, but metadata could also be written in an xml or yaml format. even if initially only annotations are supported, isolating the metadata loading behind an interface would allow to add other loaders.
• how about supporting accessor / setter methods and "virtual properties" that only exist as method but have no explicit field on a class? accessors methods could also be an alternative to the transformer callbacks

Hi, I have some suggestions. I think that in the attributes, the use of "Map" is redundant (MapTo, MapWith, MapIf, etc..). I think you can drop the "Map" prefix

To,With andIf are too common words, they could be used as attribute names by any other kind of API for different needs. I think theMap prefix probably should be kept, in case people start using attributes from other namespace along. Or they simply could be more specific ?

If I understood it well, by design, a class can be mapped only to one another class ? Isn't it a very drastic limitation ? I could want to map an entity to more than one kind of DTO, for example:

• some custom action form,
• another custom action form,
• various API endpoints with different schemas,
• ...

Is this achievable with the code in its current state ?

@nicolas-grekas rebased

Thank you@soyuka.

Congrats 🎉