From 992421031ac48a9d93d8d7ba98b2ab7f7734670f Mon Sep 17 00:00:00 2001 From: Jennifer Hickey Date: Mon, 15 Jul 2013 14:18:02 -0700 Subject: [PATCH] Fix NPEs on Jedis closePipeline/exec after move DATAREDIS-218 --- .../connection/jedis/JedisConnection.java | 4 ++-- .../AbstractConnectionIntegrationTests.java | 16 +++++++++++++ ...actConnectionPipelineIntegrationTests.java | 16 +++++++++++++ .../JRedisConnectionIntegrationTests.java | 22 ++++++++++++++++++ .../LettuceConnectionIntegrationTests.java | 23 +++++++++++++++++++ ...uceConnectionPipelineIntegrationTests.java | 22 ++++++++++++++++++ 6 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index ec0aef2ae..c8d77f739 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java @@ -717,11 +717,11 @@ public class JedisConnection implements RedisConnection { public Boolean move(byte[] key, int dbIndex) { try { if (isQueueing()) { - client.move(key, dbIndex); + transaction.move(key, dbIndex); return null; } if (isPipelined()) { - client.move(key, dbIndex); + pipeline.move(key, dbIndex); return null; } return (jedis.move(key, dbIndex) == 1); diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java index 4823db0f8..f4d788aac 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -1221,6 +1221,22 @@ public abstract class AbstractConnectionIntegrationTests { Arrays.asList(new String[] { "foo", "bar" }) }), actual); } + @Test + public void testMove() { + connection.set("foo", "bar"); + actual.add(connection.move("foo", 1)); + verifyResults( + Arrays.asList(new Object[] { true}), actual); + connection.select(1); + try { + assertEquals("bar",connection.get("foo")); + } finally { + if(connection.exists("foo")) { + connection.del("foo"); + } + } + } + protected void verifyResults(List expected, List actual) { assertEquals(expected, actual); } diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionPipelineIntegrationTests.java index cc08571fd..e826c1585 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionPipelineIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionPipelineIntegrationTests.java @@ -841,6 +841,22 @@ abstract public class AbstractConnectionPipelineIntegrationTests extends Arrays.asList(new String[] { "foo", "bar" }) }), actual); } + @Test + public void testMove() { + connection.set("foo", "bar"); + actual.add(connection.move("foo", 1)); + verifyResults( + Arrays.asList(new Object[] { 1l }), actual); + connection.select(1); + try { + assertEquals("bar",connection.get("foo")); + } finally { + if(connection.exists("foo")) { + connection.del("foo"); + } + } + } + protected void initConnection() { connection.openPipeline(); } diff --git a/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java index d3b257668..48a434748 100644 --- a/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java @@ -31,6 +31,7 @@ import org.springframework.data.redis.connection.AbstractConnectionIntegrationTe import org.springframework.data.redis.connection.DefaultSortParameters; import org.springframework.data.redis.connection.DefaultStringRedisConnection; import org.springframework.data.redis.connection.SortParameters.Order; +import org.springframework.data.redis.connection.StringRedisConnection; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -382,4 +383,25 @@ public class JRedisConnectionIntegrationTests extends AbstractConnectionIntegrat assertEquals("bar", stringSerializer.deserialize(response.getBulkData())); } + @Test + public void testMove() { + connection.set("foo", "bar"); + actual.add(connection.move("foo", 1)); + verifyResults( + Arrays.asList(new Object[] { true}), actual); + // JRedis does not support select() on existing conn, create new one + JredisConnectionFactory factory2 = new JredisConnectionFactory(); + factory2.setDatabase(1); + factory2.afterPropertiesSet(); + StringRedisConnection conn2 = new DefaultStringRedisConnection(factory2.getConnection()); + try { + assertEquals("bar",conn2.get("foo")); + } finally { + if(conn2.exists("foo")) { + conn2.del("foo"); + } + conn2.close(); + } + } + } \ No newline at end of file diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java index 0bb5501ee..fb23a483e 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java @@ -19,6 +19,7 @@ package org.springframework.data.redis.connection.lettuce; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import java.util.Arrays; import java.util.List; import org.junit.Test; @@ -26,6 +27,7 @@ import org.junit.runner.RunWith; import org.springframework.data.redis.RedisSystemException; import org.springframework.data.redis.connection.AbstractConnectionIntegrationTests; import org.springframework.data.redis.connection.DefaultStringRedisConnection; +import org.springframework.data.redis.connection.StringRedisConnection; import org.springframework.test.annotation.IfProfileValue; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -123,4 +125,25 @@ public class LettuceConnectionIntegrationTests extends AbstractConnectionIntegra public void testSelect() { connection.select(1); } + + @Test + public void testMove() { + connection.set("foo", "bar"); + actual.add(connection.move("foo", 1)); + verifyResults( + Arrays.asList(new Object[] { true}), actual); + // Lettuce does not support select when using shared conn, use a new conn factory + LettuceConnectionFactory factory2 = new LettuceConnectionFactory(); + factory2.setDatabase(1); + factory2.afterPropertiesSet(); + StringRedisConnection conn2 = new DefaultStringRedisConnection(factory2.getConnection()); + try { + assertEquals("bar",conn2.get("foo")); + } finally { + if(conn2.exists("foo")) { + conn2.del("foo"); + } + conn2.close(); + } + } } \ No newline at end of file diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java index 2b09efe70..f82517526 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java @@ -36,6 +36,7 @@ import org.junit.runner.RunWith; import org.springframework.data.redis.connection.AbstractConnectionPipelineIntegrationTests; import org.springframework.data.redis.connection.DefaultStringRedisConnection; import org.springframework.data.redis.connection.DefaultStringTuple; +import org.springframework.data.redis.connection.StringRedisConnection; import org.springframework.data.redis.connection.StringRedisConnection.StringTuple; import org.springframework.test.annotation.IfProfileValue; import org.springframework.test.context.ContextConfiguration; @@ -292,6 +293,27 @@ public class LettuceConnectionPipelineIntegrationTests extends assertFalse(exists("exp", 2500)); } + @Test + public void testMove() { + connection.set("foo", "bar"); + actual.add(connection.move("foo", 1)); + verifyResults( + Arrays.asList(new Object[] { true}), actual); + // Lettuce does not support select when using shared conn, use a new conn factory + LettuceConnectionFactory factory2 = new LettuceConnectionFactory(); + factory2.setDatabase(1); + factory2.afterPropertiesSet(); + StringRedisConnection conn2 = new DefaultStringRedisConnection(factory2.getConnection()); + try { + assertEquals("bar",conn2.get("foo")); + } finally { + if(conn2.exists("foo")) { + conn2.del("foo"); + } + conn2.close(); + } + } + @SuppressWarnings({ "rawtypes", "unchecked" }) protected Object convertResult(Object result) { Object convertedResult = super.convertResult(result);