Merge pull request #1525 from ejsmith/sentinel-port-fix

This makes it so that when using a configuration string to connect that we only parse the configuration string and not set any defaults on it until it is forwarded on to the connect methods that take `ConfigurationOptions`
StackExchange · Aug 26, 2020 · ad94bd3 · ad94bd3
2 parents 2fd5b79 + be109fd
commit ad94bd3
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 14 deletions.
diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs
@@ -485,6 +485,19 @@ public void SetDefaultPorts()
             EndPoints.SetDefaultPorts(Ssl ? 6380 : 6379);
         }
 
+        /// <summary>
+        /// Sets default config settings required for sentinel usage
+        /// </summary>
+        internal void SetSentinelDefaults()
+        {
+            // this is required when connecting to sentinel servers
+            TieBreaker = "";
+            CommandMap = CommandMap.Sentinel;
+
+            // use default sentinel port
+            EndPoints.SetDefaultPorts(26379);
+        }
+
         /// <summary>
         /// Returns the effective configuration string for this configuration, including Redis credentials.
         /// </summary>

diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs
@@ -840,7 +840,7 @@ internal ServerEndPoint AnyConnected(ServerType serverType, uint startOffset, Re
         public static Task<ConnectionMultiplexer> ConnectAsync(string configuration, TextWriter log = null)
         {
             SocketConnection.AssertDependencies();
-            return ConnectAsync(PrepareConfig(configuration), log);
+            return ConnectAsync(ConfigurationOptions.Parse(configuration), log);
         }
 
         private static async Task<ConnectionMultiplexer> ConnectImplAsync(ConfigurationOptions configuration, TextWriter log = null)
@@ -918,12 +918,7 @@ internal static ConfigurationOptions PrepareConfig(object configuration, bool se
 
             if (sentinel)
             {
-                // this is required when connecting to sentinel servers
-                config.TieBreaker = "";
-                config.CommandMap = CommandMap.Sentinel;
-
-                // use default sentinel port
-                config.EndPoints.SetDefaultPorts(26379);
+                config.SetSentinelDefaults();
 
                 return config;
             }
@@ -1017,7 +1012,7 @@ private static ConnectionMultiplexer CreateMultiplexer(ConfigurationOptions conf
         /// <param name="log">The <see cref="TextWriter"/> to log to.</param>
         public static ConnectionMultiplexer Connect(string configuration, TextWriter log = null)
         {
-            return Connect(PrepareConfig(configuration), log);
+            return Connect(ConfigurationOptions.Parse(configuration), log);
         }
 
         /// <summary>

diff --git a/tests/StackExchange.Redis.Tests/Config.cs b/tests/StackExchange.Redis.Tests/Config.cs
@@ -261,10 +261,10 @@ public void GetTime()
             {
                 var server = GetAnyMaster(muxer);
                 var serverTime = server.Time();
-                Log(serverTime.ToString(CultureInfo.InvariantCulture));
-                var delta = Math.Abs((DateTime.UtcNow - serverTime).TotalSeconds);
-
-                Assert.True(delta < 5);
+                var localTime = DateTime.UtcNow;
+                Log("Server: " + serverTime.ToString(CultureInfo.InvariantCulture));
+                Log("Local: " + localTime.ToString(CultureInfo.InvariantCulture));
+                Assert.Equal(localTime, serverTime, TimeSpan.FromSeconds(10));
             }
         }
 

diff --git a/tests/StackExchange.Redis.Tests/Sentinel.cs b/tests/StackExchange.Redis.Tests/Sentinel.cs
@@ -57,7 +57,7 @@ public Sentinel(ITestOutputHelper output) : base(output)
         [Fact]
         public void MasterConnectTest()
         {
-            var connectionString = $"{TestConfig.Current.SentinelServer}:{TestConfig.Current.SentinelPortA},serviceName={ServiceOptions.ServiceName},allowAdmin=true";
+            var connectionString = $"{TestConfig.Current.SentinelServer},serviceName={ServiceOptions.ServiceName},allowAdmin=true";
             var conn = ConnectionMultiplexer.Connect(connectionString);
 
             var db = conn.GetDatabase();
@@ -93,7 +93,7 @@ public void MasterConnectTest()
         [Fact]
         public async Task MasterConnectAsyncTest()
         {
-            var connectionString = $"{TestConfig.Current.SentinelServer}:{TestConfig.Current.SentinelPortA},serviceName={ServiceOptions.ServiceName},allowAdmin=true";
+            var connectionString = $"{TestConfig.Current.SentinelServer},serviceName={ServiceOptions.ServiceName},allowAdmin=true";
             var conn = await ConnectionMultiplexer.ConnectAsync(connectionString);
 
             var db = conn.GetDatabase();