From 09fa8ef76a5911f307e23b0320eecc21f35b5227 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 31 Aug 2024 19:07:43 +0200 Subject: [PATCH] HHH-18553 flush/evict when there is a managed instance while deleting the detached instance Signed-off-by: Gavin King --- .../src/main/java/org/hibernate/Session.java | 10 +- .../internal/DefaultDeleteEventListener.java | 115 +++++++++++------- .../orm/test/readonly/ReadOnlyProxyTest.java | 1 - 3 files changed, 75 insertions(+), 51 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/Session.java b/hibernate-core/src/main/java/org/hibernate/Session.java index 4906a39495..b947acbbc1 100644 --- a/hibernate-core/src/main/java/org/hibernate/Session.java +++ b/hibernate-core/src/main/java/org/hibernate/Session.java @@ -597,7 +597,7 @@ public interface Session extends SharedSessionContract, EntityManager { * The modes {@link LockMode#WRITE} and {@link LockMode#UPGRADE_SKIPLOCKED} * are not legal arguments to {@code lock()}. * - * @param object a persistent instance + * @param object a persistent instance associated with this session * @param lockMode the lock level */ void lock(Object object, LockMode lockMode); @@ -609,7 +609,7 @@ public interface Session extends SharedSessionContract, EntityManager { * This operation cascades to associated instances if the association is * mapped with {@link org.hibernate.annotations.CascadeType#LOCK}. * - * @param object a persistent instance + * @param object a persistent instance associated with this session * @param lockOptions the lock options * * @since 6.2 @@ -664,8 +664,12 @@ public interface Session extends SharedSessionContract, EntityManager { * Mark a persistence instance associated with this session for removal from * the underlying database. Ths operation cascades to associated instances if * the association is mapped {@link jakarta.persistence.CascadeType#REMOVE}. + *

+ * Except when operating in fully JPA-compliant mode, this operation does, + * contrary to the JPA specification, accept a detached entity instance. * - * @param object the managed persistent instance to remove + * @param object the managed persistent instance to remove, or a detached + * instance unless operating in fully JPA-compliant mode */ @Override void remove(Object object); 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 8d4db1683a..2eef49b420 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 @@ -9,6 +9,7 @@ package org.hibernate.event.internal; import org.hibernate.CacheMode; import org.hibernate.HibernateException; import org.hibernate.LockMode; +import org.hibernate.NonUniqueObjectException; import org.hibernate.TransientObjectException; import org.hibernate.action.internal.CollectionRemoveAction; import org.hibernate.action.internal.EntityDeleteAction; @@ -44,7 +45,6 @@ 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.ComponentType; @@ -52,6 +52,7 @@ import org.hibernate.type.Type; import org.hibernate.type.TypeHelper; import static org.hibernate.engine.internal.Collections.skipRemoval; +import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer; /** * Defines the default delete event listener used by hibernate for deleting entities @@ -102,7 +103,7 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback private boolean optimizeUnloadedDelete(DeleteEvent event) { final Object object = event.getObject(); - final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( object ); + final LazyInitializer lazyInitializer = extractLazyInitializer( object ); if ( lazyInitializer != null ) { if ( lazyInitializer.isUninitialized() ) { final EventSource source = event.getSession(); @@ -157,54 +158,76 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback final Object entity = persistenceContext.unproxyAndReassociate( event.getObject() ); final EntityEntry entityEntry = persistenceContext.getEntry( entity ); if ( entityEntry == null ) { - deleteTransientInstance( event, transientEntities, entity ); + deleteUnmanagedInstance( event, transientEntities, entity ); } else { deletePersistentInstance( event, transientEntities, entity, entityEntry ); } } - private void deleteTransientInstance(DeleteEvent event, DeleteContext transientEntities, Object entity) { - LOG.trace( "Entity was not persistent in delete processing" ); - + private void deleteUnmanagedInstance(DeleteEvent event, DeleteContext transientEntities, Object entity) { + LOG.trace( "Deleted entity was not associated with current session" ); final EventSource source = event.getSession(); - final EntityPersister persister = source.getEntityPersister( event.getEntityName(), entity ); if ( ForeignKeys.isTransient( persister.getEntityName(), entity, null, source ) ) { deleteTransientEntity( source, entity, persister, transientEntities ); } else { performDetachedEntityDeletionCheck( event ); + deleteDetachedEntity( event, transientEntities, entity, persister, source ); + } + } - final Object id = persister.getIdentifier( entity, source ); - if ( id == null ) { - throw new TransientObjectException( "Cannot delete instance of entity '" + persister.getEntityName() - + "' because it has a null identifier" ); + private void deleteDetachedEntity(DeleteEvent event, DeleteContext transientEntities, Object entity, EntityPersister persister, EventSource source) { + final Object id = persister.getIdentifier( entity, source ); + if ( id == null ) { + throw new TransientObjectException( "Cannot delete instance of entity '" + + persister.getEntityName() + "' because it has a null identifier" ); + } + + final PersistenceContext persistenceContext = source.getPersistenceContextInternal(); + final EntityKey key = source.generateEntityKey( id, persister); + final Object version = persister.getVersion(entity); + +// persistenceContext.checkUniqueness( key, entity ); + flushAndEvictExistingEntity( key, version, persister, source ); + + new OnUpdateVisitor( source, id, entity ).process( entity, persister ); + + final EntityEntry entityEntry = persistenceContext.addEntity( + entity, + persister.isMutable() ? Status.MANAGED : Status.READ_ONLY, + persister.getValues(entity), + key, + version, + LockMode.NONE, + true, + persister, + false + ); + persister.afterReassociate(entity, source); + + delete( event, transientEntities, source, entity, persister, id, version, entityEntry ); + } + + /** + * Since Hibernate 7, if a detached instance is passed to remove(), + * and if there is already an existing managed entity with the same + * id, flush and evict it, after checking that the versions match. + */ + private static void flushAndEvictExistingEntity( + EntityKey key, Object version, EntityPersister persister, EventSource source) { + final Object existingEntity = source.getPersistenceContextInternal().getEntity( key ); + if ( existingEntity != null ) { + source.flush(); + if ( !persister.isVersioned() + || persister.getVersionType() + .isEqual( version, persister.getVersion( existingEntity ) ) ) { + source.evict( existingEntity ); + } + else { + throw new NonUniqueObjectException( key.getIdentifier(), key.getEntityName() ); } - - final PersistenceContext persistenceContext = source.getPersistenceContextInternal(); - final EntityKey key = source.generateEntityKey( id, persister ); - - persistenceContext.checkUniqueness( key, entity); - - new OnUpdateVisitor( source, id, entity ).process( entity, persister ); - - final Object version = persister.getVersion( entity ); - - final EntityEntry entityEntry = persistenceContext.addEntity( - entity, - persister.isMutable() ? Status.MANAGED : Status.READ_ONLY, - persister.getValues( entity ), - key, - version, - LockMode.NONE, - true, - persister, - false - ); - persister.afterReassociate( entity, source ); - - delete( event, transientEntities, source, entity, persister, id, version, entityEntry ); } } @@ -287,7 +310,7 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback } private boolean hasRegisteredRemoveCallbacks(EntityPersister persister) { - Class mappedClass = persister.getMappedClass(); + final Class mappedClass = persister.getMappedClass(); return callbackRegistry.hasRegisteredCallbacks( mappedClass, CallbackType.PRE_REMOVE ) || callbackRegistry.hasRegisteredCallbacks( mappedClass, CallbackType.POST_REMOVE ); } @@ -295,25 +318,23 @@ public class DefaultDeleteEventListener implements DeleteEventListener, Callback /** * Called when we have recognized an attempt to delete a detached entity. *

- * This is perfectly valid in Hibernate usage; JPA, however, forbids this. - * Thus, this is a hook for HEM to affect this behavior. - * - * @param event The event. + * This is perfectly legal in regular Hibernate usage; the JPA spec, + * however, forbids it. */ protected void performDetachedEntityDeletionCheck(DeleteEvent event) { if ( jpaBootstrap ) { disallowDeletionOfDetached( event ); } - // ok in normal Hibernate usage to delete a detached entity; JPA however - // forbids it, thus this is a hook for HEM to affect this behavior } private void disallowDeletionOfDetached(DeleteEvent event) { - EventSource source = event.getSession(); - String entityName = event.getEntityName(); - EntityPersister persister = source.getEntityPersister( entityName, event.getObject() ); - Object id = persister.getIdentifier( event.getObject(), source ); - entityName = entityName == null ? source.guessEntityName( event.getObject() ) : entityName; + final EventSource source = event.getSession(); + final String explicitEntityName = event.getEntityName(); + final EntityPersister persister = source.getEntityPersister( explicitEntityName, event.getObject() ); + final Object id = persister.getIdentifier( event.getObject(), source ); + final String entityName = explicitEntityName == null + ? source.guessEntityName( event.getObject() ) + : explicitEntityName; throw new IllegalArgumentException( "Given entity is not associated with the persistence context [" + entityName + "#" + id + "]" ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/readonly/ReadOnlyProxyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/readonly/ReadOnlyProxyTest.java index 22a00b0a05..40bda37415 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/readonly/ReadOnlyProxyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/readonly/ReadOnlyProxyTest.java @@ -12,7 +12,6 @@ import org.hibernate.CacheMode; import org.hibernate.Hibernate; import org.hibernate.Session; import org.hibernate.Transaction; -import org.hibernate.TransientObjectException; import org.hibernate.UnresolvableObjectException; import org.hibernate.cfg.AvailableSettings; import org.hibernate.engine.spi.SessionImplementor;