Skip to content

Commit

Permalink
Removing ScriptOptions and refactoring invokeScript APIs
Browse files Browse the repository at this point in the history
Signed-off-by: TJ Zhang <[email protected]>
  • Loading branch information
TJ Zhang committed Sep 12, 2024
1 parent 2ea2850 commit 149f415
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 226 deletions.
2 changes: 1 addition & 1 deletion java/client/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ repositories {
}

dependencies {
implementation group: 'com.google.protobuf', name: 'protobuf-java', version: '4.27.1'
implementation group: 'com.google.protobuf', name: 'protobuf-java', version: '4.28.0'
implementation group: 'org.apache.commons', name: 'commons-lang3', version: '3.13.0'

implementation group: 'io.netty', name: 'netty-handler', version: '4.1.100.Final'
Expand Down
21 changes: 9 additions & 12 deletions java/client/src/main/java/glide/api/GlideClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@
import glide.api.models.Transaction;
import glide.api.models.commands.FlushMode;
import glide.api.models.commands.InfoOptions.Section;
import glide.api.models.commands.ScriptOptions;
import glide.api.models.commands.ScriptOptionsGlideString;
import glide.api.models.commands.SortOptions;
import glide.api.models.commands.SortOptionsBinary;
import glide.api.models.commands.function.FunctionRestorePolicy;
Expand Down Expand Up @@ -590,31 +588,30 @@ public CompletableFuture<Object> invokeScript(@NonNull Script script) {

@Override
public CompletableFuture<Object> invokeScript(
@NonNull Script script, @NonNull ScriptOptions options) {
@NonNull Script script, @NonNull List<String> keys, List<String> args) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script,
options.getKeys().stream().map(GlideString::gs).collect(Collectors.toList()),
options.getArgs().stream().map(GlideString::gs).collect(Collectors.toList()),
keys.stream().map(GlideString::gs).collect(Collectors.toList()),
args.stream().map(GlideString::gs).collect(Collectors.toList()),
this::handleBinaryObjectOrNullResponse);
} else {
return commandManager.submitScript(
script,
options.getKeys().stream().map(GlideString::gs).collect(Collectors.toList()),
options.getArgs().stream().map(GlideString::gs).collect(Collectors.toList()),
keys.stream().map(GlideString::gs).collect(Collectors.toList()),
args.stream().map(GlideString::gs).collect(Collectors.toList()),
this::handleObjectOrNullResponse);
}
}

@Override
public CompletableFuture<Object> invokeScript(
@NonNull Script script, @NonNull ScriptOptionsGlideString options) {
public CompletableFuture<Object> invokeScriptBinary(
@NonNull Script script, List<GlideString> keys, List<GlideString> args) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script, options.getKeys(), options.getArgs(), this::handleBinaryObjectOrNullResponse);
script, keys, args, this::handleBinaryObjectOrNullResponse);
} else {
return commandManager.submitScript(
script, options.getKeys(), options.getArgs(), this::handleObjectOrNullResponse);
return commandManager.submitScript(script, keys, args, this::handleObjectOrNullResponse);
}
}

Expand Down
21 changes: 9 additions & 12 deletions java/client/src/main/java/glide/api/GlideClusterClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@
import glide.api.models.Script;
import glide.api.models.commands.FlushMode;
import glide.api.models.commands.InfoOptions.Section;
import glide.api.models.commands.ScriptOptions;
import glide.api.models.commands.ScriptOptionsGlideString;
import glide.api.models.commands.SortClusterOptions;
import glide.api.models.commands.function.FunctionRestorePolicy;
import glide.api.models.commands.scan.ClusterScanCursor;
Expand Down Expand Up @@ -970,31 +968,30 @@ public CompletableFuture<Object> invokeScript(@NonNull Script script) {

@Override
public CompletableFuture<Object> invokeScript(
@NonNull Script script, @NonNull ScriptOptions options) {
@NonNull Script script, @NonNull List<String> keys, @NonNull List<String> args) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script,
options.getKeys().stream().map(GlideString::gs).collect(Collectors.toList()),
options.getArgs().stream().map(GlideString::gs).collect(Collectors.toList()),
keys.stream().map(GlideString::gs).collect(Collectors.toList()),
args.stream().map(GlideString::gs).collect(Collectors.toList()),
this::handleBinaryObjectOrNullResponse);
} else {
return commandManager.submitScript(
script,
options.getKeys().stream().map(GlideString::gs).collect(Collectors.toList()),
options.getArgs().stream().map(GlideString::gs).collect(Collectors.toList()),
keys.stream().map(GlideString::gs).collect(Collectors.toList()),
args.stream().map(GlideString::gs).collect(Collectors.toList()),
this::handleObjectOrNullResponse);
}
}

