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 e6c849129..47d39180d 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java @@ -456,7 +456,7 @@ public class DefaultStringRedisConnection implements StringRedisConnection { return delegate.sRandMember(key); } - public Set sRandMember(byte[] key, long count) { + public List sRandMember(byte[] key, long count) { return delegate.sRandMember(key, count); } @@ -1072,14 +1072,13 @@ public class DefaultStringRedisConnection implements StringRedisConnection { return deserialize(delegate.sRandMember(serialize(key))); } - public Set sRandMember(String key, long count) { + public List sRandMember(String key, long count) { return deserialize(delegate.sRandMember(serialize(key), count)); } public Boolean sRem(String key, String value) { return delegate.sRem(serialize(key), serialize(value)); } - public Long strLen(String key) { return delegate.strLen(serialize(key)); 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 f839dc773..c4a3c5bd3 100644 --- a/src/main/java/org/springframework/data/redis/connection/RedisSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/RedisSetCommands.java @@ -16,6 +16,7 @@ package org.springframework.data.redis.connection; +import java.util.List; import java.util.Set; /** @@ -53,5 +54,5 @@ public interface RedisSetCommands { byte[] sRandMember(byte[] key); - Set sRandMember(byte[] key, long count); + List sRandMember(byte[] key, long count); } 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 8de88419d..f36ec121a 100644 --- a/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java @@ -187,7 +187,7 @@ public interface StringRedisConnection extends RedisConnection { String sRandMember(String key); - Set sRandMember(String key, long count); + List sRandMember(String key, long count); Boolean zAdd(String key, double score, String value); 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 f532e700a..271a23802 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 @@ -1696,7 +1696,7 @@ public class JedisConnection implements RedisConnection { } } - public Set sRandMember(byte[] key, long count) { + public List sRandMember(byte[] key, long count) { throw new UnsupportedOperationException(); } diff --git a/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java b/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java index 6470497e4..9c77db5c0 100644 --- a/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java @@ -888,7 +888,7 @@ public class JredisConnection implements RedisConnection { } - public Set sRandMember(byte[] key, long count) { + public List sRandMember(byte[] key, long count) { throw new UnsupportedOperationException(); } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java index 3de5ec8ea..8632de64f 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java @@ -1484,13 +1484,17 @@ public class LettuceConnection implements RedisConnection { } } - public Set sRandMember(byte[] key, long count) { + public List sRandMember(byte[] key, long count) { + if(count < 0) { + throw new UnsupportedOperationException("sRandMember with a negative count is not supported"); + } try { if (isPipelined()) { pipeline(getAsyncConnection().srandmember(key, count)); return null; } - return getConnection().srandmember(key, count); + Set results = getConnection().srandmember(key, count); + return results != null ? new ArrayList(results) : null; } catch (Exception ex) { throw convertLettuceAccessException(ex); } diff --git a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java index 3b9394062..af0ba3411 100644 --- a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java @@ -1397,13 +1397,13 @@ public class SrpConnection implements RedisConnection { } } - public Set sRandMember(byte[] key, long count) { + public List sRandMember(byte[] key, long count) { try { if (isPipelined()) { pipeline(pipeline.srandmember(key, count)); return null; } - return SrpUtils.toSet(((MultiBulkReply)client.srandmember(key, count)).data()); + return SrpUtils.toBytesList(((MultiBulkReply)client.srandmember(key, count)).data()); } catch (Exception ex) { throw convertSrpAccessException(ex); } diff --git a/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java b/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java index 378b4b85c..85f8bc1a6 100644 --- a/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.core; import java.util.Collection; +import java.util.List; import java.util.Set; /** @@ -62,7 +63,9 @@ public interface BoundSetOperations extends BoundKeyOperations { V randomMember(); - Set randomMembers(long count); + Set distinctRandomMembers(long count); + + List randomMembers(long count); Boolean remove(Object o); diff --git a/src/main/java/org/springframework/data/redis/core/DefaultBoundSetOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultBoundSetOperations.java index 65a211098..9c2a8a2e8 100644 --- a/src/main/java/org/springframework/data/redis/core/DefaultBoundSetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/DefaultBoundSetOperations.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.core; import java.util.Collection; +import java.util.List; import java.util.Set; import org.springframework.data.redis.connection.DataType; @@ -114,11 +115,16 @@ class DefaultBoundSetOperations extends DefaultBoundKeyOperations imple } - public Set randomMembers(long count) { - return ops.randomMembers(getKey(), count); + public Set distinctRandomMembers(long count) { + return ops.distinctRandomMembers(getKey(), count); } + public List randomMembers(long count) { + return ops.randomMembers(getKey(), count); + } + + public Boolean remove(Object o) { return ops.remove(getKey(), o); } 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 71466999b..f67bc0ff2 100644 --- a/src/main/java/org/springframework/data/redis/core/DefaultSetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/DefaultSetOperations.java @@ -17,6 +17,8 @@ package org.springframework.data.redis.core; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; +import java.util.List; import java.util.Set; import org.springframework.data.redis.connection.RedisConnection; @@ -163,18 +165,38 @@ class DefaultSetOperations extends AbstractOperations implements Set } - public Set randomMembers(K key, final long count) { + public Set distinctRandomMembers(K key, final long count) { + if(count < 0) { + throw new IllegalArgumentException("Negative count not supported. " + + "Use randomMembers to allow duplicate elements."); + } final byte[] rawKey = rawKey(key); Set rawValues = execute(new RedisCallback>() { public Set doInRedis(RedisConnection connection) { - return connection.sRandMember(rawKey, count); + return new HashSet(connection.sRandMember(rawKey, count)); + } + }, true); + + return deserializeValues(rawValues); + } + + + public List randomMembers(K key, final long count) { + if(count < 0) { + throw new IllegalArgumentException("Use a positive number for count. " + + "This method is already allowing duplicate elements."); + } + final byte[] rawKey = rawKey(key); + List rawValues = execute(new RedisCallback>() { + public List doInRedis(RedisConnection connection) { + return connection.sRandMember(rawKey, - count); } }, true); return deserializeValues(rawValues); } - + public Boolean remove(K key, Object o) { final byte[] rawKey = rawKey(key); final byte[] rawValue = rawValue(o); 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 8f02eeed6..d277fa582 100644 --- a/src/main/java/org/springframework/data/redis/core/SetOperations.java +++ b/src/main/java/org/springframework/data/redis/core/SetOperations.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.core; import java.util.Collection; +import java.util.List; import java.util.Set; /** @@ -60,7 +61,9 @@ public interface SetOperations { V randomMember(K key); - Set randomMembers(K key, long count); + Set distinctRandomMembers(K key, long count); + + List randomMembers(K key, long count); Boolean remove(K key, Object o); 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 68454e611..60ea9c116 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -27,6 +27,7 @@ import static org.springframework.data.redis.SpinBarrier.waitFor; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -1202,7 +1203,7 @@ public abstract class AbstractConnectionIntegrationTests { actual.add(connection.sAdd("myset", "bar")); actual.add(connection.sAdd("myset", "baz")); actual.add(connection.sRandMember("myset", 2)); - assertTrue(((Set)convertResults().get(3)).size() == 2); + assertTrue(((Collection)convertResults().get(3)).size() == 2); } @SuppressWarnings("rawtypes") @@ -1211,8 +1212,7 @@ public abstract class AbstractConnectionIntegrationTests { public void testSRandMemberCountNegative() { actual.add(connection.sAdd("myset", "foo")); actual.add(connection.sRandMember("myset", -2)); - // APIs filter out duplicates so the negative has no effect - assertTrue(((Set)convertResults().get(1)).size() == 1); + assertEquals(Arrays.asList(new String[] {"foo", "foo"}), (List)convertResults().get(1)); } @SuppressWarnings("rawtypes") @@ -1220,7 +1220,7 @@ public abstract class AbstractConnectionIntegrationTests { @IfProfileValue(name = "redisVersion", value = "2.6") public void testSRandMemberCountKeyNotExists() { actual.add(connection.sRandMember("notexist", 2)); - assertTrue(((Set)convertResults().get(0)).isEmpty()); + assertTrue(((Collection)convertResults().get(0)).isEmpty()); } @Test diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java index e79f96760..7d98c2fc0 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionIntegrationTests.java @@ -225,6 +225,12 @@ public class LettuceConnectionIntegrationTests extends AbstractConnectionIntegra actual.add(connection.bitOp(BitOperation.NOT, "key3", "key1", "key2")); } + @Test(expected=UnsupportedOperationException.class) + @IfProfileValue(name = "redisVersion", value = "2.6") + public void testSRandMemberCountNegative() { + super.testSRandMemberCountNegative(); + } + @Test @IfProfileValue(name = "runLongTests", value = "true") public void testScriptKill() throws Exception{ diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java index 420c62ecd..81c5e08cf 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionPipelineIntegrationTests.java @@ -382,6 +382,12 @@ public class LettuceConnectionPipelineIntegrationTests extends } } + @Test(expected=UnsupportedOperationException.class) + @IfProfileValue(name = "redisVersion", value = "2.6") + public void testSRandMemberCountNegative() { + super.testSRandMemberCountNegative(); + } + @Test @IfProfileValue(name = "redisVersion", value = "2.6") public void testEvalReturnFalse() { diff --git a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java index f3927958b..2736ab110 100644 --- a/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/srp/SrpConnectionPipelineIntegrationTests.java @@ -157,27 +157,6 @@ public class SrpConnectionPipelineIntegrationTests extends verifyResults(Arrays.asList(new Object[] { 1l, 1l, 1l, 0l }), actual); } - @Test - @IfProfileValue(name = "redisVersion", value = "2.6") - public void testSRandMemberCount() { - convertResultToSet = true; - super.testSRandMemberCount(); - } - - @Test - @IfProfileValue(name = "redisVersion", value = "2.6") - public void testSRandMemberCountKeyNotExists() { - convertResultToSet = true; - super.testSRandMemberCountKeyNotExists(); - } - - @Test - @IfProfileValue(name = "redisVersion", value = "2.6") - public void testSRandMemberCountNegative() { - convertResultToSet = true; - super.testSRandMemberCountNegative(); - } - @Test public void testZIncrBy() { actual.add(connection.zAdd("myset", 2, "Bob")); 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 a4c9ae3f4..8d591b467 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 java.util.Arrays; +import java.util.List; import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -24,7 +25,9 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.redis.RedisTestProfileValueSource; import org.springframework.test.annotation.IfProfileValue; +import org.springframework.test.annotation.ProfileValueSourceConfiguration; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -36,6 +39,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("RedisTemplateTests-context.xml") +@ProfileValueSourceConfiguration(RedisTestProfileValueSource.class) public class DefaultSetOperationsTests { @Autowired @@ -55,12 +59,32 @@ public class DefaultSetOperationsTests { @Test @IfProfileValue(name = "redisVersion", value = "2.6") - public void testRandomMembers() { + public void testDistinctRandomMembers() { setOps.add("test", "foo"); setOps.add("test", "bar"); setOps.add("test", "baz"); - Set members = setOps.randomMembers("test", 2); + Set members = setOps.distinctRandomMembers("test", 2); assertEquals(2, members.size()); assertTrue(Arrays.asList(new String[] {"foo", "bar", "baz"}).containsAll(members)); } + + @Test + @IfProfileValue(name = "redisVersion", value = "2.6") + public void testRandomMembersWithDuplicates() { + setOps.add("test", "foo"); + List members = setOps.randomMembers("test", 2); + assertEquals(Arrays.asList(new String[] {"foo", "foo"}), members); + } + + @Test(expected=IllegalArgumentException.class) + @IfProfileValue(name = "redisVersion", value = "2.6") + public void testRandomMembersNegative() { + setOps.randomMembers("test", -1); + } + + @Test(expected=IllegalArgumentException.class) + @IfProfileValue(name = "redisVersion", value = "2.6") + public void testDistinctRandomMembersNegative() { + setOps.distinctRandomMembers("test", -2); + } } diff --git a/src/test/resources/org/springframework/data/redis/core/RedisTemplateTests-context.xml b/src/test/resources/org/springframework/data/redis/core/RedisTemplateTests-context.xml index 00cb8ae78..4114b87f0 100644 --- a/src/test/resources/org/springframework/data/redis/core/RedisTemplateTests-context.xml +++ b/src/test/resources/org/springframework/data/redis/core/RedisTemplateTests-context.xml @@ -3,8 +3,8 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:p="http://www.springframework.org/schema/p" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd"> - + @@ -16,7 +16,7 @@ - + \ No newline at end of file