mark the implicit RedisChannel operators as [Obsolete] (#2481)

mark the implicit RedisChannel operators as [Obsolete]
StackExchange · Jun 13, 2023 · ebd66fd · ebd66fd
1 parent 3f0be76
commit ebd66fd
Show file tree

Hide file tree

Showing 22 changed files with 167 additions and 38 deletions.
diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md
@@ -9,6 +9,7 @@ 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
@@ -1658,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
@@ -1668,6 +1670,8 @@ 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

diff --git a/src/StackExchange.Redis/RedisChannel.cs b/src/StackExchange.Redis/RedisChannel.cs
@@ -35,6 +35,23 @@ public static bool UseImplicitAutoPattern
         }
         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>
@@ -176,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.
@@ -201,6 +227,7 @@ 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;
@@ -211,6 +238,7 @@ public static implicit operator RedisChannel(string key)
         /// 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;

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()

diff --git a/tests/StackExchange.Redis.Tests/ChannelTests.cs b/tests/StackExchange.Redis.Tests/ChannelTests.cs
@@ -22,7 +22,9 @@ public void ValidateAutoPatternModeString(string name, bool useImplicitAutoPatte
             try
             {
                 RedisChannel.UseImplicitAutoPattern = useImplicitAutoPattern;
+#pragma warning disable CS0618 // we need to test the operator
                 RedisChannel channel = name;
+#pragma warning restore CS0618
                 Assert.Equal(isPatternBased, channel.IsPatternBased);
             }
             finally
@@ -71,7 +73,9 @@ public void ValidateAutoPatternModeBytes(string name, bool useImplicitAutoPatter
             try
             {
                 RedisChannel.UseImplicitAutoPattern = useImplicitAutoPattern;
+#pragma warning disable CS0618 // we need to test the operator
                 RedisChannel channel = bytes;
+#pragma warning restore CS0618
                 Assert.Equal(isPatternBased, channel.IsPatternBased);
             }
             finally
@@ -108,5 +112,42 @@ public void ValidateModeSpecifiedIgnoresGlobalSettingBytes(string name, RedisCha
                 RedisChannel.UseImplicitAutoPattern = oldValue;
             }
         }
+
+        [Theory]
+        [InlineData("abc*def", false)]
+        [InlineData("abcdef", false)]
+        [InlineData("abc*def", true)]
+        [InlineData("abcdef", true)]
+        public void ValidateLiteralPatternMode(string name, bool useImplicitAutoPattern)
+        {
+            bool oldValue = RedisChannel.UseImplicitAutoPattern;
+            try
+            {
+                RedisChannel.UseImplicitAutoPattern = useImplicitAutoPattern;
+                RedisChannel channel;
+
+                // literal, string
+                channel = RedisChannel.Literal(name);
+                Assert.False(channel.IsPatternBased);
+
+                // pattern, string
+                channel = RedisChannel.Pattern(name);
+                Assert.True(channel.IsPatternBased);
+
+                var bytes = Encoding.UTF8.GetBytes(name);
+
+                // literal, byte[]
+                channel = RedisChannel.Literal(bytes);
+                Assert.False(channel.IsPatternBased);
+
+                // pattern, byte[]
+                channel = RedisChannel.Pattern(bytes);
+                Assert.True(channel.IsPatternBased);
+            }
+            finally
+            {
+                RedisChannel.UseImplicitAutoPattern = oldValue;
+            }
+        }
     }
 }
diff --git a/tests/StackExchange.Redis.Tests/ConfigTests.cs b/tests/StackExchange.Redis.Tests/ConfigTests.cs
@@ -276,7 +276,7 @@ public void ConnectWithSubscribeDisabled()
         Assert.True(servers[0].IsConnected);
         Assert.False(servers[0].IsSubscriberConnected);
 
-        var ex = Assert.Throws<RedisCommandException>(() => conn.GetSubscriber().Subscribe(Me(), (_, _) => GC.KeepAlive(this)));
+        var ex = Assert.Throws<RedisCommandException>(() => conn.GetSubscriber().Subscribe(RedisChannel.Literal(Me()), (_, _) => GC.KeepAlive(this)));
         Assert.Equal("This operation has been disabled in the command-map and cannot be used: SUBSCRIBE", ex.Message);
     }
 

