Merge pull request #535 from mjcheetham/dynamic-provider-setting-only

Only set `credential.provider` for dynamic matches

Author: mjcheetham

src/shared/Core.Tests/HostProviderRegistryTests.cs

@@ -69,7 +69,7 @@ public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_ReturnsSupp
         }
 
         [Fact]
-        public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_SetsProviderGlobalConfig()
+        public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_StaticMatch_DoesNotSetProviderGlobalConfig()
         {
             var context = new TestCommandContext();
             var registry = new HostProviderRegistry(context);
@@ -90,14 +90,41 @@ public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_SetsProvide
 
             IHostProvider result = await registry.GetProviderAsync(input);
 
+            Assert.Same(providerMock.Object, result);
+            Assert.False(context.Git.Configuration.Global.TryGetValue(configKey, out _));
+        }
+
+        [Fact]
+        public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_DynamicMatch_SetsProviderGlobalConfig()
+        {
+            var context = new TestCommandContext();
+            var registry = new HostProviderRegistry(context);
+            var remote = new Uri("https://example.com");
+            InputArguments input = CreateInputArguments(remote);
+
+            string providerId = "myProvider";
+            string configKey = string.Format(CultureInfo.InvariantCulture,
+                "{0}.https://example.com.{1}",
+                Constants.GitConfiguration.Credential.SectionName,
+                Constants.GitConfiguration.Credential.Provider);
+
+            var providerMock = new Mock<IHostProvider>();
+            providerMock.Setup(x => x.Id).Returns(providerId);
+            providerMock.Setup(x => x.IsSupported(It.IsAny<InputArguments>())).Returns(false);
+            providerMock.Setup(x => x.IsSupported(It.IsAny<HttpResponseMessage>())).Returns(true);
+
+            registry.Register(providerMock.Object, HostProviderPriority.Normal);
+
+            IHostProvider result = await registry.GetProviderAsync(input);
+
             Assert.Same(providerMock.Object, result);
             Assert.True(context.Git.Configuration.Global.TryGetValue(configKey, out IList<string> config));
             Assert.Equal(1, config.Count);
             Assert.Equal(providerId, config[0]);
         }
 
         [Fact]
-        public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_SetsProviderGlobalConfig_HostWithPath()
+        public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_DynamicMatch_SetsProviderGlobalConfig_HostWithPath()
         {
             var context = new TestCommandContext();
             var registry = new HostProviderRegistry(context);
@@ -112,7 +139,8 @@ public async Task HostProviderRegistry_GetProvider_Auto_HasProviders_SetsProvide
 
             var providerMock = new Mock<IHostProvider>();
             providerMock.Setup(x => x.Id).Returns(providerId);
-            providerMock.Setup(x => x.IsSupported(It.IsAny<InputArguments>())).Returns(true);
+            providerMock.Setup(x => x.IsSupported(It.IsAny<InputArguments>())).Returns(false);
+            providerMock.Setup(x => x.IsSupported(It.IsAny<HttpResponseMessage>())).Returns(true);
 
             registry.Register(providerMock.Object, HostProviderPriority.Normal);
 

src/shared/Core/HostProviderRegistry.cs

@@ -220,24 +220,28 @@ await MatchProviderAsync(HostProviderPriority.Normal) ??
                                   await MatchProviderAsync(HostProviderPriority.Low) ??
                                   throw new Exception("No host provider available to service this request.");
 
-            // Set the host provider explicitly for future calls
-            IGitConfiguration gitConfig = _context.Git.GetConfiguration();
-            var keyName = string.Format(CultureInfo.InvariantCulture, "{0}.{1}.{2}",
-                Constants.GitConfiguration.Credential.SectionName, uri.ToString().TrimEnd('/'),
-                Constants.GitConfiguration.Credential.Provider);
-
-            try
-            {
-                _context.Trace.WriteLine($"Remembering host provider for '{uri}' as '{match.Id}'...");
-                gitConfig.Set(GitConfigurationLevel.Global, keyName, match.Id);
-            }
-            catch (Exception ex)
+            // If we ended up making a network call then set the host provider explicitly
+            // to avoid future calls!
+            if (probeResponse != null)
             {
-                _context.Trace.WriteLine("Failed to set host provider!");
-                _context.Trace.WriteException(ex);
+                IGitConfiguration gitConfig = _context.Git.GetConfiguration();
+                var keyName = string.Format(CultureInfo.InvariantCulture, "{0}.{1}.{2}",
+                    Constants.GitConfiguration.Credential.SectionName, uri.ToString().TrimEnd('/'),
+                    Constants.GitConfiguration.Credential.Provider);
+
+                try
+                {
+                    _context.Trace.WriteLine($"Remembering host provider for '{uri}' as '{match.Id}'...");
+                    gitConfig.Set(GitConfigurationLevel.Global, keyName, match.Id);
+                }
+                catch (Exception ex)
+                {
+                    _context.Trace.WriteLine("Failed to set host provider!");
+                    _context.Trace.WriteException(ex);
 
-                _context.Streams.Error.WriteLine("warning: failed to remember result of host provider detection!");
-                _context.Streams.Error.WriteLine($"warning: try setting this manually: `git config --global {keyName} {match.Id}`");
+                    _context.Streams.Error.WriteLine("warning: failed to remember result of host provider detection!");
+                    _context.Streams.Error.WriteLine($"warning: try setting this manually: `git config --global {keyName} {match.Id}`");
+                }
             }
 
             return match;