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

Java: Add SETRANGE command. #1235

Merged
merged 8 commits into from
Apr 11, 2024
Merged

Java: Add SETRANGE command. #1235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5a93bc4
Add `SETRANGE` command. (#183)
Yury-Fridlyand Apr 4, 2024
5075877
PR comments.
Yury-Fridlyand Apr 5, 2024
8298cb2
PR comments.
Yury-Fridlyand Apr 8, 2024
a7e9c2c
Merge remote-tracking branch 'upstream/main' into java/integ_yuryf_SE…
Yury-Fridlyand Apr 8, 2024
7906e36
Merge branch 'main' into java/integ_yuryf_SETRANGE
Yury-Fridlyand Apr 9, 2024
163859d
Typo fix.
Yury-Fridlyand Apr 10, 2024
1769fd5
Merge remote-tracking branch 'upstream/main' into java/integ_yuryf_SE…
Yury-Fridlyand Apr 10, 2024
7568dea
Update UT.
Yury-Fridlyand Apr 10, 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
1 change: 1 addition & 0 deletions glide-core/src/protobuf/redis_request.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ enum RequestType {
RPushX = 102;
LPushX = 103;
ZMScore = 104;
SetRange = 107;
}

message Command {
Expand Down
3 changes: 3 additions & 0 deletions glide-core/src/request_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ pub enum RequestType {
RPushX = 102,
LPushX = 103,
ZMScore = 104,
SetRange = 107,
}

fn get_two_word_command(first: &str, second: &str) -> Cmd {
Expand Down Expand Up @@ -227,6 +228,7 @@ impl From<::protobuf::EnumOrUnknown<ProtobufRequestType>> for RequestType {
ProtobufRequestType::Blpop => RequestType::Blpop,
ProtobufRequestType::Spop => RequestType::Spop,
ProtobufRequestType::ZMScore => RequestType::ZMScore,
ProtobufRequestType::SetRange => RequestType::SetRange,
}
}
}
Expand Down Expand Up @@ -338,6 +340,7 @@ impl RequestType {
RequestType::Blpop => Some(cmd("BLPOP")),
RequestType::Spop => Some(cmd("SPOP")),
RequestType::ZMScore => Some(cmd("ZMSCORE")),
RequestType::SetRange => Some(cmd("SETRANGE")),
}
}
}
7 changes: 7 additions & 0 deletions java/client/src/main/java/glide/api/BaseClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import static redis_request.RedisRequestOuterClass.RequestType.SIsMember;
import static redis_request.RedisRequestOuterClass.RequestType.SMembers;
import static redis_request.RedisRequestOuterClass.RequestType.SRem;
import static redis_request.RedisRequestOuterClass.RequestType.SetRange;
import static redis_request.RedisRequestOuterClass.RequestType.SetString;
import static redis_request.RedisRequestOuterClass.RequestType.Strlen;
import static redis_request.RedisRequestOuterClass.RequestType.TTL;
Expand Down Expand Up @@ -347,6 +348,12 @@ public CompletableFuture<Long> strlen(@NonNull String key) {
return commandManager.submitNewCommand(Strlen, new String[] {key}, this::handleLongResponse);
}

@Override
public CompletableFuture<Long> setrange(@NonNull String key, int offset, @NonNull String value) {
return commandManager.submitNewCommand(
SetRange, new String[] {key, Integer.toString(offset), value}, this::handleLongResponse);
}

