HHH-10708 - Accessing a lazy collection in an enhanced class deletes it afterwards

(cherry picked from commit 0e1b79d2b5)

Conflicts:
	hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java

HHH-10708 : Corrections due to backporting

hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java

@@ -16,6 +16,7 @@ import org.hibernate.EntityMode;
 import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.Session;
+import org.hibernate.collection.spi.PersistentCollection;
 import org.hibernate.engine.spi.CachedNaturalIdValueSource;
 import org.hibernate.engine.spi.EntityEntry;
 import org.hibernate.engine.spi.EntityEntryExtraState;
@@ -320,6 +321,15 @@ public abstract class AbstractEntityEntry implements Serializable, EntityEntry {
 		}
 	}
 
+	@Override
+	public void overwriteLoadedStateCollectionValue(String propertyName, PersistentCollection collection) {
+		assert propertyName != null;
+		assert loadedState != null;
+
+		final int propertyIndex = ( (UniqueKeyLoadable) persister ).getPropertyIndex( propertyName );
+		loadedState[propertyIndex] = collection;
+	}
+
 	@Override
 	public boolean requiresDirtyCheck(Object entity) {
 		return isModifiableEntity()

hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java

@@ -43,7 +43,7 @@ public final class Collections {
 	 * @param session The session
 	 */
 	public static void processUnreachableCollection(PersistentCollection coll, SessionImplementor session) {
-		if ( coll.getOwner()==null ) {
+		if ( coll.getOwner() == null ) {
 			processNeverReferencedCollection( coll, session );
 		}
 		else {
@@ -73,7 +73,7 @@ public final class Collections {
 				// the owning entity may have been deleted and its identifier unset due to
 				// identifier-rollback; in which case, try to look up its identifier from
 				// the persistence context
-				if ( session.getFactory().getSettings().isIdentifierRollbackEnabled() ) {
+				if ( session.getFactory().getSessionFactoryOptions().isIdentifierRollbackEnabled() ) {
 					final EntityEntry ownerEntry = persistenceContext.getEntry( coll.getOwner() );
 					if ( ownerEntry != null ) {
 						ownerId = ownerEntry.getId();
@@ -156,40 +156,73 @@ public final class Collections {
 			);
 		}
 
-		// The CollectionEntry.isReached() stuff is just to detect any silly users
-		// who set up circular or shared references between/to collections.
-		if ( ce.isReached() ) {
-			// We've been here before
-			throw new HibernateException(
-					"Found shared references to a collection: " + type.getRole()
-			);
-		}
-		ce.setReached( true );
-
 		final SessionFactoryImplementor factory = session.getFactory();
 		final CollectionPersister persister = factory.getCollectionPersister( type.getRole() );
+
 		ce.setCurrentPersister( persister );
 		//TODO: better to pass the id in as an argument?
 		ce.setCurrentKey( type.getKeyOfOwner( entity, session ) );
 
-		if ( LOG.isDebugEnabled() ) {
+		final boolean isBytecodeEnhanced = persister.getOwnerEntityPersister().getInstrumentationMetadata().isEnhancedForLazyLoading();
-			if ( collection.wasInitialized() ) {
+		if ( isBytecodeEnhanced && !collection.wasInitialized() ) {
-				LOG.debugf(
+			// skip it
-						"Collection found: %s, was: %s (initialized)",
+			LOG.debugf(
-						MessageHelper.collectionInfoString( persister, collection, ce.getCurrentKey(), session ),
+					"Skipping uninitialized bytecode-lazy collection: %s",
-						MessageHelper.collectionInfoString( ce.getLoadedPersister(), collection, ce.getLoadedKey(), session )
+					MessageHelper.collectionInfoString( persister, collection, ce.getCurrentKey(), session )
-				);
+			);
-			}
+			ce.setReached( true );
-			else {
+			ce.setProcessed( true );
-				LOG.debugf(
-						"Collection found: %s, was: %s (uninitialized)",
-						MessageHelper.collectionInfoString( persister, collection, ce.getCurrentKey(), session ),
-						MessageHelper.collectionInfoString( ce.getLoadedPersister(), collection, ce.getLoadedKey(), session )
-				);
-			}
 		}
+		else {
+			// The CollectionEntry.isReached() stuff is just to detect any silly users
+			// who set up circular or shared references between/to collections.
+			if ( ce.isReached() ) {
+				// We've been here beforeQuery
+				throw new HibernateException(
+						"Found shared references to a collection: " + type.getRole()
+				);
+			}
+			ce.setReached( true );
 
-		prepareCollectionForUpdate( collection, ce, factory );
+			if ( LOG.isDebugEnabled() ) {
+				if ( collection.wasInitialized() ) {
+					LOG.debugf(
+							"Collection found: %s, was: %s (initialized)",
+							MessageHelper.collectionInfoString(
+									persister,
+									collection,
+									ce.getCurrentKey(),
+									session
+							),
+							MessageHelper.collectionInfoString(
+									ce.getLoadedPersister(),
+									collection,
+									ce.getLoadedKey(),
+									session
+							)
+					);
+				}
+				else {
+					LOG.debugf(
+							"Collection found: %s, was: %s (uninitialized)",
+							MessageHelper.collectionInfoString(
+									persister,
+									collection,
+									ce.getCurrentKey(),
+									session
+							),
+							MessageHelper.collectionInfoString(
+									ce.getLoadedPersister(),
+									collection,
+									ce.getLoadedKey(),
+									session
+							)
+					);
+				}
+			}
+
+			prepareCollectionForUpdate( collection, ce, factory );
+		}
 	}
 
 	/**

hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java

@@ -169,37 +169,43 @@ public final class CollectionEntry implements Serializable {
 			loadedKey = collection.getKey();
 		}
 
-		boolean nonMutableChange = collection.isDirty() &&
+		boolean nonMutableChange = collection.isDirty()
-				getLoadedPersister()!=null &&
+				&& getLoadedPersister() != null
-				!getLoadedPersister().isMutable();
+				&& !getLoadedPersister().isMutable();
-		if (nonMutableChange) {
+		if ( nonMutableChange ) {
 			throw new HibernateException(
 					"changed an immutable collection instance: " +
 					MessageHelper.collectionInfoString( getLoadedPersister().getRole(), getLoadedKey() )
-				);
+			);
 		}
 
-		dirty(collection);
+		dirty( collection );
 
 		if ( LOG.isDebugEnabled() && collection.isDirty() && getLoadedPersister() != null ) {
-			LOG.debugf( "Collection dirty: %s",
+			LOG.debugf(
-					MessageHelper.collectionInfoString( getLoadedPersister().getRole(), getLoadedKey() ) );
+					"Collection dirty: %s",
+					MessageHelper.collectionInfoString( getLoadedPersister().getRole(), getLoadedKey() )
+			);
 		}
 
-		setDoupdate(false);
+		setReached( false );
-		setDoremove(false);
+		setProcessed( false );
-		setDorecreate(false);
+
-		setReached(false);
+		setDoupdate( false );
-		setProcessed(false);
+		setDoremove( false );
+		setDorecreate( false );
 	}
 
 	public void postInitialize(PersistentCollection collection) throws HibernateException {
-		snapshot = getLoadedPersister().isMutable() ?
+		snapshot = getLoadedPersister().isMutable()
-				collection.getSnapshot( getLoadedPersister() ) :
+				? collection.getSnapshot( getLoadedPersister() )
-				null;
+				: null;
 		collection.setSnapshot(loadedKey, role, snapshot);
-		if (getLoadedPersister().getBatchSize() > 1) {
+		if ( getLoadedPersister().getBatchSize() > 1 ) {
-			((AbstractPersistentCollection) collection).getSession().getPersistenceContext().getBatchFetchQueue().removeBatchLoadableCollection(this); 
+			( (AbstractPersistentCollection) collection ).getSession()
+					.getPersistenceContext()
+					.getBatchFetchQueue()
+					.removeBatchLoadableCollection( this );
 		}
 	}
 
@@ -273,7 +279,7 @@ public final class CollectionEntry implements Serializable {
 	}
 
 	void afterDeserialize(SessionFactoryImplementor factory) {
-		loadedPersister = ( factory == null ? null : factory.getCollectionPersister(role) );
+		loadedPersister = ( factory == null ? null : factory.getCollectionPersister( role ) );
 	}
 
 	public boolean wasDereferenced() {

hibernate-core/src/main/java/org/hibernate/engine/spi/EntityEntry.java

@@ -12,6 +12,7 @@ import java.io.Serializable;
 import java.util.Set;
 
 import org.hibernate.LockMode;
+import org.hibernate.collection.spi.PersistentCollection;
 import org.hibernate.persister.entity.EntityPersister;
 
 /**
@@ -38,6 +39,10 @@ public interface EntityEntry {
 
 	Object[] getLoadedState();
 
+	Object getLoadedValue(String propertyName);
+
+	void overwriteLoadedStateCollectionValue(String propertyName, PersistentCollection collection);
+
 	Object[] getDeletedState();
 
 	void setDeletedState(Object[] deletedState);
@@ -87,8 +92,6 @@ public interface EntityEntry {
 
 	boolean isNullifiable(boolean earlyInsert, SessionImplementor session);
 
-	Object getLoadedValue(String propertyName);
-
 	/**
 	 * Not sure this is the best method name, but the general idea here is to return {@code true} if the entity can
 	 * possibly be dirty.  This can only be the case if it is in a modifiable state (not read-only/deleted) and it

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -941,6 +941,17 @@ public abstract class AbstractEntityPersister
 					session.getPersistenceContext().addCollectionHolder( collection );
 				}
 
+				// update the "state" of the entity's EntityEntry to over-write UNFETCHED_PROPERTY reference
+				// for the collection to the just loaded collection
+				final EntityEntry ownerEntry = session.getPersistenceContext().getEntry( entity );
+				if ( ownerEntry == null ) {
+					// not good
+					throw new AssertionFailure(
+							"Could not locate EntityEntry for the collection owner in the PersistenceContext"
+					);
+				}
+				ownerEntry.overwriteLoadedStateCollectionValue( fieldName, collection );
+
 				// EARLY EXIT!!!
 				return collection;
 			}

hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/HHH_10708/UnexpectedDeleteOneTestTask.java

@@ -63,7 +63,7 @@ public class UnexpectedDeleteOneTestTask extends AbstractEnhancerTestTask {
 		Foo foo = s.get( Foo.class, fooId );
 
 		// accessing the collection results in an exception
-		foo.bar.size();
+		foo.bars.size();
 
 		s.flush();
 		s.getTransaction().commit();
@@ -88,8 +88,9 @@ public class UnexpectedDeleteOneTestTask extends AbstractEnhancerTestTask {
 		@Id	@GeneratedValue
 		int id;
 
-		@OneToMany(orphanRemoval = true, mappedBy = Bar.FOO, targetEntity = Bar.class) @Cascade(CascadeType.ALL)
+		@OneToMany(orphanRemoval = true, mappedBy = Bar.FOO, targetEntity = Bar.class)
-		Set<Bar> bar = new HashSet<>();
+		@Cascade(CascadeType.ALL)
+		Set<Bar> bars = new HashSet<>();
 	}
 
 }