[upstream] Add support for XDELEX and XACKDEL, and expand options for XADD and XTRIM #3272
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
onobc
changed the title
There was a problem hiding this comment.
Note
Thank you @viktoriya-kutsarova for the excellent deprecated detailed descriptions tells users what replacement to use w/o them having to do software archeology to figure it out. :)
Once Jenkins and PR is green and we resolve the rename concern (#3272 (comment)) this will be ready to merge.
Also note that I will do some final massaging of the commits (couple of squashes and be sure they proper links are in the commits etc..).
Thanks
src/main/java/org/springframework/data/redis/connection/jedis/StreamConverters.java
Show resolved
Hide resolved
src/main/java/org/springframework/data/redis/connection/RedisStreamCommands.java
Outdated
Show resolved
Hide resolved
mp911de
left a comment
mp911de
left a comment
There was a problem hiding this comment.
I've left a few comments. There are several public API elements (methods, classes), that are missing Javadoc along with a mix of im proper or missing @since tags. I strongly urge to refrain from Deprecated(forRemoval=true) as the current API isn't breaking anything nor imposes its usage any danger, we've just broadened feature support.
Test code is overly verbose in its current form as each test scenratio is unfolded in a separate method rather than utilizing parametrized tests. In some areas we follow a test approach that doesn't consider refactoring of the underlying code inside components that we're testing. These require cleanup along with proper formatting according to our code style.
src/test/java/org/springframework/data/redis/connection/jedis/JedisConvertersUnitTests.java
Show resolved
Hide resolved
src/test/java/org/springframework/data/redis/connection/jedis/StreamConvertersUnitTest.java
Show resolved
Hide resolved
src/test/java/org/springframework/data/redis/connection/jedis/StreamConvertersUnitTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionUnitTests.java
Show resolved
Hide resolved
src/test/java/org/springframework/data/redis/core/DefaultStreamOperationsIntegrationTests.java
Show resolved
Hide resolved
src/main/java/org/springframework/data/redis/connection/RedisStreamCommands.java
Outdated
Show resolved
Hide resolved
src/main/java/org/springframework/data/redis/connection/RedisStreamCommands.java
Show resolved
Hide resolved
src/main/java/org/springframework/data/redis/connection/RedisStreamCommands.java
Show resolved
Hide resolved
src/main/java/org/springframework/data/redis/connection/RedisStreamCommands.java
Show resolved
Hide resolved
src/main/java/org/springframework/data/redis/connection/RedisStreamCommands.java
Show resolved
Hide resolved
mp911de
added
type: task
Moves trim options into top-level class that is used by XTrim and Xdd options. Original pull request #3247 See #3232 Co-authored-by: viktoriya.kutsarova <viktoriya.kutsarova@redis.com> Signed-off-by: Chris Bono <chris.bono@broadcom.com> Signed-off-by: viktoriya.kutsarova <viktoriya.kutsarova@redis.com>
This commit performs the following polishing changes: - remove manual not null check in `TrimOptions.<init>` as it's already `@NullMarked` - annotate `RedisStreamCommands.XDelOptions` with `@NullMarked` - remove usage of `var` in main code - use since `4.1` instead of `4.0` - rename pendingReferences to deletionPolicy - use ParameterizedTest for StreamConvertersUnitTest - simplify test to not verify things it does not do - fix formatting in various classes - minor tweak to javadocs - inline converters in Lettuce StreamConverters Original pull request #3247 See #3232 Signed-off-by: Chris Bono <chris.bono@broadcom.com>
Groups the integration tests in DefaultStreamOperationsIntegrationTests for XACKDEL and XDELEX and adds an `@EnabledOnCommand` guard to each test group to prevent them from being run in environments that do not support these commands (e.g. Valkey). Original pull request #3247 See #3232 Signed-off-by: Chris Bono <chris.bono@broadcom.com>
|
Closing in favor of #3247 |
This change introduces the
Support XDELEX, XACKEX and extend XADD and XTRIM optionsfeature...See original pull request for more details.
Closes: #3232
Original Pull Request: #3247