MapsterMapper / Mapster

A fast, fun and stimulating object to object Mapper
MIT License
4.37k stars 332 forks source link

[Feature Request] Expand capability for factory methods #463

Open pdevito3 opened 2 years ago

pdevito3 commented 2 years ago

So I'm seeing a limitation with the current capabilities when using a factory method.

The Problem

Background

Say I have a (simplified) entity that looks like this:

public class Author : BaseEntity
{
    public virtual string Name { get; private set; }

    public static Author Create(AuthorForCreationDto authorForCreationDto)
    {
        var config = new TypeAdapterConfig();
        config.Apply(new AuthorProfile());
        var mapper = new MapsterMapper.Mapper(config);

        return mapper.Map<Author>(authorForCreationDto);
    }

    public void Update(AuthorForUpdateDto authorForUpdateDto)
    {
        var config = new TypeAdapterConfig();
        config.Apply(new AuthorProfile());
        var mapper = new MapsterMapper.Mapper(config);
        mapper.Map(authorForUpdateDto, this);
    }

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

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

    protected Author() { } // For EF + Mocking
}

Given this setup, when I set up my mapper and try to create an Author, I can't because I don't have a default constructor.

public class AuthorProfile : IRegister
{
    public void Register(TypeAdapterConfig config)
    {        
        config.NewConfig<Author, AuthorDto>()
            .TwoWays();
        config.NewConfig<AuthorForCreationDto, Author>()
            .TwoWays();
        config.NewConfig<Author, AuthorForUpdateDto>()
            .TwoWays();
    }
}

Error:

System.InvalidOperationException : No default constructor for type 'Author', please use 'ConstructUsing' or 'MapWith'

The Gap In Mapster

When I try and use 'ConstructUsing' or 'MapWith' like so:

config.NewConfig<AuthorForCreationDto, Author>()
    .ConstructUsing(x => Author.Create(x));

I get an error, presumably because it is recursively trying to use a mapping inside the creation factory and using the factory for the mapping.

