Skip to content

[AutoMapper] New component to automatically map a source object to a target object #30248

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

Closed
wants to merge 38 commits into from

Conversation

joelwurtz
Copy link
Contributor

@joelwurtz joelwurtz commented Feb 14, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #17516 #22051
License MIT
Doc PR todo

This PR brings a new component to the Symfony. It's a follow up on the AST Generator Component that was done on #17516 which was lacking of real buisness value and only adding technical one. (It can also certainly replace #22051, but both can be merged without conflicts)

In this component code generation is only an implementation detail, however by its nature it achieves the goal of having ultra fast normalizers in Symfony.

Description

Taken from https://github.com/AutoMapper/AutoMapper

AutoMapper is a simple little library built to solve a deceptively complex problem - getting rid of code that mapped one object to another. This type of code is rather dreary and boring to write, so why not invent a tool to do it for us?

In PHP libraries and application mapping from one object to another is fairly common:

  • ObjectNormalizer / GetSetMethodNormalizer in symfony/serializer
  • Mapping request data to object in symfony/form
  • Hydrate object from sql results in doctrine
  • Migrating legacy data to new model
  • Mapping from database model to dto objects (API / CQRS / ...)
  • ...

The goal of this component is to offer an abstraction on top of this subject. For that goal it provides an unique interface (other code is only implementation detail):

interface AutoMapperInterface
{
    /**
     * Map data from to target.
     *
     * @param array|object        $source  Any data object, which may be an object or an array
     * @param string|array|object $target  To which type of data, or data, the source should be mapped
     * @param Context             $context Options mappers have access to
     *
     * @return array|object The mapped object
     */
    public function map($source, $target, Context $context = null);
}

The source is from where the data comes from, it can be either an array or an object.
The target is where the data should be mapped to, it can be either a string (representing a type: array or class name) or directly an array or object (in that case construction of the object is avoided).

Current implementation handle all of those possiblities at the exception of the mapping from a dynamic object (array / stdClass) to another dynamic object.

Usage

Someone who wants to map an object will only have to do this:

// With class name
$target = $automapper->map($source, Foo::class);
// With existing object
$target = new Foo();
$target = $automapper->map($source, $target);
// To an array
$target = $automapper->map($source, 'array');
// From an array
$source = ['a' => 'b'];
$target = $automapper->map($source, Foo::class);

Context

Context object allow to pass options for the mapping:

// Using context
$context = new Context();
$target = $automapper->map($source, Foo::class, $context);

// Groups (serializer annotation), will only map value that match those group in source and target
$context = new Context(['groupA', 'groupB']);
// Allowed attributes, will only map specific properties (exclude others), allow nesting for sub mapping like the serializer component
$context = new Context(null, ['propertyA', 'propertyB', 'foo' => ['fooPropertyA']]);
// Ignored attributes, exclude thos propreties include others
$context = new Context(null, null, ['propertyA', 'propertyB', 'foo' => ['fooPropertyA']]);
// Set circular reference limit
$context->setCircularReferenceLimit(2);
// Set circular reference handler
$context->setCircularReferenceHandler(function () { ... });

Other features

Private properties

This component map private properties (However this can deactivated).

Nested Mapping

This component map nested class when it's possible.

Circular Reference

Default circular reference implementation is to keep them during mapping, which means somethings like:

$foo = new Foo();
$foo->setFoo($foo);

$target = $this->automapper->map($foo, 'array');

will produce an array where the foo property will be a reference to the parent.

Having that allow using this component as a DeepCloning service by mapping to the same object:

$foo = new Foo();
$foo->setFoo($foo);

$deepClonedFoo = $this->automapper->map($foo, Foo::class);

Max Depth

This component understand the Max Depth Annotation of the Serializer component and will not map after it's reached.

Name Converter

Default implementation allows you to pass a Name Converter when converting to or from an array to change the property name used.

Discriminator Mapping

This component understand the Discriminiator Mapping Annotation of the Serializer component and should correctly handle construction of object when having inheritance.

Type casting

This component will try to correctly map scalar values (going from int to string, etc ...).

History

Initial code of this component was done in an expiremental library here: https://github.com/janephp/automapper

After some works and talks with @dunglas and @nicolas-grekas i propose this library as a new component for Symfony, which can be used by other components and could help in resolving some future bugs due to PHP 7.4 (more explanations below).

Implementation

