From e1370d74f1be93a3633df3370da6f03c362056a3 Mon Sep 17 00:00:00 2001 From: Sylvain Lecoy Date: Tue, 17 Jun 2025 03:01:24 +0200 Subject: [PATCH] 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 | 1 + ...EmbeddedObjectWithASecondaryTableTest.java | 2 - ...NestedEmbeddedWithASecondaryTableTest.java | 142 ++++++++++++++++++ 6 files changed, 194 insertions(+), 21 deletions(-) create mode 100644 hibernate-core/src/test/java/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 e390b646709a..52bb2c7e7b6b 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 @@ -4,12 +4,16 @@ */ 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.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; @@ -68,6 +72,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder { private final String embeddedAttributeName; private final Map attributeConversionInfoMap; + private final List annotatedColumns; public ComponentPropertyHolder( Component component, @@ -94,6 +99,12 @@ public ComponentPropertyHolder( this.embeddedAttributeName = ""; this.attributeConversionInfoMap = processAttributeConversions( inferredData.getClassOrElementType() ); } + + if ( parent instanceof ComponentPropertyHolder parentHolder ) { + this.annotatedColumns = parentHolder.annotatedColumns; + } else { + this.annotatedColumns = new ArrayList<>(); + } } /** @@ -221,26 +232,45 @@ public String getEntityName() { @Override public void addProperty(Property property, MemberDetails attributeMemberDetails, AnnotatedColumns columns, ClassDetails 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, attributeMemberDetails, 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, attributeMemberDetails, 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 f6cfeabc8d2a..6609b6ef7977 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 @@ -352,15 +352,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.setMemberDetails( inferredData.getAttributeMember() ); 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; } @@ -455,7 +455,7 @@ static Component fillEmbeddable( if ( LOG.isDebugEnabled() ) { LOG.debug( "Binding component with path: " + subpath ); } - final PropertyHolder subholder = buildPropertyHolder( + final ComponentPropertyHolder subholder = buildPropertyHolder( component, subpath, inferredData, @@ -579,6 +579,8 @@ else if ( member.hasDirectAnnotationUsage( 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 b4c8c50057ab..066fdd4ca725 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 @@ -47,7 +47,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 fdcb1b234b5b..603f64eca1a4 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 @@ -20,6 +20,7 @@ public class EmbeddableA { @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 index 216aad906f7d..6017267d595a 100644 --- 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 @@ -15,7 +15,6 @@ import jakarta.persistence.Table; import org.hibernate.boot.MetadataSources; import org.hibernate.testing.orm.junit.JiraKey; -import org.hibernate.testing.orm.junit.NotImplementedYet; import org.hibernate.testing.orm.junit.ServiceRegistry; import org.hibernate.testing.orm.junit.ServiceRegistryScope; import org.junit.jupiter.api.Test; @@ -32,7 +31,6 @@ class NestedEmbeddedObjectWithASecondaryTableTest { @Test - @NotImplementedYet void testNestedEmbeddedAndSecondaryTables(ServiceRegistryScope registryScope) { final MetadataSources metadataSources = new MetadataSources( registryScope.getRegistry() ) .addAnnotatedClasses( Book.class, Author.class, House.class ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java new file mode 100644 index 000000000000..2fe03d676ea4 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java @@ -0,0 +1,142 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +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) { + + } +}