Best way to preserve compatibility for v4 to v5 upgrade

[webpaul]

I'm in the process of upgrading to v5 and discovering that we have hundreds of classes with cyclical references so I would like to globally set the PreserveReferences and UseDestinationValue options so I don't have to spend time tracking down which classes need this and which don't (or manually adding this call to hundreds of mappings)

Is this possible or should i just stick with v4 until the end of time?

[jbogard]

You can use ForAllTypeMaps to apply PreserveReferences to all/selected TypeMaps (your mappings will be faster than 4.x but roughly half to 1/3 as fast as normal 5.x mappings with it on versus off).

UseDestinationValue - I don't think anything changed there?

[lbargaoanu]

UseDestinationValue used to be true by default.

[webpaul]

This works for PreserveReferences for all our base Entity types to limit the scope (EF entities are where the issue is), how would I do UseDestinationValue as well?

            //compatibility changes to all maps
            mapConfig.ForAllPropertyMaps(p => p.SourceType != null && !p.SourceType.IsAssignableFrom(typeof(EntityBase)), (pm, o) => {
                //allow cyclical references like old version
                pm.TypeMap.PreserveReferences = true;
            });

[webpaul]

Hmm, and once I run this code I start getting the below error but when I comment the PreserveReferences code out I get the StackOverflowException on ones with cyclical references

        Source=Anonymously Hosted DynamicMethods Assembly
        StackTrace:
             at lambda_method(Closure , Office , OfficeModel , ResolutionContext )
             at lambda_method(Closure , Object , Object , ResolutionContext )
        InnerException: 
             HResult=-2147467262
             Message=Unable to cast object of type 'VASWeb.Models.Office.OfficeModel' to type 'VASWeb.Models.Shared.AddressModel'.
             Source=Anonymously Hosted DynamicMethods Assembly
             StackTrace:
                  at lambda_method(Closure , Office , OfficeModel , ResolutionContext )
             InnerException: 

[lbargaoanu]

You're supposed to set things through the second parameter, like you normally would.
Check the execution plan. If that doesn't help, make a repro, a gist that we can execute and see fail.

[webpaul]

I was able to make a simplified repro in our code base that just does a createmap on 3 objects and has the same cast error. When I tried to get it running as an AutoMapper test in your code base it passes though. I'm having to copy over hundreds of classes to get it to compile so I may have missed something in the process but have spent hours trying to repro it without success so far.

[webpaul]

Hah, the second I posted this I was able to get it to happen in the AutoMapper code base. I'll clean up and simplify the hierarchies to make it simpler to debug and may take a crack at trying to fix it as well.

[webpaul]

Ok, I made a pull request with a failing test. I stepped through in the debugger but wasn't able to figure out what is going on with all the expression building but I suspect it is getting confused between two similar mapping types and using the wrong one. Commenting out PreserveReferences makes it not happen but I need the PreserveReference behavior.

[webpaul]

Here is a gist with some of the variable names renamed to make it easier to debug if it helps. I'm new to all this expression building stuff so I haven't been able to figure it out yet. It's trying to assign an AddressModel to a ClientModel but I haven't been able to find a spot where I can make a fix since it is really difficult (for me) to debug these expressions. I think TypeMapPlanBuilder.CreateMapperFunc or somewhere near there is where the issue is.

https://gist.github.com/webpaul/c2a0cc8c94a8e0f9b96664789320b195

[lbargaoanu]

The problem is that in the same call you want to map a source object to different destination objects. You can work around it by changing your maps or by not setting PreserveReferences for one map. I guess the fix is to consider the destination type when caching the destination instance, but that's expressions code too.

[webpaul]

We have thousands of lines of mapping code so if the requirement to upgrade to v5 is to change all our mapping code we will just stay on v4 forever I'm afraid. I'm willing to make code changes to AutoMapper if you can give a few hints though.

I tried adding code to AM that checks the types but ending up in only making it not assign the AddressModel object at all. Is there a danger in adding an option to AutoMapper to check if the types match? Since it throws an exception now isn't that a safe thing to add anyway? If not can we put in an option into AutoMapper that makes it behave the way I need in order to not change our thousand of lines of maps?

[lbargaoanu]

Nobody said to change all your maps. Well, you did :)

[webpaul]

Well, we would have to check and most likely change thousands of lines of mapping code. I've narrowed this down to one scenario to make it easy for you guys to see the issue.

Isn't it a valid use case to have the same type map to different types? If for example I had a DB entity that mapped to 2 different view models in different parts of the app isn't that a valid use case that shouldn't throw a casting Exception in AM?

[lbargaoanu]

We'll probably fix it. But it seems strange to map the same object to two different destinations in the same map call. Maybe it's just me.

[webpaul]

I'm not sure what you mean, there are two separate calls to .CreateMap, how would you suggest doing this type of mapping normally? Unfortunately the mapping code in our app is a bit of a mess, written by legions of contractors in the past. I can eventually clean things up but was hoping to get the sweet v4 to v5 perf improvements without making too many changes...

mapConfig.CreateMap<Client, ClientModel>()
.ForMember(m => m.AddressObject, opt => opt.MapFrom(x => x));

        mapConfig.CreateMap<Client, AddressModel>()
            .ForMember(m => m.AddressInner1, o => o.MapFrom(x => x.Address1));

[lbargaoanu]

I'm talking about objects, not types. Map, not CreateMap.

[webpaul]

I see, yeah we have all our mapping calls in one place done on app startup so things aren't segmented by areas of the app. It's difficult to determine what is used where so even if I wanted to split it up I would probably cause a bunch of runtime errors as some mapping would be used in multiple areas in subtle ways, etc.

[webpaul]

What is the best way to go about testing this out? Is vNext available through nuget or do I need to manually build the solution and replace the DLL in projects that use it? When do you think it will be released via nuget?

[lbargaoanu]

Try the MyGet build.

[webpaul]

This fixed the issue and we can now run the latest 5.3 alpha version for our very complicated mapping setup. Do you know when this will be available via nuget?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.