Inject ClientSideCacheable via set method (#3882)

Author: sazzad16

Diff for: src/main/java/redis/clients/jedis/csc/CaffeineClientSideCache.java

@@ -16,15 +16,7 @@ public CaffeineClientSideCache(Cache<Long, Object> caffeineCache) {
   }
 
   public CaffeineClientSideCache(Cache<Long, Object> caffeineCache, CommandLongHasher commandHasher) {
-    this(caffeineCache, commandHasher, DefaultClientSideCacheable.INSTANCE);
-  }
-
-  public CaffeineClientSideCache(Cache<Long, Object> caffeineCache, ClientSideCacheable cacheable) {
-    this(caffeineCache, SimpleCommandHasher.INSTANCE, cacheable);
-  }
-
-  public CaffeineClientSideCache(Cache<Long, Object> caffeineCache, CommandLongHasher commandHasher, ClientSideCacheable cacheable) {
-    super(commandHasher, cacheable);
+    super(commandHasher);
     this.cache = caffeineCache;
   }
 
@@ -61,8 +53,6 @@ public static class Builder {
     // not using a default value to avoid an object creation like 'new OpenHftHashing(hashFunction)'
     private CommandLongHasher commandHasher = SimpleCommandHasher.INSTANCE;
 
-    private ClientSideCacheable cacheable = DefaultClientSideCacheable.INSTANCE;
-
     private Builder() { }
 
     public Builder maximumSize(int size) {
@@ -80,19 +70,14 @@ public Builder commandHasher(CommandLongHasher commandHasher) {
       return this;
     }
 
-    public Builder cacheable(ClientSideCacheable cacheable) {
-      this.cacheable = cacheable;
-      return this;
-    }
-
     public CaffeineClientSideCache build() {
       Caffeine cb = Caffeine.newBuilder();
 
       cb.maximumSize(maximumSize);
 
       cb.expireAfterWrite(expireTime, expireTimeUnit);
 
-      return new CaffeineClientSideCache(cb.build(), commandHasher, cacheable);
+      return new CaffeineClientSideCache(cb.build(), commandHasher);
     }
   }
 }

Diff for: src/main/java/redis/clients/jedis/csc/ClientSideCache.java

@@ -4,6 +4,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
@@ -26,15 +27,14 @@ public abstract class ClientSideCache {
 
   private final Map<ByteBuffer, Set<Long>> keyToCommandHashes = new ConcurrentHashMap<>();
   private final CommandLongHasher commandHasher;
-  private final ClientSideCacheable cacheable;
+  private ClientSideCacheable cacheable = DefaultClientSideCacheable.INSTANCE; // TODO: volatile
 
   protected ClientSideCache(CommandLongHasher commandHasher) {
-    this(commandHasher, DefaultClientSideCacheable.INSTANCE);
+    this.commandHasher = commandHasher;
   }
 
-  protected ClientSideCache(CommandLongHasher commandHasher, ClientSideCacheable cacheable) {
-    this.commandHasher = commandHasher;
-    this.cacheable = cacheable;
+  public void setCacheable(ClientSideCacheable cacheable) {
+    this.cacheable = Objects.requireNonNull(cacheable, "'cacheable' must not be null");
   }
 
   protected abstract void invalidateAllHashes();

Diff for: src/main/java/redis/clients/jedis/csc/GuavaClientSideCache.java

@@ -25,15 +25,6 @@ public GuavaClientSideCache(Cache<Long, Object> guavaCache, CommandLongHasher co
     this.cache = guavaCache;
   }
 
-  public GuavaClientSideCache(Cache<Long, Object> guavaCache, ClientSideCacheable cacheable) {
-    this(guavaCache, new GuavaCommandHasher(GuavaCommandHasher.DEFAULT_HASH_FUNCTION), cacheable);
-  }
-
-  public GuavaClientSideCache(Cache<Long, Object> cache, CommandLongHasher commandHasher, ClientSideCacheable cacheable) {
-    super(commandHasher, cacheable);
-    this.cache = cache;
-  }
-
   @Override
   protected final void invalidateAllHashes() {
     cache.invalidateAll();
@@ -68,8 +59,6 @@ public static class Builder {
     private HashFunction hashFunction = null;
     private CommandLongHasher commandHasher = null;
 
-    private ClientSideCacheable cacheable = DefaultClientSideCacheable.INSTANCE;
-
     private Builder() { }
 
     public Builder maximumSize(int size) {
@@ -94,21 +83,16 @@ public Builder commandHasher(CommandLongHasher commandHasher) {
       return this;
     }
 
-    public Builder cacheable(ClientSideCacheable cacheable) {
-      this.cacheable = cacheable;
-      return this;
-    }
-
     public GuavaClientSideCache build() {
       CacheBuilder cb = CacheBuilder.newBuilder();
 
       cb.maximumSize(maximumSize);
 
       cb.expireAfterWrite(expireTime, expireTimeUnit);
 
-      return hashFunction != null ? new GuavaClientSideCache(cb.build(), new GuavaCommandHasher(hashFunction), cacheable)
-          : commandHasher != null ? new GuavaClientSideCache(cb.build(), commandHasher, cacheable)
-              : new GuavaClientSideCache(cb.build(), cacheable);
+      return hashFunction != null ? new GuavaClientSideCache(cb.build(), new GuavaCommandHasher(hashFunction))
+          : commandHasher != null ? new GuavaClientSideCache(cb.build(), commandHasher)
+              : new GuavaClientSideCache(cb.build());
     }
   }
 }

Diff for: src/test/java/redis/clients/jedis/csc/AllowAndDenyListClientSideCacheTest.java

@@ -5,6 +5,7 @@
 import static org.junit.Assert.assertEquals;
 
 import java.util.HashMap;
+import java.util.Map;
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
@@ -14,11 +15,17 @@
 
 public class AllowAndDenyListClientSideCacheTest extends ClientSideCacheTestBase {
 
+  private static MapClientSideCache createMapClientSideCache(Map<Long, Object> map, ClientSideCacheable cacheable) {
+    MapClientSideCache mapCache = new MapClientSideCache(map);
+    mapCache.setCacheable(cacheable);
+    return mapCache;
+  }
+
   @Test
   public void none() {
     HashMap<Long, Object> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        new MapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, null, null, null)),
+        createMapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, null, null, null)),
         singleConnectionPoolConfig.get())) {
       control.set("foo", "bar");
       assertThat(map, Matchers.aMapWithSize(0));
@@ -31,7 +38,7 @@ public void none() {
   public void whiteListCommand() {
     HashMap<Long, Object> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        new MapClientSideCache(map, new AllowAndDenyListWithStringKeys(singleton(Protocol.Command.GET), null, null, null)),
+        createMapClientSideCache(map, new AllowAndDenyListWithStringKeys(singleton(Protocol.Command.GET), null, null, null)),
         singleConnectionPoolConfig.get())) {
       control.set("foo", "bar");
       assertThat(map, Matchers.aMapWithSize(0));
@@ -44,7 +51,7 @@ public void whiteListCommand() {
   public void blackListCommand() {
     HashMap<Long, Object> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        new MapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, singleton(Protocol.Command.GET), null, null)),
+        createMapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, singleton(Protocol.Command.GET), null, null)),
         singleConnectionPoolConfig.get())) {
       control.set("foo", "bar");
       assertThat(map, Matchers.aMapWithSize(0));
@@ -57,7 +64,7 @@ public void blackListCommand() {
   public void whiteListKey() {
     HashMap<Long, Object> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        new MapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, null, singleton("foo"), null)),
+        createMapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, null, singleton("foo"), null)),
         singleConnectionPoolConfig.get())) {
       control.set("foo", "bar");
       assertThat(map, Matchers.aMapWithSize(0));
@@ -70,7 +77,7 @@ public void whiteListKey() {
   public void blackListKey() {
     HashMap<Long, Object> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        new MapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, null, null, singleton("foo"))),
+        createMapClientSideCache(map, new AllowAndDenyListWithStringKeys(null, null, null, singleton("foo"))),
         singleConnectionPoolConfig.get())) {
       control.set("foo", "bar");
       assertThat(map, Matchers.aMapWithSize(0));

Diff for: src/test/java/redis/clients/jedis/csc/MapClientSideCache.java

@@ -18,11 +18,6 @@ public MapClientSideCache(Map<Long, Object> map) {
     this.cache = map;
   }
 
-  public MapClientSideCache(Map<Long, Object> cache, ClientSideCacheable cacheable) {
-    super(SimpleCommandHasher.INSTANCE, cacheable);
-    this.cache = cache;
-  }
-
   @Override
   protected final void invalidateAllHashes() {
     cache.clear();