From 77d334a30e01e84bf1d52699e3f6ab0b38298117 Mon Sep 17 00:00:00 2001 From: Sylvain Lecoy Date: Tue, 17 Jun 2025 03:01:24 +0200 Subject: [PATCH 1/2] HHH-19542: Embeddable property order for SecondaryTable is no longer causing issue --- .../internal/ComponentPropertyHolder.java | 50 ++++-- .../boot/model/internal/EmbeddableBinder.java | 18 ++- .../model/internal/PropertyHolderBuilder.java | 2 +- .../annotations/embedded/EmbeddableA.java | 4 +- ...EmbeddedObjectWithASecondaryTableTest.java | 71 +++++++++ ...NestedEmbeddedWithASecondaryTableTest.java | 144 ++++++++++++++++++ 6 files changed, 268 insertions(+), 21 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/NestedEmbeddedObjectWithASecondaryTableTest.java create mode 100644 hibernate-core/src/test/java17/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java index a9ae72117918..6875bf161378 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java @@ -6,14 +6,18 @@ */ package org.hibernate.boot.model.internal; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Objects; import org.hibernate.AnnotationException; import org.hibernate.annotations.common.reflection.XClass; import org.hibernate.annotations.common.reflection.XProperty; import org.hibernate.boot.spi.MetadataBuildingContext; import org.hibernate.boot.spi.PropertyData; +import org.hibernate.internal.util.StringHelper; import org.hibernate.mapping.AggregateColumn; import org.hibernate.mapping.Component; import org.hibernate.mapping.Join; @@ -74,6 +78,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder { private final String embeddedAttributeName; private final Map attributeConversionInfoMap; + private final List annotatedColumns; public ComponentPropertyHolder( Component component, @@ -100,6 +105,12 @@ public ComponentPropertyHolder( this.embeddedAttributeName = ""; this.attributeConversionInfoMap = processAttributeConversions( inferredData.getClassOrPluralElement() ); } + + if ( parent instanceof ComponentPropertyHolder ) { + this.annotatedColumns = ( (ComponentPropertyHolder) parent ).annotatedColumns; + } else { + this.annotatedColumns = new ArrayList<>(); + } } /** @@ -266,26 +277,45 @@ public String getEntityName() { @Override public void addProperty(Property property, AnnotatedColumns columns, XClass declaringClass) { - //Ejb3Column.checkPropertyConsistency( ); //already called earlier + //AnnotatedColumns.checkPropertyConsistency( ); //already called earlier // Check table matches between the component and the columns // if not, change the component table if no properties are set // if a property is set already the core cannot support that + final Table table = property.getValue().getTable(); + if ( !table.equals( getTable() ) ) { + if ( component.getPropertySpan() == 0 ) { + component.setTable( table ); + } + else { + throw new AnnotationException( + "Embeddable class '" + component.getComponentClassName() + + "' has properties mapped to two different tables" + + " (all properties of the embeddable class must map to the same table)" + ); + } + } if ( columns != null ) { - final Table table = columns.getTable(); - if ( !table.equals( getTable() ) ) { - if ( component.getPropertySpan() == 0 ) { - component.setTable( table ); - } - else { + annotatedColumns.addAll( columns.getColumns() ); + } + addProperty( property, declaringClass ); + } + + public void checkPropertyConsistency() { + if ( annotatedColumns.size() > 1 ) { + for ( int currentIndex = 1; currentIndex < annotatedColumns.size(); currentIndex++ ) { + final AnnotatedColumn current = annotatedColumns.get( currentIndex ); + final AnnotatedColumn previous = annotatedColumns.get( currentIndex - 1 ); + if ( !Objects.equals( + StringHelper.nullIfEmpty( current.getExplicitTableName() ), + StringHelper.nullIfEmpty( previous.getExplicitTableName() ) ) ) { throw new AnnotationException( "Embeddable class '" + component.getComponentClassName() - + "' has properties mapped to two different tables" - + " (all properties of the embeddable class must map to the same table)" + + "' has properties mapped to two different tables" + + " (all properties of the embeddable class must map to the same table)" ); } } } - addProperty( property, declaringClass ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java index e70c0efa39e8..829eef2566d3 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java @@ -244,15 +244,15 @@ private static PropertyBinder createEmbeddedProperty( final PropertyBinder binder = new PropertyBinder(); binder.setDeclaringClass( inferredData.getDeclaringClass() ); binder.setName( inferredData.getPropertyName() ); - binder.setValue(component); + binder.setValue( component ); binder.setProperty( inferredData.getProperty() ); binder.setAccessType( inferredData.getDefaultAccess() ); - binder.setEmbedded(isComponentEmbedded); - binder.setHolder(propertyHolder); - binder.setId(isId); - binder.setEntityBinder(entityBinder); - binder.setInheritanceStatePerClass(inheritanceStatePerClass); - binder.setBuildingContext(context); + binder.setEmbedded( isComponentEmbedded ); + binder.setHolder( propertyHolder ); + binder.setId( isId ); + binder.setEntityBinder( entityBinder ); + binder.setInheritanceStatePerClass( inheritanceStatePerClass ); + binder.setBuildingContext( context ); binder.makePropertyAndBind(); return binder; } @@ -343,7 +343,7 @@ static Component fillEmbeddable( final String subpath = getPath( propertyHolder, inferredData ); LOG.tracev( "Binding component with path: {0}", subpath ); - final PropertyHolder subholder = buildPropertyHolder( + final ComponentPropertyHolder subholder = buildPropertyHolder( component, subpath, inferredData, @@ -470,6 +470,8 @@ else if ( member.isAnnotationPresent( GeneratedValue.class ) ) { } } + subholder.checkPropertyConsistency(); + if ( compositeUserType != null ) { processCompositeUserType( component, compositeUserType ); } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java index ee45564cc24d..eec071de18f1 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java @@ -49,7 +49,7 @@ public static PropertyHolder buildPropertyHolder( * * @return PropertyHolder */ - public static PropertyHolder buildPropertyHolder( + public static ComponentPropertyHolder buildPropertyHolder( Component component, String path, PropertyData inferredData, diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/embedded/EmbeddableA.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/embedded/EmbeddableA.java index 04dc114022e4..41740cea5ddf 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/embedded/EmbeddableA.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/embedded/EmbeddableA.java @@ -17,11 +17,11 @@ */ @Embeddable public class EmbeddableA { - + @Embedded @AttributeOverrides({@AttributeOverride(name = "embedAttrB" , column = @Column(table = "TableB"))}) private EmbeddableB embedB; - + @Column(table = "TableB") private String embedAttrA; public EmbeddableB getEmbedB() { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/NestedEmbeddedObjectWithASecondaryTableTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/NestedEmbeddedObjectWithASecondaryTableTest.java new file mode 100644 index 000000000000..b2ccded06b15 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/NestedEmbeddedObjectWithASecondaryTableTest.java @@ -0,0 +1,71 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html. + */ +package org.hibernate.orm.test.mapping.embeddable; + +import jakarta.persistence.AttributeOverride; +import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.persistence.Embedded; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.SecondaryTable; +import jakarta.persistence.Table; +import org.hibernate.boot.MetadataSources; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.ServiceRegistryScope; +import org.junit.jupiter.api.Test; + +/** + * Test passes if Author#name is renamed to Author#aname because it will be read before house (alphabetical order). + * The issue occurs if the nested embedded is read first, due to the table calculation (ComponentPropertyHolder#addProperty) used by the embedded, which retrieves the table from the first property. + * + * @author Vincent Bouthinon + */ +@SuppressWarnings("JUnitMalformedDeclaration") +@ServiceRegistry() +@JiraKey("HHH-19272") +class NestedEmbeddedObjectWithASecondaryTableTest { + + @Test + void testNestedEmbeddedAndSecondaryTables(ServiceRegistryScope registryScope) { + final MetadataSources metadataSources = new MetadataSources( registryScope.getRegistry() ) + .addAnnotatedClasses( Book.class, Author.class, House.class ); + metadataSources.buildMetadata(); + } + + @Entity(name = "book") + @Table(name = "TBOOK") + @SecondaryTable(name = "TSECONDARYTABLE") + public static class Book { + + @Id + @GeneratedValue + private Long id; + + @AttributeOverride(name = "name", column = @Column(name = "authorName", table = "TSECONDARYTABLE")) + @Embedded + private Author author; + + } + + @Embeddable + public static class Author { + + @AttributeOverride(name = "name", column = @Column(name = "houseName", table = "TSECONDARYTABLE")) + @Embedded + private House house; + + private String name; + } + + @Embeddable + public static class House { + private String name; + } +} diff --git a/hibernate-core/src/test/java17/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java b/hibernate-core/src/test/java17/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java new file mode 100644 index 000000000000..13f40d09c6e1 --- /dev/null +++ b/hibernate-core/src/test/java17/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java @@ -0,0 +1,144 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html. + */ +package org.hibernate.orm.test.records; + +import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.SecondaryTable; +import jakarta.persistence.Table; +import org.hibernate.AnnotationException; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.ServiceRegistryScope; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +@JiraKey("HHH-19542") +@DomainModel(annotatedClasses = { + RecordNestedEmbeddedWithASecondaryTableTest.UserEntity.class +}) +@SessionFactory +class RecordNestedEmbeddedWithASecondaryTableTest { + + private UserEntity user; + + @BeforeAll + void prepare(SessionFactoryScope scope) { + scope.inTransaction( session -> { + Person person = new Person( new FullName( "Sylvain", "Lecoy" ), 38 ); + user = new UserEntity( person ); + session.persist( user ); + } ); + } + + @Test + void test(SessionFactoryScope scope) { + scope.inTransaction(session -> { + UserEntity entity = session.find( UserEntity.class, user.id ); + assertThat( entity ).isNotNull(); + assertThat( entity.id ).isEqualTo( user.id ); + assertThat( entity.person ).isNotNull(); + assertThat( entity.person.age ).isEqualTo( 38 ); + assertThat( entity.person.fullName.firstName ).isEqualTo( "Sylvain" ); + assertThat( entity.person.fullName.lastName ).isEqualTo( "Lecoy" ); + }); + } + + @Test + void test(ServiceRegistryScope scope) { + final StandardServiceRegistry registry = scope.getRegistry(); + final MetadataSources sources = new MetadataSources( registry ).addAnnotatedClass( UserEntity1.class ); + + try { + sources.buildMetadata(); + fail( "Expecting to fail" ); + } catch (AnnotationException expected) { + assertThat( expected ).hasMessageContaining( "all properties of the embeddable class must map to the same table" ); + } + } + + @Entity + @Table(name = "UserEntity") + @SecondaryTable(name = "Person") + static class UserEntity { + @Id + @GeneratedValue + private Integer id; + private Person person; + + public UserEntity( + final Person person) { + this.person = person; + } + + protected UserEntity() { + + } + } + + @Embeddable + record Person( + FullName fullName, + @Column(table = "Person") + Integer age) { + + } + + @Embeddable + record FullName( + @Column(table = "Person") + String firstName, + @Column(table = "Person") + String lastName) { + + } + + @Entity + @Table(name = "UserEntity") + @SecondaryTable(name = "Person") + public static class UserEntity1 { + @Id + @GeneratedValue + private Integer id; + private Person1 person; + + public UserEntity1( + final Person1 person) { + this.person = person; + } + + protected UserEntity1() { + + } + } + + @Embeddable + public record Person1( + FullName1 fullName, + @Column(table = "Person") + Integer age) { + + } + + @Embeddable + public record FullName1( + @Column(table = "Person") + String firstName, + String lastName) { + + } +} From 843d2f3c959ab95cffe59d6edacde240af9202a3 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Fri, 4 Jul 2025 16:42:36 +0200 Subject: [PATCH 2/2] HHH-19542 Ensure columns in embeddables default to correct table --- .../boot/model/internal/AnnotatedColumns.java | 12 +++++- .../internal/ComponentPropertyHolder.java | 41 ++++--------------- .../boot/model/internal/EmbeddableBinder.java | 4 +- .../model/internal/PropertyHolderBuilder.java | 2 +- 4 files changed, 20 insertions(+), 39 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotatedColumns.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotatedColumns.java index 96d0e30b2eeb..954a83cc4a8b 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotatedColumns.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotatedColumns.java @@ -106,7 +106,7 @@ public boolean isSecondary() { final String explicitTableName = firstColumn.getExplicitTableName(); //note: checkPropertyConsistency() is responsible for ensuring they all have the same table name return isNotEmpty( explicitTableName ) - && !getPropertyHolder().getTable().getName().equals( explicitTableName ); + && !getOwnerTable().getName().equals( explicitTableName ); } /** @@ -125,10 +125,18 @@ public Table getTable() { // all the columns have to be mapped to the same table // even though at the annotation level it looks like // they could each specify a different table - return isSecondary() ? getJoin().getTable() : getPropertyHolder().getTable(); + return isSecondary() ? getJoin().getTable() : getOwnerTable(); } } + private Table getOwnerTable() { + PropertyHolder holder = getPropertyHolder(); + while ( holder instanceof ComponentPropertyHolder ) { + holder = ( (ComponentPropertyHolder) holder ).parent; + } + return holder.getTable(); + } + public void setTable(Table table) { this.table = table; } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java index 6875bf161378..3fc789b2f906 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java @@ -6,18 +6,14 @@ */ package org.hibernate.boot.model.internal; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.Objects; import org.hibernate.AnnotationException; import org.hibernate.annotations.common.reflection.XClass; import org.hibernate.annotations.common.reflection.XProperty; import org.hibernate.boot.spi.MetadataBuildingContext; import org.hibernate.boot.spi.PropertyData; -import org.hibernate.internal.util.StringHelper; import org.hibernate.mapping.AggregateColumn; import org.hibernate.mapping.Component; import org.hibernate.mapping.Join; @@ -78,7 +74,6 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder { private final String embeddedAttributeName; private final Map attributeConversionInfoMap; - private final List annotatedColumns; public ComponentPropertyHolder( Component component, @@ -105,12 +100,6 @@ public ComponentPropertyHolder( this.embeddedAttributeName = ""; this.attributeConversionInfoMap = processAttributeConversions( inferredData.getClassOrPluralElement() ); } - - if ( parent instanceof ComponentPropertyHolder ) { - this.annotatedColumns = ( (ComponentPropertyHolder) parent ).annotatedColumns; - } else { - this.annotatedColumns = new ArrayList<>(); - } } /** @@ -281,7 +270,12 @@ public void addProperty(Property property, AnnotatedColumns columns, XClass decl // Check table matches between the component and the columns // if not, change the component table if no properties are set // if a property is set already the core cannot support that - final Table table = property.getValue().getTable(); + assert columns == null || property.getValue().getTable() == columns.getTable(); + setTable( property.getValue().getTable() ); + addProperty( property, declaringClass ); + } + + private void setTable(Table table) { if ( !table.equals( getTable() ) ) { if ( component.getPropertySpan() == 0 ) { component.setTable( table ); @@ -293,27 +287,8 @@ public void addProperty(Property property, AnnotatedColumns columns, XClass decl + " (all properties of the embeddable class must map to the same table)" ); } - } - if ( columns != null ) { - annotatedColumns.addAll( columns.getColumns() ); - } - addProperty( property, declaringClass ); - } - - public void checkPropertyConsistency() { - if ( annotatedColumns.size() > 1 ) { - for ( int currentIndex = 1; currentIndex < annotatedColumns.size(); currentIndex++ ) { - final AnnotatedColumn current = annotatedColumns.get( currentIndex ); - final AnnotatedColumn previous = annotatedColumns.get( currentIndex - 1 ); - if ( !Objects.equals( - StringHelper.nullIfEmpty( current.getExplicitTableName() ), - StringHelper.nullIfEmpty( previous.getExplicitTableName() ) ) ) { - throw new AnnotationException( - "Embeddable class '" + component.getComponentClassName() - + "' has properties mapped to two different tables" - + " (all properties of the embeddable class must map to the same table)" - ); - } + if ( parent instanceof ComponentPropertyHolder ) { + ( (ComponentPropertyHolder) parent ).setTable( table ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java index 829eef2566d3..df4348220f13 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java @@ -343,7 +343,7 @@ static Component fillEmbeddable( final String subpath = getPath( propertyHolder, inferredData ); LOG.tracev( "Binding component with path: {0}", subpath ); - final ComponentPropertyHolder subholder = buildPropertyHolder( + final PropertyHolder subholder = buildPropertyHolder( component, subpath, inferredData, @@ -470,8 +470,6 @@ else if ( member.isAnnotationPresent( GeneratedValue.class ) ) { } } - subholder.checkPropertyConsistency(); - if ( compositeUserType != null ) { processCompositeUserType( component, compositeUserType ); } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java index eec071de18f1..ee45564cc24d 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java @@ -49,7 +49,7 @@ public static PropertyHolder buildPropertyHolder( * * @return PropertyHolder */ - public static ComponentPropertyHolder buildPropertyHolder( + public static PropertyHolder buildPropertyHolder( Component component, String path, PropertyData inferredData,