@Override
public CompletableFuture<Object> invokeScript(
@NonNull Script script, @NonNull ScriptOptionsGlideString options) {
public CompletableFuture<Object> invokeScriptBinary(
@NonNull Script script, @NonNull List<GlideString> keys, @NonNull List<GlideString> args) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script, options.getKeys(), options.getArgs(), this::handleBinaryObjectOrNullResponse);
script, keys, args, this::handleBinaryObjectOrNullResponse);
} else {
return commandManager.submitScript(
script, options.getKeys(), options.getArgs(), this::handleObjectOrNullResponse);
return commandManager.submitScript(script, keys, args, this::handleObjectOrNullResponse);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
import glide.api.models.GlideString;
import glide.api.models.Script;
import glide.api.models.commands.FlushMode;
import glide.api.models.commands.ScriptOptions;
import glide.api.models.commands.ScriptOptionsGlideString;
import glide.api.models.commands.function.FunctionRestorePolicy;
import glide.api.models.configuration.ReadFrom;
import glide.api.models.configuration.RequestRoutingConfiguration.Route;
Expand Down Expand Up @@ -1073,19 +1071,19 @@ CompletableFuture<ClusterValue<Map<GlideString, Map<GlideString, Object>>>> func
* @see <a href="https://valkey.io/commands/script-load/">SCRIPT LOAD</a> and <a
* href="https://valkey.io/commands/evalsha/">EVALSHA</a> for details.
* @param script The Lua script to execute.
* @param options The script option that contains keys and arguments for the script.
* @param keys The keys that are used in the script.
* @param args The arguments for the script.
* @return A value that depends on the script that was executed.
* @example
* <pre>{@code
* try(Script luaScript = new Script("return { KEYS[1], ARGV[1] }", false)) {
* ScriptOptions scriptOptions = ScriptOptions.builder().key("foo").arg("bar").build();
* Object[] result = (Object[]) client.invokeScript(luaScript, scriptOptions).get();
* Object[] result = (Object[]) client.invokeScript(luaScript, List.of("foo"), List.of("bar")).get();
* assert result[0].equals("foo");
* assert result[1].equals("bar");
* }
* }</pre>
*/
CompletableFuture<Object> invokeScript(Script script, ScriptOptions options);
CompletableFuture<Object> invokeScript(Script script, List<String> keys, List<String> args);

/**
* Invokes a Lua script with its keys and arguments.<br>
Expand All @@ -1095,28 +1093,30 @@ CompletableFuture<ClusterValue<Map<GlideString, Map<GlideString, Object>>>> func
* using the <code>SCRIPT LOAD</code> command. After that, it will be invoked using the <code>
* EVALSHA</code> command.
*
* @param script The Lua script to execute.
* @param keys The keys that are used in the script.
* @param args The arguments for the script.
* @return A value that depends on the script that was executed.
* @apiNote When in cluster mode
* <ul>
* <li>all <code>keys</code> must map to the same hash slot.
* <li>if no <code>keys</code> are given, command will be routed to a random primary node.
* </ul>
*
* @see <a href="https://valkey.io/commands/script-load/">SCRIPT LOAD</a> and <a
* href="https://valkey.io/commands/evalsha/">EVALSHA</a> for details.
* @param script The Lua script to execute.
* @param options The script option that contains keys and arguments for the script.
* @return A value that depends on the script that was executed.
* @example
* <pre>{@code
* try(Script luaScript = new Script(gs("return { KEYS[1], ARGV[1] }", true))) {
* ScriptOptionsGlideString scriptOptions = ScriptOptionsGlideString.builder().key(gs("foo")).arg(gs("bar")).build();
* Object[] result = (Object[]) client.invokeScript(luaScript, scriptOptions).get();
* Object[] result = (Object[]) client.invokeScript(luaScript, List.of(gs("foo")), List.of(gs("bar"))).get();
* assert result[0].equals(gs("foo"));
* assert result[1].equals(gs("bar"));
* }
* }</pre>
*
* @see <a href="https://valkey.io/commands/script-load/">SCRIPT LOAD</a> and <a
* href="https://valkey.io/commands/evalsha/">EVALSHA</a> for details.
*/
CompletableFuture<Object> invokeScript(Script script, ScriptOptionsGlideString options);
CompletableFuture<Object> invokeScriptBinary(
Script script, List<GlideString> keys, List<GlideString> args);

/**
* Invokes a Lua script.<br>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
import glide.api.models.GlideString;
import glide.api.models.Script;
import glide.api.models.commands.FlushMode;
import glide.api.models.commands.ScriptOptions;
import glide.api.models.commands.ScriptOptionsGlideString;
import glide.api.models.commands.function.FunctionRestorePolicy;
import glide.api.models.configuration.ReadFrom;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;

Expand Down Expand Up @@ -456,19 +455,19 @@ CompletableFuture<Map<GlideString, Object>[]> functionListBinary(
* @see <a href="https://valkey.io/commands/script-load/">SCRIPT LOAD</a> and <a
* href="https://valkey.io/commands/evalsha/">EVALSHA</a> for details.
* @param script The Lua script to execute.
* @param options The script option that contains keys and arguments for the script.
* @param keys The keys that are used in the script.
* @param args The arguments for the script.
* @return A value that depends on the script that was executed.
* @example
* <pre>{@code
* try(Script luaScript = new Script("return { KEYS[1], ARGV[1] }", false)) {
* ScriptOptions scriptOptions = ScriptOptions.builder().key("foo").arg("bar").build();
* Object[] result = (Object[]) client.invokeScript(luaScript, scriptOptions).get();
* Object[] result = (Object[]) client.invokeScript(luaScript, List.of("foo"), List.of("bar")).get();
* assert result[0].equals("foo");
* assert result[1].equals("bar");
* }
* }</pre>
*/
CompletableFuture<Object> invokeScript(Script script, ScriptOptions options);
CompletableFuture<Object> invokeScript(Script script, List<String> keys, List<String> args);

/**
* Invokes a Lua script with its keys and arguments.<br>
Expand All @@ -478,28 +477,30 @@ CompletableFuture<Map<GlideString, Object>[]> functionListBinary(
* using the <code>SCRIPT LOAD</code> command. After that, it will be invoked using the <code>
* EVALSHA</code> command.
*
* @param script The Lua script to execute.
* @param keys The keys that are used in the script.
* @param args The arguments for the script.
* @return A value that depends on the script that was executed.
* @apiNote When in cluster mode
* <ul>
* <li>all <code>keys</code> must map to the same hash slot.
* <li>if no <code>keys</code> are given, command will be routed to a random primary node.
* </ul>
*
* @see <a href="https://valkey.io/commands/script-load/">SCRIPT LOAD</a> and <a
* href="https://valkey.io/commands/evalsha/">EVALSHA</a> for details.
* @param script The Lua script to execute.
* @param options The script option that contains keys and arguments for the script.
* @return A value that depends on the script that was executed.
* @example
* <pre>{@code
* try(Script luaScript = new Script(gs("return { KEYS[1], ARGV[1] }", true))) {
* ScriptOptionsGlideString scriptOptions = ScriptOptionsGlideString.builder().key(gs("foo")).arg(gs("bar")).build();
* Object[] result = (Object[]) client.invokeScript(luaScript, scriptOptions).get();
* Object[] result = (Object[]) client.invokeScript(luaScript, List.of(gs("foo")), List.of(gs("bar"))).get();
* assert result[0].equals(gs("foo"));
* assert result[1].equals(gs("bar"));
* }
* }</pre>
*
* @see <a href="https://valkey.io/commands/script-load/">SCRIPT LOAD</a> and <a
* href="https://valkey.io/commands/evalsha/">EVALSHA</a> for details.
*/
CompletableFuture<Object> invokeScript(Script script, ScriptOptionsGlideString options);
CompletableFuture<Object> invokeScriptBinary(
Script script, List<GlideString> keys, List<GlideString> args);

/**
* Checks existence of scripts in the script cache by their SHA1 digest.
Expand Down

This file was deleted.

This file was deleted.

24 changes: 7 additions & 17 deletions java/client/src/test/java/glide/api/GlideClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,6 @@
import glide.api.models.commands.RangeOptions.ScoreBoundary;
import glide.api.models.commands.RestoreOptions;
import glide.api.models.commands.ScoreFilter;
import glide.api.models.commands.ScriptOptions;
import glide.api.models.commands.ScriptOptionsGlideString;
import glide.api.models.commands.SetOptions;
import glide.api.models.commands.SetOptions.Expiry;
import glide.api.models.commands.SortBaseOptions;
Expand Down Expand Up @@ -1519,16 +1517,13 @@ public void invokeScript_returns_success() {

@SneakyThrows
@Test
public void invokeScript_with_ScriptOptions_returns_success() {
public void invokeScript_with_keys_args_returns_success() {
// setup
Script script = mock(Script.class);
String hash = UUID.randomUUID().toString();
when(script.getHash()).thenReturn(hash);
String payload = "hello";

ScriptOptions options =
ScriptOptions.builder().key("key1").key("key2").arg("arg1").arg("arg2").build();

CompletableFuture<Object> testResponse = new CompletableFuture<>();
testResponse.complete(payload);

Expand All @@ -1541,7 +1536,8 @@ public void invokeScript_with_ScriptOptions_returns_success() {
.thenReturn(testResponse);

// exercise
CompletableFuture<Object> response = service.invokeScript(script, options);
CompletableFuture<Object> response =
service.invokeScript(script, List.of("key1", "key2"), List.of("arg1", "arg2"));

// verify
assertEquals(testResponse, response);
Expand All @@ -1550,21 +1546,13 @@ public void invokeScript_with_ScriptOptions_returns_success() {

@SneakyThrows
@Test
public void invokeScript_with_ScriptOptionsGlideString_returns_success() {
public void invokeScriptBinary_returns_success() {
// setup
Script script = mock(Script.class);
String hash = UUID.randomUUID().toString();
when(script.getHash()).thenReturn(hash);
GlideString payload = gs("hello");

ScriptOptionsGlideString options =
ScriptOptionsGlideString.builder()
.key(gs("key1"))
.key(gs("key2"))
.arg(gs("arg1"))
.arg(gs("arg2"))
.build();

CompletableFuture<Object> testResponse = new CompletableFuture<>();
testResponse.complete(payload);

Expand All @@ -1577,7 +1565,9 @@ public void invokeScript_with_ScriptOptionsGlideString_returns_success() {
.thenReturn(testResponse);

// exercise
CompletableFuture<Object> response = service.invokeScript(script, options);
CompletableFuture<Object> response =
service.invokeScriptBinary(
script, List.of(gs("key1"), gs("key2")), List.of(gs("arg1"), gs("arg2")));

// verify
assertEquals(testResponse, response);
Expand Down
Loading

0 comments on commit 149f415

Please sign in to comment.