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

Support ignoring nested properties #396

Closed
TimothyMakkison opened this issue May 1, 2023 · 1 comment
Closed

Support ignoring nested properties #396

TimothyMakkison opened this issue May 1, 2023 · 1 comment

Comments

@TimothyMakkison
Copy link
Collaborator

The problem
Mapperly currently supports defining nested mapping properties, however it does not support ignoring nested members. In some cases this can break logic or create unwanted warnings that cannot be suppressed via mapperly.

// Here we want to ignore sources Id and UnmappedMember, 
// the target Id will be set via a different explicit mapping and UnmappedMember does not have a corresponding member.
public class NestedChild
{
    // Other members...
    public int Id { get; set; }

    public string UnmappedMember { get; set; }
}
public class NestedChildDto
{
    // Other members
    public Guid Id { get; set; }
}

// Example of nested explicit mapping
[MapProperty("Manufacturer.MyID", "Producer.Id"] // Map property with a different name in the target type
public static partial CarDto MapCarToDto(Car car);

Describe the solution you'd like
MapperIgnoreTarget and MapperIgnoreSource should be updated to support a params string similar to [MapProperty]. MappingBodyBuilders should be updated to support nested Ignore. This will likely use some recursion where each step knows its current member handle so it knows which nested ignores it corresponds to.

[MapperIgnoreSource("Producer.Id")]
//or
[MapperIgnoreSource(nameof(CarDto.Producer), nameof(ProducerDto.Id))]

Notes
I suspect this hasn't been done because it could break the resuing of similar mapping methods. If two mappings share a member type but one ingores a property, the other mapping may reuse this mapping resulting in s subtle bug. When a mapping delegate is created it should record what configurations were used to create it. When checking if a mapper already exists this can be used to ensure that a compatible mapper is reused. [MapProperty] appears to avoid this by creating the object and then updating properties via accessible setters.
I haven't confirmed if this would be a problem. It would require a lot of work to fix 😄

// Here MapToTailDto is created where Bushiness is not mapped over
[MapperIgnoreTarget("Tail.Bushiness")]
public static partial CatDto Map(Cat car);

// MapToTailDto is reused and Bushiness will not be mapped
public static partial DogDto Map(Dog car);

This would likely have to be done when doing #103 and #349 as all require reworking the mapping logic

@latonz
Copy link
Contributor

latonz commented May 1, 2023

Mapperly does only have limited support for multiple mapping configurations for the same object type mapping and there are no immediate plans to change this. What currently is supported is creating multiple user defined mappings with different configurations, but if a nested object is mapped, the first found mapping is re-used. If an object should be mapped with an entire different configuration, the approach of Mapperly is to create another mapper class. Pretty much the same applies to the ignored properties. For MapPropertyAttribute this is currently possible, however, this was implemented in the past "by accident" and will probably be deprecated in the future as this only creates confusion and the benefit isn't really big. To get the same result an additional mapping definition can be created:

[Mapper]
public MyMapper
{
    [MapProperty("Value.V1", "Value.V2")]
    public B Map(A source);
}

// leads to the same mappings as
[Mapper]
public MyMapper
{
    public B Map(A source);
    
    [MapProperty("V1", "V2")]
    private D Map(C source);
}

record A(D Value);
record B(C Value);
record D(int V1);
record C(int V2);

I don't think #349 does need bigger refactoring on the current architecture. These are just partial methods, of which the implementation will be generated after all mappings are generated. The content of the partial method could be generated based on the user defined mappings. I'm already working on this.
#103 may need some more work, as Mapperly currently is designed to only map from one type to another.

@TimothyMakkison TimothyMakkison changed the title Support ingoring nested properties Support ignoring nested properties May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants