Add ability to reset password in JedisFactory (II)

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

@@ -18,54 +18,71 @@
 /**
  * PoolableObjectFactory custom impl.
  */
-class JedisFactory implements PooledObjectFactory<Jedis> {
+public class JedisFactory implements PooledObjectFactory<Jedis> {
   private final AtomicReference<HostAndPort> hostAndPort = new AtomicReference<>();
   private final int connectionTimeout;
   private final int soTimeout;
   private final int infiniteSoTimeout;
   private final String user;
-  private final String password;
+  private volatile String password;
   private final int database;
   private final String clientName;
   private final boolean ssl;
   private final SSLSocketFactory sslSocketFactory;
   private final SSLParameters sslParameters;
   private final HostnameVerifier hostnameVerifier;
 
-  JedisFactory(final String host, final int port, final int connectionTimeout,
+  protected JedisFactory(final String host, final int port, final int connectionTimeout,
       final int soTimeout, final String password, final int database, final String clientName) {
     this(host, port, connectionTimeout, soTimeout, password, database, clientName, false, null, null, null);
   }
 
-  JedisFactory(final String host, final int port, final int connectionTimeout,
+  protected JedisFactory(final String host, final int port, final int connectionTimeout,
                final int soTimeout, final String user, final String password, final int database, final String clientName) {
     this(host, port, connectionTimeout, soTimeout, 0, user, password, database, clientName);
   }
 
-  JedisFactory(final String host, final int port, final int connectionTimeout, final int soTimeout,
+  protected JedisFactory(final String host, final int port, final int connectionTimeout, final int soTimeout,
       final int infiniteSoTimeout, final String user, final String password, final int database, final String clientName) {
     this(host, port, connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName, false, null, null, null);
   }
 
-  JedisFactory(final String host, final int port, final int connectionTimeout,
+  /**
+   * {@link #setHostAndPort(redis.clients.jedis.HostAndPort) setHostAndPort} must be called later.
+   */
+  protected JedisFactory(final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout,
+      final String user, final String password, final int database, final String clientName) {
+    this(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName, false, null, null, null);
+  }
+
+  protected JedisFactory(final String host, final int port, final int connectionTimeout,
       final int soTimeout, final String password, final int database, final String clientName,
       final boolean ssl, final SSLSocketFactory sslSocketFactory, final SSLParameters sslParameters,
       final HostnameVerifier hostnameVerifier) {
     this(host, port, connectionTimeout, soTimeout, null, password, database, clientName, ssl, sslSocketFactory, sslParameters, hostnameVerifier);
   }
 
-  JedisFactory(final String host, final int port, final int connectionTimeout,
+  protected JedisFactory(final String host, final int port, final int connectionTimeout,
                final int soTimeout, final String user, final String password, final int database, final String clientName,
                final boolean ssl, final SSLSocketFactory sslSocketFactory, final SSLParameters sslParameters,
                final HostnameVerifier hostnameVerifier) {
     this(host, port, connectionTimeout, soTimeout, 0, user, password, database, clientName, ssl, sslSocketFactory, sslParameters, hostnameVerifier);
   }
 
-  JedisFactory(final String host, final int port, final int connectionTimeout, final int soTimeout,
+  protected JedisFactory(final String host, final int port, final int connectionTimeout, final int soTimeout,
       final int infiniteSoTimeout, final String user, final String password, final int database,
       final String clientName, final boolean ssl, final SSLSocketFactory sslSocketFactory,
       final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
+    this(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName, ssl, sslSocketFactory, sslParameters, hostnameVerifier);
     this.hostAndPort.set(new HostAndPort(host, port));
+  }
+
+  /**
+   * {@link #setHostAndPort(redis.clients.jedis.HostAndPort) setHostAndPort} must be called later.
+   */
+  protected JedisFactory(final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout,
+      final String user, final String password, final int database, final String clientName, final boolean ssl,
+      final SSLSocketFactory sslSocketFactory, final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
     this.connectionTimeout = connectionTimeout;
     this.soTimeout = soTimeout;
     this.infiniteSoTimeout = infiniteSoTimeout;
@@ -79,18 +96,18 @@ class JedisFactory implements PooledObjectFactory<Jedis> {
     this.hostnameVerifier = hostnameVerifier;
   }
 
-  JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
+  protected JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
       final String clientName) {
     this(uri, connectionTimeout, soTimeout, clientName, null, null, null);
   }
 
-  JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
+  protected JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
       final String clientName, final SSLSocketFactory sslSocketFactory,
       final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
     this(uri, connectionTimeout, soTimeout, 0, clientName, sslSocketFactory, sslParameters, hostnameVerifier);
   }
 
-  JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
+  protected JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
       final int infiniteSoTimeout, final String clientName, final SSLSocketFactory sslSocketFactory,
       final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
     if (!JedisURIHelper.isValid(uri)) {
@@ -116,6 +133,27 @@ public void setHostAndPort(final HostAndPort hostAndPort) {
     this.hostAndPort.set(hostAndPort);
   }
 
+  /**
+   * @param password
+   * @throws IllegalArgumentException
+   * @see #setPassword(java.lang.String, java.lang.String)
+   */
+  public void setPassword(final String password) throws IllegalArgumentException {
+    setPassword(null, password);
+  }
+
+  /**
+   * @param user has to be the same one with which the factory is created
+   * @param password
+   * @throws IllegalArgumentException if the user is not the original user
+   */
+  public void setPassword(final String user, final String password) throws IllegalArgumentException {
+    if (!java.util.Objects.equals(this.user, user)) {
+      throw new IllegalArgumentException();
+    }
+    this.password = password;
+  }
+
   @Override
   public void activateObject(PooledObject<Jedis> pooledJedis) throws Exception {
     final BinaryJedis jedis = pooledJedis.getObject();

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

@@ -6,7 +6,7 @@
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSocketFactory;
 
-import org.apache.commons.pool2.impl.GenericObjectPool;
+import org.apache.commons.pool2.PooledObjectFactory;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
 
 import redis.clients.jedis.exceptions.JedisException;
@@ -26,29 +26,41 @@ public JedisPool(String host, int port) {
     this(new GenericObjectPoolConfig(), host, port);
   }
 
+  /**
+   * @param host
+   * @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 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));
     }
   }
 
+  /**
+   * @param host
+   * @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 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));
     }
   }
 
@@ -325,6 +337,10 @@ public JedisPool(final GenericObjectPoolConfig poolConfig, final URI uri,
         sslSocketFactory, sslParameters, hostnameVerifier));
   }
 
+  public JedisPool(GenericObjectPoolConfig poolConfig, PooledObjectFactory<Jedis> factory) {
+    super(poolConfig, factory);
+  }
+
   @Override
   public Jedis getResource() {
     Jedis jedis = super.getResource();

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

@@ -7,6 +7,10 @@
 
 public class JedisPoolAbstract extends Pool<Jedis> {
 
+  /**
+   * Using this constructor means you have to set and initialize the internalPool yourself.
+   */
+  @Deprecated
   public JedisPoolAbstract() {
     super();
   }

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

@@ -17,15 +17,7 @@ public class JedisSentinelPool extends JedisPoolAbstract {
   protected Logger log = LoggerFactory.getLogger(getClass().getName());
 
   protected final GenericObjectPoolConfig poolConfig;
-
-  protected final int connectionTimeout;
-  protected final int soTimeout;
-  protected final int infiniteSoTimeout;
-
-  protected final String user;
-  protected final String password;
-  protected final int database;
-  protected final String clientName;
+  protected final JedisFactory factory;
 
   protected int sentinelConnectionTimeout;
   protected int sentinelSoTimeout;
@@ -35,7 +27,6 @@ public class JedisSentinelPool extends JedisPoolAbstract {
 
   protected final Set<MasterListener> masterListeners = new HashSet<>();
 
-  private volatile JedisFactory factory;
   private volatile HostAndPort currentHostMaster;
 
   private final Object initPoolLock = new Object();
@@ -156,23 +147,29 @@ public JedisSentinelPool(String masterName, Set<String> sentinels,
       final String user, final String password, final int database, final String clientName,
       final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
       final String sentinelPassword, final String sentinelClientName) {
+    this(masterName, sentinels, poolConfig, new JedisFactory(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName));
+  }
 
+  public JedisSentinelPool(String masterName, Set<String> sentinels, final GenericObjectPoolConfig poolConfig, final JedisFactory factory) {
+    this(masterName, sentinels, poolConfig, factory, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, null, null);
+  }
+
+  public JedisSentinelPool(String masterName, Set<String> sentinels,
+      final GenericObjectPoolConfig poolConfig, final JedisFactory factory,
+      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
+      final String sentinelPassword, final String sentinelClientName) {
+    super(poolConfig, factory);
     this.poolConfig = poolConfig;
-    this.connectionTimeout = connectionTimeout;
-    this.soTimeout = soTimeout;
-    this.infiniteSoTimeout = infiniteSoTimeout;
-    this.user = user;
-    this.password = password;
-    this.database = database;
-    this.clientName = clientName;
+    this.factory = factory;
+
     this.sentinelConnectionTimeout = sentinelConnectionTimeout;
     this.sentinelSoTimeout = sentinelSoTimeout;
     this.sentinelUser = sentinelUser;
     this.sentinelPassword = sentinelPassword;
     this.sentinelClientName = sentinelClientName;
 
     HostAndPort master = initSentinels(sentinels, masterName);
-    initPool(master);
+    initMaster(master);
   }
 
   @Override
@@ -188,24 +185,16 @@ public HostAndPort getCurrentHostMaster() {
     return currentHostMaster;
   }
 
-  private void initPool(HostAndPort master) {
-    synchronized(initPoolLock){
+  private void initMaster(HostAndPort master) {
+    synchronized (initPoolLock) {
       if (!master.equals(currentHostMaster)) {
         currentHostMaster = master;
-        if (factory == null) {
-          factory = new JedisFactory(master.getHost(), master.getPort(), connectionTimeout,
-              soTimeout, infiniteSoTimeout, user, password, database, clientName);
-          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();
-        }
+        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
+        clearInternalPool();
 
-        log.info("Created JedisPool to master at {}", master);
+        log.info("Created JedisSentinelPool to master at {}", master);
       }
     }
   }
@@ -222,9 +211,7 @@ private HostAndPort initSentinels(Set<String> sentinels, final String masterName
 
       log.debug("Connecting to Sentinel {}", hap);
 
-      Jedis jedis = null;
-      try {
-        jedis = new Jedis(hap.getHost(), hap.getPort(), sentinelConnectionTimeout, sentinelSoTimeout);
+      try (Jedis jedis = new Jedis(hap.getHost(), hap.getPort(), sentinelConnectionTimeout, sentinelSoTimeout)) {
         if (sentinelUser != null) {
           jedis.auth(sentinelUser, sentinelPassword);
         } else if (sentinelPassword != null) {
@@ -248,14 +235,8 @@ private HostAndPort initSentinels(Set<String> sentinels, final String masterName
         log.debug("Found Redis master at {}", master);
         break;
       } catch (JedisException e) {
-        // resolves #1036, it should handle JedisException there's another chance
-        // of raising JedisDataException
-        log.warn(
-          "Cannot get master address from sentinel running @ {}. Reason: {}. Trying next one.", hap, e);
-      } finally {
-        if (jedis != null) {
-          jedis.close();
-        }
+        // resolves #1036, it should handle JedisException there's another chance of raising JedisDataException
+        log.warn("Cannot get master address from sentinel running @ {}. Reason: {}. Trying next one.", hap, e);
       }
     }
 
@@ -385,7 +366,7 @@ public void run() {
           if (masterAddr == null || masterAddr.size() != 2) {
             log.warn("Can not get master addr, master name: {}. Sentinel: {}:{}.", masterName, host, port);
           } else {
-            initPool(toHostAndPort(masterAddr));
+            initMaster(toHostAndPort(masterAddr));
           }
 
           j.subscribe(new JedisPubSub() {
@@ -398,7 +379,7 @@ public void onMessage(String channel, String message) {
               if (switchMasterMsg.length > 3) {
 
                 if (masterName.equals(switchMasterMsg[0])) {
-                  initPool(toHostAndPort(Arrays.asList(switchMasterMsg[3], switchMasterMsg[4])));
+                  initMaster(toHostAndPort(Arrays.asList(switchMasterMsg[3], switchMasterMsg[4])));
                 } else {
                   log.debug(
                     "Ignoring message on +switch-master for master name {}, our master name is {}",