Skip to content

Conversation

@beikov
Copy link
Member

@beikov beikov commented Jul 4, 2025

[Please describe here what your change is about]


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.


https://hibernate.atlassian.net/browse/HHH-19076

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds tests and refactors how mapped-superclass members are resolved to address HHH-19076.

  • Introduce two new tests (CompositeInheritanceWorkingTest and CompositeInheritanceFailTest) to illustrate working and failing composite‐ID inheritance cases.
  • Simplify resolveMappedSuperclassMember and add special‐case handling for mapped‐superclass types in identifierMemberResolver and versionMemberResolver in AttributeFactory.
  • Remove complex subtype‐inspection logic in favor of a direct getter lookup for mapped‐superclass members.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java New “working” test case for composite‐ID inheritance
hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java New “failing” test case for composite‐ID inheritance
hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java Simplify resolveMappedSuperclassMember and update identifier/version member resolvers to treat mapped‐superclass specially
Comments suppressed due to low confidence (3)

hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java:142

  • [nitpick] This method returns the field otherId but is named myId(). For clarity and consistency, rename it to getOtherId() or change the field to myId.
		public String myId() {

hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java:165

  • [nitpick] JavaBean conventions recommend getOid() instead of oid(), especially for JPA property access. Consider renaming this getter.
		public String oid() {

hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java:785

  • The new branch for MappedSuperclassDomainType in the identifier resolver is not covered by existing tests. Consider adding a unit test to verify resolution when the owner type is a mapped superclass.
		if ( identifiableType instanceof MappedSuperclassDomainType<?> ) {


}

public static class CompositeIdClass {
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

Per the JPA spec, composite‐ID classes must implement equals() and hashCode() to ensure correct identity comparison. Consider adding those methods to CompositeIdClass.

Copilot uses AI. Check for mistakes.
final Getter getter = getter( declaringEntityMapping, propertyMapping );
if ( getter instanceof PropertyAccessMapImpl.GetterImpl ) {
return new MapMember( identifierMapping.getAttributeName(), identifierMapping.getJavaType().getJavaTypeClass() );
if ( identifiableType instanceof MappedSuperclassDomainType<?> ) {
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

[nitpick] The same pattern (propertyMapping.getGetter(...).getMember()) is repeated in both identifier and version resolvers. Consider extracting this into a shared helper to reduce duplication.

Copilot uses AI. Check for mistakes.
@beikov beikov merged commit 14b8e31 into hibernate:6.6 Jul 4, 2025
29 of 32 checks passed
@beikov beikov deleted the HHH-19076-6.6 branch July 4, 2025 15:43
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