Update Azure Repos REST API class to reuse it's HttpClients

Update the Azure Repos REST API class to only create one `HttpClient`
instance, configure common headers, and reuse the same instance for
all requests.

Author: mjcheetham

src/Microsoft.AzureRepos/AzureDevOpsRestApi.cs

@@ -20,7 +20,6 @@ public interface IAzureDevOpsRestApi
 
     public class AzureDevOpsRestApi : IAzureDevOpsRestApi
     {
-
         private readonly ICommandContext _context;
         private readonly IHttpClientFactory _httpFactory;
 
@@ -44,12 +43,8 @@ public async Task<string> GetAuthorityAsync(Uri organizationUri)
             const string commonAuthority = authorityBase + "common";
             const string msaAuthority = authorityBase + "live.com";
 
-            HttpClient client = _httpFactory.GetClient();
-            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(Constants.Http.MimeTypeJson));
-
             _context.Trace.WriteLine($"HTTP: HEAD {organizationUri}");
-            using (client)
-            using (var response = await client.HeadAsync(organizationUri))
+            using (HttpResponseMessage response = await HttpClient.HeadAsync(organizationUri))
             {
                 _context.Trace.WriteLine("HTTP: Response code ignored.");
                 _context.Trace.WriteLine("Inspecting headers...");
@@ -113,14 +108,10 @@ public async Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, st
 
             Uri requestUri = new Uri(identityServiceUri, sessionTokenUrl);
 
-            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 response = await client.PostAsync(requestUri, content))
+            using (HttpRequestMessage request = CreateRequestMessage(HttpMethod.Post, requestUri, content, accessToken))
+            using (HttpResponseMessage response = await HttpClient.SendAsync(request))
             {
                 _context.Trace.WriteLine($"HTTP: Response {(int)response.StatusCode} [{response.StatusCode}]");
 
@@ -161,13 +152,9 @@ private async Task<Uri> GetIdentityServiceUriAsync(Uri organizationUri, string a
                 Query = locationServiceQuery,
             }.Uri;
 
-            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 (client)
-            using (var response = await client.GetAsync(requestUri))
+            using (HttpRequestMessage request = CreateRequestMessage(HttpMethod.Get, requestUri, bearerToken: accessToken))
+            using (HttpResponseMessage response = await HttpClient.SendAsync(request))
             {
                 _context.Trace.WriteLine($"HTTP: Response {(int)response.StatusCode} [{response.StatusCode}]");
                 if (response.IsSuccessStatusCode)
@@ -191,6 +178,24 @@ private async Task<Uri> GetIdentityServiceUriAsync(Uri organizationUri, string a
 
         private const RegexOptions CommonRegexOptions = RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase;
 
+        private HttpClient _httpClient;
+
+        private HttpClient HttpClient
+        {
+            get
+            {
+                if (_httpClient is null)
+                {
+                    _httpClient = _httpFactory.CreateClient();
+
+                    // Configure the HTTP client with standard headers for Azure Repos API calls
+                    _httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(Constants.Http.MimeTypeJson));
+                }
+
+                return _httpClient;
+            }
+        }
+
         /// <summary>
         /// Attempt to extract the authority from a Authorization Bearer header.
         /// </summary>
@@ -267,6 +272,31 @@ private static StringContent CreateAccessTokenRequestJson(Uri organizationUri, I
             return content;
         }
 
+        /// <summary>
+        /// Create an <see cref="HttpRequestMessage"/> with optional content and bearer-token authorization header.
+        /// </summary>
+        /// <param name="method">HTTP request method type.</param>
+        /// <param name="uri">Request URI.</param>
+        /// <param name="content">Optional request content.</param>
+        /// <param name="bearerToken">Optional bearer token for authorization.</param>
+        /// <returns>HTTP request message.</returns>
+        private static HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri uri, HttpContent content = null, string bearerToken = null)
+        {
+            var request = new HttpRequestMessage(method, uri);
+
+            if (!(content is null))
+            {
+                request.Content = content;
+            }
+
+            if (!(bearerToken is null))
+            {
+                request.Headers.Authorization = new AuthenticationHeaderValue(Constants.Http.WwwAuthenticateBearerScheme, bearerToken);
+            }
+
+            return request;
+        }
+
         #endregion
     }
 }

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

