[ISSUE 7] ReadWriteAccessDomainDataStorageAccess checks transaction status for cacheEvict on cachePut (#9)

rica-v3 · rica.rdo · web-flow · commit 1df0711632e8 · 2021-03-09T23:06:46.000+09:00
* Add test cases in ReadWriteAccessDomainDataRegionGroupTest

* Modify loglevel in HibernateArcusStorageAccess.contains

* Modify ReadWriteAccessDomainDataStorageAccess.putIntoCache to check tx status not lock

* Fix a test in ReadWriteAccessDomainDataRegionGroupTest

* Add tests for multiple queries in a single transaction in TransactionAccessDomainDataRegionGroupTest

* Remove unused field in ReadWriteAccessDomainDataRegionGroupTest

* Modify DomainDataHibernateArcusStorageAccess not evict its own region item if read-write access

* Override evictData in ReadWriteAccessDomainDataStorageAccess

* Remove unnecessary system log

* Add null check test verification in

testCacheEvictOnCachePut_whenDomainRegionOneEntityIsDeleted\
AndDomainRegionTwoIsUpdatedInTheSameTransaction_thenDomainRegionTwoShouldBeCacheMiss

* Modify ReadWriteAccessDomainDataStorageAccess.evictData

by reorder of conditions

* Modify DomainDataHibernateArcusStorageAccess.evictData

by reorder of conditions

Co-authored-by: rica.rdo &lt;rica.rdo@kakaopaycorp.com&gt;
diff --git a/build.gradle b/build.gradle
@@ -25,8 +25,6 @@ dependencies {
     testCompile group: 'org.hibernate', name: 'hibernate-testing', version: '5.4.27.Final'
     testCompile group: 'org.assertj', name: 'assertj-core', version: '3.18.1'
     testCompile group: 'com.h2database', name: 'h2', version: '1.4.200'
-
-    implementation group: 'com.google.guava', name: 'guava', version: '30.1-jre'
 }
 tasks.test {
     useJUnit()
diff --git a/src/main/java/com/devookim/hibernatearcus/storage/DomainDataHibernateArcusStorageAccess.java b/src/main/java/com/devookim/hibernatearcus/storage/DomainDataHibernateArcusStorageAccess.java
@@ -14,7 +14,7 @@
 
 @Slf4j
 public class DomainDataHibernateArcusStorageAccess extends HibernateArcusStorageAccess {
-    private static final HashMap<String, DomainDataHibernateArcusStorageAccess> domainDataStorageAccesses = new HashMap<>();
+    protected static final HashMap<String, DomainDataHibernateArcusStorageAccess> domainDataStorageAccesses = new HashMap<>();
 
     private final HibernateArcusStorageConfig storageAccessConfig;
     public final String entityClassName;
@@ -63,19 +63,19 @@ && contains(key)) {
 
     @Override
     public void evictData(Object key) {
-        if (storageAccessConfig.regionGroupOnCacheEvict.contains(super.CACHE_REGION) && storageAccessConfig.enableCacheEvictOnCachePut) {
+        if (storageAccessConfig.enableCacheEvictOnCachePut
+                && storageAccessConfig.regionGroupOnCacheEvict.contains(CACHE_REGION)) {
             String id = key.toString().split("#")[1];
             log.debug("regionGroupOnCacheEvict contains region: {}, id: {}", CACHE_REGION, id);
-            domainDataStorageAccesses.forEach((s, storageAccess) -> {
-                storageAccess.evictDataOnRegionGroupCacheEvict(new HibernateArcusCacheKeysFactory.EntityKey(storageAccess.entityClassName, id));
-            });
+            domainDataStorageAccesses.forEach((region, storageAccess) ->
+                    storageAccess.evictDataOnRegionGroupCacheEvict(new HibernateArcusCacheKeysFactory.EntityKey(storageAccess.entityClassName, id)));
         } else {
             super.evictData(key);
         }
     }
 
     public void evictDataOnRegionGroupCacheEvict(Object key) {
-        if (storageAccessConfig.regionGroupOnCacheEvict.contains(super.CACHE_REGION)) {
+        if (storageAccessConfig.regionGroupOnCacheEvict.contains(CACHE_REGION)) {
             log.debug("cacheEvict {} by regionGroupOnCacheEvict", key);
             super.evictData(key);
         }
diff --git a/src/main/java/com/devookim/hibernatearcus/storage/HibernateArcusStorageAccess.java b/src/main/java/com/devookim/hibernatearcus/storage/HibernateArcusStorageAccess.java
@@ -72,7 +72,7 @@ public boolean contains(Object key) {
         String generatedKey = generateKey(key);
         try {
             Object result = arcusClientFactory.getClientPool().get(generatedKey);
-            log.info("containKey for {} contains: {}", generatedKey, result != null);
+            log.trace("containKey for {} contains: {}", generatedKey, result != null);
             return result != null;
         } catch (Exception e) {
             log.error("fallbackEnabled: {} key: {} errorMsg: {}", arcusClientFactory.fallbackEnabled, generatedKey, e.getMessage());
diff --git a/src/main/java/com/devookim/hibernatearcus/storage/ReadWriteAccessDomainDataStorageAccess.java b/src/main/java/com/devookim/hibernatearcus/storage/ReadWriteAccessDomainDataStorageAccess.java
@@ -2,36 +2,51 @@
 
 import com.devookim.hibernatearcus.client.HibernateArcusClientFactory;
 import com.devookim.hibernatearcus.config.HibernateArcusStorageConfig;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
+import com.devookim.hibernatearcus.factory.HibernateArcusCacheKeysFactory;
+import lombok.extern.slf4j.Slf4j;
 import org.hibernate.cache.cfg.spi.DomainDataRegionConfig;
 import org.hibernate.cache.spi.support.AbstractReadWriteAccess;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
+import org.hibernate.resource.transaction.spi.TransactionStatus;
 
-import java.time.Duration;
-
+@Slf4j
 public class ReadWriteAccessDomainDataStorageAccess extends DomainDataHibernateArcusStorageAccess {
-    private final Cache<AbstractReadWriteAccess.SoftLockImpl, Object> readWriteAccessLocks;
     private final HibernateArcusStorageConfig storageAccessConfig;
 
     public ReadWriteAccessDomainDataStorageAccess(HibernateArcusClientFactory arcusClientFactory, String regionName, HibernateArcusStorageConfig storageAccessConfig, DomainDataRegionConfig regionConfig) {
         super(arcusClientFactory, regionName, storageAccessConfig, regionConfig);
         this.storageAccessConfig = storageAccessConfig;
-        readWriteAccessLocks = CacheBuilder.newBuilder()
-                .expireAfterWrite(Duration.ofSeconds(10))
-                .maximumSize(10000)
-                .concurrencyLevel(1000)
-                .build();
     }
 
     @Override
     public void putIntoCache(Object key, Object value, SharedSessionContractImplementor session) {
         if (storageAccessConfig.enableCacheEvictOnCachePut
                 && value instanceof AbstractReadWriteAccess.SoftLockImpl
-                && readWriteAccessLocks.getIfPresent(value) == null) {
+                && isTransactionActive(session)) {
+            log.debug("enableCacheEvictOnCachePut enabled. key: {}", key);
             evictData(key);
-            readWriteAccessLocks.put((AbstractReadWriteAccess.SoftLockImpl) value, key);
         }
         super.putIntoCache(key, value, session);
     }
+
+    @Override
+    public void evictData(Object key) {
+        if (storageAccessConfig.enableCacheEvictOnCachePut &&
+                storageAccessConfig.regionGroupOnCacheEvict.contains(CACHE_REGION)) {
+            String id = key.toString().split("#")[1];
+            log.debug("regionGroupOnCacheEvict contains region: {}, id: {}", CACHE_REGION, id);
+            domainDataStorageAccesses.forEach((region, storageAccess) -> {
+                if (!region.equals(CACHE_REGION)) {
+                    storageAccess.evictDataOnRegionGroupCacheEvict(new HibernateArcusCacheKeysFactory.EntityKey(storageAccess.entityClassName, id));
+                }
+            });
+        } else {
+            super.evictData(key);
+        }
+    }
+
+    private boolean isTransactionActive(SharedSessionContractImplementor session) {
+        return session.getTransaction() != null
+                && session.getTransaction().getStatus().equals(TransactionStatus.ACTIVE);
+    }
 }
diff --git a/src/test/java/hibernate/ReadWriteAccessDomainDataRegionGroupTest.java b/src/test/java/hibernate/ReadWriteAccessDomainDataRegionGroupTest.java
@@ -17,11 +17,10 @@
 import java.io.Serializable;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 public class ReadWriteAccessDomainDataRegionGroupTest extends BaseCoreFunctionalTestCase {
 
-    private final String collectionCacheRegionName = "hibernate.CollectionCacheTest$ParentDomainData.children";
-
     @Override
     protected Class<?>[] getAnnotatedClasses() {
         return new Class[]{DomainRegionOne.class, DomainRegionTwo.class};
@@ -72,6 +71,7 @@ public void testCacheEvictOnCachePut_whenADomainRegionOneEntityIsDeleted_thenDom
         s.beginTransaction();
         DomainRegionOne domainRegionOneFromCache = s.get(DomainRegionOne.class, id);
         s.delete(domainRegionOneFromCache);
+        s.flush();
         s.getTransaction().commit();
         s.close();
         System.out.println("============================");
@@ -82,8 +82,62 @@ public void testCacheEvictOnCachePut_whenADomainRegionOneEntityIsDeleted_thenDom
         s = openSession();
         s.beginTransaction();
         s.get(DomainRegionTwo.class, id);
+        DomainRegionOne domainRegionOneAfterDelete = s.get(DomainRegionOne.class, id);
+        s.getTransaction().commit();
+        s.close();
+        assertNull(domainRegionOneAfterDelete);
+        assertEquals(1, regionTwoStat.getMissCount());
+    }
+
+    /**
+     * When update of regionOneEntity and delete of the regionTwo are executed in a transaction, delete is called after update and the cache item that the update put is evicted.
+     * As a result, the next get operation of the updated entity will get cacheMiss
+     */
+    @Test
+    public void testCacheEvictOnCachePut_whenDomainRegionOneEntityIsDeletedAndDomainRegionTwoIsUpdatedInTheSameTransaction_thenDomainRegionTwoShouldBeCacheMiss() {
+        CacheRegionStatistics regionOneStat = sessionFactory()
+                .getStatistics().getDomainDataRegionStatistics(DomainRegionOne.regionName);
+        CacheRegionStatistics regionTwoStat = sessionFactory()
+                .getStatistics().getDomainDataRegionStatistics(DomainRegionTwo.regionName);
+        final long id = System.currentTimeMillis();
+        Session s = openSession();
+        s.beginTransaction();
+        DomainRegionOne regionOne = new DomainRegionOne(id);
+        DomainRegionTwo regionTwo = new DomainRegionTwo(id);
+        s.save(regionOne);
+        s.save(regionTwo);
+        s.flush();
+        s.getTransaction().commit();
+        s.close();
+        System.out.println("============================");
+
+        assertEquals(1, regionOneStat.getPutCount());
+        assertEquals(0, regionOneStat.getHitCount());
+        assertEquals(1, regionTwoStat.getPutCount());
+
+        s = openSession();
+        s.beginTransaction();
+        DomainRegionOne domainRegionOneFromCache = s.get(DomainRegionOne.class, id);
+        s.delete(domainRegionOneFromCache);
+        DomainRegionTwo domainRegionTwoFromCache = s.get(DomainRegionTwo.class, id);
+        domainRegionTwoFromCache.value = "updated_value";
+        s.update(domainRegionTwoFromCache);
+        s.flush();
+        s.getTransaction().commit();
+        s.close();
+        System.out.println("============================");
+
+        assertEquals(1, regionTwoStat.getHitCount());
+        assertEquals(0, regionTwoStat.getMissCount());
+
+        s = openSession();
+        s.beginTransaction();
+        DomainRegionOne domainRegionOneAfterDelete = s.get(DomainRegionOne.class, id);
+        s.get(DomainRegionTwo.class, id);
         s.getTransaction().commit();
         s.close();
+        assertNull(domainRegionOneAfterDelete);
+        assertEquals(1, regionTwoStat.getHitCount());
         assertEquals(1, regionTwoStat.getMissCount());
     }
 
@@ -129,6 +183,59 @@ public void testCacheEvictOnCachePut_whenADomainRegionOneEntityIsUpdated_thenDom
         assertEquals(1, regionTwoStat.getMissCount());
     }
 
+    /**
+     * When updates of regionOne and regionTwo are executed in a transaction, the cache items of both region entities are put.
+     * However the second update evicts the cache item of the first update.
+     * As a result, the next get operation of regionOne will get cacheMiss, and regionTwo will get cacheHit
+     */
+    @Test
+    public void testCacheEvictOnCachePut_whenDomainRegionOneEntityAndDomainRegionOneEntityAreUpdatedInTheSameTransaction_thenDomainRegionOneShouldBeCacheMissAndDomainRegionTwoShouldBeCacheHit() {
+        CacheRegionStatistics regionOneStat = sessionFactory()
+                .getStatistics().getDomainDataRegionStatistics(DomainRegionOne.regionName);
+        CacheRegionStatistics regionTwoStat = sessionFactory()
+                .getStatistics().getDomainDataRegionStatistics(DomainRegionTwo.regionName);
+        final long id = System.currentTimeMillis();
+        Session s = openSession();
+        s.beginTransaction();
+        DomainRegionOne regionOne = new DomainRegionOne(id);
+        DomainRegionTwo regionTwo = new DomainRegionTwo(id);
+        s.save(regionOne);
+        s.save(regionTwo);
+        s.flush();
+        s.getTransaction().commit();
+        s.close();
+        System.out.println("============================");
+
+        assertEquals(1, regionOneStat.getPutCount());
+        assertEquals(1, regionTwoStat.getPutCount());
+
+        s = openSession();
+        s.beginTransaction();
+        DomainRegionOne domainRegionOneFromCache = s.get(DomainRegionOne.class, id);
+        domainRegionOneFromCache.value = "updated-value";
+        s.update(domainRegionOneFromCache);
+        DomainRegionTwo domainRegionTwoFromCache = s.get(DomainRegionTwo.class, id);
+        domainRegionTwoFromCache.value = "updated_value";
+        s.update(domainRegionTwoFromCache);
+        s.flush();
+        s.getTransaction().commit();
+        s.close();
+        System.out.println("============================");
+
+        assertEquals(1, regionOneStat.getHitCount());
+        assertEquals(1, regionTwoStat.getHitCount());
+
+        s = openSession();
+        s.beginTransaction();
+        s.get(DomainRegionOne.class, id);
+        s.get(DomainRegionTwo.class, id);
+        s.getTransaction().commit();
+        s.close();
+        assertEquals(1, regionOneStat.getHitCount());
+        assertEquals(1, regionOneStat.getMissCount());
+        assertEquals(2, regionTwoStat.getHitCount());
+    }
+
     @Entity
     @NoArgsConstructor
     @Cache(usage = CacheConcurrencyStrategy.READ_WRITE, region = DomainRegionOne.regionName)
@@ -151,6 +258,7 @@ public static class DomainRegionTwo implements Serializable {
 
         @Id
         long id;
+        public String value = "";
 
         public DomainRegionTwo(long id) {
             this.id = id;
diff --git a/src/test/java/hibernate/TransactionAccessDomainDataRegionGroupTest.java b/src/test/java/hibernate/TransactionAccessDomainDataRegionGroupTest.java

Original file line number	Diff line number	Diff line change
`@@ -25,8 +25,6 @@ dependencies {`
`25`	`25`	`testCompile group: 'org.hibernate', name: 'hibernate-testing', version: '5.4.27.Final'`
`26`	`26`	`testCompile group: 'org.assertj', name: 'assertj-core', version: '3.18.1'`
`27`	`27`	`testCompile group: 'com.h2database', name: 'h2', version: '1.4.200'`
`28`		`-`
`29`		`- implementation group: 'com.google.guava', name: 'guava', version: '30.1-jre'`
`30`	`28`	`}`
`31`	`29`	`tasks.test {`
`32`	`30`	`useJUnit()`