HHH-18553 flush/evict when there is a managed instance while deleting the detached instance

Signed-off-by: Gavin King <[email protected]>

Author: gavinking

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}.
+	 * <p>
+	 * 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);

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

@@ -9,6 +9,7 @@
 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,14 +45,14 @@
 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;
 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 void onDelete(DeleteEvent event, DeleteContext transientEntities) throws
 
 	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 @@ private void delete(DeleteEvent event, DeleteContext transientEntities) {
 		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 PersistenceContext persistenceContext = source.getPersistenceContextInternal();
+		final EntityKey key = source.generateEntityKey( id, persister);
+		final Object version = persister.getVersion(entity);
 
-			persistenceContext.checkUniqueness( key, entity);
+//		persistenceContext.checkUniqueness( key, entity );
+		flushAndEvictExistingEntity( key, version, persister, source );
 
-			new OnUpdateVisitor( source, id, entity ).process( entity, persister );
+		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);
 
-			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 );
+	}
 
-			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() );
+			}
 		}
 	}
 
@@ -287,33 +310,31 @@ private static boolean hasCustomEventListeners(EventSource source) {
 	}
 
 	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 );
 	}
 
 	/**
 	 * Called when we have recognized an attempt to delete a detached entity.
 	 * <p>
-	 * 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 + "]" );
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/readonly/ReadOnlyProxyTest.java

@@ -12,7 +12,6 @@
 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;