Remove JedisPoolAbstract class, hide Pool.initPool() method and related changes (#1734)

* 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.

* Update JavaDoc formatting

Author: sazzad16

src/main/java/redis/clients/jedis/Jedis.java

@@ -17,18 +17,15 @@
 import redis.clients.jedis.commands.*;
 import redis.clients.jedis.params.*;
 import redis.clients.jedis.resps.*;
+import redis.clients.jedis.util.Pool;
 import redis.clients.jedis.util.SafeEncoder;
 import redis.clients.jedis.util.Slowlog;
 
 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;
+  private Pool<Jedis> dataSource = null;
 
   public Jedis() {
     super();
@@ -3944,7 +3941,7 @@ public Map<String, String> pubsubNumSub(String... channels) {
   @Override
   public void close() {
     if (dataSource != null) {
-      JedisPoolAbstract pool = this.dataSource;
+      Pool<Jedis> pool = this.dataSource;
       this.dataSource = null;
       if (isBroken()) {
         pool.returnBrokenResource(this);
@@ -3956,7 +3953,7 @@ public void close() {
     }
   }
 
-  public void setDataSource(JedisPoolAbstract jedisPool) {
+  public void setDataSource(Pool<Jedis> jedisPool) {
     this.dataSource = jedisPool;
   }
 

src/main/java/redis/clients/jedis/JedisPool.java

@@ -11,8 +11,9 @@
 import org.slf4j.LoggerFactory;
 
 import redis.clients.jedis.util.JedisURIHelper;
+import redis.clients.jedis.util.Pool;
 
-public class JedisPool extends JedisPoolAbstract {
+public class JedisPool extends Pool<Jedis> {
 
   private static final Logger log = LoggerFactory.getLogger(JedisPool.class);
 
@@ -29,43 +30,36 @@ public JedisPool(String host, int port) {
   }
 
   /**
+   * WARNING: This constructor only accepts a uri string as {@code url}.
+   * {@link JedisURIHelper#isValid(java.net.URI)} can be used before this.
+   * <p>
+   * To use a host string, {@link #JedisPool(java.lang.String, int)} can be used with
+   * {@link Protocol#DEFAULT_PORT}.
+   *
    * @param url
-   * @deprecated This constructor will not accept a host string in future. It will accept only a uri
-   * string. You can use {@link JedisURIHelper#isValid(java.net.URI)} before this.
    */
-  @Deprecated
   public JedisPool(final String url) {
-    URI uri = URI.create(url);
-    if (JedisURIHelper.isValid(uri)) {
-      initPool(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(uri,
-          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null));
-    } else {
-      initPool(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(url, Protocol.DEFAULT_PORT,
-          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null));
-    }
+    this(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(URI.create(url), Protocol.DEFAULT_TIMEOUT,
+        Protocol.DEFAULT_TIMEOUT, null));
   }
 
   /**
+   * WARNING: This constructor only accepts a uri string as {@code url}.
+   * {@link JedisURIHelper#isValid(java.net.URI)} can be used before this.
+   * <p>
+   * To use a host string,
+   * {@link #JedisPool(java.lang.String, int, boolean, javax.net.ssl.SSLSocketFactory, javax.net.ssl.SSLParameters, javax.net.ssl.HostnameVerifier)}
+   * can be used with {@link Protocol#DEFAULT_PORT} and {@code ssl=true}.
+   *
    * @param url
    * @param sslSocketFactory
    * @param sslParameters
    * @param hostnameVerifier
-   * @deprecated This constructor will not accept a host string in future. It will accept only a uri
-   * string. You can use {@link JedisURIHelper#isValid(java.net.URI)} before this.
    */
-  @Deprecated
   public JedisPool(final String url, final SSLSocketFactory sslSocketFactory,
       final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
-    URI uri = URI.create(url);
-    if (JedisURIHelper.isValid(uri)) {
-      initPool(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(uri,
-          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, sslSocketFactory,
-          sslParameters, hostnameVerifier));
-    } else {
-      initPool(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(url, Protocol.DEFAULT_PORT,
-          Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null,
-          false, null, null, null));
-    }
+    this(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(URI.create(url), Protocol.DEFAULT_TIMEOUT,
+        Protocol.DEFAULT_TIMEOUT, null, sslSocketFactory, sslParameters, hostnameVerifier));
   }
 
   public JedisPool(final URI uri) {

src/main/java/redis/clients/jedis/JedisPoolAbstract.java

@@ -14,8 +14,9 @@
 
 import redis.clients.jedis.exceptions.JedisConnectionException;
 import redis.clients.jedis.exceptions.JedisException;
+import redis.clients.jedis.util.Pool;
 
-public class JedisSentinelPool extends JedisPoolAbstract {
+public class JedisSentinelPool extends Pool<Jedis> {
 
   private static final Logger LOG = LoggerFactory.getLogger(JedisSentinelPool.class);
 

src/main/java/redis/clients/jedis/JedisSentinelPool.java

@@ -14,23 +14,10 @@
 
 public abstract class Pool<T> implements Closeable {
 
-  /**
-   * @deprecated This will be private in future.
-   */
-  @Deprecated
-  protected GenericObjectPool<T> internalPool;
-
-  /**
-   * Using this constructor means you have to set and initialize the internalPool yourself.
-   *
-   * @deprecated This constructor will be removed in future.
-   */
-  @Deprecated
-  public Pool() {
-  }
+  private final GenericObjectPool<T> internalPool;
 
   public Pool(final GenericObjectPoolConfig<T> poolConfig, PooledObjectFactory<T> factory) {
-    initPool(poolConfig, factory);
+    this.internalPool = new GenericObjectPool<>(factory, poolConfig);
   }
 
   @Override
@@ -42,24 +29,6 @@ public boolean isClosed() {
     return this.internalPool.isClosed();
   }
 
-  /**
-   * @param poolConfig
-   * @param factory
-   * @deprecated This method will be private in future.
-   */
-  @Deprecated
-  public void initPool(final GenericObjectPoolConfig<T> poolConfig, PooledObjectFactory<T> factory) {
-
-    if (this.internalPool != null) {
-      try {
-        closeInternalPool();
-      } catch (Exception e) {
-      }
-    }
-
-    this.internalPool = new GenericObjectPool<>(factory, poolConfig);
-  }
-
   /**
    * This call only clears idle instances, not borrowed instances.
    */

src/main/java/redis/clients/jedis/util/Pool.java

@@ -252,8 +252,7 @@ public void passivateObject(PooledObject<Jedis> p) throws Exception {
 
     GenericObjectPoolConfig<Jedis> config = new GenericObjectPoolConfig<>();
     config.setMaxTotal(1);
-    JedisPool pool = new JedisPool(config, hnp.getHost(), hnp.getPort(), 2000, "foobared");
-    pool.initPool(config, new CrashingJedisPooledObjectFactory());
+    JedisPool pool = new JedisPool(config, new CrashingJedisPooledObjectFactory());
     Jedis crashingJedis = pool.getResource();
 
     try {

src/test/java/redis/clients/jedis/tests/JedisPoolTest.java

@@ -218,62 +218,6 @@ public void ensureSafeTwiceFailover() throws InterruptedException {
     // you can test failover as much as possible
   }
 
-  @Test
-  public void returnResourceDestroysResourceOnException() {
-    class CrashingJedis extends Jedis {
-      public CrashingJedis(final HostAndPort hp) {
-        super(hp);
-      }
-
-      @Override
-      public void resetState() {
-        throw new RuntimeException();
-      }
-    }
-
-    final AtomicInteger destroyed = new AtomicInteger(0);
-
-    class CrashingJedisPooledObjectFactory implements PooledObjectFactory<Jedis> {
-
-      @Override
-      public PooledObject<Jedis> makeObject() throws Exception {
-        return new DefaultPooledObject<Jedis>(new CrashingJedis(master));
-      }
-
-      @Override
-      public void destroyObject(PooledObject<Jedis> p) throws Exception {
-        destroyed.incrementAndGet();
-      }
-
-      @Override
-      public boolean validateObject(PooledObject<Jedis> p) {
-        return true;
-      }
-
-      @Override
-      public void activateObject(PooledObject<Jedis> p) throws Exception {
-      }
-
-      @Override
-      public void passivateObject(PooledObject<Jedis> p) throws Exception {
-      }
-    }
-
-    GenericObjectPoolConfig<Jedis> config = new GenericObjectPoolConfig<>();
-    config.setMaxTotal(1);
-    JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, config, 1000,
-        "foobared", 2);
-    pool.initPool(config, new CrashingJedisPooledObjectFactory());
-    Jedis crashingJedis = pool.getResource();
-
-    try {
-      crashingJedis.close();
-    } catch (Exception ignored) {
-    }
-
-    assertEquals(1, destroyed.get());
-  }
-
   private void forceFailover(JedisSentinelPool pool) throws InterruptedException {
     HostAndPort oldMaster = pool.getCurrentHostMaster();