Skip to content
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

Inheritance with collections no longer working #69

Closed
rizi opened this issue Dec 21, 2017 · 10 comments
Closed

Inheritance with collections no longer working #69

rizi opened this issue Dec 21, 2017 · 10 comments

Comments

@rizi
Copy link

rizi commented Dec 21, 2017

We upgraded from AutoMapper 4.x to AutoMapper 6.2.1 and therefore we also upgraded AutoMapper.Collection to 3.1.3 (coming from 2.1.1).
Now our mappings no longer work with Collections<TBase> when the collection contains derived types of TBase. Because it’s quite complicated to describe I created a test solution with two projects, one project contains a a fix/workaround in AutoMapper.Collection (downloaded the source from Github and marked the parts which I've changed with //note change to make it work and one project with the NuGet package from nuget.org.

Note: Depending on where (mappings for the base or concrete classes[.CreateMap<>]) you put your .EqualityComparison(....) it behaves differently, either you get an exception --> that happens when you put a .EqualityComparison(..) to your base mappings and your derived mappings or at least if you put it only on the derived mappings.
issue_ii

And if you put the .EqualityComparison() only in your base mapping it isn't used at all.
equalitycomparisonnotused
)

It would be great if you can have a look and hopefully fix the issue.

Br
Renè
AutoMapper.CollectionIssue.zip

@TylerCarlson1
Copy link
Member

If you already have a working fix could you make a pull request for this?
And you have a failing test you can add it to Pull Request so that this issue gets checked on every build.

@rizi
Copy link
Author

rizi commented Dec 21, 2017

Unfortunately it's not a fix for everyone. It's just something that's ok for now in our application. What I can do is add a failing test via pull request. What unit test runner do you use, I didn't find a Testclass or Testmethod ( or something similar) in your tests. Br

@rizi rizi closed this as completed Dec 21, 2017
@rizi rizi reopened this Dec 21, 2017
@Tasteful
Copy link
Contributor

When reading the code it looks like the equivalent expression always is found for the concrete type.

If we take this example of mapping

                    //collection type
                    cfg.CreateMap<OrderDomain, OrderEf>()
                        .EqualityComparison((oo, dto) => BaseEquals(oo, dto))
                        ;

                    cfg.CreateMap<MailOrderDomain, MailOrderEf>()
                        .IncludeBase<OrderDomain, OrderEf>()
                        ;

and try to find the IEquivalentComparer based on the MailOrderDomain -> MailOrderEf we will not get any hits back, even that the mapping is including a base-mapping that has the EqualityComparison-set.

If I remember correctly I got the same issue in my project and without investigate why it happen, I added the EqualityComparison on all sub-mappings and get it to work.

@rizi
Copy link
Author

rizi commented Dec 21, 2017

@Tasteful can you try to fix the failing unit test in my uploaded solution? I already tried to put the .EqualityComparison() to all sub types and remove it from the base type but it never gets called for any subtype. br

Tasteful added a commit to Tasteful/Automapper.Collection that referenced this issue Dec 22, 2017
@rizi
Copy link
Author

rizi commented Dec 27, 2017

@Tasteful thx! But I dont know If IncludeBase is enough. What if someone uses .include<> instead of .IncludeBase<> because the mappins are created the other way round? Br

@Tasteful
Copy link
Contributor

@rizi I have never used the .Include<,>(), will the mapping in that case be something like the following?

cfg.CreateMap<OrderDomain, OrderEf>()
    .EqualityComparison((oo, dto) => BaseEquals(oo, dto))
    .Include<MailOrderDomain, MailOrderEf>()
    .Include<OnlineOrderDomain, OnlineOrderEf>()
    ;

cfg.CreateMap<OnlineOrderDomain, OnlineOrderEf>()
    .EqualityComparison((ood, ooe) => DerivedEquals(ood, ooe))
    ;

cfg.CreateMap<MailOrderDomain, MailOrderEf>()
    ;

instead of

cfg.CreateMap<OrderDomain, OrderEf>()
    .EqualityComparison((oo, dto) => BaseEquals(oo, dto))
    ;

cfg.CreateMap<OnlineOrderDomain, OnlineOrderEf>()
    .EqualityComparison((ood, ooe) => DerivedEquals(ood, ooe))
    .IncludeBase<OrderDomain, OrderEf>()
    ;

cfg.CreateMap<MailOrderDomain, MailOrderEf>()
    .IncludeBase<OrderDomain, OrderEf>()
    ;

@Tasteful
Copy link
Contributor

@rizi Can you try with the pre-release on myget (https://www.myget.org/feed/automapperdev/package/nuget/AutoMapper.Collection) to see if that is solving your issue?

@rizi
Copy link
Author

rizi commented Dec 28, 2017

@Tasteful, regarding the mapping that's exactly how it would look like, see: http://docs.automapper.org/en/stable/Mapping-inheritance.html

I will try the pre release as soon as possible (hopefully in the next few days)
Best regards

@rizi
Copy link
Author

rizi commented Dec 28, 2017

@Tasteful I tried the pre-release package from your myget feed and ITS WORKING FINE!
THX

@Tasteful
Copy link
Contributor

@jbogard You can close this one also, package is published on nuget.

@jbogard jbogard closed this as completed Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants