Map with immutable target

[tiller]

(not really a bug, but more of an improvement)

Mapstruct apparently offers no way to map with an immutable "target".

Context: We have a CarDto and a Car that are both immutable and use a builder generated via Lombok

@Value
@Builder(toBuilder=true)
class Car {
    final String color;
    final double price;
    final int hiddenField;
}

@Value
@Builder(toBuilder=true)
class CarDto {
    final String color;
    final double price;
}

A standard mapping like

@Mapper
class CarMapper {
    Car toEntity(CarDto dto);
}

works as intended. The generated mapper creates a new CarBuilder, fill the fields and then return the built entity

The issue: As we have hiddenField which is not present in the DTO, when we have an API call to update the Car entity we need to map the given DTO to the existing entity to preserve the value of the field.

So naively I'd do:

@Mapper
class CarMapper {
    Car toEntity(CarDto dto, @MappingTarget Car existingCar);
}

However mapstruct doesn't seem to support this use case with an immutable target. And as the documentation clearly states:

you may also set the method’s return type to the type of the target parameter, which will cause the generated implementation to update the passed mapping target and return it as well.

So yeah, it does not create a copy of the Car, it tries to update the given one... And as there are no setter, it doesn't do anything.

As mapstruct supports the builder pattern for regular mapping, it'd be great if mapstruct could get the existing entity, call toBuilder on it and then do the mapping as usual.

[tiller]

Note: It seems possible to do something like:

@Mapper
class CarMapper {
    Car toEntity(CarDto dto, @MappingTarget CarBuilder existingCar);

    default Car toEntity(CarDto dto, CarBuilder existingCar) {
        return toEntity(dto, existingCar.toBuilder());
    }
}

But:

1. it adds extra steps
2. if we have a CarDto toDto(Car) mapping for example, we can no longer use @InheritInverseConfiguration to avoid duplicated mapping
3. it kinds of violate what the javadoc says
4. it poses issues with embedded objects that needs to be automatically mapped using the .toBuilder

[filiphr]

Thanks for raising this issue. Supporting something like this can get complex really fast. There can be a lot of edge cases. I will leave this open, but currently the MapStruct team is focused on some other issues. If someone is interested in providing a PR for this functionality we can help out.

[tiller]

I thought about it for a bit, and checked out a little bit how it was currently done, and I think the feature would need to differ from @MappingTarget as the behavior'd differ too much from existing's.

An idea I had was to introduce a new annotation that'd be something like @MappingDefault that'd be used similarly to @MappingTarget but the method would return a new entity instead of the one given as argument. And basically the mapper would generate code looking like:

public Car toEntity(CarDto dto, Car defaultCar) {
    Car newCar = new Car();
    newCar.setColor(dto.getColor() != null ? dto.getColor() : defaultCar.getColor());
    // or newCarBuilder.setColor(...) but the system is already implemented to pick the builder/setter/constructor
}

I think it wouldn't imply too much changes. It probably could work also fine with referenced mapper for inner entities. Check if there is a method with @MappingDefault and if there is call it with defaultCar.getTheField()

[tiller]

@filiphr I made quick and (very) dirty POC to see how much work it'd require. Could someone of your team have a quick look on it to see if the global idea would fit within mapstruct strategy?

https://github.com/Tiller/mapstruct/pull/1 (example of generated mapper in the PR's comment)

I can't emphasize enough on the fact that it was made quickly and dirtily as I knew nothing of the mapstruct internals and I just wanted to get it to work, so I cut a lot of corners

Depending on whether you think this could be a good addition and the overall principle work, I might pursue into developing the feature

[cbornet]

Big +1 on this ! With records coming to Java, it would be great to have good support for immutables !

[jeffreyzh]

Big +1 ! Any updates on this issue?

[nyg]

Would also like to see that implemented :).