HHH-18229 Handle null owner key for collections

soul2zimate · beikov · commit a894991975d4 · 2025-03-06T16:46:02.000+01:00
(cherry picked from commit 1f08501)
diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java
@@ -201,19 +201,16 @@ private void addCollectionKey(
 			PersistenceContext persistenceContext) {
 		if ( o instanceof PersistentCollection ) {
 			final CollectionPersister collectionPersister = pluralAttributeMapping.getCollectionDescriptor();
-			final CollectionKey collectionKey = new CollectionKey(
+			final Object key = ( (AbstractEntityPersister) getPersister() ).getCollectionKey(
 					collectionPersister,
-					( (AbstractEntityPersister) getPersister() ).getCollectionKey(
-							collectionPersister,
-							getInstance(),
-							persistenceContext.getEntry( getInstance() ),
-							getSession()
-					)
-			);
-			persistenceContext.addCollectionByKey(
-					collectionKey,
-					(PersistentCollection<?>) o
+					getInstance(),
+					persistenceContext.getEntry( getInstance() ),
+					getSession()
 			);
+			if ( key != null ) {
+				final CollectionKey collectionKey = new CollectionKey( collectionPersister, key );
+				persistenceContext.addCollectionByKey( collectionKey, (PersistentCollection<?>) o );
+			}
 		}
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java
@@ -17,6 +17,7 @@
 import java.util.Map;
 import java.util.UUID;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.AssertionFailure;
 import org.hibernate.FlushMode;
 import org.hibernate.HibernateException;
@@ -42,6 +43,10 @@
 import org.hibernate.type.CompositeType;
 import org.hibernate.type.Type;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import static org.hibernate.pretty.MessageHelper.collectionInfoString;
+
 /**
  * Base class implementing {@link PersistentCollection}
  *
@@ -58,16 +63,16 @@ public abstract class AbstractPersistentCollection<E> implements Serializable, P
 
 	private transient List<DelayedOperation<E>> operationQueue;
 	private transient boolean directlyAccessible;
-	private Object owner;
+	private @Nullable Object owner;
 	private int cachedSize = -1;
 
-	private String role;
-	private Object key;
+	private @Nullable String role;
+	private @Nullable Object key;
 	// collections detect changes made via their public interface and mark
 	// themselves as dirty as a performance optimization
 	private boolean dirty;
 	protected boolean elementRemoved;
-	private Serializable storedSnapshot;
+	private @Nullable Serializable storedSnapshot;
 
 	private String sessionFactoryUuid;
 	private boolean allowLoadOutsideTransaction;
@@ -84,12 +89,12 @@ protected AbstractPersistentCollection(SharedSessionContractImplementor session)
 	}
 
 	@Override
-	public final String getRole() {
+	public final @Nullable String getRole() {
 		return role;
 	}
 
 	@Override
-	public final Object getKey() {
+	public final @Nullable Object getKey() {
 		return key;
 	}
 
@@ -120,7 +125,7 @@ public final void dirty() {
 	}
 
 	@Override
-	public final Serializable getStoredSnapshot() {
+	public final @Nullable Serializable getStoredSnapshot() {
 		return storedSnapshot;
 	}
 
@@ -1351,7 +1356,7 @@ public Object getIdentifier(Object entry, int i) {
 	}
 
 	@Override
-	public Object getOwner() {
+	public @Nullable Object getOwner() {
 		return owner;
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java
@@ -11,6 +11,7 @@
 import java.util.Iterator;
 import java.util.List;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.HibernateException;
 import org.hibernate.Incubating;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
@@ -60,7 +61,7 @@ public interface PersistentCollection<E> extends LazyInitializable {
 	 *
 	 * @return The owner
 	 */
-	Object getOwner();
+	@Nullable Object getOwner();
 
 	/**
 	 * Set the reference to the owning entity
@@ -403,14 +404,14 @@ default boolean needsUpdating(
 	 *
 	 * @return the current collection key value
 	 */
-	Object getKey();
+	@Nullable Object getKey();
 
 	/**
 	 * Get the current role name
 	 *
 	 * @return the collection role name
 	 */
-	String getRole();
+	@Nullable String getRole();
 
 	/**
 	 * Is the collection unreferenced?
@@ -459,7 +460,7 @@ default boolean isDirectlyProvidedCollection(Object collection) {
 	 *
 	 * @return The internally stored snapshot state
 	 */
-	Serializable getStoredSnapshot();
+	@Nullable Serializable getStoredSnapshot();
 
 	/**
 	 * Mark the collection as dirty
diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java
@@ -875,8 +875,10 @@ public void addUninitializedCollection(CollectionPersister persister, Persistent
 
 	@Override
 	public void addUninitializedDetachedCollection(CollectionPersister persister, PersistentCollection<?> collection) {
-		final CollectionEntry ce = new CollectionEntry( persister, collection.getKey() );
-		addCollection( collection, ce, collection.getKey() );
+		final Object key = collection.getKey();
+		assert key != null;
+		final CollectionEntry ce = new CollectionEntry( persister, key );
+		addCollection( collection, ce, key );
 		if ( persister.getBatchSize() > 1 ) {
 			getBatchFetchQueue().addBatchLoadableCollection( collection, ce );
 		}
@@ -934,7 +936,7 @@ private void addCollection(PersistentCollection<?> collection, CollectionPersist
 	@Override
 	public void addInitializedDetachedCollection(CollectionPersister collectionPersister, PersistentCollection<?> collection)
 			throws HibernateException {
-		if ( collection.isUnreferenced() ) {
+		if ( collection.isUnreferenced() || collection.getKey() == null ) {
 			//treat it just like a new collection
 			addCollection( collection, collectionPersister );
 		}
diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java
@@ -18,6 +18,7 @@
 import org.hibernate.collection.spi.PersistentCollection;
 import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
+import org.hibernate.internal.util.NullnessUtil;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.pretty.MessageHelper;
 
@@ -118,8 +119,9 @@ public CollectionEntry(PersistentCollection<?> collection, SessionFactoryImpleme
 		ignore = false;
 
 		loadedKey = collection.getKey();
+		role = collection.getRole();
 		setLoadedPersister(
-				factory.getRuntimeMetamodels().getMappingMetamodel().getCollectionDescriptor( collection.getRole() )
+				factory.getRuntimeMetamodels().getMappingMetamodel().getCollectionDescriptor( NullnessUtil.castNonNull( role ) )
 		);
 
 		snapshot = collection.getStoredSnapshot();
diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java
@@ -407,7 +407,8 @@ protected void postFlush(SessionImplementor session) throws HibernateException {
 		persistenceContext.forEachCollectionEntry(
 				(persistentCollection, collectionEntry) -> {
 					collectionEntry.postFlush( persistentCollection );
-					if ( collectionEntry.getLoadedPersister() == null ) {
+					final Object key;
+					if ( collectionEntry.getLoadedPersister() == null || ( key = collectionEntry.getLoadedKey() ) == null ) {
 						//if the collection is dereferenced, unset its session reference and remove from the session cache
 						//iter.remove(); //does not work, since the entrySet is not backed by the set
 						persistentCollection.unsetSession( session );
@@ -417,7 +418,7 @@ protected void postFlush(SessionImplementor session) throws HibernateException {
 						//otherwise recreate the mapping between the collection and its key
 						CollectionKey collectionKey = new CollectionKey(
 								collectionEntry.getLoadedPersister(),
-								collectionEntry.getLoadedKey()
+								key
 						);
 						persistenceContext.addCollectionByKey( collectionKey, persistentCollection );
 					}
diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/WrapVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/WrapVisitor.java
@@ -123,22 +123,24 @@ else if ( attributeInterceptor != null
 									entry,
 									session
 							);
-							PersistentCollection<?> collectionInstance = persistenceContext.getCollection(
-									new CollectionKey( persister, key )
-							);
-
-							if ( collectionInstance == null ) {
-								// the collection has not been initialized and new collection values have been assigned,
-								// we need to be sure to delete all the collection elements before inserting the new ones
-								collectionInstance = persister.getCollectionSemantics().instantiateWrapper(
-										key,
-										persister,
-										session
+							if ( key != null ) {
+								PersistentCollection<?> collectionInstance = persistenceContext.getCollection(
+										new CollectionKey( persister, key )
 								);
-								persistenceContext.addUninitializedCollection( persister, collectionInstance, key );
-								final CollectionEntry collectionEntry = persistenceContext.getCollectionEntry(
-										collectionInstance );
-								collectionEntry.setDoremove( true );
+
+								if ( collectionInstance == null ) {
+									// the collection has not been initialized and new collection values have been assigned,
+									// we need to be sure to delete all the collection elements before inserting the new ones
+									collectionInstance = persister.getCollectionSemantics().instantiateWrapper(
+											key,
+											persister,
+											session
+									);
+									persistenceContext.addUninitializedCollection( persister, collectionInstance, key );
+									final CollectionEntry collectionEntry =
+											persistenceContext.getCollectionEntry( collectionInstance );
+									collectionEntry.setDoremove( true );
+								}
 							}
 						}
 					}
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java
@@ -20,6 +20,7 @@
 import javax.naming.NameNotFoundException;
 import javax.naming.NamingException;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.cache.CacheException;
@@ -1690,7 +1691,7 @@ void cannotResolveNonNullableTransientDependencies(
 			+ " This is likely due to unsafe use of the session (e.g. used in multiple threads concurrently, updates during entity lifecycle hooks).",
 			id = 479
 	)
-	String collectionNotProcessedByFlush(String role);
+	String collectionNotProcessedByFlush(@Nullable String role);
 
 	@LogMessage(level = WARN)
 	@Message(value = "A ManagedEntity was associated with a stale PersistenceContext. A ManagedEntity may only be associated with one PersistenceContext at a time; %s", id = 480)
diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderSubSelectFetch.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderSubSelectFetch.java
@@ -21,6 +21,7 @@
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.engine.spi.SubselectFetch;
+import org.hibernate.internal.util.NullnessUtil;
 import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.loader.ast.spi.CollectionLoader;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
@@ -147,7 +148,7 @@ public PersistentCollection<?> load(Object triggerKey, SharedSessionContractImpl
 								persistenceContext,
 								getLoadable().getCollectionDescriptor(),
 								c,
-								c.getKey(),
+								NullnessUtil.castNonNull( c.getKey() ),
 								true
 						);
 					}
diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java
@@ -29,6 +29,7 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.AssertionFailure;
 import org.hibernate.FetchMode;
 import org.hibernate.Filter;
@@ -292,6 +293,8 @@
 import org.hibernate.type.descriptor.java.spi.JavaTypeRegistry;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
@@ -1463,6 +1466,7 @@ public Object initializeLazyProperty(String fieldName, Object entity, SharedSess
 				// see if there is already a collection instance associated with the session
 				// 		NOTE : can this ever happen?
 				final Object key = getCollectionKey( persister, entity, entry, session );
+				assert key != null;
 				PersistentCollection<?> collection = persistenceContext.getCollection( new CollectionKey( persister, key ) );
 				if ( collection == null ) {
 					collection = collectionType.instantiate( session, persister, key );
@@ -1531,7 +1535,7 @@ public Object initializeLazyProperty(String fieldName, Object entity, SharedSess
 
 	}
 
-	public Object getCollectionKey(
+	public @Nullable Object getCollectionKey(
 			CollectionPersister persister,
 			Object owner,
 			EntityEntry ownerEntry,
diff --git a/hibernate-core/src/main/java/org/hibernate/type/CollectionType.java b/hibernate-core/src/main/java/org/hibernate/type/CollectionType.java
@@ -19,6 +19,7 @@
 import java.util.SortedMap;
 import java.util.TreeMap;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.Hibernate;
 import org.hibernate.HibernateException;
 import org.hibernate.MappingException;
@@ -48,6 +49,8 @@
 
 import org.jboss.logging.Logger;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * A type that handles Hibernate {@code PersistentCollection}s (including arrays).
  *
@@ -366,7 +369,7 @@ public ForeignKeyDirection getForeignKeyDirection() {
 	 * @param session The session from which the request is originating.
 	 * @return The collection owner's key
 	 */
-	public Object getKeyOfOwner(Object owner, SharedSessionContractImplementor session) {
+	public @Nullable Object getKeyOfOwner(Object owner, SharedSessionContractImplementor session) {
 		final PersistenceContext pc = session.getPersistenceContextInternal();
 
 		EntityEntry entityEntry = pc.getEntry( owner );
@@ -380,28 +383,10 @@ public Object getKeyOfOwner(Object owner, SharedSessionContractImplementor sessi
 			return entityEntry.getId();
 		}
 		else {
-			// TODO: at the point where we are resolving collection references, we don't
-			// know if the uk value has been resolved (depends if it was earlier or
-			// later in the mapping document) - now, we could try and use e.getStatus()
-			// to decide to semiResolve(), trouble is that initializeEntity() reuses
-			// the same array for resolved and hydrated values
-			Object id = entityEntry.getLoadedState() != null
-					? entityEntry.getLoadedValue( foreignKeyPropertyName )
-					: entityEntry.getPersister().getPropertyValue( owner, foreignKeyPropertyName );
-
-			// NOTE VERY HACKISH WORKAROUND!!
-			// TODO: Fix this so it will work for non-POJO entity mode
-			Type keyType = getPersister( session ).getKeyType();
-			if ( !keyType.getReturnedClass().isInstance( id ) ) {
-				throw new UnsupportedOperationException( "Re-work support for semi-resolve" );
-//				id = keyType.semiResolve(
-//						entityEntry.getLoadedValue( foreignKeyPropertyName ),
-//						session,
-//						owner
-//				);
-			}
-
-			return id;
+			final Object loadedValue = entityEntry.getLoadedValue( foreignKeyPropertyName );
+			return loadedValue == null
+					? entityEntry.getPersister().getPropertyValue( owner, foreignKeyPropertyName )
+					: loadedValue;
 		}
 	}
 
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/collection/NonPkCompositeJoinColumnCollectionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/collection/NonPkCompositeJoinColumnCollectionTest.java
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/collection/NonPkJoinColumnCollectionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/collection/NonPkJoinColumnCollectionTest.java
