ShouldUseConstructor is ignored at runtime

[Dresel]

Hi,

we have issues preventing AutoMapper to use private / internal constructors. Consider the following simplified example:

Source/destination types

public class Source
{
}

public class Destination
{
}

Mapping configuration

var config = new MapperConfiguration(cfg =>
{
    cfg.ShouldUseConstructor = c => false;
    cfg.CreateMap<Source, Destination>();
});

Version: 10.1.1

Expected behavior

mapper.Map<Destination>(new Source()) should throw an AutoMapperConfigurationExpression (No available constructor) exception.

Actual behavior

config.AssertConfigurationIsValid() throws as expected. This is what got tested in the unit tests.

mapper.Map<Destination>(new Source()) does still uses any ctor it finds.

Steps to reproduce

class Program
{
    static void Main(string[] args)
    {
        var config = new MapperConfiguration(cfg =>
        {
            cfg.ShouldUseConstructor = c => false;
            cfg.CreateMap<Source, Destination>();
        });

        //config.AssertConfigurationIsValid(); // This throws AutoMapperConfigurationExpression
        var destination = config.CreateMapper().Map<Destination>(new Source());
    }

    class Destination
    {
    }

    public class Source
    {
    }
}

[lbargaoanu]

That test shows the initial intent. I guess it's debatable.

[Dresel]

I see. Wasn't aware that AssertConfigurationIsValid is considered mandatory. Thanks for the explanation and your time.

[lbargaoanu]

The initial discussion starts here #2698 (comment). I guess I see it the same way now, not a big deal. But if you feel strongly about it, @jbogard will probably accept a PR. Assuming the changes are trivial :)

[fiseni]

Hi,

In my opinion, the great benefit of ShouldUseConstructor was the ability to restrict the usage of given constructor types, more specifically the ability to exclude the private constructors. In this case, if no potential constructor mappings are found, it still will fall back to the private parameterless constructor (assuming it exists), which kind of defies the purpose 😞 What's the community consensus here? Is this behavior desirable?

Indeed, AssertConfigurationIsValid would catch this. But, even if by any chance you missed it, is it better to have a runtime exception rather than ending up with some inconsistent state?

Let me put a simple example

using AutoMapper;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using System;

namespace AutomapperSample
{
    public class Program
    {
        static void Main(string[] args)
        {
            var host = Host.CreateDefaultBuilder()
            .ConfigureServices((context, services) =>
            {
                services.AddScoped<MyApp>();
                services.AddAutoMapper(config =>
                {
                    config.ShouldUseConstructor = x => !x.IsPrivate;
                }, typeof(Program).Assembly);
            }).Build();

            using (var scope = host.Services.CreateScope())
            {
                var myApp = scope.ServiceProvider.GetRequiredService<MyApp>();

                myApp.Run();
            }
        }
    }

    public class MappingProfile : Profile
    {
        public MappingProfile()
        {
            ShouldUseConstructor = x => !x.IsPrivate;

            CreateMap<Bar, Foo>();
        }
    }

    public class MyApp
    {
        private readonly IMapper _mapper;

        public MyApp(IMapper mapper)
        {
            this._mapper = mapper;
        }

        public void Run()
        {
            var bar = new Bar { Name = "Test" };
            var foo = _mapper.Map<Foo>(bar);
        }
    }

    public class Bar
    {
        public string Name { get; set; }
    }

    public class Foo
    {
        public string Name { get; private set; }

        // Used specifically for EF, and should not be utilized for anything else.
        private Foo()
        {
            Console.WriteLine("The parameterless ctor is called");
        }

        // Just force the mapping to fail here
        public Foo(SomeComplexType someComplexType)
        {
            Console.WriteLine("The ctor with parameters is called");
        }
    }

    public class SomeComplexType { }
}

[lbargaoanu]

Not validating configuration is a capital offense :)

[fiseni]

Ok, I have no reply in that case 😄