Merge pull request #57 from dodopizza/friendly-settings

Make settings more usable
dodopizza · Nov 21, 2020 · 104c477 · 104c477
2 parents bb95d36 + d3be611
commit 104c477
Show file tree

Hide file tree

Showing 20 changed files with 254 additions and 293 deletions.
diff --git a/src/Dodo.HttpClient.ResiliencePolicies.Tests/CircuitBreakerPolicyTests.cs b/src/Dodo.HttpClient.ResiliencePolicies.Tests/CircuitBreakerPolicyTests.cs
@@ -17,36 +17,39 @@ public void Should_break_after_4_concurrent_calls()
 		{
 			const int retryCount = 5;
 			const int minimumThroughput = 2;
-			var retrySettings = RetryPolicySettings.Constant(retryCount, TimeSpan.FromMilliseconds(50));
-
+			var settings = new ResiliencePoliciesSettings
+			{
+				OverallTimeout = TimeSpan.FromSeconds(5),
+				RetryPolicySettings = RetryPolicySettings.Constant(retryCount, TimeSpan.FromMilliseconds(100)),
+				CircuitBreakerPolicySettings = BuildCircuitBreakerSettings(minimumThroughput),
+			};
 			var wrapper = Create.HttpClientWrapperWrapperBuilder
 				.WithStatusCode(HttpStatusCode.ServiceUnavailable)
-				.WithTimeoutOverall(TimeSpan.FromSeconds(5))
-				.WithCircuitBreakerSettings(BuildCircuitBreakerSettings(minimumThroughput))
-				.WithRetrySettings(retrySettings)
+				.WithResiliencePolicySettings(settings)
 				.Please();
 
 			const int taskCount = 4;
 			Assert.CatchAsync<BrokenCircuitException>(async () =>
 				await Helper.InvokeMultipleHttpRequests(wrapper.Client, taskCount));
 
-			Assert.AreEqual(minimumThroughput, wrapper.NumberOfCalls);
+			Assert.LessOrEqual(wrapper.NumberOfCalls, taskCount);
 		}
 
 		[Test]
 		public async Task Should_Open_Circuit_Breaker_for_RU_and_do_not_affect_EE()
 		{
 			const int retryCount = 5;
 			const int minimumThroughput = 2;
-			var retrySettings = RetryPolicySettings.Constant(retryCount, TimeSpan.FromMilliseconds(50));
-
-			var circuitBreakerSettings = BuildCircuitBreakerSettings(minimumThroughput);
+			var settings = new ResiliencePoliciesSettings
+			{
+				OverallTimeout = TimeSpan.FromSeconds(5),
+				RetryPolicySettings =RetryPolicySettings.Constant(retryCount, TimeSpan.FromMilliseconds(50)),
+				CircuitBreakerPolicySettings = BuildCircuitBreakerSettings(minimumThroughput),
+			};
 			var wrapper = Create.HttpClientWrapperWrapperBuilder
 				.WithHostAndStatusCode("ru-prod.com", HttpStatusCode.ServiceUnavailable)
 				.WithHostAndStatusCode("ee-prod.com", HttpStatusCode.OK)
-				.WithTimeoutOverall(TimeSpan.FromSeconds(5))
-				.WithCircuitBreakerSettings(circuitBreakerSettings)
-				.WithRetrySettings(retrySettings)
+				.WithResiliencePolicySettings(settings)
 				.Please();
 
 			const int taskCount = 4;
@@ -60,7 +63,7 @@ public async Task Should_Open_Circuit_Breaker_for_RU_and_do_not_affect_EE()
 			Assert.AreEqual(minimumThroughput + taskCount, wrapper.NumberOfCalls);
 		}
 
