Polishing.

Reorder methods to align with ListOperations. Simplify tests to avoid test noise. See #2692 Original pull request: #2704
spring-projects · Sep 12, 2023 · 9e473b5 · 9e473b5
1 parent 157f5e4
commit 9e473b5
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 83 deletions.
diff --git a/src/main/java/org/springframework/data/redis/core/DefaultReactiveListOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultReactiveListOperations.java
@@ -240,6 +240,14 @@ public Mono<V> leftPop(K key) {
 
 	}
 
+	@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) {
 
@@ -253,19 +261,19 @@ public Mono<V> leftPop(K key, Duration timeout) {
 	}
 
 	@Override
-	public Flux<V> leftPop(K key, long count) {
+	public Mono<V> rightPop(K key) {
 
 		Assert.notNull(key, "Key must not be null");
 
-		return createFlux(listCommands -> listCommands.lPop(rawKey(key), count).map(this::readValue));
+		return createMono(listCommands -> listCommands.rPop(rawKey(key)).map(this::readValue));
 	}
 
 	@Override
-	public Mono<V> rightPop(K key) {
+	public Flux<V> rightPop(K key, long count) {
 
 		Assert.notNull(key, "Key must not be null");
 
-		return createMono(listCommands -> listCommands.rPop(rawKey(key)).map(this::readValue));
+		return createFlux(listCommands -> listCommands.rPop(rawKey(key), count).map(this::readValue));
 	}
 
 	@Override
@@ -280,14 +288,6 @@ public Mono<V> rightPop(K key, Duration timeout) {
 						.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) {
 

diff --git a/src/main/java/org/springframework/data/redis/core/ReactiveListOperations.java b/src/main/java/org/springframework/data/redis/core/ReactiveListOperations.java
@@ -313,6 +313,17 @@ default Mono<V> move(MoveFrom<K> from, MoveTo<K> to, Duration timeout) {
 	 */
 	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>
@@ -327,23 +338,24 @@ default Mono<V> move(MoveFrom<K> from, MoveTo<K> to, Duration timeout) {
 	Mono<V> leftPop(K key, Duration timeout);
 
 	/**
-	 * Removes {@link Long count} elements from the left-side of the Redis list stored at key.
+	 * Removes and returns last element in list stored at {@code 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
+	 * @param key must not be {@literal null}.
+	 * @return
+	 * @see <a href="https://redis.io/commands/rpop">Redis Documentation: RPOP</a>
 	 */
-	Flux<V> leftPop(K key, long count);
+	Mono<V> rightPop(K key);
 
 	/**
-	 * Removes and returns last element in list stored at {@code key}.
+	 * Removes {@link Long count} elements from the right-side of the Redis list stored at key.
 	 *
 	 * @param key must not be {@literal null}.
-	 * @return
+	 * @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>
 	 */
-	Mono<V> rightPop(K key);
+	Flux<V> rightPop(K key, long count);
 
 	/**
 	 * Removes and returns last element from lists stored at {@code key}. <br>
@@ -358,16 +370,6 @@ default Mono<V> move(MoveFrom<K> from, MoveTo<K> to, Duration timeout) {
 	 */
 	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.
 	 *

diff --git a/src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt b/src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt
@@ -174,41 +174,32 @@ 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].
- *
- * @author Mark Paluch
- * @since 2.2
- */
-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()
+	leftPop(key, count).asFlow()
 
 /**
- * Coroutines variant of [ReactiveListOperations.rightPop].
+ * Coroutines variant of [ReactiveListOperations.leftPop].
  *
  * @author Mark Paluch
  * @since 2.2
  */
-suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key: K): V? =
-		rightPop(key).awaitFirstOrNull()
+suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.leftPopAndAwait(key: K, timeout: Duration): V? =
+		leftPop(key, timeout).awaitFirstOrNull()
 
 /**
  * Coroutines variant of [ReactiveListOperations.rightPop].
  *
  * @author Mark Paluch
  * @since 2.2
  */
-suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key: K, timeout: Duration): V? =
-		rightPop(key, timeout).awaitFirstOrNull()
+suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key: K): V? =
+		rightPop(key).awaitFirstOrNull()
 
 /**
  * Coroutines variant of [ReactiveListOperations.rightPop] with count.
@@ -217,7 +208,16 @@ suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key
  * @since 3.2
  */
 fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAsFlow(key: K, count:Long): Flow<V> =
-		rightPop(key, count).asFlow()
+	rightPop(key, count).asFlow()
+
+/**
+ * Coroutines variant of [ReactiveListOperations.rightPop].
+ *
+ * @author Mark Paluch
+ * @since 2.2
+ */
+suspend fun <K : Any, V : Any> ReactiveListOperations<K, V>.rightPopAndAwait(key: K, timeout: Duration): V? =
+		rightPop(key, timeout).awaitFirstOrNull()
 
 /**
  * Coroutines variant of [ReactiveListOperations.rightPopAndLeftPush].

diff --git a/...va/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java b/...va/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java
@@ -121,7 +121,7 @@ void size() {
 	@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 @@ void leftPush() {
 	@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 @@ void leftPushIfPresent() {
 	@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 @@ void leftPushWithPivot() {
 	@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 @@ void rightPush() {
 	@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 @@ void rightPushIfPresent() {
 	@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 @@ void moveWithTimeout() {
 	@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 @@ void set() {
 	@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 @@ void remove() {
 	@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 @@ void lastIndexOf() {
 	@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 @@ void leftPop() {
 		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 @@ void leftPopWithCount() {
 	@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 @@ void rightPop() {
 		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 @@ void rightPopWithCount() {
 	@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 @@ void leftPopWithMillisecondTimeoutShouldFail() {
 	@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 @@ void rightPopWithTimeout() {
 	@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 @@ void rightPopAndLeftPush() {
 	@EnabledIfLongRunningTest
 	void rightPopAndLeftPushWithTimeout() {
 
-		assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse();
+		assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class);
 
 		K source = keyFactory.instance();
 		K target = keyFactory.instance();