Java: Groom transaction docs and add some tests. (valkey-io#1820)

shohamazon · Jul 6, 2024 · 6d73d6e · 6d73d6e
1 parent 8049a73
commit 6d73d6e
Show file tree

Hide file tree

Showing 10 changed files with 976 additions and 942 deletions.
diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java
@@ -201,7 +201,6 @@
 import glide.api.commands.StreamBaseCommands;
 import glide.api.commands.StringBaseCommands;
 import glide.api.commands.TransactionsBaseCommands;
-import glide.api.models.ArgsBuilder;
 import glide.api.models.GlideString;
 import glide.api.models.PubSubMessage;
 import glide.api.models.Script;
@@ -266,6 +265,7 @@
 import glide.managers.BaseResponseResolver;
 import glide.managers.CommandManager;
 import glide.managers.ConnectionManager;
+import glide.utils.ArgsBuilder;
 import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.HashMap;

diff --git a/java/client/src/main/java/glide/api/RedisClient.java b/java/client/src/main/java/glide/api/RedisClient.java
@@ -3,7 +3,6 @@
 
 import static glide.api.models.GlideString.gs;
 import static glide.api.models.commands.SortBaseOptions.STORE_COMMAND_STRING;
-import static glide.api.models.commands.SortOptions.STORE_COMMAND_STRING;
 import static glide.api.models.commands.function.FunctionListOptions.LIBRARY_NAME_REDIS_API;
 import static glide.api.models.commands.function.FunctionListOptions.WITH_CODE_REDIS_API;
 import static glide.api.models.commands.function.FunctionLoadOptions.REPLACE;
@@ -48,7 +47,6 @@
 import glide.api.commands.ScriptingAndFunctionsCommands;
 import glide.api.commands.ServerManagementCommands;
 import glide.api.commands.TransactionsCommands;
-import glide.api.models.ArgsBuilder;
 import glide.api.models.GlideString;
 import glide.api.models.Transaction;
 import glide.api.models.commands.FlushMode;
@@ -58,6 +56,7 @@
 import glide.api.models.commands.function.FunctionRestorePolicy;
 import glide.api.models.commands.scan.ScanOptions;
 import glide.api.models.configuration.RedisClientConfiguration;
+import glide.utils.ArgsBuilder;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
@@ -100,7 +99,7 @@ public CompletableFuture<Object> customCommand(@NonNull String[] args) {
 
     @Override
     public CompletableFuture<Object[]> exec(@NonNull Transaction transaction) {
-        if (transaction.isBinarySafeOutput()) {
+        if (transaction.isBinaryOutput()) {
             return commandManager.submitNewTransaction(
                     transaction, this::handleArrayOrNullResponseBinary);
         } else {

diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java
@@ -50,7 +50,6 @@
 import glide.api.commands.ServerManagementClusterCommands;
 import glide.api.commands.TransactionsClusterCommands;
 import glide.api.logging.Logger;
-import glide.api.models.ArgsBuilder;
 import glide.api.models.ClusterTransaction;
 import glide.api.models.ClusterValue;
 import glide.api.models.GlideString;
@@ -65,6 +64,7 @@
 import glide.api.models.configuration.RequestRoutingConfiguration.SingleNodeRoute;
 import glide.ffi.resolvers.ClusterScanCursorResolver;
 import glide.managers.CommandManager;
+import glide.utils.ArgsBuilder;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -128,7 +128,7 @@ protected ClusterValue<Object> handleCustomCommandResponse(Route route, Response
 
     @Override
     public CompletableFuture<Object[]> exec(@NonNull ClusterTransaction transaction) {
-        if (transaction.isBinarySafeOutput()) {
+        if (transaction.isBinaryOutput()) {
             return commandManager.submitNewTransaction(
                     transaction, Optional.empty(), this::handleArrayOrNullResponseBinary);
         } else {
@@ -140,7 +140,7 @@ public CompletableFuture<Object[]> exec(@NonNull ClusterTransaction transaction)
     @Override
     public CompletableFuture<Object[]> exec(
             @NonNull ClusterTransaction transaction, @NonNull SingleNodeRoute route) {
-        if (transaction.isBinarySafeOutput()) {
+        if (transaction.isBinaryOutput()) {
             return commandManager.submitNewTransaction(
                     transaction, Optional.of(route), this::handleArrayOrNullResponseBinary);
         } else {

diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java
diff --git a/java/client/src/main/java/glide/api/models/ClusterTransaction.java b/java/client/src/main/java/glide/api/models/ClusterTransaction.java
@@ -6,28 +6,31 @@
 import static redis_request.RedisRequestOuterClass.RequestType.Sort;
 import static redis_request.RedisRequestOuterClass.RequestType.SortReadOnly;
 
+import glide.api.RedisClusterClient;
 import glide.api.models.commands.SortClusterOptions;
 import lombok.NonNull;
 
 /**
- * Extends BaseTransaction class for cluster mode commands. Transactions allow the execution of a
- * group of commands in a single step.
+ * Transaction implementation for cluster {@link RedisClusterClient}. Transactions allow the
+ * execution of a group of commands in a single step.
  *
- * <p>Command Response: An array of command responses is returned by the client <code>exec</code>
- * command, in the order they were given. Each element in the array represents a command given to
- * the <code>Transaction</code>. The response for each command depends on the executed Redis
- * command. Specific response types are documented alongside each method.
+ * <p>Transaction Response: An <code>array</code> of command responses is returned by the client
+ * {@link RedisClusterClient#exec} command, in the order they were given. Each element in the array
+ * represents a command given to the {@link ClusterTransaction}. The response for each command
+ * depends on the executed Redis command. Specific response types are documented alongside each
+ * method.
  *
  * @example
- *     <pre>
- *  ClusterTransaction transaction = new ClusterTransaction();
- *    .set("key", "value");
- *    .get("key");
- *  ClusterValue[] result = client.exec(transaction, route).get();
- *  // result contains: OK and "value"
- *  </pre>
+ *     <pre>{@code
+ * ClusterTransaction transaction = new ClusterTransaction();
+ *   .set("key", "value");
+ *   .get("key");
+ * Object[] result = client.exec(transaction).get();
+ * // result contains: OK and "value"
+ * }</pre>
  */
 public class ClusterTransaction extends BaseTransaction<ClusterTransaction> {
+
     @Override
     protected ClusterTransaction getThis() {
         return this;
@@ -36,9 +39,9 @@ protected ClusterTransaction getThis() {
     /**
      * Publishes message on pubsub channel in sharded mode.
      *
-     * @since Redis 7.0 and above.
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @since Valkey 7.0 and above.
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
      * @see <a href="https://valkey.io/commands/publish/">valkey.io</a> for details.
      * @param message The message to publish.
      * @param channel The channel to publish the message on.
@@ -62,10 +65,12 @@ public <ArgType> ClusterTransaction publish(
      * <br>
      * The <code>sort</code> command can be used to sort elements based on different criteria and
      * apply transformations on sorted elements.<br>
-     * To store the result into a new key, see {@link #sortStore(String, String, SortClusterOptions)}.
+     * To store the result into a new key, see {@link #sortStore(ArgType, ArgType,
+     * SortClusterOptions)}.
      *
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
+     * @see <a href="https://valkey.io/commands/sort">valkey.io</a> for details.
      * @param key The key of the list, set, or sorted set to be sorted.
      * @param sortClusterOptions The {@link SortClusterOptions}.
      * @return Command Response - An <code>Array</code> of sorted elements.
@@ -84,9 +89,10 @@ public <ArgType> ClusterTransaction sort(
      * The <code>sortReadOnly</code> command can be used to sort elements based on different criteria
      * and apply transformations on sorted elements.<br>
      *
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
-     * @since Redis 7.0 and above.
+     * @since Valkey 7.0 and above.
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
+     * @see <a href="https://valkey.io/commands/sort_ro">valkey.io</a> for details.
      * @param key The key of the list, set, or sorted set to be sorted.
      * @param sortClusterOptions The {@link SortClusterOptions}.
      * @return Command Response - An <code>Array</code> of sorted elements.
@@ -104,11 +110,12 @@ public <ArgType> ClusterTransaction sortReadOnly(
      * <code>destination</code>. The <code>sort</code> command can be used to sort elements based on
      * different criteria, apply transformations on sorted elements, and store the result in a new
      * key.<br>
-     * To get the sort result without storing it into a key, see {@link #sort(String,
-     * SortClusterOptions)} or {@link #sortReadOnly(String, SortClusterOptions)}.
+     * To get the sort result without storing it into a key, see {@link #sort(ArgType,
+     * SortClusterOptions)} or {@link #sortReadOnly(ArgType, SortClusterOptions)}.
      *
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
+     * @see <a href="https://valkey.io/commands/sort">valkey.io</a> for details.
      * @param key The key of the list, set, or sorted set to be sorted.
      * @param destination The key where the sorted result will be stored.
      * @param sortClusterOptions The {@link SortClusterOptions}.

diff --git a/java/client/src/main/java/glide/api/models/Transaction.java b/java/client/src/main/java/glide/api/models/Transaction.java
@@ -11,19 +11,19 @@
 import static redis_request.RedisRequestOuterClass.RequestType.Sort;
 import static redis_request.RedisRequestOuterClass.RequestType.SortReadOnly;
 
+import glide.api.RedisClient;
 import glide.api.models.commands.SortOptions;
 import glide.api.models.commands.scan.ScanOptions;
-import lombok.AllArgsConstructor;
 import lombok.NonNull;
 
 /**
- * Extends BaseTransaction class for Redis standalone commands. Transactions allow the execution of
- * a group of commands in a single step.
+ * Transaction implementation for standalone {@link RedisClient}. Transactions allow the execution
+ * of a group of commands in a single step.
  *
- * <p>Command Response: An array of command responses is returned by the client <code>exec</code>
- * command, in the order they were given. Each element in the array represents a command given to
- * the <code>Transaction</code>. The response for each command depends on the executed Redis
- * command. Specific response types are documented alongside each method.
+ * <p>Transaction Response: An <code>array</code> of command responses is returned by the client
+ * {@link RedisClient#exec} API, in the order they were given. Each element in the array represents
+ * a command given to the {@link Transaction}. The response for each command depends on the executed
+ * Valkey command. Specific response types are documented alongside each method.
  *
  * @example
  *     <pre>{@code
@@ -36,15 +36,15 @@
  * assert result[1].equals("value");
  * }</pre>
  */
-@AllArgsConstructor
 public class Transaction extends BaseTransaction<Transaction> {
+
     @Override
     protected Transaction getThis() {
         return this;
     }
 
     /**
-     * Changes the currently selected Redis database.
+     * Changes the currently selected server database.
      *
      * @see <a href="https://valkey.io/commands/select/">valkey.io</a> for details.
      * @param index The index of the database to select.
@@ -59,6 +59,8 @@ public Transaction select(long index) {
      * Move <code>key</code> from the currently selected database to the database specified by <code>
      * dbIndex</code>.
      *
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
      * @see <a href="https://valkey.io/commands/move/">valkey.io</a> for more details.
      * @param key The key to move.
      * @param dbIndex The index of the database to move <code>key</code> to.
@@ -77,9 +79,9 @@ public <ArgType> Transaction move(ArgType key, long dbIndex) {
      * <code>destinationDB</code>. When <code>replace</code> is true, removes the <code>destination
      * </code> key first if it already exists, otherwise performs no action.
      *
-     * @since Redis 6.2.0 and above.
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @since Valkey 6.2.0 and above.
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
      * @see <a href="https://valkey.io/commands/copy/">valkey.io</a> for details.
      * @param source The key to the source value.
      * @param destination The key where the value should be copied to.
@@ -97,9 +99,9 @@ public <ArgType> Transaction copy(
      * <code>destinationDB</code>. When <code>replace</code> is true, removes the <code>destination
      * </code> key first if it already exists, otherwise performs no action.
      *
-     * @since Redis 6.2.0 and above.
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @since Valkey 6.2.0 and above.
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
      * @see <a href="https://valkey.io/commands/copy/">valkey.io</a> for details.
      * @param source The key to the source value.
      * @param destination The key where the value should be copied to.
@@ -129,8 +131,9 @@ public <ArgType> Transaction copy(
      * apply transformations on sorted elements.<br>
      * To store the result into a new key, see {@link #sortStore(ArgType, ArgType, SortOptions)}.
      *
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
+     * @see <a href="https://valkey.io/commands/sort">valkey.io</a> for details.
      * @param key The key of the list, set, or sorted set to be sorted.
      * @param sortOptions The {@link SortOptions}.
      * @return Command Response - An <code>Array</code> of sorted elements.
@@ -147,9 +150,10 @@ public <ArgType> Transaction sort(@NonNull ArgType key, @NonNull SortOptions sor
      * The <code>sortReadOnly</code> command can be used to sort elements based on different criteria
      * and apply transformations on sorted elements.<br>
      *
-     * @since Redis 7.0 and above.
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @since Valkey 7.0 and above.
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
+     * @see <a href="https://valkey.io/commands/sort_ro">valkey.io</a> for details.
      * @param key The key of the list, set, or sorted set to be sorted.
      * @param sortOptions The {@link SortOptions}.
      * @return Command Response - An <code>Array</code> of sorted elements.
@@ -169,8 +173,9 @@ public <ArgType> Transaction sortReadOnly(
      * key.<br>
      * To get the sort result without storing it into a key, see {@link #sort(ArgType, SortOptions)}.
      *
-     * @implNote ArgType is limited to String or GlideString, any other type will throw
-     *     IllegalArgumentException
+     * @implNote {@link ArgType} is limited to {@link String} or {@link GlideString}, any other type
+     *     will throw {@link IllegalArgumentException}.
+     * @see <a href="https://valkey.io/commands/sort">valkey.io</a> for details.
      * @param key The key of the list, set, or sorted set to be sorted.
      * @param sortOptions The {@link SortOptions}.
      * @param destination The key where the sorted result will be stored.
@@ -194,7 +199,7 @@ public <ArgType> Transaction sortStore(
     /**
      * Iterates incrementally over a database for matching keys.
      *
-     * @see <a href="https://valkey.io/commands/zscan">valkey.io</a> for details.
+     * @see <a href="https://valkey.io/commands/scan">valkey.io</a> for details.
      * @param cursor The cursor that points to the next iteration of results. A value of <code>"0"
      *     </code> indicates the start of the search.
      * @return Command Response - An <code>Array</code> of <code>Objects</code>. The first element is
@@ -211,7 +216,7 @@ public <ArgType> Transaction scan(@NonNull ArgType cursor) {
     /**
      * Iterates incrementally over a database for matching keys.
      *
-     * @see <a href="https://valkey.io/commands/zscan">valkey.io</a> for details.
+     * @see <a href="https://valkey.io/commands/scan">valkey.io</a> for details.
      * @param cursor The cursor that points to the next iteration of results. A value of <code>"0"
      *     </code> indicates the start of the search.
      * @param options The {@link ScanOptions}.

diff --git a/...in/java/glide/api/models/ArgsBuilder.java → ...rc/main/java/glide/utils/ArgsBuilder.java b/...in/java/glide/api/models/ArgsBuilder.java → ...rc/main/java/glide/utils/ArgsBuilder.java
@@ -1,6 +1,7 @@
 /** Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */
-package glide.api.models;
+package glide.utils;
 
+import glide.api.models.GlideString;
 import java.util.ArrayList;
 
 /**

diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java
@@ -282,7 +282,6 @@
 import static redis_request.RedisRequestOuterClass.RequestType.ZUnion;
 import static redis_request.RedisRequestOuterClass.RequestType.ZUnionStore;
 
-import glide.api.models.ArgsBuilder;
 import glide.api.models.GlideString;
 import glide.api.models.Script;
 import glide.api.models.Transaction;
@@ -358,6 +357,7 @@
 import glide.api.models.commands.stream.StreamTrimOptions.MaxLen;
 import glide.api.models.commands.stream.StreamTrimOptions.MinId;
 import glide.managers.CommandManager;
+import glide.utils.ArgsBuilder;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;

diff --git a/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java b/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java
@@ -5,6 +5,7 @@
 import static glide.TestUtilities.assertDeepEquals;
 import static glide.TestUtilities.generateLuaLibCode;
 import static glide.api.BaseClient.OK;
+import static glide.api.models.GlideString.gs;
 import static glide.api.models.commands.SortBaseOptions.OrderBy.DESC;
 import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_PRIMARIES;
 import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleSingleNodeRoute.RANDOM;
@@ -355,9 +356,7 @@ public void test_transaction_function_dump_restore() {
         clusterClient.functionLoad(code, true).get();
 
         // Verify functionDump
-        ClusterTransaction transaction = new ClusterTransaction();
-        transaction.withBinarySafeOutput();
-        transaction.functionDump();
+        ClusterTransaction transaction = new ClusterTransaction().withBinaryOutput().functionDump();
         Object[] result = clusterClient.exec(transaction).get();
         GlideString payload = (GlideString) (result[0]);
 
@@ -424,4 +423,24 @@ public void test_transaction_xinfoStream() {
                 },
                 results);
     }
+
+    @SneakyThrows
+    @Test
+    public void binary_strings() {
+        String key = UUID.randomUUID().toString();
+        clusterClient.set(key, "_").get();
+        // use dump to ensure that we have non-string convertible bytes
+        var bytes = clusterClient.dump(gs(key)).get();
+
+        var transaction =
+                new ClusterTransaction().withBinaryOutput().set(gs(key), gs(bytes)).get(gs(key));
+
+        var responses = clusterClient.exec(transaction).get();
+
+        assertDeepEquals(
+                new Object[] {
+                    OK, gs(bytes),
+                },
+                responses);
+    }
 }