Revert back to new HttpClient instance per request

Move back to creating a new HttpClient instance per request for
few reasons:

 - The current code/pattern was broken - we were double disposing of
   the singleton client instance.
 - It makes setting headers slightly easier as the caller no longer
   must create request message with the headers set manually - just
   set the default headers on their exclusive client.

In practice we're not going to be running for a long time, and we're
not going to be making many requests, which would otherwise drain all
the available network ports.

If the individual providers wish to obtain a new HttpClient instance
and reuse it for all their web calls, they are free to do so.

Author: mjcheetham

src/Microsoft.AzureRepos/AzureDevOpsRestApi.cs

@@ -44,11 +44,12 @@ public async Task<string> GetAuthorityAsync(Uri organizationUri)
             const string commonAuthority = authorityBase + "common";
             const string msaAuthority = authorityBase + "live.com";
 
-            var headers = new[] {Constants.Http.AcceptHeader(Constants.Http.MimeTypeJson)};
+            HttpClient client = _httpFactory.GetClient();
+            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(Constants.Http.MimeTypeJson));
 
             _context.Trace.WriteLine($"HTTP: HEAD {organizationUri}");
-            using (var client = _httpFactory.GetClient())
-            using (var response = await client.SendAsync(HttpMethod.Head, organizationUri, headers))
+            using (client)
+            using (var response = await client.HeadAsync(organizationUri))
             {
                 _context.Trace.WriteLine("HTTP: Response code ignored.");
                 _context.Trace.WriteLine("Inspecting headers...");
@@ -112,16 +113,14 @@ public async Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, st
 
             Uri requestUri = new Uri(identityServiceUri, sessionTokenUrl);
 
-            var headers = new[]
-            {
-                Constants.Http.AcceptHeader(Constants.Http.MimeTypeJson),
-                Constants.Http.AuthorizationBearerHeader(accessToken)
-            };
+            HttpClient client = _httpFactory.GetClient();
+            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(Constants.Http.MimeTypeJson));
+            client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(Constants.Http.WwwAuthenticateBearerScheme, accessToken);
 
             _context.Trace.WriteLine($"HTTP: POST {requestUri}");
+            using (client)
             using (StringContent content = CreateAccessTokenRequestJson(organizationUri, scopes))
-            using (var client = _httpFactory.GetClient())
-            using (var response = await client.SendAsync(HttpMethod.Post, requestUri, headers, content))
+            using (var response = await client.PostAsync(requestUri, content))
             {
                 _context.Trace.WriteLine($"HTTP: Response {(int)response.StatusCode} [{response.StatusCode}]");
 
@@ -162,15 +161,13 @@ private async Task<Uri> GetIdentityServiceUriAsync(Uri organizationUri, string a
                 Query = locationServiceQuery,
             }.Uri;
 
-            var headers = new[]
-            {
-                Constants.Http.AcceptHeader(Constants.Http.MimeTypeJson),
-                Constants.Http.AuthorizationBearerHeader(accessToken)
-            };
+            HttpClient client = _httpFactory.GetClient();
+            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(Constants.Http.MimeTypeJson));
+            client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(Constants.Http.WwwAuthenticateBearerScheme, accessToken);
 
             _context.Trace.WriteLine($"HTTP: GET {requestUri}");
-            using (var client = _httpFactory.GetClient())
-            using (var response = await client.SendAsync(HttpMethod.Get, requestUri, headers))
+            using (client)
+            using (var response = await client.GetAsync(requestUri))
             {
                 _context.Trace.WriteLine($"HTTP: Response {(int)response.StatusCode} [{response.StatusCode}]");
                 if (response.IsSuccessStatusCode)

src/Microsoft.Git.CredentialManager/Authentication/WindowsIntegratedAuthentication.cs

@@ -34,7 +34,7 @@ public async Task<bool> GetIsSupportedAsync(Uri uri)
 
             _context.Trace.WriteLine($"HTTP: HEAD {uri}");
             using (HttpClient client = _httpFactory.GetClient())
-            using (HttpResponseMessage response = await client.SendAsync(HttpMethod.Head, uri))
+            using (HttpResponseMessage response = await client.HeadAsync(uri))
             {
                 _context.Trace.WriteLine("HTTP: Response code ignored.");
 

src/Microsoft.Git.CredentialManager/Constants.cs

@@ -20,21 +20,12 @@ public static class EnvironmentVariables
 
         public static class Http
         {
+            public const string WwwAuthenticateBasicScheme     = "Basic";
             public const string WwwAuthenticateBearerScheme    = "Bearer";
             public const string WwwAuthenticateNegotiateScheme = "Negotiate";
             public const string WwwAuthenticateNtlmScheme      = "NTLM";
 
             public const string MimeTypeJson = "application/json";
-
-            public static Header AcceptHeader(string value)
-            {
-                return new Header("Accept", new[] {value});
-            }
-
-            public static Header AuthorizationBearerHeader(string bearerToken)
-            {
-                return new Header("Authorization", new[] {$"{WwwAuthenticateBearerScheme} {bearerToken}"});
-            }
         }
 
         /// <summary>

src/Microsoft.Git.CredentialManager/HttpClientFactory.cs

@@ -19,24 +19,19 @@ public interface IHttpClientFactory
 
     public class HttpClientFactory : IHttpClientFactory
     {
-        private HttpClient _client;
-
         public HttpClient GetClient()
         {
-            if (_client is null)
-            {
-                // Initialize a new HttpClient
-                _client = new HttpClient();
+            // Initialize a new HttpClient
+            var client = new HttpClient();
 
-                // Add default headers
-                _client.DefaultRequestHeaders.UserAgent.ParseAdd(Constants.GetHttpUserAgent());
-                _client.DefaultRequestHeaders.CacheControl = new CacheControlHeaderValue
-                {
-                    NoCache = true
-                };
-            }
+            // Add default headers
+            client.DefaultRequestHeaders.UserAgent.ParseAdd(Constants.GetHttpUserAgent());
+            client.DefaultRequestHeaders.CacheControl = new CacheControlHeaderValue
+            {
+                NoCache = true
+            };
 
-            return _client;
+            return client;
         }
     }
 }

tests/Microsoft.AzureRepos.Tests/AzureDevOpsApiTests.cs

@@ -295,7 +295,7 @@ public async Task AzureDevOpsRestApi_CreatePersonalAccessTokenAsync_IdentSvcRetu
             var httpHandler = new TestHttpMessageHandler {ThrowOnUnexpectedRequest = true};
             httpHandler.Setup(HttpMethod.Get,  locSvcRequestUri,   x =>
             {
-                AssertHeader(x, Constants.Http.AcceptHeader(Constants.Http.MimeTypeJson));
+                AssertAcceptJson(x);
                 AssertBearerToken(x, accessToken);
                 return locSvcResponse;
             });

tests/Microsoft.Git.CredentialManager.Tests/HttpClientFactoryTests.cs

@@ -20,14 +20,14 @@ public void HttpClientFactory_GetClient_SetsDefaultHeaders()
         }
 
         [Fact]
-        public void HttpClientFactory_GetClient_MultipleCalls_ReturnsSameInstance()
+        public void HttpClientFactory_GetClient_MultipleCalls_ReturnsNewInstance()
         {
             var factory = new HttpClientFactory();
 
             HttpClient client1 = factory.GetClient();
             HttpClient client2 = factory.GetClient();
 
-            Assert.Same(client1, client2);
+            Assert.NotSame(client1, client2);
         }
     }
 }