Automapper: Odd behavior in Explicit Expand with inherited models

[JobaDiniz]

The full plug-and-play test can be found here: https://gist.github.com/JobaDiniz/628ceda9133843fad447152f1b9e8cc9 - just add to the test project, run it and you'll see the following results:

There are 2 test classes in the gist. The ExplicitExpansionWithIncludeBase explores the bug. The ExplicitExpansionWithIncludeBaseUsingNewKeyword shows that using the new keyword in the properties of inherited classes makes the tests pass, which is odd.

This comes from 2 .5 years ago and I strongly believe it's a bug in AutoMapper.
It seems that AutoMapper expands members regardless their depth in the object graph, which in my opinion is wrong.

Source/destination types

/*********** Source Types ***********/
        abstract class EntityBase
        {
            public EntityBase() => Id = Guid.NewGuid();
            public Guid Id { get; set; }
            public User CreatedBy { get; set; }
            public User ModifiedBy { get; set; }
        }

        class User { }
        class Computer : EntityBase { }
        class Script : EntityBase
        {
            public Computer Computer { get; set; }
        }

        /*********** Destination Types ***********/
        class EntityBaseModel
        {
            public Guid Id { get; set; }
            public UserModel CreatedBy { get; set; }
            public UserModel ModifiedBy { get; set; }
        }

        class UserModel { }
        class ComputerModel : EntityBaseModel
        {
            public new UserModel CreatedBy { get; set; } //NOTE: the 'new' keyword here make the tests pass
        }
        class ScriptModel : EntityBaseModel
        {
            public new UserModel CreatedBy { get; set; } //NOTE: the 'new' keyword here make the tests pass
            public ComputerModel Computer { get; set; }
        }

Mapping configuration

 private IConfigurationProvider BuildConfiguration()
        {
            return new MapperConfiguration(cfg =>
            {
                cfg.CreateMap<User, UserModel>();
                cfg.CreateMap<EntityBase, EntityBaseModel>()
                .MaxDepth(1)
                .ForMember(d => d.ModifiedBy, o => o.ExplicitExpansion())
                .ForMember(d => d.CreatedBy, o => o.ExplicitExpansion());
                cfg.CreateMap<Computer, ComputerModel>()
                .MaxDepth(1)
                .ForMember(d => d.ModifiedBy, o => o.ExplicitExpansion())
                .ForMember(d => d.CreatedBy, o => o.ExplicitExpansion());
                cfg.CreateMap<Script, ScriptModel>()
                .MaxDepth(1)
                .ForMember(d => d.Computer, o => o.ExplicitExpansion())
                .ForMember(d => d.ModifiedBy, o => o.ExplicitExpansion())
                .ForMember(d => d.CreatedBy, o => o.ExplicitExpansion());
            });
        }

Version: 9.0.0

Entity Framework EF 6.2.0

Expected behavior

Explicit expand is returning data that was not explicit specified.

Actual behavior

Explicit expand should honor the parameter membersToExpand.

Steps to reproduce

var scriptModel = new[] { script }.AsQueryable()
                .ProjectTo<ScriptModel>(config, c => c.Computer, c => c.CreatedBy).Single(); // Expand Computer and CreatedBy from Script

var scriptModel = new[] { script }.AsQueryable()
                .ProjectTo<ScriptModel>(config, null, "Computer.CreatedBy").Single(); // Expand Computer.CreatedBy from Script

[jbogard]

Oof. Everything looks weird here. My advice is to design your DTOs so that you don’t have to use ‘ExplicitExpansion’. I don’t use that or ‘MaxDepth’ because they’re too confusing for me personally.

Rather than fix this, I’d prefer to remove a feature that I’ve never, nor will ever, use.

[JobaDiniz]

Explicit Expansion is an awesome feature and let me elaborate in what scenario it is used by me and why it wouldn't be feasible to re-design the DTOs and why AutoMapper shouldn't remove this feature.

We have a WebApi 2.0 targeting .net Framework 4.6.2. In this API we allow clients to fetch the main resource and optionally its relationships, but not all at once, selectively.
In order to accomplish this task, some routes have string include query string parameter and clients can include relations by comma: Computer.CreatedBy,OtherRelation for example.
We can have several relations and several routes with several DTOs. It wouldn't be feasible to create a DTO for each combination of the relationships they have.

When the request is made, we split the include by comma and pass it to the ProjectTo membersToExpand parameter. This will trigger AutoMapper to build a query only with the tables and fields that are asked for, improving query performance. We could instead use standard Entity Framework Include but this will fetch all the columns in the included relationship; the beauty of ProjectTo + explicit expand is that it will only fetch the columns needed, defined in the mapper configuration.

The DTO returned from the WebApi are the same, with all relations explicitly defined in the DTO, but we only fill the ones the client specifies. The ones they do not want are not even returned in the response (event thou the property exist in DTO) because JSON serializer takes care of ripping off null properties from the response.

