ThreeMammals#1712 Bump to Polly 8.0 (ThreeMammals#1714)

* ThreeMammals#1712 Migrate to Polly 8.0 * code review post merge * post PR * ThreeMammals#1712 Migrate to Polly 8.0 * code review post merge * Update src/Ocelot.Provider.Polly/PollyQoSProvider.cs Co-authored-by: Raman Maksimchuk <[email protected]> * namespaces * Refactor QoS provider * Refactor AddPolly extension * Remove single quote because semicolon ends sentence --------- Co-authored-by: Ray <[email protected]> Co-authored-by: Raman Maksimchuk <[email protected]>
ggnaegi · Oct 13, 2023 · 3b776a7 · 3b776a7
1 parent b4265a4
commit 3b776a7
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 68 deletions.
diff --git 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<IAsyncPolicy> _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
@@ -34,7 +34,7 @@
     <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435">
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
-    <PackageReference Include="Polly" Version="7.2.3" />
+    <PackageReference Include="Polly" Version="8.0.0" />
   </ItemGroup>
   <ItemGroup>
     <PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0" />

diff --git 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<QosDelegatingHandlerDelegate>(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
@@ -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<HttpResponseMessage> 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
@@ -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<PollyQoSProvider>();
-
-            _ = 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<PollyQoSProvider>();
+                circuitBreakerPolicy = Policy
                     .Handle<HttpRequestException>()
                     .Or<TimeoutRejectedException>()
                     .Or<TimeoutException>()
                     .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
@@ -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
@@ -88,7 +88,7 @@
 		<PackageReference Include="CacheManager.Core" Version="2.0.0-beta-1629" />
 		<PackageReference Include="CacheManager.Microsoft.Extensions.Configuration" Version="2.0.0-beta-1629" />
 		<PackageReference Include="CacheManager.Microsoft.Extensions.Logging" Version="2.0.0-beta-1629" />
-		<PackageReference Include="Polly" Version="7.2.3" />
+		<PackageReference Include="Polly" Version="8.0.0" />
 		<PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0" />
 	</ItemGroup>
 </Project>
diff --git 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<IOcelotLoggerFactory>();
             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);
         }
     }
 }