Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for the @InheritConfiguration #65

Closed
Quipex opened this issue Feb 11, 2021 · 3 comments · Fixed by #130
Closed

Support for the @InheritConfiguration #65

Quipex opened this issue Feb 11, 2021 · 3 comments · Fixed by #130
Labels
Milestone

Comments

@Quipex
Copy link

Quipex commented Feb 11, 2021

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

@slutmaker
Copy link

slutmaker commented Mar 17, 2021

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

@thunderhook
Copy link
Contributor

@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:

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?

@filiphr
Copy link
Member

filiphr commented Apr 22, 2023

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

thunderhook added a commit to thunderhook/mapstruct-idea that referenced this issue Apr 24, 2023
thunderhook added a commit to thunderhook/mapstruct-idea that referenced this issue Apr 26, 2023
thunderhook added a commit to thunderhook/mapstruct-idea that referenced this issue Apr 26, 2023
thunderhook added a commit to thunderhook/mapstruct-idea that referenced this issue Apr 26, 2023
thunderhook added a commit to thunderhook/mapstruct-idea that referenced this issue Apr 30, 2023
…method and also support scenarios when using @context parameters
@filiphr filiphr modified the milestones: 1.6.0, 1.7.0 May 1, 2023
filiphr pushed a commit that referenced this issue May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants