HHH-18158, HHH-18251, HHH-18062 fix composite id handling

by rolling back HHH-15184

Signed-off-by: Gavin King <gavin@hibernate.org>

Author: gavinking

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

@@ -377,7 +377,7 @@ public void doSecondPass(Map persistentClasses) throws MappingException {
 						else {
 							if ( componentClass == Object.class ) {
 								// Object is not a valid component class, but that is what we get when using a type variable
-								component.getProperties().clear();
+								component.clearProperties();
 							}
 							else {
 								final Iterator<Property> propertyIterator = component.getPropertyIterator();

hibernate-core/src/main/java/org/hibernate/mapping/Component.java

@@ -8,7 +8,6 @@
 
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -61,6 +60,7 @@
 import org.hibernate.type.descriptor.java.spi.JavaTypeRegistry;
 import org.hibernate.usertype.CompositeUserType;
 
+import static java.util.Collections.unmodifiableList;
 import static java.util.stream.Collectors.toList;
 import static org.hibernate.generator.EventType.INSERT;
 import static org.hibernate.internal.util.StringHelper.qualify;
@@ -108,10 +108,6 @@ public class Component extends SimpleValue implements MetaAttributable, Sortable
 	private QualifiedName structName;
 	private String[] structColumnNames;
 	private transient Class<?> componentClass;
-	// lazily computed based on 'properties' field: invalidate by setting to null when properties are modified
-	private transient List<Selectable> cachedSelectables;
-	// lazily computed based on 'properties' field: invalidate by setting to null when properties are modified
-	private transient List<Column> cachedColumns;
 
 	private transient Generator builtIdentifierGenerator;
 
@@ -186,7 +182,6 @@ public void addProperty(Property p, XClass declaringClass) {
 			}
 			propertyDeclaringClasses.put( p, declaringClass.getName() );
 		}
-		propertiesListModified();
 	}
 
 	public void addProperty(Property p) {
@@ -200,45 +195,33 @@ public String getPropertyDeclaringClass(Property p) {
 		return null;
 	}
 
-	private void propertiesListModified() {
-		this.cachedSelectables = null;
-		this.cachedColumns = null;
-	}
-
 	@Override
 	public void addColumn(Column column) {
 		throw new UnsupportedOperationException("Cant add a column to a component");
 	}
 
 	@Override
 	public List<Selectable> getSelectables() {
-		if ( cachedSelectables == null ) {
-			final List<Selectable> selectables = properties.stream()
-					.flatMap( p -> p.getSelectables().stream() )
-					.collect( toList() );
-			if ( discriminator != null ) {
-				selectables.addAll( discriminator.getSelectables() );
-			}
-			cachedSelectables = selectables;
+		final List<Selectable> selectables = new ArrayList<>( properties.size() + 2 );
+		for ( Property property : properties ) {
+			selectables.addAll( property.getSelectables() );
+		}
+		if ( discriminator != null ) {
+			selectables.addAll( discriminator.getSelectables() );
 		}
-		return cachedSelectables;
+		return unmodifiableList( selectables );
 	}
 
 	@Override
 	public List<Column> getColumns() {
-		if ( cachedColumns != null ) {
-			return cachedColumns;
+		final List<Column> columns = new ArrayList<>( properties.size() + 2 );
+		for ( Property property : properties ) {
+			columns.addAll( property.getValue().getColumns() );
 		}
-		else {
-			final List<Column> columns = properties.stream()
-					.flatMap( p -> p.getValue().getColumns().stream() )
-					.collect( toList() );
-			if ( discriminator != null ) {
-				columns.addAll( discriminator.getColumns() );
-			}
-			this.cachedColumns = Collections.unmodifiableList( columns );
-			return cachedColumns;
+		if ( discriminator != null ) {
+			columns.addAll( discriminator.getColumns() );
 		}
+		return unmodifiableList( columns );
 	}
 
 	public boolean isEmbedded() {
@@ -763,6 +746,10 @@ public String[] getPropertyNames() {
 		return propertyNames;
 	}
 
+	public void clearProperties() {
+		properties.clear();
+	}
+
 	public static class StandardGenerationContextLocator
 			implements CompositeNestedGeneratedValueGenerator.GenerationContextLocator {
 		private final String entityName;
@@ -909,7 +896,6 @@ private int[] sortProperties(boolean forceRetainOriginalOrder) {
 				}
 			}
 		}
-		propertiesListModified();
 		return this.originalPropertyOrder = originalPropertyOrder;
 	}
 

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

@@ -0,0 +1,94 @@
+/*
+ * 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.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.GenerationType;
+import jakarta.persistence.Id;
+import jakarta.persistence.IdClass;
+import jakarta.persistence.ManyToOne;
+import jakarta.persistence.MapsId;
+import jakarta.persistence.OneToMany;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import static jakarta.persistence.CascadeType.MERGE;
+import static jakarta.persistence.CascadeType.PERSIST;
+import static jakarta.persistence.CascadeType.REMOVE;
+
+@DomainModel( annotatedClasses = {
+		RecordIdClassTest2.MyParentEntity.class,
+		RecordIdClassTest2.MyChildEntity.class} )
+@SessionFactory
+public class RecordIdClassTest2 {
+
+	@Test
+	public void testPersist(SessionFactoryScope scope) {
+		scope.inTransaction( s-> {
+			MyParentEntity ue = new MyParentEntity("hello");
+			MyChildEntity uae = new MyChildEntity(ue, "world");
+			ue.children.add(uae);
+			s.persist(ue);
+		});
+	}
+
+	public record MyRecord(Long code, String qualifier) {}
+
+	@Entity
+	@IdClass(MyRecord.class)
+	public static class MyChildEntity {
+
+		@Id
+		Long code;
+
+		@Id
+		String qualifier;
+
+		String text;
+
+		@ManyToOne
+		@MapsId("code")
+		private MyParentEntity parent;
+
+		public MyChildEntity(MyParentEntity parent, String qualifier) {
+			this.parent = parent;
+			this.qualifier = qualifier;
+		}
+
+		MyChildEntity() {
+		}
+	}
+
+	@Entity
+	public static class MyParentEntity {
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		Long code;
+
+		String description;
+
+		@OneToMany(
+				cascade = {PERSIST, MERGE, REMOVE},
+				mappedBy = "parent",
+				orphanRemoval = true)
+		private Set<MyChildEntity> children = new HashSet<>();
+
+		public MyParentEntity(String description) {
+			this.description = description;
+		}
+
+		MyParentEntity() {
+		}
+	}
+}