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 | 24 ++++----- .../redis/core/ReactiveListOperations.java | 40 +++++++------- .../core/ReactiveListOperationsExtensions.kt | 32 ++++++------ ...eactiveListOperationsIntegrationTests.java | 52 ++++++------------- 4 files changed, 65 insertions(+), 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 index 430daec5f9..e3e9ee8c40 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 @@ public Mono leftPop(K key) { } + @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 leftPop(K key, Duration timeout) { @@ -253,19 +261,19 @@ public Mono leftPop(K key, Duration timeout) { } @Override - public Flux leftPop(K key, long count) { + public Mono 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 rightPop(K key) { + public Flux 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 rightPop(K key, Duration timeout) { .map(popResult -> 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 6a13c75ccf..fd153ef02e 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 @@ default Mono move(MoveFrom from, MoveTo to, Duration timeout) { */ 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. @@ -327,23 +338,24 @@ default Mono move(MoveFrom from, MoveTo to, Duration timeout) { Mono 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 Redis Documentation: RPOP */ - Flux leftPop(K key, long count); + Mono 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 Redis Documentation: RPOP */ - Mono rightPop(K key); + Flux rightPop(K key, long count); /** * Removes and returns last element from lists stored at {@code key}.
@@ -358,16 +370,6 @@ default Mono move(MoveFrom from, MoveTo to, Duration timeout) { */ 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 e0c71ef1f4..f5bbc2408f 100644 --- a/src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt +++ b/src/main/kotlin/org/springframework/data/redis/core/ReactiveListOperationsExtensions.kt @@ -174,15 +174,6 @@ suspend fun ReactiveListOperations.indexAndAwait(key: K suspend fun ReactiveListOperations.leftPopAndAwait(key: K): V? = leftPop(key).awaitFirstOrNull() -/** - * Coroutines variant of [ReactiveListOperations.leftPop]. - * - * @author Mark Paluch - * @since 2.2 - */ -suspend fun ReactiveListOperations.leftPopAndAwait(key: K, timeout: Duration): V? = - leftPop(key, timeout).awaitFirstOrNull() - /** * Coroutines variant of [ReactiveListOperations.leftPop] with count. * @@ -190,16 +181,16 @@ suspend fun ReactiveListOperations.leftPopAndAwait(key: * @since 3.2 */ fun ReactiveListOperations.leftPopAsFlow(key: K, count :Long): Flow = - 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 ReactiveListOperations.rightPopAndAwait(key: K): V? = - rightPop(key).awaitFirstOrNull() +suspend fun ReactiveListOperations.leftPopAndAwait(key: K, timeout: Duration): V? = + leftPop(key, timeout).awaitFirstOrNull() /** * Coroutines variant of [ReactiveListOperations.rightPop]. @@ -207,8 +198,8 @@ suspend fun ReactiveListOperations.rightPopAndAwait(key * @author Mark Paluch * @since 2.2 */ -suspend fun ReactiveListOperations.rightPopAndAwait(key: K, timeout: Duration): V? = - rightPop(key, timeout).awaitFirstOrNull() +suspend fun ReactiveListOperations.rightPopAndAwait(key: K): V? = + rightPop(key).awaitFirstOrNull() /** * Coroutines variant of [ReactiveListOperations.rightPop] with count. @@ -217,7 +208,16 @@ suspend fun ReactiveListOperations.rightPopAndAwait(key * @since 3.2 */ fun ReactiveListOperations.rightPopAsFlow(key: K, count:Long): Flow = - rightPop(key, count).asFlow() + rightPop(key, count).asFlow() + +/** + * Coroutines variant of [ReactiveListOperations.rightPop]. + * + * @author Mark Paluch + * @since 2.2 + */ +suspend fun ReactiveListOperations.rightPopAndAwait(key: K, timeout: Duration): V? = + rightPop(key, timeout).awaitFirstOrNull() /** * 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 181d46d1b8..1d24e6431a 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 @@ 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();