From 49a2b20d76f7795b4b27e1508effb2783cd0e94d Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 26 Sep 2022 16:11:28 +0200 Subject: [PATCH] HHH-15509 enable unloaded-proxy delete for entities with owned collections --- .../internal/CollectionRemoveAction.java | 25 +++++++++-- .../action/internal/EntityDeleteAction.java | 21 ++++++++- .../internal/DefaultDeleteEventListener.java | 45 +++++++++++++------ .../entity/AbstractEntityPersister.java | 39 ++++++++-------- .../DeleteUnloadedProxyTest.java | 4 ++ .../orm/test/deleteunloaded/Parent.java | 10 +++++ 6 files changed, 106 insertions(+), 38 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionRemoveAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionRemoveAction.java index a5a76dbfa6..bde8e19e89 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionRemoveAction.java +++ b/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,6 +81,23 @@ public final class CollectionRemoveAction extends CollectionAction { 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(); @@ -88,11 +105,11 @@ public final class CollectionRemoveAction extends CollectionAction { 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(); diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java index c853232aba..20c3938144 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java +++ b/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 class EntityDeleteAction extends EntityAction { this.state = state; NaturalIdMapping naturalIdMapping = persister.getNaturalIdMapping(); - if ( naturalIdMapping != null ) { naturalIdValues = session.getPersistenceContextInternal().getNaturalIdResolutions() .removeLocalResolution( @@ -72,6 +72,23 @@ public class EntityDeleteAction extends EntityAction { } } + /** + * 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; } diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java index 5c940c5ffb..3be1930a17 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java @@ -11,6 +11,7 @@ import org.hibernate.EmptyInterceptor; 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.CascadePoint; 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,6 +40,7 @@ import org.hibernate.internal.FastSessionServices; 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; @@ -44,6 +48,7 @@ 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 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback 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 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback 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 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback 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 ); 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 088af2a2c5..38d900c8e8 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 @@ -3820,7 +3820,7 @@ public abstract class AbstractEntityPersister 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 @@ public abstract class AbstractEntityPersister @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; - if ( isImpliedOptimisticLocking && loadedState != null ) { - // we need to utilize dynamic delete statements - deleteStrings = generateSQLDeleteStrings( loadedState ); - } - else if (object!=null) { - // otherwise, utilize the static delete statements - deleteStrings = getSQLDeleteStrings(); - } - else { - deleteStrings = getSQLDeleteNoVersionCheckStrings(); - } - - for ( int j = span - 1; j >= 0; j-- ) { + 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 + return generateSQLDeleteStrings(loadedState); + } + else if ( object != null ) { + // otherwise, utilize the static delete statements + return getSQLDeleteStrings(); + } + else { + // deleting an unloaded proxy + return getSQLDeleteNoVersionCheckStrings(); + } } protected boolean isAllOrDirtyOptLocking() { @@ -4069,7 +4070,7 @@ public abstract class AbstractEntityPersister 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() ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java index e6c66fee15..22d6c190d8 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java @@ -26,6 +26,8 @@ public class DeleteUnloadedProxyTest { 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 class DeleteUnloadedProxyTest { Transaction tx = em.beginTransaction(); c.setParent(p); p.getChildren().add(c); + p.getWords().add("hello"); + p.getWords().add("world"); em.persist(p); tx.commit(); } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/Parent.java b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/Parent.java index 5801a2ca42..af89bc94d0 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/Parent.java +++ b/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 children = new HashSet<>(); + @ElementCollection + private List words = new ArrayList<>(); + public Set getChildren() { return children; } + public List getWords() { + return words; + } + public long getId() { return id; }