From e70a4ac464faea00fee933101af51aea68503a61 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 1 Dec 2020 18:20:37 +0000 Subject: [PATCH 1/3] Fix IDE warnings Fix IDE warnings about non-disposed handlers. --- .../PollyHttpClientBuilderExtensionsTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs b/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs index d8e6b3af3eeb..132897e8bc51 100644 --- a/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs +++ b/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs @@ -255,7 +255,7 @@ public async Task AddPolicyHandlerFromRegistry_Dynamic_AddsPolicyHandler() public async Task AddTransientHttpErrorPolicy_AddsPolicyHandler_HandlesStatusCode(HttpStatusCode statusCode) { // Arrange - var handler = new SequenceMessageHandler() + using var handler = new SequenceMessageHandler() { Responses = { @@ -303,7 +303,7 @@ public async Task AddTransientHttpErrorPolicy_AddsPolicyHandler_HandlesStatusCod public async Task AddTransientHttpErrorPolicy_AddsPolicyHandler_HandlesHttpRequestException() { // Arrange - var handler = new SequenceMessageHandler() + using var handler = new SequenceMessageHandler() { Responses = { From 763f72b473b4ef5feb78a777ccb607773966d9b0 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 1 Dec 2020 18:21:23 +0000 Subject: [PATCH 2/3] Update return type documentation Update the comment's type to match the method's. --- .../DependencyInjection/PollyServiceCollectionExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs index 0146d472ad17..d3f8885d95d7 100644 --- a/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -8,7 +8,7 @@ namespace Microsoft.Extensions.DependencyInjection { /// - /// Provides convenience extension methods to register and + /// Provides convenience extension methods to register and /// in the service collection. /// public static class PollyServiceCollectionExtensions @@ -19,7 +19,7 @@ public static class PollyServiceCollectionExtensions /// the newly created registry. /// /// The . - /// The newly created . + /// The newly created . public static IPolicyRegistry AddPolicyRegistry(this IServiceCollection services) { if (services == null) @@ -27,7 +27,7 @@ public static IPolicyRegistry AddPolicyRegistry(this IServiceCollection throw new ArgumentNullException(nameof(services)); } - // Create an empty registry, register and return it as an instance. This is the best way to get a + // Create an empty registry, register and return it as an instance. This is the best way to get a // single instance registered using both interfaces. var registry = new PolicyRegistry(); services.AddSingleton>(registry); From cb824bd6b268fcc3cc2a0ff81151a3a3c9ffae0a Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 1 Dec 2020 18:21:56 +0000 Subject: [PATCH 3/3] Add new AddPolicyRegistry overload Add a new overload for AddPolicyRegistry() that allows the caller to configure the policy registry using other services that are registered with the service provider, such as config. --- .../PollyServiceCollectionExtensions.cs | 37 ++++++++++++ .../Polly/src/PublicAPI.Unshipped.txt | 1 + .../PollyHttpClientBuilderExtensionsTest.cs | 60 ++++++++++++++++++- 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs index d3f8885d95d7..6fe049d11724 100644 --- a/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/HttpClientFactory/Polly/src/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -61,5 +61,42 @@ public static IPolicyRegistry AddPolicyRegistry(this IServiceCollection return registry; } + + /// + /// Registers an empty in the service collection with service types + /// , and and + /// uses the specified delegate to configure it. + /// + /// The . + /// A delegate that is used to configure an . + /// The provided . + public static IServiceCollection AddPolicyRegistry(this IServiceCollection services, Action> configureRegistry) + { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + + if (configureRegistry == null) + { + throw new ArgumentNullException(nameof(configureRegistry)); + } + + // Create an empty registry, configure it and register it as an instance. + // This is the best way to get a single instance registered using both interfaces. + services.AddSingleton(serviceProvider => + { + var registry = new PolicyRegistry(); + + configureRegistry(serviceProvider, registry); + + return registry; + }); + + services.AddSingleton>(serviceProvider => serviceProvider.GetRequiredService()); + services.AddSingleton>(serviceProvider => serviceProvider.GetRequiredService()); + + return services; + } } } diff --git a/src/HttpClientFactory/Polly/src/PublicAPI.Unshipped.txt b/src/HttpClientFactory/Polly/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..9738a0056a45 100644 --- a/src/HttpClientFactory/Polly/src/PublicAPI.Unshipped.txt +++ b/src/HttpClientFactory/Polly/src/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +~static Microsoft.Extensions.DependencyInjection.PollyServiceCollectionExtensions.AddPolicyRegistry(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action> configureRegistry) -> Microsoft.Extensions.DependencyInjection.IServiceCollection diff --git a/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs b/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs index 132897e8bc51..94d3f82bfdf2 100644 --- a/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs +++ b/src/HttpClientFactory/Polly/test/DependencyInjection/PollyHttpClientBuilderExtensionsTest.cs @@ -31,7 +31,7 @@ public PollyHttpClientBuilderExtensionsTest() // Allows the exception from our handler to propegate private IAsyncPolicy NoOpPolicy { get; } - // Matches what our client handler does + // Matches what our client handler does private IAsyncPolicy RetryPolicy { get; } [Fact] @@ -415,6 +415,59 @@ public async Task AddPolicyHandlerFromRegistry_PolicySelectorWithKey_AddsPolicyH Assert.True(registry.ContainsKey("host2")); } + [Fact] + public async Task AddPolicyHandlerFromRegistry_WithConfigureDelegate_AddsPolicyHandler() + { + var options = new PollyPolicyOptions() + { + PolicyName = "retrypolicy" + }; + + var serviceCollection = new ServiceCollection(); + + serviceCollection.AddSingleton(options); + + serviceCollection.AddPolicyRegistry((serviceProvider, registry) => + { + string policyName = serviceProvider.GetRequiredService().PolicyName; + + registry.Add>(policyName, RetryPolicy); + }); + + HttpMessageHandlerBuilder builder = null; + + // Act1 + serviceCollection.AddHttpClient("example.com", c => c.BaseAddress = new Uri("http://example.com")) + .AddPolicyHandlerFromRegistry(options.PolicyName) + .ConfigureHttpMessageHandlerBuilder(b => + { + b.PrimaryHandler = PrimaryHandler; + + builder = b; + }); + + var services = serviceCollection.BuildServiceProvider(); + var factory = services.GetRequiredService(); + + // Act2 + var client = factory.CreateClient("example.com"); + + // Assert + Assert.NotNull(client); + + Assert.Collection( + builder.AdditionalHandlers, + h => Assert.IsType(h), + h => Assert.IsType(h), + h => Assert.IsType(h)); + + // Act 3 + var response = await client.SendAsync(new HttpRequestMessage()); + + // Assert + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + } + // Throws an exception or fails on even numbered requests, otherwise succeeds. private class FaultyMessageHandler : DelegatingHandler { @@ -447,5 +500,10 @@ protected override Task SendAsync(HttpRequestMessage reques return Task.FromResult(func(request)); } } + + private class PollyPolicyOptions + { + public string PolicyName { get; set; } + } } }