Automapper: Unexpected behavior for CustomValueResolver

Created on 19 Jun 2017  路  24Comments  路  Source: AutoMapper/AutoMapper

I'm experiencing an unexpected (for me, at least) behavior mapping a DTO to an existing Entity:

Entity ret = Mapper.Map(dto, existingEntity)

I've built a simple (I hope) sample that shows the behavior (using both Automapper 5.2.0 AND 6.1.0).
Sample code here.
Basically, my aggregate is a master with a list of details, where both master and detail entity implements an interface that defines a property (a relation to a separate entity).
This relation is far complicated (not in the sample, of course), so I've defined a CustomMemberValueResolver that targets interface declaration for direct and reverse mappings.

internal class CustomMemberValueDTOResolver 
        : IMemberValueResolver<IWithNestedEntity, IWithNestedDTO, NestedEntity, NestedDTO>
{ ... } // see sample code attached
internal class CustomMemberValueEntityResolver 
        : IMemberValueResolver<IWithNestedDTO, IWithNestedEntity, NestedDTO, NestedEntity>
{ ... } // see sample code attached

Mapper.Initialize(cfg =>
{
    cfg.CreateMap<NestedEntity, NestedDTO>()
        .ReverseMap();

    cfg.CreateMap<MasterEntity, MasterDTO>()
        .ForMember(e => e.Nested,
            expr => expr.ResolveUsing<CustomMemberValueDTOResolver, NestedEntity>(e => e.Nested))
        .ReverseMap()
        .ForMember(e => e.Nested,
            expr => expr.ResolveUsing<CustomMemberValueEntityResolver, NestedDTO>(e => e.Nested));

    cfg.CreateMap<DetailEntity, DetailDTO>()
        .ForMember(e => e.Nested,
            expr => expr.ResolveUsing<CustomMemberValueDTOResolver, NestedEntity>(e => e.Nested))
        .ReverseMap()
        .ForMember(e => e.Nested,
            expr => expr.ResolveUsing<CustomMemberValueEntityResolver, NestedDTO>(e => e.Nested));
});
Mapper.AssertConfigurationIsValid();

