Upgrade to SRP 0.6

DATAREDIS-186

- Handle a few method signature changes to varargs

- Handle possibility of exec() throwing Exceptions
if ErrorReply received in Redis 2.6

gradle.properties

@@ -6,7 +6,7 @@ jedisVersion=2.1.0
 springVersion=3.1.4.RELEASE
 log4jVersion=1.2.17
 version=1.0.5.BUILD-SNAPSHOT
-srpVersion=0.5
+srpVersion=0.6
 jacksonVersion=1.8.8
 lettuceVersion=2.3.2
 mockitoVersion=1.8.5

src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java

@@ -427,17 +427,31 @@ public class SrpConnection implements RedisConnection {
 
 	public List<Object> exec() {
 		isMulti = false;
+		Exception execException = null;
+		List<Object> results = null;
 		try {
 			Future<Boolean> exec = client.exec();
-			// Need to wait on execution or subsequent non-pipelined calls like multi may read exec results
+			// Need to wait on execution or subsequent non-pipelined calls may read exec results
 			exec.get();
-			if (pipelineRequested) {
-				return null;
-			}
-			return closePipeline();
 		} catch (Exception ex) {
-			throw convertSrpAccessException(ex);
+			execException = ex;
+		} finally {
+			// ensure pipeline is closed even if error in exec
+			if (!pipelineRequested) {
+				try {
+					results = closePipeline();
+				} catch (RedisPipelineException e) {
+					if(execException == null) {
+						execException = e;
+					}
+				}
+			}
 		}
+		if(execException != null) {
+			// Intentionally convert RedisPipelineException too, it's an impl detail
+			throw convertSrpAccessException(execException);
+		}
+		return results;
 	}
 
 
@@ -1076,7 +1090,7 @@ public class SrpConnection implements RedisConnection {
 				pipeline(pipeline.brpoplpush(srcKey, dstKey, timeout));
 				return null;
 			}
-			return client.brpoplpush(srcKey, dstKey, timeout).data();
+			return (byte[]) client.brpoplpush(srcKey, dstKey, timeout).data();
 		} catch (Exception ex) {
 			throw convertSrpAccessException(ex);
 		}