-		private static ICircuitBreakerPolicySettings BuildCircuitBreakerSettings(int throughput)
+		private static CircuitBreakerPolicySettings BuildCircuitBreakerSettings(int throughput)
 		{
 			return new CircuitBreakerPolicySettings(
 				failureThreshold: 0.5,

diff --git a/src/Dodo.HttpClient.ResiliencePolicies.Tests/DSL/HttpClientWrapperBuilder.cs b/src/Dodo.HttpClient.ResiliencePolicies.Tests/DSL/HttpClientWrapperBuilder.cs
@@ -2,10 +2,7 @@
 using System.Collections.Generic;
 using System.Net;
 using System.Net.Http;
-using Dodo.HttpClientResiliencePolicies.CircuitBreakerPolicy;
-using Dodo.HttpClientResiliencePolicies.RetryPolicy;
 using Dodo.HttpClientResiliencePolicies.Tests.Fakes;
-using Dodo.HttpClientResiliencePolicies.TimeoutPolicy;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Dodo.HttpClientResiliencePolicies.Tests.DSL
@@ -18,13 +15,10 @@ public sealed class HttpClientWrapperBuilder
 		private readonly Dictionary<string, HttpStatusCode> _hostsResponseCodes =
 			new Dictionary<string, HttpStatusCode>();
 
-		private IRetryPolicySettings _retrySettings;
-		private ICircuitBreakerPolicySettings _circuitBreakerSettings;
-		private TimeSpan _timeoutPerTry = TimeSpan.FromDays(1);
-		private TimeSpan _timeoutOverall = TimeSpan.FromDays(1);
+		private ResiliencePoliciesSettings _resiliencePoliciesSettings;
 		private TimeSpan _responseLatency = TimeSpan.Zero;
-		private TimeSpan? _retryAfterSpan = null;
-		private DateTime? _retryAfterDate = null;
+		private TimeSpan? _retryAfterSpan;
+		private DateTime? _retryAfterDate;
 
 		public HttpClientWrapperBuilder WithStatusCode(HttpStatusCode statusCode)
 		{
@@ -38,27 +32,9 @@ public HttpClientWrapperBuilder WithHostAndStatusCode(string host, HttpStatusCod
 			return this;
 		}
 
-		public HttpClientWrapperBuilder WithTimeoutOverall(TimeSpan timeoutOverall)
+		public HttpClientWrapperBuilder WithResiliencePolicySettings(ResiliencePoliciesSettings resiliencePoliciesSettings)
 		{
-			_timeoutOverall = timeoutOverall;
-			return this;
-		}
-
-		public HttpClientWrapperBuilder WithTimeoutPerTry(TimeSpan timeoutPerTry)
-		{
-			_timeoutPerTry = timeoutPerTry;
-			return this;
-		}
-
-		public HttpClientWrapperBuilder WithRetrySettings(IRetryPolicySettings retrySettings)
-		{
-			_retrySettings = retrySettings;
-			return this;
-		}
-
-		public HttpClientWrapperBuilder WithCircuitBreakerSettings(ICircuitBreakerPolicySettings circuitBreakerSettings)
-		{
-			_circuitBreakerSettings = circuitBreakerSettings;
+			_resiliencePoliciesSettings = resiliencePoliciesSettings;
 			return this;
 		}
 
@@ -94,7 +70,7 @@ public HttpClientWrapper Please()
 				handler.SetRetryAfterResponseHeader(_retryAfterSpan.Value);
 			}
 
-			var settings = BuildClientSettings();
+			var settings = _resiliencePoliciesSettings ?? new ResiliencePoliciesSettings();
 			var services = new ServiceCollection();
 			services
 				.AddJsonClient<IMockJsonClient, MockJsonClient>(_uri, settings, ClientName)
@@ -106,24 +82,5 @@ public HttpClientWrapper Please()
 			             throw new NullReferenceException($"\"{nameof(factory)}\" was not created properly");
 			return new HttpClientWrapper(client, handler);
 		}
-
-		private ResiliencePoliciesSettings BuildClientSettings()
-		{
-			var defaultCircuitBreakerSettings = _circuitBreakerSettings ?? new CircuitBreakerPolicySettings
-			(
-				failureThreshold: 0.5,
-				minimumThroughput: int.MaxValue,
-				durationOfBreak: TimeSpan.FromMilliseconds(1),
-				samplingDuration: TimeSpan.FromMilliseconds(20)
-			);
-
-			return new ResiliencePoliciesSettings
-			{
-				OverallTimeoutPolicySettings = new OverallTimeoutPolicySettings(_timeoutOverall),
-				TimeoutPerTryPolicySettings = new TimeoutPerTryPolicySettings(_timeoutPerTry),
-				RetrySettings = _retrySettings ?? new RetryPolicySettings(),
-				CircuitBreakerSettings = defaultCircuitBreakerSettings
-			};
-		}
 	}
 }
