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.

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

Adding command BLMove #324

Merged
merged 3 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions glide-core/src/protobuf/redis_request.proto
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ enum RequestType {
LSet = 165;
XDel = 166;
LMove = 168;
BLMove = 169;
}

message Command {
Expand Down
3 changes: 3 additions & 0 deletions glide-core/src/request_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub enum RequestType {
LSet = 165,
XDel = 166,
LMove = 168,
BLMove = 169,
}

fn get_two_word_command(first: &str, second: &str) -> Cmd {
Expand Down Expand Up @@ -339,6 +340,7 @@ impl From<::protobuf::EnumOrUnknown<ProtobufRequestType>> for RequestType {
ProtobufRequestType::LSet => RequestType::LSet,
ProtobufRequestType::XDel => RequestType::XDel,
ProtobufRequestType::LMove => RequestType::LMove,
ProtobufRequestType::BLMove => RequestType::BLMove,
}
}
}
Expand Down Expand Up @@ -506,6 +508,7 @@ impl RequestType {
RequestType::LSet => Some(cmd("LSET")),
RequestType::XDel => Some(cmd("XDEL")),
RequestType::LMove => Some(cmd("LMOVE")),
RequestType::BLMove => Some(cmd("BLMOVE")),
}
}
}
15 changes: 15 additions & 0 deletions java/client/src/main/java/glide/api/BaseClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static glide.utils.ArrayTransformUtils.mapGeoDataToArray;
import static redis_request.RedisRequestOuterClass.RequestType.Append;
import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
Expand Down Expand Up @@ -1582,4 +1583,18 @@ public CompletableFuture<String> lmove(
new String[] {source, destination, wherefrom.toString(), whereto.toString()};
return commandManager.submitNewCommand(LMove, arguments, this::handleStringOrNullResponse);
}

@Override
public CompletableFuture<String> blmove(
@NonNull String source,
@NonNull String destination,
@NonNull ListDirection wherefrom,
@NonNull ListDirection whereto,
double timeout) {
String[] arguments =
new String[] {
source, destination, wherefrom.toString(), whereto.toString(), Double.toString(timeout)
};
return commandManager.submitNewCommand(BLMove, arguments, this::handleStringOrNullResponse);
}
}
45 changes: 45 additions & 0 deletions java/client/src/main/java/glide/api/commands/ListBaseCommands.java
Original file line number Diff line number Diff line change
Expand Up @@ -531,4 +531,49 @@ CompletableFuture<Map<String, String[]>> blmpop(
*/
CompletableFuture<String> lmove(
String source, String destination, ListDirection wherefrom, ListDirection whereto);

/**
* Blocks the connection until it pops atomically and removes the left/right-most element to the
* list stored at <code>source</code> depending on <code>wherefrom</code>, and pushes the element
* at the first/last element of the list stored at <code>destination</code> depending on <code>
* wherefrom</code>.<br>
* <code>BLMove</code> is the blocking variant of {@link #lmove(String, String, ListDirection,
* ListDirection)}.
*
* @since Redis 6.2.0 and above.
* @apiNote
* <ol>
* <li>When in cluster mode, all <code>source</code> and <code>destination</code> must map
* to the same hash slot.
* <li><code>BLMove</code> is a client blocking command, see <a
* href="https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands">Blocking
* Commands</a> for more details and best practices.
* </ol>
*
* @see <a href="https://valkey.io/commands/blmove/">valkey.io</a> for details.
* @param source The key to the source list.
* @param destination The key to the destination list.
* @param wherefrom The {@link ListDirection} the element should be removed from.
* @param whereto The {@link ListDirection} the element should be added to.
* @param timeout The number of seconds to wait for a blocking operation to complete. A value of
* <code>0</code> will block indefinitely.
* @return The popped element or <code>null</code> if <code>source</code> does not exist.
* @example
* <pre>{@code
* client.lpush("testKey1", new String[] {"two", "one"}).get();
* client.lpush("testKey2", new String[] {"four", "three"}).get();
* var result = client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1).get();
* assertEquals(result, "one");
* String[] upratedArray1 = client.lrange("testKey1", 0, -1).get();
* String[] upratedArray2 = client.lrange("testKey2", 0, -1).get();
* assertArrayEquals(new String[] {"two"}, updatedArray1);
* assertArrayEquals(new String[] {"one", "three", "four"}, updatedArray2);
* }</pre>
*/
CompletableFuture<String> blmove(
String source,
String destination,
ListDirection wherefrom,
ListDirection whereto,
double timeout);
}
40 changes: 40 additions & 0 deletions java/client/src/main/java/glide/api/models/BaseTransaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static glide.utils.ArrayTransformUtils.mapGeoDataToArray;
import static redis_request.RedisRequestOuterClass.RequestType.Append;
import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
Expand Down Expand Up @@ -3832,6 +3833,45 @@ public T lmove(
return getThis();
}

