From 95500d4d728bc6916823af83a9f09ad8b48b966f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 25 Aug 2017 14:09:46 +0200 Subject: [PATCH] DATAREDIS-674 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../jedis/JedisClusterZSetCommands.java | 5 +- .../connection/jedis/JedisConverters.java | 61 ++++++++++--------- .../connection/jedis/JedisZSetCommands.java | 18 +++--- .../jedis/JedisClusterConnectionTests.java | 12 ++++ ...disConnectionPipelineIntegrationTests.java | 8 +-- ...ConnectionTransactionIntegrationTests.java | 8 +-- .../LettuceClusterConnectionTests.java | 17 +++++- 7 files changed, 75 insertions(+), 54 deletions(-) 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() {