Skip to content

Commit 70b8e10

Browse files
committed
Allow multiple types to be registered for one name
Fixes: aspnet/AspNetCoredotnet/extensions#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<A>().AddTypedClient<B>(); ``` 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<A.SomeClient>(); services.AddHttpClient<B.SomeClient>(); ``` The type short name is used as the logical name for the client (named options) so usage like this is always a bug. \n\nCommit migrated from dotnet/extensions@f198c46
1 parent 7e3624e commit 70b8e10

File tree

3 files changed

+143
-35
lines changed

3 files changed

+143
-35
lines changed

src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,13 @@ public static IHttpClientBuilder AddTypedClient<TClient>(this IHttpClientBuilder
323323
throw new ArgumentNullException(nameof(builder));
324324
}
325325

326-
ReserveClient(builder, typeof(TClient), builder.Name);
326+
return AddTypedClientCore<TClient>(builder, validateSingleType: false);
327+
}
328+
329+
internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientBuilder builder, bool validateSingleType)
330+
where TClient : class
331+
{
332+
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);
327333

328334
builder.Services.AddTransient<TClient>(s =>
329335
{
@@ -377,7 +383,14 @@ public static IHttpClientBuilder AddTypedClient<TClient, TImplementation>(this I
377383
throw new ArgumentNullException(nameof(builder));
378384
}
379385

380-
ReserveClient(builder, typeof(TClient), builder.Name);
386+
return AddTypedClientCore<TClient, TImplementation>(builder, validateSingleType: false);
387+
}
388+
389+
internal static IHttpClientBuilder AddTypedClientCore<TClient, TImplementation>(this IHttpClientBuilder builder, bool validateSingleType)
390+
where TClient : class
391+
where TImplementation : class, TClient
392+
{
393+
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);
381394

382395
builder.Services.AddTransient<TClient>(s =>
383396
{
@@ -425,7 +438,13 @@ public static IHttpClientBuilder AddTypedClient<TClient>(this IHttpClientBuilder
425438
throw new ArgumentNullException(nameof(factory));
426439
}
427440

428-
ReserveClient(builder, typeof(TClient), builder.Name);
441+
return AddTypedClientCore<TClient>(builder, factory, validateSingleType: false);
442+
}
443+
444+
internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientBuilder builder, Func<HttpClient, TClient> factory, bool validateSingleType)
445+
where TClient : class
446+
{
447+
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);
429448

430449
builder.Services.AddTransient<TClient>(s =>
431450
{
@@ -472,7 +491,23 @@ public static IHttpClientBuilder AddTypedClient<TClient>(this IHttpClientBuilder
472491
throw new ArgumentNullException(nameof(factory));
473492
}
474493

475-
ReserveClient(builder, typeof(TClient), builder.Name);
494+
return AddTypedClientCore<TClient>(builder, factory, validateSingleType: false);
495+
}
496+
497+
internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientBuilder builder, Func<HttpClient, IServiceProvider, TClient> factory, bool validateSingleType)
498+
where TClient : class
499+
{
500+
if (builder == null)
501+
{
502+
throw new ArgumentNullException(nameof(builder));
503+
}
504+
505+
if (factory == null)
506+
{
507+
throw new ArgumentNullException(nameof(factory));
508+
}
509+
510+
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);
476511

477512
builder.Services.AddTransient<TClient>(s =>
478513
{
@@ -526,7 +561,7 @@ public static IHttpClientBuilder SetHandlerLifetime(this IHttpClientBuilder buil
526561
}
527562

528563
// See comments on HttpClientMappingRegistry.
529-
private static void ReserveClient(IHttpClientBuilder builder, Type type, string name)
564+
private static void ReserveClient(IHttpClientBuilder builder, Type type, string name, bool validateSingleType)
530565
{
531566
var registry = (HttpClientMappingRegistry)builder.Services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance;
532567
Debug.Assert(registry != null);
@@ -549,6 +584,9 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string
549584
// Check for same name registered to two types. This won't work because we rely on named options for the configuration.
550585
if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType) &&
551586

587+
// Allow using the same name with multiple types in some cases (see callers).
588+
validateSingleType &&
589+
552590
// Allow registering the same name twice to the same type (see above).
553591
type != otherType)
554592
{

src/HttpClientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
204204

205205
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
206206
var builder = new DefaultHttpClientBuilder(services, name);
207-
builder.AddTypedClient<TClient>();
207+
builder.AddTypedClientCore<TClient>(validateSingleType: true);
208208
return builder;
209209
}
210210

@@ -247,7 +247,7 @@ public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IS
247247

248248
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
249249
var builder = new DefaultHttpClientBuilder(services, name);
250-
builder.AddTypedClient<TClient, TImplementation>();
250+
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: true);
251251
return builder;
252252
}
253253

