Skip to content

Commit 020084c

Browse files
committed
refactor(redis-client): apply review feedback to Value Commands
- Register argument converters in a static initializer instead of a volatile flag with double-checked locking; register() remains as an explicit class-initialization trigger. - Rework the SetArgs/GetExArgs token loops to arrow-style switches over an iterator, with a helper that consumes and validates keyword values, replacing the tokens.get(++i) side effect. - Hardcode the "map" parameter name in requireNonEmpty and drop the redundant @SuppressWarnings("deprecation") on getset.
1 parent c343a0f commit 020084c

2 files changed

Lines changed: 51 additions & 72 deletions

File tree

extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/client/lettuce/value/LettuceReactiveValueCommandsImpl.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ public Uni<String> getrange(K key, long start, long end) {
103103
}
104104

105105
@Override
106-
@SuppressWarnings("deprecation")
107106
public Uni<V> getset(K key, V value) {
108107
nonNull(key, "key");
109108
nonNull(value, "value");
@@ -167,13 +166,13 @@ private Map<K, V> toOrderedMap(List<KeyValue<K, V>> results) {
167166

168167
@Override
169168
public Uni<Void> mset(Map<K, V> map) {
170-
requireNonEmpty(map, "map");
169+
requireNonEmpty(map);
171170
return LettuceResult.toUni(() -> async.mset(map)).replaceWithVoid();
172171
}
173172

174173
@Override
175174
public Uni<Boolean> msetnx(Map<K, V> map) {
176-
requireNonEmpty(map, "map");
175+
requireNonEmpty(map);
177176
return LettuceResult.toUni(() -> async.msetnx(map));
178177
}
179178

@@ -267,9 +266,9 @@ private static boolean isOk(String response) {
267266
return "OK".equals(response);
268267
}
269268

270-
private static <K, V> void requireNonEmpty(Map<K, V> map, String name) {
269+
private static <K, V> void requireNonEmpty(Map<K, V> map) {
271270
if (map == null || map.isEmpty()) {
272-
throw new IllegalArgumentException("`" + name + "` must not be null or empty");
271+
throw new IllegalArgumentException("`map` must not be null or empty");
273272
}
274273
}
275274
}
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.quarkus.redis.runtime.client.lettuce.value;
22

3-
import java.util.List;
3+
import java.util.Iterator;
44

55
import io.quarkus.redis.datasource.value.GetExArgs;
66
import io.quarkus.redis.datasource.value.SetArgs;
@@ -9,34 +9,30 @@
99
/**
1010
* Converters bridging Quarkus Value Command argument types to their Lettuce equivalents.
1111
* <p>
12-
* Registers itself with {@link LettuceConverterRegistry} on first use.
12+
* Registration with {@link LettuceConverterRegistry} happens in this class's static initializer.
1313
*/
1414
public final class LettuceValueCommandsConverters {
1515

16-
private static volatile boolean registered;
16+
static {
17+
LettuceConverterRegistry.registerArgConverter(SetArgs.class,
18+
LettuceValueCommandsConverters::toLettuceSetArgs);
19+
LettuceConverterRegistry.registerArgConverter(GetExArgs.class,
20+
LettuceValueCommandsConverters::toLettuceGetExArgs);
21+
}
1722

1823
private LettuceValueCommandsConverters() {
1924
// Utility class
2025
}
2126

2227
/**
23-
* Register all Value Command converters with {@link LettuceConverterRegistry}.
24-
* Idempotent and thread-safe.
28+
* Ensures the Value Command converters are registered with {@link LettuceConverterRegistry}.
29+
* <p>
30+
* The registration itself runs in this class's static initializer; calling this method simply
31+
* forces class initialization at a well-defined point. It is therefore idempotent and
32+
* thread-safe by virtue of the JVM's class-initialization guarantees.
2533
*/
2634
public static void register() {
27-
if (registered) {
28-
return;
29-
}
30-
synchronized (LettuceValueCommandsConverters.class) {
31-
if (registered) {
32-
return;
33-
}
34-
LettuceConverterRegistry.registerArgConverter(SetArgs.class,
35-
LettuceValueCommandsConverters::toLettuceSetArgs);
36-
LettuceConverterRegistry.registerArgConverter(GetExArgs.class,
37-
LettuceValueCommandsConverters::toLettuceGetExArgs);
38-
registered = true;
39-
}
35+
// No-op: registration is performed in the static initializer.
4036
}
4137

4238
/**
@@ -49,36 +45,21 @@ public static void register() {
4945
*/
5046
public static io.lettuce.core.SetArgs toLettuceSetArgs(SetArgs quarkus) {
5147
io.lettuce.core.SetArgs lettuce = new io.lettuce.core.SetArgs();
52-
List<Object> tokens = quarkus.toArgs();
53-
for (int i = 0; i < tokens.size(); i++) {
54-
String token = tokens.get(i).toString();
48+
Iterator<Object> tokens = quarkus.toArgs().iterator();
49+
while (tokens.hasNext()) {
50+
String token = tokens.next().toString();
5551
switch (token) {
56-
case "EX":
57-
lettuce.ex(Long.parseLong(tokens.get(++i).toString()));
58-
break;
59-
case "EXAT":
60-
lettuce.exAt(Long.parseLong(tokens.get(++i).toString()));
61-
break;
62-
case "PX":
63-
lettuce.px(Long.parseLong(tokens.get(++i).toString()));
64-
break;
65-
case "PXAT":
66-
lettuce.pxAt(Long.parseLong(tokens.get(++i).toString()));
67-
break;
68-
case "NX":
69-
lettuce.nx();
70-
break;
71-
case "XX":
72-
lettuce.xx();
73-
break;
74-
case "KEEPTTL":
75-
lettuce.keepttl();
76-
break;
77-
case "GET":
78-
// Handled via the dedicated setGet() method on the Lettuce API
79-
break;
80-
default:
81-
throw new IllegalStateException("Unexpected SetArgs token: " + token);
52+
case "EX" -> lettuce.ex(nextLong(tokens, token));
53+
case "EXAT" -> lettuce.exAt(nextLong(tokens, token));
54+
case "PX" -> lettuce.px(nextLong(tokens, token));
55+
case "PXAT" -> lettuce.pxAt(nextLong(tokens, token));
56+
case "NX" -> lettuce.nx();
57+
case "XX" -> lettuce.xx();
58+
case "KEEPTTL" -> lettuce.keepttl();
59+
// GET is handled via the dedicated setGet() method on the Lettuce API.
60+
case "GET" -> {
61+
}
62+
default -> throw new IllegalStateException("Unexpected SetArgs token: " + token);
8263
}
8364
}
8465
return lettuce;
@@ -89,29 +70,28 @@ public static io.lettuce.core.SetArgs toLettuceSetArgs(SetArgs quarkus) {
8970
*/
9071
public static io.lettuce.core.GetExArgs toLettuceGetExArgs(GetExArgs quarkus) {
9172
io.lettuce.core.GetExArgs lettuce = new io.lettuce.core.GetExArgs();
92-
List<Object> tokens = quarkus.toArgs();
93-
for (int i = 0; i < tokens.size(); i++) {
94-
String token = tokens.get(i).toString();
73+
Iterator<Object> tokens = quarkus.toArgs().iterator();
74+
while (tokens.hasNext()) {
75+
String token = tokens.next().toString();
9576
switch (token) {
96-
case "EX":
97-
lettuce.ex(Long.parseLong(tokens.get(++i).toString()));
98-
break;
99-
case "EXAT":
100-
lettuce.exAt(Long.parseLong(tokens.get(++i).toString()));
101-
break;
102-
case "PX":
103-
lettuce.px(Long.parseLong(tokens.get(++i).toString()));
104-
break;
105-
case "PXAT":
106-
lettuce.pxAt(Long.parseLong(tokens.get(++i).toString()));
107-
break;
108-
case "PERSIST":
109-
lettuce.persist();
110-
break;
111-
default:
112-
throw new IllegalStateException("Unexpected GetExArgs token: " + token);
77+
case "EX" -> lettuce.ex(nextLong(tokens, token));
78+
case "EXAT" -> lettuce.exAt(nextLong(tokens, token));
79+
case "PX" -> lettuce.px(nextLong(tokens, token));
80+
case "PXAT" -> lettuce.pxAt(nextLong(tokens, token));
81+
case "PERSIST" -> lettuce.persist();
82+
default -> throw new IllegalStateException("Unexpected GetExArgs token: " + token);
11383
}
11484
}
11585
return lettuce;
11686
}
87+
88+
/**
89+
* Consume and parse the value token that follows a keyword (e.g. the seconds after {@code EX}).
90+
*/
91+
private static long nextLong(Iterator<Object> tokens, String token) {
92+
if (!tokens.hasNext()) {
93+
throw new IllegalStateException("Missing value for token: " + token);
94+
}
95+
return Long.parseLong(tokens.next().toString());
96+
}
11797
}

0 commit comments

Comments
 (0)