HHH-14413 fix issue that EntityUpdateAction increments version despite veto on update

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

@@ -171,7 +171,9 @@ public class EntityUpdateAction extends EntityAction {
 		final SharedSessionContractImplementor session = getSession();
 		final Object instance = getInstance();
 
-		final boolean veto = preUpdate();
+		if ( preUpdate() ) {
+			return;
+		}
 
 		final SessionFactoryImplementor factory = session.getFactory();
 		Object previousVersion = this.previousVersion;
@@ -196,8 +198,6 @@ public class EntityUpdateAction extends EntityAction {
 		else {
 			ck = null;
 		}
-
-		if ( !veto ) {
 		persister.update(
 				id,
 				state,
@@ -209,14 +209,13 @@ public class EntityUpdateAction extends EntityAction {
 				rowId,
 				session
 		);
-		}
 
 		final EntityEntry entry = session.getPersistenceContextInternal().getEntry( instance );
 		if ( entry == null ) {
-			throw new AssertionFailure( "possible nonthreadsafe access to session" );
+			throw new AssertionFailure( "possible non thread safe access to session" );
 		}
 
-		if ( entry.getStatus()==Status.MANAGED || persister.isVersionPropertyGenerated() ) {
+		if ( entry.getStatus() == Status.MANAGED || persister.isVersionPropertyGenerated() ) {
 			// get the updated snapshot of the entity state by cloning current state;
 			// it is safe to copy in place, since by this time no-one else (should have)
 			// has a reference  to the array
@@ -242,12 +241,12 @@ public class EntityUpdateAction extends EntityAction {
 
 		final StatisticsImplementor statistics = factory.getStatistics();
 		if ( persister.canWriteToCache() ) {
-			if ( persister.isCacheInvalidationRequired() || entry.getStatus()!= Status.MANAGED ) {
-				persister.getCacheAccessStrategy().remove( session, ck);
+			if ( persister.isCacheInvalidationRequired() || entry.getStatus() != Status.MANAGED ) {
+				persister.getCacheAccessStrategy().remove( session, ck );
 			}
 			else if ( session.getCacheMode().isPutEnabled() ) {
 				//TODO: inefficient if that cache is just going to ignore the updated state!
-				final CacheEntry ce = persister.buildCacheEntry( instance,state, nextVersion, getSession() );
+				final CacheEntry ce = persister.buildCacheEntry( instance, state, nextVersion, getSession() );
 				cacheEntry = persister.getCacheEntryStructure().structure( ce );
 
 				final boolean put = cacheUpdate( persister, previousVersion, ck );
@@ -270,9 +269,10 @@ public class EntityUpdateAction extends EntityAction {
 
 		postUpdate();
 
-		if ( statistics.isStatisticsEnabled() && !veto ) {
+		if ( statistics.isStatisticsEnabled() ) {
 			statistics.updateEntity( getPersister().getEntityName() );
 		}
+
 	}
 
 	protected boolean cacheUpdate(EntityPersister persister, Object previousVersion, Object ck) {

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

@@ -0,0 +1,84 @@
+package org.hibernate.event;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Version;
+
+import org.hibernate.event.service.spi.EventListenerRegistry;
+import org.hibernate.event.spi.EventType;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Nathan Xu
+ * @author Tassilo Karge
+ */
+@TestForIssue( jiraKey = "HHH-14413" )
+public class PreUpdateEventListenerVetoTest extends BaseCoreFunctionalTestCase {
+
+	private static final Long EXAMPLE_ID_VALUE = 1L;
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { ExampleEntity.class };
+	}
+
+	@Override
+	protected void afterSessionFactoryBuilt() {
+		super.afterSessionFactoryBuilt();
+		EventListenerRegistry registry = sessionFactory().getServiceRegistry().getService( EventListenerRegistry.class );
+		registry.appendListeners(
+				EventType.PRE_UPDATE,
+				event -> true
+		);
+	}
+
+	@Before
+	public void setUp() {
+		doInHibernate( this::sessionFactory, session -> {
+			ExampleEntity entity = new ExampleEntity();
+			entity.id = EXAMPLE_ID_VALUE;
+			entity.name = "old_name";
+			session.save( entity );
+		} );
+	}
+
+	@Test
+	public void testVersionNotChangedWhenPreUpdateEventVetoed() {
+
+		doInHibernate( this::sessionFactory, session -> {
+			ExampleEntity entity = session.byId( ExampleEntity.class ).load( EXAMPLE_ID_VALUE );
+
+			entity.name = "new_name";
+			session.update( entity );
+
+			final Long versionBeforeFlush = entity.version;
+
+			session.flush();
+
+			final Long versionAfterFlush = entity.version;
+
+			assertEquals( "The entity version must not change when update is vetoed", versionBeforeFlush, versionAfterFlush );
+
+		} );
+	}
+
+	@Entity(name = "ExampleEntity")
+	public static class ExampleEntity {
+
+		@Id
+		Long id;
+
+		String name;
+
+		@Version
+		Long version;
+
+	}
+}