Automapper: Nullable properties on source being cast before conditional function called

[CicerosPatience]

I'm writing a restful api and in my use case I am trying to map a nullable type on my source to a non-nullable property on my destination. I want it to ignore mapping properties if the source value is null. I want this because it is a really elegant solution for me to deserialize a partial json request into my object and only update fields on my entity object that were actually passed to my dto. See my code below (note, I've removed all my Entity Framework junk).

These are my source (ScriptDto) and destination (Script) classes.

public class ScriptDto
    {
        public Guid Id { get; set; }
        public string Name { get; set; }
        [JsonConverter(typeof(StringEnumConverter))]
        public ScriptLanguage? Language { get; set; }
        public string Target { get; set; }
        public string ScriptBody { get; set; }
        public DateTime? LastExecuted { get; set; }
        public Guid? LastExecutedBy { get; set; }
    }

 public class Script : IEntity
    {
        public Guid Id { get; private set; }
        public string Name { get; set; }
        public ScriptLanguage Language { get; set; }
        public string Target { get; set; }
        public string ScriptBody { get; set; }
        public DateTime LastExecuted { get; set; }
    }

This is my web api routed method

[HttpPut("{id}")]
public IActionResult UpdateScript(string id, [FromBody]ScriptDto scriptDto, [FromServices]IDatabaseConfigurationLoader loader)
   {
                Script script = //code to get my script object from the database
                if (script == null) { return NotFound(); }

                 var config = new MapperConfiguration(cfg => {
                    cfg.CreateMap<ScriptDto, Script>()
                       .ForMember(y => y.Id, opt => opt.Ignore())
                       .ForAllOtherMembers(opt =>
                       {
                           opt.Condition((src, dest, srcValue) =>
                           {
                               return srcValue != null; //ignore properties on dto that are null
                           });
                       });
                   });

                _mapper = config.CreateMapper();
                //map dto to entity
                _mapper.Map(dto, script);

                //save changes to database

                return new ObjectResult(MapToDto(script)); //return updated object
   }

What I would expect to happen with this, is if the source property is null the condition function evaluates to false and the property mapping is skipped. That is not what is happening for the nullable properties on my source object. After setting a break point inside the conditional function I realized that my source property values were being cast to the destination type (I assume) before they are passed to the conditional function. So where I would expect "srcValue" to be null it was actually already cast to its non-nullable default. If for example, I sent the following json PUT request to my service at api/v1/scripts/somescriptid

{ "scriptBody": "test body" }

then it would deserialize into a DTO object with mostly null properties, with the exception of the ScriptBody property. But when it gets mapped to my destination object, the DateTime? LastExecuted property which is null (for example) has its value cast to a non-nullable DateTime prior to being passed to the conditional function. Which of course causes my conditional function to evaluate to true and thus the destination property is overwritten.

I imagine I am having this issue because I am trying to map a nullable property on my source to a non-nullable property on my destination. When I change the property on my destination object to a nullable type then it works the way I would expect. That said, I do not want nullable types where they aren't required on my entity object. I'm using the latest dev build on myget, 5.2.0-alpha-01223.

[lbargaoanu]

I don't think this is going to happen. It might break a lot of people.

[CicerosPatience]

I get that it is probably a breaking change, but I also think its the correct way to do this. The only value I see to casting the source property before its passed into the conditional function is that it validates that the properties to be mapped are of the same type. If that's necessary, it can certainly be handled in a better way, either as a separate option or by the developer handling it inside the conditional function.

Alternatively, I would suggest adding a separate function to options that allows this to be done. As it is, there is no other way that I am aware of to accomplish this use case with automapper.

[lbargaoanu]

You can check against the default value for the value type, but that's not exposed right now through the interface, so you need a cast.
Update : no need for a cast anymore.

        cfg.CreateMap<Source, Destination>()
                .ForAllOtherMembers(opt =>opt.IgnoreSourceWhenDefault());

public static class Extensions
{
    public static void IgnoreSourceWhenDefault<TSource, TDestination>(this IMemberConfigurationExpression<TSource, TDestination, object> opt)
    {
        var destinationType = opt.DestinationMember.GetMemberType();
        object defaultValue = destinationType.GetTypeInfo().IsValueType ? Activator.CreateInstance(destinationType) : null;
        opt.Condition((src, dest, srcValue) =>!Equals(srcValue, defaultValue));
    }

