HHH-15509 enable unloaded-proxy delete for entities with owned collections

Author: gavinking

hibernate-core/src/main/java/org/hibernate/action/internal/CollectionRemoveAction.java

@@ -29,7 +29,7 @@ public final class CollectionRemoveAction extends CollectionAction {
 	 *
 	 * Use this constructor when the collection is non-null.
 	 *
-	 * @param collection The collection to to remove; must be non-null
+	 * @param collection The collection to remove; must be non-null
 	 * @param persister  The collection's persister
 	 * @param id The collection key
 	 * @param emptySnapshot Indicates if the snapshot is empty
@@ -81,18 +81,35 @@ public CollectionRemoveAction(
 		this.affectedOwner = affectedOwner;
 	}
 
+	/**
+	 * Removes a persistent collection for an unloaded proxy.
+	 *
+	 * Use this constructor when the owning entity is has not been loaded.
+	 * @param persister The collection's persister
+	 * @param id The collection key
+	 * @param session The session
+	 */
+	public CollectionRemoveAction(
+			final CollectionPersister persister,
+			final Object id,
+			final SharedSessionContractImplementor session) {
+		super( persister, null, id, session );
+		emptySnapshot = false;
+		affectedOwner = null;
+	}
+
 	@Override
 	public void execute() throws HibernateException {
 		preRemove();
 
 		final SharedSessionContractImplementor session = getSession();
 
 		if ( !emptySnapshot ) {
-			// an existing collection that was either non-empty or uninitialized
+			// an existing collection that was either nonempty or uninitialized
 			// is replaced by null or a different collection
-			// (if the collection is uninitialized, hibernate has no way of
+			// (if the collection is uninitialized, Hibernate has no way of
 			// knowing if the collection is actually empty without querying the db)
-			getPersister().remove( getKey(), session);
+			getPersister().remove( getKey(), session );
 		}
 
 		final PersistentCollection<?> collection = getCollection();

hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java

@@ -39,7 +39,8 @@ public class EntityDeleteAction extends EntityAction {
 
 	/**
 	 * Constructs an EntityDeleteAction.
-	 *  @param id The entity identifier
+	 *
+	 * @param id The entity identifier
 	 * @param state The current (extracted) entity state
 	 * @param version The current entity version
 	 * @param instance The entity instance
@@ -61,7 +62,6 @@ public EntityDeleteAction(
 		this.state = state;
 
 		NaturalIdMapping naturalIdMapping = persister.getNaturalIdMapping();
-
 		if ( naturalIdMapping != null ) {
 			naturalIdValues = session.getPersistenceContextInternal().getNaturalIdResolutions()
 					.removeLocalResolution(
@@ -72,6 +72,23 @@ public EntityDeleteAction(
 		}
 	}
 
+	/**
+	 * Constructs an EntityDeleteAction for an unloaded proxy.
+	 *
+	 * @param id The entity identifier
+	 * @param persister The entity persister
+	 * @param session The session
+	 */
+	public EntityDeleteAction(
+			final Object id,
+			final EntityPersister persister,
+			final SessionImplementor session) {
+		super( session, id, null, persister );
+		this.version = null;
+		this.isCascadeDeleteEnabled = false;
+		this.state = null;
+	}
+
 	public Object getVersion() {
 		return version;
 	}

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java

@@ -11,6 +11,7 @@
 import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.TransientObjectException;
+import org.hibernate.action.internal.CollectionRemoveAction;
 import org.hibernate.action.internal.EntityDeleteAction;
 import org.hibernate.action.internal.OrphanRemovalAction;
 import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer;
@@ -21,10 +22,12 @@
 import org.hibernate.engine.internal.ForeignKeys;
 import org.hibernate.engine.internal.Nullability;
 import org.hibernate.engine.internal.Nullability.NullabilityCheckType;
+import org.hibernate.engine.spi.ActionQueue;
 import org.hibernate.engine.spi.CascadingActions;
 import org.hibernate.engine.spi.EntityEntry;
 import org.hibernate.engine.spi.EntityKey;
 import org.hibernate.engine.spi.PersistenceContext;
+import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.engine.spi.Status;
 import org.hibernate.event.service.spi.JpaBootstrapSensitive;
 import org.hibernate.event.spi.DeleteContext;
@@ -37,13 +40,15 @@
 import org.hibernate.jpa.event.spi.CallbackRegistry;
 import org.hibernate.jpa.event.spi.CallbackRegistryConsumer;
 import org.hibernate.jpa.event.spi.CallbackType;
+import org.hibernate.metamodel.spi.MappingMetamodelImplementor;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.pretty.MessageHelper;
 import org.hibernate.property.access.internal.PropertyAccessStrategyBackRefImpl;
 import org.hibernate.proxy.HibernateProxy;
 import org.hibernate.proxy.LazyInitializer;
 import org.hibernate.type.CollectionType;
+import org.hibernate.type.CompositeType;
 import org.hibernate.type.Type;
 import org.hibernate.type.TypeHelper;
 
@@ -110,17 +115,15 @@ && canBeDeletedWithoutLoading( source, persister ) ) {
 					persistenceContext.reassociateProxy( object, id );
 					if ( !persistenceContext.containsDeletedUnloadedEntityKey( key ) ) {
 						persistenceContext.registerDeletedUnloadedEntityKey( key );
-						source.getActionQueue().addAction(
-								new EntityDeleteAction(
-										id,
-										null,
-										null,
-										null,
-										persister,
-										false,
-										source
-								)
-						);
+
+						if ( persister.hasOwnedCollections() ) {
+							// we're deleting an unloaded proxy with collections
+							for ( Type type : persister.getPropertyTypes() ) { //TODO: when we enable this for subclasses use getSubclassPropertyTypeClosure()
+								deleteOwnedCollections( type, id, source, source.getActionQueue() );
+							}
+						}
+
+						source.getActionQueue().addAction( new EntityDeleteAction( id, persister, source ) );
 					}
 					return true;
 				}
@@ -129,6 +132,23 @@ && canBeDeletedWithoutLoading( source, persister ) ) {
 		return false;
 	}
 
+	private void deleteOwnedCollections(Type type, Object key, SharedSessionContractImplementor session, ActionQueue actionQueue) {
+		MappingMetamodelImplementor mappingMetamodel = session.getFactory().getMappingMetamodel();
+		if ( type.isCollectionType() ) {
+			String role = ( (CollectionType) type ).getRole();
+			CollectionPersister persister = mappingMetamodel.getCollectionDescriptor(role);
+			if ( !persister.isInverse() ) {
+				actionQueue.addAction( new CollectionRemoveAction( persister, key, session ) );
+			}
+		}
+		else if ( type.isComponentType() ) {
+			Type[] subtypes = ( (CompositeType) type ).getSubtypes();
+			for ( Type subtype : subtypes ) {
+				deleteOwnedCollections( subtype, key, session, actionQueue );
+			}
+		}
+	}
+
 	private void delete(DeleteEvent event, DeleteContext transientEntities) {
 		final PersistenceContext persistenceContext = event.getSession().getPersistenceContextInternal();
 		final Object entity = persistenceContext.unproxyAndReassociate( event.getObject() );
@@ -252,9 +272,8 @@ private void delete(
 	private boolean canBeDeletedWithoutLoading(EventSource source, EntityPersister persister) {
 		return source.getInterceptor() == EmptyInterceptor.INSTANCE
 			&& !persister.implementsLifecycle()
-			&& !persister.hasSubclasses()
+			&& !persister.hasSubclasses() //TODO: should be unnecessary, using EntityPersister.getSubclassPropertyTypeClosure(), etc
 			&& !persister.hasCascadeDelete()
-			&& !persister.hasOwnedCollections()
 			&& !persister.hasNaturalIdentifier()
 			&& !hasRegisteredRemoveCallbacks( persister )
 			&& !hasCustomEventListeners( source );

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

@@ -3820,7 +3820,7 @@ public void update(
 		if ( entry == null && !isMutable() ) {
 			throw new IllegalStateException( "Updating immutable entity that is not in session yet" );
 		}
-		if ( ( entityMetamodel.isDynamicUpdate() && dirtyFields != null ) ) {
+		if ( entityMetamodel.isDynamicUpdate() && dirtyFields != null ) {
 			// We need to generate the UPDATE SQL when dynamic-update="true"
 			propsToUpdate = getPropertiesToUpdate( dirtyFields, hasDirtyCollection );
 			// don't need to check laziness (dirty checking algorithm handles that)
@@ -4014,41 +4014,42 @@ protected void preInsertInMemoryValueGeneration(Object[] fields, Object object,
 	@Override
 	public void delete(Object id, Object version, Object object, SharedSessionContractImplementor session)
 			throws HibernateException {
-		final int span = getTableSpan();
 		boolean isImpliedOptimisticLocking = !entityMetamodel.isVersioned() && isAllOrDirtyOptLocking()
 				&& object != null; // null object signals that we're deleting an unloaded proxy
+
 		Object[] loadedState = null;
 		if ( isImpliedOptimisticLocking ) {
 			// need to treat this as if it where optimistic-lock="all" (dirty does *not* make sense);
 			// first we need to locate the "loaded" state
 			//
 			// Note, it potentially could be a proxy, so doAfterTransactionCompletion the location the safe way...
-			final EntityKey key = session.generateEntityKey( id, this );
 			final PersistenceContext persistenceContext = session.getPersistenceContextInternal();
-			Object entity = persistenceContext.getEntity( key );
+			Object entity = persistenceContext.getEntity( session.generateEntityKey( id, this ) );
 			if ( entity != null ) {
 				EntityEntry entry = persistenceContext.getEntry( entity );
 				loadedState = entry.getLoadedState();
 			}
 		}
 
-		final String[] deleteStrings;
+		final String[] deleteStrings = getSQLDeleteStrings( object, isImpliedOptimisticLocking, loadedState );
+		for ( int j = getTableSpan() - 1; j >= 0; j-- ) {
+			delete( id, version, j, object, deleteStrings[j], session, loadedState );
+		}
+	}
+
+	private String[] getSQLDeleteStrings(Object object, boolean isImpliedOptimisticLocking, Object[] loadedState) {
 		if ( isImpliedOptimisticLocking && loadedState != null ) {
 			// we need to utilize dynamic delete statements
-			deleteStrings = generateSQLDeleteStrings( loadedState );
+			return generateSQLDeleteStrings(loadedState);
 		}
-		else if (object!=null) {
+		else if ( object != null ) {
 			// otherwise, utilize the static delete statements
-			deleteStrings = getSQLDeleteStrings();
+			return getSQLDeleteStrings();
 		}
 		else {
-			deleteStrings = getSQLDeleteNoVersionCheckStrings();
+			// deleting an unloaded proxy
+			return getSQLDeleteNoVersionCheckStrings();
 		}
-
-		for ( int j = span - 1; j >= 0; j-- ) {
-			delete( id, version, j, object, deleteStrings[j], session, loadedState );
-		}
-
 	}
 
 	protected boolean isAllOrDirtyOptLocking() {
@@ -4069,7 +4070,7 @@ protected String[] generateSQLDeleteStrings(Object[] loadedState) {
 			Type[] types = getPropertyTypes();
 			for ( int i = 0; i < entityMetamodel.getPropertySpan(); i++ ) {
 				if ( isPropertyOfTable( i, j ) && versionability[i] ) {
-					// this property belongs to the table and it is not specifically
+					// This property belongs to the table, and it's not explicitly
 					// excluded from optimistic locking by optimistic-lock="false"
 					String[] propertyColumnNames = getPropertyColumnNames( i );
 					boolean[] propertyNullness = types[i].toColumnNullness( loadedState[i], getFactory() );

hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java

@@ -26,6 +26,8 @@ public void testAttached(SessionFactoryScope scope) {
             Transaction tx = em.beginTransaction();
             c.setParent(p);
             p.getChildren().add(c);
+            p.getWords().add("hello");
+            p.getWords().add("world");
             em.persist(p);
             tx.commit();
         } );
@@ -54,6 +56,8 @@ public void testDetached(SessionFactoryScope scope) {
             Transaction tx = em.beginTransaction();
             c.setParent(p);
             p.getChildren().add(c);
+            p.getWords().add("hello");
+            p.getWords().add("world");
             em.persist(p);
             tx.commit();
         } );

hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/Parent.java

@@ -1,13 +1,16 @@
 package org.hibernate.orm.test.deleteunloaded;
 
 import jakarta.persistence.CascadeType;
+import jakarta.persistence.ElementCollection;
 import jakarta.persistence.Entity;
 import jakarta.persistence.GeneratedValue;
 import jakarta.persistence.Id;
 import jakarta.persistence.OneToMany;
 import jakarta.persistence.Version;
 
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 @Entity
@@ -21,10 +24,17 @@ public class Parent {
     @OneToMany(mappedBy = "parent", cascade = CascadeType.PERSIST)
     private Set<Child> children = new HashSet<>();
 
+    @ElementCollection
+    private List<String> words = new ArrayList<>();
+
     public Set<Child> getChildren() {
         return children;
     }
 
+    public List<String> getWords() {
+        return words;
+    }
+
     public long getId() {
         return id;
     }