@@ -33,8 +33,7 @@ public async Task<bool> GetIsSupportedAsync(Uri uri)
             bool supported = false;
 
             _context.Trace.WriteLine($"HTTP: HEAD {uri}");
-            using (HttpClient client = _httpFactory.GetClient())
-            using (HttpResponseMessage response = await client.HeadAsync(uri))
+            using (HttpResponseMessage response = await HttpClient.HeadAsync(uri))
             {
                 _context.Trace.WriteLine("HTTP: Response code ignored.");
 
@@ -56,5 +55,8 @@ public async Task<bool> GetIsSupportedAsync(Uri uri)
 
             return supported;
         }
+
+        private HttpClient _httpClient;
+        private HttpClient HttpClient => _httpClient ?? (_httpClient = _httpFactory.CreateClient());
     }
 }

src/Microsoft.Git.CredentialManager/HttpClientFactory.cs

@@ -6,20 +6,28 @@
 namespace Microsoft.Git.CredentialManager
 {
     /// <summary>
-    /// Acquires <see cref="HttpClient"/>s that have been configured for use in Git Credential Manager.
+    /// Constructs <see cref="HttpClient"/>s that have been configured for use in Git Credential Manager.
     /// </summary>
     public interface IHttpClientFactory
     {
         /// <summary>
-        /// Get an instance of <see cref="HttpClient"/> with default request headers set.
+        /// Get a new instance of <see cref="HttpClient"/> with default request headers set.
         /// </summary>
-        /// <returns>Client with default headers.</returns>
-        HttpClient GetClient();
+        /// <remarks>
+        /// Callers should reuse instances of <see cref="HttpClient"/> returned from this method as long
+        /// as they are needed, rather than repeatably call this method to create new ones.
+        /// <para/>
+        /// Creating a new <see cref="HttpClient"/> consumes one free socket which may not be released
+        /// by the Operating System until sometime after the client is disposed, leading to possible free
+        /// socket exhaustion.
+        /// </remarks>
+        /// <returns>New client instance with default headers.</returns>
+        HttpClient CreateClient();
     }
 
     public class HttpClientFactory : IHttpClientFactory
     {
-        public HttpClient GetClient()
+        public HttpClient CreateClient()
         {
             // Initialize a new HttpClient
             var client = new HttpClient();

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

@@ -12,7 +12,7 @@ public void HttpClientFactory_GetClient_SetsDefaultHeaders()
         {
             var factory = new HttpClientFactory();
 
-            HttpClient client = factory.GetClient();
+            HttpClient client = factory.CreateClient();
 
             Assert.NotNull(client);
             Assert.Equal(Constants.GetHttpUserAgent(), client.DefaultRequestHeaders.UserAgent.ToString());
@@ -24,8 +24,8 @@ public void HttpClientFactory_GetClient_MultipleCalls_ReturnsNewInstance()
         {
             var factory = new HttpClientFactory();
 
-            HttpClient client1 = factory.GetClient();
-            HttpClient client2 = factory.GetClient();
+            HttpClient client1 = factory.CreateClient();
+            HttpClient client2 = factory.CreateClient();
 
             Assert.NotSame(client1, client2);
         }

tests/Microsoft.Git.CredentialManager.Tests/Objects/TestHttpClientFactory.cs

@@ -10,7 +10,7 @@ public class TestHttpClientFactory : IHttpClientFactory
 
         #region IHttpClientFactory
 
-        HttpClient IHttpClientFactory.GetClient()
+        HttpClient IHttpClientFactory.CreateClient()
         {
             return new HttpClient(MessageHandler);
         }