add conditionalSet builder for OXX and NX and change comparisonValue …

…to NonNull Signed-off-by: Maayan Shani <maayan.shani@mail.huji.ac.il>
valkey-io · Jan 26, 2025 · b09199f · b09199f
1 parent 97041b8
commit b09199f
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 17 deletions.
diff --git a/java/client/src/main/java/glide/api/commands/StringBaseCommands.java b/java/client/src/main/java/glide/api/commands/StringBaseCommands.java
@@ -209,7 +209,6 @@ public interface StringBaseCommands {
     /**
      * Sets the given key with the given value. Return value is dependent on the passed options.
      *
-     * @since Valkey 8.1 and above
      * @see <a href="https://valkey.io/commands/set/">valkey.io</a> for details.
      * @param key The key to store.
      * @param value The value to store with the given key.

diff --git a/java/client/src/main/java/glide/api/models/commands/SetOptions.java b/java/client/src/main/java/glide/api/models/commands/SetOptions.java
@@ -13,6 +13,7 @@
 import java.util.List;
 import lombok.Builder;
 import lombok.Getter;
+import lombok.NonNull;
 import lombok.RequiredArgsConstructor;
 
 /**
@@ -67,16 +68,30 @@ public static class SetOptionsBuilder {
         private ConditionalSet conditionalSet;
         private String comparisonValue;
 
+        /**
+         * Set the condition for the value to be set
+         *
+         * @param conditionalSet the condition to set (ONLY_IF_EXISTS, ONLY_IF_DOES_NOT_EXIST)
+         * @return this builder instance
+         * @throws IllegalArgumentException if the conditionalSet is ONLY_IF_EQUAL
+         */
+        public SetOptionsBuilder conditionalSet(ConditionalSet conditionalSet) {
+            if (conditionalSet == ConditionalSet.ONLY_IF_EQUAL) {
+                throw new IllegalArgumentException(
+                        "For ONLY_IF_EQUAL, use the conditionalSetIfEqualTo(String value) method.");
+            }
+            this.conditionalSet = conditionalSet;
+            this.comparisonValue = null; // Clear comparisonValue when not using ONLY_IF_EQUAL
+            return this;
+        }
+
         /**
          * Set the condition to ONLY_IF_EQUAL and specify the comparison value
          *
          * @param value the value to compare
          * @return this builder instance
          */
-        public SetOptionsBuilder conditionalSetIfEqualTo(String value) {
-            if (value == null || value.isEmpty()) {
-                throw new IllegalArgumentException("comparisonValue cannot be null or empty.");
-            }
+        public SetOptionsBuilder conditionalSetIfEqualTo(@NonNull String value) {
             this.conditionalSet = ConditionalSet.ONLY_IF_EQUAL;
             this.comparisonValue = value;
             return this;
@@ -180,18 +195,9 @@ public String[] toArgs() {
         List<String> optionArgs = new ArrayList<>();
         if (conditionalSet != null) {
             optionArgs.add(conditionalSet.valkeyApi);
-        }
-
-        // Add comparison value if ONLY_IF_EQUAL is selected
-        if (conditionalSet == ConditionalSet.ONLY_IF_EQUAL) {
-            if (comparisonValue == null) {
-                throw new IllegalArgumentException(
-                        "comparisonValue must be set when conditionalSet is ONLY_IF_EQUAL.");
+            if (conditionalSet == ConditionalSet.ONLY_IF_EQUAL) {
+                optionArgs.add(comparisonValue);
             }
-            optionArgs.add(comparisonValue);
-        } else if (comparisonValue != null) {
-            throw new IllegalArgumentException(
-                    "comparisonValue can only be set when conditionalSet is ONLY_IF_EQUAL.");
         }
 
         if (returnOldValue) {

diff --git a/java/client/src/test/java/glide/api/GlideClientTest.java b/java/client/src/test/java/glide/api/GlideClientTest.java
@@ -1022,7 +1022,7 @@ public void set_with_SetOptions_OnlyIfEqual_fails() {
         // Attempt to set `key` to `newValue` with the wrong condition
         SetOptions wrongConditionOptions =
                 SetOptions.builder()
-                        .conditionalSetIfEqualTo(newValue) // Incorrect condition: current value of key is `value`
+                        .conditionalSetIfEqualTo(newValue) // Incorrect: current value of key is `value`
                         .expiry(Expiry.UnixSeconds(60L))
                         .build();