diff --git a/src/Dodo.HttpClient.ResiliencePolicies.Tests/Dodo.HttpClient.ResiliencePolicies.Tests.csproj b/src/Dodo.HttpClient.ResiliencePolicies.Tests/Dodo.HttpClient.ResiliencePolicies.Tests.csproj
@@ -6,6 +6,7 @@
     <LangVersion>8.0</LangVersion>
     <IsPackable>false</IsPackable>
     <RootNamespace>Dodo.HttpClientResiliencePolicies.Tests</RootNamespace>
+    <WarningsAsErrors>true</WarningsAsErrors>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/src/Dodo.HttpClient.ResiliencePolicies.Tests/Helper.cs b/src/Dodo.HttpClient.ResiliencePolicies.Tests/Helper.cs
@@ -3,8 +3,6 @@
 
 namespace Dodo.HttpClientResiliencePolicies.Tests
 {
-	using HttpClient = HttpClient;
-
 	public static class Helper
 	{
 		public static async Task InvokeMultipleHttpRequests(HttpClient client, int taskCount, string uri = "http://localhost")

diff --git a/....HttpClient.ResiliencePolicies.Tests/HttpClientFactoryServiceCollectionExtensionsTests.cs b/....HttpClient.ResiliencePolicies.Tests/HttpClientFactoryServiceCollectionExtensionsTests.cs
@@ -1,7 +1,6 @@
 using System;
 using System.Net.Http;
 using Dodo.HttpClientResiliencePolicies.Tests.Fakes;
-using Dodo.HttpClientResiliencePolicies.TimeoutPolicy;
 using Microsoft.Extensions.DependencyInjection;
 using NUnit.Framework;
 
@@ -67,7 +66,7 @@ public void When_AddJsonClient_WithSpecificOverallTimeout_than_ConfiguresSpecifi
 				new Uri("http://example.com/"),
 				new ResiliencePoliciesSettings
 				{
-					OverallTimeoutPolicySettings = new OverallTimeoutPolicySettings(overallTimeout),
+					OverallTimeout = overallTimeout,
 				});
 
 			var services = serviceCollection.BuildServiceProvider();

diff --git a/src/Dodo.HttpClient.ResiliencePolicies.Tests/ResiliencePolicySettingsTests.cs b/src/Dodo.HttpClient.ResiliencePolicies.Tests/ResiliencePolicySettingsTests.cs
@@ -0,0 +1,50 @@
+using System.Net;
+using System.Threading.Tasks;
+using Dodo.HttpClientResiliencePolicies.RetryPolicy;
+using Dodo.HttpClientResiliencePolicies.Tests.DSL;
+using NUnit.Framework;
+
+namespace Dodo.HttpClientResiliencePolicies.Tests
+{
+	[TestFixture]
+	public class ResiliencePolicySettingsTests
+	{
+		[Test]
+		public async Task Should_catch_retry_in_OnRetry_handler_passed_after_RetryPolicySettings()
+		{
+			var retryCounter = 0;
+			var settings = new ResiliencePoliciesSettings
+			{
+				RetryPolicySettings = RetryPolicySettings.Constant(Defaults.Retry.RetryCount),
+				OnRetry = (_, __) => { retryCounter++; },
+			};
+			var wrapper = Create.HttpClientWrapperWrapperBuilder
+				.WithStatusCode(HttpStatusCode.ServiceUnavailable)
+				.WithResiliencePolicySettings(settings)
+				.Please();
+
+			await wrapper.Client.GetAsync("http://localhost");
+
+			Assert.AreEqual(Defaults.Retry.RetryCount, retryCounter);
+		}
+
+		[Test]
+		public async Task Should_catch_retry_in_OnRetry_handler_passed_before_RetryPolicySettings()
+		{
+			var retryCounter = 0;
+			var settings = new ResiliencePoliciesSettings
+			{
+				OnRetry = (_, __) => { retryCounter++; },
+				RetryPolicySettings = RetryPolicySettings.Constant(Defaults.Retry.RetryCount),
+			};
+			var wrapper = Create.HttpClientWrapperWrapperBuilder
+				.WithStatusCode(HttpStatusCode.ServiceUnavailable)
+				.WithResiliencePolicySettings(settings)
+				.Please();
+
+			await wrapper.Client.GetAsync("http://localhost");
+
+			Assert.AreEqual(Defaults.Retry.RetryCount, retryCounter);
+		}
+	}
+}