From f0f6049cdba1db4914ef0d837a84183fa206c227 Mon Sep 17 00:00:00 2001 From: Thomas Sauzedde Date: Fri, 10 Sep 2010 17:05:18 +0200 Subject: [PATCH 1/3] Fix processBulKReply against TCP fragmentation. see http://github.com/xetorthio/jedis/issues#issue/10 --- src/main/java/redis/clients/jedis/Protocol.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/redis/clients/jedis/Protocol.java b/src/main/java/redis/clients/jedis/Protocol.java index ff96e8b..3c1ff30 100644 --- a/src/main/java/redis/clients/jedis/Protocol.java +++ b/src/main/java/redis/clients/jedis/Protocol.java @@ -139,8 +139,11 @@ public class Protocol { return null; } byte[] read = new byte[len]; + int offset = 0; try { - is.read(read); + while(offset < len) { + offset += is.read(read, offset, (len - offset)); + } // read 2 more bytes for the command delimiter is.read(); is.read(); From 9211335f0d8ea82f69dea50229928c0205310480 Mon Sep 17 00:00:00 2001 From: Thomas Sauzedde Date: Fri, 10 Sep 2010 17:26:09 +0200 Subject: [PATCH 2/3] Add U test for fragmented processBulkReply patch. --- .../tests/FragmentedByteArrayInputStream.java | 30 +++++++++++++++++++ .../clients/jedis/tests/ProtocolTest.java | 10 +++++++ 2 files changed, 40 insertions(+) create mode 100644 src/test/java/redis/clients/jedis/tests/FragmentedByteArrayInputStream.java diff --git a/src/test/java/redis/clients/jedis/tests/FragmentedByteArrayInputStream.java b/src/test/java/redis/clients/jedis/tests/FragmentedByteArrayInputStream.java new file mode 100644 index 0000000..c57525d --- /dev/null +++ b/src/test/java/redis/clients/jedis/tests/FragmentedByteArrayInputStream.java @@ -0,0 +1,30 @@ +package redis.clients.jedis.tests; + +import java.io.ByteArrayInputStream; + +/** + * Test class the fragment a byte array for testing purpose. + */ +public class FragmentedByteArrayInputStream extends ByteArrayInputStream { + private int readMethodCallCount = 0; + public FragmentedByteArrayInputStream(final byte[] buf) { + super(buf); + } + + @Override + public synchronized int read(final byte[] b, final int off, final int len) { + readMethodCallCount++; + if (len <= 10) { + // if the len <= 10, return as usual .. + return super.read(b, off, len); + } else { + // else return the first half .. + return super.read(b, off, len / 2); + } + } + + public int getReadMethodCallCount() { + return readMethodCallCount; + } + +} diff --git a/src/test/java/redis/clients/jedis/tests/ProtocolTest.java b/src/test/java/redis/clients/jedis/tests/ProtocolTest.java index 8aa48e1..361d50e 100644 --- a/src/test/java/redis/clients/jedis/tests/ProtocolTest.java +++ b/src/test/java/redis/clients/jedis/tests/ProtocolTest.java @@ -47,6 +47,16 @@ public class ProtocolTest extends Assert { assertEquals("foobar", response); } + @Test + public void fragmentedBulkReply() { + FragmentedByteArrayInputStream fis = new FragmentedByteArrayInputStream("$30\r\n012345678901234567890123456789\r\n".getBytes()); + Protocol protocol = new Protocol(); + String response = (String) protocol.read(new DataInputStream(fis)); + assertEquals("012345678901234567890123456789", response); + assertEquals(3, fis.getReadMethodCallCount()); + } + + @Test public void nullBulkReply() { InputStream is = new ByteArrayInputStream("$-1\r\n".getBytes()); From d61f4d79d868ab2bb0377d449bf03782ca0ed16c Mon Sep 17 00:00:00 2001 From: Yaourt Date: Mon, 13 Sep 2010 10:55:58 +0200 Subject: [PATCH 3/3] Allow to execute tests against a remote server. Update Maven pom to use "redis-host" and "redis-port" env. properties. Default values point to localhost:6379. Tests updated to use this properties and also defaulted to localhost:6379. --- pom.xml | 17 ++ .../clients/jedis/tests/JedisPoolTest.java | 153 ++++++++++-------- .../clients/jedis/tests/PipeliningTest.java | 36 ++++- .../tests/commands/JedisCommandTestBase.java | 61 ++++--- .../commands/TransactionCommandsTest.java | 14 +- 5 files changed, 185 insertions(+), 96 deletions(-) diff --git a/pom.xml b/pom.xml index 9d50773..a3348ac 100644 --- a/pom.xml +++ b/pom.xml @@ -12,6 +12,12 @@ compile + + + localhost + 6379 + + @@ -23,6 +29,17 @@ 1.6 + + org.apache.maven.plugins + maven-surefire-plugin + 2.6 + + + ${redis-host} + ${redis-port} + + + diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index 4444ee6..a57e922 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.util.concurrent.TimeoutException; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import redis.clients.jedis.Jedis; @@ -11,83 +12,101 @@ import redis.clients.jedis.JedisPool; import redis.clients.jedis.Protocol; public class JedisPoolTest extends Assert { - @Test - public void checkConnections() throws TimeoutException { - JedisPool pool = new JedisPool("localhost", Protocol.DEFAULT_PORT, 2000); - pool.setResourcesNumber(10); - pool.init(); + private static String host = "localhost"; + private static int port = Protocol.DEFAULT_PORT; + + static { + final String envHost = System.getProperty("redis-host"); + final String envPort = System.getProperty("redis-port"); + if (null != envHost && 0 < envHost.length()) { + host = envHost; + } + if (null != envPort && 0 < envPort.length()) { + try { + port = Integer.parseInt(envPort); + } catch (final NumberFormatException e) {} + } + + System.out.println("Redis host to be used : " + host + ":" + port); + } + + @Test + public void checkConnections() throws TimeoutException { + JedisPool pool = new JedisPool(host, port, 2000); + pool.setResourcesNumber(10); + pool.init(); - Jedis jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.set("foo", "bar"); - assertEquals("bar", jedis.get("foo")); - pool.returnResource(jedis); - pool.destroy(); - } + Jedis jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.set("foo", "bar"); + assertEquals("bar", jedis.get("foo")); + pool.returnResource(jedis); + pool.destroy(); + } - @Test - public void checkConnectionWithDefaultPort() throws TimeoutException { - JedisPool pool = new JedisPool("localhost"); - pool.setResourcesNumber(10); - pool.init(); + @Test + public void checkConnectionWithDefaultPort() throws TimeoutException { + JedisPool pool = new JedisPool(host, port); + pool.setResourcesNumber(10); + pool.init(); - Jedis jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.set("foo", "bar"); - assertEquals("bar", jedis.get("foo")); - pool.returnResource(jedis); - pool.destroy(); - } + Jedis jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.set("foo", "bar"); + assertEquals("bar", jedis.get("foo")); + pool.returnResource(jedis); + pool.destroy(); + } - @Test - public void checkJedisIsReusedWhenReturned() throws TimeoutException { - JedisPool pool = new JedisPool("localhost"); - pool.setResourcesNumber(1); - pool.init(); + @Test + public void checkJedisIsReusedWhenReturned() throws TimeoutException { + JedisPool pool = new JedisPool(host, port); + pool.setResourcesNumber(1); + pool.init(); - Jedis jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.set("foo", "0"); - pool.returnResource(jedis); + Jedis jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.set("foo", "0"); + pool.returnResource(jedis); - jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.incr("foo"); - pool.returnResource(jedis); - pool.destroy(); - } + jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.incr("foo"); + pool.returnResource(jedis); + pool.destroy(); + } - @Test - public void checkPoolRepairedWhenJedisIsBroken() throws TimeoutException, - IOException { - JedisPool pool = new JedisPool("localhost"); - pool.setResourcesNumber(1); - pool.init(); + @Test + public void checkPoolRepairedWhenJedisIsBroken() throws TimeoutException, + IOException { + JedisPool pool = new JedisPool(host, port); + pool.setResourcesNumber(1); + pool.init(); - Jedis jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.quit(); - pool.returnBrokenResource(jedis); + Jedis jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.quit(); + pool.returnBrokenResource(jedis); - jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.incr("foo"); - pool.returnResource(jedis); - pool.destroy(); - } + jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.incr("foo"); + pool.returnResource(jedis); + pool.destroy(); + } - @Test(expected = TimeoutException.class) - public void checkPoolOverflow() throws TimeoutException { - JedisPool pool = new JedisPool("localhost"); - pool.setResourcesNumber(1); - pool.init(); + @Test(expected = TimeoutException.class) + public void checkPoolOverflow() throws TimeoutException { + JedisPool pool = new JedisPool(host, port); + pool.setResourcesNumber(1); + pool.init(); - Jedis jedis = pool.getResource(200); - jedis.auth("foobared"); - jedis.set("foo", "0"); + Jedis jedis = pool.getResource(200); + jedis.auth("foobared"); + jedis.set("foo", "0"); - Jedis newJedis = pool.getResource(200); - newJedis.auth("foobared"); - newJedis.incr("foo"); - } + Jedis newJedis = pool.getResource(200); + newJedis.auth("foobared"); + newJedis.incr("foo"); + } } \ No newline at end of file diff --git a/src/test/java/redis/clients/jedis/tests/PipeliningTest.java b/src/test/java/redis/clients/jedis/tests/PipeliningTest.java index 8816555..d596dac 100644 --- a/src/test/java/redis/clients/jedis/tests/PipeliningTest.java +++ b/src/test/java/redis/clients/jedis/tests/PipeliningTest.java @@ -6,19 +6,45 @@ import java.util.List; import junit.framework.Assert; +import org.junit.Before; import org.junit.Test; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPipeline; +import redis.clients.jedis.Protocol; public class PipeliningTest extends Assert { + private static String host = "localhost"; + private static int port = Protocol.DEFAULT_PORT; + + static { + final String envHost = System.getProperty("redis-host"); + final String envPort = System.getProperty("redis-port"); + if (null != envHost && 0 < envHost.length()) { + host = envHost; + } + if (null != envPort && 0 < envPort.length()) { + try { + port = Integer.parseInt(envPort); + } catch (final NumberFormatException e) { + } + } + + System.out.println("Redis host to be used : " + host + ":" + port); + } + + private Jedis jedis; + + @Before + public void setUp() throws Exception { + jedis = new Jedis(host, port, 500); + jedis.connect(); + jedis.auth("foobared"); + jedis.flushAll(); + } + @Test public void pipeline() throws UnknownHostException, IOException { - Jedis jedis = new Jedis("localhost"); - jedis.connect(); - jedis.auth("foobared"); - jedis.flushAll(); - List results = jedis.pipelined(new JedisPipeline() { public void execute() { client.set("foo", "bar"); diff --git a/src/test/java/redis/clients/jedis/tests/commands/JedisCommandTestBase.java b/src/test/java/redis/clients/jedis/tests/commands/JedisCommandTestBase.java index 2973e53..ede6ca1 100644 --- a/src/test/java/redis/clients/jedis/tests/commands/JedisCommandTestBase.java +++ b/src/test/java/redis/clients/jedis/tests/commands/JedisCommandTestBase.java @@ -12,31 +12,48 @@ import redis.clients.jedis.Jedis; import redis.clients.jedis.Protocol; public abstract class JedisCommandTestBase extends Assert { + protected static String host = "localhost"; + protected static int port = Protocol.DEFAULT_PORT; + static { + final String envHost = System.getProperty("redis-host"); + final String envPort = System.getProperty("redis-port"); + if (null != envHost && 0 < envHost.length()) { + host = envHost; + } + if (null != envPort && 0 < envPort.length()) { + try { + port = Integer.parseInt(envPort); + } catch (final NumberFormatException e) { + } + } - protected Jedis jedis; + System.out.println("Redis host to be used : " + host + ":" + port); + } - public JedisCommandTestBase() { - super(); - } + protected Jedis jedis; - @Before - public void setUp() throws Exception { - jedis = new Jedis("localhost", Protocol.DEFAULT_PORT, 500); - jedis.connect(); - jedis.auth("foobared"); - jedis.flushAll(); - } + public JedisCommandTestBase() { + super(); + } - @After - public void tearDown() throws Exception { - jedis.disconnect(); - } + @Before + public void setUp() throws Exception { + jedis = new Jedis(host, port, 500); + jedis.connect(); + jedis.auth("foobared"); + jedis.flushAll(); + } - protected Jedis createJedis() throws UnknownHostException, IOException { - Jedis j = new Jedis("localhost"); - j.connect(); - j.auth("foobared"); - j.flushAll(); - return j; - } + @After + public void tearDown() throws Exception { + jedis.disconnect(); + } + + protected Jedis createJedis() throws UnknownHostException, IOException { + Jedis j = new Jedis(host, port); + j.connect(); + j.auth("foobared"); + j.flushAll(); + return j; + } } \ No newline at end of file diff --git a/src/test/java/redis/clients/jedis/tests/commands/TransactionCommandsTest.java b/src/test/java/redis/clients/jedis/tests/commands/TransactionCommandsTest.java index 41f96db..77ee1f4 100644 --- a/src/test/java/redis/clients/jedis/tests/commands/TransactionCommandsTest.java +++ b/src/test/java/redis/clients/jedis/tests/commands/TransactionCommandsTest.java @@ -5,6 +5,7 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; +import org.junit.Before; import org.junit.Test; import redis.clients.jedis.Jedis; @@ -13,6 +14,17 @@ import redis.clients.jedis.Transaction; import redis.clients.jedis.TransactionBlock; public class TransactionCommandsTest extends JedisCommandTestBase { + Jedis nj; + @Before + public void setUp() throws Exception { + super.setUp(); + + nj = new Jedis(host, port, 500); + nj.connect(); + nj.auth("foobared"); + nj.flushAll(); + } + @Test public void multi() { Transaction trans = jedis.multi(); @@ -62,7 +74,6 @@ public class TransactionCommandsTest extends JedisCommandTestBase { jedis.watch("mykey"); Transaction t = jedis.multi(); - Jedis nj = new Jedis("localhost"); nj.connect(); nj.auth("foobared"); nj.set("mykey", "bar"); @@ -83,7 +94,6 @@ public class TransactionCommandsTest extends JedisCommandTestBase { assertEquals("OK", status); Transaction t = jedis.multi(); - Jedis nj = new Jedis("localhost"); nj.connect(); nj.auth("foobared"); nj.set("mykey", "bar");