jedis
jedis copied to clipboard
Fix return value of HRANDFIELD With Values when count is negative
###
Expected behavior
hrandfieldWithValues(String key, long count)
should return the count equivalent entries in case of a negative (absolute value of count) or a positive count value.
Actual behavior
hrandfieldWithValues(String key, long count)
currently returns a Map<String, String>
which does not account for the duplicates that can be returned(https://redis.io/commands/hrandfield/) when a negative count is given. This results in an inaccurate number (less than the count) of random entries being returned when there is a negative count.
Steps to reproduce:
- Create a Hash Set with size 5
- Call
hrandfieldWithValues(String key, long count)
with a count of -3. - Assert for the size of the result.
Map<String, String> hash = createHash(5);
jedis.hset(KEY, hash);
long count = -3L;
Map<String, String> result = jedis.hrandfieldWithValues(KEY, count);
assertThat(result.size()).isEqualTo(Math.abs(count));
Redis / Jedis Configuration
Jedis version: 3.7.0
Redis version: 6.2.0
Java version: 8
As per Redis documentation -
When the count is a negative value, the behavior changes as follows:
Repeating fields are possible. Exactly count fields, or an empty array if the hash is empty (non-existing key), are always returned. The order of fields in the reply is truly random.
So output may contain repeating fields when the count is a negative value. Current implementation uses "Map<String, String>" for response builder which will not account for repeating fields.
@sazzad16 A map which stores collection of values for a key will work good here ? Something like this -
public static final Builder<Map<String, List<String>>> STRING_MULTI_MAP = new Builder<Map<String, List<String>>>() {
@Override
@SuppressWarnings("unchecked")
public Map<String, List<String>> build(Object data) {
final List<byte[]> flatHash = (List<byte[]>) data;
final Map<String, List<String>> hash = new HashMap<>(flatHash.size() / 2, 1);
final Iterator<byte[]> iterator = flatHash.iterator();
while (iterator.hasNext()) {
String key = STRING.build(iterator.next());
String value = STRING.build(iterator.next());
List<String> list = hash.containsKey(key) ? hash.get(key) : new ArrayList<>();
list.add(value);
hash.put(key, list);
}
return hash;
}
@Override
public String toString() {
return "Map<String, List<String>>";
}
};
The new return type will be "Map<String, List<String>>". Count of collections for all keys will be equivalent to the absolute value of input count.
@s-sathish It would be simpler and better to return a List
of pairs.
@sazzad16 Thanks. Yes, List
of pairs works good here.
Resolved by #3425 and #3430