From e4877e994183e183562a0eedff8752fec38df28b Mon Sep 17 00:00:00 2001 From: yapei Date: Wed, 13 Sep 2017 16:10:00 -0500 Subject: [PATCH 01/16] Add ability to reset password in JedisFactory for JedisPool and JedisSentinelPool --- .../redis/clients/jedis/JedisFactory.java | 20 ++++++---- .../java/redis/clients/jedis/JedisPool.java | 5 +++ .../clients/jedis/JedisSentinelPool.java | 21 ++++++++++ .../clients/jedis/tests/JedisPoolTest.java | 38 ++++++++++++++++++ .../jedis/tests/JedisSentinelPoolTest.java | 39 +++++++++++++++++++ 5 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index 5cc0aa283a..24a4036059 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -18,13 +18,13 @@ /** * PoolableObjectFactory custom impl. */ -class JedisFactory implements PooledObjectFactory { +public class JedisFactory implements PooledObjectFactory { private final AtomicReference hostAndPort = new AtomicReference<>(); private final int connectionTimeout; private final int soTimeout; private final int infiniteSoTimeout; private final String user; - private final String password; + private final AtomicReference password = new AtomicReference<>(); private final int database; private final String clientName; private final boolean ssl; @@ -32,7 +32,7 @@ class JedisFactory implements PooledObjectFactory { private final SSLParameters sslParameters; private final HostnameVerifier hostnameVerifier; - JedisFactory(final String host, final int port, final int connectionTimeout, + public 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); } @@ -70,7 +70,7 @@ class JedisFactory implements PooledObjectFactory { this.soTimeout = soTimeout; this.infiniteSoTimeout = infiniteSoTimeout; this.user = user; - this.password = password; + this.password.set(password); this.database = database; this.clientName = clientName; this.ssl = ssl; @@ -103,7 +103,7 @@ class JedisFactory implements PooledObjectFactory { this.soTimeout = soTimeout; this.infiniteSoTimeout = infiniteSoTimeout; this.user = JedisURIHelper.getUser(uri); - this.password = JedisURIHelper.getPassword(uri); + this.password.set(JedisURIHelper.getPassword(uri)); this.database = JedisURIHelper.getDBIndex(uri); this.clientName = clientName; this.ssl = JedisURIHelper.isRedisSSLScheme(uri); @@ -116,6 +116,10 @@ public void setHostAndPort(final HostAndPort hostAndPort) { this.hostAndPort.set(hostAndPort); } + public void setPassword(final String password) { + this.password.set(password); + } + @Override public void activateObject(PooledObject pooledJedis) throws Exception { final BinaryJedis jedis = pooledJedis.getObject(); @@ -148,9 +152,9 @@ public PooledObject makeObject() throws Exception { try { jedis.connect(); if (user != null) { - jedis.auth(user, password); - } else if (password != null) { - jedis.auth(password); + jedis.auth(user, password.get()); + } else if (password.get() != null) { + jedis.auth(password.get()); } if (database != 0) { jedis.select(database); diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java index 1f962a2d54..72d86924c1 100644 --- a/src/main/java/redis/clients/jedis/JedisPool.java +++ b/src/main/java/redis/clients/jedis/JedisPool.java @@ -6,6 +6,7 @@ import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocketFactory; +import org.apache.commons.pool2.PooledObjectFactory; import org.apache.commons.pool2.impl.GenericObjectPool; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; @@ -325,6 +326,10 @@ public JedisPool(final GenericObjectPoolConfig poolConfig, final URI uri, sslSocketFactory, sslParameters, hostnameVerifier)); } + public JedisPool(GenericObjectPoolConfig poolConfig, PooledObjectFactory factory) { + super(poolConfig, factory); + } + @Override public Jedis getResource() { Jedis jedis = super.getResource(); diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java index a61076971e..5210dbad33 100644 --- a/src/main/java/redis/clients/jedis/JedisSentinelPool.java +++ b/src/main/java/redis/clients/jedis/JedisSentinelPool.java @@ -175,6 +175,27 @@ public JedisSentinelPool(String masterName, Set sentinels, initPool(master); } + public JedisSentinelPool(String masterName, Set sentinels, final GenericObjectPoolConfig poolConfig, final JedisFactory factory) { + this.poolConfig = poolConfig; + this.connectionTimeout = Protocol.DEFAULT_TIMEOUT; + this.soTimeout = Protocol.DEFAULT_TIMEOUT; + this.infiniteSoTimeout = 0; + this.user = null; + this.password = null; + this.database = Protocol.DEFAULT_DATABASE; + this.clientName = null; + this.sentinelConnectionTimeout = Protocol.DEFAULT_TIMEOUT; + this.sentinelSoTimeout = Protocol.DEFAULT_TIMEOUT; + this.sentinelUser = null; + this.sentinelPassword = null; + this.sentinelClientName = null; + this.factory = factory; + + HostAndPort master = initSentinels(sentinels, masterName); + initPool(poolConfig, factory); + initPool(master); + } + @Override public void destroy() { for (MasterListener m : masterListeners) { diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index d010382bac..b95d198922 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -16,10 +16,12 @@ import redis.clients.jedis.HostAndPort; import redis.clients.jedis.Jedis; +import redis.clients.jedis.JedisFactory; import redis.clients.jedis.JedisPool; import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.Transaction; import redis.clients.jedis.exceptions.InvalidURIException; +import redis.clients.jedis.exceptions.JedisConnectionException; import redis.clients.jedis.exceptions.JedisExhaustedPoolException; public class JedisPoolTest { @@ -382,4 +384,40 @@ private int getClientCount(final String clientList) { return clientList.split("\n").length; } + @Test + public void testResetInvalidPassword() { + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "foobared", 0, "my_shiny_client_name"); + + + try(JedisPool pool = new JedisPool(new JedisPoolConfig(), factory); + Jedis obj1 = pool.getResource();){ + obj1.set("foo", "bar"); + assertEquals("bar", obj1.get("foo")); + assertEquals(1, pool.getNumActive()); + + factory.setPassword("wrong password"); + try (Jedis obj2 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException e) {} + } + } + + @Test + public void testResetValidPassword() { + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "bad password", 0, "my_shiny_client_name"); + JedisPool pool = new JedisPool(new JedisPoolConfig(), factory); + Jedis obj = null; + try { + pool.getResource(); + fail("Could not get resource from pool"); + } catch (JedisConnectionException e) { + factory.setPassword("foobared"); + obj = pool.getResource(); + obj.set("foo", "bar"); + assertEquals("bar", obj.get("foo")); + } finally { + obj.close(); + pool.close(); + } + } } diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index ca1c72bbfc..e44ce78fc1 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; import java.util.HashSet; import java.util.Set; @@ -18,6 +19,8 @@ import redis.clients.jedis.HostAndPort; import redis.clients.jedis.Jedis; +import redis.clients.jedis.JedisFactory; +import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.JedisSentinelPool; import redis.clients.jedis.Transaction; import redis.clients.jedis.exceptions.JedisConnectionException; @@ -173,6 +176,42 @@ public void customClientName() { assertTrue(pool.isClosed()); } + + @Test + public void testResetInvalidPassword() { + JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "foobared", 0, "my_shiny_client_name"); + + try(JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory); + Jedis obj1 = pool.getResource(); ) { + obj1.set("foo", "bar"); + assertEquals("bar", obj1.get("foo")); + assertEquals(1, pool.getNumActive()); + + factory.setPassword("wrong password"); + try (Jedis obj2 = pool.getResource();) { + fail("Should not get resource from pool"); + }catch (JedisConnectionException e) {} + } + } + + @Test + public void testResetValidPassword() { + JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "wrong password", 0, "my_shiny_client_name"); + JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory); + Jedis obj = null; + try { + pool.getResource(); + fail("Could not get resource from pool"); + } catch (JedisConnectionException e) { + factory.setPassword("foobared"); + obj = pool.getResource(); + obj.set("foo", "bar"); + assertEquals("bar", obj.get("foo")); + } finally { + obj.close(); + pool.close(); + } + } @Test public void ensureSafeTwiceFailover() throws InterruptedException { From 0cbcf45dbf01b77e5d510e237b51da17ba36f5e6 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 13 Dec 2020 23:58:31 +0600 Subject: [PATCH 02/16] Modify JedisFactory tests and support ACL --- .../redis/clients/jedis/JedisFactory.java | 28 ++++++--- .../clients/jedis/tests/JedisPoolTest.java | 57 ++++++++++++------- .../JedisPoolWithCompleteCredentialsTest.java | 55 ++++++++++++++++++ .../jedis/tests/JedisSentinelPoolTest.java | 47 ++++++++------- 4 files changed, 137 insertions(+), 50 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index 24a4036059..567d00f36b 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -37,31 +37,31 @@ public JedisFactory(final String host, final int port, final int connectionTimeo this(host, port, connectionTimeout, soTimeout, password, database, clientName, false, null, null, null); } - JedisFactory(final String host, final int port, final int connectionTimeout, + public 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, + public 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, + public 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, + public 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, + public 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) { @@ -79,18 +79,18 @@ public JedisFactory(final String host, final int port, final int connectionTimeo this.hostnameVerifier = hostnameVerifier; } - JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout, + public 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, + public 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, + public 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,7 +116,17 @@ public void setHostAndPort(final HostAndPort hostAndPort) { this.hostAndPort.set(hostAndPort); } - public void setPassword(final String password) { + public void setPassword(final String password) throws IllegalArgumentException { + if (this.user != null) { + throw new IllegalArgumentException(); + } + this.password.set(password); + } + + public void setPassword(final String user, final String password) throws IllegalArgumentException { + if (!java.util.Objects.equals(this.user, user)) { + throw new IllegalArgumentException(); + } this.password.set(password); } diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index b95d198922..61a74cf047 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -387,37 +388,53 @@ private int getClientCount(final String clientList) { @Test public void testResetInvalidPassword() { JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "foobared", 0, "my_shiny_client_name"); - - try(JedisPool pool = new JedisPool(new JedisPoolConfig(), factory); - Jedis obj1 = pool.getResource();){ - obj1.set("foo", "bar"); - assertEquals("bar", obj1.get("foo")); + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { + Jedis obj1; + try (Jedis obj11 = pool.getResource()) { + obj1 = obj11; + obj11.set("foo", "bar"); + assertEquals("bar", obj11.get("foo")); + assertEquals(1, pool.getNumActive()); + } + assertEquals(0, pool.getNumActive()); + Jedis obj12 = pool.getResource(); + assertSame(obj1, obj12); assertEquals(1, pool.getNumActive()); - + factory.setPassword("wrong password"); try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException e) {} + } catch (JedisConnectionException e) { + } + assertEquals(1, pool.getNumActive()); + obj12.close(); + assertEquals(0, pool.getNumActive()); } } - + @Test public void testResetValidPassword() { JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "bad password", 0, "my_shiny_client_name"); - JedisPool pool = new JedisPool(new JedisPoolConfig(), factory); - Jedis obj = null; - try { - pool.getResource(); - fail("Could not get resource from pool"); - } catch (JedisConnectionException e) { + + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { + try (Jedis obj1 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException jce) { + } + assertEquals(0, pool.getNumActive()); + + try { + factory.setPassword("default", "foobared"); + fail(); + } catch (IllegalArgumentException iae) { + } + factory.setPassword("foobared"); - obj = pool.getResource(); - obj.set("foo", "bar"); - assertEquals("bar", obj.get("foo")); - } finally { - obj.close(); - pool.close(); + try (Jedis obj2 = pool.getResource()) { + obj2.set("foo", "bar"); + assertEquals("bar", obj2.get("foo")); + } } } } diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java index 9994ccddb5..5b5fde82d1 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -13,10 +14,12 @@ import redis.clients.jedis.HostAndPort; import redis.clients.jedis.Jedis; +import redis.clients.jedis.JedisFactory; import redis.clients.jedis.JedisPool; import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.Protocol; import redis.clients.jedis.exceptions.InvalidURIException; +import redis.clients.jedis.exceptions.JedisConnectionException; import redis.clients.jedis.exceptions.JedisExhaustedPoolException; import redis.clients.jedis.tests.utils.RedisVersionUtil; @@ -265,4 +268,56 @@ private int getClientCount(final String clientList) { return clientList.split("\n").length; } + @Test + public void testResetInvalidPassword() { + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "default", "foobared", 0, "my_shiny_client_name"); + + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { + Jedis obj1; + try (Jedis obj11 = pool.getResource()) { + obj1 = obj11; + obj11.set("foo", "bar"); + assertEquals("bar", obj11.get("foo")); + assertEquals(1, pool.getNumActive()); + } + assertEquals(0, pool.getNumActive()); + Jedis obj12 = pool.getResource(); + assertSame(obj1, obj12); + assertEquals(1, pool.getNumActive()); + + factory.setPassword("default", "wrong password"); + try (Jedis obj2 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException e) { + } + assertEquals(1, pool.getNumActive()); + obj12.close(); + assertEquals(0, pool.getNumActive()); + } + } + + @Test + public void testResetValidPassword() { + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "default", "bad password", 0, "my_shiny_client_name"); + + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { + try (Jedis obj1 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException jce) { + } + assertEquals(0, pool.getNumActive()); + + try { + factory.setPassword("foobared"); + fail(); + } catch (IllegalArgumentException iae) { + } + + factory.setPassword("default", "foobared"); + try (Jedis obj2 = pool.getResource()) { + obj2.set("foo", "bar"); + assertEquals("bar", obj2.get("foo")); + } + } + } } diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index e44ce78fc1..8417ce6325 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java @@ -180,36 +180,41 @@ public void customClientName() { @Test public void testResetInvalidPassword() { JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "foobared", 0, "my_shiny_client_name"); - - try(JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory); - Jedis obj1 = pool.getResource(); ) { - obj1.set("foo", "bar"); - assertEquals("bar", obj1.get("foo")); - assertEquals(1, pool.getNumActive()); - + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory)) { + Jedis obj1; + try (Jedis obj11 = pool.getResource()) { + obj1 = obj11; + obj11.set("foo", "bar"); + assertEquals("bar", obj11.get("foo")); + } + Jedis obj12 = pool.getResource(); + assertSame(obj1, obj12); + factory.setPassword("wrong password"); - try (Jedis obj2 = pool.getResource();) { + try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); - }catch (JedisConnectionException e) {} + } catch (JedisConnectionException e) { + } + obj12.close(); } } @Test public void testResetValidPassword() { JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "wrong password", 0, "my_shiny_client_name"); - JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory); - Jedis obj = null; - try { - pool.getResource(); - fail("Could not get resource from pool"); - } catch (JedisConnectionException e) { + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory)) { + try (Jedis obj1 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException jce) { + } + factory.setPassword("foobared"); - obj = pool.getResource(); - obj.set("foo", "bar"); - assertEquals("bar", obj.get("foo")); - } finally { - obj.close(); - pool.close(); + try (Jedis obj2 = pool.getResource()) { + obj2.set("foo", "bar"); + assertEquals("bar", obj2.get("foo")); + } } } From e1e40c5aa89968fdfbac543c02c0f08216a41f10 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 14 Dec 2020 13:57:55 +0600 Subject: [PATCH 03/16] reduce new public --- .../java/redis/clients/jedis/JedisFactory.java | 18 +++++++++--------- .../clients/jedis/tests/JedisPoolTest.java | 15 +++++++-------- .../JedisPoolWithCompleteCredentialsTest.java | 15 +++++++-------- .../jedis/tests/JedisSentinelPoolTest.java | 10 ++++------ 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index 567d00f36b..ba70b1380d 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -32,36 +32,36 @@ public class JedisFactory implements PooledObjectFactory { private final SSLParameters sslParameters; private final HostnameVerifier hostnameVerifier; - public 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); } - public 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); } - public 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); } - public 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, 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); } - public 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); } - public 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) { @@ -79,18 +79,18 @@ public JedisFactory(final String host, final int port, final int connectionTimeo this.hostnameVerifier = hostnameVerifier; } - public 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); } - public 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); } - public 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)) { diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index 61a74cf047..a394c043be 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -387,7 +387,8 @@ private int getClientCount(final String clientList) { @Test public void testResetInvalidPassword() { - JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "foobared", 0, "my_shiny_client_name"); + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, + "foobared", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { Jedis obj1; @@ -405,8 +406,7 @@ public void testResetInvalidPassword() { factory.setPassword("wrong password"); try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException e) { - } + } catch (JedisConnectionException e) { } assertEquals(1, pool.getNumActive()); obj12.close(); assertEquals(0, pool.getNumActive()); @@ -415,20 +415,19 @@ public void testResetInvalidPassword() { @Test public void testResetValidPassword() { - JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "bad password", 0, "my_shiny_client_name"); + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, + "bad password", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { try (Jedis obj1 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException jce) { - } + } catch (JedisConnectionException e) { } assertEquals(0, pool.getNumActive()); try { factory.setPassword("default", "foobared"); fail(); - } catch (IllegalArgumentException iae) { - } + } catch (IllegalArgumentException e) { } factory.setPassword("foobared"); try (Jedis obj2 = pool.getResource()) { diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java index 5b5fde82d1..3e06a39c1d 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java @@ -270,7 +270,8 @@ private int getClientCount(final String clientList) { @Test public void testResetInvalidPassword() { - JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "default", "foobared", 0, "my_shiny_client_name"); + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, + "default", "foobared", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { Jedis obj1; @@ -288,8 +289,7 @@ public void testResetInvalidPassword() { factory.setPassword("default", "wrong password"); try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException e) { - } + } catch (JedisConnectionException jce) { } assertEquals(1, pool.getNumActive()); obj12.close(); assertEquals(0, pool.getNumActive()); @@ -298,20 +298,19 @@ public void testResetInvalidPassword() { @Test public void testResetValidPassword() { - JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, "default", "bad password", 0, "my_shiny_client_name"); + JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, + "default", "bad password", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { try (Jedis obj1 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException jce) { - } + } catch (JedisConnectionException e) { } assertEquals(0, pool.getNumActive()); try { factory.setPassword("foobared"); fail(); - } catch (IllegalArgumentException iae) { - } + } catch (IllegalArgumentException e) { } factory.setPassword("default", "foobared"); try (Jedis obj2 = pool.getResource()) { diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index 8417ce6325..33860e3f6d 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java @@ -179,7 +179,7 @@ public void customClientName() { @Test public void testResetInvalidPassword() { - JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "foobared", 0, "my_shiny_client_name"); + JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "foobared", 0, "my_shiny_client_name") { }; try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory)) { Jedis obj1; @@ -194,21 +194,19 @@ public void testResetInvalidPassword() { factory.setPassword("wrong password"); try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException e) { - } + } catch (JedisConnectionException e) { } obj12.close(); } } @Test public void testResetValidPassword() { - JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "wrong password", 0, "my_shiny_client_name"); + JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "wrong password", 0, "my_shiny_client_name") { }; try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory)) { try (Jedis obj1 = pool.getResource()) { fail("Should not get resource from pool"); - } catch (JedisConnectionException jce) { - } + } catch (JedisConnectionException e) { } factory.setPassword("foobared"); try (Jedis obj2 = pool.getResource()) { From 1de4e0fee0ad297c4e62093ee049982cbd4a898a Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 14 Dec 2020 19:45:31 +0600 Subject: [PATCH 04/16] volatile password instead of AtomicReference --- .../java/redis/clients/jedis/JedisFactory.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index ba70b1380d..1c8261d0b3 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -24,7 +24,7 @@ public class JedisFactory implements PooledObjectFactory { private final int soTimeout; private final int infiniteSoTimeout; private final String user; - private final AtomicReference password = new AtomicReference<>(); + private volatile String password; private final int database; private final String clientName; private final boolean ssl; @@ -70,7 +70,7 @@ protected JedisFactory(final String host, final int port, final int connectionTi this.soTimeout = soTimeout; this.infiniteSoTimeout = infiniteSoTimeout; this.user = user; - this.password.set(password); + this.password = password; this.database = database; this.clientName = clientName; this.ssl = ssl; @@ -103,7 +103,7 @@ protected JedisFactory(final URI uri, final int connectionTimeout, final int soT this.soTimeout = soTimeout; this.infiniteSoTimeout = infiniteSoTimeout; this.user = JedisURIHelper.getUser(uri); - this.password.set(JedisURIHelper.getPassword(uri)); + this.password = JedisURIHelper.getPassword(uri); this.database = JedisURIHelper.getDBIndex(uri); this.clientName = clientName; this.ssl = JedisURIHelper.isRedisSSLScheme(uri); @@ -120,14 +120,14 @@ public void setPassword(final String password) throws IllegalArgumentException { if (this.user != null) { throw new IllegalArgumentException(); } - this.password.set(password); + this.password = password; } public void setPassword(final String user, final String password) throws IllegalArgumentException { if (!java.util.Objects.equals(this.user, user)) { throw new IllegalArgumentException(); } - this.password.set(password); + this.password = password; } @Override @@ -162,9 +162,9 @@ public PooledObject makeObject() throws Exception { try { jedis.connect(); if (user != null) { - jedis.auth(user, password.get()); - } else if (password.get() != null) { - jedis.auth(password.get()); + jedis.auth(user, password); + } else if (password != null) { + jedis.auth(password); } if (database != 0) { jedis.select(database); From ce6f7db575e40343f30422fb17122f2363a329ec Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 28 Dec 2020 20:39:22 +0600 Subject: [PATCH 05/16] apply acl tests --- .../tests/JedisPoolWithCompleteCredentialsTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java index 3e06a39c1d..6c3a1b39fe 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java @@ -271,7 +271,7 @@ private int getClientCount(final String clientList) { @Test public void testResetInvalidPassword() { JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, - "default", "foobared", 0, "my_shiny_client_name") { }; + "acljedis", "fizzbuzz", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { Jedis obj1; @@ -286,7 +286,7 @@ public void testResetInvalidPassword() { assertSame(obj1, obj12); assertEquals(1, pool.getNumActive()); - factory.setPassword("default", "wrong password"); + factory.setPassword("acljedis", "wrong password"); try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); } catch (JedisConnectionException jce) { } @@ -299,7 +299,7 @@ public void testResetInvalidPassword() { @Test public void testResetValidPassword() { JedisFactory factory = new JedisFactory(hnp.getHost(), hnp.getPort(), 2000, 2000, - "default", "bad password", 0, "my_shiny_client_name") { }; + "acljedis", "bad password", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { try (Jedis obj1 = pool.getResource()) { @@ -308,11 +308,11 @@ public void testResetValidPassword() { assertEquals(0, pool.getNumActive()); try { - factory.setPassword("foobared"); + factory.setPassword("fizzbuzz"); fail(); } catch (IllegalArgumentException e) { } - factory.setPassword("default", "foobared"); + factory.setPassword("acljedis", "fizzbuzz"); try (Jedis obj2 = pool.getResource()) { obj2.set("foo", "bar"); assertEquals("bar", obj2.get("foo")); From d638c46d09330c4ebaeee908b22b2f6db229add9 Mon Sep 17 00:00:00 2001 From: yapei Date: Wed, 13 Sep 2017 16:10:00 -0500 Subject: [PATCH 06/16] Add ability to reset password in JedisFactory for JedisPool and JedisSentinelPool --- src/test/java/redis/clients/jedis/tests/JedisPoolTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index a394c043be..14cad75f66 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -433,7 +433,7 @@ public void testResetValidPassword() { try (Jedis obj2 = pool.getResource()) { obj2.set("foo", "bar"); assertEquals("bar", obj2.get("foo")); - } + } } } } From abf582353dc313e4de2a75db4f89673dae973c1a Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 13 Dec 2020 23:58:31 +0600 Subject: [PATCH 07/16] Modify JedisFactory tests and support ACL --- src/test/java/redis/clients/jedis/tests/JedisPoolTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index 14cad75f66..a394c043be 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -433,7 +433,7 @@ public void testResetValidPassword() { try (Jedis obj2 = pool.getResource()) { obj2.set("foo", "bar"); assertEquals("bar", obj2.get("foo")); - } + } } } } From 3ae050fc1c13af0b6b2b094c95b60822f5e5626c Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 16 Dec 2020 13:21:24 +0600 Subject: [PATCH 08/16] 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. --- .../redis/clients/jedis/JedisFactory.java | 17 ++++ .../java/redis/clients/jedis/JedisPool.java | 20 ++-- .../clients/jedis/JedisPoolAbstract.java | 3 + .../clients/jedis/JedisSentinelPool.java | 94 ++++++------------- .../java/redis/clients/jedis/util/Pool.java | 19 +++- 5 files changed, 72 insertions(+), 81 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index 1c8261d0b3..f06b04a5fd 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -47,6 +47,14 @@ protected JedisFactory(final String host, final int port, final int connectionTi this(host, port, connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName, false, null, null, null); } + /** + * {@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, @@ -65,7 +73,16 @@ protected JedisFactory(final String host, final int port, final int connectionTi 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; diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java index 72d86924c1..1ddf1e6b23 100644 --- a/src/main/java/redis/clients/jedis/JedisPool.java +++ b/src/main/java/redis/clients/jedis/JedisPool.java @@ -7,7 +7,6 @@ import javax.net.ssl.SSLSocketFactory; import org.apache.commons.pool2.PooledObjectFactory; -import org.apache.commons.pool2.impl.GenericObjectPool; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; import redis.clients.jedis.exceptions.JedisException; @@ -30,12 +29,10 @@ 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)); } } @@ -43,13 +40,12 @@ 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)); } } diff --git a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java index 9089bb5a53..797a3780dd 100644 --- a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java +++ b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java @@ -7,6 +7,9 @@ public class JedisPoolAbstract extends Pool { + /** + * Using this constructor means you have to set and initialize the internalPool yourself. + */ public JedisPoolAbstract() { super(); } diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java index 5210dbad33..81760f1191 100644 --- a/src/main/java/redis/clients/jedis/JedisSentinelPool.java +++ b/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 masterListeners = new HashSet<>(); - private volatile JedisFactory factory; private volatile HostAndPort currentHostMaster; private final Object initPoolLock = new Object(); @@ -156,15 +147,21 @@ public JedisSentinelPool(String masterName, Set 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 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 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; @@ -172,28 +169,7 @@ public JedisSentinelPool(String masterName, Set sentinels, this.sentinelClientName = sentinelClientName; HostAndPort master = initSentinels(sentinels, masterName); - initPool(master); - } - - public JedisSentinelPool(String masterName, Set sentinels, final GenericObjectPoolConfig poolConfig, final JedisFactory factory) { - this.poolConfig = poolConfig; - this.connectionTimeout = Protocol.DEFAULT_TIMEOUT; - this.soTimeout = Protocol.DEFAULT_TIMEOUT; - this.infiniteSoTimeout = 0; - this.user = null; - this.password = null; - this.database = Protocol.DEFAULT_DATABASE; - this.clientName = null; - this.sentinelConnectionTimeout = Protocol.DEFAULT_TIMEOUT; - this.sentinelSoTimeout = Protocol.DEFAULT_TIMEOUT; - this.sentinelUser = null; - this.sentinelPassword = null; - this.sentinelClientName = null; - this.factory = factory; - - HostAndPort master = initSentinels(sentinels, masterName); - initPool(poolConfig, factory); - initPool(master); + initMaster(master); } @Override @@ -209,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); } } } @@ -243,9 +211,7 @@ private HostAndPort initSentinels(Set 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) { @@ -269,14 +235,8 @@ private HostAndPort initSentinels(Set 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); } } @@ -406,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() { @@ -419,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 {}", diff --git a/src/main/java/redis/clients/jedis/util/Pool.java b/src/main/java/redis/clients/jedis/util/Pool.java index e461516e35..497e641339 100644 --- a/src/main/java/redis/clients/jedis/util/Pool.java +++ b/src/main/java/redis/clients/jedis/util/Pool.java @@ -12,7 +12,11 @@ import redis.clients.jedis.exceptions.JedisExhaustedPoolException; public abstract class Pool implements Closeable { - protected GenericObjectPool internalPool; + + /** + * This will be private in future update. + */ + protected volatile GenericObjectPool internalPool; /** * Using this constructor means you have to set and initialize the internalPool yourself. @@ -45,6 +49,17 @@ public void initPool(final GenericObjectPoolConfig poolConfig, PooledObjectFacto this.internalPool = new GenericObjectPool<>(factory, poolConfig); } + /** + * This call only clears idle instances, not borrowed instances. + */ + protected void clearInternalPool() { + try { + this.internalPool.clear(); + } catch (Exception e) { + throw new JedisException("Could not clear the pool", e); + } + } + public T getResource() { try { return internalPool.borrowObject(); @@ -102,7 +117,7 @@ protected void closeInternalPool() { throw new JedisException("Could not destroy the pool", e); } } - + /** * Returns the number of instances currently borrowed from this pool. * From 2649f4df5fdc26fc3ff89393a2008050032b2b10 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 12 Jan 2021 17:08:16 +0600 Subject: [PATCH 09/16] Adddress gkorland's suggestion --- src/main/java/redis/clients/jedis/util/Pool.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/redis/clients/jedis/util/Pool.java b/src/main/java/redis/clients/jedis/util/Pool.java index 497e641339..95531bea6a 100644 --- a/src/main/java/redis/clients/jedis/util/Pool.java +++ b/src/main/java/redis/clients/jedis/util/Pool.java @@ -16,7 +16,7 @@ public abstract class Pool implements Closeable { /** * This will be private in future update. */ - protected volatile GenericObjectPool internalPool; + protected GenericObjectPool internalPool; /** * Using this constructor means you have to set and initialize the internalPool yourself. @@ -117,7 +117,7 @@ protected void closeInternalPool() { throw new JedisException("Could not destroy the pool", e); } } - + /** * Returns the number of instances currently borrowed from this pool. * From ab4e2f89b80397e4ee0cf30a1014d78aa0e5b243 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Jan 2021 14:19:28 +0600 Subject: [PATCH 10/16] Deprecation and JavaDoc --- .../java/redis/clients/jedis/JedisFactory.java | 15 +++++++++++---- src/main/java/redis/clients/jedis/JedisPool.java | 15 +++++++++++++++ .../redis/clients/jedis/JedisPoolAbstract.java | 1 + src/main/java/redis/clients/jedis/util/Pool.java | 7 +++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index f06b04a5fd..0e9e4d17f3 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -133,13 +133,20 @@ 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 { - if (this.user != null) { - throw new IllegalArgumentException(); - } - this.password = password; + 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(); diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java index 1ddf1e6b23..7af4a60385 100644 --- a/src/main/java/redis/clients/jedis/JedisPool.java +++ b/src/main/java/redis/clients/jedis/JedisPool.java @@ -26,6 +26,12 @@ 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)) { @@ -36,6 +42,15 @@ public JedisPool(final String host) { } } + /** + * @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); diff --git a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java index 797a3780dd..c94dc210ef 100644 --- a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java +++ b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java @@ -10,6 +10,7 @@ public class JedisPoolAbstract extends Pool { /** * Using this constructor means you have to set and initialize the internalPool yourself. */ + @Deprecated public JedisPoolAbstract() { super(); } diff --git a/src/main/java/redis/clients/jedis/util/Pool.java b/src/main/java/redis/clients/jedis/util/Pool.java index 95531bea6a..26262c80ca 100644 --- a/src/main/java/redis/clients/jedis/util/Pool.java +++ b/src/main/java/redis/clients/jedis/util/Pool.java @@ -21,6 +21,7 @@ public abstract class Pool implements Closeable { /** * Using this constructor means you have to set and initialize the internalPool yourself. */ + @Deprecated public Pool() { } @@ -37,6 +38,12 @@ public boolean isClosed() { return this.internalPool.isClosed(); } + /** + * @param poolConfig + * @param factory + * @deprecated This could be private in future. + */ + @Deprecated public void initPool(final GenericObjectPoolConfig poolConfig, PooledObjectFactory factory) { if (this.internalPool != null) { From 614f60068dbf81bdecb0b8b444be2c500034e789 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Jan 2021 16:05:10 +0600 Subject: [PATCH 11/16] address change requests --- .../java/redis/clients/jedis/JedisPool.java | 8 ++--- .../clients/jedis/JedisPoolAbstract.java | 2 ++ .../java/redis/clients/jedis/util/Pool.java | 2 ++ .../clients/jedis/tests/JedisPoolTest.java | 31 +++++++++---------- .../JedisPoolWithCompleteCredentialsTest.java | 31 +++++++++---------- 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java index 7af4a60385..d684eed510 100644 --- a/src/main/java/redis/clients/jedis/JedisPool.java +++ b/src/main/java/redis/clients/jedis/JedisPool.java @@ -27,17 +27,17 @@ public JedisPool(String host, int port) { } /** - * @param 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 host) { - URI uri = URI.create(host); + public JedisPool(final String url) { + URI uri = URI.create(url); if (JedisURIHelper.isValid(uri)) { initPool(new GenericObjectPoolConfig(), new JedisFactory(uri, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null)); } else { - initPool(new GenericObjectPoolConfig(), new JedisFactory(host, Protocol.DEFAULT_PORT, + initPool(new GenericObjectPoolConfig(), new JedisFactory(url, Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null)); } } diff --git a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java index c94dc210ef..d48d489433 100644 --- a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java +++ b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java @@ -9,6 +9,8 @@ public class JedisPoolAbstract extends Pool { /** * Using this constructor means you have to set and initialize the internalPool yourself. + * + * @deprecated */ @Deprecated public JedisPoolAbstract() { diff --git a/src/main/java/redis/clients/jedis/util/Pool.java b/src/main/java/redis/clients/jedis/util/Pool.java index 26262c80ca..c9a3d2e5bd 100644 --- a/src/main/java/redis/clients/jedis/util/Pool.java +++ b/src/main/java/redis/clients/jedis/util/Pool.java @@ -20,6 +20,8 @@ public abstract class Pool implements Closeable { /** * Using this constructor means you have to set and initialize the internalPool yourself. + * + * @deprecated */ @Deprecated public Pool() { diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index a394c043be..9f6b888d87 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -391,24 +391,23 @@ public void testResetInvalidPassword() { "foobared", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { - Jedis obj1; - try (Jedis obj11 = pool.getResource()) { - obj1 = obj11; - obj11.set("foo", "bar"); - assertEquals("bar", obj11.get("foo")); + Jedis obj1_ref; + try (Jedis obj1_1 = pool.getResource()) { + obj1_ref = obj1_1; + obj1_1.set("foo", "bar"); + assertEquals("bar", obj1_1.get("foo")); assertEquals(1, pool.getNumActive()); } assertEquals(0, pool.getNumActive()); - Jedis obj12 = pool.getResource(); - assertSame(obj1, obj12); - assertEquals(1, pool.getNumActive()); - - factory.setPassword("wrong password"); - try (Jedis obj2 = pool.getResource()) { - fail("Should not get resource from pool"); - } catch (JedisConnectionException e) { } - assertEquals(1, pool.getNumActive()); - obj12.close(); + try (Jedis obj1_2 = pool.getResource()) { + assertSame(obj1_ref, obj1_2); + assertEquals(1, pool.getNumActive()); + factory.setPassword("wrong password"); + try (Jedis obj2 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException e) { } + assertEquals(1, pool.getNumActive()); + } assertEquals(0, pool.getNumActive()); } } @@ -426,7 +425,7 @@ public void testResetValidPassword() { try { factory.setPassword("default", "foobared"); - fail(); + fail("Factory created without user. Setting password for user='default' should fail."); } catch (IllegalArgumentException e) { } factory.setPassword("foobared"); diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java index 6c3a1b39fe..1a34f9e7a9 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java @@ -274,24 +274,23 @@ public void testResetInvalidPassword() { "acljedis", "fizzbuzz", 0, "my_shiny_client_name") { }; try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { - Jedis obj1; - try (Jedis obj11 = pool.getResource()) { - obj1 = obj11; - obj11.set("foo", "bar"); - assertEquals("bar", obj11.get("foo")); + Jedis obj1_ref; + try (Jedis obj1_1 = pool.getResource()) { + obj1_ref = obj1_1; + obj1_1.set("foo", "bar"); + assertEquals("bar", obj1_1.get("foo")); assertEquals(1, pool.getNumActive()); } assertEquals(0, pool.getNumActive()); - Jedis obj12 = pool.getResource(); - assertSame(obj1, obj12); - assertEquals(1, pool.getNumActive()); - - factory.setPassword("acljedis", "wrong password"); - try (Jedis obj2 = pool.getResource()) { - fail("Should not get resource from pool"); - } catch (JedisConnectionException jce) { } - assertEquals(1, pool.getNumActive()); - obj12.close(); + try (Jedis obj1_2 = pool.getResource()) { + assertSame(obj1_ref, obj1_2); + assertEquals(1, pool.getNumActive()); + factory.setPassword("acljedis", "wrong password"); + try (Jedis obj2 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException jce) { } + assertEquals(1, pool.getNumActive()); + } assertEquals(0, pool.getNumActive()); } } @@ -309,7 +308,7 @@ public void testResetValidPassword() { try { factory.setPassword("fizzbuzz"); - fail(); + fail("Factory created with user='fizzbuzz'. Setting password without user should fail."); } catch (IllegalArgumentException e) { } factory.setPassword("acljedis", "fizzbuzz"); From 1385a9caffb8e1f49f0036515fbe48b539df1f6e Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Jan 2021 16:16:58 +0600 Subject: [PATCH 12/16] address change requests --- .../jedis/tests/JedisSentinelPoolTest.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index 33860e3f6d..47cb5fd4a4 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java @@ -176,29 +176,28 @@ public void customClientName() { assertTrue(pool.isClosed()); } - + @Test public void testResetInvalidPassword() { JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "foobared", 0, "my_shiny_client_name") { }; try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, new JedisPoolConfig(), factory)) { - Jedis obj1; - try (Jedis obj11 = pool.getResource()) { - obj1 = obj11; - obj11.set("foo", "bar"); - assertEquals("bar", obj11.get("foo")); + Jedis obj1_ref; + try (Jedis obj1_1 = pool.getResource()) { + obj1_ref = obj1_1; + obj1_1.set("foo", "bar"); + assertEquals("bar", obj1_1.get("foo")); + } + try (Jedis obj1_2 = pool.getResource()) { + assertSame(obj1_ref, obj1_2); + factory.setPassword("wrong password"); + try (Jedis obj2 = pool.getResource()) { + fail("Should not get resource from pool"); + } catch (JedisConnectionException e) { } } - Jedis obj12 = pool.getResource(); - assertSame(obj1, obj12); - - factory.setPassword("wrong password"); - try (Jedis obj2 = pool.getResource()) { - fail("Should not get resource from pool"); - } catch (JedisConnectionException e) { } - obj12.close(); } } - + @Test public void testResetValidPassword() { JedisFactory factory = new JedisFactory(null, 0, 2000, 2000, "wrong password", 0, "my_shiny_client_name") { }; From aa1395b5d5a2d6c7a98b3d933904255acecee0b8 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 10 Mar 2021 18:38:32 +0600 Subject: [PATCH 13/16] Unbreak (both JedisFactory and JedisSentinelPool are errored) --- .../clients/jedis/JedisSentinelPool.java | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java index 4c0dabccbf..142bdd478b 100644 --- a/src/main/java/redis/clients/jedis/JedisSentinelPool.java +++ b/src/main/java/redis/clients/jedis/JedisSentinelPool.java @@ -22,13 +22,22 @@ public class JedisSentinelPool extends JedisPoolAbstract { protected static Logger log = LoggerFactory.getLogger(JedisSentinelPool.class); protected final GenericObjectPoolConfig poolConfig; - protected final JedisFactory factory; + private final JedisFactory factory; - protected int sentinelConnectionTimeout; - protected int sentinelSoTimeout; - protected String sentinelUser; - protected String sentinelPassword; - protected String sentinelClientName; + @Deprecated protected int connectionTimeout; + @Deprecated protected int soTimeout; + @Deprecated protected int infiniteSoTimeout; + + @Deprecated protected String user; + @Deprecated protected String password; + @Deprecated protected int database; + @Deprecated protected String clientName; + + @Deprecated protected int sentinelConnectionTimeout; + @Deprecated protected int sentinelSoTimeout; + @Deprecated protected String sentinelUser; + @Deprecated protected String sentinelPassword; + @Deprecated protected String sentinelClientName; protected final Set masterListeners = new HashSet<>(); @@ -152,26 +161,26 @@ public JedisSentinelPool(String masterName, Set sentinels, 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 sentinels, final GenericObjectPoolConfig poolConfig, final JedisFactory factory) { - this(masterName, sentinels, poolConfig, factory, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, null, null); + this.connectionTimeout = connectionTimeout; + this.soTimeout = soTimeout; + this.infiniteSoTimeout = infiniteSoTimeout; + this.user = user; + this.password = password; + this.database = database; + this.clientName = clientName; + this.sentinelConnectionTimeout = sentinelConnectionTimeout; + this.sentinelSoTimeout = sentinelSoTimeout; + this.sentinelUser = sentinelUser; + this.sentinelPassword = sentinelPassword; + this.sentinelClientName = sentinelClientName; } public JedisSentinelPool(String masterName, Set sentinels, - final GenericObjectPoolConfig poolConfig, final JedisFactory factory, - final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser, - final String sentinelPassword, final String sentinelClientName) { + final GenericObjectPoolConfig poolConfig, final JedisFactory factory) { super(poolConfig, factory); this.poolConfig = poolConfig; this.factory = factory; - this.sentinelConnectionTimeout = sentinelConnectionTimeout; - this.sentinelSoTimeout = sentinelSoTimeout; - this.sentinelUser = sentinelUser; - this.sentinelPassword = sentinelPassword; - this.sentinelClientName = sentinelClientName; - HostAndPort master = initSentinels(sentinels, masterName); initMaster(master); } From dba6f844f0d65855bf9f8bcbd417c473327a987b Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 14 Mar 2021 20:32:07 +0600 Subject: [PATCH 14/16] update with config pattern --- .../jedis/DefaultJedisClientConfig.java | 18 +++++++++++++++++- .../clients/jedis/JedisClientConfig.java | 3 +++ .../redis/clients/jedis/JedisFactory.java | 19 +++---------------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index f3a0d79cf6..fc34f573e1 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -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; @@ -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; @@ -65,6 +66,21 @@ public String getPassword() { return 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 + */ + @Override + public synchronized void updatePassword(String user, String password) { + if (!Objects.equals(this.user, user)) { + throw new IllegalArgumentException(); + } + if (!Objects.equals(this.password, password)) { + this.password = password; + } + } + @Override public int getDatabase() { return database; diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index a08cac6da2..b483d5de0d 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -39,6 +39,9 @@ default String getPassword() { return null; } + default void updatePassword(String user, String password) { + } + default int getDatabase() { return Protocol.DEFAULT_DATABASE; } diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index 6da4c1f390..c34ba26aac 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -122,25 +122,12 @@ 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 { + public void setPassword(final String password) { 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; + public void setPassword(final String user, final String password) { + this.config.updatePassword(user, password); } @Override From 2c3a206409125d6a276811e82e56a7ff2839ee7c Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 15 Mar 2021 12:34:40 +0600 Subject: [PATCH 15/16] Remove checking of 'user' while updating password --- .../redis/clients/jedis/DefaultJedisClientConfig.java | 10 +--------- .../java/redis/clients/jedis/JedisClientConfig.java | 2 +- src/main/java/redis/clients/jedis/JedisFactory.java | 6 +----- .../java/redis/clients/jedis/tests/JedisPoolTest.java | 5 ----- .../tests/JedisPoolWithCompleteCredentialsTest.java | 9 ++------- 5 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index fc34f573e1..f615ca5e34 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -66,16 +66,8 @@ public String getPassword() { return 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 - */ @Override - public synchronized void updatePassword(String user, String password) { - if (!Objects.equals(this.user, user)) { - throw new IllegalArgumentException(); - } + public synchronized void updatePassword(String password) { if (!Objects.equals(this.password, password)) { this.password = password; } diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index b483d5de0d..e776aaf0d6 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -39,7 +39,7 @@ default String getPassword() { return null; } - default void updatePassword(String user, String password) { + default void updatePassword(String password) { } default int getDatabase() { diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java index c34ba26aac..c9ad312f95 100644 --- a/src/main/java/redis/clients/jedis/JedisFactory.java +++ b/src/main/java/redis/clients/jedis/JedisFactory.java @@ -123,11 +123,7 @@ public void setHostAndPort(final HostAndPort hostAndPort) { } public void setPassword(final String password) { - setPassword(null, password); - } - - public void setPassword(final String user, final String password) { - this.config.updatePassword(user, password); + this.config.updatePassword(password); } @Override diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index 680bde9706..39777884be 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -423,11 +423,6 @@ public void testResetValidPassword() { } catch (JedisConnectionException e) { } assertEquals(0, pool.getNumActive()); - try { - factory.setPassword("default", "foobared"); - fail("Factory created without user. Setting password for user='default' should fail."); - } catch (IllegalArgumentException e) { } - factory.setPassword("foobared"); try (Jedis obj2 = pool.getResource()) { obj2.set("foo", "bar"); diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java index bd04c16971..173a0b3790 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolWithCompleteCredentialsTest.java @@ -296,7 +296,7 @@ public void testResetInvalidPassword() { try (Jedis obj1_2 = pool.getResource()) { assertSame(obj1_ref, obj1_2); assertEquals(1, pool.getNumActive()); - factory.setPassword("acljedis", "wrong password"); + factory.setPassword("wrong password"); try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); } catch (JedisConnectionException jce) { } @@ -317,12 +317,7 @@ public void testResetValidPassword() { } catch (JedisConnectionException e) { } assertEquals(0, pool.getNumActive()); - try { - factory.setPassword("fizzbuzz"); - fail("Factory created with user='fizzbuzz'. Setting password without user should fail."); - } catch (IllegalArgumentException e) { } - - factory.setPassword("acljedis", "fizzbuzz"); + factory.setPassword("fizzbuzz"); try (Jedis obj2 = pool.getResource()) { obj2.set("foo", "bar"); assertEquals("bar", obj2.get("foo")); From 864169d204b65d7b755e860ce997b6a65e20fa5a Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 17 Mar 2021 20:18:57 +0600 Subject: [PATCH 16/16] update deprecation messages --- src/main/java/redis/clients/jedis/JedisPoolAbstract.java | 2 +- src/main/java/redis/clients/jedis/util/Pool.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java index e879c9ed8a..42623d4a1e 100644 --- a/src/main/java/redis/clients/jedis/JedisPoolAbstract.java +++ b/src/main/java/redis/clients/jedis/JedisPoolAbstract.java @@ -15,7 +15,7 @@ public class JedisPoolAbstract extends Pool { /** * Using this constructor means you have to set and initialize the internalPool yourself. * - * @deprecated + * @deprecated This constructor will be removed in future. */ @Deprecated public JedisPoolAbstract() { diff --git a/src/main/java/redis/clients/jedis/util/Pool.java b/src/main/java/redis/clients/jedis/util/Pool.java index 09634ca020..57e8b87e2f 100644 --- a/src/main/java/redis/clients/jedis/util/Pool.java +++ b/src/main/java/redis/clients/jedis/util/Pool.java @@ -22,7 +22,7 @@ public abstract class Pool implements Closeable { /** * Using this constructor means you have to set and initialize the internalPool yourself. * - * @deprecated This will be removed in future. + * @deprecated This constructor will be removed in future. */ @Deprecated public Pool() { @@ -44,7 +44,7 @@ public boolean isClosed() { /** * @param poolConfig * @param factory - * @deprecated This will be private in future. + * @deprecated This method will be private in future. */ @Deprecated public void initPool(final GenericObjectPoolConfig poolConfig, PooledObjectFactory factory) {