    public static Type GetMemberType(this MemberInfo memberInfo)
    {
       if (memberInfo is MethodInfo)
           return ((MethodInfo)memberInfo).ReturnType;
       if (memberInfo is PropertyInfo)
           return ((PropertyInfo)memberInfo).PropertyType;
       if (memberInfo is FieldInfo)
           return ((FieldInfo)memberInfo).FieldType;
       return null;
    }
}

[lbargaoanu]

@jbogard Maybe include this in AM? People seemed to want similar things before.

[jbogard]

What's not exposed? Just the member?

[lbargaoanu]

Yes, the destination member type. But the final code doesn't look that bad :)

[alex-yakovenko]

This bug appears when you use retrieved via NuGet instance of AutoMapper.dll. In this case added by Condition method expression obtains wrong value in sourceValue parameter. The bug disappeared after I rebuilt AutoMapper on local machine.

Here is test code which I use to reproduce the bug:

public class ConditionalParams
    {

        public interface ITestEntity
        {
            int ValueX { get; set; }
        }

        public class TestClassA : ITestEntity
        {
            private int _valueX;

            public int ValueX
            {
                get { return _valueX; }
                set { _valueX = value; }
            }

        }

        public class TestClassB : ITestEntity
        {
            private int _valueX = 7;

            public bool Updated = false;

            public int ValueX
            {
                get { return _valueX; }
                set
                {
                    _valueX = value;
                    Updated = true;
                }
            }

        }

        [Test]
        public void TestMapper()
        {

            Mapper.Initialize(cfg =>
            {
                cfg.CreateMap<ITestEntity, TestClassB>().MapOnlyIfDirty();
            });

            var a = new TestClassA
            {
                ValueX = 7
            };

            var b = new TestClassB();

            Mapper.Map(a, b);

            Assert.IsFalse(b.Updated);
        }

    }

    public static class MapperExtensions
    {
        public static IMappingExpression<TSource, TDestination> MapOnlyIfDirty<TSource, TDestination>(
            this IMappingExpression<TSource, TDestination> map)
        {
            map.ForAllMembers(source =>
            {
                source.Condition((s, d, sourceValue, destValue, rc) =>
                {
                    if (sourceValue == null)
                        return destValue != null;
                    return !sourceValue.Equals(destValue);
                });
            });
            return map;
        }
    }

[lbargaoanu]

Most likely #1634. You can use the MyGet build instead.

[ghost]

I'm still seeing this issue with the latest code from github... was there a resolution for it?

[ghost]

My setup is possibly a bit different:

    public class MyClass1
    {
        public int? IntProp { get; set; }
    }

    public class MyClass2
    {
        public int IntProp { get; set; }
    }

    [TestMethod]
    public void TestNullable()
    {
        var config = new MapperConfiguration(cfg =>
        {
            cfg.ForAllPropertyMaps(pm => true,
                (pm, c) =>
                {
                    c.Condition((s, d, sVal) => sVal != null);
                });

            cfg.CreateMissingTypeMaps = true;
        });

        var mc1 = new MyClass1
        {
            IntProp = null
        };

        var mc2 = new MyClass2
        {
            IntProp = 1
        };

        var mapper = config.CreateMapper();
        mapper.Map(mc1, mc2);
    }

In the Condition, sVal evaluates to 0 for IntProp (which is null).
It's kind of messy to set up the condition that way, but it's needed for this project, unfortunately. I'm trying to upgrade from 4.1, I think, to 5.2. There's heavy reliance on being able to build maps on-demand instead of as a single setup step (hence having to set CreateMissingTypeMaps).

[lbargaoanu]

The solution is above. CreateMissingTypeMaps is gonna get you into trouble eventually.

[ghost]

I can't check for the default value, because the default value is a legitimate value. That's why the property is Nullable. I need to prevent mapping null values for Nullable properties from the source. Note that this only seems to occur when mapping a Nullable to a non-Nullable (int? to int in my case). Nullable to Nullable works.

[lbargaoanu]

Then I guess you should try something else.

[lbargaoanu]

Or a PR. @jbogard seems OK with it.

[ghost]

I'd like to create a configuration option along the lines of "SkipNullSourceValues" rather than changing the behavior of Condition.

[lbargaoanu]

I don't like it. It could be an extension method implemented with conditions. @jbogard might disagree.

[ghost]

Unfortunately, this can't be implemented using the existing Condition method. The behavior of Condition needs to change to not cast Nullable's to the destination non-Nullable type. At least for my scenario, that's what needs to happen....

[lbargaoanu]

No, what the man said in the issue was to add another overload that received the source value before resolving.

[ghost]

Ah, in the other issue. This comment had an extension method in it as well, so that's what I first thought about. Sorry about that.... I will read the other issue in more depth.

