Skip to content

Commit ae78dab

Browse files
mgravellmgravell
committed
additional tests and LCS cleanup
1 parent f75aeeb commit ae78dab

12 files changed

Lines changed: 850 additions & 32 deletions

File tree

‎src/StackExchange.Redis/APITypes/LCSMatchResult.cs‎

Lines changed: 113 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
using System;
2+
using System.ComponentModel;
23

4+
// ReSharper disable once CheckNamespace
35
namespace StackExchange.Redis;
46

57
/// <summary>
68
/// The result of a LongestCommonSubsequence command with IDX feature.
79
/// Returns a list of the positions of each sub-match.
810
/// </summary>
11+
// ReSharper disable once InconsistentNaming
912
public readonly struct LCSMatchResult
1013
{
1114
internal static LCSMatchResult Null { get; } = new LCSMatchResult(Array.Empty<LCSMatch>(), 0);
@@ -36,20 +39,92 @@ internal LCSMatchResult(LCSMatch[] matches, long matchLength)
3639
LongestMatchLength = matchLength;
3740
}
3841

42+
/// <summary>
43+
/// Represents a position range in a string.
44+
/// </summary>
45+
// ReSharper disable once InconsistentNaming
46+
public readonly struct LCSPosition : IEquatable<LCSPosition>
47+
{
48+
/// <summary>
49+
/// The start index of the position.
50+
/// </summary>
51+
public long Start { get; }
52+
53+
/// <summary>
54+
/// The end index of the position.
55+
/// </summary>
56+
public long End { get; }
57+
58+
/// <summary>
59+
/// Returns a new Position.
60+
/// </summary>
61+
/// <param name="start">The start index.</param>
62+
/// <param name="end">The end index.</param>
63+
public LCSPosition(long start, long end)
64+
{
65+
Start = start;
66+
End = end;
67+
}
68+
69+
/// <inheritdoc/>
70+
public override string ToString() => $"[{Start}..{End}]";
71+
72+
/// <inheritdoc/>
73+
public override int GetHashCode()
74+
{
75+
unchecked
76+
{
77+
return ((int)Start * 31) + (int)End;
78+
}
79+
}
80+
81+
/// <inheritdoc/>
82+
public override bool Equals(object? obj) => obj is LCSPosition other && Equals(in other);
83+
84+
/// <summary>
85+
/// Compares this position to another for equality.
86+
/// </summary>
87+
[CLSCompliant(false)]
88+
public bool Equals(in LCSPosition other) => Start == other.Start && End == other.End;
89+
90+
/// <summary>
91+
/// Compares this position to another for equality.
92+
/// </summary>
93+
bool IEquatable<LCSPosition>.Equals(LCSPosition other) => Equals(in other);
94+
}
95+
3996
/// <summary>
4097
/// Represents a sub-match of the longest match. i.e first indexes the matched substring in each string.
4198
/// </summary>
42-
public readonly struct LCSMatch
99+
// ReSharper disable once InconsistentNaming
100+
public readonly struct LCSMatch : IEquatable<LCSMatch>
43101
{
102+
private readonly LCSPosition _first;
103+
private readonly LCSPosition _second;
104+
105+
/// <summary>
106+
/// The position of the matched substring in the first string.
107+
/// </summary>
108+
public LCSPosition First => _first;
109+
110+
/// <summary>
111+
/// The position of the matched substring in the second string.
112+
/// </summary>
113+
public LCSPosition Second => _second;
114+
44115
/// <summary>
45116
/// The first index of the matched substring in the first string.
46117
/// </summary>
47-
public long FirstStringIndex { get; }
118+
[EditorBrowsable(EditorBrowsableState.Never)]
119+
[Browsable(false)]
120+
public long FirstStringIndex => _first.Start;
48121

49122
/// <summary>
50123
/// The first index of the matched substring in the second string.
51124
/// </summary>
52-
public long SecondStringIndex { get; }
125+
[EditorBrowsable(EditorBrowsableState.Never)]
126+
[Browsable(false)]
127+
public long SecondStringIndex => _second.Start;
53128

54129
/// <summary>
55130
/// The length of the match.
@@ -59,14 +134,44 @@ public readonly struct LCSMatch
59134
/// <summary>
60135
/// Returns a new Match.
61136
/// </summary>
62-
/// <param name="firstStringIndex">The first index of the matched substring in the first string.</param>
63-
/// <param name="secondStringIndex">The first index of the matched substring in the second string.</param>
137+
/// <param name="first">The position of the matched substring in the first string.</param>
138+
/// <param name="second">The position of the matched substring in the second string.</param>
64139
/// <param name="length">The length of the match.</param>
65-
internal LCSMatch(long firstStringIndex, long secondStringIndex, long length)
140+
internal LCSMatch(LCSPosition first, LCSPosition second, long length)
66141
{
67-
FirstStringIndex = firstStringIndex;
68-
SecondStringIndex = secondStringIndex;
142+
_first = first;
143+
_second = second;
69144
Length = length;
70145
}
146+
147+
/// <inheritdoc/>
148+
public override string ToString() => $"First: {_first}, Second: {_second}, Length: {Length}";
149+
150+
/// <inheritdoc/>
151+
public override int GetHashCode()
152+
{
153+
unchecked
154+
{
155+
int hash = 17;
156+
hash = (hash * 31) + _first.GetHashCode();
157+
hash = (hash * 31) + _second.GetHashCode();
158+
hash = (hash * 31) + Length.GetHashCode();
159+
return hash;
160+
}
161+
}
162+
163+
/// <inheritdoc/>
164+
public override bool Equals(object? obj) => obj is LCSMatch other && Equals(in other);
165+
166+
/// <summary>
167+
/// Compares this match to another for equality.
168+
/// </summary>
169+
[CLSCompliant(false)]
170+
public bool Equals(in LCSMatch other) => _first.Equals(in other._first) && _second.Equals(in other._second) && Length == other.Length;
171+
172+
/// <summary>
173+
/// Compares this match to another for equality.
174+
/// </summary>
175+
bool IEquatable<LCSMatch>.Equals(LCSMatch other) => Equals(in other);
71176
}
72177
}

