Support caching null values (#3939)

* caching null results * add more assertion
redis · Aug 28, 2024 · e96e2e3 · e96e2e3
1 parent 747e391
commit e96e2e3
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 11 deletions.
diff --git a/src/main/java/redis/clients/jedis/csc/CacheConnection.java b/src/main/java/redis/clients/jedis/csc/CacheConnection.java
@@ -81,12 +81,10 @@ public <T> T executeCommand(final CommandObject<T> commandObject) {
     // CACHE MISS !!
     clientSideCache.getStats().miss();
     T value = super.executeCommand(commandObject);
-    if (value != null) {
-      cacheEntry = new CacheEntry<>(cacheKey, value, this);
-      clientSideCache.set(cacheKey, cacheEntry);
-      // this line actually provides a deep copy of cached object instance 
-      value = cacheEntry.getValue();
-    }
+    cacheEntry = new CacheEntry<>(cacheKey, value, this);
+    clientSideCache.set(cacheKey, cacheEntry);
+    // this line actually provides a deep copy of cached object instance 
+    value = cacheEntry.getValue();
     return value;
   }
 

diff --git a/src/test/java/redis/clients/jedis/csc/ClientSideCacheFunctionalityTest.java b/src/test/java/redis/clients/jedis/csc/ClientSideCacheFunctionalityTest.java
@@ -5,6 +5,7 @@
 import static org.hamcrest.Matchers.hasSize;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
@@ -510,4 +511,38 @@ public void run() {
     }
   }
 
+  @Test
+  public void testNullValue() throws InterruptedException {
+    int MAX_SIZE = 20;
+    String nonExisting = "non-existing-key";
+    control.del(nonExisting);
+
+    TestCache cache = new TestCache(MAX_SIZE, new HashMap<>(), DefaultCacheable.INSTANCE);
+
+    try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(), cache)) {
+      CacheStats stats = cache.getStats();
+
+      String val = jedis.get(nonExisting);
+      assertNull(val);
+      assertEquals(1, cache.getSize());
+      assertEquals(0, stats.getHitCount());
+      assertEquals(1, stats.getMissCount());
+
+      val = jedis.get(nonExisting);
+      assertNull(val);
+      assertEquals(1, cache.getSize());
+      assertNull(cache.getCacheEntries().iterator().next().getValue());
+      assertEquals(1, stats.getHitCount());
+      assertEquals(1, stats.getMissCount());
+
+      control.set(nonExisting, "bar");
+      val = jedis.get(nonExisting);
+      assertEquals("bar", val);
+      assertEquals(1, cache.getSize());
+      assertEquals("bar", cache.getCacheEntries().iterator().next().getValue());
+      assertEquals(1, stats.getHitCount());
+      assertEquals(2, stats.getMissCount());
+    }
+  }
+
 }
diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java
@@ -56,10 +56,9 @@ public void simpleWithSimpleMap() {
       control.del("foo");
       assertThat(map, Matchers.aMapWithSize(1));
       assertNull(jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(0));
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertThat(map, Matchers.aMapWithSize(1));
       assertNull(jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertThat(map, Matchers.aMapWithSize(1));
     }
   }
 
@@ -84,9 +83,9 @@ public void flushAllWithSimpleMap() {
       control.flushAll();
       assertThat(map, Matchers.aMapWithSize(1));
       assertNull(jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertThat(map, Matchers.aMapWithSize(1));
       assertNull(jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertThat(map, Matchers.aMapWithSize(1));
     }
   }