Polishing.

Reorder methods to align with ListOperations. Simplify tests to avoid test noise.

See #2692
Original pull request: #2704

src/main/java/org/springframework/data/redis/core/DefaultReactiveListOperations.java

@@ -240,6 +240,14 @@ class DefaultReactiveListOperations<K, V> implements ReactiveListOperations<K, V
 
 	}
 
+	@Override
+	public Flux<V> leftPop(K key, long count) {
+
+		Assert.notNull(key, "Key must not be null");
+
+		return createFlux(listCommands -> listCommands.lPop(rawKey(key), count).map(this::readValue));
+	}
+
 	@Override
 	public Mono<V> leftPop(K key, Duration timeout) {
 
@@ -252,14 +260,6 @@ class DefaultReactiveListOperations<K, V> implements ReactiveListOperations<K, V
 						.map(popResult -> readValue(popResult.getValue())));
 	}
 
-	@Override
-	public Flux<V> leftPop(K key, long count) {
-
-		Assert.notNull(key, "Key must not be null");
-
-		return createFlux(listCommands -> listCommands.lPop(rawKey(key), count).map(this::readValue));
-	}
-
 	@Override
 	public Mono<V> rightPop(K key) {
 
@@ -268,6 +268,14 @@ class DefaultReactiveListOperations<K, V> implements ReactiveListOperations<K, V
 		return createMono(listCommands -> listCommands.rPop(rawKey(key)).map(this::readValue));
 	}
 
+	@Override
+	public Flux<V> rightPop(K key, long count) {
+
+		Assert.notNull(key, "Key must not be null");
+
+		return createFlux(listCommands -> listCommands.rPop(rawKey(key), count).map(this::readValue));
+	}
+
 	@Override
 	public Mono<V> rightPop(K key, Duration timeout) {
 
@@ -280,14 +288,6 @@ class DefaultReactiveListOperations<K, V> implements ReactiveListOperations<K, V
 						.map(popResult -> readValue(popResult.getValue())));
 	}
 