The problem is that my resolver for the "ReverseMap" part cannot create a new entity when "DestMember" is null, because DestMember should NEVER BE NULL, since I'm mapping DTO from an existing entity and (for application logic) this code always is provided with a "non-null relation".
What I see from test code provided is that a mapping is called from an object with a null relation, object that actually should never exists (or even better: I'm not able to get in the logic of what happens).
I've digged also source code but actually I'm unable to see who (and where) creates this "empty" object with null relation.

I see that is difficult to explain the problem, hope that sample attached can better explain my problem.
Thanks for any kind of help.

Bug

All 24 comments

Check the execution plan. It has to add a new item to the destination collection. And that item has Nested null. That seems correct.

I'll check, but my "expected behavior" (in my mind) was to update the existing item in the collection, and not to add a new item, actually.
Thanks for pointing me to execution plan, I'm not aware of this stuff.

It doesn't work this way, collections are first cleared. If you use this with EF, you might want to look into AM.Collections. That one tries to do what you expect, matching objects by id.

ah ok, maybe you pointed me in the right direction. actually I missed in the attached sample an important piece of the puzzle, the resolver for the "details" collection that I have in the actual code and matches objects by id.

adding this

internal class DetailsResolver : IValueResolver<MasterDTO, MasterEntity, IList<DetailEntity>>
{
    public IList<DetailEntity> Resolve(MasterDTO dto, MasterEntity entity, 
        IList<DetailEntity> list, ResolutionContext context)
    {
        Console.WriteLine(@"DetailsResolver => DTO: {0}, Entity: {1}, List: {2}",
            dto, entity, list);

        if (dto == null)
            throw new ArgumentNullException("dto");
        if (entity == null)
            throw new ArgumentNullException("entity");
        if (list == null)
            throw new ArgumentNullException("list");

        // TODO: che fa il BaseEntitySetResolver
        IList<DetailEntity> result = new List<DetailEntity>();
        foreach (DetailEntity entityItem in list)
        {
            DetailDTO dtoItem = dto.Details.Single(d => d.Id == entityItem.Id);
            DetailEntity updatedEntityItem = Mapper.Map(dtoItem, entityItem);
            result.Add(updatedEntityItem);
        }
        return result;
    }
}

and related configuration

cfg.CreateMap<MasterEntity, MasterDTO>()
    ///...
    .ReverseMap()
    .ForMember(e => e.Details, expr => expr.ResolveUsing<DetailsResolver>())
    ///...

and my problem disappeared.
so now I need to understand where's the mistake in my actual code.
Thanks anyway a lot for the support.

In your resolver, make sure you're calling "context.Mapper.Map", not Mapper.Map. This ensures if you're using dependency injection it's using the correct resolvers.

AutoMapper.Collections and it's AutoMapper.Collections.EntityFramework were designed to handle this problem of updating Entities from Dtos without having to configure custom value resolvers. It just uses the Key of the Entity from EF to compare itself to Dtos and insert/update/delete accordingly.

Also when removing from navigation collections you should look into this SO Question. TL:DR If you don't make composite key with parent ID, it will orphan item on removing from list instead of deleting it from DB.

Thanks for pointing me to this code. My entities are non ef-related, but I suspect that AM.Collections may be really useful

EqualityComparison will let set up the ID comparison expression between classes. Also there's one for LINQToSQL if you are using that to do updates. Or you can make your own system for finding equality between two classes, by interface properties or what not

sorry for posting here, but I hope to give some help because actually I'm unable to detect what my code (that is still broken) differs from sample code (that works).
Checked execution plan for the part I think is related: full gist here

Plan for sample code:

var resolvedValue = ((IValueResolver<MasterDTO, MasterEntity, DetailEntitySet>)ctxt.Options.CreateInstance<DetailsResolver>()).Resolve(
    src,
    typeMapDestination,
    typeMapDestination.Details,
    ctxt);

var propertyValue = (resolvedValue == null)
    ? {
        if (((ICollection<DetailEntity>)(dest == null) ? null : typeMapDestination.Details) == null)
        {
        }
        else
        {
            (Void)if (((ICollection<DetailEntity>)(dest == null) ? null : typeMapDestination.Details).IsReadOnly)
            {
            }
            else
            {
                ((ICollection<DetailEntity>)(dest == null) ? null : typeMapDestination.Details).Clear();
            }
        }

        new DetailEntitySet();
    }
    : {
        (DetailEntitySet)DetailEntitySet collectionDestination;
        var passedDestination = (dest == null) ? null : typeMapDestination.Details;

        if (passedDestination != null)
        {
            collectionDestination = passedDestination;
            collectionDestination.Clear();
        }
        else
        {
            collectionDestination = new DetailEntitySet();
        }

        var enumerator = resolvedValue.GetEnumerator();
        while (true)
        {
            if (enumerator.MoveNext())
            {
                var item = enumerator.Current;
                collectionDestination.Add((item == null) ? null : (DetailEntity)item);
            }
            else
            {
                break;
            }
        }

        return return collectionDestination;
    };

typeMapDestination.Details = propertyValue;

Plan for actual code:

var resolvedValue =
{
    try
    {
        return (false || (src == null)) ? null : src.Righe;
    }
    catch (NullReferenceException)
    {
        return null;
    }
    catch (ArgumentNullException)
    {
        return null;
    }
};

var propertyValue = (resolvedValue == null)
    ? {
        if (((ICollection<DocumentoCorpoMG>)(dest == null) ? null : typeMapDestination.RigheDocumento) == null)
        {
        }
        else
        {
            (Void)if (((ICollection<DocumentoCorpoMG>)(dest == null) ? null : typeMapDestination.RigheDocumento).IsReadOnly)
            {
            }
            else
            {
                ((ICollection<DocumentoCorpoMG>)(dest == null) ? null : typeMapDestination.RigheDocumento).Clear();
            }
        }

        new DocumentoCorpoMGSet();
    }
    : {
        (DocumentoCorpoMGSet)DocumentoCorpoMGSet collectionDestination;
        var passedDestination = (dest == null) ? null : typeMapDestination.RigheDocumento;

        if (passedDestination != null)
        {
            collectionDestination = passedDestination;
            collectionDestination.Clear();
        }
        else
        {
            collectionDestination = new DocumentoCorpoMGSet();
        }

        var sourceArrayIndex = 0;
        while (true)
        {
            if (sourceArrayIndex < resolvedValue.Length)
            {
                var item = resolvedValue[sourceArrayIndex];
                collectionDestination.Add(
                {
                    DocumentoCorpoMG typeMapDestination;
                    return (item == null)
                        ? null
                        : {
                            typeMapDestination = null ?? new DocumentoCorpoMG();

// lot of code next

Look at the differences:

  1. different iteration mechanism
  2. missing new MyEntity() in the plan for sample code! this break my code.

Any suggestion?
Thanks anyone

As I've said before, if you use AM to map the collection, it will clear it and it will have to create the new items to add in the collection. There is no way around it. The solution is for the constructor to set up everything, so you won't have uninitialized members. Or, as you've done above, map it by hand and add the existing objects to the collection.

Yep you're right, but I cannot use AM.Collections because I have a lot of custom logic code in my entity collections, so I need to make my handler to add existing objects to the collection.
I'm pretty sure I've made a mistake in this mapper, just because I have very different execution plans for code that I suppose is barely the same.

What can I say :) Try to simplify things. Start small and build to your final setup. Or try something outside your app just to see what AM does. Of course the execution plan differs, mainly because the types are different. I don't see anything wrong with what AM does.

You're right, and thanks for support.
My sample code is for the purpose of checking something simpler that my stuff, but it's clear that I'm unable to reply the part that generates the differeces in the execution plan.
As a side node: my code is perfectly fine with AM v5.2.0, I see the problem only with v6.1.0

Bug (I think) replicated!
Now my sample fails with AM 6.1.0 and works perfecty with AM 5.2.0.
This code activated the different behavior

cfg.CreateMap<MasterEntity, MasterDTO>()
                        .ForMember(e => e.Details,
                            expr => expr.MapFrom(dst => dst.DetailsWithAnotherName))

I'm building a sample with the minimum "stuff" that still exibits the different behavior.

here the complete test sample, with both 6.1.0 and 5.2.0 dlls to make the switch.

cfg.CreateMap<MasterEntity, MasterDTO>()
   .ForMember(e => e.Details, expr => expr.MapFrom(dst => dst.Details)) // this code don't work in 6.1.0, but works in 5.2.0
   //.ForMember(e => e.Details, expr => expr.ResolveUsing<DetailDTOSetResolver>()) // this code works in 5.2.0 and 6.1.0
   // NOTE: uncommenting both lines works in 5.2.0 and 6.1.0
   .ForMember(e => e.Nested, expr => expr.ResolveUsing<CustomMemberValueDTOResolver, NestedEntity>(e => e.Nested))

as reported in the comments, different behavior is shown when explicit MapFrom is used.

Try the MyGet build.

I'out for vacation until monday, Hope to try as soon as possible. Thanks.

looks ok with 6.1.1-ci-01336

Cool.

when 6.1.1 release is planned?

The next NuGet release is months away.
There's no difference in the deployments of MyGet vs NuGet, same amount of testing goes into each build. NuGet is all about locking down the version.

I'd like to get the 6.1.1 release out a bit sooner as a bugfix release for whatever we broke in 6.1.0 :)

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.

Was this page helpful?
0 / 5 - 0 ratings