‎src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,17 @@
11
#nullable enable
2+
override StackExchange.Redis.LCSMatchResult.LCSMatch.Equals(object? obj) -> bool
3+
override StackExchange.Redis.LCSMatchResult.LCSMatch.GetHashCode() -> int
4+
override StackExchange.Redis.LCSMatchResult.LCSMatch.ToString() -> string!
5+
override StackExchange.Redis.LCSMatchResult.LCSPosition.Equals(object? obj) -> bool
6+
override StackExchange.Redis.LCSMatchResult.LCSPosition.GetHashCode() -> int
7+
override StackExchange.Redis.LCSMatchResult.LCSPosition.ToString() -> string!
8+
StackExchange.Redis.LCSMatchResult.LCSMatch.Equals(in StackExchange.Redis.LCSMatchResult.LCSMatch other) -> bool
9+
StackExchange.Redis.LCSMatchResult.LCSMatch.First.get -> StackExchange.Redis.LCSMatchResult.LCSPosition
10+
StackExchange.Redis.LCSMatchResult.LCSMatch.Second.get -> StackExchange.Redis.LCSMatchResult.LCSPosition
11+
StackExchange.Redis.LCSMatchResult.LCSPosition
12+
StackExchange.Redis.LCSMatchResult.LCSPosition.End.get -> long
13+
StackExchange.Redis.LCSMatchResult.LCSPosition.Equals(in StackExchange.Redis.LCSMatchResult.LCSPosition other) -> bool
14+
StackExchange.Redis.LCSMatchResult.LCSPosition.LCSPosition() -> void
15+
StackExchange.Redis.LCSMatchResult.LCSPosition.LCSPosition(long start, long end) -> void
16+
StackExchange.Redis.LCSMatchResult.LCSPosition.Start.get -> long
217
StackExchange.Redis.RedisType.VectorSet = 8 -> StackExchange.Redis.RedisType