@@ -292,7 +292,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
292292
AddHttpClient(services);
293293

294294
var builder = new DefaultHttpClientBuilder(services, name);
295-
builder.AddTypedClient<TClient>();
295+
builder.AddTypedClientCore<TClient>(validateSingleType: false); // Name was explicitly provided.
296296
return builder;
297297
}
298298

@@ -343,7 +343,7 @@ public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IS
343343
AddHttpClient(services);
344344

345345
var builder = new DefaultHttpClientBuilder(services, name);
346-
builder.AddTypedClient<TClient, TImplementation>();
346+
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: false); // name was explicitly provided
347347
return builder;
348348
}
349349

@@ -388,7 +388,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
388388
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
389389
var builder = new DefaultHttpClientBuilder(services, name);
390390
builder.ConfigureHttpClient(configureClient);
391-
builder.AddTypedClient<TClient>();
391+
builder.AddTypedClientCore<TClient>(validateSingleType: true);
392392
return builder;
393393
}
394394

@@ -433,7 +433,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
433433
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
434434
var builder = new DefaultHttpClientBuilder(services, name);
435435
builder.ConfigureHttpClient(configureClient);
436-
builder.AddTypedClient<TClient>();
436+
builder.AddTypedClientCore<TClient>(validateSingleType: true);
437437
return builder;
438438
}
439439

@@ -483,7 +483,7 @@ public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IS
483483
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
484484
var builder = new DefaultHttpClientBuilder(services, name);
485485
builder.ConfigureHttpClient(configureClient);
486-
builder.AddTypedClient<TClient, TImplementation>();
486+
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: true);
487487
return builder;
488488
}
489489

@@ -533,7 +533,7 @@ public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IS
533533
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
534534
var builder = new DefaultHttpClientBuilder(services, name);
535535
builder.ConfigureHttpClient(configureClient);
536-
builder.AddTypedClient<TClient, TImplementation>();
536+
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: true);
537537
return builder;
538538
}
539539

@@ -585,7 +585,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
585585

586586
var builder = new DefaultHttpClientBuilder(services, name);
587587
builder.ConfigureHttpClient(configureClient);
588-
builder.AddTypedClient<TClient>();
588+
builder.AddTypedClientCore<TClient>(validateSingleType: false); // name was explicitly provided
589589
return builder;
590590
}
591591

@@ -637,7 +637,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
637637

638638
var builder = new DefaultHttpClientBuilder(services, name);
639639
builder.ConfigureHttpClient(configureClient);
640-
builder.AddTypedClient<TClient>();
640+
builder.AddTypedClientCore<TClient>(validateSingleType: false); // name was explictly provided
641641
return builder;
642642
}
643643

@@ -694,7 +694,7 @@ public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IS
694694

695695
var builder = new DefaultHttpClientBuilder(services, name);
696696
builder.ConfigureHttpClient(configureClient);
697-
builder.AddTypedClient<TClient, TImplementation>();
697+
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: false); // name was explicitly provided
698698
return builder;
699699
}
700700

@@ -751,7 +751,7 @@ public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IS
751751

752752
var builder = new DefaultHttpClientBuilder(services, name);
753753
builder.ConfigureHttpClient(configureClient);
754-
builder.AddTypedClient<TClient, TImplementation>();
754+
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: false); // name was explicitly provided
755755
return builder;
756756
}
757757
}

src/HttpClientFactory/Http/test/DependencyInjection/HttpClientFactoryServiceCollectionExtensionsTest.cs

