Support srandmember with negative count

DATAREDIS-116

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<byte[]> sRandMember(byte[] key, long count) {
+	public List<byte[]> 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<String> sRandMember(String key, long count) {
+	public List<String> 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));

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<byte[]> sRandMember(byte[] key, long count);
+	List<byte[]> sRandMember(byte[] key, long count);
 }

src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java

@@ -187,7 +187,7 @@ public interface StringRedisConnection extends RedisConnection {
 
 	String sRandMember(String key);
 
-	Set<String> sRandMember(String key, long count);
+	List<String> sRandMember(String key, long count);
 
 	Boolean zAdd(String key, double score, String value);
 

src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java

@@ -1696,7 +1696,7 @@ public class JedisConnection implements RedisConnection {
 		}
 	}
 
-	public Set<byte[]> sRandMember(byte[] key, long count) {
+	public List<byte[]> sRandMember(byte[] key, long count) {
 		throw new UnsupportedOperationException();
 	}
 

src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java

@@ -888,7 +888,7 @@ public class JredisConnection implements RedisConnection {
 	}
 
 
-	public Set<byte[]> sRandMember(byte[] key, long count) {
+	public List<byte[]> sRandMember(byte[] key, long count) {
 		throw new UnsupportedOperationException();
 	}
 

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java

@@ -1484,13 +1484,17 @@ public class LettuceConnection implements RedisConnection {
 		}
 	}
 
-	public Set<byte[]> sRandMember(byte[] key, long count) {
+	public List<byte[]> 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<byte[]> results = getConnection().srandmember(key, count);
+			return results != null ? new ArrayList<byte[]>(results) : null;
 		} catch (Exception ex) {
 			throw convertLettuceAccessException(ex);
 		}

src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java

@@ -1397,13 +1397,13 @@ public class SrpConnection implements RedisConnection {
 		}
 	}
 
-	public Set<byte[]> sRandMember(byte[] key, long count) {
+	public List<byte[]> 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);
 		}

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<K, V> extends BoundKeyOperations<K> {
 
 	V randomMember();
 
-	Set<V> randomMembers(long count);
+	Set<V> distinctRandomMembers(long count);
+
+	List<V> randomMembers(long count);
 
 	Boolean remove(Object o);
 

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<K, V> extends DefaultBoundKeyOperations<K> imple
 	}
 
 
-	public Set<V> randomMembers(long count) {
-		return ops.randomMembers(getKey(), count);
+	public Set<V> distinctRandomMembers(long count) {
+		return ops.distinctRandomMembers(getKey(), count);
 	}
 
 	
+	public List<V> randomMembers(long count) {
+		return ops.randomMembers(getKey(), count);
+	}
+
+
 	public Boolean remove(Object o) {
 		return ops.remove(getKey(), o);
 	}

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<K, V> extends AbstractOperations<K, V> implements Set
 	}
 
 
-	public Set<V> randomMembers(K key, final long count) {
+	public Set<V> 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<byte[]> rawValues = execute(new RedisCallback<Set<byte[]>>() {
 			public Set<byte[]> doInRedis(RedisConnection connection) {
-				return connection.sRandMember(rawKey, count);
+				return new HashSet<byte[]>(connection.sRandMember(rawKey, count));
+			}
+		}, true);
+
+		return deserializeValues(rawValues);
+	}
+	
+
+	public List<V> 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<byte[]> rawValues = execute(new RedisCallback<List<byte[]>>() {
+			public List<byte[]> 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);

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<K, V> {
 
 	V randomMember(K key);
 
-	Set<V> randomMembers(K key, long count);
+	Set<V> distinctRandomMembers(K key, long count);
+
+	List<V> randomMembers(K key, long count);
 
 	Boolean remove(K key, Object o);
 

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

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{

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() {

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"));

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<String> members = setOps.randomMembers("test", 2);
+		Set<String> 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<String> 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);
+	}
 }

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">
 
-	<bean id="lettuceConnectionFactory"
-		class="org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory">
+	<bean id="srpConnectionFactory"
+		class="org.springframework.data.redis.connection.srp.SrpConnectionFactory">
 		<property name="hostName">
 			<bean class="org.springframework.data.redis.SettingsUtils"
 				factory-method="getHost" />
@@ -16,7 +16,7 @@
 	</bean>
 
 	<bean class="org.springframework.data.redis.core.StringRedisTemplate">
-		<property name="connectionFactory" ref="lettuceConnectionFactory" />
+		<property name="connectionFactory" ref="srpConnectionFactory" />
 	</bean>
 
 </beans>