‎src/StackExchange.Redis/ResultProcessor.cs‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2098,34 +2098,47 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
20982098
// Read the matches array
20992099
if (iter.Value.IsAggregate)
21002100
{
2101-
var matchCount = iter.Value.AggregateLength();
2102-
matchesArray = new LCSMatchResult.LCSMatch[matchCount];
2103-
var matchIter = iter.Value.AggregateChildren();
2104-
for (int i = 0; i < matchCount; i++)
2101+
bool failed = false;
2102+
matchesArray = iter.Value.ReadPastArray(ref failed, static (ref failed, ref reader) =>
21052103
{
2106-
if (!matchIter.MoveNext() || !matchIter.Value.IsAggregate) break;
2104+
// Don't even bother if we've already failed
2105+
if (failed) return default;
21072106

2108-
var matchReader = matchIter.Value;
2109-
var matchChildren = matchReader.AggregateChildren();
2107+
if (!reader.IsAggregate)
2108+
{
2109+
failed = true;
2110+
return default;
2111+
}
2112+
2113+
var matchChildren = reader.AggregateChildren();
21102114

2111-
// First range (2-element array)
2112-
if (!matchChildren.MoveNext() || !matchChildren.Value.IsAggregate) break;
2113-
var firstRangeIter = matchChildren.Value.AggregateChildren();
2114-
if (!firstRangeIter.MoveNext() || !firstRangeIter.Value.IsScalar) break;
2115-
var firstStringIndex = firstRangeIter.Value.TryReadInt64(out var first) ? first : 0;
2115+
// First range (2-element array: [start, end])
2116+
if (!(matchChildren.MoveNext() && TryReadPosition(ref matchChildren.Value, out var firstPos)))
2117+
{
2118+
failed = true;
2119+
return default;
2120+
}
21162121

2117-
// Second range (2-element array)
2118-
if (!matchChildren.MoveNext() || !matchChildren.Value.IsAggregate) break;
2119-
var secondRangeIter = matchChildren.Value.AggregateChildren();
2120-
if (!secondRangeIter.MoveNext() || !secondRangeIter.Value.IsScalar) break;
2121-
var secondStringIndex = secondRangeIter.Value.TryReadInt64(out var second) ? second : 0;
2122+
// Second range (2-element array: [start, end])
2123+
if (!(matchChildren.MoveNext() && TryReadPosition(ref matchChildren.Value, out var secondPos)))
2124+
{
2125+
failed = true;
2126+
return default;
2127+
}
21222128

21232129
// Length
2124-
if (!matchChildren.MoveNext() || !matchChildren.Value.IsScalar) break;
2130+
if (!(matchChildren.MoveNext() && matchChildren.Value.IsScalar))
2131+
{
2132+
failed = true;
2133+
return default;
2134+
}
21252135
var length = matchChildren.Value.TryReadInt64(out var matchLen) ? matchLen : 0;
21262136

2127-
matchesArray[i] = new LCSMatchResult.LCSMatch(firstStringIndex, secondStringIndex, length);
2128-
}
2137+
return new LCSMatchResult.LCSMatch(firstPos, secondPos, length);
2138+
});
2139+
2140+
// Check if anything went wrong
2141+
if (failed) matchesArray = null;
21292142
}
21302143
break;
21312144

@@ -2147,6 +2160,20 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
21472160
}
21482161
return false;
21492162
}
2163+
2164+
private static bool TryReadPosition(ref RespReader reader, out LCSMatchResult.LCSPosition position)
2165+
{
2166+
// Expecting a 2-element array: [start, end]
2167+
position = default;
2168+
if (!reader.IsAggregate) return false;
2169+
2170+
if (!(reader.TryMoveNext() && reader.IsScalar && reader.TryReadInt64(out var start))) return false;
2171+
2172+
if (!(reader.TryMoveNext() && reader.IsScalar && reader.TryReadInt64(out var end))) return false;
2173+
2174+
position = new LCSMatchResult.LCSPosition(start, end);
2175+
return true;
2176+
}
21502177
}
21512178

21522179
private sealed class RedisValueProcessor : ResultProcessor<RedisValue>

