From 4a5ec1891355aa3dc0dfdeafe05007b7938925d3 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Tue, 23 Jul 2019 21:08:39 -0700 Subject: [PATCH 1/2] HHH-13492 : test cases --- .../test/locking/LockRefreshTest.java | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/locking/LockRefreshTest.java diff --git a/hibernate-core/src/test/java/org/hibernate/test/locking/LockRefreshTest.java b/hibernate-core/src/test/java/org/hibernate/test/locking/LockRefreshTest.java new file mode 100644 index 000000000000..7d9a13ea710e --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/locking/LockRefreshTest.java @@ -0,0 +1,160 @@ +/* + * 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 . + */ +package org.hibernate.test.locking; + +import java.util.Arrays; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.LockModeType; +import javax.persistence.Version; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.hibernate.testing.junit4.CustomParameterized; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +/** + * @author Gail Badner + */ + +@TestForIssue(jiraKey = "HHH-13492") +@RunWith(CustomParameterized.class) +public class LockRefreshTest extends BaseNonConfigCoreFunctionalTestCase { + private final LockModeType lockModeType; + + @Parameterized.Parameters(name = "JpaComplianceCachingSetting={0}") + public static Iterable parameters() { + return Arrays.asList( + new Object[][] { + { LockModeType.OPTIMISTIC }, + { LockModeType.OPTIMISTIC_FORCE_INCREMENT } + } + ); + } + + public LockRefreshTest(LockModeType lockModeType) { + this.lockModeType = lockModeType; + } + + @Test + public void testLockRefreshUpdate() { + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + session.lock( employee, lockModeType ); + session.refresh( employee ); + employee.department = "Finance"; + } + ); + + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + assertEquals( "Finance", employee.department ); + } + ); + } + + @Test + public void testLockRefreshMerge() { + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + session.lock( employee, lockModeType ); + session.refresh( employee ); + employee.department = "Finance"; + session.merge( employee ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + assertEquals( "Finance", employee.department ); + } + ); + } + + @Test + public void testLockRefreshDelete() { + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + session.lock( employee, lockModeType ); + session.refresh( employee ); + session.delete( employee ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + assertNull( session.get( Employee.class, "Jane" ) ); + } + ); + } + + @Test + public void testLockRefreshEvict() { + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + session.lock( employee, lockModeType ); + session.refresh( employee ); + employee.department = "Finance"; + session.evict( employee ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + final Employee employee = session.get( Employee.class, "Jane" ); + assertEquals( "Software Engineering", employee.department ); + } + ); + } + + @Override + public void prepareTest() { + final Employee employee = new Employee(); + employee.name = "Jane"; + employee.department = "Software Engineering"; + + doInHibernate( + this::sessionFactory, session -> { + session.persist( employee ); + } + ); + } + + protected boolean isCleanupTestDataRequired() { + return true; + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Employee.class }; + } + + @Entity(name = "Employee") + public static class Employee { + @Id + private String name; + + private String department; + + @Version + @Column(name = "ver") + private int version; + } +} From 29adcd2e80796eba26e886230a0a63676934b9c7 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Tue, 23 Jul 2019 21:11:18 -0700 Subject: [PATCH 2/2] HHH-13492 : OptimisticLockException after locking, refreshing, and updating an entity --- .../internal/EntityIncrementVersionProcess.java | 11 +++++++---- .../action/internal/EntityVerifyVersionProcess.java | 13 +++++-------- .../OptimisticForceIncrementLockingStrategy.java | 2 +- .../dialect/lock/OptimisticLockingStrategy.java | 4 +--- .../internal/DefaultPostLoadEventListener.java | 4 ++-- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityIncrementVersionProcess.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityIncrementVersionProcess.java index 77bcaaceb3ba..6db4a6e5b231 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityIncrementVersionProcess.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityIncrementVersionProcess.java @@ -19,17 +19,14 @@ */ public class EntityIncrementVersionProcess implements BeforeTransactionCompletionProcess { private final Object object; - private final EntityEntry entry; /** * Constructs an EntityIncrementVersionProcess for the given entity. * * @param object The entity instance - * @param entry The entity's EntityEntry reference */ - public EntityIncrementVersionProcess(Object object, EntityEntry entry) { + public EntityIncrementVersionProcess(Object object) { this.object = object; - this.entry = entry; } /** @@ -39,6 +36,12 @@ public EntityIncrementVersionProcess(Object object, EntityEntry entry) { */ @Override public void doBeforeTransactionCompletion(SessionImplementor session) { + final EntityEntry entry = session.getPersistenceContext().getEntry( object ); + // Don't increment version for an entity that is not in the PersistenceContext; + if ( entry == null ) { + return; + } + final EntityPersister persister = entry.getPersister(); final Object nextVersion = persister.forceVersionIncrement( entry.getId(), entry.getVersion(), session ); entry.forceLocked( object, nextVersion ); diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityVerifyVersionProcess.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityVerifyVersionProcess.java index de813de3fd13..28ba3195e52e 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityVerifyVersionProcess.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityVerifyVersionProcess.java @@ -21,28 +21,25 @@ */ public class EntityVerifyVersionProcess implements BeforeTransactionCompletionProcess { private final Object object; - private final EntityEntry entry; /** * Constructs an EntityVerifyVersionProcess * * @param object The entity instance - * @param entry The entity's referenced EntityEntry */ - public EntityVerifyVersionProcess(Object object, EntityEntry entry) { + public EntityVerifyVersionProcess(Object object) { this.object = object; - this.entry = entry; } @Override public void doBeforeTransactionCompletion(SessionImplementor session) { - final EntityPersister persister = entry.getPersister(); - - if ( !entry.isExistsInDatabase() ) { - // HHH-9419: We cannot check for a version of an entry we ourselves deleted + final EntityEntry entry = session.getPersistenceContext().getEntry( object ); + // Don't check version for an entity that is not in the PersistenceContext; + if ( entry == null ) { return; } + final EntityPersister persister = entry.getPersister(); final Object latestVersion = persister.getCurrentVersion( entry.getId(), session ); if ( !entry.getVersion().equals( latestVersion ) ) { throw new OptimisticLockException( diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticForceIncrementLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticForceIncrementLockingStrategy.java index 61e4c4f3ee1b..d957b4444659 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticForceIncrementLockingStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticForceIncrementLockingStrategy.java @@ -50,7 +50,7 @@ public void lock(Serializable id, Object version, Object object, int timeout, Sh } final EntityEntry entry = session.getPersistenceContext().getEntry( object ); // Register the EntityIncrementVersionProcess action to run just prior to transaction commit. - ( (EventSource) session ).getActionQueue().registerProcess( new EntityIncrementVersionProcess( object, entry ) ); + ( (EventSource) session ).getActionQueue().registerProcess( new EntityIncrementVersionProcess( object ) ); } protected LockMode getLockMode() { diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticLockingStrategy.java index 262149c2aa3a..dceea8792602 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticLockingStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticLockingStrategy.java @@ -12,7 +12,6 @@ import org.hibernate.LockMode; import org.hibernate.OptimisticLockException; import org.hibernate.action.internal.EntityVerifyVersionProcess; -import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.event.spi.EventSource; import org.hibernate.persister.entity.Lockable; @@ -49,9 +48,8 @@ public void lock(Serializable id, Object version, Object object, int timeout, Sh if ( !lockable.isVersioned() ) { throw new OptimisticLockException( object, "[" + lockMode + "] not supported for non-versioned entities [" + lockable.getEntityName() + "]" ); } - final EntityEntry entry = session.getPersistenceContext().getEntry( object ); // Register the EntityVerifyVersionProcess action to run just prior to transaction commit. - ( (EventSource) session ).getActionQueue().registerProcess( new EntityVerifyVersionProcess( object, entry ) ); + ( (EventSource) session ).getActionQueue().registerProcess( new EntityVerifyVersionProcess( object ) ); } protected LockMode getLockMode() { diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPostLoadEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPostLoadEventListener.java index 5385cf13d045..cbd3449a22bb 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPostLoadEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPostLoadEventListener.java @@ -57,11 +57,11 @@ public void onPostLoad(PostLoadEvent event) { entry.forceLocked( entity, nextVersion ); } else if ( LockMode.OPTIMISTIC_FORCE_INCREMENT.equals( lockMode ) ) { - final EntityIncrementVersionProcess incrementVersion = new EntityIncrementVersionProcess( entity, entry ); + final EntityIncrementVersionProcess incrementVersion = new EntityIncrementVersionProcess( entity ); event.getSession().getActionQueue().registerProcess( incrementVersion ); } else if ( LockMode.OPTIMISTIC.equals( lockMode ) ) { - final EntityVerifyVersionProcess verifyVersion = new EntityVerifyVersionProcess( entity, entry ); + final EntityVerifyVersionProcess verifyVersion = new EntityVerifyVersionProcess( entity ); event.getSession().getActionQueue().registerProcess( verifyVersion ); }