Deprecate JedisPoolAbstract (#2361)

* Remove JedisPoolAbstract and related changes JedisPoolAbstract itself was doing nothing except acting as access point of Pool<Jedis> from redis.clients.jedis package. Using Pool<Jedis> directly seems better. * backward compatibility by deprecating JedisPoolAbstract * backward compatibility and deprecated * added logs and removed unnecessary repeated codes * Update src/main/java/redis/clients/jedis/JedisPool.java
redis · Feb 7, 2021 · aa0158c · aa0158c
1 parent 6b49fa4
commit aa0158c
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 61 deletions.
diff --git a/src/main/java/redis/clients/jedis/Jedis.java b/src/main/java/redis/clients/jedis/Jedis.java
@@ -35,6 +35,10 @@
 public class Jedis extends BinaryJedis implements JedisCommands, MultiKeyCommands,
     AdvancedJedisCommands, ScriptingCommands, BasicCommands, ClusterCommands, SentinelCommands, ModuleCommands {
 
+  /**
+   * @deprecated This will be private in future.
+   */
+  @Deprecated
   protected JedisPoolAbstract dataSource = null;
 
   public Jedis() {

diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java
@@ -1,19 +1,20 @@
 package redis.clients.jedis;
 
 import java.net.URI;
-
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSocketFactory;
 
-import org.apache.commons.pool2.impl.GenericObjectPool;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.util.JedisURIHelper;
 
 public class JedisPool extends JedisPoolAbstract {
 
+  private static final Logger log = LoggerFactory.getLogger(JedisPool.class);
+
   public JedisPool() {
     this(Protocol.DEFAULT_HOST, Protocol.DEFAULT_PORT);
   }
@@ -29,26 +30,24 @@ public JedisPool(String host, int port) {
   public JedisPool(final String host) {
     URI uri = URI.create(host);
     if (JedisURIHelper.isValid(uri)) {
-      this.internalPool = new GenericObjectPool<>(new JedisFactory(uri,
-          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null), new GenericObjectPoolConfig());
+      initPool(new GenericObjectPoolConfig(), new JedisFactory(uri, Protocol.DEFAULT_TIMEOUT,
+          Protocol.DEFAULT_TIMEOUT, null));
     } else {
-      this.internalPool = new GenericObjectPool<>(new JedisFactory(host,
-          Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null,
-          Protocol.DEFAULT_DATABASE, null), new GenericObjectPoolConfig());
+      initPool(new GenericObjectPoolConfig(), new JedisFactory(host, Protocol.DEFAULT_PORT,
+          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null));
     }
   }
 
   public JedisPool(final String host, final SSLSocketFactory sslSocketFactory,
       final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
     URI uri = URI.create(host);
     if (JedisURIHelper.isValid(uri)) {
-      this.internalPool = new GenericObjectPool<>(new JedisFactory(uri,
-          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, sslSocketFactory, sslParameters,
-          hostnameVerifier), new GenericObjectPoolConfig());
+      initPool(new GenericObjectPoolConfig(), new JedisFactory(uri, Protocol.DEFAULT_TIMEOUT,
+          Protocol.DEFAULT_TIMEOUT, null, sslSocketFactory, sslParameters, hostnameVerifier));
     } else {
-      this.internalPool = new GenericObjectPool<>(new JedisFactory(host,
-          Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null,
-          Protocol.DEFAULT_DATABASE, null, false, null, null, null), new GenericObjectPoolConfig());
+      initPool(new GenericObjectPoolConfig(), new JedisFactory(host, Protocol.DEFAULT_PORT,
+          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null,
+          false, null, null, null));
     }
   }
 
@@ -333,21 +332,14 @@ public Jedis getResource() {
   }
 
   @Override
-  protected void returnBrokenResource(final Jedis resource) {
-    if (resource != null) {
-      returnBrokenResourceObject(resource);
-    }
-  }
-
-  @Override
-  protected void returnResource(final Jedis resource) {
+  public void returnResource(final Jedis resource) {
     if (resource != null) {
       try {
         resource.resetState();
         returnResourceObject(resource);
       } catch (Exception e) {
         returnBrokenResource(resource);
-        throw new JedisException("Resource is returned to the pool as broken", e);
+        log.warn("Resource is returned to the pool as broken", e);
       }
     }
   }

diff --git a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java
@@ -5,6 +5,11 @@
 
 import redis.clients.jedis.util.Pool;
 
+/**
+ * @deprecated This class will be removed in future. If you are directly manipulating this class,
+ * you are suggested to change your code to use {@link Pool Pool&lt;Jedis&gt;} instead.
+ */
+@Deprecated
 public class JedisPoolAbstract extends Pool<Jedis> {
 
   public JedisPoolAbstract() {
@@ -14,14 +19,4 @@ public JedisPoolAbstract() {
   public JedisPoolAbstract(GenericObjectPoolConfig poolConfig, PooledObjectFactory<Jedis> factory) {
     super(poolConfig, factory);
   }
-
-  @Override
-  protected void returnBrokenResource(Jedis resource) {
-    super.returnBrokenResource(resource);
-  }
-
-  @Override
-  protected void returnResource(Jedis resource) {
-    super.returnResource(resource);
-  }
 }
diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java
@@ -14,7 +14,12 @@
 import redis.clients.jedis.exceptions.JedisException;
 
 public class JedisSentinelPool extends JedisPoolAbstract {
-  protected Logger log = LoggerFactory.getLogger(getClass().getName());
+
+  /**
+   * @deprecated This will be private in future.
+   */
+  @Deprecated
+  protected static Logger log = LoggerFactory.getLogger(JedisSentinelPool.class);
 
   protected final GenericObjectPoolConfig poolConfig;
 
@@ -197,11 +202,9 @@ private void initPool(HostAndPort master) {
           initPool(poolConfig, factory);
         } else {
           factory.setHostAndPort(currentHostMaster);
-          // although we clear the pool, we still have to check the
-          // returned object
-          // in getResource, this call only clears idle instances, not
-          // borrowed instances
-          internalPool.clear();
+          // although we clear the pool, we still have to check the returned object in getResource,
+          // this call only clears idle instances, not borrowed instances
+          clearInternalPool();
         }
 
         log.info("Created JedisPool to master at {}", master);
@@ -312,21 +315,14 @@ public Jedis getResource() {
   }
 
   @Override
-  protected void returnBrokenResource(final Jedis resource) {
-    if (resource != null) {
-      returnBrokenResourceObject(resource);
-    }
-  }
-
-  @Override
-  protected void returnResource(final Jedis resource) {
+  public void returnResource(final Jedis resource) {
     if (resource != null) {
       try {
         resource.resetState();
         returnResourceObject(resource);
       } catch (Exception e) {
         returnBrokenResource(resource);
-        throw new JedisException("Resource is returned to the pool as broken", e);
+        log.debug("Resource is returned to the pool as broken", e);
       }
     }
   }

diff --git a/src/main/java/redis/clients/jedis/ShardedJedisPool.java b/src/main/java/redis/clients/jedis/ShardedJedisPool.java
@@ -39,14 +39,7 @@ public ShardedJedis getResource() {
   }
 
   @Override
-  protected void returnBrokenResource(final ShardedJedis resource) {
-    if (resource != null) {
-      returnBrokenResourceObject(resource);
-    }
-  }
-
-  @Override
-  protected void returnResource(final ShardedJedis resource) {
+  public void returnResource(final ShardedJedis resource) {
     if (resource != null) {
       resource.resetState();
       returnResourceObject(resource);

diff --git a/src/main/java/redis/clients/jedis/util/Pool.java b/src/main/java/redis/clients/jedis/util/Pool.java
@@ -12,6 +12,11 @@
 import redis.clients.jedis.exceptions.JedisExhaustedPoolException;
 
 public abstract class Pool<T> implements Closeable {
+
+  /**
+   * @deprecated This will be private in future.
+   */
+  @Deprecated
   protected GenericObjectPool<T> internalPool;
 
   /**
@@ -45,6 +50,12 @@ public void initPool(final GenericObjectPoolConfig poolConfig, PooledObjectFacto
     this.internalPool = new GenericObjectPool<>(factory, poolConfig);
   }
 
+  protected void clearInternalPool() {
+    if (internalPool != null) {
+      internalPool.clear();
+    }
+  }
+
   public T getResource() {
     try {
       return internalPool.borrowObject();
@@ -61,23 +72,20 @@ public T getResource() {
   }
 
   protected void returnResourceObject(final T resource) {
-    if (resource == null) {
-      return;
-    }
     try {
       internalPool.returnObject(resource);
     } catch (Exception e) {
       throw new JedisException("Could not return the resource to the pool", e);
     }
   }
 
-  protected void returnBrokenResource(final T resource) {
+  public void returnBrokenResource(final T resource) {
     if (resource != null) {
       returnBrokenResourceObject(resource);
     }
   }
 
-  protected void returnResource(final T resource) {
+  public void returnResource(final T resource) {
     if (resource != null) {
       returnResourceObject(resource);
     }