@@ -1360,10 +1374,10 @@ public class SrpConnection implements RedisConnection {
 	public Long zInterStore(byte[] destKey, byte[]... sets) {
 		try {
 			if (isPipelined()) {
-				pipeline(pipeline.zinterstore(destKey, sets.length, sets));
+				pipeline(pipeline.zinterstore(destKey, sets.length, (Object[]) sets));
 				return null;
 			}
-			return client.zinterstore(destKey, sets.length, sets).data();
+			return client.zinterstore(destKey, sets.length, (Object[]) sets).data();
 		} catch (Exception ex) {
 			throw convertSrpAccessException(ex);
 		}
@@ -1762,10 +1776,10 @@ public class SrpConnection implements RedisConnection {
 	public void hMSet(byte[] key, Map<byte[], byte[]> tuple) {
 		try {
 			if (isPipelined()) {
-				pipeline(pipeline.hmset(key, SrpUtils.convert(tuple)));
+				pipeline(pipeline.hmset(key, (Object[]) SrpUtils.convert(tuple)));
 				return;
 			}
-			client.hmset(key, SrpUtils.convert(tuple));
+			client.hmset(key, (Object[]) SrpUtils.convert(tuple));
 		} catch (Exception ex) {
 			throw convertSrpAccessException(ex);
 		}

src/test/java/org/springframework/data/redis/RedisVersionUtils.java

@@ -49,8 +49,8 @@ public abstract class RedisVersionUtils {
 			String major = matcher.group(1);
 			String minor = matcher.group(2);
 			String patch = matcher.group(4);
-			return new Version(Integer.parseInt(major), Integer.parseInt(minor),
-					Integer.parseInt(patch));
+			return new Version(Integer.parseInt(major), minor != null ? Integer.parseInt(minor) : 0,
+					patch != null ? Integer.parseInt(patch) : 0);
 		}
 		throw new IllegalArgumentException("Specified version cannot be parsed");
 	}

src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionIntegrationTests.java

@@ -54,18 +54,10 @@ public class SrpConnectionIntegrationTests extends AbstractConnectionIntegration
 		connection = null;
 	}
 
-	@Ignore("DATAREDIS-169 SRP discard does not clear txReplies, results in inconsistent results on next tx exec")
-	public void testMultiDiscard() {
-	}
-
 	@Ignore("DATAREDIS-168 SRP exec throws TransactionFailedException if watched value modified")
 	public void testWatch() {
 	}
 
-	@Ignore("DATAREDIS-156 SRP bRPopLPush ClassCastException")
-	public void testBRPopLPushTimeout() {
-	}
-
 	@Test(expected = UnsupportedOperationException.class)
 	public void testZInterStoreAggWeights() {
 		super.testZInterStoreAggWeights();

src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java

@@ -20,6 +20,7 @@ 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 java.util.ArrayList;
 import java.util.Arrays;
@@ -33,7 +34,9 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.data.redis.RedisVersionUtils;
 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.RedisPipelineException;
 import org.springframework.data.redis.connection.RedisZSetCommands.Tuple;
 import org.springframework.data.redis.connection.StringRedisConnection.StringTuple;
 import org.springframework.data.redis.serializer.SerializationUtils;
@@ -54,8 +57,24 @@ import redis.reply.Reply;
 public class SrpConnectionPipelineIntegrationTests extends
 		AbstractConnectionPipelineIntegrationTests {
 
-	@Ignore("DATAREDIS-169 SRP discard does not clear txReplies, results in inconsistent results on next tx exec")
-	public void testMultiDiscard() {
+	@Test
+	public void testMultiDiscard() throws Exception {
+		DefaultStringRedisConnection conn2 = new DefaultStringRedisConnection(
+				connectionFactory.getConnection());
+		conn2.set("testitnow", "willdo");
+		connection.multi();
+		connection.set("testitnow2", "notok");
+		connection.discard();
+		// SRP throws Exception on transaction discard, so we expect pipeline exception here
+		try {
+			getResults();
+			fail("Closing the pipeline on a discarded tx should throw a RedisPipelineException");
+		}catch(RedisPipelineException e) {
+		}
+		assertEquals("willdo", connection.get("testitnow"));
+		connection.openPipeline();
+		// Ensure we can run a new tx after discarding previous one
+		testMultiExec();
 	}
 
 	@Ignore("DATAREDIS-168 SRP exec throws TransactionFailedException if watched value modified")

src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineTxIntegrationTests.java

@@ -17,13 +17,13 @@ package org.springframework.data.redis.connection.srp;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
 import static org.junit.Assume.assumeTrue;
-
+import static org.junit.Assert.fail;
 import java.util.Arrays;
 import java.util.List;
 
 import org.junit.Test;
+import org.springframework.data.redis.RedisSystemException;
 import org.springframework.data.redis.RedisVersionUtils;
 import org.springframework.data.redis.connection.RedisPipelineException;
 
@@ -33,23 +33,30 @@ import org.springframework.data.redis.connection.RedisPipelineException;
  * @author Jennifer Hickey
  *
  */
-public class SrpConnectionPipelineTxIntegrationTests extends
-		SrpConnectionTransactionIntegrationTests {
+public class SrpConnectionPipelineTxIntegrationTests extends SrpConnectionTransactionIntegrationTests {
 
 	@Test
 	public void exceptionExecuteNative() throws Exception {
-		// DATAREDIS-172 SRP ClassCastException when exec() returns an
-		// ErrorReply in Redis 2.6
-		connection.exec();
-		connection.closePipeline();
-		boolean execErrorSupported = RedisVersionUtils.atMost("2.6.4", connection);
-		initConnection();
-		assumeTrue(execErrorSupported);
 		connection.execute("ZadD", getClass() + "#foo\t0.90\titem");
 		try {
 			getResults();
-			fail("Execute failures should result in a RedisPipelineException");
-		} catch (RedisPipelineException e) {
+			fail("Expected an Exception to be thrown executing a command with syntax error");
+		}catch(RedisPipelineException e) {
+			// Redis 2.4, Exception occurs when we get the result of execute on closePipeline
+			if(RedisVersionUtils.atLeast("2.6.4", byteConnection)) {
+				fail("RedisPipelineException should not be thrown in Redis 2.6");
+			}
+		}catch(RedisSystemException e) {
+			try {
+				connection.closePipeline();
+			}catch(Exception ex) {
+				// Gonna get another error on closing the pipeline as results from execute
+			}
+			if(!RedisVersionUtils.atLeast("2.6", byteConnection)) {
+				// Redis 2.6 returns an ErrorReploy on exec, the Exception occurs when we call exec()
+				// b/c it waits on the response
+				fail("RedisSystemException should only be thrown in Redis 2.6");
+			}
 		}
 	}