Exit code is 134 (Output is too long. Showing the last 100 lines:
at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.Int32, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()

This makes sense given the current setup, but seems to literally block me from leveraging my mappings in my factories. And while the Create factory has a theoretical path, the Update factory is totally blocked as i have no way to construct it.

Discussion

Something like this is doable in Automapper with something like the below (using reflection I think?).

I don't have an immediate idea or recommendation for solving this, but I wanted to present the problem to see if you had any ideas.

Working Automapper

public class Recipe : BaseEntity
{
    public virtual string Title { get; private set; }

    public static Recipe Create(RecipeForCreationDto recipeForCreationDto)
    {
        var mapper = new Mapper(new MapperConfiguration(cfg => {
            cfg.AddProfile<RecipeProfile>();
        }));
        return mapper.Map<Recipe>(recipeForCreationDto);       
    }

    public void Update(RecipeForUpdateDto recipeForUpdateDto)
    {
        var mapper = new Mapper(new MapperConfiguration(cfg => {
            cfg.AddProfile<RecipeProfile>();
        }));
        mapper.Map(recipeForUpdateDto, this);
    }

    protected Recipe() { } // For EF + Mocking
}

public class RecipeProfile : Profile
{
    public RecipeProfile()
    {
        CreateMap<Recipe, RecipeDto>()
            .ReverseMap();
        CreateMap<Recipe, RecipeForCreationDto>()
            .ReverseMap();
        CreateMap<Recipe, RecipeForUpdateDto>()
            .ReverseMap();
    }
}
jeffward01 commented 1 year ago

This would be very cool, I am also experiencing some sort of similar issue with ValueObjects that have static methods.

I use the library Vogen: https://github.com/SteveDunn/Vogen for ValueObjects.

The value objects are structs and can only be constructed like this:


// This is my Value Object
[ValueObject<int>]
public readonly partial struct SupplierId { }

// within some method
// ... code ommited ..
var suppliedId =  SupplierId.From(42);

// ... more code below ...

It would be great if Mapster supported this

andrerav commented 1 year ago

I have some doubts that this is possible with Mapster. The reason is that Mapster must support code generation (with Mapster.Tool), and that places certain restrictions on the expressions you can use in ConstructUsing() and MapWith(). If this is a must have feature then Automapper might be a better fit. I'm happy to hear any ideas or suggestions on how these limitations might be overcome without compromising performance or code generation.

jeffward01 commented 1 year ago

I have some doubts that this is possible with Mapster. The reason is that Mapster must support code generation (with Mapster.Tool), and that places certain restrictions on the expressions you can use in ConstructUsing() and MapWith(). If this is a must have feature then Automapper might be a better fit. I'm happy to hear any ideas or suggestions on how these limitations might be overcome without compromising performance or code generation.

I see your point, and I think the major limitation hurdle here is that Mapster is SourceGenerator.

After some more trials yesterday I learned these things:

Issue

There is no way to 'globally' set a rule / configuration to 'Build' objects of a certain type. Instead, you must call .MapWith(....) each and every single time that you wish to map an object of that particular type.

Example of Issue

So for example, often we build projects that have objects, which inherit from a certain base-type. That base-type has a unique way of construction, and therefore, all the child objects have the same way of being instantiated

Lets look at SmartEnum:

public class MyPoco
{
    public Guid ProductTypeId { get; set; }
}

public class MyDto
{
    // Product type is a smart enum
    public ProductType ProductType { get; set; }
}

public sealed class ProductType : SmartEnum<ProductType>
{
    public static readonly ProductType Digital = new ProductType(nameof(Digital), 1);
    public static readonly ProductType Physical = new ProductType(nameof(Physical), 2);

    private ProductType(string name, int value) : base(name, value) { }
}

To create a type of ProductType, I must always use one of these two methods:

Note: FromName and FromValue are static constructor methods

When working with Mapster, each time I work with a SmartEnum (ProductType for example) I must do this:

config.NewConfig<int, ProductType>().MapWith(x => ProductType.FromValue(x));
config.NewConfig<string, ProductType>().MapWith(x => ProductType.FromName(x));

Proposal Idea

I saw that here, you use Reflection to set a setting for all types which inherit from a target type:

TypeAdapterConfig.GlobalSettings.ForDestinationType<IValidator>()
                    .AfterMapping(dest => dest.Validate());

NOTE: ForDestinationType above will always apply to all types assignable to IValidator. If destination class implements IValidator, it will also apply the AfterMapping config.

What if we applied that same sort of 'config-logic' to mapping?

So the API could look something like:

     TypeAdapterConfig.GlobalSettings
            .ForDestinationType<IValidator>()
            .SetFactory()
            .From<TSource>()
            .MapWith(dest => xxxx)
            .From<TOtherSource>()
            .MapWith(dest => xxxx); 

In this case, instead of IValidator it could be SmartEnum<> or ISmartEnum (Note, I don't think ISmartEnum would work in this particular example due to the lack of FromValue and FromName methods, but you get the point)

Example Useage

So basically, in the global config of Mapster, we would write:

        TypeAdapterConfig.GlobalSettings
            .ForDestinationType<ProductType>()
            .SetFactory()
            .From<string>()
            .MapWith(dest => SmartEnumMapper.Map<ProductType>(_))
            .From<Guid>()
            .MapWith(dest => SmartEnumMapper.Map<ProductType>(_));

And that would be applied globally.

Then to map, I can do something like:

public class MyRegister : IRegister
{
    public void Register(TypeAdapterConfig config)
    {
        // Previously I would need to include this:
        // I realize that currently I can put this in the global config, and it'll work
        // This is not a good example of the feature, but I hope it explains the general concept
        config.NewConfig<string, ProductType>().MapWith(_ => SmartEnumMapper.Map<ProductType>(_));
        config.NewConfig<Guid, ProductType>().MapWith(_ => SmartEnumMapper.Map<ProductType>(_));
        // ^^^^ No longer needed

        // I now only need this
        config
            .NewConfig<MyPoco, MyDto>()
            .Map(dest => dest.ProductType, source => source.ProductTypeId);
    }
}

Maybe this is not the best example because:

Using SmartEnum was not a good choice for an example, because of the static constructor methods restrict the usage of a common interface. However, if there was no static interface methods, then this would be a good example because we can then do:

        TypeAdapterConfig.GlobalSettings
            .ForDestinationType<IMySmartEnumConstructor>()
            .SetFactory()
            .From<string>()
            .MapWith(dest => SmartEnumMapper.Map<IMySmartEnumConstructor>(_))
            .From<Guid>()
            .MapWith(dest => SmartEnumMapper.Map<IMySmartEnumConstructor>(_));

And now this will be applied to ALL types that inherit from IMySmartEnumConstructor

Code to go with the example

I have a helper class I wrote for SmartEnum:

public static class SmartEnumMapper
{
    public static TSmartEnumType Map<TSmartEnumType>(object input)
        where TSmartEnumType : SmartEnum<TSmartEnumType, Guid>
    {
        return CreateSmartEnum(typeof(TSmartEnumType), input);

        TSmartEnumType CreateSmartEnum(MemberInfo sourceType, object? parameter)
        {
            bool isInputAGuidValue = IsInputGuid(input);
            string methodName = isInputAGuidValue ? "FromValue" : "FromName";
            MethodInfo targetMethod = GetFactoryMethod(typeof(TSmartEnumType), methodName);

            return (
                    targetMethod.Invoke(null, new[] { GetParameter() })
                    ?? throw new InvalidOperationException()
                ) as TSmartEnumType
                ?? throw new InvalidOperationException("The conversion was null");

            object GetParameter()
            {
                return input switch
                {
                    string inputStringValue when isInputAGuidValue => Guid.Parse(inputStringValue),
                    Guid guidValue => guidValue,
                    _ => input
                };
            }

            bool IsInputGuid(object? inputParam)
            {
                // return if input is guid
                return inputParam is Guid || Guid.TryParse(inputParam?.ToString(), out _);
            }
        }

        MethodInfo GetFactoryMethod(MemberInfo sourceType, string methodName)
        {
            Type baseSmartEnumObj = GetBaseSmartEnum();

            MethodInfo? targetMethod = baseSmartEnumObj
                .GetMethods(BindingFlags.Public | BindingFlags.Static)
                .FirstOrDefault(_ => _.Name == methodName);
            if (targetMethod == null)
            {
                throw new InvalidDataException(
                    $"We could not locate the method '{methodName}' on the type of {baseSmartEnumObj.ShortDisplayName()
                    }"
                );
            }

            return targetMethod;

            Type GetBaseSmartEnum()
            {
                object smartEnum = FormatterServices.GetUninitializedObject(typeof(TSmartEnumType));

                Type? smartEnumBaseType = smartEnum.GetType().BaseType;
                if (smartEnumBaseType == null)
                {
                    throw new InvalidDataException("The base SmartEnum type could not be found");
                }

                return smartEnumBaseType;
            }
        }
    }
} 

The smart enum class....

// The Smart Enum class
public sealed class ProductType : SmartEnum<ProductType>
{
    public static readonly ProductType Digital = new DigitalProductType();
    public static readonly ProductType Physical = new PhysicalProductType();

    private ProductType(string name, int value) : base(name, value) { }

    private sealed class PhysicalProductType : ProductType
    {
        public PhysicalProductType() : base("Physical", 1) { }
    }

    private sealed class DigitalProductType : ProductType
    {
        public DigitalProductType() : base("Digital", 2) { }
    }
}

Summary

Allow the configuration of MapWith(...) for common types, globally

Regardless that this is not a perfect example, I hope that this helped explain things.

I understand how regarding libraries like Vogen which are source generators, it would be a bit... wonky.... But in this particular example, I don't see how it would be an issue.


@andrerav - what do you think? Sorry for the long example