Add multiple reactive keys exist checker #2918
Conversation
| @@ -164,7 +184,7 @@ void renameShouldAlterKeyNameCorrectly() { | |||
| connection.keyCommands().rename(KEY_1_BBUFFER, KEY_2_BBUFFER).as(StepVerifier::create).expectNext(true) | |||
| .verifyComplete(); | |||
| assertThat(nativeCommands.exists(KEY_2)).isEqualTo(1L); | |||
| assertThat(nativeCommands.exists(KEY_1)).isEqualTo(0L); | |||
| assertThat(nativeCommands.exists(KEY_1)).isZero(); | |||
There was a problem hiding this comment.
I just changed the command to make better readability according to sonarlint rules.
spring-projects-issues
added
the
status: waiting-for-triage
| * @return {@link Mono} emitting {@literal true} if all keys exist. | ||
| * @see <a href="https://redis.io/commands/exists">Redis Documentation: EXISTS</a> | ||
| */ | ||
| default Mono<Boolean> exists(List<ByteBuffer> keys) { |
There was a problem hiding this comment.
I created a validation method which is for checking if parameterized all keys exist in redis cache. If one of them does not exist, it returns false.
@kutlueren , please check if this function is what you meant to use.
There was a problem hiding this comment.
Hey @AnneMayor
Awesome to see that you added the function which carries out what I suggested. It is actually more straight forward than I initially thought, maybe I got confused in the codebase but nice! I shall try it out as soon as it gets merged.
There was a problem hiding this comment.
This implementation calls EXISTS for each key issuing a command for each element in the list.
Redis' EXISTS command accepts multiple arguments and returns how many of the keys exist. Our interface should use the existing functionality instead of putting another layer on top of commands with a worse performance behavior.
The issue we have here is however that exists(Publisher<KeyCommand> keys) and exists(Publisher<List<KeyCommand>> keys) or exists(Publisher<MultiKeyCommand>> keys) erase to the same type and we cannot simply introduce such an override.
I suggest to implement Mono<Long> exists(List<ByteBuffer> keys) directly instead of using default methods. Also, ReactiveRedisOperations should be extended with countExistingKeys(…) similar to RedisTemplate.countExistingKeys(…).
AnneMayor
force-pushed
the
add-multiple-reactive-keys-exist-checker
branch
from
0d0df34
to
78c6c7f
Compare
AnneMayor
force-pushed
the
add-multiple-reactive-keys-exist-checker
branch
from
78c6c7f
to
cf160e7
Compare
|
Hey @AnneMayor, I just wanted to follow up on the topic :) Is this being planned to be merged soon? |
|
Hey @kutlueren , |
@mp911de would you mind taking a look at this one when you have time? |
|
@mp911de , |
| * @return {@link Mono} emitting {@literal true} if all keys exist. | ||
| * @see <a href="https://redis.io/commands/exists">Redis Documentation: EXISTS</a> | ||
| */ | ||
| default Mono<Boolean> exists(List<ByteBuffer> keys) { |
There was a problem hiding this comment.
This implementation calls EXISTS for each key issuing a command for each element in the list.
Redis' EXISTS command accepts multiple arguments and returns how many of the keys exist. Our interface should use the existing functionality instead of putting another layer on top of commands with a worse performance behavior.
The issue we have here is however that exists(Publisher<KeyCommand> keys) and exists(Publisher<List<KeyCommand>> keys) or exists(Publisher<MultiKeyCommand>> keys) erase to the same type and we cannot simply introduce such an override.
I suggest to implement Mono<Long> exists(List<ByteBuffer> keys) directly instead of using default methods. Also, ReactiveRedisOperations should be extended with countExistingKeys(…) similar to RedisTemplate.countExistingKeys(…).
Thanks a lot. I am going to apply your review within this weekend 👍 |
Resolved #2883