diff --git a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java index db98501a5..8db282daa 100644 --- a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java @@ -429,10 +429,11 @@ public class SrpConnection implements RedisConnection { isMulti = false; Exception execException = null; List results = null; + boolean resultsSet = true; try { Future exec = client.exec(); // Need to wait on execution or subsequent non-pipelined calls may read exec results - exec.get(); + resultsSet = exec.get(); } catch (Exception ex) { execException = ex; } finally { @@ -447,7 +448,8 @@ public class SrpConnection implements RedisConnection { } } } - if(execException != null) { + // If resultsSet is false, it's b/c of nil Multi-Bulk reply (watch case) and null is returned + if(execException != null && resultsSet) { // Intentionally convert RedisPipelineException too, it's an impl detail throw convertSrpAccessException(execException); } diff --git a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionIntegrationTests.java index 0049f17f1..1d7ba24c6 100644 --- a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionIntegrationTests.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assume.assumeTrue; import org.junit.After; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.data.redis.RedisVersionUtils; @@ -55,10 +54,6 @@ public class SrpConnectionIntegrationTests extends AbstractConnectionIntegration connection = null; } - @Ignore("DATAREDIS-168 SRP exec throws TransactionFailedException if watched value modified") - public void testWatch() { - } - @Test(expected = UnsupportedOperationException.class) public void testZInterStoreAggWeights() { super.testZInterStoreAggWeights(); diff --git a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java index ada6e5d38..ffbedb068 100644 --- a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java @@ -19,8 +19,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; import java.util.ArrayList; import java.util.Arrays; @@ -29,7 +29,6 @@ import java.util.List; import java.util.Properties; import java.util.Set; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.data.redis.RedisVersionUtils; @@ -78,8 +77,21 @@ public class SrpConnectionPipelineIntegrationTests extends testMultiExec(); } - @Ignore("DATAREDIS-168 SRP exec throws TransactionFailedException if watched value modified") - public void testWatch() { + // SRP sets results of all commands in the pipeline to RedisException if exec returns a + // null multi-bulk reply + @Test(expected=RedisPipelineException.class) + public void testWatch() throws Exception { + connection.set("testitnow", "willdo"); + connection.watch("testitnow".getBytes()); + DefaultStringRedisConnection conn2 = new DefaultStringRedisConnection( + connectionFactory.getConnection()); + conn2.set("testitnow", "something"); + conn2.close(); + connection.multi(); + connection.set("testitnow", "somethingelse"); + actual.add(connection.exec()); + actual.add(connection.get("testitnow")); + verifyResults(Arrays.asList(new Object[] { null, "something" }), actual); } @Test(expected = UnsupportedOperationException.class) diff --git a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionTransactionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionTransactionIntegrationTests.java index 20e1ea45e..e986bb69e 100644 --- a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionTransactionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionTransactionIntegrationTests.java @@ -45,6 +45,7 @@ public class SrpConnectionTransactionIntegrationTests extends } @Ignore + @Test public void testWatch() { } diff --git a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicTests.java b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicTests.java index e69ecd07b..688a2a503 100644 --- a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicTests.java +++ b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicTests.java @@ -139,8 +139,8 @@ public class RedisAtomicTests { // DATAREDIS-108 @Test public void testCompareSet() throws Exception { - // Txs not supported in Jredis, SRP checkAndSet not working DATAREDIS-123 - assumeTrue(!ConnectionUtils.isJredis(factory) && !ConnectionUtils.isSrp(factory)); + // Txs not supported in Jredis + assumeTrue(!ConnectionUtils.isJredis(factory)); final AtomicBoolean alreadySet = new AtomicBoolean(false); final int NUM = 50; final String KEY = getClass().getSimpleName() + ":atomic:counter";