MapsterMapper / Mapster

A fast, fun and stimulating object to object Mapper
MIT License
4.3k stars 328 forks source link

Return original value if type is the same #521

Open kendallb opened 1 year ago

kendallb commented 1 year ago

Just moved over to Mapster from AutoMapper and ran into a bug in our code based on a a behviour that AutoMapper had that I was not fully aware of. Basically if you are mapping from one type to another type so it creates an instance of the second type, if the two types are identical AutoMapper just returns the original instance rather than performing a mapping on it. Mapster will do the mapping.

In our case this triggered a bug in our code as we have change tracking on values we write to the database, and we had two scenarios where we were mapping from the DB type to the same DB type when doing collision detection and it failed because the change tracking said all columns were modified after it was mapped.

I have easily worked around this as all the mapping happens in a global helper function so I simply modified our code to behave the same way AutoMapper does, but I am wondering if it would make sense to do the same thing in Mapster for this same edge case as others moving from AutoMapper to Mapster might have the same problem?

kendallb commented 1 year ago

I am not entirely sure how best to implement this (only just forked and looked at the code), but I have written a failing unit test as the first step :)

Should be relatively simple to add code to compare if the source and dest types are the same, and then compile a lambda function to simply return the source instance as the result?

To ensure it would not trip up existing Mapster users (not sure it would?) it could be an opt in feature also.

https://github.com/kendallb/Mapster/commit/90571d08e9df6b1794cdf7dc6b9341b07a36eb68

andrerav commented 1 year ago

Hi @kendallb, I am struggling a bit to understand the use case. By "collision detection", are you referring to last save wins-scenarios in context of database transactions? Or something else?

kendallb commented 1 year ago

Something else that we do internally. It's not really relevant to this particular fix, but the key issue is that we have change tracked properties in our entities so if it does the mapping when the class type is the same, all columns get flagged as updated when they were not in the original entity that was mapped. Since AutoMapper simply passed through the original entity in this case without doing any mapping, it all worked nicely.

It seems a bit silly to map from the same class to itself, but that happens in code when you are using generic type arguments and you have a simple class where no mapping is needed, but the code needs to work as expected. So it is quite specific to our needs, but I do think others coming from AutoMapper might run into the same snag.