AutoMapper / AutoMapper.Extensions.ExpressionMapping

MIT License
143 stars 39 forks source link

MapExpression throws "Missing type map configuration or unsupported mapping" error #85

Closed henrbj closed 4 years ago

henrbj commented 4 years ago

`

        var config = new MapperConfiguration(cfg =>
        {
            cfg.AddExpressionMapping();

            cfg.CreateMap<Source, SourceDto>()
                .ForMember(o => o.Items, config => config.MapFrom(p => p.Items.Select(s => s.Name)));

            cfg.CreateMap<SourceDto, Source>()
                .ForMember(o => o.Items, config => config.MapFrom(p => p.Items.Select(s => new SubSource{Name = s})));
        });

        var mapper = config.CreateMapper();

        // This works
        Expression<Func<Source, bool>> expression1 = o => string.Equals("item1", "item2");
        var mapped1 = mapper.MapExpression<Expression<Func<SourceDto, bool>>>(expression1);

        // This does not
        Expression<Func<SourceDto, bool>> expression2 = o => string.Equals("item1", "item2");
        var mapped2 = mapper.MapExpression<Expression<Func<Source, bool>>>(expression2);`

When trying to map the above expression, automapper throws an unsupported mapping error:

Missing type map configuration or unsupported mapping.

Mapping types: String -> SubSource System.String -> ExpressionMapping.SubSource

Why does automapper need this mapping? When I add the following: cfg.CreateMap<string, SubSource>().ConvertUsing(o => new SubSource { Name = o });

I receive this error:

Expression of type 'ExpressionMapping.SubSource' cannot be used for parameter of type 'System.String' of method 'Boolean Equals(System.String, System.String)' (Parameter 'arg0')

Why does automapper try to map the first argument in string.equals to SubSource?

This error appears in 10.0.0(Automapper) and 4.0.0(Expressions). When downgrading to 9.0.0 and 3.1.2 the error is no longer thrown.

Classes: public class Source { public ICollection<SubSource> Items { get; set; } }

public class SubSource { public int ID { get; set; } public string Name { get; set; } }

public class SourceDto { public string[] Items { get; set; } }

BlaiseD commented 4 years ago

Yep - the logic in 3.1.2 was also incorrect - just exposed by changes in AutoMapper 10.0.0. Your code works in the latest preview version.

Thanks.

henrbj commented 4 years ago

That was fast! Great, the preview works!

However, if I keep the string -> SubSource map, Automapper still tries to map the first argument in string.equals to SubSource. Why is that happening?

BlaiseD commented 4 years ago

That's because it is a configured map. The expression mapper will try to map constants in an expression e.g. suba below is considered a constant and will be mapped to SubModelB if the type map exists (see issue #79 ).

SubModelA suba = new SubModelA { Id = "s1" };
Expression<Func<ModelA, bool>> filter = m => m.Sub == suba;
var mappedfilter = mapper.MapExpression<Expression<Func<ModelB, bool>>>(filter);
henrbj commented 4 years ago

I see. For the example you provided it makes sense.

But for me, this looks like dangerous logic. Given your provided example, lets say SubModelA has multiple mappings: SubModelA -> SubModelC SubModelA -> SubModelB

Which one does it use? It will map, but the expression wont be correct when SubModelA is mapped to SubModelC.

When a configuration for an individual member is added it also seems like it is applicable for all mappings. In my issue I had these mappings:

cfg.CreateMap<Source, SourceDto>()
                        .ForMember(o => o.Items, config => config.MapFrom(p => p.Items.Select(s => s.Name)));
cfg.CreateMap<SourceDto, Source>()
                        .ForMember(o => o.Items, config => config.MapFrom(p => p.Items.Select(s => new SubSource{Name = s})));

Now, if I change my expression to:

var items = new[] { "item1", "item2" };
Expression<Func<SourceDto, bool>> expression2 = o => items.Contains("");
var mapped2 = mapper.MapExpression<Expression<Func<Source, bool>>>(expression2);

It throws the error:

No generic method 'Contains' on type 'System.Linq.Enumerable' is compatible with the supplied type arguments and arguments. No type arguments should be provided if the method is non-generic.

Now if I change my items array to a list it all works because list does not have a configured map.

var items = new[] { "item1", "item2" }.ToList();
BlaiseD commented 4 years ago

You may want to download the code and inspect it. It takes into account both the configured mapping and the context of the expression itself.

It's ok to open a new issue if there is a concern - just make it clear like the current one and we'll track it from there.

BlaiseD commented 4 years ago

And PRs are welcome.

henrbj commented 4 years ago

I still believe it is the same issue. My original post was a simplification of our expression in our code. The expression looks more like this:

Expression<Func<SourceDto, bool>> expression2 = o => o => o.Active &&
                                string.Equals(o.Property1, "value1") &&
                                string.Equals(o.Property2, "value2") &&
                                stringArray.Contains(o.Name);

Because the mapping between source and sourceDto has a property of string array on one side and property of ICollection on the other, this means that in all my expression mappings I cannot use string array constant because it will automatically be mapped.

Is this correct?

I will take a look at the code, but I think there is a theorical issue that needs to be adressed before we can agree on a change.

BlaiseD commented 4 years ago

I think that's backwards :) Look at where the code is blowing up first then figure out a solution.

List<T>.Contains works because it is not a generic method unlike the Enumerable.Contains<T> which you're using. How do I know? I looked at the code.

Correct that Enumerable.Contains will fail until we find a solution.

Again: PRs are welcome.

henrbj commented 4 years ago

I looked at the code, and it became obvious that it would take me a long time to understand it to a degree where I would be confident in any solution I suggest. This will come with experience I guess (Still junior dev).

Anyway, I can not see a solution which satisifes all, because an expression can contain almost anything and getting the mapping right will be difficult.

Props to you, and hope that you find a good solution.

BlaiseD commented 4 years ago

One step at a time - we add solutions and corresponding test cases as new use cases come up.

Added Issue #87 to track the exception.