-	@Override
-	public Flux<V> rightPop(K key, long count) {
-
-		Assert.notNull(key, "Key must not be null");
-
-		return createFlux(listCommands -> listCommands.rPop(rawKey(key), count).map(this::readValue));
-	}
-
 	@Override
 	public Mono<V> rightPopAndLeftPush(K sourceKey, K destinationKey) {
 

src/main/java/org/springframework/data/redis/core/ReactiveListOperations.java

@@ -313,6 +313,17 @@ public interface ReactiveListOperations<K, V> {
 	 */
 	Mono<V> leftPop(K key);
 
+	/**
+	 * Removes {@link Long count} elements from the left-side of the Redis list stored at key.
+	 *
+	 * @param key must not be {@literal null}.
+	 * @param count {@link Long count} of the number of elements to remove from the left-side of the Redis list.
+	 * @return a {@link Flux} containing the elements removed from the Redis list.
+	 * @since 3.2
+	 * @see <a href="https://redis.io/commands/lpop">Redis Documentation: LPOP</a>
+	 */
+	Flux<V> leftPop(K key, long count);
+
 	/**
 	 * Removes and returns first element from lists stored at {@code key}. <br>
 	 * <b>Results return once an element available or {@code timeout} reached.</b>
@@ -326,16 +337,6 @@ public interface ReactiveListOperations<K, V> {
 	 */
 	Mono<V> leftPop(K key, Duration timeout);
 
-	/**
-	 * Removes {@link Long count} elements from the left-side of the Redis list stored at key.
-	 *
-	 * @param key {@link K Key} referring to the list stored in Redis; must not be {@literal null}.
-	 * @param count {@link Long count} of the number of elements to remove from the left-side of the Redis list.
-	 * @return a {@link Flux} containing the elements removed from the Redis list.
-	 * @since 3.2
-	 */
-	Flux<V> leftPop(K key, long count);
-
 	/**
 	 * Removes and returns last element in list stored at {@code key}.
 	 *
@@ -345,6 +346,17 @@ public interface ReactiveListOperations<K, V> {
 	 */
 	Mono<V> rightPop(K key);
 
+	/**
+	 * Removes {@link Long count} elements from the right-side of the Redis list stored at key.
+	 *
+	 * @param key must not be {@literal null}.
+	 * @param count {@link Long count} of the number of elements to remove from the right-side of the Redis list.
+	 * @return a {@link Flux} containing the elements removed from the Redis list.
+	 * @since 3.2
+	 * @see <a href="https://redis.io/commands/rpop">Redis Documentation: RPOP</a>
+	 */
+	Flux<V> rightPop(K key, long count);
+
 	/**
 	 * Removes and returns last element from lists stored at {@code key}. <br>
 	 * <b>Results return once an element available or {@code timeout} reached.</b>
@@ -358,16 +370,6 @@ public interface ReactiveListOperations<K, V> {
 	 */
 	Mono<V> rightPop(K key, Duration timeout);
 
-	/**
-	 * Removes {@link Long count} elements from the right-side of the Redis list stored at key.
-	 *
-	 * @param key {@link K Key} referring to the list stored in Redis; must not be {@literal null}.
-	 * @param count {@link Long count} of the number of elements to remove from the right-side of the Redis list.
-	 * @return a {@link Flux} containing the elements removed from the Redis list.
-	 * @since 3.2
-	 */
-	Flux<V> rightPop(K key, long count);
-
 	/**
 	 * Remove the last element from list at {@code sourceKey}, append it to {@code destinationKey} and return its value.
 	 *

src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt

@@ -174,6 +174,15 @@ suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.indexAndAwait(key: K
 suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAndAwait(key: K): V? =
 		leftPop(key).awaitFirstOrNull()
 
+/**
+ * Coroutines variant of [ReactiveListOperations.leftPop] with count.
+ *
+ * @author John Blum
+ * @since 3.2
+ */
+fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAsFlow(key: K, count :Long): Flow<V> =
+	leftPop(key, count).asFlow()
+
 /**
  * Coroutines variant of [ReactiveListOperations.leftPop].
  *
@@ -183,15 +192,6 @@ suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAndAwait(key:
 suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAndAwait(key: K, timeout: Duration): V? =
 		leftPop(key, timeout).awaitFirstOrNull()
 
-/**
- * Coroutines variant of [ReactiveListOperations.leftPop] with count.
- *
- * @author John Blum
- * @since 3.2
- */
-fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAsFlow(key: K, count :Long): Flow<V> =
-		leftPop(key, count).asFlow()
-
 /**
  * Coroutines variant of [ReactiveListOperations.rightPop].
  *
@@ -201,6 +201,15 @@ fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAsFlow(key: K, count
 suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key: K): V? =
 		rightPop(key).awaitFirstOrNull()
 
+/**
+ * Coroutines variant of [ReactiveListOperations.rightPop] with count.
+ *
+ * @author John Blum
+ * @since 3.2
+ */
+fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAsFlow(key: K, count:Long): Flow<V> =
+	rightPop(key, count).asFlow()
+
 /**
  * Coroutines variant of [ReactiveListOperations.rightPop].
  *
@@ -210,15 +219,6 @@ suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key
 suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key: K, timeout: Duration): V? =
 		rightPop(key, timeout).awaitFirstOrNull()
 
-/**
- * Coroutines variant of [ReactiveListOperations.rightPop] with count.
- *
- * @author John Blum
- * @since 3.2
- */
-fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAsFlow(key: K, count:Long): Flow<V> =
-		rightPop(key, count).asFlow()
-
 /**
  * Coroutines variant of [ReactiveListOperations.rightPopAndLeftPush].
  *

src/test/java/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java

@@ -121,7 +121,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void leftPush() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -147,7 +147,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void leftPushAll() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -191,7 +191,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void leftPushWithPivot() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -219,7 +219,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void rightPush() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -244,7 +244,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void rightPushAll() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -274,7 +274,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void rightPushWithPivot() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -364,7 +364,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void set() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -384,7 +384,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void remove() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -406,7 +406,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void index() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -448,7 +448,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void leftPop() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -459,20 +459,10 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 		listOperations.leftPop(key).as(StepVerifier::create).expectNext(value2).verifyComplete();
 	}
 
-	@ParameterizedRedisTest // GH-2692
-	@SuppressWarnings("all")
-	void leftPopWithNullKey() {
-
-		assertThatIllegalArgumentException()
-			.isThrownBy(() -> this.listOperations.leftPop(null, 100L))
-			.withMessage("Key must not be null")
-			.withNoCause();
-	}
-
 	@ParameterizedRedisTest // GH-2692
 	void leftPopWithCount() {
 
-		assumeThat(this.valueFactory).isInstanceOf(ByteBufferObjectFactory.class);
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -494,7 +484,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void rightPop() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -505,16 +495,6 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 		listOperations.rightPop(key).as(StepVerifier::create).expectNext(value2).verifyComplete();
 	}
 
-	@ParameterizedRedisTest // GH-2692
-	@SuppressWarnings("all")
-	void rightPopWithNullKey() {
-
-		assertThatIllegalArgumentException()
-				.isThrownBy(() -> this.listOperations.rightPop(null, 100L))
-				.withMessage("Key must not be null")
-				.withNoCause();
-	}
-
 	@ParameterizedRedisTest // GH-2692
 	void rightPopWithCount() {
 
@@ -540,7 +520,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void leftPopWithTimeout() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -562,7 +542,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void rightPopWithTimeout() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K key = keyFactory.instance();
 		V value1 = valueFactory.instance();
@@ -576,7 +556,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@ParameterizedRedisTest // DATAREDIS-602
 	void rightPopAndLeftPush() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K source = keyFactory.instance();
 		K target = keyFactory.instance();
@@ -594,7 +574,7 @@ public class DefaultReactiveListOperationsIntegrationTests<K, V> {
 	@EnabledIfLongRunningTest
 	void rightPopAndLeftPushWithTimeout() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K source = keyFactory.instance();
 		K target = keyFactory.instance();