DATAREDIS-288 - Polishing.

Added javadoc and assertions to ensure failing before actually sending data to redis and backed those with tests. Original pull request: #114.
spring-projects · Nov 17, 2014 · 57fc58f · 57fc58f
1 parent d897733
commit 57fc58f
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 34 deletions.
diff --git a/src/main/java/org/springframework/data/redis/core/AbstractOperations.java b/src/main/java/org/springframework/data/redis/core/AbstractOperations.java
@@ -121,8 +121,16 @@ byte[][] rawValues(Object... values) {
 		return rawValues;
 	}
 
+	/**
+	 * @param values must not be {@literal empty} nor contain {@literal null} values.
+	 * @return
+	 * @since 1.5
+	 */
 	byte[][] rawValues(Collection<V> values) {
 
+		Assert.notEmpty(values, "Values must not be 'null' or empty.");
+		Assert.noNullElements(values.toArray(), "Values must not contain 'null' value.");
+
 		byte[][] rawValues = new byte[values.size()][];
 		int i = 0;
 		for (V value : values) {

diff --git a/src/main/java/org/springframework/data/redis/core/DefaultListOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultListOperations.java
@@ -29,6 +29,7 @@
  * @author Costin Leau
  * @author David Liu
  * @author Thomas Darimont
+ * @author Christoph Strobl
  */
 class DefaultListOperations<K, V> extends AbstractOperations<K, V> implements ListOperations<K, V> {
 
@@ -86,6 +87,23 @@ public Long doInRedis(RedisConnection connection) {
 		}, true);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.core.ListOperations#leftPushAll(java.lang.Object, java.util.Collection)
+	 */
+	@Override
+	public Long leftPushAll(K key, Collection<V> values) {
+
+		final byte[] rawKey = rawKey(key);
+		final byte[][] rawValues = rawValues(values);
+
+		return execute(new RedisCallback<Long>() {
+			public Long doInRedis(RedisConnection connection) {
+				return connection.lPush(rawKey, rawValues);
+			}
+		}, true);
+	}
+
 	public Long leftPushIfPresent(K key, V value) {
 		final byte[] rawKey = rawKey(key);
 		final byte[] rawValue = rawValue(value);
@@ -181,6 +199,23 @@ public Long doInRedis(RedisConnection connection) {
 		}, true);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.core.ListOperations#rightPushAll(java.lang.Object, java.util.Collection)
+	 */
+	@Override
+	public Long rightPushAll(K key, Collection<V> values) {
+
+		final byte[] rawKey = rawKey(key);
+		final byte[][] rawValues = rawValues(values);
+
+		return execute(new RedisCallback<Long>() {
+			public Long doInRedis(RedisConnection connection) {
+				return connection.rPush(rawKey, rawValues);
+			}
+		}, true);
+	}
+
 	public Long rightPushIfPresent(K key, V value) {
 		final byte[] rawKey = rawKey(key);
 		final byte[] rawValue = rawValue(value);
@@ -249,30 +284,4 @@ protected byte[] inRedis(byte[] rawKey, RedisConnection connection) {
 		}, true);
 	}
 
-	@Override
-	public Long rightPushAll(K key, Collection<V> values) {
-
-		final byte[] rawKey = rawKey(key);
-		final byte[][] rawValues = rawValues(values);
-
-		return execute(new RedisCallback<Long>() {
-			public Long doInRedis(RedisConnection connection) {
-				return connection.rPush(rawKey, rawValues);
-			}
-		}, true);
-
-	}
-
-	@Override
-	public Long leftPushAll(K key, Collection<V> values) {
-
-		final byte[] rawKey = rawKey(key);
-		final byte[][] rawValues = rawValues(values);
-
-		return execute(new RedisCallback<Long>() {
-			public Long doInRedis(RedisConnection connection) {
-				return connection.lPush(rawKey, rawValues);
-			}
-		}, true);
-	}
 }
diff --git a/src/main/java/org/springframework/data/redis/core/ListOperations.java b/src/main/java/org/springframework/data/redis/core/ListOperations.java
@@ -25,6 +25,7 @@
  * @author Costin Leau
  * @author David Liu
  * @author Thomas Darimont
