diff --git a/CHANGELOG.md b/CHANGELOG.md index 82d41284a3..263ef23cdd 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/glide-core/src/client/value_conversion.rs b/glide-core/src/client/value_conversion.rs index 54a00e1d25..4a43da7da7 100644 --- a/glide-core/src/client/value_conversion.rs +++ b/glide-core/src/client/value_conversion.rs @@ -67,23 +67,9 @@ pub(crate) fn convert_to_expected_type( let result = map .into_iter() .map(|(key, inner_value)| { - let key_str = match key { - Value::BulkString(_) => key, - _ => Value::BulkString(from_owned_redis_value::(key)?.into()), - }; - match inner_value { - Value::BulkString(_) => Ok(( - key_str, - Value::Double(from_owned_redis_value::(inner_value)?), - )), - Value::Double(_) => Ok((key_str, inner_value)), - _ => Err(( - ErrorKind::TypeError, - "Response couldn't be converted to map of {string: double}", - format!("(response was {:?})", get_value_type(&inner_value)), - ) - .into()), - } + let key_str = convert_to_expected_type(key, Some(ExpectedReturnType::BulkString)).unwrap(); + let value_converted = convert_to_expected_type(inner_value, Some(ExpectedReturnType::Double)).unwrap(); + Ok((key_str, value_converted)) }) .collect::>(); @@ -141,9 +127,10 @@ pub(crate) fn convert_to_expected_type( ) .into()), }, - ExpectedReturnType::BulkString => Ok(Value::BulkString( - from_owned_redis_value::(value)?.into(), - )), + ExpectedReturnType::BulkString => match value { + Value::BulkString(_) => Ok(value), + _ => Ok(Value::BulkString(from_owned_redis_value::(value)?.into())), + }, ExpectedReturnType::SimpleString => Ok(Value::SimpleString( from_owned_redis_value::(value)?, )), @@ -340,7 +327,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(_)) => { diff --git a/java/client/src/main/java/glide/utils/ArrayTransformUtils.java b/java/client/src/main/java/glide/utils/ArrayTransformUtils.java index b44fd450f3..61903b132d 100644 --- a/java/client/src/main/java/glide/utils/ArrayTransformUtils.java +++ b/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 args) { + public static String[] convertMapToValueKeyStringArray(Map args) { return args.entrySet().stream() .flatMap(entry -> Stream.of(entry.getValue().toString(), entry.getKey())) .toArray(String[]::new); diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java index 54e5259e8d..c3e600d5c9 100644 --- a/java/integTest/src/test/java/glide/SharedCommandTests.java +++ b/java/integTest/src/test/java/glide/SharedCommandTests.java @@ -990,11 +990,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 fieldValueMap = Map.of(gs(stringField), value); @@ -1021,6 +1021,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 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 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 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")