From b07fe8d5824c5957c60b7dfe717449cfb2097269 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/JedisClusterConnection.java | 11 +++- .../connection/jedis/JedisConnection.java | 16 +++-- .../connection/jedis/JedisConverters.java | 61 ++++++++++--------- .../jedis/JedisClusterConnectionTests.java | 12 ++++ ...disConnectionPipelineIntegrationTests.java | 8 +-- ...ConnectionTransactionIntegrationTests.java | 8 +-- .../LettuceClusterConnectionTests.java | 17 +++++- 7 files changed, 83 insertions(+), 50 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java index 24ab27051..7c3bbe78e 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java +++ b/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 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); + } } /* diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index ab73f12df..211877cc9 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/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 tuples) { - if (isPipelined() || isQueueing()) { - throw new UnsupportedOperationException("zAdd of multiple fields not supported " + "in pipeline or transaction"); - } - Map args = JedisConverters.zAddArgsConvertor(tuples); + + Map 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); } 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 ad5404e1a..8cb493ebb 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 @@ -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 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(); } @@ -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 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/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java index 6966befda..610345498 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 @@ -1141,6 +1141,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 6885ce56a..c28dfde69 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 @@ -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 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")), @@ -1157,6 +1158,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() {