Skip to content

Commit 3b1473b

Browse files
alexr-bqalexr-bq
committed
Resolve PR feedback
Signed-off-by: Alex Rehnby-Martin <[email protected]>
1 parent 0cfda7a commit 3b1473b

File tree

6 files changed

+69
-12
lines changed

6 files changed

+69
-12
lines changed

‎.github/workflows/tests.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ jobs:
8282
tests:
8383
name: net${{ matrix.dotnet }}, server ${{ matrix.server.version }}, ${{ matrix.host.TARGET }}
8484
needs: get-matrices
85-
timeout-minutes: 200
85+
timeout-minutes: 100
8686
strategy:
8787
fail-fast: false
8888
matrix:

‎sources/Valkey.Glide/Commands/IBitmapCommands.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public interface IBitmapCommands
3636
/// <param name="offset">The offset in the string to set the bit at.</param>
3737
/// <param name="value">The bit value to set (true for 1, false for 0).</param>
3838
/// <param name="flags">The flags to use for this operation. Currently flags are ignored.</param>
39-
/// <returns>The original bit value stored at offset.</returns>
39+
/// <returns>The original bit value stored at offset. Returns false if the key does not exist or if the offset is beyond the string length.</returns>
4040
/// <remarks>
4141
/// <example>
4242
/// <code>
@@ -141,7 +141,7 @@ public interface IBitmapCommands
141141
/// <param name="key">The key of the string.</param>
142142
/// <param name="subCommands">The subcommands to execute (GET, SET, INCRBY).</param>
143143
/// <param name="flags">The flags to use for this operation. Currently flags are ignored.</param>
144-
/// <returns>An array of results from the executed subcommands.</returns>
144+
/// <returns>An array of results from the executed subcommands. Null responses from the server are converted to 0.</returns>
145145
/// <remarks>
146146
/// <example>
147147
/// <code>
@@ -166,7 +166,7 @@ public interface IBitmapCommands
166166
/// <param name="key">The key of the string.</param>
167167
/// <param name="subCommands">The GET subcommands to execute.</param>
168168
/// <param name="flags">The flags to use for this operation. Currently flags are ignored.</param>
169-
/// <returns>An array of results from the executed GET subcommands.</returns>
169+
/// <returns>An array of results from the executed GET subcommands. Null responses from the server are converted to 0.</returns>
170170
/// <remarks>
171171
/// <example>
172172
/// <code>

‎sources/Valkey.Glide/GlideString.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ public static GlideString ToGlideString(this double @double)
4545
: double.IsNaN(@double) ? new("nan")
4646
: new(@double.ToString("G17", System.Globalization.CultureInfo.InvariantCulture));
4747

48+
/// <summary>
49+
/// Convert a <paramref name="bool"/> to a <see cref="GlideString" /> ("1" for true, "0" for false).
50+
/// </summary>
51+
/// <param name="bool">A <see langword="bool" /> to convert.</param>
52+
/// <returns>A <see cref="GlideString" />.</returns>
53+
public static GlideString ToGlideString(this bool @bool) => new(@bool ? "1" : "0");
54+
4855
/// <summary>
4956
/// Convert a <paramref name="bytes"/> to a <see cref="GlideString" />.
5057
/// </summary>

‎sources/Valkey.Glide/Internals/Request.BitmapCommands.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ public static Cmd<long, bool> GetBitAsync(ValkeyKey key, long offset)
1212
=> new(RequestType.GetBit, [key.ToGlideString(), offset.ToGlideString()], false, response => response != 0);
1313

1414
public static Cmd<long, bool> SetBitAsync(ValkeyKey key, long offset, bool value)
15-
=> new(RequestType.SetBit, [key.ToGlideString(), offset.ToGlideString(), (value ? 1 : 0).ToGlideString()], false, response => response != 0);
15+
=> new(RequestType.SetBit, [key.ToGlideString(), offset.ToGlideString(), value.ToGlideString()], false, response => response != 0);
1616

