From ad6af3af7d022a03a578e8806cf1acba2725810e Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Thu, 21 Jan 2021 15:21:01 -0500 Subject: [PATCH] HHH-14413 fix issue that EntityUpdateAction increments version despite veto on update --- .../action/internal/EntityUpdateAction.java | 42 +++++----- .../event/PreUpdateEventListenerVetoTest.java | 84 +++++++++++++++++++ 2 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/event/PreUpdateEventListenerVetoTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java index f2cd99fee0..41bc9f9e65 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java +++ b/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,27 +198,24 @@ public class EntityUpdateAction extends EntityAction { else { ck = null; } - - if ( !veto ) { - persister.update( - id, - state, - dirtyFields, - hasDirtyCollection, - previousState, - previousVersion, - instance, - rowId, - session - ); - } + persister.update( + id, + state, + dirtyFields, + hasDirtyCollection, + previousState, + previousVersion, + instance, + 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) { diff --git a/hibernate-core/src/test/java/org/hibernate/event/PreUpdateEventListenerVetoTest.java b/hibernate-core/src/test/java/org/hibernate/event/PreUpdateEventListenerVetoTest.java new file mode 100644 index 0000000000..f3ffaa9fa5 --- /dev/null +++ b/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; + + } +}