Skip to content

fix: #2825 SubclassMapping stackoverflow exception #2962

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.

Sign up for GitHub

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

Jump to bottom
Merged
filiphr merged 7 commits into from
Aug 28, 2022
Merged

fix: #2825 SubclassMapping stackoverflow exception #2962

filiphr merged 7 commits into from
Aug 28, 2022

Conversation

@chenzijia12300
Copy link
Contributor

@chenzijia12300 chenzijia12300 commented Aug 18, 2022

filiphr
filiphr reviewed Aug 20, 2022
View reviewed changes
Copy link
Member

@filiphr filiphr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @chenzijia12300. I'll need to check it out locally, as I am not sure that the fix in the MappingResolverImpl is the right one.

filiphr
filiphr requested changes Aug 24, 2022
View reviewed changes
Copy link
Member

@filiphr filiphr left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chenzijia12300 I've adapted the change that is needed to fix this.

Can you perhaps explain why we need so many mappers in the tests? Isn't it enough to only have the AnimalMapper for this issue?

@chenzijia12300
Copy link
Contributor Author

chenzijia12300 commented Aug 25, 2022

Sorry for submitting more than one useless test class, I added multiple mappers at that time to better debug the logic of ResolvingAttempt class

@filiphr
Copy link
Member

filiphr commented Aug 28, 2022

Sorry for submitting more than one useless test class, I added multiple mappers at that time to better debug the logic of ResolvingAttempt class

No need to apologise, I was only being curious. I've pushed a commit that removes the extra test classes. Will merge once the CI is done

@filiphr filiphr merged commit 3cc2aa7 into mapstruct:main Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@filiphr filiphr filiphr requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SubclassMapping stackoverflow exception

2 participants

@chenzijia12300 @filiphr