Refactoring SendGridClient to support inject external managed HttpClient (#839)

Fix issue #670

Author: akunzai

src/SendGrid/SendGridClient.cs

@@ -1,4 +1,4 @@
-// <copyright file="SendGridClient.cs" company="Twilio SendGrid">
+// <copyright file="SendGridClient.cs" company="Twilio SendGrid">
 // Copyright (c) Twilio SendGrid. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 // </copyright>
@@ -24,12 +24,29 @@ namespace SendGrid
     /// </summary>
     public class SendGridClient : ISendGridClient
     {
-        private readonly SendGridClientOptions options = new SendGridClientOptions();
+        private const string Scheme = "Bearer";
+        private const string ContentType = "Content-Type";
+        private const string DefaultMediaType = "application/json";
+
+        /// <summary>
+        /// The <see cref="SendGridClient"/> assembly version to send in request User-Agent header
+        /// </summary>
+        private static readonly string ClientVersion = typeof(SendGridClient).GetTypeInfo().Assembly.GetName().Version.ToString();
+
+        /// <summary>
+        /// The default configuration settings to use with the SendGrid client
+        /// </summary>
+        private static readonly SendGridClientOptions DefaultOptions = new SendGridClientOptions();
+
+        /// <summary>
+        /// The configuration to use with current <see cref="SendGridClient"/> instance
+        /// </summary>
+        private readonly SendGridClientOptions options;
 
         /// <summary>
         /// The HttpClient instance to use for all calls from this SendGridClient instance.
         /// </summary>
-        private HttpClient client;
+        private readonly HttpClient client;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="SendGridClient"/> class.
@@ -42,27 +59,8 @@ public class SendGridClient : ISendGridClient
         /// <param name="urlPath">Path to endpoint (e.g. /path/to/endpoint)</param>
         /// <returns>Interface to the Twilio SendGrid REST API</returns>
         public SendGridClient(IWebProxy webProxy, string apiKey, string host = null, Dictionary<string, string> requestHeaders = null, string version = "v3", string urlPath = null)
+            : this(CreateHttpClientWithWebProxy(webProxy), new SendGridClientOptions { ApiKey = apiKey, Host = host, RequestHeaders = requestHeaders, Version = version, UrlPath = urlPath })
         {
-            // Create client with WebProxy if set
-            if (webProxy != null)
-            {
-                var httpClientHandler = new HttpClientHandler()
-                {
-                    Proxy = webProxy,
-                    PreAuthenticate = true,
-                    UseDefaultCredentials = false,
-                };
-
-                var retryHandler = new RetryDelegatingHandler(httpClientHandler, this.options.ReliabilitySettings);
-
-                this.client = new HttpClient(retryHandler);
-            }
-            else
-            {
-                this.client = this.CreateHttpClientWithRetryHandler();
-            }
-
-            this.InitiateClient(apiKey, host, requestHeaders, version, urlPath);
         }
 
         /// <summary>
@@ -86,7 +84,7 @@ public SendGridClient(SendGridClientOptions options)
         /// <param name="urlPath">Path to endpoint (e.g. /path/to/endpoint)</param>
         /// <returns>Interface to the Twilio SendGrid REST API</returns>
         public SendGridClient(HttpClient httpClient, string apiKey, string host = null, Dictionary<string, string> requestHeaders = null, string version = "v3", string urlPath = null)
-            : this(httpClient, new SendGridClientOptions() { ApiKey = apiKey, Host = host, RequestHeaders = requestHeaders, Version = version, UrlPath = urlPath })
+            : this(httpClient, new SendGridClientOptions { ApiKey = apiKey, Host = host, RequestHeaders = requestHeaders, Version = version, UrlPath = urlPath })
         {
         }
 
@@ -100,27 +98,24 @@ public SendGridClient(HttpClient httpClient, string apiKey, string host = null,
         /// <param name="urlPath">Path to endpoint (e.g. /path/to/endpoint)</param>
         /// <returns>Interface to the Twilio SendGrid REST API</returns>
         public SendGridClient(string apiKey, string host = null, Dictionary<string, string> requestHeaders = null, string version = "v3", string urlPath = null)
-            : this(httpClient: null, apiKey: apiKey, host: host, requestHeaders: requestHeaders, version: version, urlPath: urlPath)
+            : this(null, new SendGridClientOptions { ApiKey = apiKey, Host = host, RequestHeaders = requestHeaders, Version = version, UrlPath = urlPath })
         {
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="SendGridClient"/> class.
         /// </summary>
-        /// <param name="httpClient">An optional http client which may me injected in order to facilitate testing.</param>
+        /// <param name="httpClient">An optional HTTP client which may me injected in order to facilitate testing.</param>
         /// <param name="options">A <see cref="SendGridClientOptions"/> instance that defines the configuration settings to use with the client </param>
         /// <returns>Interface to the Twilio SendGrid REST API</returns>
         internal SendGridClient(HttpClient httpClient, SendGridClientOptions options)
         {
-            if (options == null)
+            this.options = options ?? throw new ArgumentNullException(nameof(options));
+            this.client = httpClient ?? CreateHttpClientWithRetryHandler();
+            if (this.options.RequestHeaders != null && this.options.RequestHeaders.TryGetValue(ContentType, out var contentType))
             {
-                throw new ArgumentNullException(nameof(options));
+                this.MediaType = contentType;
             }
-
-            this.options = options;
-            this.client = (httpClient == null) ? this.CreateHttpClientWithRetryHandler() : httpClient;
-
-            this.InitiateClient(options.ApiKey, options.Host, options.RequestHeaders, options.Version, options.UrlPath);
         }
 
         /// <summary>
@@ -157,17 +152,25 @@ public enum Method
         /// <summary>
         /// Gets or sets the path to the API resource.
         /// </summary>
-        public string UrlPath { get; set; }
+        public string UrlPath
+        {
+            get => this.options.UrlPath;
+            set => this.options.UrlPath = value;
+        }
 
         /// <summary>
         /// Gets or sets the API version.
         /// </summary>
-        public string Version { get; set; }
+        public string Version
+        {
+            get => this.options.Version;
+            set => this.options.Version = value;
+        }
 
         /// <summary>
         /// Gets or sets the request media type.
         /// </summary>
-        public string MediaType { get; set; }
+        public string MediaType { get; set; } = DefaultMediaType;
 
         /// <summary>
         /// Add the authorization header, override to customize
@@ -197,37 +200,48 @@ public virtual AuthenticationHeaderValue AddAuthorization(KeyValuePair<string, s
         /// </summary>
         /// <param name="method">HTTP verb</param>
         /// <param name="requestBody">JSON formatted string</param>
-        /// <param name="queryParams">JSON formatted query paramaters</param>
+        /// <param name="queryParams">JSON formatted query parameters</param>
         /// <param name="urlPath">The path to the API endpoint.</param>
         /// <param name="cancellationToken">Cancel the asynchronous call.</param>
         /// <returns>Response object</returns>
         /// <exception cref="Exception">The method will NOT catch and swallow exceptions generated by sending a request
-        /// through the internal http client. Any underlying exception will pass right through.
+        /// through the internal HTTP client. Any underlying exception will pass right through.
         /// In particular, this means that you may expect
-        /// a TimeoutException if you are not connected to the internet.</exception>
+        /// a TimeoutException if you are not connected to the Internet.</exception>
         public async Task<Response> RequestAsync(
             SendGridClient.Method method,
             string requestBody = null,
             string queryParams = null,
             string urlPath = null,
             CancellationToken cancellationToken = default(CancellationToken))
         {
-            var endpoint = this.client.BaseAddress + this.BuildUrl(urlPath, queryParams);
-
-            // Build the request body
-            StringContent content = null;
-            if (requestBody != null)
+            var baseAddress = new Uri(string.IsNullOrWhiteSpace(this.options.Host) ? DefaultOptions.Host : this.options.Host);
+            if (!baseAddress.OriginalString.EndsWith("/"))
             {
-                content = new StringContent(requestBody, Encoding.UTF8, this.MediaType);
+                baseAddress = new Uri(baseAddress.OriginalString + "/");
             }
 
             // Build the final request
             var request = new HttpRequestMessage
             {
                 Method = new HttpMethod(method.ToString()),
-                RequestUri = new Uri(endpoint),
-                Content = content
+                RequestUri = new Uri(baseAddress, this.BuildUrl(urlPath, queryParams)),
+                Content = requestBody == null ? null : new StringContent(requestBody, Encoding.UTF8, this.MediaType)
             };
+
+            // set header overrides
+            if (this.options.RequestHeaders?.Count > 0)
+            {
+                foreach (var header in this.options.RequestHeaders)
+                {
+                    request.Headers.TryAddWithoutValidation(header.Key, header.Value);
+                }
+            }
+
+            // set standard headers
+            request.Headers.Authorization = new AuthenticationHeaderValue(Scheme, this.options.ApiKey);
+            request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(this.MediaType));
+            request.Headers.UserAgent.TryParseAdd($"sendgrid/{ClientVersion} csharp");
             return await this.MakeRequest(request, cancellationToken).ConfigureAwait(false);
         }
 
@@ -246,6 +260,37 @@ public async Task<Response> RequestAsync(
                 cancellationToken: cancellationToken).ConfigureAwait(false);
         }
 
+        private static HttpClient CreateHttpClientWithRetryHandler()
+        {
+            return new HttpClient(new RetryDelegatingHandler(DefaultOptions.ReliabilitySettings));
+        }
+
+        /// <summary>
+        /// Create client with WebProxy if set
+        /// </summary>
+        /// <param name="webProxy">the WebProxy</param>
+        /// <returns>HttpClient with RetryDelegatingHandler and WebProxy if set</returns>
+        private static HttpClient CreateHttpClientWithWebProxy(IWebProxy webProxy)
+        {
+            if (webProxy != null)
+            {
+                var httpClientHandler = new HttpClientHandler()
+                {
+                    Proxy = webProxy,
+                    PreAuthenticate = true,
+                    UseDefaultCredentials = false,
+                };
+
+                var retryHandler = new RetryDelegatingHandler(httpClientHandler, DefaultOptions.ReliabilitySettings);
+
+                return new HttpClient(retryHandler);
+            }
+            else
+            {
+                return CreateHttpClientWithRetryHandler();
+            }
+        }
+
         /// <summary>
         /// Build the final URL
         /// </summary>
@@ -258,12 +303,12 @@ private string BuildUrl(string urlPath, string queryParams = null)
         {
             string url = null;
 
-            // create urlPAth - from parameter if overridden on call or from ctor parameter
-            var urlpath = urlPath ?? this.UrlPath;
+            // create urlPAth - from parameter if overridden on call or from constructor parameter
+            var urlpath = urlPath ?? this.options.UrlPath;
 
-            if (this.Version != null)
+            if (this.options.Version != null)
             {
-                url = this.Version + "/" + urlpath;
+                url = this.options.Version + "/" + urlpath;
             }
             else
             {
@@ -280,79 +325,19 @@ private string BuildUrl(string urlPath, string queryParams = null)
                     {
                         if (query != "?")
                         {
-                            query = query + "&";
+                            query += "&";
                         }
 
                         query = query + pair.Key + "=" + element;
                     }
                 }
 
-                url = url + query;
+                url += query;
             }
 
             return url;
         }
 
-        private HttpClient CreateHttpClientWithRetryHandler()
-        {
-            return new HttpClient(new RetryDelegatingHandler(this.options.ReliabilitySettings));
-        }
-
-        /// <summary>
-        /// Common method to initiate internal fields regardless of which constructor was used.
-        /// </summary>
-        /// <param name="apiKey">Your Twilio SendGrid API key.</param>
-        /// <param name="host">Base url (e.g. https://api.sendgrid.com)</param>
-        /// <param name="requestHeaders">A dictionary of request headers</param>
-        /// <param name="version">API version, override AddVersion to customize</param>
-        /// <param name="urlPath">Path to endpoint (e.g. /path/to/endpoint)</param>
-        private void InitiateClient(string apiKey, string host, Dictionary<string, string> requestHeaders, string version, string urlPath)
-        {
-            this.UrlPath = urlPath;
-            this.Version = version;
-
-            var baseAddress = host ?? "https://api.sendgrid.com";
-            var clientVersion = this.GetType().GetTypeInfo().Assembly.GetName().Version.ToString();
-
-            // standard headers
-            this.client.BaseAddress = new Uri(baseAddress);
-            Dictionary<string, string> headers = new Dictionary<string, string>
-            {
-                { "Authorization", "Bearer " + apiKey },
-                { "Content-Type", "application/json" },
-                { "User-Agent", "sendgrid/" + clientVersion + " csharp" },
-                { "Accept", "application/json" }
-            };
-
-            // set header overrides
-            if (requestHeaders != null)
-            {
-                foreach (var header in requestHeaders)
-                {
-                    headers[header.Key] = header.Value;
-                }
-            }
-
-            // add headers to httpClient
-            foreach (var header in headers)
-            {
-                if (header.Key == "Authorization")
-                {
-                    var split = header.Value.Split();
-                    this.client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(split[0], split[1]);
-                }
-                else if (header.Key == "Content-Type")
-                {
-                    this.client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(header.Value));
-                    this.MediaType = header.Value;
-                }
-                else
-                {
-                    this.client.DefaultRequestHeaders.Add(header.Key, header.Value);
-                }
-            }
-        }
-
         /// <summary>
         /// Parses a JSON string without removing duplicate keys.
         /// </summary>

tests/SendGrid.Tests/Integration.cs

@@ -6125,6 +6125,19 @@ public async Task TestRetryBehaviourSucceedsOnSecondAttempt()
 
             Assert.Equal(HttpStatusCode.OK, result.StatusCode);
         }
+
+        /// <summary>
+        /// Tests the conditions in issue #670.
+        /// </summary>
+        /// <returns></returns>
+        [Fact]
+        public void TestInjectSameHttpClientWithMultipleInstance()
+        {
+            var httpMessageHandler = new FixedStatusAndMessageHttpMessageHandler(HttpStatusCode.Accepted, string.Empty);
+            var clientToInject = new HttpClient(httpMessageHandler);
+            var sg1 = new SendGridClient(clientToInject, fixture.apiKey);
+            var sg2 = new SendGridClient(clientToInject, fixture.apiKey);
+        }
     }
 
     public class FakeWebProxy : IWebProxy