Skip to content

Conversation

@csaadaam
Copy link

@csaadaam csaadaam commented Jan 18, 2024

@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Jan 18, 2024

Thanks for your pull request!

This pull request appears to follow the contribution rules.

› This message was automatically generated.

@csaadaam csaadaam changed the title HHH-17652 Fix NullPointerException when mapping audited many-to-one r… HHH-17652 Fix NullPointerException when mapping audited many-to-one relationships Jan 18, 2024
@gavinking gavinking changed the title HHH-17652 Fix NullPointerException when mapping audited many-to-one relationships [Envers] HHH-17652 Fix NullPointerException when mapping audited many-to-one relationships Dec 1, 2024
Copy link
Member

@mbellade mbellade left a comment

Choose a reason for hiding this comment

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

@csaadaam thank you for your contribution! Sorry for taking so long, we have a long backlog of issues and we're trying to get to pending PRs.

I've left a small comment about code style, but other than that, your change looks reasonable to me. I would ask you to please include a test in your PR which ensures the NPE has been fixed by your changes and prevents us from re-introducing a regression in the future.

Comment on lines +199 to +201
if (referencedEntity.isAudited()) {
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

Please follow our code style guidelines.

Suggested change
if (referencedEntity.isAudited()) {
return false;
}
if ( referencedEntity.isAudited() ) {
return false;
}

@mbellade
Copy link
Member

We're also in the process or investigating a move to the Apache Software License v2. If you agree with allowing this change to be relicensed under that license, please add this text to the subject -

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

@mbellade
Copy link
Member

mbellade commented Dec 18, 2024

@csaadaam I took the liberty of looking into this issue and found that while your solution resolved the problem with your use-case it caused errors in several envers tests verifying the correct functionality of audited entities using not-found relationships.

I've added a test case and proposed a working fix here: #9453

@csaadaam
Copy link
Author

@csaadaam I took the liberty of looking into this issue and found that while your solution resolved the problem with your use-case it caused errors in several envers verifying the correct functionality of audited entities using not-found relationships.

I've added a test case and proposed a working fix here: #9453

@mbladel Thank you very much for looking into it and fixing it!

@csaadaam csaadaam closed this Dec 18, 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.

2 participants