+ * @author Christoph Strobl
  */
 public interface ListOperations<K, V> {
 
@@ -39,8 +40,10 @@ public interface ListOperations<K, V> {
 	Long leftPushAll(K key, V... values);
 
 	/**
-	 * @param key
-	 * @param values
+	 * Insert all {@literal values} at the head of the list stored at {@literal key}.
+	 * 
+	 * @param key must not be {@literal null}.
+	 * @param values must not be {@literal empty} nor contain {@literal null} values.
 	 * @return
 	 * @since 1.5
 	 */
@@ -55,8 +58,10 @@ public interface ListOperations<K, V> {
 	Long rightPushAll(K key, V... values);
 
 	/**
-	 * @param key
-	 * @param values
+	 * Insert all {@literal values} at the tail of the list stored at {@literal key}.
+	 * 
+	 * @param key must not be {@literal null}.
+	 * @param values must not be {@literal empty} nor contain {@literal null} values.
 	 * @return
 	 * @since 1.5
 	 */

diff --git a/src/test/java/org/springframework/data/redis/core/DefaultListOperationsTests.java b/src/test/java/org/springframework/data/redis/core/DefaultListOperationsTests.java
@@ -21,6 +21,7 @@
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.After;
@@ -38,6 +39,7 @@
  * 
  * @author Jennifer Hickey
  * @author Thomas Darimont
+ * @author Christoph Strobl
  * @param <K> Key test
  * @param <V> Value test
  */
@@ -178,8 +180,7 @@ public void testRightPushAll() {
 	 * @see DATAREDIS-288
 	 */
 	@Test
-	@SuppressWarnings("all")
-	// get rid of varargs warning
+	@SuppressWarnings("unchecked")
 	public void testRightPushAllCollection() {
 
 		K key = keyFactory.instance();
@@ -192,12 +193,36 @@ public void testRightPushAllCollection() {
 		assertThat(listOps.range(key, 0, -1), isEqual(Arrays.asList(new Object[] { v1, v2, v3 })));
 	}
 
+	/**
+	 * @see DATAREDIS-288
+	 */
+	@Test(expected = IllegalArgumentException.class)
+	public void rightPushAllShouldThrowExceptionWhenCalledWithEmptyCollection() {
+		listOps.rightPushAll(keyFactory.instance(), Collections.<V> emptyList());
+	}
+
+	/**
+	 * @see DATAREDIS-288
+	 */
+	@SuppressWarnings("unchecked")
+	@Test(expected = IllegalArgumentException.class)
+	public void rightPushAllShouldThrowExceptionWhenCollectionContainsNullValue() {
+		listOps.rightPushAll(keyFactory.instance(), Arrays.asList(valueFactory.instance(), null));
+	}
+
+	/**
+	 * @see DATAREDIS-288
+	 */
+	@Test(expected = IllegalArgumentException.class)
+	public void rightPushAllShouldThrowExceptionWhenCalledWithNull() {
+		listOps.rightPushAll(keyFactory.instance(), (Collection<V>) null);
+	}
+
 	/**
 	 * @see DATAREDIS-288
 	 */
 	@Test
-	@SuppressWarnings("all")
-	// get rid of varargs warning
+	@SuppressWarnings("unchecked")
 	public void testLeftPushAllCollection() {
 
 		K key = keyFactory.instance();
@@ -209,4 +234,29 @@ public void testLeftPushAllCollection() {
 		assertEquals(Long.valueOf(3), listOps.leftPushAll(key, Arrays.<V> asList(v1, v2, v3)));
 		assertThat(listOps.range(key, 0, -1), isEqual(Arrays.asList(new Object[] { v3, v2, v1 })));
 	}
+
+	/**
+	 * @see DATAREDIS-288
+	 */
+	@Test(expected = IllegalArgumentException.class)
+	public void leftPushAllShouldThrowExceptionWhenCalledWithEmptyCollection() {
+		listOps.leftPushAll(keyFactory.instance(), Collections.<V> emptyList());
+	}
+
+	/**
+	 * @see DATAREDIS-288
+	 */
+	@SuppressWarnings("unchecked")
+	@Test(expected = IllegalArgumentException.class)
+	public void leftPushAllShouldThrowExceptionWhenCollectionContainsNullValue() {
+		listOps.leftPushAll(keyFactory.instance(), Arrays.asList(valueFactory.instance(), null));
+	}
+
+	/**
+	 * @see DATAREDIS-288
+	 */
+	@Test(expected = IllegalArgumentException.class)
+	public void leftPushAllShouldThrowExceptionWhenCalledWithNull() {
+		listOps.leftPushAll(keyFactory.instance(), (Collection<V>) null);
+	}
 }