Updated patterns using IHttpClientFactory and IOptions. Updated unit tests.

acoumb · acoumb · commit 567f133e4396 · 2022-09-01T17:13:47.000+03:00
diff --git a/src/Umbraco.Forms.Integrations.Crm.Hubspot.Tests/Services/HubspotContactServiceTests.cs b/src/Umbraco.Forms.Integrations.Crm.Hubspot.Tests/Services/HubspotContactServiceTests.cs
@@ -1,5 +1,6 @@
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using Moq;
 using Moq.Protected;
 using NUnit.Framework;
@@ -14,6 +15,7 @@
 using Umbraco.Cms.Core.Services;
 using Umbraco.Forms.Core;
 using Umbraco.Forms.Core.Persistence.Dtos;
+using Umbraco.Forms.Integrations.Crm.Hubspot.Configuration;
 using Umbraco.Forms.Integrations.Crm.Hubspot.Models;
 using Umbraco.Forms.Integrations.Crm.Hubspot.Services;
 
@@ -102,10 +104,11 @@ public class HubspotContactServiceTests
         [Test]
         public async Task GetContactProperties_WithoutApiKeyConfigured_ReturnsEmptyCollectionWithLoggedWarning()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration(withApiKey: false);
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration(withApiKey: false);
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
+            var sut = new HubspotContactService(mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var result = await sut.GetContactPropertiesAsync();
 
@@ -125,13 +128,16 @@ public async Task GetContactProperties_WithoutApiKeyConfigured_ReturnsEmptyColle
         [Test]
         public async Task GetContactProperties_WithFailedRequest_ReturnsEmptyCollectionWithLoggedError()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration();
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration();
+
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
+            mockedHttpClientFactory
+                .Setup<HttpClient>(x => x.CreateClient(It.IsAny<string>()))
+                .Returns(CreateMockedHttpClient(HttpStatusCode.InternalServerError));
+            
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
-
-            var httpClient = CreateMockedHttpClient(HttpStatusCode.InternalServerError);
-            HubspotContactService.ClientFactory = () => httpClient;
+            var sut = new HubspotContactService(mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var result = await sut.GetContactPropertiesAsync();
 
@@ -151,13 +157,16 @@ public async Task GetContactProperties_WithFailedRequest_ReturnsEmptyCollectionW
         [Test]
         public async Task GetContactProperties_WithSuccessfulRequest_ReturnsMappedAndOrderedPropertyCollection()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration();
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration();
+
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
+            mockedHttpClientFactory
+                .Setup<HttpClient>(x => x.CreateClient(It.IsAny<string>()))
+                .Returns(CreateMockedHttpClient(HttpStatusCode.OK, s_contactPropertiesResponse));
+
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
-
-            var httpClient = CreateMockedHttpClient(HttpStatusCode.OK, s_contactPropertiesResponse);
-            HubspotContactService.ClientFactory = () => httpClient;
+            var sut = new HubspotContactService(mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var result = await sut.GetContactPropertiesAsync();
 
@@ -169,10 +178,11 @@ public async Task GetContactProperties_WithSuccessfulRequest_ReturnsMappedAndOrd
         [Test]
         public async Task PostContact_WithoutApiKeyConfigured_ReturnsNotConfiguredWithLoggedWarning()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration(withApiKey: false);
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration(withApiKey: false);
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
+            var sut = new HubspotContactService( mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var record = new Record();
             var fieldMappings = new List<MappedProperty>();
@@ -194,13 +204,16 @@ public async Task PostContact_WithoutApiKeyConfigured_ReturnsNotConfiguredWithLo
         [Test]
         public async Task PostContact_WithFailedRequest_ReturnsFailedWithLoggedError()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration();
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration();
+
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
+            mockedHttpClientFactory
+                .Setup<HttpClient>(x => x.CreateClient(It.IsAny<string>()))
+                .Returns(CreateMockedHttpClient(HttpStatusCode.InternalServerError));
+
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
-
-            var httpClient = CreateMockedHttpClient(HttpStatusCode.InternalServerError);
-            HubspotContactService.ClientFactory = () => httpClient;
+            var sut = new HubspotContactService(mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var record = new Record();
             var fieldMappings = new List<MappedProperty>();
@@ -222,13 +235,16 @@ public async Task PostContact_WithFailedRequest_ReturnsFailedWithLoggedError()
         [Test]
         public async Task PostContact_WithSuccessfulRequest_ReturnSuccess()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration();
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration();
+
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
+            mockedHttpClientFactory
+                .Setup<HttpClient>(x => x.CreateClient(It.IsAny<string>()))
+                .Returns(CreateMockedHttpClient(HttpStatusCode.OK));
+
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
-
-            var httpClient = CreateMockedHttpClient(HttpStatusCode.OK);
-            HubspotContactService.ClientFactory = () => httpClient;
+            var sut = new HubspotContactService(mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var formFieldId = Guid.NewGuid();
             var record = new Record();
@@ -263,13 +279,16 @@ public async Task PostContact_WithSuccessfulRequest_ReturnSuccess()
         [Test]
         public async Task PostContact_WithSuccessfulRequestAndUnmappedField_ReturnSuccessWithLoggedWarning()
         {
-            Mock<IConfiguration> mockedConfig = CreateMockedConfiguration();
+            Mock<IOptions<HubspotSettings>> mockedConfig = CreateMockedConfiguration();
+
+            var mockedHttpClientFactory = new Mock<IHttpClientFactory>();
+            mockedHttpClientFactory
+              .Setup<HttpClient>(x => x.CreateClient(It.IsAny<string>()))
+              .Returns(CreateMockedHttpClient(HttpStatusCode.OK));
+
             var mockedLogger = new Mock<ILogger<HubspotContactService>>();
             var mockedKeyValueService = new Mock<IKeyValueService>();
-            var sut = new HubspotContactService(mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
-
-            var httpClient = CreateMockedHttpClient(HttpStatusCode.OK);
-            HubspotContactService.ClientFactory = () => httpClient;
+            var sut = new HubspotContactService(mockedHttpClientFactory.Object, mockedConfig.Object, mockedLogger.Object, AppCaches.NoCache, mockedKeyValueService.Object);
 
             var formFieldId = Guid.NewGuid();
             var record = new Record();
@@ -296,16 +315,11 @@ public async Task PostContact_WithSuccessfulRequestAndUnmappedField_ReturnSucces
             Assert.AreEqual(CommandResult.Success, result);
         }
 
-        private static Mock<IConfiguration> CreateMockedConfiguration(bool withApiKey = true)
+        private static Mock<IOptions<HubspotSettings>> CreateMockedConfiguration(bool withApiKey = true)
         {
-            var mockedConfiguration = new Mock<IConfiguration>();
-            if (withApiKey)
-            {
-                mockedConfiguration
-                    .Setup(x => x[It.Is<string>(y => y == HubspotWorkflow.HubspotApiKey)])
-                    .Returns(ApiKey);
-            }
-
+            var mockedConfiguration = new Mock<IOptions<HubspotSettings>>();
+            mockedConfiguration.Setup(x => x.Value).Returns(new HubspotSettings { ApiKey = withApiKey ? ApiKey : String.Empty });
+            
             return mockedConfiguration;
         }
 
diff --git a/src/Umbraco.Forms.Integrations.Crm.Hubspot/Configuration/HubspotSettings.cs b/src/Umbraco.Forms.Integrations.Crm.Hubspot/Configuration/HubspotSettings.cs
@@ -0,0 +1,13 @@
+﻿using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Umbraco.Forms.Integrations.Crm.Hubspot.Configuration
+{
+    public class HubspotSettings
+    {
+        public string ApiKey { get; set; }
+    }
+}
diff --git a/src/Umbraco.Forms.Integrations.Crm.Hubspot/HubspotComposer.cs b/src/Umbraco.Forms.Integrations.Crm.Hubspot/HubspotComposer.cs
@@ -4,6 +4,7 @@
 using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Notifications;
 using Umbraco.Forms.Core.Providers;
+using Umbraco.Forms.Integrations.Crm.Hubspot.Configuration;
 using Umbraco.Forms.Integrations.Crm.Hubspot.Services;
 
 namespace Umbraco.Forms.Integrations.Crm.Hubspot
@@ -12,6 +13,9 @@ public class HubspotComposer : IComposer
     {
         public void Compose(IUmbracoBuilder builder)
         {
+            builder.Services.AddOptions<HubspotSettings>()
+                .Bind(builder.Config.GetSection(HubspotWorkflow.HubspotSettings));
+
             builder.Services.AddSingleton<IContactService, HubspotContactService>();
 
             builder.AddNotificationHandler<ServerVariablesParsingNotification, HubspotServerVariablesParsingHandler>();
diff --git a/src/Umbraco.Forms.Integrations.Crm.Hubspot/HubspotWorkflow.cs b/src/Umbraco.Forms.Integrations.Crm.Hubspot/HubspotWorkflow.cs
@@ -17,7 +17,7 @@ public class HubspotWorkflow : WorkflowType
     {
         public const string HubspotWorkflowId = "c47ef1ef-22b1-4b9d-acf6-f57cb8961550";
 
-        public const string HubspotApiKey = "Umbraco:Forms:Integrations:Crm:Hubspot:Settings:ApiKey";
+        public const string HubspotSettings = "Umbraco:Forms:Integrations:Crm:Hubspot:Settings";
 
         private readonly ILogger<HubspotWorkflow> _logger;
         private readonly IContactService _contactService;
diff --git a/src/Umbraco.Forms.Integrations.Crm.Hubspot/Services/HubspotContactService.cs b/src/Umbraco.Forms.Integrations.Crm.Hubspot/Services/HubspotContactService.cs
@@ -1,5 +1,6 @@
 ﻿using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using Newtonsoft.Json;
 using System;
 using System.Collections.Generic;
@@ -13,6 +14,7 @@
 using Umbraco.Cms.Core.Cache;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Forms.Core.Persistence.Dtos;
+using Umbraco.Forms.Integrations.Crm.Hubspot.Configuration;
 using Umbraco.Forms.Integrations.Crm.Hubspot.Extensions;
 using Umbraco.Forms.Integrations.Crm.Hubspot.Models;
 using Umbraco.Forms.Integrations.Crm.Hubspot.Models.Dtos;
@@ -22,17 +24,14 @@ namespace Umbraco.Forms.Integrations.Crm.Hubspot.Services
 {
     public class HubspotContactService : IContactService
     {
-        //Using a static HttpClient(see: https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/).
-        private static readonly HttpClient s_client = new HttpClient();
+        private readonly IHttpClientFactory _httpClientFactory;
 
-        //Access to the client within the class is via ClientFactory(), allowing us to mock the responses in tests.
-        public static Func<HttpClient> ClientFactory = () => s_client;
-
-        private readonly IConfiguration _configuration;
         private readonly ILogger<HubspotContactService> _logger;
         private readonly AppCaches _appCaches;
         private readonly IKeyValueService _keyValueService;
 
+        private readonly HubspotSettings _settings;
+
         private const string CrmApiHost = "https://api.hubapi.com";
         private static readonly string CrmV3ApiBaseUrl = $"{CrmApiHost}/crm/v3/";
         private const string InstallUrlFormat = "https://app-eu1.hubspot.com/oauth/authorize?client_id={0}&redirect_uri={1}&scope={2}";
@@ -48,9 +47,15 @@ public class HubspotContactService : IContactService
 
         private const string RefreshTokenDatabaseKey = "Umbraco.Forms.Integrations.Crm.Hubspot+RefreshToken";
 
-        public HubspotContactService(IConfiguration configuration, ILogger<HubspotContactService> logger, AppCaches appCaches, IKeyValueService keyValueService)
+        public HubspotContactService(
+            IHttpClientFactory httpClientFactory,
+            IOptions<HubspotSettings> options, 
+            ILogger<HubspotContactService> logger, 
+            AppCaches appCaches, 
+            IKeyValueService keyValueService)
         {
-            _configuration = configuration;
+            _httpClientFactory = httpClientFactory;
+            _settings = options.Value;
             _logger = logger;
             _appCaches = appCaches;
             _keyValueService = keyValueService;
@@ -68,7 +73,8 @@ public async Task<AuthorizationResult> AuthorizeAsync(string code)
                 RedirectUrl = OAuthRedirectUrl,
                 AuthorizationCode = code,
             };
-            var response = await GetResponse(OAuthTokenProxyUrl, HttpMethod.Post, content: tokenRequest, contentType: "application/x-www-form-urlencoded").ConfigureAwait(false);
+            var response = await GetResponse(OAuthTokenProxyUrl, 
+                HttpMethod.Post, content: tokenRequest, contentType: "application/x-www-form-urlencoded").ConfigureAwait(false);
             if (response.IsSuccessStatusCode)
             {
                 var responseContent = await response.Content.ReadAsStringAsync();
@@ -273,7 +279,7 @@ private AuthenticationDetail GetConfiguredAuthenticationDetails()
 
         private bool TryGetApiKey(out string apiKey)
         {
-            apiKey = _configuration[HubspotWorkflow.HubspotApiKey];
+            apiKey = _settings.ApiKey;
 
             return !string.IsNullOrEmpty(apiKey);
         }
@@ -340,6 +346,8 @@ private async Task<HttpResponseMessage> GetResponse(
             object content = null,
             string contentType = null)
         {
+            var httpClient = _httpClientFactory.CreateClient();
+            
             var requestMessage = new HttpRequestMessage
             {
                 Method = httpMethod,
@@ -362,7 +370,7 @@ private async Task<HttpResponseMessage> GetResponse(
                 }
             }
 
-            return await ClientFactory().SendAsync(requestMessage).ConfigureAwait(false);
+            return await httpClient.SendAsync(requestMessage).ConfigureAwait(false);
         }
 
         private static HttpContent CreateRequestContent(object data, string contentType)

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ public class HubspotWorkflow : WorkflowType`
`17`	`17`	`{`
`18`	`18`	`public const string HubspotWorkflowId = "c47ef1ef-22b1-4b9d-acf6-f57cb8961550";`
`19`	`19`
`20`		`- public const string HubspotApiKey = "Umbraco:Forms:Integrations:Crm:Hubspot:Settings:ApiKey";`
	`20`	`+ public const string HubspotSettings = "Umbraco:Forms:Integrations:Crm:Hubspot:Settings";`
`21`	`21`
`22`	`22`	`private readonly ILogger<HubspotWorkflow> _logger;`
`23`	`23`	`private readonly IContactService _contactService;`