Allow duplicate class to AddHttpClient<T>

rynowak · rynowak · commit 09b4a9becbcd · 2020-01-16T08:05:15.000-08:00
Fixes: dotnet/extensions#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<MyClient>(...)`. 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. Commit migrated from dotnet/extensions@b2165d7
diff --git a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.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/libraries/Microsoft.Extensions.Http/tests/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs b/src/libraries/Microsoft.Extensions.Http/tests/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<TestTypedClient>();
+
+            // Act
+            serviceCollection.AddHttpClient<TestTypedClient>(c =>
+            {
+                c.BaseAddress = new Uri("http://example.com");
+            });
+
+            var services = serviceCollection.BuildServiceProvider();
+
+            // Act2
+            var client = services.GetRequiredService<TestTypedClient>();
+
+            // 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<TestTypedClient>();
+
+            // Act
+            serviceCollection.AddHttpClient(nameof(TestTypedClient), c =>
+            {
+                c.BaseAddress = new Uri("http://example.com");
+            })
+            .AddTypedClient<TestTypedClient>();
+
+            var services = serviceCollection.BuildServiceProvider();
+
+            // Act2
+            var client = services.GetRequiredService<TestTypedClient>();
+
+            // 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();