17-
public static Cmd<long, long> BitCountAsync(ValkeyKey key, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte)
17+
public static Cmd<long, long> BitCountAsync(ValkeyKey key, long start, long end, StringIndexType indexType)
1818
{
1919
List<GlideString> args = [key.ToGlideString(), start.ToGlideString(), end.ToGlideString()];
2020
if (indexType != StringIndexType.Byte)
@@ -24,9 +24,9 @@ public static Cmd<long, long> BitCountAsync(ValkeyKey key, long start = 0, long
2424
return Simple<long>(RequestType.BitCount, [.. args]);
2525
}
2626

27-
public static Cmd<long, long> BitPositionAsync(ValkeyKey key, bool bit, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte)
27+
public static Cmd<long, long> BitPositionAsync(ValkeyKey key, bool bit, long start, long end, StringIndexType indexType)
2828
{
29-
List<GlideString> args = [key.ToGlideString(), (bit ? 1 : 0).ToGlideString(), start.ToGlideString(), end.ToGlideString()];
29+
List<GlideString> args = [key.ToGlideString(), bit.ToGlideString(), start.ToGlideString(), end.ToGlideString()];
3030
if (indexType != StringIndexType.Byte)
3131
{
3232
args.Add(indexType.ToLiteral().ToGlideString());

‎tests/Valkey.Glide.IntegrationTests/BitmapCommandTests.cs‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
using Valkey.Glide.Commands.Options;
44

5+
using static Valkey.Glide.Errors;
6+
57
namespace Valkey.Glide.IntegrationTests;
68

79
public class BitmapCommandTests(TestConfiguration config)
@@ -52,6 +54,19 @@ public async Task GetBit_OffsetBeyondString_ReturnsFalse(BaseClient client)
5254
Assert.False(bit);
5355
}
5456

57+
[Theory(DisableDiscoveryEnumeration = true)]
58+
[MemberData(nameof(Config.TestClients), MemberType = typeof(TestConfiguration))]
59+
public async Task GetBit_NegativeOffset_ThrowsException(BaseClient client)
60+
{
61+
string key = Guid.NewGuid().ToString();
62+
63+
// Set a string
64+
await client.StringSetAsync(key, "A");
65+
66+
// Test negative offset - should throw an exception
67+
await Assert.ThrowsAsync<RequestException>(async () => await client.StringGetBitAsync(key, -1));
68+
}
69+
5570
[Theory(DisableDiscoveryEnumeration = true)]
5671
[MemberData(nameof(Config.TestClients), MemberType = typeof(TestConfiguration))]
5772
public async Task SetBit_SetsAndReturnsOriginalValue(BaseClient client)
@@ -90,6 +105,16 @@ public async Task SetBit_NonExistentKey_CreatesKey(BaseClient client)
90105
Assert.True(currentBit);
91106
}
92107

108+
[Theory(DisableDiscoveryEnumeration = true)]
109+
[MemberData(nameof(Config.TestClients), MemberType = typeof(TestConfiguration))]
110+
public async Task SetBit_NegativeOffset_ThrowsException(BaseClient client)
111+
{
112+
string key = Guid.NewGuid().ToString();
113+
114+
// Test negative offset - should throw an exception
115+
await Assert.ThrowsAsync<RequestException>(async () => await client.StringSetBitAsync(key, -1, true));
116+
}
117+
93118
[Theory(DisableDiscoveryEnumeration = true)]
94119
[MemberData(nameof(Config.TestClients), MemberType = typeof(TestConfiguration))]
95120
public async Task GetSetBit_CombinedOperations_WorksTogether(BaseClient client)
@@ -493,30 +518,29 @@ public async Task BitField_OffsetMultiplier_WorksCorrectly(BaseClient client)
493518

494519
[Theory(DisableDiscoveryEnumeration = true)]
495520
[MemberData(nameof(Config.TestClients), MemberType = typeof(TestConfiguration))]
496-
public async Task BitField_AutoOptimization_UsesReadOnlyForGetOperations(BaseClient client)
521+
public async Task BitField_WithReadOnlyAndMixedOperations_WorksCorrectly(BaseClient client)
497522
{
498523
string key = Guid.NewGuid().ToString();
499524

500525
// Set initial value "A" (ASCII 65 = 01000001)
501526
await client.StringSetAsync(key, "A");
502527

503-
// Test with only GET operations - should automatically use BITFIELD_RO
528+
// Test with only GET operations
504529
var readOnlySubCommands = new BitFieldOptions.IBitFieldSubCommand[]
505530
{
506531
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(8), new BitFieldOptions.BitOffset(0)),
507532
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(4), new BitFieldOptions.BitOffset(0)),
508533
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(4), new BitFieldOptions.BitOffset(4))
509534
};
510535

511-
// This should internally use BITFIELD_RO since all commands are GET
512536
long[] results = await client.StringBitFieldAsync(key, readOnlySubCommands);
513537

514538
Assert.Equal(3, results.Length);
515539
Assert.Equal(65, results[0]); // Full 8 bits: 01000001 = 65
516540
Assert.Equal(4, results[1]); // First 4 bits: 0100 = 4
517541
Assert.Equal(1, results[2]); // Next 4 bits: 0001 = 1
518542

519-
// Test with mixed operations - should use regular BITFIELD
543+
// Test with mixed operations
520544
var mixedSubCommands = new BitFieldOptions.IBitFieldSubCommand[]
521545
{
522546
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(8), new BitFieldOptions.BitOffset(0)),

‎tests/Valkey.Glide.UnitTests/CommandTests.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,32 @@ public void ValidateCommandConverters()
660660
);
661661
}
662662

663+
[Fact]
664+
public void BitField_AutoOptimization_UsesCorrectRequestType()
665+
{
666+
// Test that read-only subcommands use BitFieldReadOnlyAsync
667+
var readOnlySubCommands = new BitFieldOptions.IBitFieldSubCommand[]
668+
{
669+
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(8), new BitFieldOptions.BitOffset(0)),
670+
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(4), new BitFieldOptions.BitOffset(0))
671+
};
672+
673+
// Verify that all subcommands are read-only
674+
bool allReadOnly = readOnlySubCommands.All(cmd => cmd is BitFieldOptions.IBitFieldReadOnlySubCommand);
675+
Assert.True(allReadOnly);
676+
677+
// Test that mixed subcommands don't qualify for read-only optimization
678+
var mixedSubCommands = new BitFieldOptions.IBitFieldSubCommand[]
679+
{
680+
new BitFieldOptions.BitFieldGet(BitFieldOptions.Encoding.Unsigned(8), new BitFieldOptions.BitOffset(0)),
681+
new BitFieldOptions.BitFieldSet(BitFieldOptions.Encoding.Unsigned(8), new BitFieldOptions.BitOffset(0), 100)
682+
};
683+
684+
// Verify that mixed subcommands are not all read-only
685+
bool mixedAllReadOnly = mixedSubCommands.All(cmd => cmd is BitFieldOptions.IBitFieldReadOnlySubCommand);
686+
Assert.False(mixedAllReadOnly);
687+
}
688+
663689
[Fact]
664690
public void ValidateStringCommandArrayConverters()
665691
{

0 commit comments

Comments
 (0)