[jbogard]

In that case, I think it'll be up to you to figure out how to address this issue. The MaxDepth stuff obfuscates things here though - that is supposed to affect recursive models (again, something I'd recommend against).

My guess is that we changed IncludeBase for inheriting configuration, to make it opt-in. I'd look there first? But you'll probably need to debug the expression tree generation to see what's going on here.

[jbogard]

Even more reason that I have no idea what the issue is, and since I don’t use and never have used the feature in any of my projects or clients, have zero motivation to fix it myself. But I’d welcome a PR if you figure out what is going on here.

[BlaiseD]

Looks like the code is consistent in getting members from the declared type. So here:
```c#
[Fact]
public void ComputerCreatedBy_should_be_null()
{
// arrange
var config = BuildConfiguration();
var script = CreateModel();

        // act
        var scriptModel = new[] { script }.AsQueryable()
            .ProjectTo<ScriptModel>(config, c => c.Computer, c => c.CreatedBy).Single();

        // assert
        Assert.NotNull(scriptModel.CreatedBy);
        Assert.Null(scriptModel.Computer.CreatedBy); 
    }

`Assert.NotNull` is in conflict with `Assert.Null`.  They're both referring to `EntityBaseModel.CreatedBy`.

### Proof:
```c#
//In MemberVisitor
        //protected override Expression VisitMember(MemberExpression node)
        //{
        //    _members.AddRange(node.GetMemberExpressions().Select(e => e.Member));
        //    return node;
        //}

        protected override Expression VisitMember(MemberExpression node)
        {
            _members.AddRange
            (
                node.GetMemberExpressions().Select
                (
                     //This change is hacky- just here to prove out a solution
                     //We'll probably want to return an object with MemberInfo and ParentType.
                    e => e.Expression.Type.GetMember(e.Member.Name).First()
                    //Get the member of the parent expression so the reflected type is the concrete type
                )
            );

            return node;
        }

Same thing in ReflectionHelper:

```c#
public static MemberInfo[] GetMemberPath(Type type, string fullMemberName)
{
return GetMemberPathCore().ToArray();
IEnumerable GetMemberPathCore()
{
MemberInfo property = null;
foreach (var memberName in fullMemberName.Split('.'))
{
var currentType = GetCurrentType(property, type);
//Get the member of the parent expression so the reflected type is the concrete type
yield return property = currentType.GetMember(memberName, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.FlattenHierarchy | BindingFlags.IgnoreCase).First();
//yield return property = currentType.GetFieldOrProperty(memberName);
}
}
}

In ExpressionBuilder, also extend the matching to include the concrete parent types:
```c#
                foreach (var propertyMap in typeMap.PropertyMaps
                                                   .Where(
                                                            pm =>
                                                            (
                                                                !pm.ExplicitExpansion
                                                                || request.MembersToExpand.Any
                                                                    (//can't use Contains because we're not
                                                                     //changing PrpertyMap.DestinationMember
                                                                        m => m.GetMemberType() == pm.DestinationMember.GetMemberType() 
                                                                        && m.Name == pm.DestinationMember.Name 
                                                                        && m.ReflectedType == typeMap.DestinationType
                                                                    )
                                                            )
                                                            && pm.CanResolveValue && ReflectionHelper.CanBeSet(pm.DestinationMember)
                                                         )
                                                   .OrderBy(pm => pm.DestinationName))
                {
                    var propertyExpression = new PropertyExpression(propertyMap);
                    letPropertyMaps.Push(propertyExpression);

                    CreateMemberBinding(propertyExpression);

                    letPropertyMaps.Pop();
                }

                //foreach (var propertyMap in typeMap.PropertyMaps
                //                                   .Where(pm => (!pm.ExplicitExpansion || request.MembersToExpand.Contains(pm.DestinationMember)) &&
                //                                                pm.CanResolveValue && ReflectionHelper.CanBeSet(pm.DestinationMember))
                //                                   .OrderBy(pm => pm.DestinationName))
                //{
                //    var propertyExpression = new PropertyExpression(propertyMap);
                //    letPropertyMaps.Push(propertyExpression);

                //    CreateMemberBinding(propertyExpression);

                //    letPropertyMaps.Pop();
                //}

This works but does break InheritedForPath which I think is probably fine because the test looks incorrect: destination.Nested.NestedTitle is defined in the configuration as a ReverseMap.
c# [Fact] public void Should_work() { var source = new DerivedDataModel() { OtherID = 2, Title2 = "nested test", ID = 1, Title = "test", DescendantField = "descendant field" }; var destination = Mapper.Map<DerivedModel>(source); destination.Nested.NestedID.ShouldBe(2); destination.Nested.NestedTitle.ShouldBeNull(); destination.Nested.NestedTitle2.ShouldBe("nested test"); }

Thoughts?