Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapping with constructor and default values for ignored properties? #707

Open
DumboJetEngine opened this issue May 14, 2024 · 5 comments
Open

Comments

@DumboJetEngine
Copy link

DumboJetEngine commented May 14, 2024

I try to map from class A to class B, where B is missing a property of A.
Usually, to do this you have to use .Ignore(e => e.Property) but I use a non-default constructor for initialization of the destination class (because I need to validate the incoming data within the constructor) and this fails.

Apparently, Mapster removes the ignored parameter from the constructor invocation parameters instead of using the default parameter value. And this inevitably fails.

Here is some sample code:

using Mapster;
using System.Reflection;

var config = new TypeAdapterConfig()
{
    RequireExplicitMapping = false,
    RequireDestinationMemberSource = true,
};

config.Default
    .PreserveReference(true)
    .MapToConstructor(true)
    .Unflattening(false)
    .IgnoreNullValues(false)
    .NameMatchingStrategy(new NameMatchingStrategy
    {
        SourceMemberNameConverter = input => input.ToLowerInvariant(),
        DestinationMemberNameConverter = input => input.ToLowerInvariant(),
    })
    .ShallowCopyForSameType(false);

var ctorToUse = GetConstructor<B>();
config
    .NewConfig<A, B>()
    .Ignore(e => e.Id) // <--------------- The code fails with or without this invocation.
    .MapToConstructor(ctorToUse);

var mapper = new MapsterMapper.Mapper(config);

static ConstructorInfo? GetConstructor<TDestination>()
{
    var parameterlessCtorInfo = typeof(TDestination).GetConstructor(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public, new Type[0]);

    var ctors = typeof(TDestination).GetConstructors(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
    var validCandidateCtors = ctors.Except(new[] { parameterlessCtorInfo }).ToArray();
    var ctorToUse = validCandidateCtors.Length == 1
        ? validCandidateCtors.First()
        : validCandidateCtors.OrderByDescending(c => c.GetParameters().Length).First();

    return ctorToUse;
}


var a = new A
{
    Text = "test",
};

var docKind = mapper.Map<B>(a); // <------ This fails.

class A
{
    public string? Text { get; set; }
}

class B
{
    public int Id { get; private set; }
    public string Text { get; private set; }

    public B(int id, string text)
    {
        Id = id;
        Text = text;
    }
}

My humble opinion is that this code should work and invoke the constructor using the default value for Id, which is default(int), which is 0. Omitting it during the invocation, which is what currently appears to happen, makes little sense to me.

I would also fancy an .Ignore() overload method that allows providing the value for the property, like this one:

config
    .NewConfig<A, B>()
    .Ignore(e => e.Id, 0) // Use value of 0 for id
    .Ignore(e => e.Id2, 0) // Use value of 0 for id2
    .MapToConstructor(ctorToUse);

...but perhaps this is too much to ask.

P.S. :
I know I could use .MapWith(a => new B(0, a.Text)) to manually fix the issue, but I try to keep the mapping as automated as possible, because I need to do this configuration for many types that contain a lot of constructor parameters and this would be messy. So, this is not an option for my case.

@DumboJetEngine
Copy link
Author

DumboJetEngine commented May 15, 2024

I have tried out a fix that seems to work without breaking the existing tests.
I am not an expert on the internals of Mapster, but this is the fix anyway:
image
When you are supposed to use the target class constructor and the source property getter is null, then if the property is ignored I set the getter to return the default value instead of marking it as an unmapped member.
Let me know if this makes any sense code-wise.

Note that, the problem above mostly applies for the case you use RequireDestinationMemberSource = true. I want to use this setting, because I need to know of any unmapped destination members and avoid surprises.

@DocSvartz
Copy link

Hello @DumboJetEngine
In this case A.Adapt(B) where A and B instance of relevant classes
Is your goal to always keep the Id value that is already set in instance B or replace by default from Type Id?

@DocSvartz
Copy link

DocSvartz commented Jan 21, 2025

@DumboJetEngine
use RequireDestinationMemberSource = true
and Use .Ignore(e => e.Id)
You Don't want to get an error that this field was not mapped or ignore?

@DumboJetEngine
Copy link
Author

DumboJetEngine commented Jan 21, 2025

@DocSvartz
It has been some time since I created the issue, and my memory has faded a bit, but from what I remember:
I was using ONLY constructors to initialize the objects, as in the example above.
I had already tried to use both RequireDestinationMemberSource = true and .Ignore(e => e.Id) as shown in the code above.
But the mapping of class A to B was still failing, while I was expecting it to succeed.
The fix shown in the diff above seemed to fix the issue.
You can reproduce the problem with the code I provided.
Just cerate a new .NET 6/8 console app and paste all the code in.

Is your goal to always keep the Id value that is already set in instance B or replace by default from Type Id?

I didn't care much about the preexisting Id value in class B, since the class instance was being created inside Mapster, as in the code.

@DocSvartz
Copy link

DocSvartz commented Jan 21, 2025

@DumboJetEngine Thanks. The example served as a good basis for finding the source of the problem. :)

If this is what you expected. Then most likely in the next releases. It will be added.

[TestMethod]
public void WhenClassIgnoreCtorParamGetDefaultValue()
{
    var config = new TypeAdapterConfig() 
    { 
        RequireDestinationMemberSource = true, 
    };
    config.Default
           .NameMatchingStrategy(new NameMatchingStrategy
            {
                SourceMemberNameConverter = input => input.ToLowerInvariant(),
                DestinationMemberNameConverter = input => input.ToLowerInvariant(),
            })
           ;
    config
        .NewConfig<A707, B707>()
        .MapToConstructor(GetConstructor<B707>())
        .Ignore(e => e.Id);

    var source = new A707 { Text = "test" };
    var dest = new B707(123, "Hello");

    var docKind = source.Adapt<B707>(config);  // No exception 
    var mapTotarget = source.Adapt(dest,config);

    docKind.Id.ShouldBe(0); // default value 
    mapTotarget.Id.ShouldBe(123); // ignore save destination value
    mapTotarget.Text.ShouldBe("test"); // update from source
}

 public class A707
{
    public string? Text { get; set; }
}

public class B707
{
    public int Id { get; private set; }
    public string Text { get; private set; }

    public B707(int id, string text)
    {
        Id = id;
        Text = text;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants