Skip to content

Commit 77d334a

Browse files
itsmoonrackbeikov
itsmoonrack
authored and
beikov
committed
HHH-19542: Embeddable property order for SecondaryTable is no longer causing issue
1 parent eca7408 commit 77d334a

File tree

6 files changed

+268
-21
lines changed

6 files changed

+268
-21
lines changed

‎hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,18 @@
66
*/
77
package org.hibernate.boot.model.internal;
88

9+
import java.util.ArrayList;
910
import java.util.HashMap;
11+
import java.util.List;
1012
import java.util.Map;
13+
import java.util.Objects;
1114

1215
import org.hibernate.AnnotationException;
1316
import org.hibernate.annotations.common.reflection.XClass;
1417
import org.hibernate.annotations.common.reflection.XProperty;
1518
import org.hibernate.boot.spi.MetadataBuildingContext;
1619
import org.hibernate.boot.spi.PropertyData;
20+
import org.hibernate.internal.util.StringHelper;
1721
import org.hibernate.mapping.AggregateColumn;
1822
import org.hibernate.mapping.Component;
1923
import org.hibernate.mapping.Join;
@@ -74,6 +78,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
7478

7579
private final String embeddedAttributeName;
7680
private final Map<String,AttributeConversionInfo> attributeConversionInfoMap;
81+
private final List<AnnotatedColumn> annotatedColumns;
7782

7883
public ComponentPropertyHolder(
7984
Component component,
@@ -100,6 +105,12 @@ public ComponentPropertyHolder(
100105
this.embeddedAttributeName = "";
101106
this.attributeConversionInfoMap = processAttributeConversions( inferredData.getClassOrPluralElement() );
102107
}
108+
109+
if ( parent instanceof ComponentPropertyHolder ) {
110+
this.annotatedColumns = ( (ComponentPropertyHolder) parent ).annotatedColumns;
111+
} else {
112+
this.annotatedColumns = new ArrayList<>();
113+
}
103114
}
104115

105116
/**
@@ -266,26 +277,45 @@ public String getEntityName() {
266277

267278
@Override
268279
public void addProperty(Property property, AnnotatedColumns columns, XClass declaringClass) {
269-
//Ejb3Column.checkPropertyConsistency( ); //already called earlier
280+
//AnnotatedColumns.checkPropertyConsistency( ); //already called earlier
270281
// Check table matches between the component and the columns
271282
// if not, change the component table if no properties are set
272283
// if a property is set already the core cannot support that
284+
final Table table = property.getValue().getTable();
285+
if ( !table.equals( getTable() ) ) {
286+
if ( component.getPropertySpan() == 0 ) {
287+
component.setTable( table );
288+
}
289+
else {
290+
throw new AnnotationException(
291+
"Embeddable class '" + component.getComponentClassName()
292+
+ "' has properties mapped to two different tables"
293+
+ " (all properties of the embeddable class must map to the same table)"
294+
);
295+
}
296+
}
273297
if ( columns != null ) {
274-
final Table table = columns.getTable();
275-
if ( !table.equals( getTable() ) ) {
276-
if ( component.getPropertySpan() == 0 ) {
277-
component.setTable( table );
278-
}
279-
else {
298+
annotatedColumns.addAll( columns.getColumns() );
299+
}
300+
addProperty( property, declaringClass );
301+
}
302+
303+
public void checkPropertyConsistency() {
304+
if ( annotatedColumns.size() > 1 ) {
305+
for ( int currentIndex = 1; currentIndex < annotatedColumns.size(); currentIndex++ ) {
306+
final AnnotatedColumn current = annotatedColumns.get( currentIndex );
307+
final AnnotatedColumn previous = annotatedColumns.get( currentIndex - 1 );
308+
if ( !Objects.equals(
309+
StringHelper.nullIfEmpty( current.getExplicitTableName() ),
310+
StringHelper.nullIfEmpty( previous.getExplicitTableName() ) ) ) {
280311
throw new AnnotationException(
281312
"Embeddable class '" + component.getComponentClassName()
282-
+ "' has properties mapped to two different tables"
283-
+ " (all properties of the embeddable class must map to the same table)"
313+
+ "' has properties mapped to two different tables"
314+
+ " (all properties of the embeddable class must map to the same table)"
284315
);
285316
}
286317
}
287318
}
288-
addProperty( property, declaringClass );
289319
}
290320

291321
@Override

‎hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -244,15 +244,15 @@ private static PropertyBinder createEmbeddedProperty(
244244
final PropertyBinder binder = new PropertyBinder();
245245
binder.setDeclaringClass( inferredData.getDeclaringClass() );
246246
binder.setName( inferredData.getPropertyName() );
247-
binder.setValue(component);
247+
binder.setValue( component );
248248
binder.setProperty( inferredData.getProperty() );
249249
binder.setAccessType( inferredData.getDefaultAccess() );
250-
binder.setEmbedded(isComponentEmbedded);
251-
binder.setHolder(propertyHolder);
252-
binder.setId(isId);
253-
binder.setEntityBinder(entityBinder);
254-
binder.setInheritanceStatePerClass(inheritanceStatePerClass);
255-
binder.setBuildingContext(context);
250+
binder.setEmbedded( isComponentEmbedded );
251+
binder.setHolder( propertyHolder );
252+
binder.setId( isId );
253+
binder.setEntityBinder( entityBinder );
254+
binder.setInheritanceStatePerClass( inheritanceStatePerClass );
255+
binder.setBuildingContext( context );
256256
binder.makePropertyAndBind();
257257
return binder;
258258
}
@@ -343,7 +343,7 @@ static Component fillEmbeddable(
343343

344344
final String subpath = getPath( propertyHolder, inferredData );
345345
LOG.tracev( "Binding component with path: {0}", subpath );
346-
final PropertyHolder subholder = buildPropertyHolder(
346+
final ComponentPropertyHolder subholder = buildPropertyHolder(
347347
component,
348348
subpath,
349349
inferredData,
@@ -470,6 +470,8 @@ else if ( member.isAnnotationPresent( GeneratedValue.class ) ) {
470470
}
471471
}
472472

473+
subholder.checkPropertyConsistency();
474+
473475
if ( compositeUserType != null ) {
474476
processCompositeUserType( component, compositeUserType );
475477
}

‎hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static PropertyHolder buildPropertyHolder(
4949
*
5050
* @return PropertyHolder
5151
*/
52-
public static PropertyHolder buildPropertyHolder(
52+
public static ComponentPropertyHolder buildPropertyHolder(
5353
Component component,
5454
String path,
5555
PropertyData inferredData,

‎hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/embedded/EmbeddableA.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
*/
1818
@Embeddable
1919
public class EmbeddableA {
20-
20+
2121
@Embedded
2222
@AttributeOverrides({@AttributeOverride(name = "embedAttrB" , column = @Column(table = "TableB"))})
2323
private EmbeddableB embedB;
24-
24+
@Column(table = "TableB")
2525
private String embedAttrA;
2626

2727
public EmbeddableB getEmbedB() {

‎hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/NestedEmbeddedObjectWithASecondaryTableTest.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
5+
* See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html.
6+
*/
7+
package org.hibernate.orm.test.mapping.embeddable;
8+
9+
import jakarta.persistence.AttributeOverride;
10+
import jakarta.persistence.Column;
11+
import jakarta.persistence.Embeddable;
12+
import jakarta.persistence.Embedded;
13+
import jakarta.persistence.Entity;
14+
import jakarta.persistence.GeneratedValue;
15+
import jakarta.persistence.Id;
16+
import jakarta.persistence.SecondaryTable;
17+
import jakarta.persistence.Table;
18+
import org.hibernate.boot.MetadataSources;
19+
import org.hibernate.testing.orm.junit.JiraKey;
20+
import org.hibernate.testing.orm.junit.ServiceRegistry;
21+
import org.hibernate.testing.orm.junit.ServiceRegistryScope;
22+
import org.junit.jupiter.api.Test;
23+
24+
/**
25+
* Test passes if Author#name is renamed to Author#aname because it will be read before house (alphabetical order).
26+
* 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.
27+
*
28+
* @author Vincent Bouthinon
29+
*/
30+
@SuppressWarnings("JUnitMalformedDeclaration")
31+
@ServiceRegistry()
32+
@JiraKey("HHH-19272")
33+
class NestedEmbeddedObjectWithASecondaryTableTest {
34+
35+
@Test
36+
void testNestedEmbeddedAndSecondaryTables(ServiceRegistryScope registryScope) {
37+
final MetadataSources metadataSources = new MetadataSources( registryScope.getRegistry() )
38+
.addAnnotatedClasses( Book.class, Author.class, House.class );
39+
metadataSources.buildMetadata();
40+
}
41+
42+
@Entity(name = "book")
43+
@Table(name = "TBOOK")
44+
@SecondaryTable(name = "TSECONDARYTABLE")
45+
public static class Book {
46+
47+
@Id
48+
@GeneratedValue
49+
private Long id;
50+
51+
@AttributeOverride(name = "name", column = @Column(name = "authorName", table = "TSECONDARYTABLE"))
52+
@Embedded
53+
private Author author;
54+
55+
}
56+
57+
@Embeddable
58+
public static class Author {
59+
60+
@AttributeOverride(name = "name", column = @Column(name = "houseName", table = "TSECONDARYTABLE"))
61+
@Embedded
62+
private House house;
63+
64+
private String name;
65+
}
66+
67+
@Embeddable
68+
public static class House {
69+
private String name;
70+
}
71+
}

‎hibernate-core/src/test/java17/org/hibernate/orm/test/records/RecordNestedEmbeddedWithASecondaryTableTest.java‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
5+
* See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html.
6+
*/
7+
package org.hibernate.orm.test.records;
8+
9+
import jakarta.persistence.Column;
10+
import jakarta.persistence.Embeddable;
11+
import jakarta.persistence.Entity;
12+
import jakarta.persistence.GeneratedValue;
13+
import jakarta.persistence.Id;
14+
import jakarta.persistence.SecondaryTable;
15+
import jakarta.persistence.Table;
16+
import org.hibernate.AnnotationException;
17+
import org.hibernate.boot.MetadataSources;
18+
import org.hibernate.boot.registry.StandardServiceRegistry;
19+
import org.hibernate.testing.orm.junit.DomainModel;
20+
import org.hibernate.testing.orm.junit.JiraKey;
21+
import org.hibernate.testing.orm.junit.ServiceRegistryScope;
22+
import org.hibernate.testing.orm.junit.SessionFactory;
23+
import org.hibernate.testing.orm.junit.SessionFactoryScope;
24+
import org.junit.jupiter.api.BeforeAll;
25+
import org.junit.jupiter.api.Test;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
import static org.assertj.core.api.Assertions.fail;
29+
30+
@JiraKey("HHH-19542")
31+
@DomainModel(annotatedClasses = {
32+
RecordNestedEmbeddedWithASecondaryTableTest.UserEntity.class
33+
})
34+
@SessionFactory
35+
class RecordNestedEmbeddedWithASecondaryTableTest {
36+
37+
private UserEntity user;
38+
39+
@BeforeAll
40+
void prepare(SessionFactoryScope scope) {
41+
scope.inTransaction( session -> {
42+
Person person = new Person( new FullName( "Sylvain", "Lecoy" ), 38 );
43+
user = new UserEntity( person );
44+
session.persist( user );
45+
} );
46+
}
47+
48+
@Test
49+
void test(SessionFactoryScope scope) {
50+
scope.inTransaction(session -> {
51+
UserEntity entity = session.find( UserEntity.class, user.id );
52+
assertThat( entity ).isNotNull();
53+
assertThat( entity.id ).isEqualTo( user.id );
54+
assertThat( entity.person ).isNotNull();
55+
assertThat( entity.person.age ).isEqualTo( 38 );
56+
assertThat( entity.person.fullName.firstName ).isEqualTo( "Sylvain" );
57+
assertThat( entity.person.fullName.lastName ).isEqualTo( "Lecoy" );
58+
});
59+
}
60+
61+
@Test
62+
void test(ServiceRegistryScope scope) {
63+
final StandardServiceRegistry registry = scope.getRegistry();
64+
final MetadataSources sources = new MetadataSources( registry ).addAnnotatedClass( UserEntity1.class );
65+
66+
try {
67+
sources.buildMetadata();
68+
fail( "Expecting to fail" );
69+
} catch (AnnotationException expected) {
70+
assertThat( expected ).hasMessageContaining( "all properties of the embeddable class must map to the same table" );
71+
}
72+
}
73+
74+
@Entity
75+
@Table(name = "UserEntity")
76+
@SecondaryTable(name = "Person")
77+
static class UserEntity {
78+
@Id
79+
@GeneratedValue
80+
private Integer id;
81+
private Person person;
82+
83+
public UserEntity(
84+
final Person person) {
85+
this.person = person;
86+
}
87+
88+
protected UserEntity() {
89+
90+
}
91+
}
92+
93+
@Embeddable
94+
record Person(
95+
FullName fullName,
96+
@Column(table = "Person")
97+
Integer age) {
98+
99+
}
100+
101+
@Embeddable
102+
record FullName(
103+
@Column(table = "Person")
104+
String firstName,
105+
@Column(table = "Person")
106+
String lastName) {
107+
108+
}
109+
110+
@Entity
111+
@Table(name = "UserEntity")
112+
@SecondaryTable(name = "Person")
113+
public static class UserEntity1 {
114+
@Id
115+
@GeneratedValue
116+
private Integer id;
117+
private Person1 person;
118+
119+
public UserEntity1(
120+
final Person1 person) {
121+
this.person = person;
122+
}
123+
124+
protected UserEntity1() {
125+
126+
}
127+
}
128+
129+
@Embeddable
130+
public record Person1(
131+
FullName1 fullName,
132+
@Column(table = "Person")
133+
Integer age) {
134+
135+
}
136+
137+
@Embeddable
138+
public record FullName1(
139+
@Column(table = "Person")
140+
String firstName,
141+
String lastName) {
142+
143+
}
144+
}

0 commit comments

Comments
 (0)