From 7d595ab7e4422b7adedb1283a21cee47e09b4b3e Mon Sep 17 00:00:00 2001 From: michele Date: Thu, 19 Jul 2018 15:21:29 +0200 Subject: [PATCH] DATAREDIS-852 - Return always a lower/upper bound range index using Range.Bound.unbounded() with Lettuce. We now return in List, String, and ZSet commands a lower/upper bound range index to prevent NullPointerException. Previously, unbounded ranges could render null values that were attempted to cast to primitives. Original pull request: #353. --- .../lettuce/LettuceReactiveListCommands.java | 9 +-- .../LettuceReactiveStringCommands.java | 9 +-- .../lettuce/LettuceReactiveZSetCommands.java | 26 ++++---- .../LettuceReactiveListCommandTests.java | 60 ++++++++++++++++++- .../LettuceReactiveStringCommandsTests.java | 30 +++++++++- .../LettuceReactiveZSetCommandsTests.java | 27 +++++++++ 6 files changed, 138 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommands.java index 3b28b1f03..c90b2b9a9 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommands.java @@ -38,6 +38,7 @@ import org.springframework.util.ObjectUtils; /** * @author Christoph Strobl * @author Mark Paluch + * @author Michele Mancioppi * @since 2.0 */ class LettuceReactiveListCommands implements ReactiveListCommands { @@ -116,8 +117,8 @@ class LettuceReactiveListCommands implements ReactiveListCommands { Assert.notNull(command.getKey(), "Key must not be null!"); Assert.notNull(command.getRange(), "Range must not be null!"); - Flux result = cmd.lrange(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)); + Flux result = cmd.lrange(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)); return Mono.just(new CommandResponse<>(command, result)); })); } @@ -135,8 +136,8 @@ class LettuceReactiveListCommands implements ReactiveListCommands { Assert.notNull(command.getRange(), "Range must not be null!"); return cmd - .ltrim(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)) + .ltrim(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)) .map(LettuceConverters::stringToBoolean).map(value -> new BooleanResponse<>(command, value)); })); } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommands.java index 6449c2a07..c0ec618b1 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommands.java @@ -37,6 +37,7 @@ import org.springframework.util.Assert; /** * @author Christoph Strobl * @author Mark Paluch + * @author Michele Mancioppi * @since 2.0 */ class LettuceReactiveStringCommands implements ReactiveStringCommands { @@ -247,8 +248,8 @@ class LettuceReactiveStringCommands implements ReactiveStringCommands { Range range = command.getRange(); - return cmd.getrange(command.getKey(), range.getLowerBound().getValue().orElse(null), - range.getUpperBound().getValue().orElse(null)).map((value) -> new ByteBufferResponse<>(command, value)); + return cmd.getrange(command.getKey(), range.getLowerBound().getValue().orElse(0L), + range.getUpperBound().getValue().orElse(Long.MAX_VALUE)).map((value) -> new ByteBufferResponse<>(command, value)); })); } @@ -318,8 +319,8 @@ class LettuceReactiveStringCommands implements ReactiveStringCommands { Range range = command.getRange(); - return (!Range.unbounded().equals(range) ? cmd.bitcount(command.getKey(), range.getLowerBound().getValue().orElse(null), - range.getUpperBound().getValue().orElse(null)) : cmd.bitcount(command.getKey())) + return (!Range.unbounded().equals(range) ? cmd.bitcount(command.getKey(), range.getLowerBound().getValue().orElse(0L), + range.getUpperBound().getValue().orElse(Long.MAX_VALUE)) : cmd.bitcount(command.getKey())) .map(responseValue -> new NumericResponse<>(command, responseValue)); })); } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommands.java index a33e2caab..eb8ec00e4 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommands.java @@ -46,6 +46,7 @@ import org.springframework.util.StringUtils; /** * @author Christoph Strobl * @author Mark Paluch + * @author Michele Mancioppi * @since 2.0 */ class LettuceReactiveZSetCommands implements ReactiveZSetCommands { @@ -186,28 +187,28 @@ class LettuceReactiveZSetCommands implements ReactiveZSetCommands { if (command.isWithScores()) { result = cmd - .zrangeWithScores(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)) + .zrangeWithScores(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)) .map(sc -> (Tuple) new DefaultTuple(getBytes(sc), sc.getScore())); } else { result = cmd - .zrange(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)) + .zrange(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)) .map(value -> (Tuple) new DefaultTuple(ByteUtils.getBytes(value), Double.NaN)); } } else { if (command.isWithScores()) { result = cmd - .zrevrangeWithScores(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)) + .zrevrangeWithScores(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)) .map(sc -> (Tuple) new DefaultTuple(getBytes(sc), sc.getScore())); } else { result = cmd - .zrevrange(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)) + .zrevrange(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)) .map(value -> (Tuple) new DefaultTuple(ByteUtils.getBytes(value), Double.NaN)); } } @@ -355,8 +356,8 @@ class LettuceReactiveZSetCommands implements ReactiveZSetCommands { Assert.notNull(command.getRange(), "Range must not be null!"); return cmd - .zremrangebyrank(command.getKey(), command.getRange().getLowerBound().getValue().orElse(null), - command.getRange().getUpperBound().getValue().orElse(null)) + .zremrangebyrank(command.getKey(), command.getRange().getLowerBound().getValue().orElse(0L), + command.getRange().getUpperBound().getValue().orElse(Long.MAX_VALUE)) .map(value -> new NumericResponse<>(command, value)); })); } @@ -525,11 +526,14 @@ class LettuceReactiveZSetCommands implements ReactiveZSetCommands { Boolean upper) { return (source) -> { - Boolean inclusive = upper ? source.getUpperBound().isInclusive() : source.getLowerBound().isInclusive(); Object value = upper ? source.getUpperBound().getValue().orElse(null) : source.getLowerBound().getValue().orElse(null); + if (value == null) { + return Boundary.unbounded(); + } + if (value instanceof Number) { return inclusive ? Boundary.including((Number) value) : Boundary.excluding((Number) value); } diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommandTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommandTests.java index 36789d37e..ccea4c841 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommandTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveListCommandTests.java @@ -21,24 +21,28 @@ import static org.hamcrest.core.IsEqual.*; import static org.hamcrest.core.IsNot.*; import static org.junit.Assert.*; import static org.junit.Assume.assumeThat; +import static org.springframework.data.domain.Range.Bound.*; import java.nio.ByteBuffer; import java.time.Duration; import java.util.Arrays; -import org.junit.Assume; import org.junit.Test; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.domain.Range; import org.springframework.data.redis.connection.ReactiveListCommands.PopResult; import org.springframework.data.redis.connection.ReactiveListCommands.PushCommand; +import org.springframework.data.redis.connection.ReactiveRedisConnection; +import org.springframework.data.redis.connection.ReactiveRedisConnection.CommandResponse; +import org.springframework.data.redis.connection.ReactiveRedisConnection.RangeCommand; import org.springframework.data.redis.connection.RedisListCommands.Position; - -import org.springframework.data.redis.test.util.LettuceRedisClientProvider; import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; /** * @author Christoph Strobl * @author Mark Paluch + * @author Michele Mancioppi */ public class LettuceReactiveListCommandTests extends LettuceReactiveCommandsTestsBase { @@ -106,6 +110,32 @@ public class LettuceReactiveListCommandTests extends LettuceReactiveCommandsTest contains(VALUE_2_BBUFFER, VALUE_3_BBUFFER)); } + @Test // DATAREDIS-852 + public void lRangeShouldReturnValuesCorrectlyWithMinUnbounded() { + + nativeCommands.rpush(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + RangeCommand rangeCommand = RangeCommand.key(KEY_1_BBUFFER).within(Range.of(unbounded(), inclusive(1L))); + + StepVerifier.create(connection.listCommands().lRange(Mono.just(rangeCommand)).flatMap(CommandResponse::getOutput)) // + .expectNext(VALUE_1_BBUFFER) + .expectNext(VALUE_2_BBUFFER) + .verifyComplete(); + } + + @Test // DATAREDIS-852 + public void lRangeShouldReturnValuesCorrectlyWithMaxUnbounded() { + + nativeCommands.rpush(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + RangeCommand rangeCommand = RangeCommand.key(KEY_1_BBUFFER).within(Range.of(inclusive(1L), unbounded())); + + StepVerifier.create(connection.listCommands().lRange(Mono.just(rangeCommand)).flatMap(CommandResponse::getOutput)) // + .expectNext(VALUE_2_BBUFFER) + .expectNext(VALUE_3_BBUFFER) + .verifyComplete(); + } + @Test // DATAREDIS-525 public void lTrimShouldReturnValuesCorrectly() { @@ -115,6 +145,30 @@ public class LettuceReactiveListCommandTests extends LettuceReactiveCommandsTest assertThat(nativeCommands.lrange(KEY_1, 0, -1), not(contains(VALUE_1_BBUFFER))); } + @Test // DATAREDIS-852 + public void lTrimShouldReturnValuesCorrectlyWithMinUnbounded() { + + nativeCommands.rpush(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + RangeCommand rangeCommand = RangeCommand.key(KEY_1_BBUFFER).within(Range.of(unbounded(), inclusive(1L))); + + StepVerifier.create(connection.listCommands().lTrim(Mono.just(rangeCommand))) // + .expectNext(new ReactiveRedisConnection.BooleanResponse<>(rangeCommand, true)) // + .verifyComplete(); + } + + @Test // DATAREDIS-852 + public void lTrimShouldReturnValuesCorrectlyWithMaxUnbounded() { + + nativeCommands.rpush(KEY_1, VALUE_1, VALUE_2, VALUE_3); + + RangeCommand rangeCommand = RangeCommand.key(KEY_1_BBUFFER).within(Range.of(inclusive(1L), unbounded())); + + StepVerifier.create(connection.listCommands().lTrim(Mono.just(rangeCommand))) // + .expectNext(new ReactiveRedisConnection.BooleanResponse<>(rangeCommand, true)) // + .verifyComplete(); + } + @Test // DATAREDIS-525 public void lIndexShouldReturnValueCorrectly() { diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommandsTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommandsTests.java index 58d0e1d93..f6a70833a 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommandsTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveStringCommandsTests.java @@ -36,18 +36,22 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.Test; +import org.springframework.data.domain.Range; +import org.springframework.data.domain.Range.Bound; +import org.springframework.data.redis.connection.ReactiveRedisConnection; import org.springframework.data.redis.connection.ReactiveRedisConnection.ByteBufferResponse; import org.springframework.data.redis.connection.ReactiveRedisConnection.CommandResponse; import org.springframework.data.redis.connection.ReactiveRedisConnection.KeyCommand; import org.springframework.data.redis.connection.ReactiveRedisConnection.MultiValueResponse; +import org.springframework.data.redis.connection.ReactiveRedisConnection.RangeCommand; import org.springframework.data.redis.connection.ReactiveStringCommands.SetCommand; import org.springframework.data.redis.connection.RedisStringCommands.BitOperation; import org.springframework.data.redis.core.types.Expiration; -import org.springframework.data.redis.test.util.LettuceRedisClientProvider; /** * @author Christoph Strobl * @author Mark Paluch + * @author Michele Mancioppi */ public class LettuceReactiveStringCommandsTests extends LettuceReactiveCommandsTestsBase { @@ -288,6 +292,30 @@ public class LettuceReactiveStringCommandsTests extends LettuceReactiveCommandsT .verifyComplete(); } + @Test // DATAREDIS-525 + public void getRangeShouldReturnSubstringCorrectlyWithMinUnbound() { + + nativeCommands.set(KEY_1, VALUE_1); + + RangeCommand rangeCommand = RangeCommand.key(KEY_1_BBUFFER).within(Range.of(Bound.unbounded(), Bound.inclusive(2L))); + + StepVerifier.create(connection.stringCommands().getRange(Mono.just(rangeCommand))) // + .expectNext(new ReactiveRedisConnection.ByteBufferResponse<>(rangeCommand, ByteBuffer.wrap("val".getBytes()))) + .verifyComplete(); + } + + @Test // DATAREDIS-525 + public void getRangeShouldReturnSubstringCorrectlyWithMaxUnbound() { + + nativeCommands.set(KEY_1, VALUE_1); + + RangeCommand rangeCommand = RangeCommand.key(KEY_1_BBUFFER).within(Range.of(Bound.inclusive(0L), Bound.unbounded())); + + StepVerifier.create(connection.stringCommands().getRange(Mono.just(rangeCommand))) // + .expectNext(new ReactiveRedisConnection.ByteBufferResponse<>(rangeCommand, ByteBuffer.wrap(VALUE_1.getBytes()))) + .verifyComplete(); + } + @Test // DATAREDIS-525 public void setRangeShouldReturnNewStringLengthCorrectly() { diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommandsTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommandsTests.java index f0fd0177a..455d165a6 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommandsTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveZSetCommandsTests.java @@ -33,6 +33,7 @@ import org.springframework.data.redis.test.util.LettuceRedisClientProvider; /** * @author Christoph Strobl * @author Mark Paluch + * @author Michele Mancioppi */ public class LettuceReactiveZSetCommandsTests extends LettuceReactiveCommandsTestsBase { @@ -140,6 +141,32 @@ public class LettuceReactiveZSetCommandsTests extends LettuceReactiveCommandsTes .verifyComplete(); } + @Test // DATAREDIS-852 + public void zRangeByScoreShouldReturnValuesCorrectlyWithMinUnbounded() { + + nativeCommands.zadd(KEY_1, 1D, VALUE_1); + nativeCommands.zadd(KEY_1, 2D, VALUE_2); + nativeCommands.zadd(KEY_1, 3D, VALUE_3); + + StepVerifier.create(connection.zSetCommands().zRangeByScore(KEY_1_BBUFFER, Range.of(Range.Bound.unbounded(), + Range.Bound.inclusive(3D)))) // + .expectNext(VALUE_1_BBUFFER, VALUE_2_BBUFFER, VALUE_3_BBUFFER) // + .verifyComplete(); + } + + @Test // DATAREDIS-852 + public void zRangeByScoreShouldReturnValuesCorrectlyWithMaxUnbounded() { + + nativeCommands.zadd(KEY_1, 1D, VALUE_1); + nativeCommands.zadd(KEY_1, 2D, VALUE_2); + nativeCommands.zadd(KEY_1, 3D, VALUE_3); + + StepVerifier.create(connection.zSetCommands().zRangeByScore(KEY_1_BBUFFER, Range.of(Range.Bound.inclusive(0D), + Range.Bound.unbounded()))) // + .expectNext(VALUE_1_BBUFFER, VALUE_2_BBUFFER, VALUE_3_BBUFFER) // + .verifyComplete(); + } + @Test // DATAREDIS-525 public void zRangeByScoreShouldReturnValuesCorrectlyWithMinExclusion() {