Java: Expanded tests for converting non UTF-8 bytes to Strings

CHANGELOG.md

@@ -1,5 +1,6 @@
 #### Changes
 * Node: Added `invokeScript` API with routing for cluster client ([#2284](https://github.com/valkey-io/valkey-glide/pull/2284))
+* Java: Expanded tests for converting non UTF-8 bytes to Strings ([#2286](https://github.com/valkey-io/valkey-glide/pull/2286))
 * Python: Replace instances of Redis with Valkey ([#2266](https://github.com/valkey-io/valkey-glide/pull/2266))
 * Java: Replace instances of Redis with Valkey ([#2268](https://github.com/valkey-io/valkey-glide/pull/2268))
 * Node: Replace instances of Redis with Valkey ([#2260](https://github.com/valkey-io/valkey-glide/pull/2260))

glide-core/src/client/value_conversion.rs

@@ -141,9 +141,19 @@ pub(crate) fn convert_to_expected_type(
             )
                 .into()),
         },
-        ExpectedReturnType::BulkString => Ok(Value::BulkString(
-            from_owned_redis_value::<String>(value)?.into(),
-        )),
+        ExpectedReturnType::BulkString => match value {
+            Value::Okay | Value::Nil | Value::BulkString(_) => Ok(value),
+            Value::Int(_) | Value::Double(_) | Value::Boolean(_) | Value::BigNumber(_) => Ok(Value::BulkString(
+                from_owned_redis_value::<String>(value)?.into(),
+            )),
+            // Don't convert simple strings or other types (e.g. arrays) to a BulkString
+            _ => Err((
+                ErrorKind::TypeError,
+                "Response couldn't be converted to BulkString",
+                format!("(response was {:?})", get_value_type(&value)),
+            )
+                .into()),
+        },
         ExpectedReturnType::SimpleString => Ok(Value::SimpleString(
             from_owned_redis_value::<String>(value)?,
         )),
@@ -340,7 +350,7 @@ pub(crate) fn convert_to_expected_type(
         //
         // Example:
         // let input = ["key", "val1", "val2"]
-        // let expected =("key", vec!["val1", "val2"])
+        // let output = ("key", vec!["val1", "val2"])
         ExpectedReturnType::ArrayOfStringAndArrays => match value {
             Value::Nil => Ok(value),
             Value::Array(array) if array.len() == 2 && matches!(array[1], Value::Array(_)) => {

java/client/src/main/java/glide/utils/ArrayTransformUtils.java

@@ -86,10 +86,10 @@ public static GlideString[] convertNestedArrayToKeyValueGlideStringArray(GlideSt
      * Converts a map of string keys and values of any type into an array of strings with alternating
      * values and keys.
      *
-     * @param args Map of string keys to values of any type to convert.
+     * @param args Map of string keys to values of Double type to convert.
      * @return Array of strings [value1.toString(), key1, value2.toString(), key2, ...].
      */
-    public static String[] convertMapToValueKeyStringArray(Map<String, ?> args) {
+    public static String[] convertMapToValueKeyStringArray(Map<String, Double> args) {
         return args.entrySet().stream()
                 .flatMap(entry -> Stream.of(entry.getValue().toString(), entry.getKey()))
                 .toArray(String[]::new);

java/integTest/src/test/java/glide/SharedCommandTests.java

@@ -992,11 +992,11 @@ public void hset_hget_existing_fields_non_existing_fields(BaseClient client) {
     @SneakyThrows
     @ParameterizedTest(autoCloseArguments = false)
     @MethodSource("getClients")
-    public void non_UTF8_GlideString_test(BaseClient client) {
+    public void non_UTF8_GlideString_map(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 hashNonUTF8Key = gs(new byte[] {(byte) 0xDD});
         GlideString value = gs(nonUTF8Bytes);
         String stringField = "field";
         Map<GlideString, GlideString> fieldValueMap = Map.of(gs(stringField), value);
@@ -1023,6 +1023,150 @@ public void non_UTF8_GlideString_test(BaseClient client) {
                 client.hget(hashNonUTF8Key, gs(stringField)).get().toString());
     }
 
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void non_UTF8_GlideString_map_with_double(BaseClient client) {
+        byte[] nonUTF8Bytes = new byte[] {(byte) 0xEE};
+        GlideString key = gs(UUID.randomUUID().toString());
+        GlideString nonUTF8Key = gs(new byte[] {(byte) 0xEF});
+        Map<GlideString, Double> membersScores =
+                Map.of(gs(nonUTF8Bytes), 1.0, gs("two"), 2.0, gs("three"), 3.0);
+
+        // Testing map values using byte[] that cannot be converted to UTF-8 Strings.
+        assertEquals(3, client.zadd(key, membersScores).get());
+        assertThrows(
+                ExecutionException.class,
+                () -> client.zrange(key.toString(), new RangeByIndex(0, 1)).get());
+
+        // Testing keys for a map using byte[] that cannot be converted to UTF-8 Strings returns bytes.
+        assertEquals(3, client.zadd(nonUTF8Key, membersScores).get());
+        // No error is thrown as GlideString will be returned when arguments are GlideStrings.
+        assertDeepEquals(
+                new GlideString[] {gs(nonUTF8Bytes), gs("two"), gs("three")},
+                client.zrange(nonUTF8Key, new RangeByIndex(0, -1)).get());
+
+        // Converting non UTF-8 bytes result to String returns a message.
+        assertEquals(
+                "Value not convertible to string: byte[] 13",
+                client.zrange(nonUTF8Key, new RangeByIndex(0, -1)).get()[0].toString());
+    }
+
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void non_UTF8_GlideString_nested_array(BaseClient client) {
+        byte[] nonUTF8Bytes = new byte[] {(byte) 0xEE};
+        GlideString key = gs(UUID.randomUUID().toString());
+        GlideString nonUTF8Key = gs(new byte[] {(byte) 0xFF});
+        GlideString field = gs(nonUTF8Bytes);
+        GlideString value1 = gs(nonUTF8Bytes);
+        GlideString value2 = gs("foobar");
+        GlideString[][] entry = new GlideString[][] {{field, value1}, {field, value2}};
+
+        // Testing stream values using byte[] that cannot be converted to UTF-8 Strings.
+        client.xadd(key, entry).get();
+        assertThrows(
+                ExecutionException.class,
+                () -> client.xrange(key.toString(), InfRangeBound.MIN, InfRangeBound.MAX).get());
+
+        // Testing keys for a stream using byte[] that cannot be converted to UTF-8 Strings returns
+        // bytes.
+        GlideString streamId = client.xadd(nonUTF8Key, entry).get();
+        // No error is thrown as GlideString will be returned when arguments are GlideStrings.
+        Map<GlideString, GlideString[][]> expected =
+                Map.of(streamId, new GlideString[][] {{field, value1}, {field, value2}});
+        assertDeepEquals(
+                expected, client.xrange(nonUTF8Key, InfRangeBound.MIN, InfRangeBound.MAX).get());
+
+        // Converting non UTF-8 bytes result to String returns a message.
+        assertEquals(
+                "Value not convertible to string: byte[] 13",
+                client
+                        .xrange(nonUTF8Key, InfRangeBound.MIN, InfRangeBound.MAX)
+                        .get()
+                        .get(streamId)[0][0]
+                        .toString());
+    }
+
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void non_UTF8_GlideString_map_with_geospatial(BaseClient client) {
+        byte[] nonUTF8Bytes = new byte[] {(byte) 0xEE};
+        GlideString key = gs(UUID.randomUUID().toString());
+        GlideString nonUTF8Key = gs(new byte[] {(byte) 0xDF});
+        Map<GlideString, GeospatialData> membersToCoordinates = new HashMap<>();
+        membersToCoordinates.put(gs(nonUTF8Bytes), new GeospatialData(13.361389, 38.115556));
+        membersToCoordinates.put(gs("Catania"), new GeospatialData(15.087269, 37.502669));
+
+        // Testing geospatial values using byte[] that cannot be converted to UTF-8 Strings.
+        assertEquals(2, client.geoadd(key, membersToCoordinates).get());
+        assertThrows(
+                ExecutionException.class,
+                () ->
+                        client
+                                .geosearch(
+                                        key.toString(),
+                                        new CoordOrigin(new GeospatialData(15, 37)),
+                                        new GeoSearchShape(400, 400, GeoUnit.KILOMETERS))
+                                .get());
+
+        // Testing keys for geospatial using byte[] that cannot be converted to UTF-8 Strings returns
+        // bytes.
+        assertEquals(2, client.geoadd(nonUTF8Key, membersToCoordinates).get());
+        // No error is thrown as GlideString will be returned when arguments are GlideStrings.
+        assertTrue(
+                Set.of(new GlideString[] {gs(nonUTF8Bytes), gs("Catania")})
+                        .containsAll(
+                                Set.of(
+                                        client
+                                                .geosearch(
+                                                        nonUTF8Key,
+                                                        new CoordOrigin(new GeospatialData(15, 37)),
+                                                        new GeoSearchShape(400, 400, GeoUnit.KILOMETERS))
+                                                .get())));
+
+        // Converting non UTF-8 bytes result to String returns a message.
+        assertEquals(
+                "Value not convertible to string: byte[] 13",
+                client
+                        .geosearch(
+                                nonUTF8Key,
+                                new CoordOrigin(new GeospatialData(15, 37)),
+                                new GeoSearchShape(400, 400, GeoUnit.KILOMETERS))
+                        .get()[0]
+                        .toString());
+    }
+
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void non_UTF8_GlideString_map_of_arrays(BaseClient client) {
+        byte[] nonUTF8Bytes = new byte[] {(byte) 0xEE};
+        GlideString key = gs(UUID.randomUUID().toString());
+        GlideString nonUTF8Key = gs(new byte[] {(byte) 0xFE});
+        GlideString[] lpushArgs = {gs(nonUTF8Bytes), gs("two")};
+
+        // Testing map of arrays using byte[] that cannot be converted to UTF-8 Strings.
+        assertEquals(2, client.lpush(key, lpushArgs).get());
+        // trying to take a string from a key, but the key stores a non-string-compatible value
+        // decoding failed and value lost
+        assertThrows(
+                ExecutionException.class,
+                () -> client.lmpop(new String[] {key.toString()}, ListDirection.RIGHT).get());
+
+        // Testing map of arrays using byte[] that cannot be converted to UTF-8 Strings returns bytes.
+        assertEquals(2, client.lpush(nonUTF8Key, lpushArgs).get());
+        // No error is thrown as GlideString will be returned when arguments are GlideStrings.
+        var popResult = client.lmpop(new GlideString[] {nonUTF8Key}, ListDirection.RIGHT).get();
+        assertDeepEquals(Map.of(nonUTF8Key, new GlideString[] {gs(nonUTF8Bytes)}), popResult);
+
+        // Converting non UTF-8 bytes result to String returns a message.
+        assertEquals(
+                "Value not convertible to string: byte[] 13", popResult.get(nonUTF8Key)[0].toString());
+    }
+
     @SneakyThrows
     @ParameterizedTest(autoCloseArguments = false)
     @MethodSource("getClients")