HHH-11721 - PreInsertEventListener that just vetoed ends up throwing NullPointerException because the entity is still managed

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

@@ -37,6 +37,8 @@ public abstract class EntityAction
 	private transient SharedSessionContractImplementor session;
 	private transient EntityPersister persister;
 
+	private transient boolean veto;
+
 	/**
 	 * Instantiate an action.
 	 *
@@ -53,6 +55,14 @@ public abstract class EntityAction
 		this.persister = persister;
 	}
 
+	public boolean isVeto() {
+		return veto;
+	}
+
+	public void setVeto(boolean veto) {
+		this.veto = veto;
+	}
+
 	@Override
 	public BeforeTransactionCompletionProcess getBeforeTransactionCompletionProcess() {
 		return null;

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

@@ -0,0 +1,33 @@
+/*
+ * 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.action.internal;
+import org.hibernate.HibernateException;
+
+/**
+ * An exception indicating that an {@link org.hibernate.action.internal.EntityAction} was vetoed.
+ *
+ * @author Vlad Mihalcea
+ */
+public class EntityActionVetoException extends HibernateException {
+
+	private final EntityAction entityAction;
+
+	/**
+	 * Constructs a EntityActionVetoException
+	 *
+	 * @param message Message explaining the exception condition
+	 * @param entityAction The {@link org.hibernate.action.internal.EntityAction} was vetoed that was vetoed.
+	 */
+	public EntityActionVetoException(String message, EntityAction entityAction) {
+		super( message );
+		this.entityAction = entityAction;
+	}
+
+	public EntityAction getEntityAction() {
+		return entityAction;
+	}
+}

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

@@ -72,12 +72,12 @@ public final class EntityIdentityInsertAction extends AbstractEntityInsertAction
 		final SharedSessionContractImplementor session = getSession();
 		final Object instance = getInstance();
 
-		final boolean veto = preInsert();
+		setVeto( preInsert() );
 
 		// Don't need to lock the cache here, since if someone
 		// else inserted the same pk first, the insert would fail
 
