Mapping of conditional expressions throws a null reference exception

[Erythnul]

Hi,

We are using your expression mapping to translate our mapping expressions from our DTO definitions into our entity definitions, however, we're running into a problem. This is

We are mapping a conditional expression on a nested class, as shown in the following code:

    public class TestEntity
    {
        public Guid Id { get; set; }
        public int ConditionField { get; set; }
        public int ToBeNestedField { get; set; }
    }

    public class TestDTO
    {
        public Guid Id { get; set; }
        public TestDTONestedClass NestedClass { get; set; }
    }
    public class TestDTONestedClass
    {
        public int? NestedField { get; set; }
    }

    public class TestMapperProfile : Profile
    {
        public TestMapperProfile()
        {
            CreateMap<TestEntity, TestDTO>()
                .ForMember(dst => dst.Id, opt => opt.MapFrom(src => src.Id))
                .ForMember(dst => dst.NestedClass, opt => opt.MapFrom(src => src.ConditionField == 1 ? src : default));

            CreateMap<TestEntity, TestDTONestedClass>()
                .ForMember(dst => dst.NestedField, opt => opt.MapFrom(src => (int?)src.ToBeNestedField));
        }
    }

    [TestMethod]
    public void MapExpressionOnNestedClass()
    {
        var config = new MapperConfiguration(cfg =>
        {
            cfg.AddProfile(new TestMapperProfile());
            cfg.AddExpressionMapping();
        });

        var mapper = new Mapper(config);

        Expression<Func<TestDTO, bool>> expr = x => x.NestedClass.NestedField == 1;

        var mappedExpression = mapper.Map<Expression<Func<TestEntity, bool>>>(expr); //throws

        Assert.IsNotNull(mappedExpression);
    }

When the code arrives at the GetMemberFullName() method, it tries to cast the Conditional Expression to a MemberExpression, which results in a null reference exception, as shown below:

	public static string GetMemberFullName(this LambdaExpression expr)
	{
		if (expr.Body.NodeType == ExpressionType.Parameter)
			return string.Empty;
	
		MemberExpression me;
		switch (expr.Body.NodeType) // NodeType == ExpressionType.Conditional
		{
			case ExpressionType.Convert:
			case ExpressionType.ConvertChecked:
				me = (expr.Body as UnaryExpression)?.Operand as MemberExpression;
				break;
			default:
				me = expr.Body as MemberExpression; //Results in me == null
				break;
		}
	
		return me.GetPropertyFullName();
	}
	
	public static string GetPropertyFullName(this Expression expression)
	{
		const string period = ".";
	
		//the node represents parameter of the expression
		switch (expression.NodeType) // throws NullReferenceException
		{
			case ExpressionType.MemberAccess:
				var memberExpression = (MemberExpression)expression;
				var parentFullName = memberExpression.Expression.GetPropertyFullName();
				return string.IsNullOrEmpty(parentFullName)
					? memberExpression.Member.Name
					: string.Concat(memberExpression.Expression.GetPropertyFullName(), period, memberExpression.Member.Name);
			default:
				return string.Empty;
		}
	}

Returning an empty string (as would be the default in GetPropertyFullName) seems to work well, but I can't tell if this would break anything else because of my lack of knowledge about the rest of the code in this package.

In our case, I think we're expecting the input expression to be mapped as follows:

Input: x => (x.NestedClass.NestedField == Convert(1))

Output: x => (Convert(x.ToBeNestedField) == Convert(1))

Would love to hear any advice! Thanks in advance.

[BlaiseD]

Your mapping is correct:

It can be

            CreateMap<TestEntity, TestDTO>()
                .ForMember(dst => dst.Id, opt => opt.MapFrom(src => src.Id))
                .ForMember(dst => dst.NestedClass, opt => opt.MapFrom(src => src));

            CreateMap<TestEntity, TestDTONestedClass>()
                .ForMember(dst => dst.NestedField, opt => opt.MapFrom(src => (int?)src.ToBeNestedField));

OR

            CreateMap<TestEntity, TestDTO>()
                .ForMember(dst => dst.Id, opt => opt.MapFrom(src => src.Id))
                .ForPath(dst => dst.NestedClass.NestedField, opt => opt.MapFrom(src => src.ConditionField == 1 ? (int?)src.ToBeNestedField : (int?)default));

