riok / mapperly

A .NET source generator for generating object mappings. No runtime reflection.
https://mapperly.riok.app
Apache License 2.0
2.92k stars 147 forks source link

Mark non-nullable property as nullable #771

Closed rekarpcpro closed 1 year ago

rekarpcpro commented 1 year ago

Hi

I used mapperly recently and fell love with it to be honest. But unfortunately I found a use case where you have to do some manual work to go though it from mapping a db model to a dto.

Problem

Lets say we have this model

class User {
    public string Id{ get; set; } = null!;
    public string Name{ get; set; } = null!;
    public Region Region { get; set; } = null!;  // Another db model
}

and with its corresponding dto

class UserDto {
    public string Id{ get; set; } = null!;
    public string Name{ get; set; } = null!;
    public Region RegionDto { get; set; } = null!;
}

where know all users has region so we don't have to mark it as nullable (this create the problem). But where we use Entity Framework for example, sometime we just need to fetch the user model not including the Region model. In that case the user model's Region is null in that object. So when the mapperly tries to map this object to UserDto, you will get null exception because if the field not marked as nullable (public Region Region { get; set; }) it will not check for nullability.

Workaround

[Mapper]
public static partial class UserMapper
{
    public static UserDto ToUserDto(this User user) // custom mapper wrapper
    {
        var dto = user.ToDto();

        if (user.Region is not null) // annoying warning here
        {
            dto.Region = user.Region .ToRegionDto(); // uses region mapper in another file
        }

        return dto;
    }

    [MapperIgnoreSource(nameof(User.Region))]
    private static partial UserDto ToDto(this User user); // mapperly mapper
}

As you see, you have to ignore the Region field and check it for nullability manually for your self.

I don't have any knowledge in source generation that kind of stuff to provide any solution. But it would be not bad if this is a attribute to tell the mapper that you have to check nullability on a certain field then map it to destination model even if the field not marked as nullable.

TimothyMakkison commented 1 year ago

Hey, pleased to hear your enjoying mapperly. I haven't tested it, but would Mapper(ThrowOnMappingNullMismatch = false) work?

rekarpcpro commented 1 year ago

Thanks for your replay.

Unfortunately no. Because both the DB model and the DTO have Region that are non-nullable, The part of the region mapping that causes the error is that it assumes the Region has value and tries to map it to its DTO.

Generated code:

public static partial global::UserDto Map(this global::User source)
{
    var target = new global::UserDto();
    target.Id = source.Id;
    target.Region = MapToRegionDto(source.Region); // this doesn't check if region is null or not 
    target.Name = source.Name;
    return target;
}

private static global::RegionDto MapToRegionDto(global::Region source)
{
    var target = new global::RegionDto();
    target.Id = source.Id; // this throws NullReferenceException 
    target.Name = source.Name;
    return target;
}

As I said, wouldn't it be good to have an attribute that tells the generator that it has to check the nullability of a non-nullable property?

Thanks for your help.

latonz commented 1 year ago

Probably your Region property is not annotated correctly... That's probably also why your workaround is "annoying". If the property can be null at runtime (the navigation is not fetched in every code path / query), why not mark it as nullable?

Mapperly won't provide attributes to fix code not annotated correctly.