-		if ( !veto ) {
+		if ( !isVeto() ) {
 			generatedId = persister.insert( getState(), instance, session );
 			if ( persister.hasInsertGeneratedProperties() ) {
 				persister.processInsertGeneratedProperties( generatedId, instance, getState(), session );
@@ -101,7 +101,7 @@ public final class EntityIdentityInsertAction extends AbstractEntityInsertAction
 
 		postInsert();
 
-		if ( session.getFactory().getStatistics().isStatisticsEnabled() && !veto ) {
+		if ( session.getFactory().getStatistics().isStatisticsEnabled() && !isVeto() ) {
 			session.getFactory().getStatisticsImplementor().insertEntity( getPersister().getEntityName() );
 		}
 

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

@@ -30,6 +30,7 @@ import org.hibernate.action.internal.BulkOperationCleanupAction;
 import org.hibernate.action.internal.CollectionRecreateAction;
 import org.hibernate.action.internal.CollectionRemoveAction;
 import org.hibernate.action.internal.CollectionUpdateAction;
+import org.hibernate.action.internal.EntityActionVetoException;
 import org.hibernate.action.internal.EntityDeleteAction;
 import org.hibernate.action.internal.EntityIdentityInsertAction;
 import org.hibernate.action.internal.EntityInsertAction;
@@ -283,13 +284,22 @@ public class ActionQueue {
 			LOG.trace( "Adding resolved non-early insert action." );
 			addAction( AbstractEntityInsertAction.class, insert );
 		}
+		if ( !insert.isVeto() ) {
 			insert.makeEntityManaged();
+
 			if( unresolvedInsertions != null ) {
 				for ( AbstractEntityInsertAction resolvedAction : unresolvedInsertions.resolveDependentActions( insert.getInstance(), session ) ) {
 					addResolvedEntityInsertAction( resolvedAction );
 				}
 			}
 		}
+		else {
+			throw new EntityActionVetoException(
+				"The EntityInsertAction was vetoed.",
+				insert
+			);
+		}
+	}
 
 	@SuppressWarnings("unchecked")
 	private <T extends Executable & Comparable & Serializable> void addAction(Class<T> executableClass, T action) {

hibernate-core/src/test/java/org/hibernate/event/PreInsertEventListenerVetoBidirectionalTest.java

@@ -0,0 +1,138 @@
+package org.hibernate.event;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.OneToOne;
+
+import org.hibernate.TransientObjectException;
+import org.hibernate.action.internal.EntityActionVetoException;
+import org.hibernate.event.service.spi.EventListenerRegistry;
+import org.hibernate.event.spi.EventType;
+import org.hibernate.event.spi.PreInsertEventListener;
+
+import org.hibernate.testing.DialectChecks;
+import org.hibernate.testing.RequiresDialectFeature;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Chris Cranford
+ */
+@RequiresDialectFeature(DialectChecks.SupportsIdentityColumns.class)
+@TestForIssue(jiraKey = "HHH-11721")
+public class PreInsertEventListenerVetoBidirectionalTest extends BaseCoreFunctionalTestCase {
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { Child.class, Parent.class };
+	}
+
+	@Override
+	protected void afterSessionFactoryBuilt() {
+		super.afterSessionFactoryBuilt();
+		EventListenerRegistry registry = sessionFactory().getServiceRegistry()
+				.getService( EventListenerRegistry.class );
+		registry.appendListeners(
+				EventType.PRE_INSERT,
+				(PreInsertEventListener) event -> event.getEntity() instanceof Parent
+		);
+	}
+
+	@Test(expected = EntityActionVetoException.class)
+	public void testVeto() {
+		doInHibernate( this::sessionFactory, session -> {
+			Parent parent = new Parent();
+			parent.setField1( "f1" );
+			parent.setfield2( "f2" );
+
+			Child child = new Child();
+			parent.setChild( child );
+
+			session.save( parent );
+		} );
+
+		fail( "Should have thrown EntityActionVetoException!" );
+	}
+
+	@Entity(name = "Child")
+	public static class Child {
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		private Integer id;
+
+		@OneToOne
+		private Parent parent;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public Parent getParent() {
+			return parent;
+		}
+
+		public void setParent(Parent parent) {
+			this.parent = parent;
+		}
+	}
+
+	@Entity(name = "Parent")
+	public static class Parent {
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		private Integer id;
+
+		private String field1;
+
+		private String field2;
+
+		@OneToOne(cascade = CascadeType.ALL, mappedBy = "parent")
+		private Child child;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public String getField1() {
+			return field1;
+		}
+
+		public void setField1(String field1) {
+			this.field1 = field1;
+		}
+
+		public String getField2() {
+			return field2;
+		}
+
+		public void setfield2(String field2) {
+			this.field2 = field2;
+		}
+
+		public Child getChild() {
+			return child;
+		}
+
+		public void setChild(Child child) {
+			this.child = child;
+			child.setParent( this );
+		}
+	}
+}

hibernate-core/src/test/java/org/hibernate/event/PreInsertEventListenerVetoUnidirectionalTest.java

@@ -0,0 +1,122 @@
+package org.hibernate.event;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.OneToOne;
+
+import org.hibernate.TransientObjectException;
+import org.hibernate.action.internal.EntityActionVetoException;
+import org.hibernate.event.service.spi.EventListenerRegistry;
+import org.hibernate.event.spi.EventType;
+import org.hibernate.event.spi.PreInsertEventListener;
+
+import org.hibernate.testing.DialectChecks;
+import org.hibernate.testing.RequiresDialectFeature;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Chris Cranford
+ */
+@RequiresDialectFeature(DialectChecks.SupportsIdentityColumns.class)
+@TestForIssue( jiraKey = "HHH-11721" )
+public class PreInsertEventListenerVetoUnidirectionalTest extends BaseCoreFunctionalTestCase {
+    @Override
+    protected Class<?>[] getAnnotatedClasses() {
+        return new Class<?>[] { Child.class, Parent.class };
+    }
+
+    @Override
+    protected void afterSessionFactoryBuilt() {
+        super.afterSessionFactoryBuilt();
+        EventListenerRegistry registry = sessionFactory().getServiceRegistry().getService( EventListenerRegistry.class );
+        registry.appendListeners(
+                EventType.PRE_INSERT,
+                (PreInsertEventListener) event -> event.getEntity() instanceof Parent
+        );
+    }
+
+    @Test(expected = EntityActionVetoException.class)
+    public void testVeto() {
+        doInHibernate( this::sessionFactory, session -> {
+            Parent parent = new Parent();
+            parent.setField1( "f1" );
+            parent.setfield2( "f2" );
+
+            Child child = new Child();
+            child.setParent( parent );
+
+            session.save( child );
+        } );
+
+        fail( "Should have thrown EntityActionVetoException!" );
+    }
+
+    @Entity(name = "Child")
+    public static class Child {
+        @Id
+        @GeneratedValue(strategy = GenerationType.IDENTITY)
+        private Integer id;
+
+        @OneToOne(cascade = CascadeType.ALL)
+        private Parent parent;
+
+        public Integer getId() {
+            return id;
+        }
+
+        public void setId(Integer id) {
+            this.id = id;
+        }
+
+        public Parent getParent() {
+            return parent;
+        }
+
+        public void setParent(Parent parent) {
+            this.parent = parent;
+        }
+    }
+
+    @Entity(name = "Parent")
+    public static class Parent {
+        @Id
+        @GeneratedValue(strategy = GenerationType.IDENTITY)
+        private Integer id;
+        private String field1;
+        private String field2;
+
+        public Integer getId() {
+            return id;
+        }
+
+        public void setId(Integer id) {
+            this.id = id;
+        }
+
+        public String getField1() {
+            return field1;
+        }
+
+        public void setField1(String field1) {
+            this.field1 = field1;
+        }
+
+        public String getField2() {
+            return field2;
+        }
+
+        public void setfield2(String field2) {
+            this.field2 = field2;
+        }
+    }
+}