DATAREDIS-674 - Polishing.

Rename JedisConverters.zAddArgsConvertor(…) to toTupleMap(…) and reorder method to group with other tuple conversion methods. Add author tags.

Create tests for zadd with tuple using Redis Cluster. Enable pipelining and transactions for zadd using Jedis. Adopt tests.

Original pull request: #263.

src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java

@@ -1600,11 +1600,18 @@ public class JedisClusterConnection implements RedisClusterConnection {
 		}
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisZSetCommands#zAdd(byte[], java.util.Set)
+	 */
 	@Override
 	public Long zAdd(byte[] key, Set<Tuple> tuples) {
 
-		// TODO: need to move the tuple conversion form jedisconnection.
-		throw new UnsupportedOperationException();
+		try {
+			return cluster.zadd(key, JedisConverters.toTupleMap(tuples));
+		} catch (Exception ex) {
+			throw convertJedisAccessException(ex);
+		}
 	}
 
 	/*

src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java

@@ -2229,12 +2229,18 @@ public class JedisConnection extends AbstractRedisConnection {
 	}
 
 	public Long zAdd(byte[] key, Set<Tuple> tuples) {
-		if (isPipelined() || isQueueing()) {
-			throw new UnsupportedOperationException("zAdd of multiple fields not supported " + "in pipeline or transaction");
-		}
-		Map<byte[], Double> args = JedisConverters.zAddArgsConvertor(tuples);
+
+		Map<byte[], Double> mappedTuples = JedisConverters.toTupleMap(tuples);
 		try {
-			return jedis.zadd(key, args);
+			if (isPipelined()) {
+				pipeline(new JedisResult(pipeline.zadd(key, mappedTuples)));
+				return null;
+			}
+			if (isQueueing()) {
+				transaction(new JedisResult(transaction.zadd(key, mappedTuples)));
+				return null;
+			}
+			return jedis.zadd(key, mappedTuples);
 		} catch (Exception ex) {
 			throw convertJedisAccessException(ex);
 		}

src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java

@@ -266,6 +266,38 @@ abstract public class JedisConverters extends Converters {
 		return TUPLE_SET_TO_TUPLE_SET.convert(source);
 	}
 
+	/**
+	 * Map a {@link Set} of {@link Tuple} by {@code value} to its {@code score}.
+	 *
+	 * @param tuples must not be {@literal null}.
+	 * @return
+	 * @since 1.8.7
+	 */
+	public static Map<byte[], Double> toTupleMap(Set<Tuple> tuples) {
+
+		Assert.notNull(tuples, "Tuple set must not be null!");
+
+		Map<byte[], Double> args = new LinkedHashMap<byte[], Double>(tuples.size(), 1);
+		Set<Double> scores = new HashSet<Double>(tuples.size(), 1);
+
+		boolean isAtLeastJedis24 = JedisVersionUtil.atLeastJedis24();
+
+		for (Tuple tuple : tuples) {
+
+			if (!isAtLeastJedis24) {
+				if (scores.contains(tuple.getScore())) {
+					throw new UnsupportedOperationException(
+							"Bulk add of multiple elements with the same score is not supported. Add the elements individually.");
+				}
+				scores.add(tuple.getScore());
+			}
+
+			args.put(tuple.getValue(), tuple.getScore());
+		}
+
+		return args;
+	}
+
 	public static byte[] toBytes(Integer source) {
 		return String.valueOf(source).getBytes();
 	}
@@ -688,33 +720,4 @@ abstract public class JedisConverters extends Converters {
 			}
 		}
 	}
-
-	/**
-	 * Convert tuples to map of bytes and double.
-	 * Bytes represents the value of the element and double is for the score.
-	 * @param tuples
-	 * @return
-	 */
-	public static Map<byte[], Double> zAddArgsConvertor(Set<Tuple> tuples) {
-
-		Map<byte[], Double> args = new LinkedHashMap<byte[], Double>(tuples.size(), 1);
-		Set<Double> scores = new HashSet<Double>(tuples.size(), 1);
-
-		boolean isAtLeastJedis24 = JedisVersionUtil.atLeastJedis24();
-
-		for (Tuple tuple : tuples) {
-
-			if (!isAtLeastJedis24) {
-				if (scores.contains(tuple.getScore())) {
-					throw new UnsupportedOperationException(
-						"Bulk add of multiple elements with the same score is not supported. Add the elements individually.");
-				}
-				scores.add(tuple.getScore());
-			}
-
-			args.put(tuple.getValue(), tuple.getScore());
-		}
-
-		return args;
-	}
 }

src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java

@@ -1141,6 +1141,18 @@ public class JedisClusterConnectionTests implements ClusterConnectionTests {
 		assertThat(nativeConnection.zcard(KEY_1_BYTES), is(2L));
 	}
 
