INTERNAL: put NullValue instance to Arcus and frontCache

naver · Jul 5, 2024 · 2bc30d4 · 2bc30d4
1 parent 50d116c
commit 2bc30d4
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 40 deletions.
diff --git a/src/main/java/com/navercorp/arcus/spring/cache/ArcusCache.java b/src/main/java/com/navercorp/arcus/spring/cache/ArcusCache.java
@@ -189,9 +189,15 @@ private <T> T loadValue(String arcusKey, Callable<T> valueLoader) {
 
   @Override
   public void put(final Object key, final Object value) {
+    if (value == null && !isAllowNullValues()) {
+      throw new IllegalArgumentException(String.format("Cache '%s' does not allow 'null' values. " +
+              "Avoid storing null via '@Cacheable(unless=\"#result == null\")' or configure ArcusCache " +
+              "to allow 'null' via ArcusCacheConfiguration.", name));
+    }
+
     String arcusKey = createArcusKey(key);
     try {
-      putValue(arcusKey, value);
+      putValue(arcusKey, toStoreValue(value));
     } catch (Exception e) {
       if (wantToGetException) {
         throw toRuntimeException(e);
@@ -211,37 +217,22 @@ public void put(final Object key, final Object value) {
   @Nullable
   @Override
   public ValueWrapper putIfAbsent(Object key, Object value) {
-    String arcusKey = createArcusKey(key);
-    logger.debug("trying to add key: {}", arcusKey);
-
-    if (value == null) {
-      logger.info("arcus cannot putIfAbsent NULL value. key: {}", arcusKey);
-      return toValueWrapper(lookup(key));
+    if (value == null && !isAllowNullValues()) {
+      logger.info(String.format("Cache '%s' does not allow 'null' values. " +
+              "Avoid storing null via '@Cacheable(unless=\"#result == null\")' or configure ArcusCache " +
+              "to allow 'null' via ArcusCacheConfiguration.", name));
+      return super.get(key);
     }
 
+    String arcusKey = createArcusKey(key);
     try {
-      OperationFuture<Boolean> future;
-      if (operationTranscoder != null) {
-        future = arcusClient.add(arcusKey, expireSeconds, value, operationTranscoder);
-      } else {
-        future = arcusClient.add(arcusKey, expireSeconds, value);
-      }
-
-      boolean success = future.get(timeoutMilliSeconds, TimeUnit.MILLISECONDS);
-      if (!success) {
-        OperationStatus status = future.getStatus();
-        logger.info("failed to putIfAbsent a key: {}, status: {}", arcusKey, status.getMessage());
-      } else if (arcusFrontCache != null) {
-        arcusFrontCache.set(arcusKey, value, frontExpireSeconds);
-      }
-
-      return success ? null : toValueWrapper(getValue(arcusKey));
+      return putIfAbsentValue(arcusKey, toStoreValue(value));
     } catch (Exception e) {
       if (wantToGetException) {
         throw toRuntimeException(e);
       }
       logger.info("failed to putIfAbsent. error: {}, key: {}", e.getMessage(), arcusKey);
-      return toValueWrapper(lookup(key));
+      return super.get(key);
     }
   }
 
@@ -494,11 +485,6 @@ private Object getValue(String arcusKey) throws Exception {
   private void putValue(String arcusKey, Object value) throws Exception {
     logger.debug("trying to put key: {}", arcusKey);
 
-    if (value == null) {
-      logger.info("arcus cannot put NULL value. key: {}", arcusKey);
-      return;
-    }
-
     boolean success = false;
 
     try {
@@ -521,4 +507,25 @@ private void putValue(String arcusKey, Object value) throws Exception {
     }
   }
 
+  private ValueWrapper putIfAbsentValue(String arcusKey, Object value) throws Exception {
+    logger.debug("trying to add(putIfAbsent) key: {}", arcusKey);
+
+    OperationFuture<Boolean> future;
+    if (operationTranscoder != null) {
+      future = arcusClient.add(arcusKey, expireSeconds, value, operationTranscoder);
+    } else {
+      future = arcusClient.add(arcusKey, expireSeconds, value);
+    }
+
+    boolean success = future.get(timeoutMilliSeconds, TimeUnit.MILLISECONDS);
+    if (!success) {
+      OperationStatus status = future.getStatus();
+      logger.info("failed to putIfAbsent a key: {}, status: {}", arcusKey, status.getMessage());
+    } else if (arcusFrontCache != null) {
+      arcusFrontCache.set(arcusKey, value, frontExpireSeconds);
+    }
+
+    return success ? null : toValueWrapper(getValue(arcusKey));
+  }
+
 }
diff --git a/src/test/java/com/navercorp/arcus/spring/cache/ArcusCacheIntegrationTest.java b/src/test/java/com/navercorp/arcus/spring/cache/ArcusCacheIntegrationTest.java
@@ -42,6 +42,9 @@ public class ArcusCacheIntegrationTest {
   @Autowired
   private ArcusCache arcusCache;
 
+  @Autowired
+  private ArcusCache arcusCacheWithoutAllowingNullValue;
+
   @After
   public void tearDown() {
     arcusCache.evict(TEST_KEY);
@@ -152,11 +155,18 @@ public void testExternalizableAndSerializable() {
   }
 
   @Test
-  public void putTheNullValue() {
+  public void putTheNullValueIfAllowNullValuesIsTrue() {
+
     arcusCache.put(TEST_KEY, null);
 
-    Object result = arcusCache.get(TEST_KEY);
-    assertNull(result);
+    Cache.ValueWrapper result = arcusCache.get(TEST_KEY);
+    assertNotNull(result);
+    assertNull(result.get());
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void putTheNullValueIfAllowNullValuesIsFalse() {
+    arcusCacheWithoutAllowingNullValue.put(TEST_KEY, null);
   }
 
   @Test

diff --git a/src/test/java/com/navercorp/arcus/spring/cache/ArcusCacheTest.java b/src/test/java/com/navercorp/arcus/spring/cache/ArcusCacheTest.java
@@ -1300,10 +1300,8 @@ public void testPutIfAbsent_FrontCache_Null() {
     arcusCache.setExpireSeconds(EXPIRE_SECONDS);
     arcusCache.setFrontExpireSeconds(FRONT_EXPIRE_SECONDS);
     arcusCache.setWantToGetException(true);
-    when(arcusClientPool.add(arcusKey, EXPIRE_SECONDS, VALUE))
-        .thenReturn(createOperationFuture(true));
-    when(arcusClientPool.asyncGet(arcusKey))
-        .thenReturn(createGetFuture(VALUE));
+    when(arcusClientPool.add(arcusKey, EXPIRE_SECONDS, NullValue.INSTANCE))
+            .thenReturn(createOperationFuture(true));
 
     // when
     try {
@@ -1313,12 +1311,12 @@ public void testPutIfAbsent_FrontCache_Null() {
     }
 
     // then
-    verify(arcusClientPool, never())
-        .add(arcusKey, EXPIRE_SECONDS, VALUE);
     verify(arcusClientPool, times(1))
-        .asyncGet(arcusKey);
+            .add(arcusKey, EXPIRE_SECONDS, NullValue.INSTANCE);
     verify(arcusFrontCache, times(1))
-        .set(arcusKey, VALUE, FRONT_EXPIRE_SECONDS);
+            .set(arcusKey, NullValue.INSTANCE, FRONT_EXPIRE_SECONDS);
+    verify(arcusClientPool, never())
+            .asyncGet(arcusKey);
     assertNull(exception);
   }
 

diff --git a/src/test/resources/arcus_spring_arcusCache_test.xml b/src/test/resources/arcus_spring_arcusCache_test.xml
@@ -21,4 +21,17 @@
           p:expireSeconds="3000"
           p:serviceId="testService-"
           p:prefix="testPrefix"/>
+
+    <bean id="arcusCacheConfiguration" class="com.navercorp.arcus.spring.cache.ArcusCacheConfiguration"
+          p:allowNullValues="false"
+          p:timeoutMilliSeconds="500"
+          p:expireSeconds="3000"
+          p:serviceId="testService-"
+          p:prefix="testService-"/>
+
+    <bean id="arcusCacheWithoutAllowingNullValue" class="com.navercorp.arcus.spring.cache.ArcusCache">
+        <constructor-arg name="name" value="arcusCache"/>
+        <constructor-arg name="clientPool" ref="arcusClient"/>
+        <constructor-arg name="configuration" ref="arcusCacheConfiguration"/>
+    </bean>
 </beans>