Skip to content

Commit b1e2d80

Browse files
gsmetgsmet
authored
Merge pull request #49501 from Ladicek/fix-redis-cursors
Redis Client: fix *SCAN cursor handling
2 parents 391bc11 + 42e0112 commit b1e2d80

File tree

10 files changed

+59
-70
lines changed

10 files changed

+59
-70
lines changed

‎extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/Cursor.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,24 @@ public interface Cursor<T> {
44

55
/**
66
* The cursor id when no operations have been emitted yet.
7+
*
8+
* @deprecated Previously, this value was -1, assuming that this value is never produced by Redis as an actual cursor.
9+
* However, Redis uses unsigned 64-bit integers as cursor values, so the assumption was wrong
10+
* (-1 as 64-bit signed integer is 0xFFFF_FFFF_FFFF_FFFF, which is the biggest unsigned 64-bit integer).
11+
* This should have never been exposed publicly and should not be relied upon.
12+
* <p>
13+
* The current value is 0, which is the correct initial <em>and</em> final cursor value in Redis.
714
*/
15+
@Deprecated(forRemoval = true, since = "3.26")
816
long INITIAL_CURSOR_ID = ReactiveCursor.INITIAL_CURSOR_ID;
917

1018
boolean hasNext();
1119

1220
T next();
1321

22+
/**
23+
* @deprecated see {@link #INITIAL_CURSOR_ID}
24+
*/
25+
@Deprecated(forRemoval = true, since = "3.26")
1426
long cursorId();
1527
}

‎extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/ReactiveCursor.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,24 @@ public interface ReactiveCursor<T> {
66

77
/**
88
* The cursor id when no operations have been emitted yet.
9+
*
10+
* @deprecated Previously, this value was -1, assuming that this value is never produced by Redis as an actual cursor.
11+
* However, Redis uses unsigned 64-bit integers as cursor values, so the assumption was wrong
12+
* (-1 as 64-bit signed integer is 0xFFFF_FFFF_FFFF_FFFF, which is the biggest unsigned 64-bit integer).
13+
* This should have never been exposed publicly and should not be relied upon.
14+
* <p>
15+
* The current value is 0, which is the correct initial <em>and</em> final cursor value in Redis.
916
*/
10-
long INITIAL_CURSOR_ID = -1L;
17+
@Deprecated(forRemoval = true, since = "3.26")
18+
long INITIAL_CURSOR_ID = 0L;
1119

1220
boolean hasNext();
1321

1422
Uni<T> next();
1523

24+
/**
25+
* @deprecated see {@link #INITIAL_CURSOR_ID}
26+
*/
27+
@Deprecated(forRemoval = true, since = "3.26")
1628
long cursorId();
1729
}

‎extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/HScanReactiveCursorImpl.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class HScanReactiveCursorImpl<F, V> extends AbstractRedisCommands impleme
1818
private final Type typeOfField;
1919
private final Type typeOfValue;
2020
private long cursor;
21+
private boolean initial;
2122
private final List<String> extra = new ArrayList<>();
2223

