diff --git a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java index b70438bb3..97e0ae622 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java @@ -868,6 +868,20 @@ public class DefaultStringRedisConnection implements StringRedisConnection, Deco return result; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisSetCommands#sPop(byte[], long) + */ + @Override + public List sPop(byte[] key, long count) { + + List result = delegate.sPop(key, count); + if (isFutureConversion()) { + addResultConverter(identityConverter); + } + return result; + } + public byte[] sRandMember(byte[] key) { byte[] result = delegate.sRandMember(key); if (isFutureConversion()) { @@ -2032,6 +2046,22 @@ public class DefaultStringRedisConnection implements StringRedisConnection, Deco return bytesToString.convert(result); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.StringRedisConnection#sPop(java.lang.String, long) + */ + @Override + public List sPop(String key, long count) { + + List result = delegate.sPop(serialize(key), count); + + if (isFutureConversion()) { + addResultConverter(byteListToStringList); + } + + return byteListToStringList.convert(result); + } + public String sRandMember(String key) { byte[] result = delegate.sRandMember(serialize(key)); if (isFutureConversion()) { diff --git a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java index 7eb8edf26..60b89aa8f 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java @@ -563,6 +563,13 @@ public interface DefaultedRedisConnection extends RedisConnection { return setCommands().sPop(key); } + /** @deprecated in favor of {@link RedisConnection#setCommands()}}. */ + @Override + @Deprecated + default List sPop(byte[] key, long count) { + return setCommands().sPop(key, count); + } + /** @deprecated in favor of {@link RedisConnection#setCommands()}}. */ @Override @Deprecated diff --git a/src/main/java/org/springframework/data/redis/connection/ReactiveSetCommands.java b/src/main/java/org/springframework/data/redis/connection/ReactiveSetCommands.java index bbface735..366527d93 100644 --- a/src/main/java/org/springframework/data/redis/connection/ReactiveSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/ReactiveSetCommands.java @@ -251,6 +251,39 @@ public interface ReactiveSetCommands { */ Flux> sRem(Publisher commands); + /** + * {@code SPOP} command parameters. + * + * @author Christoph Strobl + * @see Redis Documentation: SPOP + */ + class SPopCommand extends KeyCommand { + + private final long count; + + private SPopCommand(ByteBuffer key, long count) { + + super(key); + this.count = count; + } + + public static SPopCommand one() { + return new SPopCommand(null, 1L); + } + + public static SPopCommand members(long count) { + return new SPopCommand(null, count); + } + + public SPopCommand from(ByteBuffer key) { + return new SPopCommand(key, count); + } + + public long getCount() { + return count; + } + } + /** * Remove and return a random member from set at {@literal key}. * @@ -262,9 +295,32 @@ public interface ReactiveSetCommands { Assert.notNull(key, "Key must not be null!"); - return sPop(Mono.just(new KeyCommand(key))).next().map(ByteBufferResponse::getOutput); + return sPop(Mono.just(SPopCommand.one().from(key))).next().map(ByteBufferResponse::getOutput); } + /** + * Remove and return a random member from set at {@literal key}. + * + * @param key must not be {@literal null}. + * @return + * @see Redis Documentation: SPOP + */ + default Flux sPop(ByteBuffer key, long count) { + + Assert.notNull(key, "Key must not be null!"); + + return sPop(SPopCommand.members(count).from(key)); + } + + /** + * Remove and return a random member from set at {@literal key}. + * + * @param command must not be {@literal null}. + * @return + * @see Redis Documentation: SPOP + */ + Flux sPop(SPopCommand command); + /** * Remove and return a random member from set at {@link KeyCommand#getKey()} * diff --git a/src/main/java/org/springframework/data/redis/connection/RedisSetCommands.java b/src/main/java/org/springframework/data/redis/connection/RedisSetCommands.java index 818ff5e27..3c9a0c875 100644 --- a/src/main/java/org/springframework/data/redis/connection/RedisSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/RedisSetCommands.java @@ -59,6 +59,17 @@ public interface RedisSetCommands { */ byte[] sPop(byte[] key); + /** + * Remove and return {@code count} random members from set at {@code key}. + * + * @param key must not be {@literal null}. + * @param count the number of random members to pop from the set. + * @return empty {@link List} if set does not exist. Never {@literal null}. + * @see Redis Documentation: SPOP + * @since 2.0 + */ + List sPop(byte[] key, long count); + /** * Move {@code value} from {@code srcKey} to {@code destKey} * diff --git a/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java b/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java index 714c9c4f7..9e12f8361 100644 --- a/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java @@ -781,6 +781,18 @@ public interface StringRedisConnection extends RedisConnection { */ String sPop(String key); + /** + * Remove and return {@code count} random members from set at {@code key}. + * + * @param key must not be {@literal null}. + * @param count the number of random members to return. + * @return empty {@link List} if {@literal key} does not exist. + * @see Redis Documentation: SPOP + * @see RedisSetCommands#sPop(byte[], long) + * @since 2.0 + */ + List sPop(String key, long count); + /** * Move {@code value} from {@code srcKey} to {@code destKey} * diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterSetCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterSetCommands.java index 8b9ef2ecd..42a4169cc 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterSetCommands.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.connection.jedis; import redis.clients.jedis.ScanParams; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -87,6 +88,20 @@ class JedisClusterSetCommands implements RedisSetCommands { } } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisSetCommands#sPop(byte[], long) + */ + @Override + public List sPop(byte[] key, long count) { + + try { + return new ArrayList<>(connection.getCluster().spop(key, count)); + } catch (Exception ex) { + throw convertJedisAccessException(ex); + } + } + /* * (non-Javadoc) * @see org.springframework.data.redis.connection.RedisSetCommands#sMove(byte[], byte[], byte[]) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisSetCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisSetCommands.java index c1f0403a0..003d3acaa 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisSetCommands.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.connection.jedis; import redis.clients.jedis.ScanParams; +import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -251,6 +252,28 @@ class JedisSetCommands implements RedisSetCommands { } } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisSetCommands#sPop(byte[], long) + */ + @Override + public List sPop(byte[] key, long count) { + + try { + if (isPipelined()) { + pipeline(connection.newJedisResult(connection.getPipeline().spop(key, count), ArrayList::new)); + return null; + } + if (isQueueing()) { + transaction(connection.newJedisResult(connection.getTransaction().spop(key, count), ArrayList::new)); + return null; + } + return new ArrayList<>(connection.getJedis().spop(key, count)); + } catch (Exception ex) { + throw convertJedisAccessException(ex); + } + } + /* * (non-Javadoc) * @see org.springframework.data.redis.connection.RedisSetCommands#sRandMember(byte[]) diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommands.java index eda75c42c..c3c82a549 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommands.java @@ -99,6 +99,19 @@ class LettuceReactiveSetCommands implements ReactiveSetCommands { })); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.ReactiveSetCommands#sPop(org.springframework.data.redis.connection.ReactiveSetCommands.SPopCommand) + */ + @Override + public Flux sPop(SPopCommand command) { + + Assert.notNull(command, "Command must not be null!"); + Assert.notNull(command.getKey(), "Key must not be null!"); + + return connection.execute(cmd -> cmd.spop(command.getKey(), command.getCount())); + } + /* * (non-Javadoc) * @see org.springframework.data.redis.connection.ReactiveSetCommands#sMove(org.reactivestreams.Publisher) diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceSetCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceSetCommands.java index 97ae364ed..a32e6690a 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceSetCommands.java @@ -21,9 +21,11 @@ import io.lettuce.core.ValueScanCursor; import io.lettuce.core.cluster.api.async.RedisClusterAsyncCommands; import io.lettuce.core.cluster.api.sync.RedisClusterCommands; +import java.util.ArrayList; import java.util.List; import java.util.Set; +import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; import org.springframework.data.redis.connection.RedisSetCommands; import org.springframework.data.redis.connection.lettuce.LettuceConnection.LettuceResult; @@ -255,6 +257,29 @@ class LettuceSetCommands implements RedisSetCommands { } } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisSetCommands#sPop(byte[], long) + */ + @Override + public List sPop(byte[] key, long count) { + try { + if (isPipelined()) { + pipeline(connection.newLettuceResult(getAsyncConnection().spop(key, count), + (Converter, List>) ArrayList::new)); + return null; + } + if (isQueueing()) { + transaction(connection.newLettuceTxResult(getConnection().spop(key, count), + (Converter, List>) ArrayList::new)); + return null; + } + return new ArrayList<>(getConnection().spop(key, count)); + } catch (Exception ex) { + throw convertLettuceAccessException(ex); + } + } + /* * (non-Javadoc) * @see org.springframework.data.redis.connection.RedisSetCommands#sRandMember(byte[]) diff --git a/src/main/java/org/springframework/data/redis/core/DefaultReactiveSetOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultReactiveSetOperations.java index fd7841107..068ba7bcd 100644 --- a/src/main/java/org/springframework/data/redis/core/DefaultReactiveSetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/DefaultReactiveSetOperations.java @@ -106,6 +106,18 @@ public class DefaultReactiveSetOperations implements ReactiveSetOperations return createMono(connection -> connection.sPop(rawKey(key)).map(this::readValue)); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.ReactiveSetOperations#pop(java.lang.Object, long) + */ + @Override + public Flux pop(K key, long count) { + + Assert.notNull(key, "Key must not be null!"); + + return createFlux(connection -> connection.sPop(rawKey(key), count).map(this::readValue)); + } + /* (non-Javadoc) * @see org.springframework.data.redis.core.ReactiveSetOperations#move(java.lang.Object, java.lang.Object, java.lang.Object) */ diff --git a/src/main/java/org/springframework/data/redis/core/DefaultSetOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultSetOperations.java index f70301469..288556ede 100644 --- a/src/main/java/org/springframework/data/redis/core/DefaultSetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/DefaultSetOperations.java @@ -206,6 +206,19 @@ class DefaultSetOperations extends AbstractOperations implements Set }, true); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.SetOperations(java.lang.Object, long) + */ + @Override + public List pop(K key, long count) { + + final byte[] rawKey = rawKey(key); + + List rawValues = execute(connection -> connection.sPop(rawKey, count), true); + return deserializeValues(rawValues); + } + public Long size(K key) { final byte[] rawKey = rawKey(key); return execute(new RedisCallback() { diff --git a/src/main/java/org/springframework/data/redis/core/ReactiveSetOperations.java b/src/main/java/org/springframework/data/redis/core/ReactiveSetOperations.java index 071150159..36df4192a 100644 --- a/src/main/java/org/springframework/data/redis/core/ReactiveSetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/ReactiveSetOperations.java @@ -59,6 +59,16 @@ public interface ReactiveSetOperations { */ Mono pop(K key); + /** + * Remove and return {@code count} random members from set at {@code key}. + * + * @param key must not be {@literal null}. + * @param count nr of members to return + * @return {@link Flux} emitting random members. + * @see Redis Documentation: SPOP + */ + Flux pop(K key, long count); + /** * Move {@code value} from {@code key} to {@code destKey} * diff --git a/src/main/java/org/springframework/data/redis/core/SetOperations.java b/src/main/java/org/springframework/data/redis/core/SetOperations.java index 81cdcb269..6af29efaf 100644 --- a/src/main/java/org/springframework/data/redis/core/SetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/SetOperations.java @@ -57,6 +57,17 @@ public interface SetOperations { */ V pop(K key); + /** + * Remove and return {@code count} random members from set at {@code key}. + * + * @param key must not be {@literal null}. + * @param count nr of members to return. + * @return empty {@link List} if key does not exist. Never {@literal null}. + * @see Redis Documentation: SPOP + * @since 2.0 + */ + List pop(K key, long count); + /** * Move {@code value} from {@code key} to {@code destKey} * diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java index 6fb1f475e..cae527f81 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -27,7 +27,18 @@ import static org.springframework.data.redis.connection.RedisGeoCommands.Distanc import static org.springframework.data.redis.connection.RedisGeoCommands.GeoRadiusCommandArgs.*; import static org.springframework.data.redis.core.ScanOptions.*; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.UUID; import java.util.concurrent.BlockingDeque; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingDeque; @@ -36,6 +47,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.hamcrest.core.IsNot; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; import org.junit.Ignore; import org.junit.Rule; @@ -1458,6 +1470,17 @@ public abstract class AbstractConnectionIntegrationTests { new HashSet(Arrays.asList(new String[] { "foo", "bar" })).contains((String) getResults().get(2))); } + @Test // DATAREDIS-688 + public void testSPopWithCount() { + + actual.add(connection.sAdd("myset", "foo")); + actual.add(connection.sAdd("myset", "bar")); + actual.add(connection.sAdd("myset", "baz")); + actual.add(connection.sPop("myset", 2)); + + assertThat((Collection) getResults().get(3), hasSize(2)); + } + @Test public void testSRandMember() { actual.add(connection.sAdd("myset", "foo")); 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 6665e326d..ca9d9b4b8 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 @@ -28,7 +28,16 @@ import redis.clients.jedis.JedisPool; import java.io.IOException; import java.nio.charset.Charset; -import java.util.*; +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; +import java.util.Properties; +import java.util.Set; import java.util.concurrent.TimeUnit; import org.junit.After; @@ -919,6 +928,15 @@ public class JedisClusterConnectionTests implements ClusterConnectionTests { assertThat(clusterConnection.sPop(KEY_1_BYTES), notNullValue()); } + @Test // DATAREDIS-668 + public void sPopWithCountShouldPopValueFromSetCorrectly() { + + nativeConnection.sadd(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + assertThat(clusterConnection.setCommands().sPop(KEY_1_BYTES, 2), hasSize(2)); + assertThat(nativeConnection.scard(KEY_1), is(1L)); + } + @Test // DATAREDIS-315 public void sMoveShouldWorkWhenKeysMapToSameSlot() { 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 dbd836241..07c911ed8 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 @@ -934,6 +934,15 @@ public class LettuceClusterConnectionTests implements ClusterConnectionTests { assertThat(clusterConnection.sPop(KEY_1_BYTES), notNullValue()); } + @Test // DATAREDIS-668 + public void sPopWithCountShouldPopValueFromSetCorrectly() { + + nativeConnection.sadd(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + assertThat(clusterConnection.setCommands().sPop(KEY_1_BYTES, 2), hasSize(2)); + assertThat(nativeConnection.scard(KEY_1), is(1L)); + } + @Test // DATAREDIS-315 public void sMoveShouldWorkWhenKeysMapToSameSlot() { diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommandsTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommandsTests.java index 84bb5ce3b..ed91d724c 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommandsTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveSetCommandsTests.java @@ -69,6 +69,14 @@ public class LettuceReactiveSetCommandsTests extends LettuceReactiveCommandsTest assertThat(connection.setCommands().sPop(KEY_1_BBUFFER).block(), is(notNullValue())); } + @Test // DATAREDIS-668 + public void sPopCountShouldRetrieveValues() { + + nativeCommands.sadd(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + StepVerifier.create(connection.setCommands().sPop(KEY_1_BBUFFER, 2)).expectNextCount(2).verifyComplete(); + } + @Test // DATAREDIS-525 public void sPopShouldReturnNullWhenNotPresent() { assertThat(connection.setCommands().sPop(KEY_1_BBUFFER).block(), is(nullValue())); diff --git a/src/test/java/org/springframework/data/redis/core/DefaultReactiveSetOperationsIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/DefaultReactiveSetOperationsIntegrationTests.java index 7ec3fff66..2557ab955 100644 --- a/src/test/java/org/springframework/data/redis/core/DefaultReactiveSetOperationsIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/core/DefaultReactiveSetOperationsIntegrationTests.java @@ -133,6 +133,21 @@ public class DefaultReactiveSetOperationsIntegrationTests { }).verifyComplete(); } + @Test // DATAREDIS-668 + public void popWithCount() { + + assumeFalse(valueFactory instanceof ByteBufferObjectFactory); + + K key = keyFactory.instance(); + V value1 = valueFactory.instance(); + V value2 = valueFactory.instance(); + V value3 = valueFactory.instance(); + + StepVerifier.create(setOperations.add(key, value1, value2, value3)).expectNext(3L).verifyComplete(); + StepVerifier.create(setOperations.pop(key, 2)).expectNextCount(2).verifyComplete(); + StepVerifier.create(setOperations.size(key)).expectNext(1L).verifyComplete(); + } + @Test // DATAREDIS-602 public void move() { diff --git a/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsTests.java b/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsTests.java index 8dca965c0..d1cf4988b 100644 --- a/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsTests.java +++ b/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsTests.java @@ -16,6 +16,7 @@ package org.springframework.data.redis.core; import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.collection.IsCollectionWithSize.hasSize; import static org.junit.Assert.*; import static org.junit.Assume.*; import static org.springframework.data.redis.matcher.RedisTestMatchers.*; @@ -28,6 +29,7 @@ import java.util.List; import java.util.Set; import org.hamcrest.CoreMatchers; +import org.hamcrest.collection.IsCollectionWithSize; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -175,6 +177,20 @@ public class DefaultSetOperationsTests { assertTrue(setOps.members(key).isEmpty()); } + @Test // DATAREDIS-668 + public void testPopWithCount() { + + K key = keyFactory.instance(); + V v1 = valueFactory.instance(); + V v2 = valueFactory.instance(); + V v3 = valueFactory.instance(); + setOps.add(key, v1, v2, v3); + + List result = setOps.pop(key, 2); + assertThat(result, hasSize(2)); + assertThat(result.get(0), instanceOf(v1.getClass())); + } + @SuppressWarnings("unchecked") @Test public void testRandomMember() {