but please leave this open. It may be possible to throw a better exception message.

[Erythnul]

Unfortunately, using ForPath is not an option for us as we're using ProjectTo<> (which doesn't support ForPath) to build the final queries. The first option works when I change it to be like this (which I think was what you meant):

            CreateMap<TestEntity, TestDTO>()
                .ForMember(dst => dst.Id, opt => opt.MapFrom(src => src.Id))
                .ForMember(dst => dst.NestedClass, opt => opt.MapFrom(src => src));

            CreateMap<TestEntity, TestDTONestedClass>()
                .ForMember(dst => dst.NestedField, opt => opt.MapFrom(src => src.ConditionField == 1 ? (int?)src.ToBeNestedField : default));

It's a shame that we can't put the conditional on the nestedclass itself as, in our real code, we'd be duplicating the condition check many times, for each field in the nested class.
Also, we'd be creating an instance of the nested class where all the contents will be filled with defaults, this might break our API contract on nullability, but we might be able to figure out an alternative way.

Changing it to the above changes the output to be like this:

    x => (IIF((x.ConditionField == 1), Convert(x.ToBeNestedField), null) == Convert(1))

which is definitely a better outcome than the one I had initially put in my issue above.

Would love to see if we can figure out a way to still get closer to the original functionality of the mapper, but this definitely helps a lot, thank you!

[BlaiseD]

I think it may depend on your approach.

Can you post an example of how you're combining mapping expressions with ProjectTo()? Also, where does ForPath cause the failure?

[Erythnul]

The ForPath failure is unrelated to the expression mapping, it's inherent to the AutoMapper ProjectTo usage itself. (also mentioned in the docs (http://docs.automapper.org/en/stable/Queryable-Extensions.html#supported-mapping-options)

What we basically do is like this:

• API receives a request for a list of results with a query-string to filter the list
• Translate the query-string to DTO-modeled filter expressions
• Map the expressions from the DTO models to our Entity models
• Query the database using Entity Framework, IQueryables and ProjectTo, with additional Where statements for each mapped filter expression.

Pseudocode for the database queries:

var query = context.Set<TEntity>.Where(x=>x.Id == id);

foreach (var filterExpr in filterExpressions)
{
    var mappedFilterByExpression = this.mapper.Map<Expression<Func<TEntity, bool>>>(filterExpr);
    query = query.Where(mappedFilterByExpression);
}

return await query.ProjectTo<TModelOut>(this.mapper.ConfigurationProvider).ToListAsync();

This works well as long as the mappings we create are expression mapping compatible, but we're starting to run into more issues when we introduce the nesting.

From the testing I've done so far:

• ForPath works perfectly for Expression mapping, but will return null for the nested class when using ProjectTo
• Explicit nested class mapping as you proposed will work for both, but instantiates a nested class where we would prefer no nested class for our API contract. Theoretically fixable by setting the nested class to null after the projection, though that does result in some clunky code.

[BlaiseD]

Ok I see. i'm going to assume you've already considered mapper.Map<IEnumerable<TData>, IEnumerable<TModel>> but that's also an option using EFCore or EF6.

[Erythnul]

We've used the mapper.Map<IEnumerable, IEnumerable> method in the past, but the great benefit of using projection is the eager loading of specific values from related entities instead of the whole related entity, which we do quite want to keep to optimise the amount of data being pulled from the DB.
The combination of projection and expression mapping does seem to make the available design space for the mapper definitions quite limited though ;)

I didn't know about the OData extension package, interesting to see your solution to these same problems, thanks!

We're currently on EF6, we plan to use Core for future projects now that EFCore 3 is available and to migrate older code somewhere in the future.

I'm going to discuss this with some colleagues to see where we're going to go with our code to get the best results.

[BlaiseD]

I think the approach used in the OData extensions package mapping includes and filter expressions should allow you to apply the specific eager loading to your IQueryable<TData>. Feel free to put a repo demonstrating the challenges.

[BlaiseD]

Sorry I had completely misread your issue. I was answering your question as if your original code was .ForMember(dst => dst.NestedClass, opt => opt.MapFrom(src => src.ConditionField)); (mapping ConditionField to NestedClass - too many long days).

Please try the myGet build.