Skip to content

Commit

Permalink
Java: Expanded tests for converting non UTF-8 bytes to Strings (valke…
Browse files Browse the repository at this point in the history 
…y-io#2286)

* Added tests for xadd, lmpop, geoadd, and zadd

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

* Added Changelog entry

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

* Applied Spotless

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

* Added new keys for tests

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

* Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

* Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

---------

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>
Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
  • Loading branch information
@GumpacG @Yury-Fridlyand
GumpacG and Yury-Fridlyand authored Sep 13, 2024
1 parent 3092db4 commit 4cf70d2
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
29 changes: 8 additions & 21 deletions glide-core/src/client/value_conversion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>(key)?.into()),
};
match inner_value {
Value::BulkString(_) => Ok((
key_str,
Value::Double(from_owned_redis_value::<f64>(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::<RedisResult<_>>();

Expand Down Expand Up @@ -141,9 +127,10 @@ 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::BulkString(_) => Ok(value),
_ => Ok(Value::BulkString(from_owned_redis_value::<String>(value)?.into())),
},
ExpectedReturnType::SimpleString => Ok(Value::SimpleString(
from_owned_redis_value::<String>(value)?,
)),
Expand Down Expand Up @@ -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(_)) => {
Expand Down
4 changes: 2 additions & 2 deletions java/client/src/main/java/glide/utils/ArrayTransformUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
148 changes: 146 additions & 2 deletions java/integTest/src/test/java/glide/SharedCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<GlideString, GlideString> fieldValueMap = Map.of(gs(stringField), value);
Expand All @@ -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<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")
Expand Down

0 comments on commit 4cf70d2

Please sign in to comment.