From 9e473b5dcda747d2f9517ccd9bc08113cebc06b1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 12 Sep 2023 10:23:08 +0200 Subject: [PATCH] Polishing. Reorder methods to align with ListOperations. Simplify tests to avoid test noise. See #2692 Original pull request: #2704 --- .../core/DefaultReactiveListOperations.java | 32 ++++++------ .../redis/core/ReactiveListOperations.java | 42 ++++++++------- .../core/ReactiveListOperationsExtensions.kt | 36 ++++++------- ...eactiveListOperationsIntegrationTests.java | 52 ++++++------------- 4 files changed, 72 insertions(+), 90 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 index 430daec5f..e3e9ee8c4 100644 --- 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 @@ class DefaultReactiveListOperations implements ReactiveListOperations 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 leftPop(K key, Duration timeout) { @@ -252,14 +260,6 @@ class DefaultReactiveListOperations implements ReactiveListOperations readValue(popResult.getValue()))); } - @Override - public Flux 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 rightPop(K key) { @@ -268,6 +268,14 @@ class DefaultReactiveListOperations implements ReactiveListOperations listCommands.rPop(rawKey(key)).map(this::readValue)); } + @Override + public Flux 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 rightPop(K key, Duration timeout) { @@ -280,14 +288,6 @@ class DefaultReactiveListOperations implements ReactiveListOperations readValue(popResult.getValue()))); } - @Override - public Flux 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 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 index 6a13c75cc..fd153ef02 100644 --- 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 @@ public interface ReactiveListOperations { */ Mono 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 Redis Documentation: LPOP + */ + Flux leftPop(K key, long count); + /** * Removes and returns first element from lists stored at {@code key}.
* Results return once an element available or {@code timeout} reached. @@ -326,16 +337,6 @@ public interface ReactiveListOperations { */ Mono 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 leftPop(K key, long count); - /** * Removes and returns last element in list stored at {@code key}. * @@ -345,6 +346,17 @@ public interface ReactiveListOperations { */ Mono 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 Redis Documentation: RPOP + */ + Flux rightPop(K key, long count); + /** * Removes and returns last element from lists stored at {@code key}.
* Results return once an element available or {@code timeout} reached. @@ -358,16 +370,6 @@ public interface ReactiveListOperations { */ Mono 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 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 index e0c71ef1f..f5bbc2408 100644 --- a/src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt +++ b/src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt @@ -174,6 +174,15 @@ suspend fun ReactiveListOperations.indexAndAwait(key: K suspend fun ReactiveListOperations.leftPopAndAwait(key: K): V? = leftPop(key).awaitFirstOrNull() +/** + * Coroutines variant of [ReactiveListOperations.leftPop] with count. + * + * @author John Blum + * @since 3.2 + */ +fun ReactiveListOperations.leftPopAsFlow(key: K, count :Long): Flow = + leftPop(key, count).asFlow() + /** * Coroutines variant of [ReactiveListOperations.leftPop]. * @@ -183,15 +192,6 @@ suspend fun ReactiveListOperations.leftPopAndAwait(key: suspend fun ReactiveListOperations.leftPopAndAwait(key: K, timeout: Duration): V? = leftPop(key, timeout).awaitFirstOrNull() -/** - * Coroutines variant of [ReactiveListOperations.leftPop] with count. - * - * @author John Blum - * @since 3.2 - */ -fun ReactiveListOperations.leftPopAsFlow(key: K, count :Long): Flow = - leftPop(key, count).asFlow() - /** * Coroutines variant of [ReactiveListOperations.rightPop]. * @@ -201,6 +201,15 @@ fun ReactiveListOperations.leftPopAsFlow(key: K, count suspend fun ReactiveListOperations.rightPopAndAwait(key: K): V? = rightPop(key).awaitFirstOrNull() +/** + * Coroutines variant of [ReactiveListOperations.rightPop] with count. + * + * @author John Blum + * @since 3.2 + */ +fun ReactiveListOperations.rightPopAsFlow(key: K, count:Long): Flow = + rightPop(key, count).asFlow() + /** * Coroutines variant of [ReactiveListOperations.rightPop]. * @@ -210,15 +219,6 @@ suspend fun ReactiveListOperations.rightPopAndAwait(key suspend fun ReactiveListOperations.rightPopAndAwait(key: K, timeout: Duration): V? = rightPop(key, timeout).awaitFirstOrNull() -/** - * Coroutines variant of [ReactiveListOperations.rightPop] with count. - * - * @author John Blum - * @since 3.2 - */ -fun ReactiveListOperations.rightPopAsFlow(key: K, count:Long): Flow = - rightPop(key, count).asFlow() - /** * Coroutines variant of [ReactiveListOperations.rightPopAndLeftPush]. * diff --git a/src/test/java/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java index 181d46d1b..1d24e6431 100644 --- a/src/test/java/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/core/DefaultReactiveListOperationsIntegrationTests.java @@ -121,7 +121,7 @@ public class DefaultReactiveListOperationsIntegrationTests { @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 { @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 { @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 { @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 { @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 { @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 { @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 { @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 { @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 { @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 { 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 { @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 { 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 { @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 { @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 { @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 { @EnabledIfLongRunningTest void rightPopAndLeftPushWithTimeout() { - assumeThat(valueFactory instanceof ByteBufferObjectFactory).isFalse(); + assumeThat(this.valueFactory).isNotInstanceOf(ByteBufferObjectFactory.class); K source = keyFactory.instance(); K target = keyFactory.instance();