Skip to content

Conversation

@Obolrom
Copy link
Contributor

@Obolrom Obolrom commented May 28, 2024

Fixes #3609

  • Add test case to cover the bug
  • Fix bug

@filiphr
Copy link
Member

filiphr commented Jun 1, 2024

@Obolrom do you already have some ideas about the bug fix or do you need me to have a look at it as well?

@Obolrom
Copy link
Contributor Author

Obolrom commented Jun 1, 2024

@filiphr, I am trying to figure out how to fix the bug properly.
As I understand we should do something around these properties

private Map<String, Accessor> unprocessedConstructorProperties;
private Map<String, Accessor> unprocessedTargetProperties;
private Map<String, Accessor> unprocessedSourceProperties;
private Set<String> missingIgnoredSourceProperties;

in the BeanMappingMethod class.

My first attempt to fix it led to 200 failed tests. So, I would appreciate any hint

@filiphr
Copy link
Member

filiphr commented Jun 2, 2024

@Obolrom, I believe that it is somewhere in the BeanMappingMethos.Builder#createSubclassMapping. Or more specifically in forgeSubclassMapping. I think that we are not passing all the information from the BeanMapping into the forged methods, but looking at the code it should work. Perhaps it is best if I look into it, no need to waste on time on this.

@filiphr
Copy link
Member

filiphr commented Jul 6, 2024

@Obolrom did my hint help you out a bit? If not, I'll take over and do the fix.

@Obolrom
Copy link
Contributor Author

Obolrom commented Jul 13, 2024

@filiphr by some reason it doesn't work anyway. Could you please take a look?

@Obolrom Obolrom marked this pull request as ready for review July 14, 2024 06:23
reduce the complexity of the test + reduce the complexity of the fix
@filiphr
Copy link
Member

filiphr commented Jul 20, 2024

Thanks @Obolrom and @thunderhook for working on this.

@thunderhook I was not too happy with the fix. I changed it a bit. Have a look at how it looks like, I prefer when things are a bit more explicit than having something special in the generic method.

@Obolrom, I reduced the test case a bit to only cover what the actual bug is and reduce the complexity of it.

@filiphr filiphr merged commit df49ce5 into mapstruct:main Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@SubclassMapping not working with @BeanMapping.ignoreUnmappedSourceProperties

3 participants