‎tests/StackExchange.Redis.Tests/ResultProcessorUnitTests/AutoConfigure.cs‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
using Xunit;
2+
3+
namespace StackExchange.Redis.Tests.ResultProcessorUnitTests;
4+
5+
public class AutoConfigure(ITestOutputHelper log) : ResultProcessorUnitTest(log)
6+
{
7+
[Fact]
8+
public void ClientId_Integer_Success()
9+
{
10+
// CLIENT ID response
11+
var resp = ":11\r\n";
12+
var message = Message.Create(-1, default, RedisCommand.CLIENT);
13+
14+
// Note: This will return false because we don't have a real connection with a server endpoint
15+
// The processor will throw because it can't set the connection ID without a real connection
16+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
17+
18+
Assert.False(success);
19+
Assert.NotNull(exception);
20+
Assert.IsType<RedisConnectionException>(exception);
21+
}
22+
23+
[Fact]
24+
public void Info_BulkString_Success()
25+
{
26+
// INFO response with replication info
27+
var info = "# Replication\r\n" +
28+
"role:master\r\n" +
29+
"connected_slaves:0\r\n" +
30+
"master_failover_state:no-failover\r\n" +
31+
"master_replid:8c3e3c3e3c3e3c3e3c3e3c3e3c3e3c3e3c3e3c3e\r\n" +
32+
"master_replid2:0000000000000000000000000000000000000000\r\n" +
33+
"master_repl_offset:0\r\n" +
34+
"second_repl_offset:-1\r\n" +
35+
"repl_backlog_active:0\r\n" +
36+
"repl_backlog_size:1048576\r\n" +
37+
"repl_backlog_first_byte_offset:0\r\n" +
38+
"repl_backlog_histlen:0\r\n";
39+
40+
var resp = $"${info.Length}\r\n{info}\r\n";
41+
var message = Message.Create(-1, default, RedisCommand.INFO);
42+
43+
// Note: This will return false because we don't have a real connection with a server endpoint
44+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
45+
46+
Assert.False(success);
47+
Assert.NotNull(exception);
48+
Assert.IsType<RedisConnectionException>(exception);
49+
}
50+
51+
[Fact]
52+
public void Info_WithVersion_Success()
53+
{
54+
// INFO response with version info
55+
var info = "# Server\r\n" +
56+
"redis_version:7.2.4\r\n" +
57+
"redis_git_sha1:00000000\r\n" +
58+
"redis_mode:standalone\r\n" +
59+
"os:Linux 5.15.0-1-amd64 x86_64\r\n" +
60+
"arch_bits:64\r\n";
61+
62+
var resp = $"${info.Length}\r\n{info}\r\n";
63+
var message = Message.Create(-1, default, RedisCommand.INFO);
64+
65+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
66+
67+
Assert.False(success);
68+
Assert.NotNull(exception);
69+
Assert.IsType<RedisConnectionException>(exception);
70+
}
71+
72+
[Fact]
73+
public void Info_EmptyString_Success()
74+
{
75+
// Empty INFO response
76+
var resp = "$0\r\n\r\n";
77+
var message = Message.Create(-1, default, RedisCommand.INFO);
78+
79+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
80+
81+
Assert.False(success);
82+
Assert.NotNull(exception);
83+
Assert.IsType<RedisConnectionException>(exception);
84+
}
85+
86+
[Fact]
87+
public void Info_Null_Success()
88+
{
89+
// Null INFO response
90+
var resp = "$-1\r\n";
91+
var message = Message.Create(-1, default, RedisCommand.INFO);
92+
93+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
94+
95+
Assert.False(success);
96+
Assert.NotNull(exception);
97+
Assert.IsType<RedisConnectionException>(exception);
98+
}
99+
100+
[Fact]
101+
public void Config_Array_Success()
102+
{
103+
// CONFIG GET timeout response
104+
var resp = "*2\r\n" +
105+
"$7\r\ntimeout\r\n" +
106+
"$3\r\n300\r\n";
107+
var message = Message.Create(-1, default, RedisCommand.CONFIG);
108+
109+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
110+
111+
Assert.False(success);
112+
Assert.NotNull(exception);
113+
Assert.IsType<RedisConnectionException>(exception);
114+
}
115+
116+
[Fact]
117+
public void ReadonlyError_Success()
118+
{
119+
// READONLY error response
120+
var resp = "-READONLY You can't write against a read only replica.\r\n";
121+
var message = DummyMessage();
122+
123+
var success = TryExecute(resp, ResultProcessor.AutoConfigure, out bool result, out var exception, message);
124+
125+
// Should handle the error - returns RedisServerException for error responses
126+
Assert.False(success);
127+
Assert.NotNull(exception);
128+
Assert.IsType<RedisServerException>(exception);
129+
}
130+
}

0 commit comments

Comments
 (0)