From b2165d7ef5b23442c7fa8cf7ca5ff6e7a10174df Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Sun, 24 Nov 2019 14:34:58 -0800 Subject: [PATCH 1/4] Allow duplicate class to AddHttpClient Fixes: #2077 In 3.0 we introduced validation to try and prevent some cases of invalid usage on the factory that had lead to user bug reports. Unfortunately we blocked a few legitimate usage scenarios behind expections. In this case the common usage is for a library to register a typed client with `AddHttpClient(...)`. User code can then collaborate by calling the same thing, and interacting with the builder that's returned. This change explicitly allows this pattern by fine-tuning the validation. --- .../HttpClientBuilderExtensions.cs | 12 ++++- ...tFactoryServiceCollectionExtensionsTest.cs | 49 ++++++++++++++++++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs index 10a1fe79e43..3f1fa5bf227 100644 --- a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs +++ b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs @@ -532,7 +532,12 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string Debug.Assert(registry != null); // Check for same type registered twice. This can't work because typed clients have to be unique for DI to function. - if (registry.TypedClientRegistrations.TryGetValue(type, out var otherName)) + if (registry.TypedClientRegistrations.TryGetValue(type, out var otherName) && + + // Allow duplicate registrations with the same name. This is usually someone calling "AddHttpClient" once + // as part of a library, and the consumer of that library doing the same thing to add further configuration. + // See: https://github.com/aspnet/Extensions/issues/2077 + !string.Equals(name, otherName, StringComparison.Ordinal)) { var message = $"The HttpClient factory already has a registered client with the type '{type.FullName}'. " + @@ -542,7 +547,10 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string } // Check for same name registered to two types. This won't work because we rely on named options for the configuration. - if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType)) + if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType) && + + // Allow registering the same name twice to the same type (see above). + type != otherType) { var message = $"The HttpClient factory already has a registered client with the name '{name}', bound to the type '{otherType.FullName}'. " + diff --git a/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs b/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs index d4e9c1fa1da..7875f4c5944 100644 --- a/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs +++ b/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs @@ -347,7 +347,52 @@ public void AddHttpClient_AddTypedClient_WithDelegate_ConfiguresNamedClient() } [Fact] - public void AddHttpClient_AddSameTypedClientTwice_ThrowsError() + public void AddHttpClient_AddSameTypedClientTwice_WithSameName_Works() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddHttpClient(); + + // Act + serviceCollection.AddHttpClient(c => + { + c.BaseAddress = new Uri("http://example.com"); + }); + + var services = serviceCollection.BuildServiceProvider(); + + // Act2 + var client = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); + } + + [Fact] + public void AddHttpClient_AddSameTypedClientTwice_WithSameName_WithAddTypedClient_Works() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddHttpClient(); + + // Act + serviceCollection.AddHttpClient(nameof(TestTypedClient), c => + { + c.BaseAddress = new Uri("http://example.com"); + }) + .AddTypedClient(); + + var services = serviceCollection.BuildServiceProvider(); + + // Act2 + var client = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); + } + + [Fact] + public void AddHttpClient_AddSameTypedClientTwice_WithDifferentNames_ThrowsError() { // Arrange var serviceCollection = new ServiceCollection(); @@ -365,7 +410,7 @@ public void AddHttpClient_AddSameTypedClientTwice_ThrowsError() } [Fact] - public void AddHttpClient_AddSameTypedClientTwice_WithAddTypedClient_ThrowsError() + public void AddHttpClient_AddSameTypedClientTwice_WithDifferentNames_WithAddTypedClient_ThrowsError() { // Arrange var serviceCollection = new ServiceCollection(); From f198c4653a310e4d9957cc37a22a649ef020e69e Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Sun, 24 Nov 2019 15:05:46 -0800 Subject: [PATCH 2/4] Allow multiple types to be registered for one name Fixes: aspnet/AspNetCore#13346 In 3.0 we added validation to try and report exceptions for some common HttClient factory usage mistakes in the registration code. Unfortunately we blocked from legitimate usage cases. In this case, users are blocked from associating multiple types with the same logical 'name'. Ex: ```C# services.AddHttpClient("Foo").AddTypedClient().AddTypedClient(); ``` This is useful and should be allowed because it's a good way to DRY up configuration code. ---- This change relaxes the validation when `AddTypedClient` is called, but not when `AddHttpClient` is called without supplying a name. We still want to block cases like the following: ```C# services.AddHttpClient(); services.AddHttpClient(); ``` The type short name is used as the logical name for the client (named options) so usage like this is always a bug. --- .../HttpClientBuilderExtensions.cs | 48 +++++++- ...lientFactoryServiceCollectionExtensions.cs | 24 ++-- ...tFactoryServiceCollectionExtensionsTest.cs | 106 +++++++++++++++--- 3 files changed, 143 insertions(+), 35 deletions(-) diff --git a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs index 3f1fa5bf227..268c1693cd0 100644 --- a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs +++ b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs @@ -323,7 +323,13 @@ public static IHttpClientBuilder AddTypedClient(this IHttpClientBuilder throw new ArgumentNullException(nameof(builder)); } - ReserveClient(builder, typeof(TClient), builder.Name); + return AddTypedClientCore(builder, validateSingleType: false); + } + + internal static IHttpClientBuilder AddTypedClientCore(this IHttpClientBuilder builder, bool validateSingleType) + where TClient : class + { + ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType); builder.Services.AddTransient(s => { @@ -377,7 +383,14 @@ public static IHttpClientBuilder AddTypedClient(this I throw new ArgumentNullException(nameof(builder)); } - ReserveClient(builder, typeof(TClient), builder.Name); + return AddTypedClientCore(builder, validateSingleType: false); + } + + internal static IHttpClientBuilder AddTypedClientCore(this IHttpClientBuilder builder, bool validateSingleType) + where TClient : class + where TImplementation : class, TClient + { + ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType); builder.Services.AddTransient(s => { @@ -425,7 +438,13 @@ public static IHttpClientBuilder AddTypedClient(this IHttpClientBuilder throw new ArgumentNullException(nameof(factory)); } - ReserveClient(builder, typeof(TClient), builder.Name); + return AddTypedClientCore(builder, factory, validateSingleType: false); + } + + internal static IHttpClientBuilder AddTypedClientCore(this IHttpClientBuilder builder, Func factory, bool validateSingleType) + where TClient : class + { + ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType); builder.Services.AddTransient(s => { @@ -472,7 +491,23 @@ public static IHttpClientBuilder AddTypedClient(this IHttpClientBuilder throw new ArgumentNullException(nameof(factory)); } - ReserveClient(builder, typeof(TClient), builder.Name); + return AddTypedClientCore(builder, factory, validateSingleType: false); + } + + internal static IHttpClientBuilder AddTypedClientCore(this IHttpClientBuilder builder, Func factory, bool validateSingleType) + where TClient : class + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + if (factory == null) + { + throw new ArgumentNullException(nameof(factory)); + } + + ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType); builder.Services.AddTransient(s => { @@ -526,7 +561,7 @@ public static IHttpClientBuilder SetHandlerLifetime(this IHttpClientBuilder buil } // See comments on HttpClientMappingRegistry. - private static void ReserveClient(IHttpClientBuilder builder, Type type, string name) + private static void ReserveClient(IHttpClientBuilder builder, Type type, string name, bool validateSingleType) { var registry = (HttpClientMappingRegistry)builder.Services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance; Debug.Assert(registry != null); @@ -549,6 +584,9 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string // Check for same name registered to two types. This won't work because we rely on named options for the configuration. if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType) && + // Allow using the same name with multiple types in some cases (see callers). + validateSingleType && + // Allow registering the same name twice to the same type (see above). type != otherType) { diff --git a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs index 43105a78703..45ebf011b33 100644 --- a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs +++ b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs @@ -204,7 +204,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false); var builder = new DefaultHttpClientBuilder(services, name); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: true); return builder; } @@ -247,7 +247,7 @@ public static IHttpClientBuilder AddHttpClient(this IS var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false); var builder = new DefaultHttpClientBuilder(services, name); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: true); return builder; } @@ -292,7 +292,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection AddHttpClient(services); var builder = new DefaultHttpClientBuilder(services, name); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: false); // Name was explicitly provided. return builder; } @@ -343,7 +343,7 @@ public static IHttpClientBuilder AddHttpClient(this IS AddHttpClient(services); var builder = new DefaultHttpClientBuilder(services, name); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: false); // name was explicitly provided return builder; } @@ -388,7 +388,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false); var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: true); return builder; } @@ -433,7 +433,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false); var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: true); return builder; } @@ -483,7 +483,7 @@ public static IHttpClientBuilder AddHttpClient(this IS var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false); var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: true); return builder; } @@ -533,7 +533,7 @@ public static IHttpClientBuilder AddHttpClient(this IS var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false); var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: true); return builder; } @@ -585,7 +585,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: false); // name was explicitly provided return builder; } @@ -637,7 +637,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: false); // name was explictly provided return builder; } @@ -694,7 +694,7 @@ public static IHttpClientBuilder AddHttpClient(this IS var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: false); // name was explicitly provided return builder; } @@ -751,7 +751,7 @@ public static IHttpClientBuilder AddHttpClient(this IS var builder = new DefaultHttpClientBuilder(services, name); builder.ConfigureHttpClient(configureClient); - builder.AddTypedClient(); + builder.AddTypedClientCore(validateSingleType: false); // name was explicitly provided return builder; } } diff --git a/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs b/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs index 7875f4c5944..a5c07b9b678 100644 --- a/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs +++ b/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs @@ -28,7 +28,7 @@ public void AddHttpClient_IsSelfContained_CanCreateClient() var serviceCollection = new ServiceCollection(); // Act1 - serviceCollection.AddHttpClient(); + serviceCollection.AddHttpClient(); var services = serviceCollection.BuildServiceProvider(); var options = services.GetRequiredService>(); @@ -446,21 +446,88 @@ public void AddHttpClient_AddSameNameWithTypedClientTwice_ThrowsError() } [Fact] - public void AddHttpClient_AddSameNameWithTypedClientTwice_WithAddTypedClient_ThrowsError() + public void AddHttpClient_AddSameNameWithTypedClientTwice_WithAddTypedClient_IsAllowed() { // Arrange var serviceCollection = new ServiceCollection(); - serviceCollection.AddHttpClient(); + serviceCollection.AddHttpClient(c => + { + c.BaseAddress = new Uri("http://example.com"); + }); // Act - var ex = Assert.Throws(() => serviceCollection.AddHttpClient("TestTypedClient").AddTypedClient()); + serviceCollection.AddHttpClient("TestTypedClient").AddTypedClient(); + + var services = serviceCollection.BuildServiceProvider(); + + // Act + var client = services.GetRequiredService(); // Assert - Assert.Equal( - "The HttpClient factory already has a registered client with the name 'TestTypedClient', bound to the type 'Microsoft.Extensions.Http.TestTypedClient'. " + - "Client names are computed based on the type name without considering the namespace ('TestTypedClient'). " + - "Use an overload of AddHttpClient that accepts a string and provide a unique name to resolve the conflict.", - ex.Message); + Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); + + // Act + var client2 = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client2.HttpClient.BaseAddress.AbsoluteUri); + } + + [Fact] + public void AddHttpClient_AddSameNameWithTypedClientTwice_WithExplicitName_IsAllowed() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddHttpClient(c => + { + c.BaseAddress = new Uri("http://example.com"); + }); + + // Act + serviceCollection.AddHttpClient("TestTypedClient"); + + var services = serviceCollection.BuildServiceProvider(); + + // Act + var client = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); + + // Act + var client2 = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client2.HttpClient.BaseAddress.AbsoluteUri); + } + + [Fact] + public void AddHttpClient_RegisteringMultipleTypes_WithAddTypedClient_IsAllowed() + { + // Arrange + var serviceCollection = new ServiceCollection(); + + // Act + serviceCollection.AddHttpClient("Test", c => + { + c.BaseAddress = new Uri("http://example.com"); + }) + .AddTypedClient() + .AddTypedClient(); + + var services = serviceCollection.BuildServiceProvider(); + + // Act + var client = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); + + // Act + var client2 = services.GetRequiredService(); + + // Assert + Assert.Equal("http://example.com/", client2.HttpClient.BaseAddress.AbsoluteUri); } [Fact] @@ -475,7 +542,7 @@ public void AddHttpClient_AddTypedClient_WithServiceDelegate_ConfiguresNamedClie }); // Act - serviceCollection.AddHttpClient("test").AddTypedClient((c,s) => + serviceCollection.AddHttpClient("test").AddTypedClient((c, s) => { Assert.Equal("http://example.com/", c.BaseAddress.AbsoluteUri); c.BaseAddress = new Uri("http://example2.com"); @@ -564,7 +631,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresClient() }); // Act1 - serviceCollection.AddHttpClient((s,c) => + serviceCollection.AddHttpClient((s, c) => { var options = s.GetRequiredService>(); c.BaseAddress = new Uri(options.Value.BaseAddress); @@ -574,7 +641,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresClient() // Act2 var client = services.GetRequiredService(); - + // Assert Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); } @@ -591,7 +658,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co }); // Act1 - serviceCollection.AddHttpClient((s,c) => + serviceCollection.AddHttpClient((s, c) => { var options = s.GetRequiredService>(); c.BaseAddress = new Uri(options.Value.BaseAddress); @@ -601,7 +668,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co // Act2 var client = services.GetRequiredService(); - + // Assert Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); } @@ -618,7 +685,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresNamedClie }); // Act1 - serviceCollection.AddHttpClient("test", (s,c) => + serviceCollection.AddHttpClient("test", (s, c) => { var options = s.GetRequiredService>(); c.BaseAddress = new Uri(options.Value.BaseAddress); @@ -628,7 +695,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresNamedClie // Act2 var client = services.GetRequiredService(); - + // Assert Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); } @@ -645,7 +712,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co }); // Act1 - serviceCollection.AddHttpClient("test", (s,c) => + serviceCollection.AddHttpClient("test", (s, c) => { var options = s.GetRequiredService>(); c.BaseAddress = new Uri(options.Value.BaseAddress); @@ -655,7 +722,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co // Act2 var client = services.GetRequiredService(); - + // Assert Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri); } @@ -1171,6 +1238,9 @@ public class TestTypedClient { public TestTypedClient(HttpClient httpClient) { + HttpClient = httpClient; } + + public HttpClient HttpClient { get; } } } From 9c9ac5965adac3bef161317b239cc1c82f3c1f37 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 16 Jan 2020 08:16:36 -0800 Subject: [PATCH 3/4] WIP --- .../HttpClientBuilderExtensions.cs | 23 ++++--------------- .../HttpClientMappingRegistry.cs | 2 -- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs index 268c1693cd0..d4e97d92fed 100644 --- a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs +++ b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs @@ -566,28 +566,13 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string var registry = (HttpClientMappingRegistry)builder.Services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance; Debug.Assert(registry != null); - // Check for same type registered twice. This can't work because typed clients have to be unique for DI to function. - if (registry.TypedClientRegistrations.TryGetValue(type, out var otherName) && - - // Allow duplicate registrations with the same name. This is usually someone calling "AddHttpClient" once - // as part of a library, and the consumer of that library doing the same thing to add further configuration. - // See: https://github.com/aspnet/Extensions/issues/2077 - !string.Equals(name, otherName, StringComparison.Ordinal)) - { - var message = - $"The HttpClient factory already has a registered client with the type '{type.FullName}'. " + - $"Client types must be unique. " + - $"Consider using inheritance to create multiple unique types with the same API surface."; - throw new InvalidOperationException(message); - } - // Check for same name registered to two types. This won't work because we rely on named options for the configuration. if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType) && // Allow using the same name with multiple types in some cases (see callers). validateSingleType && - // Allow registering the same name twice to the same type (see above). + // Allow registering the same name twice to the same type. type != otherType) { var message = @@ -597,8 +582,10 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string throw new InvalidOperationException(message); } - registry.TypedClientRegistrations[type] = name; - registry.NamedClientRegistrations[name] = type; + if (validateSingleType) + { + registry.NamedClientRegistrations[name] = type; + } } } } diff --git a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientMappingRegistry.cs b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientMappingRegistry.cs index c3fcf462a79..ce809af768f 100644 --- a/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientMappingRegistry.cs +++ b/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientMappingRegistry.cs @@ -13,8 +13,6 @@ namespace Microsoft.Extensions.DependencyInjection // See: https://github.com/aspnet/Extensions/issues/960 internal class HttpClientMappingRegistry { - public Dictionary TypedClientRegistrations { get; } = new Dictionary(); - public Dictionary NamedClientRegistrations { get; } = new Dictionary(); } } From edcf5f2859b8a94c82df5c03930f9854828e9dac Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 16 Jan 2020 08:36:56 -0800 Subject: [PATCH 4/4] All duplicates of the same type with different names Also reported as part of #2077 In this pattern users register many instances of the same client with different configurations, and multiplex between them in a round-robin fashion. --- ...tFactoryServiceCollectionExtensionsTest.cs | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs b/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs index a5c07b9b678..02849a87b3b 100644 --- a/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs +++ b/src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq; using System.Net; using System.Net.Http; using System.Threading; @@ -392,39 +393,37 @@ public void AddHttpClient_AddSameTypedClientTwice_WithSameName_WithAddTypedClien } [Fact] - public void AddHttpClient_AddSameTypedClientTwice_WithDifferentNames_ThrowsError() + public void AddHttpClient_AddSameTypedClientTwice_WithDifferentNames_IsAllowed() { // Arrange var serviceCollection = new ServiceCollection(); - serviceCollection.AddHttpClient(); + serviceCollection.AddHttpClient("Test1"); + serviceCollection.AddHttpClient("Test2"); + + var services = serviceCollection.BuildServiceProvider(); // Act - var ex = Assert.Throws(() => serviceCollection.AddHttpClient("Test")); + var clients = services.GetRequiredService>(); // Assert - Assert.Equal( - "The HttpClient factory already has a registered client with the type 'Microsoft.Extensions.Http.TestTypedClient'. " + - "Client types must be unique. " + - "Consider using inheritance to create multiple unique types with the same API surface.", - ex.Message); + Assert.Equal(2, clients.Count()); } [Fact] - public void AddHttpClient_AddSameTypedClientTwice_WithDifferentNames_WithAddTypedClient_ThrowsError() + public void AddHttpClient_AddSameTypedClientTwice_WithDifferentNames_WithAddTypedClient_IsAllowed() { // Arrange var serviceCollection = new ServiceCollection(); serviceCollection.AddHttpClient(); + serviceCollection.AddHttpClient("Test").AddTypedClient(); + + var services = serviceCollection.BuildServiceProvider(); // Act - var ex = Assert.Throws(() => serviceCollection.AddHttpClient("Test").AddTypedClient()); + var clients = services.GetRequiredService>(); // Assert - Assert.Equal( - "The HttpClient factory already has a registered client with the type 'Microsoft.Extensions.Http.TestTypedClient'. " + - "Client types must be unique. " + - "Consider using inheritance to create multiple unique types with the same API surface.", - ex.Message); + Assert.Equal(2, clients.Count()); } [Fact]