From 0a7d8e2151e66927107c72bb36b29afc790b76db Mon Sep 17 00:00:00 2001 From: Daniel Mensinger Date: Mon, 27 Jan 2025 15:13:45 +0100 Subject: [PATCH 1/3] HHH-19076 Reproducer testcase --- .../CompositeInheritanceFailTest.java | 129 ++++++++++++++++++ .../CompositeInheritanceWorkingTest.java | 129 ++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java new file mode 100644 index 000000000000..06dbdf3cfd2a --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java @@ -0,0 +1,129 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.mapping.identifier.composite; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.IdClass; +import jakarta.persistence.MappedSuperclass; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.Setting; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * This test Fails + */ +@DomainModel( + annotatedClasses = { + CompositeInheritanceFailTest.TupAbstractEntity.class, + CompositeInheritanceFailTest.DummyEntity.class, + CompositeInheritanceFailTest.TestEntity.class, // Here the class is called TestEntity + } +) +@ServiceRegistry( + settings = { + // For your own convenience to see generated queries: + @Setting(name = AvailableSettings.SHOW_SQL, value = "true"), + @Setting(name = AvailableSettings.FORMAT_SQL, value = "true"), + // @Setting( name = AvailableSettings.GENERATE_STATISTICS, value = "true" ), + } +) +@SessionFactory +@Jira("HHH-19076") +public class CompositeInheritanceFailTest { + + @Test + void hhh19076FailingTest(SessionFactoryScope scope) { + scope.inTransaction( em -> { + TestEntity e1 = new TestEntity("foo", "bar"); + em.persist(e1); + + CompositeIdClass key = e1.getCompositeId(); + TestEntity e2 = em.find(TestEntity.class, key); + assertNotNull(e2); + } ); + } + + @MappedSuperclass + public static abstract class TupAbstractEntity { + @Id + private String oid = null; + + + @SuppressWarnings("this-escape") + protected TupAbstractEntity() { + } + + protected TupAbstractEntity(String oid) { + this.oid = oid; + } + + public String getOid() { + return oid; + } + + } + + @Entity + public static class DummyEntity extends TupAbstractEntity { + } + + @Entity + @IdClass(CompositeIdClass.class) + public static class TestEntity extends TupAbstractEntity { + + @Id + private String myId; + + protected TestEntity() { + // for JPA + } + + public TestEntity(String oid, String myId) { + super(oid); + this.myId = myId; + } + + public String myId() { + return myId; + } + + public CompositeIdClass getCompositeId() { + return new CompositeIdClass(getOid(), myId); + } + + } + + public static class CompositeIdClass { + + private String oid; + private String myId; + + public CompositeIdClass(String oid, String myId) { + this.oid = oid; + this.myId = myId; + } + + public CompositeIdClass() { + } + + public String oid() { + return oid; + } + + public String myId() { + return myId; + } + + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java new file mode 100644 index 000000000000..4134dffa2e99 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java @@ -0,0 +1,129 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.mapping.identifier.composite; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.IdClass; +import jakarta.persistence.MappedSuperclass; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.Setting; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * This test works for some reason... + */ +@DomainModel( + annotatedClasses = { + CompositeInheritanceWorkingTest.TupAbstractEntity.class, + CompositeInheritanceWorkingTest.DummyEntity.class, + CompositeInheritanceWorkingTest.FooEntity.class, // And here the class is called FooEntity and this works for some reason + } +) +@ServiceRegistry( + settings = { + // For your own convenience to see generated queries: + @Setting(name = AvailableSettings.SHOW_SQL, value = "true"), + @Setting(name = AvailableSettings.FORMAT_SQL, value = "true"), + // @Setting( name = AvailableSettings.GENERATE_STATISTICS, value = "true" ), + } +) +@SessionFactory +@Jira("HHH-19076") +public class CompositeInheritanceWorkingTest { + + @Test + void hhh19076WorkingTest(SessionFactoryScope scope) { + scope.inTransaction( em -> { + FooEntity e1 = new FooEntity("foo", "bar"); + em.persist(e1); + + CompositeIdClass key = e1.getCompositeId(); + FooEntity e2 = em.find( FooEntity.class, key); + assertNotNull(e2); + } ); + } + + @MappedSuperclass + public static abstract class TupAbstractEntity { + @Id + private String oid = null; + + + @SuppressWarnings("this-escape") + protected TupAbstractEntity() { + } + + protected TupAbstractEntity(String oid) { + this.oid = oid; + } + + public String getOid() { + return oid; + } + + } + + @Entity + public static class DummyEntity extends TupAbstractEntity { + } + + @Entity + @IdClass(CompositeIdClass.class) + public static class FooEntity extends TupAbstractEntity { + + @Id + private String myId; + + protected FooEntity() { + // for JPA + } + + public FooEntity(String oid, String myId) { + super(oid); + this.myId = myId; + } + + public String myId() { + return myId; + } + + public CompositeIdClass getCompositeId() { + return new CompositeIdClass(getOid(), myId); + } + + } + + public static class CompositeIdClass { + + private String oid; + private String myId; + + public CompositeIdClass(String oid, String myId) { + this.oid = oid; + this.myId = myId; + } + + public CompositeIdClass() { + } + + public String oid() { + return oid; + } + + public String myId() { + return myId; + } + + } + +} From 83d9f4ce78029d951a71f3c5afb2956a751407a4 Mon Sep 17 00:00:00 2001 From: Daniel Mensinger Date: Mon, 30 Jun 2025 15:14:29 +0200 Subject: [PATCH 2/3] HHH-19076 Add second composite key entity to the failing test --- .../CompositeInheritanceFailTest.java | 70 +++++++++++++++++++ .../CompositeInheritanceWorkingTest.java | 70 +++++++++++++++++++ 2 files changed, 140 insertions(+) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java index 06dbdf3cfd2a..ede98a79273b 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceFailTest.java @@ -8,6 +8,7 @@ import jakarta.persistence.Id; import jakarta.persistence.IdClass; import jakarta.persistence.MappedSuperclass; +import jakarta.persistence.Version; import org.hibernate.cfg.AvailableSettings; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.Jira; @@ -27,6 +28,7 @@ CompositeInheritanceFailTest.TupAbstractEntity.class, CompositeInheritanceFailTest.DummyEntity.class, CompositeInheritanceFailTest.TestEntity.class, // Here the class is called TestEntity + CompositeInheritanceFailTest.Test2Entity.class, } ) @ServiceRegistry( @@ -53,6 +55,18 @@ void hhh19076FailingTest(SessionFactoryScope scope) { } ); } + @Test + void hhh19076FailingTest2(SessionFactoryScope scope) { + scope.inTransaction( em -> { + Test2Entity e1 = new Test2Entity("foo", "xxxxxx"); + em.persist(e1); + + CompositeId2Class key = e1.getCompositeId(); + Test2Entity e2 = em.find(Test2Entity.class, key); + assertNotNull(e2); + } ); + } + @MappedSuperclass public static abstract class TupAbstractEntity { @Id @@ -103,6 +117,39 @@ public CompositeIdClass getCompositeId() { } + @Entity + @IdClass(CompositeId2Class.class) + public static class Test2Entity extends TupAbstractEntity { + + @Id + private String otherId; + + @Version + private long tanum = 0; + + protected Test2Entity() { + // for JPA + } + + public Test2Entity(String oid, String otherId) { + super(oid); + this.otherId = otherId; + } + + public String myId() { + return otherId; + } + + public long tanum() { + return tanum; + } + + public CompositeId2Class getCompositeId() { + return new CompositeId2Class(getOid(), otherId); + } + + } + public static class CompositeIdClass { private String oid; @@ -126,4 +173,27 @@ public String myId() { } + public static class CompositeId2Class { + + private String oid; + private String otherId; + + public CompositeId2Class(String oid, String otherId) { + this.oid = oid; + this.otherId = otherId; + } + + public CompositeId2Class() { + } + + public String oid() { + return oid; + } + + public String otherId() { + return otherId; + } + + } + } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java index 4134dffa2e99..2a3c81603785 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/identifier/composite/CompositeInheritanceWorkingTest.java @@ -8,6 +8,7 @@ import jakarta.persistence.Id; import jakarta.persistence.IdClass; import jakarta.persistence.MappedSuperclass; +import jakarta.persistence.Version; import org.hibernate.cfg.AvailableSettings; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.Jira; @@ -27,6 +28,7 @@ CompositeInheritanceWorkingTest.TupAbstractEntity.class, CompositeInheritanceWorkingTest.DummyEntity.class, CompositeInheritanceWorkingTest.FooEntity.class, // And here the class is called FooEntity and this works for some reason + CompositeInheritanceWorkingTest.Test2Entity.class, } ) @ServiceRegistry( @@ -53,11 +55,26 @@ void hhh19076WorkingTest(SessionFactoryScope scope) { } ); } + @Test + void hhh19076FailingTest2(SessionFactoryScope scope) { + scope.inTransaction( em -> { + Test2Entity e1 = new Test2Entity("foo", "xxxxxx"); + em.persist(e1); + + CompositeId2Class key = e1.getCompositeId(); + Test2Entity e2 = em.find( Test2Entity.class, key); + assertNotNull(e2); + } ); + } + @MappedSuperclass public static abstract class TupAbstractEntity { @Id private String oid = null; + @Version + private long tanum = 0; + @SuppressWarnings("this-escape") protected TupAbstractEntity() { @@ -71,6 +88,9 @@ public String getOid() { return oid; } + public long getTanum() { + return tanum; + } } @Entity @@ -103,6 +123,32 @@ public CompositeIdClass getCompositeId() { } + @Entity + @IdClass(CompositeId2Class.class) + public static class Test2Entity extends TupAbstractEntity { + + @Id + private String otherId; + + protected Test2Entity() { + // for JPA + } + + public Test2Entity(String oid, String otherId) { + super(oid); + this.otherId = otherId; + } + + public String myId() { + return otherId; + } + + public CompositeId2Class getCompositeId() { + return new CompositeId2Class(getOid(), otherId); + } + + } + public static class CompositeIdClass { private String oid; @@ -126,4 +172,28 @@ public String myId() { } + + public static class CompositeId2Class { + + private String oid; + private String otherId; + + public CompositeId2Class(String oid, String otherId) { + this.oid = oid; + this.otherId = otherId; + } + + public CompositeId2Class() { + } + + public String oid() { + return oid; + } + + public String otherId() { + return otherId; + } + + } + } From 56a32761482483af3d7892454534505afd1bad2c Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 3 Jul 2025 17:45:57 +0200 Subject: [PATCH 3/3] HHH-19076 Resolve member of MappedSuperclass specially --- .../metamodel/internal/AttributeFactory.java | 95 +++++++++++-------- 1 file changed, 55 insertions(+), 40 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java index 164f3e54fc68..bedbebb48481 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java @@ -58,6 +58,7 @@ import org.hibernate.property.access.internal.PropertyAccessMapImpl; import org.hibernate.property.access.spi.Getter; import org.hibernate.query.sqm.tree.domain.SqmDomainType; +import org.hibernate.query.sqm.tree.domain.SqmMappedSuperclassDomainType; import org.hibernate.type.AnyType; import org.hibernate.type.BasicType; import org.hibernate.type.CollectionType; @@ -740,56 +741,70 @@ private static Member resolveMappedSuperclassMember( Property property, MappedSuperclassDomainType ownerType, MetadataContext context) { - final EntityPersister declaringEntity = - getDeclaringEntity( (AbstractIdentifiableType) ownerType, context ); - if ( declaringEntity != null ) { - return resolveEntityMember( property, declaringEntity ); - } - else { - final ManagedDomainType subType = ownerType.getSubTypes().iterator().next(); - final Type.PersistenceType persistenceType = subType.getPersistenceType(); - return switch ( persistenceType ) { - case ENTITY -> - resolveEntityMember( property, - getDeclaringEntity( (AbstractIdentifiableType) subType, context ) ); - case MAPPED_SUPERCLASS -> - resolveMappedSuperclassMember( property, (MappedSuperclassDomainType) subType, context ); - case EMBEDDABLE -> - resolveEmbeddedMember( property, (EmbeddableDomainType) subType, context ); - default -> throw new IllegalArgumentException( "Unexpected PersistenceType: " + persistenceType ); - }; - } + return property.getGetter( ownerType.getJavaType() ).getMember(); +// final EntityPersister declaringEntity = +// getDeclaringEntity( (AbstractIdentifiableType) ownerType, context ); +// if ( declaringEntity != null ) { +// return resolveEntityMember( property, declaringEntity ); +// } +// else { +// final ManagedDomainType subType = ownerType.getSubTypes().iterator().next(); +// final Type.PersistenceType persistenceType = subType.getPersistenceType(); +// return switch ( persistenceType ) { +// case ENTITY -> +// resolveEntityMember( property, +// getDeclaringEntity( (AbstractIdentifiableType) subType, context ) ); +// case MAPPED_SUPERCLASS -> +// resolveMappedSuperclassMember( property, (MappedSuperclassDomainType) subType, context ); +// case EMBEDDABLE -> +// resolveEmbeddedMember( property, (EmbeddableDomainType) subType, context ); +// default -> throw new IllegalArgumentException( "Unexpected PersistenceType: " + persistenceType ); +// }; +// } } 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() ); - + if ( identifiableType instanceof SqmMappedSuperclassDomainType ) { + return attributeContext.getPropertyMapping() + .getGetter( identifiableType.getJavaType() ) + .getMember(); + } + else { + 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() ); + } }; private final MemberResolver versionMemberResolver = (attributeContext, metadataContext) -> { final AbstractIdentifiableType identifiableType = (AbstractIdentifiableType) attributeContext.getOwnerType(); - final EntityPersister entityPersister = getDeclaringEntity( identifiableType, metadataContext ); - final EntityVersionMapping versionMapping = entityPersister.getVersionMapping(); - assert entityPersister.isVersioned(); - assert versionMapping != null; - - final String versionPropertyName = attributeContext.getPropertyMapping().getName(); - if ( !versionPropertyName.equals( versionMapping.getVersionAttribute().getAttributeName() ) ) { - // this should never happen, but to be safe... - throw new IllegalArgumentException( "Given property did not match declared version property" ); - } - return getter( entityPersister, attributeContext.getPropertyMapping(), - versionPropertyName, versionMapping.getJavaType().getJavaTypeClass() ); + if ( identifiableType instanceof SqmMappedSuperclassDomainType ) { + return attributeContext.getPropertyMapping() + .getGetter( identifiableType.getJavaType() ) + .getMember(); + } + else { + final EntityPersister entityPersister = getDeclaringEntity( identifiableType, metadataContext ); + final EntityVersionMapping versionMapping = entityPersister.getVersionMapping(); + assert entityPersister.isVersioned(); + assert versionMapping != null; + + final String versionPropertyName = attributeContext.getPropertyMapping().getName(); + if ( !versionPropertyName.equals( versionMapping.getVersionAttribute().getAttributeName() ) ) { + // this should never happen, but to be safe... + throw new IllegalArgumentException( "Given property did not match declared version property" ); + } + return getter( entityPersister, attributeContext.getPropertyMapping(), + versionPropertyName, versionMapping.getJavaType().getJavaTypeClass() ); + } }; private static Member getter(EntityPersister persister, Property property, String name, Class type) {