-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
HHH-19076 expecting IdClass mapping sessionfactory error with specific @IdClass setup with inheritence #9795
base: main
Are you sure you want to change the base?
Conversation
…dentifier property name, but there is no identifier mapping
1cce9b4
to
ec25365
Compare
if ( mapping == null ) { | ||
// Member is identifier, but there is no identifier property mapping ... | ||
// ... this *should* indicate we have an IdClass mapping | ||
return resolveVirtualIdentifierMember( property, declaringEntity ); |
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 might be missing something, but I don't see how this would work: resolveVirtualIdentifierMember
calls entityPersister.getIdentifierMapping()
, expecting it to be non-null, so I would think this triggers an NPE.
Also, IIRC getIdentifierPropertyName()
returns null
for id-class mappings so we would always default to getter
, which is incorrect.
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 mistake, at this point resolveVirtualIdentifierMember
should be invoked only when EntityIdentifierMapping
has property nature
set to VIRTUAL
. But, I am not sure if such situation can ever occur. In all other test cases (except this one that was failing) resolveVirtualIdentifierMember
has been invoked from org.hibernate.metamodel.internal.AttributeFactory#buildIdAttribute
via identifierMemberResolver
. In all other situations org.hibernate.metamodel.internal.AttributeFactory#resolveEntityMember
will really always default to getter
.
Actually, this is exactly this:
private final MemberResolver identifierMemberResolver = (attributeContext, metadataContext) -> {
final AbstractIdentifiableType<?> identifiableType =
(AbstractIdentifiableType<?>) attributeContext.getOwnerType();
final EntityPersister declaringEntityMapping = getDeclaringEntity( identifiableType, metadataContext );
final EntityIdentifierMapping identifierMapping = declaringEntityMapping.getIdentifierMapping();
final Property propertyMapping = attributeContext.getPropertyMapping();
return !propertyMapping.getName().equals( identifierMapping.getAttributeName() )
// this *should* indicate processing part of an IdClass...
? virtualIdentifierMemberResolver.resolveMember( attributeContext, metadataContext )
: getter( declaringEntityMapping, propertyMapping,
identifierMapping.getAttributeName(), identifierMapping.getJavaType().getJavaTypeClass() );
};
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 change will most likely just hide the real problem, not fix it. It seems that order of entity classes is, at least in this case, important, although it should not be since those two classes only have common mapped superclass, but are not related otherwise.
Difference in processing between two tests is that in "good" one TestEntity
is processed first, while in one that fails first is processed DummyEntity
When in org.hibernate.metamodel.internal.AttributeFactory
identifierMemberResolver
is invoked with identifiable type TupAbstractEntity
declared entity mapping in good case is TestEntity
, and in bad one DummyEntity
. All this going on while in org.hibernate.metamodel.internal.MetadataContext#wrapUp
method applyIdMetadata
is invoked from superclass branch.
Do not know why is so (and, at the moment, do not have time to find out), but I am very sure that this is wrong.
) | ||
@SessionFactory | ||
@Jira("HHH-19076") | ||
public class CompositeInheritanceWorkingTest { |
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.
No need for a "working" test case, if you really wish to include both versions of the mappings you can do so in a single test class and separate test methods - but also having only the problematic case is fine.
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've simply (re)used commit with both test cases, but you are right. I will simply remove test that was passing without any changes.
Those two tests are practically identical, only one static class name is different. When this name is changed to be equal in both classes, only difference is in test class and method names. Then, without any change in main code still one test is failing, but other is passing. So, I guess that actual case of the problem is somewhere else.
I will check that, until then this PR should be marked as draft.
Jira issue HHH-19076
If property name is equal to entity property identifier name, but entity property mapping is null, then member should be resolved as virtual identifier member. Otherwise, it should be resolved using
getter
method-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.