From 0f918b4d42745f1b075063a93c702aa0d3f3b4f9 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 20 May 2016 08:49:21 -0500 Subject: [PATCH] HHH-10708 - Accessing a lazy collection in an enhanced class deletes it afterwards (cherry picked from commit 0e1b79d2b52d7281bf977c3c0ec691f6822011eb) Conflicts: hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java HHH-10708 : Corrections due to backporting --- .../engine/internal/AbstractEntityEntry.java | 10 +++ .../engine/internal/Collections.java | 89 +++++++++++++------ .../hibernate/engine/spi/CollectionEntry.java | 44 +++++---- .../org/hibernate/engine/spi/EntityEntry.java | 7 +- .../entity/AbstractEntityPersister.java | 11 +++ .../UnexpectedDeleteOneTestTask.java | 7 +- 6 files changed, 116 insertions(+), 52 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java index 67e7ad4bd7..ed4dd32662 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java +++ b/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() diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java index ed5f08b723..5412a16cd4 100755 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java +++ b/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() ) { - 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 ) - ); - } + final boolean isBytecodeEnhanced = persister.getOwnerEntityPersister().getInstrumentationMetadata().isEnhancedForLazyLoading(); + if ( isBytecodeEnhanced && !collection.wasInitialized() ) { + // skip it + LOG.debugf( + "Skipping uninitialized bytecode-lazy collection: %s", + MessageHelper.collectionInfoString( persister, collection, ce.getCurrentKey(), session ) + ); + ce.setReached( true ); + ce.setProcessed( true ); } + 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 ); + } } /** 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 index f5cf459d3e..ff1726da8e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java +++ b/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() && - getLoadedPersister()!=null && - !getLoadedPersister().isMutable(); - if (nonMutableChange) { + boolean nonMutableChange = collection.isDirty() + && getLoadedPersister() != null + && !getLoadedPersister().isMutable(); + 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", - MessageHelper.collectionInfoString( getLoadedPersister().getRole(), getLoadedKey() ) ); + LOG.debugf( + "Collection dirty: %s", + MessageHelper.collectionInfoString( getLoadedPersister().getRole(), getLoadedKey() ) + ); } - setDoupdate(false); - setDoremove(false); - setDorecreate(false); - setReached(false); - setProcessed(false); + setReached( false ); + setProcessed( false ); + + setDoupdate( false ); + setDoremove( false ); + setDorecreate( false ); } public void postInitialize(PersistentCollection collection) throws HibernateException { - snapshot = getLoadedPersister().isMutable() ? - collection.getSnapshot( getLoadedPersister() ) : - null; + snapshot = getLoadedPersister().isMutable() + ? collection.getSnapshot( getLoadedPersister() ) + : null; collection.setSnapshot(loadedKey, role, snapshot); - if (getLoadedPersister().getBatchSize() > 1) { - ((AbstractPersistentCollection) collection).getSession().getPersistenceContext().getBatchFetchQueue().removeBatchLoadableCollection(this); + if ( getLoadedPersister().getBatchSize() > 1 ) { + ( (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() { diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityEntry.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityEntry.java index fadbfc1bf3..4f04d7eb11 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityEntry.java +++ b/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 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 index 2c3ea6786a..0c4a9908e4 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/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; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/HHH_10708/UnexpectedDeleteOneTestTask.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/HHH_10708/UnexpectedDeleteOneTestTask.java index b0c683f100..bbdab72880 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/HHH_10708/UnexpectedDeleteOneTestTask.java +++ b/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) - Set bar = new HashSet<>(); + @OneToMany(orphanRemoval = true, mappedBy = Bar.FOO, targetEntity = Bar.class) + @Cascade(CascadeType.ALL) + Set bars = new HashSet<>(); } } \ No newline at end of file