Give the possibility to clone objects without UseDeepCloning

[samusaran]

Is your feature request related to a problem? Please describe.
Hi, I currently use AutoMapper to clone EF entities. The main use is either to support actually copies in the business logic (users can copy business objects) or to support forms which can be cancelled.

Given that I heavily use navigations, my object graph may be complex in some cases, which leads to UseDeepCloning to be impossible to use (maybe I want to clone a small object deep in the graph and I find myself with hundreds of new objects, which between all the other issues, breaks EF Change Tracking).

What I'm looking for is to be able to clone JUST the object I pass to the mapper, all the references either stay exactly the same, or I handle them manually.

Describe the solution you'd like
The solution I'm looking for is a middleground between UseDeepCloning true and false. Because false outright refuses to do any work and just outputs something like this:

        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.3.1.0")]
        private static partial global::Tables.OfferComponentLTR ToNewComponentLTR(global::Tables.OfferComponentLTR ltr)
        {
            return ltr;
        }

But with Deep Cloning enabled I found myself in the situation where I need to fight Mapperly to "undo" most of the deep cloning of references because I just don't need it.

I'm just expecting a normal mapping like:

        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.3.1.0")]
        private static partial global::Tables.OfferComponentLTR ToNewComponentLTR(global::Tables.OfferComponentLTR ltr)
        {
            var target = new Tables.OfferComponentLTR();
            target.FirstProp = source.FirstProp;
            target.SecondProp = source.SecondProp;
            ....
        }

Of course I do expect nothing more than an object with all properties set exactly the same, so even references should be all the same.

Describe alternatives you've considered
I've tried to work with UseDeepCloning enabled but the burden is extremely high, and it basically defeats the simplicity of Mapperly. On my codebase the actual result is way way worse than AutoMapper, to the degree that I mostly need to fight against Mapperly to avoid it to clone parent and child objects.

Also I've considered to wrap "patching" map method manually like this:

    private static OfferComponentServiceProduct ToNewServiceProduct(this OfferComponentServiceProduct product)
    {
        var target = new OfferComponentServiceProduct();
        product.MapServiceProduct(target);
        return target;
    }

And it works fine, but one thing lost here is the support for init-only properties, which must be either handled manually or changed to full auto-properties.

Additional context
I know that one possible solution could be to introduce DTOs, but it would be like asking me to basically change the whole application just to enable mapperly to generate something which can already do.

Without this feature Mapperly lose much of it's appeal for our application.

(tested on Mapperly 4.3.1)

[latonz]

How would you expect the API to configure this to look like?
Some ideas:

• a new bool MapperAttribute.UseDeepCloningForUserMappings when true Mapperly would always use deep cloning for all user-defined mappings
• a new [MapperUseDeepCloning(bool)] to be applied on mapping method declarations

What are your thoughts on this? Would this address your use-case?

[samusaran]

I'm new to Mapperly but in my opinion I would probably avoid the term Deep Cloning because what I'm asking is actually a shallow clone.

Take this two examples (assuming UseDeepCloning == false):

public static partial UserHierarchyRole ToNewUser(this UserHierarchyRole user) // same target type

public static partial UserEditModel ToDto(this UserHierarchyRole user) // different target type

Those two mapping methods are basically asking for the same thing, but the code generated is different, because while the second example creates a mapping as expected, the first mapping just returns the user instance.

So I could think of:

• bool MapperAttribute.ForceShallowCloningOnSameTargetType: very specific attribute that would force a standard mapping generation when using same source-target types. I don't really know how many people would need that tho.
• [MapperAvoidReturningSourceReference(bool)] on mapping methods, this one is probably more straight-forward, and probably easier to discover if a user get caught in the same issue

Let me know if the wording follows your guidelines.

I would be happy to contribute, but I don't promise anything because I still haven't took a look at Mapperly codebase 😅

[latonz]

You're absolutely correct with the fact that is not a deep clone.
What about MapperAttribute.UseShallowCloningForUserMappings / [MapperUseShallowCloning(bool)]? IMO we should include the term user mapping as it only applies to user defined mappings, no?

[samusaran]

I've built a quick solution in the commit above :)

It seems to work fine, I basically added the attribute [MapperUseShallowCloning] and used it to condition the direct assignment mapping and the implicit cast. It doesn't seems to need handling of other use cases. All tests pass (except the Public API Surface which I don't understand how to handle 😅).

I'm ofc open to feedback, and I can open a PR when it's ok.

[latonz]

Thanks for the work!

Feedback / inputs:

• The public api test is a verifyTest which needs its snapshot to be updated, see contributor docs and verifyTests docs.
• IMO we should implement both, setting it at a method level ([MapperUseShallowCloning(bool)]) but also setting it on the mapper class (MapperAttribute.UseShallowCloningForUserMappings).
• The configuration should be read through MapperConfigurationReader, see how other configuration attributes are read.
• On a first glance all references to MappingConfiguration.UseDeepCloning need to be updated, it probably makes sense to introduce a new bool MappingBuilderContext.UseCloning => Configuration.UseDeepCloning || (HasUserSymbol && Configuration.UseShallowCloning);
• If [MapperUseShallowCloning] is applied on a queryable mapping it should diagnostic RMG065
• Tests need to be added with reference handling enabled (especially for methods without a reference handler parameter)

[samusaran]

Ok thank you!

I'll work on it in the next days.

Do you think this change will still be included in 4.*? Or will it be pushed to 5.0?

[latonz]

Pushed to 5.0

[samusaran]

Hi @latonz , I thought a bit about your suggestions and I wanted to add something.

By allowing the combination of shallow cloning and deep cloning (ex [Mapper(UseDeepCloning = true, UseShallowCloningForUserMappings = true)]) don't we risk of having conflicting configuration?

From a user perspective, maybe an enum would be better? Something like:

• CloningBehaviour.Default or None: keeps the current behaviour of direct assignment and new object mapping, can be omitted
• CloningBehaviour.DeepCloning: Same as UseDeepCloning = true
• CloningBehaviour.ShallowCloning: Same as proposed solution, so always use new object mapping, even when direct assignment or cast would be possible

This would render UseDeepCloning obsolete in the beginning (and it would map to CloningBehaviour.DeepCloning in the background). Going this way for a major release would also be justified, as breaking changes are "accepted".

In this way it's also easier for us to understand which cloning behaviour we should use.

[latonz]

Using a new enum makes sense, sgtm. I’d mark UseDeepCloning as deprecated and emit a warning diagnostic when both CloningBehaviour and UseDeepCloning are used together.