mapstruct-idea icon indicating copy to clipboard operation
mapstruct-idea copied to clipboard

Support for the @InheritConfiguration

Open Quipex opened this issue 4 years ago • 1 comments

Let's say we have an A class. For it there is ADto class. We create a mapping ADto A_to_ADto(A source) and specify all the mappings with @Mapping annotation. Now we want ASpecialDto class that extends ADto and adds some new props that weren't mapped before. We use @InheritConfiguration on a new mapping ASpecialDto A_to_ASpecialDto(A source) that uses previous mapping for our needs. The plugin can't detect that we are inheriting the config and proposes to define mapping once again.

This issue may correlate with #35 image

Quipex avatar Feb 11 '21 11:02 Quipex

+1 изображение

slutmaker avatar Mar 17 '21 09:03 slutmaker

@filiphr I would like to implement this feature.

As far as I've seen, the following part of the documentation covers all possibilities of the requirement:

One method A can inherit the configuration from another method B if all types of A (source types and result type) are assignable to the corresponding types of B. Methods that are considered for inheritance need to be defined in the current mapper, a super class/interface, or in the shared configuration interface (as described in Shared configurations). In case more than one method is applicable as source for the inheritance, the method name must be specified within the annotation: @InheritConfiguration( name = "carDtoToCar" ). A method can use @InheritConfiguration and override or amend the configuration by additionally applying @Mapping, @BeanMapping, etc.

I think I can use the following test cases from the mapstruct project:

  • https://github.com/mapstruct/mapstruct/tree/main/processor/src/test/java/org/mapstruct/ap/test/inheritance
  • https://github.com/mapstruct/mapstruct/tree/main/processor/src/test/java/org/mapstruct/ap/test/inheritedmappingmethod
  • https://github.com/mapstruct/mapstruct/tree/main/processor/src/test/java/org/mapstruct/ap/test/inheritfromconfig

Covering all edge cases would probably result in too many code changes for you to review (or for me, it would take too much time to implement it).

What do you think? What would be the main requirements to get this feature live?

Also, do you think of another edge cases you can think of (or stumbled across yourself) that are important?

thunderhook avatar Apr 18 '23 20:04 thunderhook

You can use some of those tests cases indeed. However, as you said:

Covering all edge cases would probably result in too many code changes for you to review (or for me, it would take too much time to implement it).

I think that we can scope it down a bit. We can skip map and iterable mappings. I think that the main requirements are the following:

  • Have some util that will return you back the method that should be applied for @InheritConfiguration. For this we should take the defined name in @InheritConfiguration (if set). If we fine multiple or none we then just ignore.
    • You can use MapstructAnnotationUtils#findReferencedMappers to find the mappers in which you need to search for methods. Check SourceMethod#canInheritFrom (from the MapStruct processor) to see what we apply to check if the method can be inherited from.
    • Make sure that the method itself is not applied in the inheritance. We had a silly bug like that in the processor.
  • Once we have that you run it through TargetUtils#findAllDefinedMappingTargets

We can skip the fact that found method might also have @InheritConfiguration.

In your place I would also try to create smaller and simpler examples than the ones we have in the processor, or try to find the most basic ones we have:

  • @InheritConfiguration in the same class
  • @InheritConfiguration in Mapper#uses
  • @InheritConfiguration in MapperConfig
  • @InheritConfiguration with defined name

filiphr avatar Apr 22 '23 17:04 filiphr