Broken iterator in Response in zrangebyscore with WITHSCORES parameter

[annashumylo]

Hi,

I detected a bug in RedisAPI.rxZrangebyscore()- When I call it with parameter WITHSCORE and try to use iterator on Response, it jumps over the next value. I found out that in MultiType.java asMap flag was set to true, but rxZrangebyscore in this case is a list with values: [value, score]. Also Response.getKeys() will return an empty set which should be correct. Actually this whole mix of map and list in the Multi response is a bit tricky and sometimes confusing.

The workaround to it would be to use for loop and Response.get(item count). This seems to work.

Redis client version 4.0.0.Beta2.

I wrote a simple test to reproduce this behavior:

@Test
    public void buggyTestRxZrangebyscore() {
        List<String> sortedSet = Arrays.asList("testKey", "1.0", "testValue1", "2.0", "testValue2");
        redisAPI.zadd(sortedSet);

        List<String> zRangebyscoreRequest = Arrays.asList("testKey", "0.0", "3.0", "WITHSCORES");
        Response response = redisAPI.rxZrangebyscore(zRangebyscoreRequest).blockingGet();
        Assertions.assertEquals(ResponseType.MULTI, response.type());
        //verify that keys are absent
        Assertions.assertEquals(0, response.getKeys().size());
        //collect all the values (buggy)
        List<String> buggyList = new ArrayList<>();
        for (Response r : response) {
            buggyList.add(r.toString());
        }
        //verify that first value is present
        Assertions.assertEquals("[testValue1, 1.0]", buggyList.get(0));
        Assertions.assertEquals(1, buggyList.size());

        //collect all the values (correct)
        List<String> correctList = new ArrayList<>();
        for (int i = 0; i < response.size(); i++) {
            correctList.add(response.get(i).toString());
        }
        //verify both values are present
        Assertions.assertEquals("[testValue1, 1.0]", correctList.get(0));
        Assertions.assertEquals("[testValue2, 2.0]", correctList.get(1));
        Assertions.assertEquals(2, correctList.size());
    }

So here you can ensure that lists created by foreach (which uses iterator) and by manual getting elements by counter are different.

[annashumylo]

Beta3 has the same issue.

[aesteve]

I think I got a similar issue while upgrading from milestone4 to Beta3.

I'm using rxHgetall which used to return an iterator over key, value, key, value. I used to use this iterator then retain even entries (the values) but now it returns a multi. Since multi is iterable, I thought I could just iterate over entries (and get rid of the odd/even stuff) but I can't. I had to:

                    redis.rxHgetall(command)
                        .map { resp ->
                          resp.keys.map(resp::get)
                                .map(::decode)
                        }

It seems response is not iterable since it's a map as indicated in this bug report the API sounds counter-intuitive

[pmlopes]

Hi, this isn't a bug per se. In redis < 6 the protocol only defined a few types, map wasn't one. On >=6 it defines many new types, where map is one.

So I assume you're testing with 6 (and as the client now fully supports all types) you observe this.

You probably should call keys() and get based on that, instead of iterate.

[pmlopes]

Regarding the original issue, there was a way the iterator was looping, and it's fixed on master. So the next beta should fix it

[aesteve]

Thanks for your answer @pmlopes and sorry for squatting this issue I thought it was related. Should we discuss usability in another issue?

Sounds weird to me that a Map result is not considered iterable.

[pmlopes]

@aesteve yes, let's open a second one. I think we could make it iterate on keys (maybe)