Java: Fix byte GlideString conversion to String bug (#2271)

* Java: Fix byte GlideString conversion to String bug Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>
valkey-io · Sep 11, 2024 · cf5483f · cf5483f
1 parent ba7af10
commit cf5483f
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -124,6 +124,7 @@
 * Core: Change FUNCTION STATS command to return multi node response for standalone mode ([#2117](https://github.com/valkey-io/valkey-glide/pull/2117))
 
 #### Fixes
+* Java: Fix GlideString conversion from byte to String ([#2271](https://github.com/valkey-io/valkey-glide/pull/2271))
 * Java: Add overloads for XADD to allow duplicate entry keys ([#1970](https://github.com/valkey-io/valkey-glide/pull/1970))
 * Node: Fix ZADD bug where command could not be called with only the `changed` optional parameter ([#1995](https://github.com/valkey-io/valkey-glide/pull/1995))
 * Java: `XRange`/`XRevRange` should return `null` instead of `GlideException` when given a negative count ([#1920](https://github.com/valkey-io/valkey-glide/pull/1920))

diff --git a/java/client/src/main/java/glide/utils/ArrayTransformUtils.java b/java/client/src/main/java/glide/utils/ArrayTransformUtils.java
@@ -32,15 +32,15 @@ public static String[] convertMapToKeyValueStringArray(Map<String, ?> args) {
     }
 
     /**
-     * Converts a map of GlideString keys and values of any type in to an array of GlideStrings with
-     * alternating keys and values.
+     * Converts a map of GlideString keys and values to an array of GlideStrings.
      *
-     * @param args Map of GlideString keys to values of any type to convert.
+     * @param args Map of GlideString keys to values of GlideString.
      * @return Array of strings [key1, gs(value1.toString()), key2, gs(value2.toString()), ...].
      */
-    public static GlideString[] convertMapToKeyValueGlideStringArray(Map<GlideString, ?> args) {
+    public static GlideString[] convertMapToKeyValueGlideStringArray(
+            Map<GlideString, GlideString> args) {
         return args.entrySet().stream()
-                .flatMap(entry -> Stream.of(entry.getKey(), GlideString.gs(entry.getValue().toString())))
+                .flatMap(entry -> Stream.of(entry.getKey(), entry.getValue()))
                 .toArray(GlideString[]::new);
     }
 
@@ -64,10 +64,10 @@ public static String[] convertNestedArrayToKeyValueStringArray(String[][] args)
     }
 
     /**
-     * Converts a nested array of GlideString keys and values of any type in to an array of
-     * GlideStrings with alternating keys and values.
+     * Converts a nested array of GlideString keys and values in to an array of GlideStrings with
+     * alternating keys and values.
      *
-     * @param args Nested array of GlideString keys to values of any type to convert.
+     * @param args Nested array of GlideString keys and values to convert.
      * @return Array of strings [key1, gs(value1.toString()), key2, gs(value2.toString()), ...].
      */
     public static GlideString[] convertNestedArrayToKeyValueGlideStringArray(GlideString[][] args) {
@@ -78,7 +78,7 @@ public static GlideString[] convertNestedArrayToKeyValueGlideStringArray(GlideSt
             }
         }
         return Arrays.stream(args)
-                .flatMap(entry -> Stream.of(entry[0], GlideString.gs(entry[1].toString())))
+                .flatMap(entry -> Stream.of(entry[0], entry[1]))
                 .toArray(GlideString[]::new);
     }
 
@@ -99,10 +99,10 @@ public static String[] convertMapToValueKeyStringArray(Map<String, ?> args) {
      * Converts a map of GlideString keys and values of any type into an array of GlideStrings with
      * alternating values and keys.
      *
-     * @param args Map of GlideString keys to values of any type to convert.
+     * @param args Map of GlideString keys to values of Double type to convert.
      * @return Array of GlideStrings [gs(value1.toString()), key1, gs(value2.toString()), key2, ...].
      */
-    public static GlideString[] convertMapToValueKeyStringArrayBinary(Map<GlideString, ?> args) {
+    public static GlideString[] convertMapToValueKeyStringArrayBinary(Map<GlideString, Double> args) {
         return args.entrySet().stream()
                 .flatMap(entry -> Stream.of(gs(entry.getValue().toString()), entry.getKey()))
                 .toArray(GlideString[]::new);

diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java
@@ -984,6 +984,40 @@ public void hset_hget_existing_fields_non_existing_fields(BaseClient client) {
         assertNull(client.hget(key, "non_existing_field").get());
     }
 
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void non_UTF8_GlideString_test(BaseClient client) {
+        byte[] nonUTF8Bytes = new byte[] {(byte) 0xEE};
+        GlideString key = gs(nonUTF8Bytes);
+        GlideString hashKey = gs(UUID.randomUUID().toString());
+        GlideString hashNonUTF8Key = gs(new byte[] {(byte) 0xFF});
+        GlideString value = gs(nonUTF8Bytes);
+        String stringField = "field";
+        Map<GlideString, GlideString> fieldValueMap = Map.of(gs(stringField), value);
+
+        // Testing keys and values using byte[] that cannot be converted to UTF-8 Strings.
+        assertEquals(OK, client.set(key, value).get());
+        assertEquals(value, client.get(key).get());
+
+        // Testing set values using byte[] that cannot be converted to UTF-8 Strings.
+        assertEquals(1, client.hset(hashKey, fieldValueMap).get());
+        assertDeepEquals(new GlideString[] {gs(stringField)}, client.hkeys(hashKey).get());
+        assertThrows(
+                ExecutionException.class, () -> client.hget(hashKey.toString(), stringField).get());
+
+        // Testing keys for a set using byte[] that cannot be converted to UTF-8 Strings returns bytes.
+        assertEquals(1, client.hset(hashNonUTF8Key, fieldValueMap).get());
+        assertDeepEquals(new GlideString[] {gs(stringField)}, client.hkeys(hashNonUTF8Key).get());
+        // No error is thrown as GlideString will be returned when arguments are GlideStrings.
+        assertEquals(value, client.hget(hashNonUTF8Key, gs(stringField)).get());
+
+        // Converting non UTF-8 bytes result to String returns a message.
+        assertEquals(
+                "Value not convertible to string: byte[] 13",
+                client.hget(hashNonUTF8Key, gs(stringField)).get().toString());
+    }
+
     @SneakyThrows
     @ParameterizedTest(autoCloseArguments = false)
     @MethodSource("getClients")