Lines changed: 88 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public void AddHttpClient_IsSelfContained_CanCreateClient()
2828
var serviceCollection = new ServiceCollection();
2929

3030
// Act1
31-
serviceCollection.AddHttpClient();
31+
serviceCollection.AddHttpClient();
3232

3333
var services = serviceCollection.BuildServiceProvider();
3434
var options = services.GetRequiredService<IOptionsMonitor<HttpClientFactoryOptions>>();
@@ -446,21 +446,88 @@ public void AddHttpClient_AddSameNameWithTypedClientTwice_ThrowsError()
446446
}
447447

448448
[Fact]
449-
public void AddHttpClient_AddSameNameWithTypedClientTwice_WithAddTypedClient_ThrowsError()
449+
public void AddHttpClient_AddSameNameWithTypedClientTwice_WithAddTypedClient_IsAllowed()
450450
{
451451
// Arrange
452452
var serviceCollection = new ServiceCollection();
453-
serviceCollection.AddHttpClient<TestTypedClient>();
453+
serviceCollection.AddHttpClient<TestTypedClient>(c =>
454+
{
455+
c.BaseAddress = new Uri("http://example.com");
456+
});
454457

455458
// Act
456-
var ex = Assert.Throws<InvalidOperationException>(() => serviceCollection.AddHttpClient("TestTypedClient").AddTypedClient<AnotherNamespace.TestTypedClient>());
459+
serviceCollection.AddHttpClient("TestTypedClient").AddTypedClient<AnotherNamespace.TestTypedClient>();
460+
461+
var services = serviceCollection.BuildServiceProvider();
462+
463+
// Act
464+
var client = services.GetRequiredService<TestTypedClient>();
457465

458466
// Assert
459-
Assert.Equal(
460-
"The HttpClient factory already has a registered client with the name 'TestTypedClient', bound to the type 'Microsoft.Extensions.Http.TestTypedClient'. " +
461-
"Client names are computed based on the type name without considering the namespace ('TestTypedClient'). " +
462-
"Use an overload of AddHttpClient that accepts a string and provide a unique name to resolve the conflict.",
463-
ex.Message);
467+
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
468+
469+
// Act
470+
var client2 = services.GetRequiredService<AnotherNamespace.TestTypedClient>();
471+
472+
// Assert
473+
Assert.Equal("http://example.com/", client2.HttpClient.BaseAddress.AbsoluteUri);
474+
}
475+
476+
[Fact]
477+
public void AddHttpClient_AddSameNameWithTypedClientTwice_WithExplicitName_IsAllowed()
478+
{
479+
// Arrange
480+
var serviceCollection = new ServiceCollection();
481+
serviceCollection.AddHttpClient<TestTypedClient>(c =>
482+
{
483+
c.BaseAddress = new Uri("http://example.com");
484+
});
485+
486+
// Act
487+
serviceCollection.AddHttpClient<AnotherNamespace.TestTypedClient>("TestTypedClient");
488+
489+
var services = serviceCollection.BuildServiceProvider();
490+
491+
// Act
492+
var client = services.GetRequiredService<TestTypedClient>();
493+
494+
// Assert
495+
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
496+
497+
// Act
498+
var client2 = services.GetRequiredService<AnotherNamespace.TestTypedClient>();
499+
500+
// Assert
501+
Assert.Equal("http://example.com/", client2.HttpClient.BaseAddress.AbsoluteUri);
502+
}
503+
504+
[Fact]
505+
public void AddHttpClient_RegisteringMultipleTypes_WithAddTypedClient_IsAllowed()
506+
{
507+
// Arrange
508+
var serviceCollection = new ServiceCollection();
509+
510+
// Act
511+
serviceCollection.AddHttpClient("Test", c =>
512+
{
513+
c.BaseAddress = new Uri("http://example.com");
514+
})
515+
.AddTypedClient<TestTypedClient>()
516+
.AddTypedClient<AnotherNamespace.TestTypedClient>();
517+
518+
var services = serviceCollection.BuildServiceProvider();
519+
520+
// Act
521+
var client = services.GetRequiredService<TestTypedClient>();
522+
523+
// Assert
524+
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
525+
526+
// Act
527+
var client2 = services.GetRequiredService<AnotherNamespace.TestTypedClient>();
528+
529+
// Assert
530+
Assert.Equal("http://example.com/", client2.HttpClient.BaseAddress.AbsoluteUri);
464531
}
465532

466533
[Fact]
@@ -475,7 +542,7 @@ public void AddHttpClient_AddTypedClient_WithServiceDelegate_ConfiguresNamedClie
475542
});
476543

477544
// Act
478-
serviceCollection.AddHttpClient("test").AddTypedClient<TestTypedClient>((c,s) =>
545+
serviceCollection.AddHttpClient("test").AddTypedClient<TestTypedClient>((c, s) =>
479546
{
480547
Assert.Equal("http://example.com/", c.BaseAddress.AbsoluteUri);
481548
c.BaseAddress = new Uri("http://example2.com");
@@ -564,7 +631,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresClient()
564631
});
565632

566633
// Act1
567-
serviceCollection.AddHttpClient<TestTypedClient>((s,c) =>
634+
serviceCollection.AddHttpClient<TestTypedClient>((s, c) =>
568635
{
569636
var options = s.GetRequiredService<IOptions<OtherTestOptions>>();
570637
c.BaseAddress = new Uri(options.Value.BaseAddress);
@@ -574,7 +641,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresClient()
574641

575642
// Act2
576643
var client = services.GetRequiredService<TestTypedClient>();
577-
644+
578645
// Assert
579646
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
580647
}
@@ -591,7 +658,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co
591658
});
592659

593660
// Act1
594-
serviceCollection.AddHttpClient<ITestTypedClient, TestTypedClient>((s,c) =>
661+
serviceCollection.AddHttpClient<ITestTypedClient, TestTypedClient>((s, c) =>
595662
{
596663
var options = s.GetRequiredService<IOptions<OtherTestOptions>>();
597664
c.BaseAddress = new Uri(options.Value.BaseAddress);
@@ -601,7 +668,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co
601668

602669
// Act2
603670
var client = services.GetRequiredService<ITestTypedClient>();
604-
671+
605672
// Assert
606673
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
607674
}
@@ -618,7 +685,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresNamedClie
618685
});
619686

620687
// Act1
621-
serviceCollection.AddHttpClient<TestTypedClient>("test", (s,c) =>
688+
serviceCollection.AddHttpClient<TestTypedClient>("test", (s, c) =>
622689
{
623690
var options = s.GetRequiredService<IOptions<OtherTestOptions>>();
624691
c.BaseAddress = new Uri(options.Value.BaseAddress);
@@ -628,7 +695,7 @@ public void AddHttpClient_WithTypedClient_AndServiceDelegate_ConfiguresNamedClie
628695

629696
// Act2
630697
var client = services.GetRequiredService<TestTypedClient>();
631-
698+
632699
// Assert
633700
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
634701
}
@@ -645,7 +712,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co
645712
});
646713

647714
// Act1
648-
serviceCollection.AddHttpClient<ITestTypedClient, TestTypedClient>("test", (s,c) =>
715+
serviceCollection.AddHttpClient<ITestTypedClient, TestTypedClient>("test", (s, c) =>
649716
{
650717
var options = s.GetRequiredService<IOptions<OtherTestOptions>>();
651718
c.BaseAddress = new Uri(options.Value.BaseAddress);
@@ -655,7 +722,7 @@ public void AddHttpClient_WithTypedClientAndImplementation_AndServiceDelegate_Co
655722

656723
// Act2
657724
var client = services.GetRequiredService<ITestTypedClient>();
658-
725+
659726
// Assert
660727
Assert.Equal("http://example.com/", client.HttpClient.BaseAddress.AbsoluteUri);
661728
}
@@ -1171,6 +1238,9 @@ public class TestTypedClient
11711238
{
11721239
public TestTypedClient(HttpClient httpClient)
11731240
{
1241+
HttpClient = httpClient;
11741242
}
1243+
1244+
public HttpClient HttpClient { get; }
11751245
}
11761246
}

0 commit comments

Comments
 (0)