From c412c373783aea7d17e84048cf9884437e7c5fab Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Thu, 16 Jan 2025 16:42:36 +0100 Subject: [PATCH 1/7] Add API to clear the http client resilience handlers --- ...eHttpClientBuilderExtensions.Resilience.cs | 18 +++++++++ ...ClientBuilderExtensionsTests.Resilience.cs | 40 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs index 1829aefa101..707aee59f24 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs @@ -75,6 +75,24 @@ public static IHttpResiliencePipelineBuilder AddResilienceHandler( return pipelineBuilder; } + /// + /// Removes all resilience handlers registered earlier. + /// + /// The builder instance. + /// The value of . + public static IHttpClientBuilder RemoveAllResilienceHandlers(this IHttpClientBuilder builder) + { + _ = Throw.IfNull(builder); + _ = builder.ConfigureAdditionalHttpMessageHandlers(static (handlers, _) => + { + for (int i = handlers.Count - 1; i >=0; i--) + { + handlers.RemoveAt(i); + } + }); + return builder; + } + private static Func> CreatePipelineSelector(IServiceProvider serviceProvider, string pipelineName) { var resilienceProvider = serviceProvider.GetRequiredService>(); diff --git a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs index 5ceb74f2217..4d3ca1580ce 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs @@ -286,6 +286,46 @@ public void AddResilienceHandler_AuthorityByCustomSelector_NotValidated() Assert.NotNull(factory.CreateClient("my-client")); } + [Fact] + public void RemoveAllResilienceHandlers_ArgumentValidation() + { + var services = new ServiceCollection(); + IHttpClientBuilder? builder = null; + Assert.Throws(() => builder!.AddResilienceHandler("pipeline-name", _ => { })); + Assert.Throws(() => builder!.AddResilienceHandler("pipeline-name", (_, _) => { })); + } + + [Fact] + public void RemoveAllResilienceHandlers_EnsureServicesRemoved() + { + var services = new ServiceCollection().AddLogging().AddMetrics().AddRedaction(); + + IHttpClientBuilder? customBuilder = services.AddHttpClient("custom"); + + customBuilder.AddResilienceHandler("pipeline-name", _ => {}); + customBuilder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + { + Assert.Single(handlers); + }); + customBuilder.RemoveAllResilienceHandlers(); + + customBuilder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + { + Assert.Empty(handlers); + }); + + customBuilder.RemoveAllResilienceHandlers().AddStandardResilienceHandler(); + + customBuilder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + { + Assert.Single(handlers); + }); + + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + services.BuildServiceProvider().GetRequiredService().CreateClient("custom"); + } + + private void ConfigureBuilder(ResiliencePipelineBuilder builder) => builder.AddTimeout(TimeSpan.FromSeconds(1)); private class TestMetricsEnricher : MeteringEnricher From c4f1e302e539a16aedb14288acfab8a911a0d159 Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Fri, 17 Jan 2025 14:20:13 +0100 Subject: [PATCH 2/7] Additional tests --- ...ClientBuilderExtensionsTests.Resilience.cs | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs index 4d3ca1580ce..809d33a319f 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs @@ -296,27 +296,38 @@ public void RemoveAllResilienceHandlers_ArgumentValidation() } [Fact] - public void RemoveAllResilienceHandlers_EnsureServicesRemoved() + public void RemoveAllResilienceHandlers_EnsureHandlersRemoved() { - var services = new ServiceCollection().AddLogging().AddMetrics().AddRedaction(); + var services = new ServiceCollection(); + + IHttpClientBuilder? builder = services.AddHttpClient("custom"); - IHttpClientBuilder? customBuilder = services.AddHttpClient("custom"); + builder.AddStandardResilienceHandler(); - customBuilder.AddResilienceHandler("pipeline-name", _ => {}); - customBuilder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + builder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => { Assert.Single(handlers); }); - customBuilder.RemoveAllResilienceHandlers(); - customBuilder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + builder.RemoveAllResilienceHandlers(); + + builder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => { Assert.Empty(handlers); }); - customBuilder.RemoveAllResilienceHandlers().AddStandardResilienceHandler(); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + services.BuildServiceProvider().GetRequiredService().CreateClient("custom"); + } + + [Fact] + public void RemoveAllResilienceHandlers_AddHandlersAfterRemoval() + { + var services = new ServiceCollection(); - customBuilder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + IHttpClientBuilder? builder = services.AddHttpClient("custom"); + builder.RemoveAllResilienceHandlers().AddStandardResilienceHandler(); + builder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => { Assert.Single(handlers); }); From 13b53b490835a0d4c1a99c2ce20790479b91f57c Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Fri, 17 Jan 2025 16:45:14 +0100 Subject: [PATCH 3/7] Add experimental attribute to the new API --- .../ResilienceHttpClientBuilderExtensions.Resilience.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs index 707aee59f24..6810868a0c6 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs @@ -2,11 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics.CodeAnalysis; using System.Net.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.ExceptionSummarization; using Microsoft.Extensions.Http.Resilience; using Microsoft.Extensions.Http.Resilience.Internal; +using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; using Polly; using Polly.Registry; @@ -80,6 +82,7 @@ public static IHttpResiliencePipelineBuilder AddResilienceHandler( /// /// The builder instance. /// The value of . + [Experimental(diagnosticId: DiagnosticIds.Experiments.Resilience, UrlFormat = DiagnosticIds.UrlFormat)] public static IHttpClientBuilder RemoveAllResilienceHandlers(this IHttpClientBuilder builder) { _ = Throw.IfNull(builder); From fa6bb12131f4a6bda3a82d41fac2670fda6c0030 Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Mon, 20 Jan 2025 11:46:55 +0100 Subject: [PATCH 4/7] Fix formatting --- .../ResilienceHttpClientBuilderExtensions.Resilience.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs index 6810868a0c6..96a30cb913e 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs @@ -88,7 +88,7 @@ public static IHttpClientBuilder RemoveAllResilienceHandlers(this IHttpClientBui _ = Throw.IfNull(builder); _ = builder.ConfigureAdditionalHttpMessageHandlers(static (handlers, _) => { - for (int i = handlers.Count - 1; i >=0; i--) + for (int i = handlers.Count - 1; i >= 0; i--) { handlers.RemoveAt(i); } From eec12a34b8ad7712558797ee26beba3c1547acef Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Mon, 20 Jan 2025 12:08:01 +0100 Subject: [PATCH 5/7] Add the check for the handler type, formatting --- .../ResilienceHttpClientBuilderExtensions.Resilience.cs | 6 +++++- .../HttpClientBuilderExtensionsTests.Resilience.cs | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs index 96a30cb913e..8443f92f89a 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Net.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.ExceptionSummarization; @@ -90,7 +91,10 @@ public static IHttpClientBuilder RemoveAllResilienceHandlers(this IHttpClientBui { for (int i = handlers.Count - 1; i >= 0; i--) { - handlers.RemoveAt(i); + if (handlers[i] is ResilienceHandler) + { + handlers.RemoveAt(i); + } } }); return builder; diff --git a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs index 809d33a319f..033898f02eb 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs @@ -336,7 +336,6 @@ public void RemoveAllResilienceHandlers_AddHandlersAfterRemoval() services.BuildServiceProvider().GetRequiredService().CreateClient("custom"); } - private void ConfigureBuilder(ResiliencePipelineBuilder builder) => builder.AddTimeout(TimeSpan.FromSeconds(1)); private class TestMetricsEnricher : MeteringEnricher From 0a33c7bbfd5c3d0823dbf3b4b3e1b714e6d493a6 Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Mon, 20 Jan 2025 15:11:11 +0100 Subject: [PATCH 6/7] Formatting --- .../ResilienceHttpClientBuilderExtensions.Resilience.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs index 8443f92f89a..48a565db41b 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.Resilience.cs @@ -3,7 +3,6 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Linq; using System.Net.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.ExceptionSummarization; From c2267073a071a279ae431b4dc00946b134f1bf2f Mon Sep 17 00:00:00 2001 From: Dmytro Bohdanov Date: Mon, 20 Jan 2025 17:08:54 +0100 Subject: [PATCH 7/7] Additional test to ensure that new API removes only resilience handlers --- ...ClientBuilderExtensionsTests.Resilience.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs index 033898f02eb..90803fd9efc 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Resilience.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Net; using System.Net.Http; using System.Threading.Tasks; @@ -336,6 +337,28 @@ public void RemoveAllResilienceHandlers_AddHandlersAfterRemoval() services.BuildServiceProvider().GetRequiredService().CreateClient("custom"); } + [Fact] + public void RemoveAllResilienceHandlers_EnsureOnlyResilienceHandlersRemoved() + { + var services = new ServiceCollection(); + + IHttpClientBuilder? builder = services.AddHttpClient("custom"); + + builder.AddHttpMessageHandler(() => new TestHandlerStub(HttpStatusCode.OK)); + builder.AddStandardResilienceHandler(); + + builder.RemoveAllResilienceHandlers(); + + builder.ConfigureAdditionalHttpMessageHandlers((handlers, _) => + { + Assert.Single(handlers); + Assert.Equal("TestHandlerStub", handlers.First().GetType().Name); + }); + + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + services.BuildServiceProvider().GetRequiredService().CreateClient("custom"); + } + private void ConfigureBuilder(ResiliencePipelineBuilder builder) => builder.AddTimeout(TimeSpan.FromSeconds(1)); private class TestMetricsEnricher : MeteringEnricher