-
-
Notifications
You must be signed in to change notification settings - Fork 915
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
#3591 Potential fix for duplicate methods generated. #3604
base: main
Are you sure you want to change the base?
Conversation
Good Catch, the question is now what leads to this behaviour? And is your provided solution just a workaround/bandaid for the actual problem. But I tried your solution and it works. Maybe it's something caused by the recursive nature here? |
processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingOptions.java
Outdated
Show resolved
Hide resolved
Thanks for the feedback. Yes, the recursive structure of the bean must be the cause, but the MapperCreationProcess ends "successfully" and the duplication error only becomes visible during compilation. At some point, the mapper decides not to create any more methods although the equals method fails. Edited: |
Edited: I have added a test for StreamMapping and MapMapping. |
Okay, everything should be done now. I hope I haven't missed anything. I still have a question: |
This is a check if every file contains the copyright header at the beginning of a file.
|
processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java
Outdated
Show resolved
Hide resolved
this.selectionParameters = selectionParameters; | ||
|
||
if ( selectionParameters == null ) { | ||
this.selectionParameters = SelectionParameters.forSourceRHS( null ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a new factory method would be better than using the null as parameter? Just a thought, my architecture knowledge for mapstruct is not big enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right - it could be confusing. I have changed that to SelectionParameters.emptyInstance()
processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java
Show resolved
Hide resolved
.../src/test/java/org/mapstruct/ap/test/bugs/_3591/CompilationErrorDuplicateMethodsBugTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/org/mapstruct/ap/test/bugs/_3591/CompilationErrorDuplicateMethodsBugTest.java
Show resolved
Hide resolved
It's working! Pretty cool! :) Lets see what the maintainers have to say |
Is this Issue related? #3532 |
Potential fix for: #3591
I am not sure if my solution is an acceptable one, but it solves this bug for now.
Problem: There were two methods generated and both are written to the mapper class because the selectionParameters in the ContainerMappingMethods are not equal and therefore the methods are not equal and finally both are used.
Solution: MappingOption now returns null for selectionParameters if no values have been set.
I am still confused why two methods of beanListToBeanDtoList are forged at all. I would like to take another look at this.