Default implementation use code generation for mapping, it reads once the metadata needed to build the mapper then write PHP code, after this, no metadata reading or analysis is done, only the generated mapper is used.

This allow for very fast mapping, here is some benchmarks using the library where the code comes from (jane/automapper):

Benchmark on the component on serializer part only (code source here https://github.com/joelwurtz/ivory-serializer-benchmark/tree/symfony/automapper):

benchmark

Example of generated code

Normalizer Bridge

A normalizer Bridge is available where its goal is to be 100% feature compatible with the ObjectNormalizer of the symfony/serializer component. The goal of this bridge is not to replace the ObjectNormalizer but rather providing a very fast alternative.

As shown in the benchmark above, using this bridge leads up to more than 8x speed increase in normalization.

Existing PHP librairies on automapping

Has some nice features poke @mark-gerarts some API are based on your works and C# automapper, would be nice to have you here for your input.

Things that needs to be done

This PR should be mostly readly in terms of code and functionnalities for a first stage, however there is still some things to be done:

  • Tests, tests and tests: i did not copy the old tests as i want to rewrite them for this component (will be done in the next days)
  • Documentation: documentation explaining this new component (will be done once the main interface and usage is ok)

Future consideration

Things that could be done but not in this PR:

  • symfony/form bridge for mapping request data to object
  • symfony/framework-bundle integration
  • symfony/validator integration:

PHP 7.4 may give a problem to the symfony/validator component where typed properties can be problem, given a class like this:

class Foo {
    /** @Assert\NotNull() */
    public int $foo;
}

An user may send a null value (in a form by example or json), and PHP will raise an error before the validation, since the validation occurs on the mapped object.

This component can help resolving this case with the actual behavior:

  • Create a dummy class with the same properties as Foo but without type checking
  • Mapping user data to this dummy class (using the automapper component)
  • Validating this dummy class with the metadata from the Foo class
  • Mapping the dummy object to the foo class (using the automapper component)

Feel free to challenge as much as possible

@lyrixx
Copy link
Member

lyrixx commented Feb 14, 2019

@ostrolucky Could you elaborate about your vote? May be you miss-clicked :)

@ostrolucky
Copy link
Contributor

I just don't see any justification why creating new component instead of embracing existing one. Poster is aware there is plenty packages in this field already, why create new one? Core team can barely keep up even with issues and PRs in existing set of components.

@nicolas-grekas
Copy link
Member

Core-team can grow. But actually, that's an important part of adding a new component: who will maintain it in the long run? @joelwurtz we know you as a regular OSS contributor - not in Symfony itself but eg httpplug if I'm not wrong? What would be your stand on this topic?

@joelwurtz
Copy link
Contributor Author

On this subject, this library was developed under the jane organization which is sponsored by my company (jolicode.com) by giving me time for working on it.

It would be the same for this component (my company will give me time to contribute to this like it's already done for @lyrixx). I totally understand your concern and i'm really motivated to be a regular contributor on this component.

Poster is aware there is plenty packages in this field already, why create new one?

This is mainly because i think there is some values in using the AutoMapperInterface in other components of Symfony.

This implementation (compared to others) is really fast and also allow feature compatiblity with the ObjectNormalizer of Symfony and it would help us in some of our projects to boost their performances.

Also the proposed interface could be used by existing automapper librairies (that's why i ping author of automapper plus), so an user could switch its implementation.

@linaori
Copy link
Contributor

linaori commented Feb 14, 2019

There's a bundle for automapping actually: https://github.com/mark-gerarts/automapper-plus-bundle

@zanbaldwin
Copy link
Member

I can see this being really useful for performance, especially in APIs which require the deserialization of request data and the serialization of responses (the main culprit for response times in our current project is denormalization).

The following may be completely unrelated and entirely out-of-scope, but after reading this PR my brain jumped to the following possibility and I would love to see some form of it come into Symfony 5:

Currently, the following code is pretty common:

// (ignoring possible exceptions thrown)
$requestData = $request->request->all();
$model = $serializer->deserialize($requestData, MyModel::class);
$validator->validate($model);

Whereas if the validator had the ability to generate constraint lists for the normalized version of objects, it would create a nice, performant data flow when combined with the AutoMapper. It would negate the need for the serializer altogether, and the form component could skip DTO's altogether while handling the strictly-typed class properties coming to PHP in 7.4.

$constraints = $validator->determineConstraintStructureForNormalizedObject(MyModel::class);
// Constraint list for this model can be cached during a custom warmup.
if ($validator->validate($requestData, $constraints)) {
    $object = $automapper->getMapper(MyModel::class)->map($requestData);
}

I've wanted this for a while and this seemed like the closest opportunity to rant about it 😉

@ostrolucky
Copy link
Contributor

ostrolucky commented Feb 14, 2019

I don't see how are reasons given valid, which were:

  • performance: Is it not possible to improve performance of existing components?
  • usage of interfaces in symfony components: I don't know about rule which forbids symfony components to implement 3rd party interfaces. IMO this is better solved with PSR. It's surprising there is still no attempt of Serializer PSR.
  • usage of interfaces in 3rd party components: As far as I see, people don't use SF interfaces in 3rd party components much even now

Also, this overlaps domain of Serializer. What makes this different from deserialization?

@joelwurtz
Copy link
Contributor Author

joelwurtz commented Feb 14, 2019

@zanbaldwin Yes, it's also one of the reason i bring this component (see the last part about validation and 7.4 in my PR) to helps such case in validation. Not sure how this would be done in details but this component could definitely help.

@ostrolucky

  • performance: Is it not possible to improve performance of existing components?

Yes but it will always be limited by design, current normalizers need to check metadata (i do not speak about reading them, there is cache for that, speaking about code added to do that when metatdata X is equal to this).

Here metadata change the way the code is generated, so no need to even read them on subsequent requests.

Some prs where done to generated normalizers but they were never merged, as the ObjectNormalizer has a lot of features and it's really hard to keep BC compliance.

Here the proposed Bridge is offered as an alternative to the ObjectNormalizer, both could coexist, they have the same purpose but differents implementations and may have in the long term different features also. In my view i would see this Bridge as an normalizer focused on performance rather than features, where the object normalizer could be one focused on features and less on performance.

Speed of ObjectNormalizer is really fine when normalizing small to medium result set, however on large data set it's really one of the main blocker.

  • usage of interfaces in symfony components: I don't know about rule which forbids symfony components to implement 3rd party interfaces. IMO this is better solved with PSR. It's surprising there is still no attempt of Serializer PSR.

This is more about having the possibilites to implement needed thing if using an automapper in the symfony/form or symfony/validator component. As it if miss something then making a PR that modify both component allow for better workflow in contribution (syncing thing is hard).

  • usage of interfaces in 3rd party components: As far as I see, people don't use SF interfaces in 3rd party components much even now

I agree this is very rare, and IMO that's why https://github.com/symfony/symfony/tree/master/src/Symfony/Contracts was done. This interface could be part of it so it would allow to require it without having all code about this implementation.

Also, this overlaps domain of Serializer. What makes this different from deserialization?

Don't agree here, serialiazing and mapping are 2 differents problems.

It may overlap on the normalization, since both can do object to array, and array to object convertion.

But this component is not about encoding to json or other formats. Neither is serializer component about mapping between two objects (although this may be done by using an array as a proxy data).

If we want to be perfect, (but will not be since there is BC to keep in mind), serializer component should use this component for its default implementation.

@OskarStark
Copy link
Contributor

One question, why not name it symfony/mapper? What exactly is meant “Auto”-matic here?

@joelwurtz
Copy link
Contributor Author

"Auto" part is for automatically mapping properties without having to write a custom config.

This name also comes from the popular C# https://github.com/AutoMapper/AutoMapper which is where the original idea comes from.

But i don't mind having it named only "Symfony/Mapper" or whatever suits better.

Also updated PR with new benchmark on the component itself (8x performance compared to ObjectNormalizer)

@mark-gerarts
Copy link

Thank you for the mention @joelwurtz. I've been following the development of https://github.com/janephp/automapper for a while now. It's obvious that a lot of work went into this, and it shows (especially performance-wise). I'll list my thoughts here in no particular order.

  • I'm obviously a bit biased, but I would love to see this as a component. I tend to make use of (auto)mapping quite a lot, and having a standardised, high-quality library with great performance would be a nice thing.
  • While there are some overlaps with the serializer, I have to agree with @joelwurtz that they're dealing with two different problems.
  • I'd be more than happy to implement a common interface, even if this PR doesn't land. The ability to change implementations would be a good thing.
  • In addition to @OskarStark's question, I originally named my library as a homage to .NET's AutoMapper, since a great deal of my inspiration was taken from there.

@joelwurtz
Copy link
Contributor Author

Just add an option to disallow attribute checking (it removes support for allowed and ignored attributes, something that IMO is not used everywhere, however it's really useful on normalizer when doing graphql by exemple).

When disabling this check it offers a 15% performance gain (compared to activating this option), and 11x gain compared to ObjectNormalizer

deepinscreenshot_select-area_20190215113512

@joelwurtz
Copy link
Contributor Author

@mark-gerarts Thanks you for coming in and for your view,

As for the common interface, it we want to go that way this is something that need to be added in symfony/contracts IMO, but waiting for core contributors view on this subject before going any further.

mark-gerarts added a commit to mark-gerarts/automapper-plus that referenced this pull request Mar 17, 2019
This is a step in the direction of using a common interface:
symfony/symfony#30248
@lyrixx
Copy link
Member

lyrixx commented Mar 22, 2019

@joelwurtz Could add @experimental ?

@symfony/deciders What do you think? IMHO this would be nice to get this in 4.3

@pbowyer
Copy link
Contributor

pbowyer commented Mar 23, 2019

Another for the "Existing Libraries" list is https://github.com/Seacommerce/php-mapper.

@smoelker
Copy link

I really like to see a mapper component being added to the Symfony framework.

We created the seacommerce/mapper library (thanks for the reference @pbowyer) in an attempt to combine the power of the jane/automapper library and the interface of mark-gerarts library plus one additional feature that I call "validation".

Validation is an early( AOT) warning system for invalid mapping configurations and should check at least for:

  • Existence of configured properties.
  • Completeness of the configuration. In other words, are all target properties mapped (either automatically or manually) or otherwise explicitly ignored.

And, additionally

  • Check for property type incompatibility paired with the option to register value converters that can be reused over configurations.

It's very important to me to have validation because our systems (like most systems) are under constant development (adding properties, removing properties, refactoring, new mappings, etc). In PHP, there is no language construct to reference properties (like C#'s nameof) and therefore we can only use string constants to configure mappings which is not refactor friendly. It's very easy to miss a property or make a typo and this can only be detected at runtime (whether or not using unit tests).

What are your thoughts on AOT validation?

@joelwurtz
Copy link
Contributor Author

joelwurtz commented Mar 24, 2019

Hey @smoelker thanks for raising those questions:

Existence of configured properties.

Default implementation of the mapper rely on the symfony/property-info component to map properties between objects, so properties should exist by default, but really depends on the extractor implementation.

The goal here is to blindly trust extractor implementation, so someone can propose its own extractor and remove properties or add virtual one.

Completeness of the configuration. In other words, are all target properties mapped (either automatically or manually) or otherwise explicitly ignored.

It's not checked but could be added as an option or different implementation in a future PR, as you point out this can make sense in some configurations to ensure that each target property has been correctly handled.

Also this is only needed between two objects (as from / to array mapping always have all properties); and, this is something that should be not so hard, mapping properties between objects is done here: https://github.com/symfony/symfony/pull/30248/files#diff-5c94bcb9eaccdbd9585dcd26e7a924a0R42

But would need the possibility to add ignored property also (which should be done in the property info extractor, so other components can profit of it)

Check for property type incompatibility paired with the option to register value converters that can be reused over configurations.

ATM there is a lot of work to correctly map a specific type to another one, current implementation has been made internal as a start but can be added into the public API.

For each property we have:

  • A list of types for the input
  • A list of types for the output

Then there is the TransformerFactoryInterface https://github.com/symfony/symfony/pull/30248/files#diff-102cf9c93dd80aec0f645a4af77a750a which has the goal to give the correct TransformerInterface for this option

Some helpers are also present for the factory and transformer to only deal with a single input / output type.

So someone can add it's own pair of TransformerFactoryInterface and TransformerInterface in order to handle a specific value converter

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Good job, I would love to see this component in 4.3. It will help improving Serializer's performance.

$hash = $mapperGeneratorMetadata->getHash();
$registry = $this->getRegistry();

if (!isset($registry[$className]) || $registry[$className] !== $hash || !file_exists($classPath)) {
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we use the existing Symfony file tracking infrastructure instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with that system, can you give me some hint where i can find other code using this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping @dunglas

@lsmith77
Copy link
Contributor

I also think that having some sort of mapper solution would be a useful addition.

A minor but potentially important thing: I am wondering about is the name. From my understanding "auto mapper" is mostly derived from the .Net/C# auto mapper lib, which is the source inspiration. Do we want to stick to the auto in there? Unless this has become a defacto lingo for such mappers, I wonder how many users benefit from this prefix and how much confusion it creates where people just expect it to be some automatic solution (while it is somewhat automatic, in many situations the mapping needs in-depth configuration)

@joelwurtz
Copy link
Contributor Author

A minor but potentially important thing: I am wondering about is the name. From my understanding "auto mapper" is mostly derived from the .Net/C# auto mapper lib, which is the source inspiration. Do we want to stick to the auto in there? Unless this has become a defacto lingo for such mappers, I wonder how many users benefit from this prefix and how much confusion it creates where people just expect it to be some automatic solution (while it is somewhat automatic, in many situations the mapping needs in-depth configuration)

I don't mind having it named symfony/mapper, but i like also the auto as the goal is to do the less configuration possible (and be as much automatic as we can).

@dunglas thanks for you review, about some classes that need to be moved into their own components, i see some options:

  • Do the change in this PR
  • Do the change in another PR before this get (potentially) merged
  • Do the change in another PR after this get (potentially) merged

Let me know which option do you prefer.

Also, This component is using nikic/php-parser for generating code. Although i prefer to use this library to generate code, i also understand this is mainly a subjective point of view (and objectively there is arguments for pro / con).

It is totally possible to not use this library and only use string as i know some people are not comfortable using this library for code generation, please let me know if we should keep this dependency or not.

@joelwurtz joelwurtz force-pushed the symfony/automapper branch 2 times, most recently from b60df86 to 08407c0 Compare March 26, 2019 00:36
@chalasr
Copy link
Member

chalasr commented Mar 26, 2019

about some classes that need to be moved into their own components, i see some options:

Moving existing classes should be discussed in another PR, but classes introduced here should be moved in this PR to avoid the need for deprecating them later.

@fabpot
Copy link
Member

fabpot commented Oct 7, 2020

We've just moved away from master as the main branch to use 5.x instead. Unfortunately, I cannot reopen the PR and change the target branch to 5.x. Can you open a new PR referencing this one to not loose the discussion? Thank you for your understanding and for your help.

@joelwurtz
Copy link
Contributor Author

Personal note about this PR (ref #51741)

I made this PR a while back, there was some interest but maybe not enough at this time ?

Since there seems to be no interest for such component i let the PR died, for the record : it was fully ready it needed review / discussion with core contributors certainly but there was not such discussions so i did not go further and we went ahead having this as an external library.

If such a component is needed (as said by some core contributors) why not contacting people that have already tried to do this ? a friendly ping ? a dm on slack ?
There was a lot of times and effort being invested in this PR and in his feedback, disregarding this seems not normal to all people involved in the previous discussion.

@dunglas
Copy link
Member

dunglas commented Sep 26, 2023

@joelwurtz Sorry for the lack of communication. The new proposal was completely unplanned and emerged from discussions following @weaverryan's talk at API Platform Conference and the new SymfonyCasts micro-mapper library he revealed during it.

I'll not speak for him, but I think that following these in-person discussions, @soyuka just created a Proof of Concept in the train (which has a totally different design than this PR) and opened a PR to discuss it, which is, in my opinion, a sane practice encouraging collaboration (everybody can review, discuss, improve, criticize...).

Regarding this PR, it was closed 2 years ago and the need is still there. Personally, as we discussed again with @Korbeil at SymfonyLive Paris, I've always been in favor of having an automapper in Symfony, and, as demonstrated at API Platform Con, there is still a very strong need for such a component in Symfony.

I didn't review thoroughly this PR (that's why I didn't formally "approve" it, even if I voted with a positive emoji), but I was for its merge when ready. I must admit that it's easier for me to find the time to review proposals of 500 lines of code than ones of thousands (it's also why I would love if #51718 could be more focused and... small :D). And it seems to be the same for most of the core contributors (including your colleagues).

Now we have several competing options (and that's great):

  1. Add AutoMapper component Korbeil/symfony#1, which has the benefit of being fast, but is complex and will be hard to maintain
  2. [ObjectMapper] Object to Object mapper component #51741, which is way less complex and has a modern DX, but will probably be slower
  3.  [JsonEncoder][Serializer] Introducing the component #51718, even if not totally related, that improves JSON encoding/decoding performance and DX, but is also complex and may be hard to maintain

In any case, we now have until 7.1 to confront, improve, and maybe merge proposals (if beneficial) and I'm sure that we'll finally get fast and easy-to-use new components in the end.

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.