diff --git a/src/Ocelot.Provider.Polly/CircuitBreaker.cs b/src/Ocelot.Provider.Polly/CircuitBreaker.cs index ce2a89bf2..c9be6d924 100644 --- a/src/Ocelot.Provider.Polly/CircuitBreaker.cs +++ b/src/Ocelot.Provider.Polly/CircuitBreaker.cs @@ -1,19 +1,12 @@ -using Polly; - namespace Ocelot.Provider.Polly { public class CircuitBreaker { - private readonly List _policies = new(); - public CircuitBreaker(params IAsyncPolicy[] policies) { - foreach (var policy in policies.Where(p => p != null)) - { - _policies.Add(policy); - } + Policies = policies.Where(p => p != null).ToArray(); } - public IAsyncPolicy[] Policies => _policies.ToArray(); + public IAsyncPolicy[] Policies { get; } } } diff --git a/src/Ocelot.Provider.Polly/Ocelot.Provider.Polly.csproj b/src/Ocelot.Provider.Polly/Ocelot.Provider.Polly.csproj index 4ffcb9eb6..78150d630 100644 --- a/src/Ocelot.Provider.Polly/Ocelot.Provider.Polly.csproj +++ b/src/Ocelot.Provider.Polly/Ocelot.Provider.Polly.csproj @@ -34,7 +34,7 @@ all - + diff --git a/src/Ocelot.Provider.Polly/OcelotBuilderExtensions.cs b/src/Ocelot.Provider.Polly/OcelotBuilderExtensions.cs index 0d167f571..229b2171e 100644 --- a/src/Ocelot.Provider.Polly/OcelotBuilderExtensions.cs +++ b/src/Ocelot.Provider.Polly/OcelotBuilderExtensions.cs @@ -20,15 +20,13 @@ public static IOcelotBuilder AddPolly(this IOcelotBuilder builder) {typeof(BrokenCircuitException), e => new RequestTimedOutError(e)}, }; - builder.Services.AddSingleton(errorMapping); - - static DelegatingHandler QosDelegatingHandlerDelegate(DownstreamRoute route, IOcelotLoggerFactory logger) - { - return new PollyCircuitBreakingDelegatingHandler(new PollyQoSProvider(route, logger), logger); - } - - builder.Services.AddSingleton((QosDelegatingHandlerDelegate)QosDelegatingHandlerDelegate); + builder.Services + .AddSingleton(errorMapping) + .AddSingleton(GetDelegatingHandler); return builder; - } + } + + private static DelegatingHandler GetDelegatingHandler(DownstreamRoute route, IOcelotLoggerFactory logger) + => new PollyCircuitBreakingDelegatingHandler(new PollyQoSProvider(route, logger), logger); } } diff --git a/src/Ocelot.Provider.Polly/PollyCircuitBreakingDelegatingHandler.cs b/src/Ocelot.Provider.Polly/PollyCircuitBreakingDelegatingHandler.cs index 9153b70ce..94d66962f 100644 --- a/src/Ocelot.Provider.Polly/PollyCircuitBreakingDelegatingHandler.cs +++ b/src/Ocelot.Provider.Polly/PollyCircuitBreakingDelegatingHandler.cs @@ -1,6 +1,5 @@ using Ocelot.Logging; using Ocelot.Provider.Polly.Interfaces; -using Polly; using Polly.CircuitBreaker; namespace Ocelot.Provider.Polly @@ -28,7 +27,7 @@ protected override async Task SendAsync(HttpRequestMessage return await base.SendAsync(request, cancellationToken); } - IAsyncPolicy policy = policies.Length > 1 + var policy = policies.Length > 1 ? Policy.WrapAsync(policies) : policies[0]; diff --git a/src/Ocelot.Provider.Polly/PollyQoSProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSProvider.cs index 420939de4..58a918162 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSProvider.cs @@ -1,65 +1,54 @@ -using Ocelot.Configuration; -using Ocelot.Logging; -using Ocelot.Provider.Polly.Interfaces; -using Polly; -using Polly.CircuitBreaker; -using Polly.Timeout; +using Ocelot.Configuration; +using Ocelot.Logging; +using Ocelot.Provider.Polly.Interfaces; +using Polly.CircuitBreaker; +using Polly.Timeout; namespace Ocelot.Provider.Polly { public class PollyQoSProvider : IPollyQoSProvider { - private readonly AsyncCircuitBreakerPolicy _circuitBreakerPolicy; - private readonly AsyncTimeoutPolicy _timeoutPolicy; - private readonly IOcelotLogger _logger; - - public PollyQoSProvider(AsyncCircuitBreakerPolicy circuitBreakerPolicy, AsyncTimeoutPolicy timeoutPolicy, IOcelotLogger logger) - { - _circuitBreakerPolicy = circuitBreakerPolicy; - _timeoutPolicy = timeoutPolicy; - _logger = logger; - } - public PollyQoSProvider(DownstreamRoute route, IOcelotLoggerFactory loggerFactory) { - _logger = loggerFactory.CreateLogger(); - - _ = Enum.TryParse(route.QosOptions.TimeoutStrategy, out TimeoutStrategy strategy); - - _timeoutPolicy = Policy.TimeoutAsync(TimeSpan.FromMilliseconds(route.QosOptions.TimeoutValue), strategy); - + AsyncCircuitBreakerPolicy circuitBreakerPolicy = null; if (route.QosOptions.ExceptionsAllowedBeforeBreaking > 0) - { - _circuitBreakerPolicy = Policy + { + var info = $"Route: {GetRouteName(route)}; Breaker logging in {nameof(PollyQoSProvider)}: "; + var logger = loggerFactory.CreateLogger(); + circuitBreakerPolicy = Policy .Handle() .Or() .Or() .CircuitBreakerAsync( exceptionsAllowedBeforeBreaking: route.QosOptions.ExceptionsAllowedBeforeBreaking, durationOfBreak: TimeSpan.FromMilliseconds(route.QosOptions.DurationOfBreak), - onBreak: (ex, breakDelay) => - { - _logger.LogError( - ".Breaker logging: Breaking the circuit for " + breakDelay.TotalMilliseconds + "ms!", ex); - }, + onBreak: (ex, breakDelay) => + logger.LogError(info + $"Breaking the circuit for {breakDelay.TotalMilliseconds} ms!", ex), onReset: () => - { - _logger.LogDebug(".Breaker logging: Call ok! Closed the circuit again."); - }, + logger.LogDebug(info + "Call OK! Closed the circuit again."), onHalfOpen: () => - { - _logger.LogDebug(".Breaker logging: Half-open; next call is a trial."); - } + logger.LogDebug(info + "Half-open; Next call is a trial.") ); } - else - { - _circuitBreakerPolicy = null; - } - CircuitBreaker = new CircuitBreaker(_circuitBreakerPolicy, _timeoutPolicy); + _ = Enum.TryParse(route.QosOptions.TimeoutStrategy, out TimeoutStrategy strategy); + var timeoutPolicy = Policy.TimeoutAsync(TimeSpan.FromMilliseconds(route.QosOptions.TimeoutValue), strategy); + CircuitBreaker = new CircuitBreaker(circuitBreakerPolicy, timeoutPolicy); + } + + private const string ObsoleteConstructorMessage = $"Use the constructor {nameof(PollyQoSProvider)}({nameof(DownstreamRoute)} route, {nameof(IOcelotLoggerFactory)} loggerFactory)!"; + + [Obsolete(ObsoleteConstructorMessage)] + public PollyQoSProvider(AsyncCircuitBreakerPolicy circuitBreakerPolicy, AsyncTimeoutPolicy timeoutPolicy, IOcelotLogger logger) + { + throw new NotSupportedException(ObsoleteConstructorMessage); } - public CircuitBreaker CircuitBreaker { get; } + public CircuitBreaker CircuitBreaker { get; } + + private static string GetRouteName(DownstreamRoute route) + => string.IsNullOrWhiteSpace(route.ServiceName) + ? route.UpstreamPathTemplate?.Template ?? route.DownstreamPathTemplate?.Value ?? string.Empty + : route.ServiceName; } } diff --git a/src/Ocelot.Provider.Polly/Usings.cs b/src/Ocelot.Provider.Polly/Usings.cs index 0cc4c6d4f..fb5c12dc6 100644 --- a/src/Ocelot.Provider.Polly/Usings.cs +++ b/src/Ocelot.Provider.Polly/Usings.cs @@ -1,12 +1,10 @@ // Default Microsoft.NET.Sdk namespaces global using System; global using System.Collections.Generic; -global using System.IO; global using System.Linq; global using System.Net.Http; global using System.Threading; global using System.Threading.Tasks; // Project extra global namespaces -global using Ocelot; global using Polly; diff --git a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj index 0c9fe46f7..8f0638fc5 100644 --- a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj +++ b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj @@ -88,7 +88,7 @@ - + diff --git a/test/Ocelot.UnitTests/Polly/PollyQoSProviderTests.cs b/test/Ocelot.UnitTests/Polly/PollyQoSProviderTests.cs index e04d56411..537712e01 100644 --- a/test/Ocelot.UnitTests/Polly/PollyQoSProviderTests.cs +++ b/test/Ocelot.UnitTests/Polly/PollyQoSProviderTests.cs @@ -1,7 +1,9 @@ using Ocelot.Configuration.Builder; using Ocelot.Logging; using Ocelot.Provider.Polly; - +using Polly.CircuitBreaker; +using Polly.Timeout; + namespace Ocelot.UnitTests.Polly { public class PollyQoSProviderTests @@ -18,7 +20,11 @@ public void Should_build() .Build(); var factory = new Mock(); var pollyQoSProvider = new PollyQoSProvider(route, factory.Object); - pollyQoSProvider.CircuitBreaker.ShouldNotBeNull(); + var policies = pollyQoSProvider.CircuitBreaker.ShouldNotBeNull() + .Policies.ShouldNotBeNull(); + policies.Length.ShouldBeGreaterThan(0); + policies.ShouldContain(p => p is AsyncCircuitBreakerPolicy); + policies.ShouldContain(p => p is AsyncTimeoutPolicy); } } }