2324
public <K> HScanReactiveCursorImpl(RedisCommandExecutor redis, K key, Marshaller marshaller, Type typeOfField,
@@ -27,24 +28,21 @@ public <K> HScanReactiveCursorImpl(RedisCommandExecutor redis, K key, Marshaller
2728
this.key = marshaller.encode(key);
2829
this.typeOfField = typeOfField;
2930
this.typeOfValue = typeOfValue;
30-
this.cursor = INITIAL_CURSOR_ID;
31+
this.cursor = 0;
32+
this.initial = true;
3133
this.extra.addAll(extra);
3234
}
3335

3436
@Override
3537
public boolean hasNext() {
36-
return cursor != 0;
38+
return initial || cursor != 0;
3739
}
3840

3941
@Override
4042
public Uni<Map<F, V>> next() {
41-
long pos = cursor == INITIAL_CURSOR_ID ? 0 : cursor;
42-
RedisCommand cmd = RedisCommand.of(Command.HSCAN);
43-
cmd.put(key);
44-
cmd.put(pos);
45-
cmd.putAll(extra);
46-
return execute(cmd)
47-
.invoke(response -> cursor = response.get(0).toLong())
43+
initial = false;
44+
return execute(RedisCommand.of(Command.HSCAN).put(key).put(Long.toUnsignedString(cursor)).putAll(extra))
45+
.invoke(response -> cursor = Long.parseUnsignedLong(response.get(0).toString()))
4846
.map(response -> decode(response.get(1)));
4947
}
5048

‎extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/SScanReactiveCursorImpl.java‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import java.util.ArrayList;
55
import java.util.List;
66

7-
import io.quarkus.redis.datasource.ReactiveCursor;
87
import io.quarkus.redis.datasource.set.ReactiveSScanCursor;
98
import io.smallrye.mutiny.Multi;
109
import io.smallrye.mutiny.Uni;
@@ -17,32 +16,30 @@ public class SScanReactiveCursorImpl<V> extends AbstractRedisCommands implements
1716
private final Type typeOfValue;
1817
private final Marshaller marshaller;
1918
private long cursor;
19+
private boolean initial;
2020
private final List<String> extra = new ArrayList<>();
2121

2222
public <K> SScanReactiveCursorImpl(RedisCommandExecutor redis, K key, Marshaller marshaller,
2323
Type typeOfValue, List<String> extra) {
2424
super(redis, marshaller);
2525
this.key = marshaller.encode(key);
26-
this.cursor = ReactiveCursor.INITIAL_CURSOR_ID;
26+
this.cursor = 0;
27+
this.initial = true;
2728
this.marshaller = marshaller;
2829
this.typeOfValue = typeOfValue;
2930
this.extra.addAll(extra);
3031
}
3132

3233
@Override
3334
public boolean hasNext() {
34-
return cursor != 0;
35+
return initial || cursor != 0;
3536
}
3637

3738
@Override
3839
public Uni<List<V>> next() {
39-
long pos = cursor == INITIAL_CURSOR_ID ? 0 : cursor;
40-
RedisCommand cmd = RedisCommand.of(Command.SSCAN);
41-
cmd.put(key);
42-
cmd.put(Long.toString(pos));
43-
cmd.putAll(extra);
44-
return execute(cmd)
45-
.invoke(response -> cursor = response.get(0).toLong())
40+
initial = false;
41+
return execute(RedisCommand.of(Command.SSCAN).put(key).put(Long.toUnsignedString(cursor)).putAll(extra))
42+
.invoke(response -> cursor = Long.parseUnsignedLong(response.get(0).toString()))
4643
.map(response -> {
4744
Response array = response.get(1);
4845
List<V> list = new ArrayList<>();

‎extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/ScanReactiveCursorImpl.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import java.util.List;
66
import java.util.Set;
77

8-
import io.quarkus.redis.datasource.ReactiveCursor;
98
import io.quarkus.redis.datasource.keys.ReactiveKeyScanCursor;
109
import io.smallrye.mutiny.Multi;
1110
import io.smallrye.mutiny.Uni;
@@ -15,11 +14,13 @@ public class ScanReactiveCursorImpl<K> extends AbstractRedisCommands implements
1514

1615
private final Type typeOfKey;
1716
private long cursor;
17+
private boolean initial;
1818
private final List<String> extra = new ArrayList<>();
1919

2020
public ScanReactiveCursorImpl(RedisCommandExecutor redis, Marshaller marshaller, Type typeOfKey, List<String> extra) {
2121
super(redis, marshaller);
22-
this.cursor = ReactiveCursor.INITIAL_CURSOR_ID;
22+
this.cursor = 0;
23+
this.initial = true;
2324
this.typeOfKey = typeOfKey;
2425
this.extra.addAll(extra);
2526
}
@@ -31,14 +32,14 @@ public long cursorId() {
3132

3233
@Override
3334
public boolean hasNext() {
34-
return cursor != 0;
35+
return initial || cursor != 0;
3536
}
3637

3738
@Override
3839
public Uni<Set<K>> next() {
39-
long pos = cursor == INITIAL_CURSOR_ID ? 0 : cursor;
40-
return execute(RedisCommand.of(Command.SCAN).put(pos).putAll(extra))
41-
.invoke(response -> cursor = response.get(0).toLong())
40+
initial = false;
41+
return execute(RedisCommand.of(Command.SCAN).put(Long.toUnsignedString(cursor)).putAll(extra))
42+
.invoke(response -> cursor = Long.parseUnsignedLong(response.get(0).toString()))
4243
.map(response -> marshaller.decodeAsSet(response.get(1), typeOfKey));
4344
}
4445

‎extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/ZScanReactiveCursorImpl.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,29 @@ public class ZScanReactiveCursorImpl<V> extends AbstractRedisCommands implements
1616
private final byte[] key;
1717
private final Type typeOfValue;
1818
private long cursor;
19+
private boolean initial;
1920
private final List<String> extra = new ArrayList<>();
2021

2122
public <K> ZScanReactiveCursorImpl(RedisCommandExecutor redis, K key, Marshaller marshaller, Type typeOfValue,
2223
List<String> extra) {
2324
super(redis, marshaller);
2425
this.key = marshaller.encode(key);
25-
this.cursor = INITIAL_CURSOR_ID;
26+
this.cursor = 0;
27+
this.initial = true;
2628
this.typeOfValue = typeOfValue;
2729
this.extra.addAll(extra);
2830
}
2931

3032
@Override
3133
public boolean hasNext() {
32-
return cursor != 0;
34+
return initial || cursor != 0;
3335
}
3436

3537
@Override
3638
public Uni<List<ScoredValue<V>>> next() {
37-
RedisCommand cmd = RedisCommand.of(Command.ZSCAN);
38-
long pos = cursor == INITIAL_CURSOR_ID ? 0 : cursor;
39-
cmd.put(key);
40-
cmd.put(Long.toString(pos));
41-
cmd.putAll(extra);
42-
return execute(cmd)
43-
.invoke(response -> cursor = response.get(0).toLong())
39+
initial = false;
40+
return execute(RedisCommand.of(Command.ZSCAN).put(key).put(Long.toUnsignedString(cursor)).putAll(extra))
41+
.invoke(response -> cursor = Long.parseUnsignedLong(response.get(0).toString()))
4442
.map(response -> {
4543
Response array = response.get(1);
4644
V value = null;

‎extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/HashCommandsTest.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,11 +284,9 @@ void hscan() {
284284
hash.hset(key, "one", Person.person0);
285285
HashScanCursor<String, Person> cursor = hash.hscan(key);
286286

287-
assertThat(cursor.cursorId()).isEqualTo(-1L);
288287
assertThat(cursor.hasNext()).isTrue();
289288
Map<String, Person> next = cursor.next();
290289

291-
assertThat(cursor.cursorId()).isEqualTo(0);
292290
assertThat(next).containsExactly(entry("one", Person.person0));
293291
assertThat(cursor.hasNext()).isFalse();
294292
}
@@ -297,11 +295,10 @@ void hscan() {
297295
void hscanEmpty() {
298296
HashScanCursor<String, Person> cursor = hash.hscan(key);
299297

300-
assertThat(cursor.cursorId()).isEqualTo(-1L);
301298
assertThat(cursor.hasNext()).isTrue();
302299
Map<String, Person> next = cursor.next();
303300

304-
assertThat(cursor.cursorId()).isEqualTo(0);
301+
assertThat(cursor.hasNext()).isFalse();
305302
assertThat(next).isEmpty();
306303
}
307304

@@ -324,11 +321,9 @@ void hscanWithArgs() {
324321
hash.hset(key, "three", Person.person2);
325322
HashScanCursor<String, Person> cursor = hash.hscan(key, new ScanArgs().count(3));
326323

327-
assertThat(cursor.cursorId()).isEqualTo(-1L);
328324
assertThat(cursor.hasNext()).isTrue();
329325
Map<String, Person> next = cursor.next();
330326

331-
assertThat(cursor.cursorId()).isEqualTo(0);
332327
assertThat(next).containsExactly(entry("one", Person.person0), entry("two", Person.person1),
333328
entry("three", Person.person2));
334329
assertThat(cursor.hasNext()).isFalse();

‎extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/KeyCommandsTest.java‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -392,38 +392,34 @@ void type() {
392392
void scan() {
393393
values.set(key, Person.person7);
394394
KeyScanCursor<String> cursor = keys.scan();
395-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
395+
assertThat(cursor.hasNext()).isTrue();
396396
assertThat(cursor.next()).containsExactly(key);
397397
assertThat(cursor.hasNext()).isFalse();
398-
assertThat(cursor.cursorId()).isEqualTo(0);
399398
}
400399

401400
@Test
402401
void scanEmpty() {
403402
KeyScanCursor<String> cursor = keys.scan();
404-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
403+
assertThat(cursor.hasNext()).isTrue();
405404
assertThat(cursor.next()).isEmpty();
406405
assertThat(cursor.hasNext()).isFalse();
407-
assertThat(cursor.cursorId()).isEqualTo(0);
408406
}
409407

410408
@Test
411409
void scanIterableEmpty() {
412410
KeyScanCursor<String> cursor = keys.scan();
413-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
411+
assertThat(cursor.hasNext()).isTrue();
414412
assertThat(cursor.toIterable()).isEmpty();
415413
assertThat(cursor.hasNext()).isFalse();
416-
assertThat(cursor.cursorId()).isEqualTo(0);
417414
}
418415

419416
@Test
420417
void scanWithArgs() {
421418
values.set(key, Person.person7);
422419
KeyScanCursor<String> cursor = keys.scan(new KeyScanArgs().count(10));
423-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
420+
assertThat(cursor.hasNext()).isTrue();
424421
assertThat(cursor.next()).containsExactly(key);
425422
assertThat(cursor.hasNext()).isFalse();
426-
assertThat(cursor.cursorId()).isEqualTo(0);
427423
}
428424

429425
@Test
@@ -446,7 +442,6 @@ void scanMultiple() {
446442

447443
KeyScanCursor<String> cursor = keys.scan(new KeyScanArgs().count(12));
448444

449-
assertThat(cursor.cursorId()).isNotEqualTo(0);
450445
assertThat(cursor.hasNext()).isTrue();
451446

452447
Set<String> check = new HashSet<>(cursor.next());
@@ -481,10 +476,9 @@ void scanMatch() {
481476
Set<String> expect = new HashSet<>();
482477
populateMany(expect);
483478
KeyScanCursor<String> cursor = keys.scan(new KeyScanArgs().count(200).match(key + "*"));
484-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
479+
assertThat(cursor.hasNext()).isTrue();
485480
assertThat(cursor.next()).hasSize(expect.size());
486481
assertThat(cursor.hasNext()).isFalse();
487-
assertThat(cursor.cursorId()).isEqualTo(0);
488482
}
489483

490484
void populateMany(Set<String> expect) {

‎extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/SetCommandsTest.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,10 @@ void sscan() {
199199
sets.sadd(key, person1);
200200
SScanCursor<Person> cursor = sets.sscan(key);
201201

202-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
203202
assertThat(cursor.hasNext()).isTrue();
204203

205204
List<Person> list = cursor.next();
206205

207-
assertThat(cursor.cursorId()).isEqualTo(0);
208206
assertThat(cursor.hasNext()).isFalse();
209207
assertThat(list).hasSize(1).containsExactly(person1);
210208
}
@@ -213,12 +211,10 @@ void sscan() {
213211
void sscanEmpty() {
214212
SScanCursor<Person> cursor = sets.sscan(key);
215213

216-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
217214
assertThat(cursor.hasNext()).isTrue();
218215

219216
List<Person> list = cursor.next();
220217

221-
assertThat(cursor.cursorId()).isEqualTo(0);
222218
assertThat(cursor.hasNext()).isFalse();
223219
assertThat(list).isEmpty();
224220
}
@@ -227,7 +223,6 @@ void sscanEmpty() {
227223
void sscanEmptyAsIterable() {
228224
SScanCursor<Person> cursor = sets.sscan(key);
229225

230-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
231226
assertThat(cursor.hasNext()).isTrue();
232227

233228
Iterable<Person> iterable = cursor.toIterable();
@@ -240,12 +235,10 @@ void sscanWithCursorAndArgs() {
240235
sets.sadd(key, person1);
241236
SScanCursor<Person> cursor = sets.sscan(key, new ScanArgs().count(3));
242237

243-
assertThat(cursor.cursorId()).isEqualTo(Cursor.INITIAL_CURSOR_ID);
244238
assertThat(cursor.hasNext()).isTrue();
245239

246240
List<Person> list = cursor.next();
247241

248-
assertThat(cursor.cursorId()).isEqualTo(0);
249242
assertThat(cursor.hasNext()).isFalse();
250243
assertThat(list).hasSize(1).containsExactly(person1);
251244

0 commit comments

Comments
 (0)