add RedisChannel UseImplicitAutoPattern and IsPatternBased (#2480)

* add RedisChannel UseImplicitAutoPattern and IsPatternBased * mark the implicit RedisChannel operators as [Obsolete] (#2481)
StackExchange · Jun 13, 2023 · 8abe002 · 8abe002
1 parent ae6419a
commit 8abe002
Show file tree

Hide file tree

Showing 23 changed files with 322 additions and 50 deletions.
diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md
@@ -8,6 +8,8 @@ Current package versions:
 
 ## Unreleased
 
+- Fix [#2479](https://github.com/StackExchange/StackExchange.Redis/issues/2479): Add `RedisChannel.UseImplicitAutoPattern` (global) and `RedisChannel.IsPatternBased` ([#2480 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2480))
+- Fix [#2479](https://github.com/StackExchange/StackExchange.Redis/issues/2479): Mark `RedisChannel` conversion operators as obsolete; add `RedisChannel.Literal` and `RedisChannel.Pattern` helpers ([#2481 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2481))
 - Fix [#2449](https://github.com/StackExchange/StackExchange.Redis/issues/2449): Update `Pipelines.Sockets.Unofficial` to `v2.2.8` to support native AOT ([#2456 by eerhardt](https://github.com/StackExchange/StackExchange.Redis/pull/2456))
 
 ## 2.6.111

diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs
@@ -860,7 +860,7 @@ private ConfigurationOptions DoParse(string configuration, bool ignoreUnknown)
                             ClientName = value;
                             break;
                         case OptionKeys.ChannelPrefix:
-                            ChannelPrefix = value;
+                            ChannelPrefix = RedisChannel.Literal(value);
                             break;
                         case OptionKeys.ConfigChannel:
                             ConfigurationChannel = value;

diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs
@@ -1,11 +1,11 @@
-using System;
+using Pipelines.Sockets.Unofficial;
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Net;
 using System.Threading;
 using System.Threading.Tasks;
-using Pipelines.Sockets.Unofficial;
 
 namespace StackExchange.Redis;
 
@@ -30,9 +30,9 @@ internal void InitializeSentinel(LogProxy? logProxy)
         // Subscribe to sentinel change events
         ISubscriber sub = GetSubscriber();
 
-        if (sub.SubscribedEndpoint("+switch-master") == null)
+        if (sub.SubscribedEndpoint(RedisChannel.Literal("+switch-master")) == null)
         {
-            sub.Subscribe("+switch-master", (__, message) =>
+            sub.Subscribe(RedisChannel.Literal("+switch-master"), (__, message) =>
             {
                 string[] messageParts = ((string)message!).Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
                 // We don't care about the result of this - we're just trying
@@ -68,9 +68,9 @@ internal void InitializeSentinel(LogProxy? logProxy)
             ReconfigureAsync(first: false, reconfigureAll: true, logProxy, e.EndPoint, "Lost sentinel connection", false).Wait();
 
         // Subscribe to new sentinels being added
-        if (sub.SubscribedEndpoint("+sentinel") == null)
+        if (sub.SubscribedEndpoint(RedisChannel.Literal("+sentinel")) == null)
         {
-            sub.Subscribe("+sentinel", (_, message) =>
+            sub.Subscribe(RedisChannel.Literal("+sentinel"), (_, message) =>
             {
                 string[] messageParts = ((string)message!).Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
                 UpdateSentinelAddressList(messageParts[0]);

diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs
@@ -2221,7 +2221,7 @@ public long PublishReconfigure(CommandFlags flags = CommandFlags.None)
 
         private long PublishReconfigureImpl(CommandFlags flags) =>
             ConfigurationChangedChannel is byte[] channel
-                ? GetSubscriber().Publish(channel, RedisLiterals.Wildcard, flags)
+                ? GetSubscriber().Publish(RedisChannel.Literal(channel), RedisLiterals.Wildcard, flags)
                 : 0;
 
         /// <summary>
@@ -2231,7 +2231,7 @@ ConfigurationChangedChannel is byte[] channel
         /// <returns>The number of instances known to have received the message (however, the actual number can be higher).</returns>
         public Task<long> PublishReconfigureAsync(CommandFlags flags = CommandFlags.None) =>
             ConfigurationChangedChannel is byte[] channel
-                ? GetSubscriber().PublishAsync(channel, RedisLiterals.Wildcard, flags)
+                ? GetSubscriber().PublishAsync(RedisChannel.Literal(channel), RedisLiterals.Wildcard, flags)
                 : CompletedTask<long>.Default(null);
 
         /// <summary>

diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs
@@ -841,8 +841,11 @@ protected RedisValue SortGetToInner(RedisValue outer) =>
             }
         }
 
-        protected RedisChannel ToInner(RedisChannel outer) =>
-            RedisKey.ConcatenateBytes(Prefix, null, (byte[]?)outer);
+        protected RedisChannel ToInner(RedisChannel outer)
+        {
+            var combined = RedisKey.ConcatenateBytes(Prefix, null, (byte[]?)outer);
+            return new RedisChannel(combined, outer.IsPatternBased ? RedisChannel.PatternMode.Pattern : RedisChannel.PatternMode.Literal);
+        }
 
         private Func<RedisKey, RedisKey>? mapFunction;
         protected Func<RedisKey, RedisKey> GetMapFunction() =>

diff --git a/src/StackExchange.Redis/Maintenance/AzureMaintenanceEvent.cs b/src/StackExchange.Redis/Maintenance/AzureMaintenanceEvent.cs
@@ -130,7 +130,7 @@ internal async static Task AddListenerAsync(ConnectionMultiplexer multiplexer, A
                     return;
                 }
 
-                await sub.SubscribeAsync(PubSubChannelName, async (_, message) =>
+                await sub.SubscribeAsync(RedisChannel.Literal(PubSubChannelName), async (_, message) =>
                 {
                     var newMessage = new AzureMaintenanceEvent(message!);
                     newMessage.NotifyMultiplexer(multiplexer);

diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs
@@ -373,7 +373,7 @@ internal void KeepAlive()
                     else if (commandMap.IsAvailable(RedisCommand.UNSUBSCRIBE))
                     {
                         msg = Message.Create(-1, CommandFlags.FireAndForget, RedisCommand.UNSUBSCRIBE,
-                            (RedisChannel)Multiplexer.UniqueId);
+                            RedisChannel.Literal(Multiplexer.UniqueId));
                         msg.SetSource(ResultProcessor.TrackSubscriptions, null);
                     }
                     break;

diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt
@@ -1259,6 +1259,7 @@ StackExchange.Redis.Proxy.Twemproxy = 1 -> StackExchange.Redis.Proxy
 StackExchange.Redis.RedisChannel
 StackExchange.Redis.RedisChannel.Equals(StackExchange.Redis.RedisChannel other) -> bool
 StackExchange.Redis.RedisChannel.IsNullOrEmpty.get -> bool
+StackExchange.Redis.RedisChannel.IsPatternBased.get -> bool
 StackExchange.Redis.RedisChannel.PatternMode
 StackExchange.Redis.RedisChannel.PatternMode.Auto = 0 -> StackExchange.Redis.RedisChannel.PatternMode
 StackExchange.Redis.RedisChannel.PatternMode.Literal = 1 -> StackExchange.Redis.RedisChannel.PatternMode
@@ -1657,6 +1658,8 @@ static StackExchange.Redis.RedisChannel.implicit operator byte[]?(StackExchange.
 static StackExchange.Redis.RedisChannel.implicit operator StackExchange.Redis.RedisChannel(byte[]? key) -> StackExchange.Redis.RedisChannel
 static StackExchange.Redis.RedisChannel.implicit operator StackExchange.Redis.RedisChannel(string! key) -> StackExchange.Redis.RedisChannel
 static StackExchange.Redis.RedisChannel.implicit operator string?(StackExchange.Redis.RedisChannel key) -> string?
+static StackExchange.Redis.RedisChannel.Literal(byte[]! value) -> StackExchange.Redis.RedisChannel
+static StackExchange.Redis.RedisChannel.Literal(string! value) -> StackExchange.Redis.RedisChannel
 static StackExchange.Redis.RedisChannel.operator !=(byte[]! x, StackExchange.Redis.RedisChannel y) -> bool
 static StackExchange.Redis.RedisChannel.operator !=(StackExchange.Redis.RedisChannel x, byte[]! y) -> bool
 static StackExchange.Redis.RedisChannel.operator !=(StackExchange.Redis.RedisChannel x, StackExchange.Redis.RedisChannel y) -> bool
@@ -1667,6 +1670,10 @@ static StackExchange.Redis.RedisChannel.operator ==(StackExchange.Redis.RedisCha
 static StackExchange.Redis.RedisChannel.operator ==(StackExchange.Redis.RedisChannel x, StackExchange.Redis.RedisChannel y) -> bool
 static StackExchange.Redis.RedisChannel.operator ==(StackExchange.Redis.RedisChannel x, string! y) -> bool
 static StackExchange.Redis.RedisChannel.operator ==(string! x, StackExchange.Redis.RedisChannel y) -> bool
+static StackExchange.Redis.RedisChannel.Pattern(byte[]! value) -> StackExchange.Redis.RedisChannel
+static StackExchange.Redis.RedisChannel.Pattern(string! value) -> StackExchange.Redis.RedisChannel
+static StackExchange.Redis.RedisChannel.UseImplicitAutoPattern.get -> bool
+static StackExchange.Redis.RedisChannel.UseImplicitAutoPattern.set -> void
 static StackExchange.Redis.RedisFeatures.operator !=(StackExchange.Redis.RedisFeatures left, StackExchange.Redis.RedisFeatures right) -> bool
 static StackExchange.Redis.RedisFeatures.operator ==(StackExchange.Redis.RedisFeatures left, StackExchange.Redis.RedisFeatures right) -> bool
 static StackExchange.Redis.RedisKey.implicit operator byte[]?(StackExchange.Redis.RedisKey key) -> byte[]?

diff --git a/src/StackExchange.Redis/RedisChannel.cs b/src/StackExchange.Redis/RedisChannel.cs
@@ -9,33 +9,67 @@ namespace StackExchange.Redis
     public readonly struct RedisChannel : IEquatable<RedisChannel>
     {
         internal readonly byte[]? Value;
-        internal readonly bool IsPatternBased;
+        internal readonly bool _isPatternBased;
 
         /// <summary>
         /// Indicates whether the channel-name is either null or a zero-length value.
         /// </summary>
         public bool IsNullOrEmpty => Value == null || Value.Length == 0;
 
+        /// <summary>
+        /// Indicates whether this channel represents a wildcard pattern (see <c>PSUBSCRIBE</c>)
+        /// </summary>
+        public bool IsPatternBased => _isPatternBased;
+
         internal bool IsNull => Value == null;
 
+
+        /// <summary>
+        /// Indicates whether channels should use <see cref="PatternMode.Auto"/> when no <see cref="PatternMode"/>
+        /// is specified; this is enabled by default, but can be disabled to avoid unexpected wildcard scenarios.
+        /// </summary>
+        public static bool UseImplicitAutoPattern
+        {
+            get => s_DefaultPatternMode == PatternMode.Auto;
+            set => s_DefaultPatternMode = value ? PatternMode.Auto : PatternMode.Literal;
+        }
+        private static PatternMode s_DefaultPatternMode = PatternMode.Auto;
+
+        /// <summary>
+        /// Creates a new <see cref="RedisChannel"/> that does not act as a wildcard subscription
+        /// </summary>
+        public static RedisChannel Literal(string value) => new RedisChannel(value, PatternMode.Literal);
+        /// <summary>
+        /// Creates a new <see cref="RedisChannel"/> that does not act as a wildcard subscription
+        /// </summary>
+        public static RedisChannel Literal(byte[] value) => new RedisChannel(value, PatternMode.Literal);
+        /// <summary>
+        /// Creates a new <see cref="RedisChannel"/> that acts as a wildcard subscription
+        /// </summary>
+        public static RedisChannel Pattern(string value) => new RedisChannel(value, PatternMode.Pattern);
+        /// <summary>
+        /// Creates a new <see cref="RedisChannel"/> that acts as a wildcard subscription
+        /// </summary>
+        public static RedisChannel Pattern(byte[] value) => new RedisChannel(value, PatternMode.Pattern);
+
         /// <summary>
         /// Create a new redis channel from a buffer, explicitly controlling the pattern mode.
         /// </summary>
         /// <param name="value">The name of the channel to create.</param>
         /// <param name="mode">The mode for name matching.</param>
-        public RedisChannel(byte[]? value, PatternMode mode) : this(value, DeterminePatternBased(value, mode)) {}
+        public RedisChannel(byte[]? value, PatternMode mode) : this(value, DeterminePatternBased(value, mode)) { }
 
         /// <summary>
         /// Create a new redis channel from a string, explicitly controlling the pattern mode.
         /// </summary>
         /// <param name="value">The string name of the channel to create.</param>
         /// <param name="mode">The mode for name matching.</param>
-        public RedisChannel(string value, PatternMode mode) : this(value == null ? null : Encoding.UTF8.GetBytes(value), mode) {}
+        public RedisChannel(string value, PatternMode mode) : this(value == null ? null : Encoding.UTF8.GetBytes(value), mode) { }
 
         private RedisChannel(byte[]? value, bool isPatternBased)
         {
             Value = value;
-            IsPatternBased = isPatternBased;
+            _isPatternBased = isPatternBased;
         }
 
         private static bool DeterminePatternBased(byte[]? value, PatternMode mode) => mode switch
@@ -87,7 +121,7 @@ private RedisChannel(byte[]? value, bool isPatternBased)
         /// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
         /// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
         public static bool operator ==(RedisChannel x, RedisChannel y) =>
-            x.IsPatternBased == y.IsPatternBased && RedisValue.Equals(x.Value, y.Value);
+            x._isPatternBased == y._isPatternBased && RedisValue.Equals(x.Value, y.Value);
 
         /// <summary>
         /// Indicate whether two channel names are equal.
@@ -135,10 +169,10 @@ private RedisChannel(byte[]? value, bool isPatternBased)
         /// Indicate whether two channel names are equal.
         /// </summary>
         /// <param name="other">The <see cref="RedisChannel"/> to compare to.</param>
-        public bool Equals(RedisChannel other) => IsPatternBased == other.IsPatternBased && RedisValue.Equals(Value, other.Value);
+        public bool Equals(RedisChannel other) => _isPatternBased == other._isPatternBased && RedisValue.Equals(Value, other.Value);
 
         /// <inheritdoc/>
-        public override int GetHashCode() => RedisValue.GetHashCode(Value) + (IsPatternBased ? 1 : 0);
+        public override int GetHashCode() => RedisValue.GetHashCode(Value) + (_isPatternBased ? 1 : 0);
 
         /// <summary>
         /// Obtains a string representation of the channel name.
@@ -159,7 +193,16 @@ internal void AssertNotNull()
             if (IsNull) throw new ArgumentException("A null key is not valid in this context");
         }
 
-        internal RedisChannel Clone() => (byte[]?)Value?.Clone() ?? default;
+        internal RedisChannel Clone()
+        {
+            if (Value is null || Value.Length == 0)
+            {
+                // no need to duplicate anything
+                return this;
+            }
+            var copy = (byte[])Value.Clone(); // defensive array copy
+            return new RedisChannel(copy, _isPatternBased);
+        }
 
         /// <summary>
         /// The matching pattern for this channel.
@@ -184,33 +227,35 @@ public enum PatternMode
         /// Create a channel name from a <see cref="string"/>.
         /// </summary>
         /// <param name="key">The string to get a channel from.</param>
+        [Obsolete("It is preferable to explicitly specify a " + nameof(PatternMode) + ", or use the " + nameof(Literal) + "/" + nameof(Pattern) + " methods", error: false)]
         public static implicit operator RedisChannel(string key)
         {
             if (key == null) return default;
-            return new RedisChannel(Encoding.UTF8.GetBytes(key), PatternMode.Auto);
+            return new RedisChannel(Encoding.UTF8.GetBytes(key), s_DefaultPatternMode);
         }
 
         /// <summary>
         /// Create a channel name from a <see cref="T:byte[]"/>.
         /// </summary>
         /// <param name="key">The byte array to get a channel from.</param>
+        [Obsolete("It is preferable to explicitly specify a " + nameof(PatternMode) + ", or use the " + nameof(Literal) + "/" + nameof(Pattern) + " methods", error: false)]
         public static implicit operator RedisChannel(byte[]? key)
         {
             if (key == null) return default;
-            return new RedisChannel(key, PatternMode.Auto);
+            return new RedisChannel(key, s_DefaultPatternMode);
         }
 
         /// <summary>
         /// Obtain the channel name as a <see cref="T:byte[]"/>.
         /// </summary>
         /// <param name="key">The channel to get a byte[] from.</param>
-        public static implicit operator byte[]? (RedisChannel key) => key.Value;
+        public static implicit operator byte[]?(RedisChannel key) => key.Value;
 
         /// <summary>
         /// Obtain the channel name as a <see cref="string"/>.
         /// </summary>
         /// <param name="key">The channel to get a string from.</param>
-        public static implicit operator string? (RedisChannel key)
+        public static implicit operator string?(RedisChannel key)
         {
             var arr = key.Value;
             if (arr == null)
@@ -226,5 +271,15 @@ public static implicit operator RedisChannel(byte[]? key)
                 return BitConverter.ToString(arr);
             }
         }
+
+#if DEBUG
+        // these exist *purely* to ensure that we never add them later *without*
+        // giving due consideration to the default pattern mode (UseImplicitAutoPattern)
+        // (since we don't ship them, we don't need them in release)
+        [Obsolete("Watch for " + nameof(UseImplicitAutoPattern), error: true)]
+        private RedisChannel(string value) => throw new NotSupportedException();
+        [Obsolete("Watch for " + nameof(UseImplicitAutoPattern), error: true)]
+        private RedisChannel(byte[]? value) => throw new NotSupportedException();
+#endif
     }
 }
diff --git a/src/StackExchange.Redis/RedisSubscriber.cs b/src/StackExchange.Redis/RedisSubscriber.cs
@@ -159,7 +159,7 @@ public Subscription(CommandFlags flags)
             /// </summary>
             internal Message GetMessage(RedisChannel channel, SubscriptionAction action, CommandFlags flags, bool internalCall)
             {
-                var isPattern = channel.IsPatternBased;
+                var isPattern = channel._isPatternBased;
                 var command = action switch
                 {
                     SubscriptionAction.Subscribe when isPattern => RedisCommand.PSUBSCRIBE,

diff --git a/src/StackExchange.Redis/ServerEndPoint.cs b/src/StackExchange.Redis/ServerEndPoint.cs
@@ -979,7 +979,7 @@ private async Task HandshakeAsync(PhysicalConnection connection, LogProxy? log)
                 var configChannel = Multiplexer.ConfigurationChangedChannel;
                 if (configChannel != null)
                 {
-                    msg = Message.Create(-1, CommandFlags.FireAndForget, RedisCommand.SUBSCRIBE, (RedisChannel)configChannel);
+                    msg = Message.Create(-1, CommandFlags.FireAndForget, RedisCommand.SUBSCRIBE, RedisChannel.Literal(configChannel));
                     // Note: this is NOT internal, we want it to queue in a backlog for sending when ready if necessary
                     await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.TrackSubscriptions).ForAwait();
                 }

diff --git a/tests/ConsoleTest/Program.cs b/tests/ConsoleTest/Program.cs
@@ -110,7 +110,7 @@ static void ParallelRun(int taskId, ConnectionMultiplexer connection)
 static void MassPublish(ConnectionMultiplexer connection)
 {
     var subscriber = connection.GetSubscriber();
-    Parallel.For(0, 1000, _ => subscriber.Publish("cache-events:cache-testing", "hey"));
+    Parallel.For(0, 1000, _ => subscriber.Publish(new RedisChannel("cache-events:cache-testing", RedisChannel.PatternMode.Literal), "hey"));
 }
 
 static string GetLibVersion()