[lbargaoanu]

An idea for a possible workaround.
```
cfg.ForAllPropertyMaps(pm => true,
(pm, c) =>
{
c.ResolveUsing(new Resolver(), pm.SourceMember.Name);
});

class Resolver : IMemberValueResolver
{
public int Resolve(object source, object destination, int? sourceMember, int destinationMember, ResolutionContext context)
{
return sourceMember ?? destinationMember;
}
}

[ghost]

Nice! Thanks for the suggestion! It works until I tried adding another property to the models:

    public class MyClass1
    {
        public int? IntProp { get; set; }
        public string StrProp { get; set; }
    }

    public class MyClass2
    {
        public int IntProp { get; set; }
        public string StrProp { get; set; }
    }

I had to add a filter for the PropertyMap:

    cfg.ForAllPropertyMaps(pm => pm.SourceType == typeof(Nullable<int>),
    (pm, c) =>
    {
        c.ResolveUsing<object, object, int?, int>(new Resolver(), pm.SourceMember.Name);
    });

Which means I'll need to add resolvers for all the various Nullable types. Not a pretty solution, but it at least allows us to move forward for right now.
And then hope that at some point in the future we actually have time to clean up the model and create proper profiles and maps for everything.

[lbargaoanu]

Well, you can make one generic resolver and create it with reflection, but yes, it's not pretty :)

[lbargaoanu]

There is an even simpler version which probably does a lot more boxing.

    var config = new MapperConfiguration(cfg =>
    {
        cfg.ForAllPropertyMaps(
            pm => Nullable.GetUnderlyingType(pm.SourceType) == pm.DestinationPropertyType,
            (pm, c) => c.ResolveUsing<object, object, object, object>(new Resolver(), pm.SourceMember.Name));
    });

    class Resolver : IMemberValueResolver<object, object, object, object>
    {
        public object Resolve(object source, object destination, object sourceMember, object destinationMember, ResolutionContext context)
        {
            return sourceMember ?? destinationMember;
        }
    }

[ghost]

Ah, nice... I'll give that a try! At this point type-casting/boxing performance issues aren't much of a concern for me....

[michaelaird]

I'm not sure if this is the same issue but I'm getting a similar problem with accessing collections even though the condition is not met.

This worked in 5.1.1

.ForMember(model => model.EmergencyContact1FullName, opt =>
                {
                    opt.Condition(source => source.EmergencyContacts.Count > 0);
                    opt.MapFrom(source => source.EmergencyContacts[0].FullName);
                })

but throws "Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index" in 5.2 (and 5.3).

If this is not the same issue, I can open another issue with full repro code.

[lbargaoanu]

@michaelaird I think it's the same, try a precondition.

[michaelaird]

Thanks @lbargaoanu ! PreCondition seems to work perfectly. I couldn't find anything in the documentation describing PreCondition vs Condition.

[lbargaoanu]

Well, this is the explanation. So, precondition, resolve, condition and map.

[hmvs]

So i have the same situation as @markjhvt and @YumsTheGreat. We are doing Web API, and nullvalues just indicates that value is not passed and we should not update it.

So as i understand the workaround is:
https://github.com/AutoMapper/AutoMapper/issues/1703#issuecomment-265670132

But as i understand in this way this configuration will work for all entities so in this case we should have two separate configurations for DTOs that should not touch null properties and usual one that updates null properties.

The question is:
Is this functionality is planned for per entity configuration.
Like we had in version 4.2.1
configuration.CreateMap<MyModelPatch, MyModel>().ForAllMembers(_ => _.Condition(con => con.IsSourceValueNull == false));

[lbargaoanu]

That should work now. Also ForAllPropertyMaps has a filter you can use to achieve the same thing. A precondition would receive the original value before resolving. Choose what fits best :)

[lbargaoanu]

I've updated the docs.

[hmvs]

@lbargaoanu
I have a bit different situation i have

class Foo{
  Guid? baz;
...
}

class Bar { 
  Guid baz; 
...
}

I want to be able to map all the properties which are not null. But i want to do it for specific Types. But i don't want to specify properties one by one.
In previous version i was able to do it via:
configuration.CreateMap<Foo, Bar>().ForAllMembers(_ => _.Condition(con => con.IsSourceValueNull == false));

In current version in conditions source value for Guid? is default(Guid). But for me default(Guid) is a signal that i should set it to exact value (empty guid).

[lbargaoanu]

As I said, a precondition :)

[hmvs]

@lbargaoanu Thanks. Could you please show an example of precondition for all properties?

[lbargaoanu]

configuration.CreateMap<Foo, Bar>().ForAllMembers(_ => _.PreCondition(con => con.IsSourceValueNull == false));

[hmvs]

Strange, i can't see IsSourceValueNull. Do we have it in 5.2.0?

[lbargaoanu]

:) No, I was trying to show how it's similar to Condition.

[hmvs]

Amm... :) I don't get it. So how can i use it if we do not have this function? :) I see how it can be used to ignore nullvalues by specifying all the properties manually. But it's not what i want to do.

[lbargaoanu]

Check the extension method above for inspiration.

[lbargaoanu]

I guess a precondition won't work because you don't have an overload that takes the source property value. So you're back to the original solution, ForAllPropertyMaps with extra filters or the same idea with ForAllMembers.

[lbargaoanu]

It shouldn't be difficult to add the missing PreCondition overload, it's similar to Condition.

[krishnandusarkar]

@lbargaoanu Unfortunately https://github.com/AutoMapper/AutoMapper/issues/1703#issuecomment-265670132 is not working for me.

Even though sourceMember is null but in the end it evaluates to 0. Any idea what's might be wrong?

[lbargaoanu]

A repro would help. Make a gist that we can execute and see fail.

[krishnandusarkar]

@lbargaoanu Well, it's working for int? and bool? but not for string, if I don't send the string value from WebAPI. The incoming model shows null, but after mapping, it maps the destination to null too. Any idea how to avoid that?

[lbargaoanu]

Strings have nothing to do with that comment. Null maps to null, unless AllowNullDestinationValues is false.

[krishnandusarkar]

@lbargaoanu Ahhh..what I mean is, if I don't send any values i.e. it's null then it should not map to destination property. So the solution above suggested by you is working for int? and bool? but if I don't send a string property, it's overriting the destination member property with null which is what I don't want.

[lbargaoanu]

You need a condition for that. The comment was only for that particular case. Cargo cult programming won't get you anywhere.

[krishnandusarkar]

@lbargaoanu Well, this is what I was asking. What kind of condition should I put in there? Do I need another resolver for string? Or I can plug the code in same Resolver. Sorry, but I do not get the approach.

[levitatejay]

Here is a solution that works with ForAllMembers if all the properties have the same names between objects

    class IgnoreNullSourceValues : IMemberValueResolver<object, object, object, object>
    {
        public object Resolve(object source, object destination, object sourceMember, object destinationMember, ResolutionContext context)
        {
            return sourceMember ?? destinationMember;
        }
    }

and use it like so:

cfg.CreateMap<SourceObject, DestObject>()
                .ForAllMembers(a => a.ResolveUsing<IgnoreNullSourceValues, object>(a.DestinationMember.Name));

[GFoley83]

@levitatejay How would I update your Resolve method to allow for the scenario when there's no destination / source member to map to? E.g. if my source object doesn't have a property that the destination has.

[petrosmm]

in https://github.com/AutoMapper/AutoMapper/issues/1703#issuecomment-265670132 @lbargaoanu used Nullable.GetUnderlyingType(pm.SourceType) why would he/she have needed to use that method instead of just using pm.SourceType; I am not sure I understand the intent by it? Also if I used pm.SourceType.UnderlyingSystemType instead what issue could I run into?

[lbargaoanu]

Because this happens only in this particular case, for the rest, the source member is null as expected.

[Yon2068]

This is an extension

Works for all Nullable types

How to use it?

AutoMapper.Mapper.Initialize(cfg => { cfg.IgnoreSourceWhenNull(); });

Here's the extension

AutoMapperExtensions.cs.zip

[lbargaoanu]

You don't need more than one instance of that resolver. You can create it in your extension and pass that instance.

[Yon2068]

@lbargaoanu
With this extension:
ObjectId can't convert to string
How do I do that?

[lbargaoanu]

I have no idea what you're saying.

[Yon2068]

@lbargaoanu
System.InvalidCastException : Unable to cast object of type 'MongoDB.Bson.ObjectId' to type 'System.String'.

[lbargaoanu]

That's most likely an issue with your own code.

[Yon2068]

I using ForAllPropertyMaps to ignore all Nullable<>
ForAllPropertyMaps()

```c#
var isNullable = pm.SourceType.IsGenericType
&& (pm.SourceType.GetGenericTypeDefinition() == typeof(Nullable<>));
return isNullable || pm.SourceType.IsClass || pm.SourceType.IsValueType || pm.SourceType.IsPrimitive;

**Resolve()**
```c#
return sourceMember ?? destinationMember;

[lock[bot]]

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.