diff --git a/docs/exp/StringToRedisValue.md b/docs/exp/StringToRedisValue.md new file mode 100644 index 000000000..961e966f2 --- /dev/null +++ b/docs/exp/StringToRedisValue.md @@ -0,0 +1,18 @@ +StringToRedisValue +- + +Oh dear, it looks like you fell into a `DEBUG` warning. **Don't panic!** You're probably fine. + +Short version, *if you're in the core library when seeing this*: + +1. If you're *writing* data (to the server), you're probably fine - just add `.AsRedisValue()` to say "trust me bro"; all strings are valid as `RedisValue` content - you're fine. If the value + is a literal (i.e. `"some const string"`) or a `const`, you might want to look at `RedisLiterals` and the `.FromRaw(...)` API, which can be more efficient, but otherwise: you do you! +2. However, if you're *reading* data (from the server), then you need to be careful - there's a good chance you've used `reader.ReadString()` when you meant to use `reader.ReadRedisValue()`. + +The impact of *2* would be that *string* payloads work fine, but you silently corrupt *binary* payloads. Additionally, if the value is *small* or an obvious integer, `RedisValue` may +be more efficient (zero-allocation), and even for non-trivial text-like payloads: using `RedisValue` defers the UTF8 decode cost to the caller. So: ask yourself - is this actually +a `string` in all cases? + +This warning only exists in local `DEBUG` builds - it doesn't exist in the public package. + +If you're actually in an auxiliary package (tests, benchmarks, etc): we care less. Feel free to add `$(NoWarn);StringToRedisValue` to the csproj. \ No newline at end of file diff --git a/src/RESPite.Benchmark/RESPite.Benchmark.csproj b/src/RESPite.Benchmark/RESPite.Benchmark.csproj index d2e755738..8397870af 100644 --- a/src/RESPite.Benchmark/RESPite.Benchmark.csproj +++ b/src/RESPite.Benchmark/RESPite.Benchmark.csproj @@ -16,6 +16,7 @@ 2025 - $([System.DateTime]::Now.Year) Marc Gravell 3 + $(NoWarn);StringToRedisValue diff --git a/src/RESPite/Shared/Experiments.cs b/src/RESPite/Shared/Experiments.cs index 1d9a091fa..304b1f624 100644 --- a/src/RESPite/Shared/Experiments.cs +++ b/src/RESPite/Shared/Experiments.cs @@ -5,7 +5,8 @@ // where SomeFeature has the next label, for example "SER042", and /docs/exp/SER042.md exists internal static class Experiments { - public const string UrlFormat = "https://stackexchange.github.io/StackExchange.Redis/exp/"; + // note: {0} is substituted with the DiagnosticId by the analyzer, e.g. .../exp/SER002 + public const string UrlFormat = "https://stackexchange.github.io/StackExchange.Redis/exp/{0}"; // ReSharper disable InconsistentNaming public const string Server_8_4 = "SER002"; @@ -13,7 +14,12 @@ internal static class Experiments public const string Respite = "SER004"; public const string UnitTesting = "SER005"; public const string Server_8_8 = "SER006"; + // ReSharper restore InconsistentNaming + + // this one is not a real experiment; it exists to help me + // spot bad API uses, via a DEBUG symbol + public const string StringToRedisValue = "StringToRedisValue"; } } diff --git a/src/StackExchange.Redis/APITypes/ClientKillFilter.cs b/src/StackExchange.Redis/APITypes/ClientKillFilter.cs index 3d1883549..ac6ab4e8e 100644 --- a/src/StackExchange.Redis/APITypes/ClientKillFilter.cs +++ b/src/StackExchange.Redis/APITypes/ClientKillFilter.cs @@ -152,17 +152,17 @@ internal List ToList(bool withReplicaCommands) if (Username != null) { parts.Add(RedisLiterals.USERNAME); - parts.Add(Username); + parts.Add(Username.AsRedisValue()); } if (Endpoint != null) { parts.Add(RedisLiterals.ADDR); - parts.Add((RedisValue)Format.ToString(Endpoint)); + parts.Add(Format.ToString(Endpoint).AsRedisValue()); } if (ServerEndpoint != null) { parts.Add(RedisLiterals.LADDR); - parts.Add((RedisValue)Format.ToString(ServerEndpoint)); + parts.Add(Format.ToString(ServerEndpoint).AsRedisValue()); } if (SkipMe != null) { diff --git a/src/StackExchange.Redis/APITypes/GeoPosition.cs b/src/StackExchange.Redis/APITypes/GeoPosition.cs index 6e53b3e32..b3b3cdad7 100644 --- a/src/StackExchange.Redis/APITypes/GeoPosition.cs +++ b/src/StackExchange.Redis/APITypes/GeoPosition.cs @@ -7,12 +7,12 @@ namespace StackExchange.Redis; /// public readonly struct GeoPosition : IEquatable { - internal static string GetRedisUnit(GeoUnit unit) => unit switch + internal static RedisValue GetRedisUnit(GeoUnit unit) => unit switch { - GeoUnit.Meters => "m", - GeoUnit.Kilometers => "km", - GeoUnit.Miles => "mi", - GeoUnit.Feet => "ft", + GeoUnit.Meters => RedisLiterals.m, + GeoUnit.Kilometers => RedisLiterals.km, + GeoUnit.Miles => RedisLiterals.mi, + GeoUnit.Feet => RedisLiterals.ft, _ => throw new ArgumentOutOfRangeException(nameof(unit)), }; diff --git a/src/StackExchange.Redis/ArrayGrepRequest.cs b/src/StackExchange.Redis/ArrayGrepRequest.cs index 5fd7b0dd2..bc691053f 100644 --- a/src/StackExchange.Redis/ArrayGrepRequest.cs +++ b/src/StackExchange.Redis/ArrayGrepRequest.cs @@ -367,7 +367,7 @@ private static void AddIndex(in MessageWriter writer, RedisArrayIndex? index, Re protected override void WriteImpl(in MessageWriter writer) { writer.WriteHeader(Command, ArgCount); - writer.WriteBulkString(key); + writer.Write(key); if (request.IsReversed) { AddIndex(writer, request.End, "$1\r\n+\r\n"u8); diff --git a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs index a89911dc4..42a85b710 100644 --- a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs +++ b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs @@ -193,7 +193,7 @@ public static async Task ReplayAsync(Stream @out, Stream @in, Action(); - return Array.ConvertAll(values, x => (RedisValue)x); + return Array.ConvertAll(values, x => x.AsRedisValue()); } + // just like the implicit conversion operator, but by making it + // explicit, we're making it very clear that this is intentional, + // and avoiding the DEBUG check + internal static RedisValue AsRedisValue(this string? value) + => value is null ? RedisValue.Null : new(value); + /// /// Create an array of strings from an array of values. /// diff --git a/src/StackExchange.Redis/Increx.IncrexMessage.cs b/src/StackExchange.Redis/Increx.IncrexMessage.cs index dc8421c24..6941e60e6 100644 --- a/src/StackExchange.Redis/Increx.IncrexMessage.cs +++ b/src/StackExchange.Redis/Increx.IncrexMessage.cs @@ -32,7 +32,7 @@ public override int ArgCount protected override void WriteImpl(in MessageWriter writer) { writer.WriteHeader(Command, ArgCount); - writer.WriteBulkString(Key); + writer.Write(Key); WriteIncrementKindAndValue(writer); WriteBounds(writer); WriteOptions(writer); diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs index 6efd8d68e..952fe0ed3 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs @@ -938,10 +938,10 @@ protected RedisValue ToInner(RedisValue outer) => RedisKey.ConcatenateBytes(Prefix, null, (byte[]?)outer); protected RedisValue SortByToInner(RedisValue outer) => - (outer == "nosort") ? outer : ToInner(outer); + (outer == RedisLiterals.nosort) ? outer : ToInner(outer); protected RedisValue SortGetToInner(RedisValue outer) => - (outer == "#") ? outer : ToInner(outer); + (outer == RedisLiterals.HashSymbol) ? outer : ToInner(outer); [return: NotNullIfNotNull("outer")] protected RedisValue[]? SortGetToInner(RedisValue[]? outer) diff --git a/src/StackExchange.Redis/MessageWriter.cs b/src/StackExchange.Redis/MessageWriter.cs index a95edd776..93e63b58d 100644 --- a/src/StackExchange.Redis/MessageWriter.cs +++ b/src/StackExchange.Redis/MessageWriter.cs @@ -80,12 +80,29 @@ internal void Write(in RedisChannel channel) internal void WriteBulkString(in RedisValue value) => WriteBulkString(value, _writer); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void WriteBulkString(string? value) + { + if (value is null) + { + WriteRaw(NullBulkString); + } + else if (value.Length is 0) + { + WriteRaw(EmptyBulkString); + } + else + { + WriteUnifiedPrefixedString(_writer, null, value); + } + } + internal static void WriteBulkString(in RedisValue value, IBufferWriter writer) { switch (value.Type) { case RedisValue.StorageType.Null: - WriteUnifiedBlob(writer, (byte[]?)null); + writer.Write(NullBulkString); break; case RedisValue.StorageType.Int64: WriteUnifiedInt64(writer, value.OverlappedValueInt64); diff --git a/src/StackExchange.Redis/PublicAPI/Debug/PublicAPI.txt b/src/StackExchange.Redis/PublicAPI/Debug/PublicAPI.txt new file mode 100644 index 000000000..690e2d9c9 --- /dev/null +++ b/src/StackExchange.Redis/PublicAPI/Debug/PublicAPI.txt @@ -0,0 +1,2 @@ +#nullable enable +[StringToRedisValue]static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(string? value) -> StackExchange.Redis.RedisValue diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt index cc104c577..7e8381c6e 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt @@ -1837,7 +1837,6 @@ static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.Redi static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(int? value) -> StackExchange.Redis.RedisValue static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(long value) -> StackExchange.Redis.RedisValue static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(long? value) -> StackExchange.Redis.RedisValue -static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(string? value) -> StackExchange.Redis.RedisValue static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(System.Memory value) -> StackExchange.Redis.RedisValue static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(System.ReadOnlyMemory value) -> StackExchange.Redis.RedisValue static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(uint value) -> StackExchange.Redis.RedisValue diff --git a/src/StackExchange.Redis/PublicAPI/Release/PublicAPI.txt b/src/StackExchange.Redis/PublicAPI/Release/PublicAPI.txt new file mode 100644 index 000000000..bb19df634 --- /dev/null +++ b/src/StackExchange.Redis/PublicAPI/Release/PublicAPI.txt @@ -0,0 +1,2 @@ +#nullable enable +static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(string? value) -> StackExchange.Redis.RedisValue diff --git a/src/StackExchange.Redis/RedisDatabase.VectorSets.cs b/src/StackExchange.Redis/RedisDatabase.VectorSets.cs index bcaa0e6c0..7b29b0da9 100644 --- a/src/StackExchange.Redis/RedisDatabase.VectorSets.cs +++ b/src/StackExchange.Redis/RedisDatabase.VectorSets.cs @@ -86,7 +86,7 @@ public bool VectorSetRemove(RedisKey key, RedisValue member, CommandFlags flags public bool VectorSetSetAttributesJson(RedisKey key, RedisValue member, string attributesJson, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson); + var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.Boolean); } @@ -178,7 +178,7 @@ public Task VectorSetRemoveAsync(RedisKey key, RedisValue member, CommandF public Task VectorSetSetAttributesJsonAsync(RedisKey key, RedisValue member, string attributesJson, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson); + var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.Boolean); } @@ -205,7 +205,7 @@ static RedisValue GetTerminator(RedisValue value, Exclude exclude, bool isStart) if (value.IsNull) return isStart ? RedisLiterals.MinusSymbol : RedisLiterals.PlusSymbol; var mask = isStart ? Exclude.Start : Exclude.Stop; var isExclusive = (exclude & mask) != 0; - return (isExclusive ? "(" : "[") + value; + return ((isExclusive ? "(" : "[") + value).AsRedisValue(); } var from = GetTerminator(start, exclude, true); diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index f06bcc699..b217712bd 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -1311,7 +1311,8 @@ private sealed class KeyMigrateCommandMessage : Message.CommandKeyBase // MIGRAT private readonly MigrateOptions migrateOptions; private readonly int timeoutMilliseconds; private readonly int toDatabase; - private readonly RedisValue toHost, toPort; + private readonly string toHost; + private readonly int toPort; public KeyMigrateCommandMessage(int db, RedisKey key, EndPoint toServer, int toDatabase, int timeoutMilliseconds, MigrateOptions migrateOptions, CommandFlags flags) : base(db, flags, RedisCommand.MIGRATE, key) @@ -1319,7 +1320,7 @@ public KeyMigrateCommandMessage(int db, RedisKey key, EndPoint toServer, int toD if (toServer == null) throw new ArgumentNullException(nameof(toServer)); if (!Format.TryGetHostPort(toServer, out string? toHost, out int? toPort)) throw new ArgumentException($"Couldn't get host and port from {toServer}", nameof(toServer)); this.toHost = toHost; - this.toPort = toPort; + this.toPort = toPort.Value; if (toDatabase < 0) throw new ArgumentOutOfRangeException(nameof(toDatabase)); this.toDatabase = toDatabase; this.timeoutMilliseconds = timeoutMilliseconds; @@ -4392,7 +4393,7 @@ private static RedisValue GetRange(double value, Exclude exclude, bool isStart) { if ((exclude & Exclude.Stop) == 0) return value; // inclusive is default } - return "(" + Format.ToString(value); // '(' prefix means exclusive + return ("(" + Format.ToString(value)).AsRedisValue(); // '(' prefix means exclusive } private Message GetRestoreMessage(RedisKey key, byte[] value, TimeSpan? expiry, CommandFlags flags) @@ -4898,7 +4899,7 @@ private Message GetStreamPendingMessagesMessage(RedisKey key, RedisValue groupNa values[offset++] = groupName; if (minIdleTimeInMs is not null) { - values[offset++] = "IDLE"; + values[offset++] = RedisLiterals.IDLE; values[offset++] = minIdleTimeInMs; } values[offset++] = minId ?? StreamConstants.ReadMinValue; @@ -5480,7 +5481,7 @@ protected override void WriteImpl(in MessageWriter writer) { writer.WriteHeader(Command, 2); writer.WriteRaw("$4\r\nLOAD\r\n"u8); - writer.WriteBulkString((RedisValue)Script); + writer.WriteBulkString(Script); } public override int ArgCount => 2; } @@ -5675,12 +5676,12 @@ protected override void WriteImpl(in MessageWriter writer) else if (asciiHash != null) { writer.WriteHeader(RedisCommand.EVALSHA, 2 + keys.Length + values.Length); - writer.WriteBulkString((RedisValue)asciiHash); + writer.WriteBulkString(asciiHash); } else { writer.WriteHeader(RedisCommand.EVAL, 2 + keys.Length + values.Length); - writer.WriteBulkString((RedisValue)script); + writer.WriteBulkString(script); } writer.WriteBulkString(keys.Length); for (int i = 0; i < keys.Length; i++) @@ -5757,15 +5758,15 @@ private static Message CreateSortedSetRangeStoreMessage( RedisValue formattedStart = exclude switch { - Exclude.Both or Exclude.Start => $"({start}", - _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{start}", + Exclude.Both or Exclude.Start => $"({start}".AsRedisValue(), + _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{start}".AsRedisValue(), _ => start, }; RedisValue formattedStop = exclude switch { - Exclude.Both or Exclude.Stop => $"({stop}", - _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{stop}", + Exclude.Both or Exclude.Stop => $"({stop}".AsRedisValue(), + _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{stop}".AsRedisValue(), _ => stop, }; diff --git a/src/StackExchange.Redis/RedisKey.cs b/src/StackExchange.Redis/RedisKey.cs index 7550e7087..85f9ced59 100644 --- a/src/StackExchange.Redis/RedisKey.cs +++ b/src/StackExchange.Redis/RedisKey.cs @@ -222,7 +222,7 @@ public override int GetHashCode() internal RedisValue AsRedisValue() { - if (KeyPrefix == null && KeyValue is string keyString) return keyString; + if (KeyPrefix == null && KeyValue is string keyString) return keyString.AsRedisValue(); return (byte[]?)this; } diff --git a/src/StackExchange.Redis/RedisLiterals.cs b/src/StackExchange.Redis/RedisLiterals.cs index 52790f187..9709eb02e 100644 --- a/src/StackExchange.Redis/RedisLiterals.cs +++ b/src/StackExchange.Redis/RedisLiterals.cs @@ -48,9 +48,11 @@ public static readonly RedisValue GETKEYS = RedisValue.FromRaw("GETKEYS"u8), GETNAME = RedisValue.FromRaw("GETNAME"u8), GT = RedisValue.FromRaw("GT"u8), + HashSymbol = RedisValue.FromRaw("#"u8), HISTORY = RedisValue.FromRaw("HISTORY"u8), ID = RedisValue.FromRaw("ID"u8), IDX = RedisValue.FromRaw("IDX"u8), + IDLE = RedisValue.FromRaw("IDLE"u8), IDLETIME = RedisValue.FromRaw("IDLETIME"u8), IDMP = RedisValue.FromRaw("IDMP"u8), IDMPAUTO = RedisValue.FromRaw("IDMPAUTO"u8), @@ -75,8 +77,10 @@ public static readonly RedisValue MIN = RedisValue.FromRaw("MIN"u8), MINMATCHLEN = RedisValue.FromRaw("MINMATCHLEN"u8), MODULE = RedisValue.FromRaw("MODULE"u8), + NO = RedisValue.FromRaw("NO"u8), NODES = RedisValue.FromRaw("NODES"u8), NOSAVE = RedisValue.FromRaw("NOSAVE"u8), + nosort = RedisValue.FromRaw("nosort"u8), NOT = RedisValue.FromRaw("NOT"u8), NOVALUES = RedisValue.FromRaw("NOVALUES"u8), NUMPAT = RedisValue.FromRaw("NUMPAT"u8), @@ -165,7 +169,6 @@ public static readonly RedisValue // misc (config, etc) databases = RedisValue.FromRaw("databases"u8), - master = RedisValue.FromRaw("master"u8), no = RedisValue.FromRaw("no"u8), normal = RedisValue.FromRaw("normal"u8), pubsub = RedisValue.FromRaw("pubsub"u8), diff --git a/src/StackExchange.Redis/RedisServer.cs b/src/StackExchange.Redis/RedisServer.cs index ef58d3bbf..61918347f 100644 --- a/src/StackExchange.Redis/RedisServer.cs +++ b/src/StackExchange.Redis/RedisServer.cs @@ -55,13 +55,13 @@ public bool AllowReplicaWrites public void ClientKill(EndPoint endpoint, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.CLIENT, RedisLiterals.KILL, (RedisValue)Format.ToString(endpoint)); + var msg = Message.Create(-1, flags, RedisCommand.CLIENT, RedisLiterals.KILL, Format.ToString(endpoint).AsRedisValue()); ExecuteSync(msg, ResultProcessor.DemandOK); } public Task ClientKillAsync(EndPoint endpoint, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.CLIENT, RedisLiterals.KILL, (RedisValue)Format.ToString(endpoint)); + var msg = Message.Create(-1, flags, RedisCommand.CLIENT, RedisLiterals.KILL, Format.ToString(endpoint).AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.DemandOK); } @@ -607,14 +607,14 @@ internal static Message CreateReplicaOfMessage(ServerEndPoint sendMessageTo, End RedisValue host, port; if (primaryEndpoint == null) { - host = "NO"; - port = "ONE"; + host = RedisLiterals.NO; + port = RedisLiterals.ONE; } else { if (Format.TryGetHostPort(primaryEndpoint, out string? hostRaw, out int? portRaw)) { - host = hostRaw; + host = hostRaw.AsRedisValue(); port = portRaw; } else @@ -934,61 +934,61 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes public EndPoint? SentinelGetMasterAddressByName(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.GETMASTERADDRBYNAME, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.GETMASTERADDRBYNAME, serviceName.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.SentinelPrimaryEndpoint); } public Task SentinelGetMasterAddressByNameAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.GETMASTERADDRBYNAME, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.GETMASTERADDRBYNAME, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.SentinelPrimaryEndpoint); } public EndPoint[] SentinelGetSentinelAddresses(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, serviceName.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.SentinelAddressesEndPoints, defaultValue: Array.Empty()); } public Task SentinelGetSentinelAddressesAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.SentinelAddressesEndPoints, defaultValue: Array.Empty()); } public EndPoint[] SentinelGetReplicaAddresses(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, serviceName.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.SentinelAddressesEndPoints, defaultValue: Array.Empty()); } public Task SentinelGetReplicaAddressesAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.SentinelAddressesEndPoints, defaultValue: Array.Empty()); } public KeyValuePair[] SentinelMaster(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.MASTER, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.MASTER, serviceName.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.StringPairInterleaved, defaultValue: Array.Empty>()); } public Task[]> SentinelMasterAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.MASTER, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.MASTER, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.StringPairInterleaved, defaultValue: Array.Empty>()); } public void SentinelFailover(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.FAILOVER, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.FAILOVER, serviceName.AsRedisValue()); ExecuteSync(msg, ResultProcessor.DemandOK); } public Task SentinelFailoverAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.FAILOVER, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.FAILOVER, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.DemandOK); } @@ -1010,7 +1010,7 @@ KeyValuePair[][] IServer.SentinelSlaves(string serviceName, Comm public KeyValuePair[][] SentinelReplicas(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, serviceName.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.SentinelArrayOfArrays, defaultValue: Array.Empty[]>()); } @@ -1020,19 +1020,19 @@ Task[][]> IServer.SentinelSlavesAsync(string servic public Task[][]> SentinelReplicasAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, Features.ReplicaCommands ? RedisLiterals.REPLICAS : RedisLiterals.SLAVES, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.SentinelArrayOfArrays, defaultValue: Array.Empty[]>()); } public KeyValuePair[][] SentinelSentinels(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, serviceName.AsRedisValue()); return ExecuteSync(msg, ResultProcessor.SentinelArrayOfArrays, defaultValue: Array.Empty[]>()); } public Task[][]> SentinelSentinelsAsync(string serviceName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, (RedisValue)serviceName); + var msg = Message.Create(-1, flags, RedisCommand.SENTINEL, RedisLiterals.SENTINELS, serviceName.AsRedisValue()); return ExecuteAsync(msg, ResultProcessor.SentinelArrayOfArrays, defaultValue: Array.Empty[]>()); } @@ -1096,12 +1096,12 @@ private static Message LatencyResetCommand(string[]? eventNames, CommandFlags fl case 0: return Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.RESET); case 1: - return Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.RESET, (RedisValue)eventNames[0]); + return Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.RESET, eventNames[0].AsRedisValue()); default: var arr = new RedisValue[eventNames.Length + 1]; arr[0] = RedisLiterals.RESET; for (int i = 0; i < eventNames.Length; i++) - arr[i + 1] = eventNames[i]; + arr[i + 1] = eventNames[i].AsRedisValue(); return Message.Create(-1, flags, RedisCommand.LATENCY, arr); } } @@ -1119,13 +1119,13 @@ public long LatencyReset(string[]? eventNames = null, CommandFlags flags = Comma public Task LatencyHistoryAsync(string eventName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.HISTORY, (RedisValue)eventName); + var msg = Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.HISTORY, eventName.AsRedisValue()); return ExecuteAsync(msg, LatencyHistoryEntry.ToArray, defaultValue: Array.Empty()); } public LatencyHistoryEntry[] LatencyHistory(string eventName, CommandFlags flags = CommandFlags.None) { - var msg = Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.HISTORY, (RedisValue)eventName); + var msg = Message.Create(-1, flags, RedisCommand.LATENCY, RedisLiterals.HISTORY, eventName.AsRedisValue()); return ExecuteSync(msg, LatencyHistoryEntry.ToArray, defaultValue: Array.Empty()); } diff --git a/src/StackExchange.Redis/RedisValue.cs b/src/StackExchange.Redis/RedisValue.cs index fb2a6c0b0..5792abd60 100644 --- a/src/StackExchange.Redis/RedisValue.cs +++ b/src/StackExchange.Redis/RedisValue.cs @@ -11,6 +11,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; +using RESPite; namespace StackExchange.Redis { @@ -780,17 +781,17 @@ internal static RedisValue TryParse(object? obj, out bool valid) switch (obj) { case null: return Null; - case string v: return v; + case string v: return new(v); case int v: return v; case uint v: return v; - case double v: return v; - case byte[] v: return v; + case double v: return new(v); + case byte[] v: return new(v); case bool v: return v; case long v: return v; case ulong v: return v; - case float v: return v; - case ReadOnlyMemory v: return v; - case Memory v: return v; + case float v: return new(v); + case ReadOnlyMemory v: return new(v); + case Memory v: return new(v); case RedisValue v: return v; default: valid = false; @@ -894,7 +895,13 @@ public static implicit operator RedisValue(ReadOnlySequence value) /// Creates a new from an . /// /// The to convert to a . +#if DEBUG + // if you're here to ask "why is my compiler warning me": go read StringToRedisValue.md + [Experimental(Experiments.StringToRedisValue, UrlFormat = Experiments.UrlFormat)] +#endif +#pragma warning disable RS0016 public static implicit operator RedisValue(string? value) => value is null ? Null : new(value); +#pragma warning restore RS0016 /// /// Creates a new from an . diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs index 55b502c40..14acb54c0 100644 --- a/src/StackExchange.Redis/ResultProcessor.cs +++ b/src/StackExchange.Redis/ResultProcessor.cs @@ -1901,7 +1901,7 @@ private static GeoRadiusResult Parse(ref RespReader reader, GeoRadiusOptions opt if (options == GeoRadiusOptions.None) { // Without any WITH option specified, the command just returns a linear array like ["New York","Milan","Paris"]. - return new GeoRadiusResult(reader.ReadString(), null, null, null); + return new GeoRadiusResult(reader.ReadRedisValue(), null, null, null); } // If WITHCOORD, WITHDIST or WITHHASH options are specified, the command returns an array of arrays, where each sub-array represents a single item. @@ -1913,12 +1913,12 @@ private static GeoRadiusResult Parse(ref RespReader reader, GeoRadiusOptions opt reader.MoveNext(); // Move to first element in the sub-array // the first item in the sub-array is always the name of the returned item. - var member = reader.ReadString(); + var member = reader.ReadRedisValue(); /* The other information is returned in the following order as successive elements of the sub-array. The distance from the center as a floating point number, in the same unit specified in the radius. The geohash integer. -The coordinates as a two items x,y array (longitude,latitude). +The coordinates as an array of two items x,y (longitude,latitude). */ double? distance = null; GeoPosition? position = null; diff --git a/src/StackExchange.Redis/Role.cs b/src/StackExchange.Redis/Role.cs index 587194026..d69b8ea54 100644 --- a/src/StackExchange.Redis/Role.cs +++ b/src/StackExchange.Redis/Role.cs @@ -8,6 +8,8 @@ namespace StackExchange.Redis /// public abstract class Role { + internal const string LabelForMaster = "master"; + internal static Unknown Null { get; } = new Unknown(""); /// @@ -67,7 +69,7 @@ internal Replica(string ip, int port, long offset) public override string ToString() => $"{Ip}:{Port} - {ReplicationOffset}"; } - internal Master(long offset, ICollection replicas) : base(RedisLiterals.master!) + internal Master(long offset, ICollection replicas) : base(LabelForMaster) { ReplicationOffset = offset; Replicas = replicas; diff --git a/src/StackExchange.Redis/ServerEndPoint.cs b/src/StackExchange.Redis/ServerEndPoint.cs index eca06f30d..f191c1c3b 100644 --- a/src/StackExchange.Redis/ServerEndPoint.cs +++ b/src/StackExchange.Redis/ServerEndPoint.cs @@ -1020,14 +1020,14 @@ private async Task HandshakeAsync(PhysicalConnection connection, ILogger? log) if (!string.IsNullOrWhiteSpace(user) && Multiplexer.CommandMap.IsAvailable(RedisCommand.AUTH)) { log?.LogInformationAuthenticatingUserPassword(new(this)); - msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.AUTH, (RedisValue)user, (RedisValue)password); + msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.AUTH, user.AsRedisValue(), password.AsRedisValue()); msg.SetInternalCall(); await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.DemandOK).ForAwait(); } else if (!string.IsNullOrWhiteSpace(password) && Multiplexer.CommandMap.IsAvailable(RedisCommand.AUTH)) { log?.LogInformationAuthenticatingPassword(new(this)); - msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.AUTH, (RedisValue)password); + msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.AUTH, password.AsRedisValue()); msg.SetInternalCall(); await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.DemandOK).ForAwait(); } @@ -1037,7 +1037,7 @@ private async Task HandshakeAsync(PhysicalConnection connection, ILogger? log) if (!string.IsNullOrWhiteSpace(clientName)) { log?.LogInformationSettingClientName(new(this), clientName); - msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.CLIENT, RedisLiterals.SETNAME, (RedisValue)clientName); + msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.CLIENT, RedisLiterals.SETNAME, clientName.AsRedisValue()); msg.SetInternalCall(); await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.DemandOK).ForAwait(); } @@ -1051,7 +1051,7 @@ private async Task HandshakeAsync(PhysicalConnection connection, ILogger? log) var libName = Multiplexer.GetFullLibraryName(); if (!string.IsNullOrWhiteSpace(libName)) { - msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.CLIENT, RedisLiterals.SETINFO, RedisLiterals.lib_name, libName); + msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.CLIENT, RedisLiterals.SETINFO, RedisLiterals.lib_name, libName.AsRedisValue()); msg.SetInternalCall(); await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.DemandOK).ForAwait(); } @@ -1059,7 +1059,7 @@ private async Task HandshakeAsync(PhysicalConnection connection, ILogger? log) var version = ClientInfoSanitize(Utils.GetLibVersion()); if (!string.IsNullOrWhiteSpace(version)) { - msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.CLIENT, RedisLiterals.SETINFO, RedisLiterals.lib_ver, version); + msg = Message.Create(-1, CommandFlags.FireAndForget | Message.NoFlushFlag, RedisCommand.CLIENT, RedisLiterals.SETINFO, RedisLiterals.lib_ver, version.AsRedisValue()); msg.SetInternalCall(); await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.DemandOK).ForAwait(); } diff --git a/src/StackExchange.Redis/StackExchange.Redis.csproj b/src/StackExchange.Redis/StackExchange.Redis.csproj index d3dd60aae..705b07eec 100644 --- a/src/StackExchange.Redis/StackExchange.Redis.csproj +++ b/src/StackExchange.Redis/StackExchange.Redis.csproj @@ -43,6 +43,8 @@ + + diff --git a/src/StackExchange.Redis/StreamConstants.cs b/src/StackExchange.Redis/StreamConstants.cs index 92c37222a..b63489e01 100644 --- a/src/StackExchange.Redis/StreamConstants.cs +++ b/src/StackExchange.Redis/StreamConstants.cs @@ -10,64 +10,64 @@ internal static class StreamConstants /// /// The "~" value used with the MAXLEN option. /// - internal static readonly RedisValue ApproximateMaxLen = "~"; + internal static readonly RedisValue ApproximateMaxLen = RedisValue.FromRaw("~"u8); /// /// The "*" value used with the XADD command. /// - internal static readonly RedisValue AutoGeneratedId = "*"; + internal static readonly RedisValue AutoGeneratedId = RedisValue.FromRaw("*"u8); /// /// The "$" value used in the XGROUP command. Indicates reading only new messages from the stream. /// - internal static readonly RedisValue NewMessages = "$"; + internal static readonly RedisValue NewMessages = RedisValue.FromRaw("$"u8); /// /// The "0" value used in the XGROUP command. Indicates reading all messages from the stream. /// - internal static readonly RedisValue AllMessages = "0"; + internal static readonly RedisValue AllMessages = RedisValue.FromRaw("0"u8); /// /// The "-" value used in the XRANGE, XREAD, and XREADGROUP commands. Indicates the minimum message ID from the stream. /// - internal static readonly RedisValue ReadMinValue = "-"; + internal static readonly RedisValue ReadMinValue = RedisValue.FromRaw("-"u8); /// /// The "+" value used in the XRANGE, XREAD, and XREADGROUP commands. Indicates the maximum message ID from the stream. /// - internal static readonly RedisValue ReadMaxValue = "+"; + internal static readonly RedisValue ReadMaxValue = RedisValue.FromRaw("+"u8); /// /// The ">" value used in the XREADGROUP command. Use this to read messages that have not been delivered to a consumer group. /// - internal static readonly RedisValue UndeliveredMessages = ">"; + internal static readonly RedisValue UndeliveredMessages = RedisValue.FromRaw(">"u8); - internal static readonly RedisValue Consumers = "CONSUMERS"; + internal static readonly RedisValue Consumers = RedisValue.FromRaw("CONSUMERS"u8); - internal static readonly RedisValue Count = "COUNT"; + internal static readonly RedisValue Count = RedisValue.FromRaw("COUNT"u8); - internal static readonly RedisValue Create = "CREATE"; + internal static readonly RedisValue Create = RedisValue.FromRaw("CREATE"u8); - internal static readonly RedisValue DeleteConsumer = "DELCONSUMER"; + internal static readonly RedisValue DeleteConsumer = RedisValue.FromRaw("DELCONSUMER"u8); - internal static readonly RedisValue Destroy = "DESTROY"; + internal static readonly RedisValue Destroy = RedisValue.FromRaw("DESTROY"u8); - internal static readonly RedisValue Groups = "GROUPS"; + internal static readonly RedisValue Groups = RedisValue.FromRaw("GROUPS"u8); - internal static readonly RedisValue JustId = "JUSTID"; + internal static readonly RedisValue JustId = RedisValue.FromRaw("JUSTID"u8); - internal static readonly RedisValue SetId = "SETID"; + internal static readonly RedisValue SetId = RedisValue.FromRaw("SETID"u8); - internal static readonly RedisValue MaxLen = "MAXLEN"; - internal static readonly RedisValue MinId = "MINID"; + internal static readonly RedisValue MaxLen = RedisValue.FromRaw("MAXLEN"u8); + internal static readonly RedisValue MinId = RedisValue.FromRaw("MINID"u8); - internal static readonly RedisValue MkStream = "MKSTREAM"; + internal static readonly RedisValue MkStream = RedisValue.FromRaw("MKSTREAM"u8); - internal static readonly RedisValue Stream = "STREAM"; + internal static readonly RedisValue Stream = RedisValue.FromRaw("STREAM"u8); - private static readonly RedisValue KeepRef = "KEEPREF", DelRef = "DELREF", Acked = "ACKED"; + private static readonly RedisValue KeepRef = RedisValue.FromRaw("KEEPREF"u8), DelRef = RedisValue.FromRaw("DELREF"u8), Acked = RedisValue.FromRaw("ACKED"u8); - internal static readonly RedisValue Ids = "IDS"; + internal static readonly RedisValue Ids = RedisValue.FromRaw("IDS"u8); internal static RedisValue GetMode(StreamTrimMode mode) => mode switch { diff --git a/tests/Directory.Build.targets b/tests/Directory.Build.targets index e1d7c8887..bed51fd15 100644 --- a/tests/Directory.Build.targets +++ b/tests/Directory.Build.targets @@ -1,6 +1,7 @@ false + $(NoWarn);StringToRedisValue diff --git a/tests/StackExchange.Redis.Tests/Issues/Issue3123.cs b/tests/StackExchange.Redis.Tests/Issues/Issue3123.cs new file mode 100644 index 000000000..5afa10d77 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/Issues/Issue3123.cs @@ -0,0 +1,41 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests.Issues; + +public class Issue3123(ITestOutputHelper output, SharedConnectionFixture? fixture) : TestBase(output, fixture) +{ + [Fact] + public async Task Run() + { + await using var conn = Create(); + var db = conn.GetDatabase(); + var key = Me(); + await db.KeyDeleteAsync(key, flags: CommandFlags.FireAndForget); + + Guid guid = Guid.NewGuid(); + byte[] payload = guid.ToByteArray(); + + await db.GeoAddAsync( + key, + longitude: -77.0365, + latitude: 38.8977, + member: payload, + flags: CommandFlags.FireAndForget); + + GeoSearchCircle commonSearchCircle = new(1, GeoUnit.Kilometers); + + GeoRadiusResult[] results = + await db.GeoSearchAsync( + key, + longitude: -77.0365, + latitude: 38.8977, + shape: commonSearchCircle); + var result = Assert.Single(results); + + byte[] final = (byte[])result.Member!; + Assert.True(final.SequenceEqual(payload)); + Assert.Equal(guid, new Guid(final)); + } +} diff --git a/tests/StackExchange.Redis.Tests/RoleTests.cs b/tests/StackExchange.Redis.Tests/RoleTests.cs index 92f66e0e4..054e39f48 100644 --- a/tests/StackExchange.Redis.Tests/RoleTests.cs +++ b/tests/StackExchange.Redis.Tests/RoleTests.cs @@ -41,7 +41,7 @@ public async Task PrimaryRole(bool allowAdmin) // should work with or without ad } } Assert.NotNull(role); - Assert.Equal(role.Value, RedisLiterals.master); + Assert.Equal(Role.LabelForMaster, role.Value); var primary = role as Role.Master; Assert.NotNull(primary); Assert.NotNull(primary.Replicas); diff --git a/toys/StackExchange.Redis.Server/StackExchange.Redis.Server.csproj b/toys/StackExchange.Redis.Server/StackExchange.Redis.Server.csproj index 3e800c817..4151146bf 100644 --- a/toys/StackExchange.Redis.Server/StackExchange.Redis.Server.csproj +++ b/toys/StackExchange.Redis.Server/StackExchange.Redis.Server.csproj @@ -9,7 +9,7 @@ Server;Async;Redis;Cache;PubSub;Messaging Library true - $(NoWarn);CS1591 + $(NoWarn);CS1591;StringToRedisValue