mapstruct / mapstruct

An annotation processor for generating type-safe bean mappers
https://mapstruct.org/
Other
7.06k stars 946 forks source link

Is there a better way to configure updating existing objects? #2286

Open Sam-Kruglov opened 3 years ago

Sam-Kruglov commented 3 years ago

I wish there would be a central place where I could define mapping configuration for "creating new objects" and for "updating existing objects" without having to repeat myself or spelling out the inconvenient delegation. Currently, I do something like the following except that I extract the configurations into CreatorMapperConfig.class and UpdatorMapperConfig.class using @MapperConfig:

//setters, getters, constructors, etc.
class User {
    String firstName;
    String lastName;
    String email;
    String encodedPassword;
    List<Role> roles;
}

class GetUserDto {
    String email;
    String firstName;
    String lastName;
}

class ChangeUserDto {
    String firstName;
    String lastName;
}

@Mapper(
        componentModel = "spring",
        unmappedTargetPolicy = ReportingPolicy.ERROR,
        typeConversionPolicy = ReportingPolicy.WARN
)
public abstract class UserMapper {

    @Autowired
    protected Updater updater;

    public abstract GetUserDto toGetUserDto(User user);

    public void updateUser(User user, ChangeUserDto changeDto) {
        updater.updateUser(user, changeDto);
    }

    @Mapper(
            componentModel = "spring",
            unmappedSourcePolicy = ReportingPolicy.ERROR,
            unmappedTargetPolicy = ReportingPolicy.IGNORE,
            typeConversionPolicy = ReportingPolicy.WARN,
            collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED,
            nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
            nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS
    )
    interface Updater {

        void updateUser(@MappingTarget User user, ChangeUserDto changeDto);
    }
}
filiphr commented 3 years ago

I am not quite following. What is wrong with using @MapperConfig?

Sam-Kruglov commented 3 years ago

@filiphr nothing is but I think that I will have to always create 2 separate mappers, i.e. one for creating and one for updating, while the settings will always match. So, I wish there would be an option to configure creation once and updating once and so that I could write all method in a single mapper. Something like this:

//setters, getters, constructors, etc.
class User {
    String firstName;
    String lastName;
    String email;
    String encodedPassword;
    List<Role> roles;
}

class GetUserDto {
    String email;
    String firstName;
    String lastName;
}

class ChangeUserDto {
    String firstName;
    String lastName;
}

@Mapper(
    // could do these through CreationMapperConfig.class and UpdateMapperConfig.class that are annotated with @MapperConfig each but I think this is better although I don't mind either way
    creationConfig= @MapperConfig(
        componentModel = "spring",
        unmappedTargetPolicy = ReportingPolicy.ERROR,
        typeConversionPolicy = ReportingPolicy.WARN
    ), 
    updateConfig=@MapperConfig(
        componentModel = "spring",
        unmappedSourcePolicy = ReportingPolicy.ERROR,
        unmappedTargetPolicy = ReportingPolicy.IGNORE,
        typeConversionPolicy = ReportingPolicy.WARN,
        collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED,
        nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
        nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS
    )
)
... 
public @interface MyMapper{} // meta-annotation

@MyMapper
public interface UserMapper {
    GetUserDto toGetUserDto(User user);
    void updateUser(@MappingTarget User user, ChangeUserDto changeDto);
}

That would be a lot less code for me in comparison to the above. Also, doing delegation (from UserMapper to UserMapper.Updater) in java is very cumbersome.

filiphr commented 3 years ago

We have plans to support meta annotations so you could in the end achieve something with it. However, we are far from defining the APIs for that.

Adding creationConfig and updateConfig to the @Mapper is not an option for us.

If you have a specific patterns you can always write your own annotation processor that would generate the MapStruct mappers and then the MapStruct processor will generate the mapping implementations.

Sam-Kruglov commented 3 years ago

@filiphr thanks for the insight. But do you see how it might make sense to configure the mapper separately for these purposes? For example, in this case, ChangeUserDto only has firstName and lastName, so I don't want mapstruct to complain about User#roles not being updated, it just doesn't make sense. But I do want it to complain if I create a User object but don't initialize the User#roles, hence I want to have unmappedSourcePolicy = ReportingPolicy.ERROR, unmappedTargetPolicy = ReportingPolicy.IGNORE for the update and unmappedSourcePolicy = ReportingPolicy.IGNORE, unmappedTargetPolicy = ReportingPolicy.ERROR for the create. It doesn't have to be like I described but somehow. If you don't see any sense in that either then we can close this issue.

filiphr commented 3 years ago

Yes I understand your use case perfectly. Ideally you would be able to configure in the following way:

@Mapper
public interface UserMapper {

    @BeanMapping(...)
    GetUserDto toGetUserDto(User user);

    @BeanMapping(...)
    void updateUser(@MappingTarget User user, ChangeUserDto changeDto);
}

However, we don't support all the options that are in the @Mapper in the @BeanMapping

Sam-Kruglov commented 3 years ago

Thanks! Yeah, it doesn't have the most important one - to ignore unmapped target properties. But even if it did, I'd have to add this annotation on every update method. I think in that case my delegation solution is better but ideally it would just be able to apply different settings to void methods.