diff --git a/src/main/java/org/springframework/data/redis/core/AbstractOperations.java b/src/main/java/org/springframework/data/redis/core/AbstractOperations.java index 55cd771c7..e2dc42b99 100644 --- 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 @@ abstract class AbstractOperations { return rawValues; } + /** + * @param values must not be {@literal empty} nor contain {@literal null} values. + * @return + * @since 1.5 + */ byte[][] rawValues(Collection 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 index 4720afd0c..743fdda24 100644 --- 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 @@ import org.springframework.util.CollectionUtils; * @author Costin Leau * @author David Liu * @author Thomas Darimont + * @author Christoph Strobl */ class DefaultListOperations extends AbstractOperations implements ListOperations { @@ -86,6 +87,23 @@ class DefaultListOperations extends AbstractOperations implements Li }, true); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.ListOperations#leftPushAll(java.lang.Object, java.util.Collection) + */ + @Override + public Long leftPushAll(K key, Collection values) { + + final byte[] rawKey = rawKey(key); + final byte[][] rawValues = rawValues(values); + + return execute(new RedisCallback() { + 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 @@ class DefaultListOperations extends AbstractOperations implements Li }, true); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.ListOperations#rightPushAll(java.lang.Object, java.util.Collection) + */ + @Override + public Long rightPushAll(K key, Collection values) { + + final byte[] rawKey = rawKey(key); + final byte[][] rawValues = rawValues(values); + + return execute(new RedisCallback() { + 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 @@ class DefaultListOperations extends AbstractOperations implements Li }, true); } - @Override - public Long rightPushAll(K key, Collection values) { - - final byte[] rawKey = rawKey(key); - final byte[][] rawValues = rawValues(values); - - return execute(new RedisCallback() { - public Long doInRedis(RedisConnection connection) { - return connection.rPush(rawKey, rawValues); - } - }, true); - - } - - @Override - public Long leftPushAll(K key, Collection values) { - - final byte[] rawKey = rawKey(key); - final byte[][] rawValues = rawValues(values); - - return execute(new RedisCallback() { - 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 index 6d558e419..e816bb2e1 100644 --- 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 @@ import java.util.concurrent.TimeUnit; * @author Costin Leau * @author David Liu * @author Thomas Darimont + * @author Christoph Strobl */ public interface ListOperations { @@ -39,8 +40,10 @@ public interface ListOperations { 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 { 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 index 4c705c337..275fae5d4 100644 --- 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 static org.springframework.data.redis.matcher.RedisTestMatchers.*; 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 @@ import org.springframework.data.redis.connection.RedisConnection; * * @author Jennifer Hickey * @author Thomas Darimont + * @author Christoph Strobl * @param Key test * @param Value test */ @@ -178,8 +180,7 @@ public class DefaultListOperationsTests { * @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 class DefaultListOperationsTests { 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. 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) 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 class DefaultListOperationsTests { assertEquals(Long.valueOf(3), listOps.leftPushAll(key, Arrays. 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. 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) null); + } }