From b09199f3dc085130893445ed2e165fb738815030 Mon Sep 17 00:00:00 2001 From: Maayan Shani <maayan.shani@mail.huji.ac.il> Date: Sun, 26 Jan 2025 06:49:35 +0000 Subject: [PATCH] add conditionalSet builder for OXX and NX and change comparisonValue to NonNull Signed-off-by: Maayan Shani <maayan.shani@mail.huji.ac.il> --- .../api/commands/StringBaseCommands.java | 1 - .../glide/api/models/commands/SetOptions.java | 36 +++++++++++-------- .../test/java/glide/api/GlideClientTest.java | 2 +- 3 files changed, 22 insertions(+), 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 index c2612198c3..3a5e87fc81 100644 --- 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 index 6de5d8e908..31ef5d4267 100644 --- 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 index 1b896dedc9..2fdcb18c17 100644 --- 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();