Skip to content

Commit c1f0bcc

Browse files
jabolinathachlp
jabolina
authored and
thachlp
committed
fix: Lcs response parse (redis#2970)
* fix: Lcs response parse * Handle parsing `len` and `matches` in any order; * Increase test coverage * Add more unit tests to handle the different cases; * Add integration tests when the `LCS` command is available; * Update the StringMatchResultOutput to avoid keeping track of withIdx parameter. This allows the use in the command factory.
1 parent 30e91f5 commit c1f0bcc

File tree

5 files changed

+269
-27
lines changed

5 files changed

+269
-27
lines changed

‎src/main/java/io/lettuce/core/RedisCommandBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2909,7 +2909,7 @@ Command<K, V, StringMatchResult> stralgoLcs(StrAlgoArgs strAlgoArgs) {
29092909

29102910
CommandArgs<K, V> args = new CommandArgs<>(codec);
29112911
strAlgoArgs.build(args);
2912-
return createCommand(STRALGO, new StringMatchResultOutput<>(codec, strAlgoArgs.isWithIdx()), args);
2912+
return createCommand(STRALGO, new StringMatchResultOutput<>(codec), args);
29132913
}
29142914

29152915
Command<K, V, Set<V>> sunion(K... keys) {

‎src/main/java/io/lettuce/core/dynamic/output/OutputRegistry.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ public class OutputRegistry {
5656
register(registry, StringListOutput.class, StringListOutput::new);
5757
register(registry, VoidOutput.class, VoidOutput::new);
5858

59+
register(registry, StringMatchResultOutput.class, StringMatchResultOutput::new);
60+
5961
BUILTIN.putAll(registry);
6062
}
6163

‎src/main/java/io/lettuce/core/output/StringMatchResultOutput.java‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,16 @@
1919
*/
2020
package io.lettuce.core.output;
2121

22-
import static io.lettuce.core.StringMatchResult.MatchedPosition;
23-
import static io.lettuce.core.StringMatchResult.Position;
22+
import io.lettuce.core.StringMatchResult;
23+
import io.lettuce.core.codec.RedisCodec;
2424

2525
import java.nio.ByteBuffer;
26+
import java.nio.charset.StandardCharsets;
2627
import java.util.ArrayList;
2728
import java.util.List;
2829

29-
import io.lettuce.core.StringMatchResult;
30-
import io.lettuce.core.codec.RedisCodec;
30+
import static io.lettuce.core.StringMatchResult.MatchedPosition;
31+
import static io.lettuce.core.StringMatchResult.Position;
3132

3233
/**
3334
* Command output for {@code STRALGO} returning {@link StringMatchResult}.
@@ -37,38 +38,39 @@
3738
*/
3839
public class StringMatchResultOutput<K, V> extends CommandOutput<K, V, StringMatchResult> {
3940

40-
private final boolean withIdx;
41+
private static final ByteBuffer LEN = StandardCharsets.US_ASCII.encode("len");
4142

4243
private String matchString;
4344

4445
private int len;
4546

4647
private List<Long> positions;
4748

49+
private boolean readingLen = true;
50+
4851
private final List<MatchedPosition> matchedPositions = new ArrayList<>();
4952

50-
public StringMatchResultOutput(RedisCodec<K, V> codec, boolean withIdx) {
53+
public StringMatchResultOutput(RedisCodec<K, V> codec) {
5154
super(codec, null);
52-
this.withIdx = withIdx;
5355
}
5456

5557
@Override
5658
public void set(ByteBuffer bytes) {
57-
58-
if (!withIdx && matchString == null) {
59-
matchString = (String) codec.decodeKey(bytes);
60-
}
59+
matchString = (String) codec.decodeKey(bytes);
60+
readingLen = LEN.equals(bytes);
6161
}
6262

6363
@Override
6464
public void set(long integer) {
65-
66-
this.len = (int) integer;
67-
68-
if (positions == null) {
69-
positions = new ArrayList<>();
65+
if (readingLen) {
66+
this.len = (int) integer;
67+
} else {
68+
if (positions == null) {
69+
positions = new ArrayList<>();
70+
}
71+
positions.add(integer);
7072
}
71-
positions.add(integer);
73+
matchString = null;
7274
}
7375

7476
@Override

‎src/test/java/io/lettuce/core/commands/StringCommandIntegrationTests.java‎

Lines changed: 125 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121

2222
import static io.lettuce.TestTags.INTEGRATION_TEST;
2323
import static io.lettuce.core.SetArgs.Builder.*;
24-
import static io.lettuce.core.StringMatchResult.*;
25-
import static org.assertj.core.api.Assertions.*;
24+
import static io.lettuce.core.StringMatchResult.Position;
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2627

28+
import java.lang.reflect.Proxy;
2729
import java.time.Duration;
2830
import java.time.Instant;
2931
import java.util.LinkedHashMap;
@@ -32,20 +34,21 @@
3234

3335
import javax.inject.Inject;
3436

37+
import org.junit.jupiter.api.Assumptions;
3538
import org.junit.jupiter.api.BeforeEach;
3639
import org.junit.jupiter.api.Tag;
3740
import org.junit.jupiter.api.Test;
3841
import org.junit.jupiter.api.TestInstance;
3942
import org.junit.jupiter.api.extension.ExtendWith;
4043

41-
import io.lettuce.core.GetExArgs;
42-
import io.lettuce.core.KeyValue;
43-
import io.lettuce.core.RedisException;
44-
import io.lettuce.core.SetArgs;
45-
import io.lettuce.core.StrAlgoArgs;
46-
import io.lettuce.core.StringMatchResult;
47-
import io.lettuce.core.TestSupport;
44+
import io.lettuce.core.*;
45+
import io.lettuce.core.api.StatefulConnection;
46+
import io.lettuce.core.api.StatefulRedisConnection;
4847
import io.lettuce.core.api.sync.RedisCommands;
48+
import io.lettuce.core.dynamic.Commands;
49+
import io.lettuce.core.dynamic.RedisCommandFactory;
50+
import io.lettuce.core.dynamic.annotation.Command;
51+
import io.lettuce.core.dynamic.annotation.Param;
4952
import io.lettuce.test.KeyValueStreamingAdapter;
5053
import io.lettuce.test.LettuceExtension;
5154
import io.lettuce.test.condition.EnabledOnCommand;
@@ -376,4 +379,117 @@ void strAlgoWithIdx() {
376379
assertThat(matchResult.getLen()).isEqualTo(6);
377380
}
378381

382+
@Test
383+
@EnabledOnCommand("LCS")
384+
void lcs() {
385+
redis.set("key1", "ohmytext");
386+
redis.set("key2", "mynewtext");
387+
388+
// LCS key1 key2
389+
CustomStringCommands commands = CustomStringCommands.instance(getConnection());
390+
StringMatchResult matchResult = commands.lcs("key1", "key2");
391+
assertThat(matchResult.getMatchString()).isEqualTo("mytext");
392+
393+
// LCS a b IDX MINMATCHLEN 4 WITHMATCHLEN
394+
// Keys don't exist.
395+
matchResult = commands.lcsMinMatchLenWithMatchLen("a", "b", 4);
396+
assertThat(matchResult.getMatchString()).isNullOrEmpty();
397+
assertThat(matchResult.getLen()).isEqualTo(0);
398+
}
399+
400+
@Test
401+
@EnabledOnCommand("LCS")
402+
void lcsUsingKeys() {
403+
404+
redis.set("key1{k}", "ohmytext");
405+
redis.set("key2{k}", "mynewtext");
406+
407+
CustomStringCommands commands = CustomStringCommands.instance(getConnection());
408+
409+
StringMatchResult matchResult = commands.lcs("key1{k}", "key2{k}");
410+
assertThat(matchResult.getMatchString()).isEqualTo("mytext");
411+
412+
// STRALGO LCS STRINGS a b
413+
matchResult = commands.lcsMinMatchLenWithMatchLen("a", "b", 4);
414+
assertThat(matchResult.getMatchString()).isNullOrEmpty();
415+
assertThat(matchResult.getLen()).isEqualTo(0);
416+
}
417+
418+
@Test
419+
@EnabledOnCommand("LCS")
420+
void lcsJustLen() {
421+
redis.set("one", "ohmytext");
422+
redis.set("two", "mynewtext");
423+
424+
CustomStringCommands commands = CustomStringCommands.instance(getConnection());
425+
426+
StringMatchResult matchResult = commands.lcsLen("one", "two");
427+
428+
assertThat(matchResult.getLen()).isEqualTo(6);
429+
}
430+
431+
@Test
432+
@EnabledOnCommand("LCS")
433+
void lcsWithMinMatchLen() {
434+
redis.set("key1", "ohmytext");
435+
redis.set("key2", "mynewtext");
436+
437+
CustomStringCommands commands = CustomStringCommands.instance(getConnection());
438+
439+
StringMatchResult matchResult = commands.lcsMinMatchLen("key1", "key2", 4);
440+
441+
assertThat(matchResult.getMatchString()).isEqualTo("mytext");
442+
}
443+
444+
@Test
445+
@EnabledOnCommand("LCS")
446+
void lcsMinMatchLenIdxMatchLen() {
447+
redis.set("key1", "ohmytext");
448+
redis.set("key2", "mynewtext");
449+
450+
CustomStringCommands commands = CustomStringCommands.instance(getConnection());
451+
452+
// LCS key1 key2 IDX MINMATCHLEN 4 WITHMATCHLEN
453+
StringMatchResult matchResult = commands.lcsMinMatchLenWithMatchLen("key1", "key2", 4);
454+
455+
assertThat(matchResult.getMatches()).hasSize(1);
456+
assertThat(matchResult.getMatches().get(0).getMatchLen()).isEqualTo(4);
457+
458+
Position a = matchResult.getMatches().get(0).getA();
459+
Position b = matchResult.getMatches().get(0).getB();
460+
461+
assertThat(a.getStart()).isEqualTo(4);
462+
assertThat(a.getEnd()).isEqualTo(7);
463+
assertThat(b.getStart()).isEqualTo(5);
464+
assertThat(b.getEnd()).isEqualTo(8);
465+
assertThat(matchResult.getLen()).isEqualTo(6);
466+
}
467+
468+
protected StatefulConnection<String, String> getConnection() {
469+
StatefulRedisConnection<String, String> src = redis.getStatefulConnection();
470+
Assumptions.assumeFalse(Proxy.isProxyClass(src.getClass()), "Redis connection is proxy, skipping.");
471+
return src;
472+
}
473+
474+
private interface CustomStringCommands extends Commands {
475+
476+
@Command("LCS :k1 :k2")
477+
StringMatchResult lcs(@Param("k1") String k1, @Param("k2") String k2);
478+
479+
@Command("LCS :k1 :k2 LEN")
480+
StringMatchResult lcsLen(@Param("k1") String k1, @Param("k2") String k2);
481+
482+
@Command("LCS :k1 :k2 MINMATCHLEN :mml")
483+
StringMatchResult lcsMinMatchLen(@Param("k1") String k1, @Param("k2") String k2, @Param("mml") int mml);
484+
485+
@Command("LCS :k1 :k2 IDX MINMATCHLEN :mml WITHMATCHLEN")
486+
StringMatchResult lcsMinMatchLenWithMatchLen(@Param("k1") String k1, @Param("k2") String k2, @Param("mml") int mml);
487+
488+
static CustomStringCommands instance(StatefulConnection<String, String> conn) {
489+
RedisCommandFactory factory = new RedisCommandFactory(conn);
490+
return factory.getCommands(CustomStringCommands.class);
491+
}
492+
493+
}
494+
379495
}

‎src/test/java/io/lettuce/core/output/StringMatchResultOutputUnitTests.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package io.lettuce.core.output;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.nio.ByteBuffer;
6+
import java.nio.charset.StandardCharsets;
7+
import java.util.ArrayList;
8+
import java.util.HashMap;
9+
import java.util.List;
10+
import java.util.Map;
11+
12+
import io.netty.buffer.Unpooled;
13+
import org.junit.jupiter.api.Test;
14+
15+
import io.lettuce.core.StringMatchResult;
16+
import io.lettuce.core.codec.StringCodec;
17+
import io.lettuce.core.protocol.ProtocolVersion;
18+
import io.lettuce.core.protocol.RedisStateMachine;
19+
20+
public class StringMatchResultOutputUnitTests {
21+
22+
@Test
23+
void parseManually() {
24+
byte[] rawOne = "%2\r\n$7\r\nmatches\r\n*1\r\n*3\r\n*2\r\n:4\r\n:7\r\n*2\r\n:5\r\n:8\r\n:4\r\n$3\r\nlen\r\n:6\r\n"
25+
.getBytes(StandardCharsets.US_ASCII);
26+
byte[] rawTwo = "%2\r\n$3\r\nlen\r\n:6\r\n$7\r\nmatches\r\n*1\r\n*3\r\n*2\r\n:4\r\n:7\r\n*2\r\n:5\r\n:8\r\n:4\r\n"
27+
.getBytes(StandardCharsets.US_ASCII);
28+
RedisStateMachine rsm = new RedisStateMachine();
29+
rsm.setProtocolVersion(ProtocolVersion.RESP3);
30+
31+
StringMatchResultOutput<String, String> o1 = new StringMatchResultOutput<>(StringCodec.ASCII);
32+
assertThat(rsm.decode(Unpooled.wrappedBuffer(rawOne), o1)).isTrue();
33+
34+
StringMatchResultOutput<String, String> o2 = new StringMatchResultOutput<>(StringCodec.ASCII);
35+
assertThat(rsm.decode(Unpooled.wrappedBuffer(rawTwo), o2)).isTrue();
36+
37+
Map<String, Object> res1 = transform(o1.get());
38+
Map<String, Object> res2 = transform(o2.get());
39+
40+
assertThat(res1).isEqualTo(res2);
41+
}
42+
43+
private Map<String, Object> transform(StringMatchResult result) {
44+
Map<String, Object> obj = new HashMap<>();
45+
List<Object> matches = new ArrayList<>();
46+
for (StringMatchResult.MatchedPosition match : result.getMatches()) {
47+
Map<String, Object> intra = new HashMap<>();
48+
Map<String, Object> a = new HashMap<>();
49+
Map<String, Object> b = new HashMap<>();
50+
a.put("start", match.getA().getStart());
51+
a.put("end", match.getA().getEnd());
52+
53+
b.put("start", match.getB().getStart());
54+
b.put("end", match.getB().getEnd());
55+
intra.put("a", a);
56+
intra.put("b", b);
57+
intra.put("matchLen", match.getMatchLen());
58+
matches.add(intra);
59+
}
60+
obj.put("matches", matches);
61+
obj.put("len", result.getLen());
62+
return obj;
63+
}
64+
65+
@Test
66+
void parseOnlyStringMatch() {
67+
StringMatchResultOutput<String, String> output = new StringMatchResultOutput<>(StringCodec.ASCII);
68+
69+
String matchString = "some-string";
70+
output.set(ByteBuffer.wrap(matchString.getBytes()));
71+
output.complete(0);
72+
73+
StringMatchResult result = output.get();
74+
assertThat(result.getMatchString()).isEqualTo(matchString);
75+
assertThat(result.getMatches()).isEmpty();
76+
assertThat(result.getLen()).isZero();
77+
}
78+
79+
@Test
80+
void parseOnlyLen() {
81+
StringMatchResultOutput<String, String> output = new StringMatchResultOutput<>(StringCodec.ASCII);
82+
83+
output.set(42);
84+
output.complete(0);
85+
86+
StringMatchResult result = output.get();
87+
assertThat(result.getMatchString()).isNull();
88+
assertThat(result.getMatches()).isEmpty();
89+
assertThat(result.getLen()).isEqualTo(42);
90+
}
91+
92+
@Test
93+
void parseLenAndMatchesWithIdx() {
94+
StringMatchResultOutput<String, String> output = new StringMatchResultOutput<>(StringCodec.ASCII);
95+
96+
output.set(ByteBuffer.wrap("len".getBytes()));
97+
output.set(42);
98+
99+
output.set(ByteBuffer.wrap("matches".getBytes()));
100+
output.set(0);
101+
output.set(5);
102+
output.set(10);
103+
output.set(15);
104+
105+
output.complete(2);
106+
output.complete(0);
107+
108+
StringMatchResult result = output.get();
109+
110+
assertThat(result.getMatchString()).isNull();
111+
assertThat(result.getLen()).isEqualTo(42);
112+
assertThat(result.getMatches()).hasSize(1).satisfies(m -> assertMatchedPositions(m.get(0), 0, 5, 10, 15));
113+
}
114+
115+
private void assertMatchedPositions(StringMatchResult.MatchedPosition match, int... expected) {
116+
assertThat(match.getA().getStart()).isEqualTo(expected[0]);
117+
assertThat(match.getA().getEnd()).isEqualTo(expected[1]);
118+
assertThat(match.getB().getStart()).isEqualTo(expected[2]);
119+
assertThat(match.getB().getEnd()).isEqualTo(expected[3]);
120+
}
121+
122+
}

0 commit comments

Comments
 (0)