/**
* Blocks the connection until it atomically pops and removes the left/right-most element to the
* list stored at <code>source</code> depending on <code>wherefrom</code>, and pushes the element
* at the first/last element of the list stored at <code>destination</code> depending on <code>
* wherefrom</code>.<br>
* <code>BLMove</code> is the blocking variant of {@link #lmove(String, String, ListDirection,
* ListDirection)}.
*
* @since Redis 6.2.0 and above.
* @apiNote <code>BLMove</code> is a client blocking command, see <a
* href="https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands">Blocking
* Commands</a> for more details and best practices.
* @see <a href="https://valkey.io/commands/blmove/">valkey.io</a> for details.
* @param source The key to the source list.
* @param destination The key to the destination list.
* @param wherefrom The {@link ListDirection} the element should be removed from.
* @param whereto The {@link ListDirection} the element should be added to.
* @param timeout The number of seconds to wait for a blocking operation to complete. A value of
* <code>0</code> will block indefinitely.
* @return Command Response - The popped element or <code>null</code> if <code>source</code> does
* not exist.
*/
public T blmove(
@NonNull String source,
@NonNull String destination,
@NonNull ListDirection wherefrom,
@NonNull ListDirection whereto,
double timeout) {
ArgsArray commandArgs =
buildArgs(
source,
destination,
wherefrom.toString(),
whereto.toString(),
Double.toString(timeout));
protobufTransaction.addCommands(buildCommand(BLMove, commandArgs));
return getThis();
}

/** Build protobuf {@link Command} object for given command and arguments. */
protected Command buildCommand(RequestType requestType) {
return buildCommand(requestType, buildArgs());
Expand Down
28 changes: 28 additions & 0 deletions java/client/src/test/java/glide/api/RedisClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static org.mockito.Mockito.when;
import static redis_request.RedisRequestOuterClass.RequestType.Append;
import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
Expand Down Expand Up @@ -5229,4 +5230,31 @@ public void lset_returns_success() {
assertEquals(testResponse, response);
assertEquals(OK, payload);
}

@SneakyThrows
@Test
public void blmove_returns_success() {
// setup
String key1 = "testKey";
String key2 = "testKey2";
ListDirection wherefrom = ListDirection.LEFT;
ListDirection whereto = ListDirection.RIGHT;
String[] arguments = new String[] {key1, key2, wherefrom.toString(), whereto.toString(), "0.1"};
String value = "one";

CompletableFuture<String> testResponse = new CompletableFuture<>();
testResponse.complete(value);

// match on protobuf request
when(commandManager.<String>submitNewCommand(eq(BLMove), eq(arguments), any()))
.thenReturn(testResponse);

// exercise
CompletableFuture<String> response = service.blmove(key1, key2, wherefrom, whereto, 0.1);
String payload = response.get();

// verify
assertEquals(testResponse, response);
assertEquals(value, payload);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static redis_request.RedisRequestOuterClass.RequestType.Append;
import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
Expand Down Expand Up @@ -873,6 +874,9 @@ InfScoreBound.NEGATIVE_INFINITY, new ScoreBoundary(3, false), new Limit(1, 2)),
transaction.lmove("key1", "key2", ListDirection.LEFT, ListDirection.LEFT);
results.add(Pair.of(LMove, buildArgs("key1", "key2", "LEFT", "LEFT")));

transaction.blmove("key1", "key2", ListDirection.LEFT, ListDirection.LEFT, 0.1);
results.add(Pair.of(BLMove, buildArgs("key1", "key2", "LEFT", "LEFT", "0.1")));

var protobufTransaction = transaction.getProtobufTransaction().build();

for (int idx = 0; idx < protobufTransaction.getCommandsCount(); idx++) {
Expand Down
89 changes: 89 additions & 0 deletions java/integTest/src/test/java/glide/SharedCommandTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -4325,4 +4325,93 @@ public void lmove(BaseClient client) {
() -> client.lmove(key1, nonListKey, ListDirection.LEFT, ListDirection.LEFT).get());
assertInstanceOf(RequestException.class, executionException2.getCause());
}

@SneakyThrows
@ParameterizedTest(autoCloseArguments = false)
@MethodSource("getClients")
public void blmove(BaseClient client) {
assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("6.2.0"), "This feature added in redis 6.2.0");
// setup
String key1 = "{key}-1" + UUID.randomUUID();
String key2 = "{key}-2" + UUID.randomUUID();
String nonExistingKey = "{key}-3" + UUID.randomUUID();
String nonListKey = "{key}-4" + UUID.randomUUID();
String[] lpushArgs1 = {"four", "three", "two", "one"};
String[] lpushArgs2 = {"six", "five", "four"};
double timeout = 1;

// source does not exist or is empty
assertNull(client.blmove(key1, key2, ListDirection.LEFT, ListDirection.RIGHT, timeout).get());

// only source exists, only source elements gets popped, creates a list at nonExistingKey
assertEquals(lpushArgs1.length, client.lpush(key1, lpushArgs1).get());
assertEquals(
"four",
client
.blmove(key1, nonExistingKey, ListDirection.RIGHT, ListDirection.LEFT, timeout)
.get());
assertArrayEquals(new String[] {"one", "two", "three"}, client.lrange(key1, 0, -1).get());

// source and destination are the same, performing list rotation, "three" gets popped and added
// back
assertEquals(
"one", client.blmove(key1, key1, ListDirection.LEFT, ListDirection.LEFT, timeout).get());
assertArrayEquals(new String[] {"one", "two", "three"}, client.lrange(key1, 0, -1).get());

// normal use case, "three" gets popped and added to the left of destination
assertEquals(lpushArgs2.length, client.lpush(key2, lpushArgs2).get());
assertEquals(
"three", client.blmove(key1, key2, ListDirection.RIGHT, ListDirection.LEFT, timeout).get());
assertArrayEquals(new String[] {"one", "two"}, client.lrange(key1, 0, -1).get());
assertArrayEquals(
new String[] {"three", "four", "five", "six"}, client.lrange(key2, 0, -1).get());

// source exists but is not a list type key
assertEquals(OK, client.set(nonListKey, "NotAList").get());
ExecutionException executionException =
assertThrows(
ExecutionException.class,
() ->
client
.blmove(nonListKey, key1, ListDirection.LEFT, ListDirection.LEFT, timeout)
.get());
assertInstanceOf(RequestException.class, executionException.getCause());

// destination exists but is not a list type key
ExecutionException executionException2 =
assertThrows(
ExecutionException.class,
() ->
client
.blmove(key1, nonListKey, ListDirection.LEFT, ListDirection.LEFT, timeout)
.get());
assertInstanceOf(RequestException.class, executionException2.getCause());
}

