Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple reactive keys exist checker #2918

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add multiple reactive keys exist checker #2918

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf160e7
Add multiple reactive keys exist checker
AnneMayor May 26, 2024
aa34126
Merge branch 'spring-projects:main' into add-multiple-reactive-keys-e…
AnneMayor Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/main/java/org/springframework/data/redis/connection/ReactiveKeyCommands.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,20 @@ default Mono<Boolean> exists(ByteBuffer key) {

return exists(Mono.just(new KeyCommand(key))).next().map(BooleanResponse::getOutput);
}
/**
* Determine if given all {@literal keys} exist.
*
* @param keys must not be {@literal null} or {@literal empty}.
* @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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(…).


Assert.notNull(keys, "Keys must not be null");
Assert.notEmpty(keys, "Keys must not be empty");

return exists(Flux.fromIterable(keys).map(KeyCommand::new)).all(BooleanResponse::getOutput);
}

/**
* Determine if given {@literal key} exists.
Expand Down
28 changes: 25 additions & 3 deletions ...ngframework/data/redis/connection/lettuce/LettuceReactiveKeyCommandsIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,28 @@ void existsShouldReturnFalseForNonExistingKeys() {
connection.keyCommands().exists(KEY_1_BBUFFER).as(StepVerifier::create).expectNext(false).verifyComplete();
}

@ParameterizedRedisTest
void existsShouldReturnTrueWhenKeysExist() {

nativeCommands.set(KEY_1, VALUE_1);
nativeCommands.set(KEY_2, VALUE_2);

connection.keyCommands().exists(Arrays.asList(KEY_1_BBUFFER, KEY_2_BBUFFER)).as(StepVerifier::create)
.expectNext(true)
.verifyComplete();
}

@ParameterizedRedisTest
void existsShouldReturnFalseWhenKeysDoNotExist() {

nativeCommands.set(KEY_1, VALUE_1);

connection.keyCommands().exists(Arrays.asList(KEY_1_BBUFFER, KEY_2_BBUFFER)).as(StepVerifier::create)
.expectNext(false) //
.verifyComplete();
}


@ParameterizedRedisTest // DATAREDIS-525
void typeShouldReturnTypeCorrectly() {

Expand Down Expand Up @@ -164,7 +186,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();
Copy link
Contributor Author

@AnneMayor AnneMayor May 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just changed the command to make better readability according to sonarlint rules.

}

@ParameterizedRedisTest // DATAREDIS-525
Expand All @@ -183,7 +205,7 @@ void renameNXShouldAlterKeyNameCorrectly() {
.verifyComplete();

assertThat(nativeCommands.exists(KEY_2)).isEqualTo(1L);
assertThat(nativeCommands.exists(KEY_1)).isEqualTo(0L);
assertThat(nativeCommands.exists(KEY_1)).isZero();
}

@ParameterizedRedisTest // DATAREDIS-525
Expand Down Expand Up @@ -395,7 +417,7 @@ void shouldMoveToDatabase() {
.expectNext(true) //
.expectComplete() //
.verify();
assertThat(nativeCommands.exists(KEY_1)).isEqualTo(0L);
assertThat(nativeCommands.exists(KEY_1)).isZero();
}

@ParameterizedRedisTest // DATAREDIS-694
Expand Down