Skip to content

Commit

Permalink
Add ability to reset password in JedisFactory (II) (#2315)
Browse files Browse the repository at this point in the history 
* Add ability to reset password in JedisFactory for JedisPool and JedisSentinelPool

* Modify JedisFactory tests and support ACL

* reduce new public

* volatile password instead of AtomicReference

* apply acl tests

* Add ability to reset password in JedisFactory for JedisPool and JedisSentinelPool

* Modify JedisFactory tests and support ACL

* Modify JedisFactory and Pool

    - Added JedisFactory constructor without host and port as there is option to set those later.

    - Prepared to private internalPool in Pool.

* Adddress gkorland's suggestion

* Deprecation and JavaDoc

* address change requests

* address change requests

* Unbreak (both JedisFactory and JedisSentinelPool are errored)

* update with config pattern

* Remove checking of 'user' while updating password

* update deprecation messages

Co-authored-by: yapei <yapei@cisco.com>
  • Loading branch information
@sazzad16
sazzad16 and yapei authored Mar 17, 2021
1 parent a7d2147 commit 17b4550
Show file tree
Hide file tree
Showing 10 changed files with 265 additions and 61 deletions.
10 changes: 9 additions & 1 deletion src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package redis.clients.jedis;

import java.util.Objects;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocketFactory;
Expand All @@ -11,7 +12,7 @@ public final class DefaultJedisClientConfig implements JedisClientConfig {
private final int infiniteSoTimeoutMillis;

private final String user;
private final String password;
private volatile String password;
private final int database;
private final String clientName;

Expand Down Expand Up @@ -65,6 +66,13 @@ public String getPassword() {
return password;
}

@Override
public synchronized void updatePassword(String password) {
if (!Objects.equals(this.password, password)) {
this.password = password;
}
}

@Override
public int getDatabase() {
return database;
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/redis/clients/jedis/JedisClientConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ default String getPassword() {
return null;
}

default void updatePassword(String password) {
}

default int getDatabase() {
return Protocol.DEFAULT_DATABASE;
}
Expand Down
43 changes: 32 additions & 11 deletions src/main/java/redis/clients/jedis/JedisFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,72 +20,89 @@
/**
* PoolableObjectFactory custom impl.
*/
class JedisFactory implements PooledObjectFactory<Jedis> {
public class JedisFactory implements PooledObjectFactory<Jedis> {

private static final Logger logger = LoggerFactory.getLogger(JedisFactory.class);

private final AtomicReference<HostAndPort> hostAndPort = new AtomicReference<>();

private final JedisClientConfig config;

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 HostAndPort hostAndPort, final JedisClientConfig clientConfig) {
protected JedisFactory(final HostAndPort hostAndPort, final JedisClientConfig clientConfig) {
this.hostAndPort.set(hostAndPort);
this.config = DefaultJedisClientConfig.copyConfig(clientConfig);
}

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.config = DefaultJedisClientConfig.builder().withConnectionTimeoutMillis(connectionTimeout)
.withSoTimeoutMillis(soTimeout).withInfiniteSoTimeoutMillis(infiniteSoTimeout).withUser(user)
.withPassword(password).withDatabse(database).withClientName(clientName)
.withSsl(ssl).withSslSocketFactory(sslSocketFactory)
.withSslParameters(sslParameters).withHostnameVerifier(hostnameVerifier).build();
}

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)) {
Expand All @@ -105,6 +122,10 @@ public void setHostAndPort(final HostAndPort hostAndPort) {
this.hostAndPort.set(hostAndPort);
}

public void setPassword(final String password) {
this.config.updatePassword(password);
}

@Override
public void activateObject(PooledObject<Jedis> pooledJedis) throws Exception {
final BinaryJedis jedis = pooledJedis.getObject();
Expand Down
39 changes: 29 additions & 10 deletions src/main/java/redis/clients/jedis/JedisPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocketFactory;

import org.apache.commons.pool2.PooledObjectFactory;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -27,29 +28,43 @@ public JedisPool(String host, int port) {
this(new GenericObjectPoolConfig<Jedis>(), host, port);
}

public JedisPool(final String host) {
URI uri = URI.create(host);
/**
* @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(host, Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null));
initPool(new GenericObjectPoolConfig<Jedis>(), new JedisFactory(url, Protocol.DEFAULT_PORT,
Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null));
}
}

public JedisPool(final String host, final SSLSocketFactory sslSocketFactory,
/**
* @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(host);
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(host, Protocol.DEFAULT_PORT,
Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE,
null, false, null, null, null));
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));
}
}

Expand Down Expand Up @@ -337,6 +352,10 @@ public JedisPool(final GenericObjectPoolConfig<Jedis> 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();
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/redis/clients/jedis/JedisPoolAbstract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
@Deprecated
public class JedisPoolAbstract extends Pool<Jedis> {

/**
* Using this constructor means you have to set and initialize the internalPool yourself.
*
* @deprecated This constructor will be removed in future.
*/
@Deprecated
public JedisPoolAbstract() {
super();
}
Expand Down
71 changes: 34 additions & 37 deletions src/main/java/redis/clients/jedis/JedisSentinelPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,25 @@ public class JedisSentinelPool extends JedisPoolAbstract {
protected static Logger log = LoggerFactory.getLogger(JedisSentinelPool.class);

protected final GenericObjectPoolConfig<Jedis> poolConfig;
private final JedisFactory factory;

protected final int connectionTimeout;
protected final int soTimeout;
protected final int infiniteSoTimeout;
@Deprecated protected int connectionTimeout;
@Deprecated protected int soTimeout;
@Deprecated protected int infiniteSoTimeout;

protected final String user;
protected final String password;
protected final int database;
protected final String clientName;
@Deprecated protected String user;
@Deprecated protected String password;
@Deprecated protected int database;
@Deprecated protected String clientName;

protected int sentinelConnectionTimeout;
protected int sentinelSoTimeout;
protected String sentinelUser;
protected String sentinelPassword;
protected String sentinelClientName;
@Deprecated protected int sentinelConnectionTimeout;
@Deprecated protected int sentinelSoTimeout;
@Deprecated protected String sentinelUser;
@Deprecated protected String sentinelPassword;
@Deprecated protected String sentinelClientName;

protected final Set<MasterListener> masterListeners = new HashSet<>();

private volatile JedisFactory factory;
private volatile HostAndPort currentHostMaster;

private final Object initPoolLock = new Object();
Expand Down Expand Up @@ -160,8 +160,7 @@ 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.poolConfig = poolConfig;
this(masterName, sentinels, poolConfig, new JedisFactory(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName));
this.connectionTimeout = connectionTimeout;
this.soTimeout = soTimeout;
this.infiniteSoTimeout = infiniteSoTimeout;
Expand All @@ -174,9 +173,16 @@ public JedisSentinelPool(String masterName, Set<String> sentinels,
this.sentinelUser = sentinelUser;
this.sentinelPassword = sentinelPassword;
this.sentinelClientName = sentinelClientName;
}

public JedisSentinelPool(String masterName, Set<String> sentinels,
final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory) {
super(poolConfig, factory);
this.poolConfig = poolConfig;
this.factory = factory;

HostAndPort master = initSentinels(sentinels, masterName);
initPool(master);
initMaster(master);
}

@Override
Expand All @@ -192,22 +198,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
clearInternalPool();
}
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);
}
}
}
Expand All @@ -224,8 +224,7 @@ private HostAndPort initSentinels(Set<String> sentinels, final String masterName

log.debug("Connecting to Sentinel {}", hap);


try (Jedis 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) {
Expand All @@ -249,10 +248,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);
// 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);
}
}

Expand Down Expand Up @@ -375,7 +372,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() {
Expand All @@ -388,7 +385,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 {}",
Expand Down
Loading

0 comments on commit 17b4550

Please sign in to comment.