vert-x3 / vertx-redis-client

Redis client for Vert.x
http://vertx.io
Apache License 2.0
129 stars 116 forks source link

rxSrandmember -> response.getKeys method returns even keys. #237

Closed pjgg closed 3 years ago

pjgg commented 4 years ago

Version

3.9.1

Context

When I make a rxSrandmember mySet 5 query, I should retrieve 5 randoms keys from a collection. Instead, I retrieve the half of the keys or an exception "Number of key is not even"

Do you have a reproducer?

Redis Raw query(Expected behavior):

127.0.0.1:6379> SADD myset one two three
(integer) 3

127.0.0.1:6379> SRANDMEMBER myset 2
1) "two"
2) "three"

Vertx API (Rx):

var count = "5";
var key = "myset"
con.rxSrandmember(Arrays.asList(key, count)).map(response -> response.getKeys());

Note: Response has the right amount of keys. BUT getKeys filter the half.

SourceCode: https://github.com/vert-x3/vertx-redis-client/blob/master/src/main/java/io/vertx/redis/client/impl/types/MultiType.java#L79

(looks that is the intended behavior, but doesn't match with the excepted one). Walkaround: ReWrite getKeys

return con.rxSrandmember(Arrays.asList(key, Integer.toString(count)))
                .map(response -> {
                    //response.getKeys()
                   getKeys(response);
                });

private Set<String> getKeys(Response response) {
        final Set<String> keys = new HashSet<>();
        for (int i = 0; i < response.size(); i++) {
            keys.add(response.get(i).toString());
        }
        return keys;
    }

Extras

RedisVersion: "redis:6.0.5-alpine3.12" (Docker)

pendula95 commented 4 years ago

You are looking at this wrong. method response.getKeys() returns keys that are specified in request, not result (elements of set) of the command. To get results (random elements from list) please call something like this: response.stream().map(Response::toString).collect(Collectors.toList()));

pendula95 commented 4 years ago

Nope, my bad. Looks like there is some strange logic with this even number that is required. @pmlopes I can not see from history or comments why this is implemented like this. Even naming convention of the method does not seems logical at all. Can you please elaborate on this, I would like to fix this.

pendula95 commented 4 years ago

I guess this was intended for return values that are map like. Like a hash set so even memebers of result set would be keys and uneven would be values. key1, value1, key2, value2, key3, value3 The "free form" interface that vertx redis client exposes assumes you know which return type you are expecting so you will call methods accordingly. For set like Results you shouldn't use methods public Response get(String key) and public Set<String> getKeys() as there are no keys in result and you can not retrieve values by key values.

pmlopes commented 4 years ago

Hi @pjgg, the last comment from @pendula95 is correct. The key keys is supposed to allow a response to be used like a map. This is useful for example when working hgetall https://redis.io/commands/hgetall

Where a common response is:

redis>  HGETALL myhash

1) "field1"
2) "Hello"
3) "field2"
4) "World"

In this case you can do:

for (String key : response.getKeys()) {
  System.out.println("Key is: " + key);
  System.out.println("Value is: " + response.get(key));
}

If you just want all values as a list then use the iterator or use size in a loop:

for (int i = 0; i < response.size(); i++) {
  System.out.println(response.get(i).asString());
}