diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterZSetCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterZSetCommands.java index 22a96d29d..9d72bc957 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterZSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterZSetCommands.java @@ -19,7 +19,6 @@ import redis.clients.jedis.ScanParams; import redis.clients.jedis.ZParams; import java.util.Set; -import java.util.Map; import org.springframework.dao.DataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -66,10 +65,8 @@ class JedisClusterZSetCommands implements RedisZSetCommands { @Override public Long zAdd(byte[] key, Set tuples) { - Map args = JedisConverters.zAddArgsConvertor(tuples); - try { - return connection.getCluster().zadd(key, args); + return connection.getCluster().zadd(key, JedisConverters.toTupleMap(tuples)); } catch (Exception ex) { throw convertJedisAccessException(ex); } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java index 17d91ba91..6a9899cfa 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java @@ -238,6 +238,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 2.0 + */ + public static Map toTupleMap(Set tuples) { + + Assert.notNull(tuples, "Tuple set must not be null!"); + + Map args = new LinkedHashMap<>(tuples.size(), 1); + Set scores = new HashSet<>(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(); } @@ -660,33 +692,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 zAddArgsConvertor(Set tuples) { - - Map args = new LinkedHashMap<>(tuples.size(), 1); - Set scores = new HashSet<>(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; - } } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisZSetCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisZSetCommands.java index 5b6078ddc..8fe879a54 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisZSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisZSetCommands.java @@ -19,7 +19,6 @@ import redis.clients.jedis.ScanParams; import redis.clients.jedis.ScanResult; import redis.clients.jedis.ZParams; -import java.util.Map; import java.util.Set; import org.springframework.data.redis.connection.RedisZSetCommands; @@ -32,6 +31,7 @@ import org.springframework.util.Assert; /** * @author Christoph Strobl + * @author Clement Ong * @since 2.0 */ class JedisZSetCommands implements RedisZSetCommands { @@ -73,13 +73,17 @@ class JedisZSetCommands implements RedisZSetCommands { @Override public Long zAdd(byte[] key, Set tuples) { - if (isPipelined() || isQueueing()) { - throw new UnsupportedOperationException("zAdd of multiple fields not supported " + "in pipeline or transaction"); - } - - Map args = JedisConverters.zAddArgsConvertor(tuples); try { - return connection.getJedis().zadd(key, args); + if (isPipelined()) { + pipeline(connection.newJedisResult(connection.getPipeline().zadd(key, JedisConverters.toTupleMap(tuples)))); + return null; + } + if (isQueueing()) { + transaction( + connection.newJedisResult(connection.getTransaction().zadd(key, JedisConverters.toTupleMap(tuples)))); + return null; + } + return connection.getJedis().zadd(key, JedisConverters.toTupleMap(tuples)); } catch (Exception ex) { throw convertJedisAccessException(ex); } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java index 07d9475c3..7e18db6f8 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java @@ -1159,6 +1159,18 @@ public class JedisClusterConnectionTests implements ClusterConnectionTests { assertThat(nativeConnection.zcard(KEY_1_BYTES), is(2L)); } + @Test // DATAREDIS-674 + public void zAddShouldAddMultipleValuesWithScoreCorrectly() { + + Set tuples = new HashSet<>(); + 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() { diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java index e55f5470d..aaf4f342e 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java +++ b/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(); diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java index 60943daec..e6fe8f464 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java @@ -30,8 +30,9 @@ import org.springframework.test.context.ContextConfiguration; *

* 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() { diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java index 6c5ec5325..754538189 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java @@ -32,6 +32,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.Map; @@ -92,8 +93,8 @@ public class LettuceClusterConnectionTests implements ClusterConnectionTests { static final GeoLocation CATANIA = new GeoLocation<>("catania", POINT_CATANIA); static final GeoLocation PALERMO = new GeoLocation<>("palermo", POINT_PALERMO); - static final GeoLocation ARIGENTO_BYTES = new GeoLocation<>( - "arigento".getBytes(Charset.forName("UTF-8")), POINT_ARIGENTO); + static final GeoLocation ARIGENTO_BYTES = new GeoLocation<>("arigento".getBytes(Charset.forName("UTF-8")), + POINT_ARIGENTO); static final GeoLocation CATANIA_BYTES = new GeoLocation<>("catania".getBytes(Charset.forName("UTF-8")), POINT_CATANIA); static final GeoLocation PALERMO_BYTES = new GeoLocation<>("palermo".getBytes(Charset.forName("UTF-8")), @@ -1165,6 +1166,18 @@ public class LettuceClusterConnectionTests implements ClusterConnectionTests { assertThat(nativeConnection.zcard(KEY_1), is(2L)); } + @Test // DATAREDIS-674 + public void zAddShouldAddMultipleValuesWithScoreCorrectly() { + + Set tuples = new HashSet<>(); + 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() {