+	@Test // DATAREDIS-674
+	public void zAddShouldAddMultipleValuesWithScoreCorrectly() {
+
+		Set<Tuple> tuples = new HashSet<Tuple>();
+		tuples.add(new DefaultTuple(VALUE_1_BYTES, 10D));
+		tuples.add(new DefaultTuple(VALUE_2_BYTES, 20D));
+
+		clusterConnection.zAdd(KEY_1_BYTES, tuples);
+
+		assertThat(nativeConnection.zcard(KEY_1_BYTES), is(2L));
+	}
+
 	@Test // DATAREDIS-315
 	public void zRemShouldRemoveValueWithScoreCorrectly() {
 

src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java

@@ -37,10 +37,11 @@ import redis.clients.jedis.JedisPoolConfig;
 
 /**
  * Integration test of {@link JedisConnection} pipeline functionality
- * 
+ *
  * @author Jennifer Hickey
  * @author Christoph Strobl
  * @author Thomas Darimont
+ * @author Mark Paluch
  */
 @RunWith(RelaxedJUnit4ClassRunner.class)
 @ContextConfiguration("JedisConnectionIntegrationTests-context.xml")
@@ -250,11 +251,6 @@ public class JedisConnectionPipelineIntegrationTests extends AbstractConnectionP
 		super.testInfoBySection();
 	}
 
-	@Test(expected = UnsupportedOperationException.class)
-	public void testZAddMultiple() {
-		super.testZAddMultiple();
-	}
-
 	@Test(expected = UnsupportedOperationException.class) // DATAREDIS-269
 	public void clientSetNameWorksCorrectly() {
 		super.clientSetNameWorksCorrectly();

src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java

@@ -30,8 +30,9 @@ import org.springframework.test.context.ContextConfiguration;
  * <p>
  * Each method of {@link JedisConnection} behaves differently if executed with a transaction (i.e. between multi and
  * exec or discard calls), so this test covers those branching points
- * 
+ *
  * @author Jennifer Hickey
+ * @author Mark Paluch
  */
 @RunWith(RelaxedJUnit4ClassRunner.class)
 @ContextConfiguration("JedisConnectionIntegrationTests-context.xml")
@@ -181,11 +182,6 @@ public class JedisConnectionTransactionIntegrationTests extends AbstractConnecti
 		super.testInfoBySection();
 	}
 
-	@Test(expected = UnsupportedOperationException.class)
-	public void testZAddMultiple() {
-		super.testZAddMultiple();
-	}
-
 	@Test(expected = InvalidDataAccessApiUsageException.class)
 	@IfProfileValue(name = "redisVersion", value = "2.6+")
 	public void testRestoreBadData() {

src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java

@@ -27,6 +27,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.ListIterator;
@@ -93,8 +94,8 @@ public class LettuceClusterConnectionTests implements ClusterConnectionTests {
 	static final GeoLocation<String> CATANIA = new GeoLocation<String>("catania", POINT_CATANIA);
 	static final GeoLocation<String> PALERMO = new GeoLocation<String>("palermo", POINT_PALERMO);
 
-	static final GeoLocation<byte[]> ARIGENTO_BYTES = new GeoLocation<byte[]>(
-			"arigento".getBytes(Charset.forName("UTF-8")), POINT_ARIGENTO);
+	static final GeoLocation<byte[]> ARIGENTO_BYTES = new GeoLocation<byte[]>("arigento".getBytes(Charset.forName("UTF-8")),
+			POINT_ARIGENTO);
 	static final GeoLocation<byte[]> CATANIA_BYTES = new GeoLocation<byte[]>("catania".getBytes(Charset.forName("UTF-8")),
 			POINT_CATANIA);
 	static final GeoLocation<byte[]> PALERMO_BYTES = new GeoLocation<byte[]>("palermo".getBytes(Charset.forName("UTF-8")),
@@ -1157,6 +1158,18 @@ public class LettuceClusterConnectionTests implements ClusterConnectionTests {
 		assertThat(nativeConnection.zcard(KEY_1), is(2L));
 	}
 
+	@Test // DATAREDIS-674
+	public void zAddShouldAddMultipleValuesWithScoreCorrectly() {
+
+		Set<Tuple> tuples = new HashSet<Tuple>();
+		tuples.add(new DefaultTuple(VALUE_1_BYTES, 10D));
+		tuples.add(new DefaultTuple(VALUE_2_BYTES, 20D));
+
+		clusterConnection.zAdd(KEY_1_BYTES, tuples);
+
+		assertThat(nativeConnection.zcard(KEY_1), is(2L));
+	}
+
 	@Test // DATAREDIS-315
 	public void zRemShouldRemoveValueWithScoreCorrectly() {