Adding Cache class to CacheConfig (#3942)

* adding cacheclass to cacheconfig * - add cachefactory test * - revert connection ctors to public - udpate some tests with UnifiedJedis.getCache - add ping to flaky tests * remove unnecessary anonymous types * change ctor access modifiers * fix test name * make cachefactory methods static * removing pings due to still flaky with inv messages * - drop CustomCache in tests and use TestCache - check null cacheable issue with defaultcache - support both ctors in custom cache classes regarding to value of cacheconfig.cacheable * remove unncessary maxsize * - remove inline anonymious
redis · Aug 29, 2024 · ce210ea · ce210ea
1 parent 1d213bb
commit ce210ea
Show file tree

Hide file tree

Showing 15 changed files with 233 additions and 184 deletions.
diff --git a/src/main/java/redis/clients/jedis/JedisCluster.java b/src/main/java/redis/clients/jedis/JedisCluster.java
@@ -12,7 +12,7 @@
 import redis.clients.jedis.providers.ClusterConnectionProvider;
 import redis.clients.jedis.csc.Cache;
 import redis.clients.jedis.csc.CacheConfig;
-import redis.clients.jedis.csc.CacheProvider;
+import redis.clients.jedis.csc.CacheFactory;
 import redis.clients.jedis.util.JedisClusterCRC16;
 
 public class JedisCluster extends UnifiedJedis {
@@ -223,7 +223,7 @@ private JedisCluster(ClusterConnectionProvider provider, int maxAttempts, Durati
 
   @Experimental
   public JedisCluster(Set<HostAndPort> hnp, JedisClientConfig jedisClientConfig, CacheConfig cacheConfig) {
-    this(hnp, jedisClientConfig, new CacheProvider().getCache(cacheConfig));
+    this(hnp, jedisClientConfig, CacheFactory.getCache(cacheConfig));
   }
 
   @Experimental

diff --git a/src/main/java/redis/clients/jedis/JedisPooled.java b/src/main/java/redis/clients/jedis/JedisPooled.java
@@ -10,7 +10,7 @@
 import redis.clients.jedis.annots.Experimental;
 import redis.clients.jedis.csc.Cache;
 import redis.clients.jedis.csc.CacheConfig;
-import redis.clients.jedis.csc.CacheProvider;
+import redis.clients.jedis.csc.CacheFactory;
 import redis.clients.jedis.providers.PooledConnectionProvider;
 import redis.clients.jedis.util.JedisURIHelper;
 import redis.clients.jedis.util.Pool;
@@ -81,7 +81,7 @@ public JedisPooled(final HostAndPort hostAndPort, final JedisClientConfig client
 
   @Experimental
   public JedisPooled(final HostAndPort hostAndPort, final JedisClientConfig clientConfig, CacheConfig cacheConfig) {
-    this(hostAndPort, clientConfig, new CacheProvider().getCache(cacheConfig));
+    this(hostAndPort, clientConfig, CacheFactory.getCache(cacheConfig));
   }
 
   @Experimental
@@ -392,7 +392,7 @@ public JedisPooled(final HostAndPort hostAndPort, final JedisClientConfig client
   @Experimental
   public JedisPooled(final HostAndPort hostAndPort, final JedisClientConfig clientConfig, CacheConfig cacheConfig,
       final GenericObjectPoolConfig<Connection> poolConfig) {
-    this(hostAndPort, clientConfig, new CacheProvider().getCache(cacheConfig), poolConfig);
+    this(hostAndPort, clientConfig, CacheFactory.getCache(cacheConfig), poolConfig);
   }
 
   @Experimental

diff --git a/src/main/java/redis/clients/jedis/JedisSentineled.java b/src/main/java/redis/clients/jedis/JedisSentineled.java
@@ -5,7 +5,7 @@
 import redis.clients.jedis.annots.Experimental;
 import redis.clients.jedis.csc.Cache;
 import redis.clients.jedis.csc.CacheConfig;
-import redis.clients.jedis.csc.CacheProvider;
+import redis.clients.jedis.csc.CacheFactory;
 import redis.clients.jedis.providers.SentineledConnectionProvider;
 
 public class JedisSentineled extends UnifiedJedis {
@@ -19,7 +19,7 @@ public JedisSentineled(String masterName, final JedisClientConfig masterClientCo
   @Experimental
   public JedisSentineled(String masterName, final JedisClientConfig masterClientConfig, CacheConfig cacheConfig,
       Set<HostAndPort> sentinels, final JedisClientConfig sentinelClientConfig) {
-    this(masterName, masterClientConfig, new CacheProvider().getCache(cacheConfig),
+    this(masterName, masterClientConfig, CacheFactory.getCache(cacheConfig),
         sentinels, sentinelClientConfig);
   }
 

diff --git a/src/main/java/redis/clients/jedis/UnifiedJedis.java b/src/main/java/redis/clients/jedis/UnifiedJedis.java
@@ -22,7 +22,7 @@
 import redis.clients.jedis.csc.Cache;
 import redis.clients.jedis.csc.CacheConfig;
 import redis.clients.jedis.csc.CacheConnection;
-import redis.clients.jedis.csc.CacheProvider;
+import redis.clients.jedis.csc.CacheFactory;
 import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.executors.*;
 import redis.clients.jedis.gears.TFunctionListParams;
@@ -100,7 +100,7 @@ public UnifiedJedis(HostAndPort hostAndPort, JedisClientConfig clientConfig) {
 
   @Experimental
   public UnifiedJedis(HostAndPort hostAndPort, JedisClientConfig clientConfig, CacheConfig cacheConfig) {
-    this(hostAndPort, clientConfig, new CacheProvider().getCache(cacheConfig));
+    this(hostAndPort, clientConfig, CacheFactory.getCache(cacheConfig));
   }
 
   @Experimental

diff --git a/src/main/java/redis/clients/jedis/csc/CacheConfig.java b/src/main/java/redis/clients/jedis/csc/CacheConfig.java
@@ -5,6 +5,7 @@ public class CacheConfig {
     private int maxSize;
     private Cacheable cacheable;
     private EvictionPolicy evictionPolicy;
+    private Class cacheClass;
 
     public int getMaxSize() {
         return maxSize;
@@ -18,14 +19,19 @@ public EvictionPolicy getEvictionPolicy() {
         return evictionPolicy;
     }
 
+    public Class getCacheClass() {
+        return cacheClass;
+    }
     public static Builder builder() {
         return new Builder();
     }
 
     public static class Builder {
-        private int maxSize;
+        private final int DEFAULT_MAX_SIZE = 10000;
+        private int maxSize = DEFAULT_MAX_SIZE;
         private Cacheable cacheable = DefaultCacheable.INSTANCE;
         private EvictionPolicy evictionPolicy;
+        private Class cacheClass;
 
         public Builder maxSize(int maxSize) {
             this.maxSize = maxSize;
@@ -42,11 +48,17 @@ public Builder cacheable(Cacheable cacheable) {
             return this;
         }
 
+        public Builder cacheClass(Class cacheClass) {
+            this.cacheClass = cacheClass;
+            return this;
+        }
+
         public CacheConfig build() {
             CacheConfig cacheConfig = new CacheConfig();
             cacheConfig.maxSize = this.maxSize;
             cacheConfig.cacheable = this.cacheable;
             cacheConfig.evictionPolicy = this.evictionPolicy;
+            cacheConfig.cacheClass = this.cacheClass;
             return cacheConfig;
         }
     }

diff --git a/src/main/java/redis/clients/jedis/csc/CacheFactory.java b/src/main/java/redis/clients/jedis/csc/CacheFactory.java
@@ -0,0 +1,63 @@
+package redis.clients.jedis.csc;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
+
+import redis.clients.jedis.exceptions.JedisCacheException;
+
+public final class CacheFactory {
+
+    public static Cache getCache(CacheConfig config) {
+        if (config.getCacheClass() == null) {
+            if (config.getCacheable() == null) {
+                throw new JedisCacheException("Cacheable is required to create the default cache!");
+            }
+            return new DefaultCache(config.getMaxSize(), config.getCacheable(), getEvictionPolicy(config));
+        }
+        return instantiateCustomCache(config);
+    }
+
+    private static Cache instantiateCustomCache(CacheConfig config) {
+        try {
+            if (config.getCacheable() != null) {
+                Constructor ctorWithCacheable = findConstructorWithCacheable(config.getCacheClass());
+                if (ctorWithCacheable != null) {
+                    return (Cache) ctorWithCacheable.newInstance(config.getMaxSize(), getEvictionPolicy(config), config.getCacheable());
+                }
+            }
+            Constructor ctor = getConstructor(config.getCacheClass());
+            return (Cache) ctor.newInstance(config.getMaxSize(), getEvictionPolicy(config));
+        } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException
+                | SecurityException e) {
+            throw new JedisCacheException("Failed to insantiate custom cache type!", e);
+        }
+    }
+
+    private static Constructor findConstructorWithCacheable(Class customCacheType) {
+        return Arrays.stream(customCacheType.getConstructors())
+                .filter(ctor -> Arrays.equals(ctor.getParameterTypes(), new Class[] { int.class, EvictionPolicy.class, Cacheable.class }))
+                .findFirst().orElse(null);
+    }
+
+    private static Constructor getConstructor(Class customCacheType) {
+        try {
+            return customCacheType.getConstructor(int.class, EvictionPolicy.class);
+        } catch (NoSuchMethodException e) {
+            String className = customCacheType.getName();
+            throw new JedisCacheException(String.format(
+                "Failed to find compatible constructor for custom cache type!  Provide one of these;"
+                        // give hints about the compatible constructors
+                        + "\n - %s(int maxSize, EvictionPolicy evictionPolicy)\n - %s(int maxSize, EvictionPolicy evictionPolicy, Cacheable cacheable)",
+                className, className), e);
+        }
+    }
+
+    private static EvictionPolicy getEvictionPolicy(CacheConfig config) {
+        if (config.getEvictionPolicy() == null) {
+            // It will be default to LRUEviction, until we have other eviction implementations
+            return new LRUEviction(config.getMaxSize());
+        }
+        return config.getEvictionPolicy();
+    }
+}
diff --git a/src/main/java/redis/clients/jedis/csc/CacheProvider.java b/src/main/java/redis/clients/jedis/csc/CacheProvider.java
diff --git a/src/main/java/redis/clients/jedis/csc/DefaultCache.java b/src/main/java/redis/clients/jedis/csc/DefaultCache.java
@@ -21,6 +21,10 @@ protected DefaultCache(int maximumSize, Cacheable cacheable) {
         this(maximumSize, new HashMap<CacheKey, CacheEntry>(), cacheable, new LRUEviction(maximumSize));
     }
 
+    protected DefaultCache(int maximumSize, Cacheable cacheable, EvictionPolicy evictionPolicy) {
+        this(maximumSize, new HashMap<CacheKey, CacheEntry>(), cacheable, evictionPolicy);
+    }
+
     protected DefaultCache(int maximumSize, Map<CacheKey, CacheEntry> map, Cacheable cacheable, EvictionPolicy evictionPolicy) {
         super(maximumSize, cacheable);
         this.cache = map;

diff --git a/src/test/java/redis/clients/jedis/csc/AllowAndDenyListCacheableTest.java b/src/test/java/redis/clients/jedis/csc/AllowAndDenyListCacheableTest.java
@@ -1,12 +1,8 @@
 package redis.clients.jedis.csc;
 
 import static java.util.Collections.singleton;
-import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 
-import java.util.HashMap;
-import java.util.Map;
-import org.hamcrest.Matchers;
 import org.junit.Test;
 
 import redis.clients.jedis.JedisPooled;
@@ -15,73 +11,69 @@
 
 public class AllowAndDenyListCacheableTest extends ClientSideCacheTestBase {
 
-  private static Cache createTestCache(Map<CacheKey, CacheEntry> map, Cacheable cacheable) {
-    Cache mapCache = new TestCache(map, cacheable);
-    return mapCache;
+  private static CacheConfig createConfig(Cacheable cacheable) {
+    return CacheConfig.builder().cacheable(cacheable).cacheClass(TestCache.class).build();
   }
 
   @Test
   public void none() {
-    HashMap<CacheKey, CacheEntry> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        createTestCache(map, new AllowAndDenyListWithStringKeys(null, null, null, null)),
-        singleConnectionPoolConfig.get())) {
+        createConfig(new AllowAndDenyListWithStringKeys(null, null, null, null)), singleConnectionPoolConfig.get())) {
+      Cache cache = jedis.getCache();
       control.set("foo", "bar");
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertEquals(0, cache.getSize());
       assertEquals("bar", jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(1));
+      assertEquals(1, cache.getSize());
     }
   }
 
   @Test
   public void whiteListCommand() {
-    HashMap<CacheKey, CacheEntry> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        createTestCache(map, new AllowAndDenyListWithStringKeys(singleton(Protocol.Command.GET), null, null, null)),
+        createConfig(new AllowAndDenyListWithStringKeys(singleton(Protocol.Command.GET), null, null, null)),
         singleConnectionPoolConfig.get())) {
+      Cache cache = jedis.getCache();
       control.set("foo", "bar");
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertEquals(0, cache.getSize());
       assertEquals("bar", jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(1));
+      assertEquals(1, cache.getSize());
     }
   }
 
   @Test
   public void blackListCommand() {
-    HashMap<CacheKey, CacheEntry> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        createTestCache(map, new AllowAndDenyListWithStringKeys(null, singleton(Protocol.Command.GET), null, null)),
+        createConfig(new AllowAndDenyListWithStringKeys(null, singleton(Protocol.Command.GET), null, null)),
         singleConnectionPoolConfig.get())) {
+      Cache cache = jedis.getCache();
       control.set("foo", "bar");
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertEquals(0, cache.getSize());
       assertEquals("bar", jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertEquals(0, cache.getSize());
     }
   }
 
   @Test
   public void whiteListKey() {
-    HashMap<CacheKey, CacheEntry> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        createTestCache(map, new AllowAndDenyListWithStringKeys(null, null, singleton("foo"), null)),
-        singleConnectionPoolConfig.get())) {
+        createConfig(new AllowAndDenyListWithStringKeys(null, null, singleton("foo"), null)), singleConnectionPoolConfig.get())) {
       control.set("foo", "bar");
-      assertThat(map, Matchers.aMapWithSize(0));
+      Cache cache = jedis.getCache();
+      assertEquals(0, cache.getSize());
       assertEquals("bar", jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(1));
+      assertEquals(1, cache.getSize());
     }
   }
 
   @Test
   public void blackListKey() {
-    HashMap<CacheKey, CacheEntry> map = new HashMap<>();
     try (JedisPooled jedis = new JedisPooled(hnp, clientConfig.get(),
-        createTestCache(map, new AllowAndDenyListWithStringKeys(null, null, null, singleton("foo"))),
-        singleConnectionPoolConfig.get())) {
+        createConfig(new AllowAndDenyListWithStringKeys(null, null, null, singleton("foo"))), singleConnectionPoolConfig.get())) {
+      Cache cache = jedis.getCache();
       control.set("foo", "bar");
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertEquals(0, cache.getSize());
       assertEquals("bar", jedis.get("foo"));
-      assertThat(map, Matchers.aMapWithSize(0));
+      assertEquals(0, cache.getSize());
     }
   }
 }