diff --git a/tests/StackExchange.Redis.Tests/Issues/Issue1101Tests.cs b/tests/StackExchange.Redis.Tests/Issues/Issue1101Tests.cs
@@ -28,7 +28,7 @@ public async Task ExecuteWithUnsubscribeViaChannel()
     {
         using var conn = Create(log: Writer);
 
-        RedisChannel name = Me();
+        RedisChannel name = RedisChannel.Literal(Me());
         var pubsub = conn.GetSubscriber();
         AssertCounts(pubsub, name, false, 0, 0);
 
@@ -93,7 +93,7 @@ public async Task ExecuteWithUnsubscribeViaSubscriber()
     {
         using var conn = Create(shared: false, log: Writer);
 
-        RedisChannel name = Me();
+        RedisChannel name = RedisChannel.Literal(Me());
         var pubsub = conn.GetSubscriber();
         AssertCounts(pubsub, name, false, 0, 0);
 
@@ -144,7 +144,7 @@ public async Task ExecuteWithUnsubscribeViaClearAll()
     {
         using var conn = Create(log: Writer);
 
-        RedisChannel name = Me();
+        RedisChannel name = RedisChannel.Literal(Me());
         var pubsub = conn.GetSubscriber();
         AssertCounts(pubsub, name, false, 0, 0);
 

diff --git a/tests/StackExchange.Redis.Tests/KeyPrefixedDatabaseTests.cs b/tests/StackExchange.Redis.Tests/KeyPrefixedDatabaseTests.cs
@@ -568,8 +568,10 @@ public void LockTake()
     [Fact]
     public void Publish()
     {
+#pragma warning disable CS0618
         prefixed.Publish("channel", "message", CommandFlags.None);
         mock.Verify(_ => _.Publish("prefix:channel", "message", CommandFlags.None));
+#pragma warning restore CS0618
     }
 
     [Fact]

diff --git a/tests/StackExchange.Redis.Tests/KeyPrefixedTests.cs b/tests/StackExchange.Redis.Tests/KeyPrefixedTests.cs
@@ -528,8 +528,8 @@ public void LockTakeAsync()
         [Fact]
         public void PublishAsync()
         {
-            prefixed.PublishAsync("channel", "message", CommandFlags.None);
-            mock.Verify(_ => _.PublishAsync("prefix:channel", "message", CommandFlags.None));
+            prefixed.PublishAsync(RedisChannel.Literal("channel"), "message", CommandFlags.None);
+            mock.Verify(_ => _.PublishAsync(RedisChannel.Literal("prefix:channel"), "message", CommandFlags.None));
         }
 
         [Fact]

diff --git a/tests/StackExchange.Redis.Tests/PreserveOrderTests.cs b/tests/StackExchange.Redis.Tests/PreserveOrderTests.cs
@@ -22,7 +22,7 @@ public void Execute()
         var received = new List<int>();
         Log("Subscribing...");
         const int COUNT = 500;
-        sub.Subscribe(channel, (_, message) =>
+        sub.Subscribe(RedisChannel.Literal(channel), (_, message) =>
         {
             lock (received)
             {
@@ -42,7 +42,7 @@ public void Execute()
             // it all goes to the server and back
             for (int i = 0; i < COUNT; i++)
             {
-                sub.Publish(channel, i, CommandFlags.FireAndForget);
+                sub.Publish(RedisChannel.Literal(channel), i, CommandFlags.FireAndForget);
             }
 
             Log("Allowing time for delivery etc...");

diff --git a/tests/StackExchange.Redis.Tests/PubSubCommandTests.cs b/tests/StackExchange.Redis.Tests/PubSubCommandTests.cs
@@ -17,10 +17,12 @@ public void SubscriberCount()
     {
         using var conn = Create();
 
+#pragma warning disable CS0618
         RedisChannel channel = Me() + Guid.NewGuid();
         var server = conn.GetServer(conn.GetEndPoints()[0]);
 
         var channels = server.SubscriptionChannels(Me() + "*");
+#pragma warning restore CS0618
         Assert.DoesNotContain(channel, channels);
 
         _ = server.SubscriptionPatternCount();
@@ -30,7 +32,9 @@ public void SubscriberCount()
         count = server.SubscriptionSubscriberCount(channel);
         Assert.Equal(1, count);
 
+#pragma warning disable CS0618
         channels = server.SubscriptionChannels(Me() + "*");
+#pragma warning restore CS0618
         Assert.Contains(channel, channels);
     }
 
@@ -39,10 +43,14 @@ public async Task SubscriberCountAsync()
     {
         using var conn = Create();
 
+#pragma warning disable CS0618
         RedisChannel channel = Me() + Guid.NewGuid();
+#pragma warning restore CS0618
         var server = conn.GetServer(conn.GetEndPoints()[0]);
 
+#pragma warning disable CS0618
         var channels = await server.SubscriptionChannelsAsync(Me() + "*").WithTimeout(2000);
+#pragma warning restore CS0618
         Assert.DoesNotContain(channel, channels);
 
         _ = await server.SubscriptionPatternCountAsync().WithTimeout(2000);
@@ -52,7 +60,9 @@ public async Task SubscriberCountAsync()
         count = await server.SubscriptionSubscriberCountAsync(channel).WithTimeout(2000);
         Assert.Equal(1, count);
 
+#pragma warning disable CS0618
         channels = await server.SubscriptionChannelsAsync(Me() + "*").WithTimeout(2000);
+#pragma warning restore CS0618
         Assert.Contains(channel, channels);
     }
 }