From dc054268fa158600806ce3d38185e670369d2a18 Mon Sep 17 00:00:00 2001 From: Henning Schmiedehausen Date: Thu, 20 Feb 2014 18:22:44 -0800 Subject: [PATCH 1/3] Add Closeable to JedisPool. This allows JedisPool instances to also participate in try-with-resources. Adds tests (both for JedisPool and the Jedis code itself). --- src/main/java/redis/clients/util/Pool.java | 15 +++++- .../redis/clients/jedis/tests/Closer.java | 39 ++++++++++++++ .../jedis/tests/ConnectionCloseTest.java | 51 +++++++++++++++++++ .../clients/jedis/tests/JedisPoolTest.java | 24 +++++++++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 src/test/java/redis/clients/jedis/tests/Closer.java create mode 100644 src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java diff --git a/src/main/java/redis/clients/util/Pool.java b/src/main/java/redis/clients/util/Pool.java index 09d8ebb..4ac3524 100644 --- a/src/main/java/redis/clients/util/Pool.java +++ b/src/main/java/redis/clients/util/Pool.java @@ -1,5 +1,7 @@ package redis.clients.util; +import java.io.Closeable; + import org.apache.commons.pool2.PooledObjectFactory; import org.apache.commons.pool2.impl.GenericObjectPool; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; @@ -7,7 +9,7 @@ import org.apache.commons.pool2.impl.GenericObjectPoolConfig; import redis.clients.jedis.exceptions.JedisConnectionException; import redis.clients.jedis.exceptions.JedisException; -public abstract class Pool { +public abstract class Pool implements Closeable { protected GenericObjectPool internalPool; /** @@ -17,6 +19,15 @@ public abstract class Pool { public Pool() { } + @Override + public void close() { + closeInternalPool(); + } + + public boolean isClosed() { + return this.internalPool.isClosed(); + } + public Pool(final GenericObjectPoolConfig poolConfig, PooledObjectFactory factory) { initPool(poolConfig, factory); @@ -81,4 +92,4 @@ public abstract class Pool { throw new JedisException("Could not destroy the pool", e); } } -} \ No newline at end of file +} diff --git a/src/test/java/redis/clients/jedis/tests/Closer.java b/src/test/java/redis/clients/jedis/tests/Closer.java new file mode 100644 index 0000000..5c7d801 --- /dev/null +++ b/src/test/java/redis/clients/jedis/tests/Closer.java @@ -0,0 +1,39 @@ +package redis.clients.jedis.tests; + +import java.io.Closeable; +import java.io.IOException; +import java.util.HashSet; +import java.util.Set; + +class Closer implements Closeable { + private final Set elements = new HashSet(); + + synchronized T register(T element) { + if (element != null) { + elements.add(element); + } + return element; + } + + public synchronized void close() throws IOException { + Throwable caught = null; + + for (Closeable element : elements) { + try { + element.close(); + } + catch (Throwable t) { + caught = t; + } + } + + elements.clear(); + + if (caught != null) { + if (caught instanceof IOException) { + throw (IOException) caught; + } + throw (RuntimeException) caught; + } + } +} diff --git a/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java b/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java new file mode 100644 index 0000000..77cb48d --- /dev/null +++ b/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java @@ -0,0 +1,51 @@ +package redis.clients.jedis.tests; + +import java.io.Closeable; +import java.io.IOException; +import java.util.HashSet; +import java.util.Set; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import redis.clients.jedis.Connection; +import redis.clients.jedis.exceptions.JedisConnectionException; + +public class ConnectionCloseTest extends Assert { + + private final Closer closer = new Closer(); + + private Connection client; + + @Before + public void setUp() throws Exception { + client = closer.register(new Connection()); + } + + @After + public void tearDown() throws Exception { + closer.close(); + } + + @Test(expected = JedisConnectionException.class) + public void checkUnkownHost() { + client.setHost("someunknownhost"); + client.connect(); + } + + @Test(expected = JedisConnectionException.class) + public void checkWrongPort() { + client.setHost("localhost"); + client.setPort(55665); + client.connect(); + } + + @Test + public void connectIfNotConnectedWhenSettingTimeoutInfinite() { + client.setHost("localhost"); + client.setPort(6379); + client.setTimeoutInfinite(); + } +} diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index a501024..d24f5be 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -27,6 +27,21 @@ public class JedisPoolTest extends Assert { assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); pool.destroy(); + assertTrue(pool.isClosed()); + } + + @Test + public void checkCloseableConnections() throws Exception { + Closer closer = new Closer(); + JedisPool pool = closer.register(new JedisPool(new JedisPoolConfig(), hnp.getHost(), + hnp.getPort(), 2000)); + Jedis jedis = pool.getResource(); + jedis.auth("foobared"); + jedis.set("foo", "bar"); + assertEquals("bar", jedis.get("foo")); + pool.returnResource(jedis); + closer.close(); + assertTrue(pool.isClosed()); } @Test @@ -39,6 +54,7 @@ public class JedisPoolTest extends Assert { assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); pool.destroy(); + assertTrue(pool.isClosed()); } @Test @@ -56,6 +72,7 @@ public class JedisPoolTest extends Assert { jedis.incr("foo"); pool.returnResource(jedis); pool.destroy(); + assertTrue(pool.isClosed()); } @Test @@ -72,6 +89,7 @@ public class JedisPoolTest extends Assert { jedis.incr("foo"); pool.returnResource(jedis); pool.destroy(); + assertTrue(pool.isClosed()); } @Test(expected = JedisConnectionException.class) @@ -99,6 +117,7 @@ public class JedisPoolTest extends Assert { jedis.set("foo", "bar"); pool.returnResource(jedis); pool.destroy(); + assertTrue(pool.isClosed()); } @Test @@ -110,6 +129,7 @@ public class JedisPoolTest extends Assert { assertEquals("bar", jedis0.get("foo")); pool0.returnResource(jedis0); pool0.destroy(); + assertTrue(pool0.isClosed()); JedisPool pool1 = new JedisPool(new JedisPoolConfig(), hnp.getHost(), hnp.getPort(), 2000, "foobared", 1); @@ -117,6 +137,7 @@ public class JedisPoolTest extends Assert { assertNull(jedis1.get("foo")); pool1.returnResource(jedis1); pool1.destroy(); + assertTrue(pool1.isClosed()); } @Test @@ -163,6 +184,7 @@ public class JedisPoolTest extends Assert { pool.returnResource(jedis1); pool.destroy(); + assertTrue(pool.isClosed()); } @Test @@ -176,6 +198,7 @@ public class JedisPoolTest extends Assert { pool0.returnResource(jedis); pool0.destroy(); + assertTrue(pool0.isClosed()); } @Test @@ -197,5 +220,6 @@ public class JedisPoolTest extends Assert { assertEquals("jedis", jedis2.get("hello")); pool.returnResource(jedis2); pool.destroy(); + assertTrue(pool.isClosed()); } } From c0697cd6d7cb8579d6a476c9cfc1e40c61e58667 Mon Sep 17 00:00:00 2001 From: Jungtaek Lim Date: Sun, 16 Mar 2014 22:58:26 +0900 Subject: [PATCH 2/3] Add "closeable" unit tests to JedisSentinelPool, ShardedJedisPoolTest --- .../jedis/tests/JedisSentinelPoolTest.java | 16 ++++++++++++++++ .../jedis/tests/ShardedJedisPoolTest.java | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index c8df9c5..c88bd55 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java @@ -34,6 +34,22 @@ public class JedisSentinelPoolTest extends JedisTestBase { sentinelJedis1 = new Jedis(sentinel1.getHost(), sentinel1.getPort()); } + + @Test + public void checkCloseableConnections() throws Exception { + GenericObjectPoolConfig config = new GenericObjectPoolConfig(); + + Closer closer = new Closer(); + JedisSentinelPool pool = closer.register(new JedisSentinelPool( + MASTER_NAME, sentinels, config, 1000, "foobared", 2)); + Jedis jedis = pool.getResource(); + jedis.auth("foobared"); + jedis.set("foo", "bar"); + assertEquals("bar", jedis.get("foo")); + pool.returnResource(jedis); + closer.close(); + assertTrue(pool.isClosed()); + } @Test public void ensureSafeTwiceFailover() throws InterruptedException { diff --git a/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java index ee7951b..272656c 100644 --- a/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java @@ -54,6 +54,19 @@ public class ShardedJedisPoolTest extends Assert { pool.destroy(); } + @Test + public void checkCloseableConnections() throws Exception { + Closer closer = new Closer(); + ShardedJedisPool pool = closer.register(new ShardedJedisPool( + new GenericObjectPoolConfig(), shards)); + ShardedJedis jedis = pool.getResource(); + jedis.set("foo", "bar"); + assertEquals("bar", jedis.get("foo")); + pool.returnResource(jedis); + closer.close(); + assertTrue(pool.isClosed()); + } + @Test public void checkConnectionWithDefaultPort() { ShardedJedisPool pool = new ShardedJedisPool( From f931a4fc81871710a9ba4dc62ca62e035c9be5e7 Mon Sep 17 00:00:00 2001 From: Jungtaek Lim Date: Wed, 10 Sep 2014 21:15:02 +0900 Subject: [PATCH 3/3] Replace Closer class to use its close() method --- .../redis/clients/jedis/tests/Closer.java | 39 ------------------- .../jedis/tests/ConnectionCloseTest.java | 27 +++++-------- .../clients/jedis/tests/JedisPoolTest.java | 7 ++-- .../jedis/tests/JedisSentinelPoolTest.java | 7 ++-- .../jedis/tests/ShardedJedisPoolTest.java | 7 ++-- 5 files changed, 19 insertions(+), 68 deletions(-) delete mode 100644 src/test/java/redis/clients/jedis/tests/Closer.java diff --git a/src/test/java/redis/clients/jedis/tests/Closer.java b/src/test/java/redis/clients/jedis/tests/Closer.java deleted file mode 100644 index 5c7d801..0000000 --- a/src/test/java/redis/clients/jedis/tests/Closer.java +++ /dev/null @@ -1,39 +0,0 @@ -package redis.clients.jedis.tests; - -import java.io.Closeable; -import java.io.IOException; -import java.util.HashSet; -import java.util.Set; - -class Closer implements Closeable { - private final Set elements = new HashSet(); - - synchronized T register(T element) { - if (element != null) { - elements.add(element); - } - return element; - } - - public synchronized void close() throws IOException { - Throwable caught = null; - - for (Closeable element : elements) { - try { - element.close(); - } - catch (Throwable t) { - caught = t; - } - } - - elements.clear(); - - if (caught != null) { - if (caught instanceof IOException) { - throw (IOException) caught; - } - throw (RuntimeException) caught; - } - } -} diff --git a/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java b/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java index 77cb48d..5717641 100644 --- a/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java +++ b/src/test/java/redis/clients/jedis/tests/ConnectionCloseTest.java @@ -1,10 +1,5 @@ package redis.clients.jedis.tests; -import java.io.Closeable; -import java.io.IOException; -import java.util.HashSet; -import java.util.Set; - import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -15,37 +10,35 @@ import redis.clients.jedis.exceptions.JedisConnectionException; public class ConnectionCloseTest extends Assert { - private final Closer closer = new Closer(); - private Connection client; @Before public void setUp() throws Exception { - client = closer.register(new Connection()); + client = new Connection(); } @After public void tearDown() throws Exception { - closer.close(); + client.close(); } @Test(expected = JedisConnectionException.class) public void checkUnkownHost() { - client.setHost("someunknownhost"); - client.connect(); + client.setHost("someunknownhost"); + client.connect(); } @Test(expected = JedisConnectionException.class) public void checkWrongPort() { - client.setHost("localhost"); - client.setPort(55665); - client.connect(); + client.setHost("localhost"); + client.setPort(55665); + client.connect(); } @Test public void connectIfNotConnectedWhenSettingTimeoutInfinite() { - client.setHost("localhost"); - client.setPort(6379); - client.setTimeoutInfinite(); + client.setHost("localhost"); + client.setPort(6379); + client.setTimeoutInfinite(); } } diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index 58db11d..becf91f 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -32,15 +32,14 @@ public class JedisPoolTest extends Assert { @Test public void checkCloseableConnections() throws Exception { - Closer closer = new Closer(); - JedisPool pool = closer.register(new JedisPool(new JedisPoolConfig(), hnp.getHost(), - hnp.getPort(), 2000)); + JedisPool pool = new JedisPool(new JedisPoolConfig(), hnp.getHost(), + hnp.getPort(), 2000); Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); - closer.close(); + pool.close(); assertTrue(pool.isClosed()); } diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index c88bd55..87bc1b9 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java @@ -39,15 +39,14 @@ public class JedisSentinelPoolTest extends JedisTestBase { public void checkCloseableConnections() throws Exception { GenericObjectPoolConfig config = new GenericObjectPoolConfig(); - Closer closer = new Closer(); - JedisSentinelPool pool = closer.register(new JedisSentinelPool( - MASTER_NAME, sentinels, config, 1000, "foobared", 2)); + JedisSentinelPool pool = new JedisSentinelPool( + MASTER_NAME, sentinels, config, 1000, "foobared", 2); Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); - closer.close(); + pool.close(); assertTrue(pool.isClosed()); } diff --git a/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java index 272656c..884361d 100644 --- a/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java @@ -56,14 +56,13 @@ public class ShardedJedisPoolTest extends Assert { @Test public void checkCloseableConnections() throws Exception { - Closer closer = new Closer(); - ShardedJedisPool pool = closer.register(new ShardedJedisPool( - new GenericObjectPoolConfig(), shards)); + ShardedJedisPool pool = new ShardedJedisPool( + new GenericObjectPoolConfig(), shards); ShardedJedis jedis = pool.getResource(); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); - closer.close(); + pool.close(); assertTrue(pool.isClosed()); }