@Override
public CompletableFuture<String> hget(@NonNull String key, @NonNull String field) {
return commandManager.submitNewCommand(
Expand Down
22 changes: 22 additions & 0 deletions java/client/src/main/java/glide/api/commands/StringBaseCommands.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,26 @@ public interface StringBaseCommands {
* }</pre>
*/
CompletableFuture<Long> strlen(String key);

/**
* Overwrites part of the string stored at <code>key</code>, starting at the specified <code>
* offset</code>, for the entire length of <code>value</code>.<br>
* If the <code>offset</code> is larger than the current length of the string at <code>key</code>,
* the string is padded with zero bytes to make <code>offset</code> fit. Creates the <code>key
* </code> if it doesn't exist.
*
* @see <a href="https://redis.io/commands/setrange/">redis.io</a> for details.
* @param key The key of the string to update.
* @param offset The position in the string where <code>value</code> should be written.
* @param value The string written with <code>offset</code>.
* @return The length of the string stored at <code>key</code> after it was modified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If key holds a value that is not a string, an error is returned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do not document error cases for all commands to keep docs easy.
This comment is applicable to all commands - do you want to add it to a common place? readme?

* @example
* <pre>{@code
* Long len = client.setrange("key", 6, "GLIDE").get();
* assert len == 11L; // New key was created with length of 11 symbols
* String value = client.get("key").get();
* assert value.equals("\0\0\0\0\0\0GLIDE"); // The string was padded with zero bytes
* }</pre>
*/
CompletableFuture<Long> setrange(String key, int offset, String value);
}
21 changes: 21 additions & 0 deletions java/client/src/main/java/glide/api/models/BaseTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import static redis_request.RedisRequestOuterClass.RequestType.SIsMember;
import static redis_request.RedisRequestOuterClass.RequestType.SMembers;
import static redis_request.RedisRequestOuterClass.RequestType.SRem;
import static redis_request.RedisRequestOuterClass.RequestType.SetRange;
import static redis_request.RedisRequestOuterClass.RequestType.SetString;
import static redis_request.RedisRequestOuterClass.RequestType.Strlen;
import static redis_request.RedisRequestOuterClass.RequestType.TTL;
Expand Down Expand Up @@ -403,6 +404,26 @@ public T strlen(@NonNull String key) {
return getThis();
}

/**
* Overwrites part of the string stored at <code>key</code>, starting at the specified <code>
* offset</code>, for the entire length of <code>value</code>.<br>
* If the <code>offset</code> is larger than the current length of the string at <code>key</code>,
* the string is padded with zero bytes to make <code>offset</code> fit. Creates the <code>key
* </code> if it doesn't exist.
*
* @see <a href="https://redis.io/commands/setrange/">redis.io</a> for details.
* @param key The key of the string to update.
* @param offset The position in the string where <code>value</code> should be written.
* @param value The string written with <code>offset</code>.
* @return Command Response - The length of the string stored at <code>key</code> after it was
* modified.
*/
public T setrange(@NonNull String key, int offset, @NonNull String value) {
ArgsArray commandArgs = buildArgs(key, Integer.toString(offset), value);
protobufTransaction.addCommands(buildCommand(SetRange, commandArgs));
return getThis();
}

/**
* Retrieves the value associated with <code>field</code> in the hash stored at <code>key</code>.
*
Expand Down
27 changes: 27 additions & 0 deletions java/client/src/test/java/glide/api/RedisClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import static redis_request.RedisRequestOuterClass.RequestType.SMembers;
import static redis_request.RedisRequestOuterClass.RequestType.SRem;
import static redis_request.RedisRequestOuterClass.RequestType.Select;
import static redis_request.RedisRequestOuterClass.RequestType.SetRange;
import static redis_request.RedisRequestOuterClass.RequestType.SetString;
import static redis_request.RedisRequestOuterClass.RequestType.Strlen;
import static redis_request.RedisRequestOuterClass.RequestType.TTL;
Expand Down Expand Up @@ -977,6 +978,32 @@ public void strlen_returns_success() {
assertEquals(value, payload);
}

@SneakyThrows
@Test
public void setrange_returns_success() {
// setup
String key = "testKey";
int offset = 42;
String str = "pewpew";
String[] arguments = new String[] {key, Integer.toString(offset), str};
Long value = 10L;
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: value -> responseValue (value is confusing in the key-value context)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are 99 tests in this file and almost all of them have variable named "value".
I'm going to refactor tests in another PR (and rename the var). Is it OK for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

commandResult or futureResult is even less ambiguous.


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

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

// exercise
CompletableFuture<Long> response = service.setrange(key, offset, str);
Long payload = response.get();

// verify
assertEquals(testResponse, response);
assertEquals(value, payload);
}

@SneakyThrows
@Test
public void hget_success() {
Expand Down
4 changes: 4 additions & 0 deletions java/client/src/test/java/glide/api/models/TransactionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import static redis_request.RedisRequestOuterClass.RequestType.SIsMember;
import static redis_request.RedisRequestOuterClass.RequestType.SMembers;
import static redis_request.RedisRequestOuterClass.RequestType.SRem;
import static redis_request.RedisRequestOuterClass.RequestType.SetRange;
import static redis_request.RedisRequestOuterClass.RequestType.SetString;
import static redis_request.RedisRequestOuterClass.RequestType.Strlen;
import static redis_request.RedisRequestOuterClass.RequestType.TTL;
Expand Down Expand Up @@ -167,6 +168,9 @@ public void transaction_builds_protobuf_request(BaseTransaction<?> transaction)
transaction.strlen("key");
results.add(Pair.of(Strlen, buildArgs("key")));

transaction.setrange("key", 42, "str");
results.add(Pair.of(SetRange, buildArgs("key", "42", "str")));

transaction.hset("key", Map.of("field", "value"));
results.add(Pair.of(HashSet, buildArgs("key", "field", "value")));

Expand Down
28 changes: 28 additions & 0 deletions java/integTest/src/test/java/glide/SharedCommandTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,34 @@ public void strlen(BaseClient client) {
assertTrue(exception.getCause() instanceof RequestException);
}

@SneakyThrows
@ParameterizedTest
@MethodSource("getClients")
public void setrange(BaseClient client) {
String stringKey = UUID.randomUUID().toString();
String nonStringKey = UUID.randomUUID().toString();
// new key
assertEquals(11L, client.setrange(stringKey, 0, "Hello world").get());
// existing key
assertEquals(11L, client.setrange(stringKey, 6, "GLIDE").get());
assertEquals("Hello GLIDE", client.get(stringKey).get());

// offset > len
assertEquals(20L, client.setrange(stringKey, 15, "GLIDE").get());
assertEquals("Hello GLIDE\0\0\0\0GLIDE", client.get(stringKey).get());
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved

// non-string key
assertEquals(1, client.lpush(nonStringKey, new String[] {"_"}).get());
Exception exception =
assertThrows(ExecutionException.class, () -> client.setrange(nonStringKey, 0, "_").get());
assertTrue(exception.getCause() instanceof RequestException);
exception =
assertThrows(
ExecutionException.class,
() -> client.setrange(stringKey, Integer.MAX_VALUE, "_").get());
assertTrue(exception.getCause() instanceof RequestException);
}

@SneakyThrows
@ParameterizedTest
@MethodSource("getClients")
Expand Down
2 changes: 2 additions & 0 deletions java/integTest/src/test/java/glide/TransactionTestUtilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public static BaseTransaction<?> transactionTest(BaseTransaction<?> baseTransact
baseTransaction.incrByFloat(key3, 0.5);

baseTransaction.unlink(new String[] {key3});
baseTransaction.setrange(key3, 0, "GLIDE");

baseTransaction.hset(key4, Map.of(field1, value1, field2, value2));
baseTransaction.hget(key4, field1);
Expand Down Expand Up @@ -161,6 +162,7 @@ public static Object[] transactionTestResult() {
0L,
0.5,
1L,
5L, // setrange(key3, 0, "GLIDE")
2L,
value1,
2L, // hlen(key4)
Expand Down
Loading