Skip to content

Commit d58df40

Browse files
author
Donal Evans
authored
GEODE-9519: Implement Radish ZSCAN command (#6831)
- Change implementation to return integer cursor instead of BigInteger as there was no possible way for the returned value for cursor to be greater than Integer.MAX_VALUE - Move scan execution logic to AbstractScanExecutor to allow inheritance - Do not attempt to handle CURSOR values greater than Long.MAX_VALUE for HSCAN and ZSCAN - Use scan count argument directly rather than looping in RedisSortedSet and RedisHash - Minor refactor of how scan empty results are returned - Add ZSCAN to list of supported commands - Add tests for ZSCAN Authored-by: Donal Evans <doevans@vmware.com>
1 parent 0a579bd commit d58df40

22 files changed

Lines changed: 996 additions & 145 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more contributor license
3+
* agreements. See the NOTICE file distributed with this work for additional information regarding
4+
* copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
5+
* "License"); you may not use this file except in compliance with the License. You may obtain a
6+
* copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software distributed under the License
11+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
12+
* or implied. See the License for the specific language governing permissions and limitations under
13+
* the License.
14+
*/
15+
package org.apache.geode.redis.internal.executor.sortedset;
16+
17+
import org.junit.ClassRule;
18+
19+
import org.apache.geode.redis.NativeRedisClusterTestRule;
20+
21+
public class ZScanNativeRedisIntegrationTest extends AbstractZScanIntegrationTest {
22+
23+
@ClassRule
24+
public static NativeRedisClusterTestRule redis = new NativeRedisClusterTestRule();
25+
26+
@Override
27+
public int getPort() {
28+
return redis.getExposedPorts().get(0);
29+
}
30+
31+
@Override
32+
public void flushAll() {
33+
redis.flushAll();
34+
}
35+
}

geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHScanIntegrationTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static org.assertj.core.api.Assertions.assertThat;
2727
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2828

29+
import java.math.BigInteger;
2930
import java.util.ArrayList;
3031
import java.util.Arrays;
3132
import java.util.Comparator;
@@ -57,6 +58,7 @@ public abstract class AbstractHScanIntegrationTest implements RedisIntegrationTe
5758
public static final String HASH_KEY = "key";
5859
public static final int SLOT_FOR_KEY = CRC16.calculate(HASH_KEY) % RegionProvider.REDIS_SLOTS;
5960
public static final String ZERO_CURSOR = "0";
61+
public static final BigInteger UNSIGNED_LONG_CAPACITY = new BigInteger("18446744073709551615");
6062

6163
public static final String FIELD_ONE = "1";
6264
public static final String VALUE_ONE = "yellow";
@@ -240,6 +242,20 @@ public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
240242
assertThat(new HashSet<>(allEntries)).isEqualTo(entryMap.entrySet());
241243
}
242244

245+
@Test
246+
public void shouldReturnError_givenCursorGreaterThanUnsignedLongMaxValue() {
247+
jedis.hset(HASH_KEY, FIELD_ONE, VALUE_ONE);
248+
249+
assertThatThrownBy(() -> jedis.sendCommand(HASH_KEY, Protocol.Command.HSCAN, HASH_KEY,
250+
UNSIGNED_LONG_CAPACITY.add(new BigInteger("10")).toString()))
251+
.hasMessageContaining(ERROR_CURSOR);
252+
}
253+
254+
@Test
255+
public void shouldNotError_givenCursorEqualToLongMaxValue() {
256+
jedis.hset(HASH_KEY, FIELD_ONE, VALUE_ONE);
257+
jedis.sendCommand(HASH_KEY, Protocol.Command.HSCAN, HASH_KEY, String.valueOf(Long.MAX_VALUE));
258+
}
243259

244260
@Test
245261
public void givenInvalidRegexSyntax_returnsEmptyArray() {

geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/HScanIntegrationTest.java

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
import static org.apache.geode.redis.internal.RedisConstants.ERROR_CURSOR;
2020
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2121

22-
import java.math.BigInteger;
23-
2422
import org.junit.ClassRule;
2523
import org.junit.Test;
24+
import redis.clients.jedis.Protocol;
2625

2726
import org.apache.geode.redis.GeodeRedisServerRule;
2827

2928
public class HScanIntegrationTest extends AbstractHScanIntegrationTest {
29+
String GREATER_THAN_LONG_MAX = "9_223_372_036_854_775_808";
3030

3131
@ClassRule
3232
public static GeodeRedisServerRule server = new GeodeRedisServerRule();
@@ -36,27 +36,15 @@ public int getPort() {
3636
return server.getPort();
3737
}
3838

39-
40-
// Note: these tests will not pass native redis, so included here in concrete test class
41-
@Test
42-
public void givenCursorGreaterThanIntMaxValue_returnsCursorError() {
43-
int largestCursorValue = Integer.MAX_VALUE;
44-
45-
BigInteger tooBigCursor =
46-
new BigInteger(String.valueOf(largestCursorValue)).add(BigInteger.valueOf(1));
47-
48-
assertThatThrownBy(() -> jedis.hscan("a", tooBigCursor.toString()))
49-
.hasMessageContaining(ERROR_CURSOR);
50-
}
51-
39+
// Redis allows CURSOR values up to UNSIGNED_LONG_CAPACITY, but behaviour for CURSOR values
40+
// greater than Integer.MAX_VALUE is undefined, so we choose to return an error if a value greater
41+
// than Long.MAX_VALUE is passed
5242
@Test
53-
public void givenCursorLessThanIntMinValue_returnsCursorError() {
54-
int smallestCursorValue = Integer.MIN_VALUE;
55-
56-
BigInteger tooSmallCursor =
57-
new BigInteger(String.valueOf(smallestCursorValue)).subtract(BigInteger.valueOf(1));
43+
public void shouldReturnError_givenCursorGreaterThanLongMaxValue() {
44+
jedis.hset(HASH_KEY, FIELD_ONE, VALUE_ONE);
5845

59-
assertThatThrownBy(() -> jedis.hscan("a", tooSmallCursor.toString()))
60-
.hasMessageContaining(ERROR_CURSOR);
46+
assertThatThrownBy(() -> jedis.sendCommand(HASH_KEY, Protocol.Command.HSCAN, HASH_KEY,
47+
GREATER_THAN_LONG_MAX))
48+
.hasMessageContaining(ERROR_CURSOR);
6149
}
6250
}

geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,11 @@ public void testZrevrank() {
348348
runCommandAndAssertHitsAndMisses(SORTED_SET_KEY, (k, m) -> jedis.zrevrank(k, m));
349349
}
350350

351+
@Test
352+
public void testZscan() {
353+
runCommandAndAssertHitsAndMisses(SORTED_SET_KEY, (k, v) -> jedis.zscan(k, v));
354+
}
355+
351356
@Test
352357
public void testZscore() {
353358
runCommandAndAssertHitsAndMisses(SORTED_SET_KEY, (k, v) -> jedis.zscore(k, v));

0 commit comments

Comments
 (0)