@SneakyThrows
@ParameterizedTest(autoCloseArguments = false)
@MethodSource("getClients")
public void blmove_timeout_check(BaseClient client) {
assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("6.2.0"), "This feature added in redis 6.2.0");
String key1 = "{key}-1" + UUID.randomUUID();
String key2 = "{key}-2" + UUID.randomUUID();
// create new client with default request timeout (250 millis)
try (var testClient =
client instanceof RedisClient
? RedisClient.CreateClient(commonClientConfig().build()).get()
: RedisClusterClient.CreateClient(commonClusterClientConfig().build()).get()) {

// ensure that commands doesn't time out even if timeout > request timeout
assertNull(testClient.blmove(key1, key2, ListDirection.LEFT, ListDirection.LEFT, 1).get());

// with 0 timeout (no timeout) should never time out,
// but we wrap the test with timeout to avoid test failing or stuck forever
assertThrows(
TimeoutException.class, // <- future timeout, not command timeout
() ->
testClient
.blmove(key1, key2, ListDirection.LEFT, ListDirection.LEFT, 0)
.get(3, TimeUnit.SECONDS));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,10 @@ private static Object[] listCommands(BaseTransaction<?> transaction) {
.lmove(listKey7, listKey7, ListDirection.LEFT, ListDirection.LEFT)
.lmove(listKey6, listKey7, ListDirection.LEFT, ListDirection.RIGHT)
.lrange(listKey6, 0, -1)
.lrange(listKey7, 0, -1)
.blmove(listKey7, listKey7, ListDirection.LEFT, ListDirection.LEFT, 0.1)
.blmove(listKey7, listKey6, ListDirection.RIGHT, ListDirection.LEFT, 0.1)
.lrange(listKey6, 0, -1)
.lrange(listKey7, 0, -1);
}

Expand Down Expand Up @@ -345,6 +349,10 @@ private static Object[] listCommands(BaseTransaction<?> transaction) {
value1, // lmove(listKey6, listKey5, RIGHT, LEFT)
new String[] {value2, value3}, // lrange(listKey6, 0, -1)
new String[] {value3, value2, value1, value1}, // lrange(listKey7, 0, -1);
value3, // blmove(listKey7, listKey7, LEFT, LEFT, 0.1)
value1, // blmove(listKey7, listKey6, RIGHT, LEFT, 0.1)
new String[] {value1, value2, value3}, // lrange(listKey6, 0, -1)
new String[] {value3, value2, value1}, // lrange(listKey7, 0, -1)
});
}

Expand Down
6 changes: 5 additions & 1 deletion java/integTest/src/test/java/glide/cluster/CommandTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,11 @@ public static Stream<Arguments> callCrossSlotCommandsWhichShouldFail() {
Arguments.of(
"lmove",
"6.2.0",
clusterClient.lmove("abc", "def", ListDirection.LEFT, ListDirection.LEFT)));
clusterClient.lmove("abc", "def", ListDirection.LEFT, ListDirection.LEFT)),
Arguments.of(
"blmove",
"6.2.0",
clusterClient.blmove("abc", "def", ListDirection.LEFT, ListDirection.LEFT, 1)));
}

@SneakyThrows
Expand Down
Loading