Respect only explicit mappings but fail on unmapped source fields

[thiko]

Use case

Imagine a PATCH request with several JsonNullable fields.

class MyDto {
  private JsonNullable<String> firstname;
  private JsonNullable<String> surname
}

The mapper should overwrite present fields only:

@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
        unmappedSourcePolicy = ReportingPolicy.ERROR)
interface MyPartialUpdateMapper {

  @Mapping(source = "firstname", target = "firstname", conditionExpression = "java(updatedContent.getFirstname().isPresent())")
  @BeanMapping(ignoreByDefault = true)
  void updateMyDomainObject(@MappingTarget MyDomainObject domain, MyDto updatedContent);

  default <T> T fromJsonNullable(JsonNullable<T> nullableValue) {
      if(nullableValue.isPresent()) {
          return nullableValue.get();
      }
      return null;
  }
}

The @BeanMapping(ignoreByDefault = true) is used to ensure that only those fields in the domain object that are set in the Patch-DTO are overwritten. Unfortunately, the unmappedSourcePolicy = ReportingPolicy.ERROR is ignored in this case.

As far as I can see, there is no way to enforce explicit mapping and cause the build to fail if one of the mappings is forgotten. This feature would make the mappings much better maintainable in such situations.

Generated Code

No response

Possible workarounds

No response

MapStruct Version

1.5.3.Final

[filiphr]

You might be right @thiko. Have a look at #3391 (comment) as well. In the initial implementation of ignoreByDefault it was as you've asked. Then we got #2560 and in 1.5.0.Beta2 we also started ignore unmapped source properties. Perhaps we need to revert that

[thiko]

Thanks for your response @filiphr !
Yes reverting this would probably fix it for my scenario.

The desired behavior for BeanMappingMethod#reportErrorForUnmappedSourcePropertiesIfRequired(..) in "my" use-case:

• ignoreByDefault == true && unmappedSourcePolicy == ERROR => No auto-generated field mappings but raise Exception on unmapped source properties
• ignoreByDefault == true && unmappedSourcePolicy == IGNORE => No auto-generated field mappings and no Exception (or warning) on unmapped source properties

But I dont not really understand how #2560 happend. In his case it was ignoreByDefault==true && unmappedSourcePolicy == IGNORE which should be covered with the "old" BeanMappingMethod#reportErrorForUnmappedSourcePropertiesIfRequired

if ( !unprocessedSourceProperties.isEmpty() && unmappedSourcePolicy.requiresReport() ) {
...
} 

As long as unmappedSourcePolicy.requiresReport() is false for ReportingPolicy.IGNORE.

[filiphr]

But I dont not really understand how #2560 happened. In his case it was ignoreByDefault==true && unmappedSourcePolicy == IGNORE which should be covered with the "old"

I think that you saw != as ==.

In any case, I believe that perhaps such a breaking change could be added for this. The defined unmappedSourcePolicy should be used to decide if unmapped source properties should be reported or not. The main purpose of the ignoreByDefault was to avoid the people needing to type @Mapping(target = "test", ignore = true) for all the properties that they do not want to map, i.e. to avoid the implicit mapping. The fact that ignoreByDefault ignores the unmappedTargetPolicy is just a side effect of the fact that all target properties are actually explicitly ignored.

What do you think @Zegveld and @thunderhook?

[thiko]

I think that you saw != as ==.

Yes I did :-)

[Zegveld]

Having ignoreByDefault=true influence source properties feels also weird, as that should be a shortcut to avoid a large amount of @Mapping(target="...", ignore=true) lines. If I would write out all those mappings I suddenly have different behavior. So I agree with reverting #2560.

I would even go as far as saying that the combination of ignoreByDefault=true and usage of unmappedTargetPolicy directly defined on the same @BeanMapping should give a compile error. Just to prevent someone from making a mistake with combining ignoreByDefault=true, unmappedSourcePolicy=IGNORE and by accident autocomplete into unmappedTargetPolicy.

As a final note, we need to update the reference guide for unmappedTargetPolicy. Because it says the following:

The default reporting policy to be applied in case an attribute of the target object of a mapping method is not populated with a source value.

In case of a constant, expression, or ignore it should still report according to the text, because source is also an explicit value type present.

Better would be something in the lines of:

The default reporting policy to be applied in case the handling of an attribute of the target object of a mapping method is not defined.

[filiphr]

So I agree with reverting #2560.

Great, I'll look into implementing this

I would even go as far as saying that the combination of ignoreByDefault=true and usage of unmappedTargetPolicy directly defined on the same @BeanMapping should give a compile error.

I don't see a big benefit in doing this. If someone does it then it'll be ignored, if they wanted to use unmappedSourcePolicy and they've made a mistake they'll have a different error anyways

As a final note, we need to update the reference guide for unmappedTargetPolicy.

That does make sense.

[thunderhook]

What do you think @Zegveld and @thunderhook?

@filiphr Nothing more to add, I also agree to revert #2560. 👍