diff --git a/src/main/java/redis/clients/jedis/Jedis.java b/src/main/java/redis/clients/jedis/Jedis.java index f6cf43f167..14afd37ab4 100644 --- a/src/main/java/redis/clients/jedis/Jedis.java +++ b/src/main/java/redis/clients/jedis/Jedis.java @@ -47,6 +47,7 @@ public class Jedis implements ServerCommands, DatabaseCommands, JedisCommands, J protected static final byte[][] DUMMY_ARRAY = new byte[0][]; private Pool dataSource = null; + private Pool connectionSource = null; public Jedis() { connection = new Connection(); @@ -297,9 +298,24 @@ public void resetState() { } protected void setDataSource(Pool jedisPool) { + assertNotPooled(); this.dataSource = jedisPool; } + protected void setConnectionSource(Pool connectionPool) { + assertNotPooled(); + this.connectionSource = connectionPool; + } + + private void assertNotPooled() { + if (connectionSource != null) { + throw new IllegalStateException("Connection source is already set."); + } + if (dataSource != null) { + throw new IllegalStateException("Data source is already set."); + } + } + @Override public void close() { if (dataSource != null) { @@ -310,6 +326,14 @@ public void close() { } else { pool.returnResource(this); } + } else if (connectionSource != null) { + Pool pool = connectionSource; + this.connectionSource = null; + if (isBroken()) { + pool.returnBrokenResource(connection); + } else { + pool.returnResource(connection); + } } else { connection.close(); } diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java index 411247da69..cd10131c82 100644 --- a/src/main/java/redis/clients/jedis/JedisPool.java +++ b/src/main/java/redis/clients/jedis/JedisPool.java @@ -1,8 +1,6 @@ package redis.clients.jedis; import java.net.URI; -import java.util.function.Consumer; -import java.util.function.Function; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocketFactory; @@ -16,7 +14,7 @@ import redis.clients.jedis.util.Pool; // Legacy -public class JedisPool extends Pool { +public class JedisPool extends Pool implements JedisProvider { private static final Logger log = LoggerFactory.getLogger(JedisPool.class); @@ -395,16 +393,5 @@ public void returnResource(final Jedis resource) { } } - public void withResource(Consumer consumer) { - try (Jedis jedis = this.getResource()) { - consumer.accept(jedis); - } - } - - public K withResourceGet(Function function) { - try (Jedis jedis = this.getResource()) { - return function.apply(jedis); - } - } } diff --git a/src/main/java/redis/clients/jedis/JedisPooled.java b/src/main/java/redis/clients/jedis/JedisPooled.java index ece0341977..10a4b84974 100644 --- a/src/main/java/redis/clients/jedis/JedisPooled.java +++ b/src/main/java/redis/clients/jedis/JedisPooled.java @@ -18,7 +18,7 @@ import redis.clients.jedis.util.JedisURIHelper; import redis.clients.jedis.util.Pool; -public class JedisPooled extends UnifiedJedis { +public class JedisPooled extends UnifiedJedis implements JedisProvider { public JedisPooled() { this(Protocol.DEFAULT_HOST, Protocol.DEFAULT_PORT); @@ -463,4 +463,20 @@ public final Pool getPool() { public Pipeline pipelined() { return (Pipeline) super.pipelined(); } + + @Override + public Jedis getResource() { + Pool pool = getPool(); + Connection conn = pool.getResource(); + Jedis jedis = new Jedis(conn); + jedis.setConnectionSource(pool); + return jedis; + } + + @Override + public void returnResource(final Jedis resource) { + Pool pool = getPool(); + pool.returnResource(resource.getConnection()); + } + } diff --git a/src/main/java/redis/clients/jedis/JedisProvider.java b/src/main/java/redis/clients/jedis/JedisProvider.java new file mode 100644 index 0000000000..3a87dc13a7 --- /dev/null +++ b/src/main/java/redis/clients/jedis/JedisProvider.java @@ -0,0 +1,44 @@ +package redis.clients.jedis; + +import java.util.function.Consumer; +import java.util.function.Function; + +/** + * Interface providing Jedis resources from a pool + */ +public interface JedisProvider { + + /** + * Get a Jedis resource from the pool + * @return Jedis object + */ + Jedis getResource(); + + /** + * Return a Jedis resource to the pool + * @param resource Jedis object + */ + void returnResource(final Jedis resource); + + /** + * Execute a consumer with a Jedis resource + * @param consumer code to execute with the Jedis resource + */ + default void withResource(Consumer consumer) { + try (Jedis jedis = this.getResource()) { + consumer.accept(jedis); + } + } + + /** + * Execute a function with a Jedis resource and return a value + * @param function code to execute with the Jedis resource + * @return return value from the function + */ + default K withResourceGet(Function function) { + try (Jedis jedis = this.getResource()) { + return function.apply(jedis); + } + } + +} diff --git a/src/test/java/redis/clients/jedis/JedisPoolTest.java b/src/test/java/redis/clients/jedis/JedisPoolTest.java index 77be4005e9..16582ac793 100644 --- a/src/test/java/redis/clients/jedis/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/JedisPoolTest.java @@ -33,9 +33,11 @@ @Tag("integration") public class JedisPoolTest { - private static final EndpointConfig endpointStandalone0 = HostAndPorts.getRedisEndpoint("standalone0"); + private static final EndpointConfig endpointStandalone0 = HostAndPorts + .getRedisEndpoint("standalone0"); - private static final EndpointConfig endpointStandalone1 = HostAndPorts.getRedisEndpoint("standalone1"); + private static final EndpointConfig endpointStandalone1 = HostAndPorts + .getRedisEndpoint("standalone1"); private String testKey; private String testValue; @@ -48,7 +50,8 @@ public void setUpTestKey(TestInfo testInfo) { @Test public void checkConnections() { - JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000); + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000); try (Jedis jedis = pool.getResource()) { jedis.auth(endpointStandalone0.getPassword()); jedis.set("foo", "bar"); @@ -60,7 +63,8 @@ public void checkConnections() { @Test public void checkResourceWithConfig() { - try (JedisPool pool = new JedisPool(HostAndPorts.getRedisEndpoint("standalone7-with-lfu-policy").getHostAndPort(), + try (JedisPool pool = new JedisPool( + HostAndPorts.getRedisEndpoint("standalone7-with-lfu-policy").getHostAndPort(), DefaultJedisClientConfig.builder().socketTimeoutMillis(5000).build())) { try (Jedis jedis = pool.getResource()) { @@ -72,7 +76,8 @@ public void checkResourceWithConfig() { @Test public void checkCloseableConnections() throws Exception { - JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000); + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000); try (Jedis jedis = pool.getResource()) { jedis.auth(endpointStandalone0.getPassword()); jedis.set("foo", "bar"); @@ -99,7 +104,8 @@ public void checkResourceIsClosableAndReusable() { GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setMaxTotal(1); config.setBlockWhenExhausted(false); - try (JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword(), 0, + try (JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword(), 0, "closable-reusable-pool", false, null, null, null)) { Jedis jedis = pool.getResource(); @@ -115,7 +121,8 @@ public void checkResourceIsClosableAndReusable() { @Test public void checkPoolRepairedWhenJedisIsBroken() { - JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort()); + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort()); try (Jedis jedis = pool.getResource()) { jedis.auth(endpointStandalone0.getPassword()); jedis.set("foo", "0"); @@ -135,7 +142,9 @@ public void checkPoolOverflow() { GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setMaxTotal(1); config.setBlockWhenExhausted(false); - try (JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), endpointStandalone0.getPort()); + try ( + JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), + endpointStandalone0.getPort()); Jedis jedis = pool.getResource()) { jedis.auth(endpointStandalone0.getPassword()); @@ -147,7 +156,8 @@ public void checkPoolOverflow() { public void securePool() { JedisPoolConfig config = new JedisPoolConfig(); config.setTestOnBorrow(true); - JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword()); + JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword()); try (Jedis jedis = pool.getResource()) { jedis.set("foo", "bar"); } @@ -157,14 +167,18 @@ public void securePool() { @Test public void nonDefaultDatabase() { - try (JedisPool pool0 = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, - endpointStandalone0.getPassword()); Jedis jedis0 = pool0.getResource()) { + try ( + JedisPool pool0 = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword()); + Jedis jedis0 = pool0.getResource()) { jedis0.set("foo", "bar"); assertEquals("bar", jedis0.get("foo")); } - try (JedisPool pool1 = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, - endpointStandalone0.getPassword(), 1); Jedis jedis1 = pool1.getResource()) { + try ( + JedisPool pool1 = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword(), 1); + Jedis jedis1 = pool1.getResource()) { assertNull(jedis1.get("foo")); } } @@ -177,8 +191,9 @@ public void startWithUrlString() { j.set("foo", "bar"); } - try (JedisPool pool = new JedisPool( - endpointStandalone1.getURIBuilder().credentials("", endpointStandalone1.getPassword()).path("/2").build()); + try ( + JedisPool pool = new JedisPool(endpointStandalone1.getURIBuilder() + .credentials("", endpointStandalone1.getPassword()).path("/2").build()); Jedis jedis = pool.getResource()) { assertEquals("PONG", jedis.ping()); assertEquals("bar", jedis.get("foo")); @@ -193,8 +208,9 @@ public void startWithUrl() throws URISyntaxException { j.set("foo", "bar"); } - try (JedisPool pool = new JedisPool( - endpointStandalone1.getURIBuilder().credentials("", endpointStandalone1.getPassword()).path("/2").build()); + try ( + JedisPool pool = new JedisPool(endpointStandalone1.getURIBuilder() + .credentials("", endpointStandalone1.getPassword()).path("/2").build()); Jedis jedis = pool.getResource()) { assertEquals("bar", jedis.get("foo")); } @@ -202,7 +218,7 @@ public void startWithUrl() throws URISyntaxException { @Test public void shouldThrowInvalidURIExceptionForInvalidURI() throws URISyntaxException { - assertThrows(InvalidURIException.class, ()->new JedisPool(new URI("localhost:6380")).close()); + assertThrows(InvalidURIException.class, () -> new JedisPool(new URI("localhost:6380")).close()); } @Test @@ -213,8 +229,8 @@ public void allowUrlWithNoDBAndNoPassword() throws URISyntaxException { @Test public void selectDatabaseOnActivation() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, - endpointStandalone0.getPassword())) { + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword())) { Jedis jedis0 = pool.getResource(); assertEquals(0, jedis0.getDB()); @@ -234,8 +250,11 @@ public void selectDatabaseOnActivation() { @Test public void customClientName() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, - endpointStandalone0.getPassword(), 0, "my_shiny_client_name"); Jedis jedis = pool.getResource()) { + try ( + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword(), 0, + "my_shiny_client_name"); + Jedis jedis = pool.getResource()) { assertEquals("my_shiny_client_name", jedis.clientGetname()); } @@ -243,11 +262,14 @@ public void customClientName() { @Test public void invalidClientName() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, - endpointStandalone0.getPassword(), 0, "invalid client name"); Jedis jedis = pool.getResource()) { + try ( + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword(), 0, + "invalid client name"); + Jedis jedis = pool.getResource()) { } catch (Exception e) { if (!e.getMessage().startsWith("client info cannot contain space")) { - fail("invalid client name test fail"); + fail("invalid client name test fail"); } } } @@ -308,7 +330,8 @@ public void returnResourceShouldResetState() { GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setMaxTotal(1); config.setBlockWhenExhausted(false); - JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword()); + JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword()); Jedis jedis = pool.getResource(); try { @@ -328,10 +351,37 @@ public void returnResourceShouldResetState() { assertTrue(pool.isClosed()); } + @Test + public void returnResourceShouldResetStateWithGetResource() { + GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); + config.setMaxTotal(1); + config.setBlockWhenExhausted(false); + JedisPool pool = new JedisPool(config, endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword()); + + Jedis jedis = pool.getResource(); + try { + jedis.set("hello", "jedis"); + Transaction t = jedis.multi(); + t.set("hello", "world"); + } finally { + jedis.close(); + } + + pool.withResource(jedis2 -> { + assertSame(jedis, jedis2); + assertEquals("jedis", jedis2.get("hello")); + }); + + pool.close(); + assertTrue(pool.isClosed()); + } + @Test public void getNumActiveWhenPoolIsClosed() { - JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000, - endpointStandalone0.getPassword(), 0, "my_shiny_client_name"); + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, endpointStandalone0.getPassword(), 0, + "my_shiny_client_name"); try (Jedis j = pool.getResource()) { j.ping(); @@ -343,7 +393,8 @@ public void getNumActiveWhenPoolIsClosed() { @Test public void getNumActiveReturnsTheCorrectNumber() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000)) { + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000)) { Jedis jedis = pool.getResource(); jedis.auth(endpointStandalone0.getPassword()); jedis.set("foo", "bar"); @@ -368,7 +419,8 @@ public void getNumActiveReturnsTheCorrectNumber() { @Test public void testAddObject() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000)) { + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000)) { pool.addObjects(1); assertEquals(1, pool.getNumIdle()); } @@ -376,7 +428,8 @@ public void testAddObject() { @Test public void closeResourceTwice() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000)) { + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000)) { Jedis j = pool.getResource(); j.auth(endpointStandalone0.getPassword()); j.ping(); @@ -387,7 +440,8 @@ public void closeResourceTwice() { @Test public void closeBrokenResourceTwice() { - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), endpointStandalone0.getPort(), 2000)) { + try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000)) { Jedis j = pool.getResource(); try { // make connection broken @@ -406,8 +460,9 @@ public void closeBrokenResourceTwice() { public void testCloseConnectionOnMakeObject() { JedisPoolConfig config = new JedisPoolConfig(); config.setTestOnBorrow(true); - try (JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), - endpointStandalone0.getPort(), 2000, "wrong pass"); + try ( + JedisPool pool = new JedisPool(new JedisPoolConfig(), endpointStandalone0.getHost(), + endpointStandalone0.getPort(), 2000, "wrong pass"); Jedis jedis = new Jedis(endpointStandalone0.getURIBuilder().defaultCredentials().build())) { int currentClientCount = getClientCount(jedis.clientList()); assertThrows(JedisAccessControlException.class, pool::getResource); @@ -424,10 +479,11 @@ private int getClientCount(final String clientList) { @Test public void testResetInvalidCredentials() { - DefaultRedisCredentialsProvider credentialsProvider - = new DefaultRedisCredentialsProvider(new DefaultRedisCredentials(null, endpointStandalone0.getPassword())); - JedisFactory factory = new JedisFactory(endpointStandalone0.getHostAndPort(), DefaultJedisClientConfig.builder() - .credentialsProvider(credentialsProvider).clientName("my_shiny_client_name").build()); + DefaultRedisCredentialsProvider credentialsProvider = new DefaultRedisCredentialsProvider( + new DefaultRedisCredentials(null, endpointStandalone0.getPassword())); + JedisFactory factory = new JedisFactory(endpointStandalone0.getHostAndPort(), + DefaultJedisClientConfig.builder().credentialsProvider(credentialsProvider) + .clientName("my_shiny_client_name").build()); try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { Jedis obj1_ref; @@ -445,7 +501,7 @@ public void testResetInvalidCredentials() { try (Jedis obj2 = pool.getResource()) { fail("Should not get resource from pool"); } catch (JedisException e) { - //ignore + // ignore } assertEquals(1, pool.getNumActive()); } @@ -455,20 +511,22 @@ public void testResetInvalidCredentials() { @Test public void testResetValidCredentials() { - DefaultRedisCredentialsProvider credentialsProvider - = new DefaultRedisCredentialsProvider(new DefaultRedisCredentials(null, "bad password")); - JedisFactory factory = new JedisFactory(endpointStandalone0.getHostAndPort(), DefaultJedisClientConfig.builder() - .credentialsProvider(credentialsProvider).clientName("my_shiny_client_name").build()); + DefaultRedisCredentialsProvider credentialsProvider = new DefaultRedisCredentialsProvider( + new DefaultRedisCredentials(null, "bad password")); + JedisFactory factory = new JedisFactory(endpointStandalone0.getHostAndPort(), + DefaultJedisClientConfig.builder().credentialsProvider(credentialsProvider) + .clientName("my_shiny_client_name").build()); try (JedisPool pool = new JedisPool(new JedisPoolConfig(), factory)) { try (Jedis obj1 = pool.getResource()) { fail("Should not get resource from pool"); } catch (JedisException e) { - //ignore + // ignore } assertEquals(0, pool.getNumActive()); - credentialsProvider.setCredentials(new DefaultRedisCredentials(null, endpointStandalone0.getPassword())); + credentialsProvider + .setCredentials(new DefaultRedisCredentials(null, endpointStandalone0.getPassword())); try (Jedis obj2 = pool.getResource()) { obj2.set("foo", "bar"); assertEquals("bar", obj2.get("foo")); diff --git a/src/test/java/redis/clients/jedis/JedisPooledTest.java b/src/test/java/redis/clients/jedis/JedisPooledTest.java index a7dead0b01..2b501d324c 100644 --- a/src/test/java/redis/clients/jedis/JedisPooledTest.java +++ b/src/test/java/redis/clients/jedis/JedisPooledTest.java @@ -6,6 +6,7 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -24,10 +25,10 @@ @Tag("integration") public class JedisPooledTest { - private static final EndpointConfig endpointStandalone7 = HostAndPorts.getRedisEndpoint( - "standalone7-with-lfu-policy"); - private static final EndpointConfig endpointStandalone1 = HostAndPorts.getRedisEndpoint( - "standalone1"); // password protected + private static final EndpointConfig endpointStandalone7 = HostAndPorts + .getRedisEndpoint("standalone7-with-lfu-policy"); + private static final EndpointConfig endpointStandalone1 = HostAndPorts + .getRedisEndpoint("standalone1"); // password protected @Test public void checkCloseableConnections() { @@ -169,6 +170,27 @@ public void getNumActiveReturnsTheCorrectNumber() { } } + @Test + public void getNumActiveReturnsTheCorrectNumberWithJedis() { + try (JedisPooled pool = JedisPooled.builder() + .hostAndPort(endpointStandalone7.getHost(), endpointStandalone7.getPort()) + .clientConfig(DefaultJedisClientConfig.builder().timeoutMillis(2000).build()) + .poolConfig(new ConnectionPoolConfig()).build()) { + + Connection jedis = pool.getPool().getResource(); + assertEquals(1, pool.getPool().getNumActive()); + + Jedis jedis2 = pool.getResource(); + assertEquals(2, pool.getPool().getNumActive()); + + jedis.close(); + assertEquals(1, pool.getPool().getNumActive()); + + jedis2.close(); + assertEquals(0, pool.getPool().getNumActive()); + } + } + @Test public void closeResourceTwice() { try (JedisPooled pool = JedisPooled.builder() @@ -204,8 +226,8 @@ public void closeBrokenResourceTwice() { @Test public void testResetValidCredentials() { - DefaultRedisCredentialsProvider credentialsProvider = - new DefaultRedisCredentialsProvider(new DefaultRedisCredentials(null, "bad password")); + DefaultRedisCredentialsProvider credentialsProvider = new DefaultRedisCredentialsProvider( + new DefaultRedisCredentials(null, "bad password")); try (JedisPooled pool = JedisPooled.builder().hostAndPort(endpointStandalone1.getHostAndPort()) .clientConfig( @@ -214,14 +236,65 @@ public void testResetValidCredentials() { try { pool.get("foo"); fail("Should not get resource from pool"); - } catch (JedisException e) { } + } catch (JedisException e) { + } assertEquals(0, pool.getPool().getNumActive()); - credentialsProvider.setCredentials(new DefaultRedisCredentials(null, endpointStandalone1.getPassword())); + credentialsProvider + .setCredentials(new DefaultRedisCredentials(null, endpointStandalone1.getPassword())); assertThat(pool.get("foo"), anything()); } } + @Test + public void testWithJedisPoolDoWithConnection() { + try (JedisPooled pool = JedisPooled.builder() + .fromURI(endpointStandalone1.getURIBuilder() + .credentials("", endpointStandalone1.getPassword()).path("/2").build().toString()) + .build();) { + pool.withResource(jedis -> { + jedis.auth(endpointStandalone1.getPassword()); + jedis.set("foo", "bar"); + assertEquals("bar", jedis.get("foo")); + }); + pool.withResource(jedis -> assertNotNull(jedis.ping())); + } + } + + @Test + public void testWithJedisPoolGetWithConnection() { + try (JedisPooled pool = JedisPooled.builder() + .fromURI(endpointStandalone1.getURIBuilder() + .credentials("", endpointStandalone1.getPassword()).path("/2").build().toString()) + .build();) { + String result = pool.withResourceGet(jedis -> { + jedis.auth(endpointStandalone1.getPassword()); + jedis.set("foo", "bar"); + return jedis.get("foo"); + }); + String ping = pool.withResourceGet(Jedis::ping); + assertEquals("bar", result); + assertNotNull(ping); + } + } + + @Test + public void testWithJedisPoolGetWithReturnConnection() { + try (JedisPooled pool = JedisPooled.builder() + .fromURI(endpointStandalone1.getURIBuilder() + .credentials("", endpointStandalone1.getPassword()).path("/2").build().toString()) + .build();) { + Jedis jedis = pool.getResource(); + jedis.auth(endpointStandalone1.getPassword()); + jedis.set("foo", "bar"); + String result = jedis.get("foo"); + String ping = pool.withResourceGet(Jedis::ping); + pool.returnResource(jedis); + assertEquals("bar", result); + assertNotNull(ping); + } + } + @Test public void testCredentialsProvider() { final AtomicInteger prepareCount = new AtomicInteger(); @@ -265,7 +338,8 @@ public void cleanUp() { } }; - // TODO: do it without the help of pool config; from Connection constructor? (configurable) force ping? + // TODO: do it without the help of pool config; from Connection constructor? (configurable) + // force ping? GenericObjectPoolConfig poolConfig = new GenericObjectPoolConfig<>(); poolConfig.setMaxTotal(1); poolConfig.setTestOnBorrow(true); @@ -277,9 +351,10 @@ public void cleanUp() { pool.get("foo"); fail("Should not get resource from pool"); } catch (JedisException e) { - //ignore + // ignore } - assertEquals(0, pool.getPool().getNumActive() + pool.getPool().getNumIdle() + pool.getPool().getNumWaiters()); + assertEquals(0, pool.getPool().getNumActive() + pool.getPool().getNumIdle() + + pool.getPool().getNumWaiters()); assertThat(prepareCount.getAndSet(0), greaterThanOrEqualTo(1)); assertThat(cleanupCount.getAndSet(0), greaterThanOrEqualTo(1)); diff --git a/src/test/java/redis/clients/jedis/JedisPooledUnitTest.java b/src/test/java/redis/clients/jedis/JedisPooledUnitTest.java new file mode 100644 index 0000000000..c1f99548e0 --- /dev/null +++ b/src/test/java/redis/clients/jedis/JedisPooledUnitTest.java @@ -0,0 +1,112 @@ +package redis.clients.jedis; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedConstruction; +import org.mockito.Mockito; +import redis.clients.jedis.providers.PooledConnectionProvider; +import redis.clients.jedis.util.Pool; + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class JedisPooledUnitTest { + + private JedisPooled pooled; + private Pool mockPool; + private PooledConnectionProvider mockProvider; + private Connection mockConnection; + + private AtomicInteger pings = new AtomicInteger(0); + + @BeforeEach + public void setUp() throws Exception { + mockConnection = mock(Connection.class); + Mockito.doAnswer(ioc -> { + pings.incrementAndGet(); + return null; + }).when(mockConnection).sendCommand(eq(Protocol.Command.PING)); + when(mockConnection.getStatusCodeReply()) + .thenAnswer(ioc -> "PONG" + System.currentTimeMillis()); + mockPool = mock(Pool.class); + mockProvider = mock(PooledConnectionProvider.class); + when(mockProvider.getPool()).thenReturn(mockPool); + when(mockPool.getResource()).thenReturn(mockConnection); + pooled = spy(new JedisPooled(mockProvider)); + } + + @AfterEach + public void tearDown() { + if (pooled != null) { + pooled.close(); + } + } + + @Test + public void testWithResourceClosesConnection() { + pooled.withResource(jedis -> assertEquals(mockConnection, jedis.getConnection())); + + verify(pooled, times(1)).getPool(); + verify(mockPool, times(1)).returnResource(eq(mockConnection)); + } + + @Test + public void testWithResourceGetClosesConnection() { + String result = pooled.withResourceGet(Jedis::ping); + assertNotNull(result); + assertEquals(1, pings.get()); + verify(pooled, times(1)).getPool(); + verify(mockPool, times(1)).returnResource(eq(mockConnection)); + } + + @Test + public void testWithResourceGetReturnsConnection() { + Jedis jedis = pooled.getResource(); + String result = jedis.ping(); + pooled.returnResource(jedis); + assertNotNull(result); + assertEquals(1, pings.get()); + verify(pooled, times(2)).getPool(); + verify(mockPool, times(1)).returnResource(eq(mockConnection)); + } + + @Test + public void testWithResourceGetSetData() { + AtomicReference settedValue = new AtomicReference<>(""); + try (MockedConstruction mockConstruction = Mockito.mockConstruction(Jedis.class, + (mock, context) -> when(mock.set(eq("test-key"), anyString())).thenAnswer(ioc -> { + settedValue.set(ioc.getArgument(1)); + return "OK"; + }))) { + pooled.withResource(jedis -> jedis.set("test-key", "test-result")); + assertEquals("test-result", settedValue.get()); + verify(pooled, times(1)).getPool(); + Jedis mockJedis = mockConstruction.constructed().get(0); + verify(mockJedis, times(1)).close(); + } + } + + @Test + public void testWithResourceGetReturnsResult() { + try (MockedConstruction mockConstruction = Mockito.mockConstruction(Jedis.class, + (mock, context) -> when(mock.get(eq("test-key"))).thenReturn("test-result"))) { + String result = pooled.withResourceGet(jedis -> jedis.get("test-key")); + assertEquals("test-result", result); + verify(pooled, times(1)).getPool(); + Jedis mockJedis = mockConstruction.constructed().get(0); + verify(mockJedis, times(1)).close(); + } + } + +} diff --git a/src/test/java/redis/clients/jedis/JedisProviderTest.java b/src/test/java/redis/clients/jedis/JedisProviderTest.java new file mode 100644 index 0000000000..080edb3ef8 --- /dev/null +++ b/src/test/java/redis/clients/jedis/JedisProviderTest.java @@ -0,0 +1,100 @@ +package redis.clients.jedis; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class JedisProviderTest { + + private TestJedisProvider provider = spy(new TestJedisProvider()); + + @Test + public void testWithResourceClosesConnection() { + provider.withResource(Jedis::ping); + + verify(provider, times(1)).getResource(); + assertEquals(1, provider.getBorrowed()); + assertEquals(1, provider.getClosed()); + } + + @Test + public void testWithResourceGetClosesConnection() { + String result = provider.withResourceGet(jedis -> jedis.ping()); + + verify(provider, times(1)).getResource(); + assertEquals(1, provider.getBorrowed()); + assertEquals(1, provider.getClosed()); + assertNotNull(result); + } + + @Test + public void testWithSomeResources() { + provider.withResource(Jedis::ping); + String result = provider.withResourceGet(Jedis::ping); + provider.withResource(Jedis::ping); + Jedis jedis = provider.getResource(); + assertEquals(4, provider.getBorrowed()); + assertEquals(3, provider.getClosed()); + assertNotNull(result); + jedis.close(); + assertEquals(4, provider.getClosed()); + } + + @Test + public void tesReturntWithSomeResources() { + provider.withResource(Jedis::ping); + String result = provider.withResourceGet(Jedis::ping); + provider.withResource(Jedis::ping); + Jedis jedis = provider.getResource(); + assertEquals(4, provider.getBorrowed()); + assertEquals(3, provider.getClosed()); + assertNotNull(result); + provider.returnResource(jedis); + assertEquals(4, provider.getClosed()); + } + + static class TestJedisProvider implements JedisProvider { + + private Jedis jedis; + private AtomicInteger borrowed = new AtomicInteger(0); + private AtomicInteger closed = new AtomicInteger(0); + + TestJedisProvider() { + jedis = mock(Jedis.class); + Mockito.doAnswer(ioc -> { + closed.incrementAndGet(); + return null; + }).when(jedis).close(); + when(jedis.ping()).thenAnswer(ioc -> "PONG_" + System.nanoTime()); + } + + @Override + public Jedis getResource() { + borrowed.incrementAndGet(); + return jedis; + } + + @Override + public void returnResource(Jedis resource) { + jedis.close(); + } + + public int getBorrowed() { + return borrowed.get(); + } + + public int getClosed() { + return closed.get(); + } + + } +}