-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
HHH-19020 - provide more info in case a declared MappedSuperclass is not found #10739
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
Open
jrenaat
wants to merge
1
commit into
hibernate:main
Choose a base branch
from
jrenaat:HHH-19020_NPE
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This message is slightly better than an NPE, but it doesn't really tell the user what they did wrong, nor what they should do to fix it.
Also, shouldn't it be a
MappingException
?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.
I think we might want to understand why the
superclass
is null, too, since looking at the code-path that leads here (from the Jira's stack trace) it looks like we're processing an embeddable that extends from a@MappedSuperclass
. I fear that we might be processing the embeddable before its parent, I remember fixing something similar for https://hibernate.atlassian.net/browse/HHH-18103.Uh oh!
There was an error while loading. Please reload this page.
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.
One of the issue's comments states "the superclass of the embedded id was not part of the mapped resources that Spring registers with Hibernate", so it seems like this was an error in Spring, and this was just an attempt to cater for the user's - I think correct - request for more useful (than an NPE) info on what happened.
@gavinking This is also why I chose to throw a more generic HibernateException, since we may not be sure what the cause of the missing MappedSuperclass is?
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.
I agree with @mbellade that we should dig a bit deeper into this, via the test case given here: https://github.com/lukaseckert/hibernate-test-case-HHH-19020/tree/main/orm.
I think you know what I think of the idea of taking at face value any claim made by a user in an issue report. :)
Uh oh!
There was an error while loading. Please reload this page.
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.
My findings:
@mbellade thoughts?
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.
Before my fix, the metamodel for embeddables extending from mapped superclasses was simply wrong (the attributes were not considered as defined by the superclass).
That doesn't give me much context as to why the mapped superclass might be null, though. As I said in my previous comment, it would be nice to understand the cause of it as there might be a bug there - I can take a look at the issue and try isolating a test case if you're having trouble with that.