From 8d16cd84684fb2538cc1391122cb9f523276a651 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 13:44:43 +0100 Subject: [PATCH 1/7] Fill in the neglected bitmap commands: BITFIELD, and an open-ended BITPOS Two gaps in the bitmap family (https://redis.io/docs/latest/commands/?group=bitmap). BITFIELD and BITFIELD_RO were entirely unimplemented - the only trace in the repo was a commented-out line in the envoy exclusion list. Added as: long? StringBitField(key, BitFieldOperation, flags) Lease? StringBitField(key, ReadOnlyMemory, flags) plus BitFieldEncoding (Signed/Unsigned, and named Int8..Int64/UInt8..UInt63), BitFieldOffset (Bits/Index, the latter being the server's #N form), BitFieldOverflow (Wrap/Saturate/Fail) and BitFieldOperation (Get/Set/ IncrementBy). Signedness lives with the width rather than in a flags enum, and #N lives on the offset, so no flag silently reinterprets another argument. Every legal encoding (i1-i64, u1-u63) fits losslessly in a signed 64-bit integer, which is exactly why u64 does not exist, so the results are plain long - nullable only because OVERFLOW FAIL reports a nil for the operation it skipped. There is deliberately a named UInt63 where UInt64 would sit, to document the ceiling at the point someone reaches for it. The OVERFLOW token is sticky server-side and untouched by GET, so it is emitted only when the required mode changes, and never for a leading run of WRAP (the server default). An all-GET batch is issued as BITFIELD_RO where the server supports it (6.0+) so a replica can serve it; anything that writes stays as BITFIELD, which is primary-only. An empty batch short-circuits locally. Retry category follows the payload: accumulating by default, demoted to last-wins when there is no INCRBY. Separately, StringBitPosition could not express an open-ended range. It always sent an explicit end, and for a clear-bit search the server distinguishes the two: with an explicit end an all-set string yields -1, whereas with no end the result is the first bit past the end of the string. Passing StringIndex.Unbounded as the end now omits it; that requires StringIndexType .Byte, since BYTE/BIT is only legal after an explicit end, and silently dropping the token would reinterpret start from bits to bytes. Implemented as a sentinel rather than an overload because a no-end overload would be CS0121-ambiguous with the existing one at every call site. --- src/StackExchange.Redis/BitFieldEncoding.cs | 130 +++++++++++++++ src/StackExchange.Redis/BitFieldOffset.cs | 69 ++++++++ src/StackExchange.Redis/BitFieldOperation.cs | 122 ++++++++++++++ .../Enums/BitFieldOverflow.cs | 24 +++ .../Enums/CommandFlags.Category.cs | 4 +- src/StackExchange.Redis/Enums/RedisCommand.cs | 6 + .../Interfaces/IDatabase.cs | 54 +++++- .../Interfaces/IDatabaseAsync.cs | 8 + .../KeyspaceIsolation/KeyPrefixed.cs | 6 + .../KeyspaceIsolation/KeyPrefixedDatabase.cs | 6 + .../PublicAPI/PublicAPI.Unshipped.txt | 53 ++++++ src/StackExchange.Redis/RedisDatabase.cs | 156 ++++++++++++++++-- src/StackExchange.Redis/RedisFeatures.cs | 5 + src/StackExchange.Redis/RedisLiterals.cs | 13 ++ .../ResultProcessor.Lease.cs | 8 + src/StackExchange.Redis/StringIndex.cs | 24 +++ .../BitFieldTypeTests.cs | 107 ++++++++++++ tests/StackExchange.Redis.Tests/BitTests.cs | 135 ++++++++++++++- .../RoundTripUnitTests/BitFieldRoundTrip.cs | 120 ++++++++++++++ .../BitPositionRoundTrip.cs | 60 +++++++ 20 files changed, 1096 insertions(+), 14 deletions(-) create mode 100644 src/StackExchange.Redis/BitFieldEncoding.cs create mode 100644 src/StackExchange.Redis/BitFieldOffset.cs create mode 100644 src/StackExchange.Redis/BitFieldOperation.cs create mode 100644 src/StackExchange.Redis/Enums/BitFieldOverflow.cs create mode 100644 src/StackExchange.Redis/StringIndex.cs create mode 100644 tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs create mode 100644 tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs create mode 100644 tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitPositionRoundTrip.cs diff --git a/src/StackExchange.Redis/BitFieldEncoding.cs b/src/StackExchange.Redis/BitFieldEncoding.cs new file mode 100644 index 0000000000..d30b99d1e7 --- /dev/null +++ b/src/StackExchange.Redis/BitFieldEncoding.cs @@ -0,0 +1,130 @@ +using System; + +namespace StackExchange.Redis; + +/// +/// The width and signedness of a single bitfield, as used by the BITFIELD operations +/// on ; for example u8 or i53. +/// +/// +/// Signed encodings may be 1-64 bits wide; unsigned encodings are limited to 63 bits, because the +/// server reports every value as a signed 64-bit integer. Every legal encoding therefore fits +/// losslessly in . +/// +public readonly struct BitFieldEncoding : IEquatable +{ + // negative: signed; positive: unsigned; zero: default (not a legal encoding) + private readonly sbyte _value; + + private BitFieldEncoding(sbyte value) => _value = value; + + /// A signed bitfield bits wide; 1-64. + /// The width of the field, in bits. + public static BitFieldEncoding Signed(int width) + { + if (width is < 1 or > 64) + { + throw new ArgumentOutOfRangeException(nameof(width), "Signed bitfield encodings must be 1-64 bits wide."); + } + + return new((sbyte)-width); + } + + /// An unsigned bitfield bits wide; 1-63. + /// The width of the field, in bits. + public static BitFieldEncoding Unsigned(int width) + { + if (width is < 1 or > 63) + { + // the ceiling is 63 rather than 64 because every value comes back as a signed 64-bit + // integer, so u64 has nowhere to go; the server rejects it for the same reason + var message = width >= 64 + ? "Unsigned bitfield encodings are limited to 63 bits, because the reply is a signed 64-bit integer; use UInt63 or Int64." + : "Unsigned bitfield encodings must be 1-63 bits wide."; + throw new ArgumentOutOfRangeException(nameof(width), message); + } + + return new((sbyte)width); + } + + /// A signed 8-bit field, i8. + public static BitFieldEncoding Int8 => new(-8); + + /// A signed 16-bit field, i16. + public static BitFieldEncoding Int16 => new(-16); + + /// A signed 32-bit field, i32. + public static BitFieldEncoding Int32 => new(-32); + + /// A signed 64-bit field, i64; the widest field the server supports. + public static BitFieldEncoding Int64 => new(-64); + + /// An unsigned 8-bit field, u8. + public static BitFieldEncoding UInt8 => new(8); + + /// An unsigned 16-bit field, u16. + public static BitFieldEncoding UInt16 => new(16); + + /// An unsigned 32-bit field, u32. + public static BitFieldEncoding UInt32 => new(32); + + /// + /// An unsigned 63-bit field, u63; the widest unsigned field the server supports. There is + /// deliberately no UInt64: the reply is a signed 64-bit integer, so u64 could not be + /// represented, and the server rejects it. + /// + public static BitFieldEncoding UInt63 => new(63); + + /// The width of the field, in bits. + public int Width => _value < 0 ? -_value : _value; + + /// Whether the field is signed. + public bool IsSigned => _value < 0; + + internal bool IsDefault => _value == 0; + + internal RedisValue ToLiteral() + { + if (_value == 0) ThrowDefault(); + return Literals[_value + 64]; + + static void ThrowDefault() => throw new ArgumentException( + $"A {nameof(BitFieldEncoding)} must be created via {nameof(Signed)}, {nameof(Unsigned)}, or one of the named encodings.", + nameof(BitFieldEncoding)); + } + + // i64..i1 (indexes 0..63), unused (64), u1..u63 (65..127) + private static readonly RedisValue[] Literals = CreateLiterals(); + + private static RedisValue[] CreateLiterals() + { + var arr = new RedisValue[128]; + for (int width = 1; width <= 64; width++) + { + arr[64 - width] = "i" + width.ToString(System.Globalization.CultureInfo.InvariantCulture); + if (width <= 63) + { + arr[64 + width] = "u" + width.ToString(System.Globalization.CultureInfo.InvariantCulture); + } + } + return arr; + } + + /// + public override string ToString() => _value == 0 ? "(default)" : (string)ToLiteral()!; + + /// + public bool Equals(BitFieldEncoding other) => _value == other._value; + + /// + public override bool Equals(object? obj) => obj is BitFieldEncoding other && Equals(other); + + /// + public override int GetHashCode() => _value; + + /// Compares two values for equality. + public static bool operator ==(BitFieldEncoding x, BitFieldEncoding y) => x._value == y._value; + + /// Compares two values for non-equality. + public static bool operator !=(BitFieldEncoding x, BitFieldEncoding y) => x._value != y._value; +} diff --git a/src/StackExchange.Redis/BitFieldOffset.cs b/src/StackExchange.Redis/BitFieldOffset.cs new file mode 100644 index 0000000000..4cc3487df4 --- /dev/null +++ b/src/StackExchange.Redis/BitFieldOffset.cs @@ -0,0 +1,69 @@ +using System; +using System.Globalization; + +namespace StackExchange.Redis; + +/// +/// Where a bitfield sits inside a string: either an absolute bit offset, or - via +/// - a zero-based index into an array of consecutive fields of the same +/// width, which the server multiplies out for us (the # form). +/// +/// +public readonly struct BitFieldOffset : IEquatable +{ + private readonly long _value; + private readonly bool _isIndex; + + private BitFieldOffset(long value, bool isIndex) + { + _value = value; + _isIndex = isIndex; + } + + /// + /// An absolute offset, in bits, from the start of the string. + /// + /// The offset, in bits. + public static BitFieldOffset Bits(long offset) => offset < 0 + ? throw new ArgumentOutOfRangeException(nameof(offset), "A bitfield offset cannot be negative.") + : new(offset, false); + + /// + /// A zero-based index into an array of consecutive fields of the encoding's own width; the + /// server multiplies the index by the width, so Index(2) of a u8 is bit 16. + /// + /// The index of the field. + public static BitFieldOffset Index(long index) => index < 0 + ? throw new ArgumentOutOfRangeException(nameof(index), "A bitfield index cannot be negative.") + : new(index, true); + + /// + /// Creates an absolute bit offset; equivalent to . + /// + /// The offset, in bits. + public static implicit operator BitFieldOffset(long offset) => Bits(offset); + + internal RedisValue ToLiteral() => _isIndex + ? (RedisValue)("#" + _value.ToString(CultureInfo.InvariantCulture)) + : (RedisValue)_value; + + /// + public override string ToString() => _isIndex + ? "#" + _value.ToString(CultureInfo.InvariantCulture) + : _value.ToString(CultureInfo.InvariantCulture); + + /// + public bool Equals(BitFieldOffset other) => _value == other._value && _isIndex == other._isIndex; + + /// + public override bool Equals(object? obj) => obj is BitFieldOffset other && Equals(other); + + /// + public override int GetHashCode() => _isIndex ? ~_value.GetHashCode() : _value.GetHashCode(); + + /// Compares two values for equality. + public static bool operator ==(BitFieldOffset x, BitFieldOffset y) => x.Equals(y); + + /// Compares two values for non-equality. + public static bool operator !=(BitFieldOffset x, BitFieldOffset y) => !x.Equals(y); +} diff --git a/src/StackExchange.Redis/BitFieldOperation.cs b/src/StackExchange.Redis/BitFieldOperation.cs new file mode 100644 index 0000000000..13234122be --- /dev/null +++ b/src/StackExchange.Redis/BitFieldOperation.cs @@ -0,0 +1,122 @@ +using System; + +namespace StackExchange.Redis; + +/// +/// A single BITFIELD sub-operation; see , and +/// . +/// +/// +public readonly struct BitFieldOperation : IEquatable +{ + internal enum OperationKind : byte + { + None = 0, + Get, + Set, + IncrementBy, + } + + private readonly BitFieldOffset _offset; + private readonly BitFieldEncoding _encoding; + private readonly long _value; + private readonly BitFieldOverflow _overflow; + private readonly OperationKind _kind; + + private BitFieldOperation(OperationKind kind, BitFieldEncoding encoding, BitFieldOffset offset, long value, BitFieldOverflow overflow) + { + if (encoding.IsDefault) + { + throw new ArgumentException( + $"A {nameof(BitFieldEncoding)} must be created via {nameof(BitFieldEncoding)}.{nameof(BitFieldEncoding.Signed)}, {nameof(BitFieldEncoding.Unsigned)}, or one of the named encodings.", + nameof(encoding)); + } + if (overflow is < BitFieldOverflow.Wrap or > BitFieldOverflow.Fail) + { + throw new ArgumentOutOfRangeException(nameof(overflow)); + } + + _kind = kind; + _encoding = encoding; + _offset = offset; + _value = value; + _overflow = overflow; + } + + /// + /// Reads the field at . A field that lies beyond the end of the string + /// reads as zero, and no applies. + /// + /// The width and signedness of the field. + /// The location of the field. + public static BitFieldOperation Get(BitFieldEncoding encoding, BitFieldOffset offset) => + new(OperationKind.Get, encoding, offset, 0, BitFieldOverflow.Wrap); + + /// + /// Writes to the field at , returning the + /// previous value; the string is zero-extended as needed. + /// + /// The width and signedness of the field. + /// The location of the field. + /// The value to write. + /// How to handle a value that does not fit the encoding. + public static BitFieldOperation Set(BitFieldEncoding encoding, BitFieldOffset offset, long value, BitFieldOverflow overflow = BitFieldOverflow.Wrap) => + new(OperationKind.Set, encoding, offset, value, overflow); + + /// + /// Increments the field at by (which may be + /// negative), returning the new value; the string is zero-extended as needed. + /// + /// The width and signedness of the field. + /// The location of the field. + /// The amount to increment by. + /// How to handle an increment that does not fit the encoding. + public static BitFieldOperation IncrementBy(BitFieldEncoding encoding, BitFieldOffset offset, long value, BitFieldOverflow overflow = BitFieldOverflow.Wrap) => + new(OperationKind.IncrementBy, encoding, offset, value, overflow); + + internal OperationKind Kind => _kind; + + internal BitFieldEncoding Encoding => _encoding; + + internal BitFieldOffset Offset => _offset; + + internal long Value => _value; + + /// + /// The overflow behavior of this operation; for a + /// , which cannot overflow. + /// + internal BitFieldOverflow Overflow => _overflow; + + /// + public override string ToString() => _kind switch + { + OperationKind.Get => $"GET {_encoding} {_offset}", + OperationKind.Set => $"SET {_encoding} {_offset} {_value} ({_overflow})", + OperationKind.IncrementBy => $"INCRBY {_encoding} {_offset} {_value} ({_overflow})", + _ => "(default)", + }; + + /// + public bool Equals(BitFieldOperation other) => + _kind == other._kind && _encoding == other._encoding && _offset == other._offset + && _value == other._value && _overflow == other._overflow; + + /// + public override bool Equals(object? obj) => obj is BitFieldOperation other && Equals(other); + + /// + public override int GetHashCode() + { + var hash = ((int)_kind * 397) ^ ((int)_overflow * 31); + hash = (hash * 397) ^ _encoding.GetHashCode(); + hash = (hash * 397) ^ _offset.GetHashCode(); + return (hash * 397) ^ _value.GetHashCode(); + } + + /// Compares two values for equality. + public static bool operator ==(BitFieldOperation x, BitFieldOperation y) => x.Equals(y); + + /// Compares two values for non-equality. + public static bool operator !=(BitFieldOperation x, BitFieldOperation y) => !x.Equals(y); +} diff --git a/src/StackExchange.Redis/Enums/BitFieldOverflow.cs b/src/StackExchange.Redis/Enums/BitFieldOverflow.cs new file mode 100644 index 0000000000..35a149e134 --- /dev/null +++ b/src/StackExchange.Redis/Enums/BitFieldOverflow.cs @@ -0,0 +1,24 @@ +namespace StackExchange.Redis; + +/// +/// How BITFIELD should behave when a SET or INCRBY operation would exceed the +/// range of the target . +/// +/// +public enum BitFieldOverflow +{ + /// + /// Wrap around, using the usual two's-complement arithmetic; this is the server default. + /// + Wrap, + + /// + /// Saturate at the minimum or maximum value of the encoding. + /// + Saturate, + + /// + /// Perform no operation at all, reporting no value for the failed operation. + /// + Fail, +} diff --git a/src/StackExchange.Redis/Enums/CommandFlags.Category.cs b/src/StackExchange.Redis/Enums/CommandFlags.Category.cs index 7f5e268ae7..d13417492c 100644 --- a/src/StackExchange.Redis/Enums/CommandFlags.Category.cs +++ b/src/StackExchange.Redis/Enums/CommandFlags.Category.cs @@ -1,4 +1,4 @@ -namespace StackExchange.Redis; +namespace StackExchange.Redis; internal static class CommandFlagsExtensions { @@ -159,6 +159,7 @@ static CommandFlags DefaultCategory(RedisCommand command) case RedisCommand.ZUNION: case RedisCommand.ZINTERCARD: case RedisCommand.BITCOUNT: + case RedisCommand.BITFIELD_RO: case RedisCommand.BITPOS: case RedisCommand.GETBIT: case RedisCommand.PFCOUNT: @@ -287,6 +288,7 @@ static CommandFlags DefaultCategory(RedisCommand command) case RedisCommand.ZMPOP: case RedisCommand.XADD: // explicit ids and IDMP/IDMPAUTO are demoted where we can see the args case RedisCommand.PFMERGE: // destination accumulates union each call + case RedisCommand.BITFIELD: // INCRBY sub-ops compound; demoted to last-wins when the payload has none return CommandFlags.CommandRetryWriteAccumulating; // ========================================================================== diff --git a/src/StackExchange.Redis/Enums/RedisCommand.cs b/src/StackExchange.Redis/Enums/RedisCommand.cs index d3e3d44a93..9f1eaa1931 100644 --- a/src/StackExchange.Redis/Enums/RedisCommand.cs +++ b/src/StackExchange.Redis/Enums/RedisCommand.cs @@ -35,6 +35,10 @@ internal enum RedisCommand BGREWRITEAOF, BGSAVE, BITCOUNT, + [AsciiHash("BITFIELD")] + BITFIELD, + [AsciiHash("BITFIELD_RO")] + BITFIELD_RO, BITOP, BITPOS, BLPOP, @@ -346,6 +350,7 @@ internal static bool IsPrimaryOnly(this RedisCommand command) case RedisCommand.ARRING: case RedisCommand.ARSEEK: case RedisCommand.ARSET: + case RedisCommand.BITFIELD: case RedisCommand.BITOP: case RedisCommand.BLPOP: case RedisCommand.BRPOP: @@ -462,6 +467,7 @@ internal static bool IsPrimaryOnly(this RedisCommand command) case RedisCommand.BGREWRITEAOF: case RedisCommand.BGSAVE: case RedisCommand.BITCOUNT: + case RedisCommand.BITFIELD_RO: case RedisCommand.BITPOS: case RedisCommand.CLIENT: case RedisCommand.CLUSTER: diff --git a/src/StackExchange.Redis/Interfaces/IDatabase.cs b/src/StackExchange.Redis/Interfaces/IDatabase.cs index 141ed8bf42..c567653309 100644 --- a/src/StackExchange.Redis/Interfaces/IDatabase.cs +++ b/src/StackExchange.Redis/Interfaces/IDatabase.cs @@ -3351,6 +3351,46 @@ IEnumerable SortedSetScan( /// long StringBitCount(RedisKey key, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None); + /// + /// Performs a single arbitrary-width integer operation on the string at , + /// treating it as an array of bitfields. + /// + /// The key of the string. + /// The operation to perform. + /// The flags to use for this operation. + /// + /// The value read (for ), the previous value (for + /// ) or the new value (for + /// ); if the operation was + /// skipped because of . + /// + /// +#pragma warning disable RS0026 // competing overloads - disambiguated via parameter types + long? StringBitField(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None); + + /// + /// Performs a batch of arbitrary-width integer operations on the string at , + /// treating it as an array of bitfields. The operations are applied in order, and the result has + /// one element per operation. + /// + /// The key of the string. + /// The operations to perform; an empty batch is a no-op. + /// The flags to use for this operation. + /// + /// One element per operation, in order; an element is if that operation was + /// skipped because of . The caller owns the returned lease. + /// + /// + /// + /// + /// If every operation is a this is issued as BITFIELD_RO + /// where the server supports it (6.0 and above), so it can be served by a replica; otherwise it is + /// issued as BITFIELD, which the server treats as a write and will only accept on a primary. + /// + /// + Lease? StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None); +#pragma warning restore RS0026 + /// /// Perform a bitwise operation between multiple keys (containing string values) and store the result in the destination key. /// The BITOP command supports four bitwise operations; note that NOT is a unary operator: the second key should be omitted in this case @@ -3392,14 +3432,24 @@ IEnumerable SortedSetScan( /// The key of the string. /// True to check for the first 1 bit, false to check for the first 0 bit. /// The position to start looking (defaults to 0). - /// The position to stop looking (defaults to -1, unlimited). + /// The position to stop looking (defaults to -1, the last byte); leaves the range open-ended, which is not the same thing when is false - see the remarks. /// In Redis 7+, we can choose if and specify a bit index or byte index (defaults to ). /// The flags to use for this operation. /// /// The command returns the position of the first bit set to 1 or 0 according to the request. /// If we look for set bits(the bit argument is 1) and the string is empty or composed of just zero bytes, -1 is returned. /// - /// + /// + /// + /// + /// When looking for a clear bit ( is false), the server treats an explicit + /// range differently from an open-ended one: with an explicit the search is confined + /// to the string, so a string of all-set bits gives -1, whereas with no end at all the result is the first + /// bit past the end of the string. Pass as for + /// the latter; that requires , since the server accepts a bit/byte index + /// type only after an explicit end. + /// + /// long StringBitPosition(RedisKey key, bool bit, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None); /// diff --git a/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs b/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs index 84403ab32a..6eb2bf07bb 100644 --- a/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs +++ b/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs @@ -823,6 +823,14 @@ IAsyncEnumerable SortedSetScanAsync( /// Task StringBitCountAsync(RedisKey key, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None); + /// +#pragma warning disable RS0026 // competing overloads - disambiguated via parameter types + Task StringBitFieldAsync(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None); + + /// + Task?> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None); +#pragma warning restore RS0026 + /// Task StringBitOperationAsync(Bitwise operation, RedisKey destination, RedisKey first, RedisKey second = default, CommandFlags flags = CommandFlags.None); diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs index d2b1a815db..5c4e28dc8f 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs @@ -784,6 +784,12 @@ public Task StringBitCountAsync(RedisKey key, long start, long end, Comman public Task StringBitCountAsync(RedisKey key, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None) => Inner.StringBitCountAsync(ToInner(key), start, end, indexType, flags); + public Task StringBitFieldAsync(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) => + Inner.StringBitFieldAsync(ToInner(key), operation, flags); + + public Task?> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) => + Inner.StringBitFieldAsync(ToInner(key), operations, flags); + public Task StringBitOperationAsync(Bitwise operation, RedisKey destination, RedisKey[] keys, CommandFlags flags = CommandFlags.None) => Inner.StringBitOperationAsync(operation, ToInner(destination), ToInner(keys), flags); diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs index 1ba10eccad..fc9e3b86f4 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs @@ -751,6 +751,12 @@ public long StringBitCount(RedisKey key, long start, long end, CommandFlags flag public long StringBitCount(RedisKey key, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None) => Inner.StringBitCount(ToInner(key), start, end, indexType, flags); + public long? StringBitField(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) => + Inner.StringBitField(ToInner(key), operation, flags); + + public Lease? StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) => + Inner.StringBitField(ToInner(key), operations, flags); + public long StringBitOperation(Bitwise operation, RedisKey destination, RedisKey[] keys, CommandFlags flags = CommandFlags.None) => Inner.StringBitOperation(operation, ToInner(destination), ToInner(keys), flags); diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index 3e6df85273..4167013207 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -58,3 +58,56 @@ StackExchange.Redis.StreamAddOptions.MinId.init -> void StackExchange.Redis.StreamAddOptions.StreamAddOptions() -> void StackExchange.Redis.StreamAddOptions.TrimMode.get -> StackExchange.Redis.StreamTrimMode StackExchange.Redis.StreamAddOptions.TrimMode.init -> void +StackExchange.Redis.StringIndex +const StackExchange.Redis.StringIndex.Unbounded = -9223372036854775808 -> long +override StackExchange.Redis.BitFieldEncoding.Equals(object? obj) -> bool +override StackExchange.Redis.BitFieldEncoding.GetHashCode() -> int +override StackExchange.Redis.BitFieldEncoding.ToString() -> string! +override StackExchange.Redis.BitFieldOffset.Equals(object? obj) -> bool +override StackExchange.Redis.BitFieldOffset.GetHashCode() -> int +override StackExchange.Redis.BitFieldOffset.ToString() -> string! +override StackExchange.Redis.BitFieldOperation.Equals(object? obj) -> bool +override StackExchange.Redis.BitFieldOperation.GetHashCode() -> int +override StackExchange.Redis.BitFieldOperation.ToString() -> string! +StackExchange.Redis.BitFieldEncoding +StackExchange.Redis.BitFieldEncoding.BitFieldEncoding() -> void +StackExchange.Redis.BitFieldEncoding.Equals(StackExchange.Redis.BitFieldEncoding other) -> bool +StackExchange.Redis.BitFieldEncoding.IsSigned.get -> bool +StackExchange.Redis.BitFieldEncoding.Width.get -> int +StackExchange.Redis.BitFieldOffset +StackExchange.Redis.BitFieldOffset.BitFieldOffset() -> void +StackExchange.Redis.BitFieldOffset.Equals(StackExchange.Redis.BitFieldOffset other) -> bool +StackExchange.Redis.BitFieldOperation +StackExchange.Redis.BitFieldOperation.BitFieldOperation() -> void +StackExchange.Redis.BitFieldOperation.Equals(StackExchange.Redis.BitFieldOperation other) -> bool +StackExchange.Redis.BitFieldOverflow +StackExchange.Redis.BitFieldOverflow.Fail = 2 -> StackExchange.Redis.BitFieldOverflow +StackExchange.Redis.BitFieldOverflow.Saturate = 1 -> StackExchange.Redis.BitFieldOverflow +StackExchange.Redis.BitFieldOverflow.Wrap = 0 -> StackExchange.Redis.BitFieldOverflow +StackExchange.Redis.IDatabaseAsync.StringBitFieldAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.BitFieldOperation operation, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task! +StackExchange.Redis.IDatabaseAsync.StringBitFieldAsync(StackExchange.Redis.RedisKey key, System.ReadOnlyMemory operations, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task?>! +StackExchange.Redis.IDatabase.StringBitField(StackExchange.Redis.RedisKey key, StackExchange.Redis.BitFieldOperation operation, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> long? +StackExchange.Redis.IDatabase.StringBitField(StackExchange.Redis.RedisKey key, System.ReadOnlyMemory operations, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> StackExchange.Redis.Lease? +StackExchange.Redis.RedisFeatures.BitFieldReadOnly.get -> bool +static StackExchange.Redis.BitFieldEncoding.Int16.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.Int32.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.Int64.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.Int8.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.operator ==(StackExchange.Redis.BitFieldEncoding x, StackExchange.Redis.BitFieldEncoding y) -> bool +static StackExchange.Redis.BitFieldEncoding.operator !=(StackExchange.Redis.BitFieldEncoding x, StackExchange.Redis.BitFieldEncoding y) -> bool +static StackExchange.Redis.BitFieldEncoding.Signed(int width) -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.UInt16.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.UInt32.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.UInt63.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.UInt8.get -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldEncoding.Unsigned(int width) -> StackExchange.Redis.BitFieldEncoding +static StackExchange.Redis.BitFieldOffset.Bits(long offset) -> StackExchange.Redis.BitFieldOffset +static StackExchange.Redis.BitFieldOffset.implicit operator StackExchange.Redis.BitFieldOffset(long offset) -> StackExchange.Redis.BitFieldOffset +static StackExchange.Redis.BitFieldOffset.Index(long index) -> StackExchange.Redis.BitFieldOffset +static StackExchange.Redis.BitFieldOffset.operator ==(StackExchange.Redis.BitFieldOffset x, StackExchange.Redis.BitFieldOffset y) -> bool +static StackExchange.Redis.BitFieldOffset.operator !=(StackExchange.Redis.BitFieldOffset x, StackExchange.Redis.BitFieldOffset y) -> bool +static StackExchange.Redis.BitFieldOperation.Get(StackExchange.Redis.BitFieldEncoding encoding, StackExchange.Redis.BitFieldOffset offset) -> StackExchange.Redis.BitFieldOperation +static StackExchange.Redis.BitFieldOperation.IncrementBy(StackExchange.Redis.BitFieldEncoding encoding, StackExchange.Redis.BitFieldOffset offset, long value, StackExchange.Redis.BitFieldOverflow overflow = StackExchange.Redis.BitFieldOverflow.Wrap) -> StackExchange.Redis.BitFieldOperation +static StackExchange.Redis.BitFieldOperation.operator ==(StackExchange.Redis.BitFieldOperation x, StackExchange.Redis.BitFieldOperation y) -> bool +static StackExchange.Redis.BitFieldOperation.operator !=(StackExchange.Redis.BitFieldOperation x, StackExchange.Redis.BitFieldOperation y) -> bool +static StackExchange.Redis.BitFieldOperation.Set(StackExchange.Redis.BitFieldEncoding encoding, StackExchange.Redis.BitFieldOffset offset, long value, StackExchange.Redis.BitFieldOverflow overflow = StackExchange.Redis.BitFieldOverflow.Wrap) -> StackExchange.Redis.BitFieldOperation diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index 172dd3c8d8..6972ad847d 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -3761,6 +3761,34 @@ public Task StringBitCountAsync(RedisKey key, long start = 0, long end = - return ExecuteAsync(msg, ResultProcessor.Int64); } + public long? StringBitField(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) + { + ReadOnlySpan operations = [operation]; + var msg = GetBitFieldMessage(key, operations, flags, out var server); + return ExecuteSync(msg, ResultProcessor.NullableInt64, server); + } + + public Task StringBitFieldAsync(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) + { + ReadOnlySpan operations = [operation]; + var msg = GetBitFieldMessage(key, operations, flags, out var server); + return ExecuteAsync(msg, ResultProcessor.NullableInt64, server); + } + + public Lease? StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) + { + if (operations.IsEmpty) return Lease.Empty; // no operations, no reply elements + var msg = GetBitFieldMessage(key, operations.Span, flags, out var server); + return ExecuteSync(msg, ResultProcessor.LeaseNullableInt64, server); + } + + public Task?> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) + { + if (operations.IsEmpty) return CompletedTask?>.FromDefault(Lease.Empty, asyncState); + var msg = GetBitFieldMessage(key, operations.Span, flags, out var server); + return ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, server); + } + public long StringBitOperation(Bitwise operation, RedisKey destination, RedisKey first, RedisKey second, CommandFlags flags = CommandFlags.None) { var msg = GetStringBitOperationMessage(operation, destination, first, second, flags); @@ -3790,11 +3818,7 @@ public long StringBitPosition(RedisKey key, bool bit, long start, long end, Comm public long StringBitPosition(RedisKey key, bool bit, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None) { - var msg = indexType switch - { - StringIndexType.Byte => Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start, end), - _ => Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start, end, indexType.ToLiteral()), - }; + var msg = GetStringBitPositionMessage(key, bit, start, end, indexType, flags); return ExecuteSync(msg, ResultProcessor.Int64); } @@ -3803,11 +3827,7 @@ public Task StringBitPositionAsync(RedisKey key, bool bit, long start, lon public Task StringBitPositionAsync(RedisKey key, bool bit, long start = 0, long end = -1, StringIndexType indexType = StringIndexType.Byte, CommandFlags flags = CommandFlags.None) { - var msg = indexType switch - { - StringIndexType.Byte => Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start, end), - _ => Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start, end, indexType.ToLiteral()), - }; + var msg = GetStringBitPositionMessage(key, bit, start, end, indexType, flags); return ExecuteAsync(msg, ResultProcessor.Int64); } @@ -5402,6 +5422,122 @@ private Message GetStringBitOperationMessage(Bitwise operation, RedisKey destina return Message.CreateInSlot(Database, slot, flags, RedisCommand.BITOP, new[] { op, destination.AsRedisValue(), first.AsRedisValue(), second.AsRedisValue() }); } + private Message GetBitFieldMessage(in RedisKey key, ReadOnlySpan operations, CommandFlags flags, out ServerEndPoint? server) + { + var values = BuildBitFieldPayload(operations, out var allGet, out var anyIncrement); + + var command = RedisCommand.BITFIELD; + server = null; + if (allGet) + { + // every operation is a read, so BITFIELD_RO will do - and unlike BITFIELD, a replica + // will accept it + var features = GetFeatures(key, flags, RedisCommand.BITFIELD_RO, out server); + if (server is not null && features.BitFieldReadOnly && multiplexer.CommandMap.IsAvailable(RedisCommand.BITFIELD_RO)) + { + command = RedisCommand.BITFIELD_RO; + } + else + { + server = null; // BITFIELD is primary-only; forget the read-eligible server we picked + } + } + else if (!anyIncrement) + { + // SET is positional, so a replay lands on the same value; only INCRBY compounds + flags = flags.WithCategory(CommandFlags.CommandRetryWriteLastWins); + } + + return Message.Create(Database, flags, command, key, values); + } + + /// + /// Renders the sub-operations of a BITFIELD call. The OVERFLOW token is sticky on the server + /// (and untouched by GET), so it is emitted only when the required mode changes - and never for + /// a leading run of , which is the server's own default. + /// + internal static RedisValue[] BuildBitFieldPayload(ReadOnlySpan operations, out bool allGet, out bool anyIncrement) + { + // first pass: size the payload, and note what kind of operations we have + allGet = true; + anyIncrement = false; + int count = 0; + var overflow = BitFieldOverflow.Wrap; + foreach (ref readonly var op in operations) + { + switch (op.Kind) + { + case BitFieldOperation.OperationKind.Get: + count += 3; // GET, encoding, offset + break; + case BitFieldOperation.OperationKind.Set: + case BitFieldOperation.OperationKind.IncrementBy: + allGet = false; + anyIncrement |= op.Kind == BitFieldOperation.OperationKind.IncrementBy; + if (op.Overflow != overflow) + { + overflow = op.Overflow; + count += 2; // OVERFLOW, mode + } + count += 4; // SET/INCRBY, encoding, offset, value + break; + default: + throw new ArgumentException($"A default {nameof(BitFieldOperation)} is not a valid operation.", nameof(operations)); + } + } + + // second pass: write it + var values = new RedisValue[count]; + int index = 0; + overflow = BitFieldOverflow.Wrap; + foreach (ref readonly var op in operations) + { + if (op.Kind != BitFieldOperation.OperationKind.Get && op.Overflow != overflow) + { + overflow = op.Overflow; + values[index++] = RedisLiterals.OVERFLOW; + values[index++] = RedisLiterals.Get(overflow); + } + + values[index++] = op.Kind switch + { + BitFieldOperation.OperationKind.Get => RedisLiterals.GET, + BitFieldOperation.OperationKind.Set => RedisLiterals.SET, + _ => RedisLiterals.INCRBY, + }; + values[index++] = op.Encoding.ToLiteral(); + values[index++] = op.Offset.ToLiteral(); + if (op.Kind != BitFieldOperation.OperationKind.Get) + { + values[index++] = op.Value; + } + } + + Debug.Assert(index == count, "bitfield payload size mismatch"); + return values; + } + + internal Message GetStringBitPositionMessage(in RedisKey key, bool bit, long start, long end, StringIndexType indexType, CommandFlags flags) + { + if (end == StringIndex.Unbounded) + { + // BITPOS takes the BYTE/BIT token only *after* an explicit end, so an open-ended range and a + // bit index cannot be expressed together; dropping the token silently would reinterpret start + if (indexType != StringIndexType.Byte) + { + throw new ArgumentException($"{nameof(StringIndex)}.{nameof(StringIndex.Unbounded)} requires {nameof(StringIndexType)}.{nameof(StringIndexType.Byte)}; the server accepts a bit/byte index type only after an explicit end.", nameof(indexType)); + } + + return Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start); + } + + return indexType switch + { + StringIndexType.Byte => Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start, end), + _ => Message.Create(Database, flags, RedisCommand.BITPOS, key, bit, start, end, indexType.ToLiteral()), + }; + } + internal Message GetStringGetExMessage(in RedisKey key, Expiration expiry, CommandFlags flags = CommandFlags.None) { var tokens = expiry.GetTokenCount(allowEnx: false); diff --git a/src/StackExchange.Redis/RedisFeatures.cs b/src/StackExchange.Redis/RedisFeatures.cs index 3e30a65dcb..9bfe8b27ce 100644 --- a/src/StackExchange.Redis/RedisFeatures.cs +++ b/src/StackExchange.Redis/RedisFeatures.cs @@ -74,6 +74,11 @@ public RedisFeatures(Version version) /// public bool BitwiseOperations => Version.IsAtLeast(v2_6_0); + /// + /// Is BITFIELD_RO available? + /// + public bool BitFieldReadOnly => Version.IsAtLeast(v6_0_0); + /// /// Is CLIENT SETNAME available? /// diff --git a/src/StackExchange.Redis/RedisLiterals.cs b/src/StackExchange.Redis/RedisLiterals.cs index 12b85ea2f8..a1799c72d4 100644 --- a/src/StackExchange.Redis/RedisLiterals.cs +++ b/src/StackExchange.Redis/RedisLiterals.cs @@ -40,6 +40,7 @@ public static readonly RedisValue EXACTLY = RedisValue.FromRaw("EXACTLY"u8), EXAT = RedisValue.FromRaw("EXAT"u8), EXISTS = RedisValue.FromRaw("EXISTS"u8), + FAIL = RedisValue.FromRaw("FAIL"u8), FIELDS = RedisValue.FromRaw("FIELDS"u8), FILTERBY = RedisValue.FromRaw("FILTERBY"u8), FLUSH = RedisValue.FromRaw("FLUSH"u8), @@ -61,6 +62,7 @@ public static readonly RedisValue IDMPAUTO = RedisValue.FromRaw("IDMPAUTO"u8), IDMP_DURATION = RedisValue.FromRaw("IDMP-DURATION"u8), IDMP_MAXSIZE = RedisValue.FromRaw("IDMP-MAXSIZE"u8), + INCRBY = RedisValue.FromRaw("INCRBY"u8), KEEPTTL = RedisValue.FromRaw("KEEPTTL"u8), KILL = RedisValue.FromRaw("KILL"u8), LADDR = RedisValue.FromRaw("LADDR"u8), @@ -94,6 +96,7 @@ public static readonly RedisValue OBO = RedisValue.FromRaw("OBO"u8), ONE = RedisValue.FromRaw("ONE"u8), OR = RedisValue.FromRaw("OR"u8), + OVERFLOW = RedisValue.FromRaw("OVERFLOW"u8), PATTERN = RedisValue.FromRaw("PATTERN"u8), PAUSE = RedisValue.FromRaw("PAUSE"u8), PERSIST = RedisValue.FromRaw("PERSIST"u8), @@ -110,6 +113,7 @@ public static readonly RedisValue REV = RedisValue.FromRaw("REV"u8), REWRITE = RedisValue.FromRaw("REWRITE"u8), RIGHT = RedisValue.FromRaw("RIGHT"u8), + SAT = RedisValue.FromRaw("SAT"u8), SAVE = RedisValue.FromRaw("SAVE"u8), SEGFAULT = RedisValue.FromRaw("SEGFAULT"u8), SET = RedisValue.FromRaw("SET"u8), @@ -127,6 +131,7 @@ public static readonly RedisValue WITHMATCHLEN = RedisValue.FromRaw("WITHMATCHLEN"u8), WITHSCORES = RedisValue.FromRaw("WITHSCORES"u8), WITHVALUES = RedisValue.FromRaw("WITHVALUES"u8), + WRAP = RedisValue.FromRaw("WRAP"u8), XOR = RedisValue.FromRaw("XOR"u8), XX = RedisValue.FromRaw("XX"u8), @@ -188,6 +193,14 @@ public static readonly RedisValue timeout = RedisValue.FromRaw("timeout"u8), yes = RedisValue.FromRaw("yes"u8); + internal static RedisValue Get(BitFieldOverflow overflow) => overflow switch + { + BitFieldOverflow.Wrap => WRAP, + BitFieldOverflow.Saturate => SAT, + BitFieldOverflow.Fail => FAIL, + _ => throw new ArgumentOutOfRangeException(nameof(overflow)), + }; + internal static RedisValue Get(Bitwise operation) => operation switch { Bitwise.And => AND, diff --git a/src/StackExchange.Redis/ResultProcessor.Lease.cs b/src/StackExchange.Redis/ResultProcessor.Lease.cs index 4544321fe6..f2a2a6a296 100644 --- a/src/StackExchange.Redis/ResultProcessor.Lease.cs +++ b/src/StackExchange.Redis/ResultProcessor.Lease.cs @@ -8,6 +8,8 @@ internal abstract partial class ResultProcessor // Lease result processors public static readonly ResultProcessor?> LeaseFloat32 = new LeaseFloat32Processor(); + public static readonly ResultProcessor?> LeaseNullableInt64 = new LeaseNullableInt64Processor(); + public static readonly ResultProcessor> Lease = new LeaseProcessor(); @@ -141,6 +143,12 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes } } + private sealed class LeaseNullableInt64Processor : LeaseProcessor + { + // BITFIELD reports a nil element for any operation skipped by OVERFLOW FAIL + protected override long? TryParse(ref RespReader reader) => reader.IsNull ? null : reader.ReadInt64(); + } + private sealed class LeaseFloat32Processor : LeaseProcessor { protected override float TryParse(ref RespReader reader) => (float)reader.ReadDouble(); diff --git a/src/StackExchange.Redis/StringIndex.cs b/src/StackExchange.Redis/StringIndex.cs new file mode 100644 index 0000000000..dcf0d6ea2b --- /dev/null +++ b/src/StackExchange.Redis/StringIndex.cs @@ -0,0 +1,24 @@ +namespace StackExchange.Redis; + +/// +/// Well-known index values for the bitmap operations that take a start/end range. +/// +public static class StringIndex +{ + /// + /// Indicates that no end index should be sent to the server, leaving the range open-ended. + /// + /// + /// + /// Valid only as the end of + /// (and the async equivalent), and only in combination with : BITPOS + /// accepts a bit/byte index type only after an explicit end. + /// + /// + /// This is not interchangeable with -1. When searching for a clear bit, an explicit end restricts the + /// search to the string itself, so an all-set string yields -1; an open-ended range instead reports the first + /// bit beyond the end of the string. + /// + /// + public const long Unbounded = long.MinValue; +} diff --git a/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs b/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs new file mode 100644 index 0000000000..f560009845 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs @@ -0,0 +1,107 @@ +using System; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// Unit tests for the BITFIELD value types; no server required. +/// +public class BitFieldTypeTests +{ + [Theory] + [InlineData(1, "i1")] + [InlineData(8, "i8")] + [InlineData(53, "i53")] + [InlineData(64, "i64")] + public void SignedWidths(int width, string expected) + { + var encoding = BitFieldEncoding.Signed(width); + Assert.Equal(expected, encoding.ToString()); + Assert.Equal(width, encoding.Width); + Assert.True(encoding.IsSigned); + } + + [Theory] + [InlineData(1, "u1")] + [InlineData(8, "u8")] + [InlineData(63, "u63")] + public void UnsignedWidths(int width, string expected) + { + var encoding = BitFieldEncoding.Unsigned(width); + Assert.Equal(expected, encoding.ToString()); + Assert.Equal(width, encoding.Width); + Assert.False(encoding.IsSigned); + } + + [Fact] + public void NamedEncodingsMatchTheirFactories() + { + Assert.Equal(BitFieldEncoding.Signed(8), BitFieldEncoding.Int8); + Assert.Equal(BitFieldEncoding.Signed(16), BitFieldEncoding.Int16); + Assert.Equal(BitFieldEncoding.Signed(32), BitFieldEncoding.Int32); + Assert.Equal(BitFieldEncoding.Signed(64), BitFieldEncoding.Int64); + Assert.Equal(BitFieldEncoding.Unsigned(8), BitFieldEncoding.UInt8); + Assert.Equal(BitFieldEncoding.Unsigned(16), BitFieldEncoding.UInt16); + Assert.Equal(BitFieldEncoding.Unsigned(32), BitFieldEncoding.UInt32); + Assert.Equal(BitFieldEncoding.Unsigned(63), BitFieldEncoding.UInt63); + Assert.NotEqual(BitFieldEncoding.Int8, BitFieldEncoding.UInt8); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(65)] + public void SignedRejectsIllegalWidths(int width) => + Assert.Throws(() => BitFieldEncoding.Signed(width)); + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(64)] + public void UnsignedRejectsIllegalWidths(int width) => + Assert.Throws(() => BitFieldEncoding.Unsigned(width)); + + [Fact] + public void UnsignedSixtyFourExplainsItself() + { + // the obvious thing to reach for; the message should say why it cannot work, and what to use + var ex = Assert.Throws(() => BitFieldEncoding.Unsigned(64)); + Assert.Contains("UInt63", ex.Message); + Assert.Contains("Int64", ex.Message); + } + + [Fact] + public void OffsetForms() + { + Assert.Equal("100", BitFieldOffset.Bits(100).ToString()); + Assert.Equal("#100", BitFieldOffset.Index(100).ToString()); + Assert.Equal(BitFieldOffset.Bits(3), (BitFieldOffset)3L); // implicit long is a bit offset + Assert.NotEqual(BitFieldOffset.Bits(3), BitFieldOffset.Index(3)); + } + + [Fact] + public void OffsetsCannotBeNegative() + { + Assert.Throws(() => BitFieldOffset.Bits(-1)); + Assert.Throws(() => BitFieldOffset.Index(-1)); + } + + [Fact] + public void DefaultEncodingIsRejectedAtConstruction() + { + var ex = Assert.Throws(() => BitFieldOperation.Get(default, 0)); + Assert.Equal("encoding", ex.ParamName); + } + + [Fact] + public void UnknownOverflowIsRejected() => + Assert.Throws(() => BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1, (BitFieldOverflow)42)); + + [Fact] + public void OperationToString() + { + Assert.Equal("GET u8 #1", BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Index(1)).ToString()); + Assert.Equal("SET i16 32 -5 (Saturate)", BitFieldOperation.Set(BitFieldEncoding.Int16, 32, -5, BitFieldOverflow.Saturate).ToString()); + Assert.Equal("INCRBY i8 0 1 (Wrap)", BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 1).ToString()); + } +} diff --git a/tests/StackExchange.Redis.Tests/BitTests.cs b/tests/StackExchange.Redis.Tests/BitTests.cs index b4c032366d..45ee9a28c9 100644 --- a/tests/StackExchange.Redis.Tests/BitTests.cs +++ b/tests/StackExchange.Redis.Tests/BitTests.cs @@ -1,4 +1,7 @@ -using System.Threading.Tasks; +using System; +using System.Linq; +using System.Threading.Tasks; +using StackExchange.Redis.Profiling; using Xunit; namespace StackExchange.Redis.Tests; @@ -18,4 +21,134 @@ public async Task BasicOps() Assert.True(db.StringGetBit(key, 10)); Assert.False(db.StringGetBit(key, 11)); } + + [Fact] + public async Task BitPositionUnboundedEndLooksPastEndOfString() + { + await using var conn = Create(); + var db = conn.GetDatabase(); + RedisKey key = Me(); + + db.KeyDelete(key, CommandFlags.FireAndForget); + for (int i = 0; i < 8; i++) + { + db.StringSetBit(key, i, true, CommandFlags.FireAndForget); + } + Assert.Equal(1, db.StringLength(key)); + + // an explicit end confines the search to the string, which has no clear bit + Assert.Equal(-1, db.StringBitPosition(key, false)); + Assert.Equal(-1, await db.StringBitPositionAsync(key, false, 0, -1)); + + // an open-ended range reports the first clear bit past the end of the string + Assert.Equal(8, db.StringBitPosition(key, false, 0, StringIndex.Unbounded)); + Assert.Equal(8, await db.StringBitPositionAsync(key, false, 0, StringIndex.Unbounded)); + + // set bits are unaffected either way + Assert.Equal(0, db.StringBitPosition(key, true)); + Assert.Equal(0, db.StringBitPosition(key, true, 0, StringIndex.Unbounded)); + } + + [Fact] + public async Task BitFieldBasicOps() + { + await using var conn = Create(require: RedisFeatures.v3_2_0); + var db = conn.GetDatabase(); + RedisKey key = Me(); + + db.KeyDelete(key, CommandFlags.FireAndForget); + + // SET reports the previous value, GET the current one; the #N form indexes by width + Assert.Equal(0, db.StringBitField(key, BitFieldOperation.Set(BitFieldEncoding.UInt8, BitFieldOffset.Index(1), 255))); + Assert.Equal(255, await db.StringBitFieldAsync(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Index(1)))); + Assert.Equal(255, db.StringBitField(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, 8))); // ... same field, by bit + Assert.Equal(0, db.StringBitField(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0))); + + // INCRBY reports the new value + Assert.Equal(-1, db.StringBitField(key, BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, -1))); + } + + [Fact] + public async Task BitFieldBatchAppliesInOrder() + { + await using var conn = Create(require: RedisFeatures.v3_2_0); + var db = conn.GetDatabase(); + RedisKey key = Me(); + + db.KeyDelete(key, CommandFlags.FireAndForget); + + using var lease = await db.StringBitFieldAsync( + key, + new[] + { + BitFieldOperation.Set(BitFieldEncoding.Int8, 0, 100), + BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Saturate), + BitFieldOperation.Get(BitFieldEncoding.Int8, 0), + BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Fail), + BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Wrap), + }); + + Assert.NotNull(lease); + + // the FAIL element is null; note the sticky OVERFLOW state survives the intervening GET + Assert.Equal(new long?[] { 0, 127, 127, null, -29 }, lease.Span.ToArray()); + } + + [Fact] + public async Task BitFieldEmptyBatchIsANoOp() + { + await using var conn = Create(require: RedisFeatures.v3_2_0); + var db = conn.GetDatabase(); + RedisKey key = Me(); + + using var lease = db.StringBitField(key, ReadOnlyMemory.Empty); + Assert.NotNull(lease); + Assert.True(lease.IsEmpty); + + using var leaseAsync = await db.StringBitFieldAsync(key, ReadOnlyMemory.Empty); + Assert.NotNull(leaseAsync); + Assert.True(leaseAsync.IsEmpty); + } + + [Fact] + public async Task BitFieldAllGetGoesOutAsReadOnlyAndReachesAReplica() + { + await using var conn = Create( + require: RedisFeatures.v6_0_0, + configuration: TestConfig.Current.PrimaryServerAndPort + "," + TestConfig.Current.ReplicaServerAndPort); + Assert.SkipUnless( + conn.GetEndPoints().Any(ep => conn.GetServer(ep).IsReplica), + "No replica in this configuration"); + + var db = conn.GetDatabase(); + RedisKey key = Me(); + var session = new ProfilingSession(); + conn.RegisterProfiler(() => session); + + // an all-GET batch is issued as BITFIELD_RO, so a replica will serve it; the value itself is + // not asserted here - reading from a replica makes that a race with replication + using var lease = db.StringBitField( + key, + new[] { BitFieldOperation.Get(BitFieldEncoding.UInt8, 0) }, + CommandFlags.DemandReplica); + Assert.NotNull(lease); + Assert.Equal(1, lease.Length); + Assert.NotNull(db.StringBitField(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), CommandFlags.DemandReplica)); + + // ... whereas anything that writes stays as BITFIELD + db.StringBitField(key, BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1)); + + var commands = session.FinishProfiling().ToList(); + foreach (var command in commands) + { + Log($"{command.Command} -> {command.EndPoint}"); + } + + var readOnly = commands.Where(c => c.Command == "BITFIELD_RO").ToList(); + Assert.Equal(2, readOnly.Count); + Assert.All(readOnly, c => Assert.True(conn.GetServer(c.EndPoint).IsReplica)); + + var write = Assert.Single(commands, c => c.Command == "BITFIELD"); + Assert.False(conn.GetServer(write.EndPoint).IsReplica); + } } diff --git a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs new file mode 100644 index 0000000000..b7b8f3456a --- /dev/null +++ b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs @@ -0,0 +1,120 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests.RoundTripUnitTests; + +public class BitFieldRoundTrip(ITestOutputHelper log) +{ + private static Message Message(RedisCommand command, params BitFieldOperation[] operations) + { + var payload = RedisDatabase.BuildBitFieldPayload(operations, out _, out _); + return StackExchange.Redis.Message.Create(0, CommandFlags.None, command, (RedisKey)"k", payload); + } + + [Fact(Timeout = 1000)] + public async Task Increment_RoundTrips() + { + var msg = Message(RedisCommand.BITFIELD, BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 1)); + const string RequestResp = "*6\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n$6\r\nINCRBY\r\n$2\r\ni8\r\n$1\r\n0\r\n$1\r\n1\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*1\r\n:1\r\n", log: log); + Assert.Equal(new long?[] { 1 }, result!.Span.ToArray()); + } + + [Fact(Timeout = 1000)] + public async Task AllGet_UsesReadOnlyCommand() + { + var msg = Message( + RedisCommand.BITFIELD_RO, + BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), + BitFieldOperation.Get(BitFieldEncoding.UInt63, BitFieldOffset.Index(2))); + const string RequestResp = "*8\r\n$11\r\nBITFIELD_RO\r\n$1\r\nk\r\n$3\r\nGET\r\n$2\r\nu8\r\n$1\r\n0\r\n$3\r\nGET\r\n$3\r\nu63\r\n$2\r\n#2\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*2\r\n:255\r\n:0\r\n", log: log); + Assert.Equal(new long?[] { 255, 0 }, result!.Span.ToArray()); + } + + [Fact(Timeout = 1000)] + public async Task LeadingWrap_IsNotEmitted() + { + // WRAP is the server default, so there is nothing to say + var msg = Message(RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.Int64, 0, 5, BitFieldOverflow.Wrap)); + const string RequestResp = "*6\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n$3\r\nSET\r\n$3\r\ni64\r\n$1\r\n0\r\n$1\r\n5\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*1\r\n:0\r\n", log: log); + Assert.Equal(new long?[] { 0 }, result!.Span.ToArray()); + } + + [Fact(Timeout = 1000)] + public async Task Overflow_IsEmittedOnlyWhenItChanges() + { + var msg = Message( + RedisCommand.BITFIELD, + BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1, BitFieldOverflow.Saturate), + BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, BitFieldOffset.Index(1), 2, BitFieldOverflow.Saturate), + BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 3, BitFieldOverflow.Wrap)); + const string RequestResp = "*18\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n" + + "$8\r\nOVERFLOW\r\n$3\r\nSAT\r\n$3\r\nSET\r\n$2\r\nu8\r\n$1\r\n0\r\n$1\r\n1\r\n" + + "$6\r\nINCRBY\r\n$2\r\nu8\r\n$2\r\n#1\r\n$1\r\n2\r\n" + + "$8\r\nOVERFLOW\r\n$4\r\nWRAP\r\n$3\r\nSET\r\n$2\r\nu8\r\n$1\r\n0\r\n$1\r\n3\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*3\r\n:0\r\n:2\r\n:1\r\n", log: log); + Assert.Equal(new long?[] { 0, 2, 1 }, result!.Span.ToArray()); + } + + [Fact(Timeout = 1000)] + public async Task Overflow_SurvivesAnInterveningGet_AndFailReportsNull() + { + // the sticky OVERFLOW state is not reset or consumed by a GET, so the second INCRBY needs no token + var msg = Message( + RedisCommand.BITFIELD, + BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Fail), + BitFieldOperation.Get(BitFieldEncoding.Int8, 0), + BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Fail)); + const string RequestResp = "*15\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n" + + "$8\r\nOVERFLOW\r\n$4\r\nFAIL\r\n$6\r\nINCRBY\r\n$2\r\ni8\r\n$1\r\n0\r\n$3\r\n100\r\n" + + "$3\r\nGET\r\n$2\r\ni8\r\n$1\r\n0\r\n" + + "$6\r\nINCRBY\r\n$2\r\ni8\r\n$1\r\n0\r\n$3\r\n100\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*3\r\n:100\r\n:100\r\n$-1\r\n", log: log); + Assert.Equal(new long?[] { 100, 100, null }, result!.Span.ToArray()); + } + + [Fact] + public void Classification_ReflectsTheOperations() + { + RedisDatabase.BuildBitFieldPayload([BitFieldOperation.Get(BitFieldEncoding.UInt8, 0)], out var allGet, out var anyIncrement); + Assert.True(allGet); + Assert.False(anyIncrement); + + RedisDatabase.BuildBitFieldPayload( + [BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1)], + out allGet, + out anyIncrement); + Assert.False(allGet); + Assert.False(anyIncrement); // SET only: eligible for last-wins retry + + RedisDatabase.BuildBitFieldPayload( + [BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1), BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, 0, 1)], + out allGet, + out anyIncrement); + Assert.False(allGet); + Assert.True(anyIncrement); + } + + [Fact] + public void DefaultOperation_IsRejected() + { + var ex = Assert.Throws(() => RedisDatabase.BuildBitFieldPayload(new BitFieldOperation[1], out _, out _)); + Assert.Equal("operations", ex.ParamName); + } + + [Fact] + public void ReadOnlyVariantIsNotPrimaryOnly() + { + // BITFIELD is a write server-side even when every sub-operation is a GET; BITFIELD_RO is not + Assert.True(RedisCommand.BITFIELD.IsPrimaryOnly()); + Assert.False(RedisCommand.BITFIELD_RO.IsPrimaryOnly()); + } +} diff --git a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitPositionRoundTrip.cs b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitPositionRoundTrip.cs new file mode 100644 index 0000000000..d083bbb68b --- /dev/null +++ b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitPositionRoundTrip.cs @@ -0,0 +1,60 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests.RoundTripUnitTests; + +public class BitPositionRoundTrip(ITestOutputHelper log) +{ + [Fact(Timeout = 1000)] + public async Task ExplicitEnd_IsSent() + { + var db = new RedisDatabase(null!, 0, null); + var msg = db.GetStringBitPositionMessage("k", false, 0, -1, StringIndexType.Byte, CommandFlags.None); + const string RequestResp = "*5\r\n$6\r\nBITPOS\r\n$1\r\nk\r\n$1\r\n0\r\n$1\r\n0\r\n$2\r\n-1\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.Int64, RequestResp, ":-1\r\n", log: log); + Assert.Equal(-1, result); + } + + [Fact(Timeout = 1000)] + public async Task ExplicitEnd_WithBitIndexType_SendsToken() + { + var db = new RedisDatabase(null!, 0, null); + var msg = db.GetStringBitPositionMessage("k", false, 0, 7, StringIndexType.Bit, CommandFlags.None); + const string RequestResp = "*6\r\n$6\r\nBITPOS\r\n$1\r\nk\r\n$1\r\n0\r\n$1\r\n0\r\n$1\r\n7\r\n$3\r\nBIT\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.Int64, RequestResp, ":-1\r\n", log: log); + Assert.Equal(-1, result); + } + + [Fact(Timeout = 1000)] + public async Task UnboundedEnd_OmitsEnd() + { + var db = new RedisDatabase(null!, 0, null); + var msg = db.GetStringBitPositionMessage("k", false, 0, StringIndex.Unbounded, StringIndexType.Byte, CommandFlags.None); + const string RequestResp = "*4\r\n$6\r\nBITPOS\r\n$1\r\nk\r\n$1\r\n0\r\n$1\r\n0\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.Int64, RequestResp, ":8\r\n", log: log); + Assert.Equal(8, result); + } + + [Fact(Timeout = 1000)] + public async Task UnboundedEnd_KeepsStart() + { + var db = new RedisDatabase(null!, 0, null); + var msg = db.GetStringBitPositionMessage("k", true, 2, StringIndex.Unbounded, StringIndexType.Byte, CommandFlags.None); + const string RequestResp = "*4\r\n$6\r\nBITPOS\r\n$1\r\nk\r\n$1\r\n1\r\n$1\r\n2\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.Int64, RequestResp, ":16\r\n", log: log); + Assert.Equal(16, result); + } + + [Fact] + public void UnboundedEnd_RejectsBitIndexType() + { + var db = new RedisDatabase(null!, 0, null); + var ex = Assert.Throws(() => db.GetStringBitPositionMessage("k", false, 0, StringIndex.Unbounded, StringIndexType.Bit, CommandFlags.None)); + Assert.Equal("indexType", ex.ParamName); + } +} From dc8b27383f4e8126fd5010abe9c82a0e2b7129de Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 13:51:44 +0100 Subject: [PATCH 2/7] Name the unit, not the mechanism, on BitFieldOffset Bits/Index both said the wrong thing: Index named the mechanism ("an index") when the question is *of what* - bits or elements - and Bits(100) read as a quantity ("100 bits") rather than a position ("bit 100"). Singular noun plus ordinal, symmetrically: Bit(600) is bit 600, Element(3) is element 3, i.e. the #3 form the server multiplies by the width. Element rather than Field because "field" is already carrying weight in BitFieldEncoding and BitFieldOperation, and would read as naming a field rather than counting one. --- src/StackExchange.Redis/BitFieldOffset.cs | 48 ++++++++++--------- .../PublicAPI/PublicAPI.Unshipped.txt | 6 +-- .../BitFieldTypeTests.cs | 14 +++--- tests/StackExchange.Redis.Tests/BitTests.cs | 4 +- .../RoundTripUnitTests/BitFieldRoundTrip.cs | 4 +- 5 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/StackExchange.Redis/BitFieldOffset.cs b/src/StackExchange.Redis/BitFieldOffset.cs index 4cc3487df4..9d82087573 100644 --- a/src/StackExchange.Redis/BitFieldOffset.cs +++ b/src/StackExchange.Redis/BitFieldOffset.cs @@ -4,62 +4,64 @@ namespace StackExchange.Redis; /// -/// Where a bitfield sits inside a string: either an absolute bit offset, or - via -/// - a zero-based index into an array of consecutive fields of the same -/// width, which the server multiplies out for us (the # form). +/// Where a bitfield sits inside a string: either a bit position, or - via +/// - the position of one element in an array of consecutive fields of +/// the same width, which the server multiplies out for us (the # form). /// /// public readonly struct BitFieldOffset : IEquatable { private readonly long _value; - private readonly bool _isIndex; + private readonly bool _isElement; private BitFieldOffset(long value, bool isIndex) { _value = value; - _isIndex = isIndex; + _isElement = isIndex; } /// - /// An absolute offset, in bits, from the start of the string. + /// The zero-based bit position, counted from the start of the string; Bit(16) is the + /// seventeenth bit. /// - /// The offset, in bits. - public static BitFieldOffset Bits(long offset) => offset < 0 - ? throw new ArgumentOutOfRangeException(nameof(offset), "A bitfield offset cannot be negative.") - : new(offset, false); + /// The bit position. + public static BitFieldOffset Bit(long bit) => bit < 0 + ? throw new ArgumentOutOfRangeException(nameof(bit), "A bitfield bit position cannot be negative.") + : new(bit, false); /// - /// A zero-based index into an array of consecutive fields of the encoding's own width; the - /// server multiplies the index by the width, so Index(2) of a u8 is bit 16. + /// The zero-based position of one element in an array of consecutive fields of the encoding's + /// own width; the server multiplies it by the width, so Element(2) of a u8 is + /// bit 16. /// - /// The index of the field. - public static BitFieldOffset Index(long index) => index < 0 - ? throw new ArgumentOutOfRangeException(nameof(index), "A bitfield index cannot be negative.") - : new(index, true); + /// The element position. + public static BitFieldOffset Element(long element) => element < 0 + ? throw new ArgumentOutOfRangeException(nameof(element), "A bitfield element position cannot be negative.") + : new(element, true); /// - /// Creates an absolute bit offset; equivalent to . + /// Creates a bit position; equivalent to . /// - /// The offset, in bits. - public static implicit operator BitFieldOffset(long offset) => Bits(offset); + /// The bit position. + public static implicit operator BitFieldOffset(long bit) => Bit(bit); - internal RedisValue ToLiteral() => _isIndex + internal RedisValue ToLiteral() => _isElement ? (RedisValue)("#" + _value.ToString(CultureInfo.InvariantCulture)) : (RedisValue)_value; /// - public override string ToString() => _isIndex + public override string ToString() => _isElement ? "#" + _value.ToString(CultureInfo.InvariantCulture) : _value.ToString(CultureInfo.InvariantCulture); /// - public bool Equals(BitFieldOffset other) => _value == other._value && _isIndex == other._isIndex; + public bool Equals(BitFieldOffset other) => _value == other._value && _isElement == other._isElement; /// public override bool Equals(object? obj) => obj is BitFieldOffset other && Equals(other); /// - public override int GetHashCode() => _isIndex ? ~_value.GetHashCode() : _value.GetHashCode(); + public override int GetHashCode() => _isElement ? ~_value.GetHashCode() : _value.GetHashCode(); /// Compares two values for equality. public static bool operator ==(BitFieldOffset x, BitFieldOffset y) => x.Equals(y); diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index 4167013207..c865628bfc 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -101,9 +101,9 @@ static StackExchange.Redis.BitFieldEncoding.UInt32.get -> StackExchange.Redis.Bi static StackExchange.Redis.BitFieldEncoding.UInt63.get -> StackExchange.Redis.BitFieldEncoding static StackExchange.Redis.BitFieldEncoding.UInt8.get -> StackExchange.Redis.BitFieldEncoding static StackExchange.Redis.BitFieldEncoding.Unsigned(int width) -> StackExchange.Redis.BitFieldEncoding -static StackExchange.Redis.BitFieldOffset.Bits(long offset) -> StackExchange.Redis.BitFieldOffset -static StackExchange.Redis.BitFieldOffset.implicit operator StackExchange.Redis.BitFieldOffset(long offset) -> StackExchange.Redis.BitFieldOffset -static StackExchange.Redis.BitFieldOffset.Index(long index) -> StackExchange.Redis.BitFieldOffset +static StackExchange.Redis.BitFieldOffset.Bit(long bit) -> StackExchange.Redis.BitFieldOffset +static StackExchange.Redis.BitFieldOffset.implicit operator StackExchange.Redis.BitFieldOffset(long bit) -> StackExchange.Redis.BitFieldOffset +static StackExchange.Redis.BitFieldOffset.Element(long element) -> StackExchange.Redis.BitFieldOffset static StackExchange.Redis.BitFieldOffset.operator ==(StackExchange.Redis.BitFieldOffset x, StackExchange.Redis.BitFieldOffset y) -> bool static StackExchange.Redis.BitFieldOffset.operator !=(StackExchange.Redis.BitFieldOffset x, StackExchange.Redis.BitFieldOffset y) -> bool static StackExchange.Redis.BitFieldOperation.Get(StackExchange.Redis.BitFieldEncoding encoding, StackExchange.Redis.BitFieldOffset offset) -> StackExchange.Redis.BitFieldOperation diff --git a/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs b/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs index f560009845..fd768c940e 100644 --- a/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs +++ b/tests/StackExchange.Redis.Tests/BitFieldTypeTests.cs @@ -73,17 +73,17 @@ public void UnsignedSixtyFourExplainsItself() [Fact] public void OffsetForms() { - Assert.Equal("100", BitFieldOffset.Bits(100).ToString()); - Assert.Equal("#100", BitFieldOffset.Index(100).ToString()); - Assert.Equal(BitFieldOffset.Bits(3), (BitFieldOffset)3L); // implicit long is a bit offset - Assert.NotEqual(BitFieldOffset.Bits(3), BitFieldOffset.Index(3)); + Assert.Equal("100", BitFieldOffset.Bit(100).ToString()); + Assert.Equal("#100", BitFieldOffset.Element(100).ToString()); + Assert.Equal(BitFieldOffset.Bit(3), (BitFieldOffset)3L); // implicit long is a bit offset + Assert.NotEqual(BitFieldOffset.Bit(3), BitFieldOffset.Element(3)); } [Fact] public void OffsetsCannotBeNegative() { - Assert.Throws(() => BitFieldOffset.Bits(-1)); - Assert.Throws(() => BitFieldOffset.Index(-1)); + Assert.Throws(() => BitFieldOffset.Bit(-1)); + Assert.Throws(() => BitFieldOffset.Element(-1)); } [Fact] @@ -100,7 +100,7 @@ public void UnknownOverflowIsRejected() => [Fact] public void OperationToString() { - Assert.Equal("GET u8 #1", BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Index(1)).ToString()); + Assert.Equal("GET u8 #1", BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Element(1)).ToString()); Assert.Equal("SET i16 32 -5 (Saturate)", BitFieldOperation.Set(BitFieldEncoding.Int16, 32, -5, BitFieldOverflow.Saturate).ToString()); Assert.Equal("INCRBY i8 0 1 (Wrap)", BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 1).ToString()); } diff --git a/tests/StackExchange.Redis.Tests/BitTests.cs b/tests/StackExchange.Redis.Tests/BitTests.cs index 45ee9a28c9..287b1cc9b2 100644 --- a/tests/StackExchange.Redis.Tests/BitTests.cs +++ b/tests/StackExchange.Redis.Tests/BitTests.cs @@ -59,8 +59,8 @@ public async Task BitFieldBasicOps() db.KeyDelete(key, CommandFlags.FireAndForget); // SET reports the previous value, GET the current one; the #N form indexes by width - Assert.Equal(0, db.StringBitField(key, BitFieldOperation.Set(BitFieldEncoding.UInt8, BitFieldOffset.Index(1), 255))); - Assert.Equal(255, await db.StringBitFieldAsync(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Index(1)))); + Assert.Equal(0, db.StringBitField(key, BitFieldOperation.Set(BitFieldEncoding.UInt8, BitFieldOffset.Element(1), 255))); + Assert.Equal(255, await db.StringBitFieldAsync(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Element(1)))); Assert.Equal(255, db.StringBitField(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, 8))); // ... same field, by bit Assert.Equal(0, db.StringBitField(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0))); diff --git a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs index b7b8f3456a..df66935559 100644 --- a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs +++ b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs @@ -28,7 +28,7 @@ public async Task AllGet_UsesReadOnlyCommand() var msg = Message( RedisCommand.BITFIELD_RO, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), - BitFieldOperation.Get(BitFieldEncoding.UInt63, BitFieldOffset.Index(2))); + BitFieldOperation.Get(BitFieldEncoding.UInt63, BitFieldOffset.Element(2))); const string RequestResp = "*8\r\n$11\r\nBITFIELD_RO\r\n$1\r\nk\r\n$3\r\nGET\r\n$2\r\nu8\r\n$1\r\n0\r\n$3\r\nGET\r\n$3\r\nu63\r\n$2\r\n#2\r\n"; var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*2\r\n:255\r\n:0\r\n", log: log); @@ -52,7 +52,7 @@ public async Task Overflow_IsEmittedOnlyWhenItChanges() var msg = Message( RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1, BitFieldOverflow.Saturate), - BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, BitFieldOffset.Index(1), 2, BitFieldOverflow.Saturate), + BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, BitFieldOffset.Element(1), 2, BitFieldOverflow.Saturate), BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 3, BitFieldOverflow.Wrap)); const string RequestResp = "*18\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n" + "$8\r\nOVERFLOW\r\n$3\r\nSAT\r\n$3\r\nSET\r\n$2\r\nu8\r\n$1\r\n0\r\n$1\r\n1\r\n" From 0756f6dec6d104aedaf63e92f99a10b762b4d664 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 13:58:42 +0100 Subject: [PATCH 3/7] Drop the meaningless outer null from the BITFIELD batch result The reply is always an array with one element per operation - empty for no operations - so a null *array* was unreachable; the only way to see one was a non-observable result such as fire-and-forget, where ExecuteSync hands back default(T). That made callers null-check for a state that never carries information, while the inner long? (an operation skipped by OVERFLOW FAIL) is the nullability that actually means something. Use the established defaultValue: idiom instead - as GeoPosition/GeoHash do for their arrays - so the batch returns a non-null Lease and yields an empty lease when there is nothing to observe. The single-operation overload keeps its long?, where null already means FAIL. --- src/StackExchange.Redis/Interfaces/IDatabase.cs | 5 +++-- src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs | 2 +- .../KeyspaceIsolation/KeyPrefixed.cs | 2 +- .../KeyspaceIsolation/KeyPrefixedDatabase.cs | 2 +- .../PublicAPI/PublicAPI.Unshipped.txt | 4 ++-- src/StackExchange.Redis/RedisDatabase.cs | 10 +++++----- src/StackExchange.Redis/ResultProcessor.Lease.cs | 5 ++++- tests/StackExchange.Redis.Tests/BitTests.cs | 5 ----- 8 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/StackExchange.Redis/Interfaces/IDatabase.cs b/src/StackExchange.Redis/Interfaces/IDatabase.cs index c567653309..96ab6142e7 100644 --- a/src/StackExchange.Redis/Interfaces/IDatabase.cs +++ b/src/StackExchange.Redis/Interfaces/IDatabase.cs @@ -3378,7 +3378,8 @@ IEnumerable SortedSetScan( /// The flags to use for this operation. /// /// One element per operation, in order; an element is if that operation was - /// skipped because of . The caller owns the returned lease. + /// skipped because of . Empty for an empty batch, or when the + /// result is not observable (). The caller owns the lease. /// /// /// @@ -3388,7 +3389,7 @@ IEnumerable SortedSetScan( /// issued as BITFIELD, which the server treats as a write and will only accept on a primary. /// /// - Lease? StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None); + Lease StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None); #pragma warning restore RS0026 /// diff --git a/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs b/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs index 6eb2bf07bb..b172375abf 100644 --- a/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs +++ b/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs @@ -828,7 +828,7 @@ IAsyncEnumerable SortedSetScanAsync( Task StringBitFieldAsync(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None); /// - Task?> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None); + Task> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None); #pragma warning restore RS0026 /// diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs index 5c4e28dc8f..5dee9cda3b 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs @@ -787,7 +787,7 @@ public Task StringBitCountAsync(RedisKey key, long start = 0, long end = - public Task StringBitFieldAsync(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) => Inner.StringBitFieldAsync(ToInner(key), operation, flags); - public Task?> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) => + public Task> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) => Inner.StringBitFieldAsync(ToInner(key), operations, flags); public Task StringBitOperationAsync(Bitwise operation, RedisKey destination, RedisKey[] keys, CommandFlags flags = CommandFlags.None) => diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs index fc9e3b86f4..fa7c438498 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixedDatabase.cs @@ -754,7 +754,7 @@ public long StringBitCount(RedisKey key, long start = 0, long end = -1, StringIn public long? StringBitField(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) => Inner.StringBitField(ToInner(key), operation, flags); - public Lease? StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) => + public Lease StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) => Inner.StringBitField(ToInner(key), operations, flags); public long StringBitOperation(Bitwise operation, RedisKey destination, RedisKey[] keys, CommandFlags flags = CommandFlags.None) => diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index c865628bfc..419c74dc23 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -85,9 +85,9 @@ StackExchange.Redis.BitFieldOverflow.Fail = 2 -> StackExchange.Redis.BitFieldOve StackExchange.Redis.BitFieldOverflow.Saturate = 1 -> StackExchange.Redis.BitFieldOverflow StackExchange.Redis.BitFieldOverflow.Wrap = 0 -> StackExchange.Redis.BitFieldOverflow StackExchange.Redis.IDatabaseAsync.StringBitFieldAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.BitFieldOperation operation, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task! -StackExchange.Redis.IDatabaseAsync.StringBitFieldAsync(StackExchange.Redis.RedisKey key, System.ReadOnlyMemory operations, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task?>! +StackExchange.Redis.IDatabaseAsync.StringBitFieldAsync(StackExchange.Redis.RedisKey key, System.ReadOnlyMemory operations, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task!>! StackExchange.Redis.IDatabase.StringBitField(StackExchange.Redis.RedisKey key, StackExchange.Redis.BitFieldOperation operation, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> long? -StackExchange.Redis.IDatabase.StringBitField(StackExchange.Redis.RedisKey key, System.ReadOnlyMemory operations, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> StackExchange.Redis.Lease? +StackExchange.Redis.IDatabase.StringBitField(StackExchange.Redis.RedisKey key, System.ReadOnlyMemory operations, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> StackExchange.Redis.Lease! StackExchange.Redis.RedisFeatures.BitFieldReadOnly.get -> bool static StackExchange.Redis.BitFieldEncoding.Int16.get -> StackExchange.Redis.BitFieldEncoding static StackExchange.Redis.BitFieldEncoding.Int32.get -> StackExchange.Redis.BitFieldEncoding diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index 6972ad847d..efe103544b 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -3775,18 +3775,18 @@ public Task StringBitCountAsync(RedisKey key, long start = 0, long end = - return ExecuteAsync(msg, ResultProcessor.NullableInt64, server); } - public Lease? StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) + public Lease StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) { if (operations.IsEmpty) return Lease.Empty; // no operations, no reply elements var msg = GetBitFieldMessage(key, operations.Span, flags, out var server); - return ExecuteSync(msg, ResultProcessor.LeaseNullableInt64, server); + return ExecuteSync(msg, ResultProcessor.LeaseNullableInt64, server, defaultValue: Lease.Empty); } - public Task?> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) + public Task> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) { - if (operations.IsEmpty) return CompletedTask?>.FromDefault(Lease.Empty, asyncState); + if (operations.IsEmpty) return CompletedTask>.FromDefault(Lease.Empty, asyncState); var msg = GetBitFieldMessage(key, operations.Span, flags, out var server); - return ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, server); + return ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, Lease.Empty, server); } public long StringBitOperation(Bitwise operation, RedisKey destination, RedisKey first, RedisKey second, CommandFlags flags = CommandFlags.None) diff --git a/src/StackExchange.Redis/ResultProcessor.Lease.cs b/src/StackExchange.Redis/ResultProcessor.Lease.cs index f2a2a6a296..919c8f42fc 100644 --- a/src/StackExchange.Redis/ResultProcessor.Lease.cs +++ b/src/StackExchange.Redis/ResultProcessor.Lease.cs @@ -8,7 +8,10 @@ internal abstract partial class ResultProcessor // Lease result processors public static readonly ResultProcessor?> LeaseFloat32 = new LeaseFloat32Processor(); - public static readonly ResultProcessor?> LeaseNullableInt64 = new LeaseNullableInt64Processor(); + // outer type is deliberately non-null: BITFIELD always replies with an array (empty for no + // operations), so the only null would come from a non-observable result such as fire-and-forget, + // which the callers handle with an explicit empty default + public static readonly ResultProcessor> LeaseNullableInt64 = new LeaseNullableInt64Processor()!; public static readonly ResultProcessor> Lease = new LeaseProcessor(); diff --git a/tests/StackExchange.Redis.Tests/BitTests.cs b/tests/StackExchange.Redis.Tests/BitTests.cs index 287b1cc9b2..71f00b90d4 100644 --- a/tests/StackExchange.Redis.Tests/BitTests.cs +++ b/tests/StackExchange.Redis.Tests/BitTests.cs @@ -88,8 +88,6 @@ public async Task BitFieldBatchAppliesInOrder() BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Wrap), }); - Assert.NotNull(lease); - // the FAIL element is null; note the sticky OVERFLOW state survives the intervening GET Assert.Equal(new long?[] { 0, 127, 127, null, -29 }, lease.Span.ToArray()); } @@ -102,11 +100,9 @@ public async Task BitFieldEmptyBatchIsANoOp() RedisKey key = Me(); using var lease = db.StringBitField(key, ReadOnlyMemory.Empty); - Assert.NotNull(lease); Assert.True(lease.IsEmpty); using var leaseAsync = await db.StringBitFieldAsync(key, ReadOnlyMemory.Empty); - Assert.NotNull(leaseAsync); Assert.True(leaseAsync.IsEmpty); } @@ -131,7 +127,6 @@ public async Task BitFieldAllGetGoesOutAsReadOnlyAndReachesAReplica() key, new[] { BitFieldOperation.Get(BitFieldEncoding.UInt8, 0) }, CommandFlags.DemandReplica); - Assert.NotNull(lease); Assert.Equal(1, lease.Length); Assert.NotNull(db.StringBitField(key, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), CommandFlags.DemandReplica)); From bae1b57a8d5bc49d9e853cd7c71e0b1803d16c4b Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 14:09:29 +0100 Subject: [PATCH 4/7] Write BITFIELD directly rather than building a RedisValue[] Follows the HotKeysStartMessage shape: a Message subclass that renders itself in WriteImpl, with ArgCount computed (and the payload validated) in the constructor, so a bad operation still throws at the caller rather than mid-write on the connection. This drops the intermediate RedisValue[] entirely - the batch form keeps the caller's ReadOnlyMemory, as the existing keyed-values messages do with caller arrays, and the single-operation form holds the operation inline, so that path now allocates nothing but the message. It also obviates the literals: the sub-command and overflow tokens are framed compile-time u8 spans written raw, which retires the RedisLiterals additions and the 128-entry encoding table behind ToLiteral. The encoding is written as one framed bulk string into a scratch buffer hoisted out of the loop: the width is 1-64, so the length prefix is always a single digit and it is either $2\r\nXX\r\n or $3\r\nXXX\r\n. The element offset shares that buffer for its '#' plus digits, but keeps WriteBulkString since its own length prefix varies. Round-trip tests now build the messages directly, as SetCardinalityRoundTrip does, which also retires the internal payload-builder seam that only existed for testability; new cases cover the encoding and offset framing at both prefix widths, ArgCount, and that a single operation writes byte-for-byte the same as a one-element batch. --- src/StackExchange.Redis/BitFieldEncoding.cs | 51 +++--- src/StackExchange.Redis/BitFieldOffset.cs | 15 +- .../BitFieldOperation.Message.cs | 149 ++++++++++++++++++ src/StackExchange.Redis/RedisDatabase.cs | 121 +++++--------- src/StackExchange.Redis/RedisLiterals.cs | 13 -- .../StackExchange.Redis.csproj | 1 + .../RoundTripUnitTests/BitFieldRoundTrip.cs | 112 ++++++++----- 7 files changed, 310 insertions(+), 152 deletions(-) create mode 100644 src/StackExchange.Redis/BitFieldOperation.Message.cs diff --git a/src/StackExchange.Redis/BitFieldEncoding.cs b/src/StackExchange.Redis/BitFieldEncoding.cs index d30b99d1e7..eb1501fa58 100644 --- a/src/StackExchange.Redis/BitFieldEncoding.cs +++ b/src/StackExchange.Redis/BitFieldEncoding.cs @@ -83,35 +83,46 @@ public static BitFieldEncoding Unsigned(int width) internal bool IsDefault => _value == 0; - internal RedisValue ToLiteral() + /// + /// Writes this encoding as a complete bulk string - $2\r\ni8\r\n or $3\r\ni64\r\n - since + /// the width is 1-64 and so the length prefix is always a single digit. + /// + internal void Write(in MessageWriter writer, Span scratch) { if (_value == 0) ThrowDefault(); - return Literals[_value + 64]; + + int width = Width, len; + scratch[0] = (byte)'$'; + scratch[2] = (byte)'\r'; + scratch[3] = (byte)'\n'; + scratch[4] = _value < 0 ? (byte)'i' : (byte)'u'; + if (width >= 10) + { + scratch[1] = (byte)'3'; + scratch[5] = (byte)('0' + (width / 10)); + scratch[6] = (byte)('0' + (width % 10)); + len = 7; + } + else + { + scratch[1] = (byte)'2'; + scratch[5] = (byte)('0' + width); + len = 6; + } + + scratch[len++] = (byte)'\r'; + scratch[len++] = (byte)'\n'; + writer.WriteRaw(scratch.Slice(0, len)); static void ThrowDefault() => throw new ArgumentException( $"A {nameof(BitFieldEncoding)} must be created via {nameof(Signed)}, {nameof(Unsigned)}, or one of the named encodings.", nameof(BitFieldEncoding)); } - // i64..i1 (indexes 0..63), unused (64), u1..u63 (65..127) - private static readonly RedisValue[] Literals = CreateLiterals(); - - private static RedisValue[] CreateLiterals() - { - var arr = new RedisValue[128]; - for (int width = 1; width <= 64; width++) - { - arr[64 - width] = "i" + width.ToString(System.Globalization.CultureInfo.InvariantCulture); - if (width <= 63) - { - arr[64 + width] = "u" + width.ToString(System.Globalization.CultureInfo.InvariantCulture); - } - } - return arr; - } - /// - public override string ToString() => _value == 0 ? "(default)" : (string)ToLiteral()!; + public override string ToString() => _value == 0 + ? "(default)" + : (IsSigned ? "i" : "u") + Width.ToString(System.Globalization.CultureInfo.InvariantCulture); /// public bool Equals(BitFieldEncoding other) => _value == other._value; diff --git a/src/StackExchange.Redis/BitFieldOffset.cs b/src/StackExchange.Redis/BitFieldOffset.cs index 9d82087573..517192fb19 100644 --- a/src/StackExchange.Redis/BitFieldOffset.cs +++ b/src/StackExchange.Redis/BitFieldOffset.cs @@ -45,9 +45,18 @@ public static BitFieldOffset Element(long element) => element < 0 /// The bit position. public static implicit operator BitFieldOffset(long bit) => Bit(bit); - internal RedisValue ToLiteral() => _isElement - ? (RedisValue)("#" + _value.ToString(CultureInfo.InvariantCulture)) - : (RedisValue)_value; + internal void Write(in MessageWriter writer, Span scratch) + { + if (!_isElement) + { + writer.WriteBulkString(_value); + return; + } + + scratch[0] = (byte)'#'; + var len = Format.FormatInt64(_value, scratch.Slice(1)) + 1; + writer.WriteBulkString(scratch.Slice(0, len)); + } /// public override string ToString() => _isElement diff --git a/src/StackExchange.Redis/BitFieldOperation.Message.cs b/src/StackExchange.Redis/BitFieldOperation.Message.cs new file mode 100644 index 0000000000..0b52dd654e --- /dev/null +++ b/src/StackExchange.Redis/BitFieldOperation.Message.cs @@ -0,0 +1,149 @@ +using System; + +namespace StackExchange.Redis; + +internal partial class RedisDatabase +{ + /// + /// BITFIELD/BITFIELD_RO: the key, then one clause per operation. + /// + /// + /// The OVERFLOW token is sticky server-side, and is neither reset nor consumed by a + /// GET, so it is written only when the required mode changes - and never for a leading run + /// of , which is the server's own default. + /// + internal abstract class BitFieldMessageBase : Message.CommandKeyBase + { + protected BitFieldMessageBase(int db, CommandFlags flags, RedisCommand command, in RedisKey key) + : base(db, flags, command, key) + { + } + + /// + /// Counts the arguments needed by a set of operations, including the key, and rejects anything + /// that cannot be written - while we can still throw at the caller rather than mid-write. + /// + protected static int CountArgs(ReadOnlySpan operations, string paramName) + { + int count = 1; // the key + var overflow = BitFieldOverflow.Wrap; + foreach (ref readonly var op in operations) + { + switch (op.Kind) + { + case BitFieldOperation.OperationKind.Get: + count += 3; // GET, encoding, offset + break; + case BitFieldOperation.OperationKind.Set: + case BitFieldOperation.OperationKind.IncrementBy: + if (op.Overflow != overflow) + { + overflow = op.Overflow; + count += 2; // OVERFLOW, mode + } + + count += 4; // SET/INCRBY, encoding, offset, value + break; + default: + throw new ArgumentException($"A default {nameof(BitFieldOperation)} is not a valid operation.", paramName); + } + } + + return count; + } + + protected static void WriteOperations(in MessageWriter writer, ReadOnlySpan operations) + { + // enough for the widest thing either part writes: a framed encoding ("$3\r\ni64\r\n", 9) or + // an element offset payload ('#' plus an int64, 21) + Span scratch = stackalloc byte[Format.MaxInt64TextLen + 1]; + var overflow = BitFieldOverflow.Wrap; + foreach (ref readonly var op in operations) + { + if (op.Kind != BitFieldOperation.OperationKind.Get && op.Overflow != overflow) + { + overflow = op.Overflow; + writer.WriteRaw("$8\r\nOVERFLOW\r\n"u8); + switch (overflow) + { + case BitFieldOverflow.Saturate: + writer.WriteRaw("$3\r\nSAT\r\n"u8); + break; + case BitFieldOverflow.Fail: + writer.WriteRaw("$4\r\nFAIL\r\n"u8); + break; + default: + writer.WriteRaw("$4\r\nWRAP\r\n"u8); + break; + } + } + + switch (op.Kind) + { + case BitFieldOperation.OperationKind.Get: + writer.WriteRaw("$3\r\nGET\r\n"u8); + break; + case BitFieldOperation.OperationKind.Set: + writer.WriteRaw("$3\r\nSET\r\n"u8); + break; + default: + writer.WriteRaw("$6\r\nINCRBY\r\n"u8); + break; + } + + op.Encoding.Write(in writer, scratch); + op.Offset.Write(in writer, scratch); + if (op.Kind != BitFieldOperation.OperationKind.Get) + { + writer.WriteBulkString(op.Value); + } + } + } + } + + /// + internal sealed class BitFieldMessage : BitFieldMessageBase + { + private readonly ReadOnlyMemory _operations; + private readonly int _argCount; + + public BitFieldMessage(int db, CommandFlags flags, RedisCommand command, in RedisKey key, ReadOnlyMemory operations) + : base(db, flags, command, key) + { + _operations = operations; + _argCount = CountArgs(operations.Span, nameof(operations)); + } + + protected override void WriteImpl(in MessageWriter writer) + { + writer.WriteHeader(Command, _argCount); + writer.Write(Key); + WriteOperations(in writer, _operations.Span); + } + + public override int ArgCount => _argCount; + } + + /// + internal sealed class BitFieldSingleMessage : BitFieldMessageBase + { + private readonly BitFieldOperation _operation; + private readonly int _argCount; + + public BitFieldSingleMessage(int db, CommandFlags flags, RedisCommand command, in RedisKey key, in BitFieldOperation operation) + : base(db, flags, command, key) + { + _operation = operation; + _argCount = CountArgs([operation], nameof(operation)); + } + + protected override void WriteImpl(in MessageWriter writer) + { + writer.WriteHeader(Command, _argCount); + writer.Write(Key); + WriteOperations(in writer, [_operation]); + } + + public override int ArgCount => _argCount; + } +} diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index efe103544b..642e1b712a 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -3763,29 +3763,27 @@ public Task StringBitCountAsync(RedisKey key, long start = 0, long end = - public long? StringBitField(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) { - ReadOnlySpan operations = [operation]; - var msg = GetBitFieldMessage(key, operations, flags, out var server); + var msg = GetBitFieldMessage(key, operation, flags, out var server); return ExecuteSync(msg, ResultProcessor.NullableInt64, server); } public Task StringBitFieldAsync(RedisKey key, BitFieldOperation operation, CommandFlags flags = CommandFlags.None) { - ReadOnlySpan operations = [operation]; - var msg = GetBitFieldMessage(key, operations, flags, out var server); + var msg = GetBitFieldMessage(key, operation, flags, out var server); return ExecuteAsync(msg, ResultProcessor.NullableInt64, server); } public Lease StringBitField(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) { if (operations.IsEmpty) return Lease.Empty; // no operations, no reply elements - var msg = GetBitFieldMessage(key, operations.Span, flags, out var server); + var msg = GetBitFieldMessage(key, operations, flags, out var server); return ExecuteSync(msg, ResultProcessor.LeaseNullableInt64, server, defaultValue: Lease.Empty); } public Task> StringBitFieldAsync(RedisKey key, ReadOnlyMemory operations, CommandFlags flags = CommandFlags.None) { if (operations.IsEmpty) return CompletedTask>.FromDefault(Lease.Empty, asyncState); - var msg = GetBitFieldMessage(key, operations.Span, flags, out var server); + var msg = GetBitFieldMessage(key, operations, flags, out var server); return ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, Lease.Empty, server); } @@ -5422,99 +5420,62 @@ private Message GetStringBitOperationMessage(Bitwise operation, RedisKey destina return Message.CreateInSlot(Database, slot, flags, RedisCommand.BITOP, new[] { op, destination.AsRedisValue(), first.AsRedisValue(), second.AsRedisValue() }); } - private Message GetBitFieldMessage(in RedisKey key, ReadOnlySpan operations, CommandFlags flags, out ServerEndPoint? server) - { - var values = BuildBitFieldPayload(operations, out var allGet, out var anyIncrement); - - var command = RedisCommand.BITFIELD; - server = null; - if (allGet) - { - // every operation is a read, so BITFIELD_RO will do - and unlike BITFIELD, a replica - // will accept it - var features = GetFeatures(key, flags, RedisCommand.BITFIELD_RO, out server); - if (server is not null && features.BitFieldReadOnly && multiplexer.CommandMap.IsAvailable(RedisCommand.BITFIELD_RO)) - { - command = RedisCommand.BITFIELD_RO; - } - else - { - server = null; // BITFIELD is primary-only; forget the read-eligible server we picked - } - } - else if (!anyIncrement) - { - // SET is positional, so a replay lands on the same value; only INCRBY compounds - flags = flags.WithCategory(CommandFlags.CommandRetryWriteLastWins); - } - - return Message.Create(Database, flags, command, key, values); - } - - /// - /// Renders the sub-operations of a BITFIELD call. The OVERFLOW token is sticky on the server - /// (and untouched by GET), so it is emitted only when the required mode changes - and never for - /// a leading run of , which is the server's own default. - /// - internal static RedisValue[] BuildBitFieldPayload(ReadOnlySpan operations, out bool allGet, out bool anyIncrement) + private Message GetBitFieldMessage(in RedisKey key, ReadOnlyMemory operations, CommandFlags flags, out ServerEndPoint? server) { - // first pass: size the payload, and note what kind of operations we have - allGet = true; - anyIncrement = false; - int count = 0; - var overflow = BitFieldOverflow.Wrap; - foreach (ref readonly var op in operations) + bool allGet = true, anyIncrement = false; + foreach (ref readonly var op in operations.Span) { switch (op.Kind) { case BitFieldOperation.OperationKind.Get: - count += 3; // GET, encoding, offset break; - case BitFieldOperation.OperationKind.Set: case BitFieldOperation.OperationKind.IncrementBy: + anyIncrement = true; allGet = false; - anyIncrement |= op.Kind == BitFieldOperation.OperationKind.IncrementBy; - if (op.Overflow != overflow) - { - overflow = op.Overflow; - count += 2; // OVERFLOW, mode - } - count += 4; // SET/INCRBY, encoding, offset, value break; default: - throw new ArgumentException($"A default {nameof(BitFieldOperation)} is not a valid operation.", nameof(operations)); + allGet = false; + break; } } - // second pass: write it - var values = new RedisValue[count]; - int index = 0; - overflow = BitFieldOverflow.Wrap; - foreach (ref readonly var op in operations) + var command = SelectBitFieldCommand(key, allGet, anyIncrement, ref flags, out server); + return new BitFieldMessage(Database, flags, command, key, operations); + } + + private Message GetBitFieldMessage(in RedisKey key, in BitFieldOperation operation, CommandFlags flags, out ServerEndPoint? server) + { + var command = SelectBitFieldCommand( + key, + allGet: operation.Kind == BitFieldOperation.OperationKind.Get, + anyIncrement: operation.Kind == BitFieldOperation.OperationKind.IncrementBy, + ref flags, + out server); + return new BitFieldSingleMessage(Database, flags, command, key, operation); + } + + private RedisCommand SelectBitFieldCommand(in RedisKey key, bool allGet, bool anyIncrement, ref CommandFlags flags, out ServerEndPoint? server) + { + server = null; + if (allGet) { - if (op.Kind != BitFieldOperation.OperationKind.Get && op.Overflow != overflow) + // every operation is a read, so BITFIELD_RO will do - and unlike BITFIELD, a replica + // will accept it + var features = GetFeatures(key, flags, RedisCommand.BITFIELD_RO, out server); + if (server is not null && features.BitFieldReadOnly && multiplexer.CommandMap.IsAvailable(RedisCommand.BITFIELD_RO)) { - overflow = op.Overflow; - values[index++] = RedisLiterals.OVERFLOW; - values[index++] = RedisLiterals.Get(overflow); + return RedisCommand.BITFIELD_RO; } - values[index++] = op.Kind switch - { - BitFieldOperation.OperationKind.Get => RedisLiterals.GET, - BitFieldOperation.OperationKind.Set => RedisLiterals.SET, - _ => RedisLiterals.INCRBY, - }; - values[index++] = op.Encoding.ToLiteral(); - values[index++] = op.Offset.ToLiteral(); - if (op.Kind != BitFieldOperation.OperationKind.Get) - { - values[index++] = op.Value; - } + server = null; // BITFIELD is primary-only; forget the read-eligible server we picked + } + else if (!anyIncrement) + { + // SET is positional, so a replay lands on the same value; only INCRBY compounds + flags = flags.WithCategory(CommandFlags.CommandRetryWriteLastWins); } - Debug.Assert(index == count, "bitfield payload size mismatch"); - return values; + return RedisCommand.BITFIELD; } internal Message GetStringBitPositionMessage(in RedisKey key, bool bit, long start, long end, StringIndexType indexType, CommandFlags flags) diff --git a/src/StackExchange.Redis/RedisLiterals.cs b/src/StackExchange.Redis/RedisLiterals.cs index a1799c72d4..12b85ea2f8 100644 --- a/src/StackExchange.Redis/RedisLiterals.cs +++ b/src/StackExchange.Redis/RedisLiterals.cs @@ -40,7 +40,6 @@ public static readonly RedisValue EXACTLY = RedisValue.FromRaw("EXACTLY"u8), EXAT = RedisValue.FromRaw("EXAT"u8), EXISTS = RedisValue.FromRaw("EXISTS"u8), - FAIL = RedisValue.FromRaw("FAIL"u8), FIELDS = RedisValue.FromRaw("FIELDS"u8), FILTERBY = RedisValue.FromRaw("FILTERBY"u8), FLUSH = RedisValue.FromRaw("FLUSH"u8), @@ -62,7 +61,6 @@ public static readonly RedisValue IDMPAUTO = RedisValue.FromRaw("IDMPAUTO"u8), IDMP_DURATION = RedisValue.FromRaw("IDMP-DURATION"u8), IDMP_MAXSIZE = RedisValue.FromRaw("IDMP-MAXSIZE"u8), - INCRBY = RedisValue.FromRaw("INCRBY"u8), KEEPTTL = RedisValue.FromRaw("KEEPTTL"u8), KILL = RedisValue.FromRaw("KILL"u8), LADDR = RedisValue.FromRaw("LADDR"u8), @@ -96,7 +94,6 @@ public static readonly RedisValue OBO = RedisValue.FromRaw("OBO"u8), ONE = RedisValue.FromRaw("ONE"u8), OR = RedisValue.FromRaw("OR"u8), - OVERFLOW = RedisValue.FromRaw("OVERFLOW"u8), PATTERN = RedisValue.FromRaw("PATTERN"u8), PAUSE = RedisValue.FromRaw("PAUSE"u8), PERSIST = RedisValue.FromRaw("PERSIST"u8), @@ -113,7 +110,6 @@ public static readonly RedisValue REV = RedisValue.FromRaw("REV"u8), REWRITE = RedisValue.FromRaw("REWRITE"u8), RIGHT = RedisValue.FromRaw("RIGHT"u8), - SAT = RedisValue.FromRaw("SAT"u8), SAVE = RedisValue.FromRaw("SAVE"u8), SEGFAULT = RedisValue.FromRaw("SEGFAULT"u8), SET = RedisValue.FromRaw("SET"u8), @@ -131,7 +127,6 @@ public static readonly RedisValue WITHMATCHLEN = RedisValue.FromRaw("WITHMATCHLEN"u8), WITHSCORES = RedisValue.FromRaw("WITHSCORES"u8), WITHVALUES = RedisValue.FromRaw("WITHVALUES"u8), - WRAP = RedisValue.FromRaw("WRAP"u8), XOR = RedisValue.FromRaw("XOR"u8), XX = RedisValue.FromRaw("XX"u8), @@ -193,14 +188,6 @@ public static readonly RedisValue timeout = RedisValue.FromRaw("timeout"u8), yes = RedisValue.FromRaw("yes"u8); - internal static RedisValue Get(BitFieldOverflow overflow) => overflow switch - { - BitFieldOverflow.Wrap => WRAP, - BitFieldOverflow.Saturate => SAT, - BitFieldOverflow.Fail => FAIL, - _ => throw new ArgumentOutOfRangeException(nameof(overflow)), - }; - internal static RedisValue Get(Bitwise operation) => operation switch { Bitwise.And => AND, diff --git a/src/StackExchange.Redis/StackExchange.Redis.csproj b/src/StackExchange.Redis/StackExchange.Redis.csproj index 2d04242ab9..587af06229 100644 --- a/src/StackExchange.Redis/StackExchange.Redis.csproj +++ b/src/StackExchange.Redis/StackExchange.Redis.csproj @@ -40,6 +40,7 @@ + diff --git a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs index df66935559..5cac1fb013 100644 --- a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs +++ b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs @@ -6,50 +6,90 @@ namespace StackExchange.Redis.Tests.RoundTripUnitTests; public class BitFieldRoundTrip(ITestOutputHelper log) { - private static Message Message(RedisCommand command, params BitFieldOperation[] operations) - { - var payload = RedisDatabase.BuildBitFieldPayload(operations, out _, out _); - return StackExchange.Redis.Message.Create(0, CommandFlags.None, command, (RedisKey)"k", payload); - } + private static Message Batch(RedisCommand command, params BitFieldOperation[] operations) => + new RedisDatabase.BitFieldMessage(0, CommandFlags.None, command, (RedisKey)"k", operations); + + private static Message Single(RedisCommand command, BitFieldOperation operation) => + new RedisDatabase.BitFieldSingleMessage(0, CommandFlags.None, command, (RedisKey)"k", operation); [Fact(Timeout = 1000)] public async Task Increment_RoundTrips() { - var msg = Message(RedisCommand.BITFIELD, BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 1)); + var msg = Batch(RedisCommand.BITFIELD, BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 1)); const string RequestResp = "*6\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n$6\r\nINCRBY\r\n$2\r\ni8\r\n$1\r\n0\r\n$1\r\n1\r\n"; var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*1\r\n:1\r\n", log: log); - Assert.Equal(new long?[] { 1 }, result!.Span.ToArray()); + Assert.Equal(new long?[] { 1 }, result.Span.ToArray()); + } + + [Fact(Timeout = 1000)] + public async Task SingleOperation_WritesTheSameAsAUnitBatch() + { + // the single-operation overload avoids the array, but must produce identical bytes + var msg = Single(RedisCommand.BITFIELD, BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 1)); + const string RequestResp = "*6\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n$6\r\nINCRBY\r\n$2\r\ni8\r\n$1\r\n0\r\n$1\r\n1\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.NullableInt64, RequestResp, "*1\r\n:1\r\n", log: log); + Assert.Equal(1, result); } [Fact(Timeout = 1000)] public async Task AllGet_UsesReadOnlyCommand() { - var msg = Message( + var msg = Batch( RedisCommand.BITFIELD_RO, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), BitFieldOperation.Get(BitFieldEncoding.UInt63, BitFieldOffset.Element(2))); const string RequestResp = "*8\r\n$11\r\nBITFIELD_RO\r\n$1\r\nk\r\n$3\r\nGET\r\n$2\r\nu8\r\n$1\r\n0\r\n$3\r\nGET\r\n$3\r\nu63\r\n$2\r\n#2\r\n"; var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*2\r\n:255\r\n:0\r\n", log: log); - Assert.Equal(new long?[] { 255, 0 }, result!.Span.ToArray()); + Assert.Equal(new long?[] { 255, 0 }, result.Span.ToArray()); + } + + [Theory(Timeout = 1000)] + [InlineData(1, "$2\r\ni1\r\n")] + [InlineData(9, "$2\r\ni9\r\n")] + [InlineData(10, "$3\r\ni10\r\n")] + [InlineData(64, "$3\r\ni64\r\n")] + public async Task EncodingWidths_AreFramedCorrectly(int width, string encodingResp) + { + // the width is 1-64, so the length prefix is always one digit and the whole bulk string is + // written in one go + var msg = Batch(RedisCommand.BITFIELD_RO, BitFieldOperation.Get(BitFieldEncoding.Signed(width), 0)); + var requestResp = "*5\r\n$11\r\nBITFIELD_RO\r\n$1\r\nk\r\n$3\r\nGET\r\n" + encodingResp + "$1\r\n0\r\n"; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, requestResp, "*1\r\n:0\r\n", log: log); + Assert.Equal(new long?[] { 0 }, result.Span.ToArray()); + } + + [Theory(Timeout = 1000)] + [InlineData(0, "$2\r\n#0\r\n")] + [InlineData(9, "$2\r\n#9\r\n")] + [InlineData(1234567890123, "$14\r\n#1234567890123\r\n")] + public async Task ElementOffsets_AreFramedCorrectly(long element, string offsetResp) + { + var msg = Batch(RedisCommand.BITFIELD_RO, BitFieldOperation.Get(BitFieldEncoding.UInt8, BitFieldOffset.Element(element))); + var requestResp = "*5\r\n$11\r\nBITFIELD_RO\r\n$1\r\nk\r\n$3\r\nGET\r\n$2\r\nu8\r\n" + offsetResp; + + var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, requestResp, "*1\r\n:0\r\n", log: log); + Assert.Equal(new long?[] { 0 }, result.Span.ToArray()); } [Fact(Timeout = 1000)] public async Task LeadingWrap_IsNotEmitted() { // WRAP is the server default, so there is nothing to say - var msg = Message(RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.Int64, 0, 5, BitFieldOverflow.Wrap)); + var msg = Batch(RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.Int64, 0, 5, BitFieldOverflow.Wrap)); const string RequestResp = "*6\r\n$8\r\nBITFIELD\r\n$1\r\nk\r\n$3\r\nSET\r\n$3\r\ni64\r\n$1\r\n0\r\n$1\r\n5\r\n"; var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*1\r\n:0\r\n", log: log); - Assert.Equal(new long?[] { 0 }, result!.Span.ToArray()); + Assert.Equal(new long?[] { 0 }, result.Span.ToArray()); } [Fact(Timeout = 1000)] public async Task Overflow_IsEmittedOnlyWhenItChanges() { - var msg = Message( + var msg = Batch( RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1, BitFieldOverflow.Saturate), BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, BitFieldOffset.Element(1), 2, BitFieldOverflow.Saturate), @@ -60,14 +100,14 @@ public async Task Overflow_IsEmittedOnlyWhenItChanges() + "$8\r\nOVERFLOW\r\n$4\r\nWRAP\r\n$3\r\nSET\r\n$2\r\nu8\r\n$1\r\n0\r\n$1\r\n3\r\n"; var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*3\r\n:0\r\n:2\r\n:1\r\n", log: log); - Assert.Equal(new long?[] { 0, 2, 1 }, result!.Span.ToArray()); + Assert.Equal(new long?[] { 0, 2, 1 }, result.Span.ToArray()); } [Fact(Timeout = 1000)] public async Task Overflow_SurvivesAnInterveningGet_AndFailReportsNull() { // the sticky OVERFLOW state is not reset or consumed by a GET, so the second INCRBY needs no token - var msg = Message( + var msg = Batch( RedisCommand.BITFIELD, BitFieldOperation.IncrementBy(BitFieldEncoding.Int8, 0, 100, BitFieldOverflow.Fail), BitFieldOperation.Get(BitFieldEncoding.Int8, 0), @@ -78,36 +118,36 @@ public async Task Overflow_SurvivesAnInterveningGet_AndFailReportsNull() + "$6\r\nINCRBY\r\n$2\r\ni8\r\n$1\r\n0\r\n$3\r\n100\r\n"; var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.LeaseNullableInt64, RequestResp, "*3\r\n:100\r\n:100\r\n$-1\r\n", log: log); - Assert.Equal(new long?[] { 100, 100, null }, result!.Span.ToArray()); + Assert.Equal(new long?[] { 100, 100, null }, result.Span.ToArray()); } [Fact] - public void Classification_ReflectsTheOperations() + public void ArgCountMatchesTheOperations() { - RedisDatabase.BuildBitFieldPayload([BitFieldOperation.Get(BitFieldEncoding.UInt8, 0)], out var allGet, out var anyIncrement); - Assert.True(allGet); - Assert.False(anyIncrement); - - RedisDatabase.BuildBitFieldPayload( - [BitFieldOperation.Get(BitFieldEncoding.UInt8, 0), BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1)], - out allGet, - out anyIncrement); - Assert.False(allGet); - Assert.False(anyIncrement); // SET only: eligible for last-wins retry - - RedisDatabase.BuildBitFieldPayload( - [BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1), BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, 0, 1)], - out allGet, - out anyIncrement); - Assert.False(allGet); - Assert.True(anyIncrement); + // key + (SET, enc, off, value) + Assert.Equal(5, Batch(RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1)).ArgCount); + Assert.Equal(5, Single(RedisCommand.BITFIELD, BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1)).ArgCount); + + // key + (GET, enc, off) + Assert.Equal(4, Batch(RedisCommand.BITFIELD_RO, BitFieldOperation.Get(BitFieldEncoding.UInt8, 0)).ArgCount); + + // key + (OVERFLOW, mode) + 2x(INCRBY, enc, off, value) + Assert.Equal( + 11, + Batch( + RedisCommand.BITFIELD, + BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, 0, 1, BitFieldOverflow.Fail), + BitFieldOperation.IncrementBy(BitFieldEncoding.UInt8, 0, 1, BitFieldOverflow.Fail)).ArgCount); } [Fact] - public void DefaultOperation_IsRejected() + public void DefaultOperation_IsRejectedBeforeTheMessageIsQueued() { - var ex = Assert.Throws(() => RedisDatabase.BuildBitFieldPayload(new BitFieldOperation[1], out _, out _)); - Assert.Equal("operations", ex.ParamName); + var batch = Assert.Throws(() => Batch(RedisCommand.BITFIELD, default(BitFieldOperation))); + Assert.Equal("operations", batch.ParamName); + + var single = Assert.Throws(() => Single(RedisCommand.BITFIELD, default(BitFieldOperation))); + Assert.Equal("operation", single.ParamName); } [Fact] From 7cf7d0f7dda917bd0ad6787a10a9fdec9dab3d2a Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 14:21:00 +0100 Subject: [PATCH 5/7] Fail rather than guess in the BITFIELD writer The kind switch used its default arm for INCRBY, so an operation that was somehow neither GET nor SET would have been written as the most destructive of the three. Reject it instead: guessing is worse than failing here, because a wrong sub-command corrupts data and one of the wrong arity corrupts the connection - a GET written where the header promised four arguments leaves the server parsing the next command's bytes as arguments. The overflow arm keeps WRAP, and that asymmetry is deliberate: its argument count comes from the transition (op.Overflow != overflow) rather than the mode itself, so an unknown mode is shape-neutral and the server's own default is the harmless answer. CountArgs already rejected this at construction, but only watertight for the single-operation form, which copies. The batch form aliases the caller's memory, and unlike the messages holding a RedisValue[] - where ArgCount is the array length and content cannot change the shape - here the shape depends on the contents, so mutating the array between issue and write would desync the frame. Re-check before the header is written, so a violated contract fails with nothing written rather than corrupting the connection. --- .../BitFieldOperation.Message.cs | 42 +++++++++++++++---- .../RoundTripUnitTests/BitFieldRoundTrip.cs | 18 ++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/StackExchange.Redis/BitFieldOperation.Message.cs b/src/StackExchange.Redis/BitFieldOperation.Message.cs index 0b52dd654e..f3678944c9 100644 --- a/src/StackExchange.Redis/BitFieldOperation.Message.cs +++ b/src/StackExchange.Redis/BitFieldOperation.Message.cs @@ -20,12 +20,12 @@ protected BitFieldMessageBase(int db, CommandFlags flags, RedisCommand command, } /// - /// Counts the arguments needed by a set of operations, including the key, and rejects anything - /// that cannot be written - while we can still throw at the caller rather than mid-write. + /// Counts the arguments needed by a set of operations, including the key; + /// if any of them cannot be written. /// - protected static int CountArgs(ReadOnlySpan operations, string paramName) + protected static bool TryCountArgs(ReadOnlySpan operations, out int count) { - int count = 1; // the key + count = 1; // the key var overflow = BitFieldOverflow.Wrap; foreach (ref readonly var op in operations) { @@ -45,13 +45,22 @@ protected static int CountArgs(ReadOnlySpan operations, strin count += 4; // SET/INCRBY, encoding, offset, value break; default: - throw new ArgumentException($"A default {nameof(BitFieldOperation)} is not a valid operation.", paramName); + return false; } } - return count; + return true; } + /// + /// Counts the arguments, throwing at the caller - rather than mid-write - if the operations + /// cannot be written. + /// + protected static int CountArgs(ReadOnlySpan operations, string paramName) => + TryCountArgs(operations, out var count) + ? count + : throw new ArgumentException($"A default {nameof(BitFieldOperation)} is not a valid operation.", paramName); + protected static void WriteOperations(in MessageWriter writer, ReadOnlySpan operations) { // enough for the widest thing either part writes: a framed encoding ("$3\r\ni64\r\n", 9) or @@ -73,6 +82,8 @@ protected static void WriteOperations(in MessageWriter writer, ReadOnlySpan _argCount; diff --git a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs index 5cac1fb013..6105d15dfa 100644 --- a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs +++ b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs @@ -150,6 +150,24 @@ public void DefaultOperation_IsRejectedBeforeTheMessageIsQueued() Assert.Equal("operation", single.ParamName); } + [Fact] + public void MutatingTheOperationsAfterIssue_FailsBeforeAnythingIsWritten() + { + // the batch form aliases the caller's memory rather than copying it, and the shape depends on + // the contents, so a mutation has to be caught before the header goes out + var operations = new[] { BitFieldOperation.Set(BitFieldEncoding.UInt8, 0, 1) }; + var msg = new RedisDatabase.BitFieldMessage(0, CommandFlags.None, RedisCommand.BITFIELD, (RedisKey)"k", operations); + + operations[0] = default; + + using var conn = new TestConnection(startReading: false); + var box = TaskResultBox.Create(out _, null); + msg.SetSource(box, ResultProcessor.NullableInt64); + + Assert.Throws(() => conn.WriteOutbound(msg)); + Assert.Empty(conn.GetOutboundData().ToArray()); + } + [Fact] public void ReadOnlyVariantIsNotPrimaryOnly() { From 0c99403d1e6dd1277a6921dda3a04abea816e880 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 14:26:55 +0100 Subject: [PATCH 6/7] Keep the single-operation path actually allocation-free `[operation]` for a ReadOnlySpan parameter only avoids the heap where the by-ref span constructor exists. Checking the IL per framework: net8.0 and net10.0 emit ReadOnlySpan::.ctor(!0&) over a stack local, but net461, netstandard2.0, net472 and net6.0 all emit `newarr BitFieldOperation` plus the array-based constructor - twice per call, once counting in the constructor and once writing - which is exactly the allocation that holding the operation inline was meant to avoid, on four of the six frameworks we ship. So don't take a span of operations at all on that path: the count and write steps become per-operation helpers taking `in BitFieldOperation`, and the batch form loops them over its span. The IL now shows no newarr in either message class on any framework, the only localloc being the intended scratch buffer. Also note in passing that the batch form's pre-write re-check becomes redundant once the write path serializes on the calling thread, since that removes the interval in which the aliased operations could be mutated at all. --- .../BitFieldOperation.Message.cs | 155 +++++++++++------- 1 file changed, 92 insertions(+), 63 deletions(-) diff --git a/src/StackExchange.Redis/BitFieldOperation.Message.cs b/src/StackExchange.Redis/BitFieldOperation.Message.cs index f3678944c9..4b2ba6b60e 100644 --- a/src/StackExchange.Redis/BitFieldOperation.Message.cs +++ b/src/StackExchange.Redis/BitFieldOperation.Message.cs @@ -20,8 +20,34 @@ protected BitFieldMessageBase(int db, CommandFlags flags, RedisCommand command, } /// - /// Counts the arguments needed by a set of operations, including the key; - /// if any of them cannot be written. + /// Accumulates the arguments needed by one operation, tracking the sticky overflow mode; + /// if it cannot be written. + /// + protected static bool TryCountArgs(in BitFieldOperation operation, ref BitFieldOverflow overflow, ref int count) + { + switch (operation.Kind) + { + case BitFieldOperation.OperationKind.Get: + count += 3; // GET, encoding, offset + return true; + case BitFieldOperation.OperationKind.Set: + case BitFieldOperation.OperationKind.IncrementBy: + if (operation.Overflow != overflow) + { + overflow = operation.Overflow; + count += 2; // OVERFLOW, mode + } + + count += 4; // SET/INCRBY, encoding, offset, value + return true; + default: + return false; + } + } + + /// + /// Counts the arguments needed by a set of operations, including the key; + /// if any of them cannot be written. /// protected static bool TryCountArgs(ReadOnlySpan operations, out int count) { @@ -29,24 +55,7 @@ protected static bool TryCountArgs(ReadOnlySpan operations, o var overflow = BitFieldOverflow.Wrap; foreach (ref readonly var op in operations) { - switch (op.Kind) - { - case BitFieldOperation.OperationKind.Get: - count += 3; // GET, encoding, offset - break; - case BitFieldOperation.OperationKind.Set: - case BitFieldOperation.OperationKind.IncrementBy: - if (op.Overflow != overflow) - { - overflow = op.Overflow; - count += 2; // OVERFLOW, mode - } - - count += 4; // SET/INCRBY, encoding, offset, value - break; - default: - return false; - } + if (!TryCountArgs(in op, ref overflow, ref count)) return false; } return true; @@ -57,9 +66,10 @@ protected static bool TryCountArgs(ReadOnlySpan operations, o /// cannot be written. /// protected static int CountArgs(ReadOnlySpan operations, string paramName) => - TryCountArgs(operations, out var count) - ? count - : throw new ArgumentException($"A default {nameof(BitFieldOperation)} is not a valid operation.", paramName); + TryCountArgs(operations, out var count) ? count : throw InvalidOperation(paramName); + + protected static ArgumentException InvalidOperation(string paramName) => + new($"A default {nameof(BitFieldOperation)} is not a valid operation.", paramName); protected static void WriteOperations(in MessageWriter writer, ReadOnlySpan operations) { @@ -69,50 +79,58 @@ protected static void WriteOperations(in MessageWriter writer, ReadOnlySpan + /// Writes one operation, emitting the OVERFLOW token only when the sticky mode changes. + /// + protected static void WriteOperation(in MessageWriter writer, in BitFieldOperation operation, ref BitFieldOverflow overflow, Span scratch) + { + if (operation.Kind != BitFieldOperation.OperationKind.Get && operation.Overflow != overflow) + { + overflow = operation.Overflow; + writer.WriteRaw("$8\r\nOVERFLOW\r\n"u8); + switch (overflow) { - case BitFieldOperation.OperationKind.Get: - writer.WriteRaw("$3\r\nGET\r\n"u8); - break; - case BitFieldOperation.OperationKind.Set: - writer.WriteRaw("$3\r\nSET\r\n"u8); + case BitFieldOverflow.Saturate: + writer.WriteRaw("$3\r\nSAT\r\n"u8); break; - case BitFieldOperation.OperationKind.IncrementBy: - writer.WriteRaw("$6\r\nINCRBY\r\n"u8); + case BitFieldOverflow.Fail: + writer.WriteRaw("$4\r\nFAIL\r\n"u8); break; default: - // unreachable: the callers check the operations before writing the header. - // Guessing here would be worse than failing - a wrong sub-command corrupts - // data, and one of the wrong arity corrupts the connection - throw new InvalidOperationException($"A default {nameof(BitFieldOperation)} is not a valid operation."); + // shape-neutral (the count comes from the transition, not the mode), so + // the server's own default is the safe answer here + writer.WriteRaw("$4\r\nWRAP\r\n"u8); + break; } + } - op.Encoding.Write(in writer, scratch); - op.Offset.Write(in writer, scratch); - if (op.Kind != BitFieldOperation.OperationKind.Get) - { - writer.WriteBulkString(op.Value); - } + switch (operation.Kind) + { + case BitFieldOperation.OperationKind.Get: + writer.WriteRaw("$3\r\nGET\r\n"u8); + break; + case BitFieldOperation.OperationKind.Set: + writer.WriteRaw("$3\r\nSET\r\n"u8); + break; + case BitFieldOperation.OperationKind.IncrementBy: + writer.WriteRaw("$6\r\nINCRBY\r\n"u8); + break; + default: + // unreachable: the callers check the operations before writing the header. + // Guessing here would be worse than failing - a wrong sub-command corrupts + // data, and one of the wrong arity corrupts the connection + throw new InvalidOperationException($"A default {nameof(BitFieldOperation)} is not a valid operation."); + } + + operation.Encoding.Write(in writer, scratch); + operation.Offset.Write(in writer, scratch); + if (operation.Kind != BitFieldOperation.OperationKind.Get) + { + writer.WriteBulkString(operation.Value); } } } @@ -135,7 +153,9 @@ protected override void WriteImpl(in MessageWriter writer) // we alias the caller's memory rather than copying it, and here - unlike the messages that // hold a RedisValue[] - the *shape* depends on the contents, so a mutation between issue // and write would desync the frame. Re-check before the header goes out, so that a - // violated contract fails with nothing written rather than corrupting the connection + // violated contract fails with nothing written rather than corrupting the connection. + // Note this becomes redundant once the write path serializes on the calling thread: with + // no deferral there is no interval in which reasonable code could mutate the operations var operations = _operations.Span; if (!TryCountArgs(operations, out var count) || count != _argCount) { @@ -160,14 +180,23 @@ public BitFieldSingleMessage(int db, CommandFlags flags, RedisCommand command, i : base(db, flags, command, key) { _operation = operation; - _argCount = CountArgs([operation], nameof(operation)); + + // deliberately not `[operation]`: a collection expression of one element compiles to a + // heap array on the target frameworks without the by-ref span constructor, which would + // undo the point of holding the operation inline + _argCount = 1; // the key + var overflow = BitFieldOverflow.Wrap; + if (!TryCountArgs(in operation, ref overflow, ref _argCount)) throw InvalidOperation(nameof(operation)); } protected override void WriteImpl(in MessageWriter writer) { writer.WriteHeader(Command, _argCount); writer.Write(Key); - WriteOperations(in writer, [_operation]); + + Span scratch = stackalloc byte[Format.MaxInt64TextLen + 1]; + var overflow = BitFieldOverflow.Wrap; + WriteOperation(in writer, in _operation, ref overflow, scratch); } public override int ArgCount => _argCount; From 32d2fcaf5d2f4bf53cd3cc3814e00077326b70b9 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 25 Aug 2026 14:35:48 +0100 Subject: [PATCH 7/7] Give an all-GET BITFIELD the read-only retry category Routing and replay safety are separate axes, and the fallback path conflated them. When every sub-operation is a GET but BITFIELD_RO is unavailable - a server below 6.0, or the command disabled or renamed in the command map - the message went out as BITFIELD and inherited its WriteAccumulating default, despite there being nothing to accumulate. The server insisting BITFIELD is a write governs which servers will accept it, not whether replaying it is safe. Apply CommandRetryReadOnly for any all-GET payload, whichever of the two commands we end up issuing; BITFIELD_RO already had it by default, so this only changes the fallback. An explicitly requested category still wins, as WithCategory has always done. The decision is now a pure static taking the availability as a parameter, which is the only way to observe it - the flags a message ends up with are not visible from outside - so all four payload shapes plus the explicit-category case are covered directly. --- src/StackExchange.Redis/RedisDatabase.cs | 33 +++++++++++++++---- .../RoundTripUnitTests/BitFieldRoundTrip.cs | 25 ++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index 642e1b712a..011c38c80f 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -5439,13 +5439,13 @@ private Message GetBitFieldMessage(in RedisKey key, ReadOnlyMemory + /// Chooses the command, and the retry category the payload deserves - which is a separate axis + /// from routing: the server treats BITFIELD as a write however read-only its sub-operations are, + /// but that governs which servers will accept it, not whether replaying it is safe. + /// + internal static RedisCommand SelectBitFieldCommand(bool allGet, bool anyIncrement, bool readOnlyAvailable, ref CommandFlags flags) + { + if (allGet) + { + // nothing to replay, whichever of the two commands we end up issuing + flags = flags.WithCategory(CommandFlags.CommandRetryReadOnly); + return readOnlyAvailable ? RedisCommand.BITFIELD_RO : RedisCommand.BITFIELD; } - else if (!anyIncrement) + + if (!anyIncrement) { // SET is positional, so a replay lands on the same value; only INCRBY compounds flags = flags.WithCategory(CommandFlags.CommandRetryWriteLastWins); diff --git a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs index 6105d15dfa..8bca76ec80 100644 --- a/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs +++ b/tests/StackExchange.Redis.Tests/RoundTripUnitTests/BitFieldRoundTrip.cs @@ -168,6 +168,31 @@ public void MutatingTheOperationsAfterIssue_FailsBeforeAnythingIsWritten() Assert.Empty(conn.GetOutboundData().ToArray()); } + [Theory] + // all-GET: no side effects to replay, whichever command we end up issuing + [InlineData(true, false, true, "BITFIELD_RO", CommandFlags.CommandRetryReadOnly)] + [InlineData(true, false, false, "BITFIELD", CommandFlags.CommandRetryReadOnly)] + // SET only: a replay lands on the same value, because the offset is positional + [InlineData(false, false, false, "BITFIELD", CommandFlags.CommandRetryWriteLastWins)] + // anything with an INCRBY compounds, so it keeps BITFIELD's accumulating default + [InlineData(false, true, false, "BITFIELD", CommandFlags.None)] + public void CommandAndRetryCategoryFollowThePayload(bool allGet, bool anyIncrement, bool readOnlyAvailable, string expectedCommand, CommandFlags expectedCategory) + { + var flags = CommandFlags.None; + Assert.Equal(expectedCommand, RedisDatabase.SelectBitFieldCommand(allGet, anyIncrement, readOnlyAvailable, ref flags).ToString()); + + // CommandFlags.None here means "no opinion", leaving BITFIELD's own accumulating default in place + Assert.Equal(expectedCategory, flags & Message.MaskRetryCategory); + } + + [Fact] + public void AnExplicitRetryCategoryIsNotOverridden() + { + var flags = CommandFlags.CommandRetryNever; + Assert.Equal(RedisCommand.BITFIELD_RO, RedisDatabase.SelectBitFieldCommand(allGet: true, anyIncrement: false, readOnlyAvailable: true, ref flags)); + Assert.Equal(CommandFlags.CommandRetryNever, flags & Message.MaskRetryCategory); + } + [Fact] public void ReadOnlyVariantIsNotPrimaryOnly() {