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

Dynamic map of base class influences child class map #3025

Closed
matthiaslischka opened this issue Apr 10, 2019 · 8 comments
Closed

Dynamic map of base class influences child class map #3025

matthiaslischka opened this issue Apr 10, 2019 · 8 comments

Comments

@matthiaslischka
Copy link

Hi,
we stumbled across some odd behavior with dynamic maps.

void Main()
{
    AutoMapper.Mapper.Reset();
    AutoMapper.Mapper.Initialize(cfg => 
        { 
            cfg.CreateMissingTypeMaps = true; 
            cfg.ValidateInlineMaps = false; 
        }); 

    var model = new Model { Name = "Matthias", VersionNumber = 123 };

    var bar = AutoMapper.Mapper.Map(model, new Bar());
    var foo = AutoMapper.Mapper.Map(model, new Foo()); //VersionNumber null because BaseClass Bar was mapped before and does not have property VersionNumber. Comment out the line above and VersionNumber will be 123
		
    bar.Dump();
    foo.Dump();
}

class Model {
    public string Name {get; set;}
    public long VersionNumber {get; set;}
}

class Bar {
    public string Name {get;set;}
}

class Foo : Bar {
    public long? VersionNumber {get; set;}
}

foo.VersionNumber will be null when I call a dynamic map on the base class before and set to 123 when I don't. That's strange to me.
When I try the same with types and not concrete objects it throws an exception. I guess because the dynamically generated map for the base class Bar can not be used for Foo. But the exception message is strange.

var bar = AutoMapper.Mapper.Map<Bar>(model);
var foo = AutoMapper.Mapper.Map<Foo>(model);

// InvalidCastException: Unable to cast object of type 'Bar' to type 'Foo'.

We already learned that we will not use dynamic maps anymore. Just wanted to share our findings and ask if this behavior is intentional or a unknown side effect.

Thanks and BR Matthias

@lbargaoanu
Copy link
Member

I've already answered this on SO.

@matthiaslischka
Copy link
Author

matthiaslischka commented Apr 10, 2019

Sry, but no, you did not. "Dynamic maps work only in the simplest cases" is not a valid answer to the questions at hand but rather a give-up imho.
The questions at hand are:

  • Is this known?
  • Is this even correct or not simply a bug? (imho this is a bug, because I would expect dynamic mapping to create a map for the concrete class regardless if there is already a map for the base class or not --> behave the same way no matter what was called before)
  • Why does it throw when mapping to type but not when mapping to concrete object?
  • Is this the correct exception message when mapping to type? "Unable to cast object of type 'Bar' to type 'Foo'." when what I do is map model to Foo, considering Bar is no where near this code fragment and the message comes "out of the blue".
  • Is this somewhere documented? Are people aware of this?
  • Should this be documented?

BR Matthias

@jbogard
Copy link
Member

jbogard commented Apr 10, 2019

Dynamic mapping only works in the very simple cases, isolated maps etc. Because you start to run into the order in which maps are used that affect mapping plans, I generally don't advise doing what you described above.

I'm not too interested in documenting edge cases of dynamic mapping, and would rather just describe the scenarios in which you should use it - small, simple applications. Past that is 🤷‍♂️

@matthiaslischka
Copy link
Author

matthiaslischka commented Apr 10, 2019

Is this one of the rare occasions of "it's a feature not a bug" or why do we not agree that this is a bug?

Why does dynamic map not create a map for the type just because a base-type map already exists?
Why not always create a map for the concrete type regardless if any base type map exists?

@jbogard
Copy link
Member

jbogard commented Apr 10, 2019

No I think it's neither a feature nor a bug - it just is. Honestly it's issues like this that lead me to want to get rid of this feature altogether. There's never going to be a good way to document behavior, or provide "right" behavior since so much depends on the order in which things happen, and our up-front configuration is designed to resolve all these scenarios. JIT mapping will never, and I don't ever expect it to.

@matthiaslischka
Copy link
Author

matthiaslischka commented Apr 10, 2019

I totally agree. It's too much magic. I was thinking that you could reduce the magic and "state" by not looking for base-class maps and always create a clean map for the concrete type at hand, but I can see now that you don't want to pick up on this idea. ^^

edit: I do not agree that odd behavior should not be documented so I created a small blog post for anybody running into similar problems.

@jbogard
Copy link
Member

jbogard commented Apr 12, 2019

I don't see a way to address this behavior without breaking other scenarios. If you have a PR that doesn't break a bunch of other tests, we can take a look.

@lock
Copy link

lock bot commented May 13, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators May 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants