HHH-18212 Fix transient check for entities deleted during the same flush

Author: mbellade

hibernate-core/src/main/java/org/hibernate/engine/spi/CascadingActions.java

@@ -382,28 +382,30 @@ public void cascade(
 					// a proxy is always non-transient
 					// and ForeignKeys.isTransient()
 					// is not written to expect a proxy
-					&& !isHibernateProxy( child )
-					// if it's associated with the session
-					// we are good, even if it's not yet
-					// inserted, since ordering problems
-					// are detected and handled elsewhere
-					&& !isInManagedState( child, session )
-					// TODO: check if it is a merged entity which has not yet been flushed
-					// Currently this throws if you directly reference a new transient
-					// instance after a call to merge() that results in its managed copy
-					// being scheduled for insertion, if the insert has not yet occurred.
-					// This is not terrible: it's more correct to "swap" the reference to
-					// point to the managed instance, but it's probably too heavy-handed.
-					&& isTransient( entityName, child, null, session ) ) {
-				throw new TransientObjectException( "persistent instance references an unsaved transient instance of '"
-						+ entityName + "' (save the transient instance before flushing)" );
-				//TODO: should be TransientPropertyValueException
+					&& !isHibernateProxy( child ) ) {
+				// if it's associated with the session
+				// we are good, even if it's not yet
+				// inserted, since ordering problems
+				// are detected and handled elsewhere
+				final EntityEntry entry = session.getPersistenceContextInternal().getEntry( child );
+				if ( !isInManagedState( entry )
+						// TODO: check if it is a merged entity which has not yet been flushed
+						// Currently this throws if you directly reference a new transient
+						// instance after a call to merge() that results in its managed copy
+						// being scheduled for insertion, if the insert has not yet occurred.
+						// This is not terrible: it's more correct to "swap" the reference to
+						// point to the managed instance, but it's probably too heavy-handed.
+						&& ( entry != null && entry.getStatus() == Status.DELETED || isTransient( entityName, child, null, session ) ) ) {
+					throw new TransientObjectException( "persistent instance references an unsaved transient instance of '" +
+									entityName + "' (save the transient instance before flushing)" );
+					//TODO: should be TransientPropertyValueException
 //				throw new TransientPropertyValueException(
 //						"object references an unsaved transient instance - save the transient instance before flushing",
 //						entityName,
 //						persister.getEntityName(),
 //						persister.getPropertyNames()[propertyIndex]
 //				);
+				}
 			}
 		}
 
@@ -441,8 +443,7 @@ public String toString() {
 		}
 	};
 
-	private static boolean isInManagedState(Object child, EventSource session) {
-		final EntityEntry entry = session.getPersistenceContextInternal().getEntry( child );
+	private static boolean isInManagedState(EntityEntry entry) {
 		if ( entry == null ) {
 			return false;
 		}

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

@@ -160,9 +160,19 @@ private void prepareEntityFlushes(EventSource session, PersistenceContext persis
 		// processed, so that all entities which will be persisted are
 		// persistent when we do the check (I wonder if we could move this
 		// into Nullability, instead of abusing the Cascade infrastructure)
-		persistenceContext.getEntitiesByKey().forEach( (entry, entity) -> {
-			Cascade.cascade( CascadingActions.CHECK_ON_FLUSH, CascadePoint.BEFORE_FLUSH, session, entry.getPersister(), entity, null );
-		} );
+		for ( Map.Entry<Object, EntityEntry> me : persistenceContext.reentrantSafeEntityEntries() ) {
+			final EntityEntry entry = me.getValue();
+			if ( flushable( entry ) ) {
+				Cascade.cascade(
+						CascadingActions.CHECK_ON_FLUSH,
+						CascadePoint.BEFORE_FLUSH,
+						session,
+						entry.getPersister(),
+						me.getKey(),
+						null
+				);
+			}
+		}
 	}
 
 	private static boolean flushable(EntityEntry entry) {

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/embedded/EmbeddableWithManyToOneCircularityTest.java

@@ -59,17 +59,15 @@ public void setUp(SessionFactoryScope scope) {
 	public void tearDown(SessionFactoryScope scope) {
 		scope.inTransaction(
 				session -> {
-					session.createQuery( "from EntityTest", EntityTest.class ).list().forEach(
-							entityTest -> {
-								session.delete( entityTest );
-							}
-					);
-
-					session.createQuery( "from EntityTest2", EntityTest2.class ).list().forEach(
-							entityTest -> {
-								session.delete( entityTest );
-							}
-					);
+					session.createQuery( "from EntityTest", EntityTest.class ).getResultList().forEach( entity -> {
+						final EntityTest2 entity2 = entity.getEntity2();
+						if ( entity2 != null && entity2.getEmbeddedAttribute() != null ) {
+							entity2.getEmbeddedAttribute().setEntity( null );
+						}
+						session.remove( entity );
+					} );
+					session.flush();
+					session.createMutationQuery( "delete from EntityTest2" ).executeUpdate();
 				}
 		);
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/embedded/EmbeddableWithManyToOneTest.java

@@ -61,17 +61,14 @@ public void setUp(SessionFactoryScope scope) {
 	public void tearDown(SessionFactoryScope scope) {
 		scope.inTransaction(
 				session -> {
-					session.createQuery( "from EntityTest", EntityTest.class ).list().forEach(
-							entityTest -> {
-								session.delete( entityTest );
-							}
-					);
-
-					session.createQuery( "from EntityTest2", EntityTest2.class ).list().forEach(
-							entityTest -> {
-								session.delete( entityTest );
-							}
-					);
+					session.createQuery( "from EntityTest", EntityTest.class ).getResultList().forEach( entity -> {
+						final EntityTest2 entity2 = entity.getEntity2();
+						if ( entity2 != null && entity2.getEmbeddedAttribute() != null ) {
+							entity2.getEmbeddedAttribute().setEntity( null );
+						}
+						session.remove( entity );
+					} );
+					session.createMutationQuery( "delete from EntityTest2" ).executeUpdate();
 				}
 		);
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/formula/ManyToManyNotIgnoreLazyFetchingTest.java

@@ -80,6 +80,7 @@ protected void afterEntityManagerFactoryBuilt() {
 			entityManager.flush();
 
 			entityManager.remove(code);
+			stock1.getCodes().remove( code );
 		} );
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/manytoone/ManyToOneJoinTest.java

@@ -18,6 +18,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 
 /**
  * @author Emmanuel Bernard
@@ -36,18 +37,9 @@ public class ManyToOneJoinTest {
 	public void teardDown(SessionFactoryScope scope) {
 		scope.inTransaction(
 				session -> {
-					List<ForestType> forestTypes = session.createQuery( "from ForestType" ).list();
-					forestTypes.forEach(
-							forestType -> {
-								forestType.getTrees().forEach(
-										tree ->{
-											session.delete( tree.getForestType() );
-											session.delete( tree );
-										}
-								);
-								session.delete( forestType );
-							}
-					);
+					session.createMutationQuery( "delete from TreeType" ).executeUpdate();
+					session.createMutationQuery( "delete from ForestType" ).executeUpdate();
+					session.createMutationQuery( "delete from BiggestForest" ).executeUpdate();
 				}
 		);
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/onetoone/OptionalOneToOneMappedByTest.java

@@ -22,6 +22,7 @@
 import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
@@ -182,6 +183,8 @@ public void testBidirQueryEntityProperty() {
 //					.uniqueResult();
 
 			session.delete( personAddress );
+			assertNotSame( person, personAddress.getPerson() );
+			personAddress.getPerson().setPersonAddress( null );
 		} );
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/lazy/proxy/inlinedirtychecking/LoadUninitializedCollectionTest.java

@@ -116,6 +116,7 @@ public void tearDown(SessionFactoryScope scope) {
 					 bank.getDepartments().forEach(
 							 department -> entityManager.remove( department )
 					 );
+					 bank.getDepartments().clear();
 					 List<BankAccount> accounts = entityManager.createQuery( "from BankAccount" ).getResultList();
 
 					 accounts.forEach(

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/cascade/multicircle/MultiCircleJpaCascadeTest.java

@@ -258,8 +258,7 @@ public void testPersistThenUpdateNoCascadeToTransient3(EntityManagerFactoryScope
 						IllegalStateException ise = (IllegalStateException) ex.getCause();
 						assertTyping( TransientObjectException.class, ise.getCause() );
 						String message = ise.getCause().getMessage();
-						assertTrue( message.contains("'org.hibernate.orm.test.jpa.cascade.multicircle.F'") );
-						assertTrue( message.contains("'g'") );
+						assertTrue( message.contains("org.hibernate.orm.test.jpa.cascade.multicircle") );
 					}
 					finally {
 						entityManager.getTransaction().rollback();

hibernate-core/src/test/java/org/hibernate/orm/test/onetoone/bidirectional/BidirectionalOneToOneCascadeRemoveTest.java

@@ -0,0 +1,121 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.onetoone.bidirectional;
+
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.CascadeType;
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.JoinColumn;
+import jakarta.persistence.OneToOne;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Marco Belladelli
+ */
+@DomainModel( annotatedClasses = {
+		BidirectionalOneToOneCascadeRemoveTest.A.class,
+		BidirectionalOneToOneCascadeRemoveTest.B.class,
+} )
+@SessionFactory
+public class BidirectionalOneToOneCascadeRemoveTest {
+	@Test
+	public void testWithFlush(SessionFactoryScope scope) {
+		scope.inTransaction( session -> {
+			final A a1 = new A( "1", "a1", 1 );
+			session.persist( a1 );
+			final B bRef = new B( "2", "b2", 2, a1 );
+			session.persist( bRef );
+			session.flush();
+
+			session.remove( bRef );
+		} );
+		scope.inTransaction( session -> {
+			assertThat( session.find( A.class, "1" ) ).isNull();
+			assertThat( session.find( B.class, "2" ) ).isNull();
+		} );
+	}
+
+	@Test
+	public void testWithoutFlush(SessionFactoryScope scope) {
+		scope.inTransaction( session -> {
+			final A a1 = new A( "1", "a1", 1 );
+			session.persist( a1 );
+			final B bRef = new B( "2", "b2", 2, a1 );
+			session.persist( bRef );
+
+			session.remove( bRef );
+		} );
+		scope.inTransaction( session -> {
+			assertThat( session.find( A.class, "1" ) ).isNull();
+			assertThat( session.find( B.class, "2" ) ).isNull();
+		} );
+	}
+
+	@Entity( name = "EntityA" )
+	static class A {
+		@Id
+		protected String id;
+
+		@Column( name = "name_col" )
+		protected String name;
+
+		@Column( name = "value_col" )
+		protected int value;
+
+
+		@OneToOne( mappedBy = "a1" )
+		protected B b1;
+
+		public A() {
+		}
+
+		public A(String id, String name, int value) {
+			this.id = id;
+			this.name = name;
+			this.value = value;
+		}
+	}
+
+	@Entity( name = "EntityB" )
+	static class B {
+		@Id
+		protected String id;
+
+		@Column( name = "name_col" )
+		protected String name;
+
+		@Column( name = "value_col" )
+		protected int value;
+
+		// ===========================================================
+		// relationship fields
+
+		@OneToOne( cascade = CascadeType.REMOVE )
+		@JoinColumn( name = "a1_id" )
+		protected A a1;
+
+		// ===========================================================
+		// constructors
+
+		public B() {
+		}
+
+		public B(String id, String name, int value, A a1) {
+			this.id = id;
+			this.name = name;
+			this.value = value;
+			this.a1 = a1;
+		}
+	}
+}