Skip to content

Commit 08a9b86

Browse files
mkArtakMSFTmkArtakMSFT
authored
Merge pull request #2710 from dotnet/rynowak/http-client-factory-fixes
Fixes for some servicing-proposed HttpClient Factory issues
2 parents 46214d8 + edcf5f2 commit 08a9b86

File tree

4 files changed

+209
-64
lines changed

4 files changed

+209
-64
lines changed

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

+51-18
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,23 +561,19 @@ 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);
533568

534-
// Check for same type registered twice. This can't work because typed clients have to be unique for DI to function.
535-
if (registry.TypedClientRegistrations.TryGetValue(type, out var otherName))
536-
{
537-
var message =
538-
$"The HttpClient factory already has a registered client with the type '{type.FullName}'. " +
539-
$"Client types must be unique. " +
540-
$"Consider using inheritance to create multiple unique types with the same API surface.";
541-
throw new InvalidOperationException(message);
542-
}
543-
544569
// Check for same name registered to two types. This won't work because we rely on named options for the configuration.
545-
if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType))
570+
if (registry.NamedClientRegistrations.TryGetValue(name, out var otherType) &&
571+
572+
// Allow using the same name with multiple types in some cases (see callers).
573+
validateSingleType &&
574+
575+
// Allow registering the same name twice to the same type.
576+
type != otherType)
546577
{
547578
var message =
548579
$"The HttpClient factory already has a registered client with the name '{name}', bound to the type '{otherType.FullName}'. " +
@@ -551,8 +582,10 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string
551582
throw new InvalidOperationException(message);
552583
}
553584

554-
registry.TypedClientRegistrations[type] = name;
555-
registry.NamedClientRegistrations[name] = type;
585+
if (validateSingleType)
586+
{
587+
registry.NamedClientRegistrations[name] = type;
588+
}
556589
}
557590
}
558591
}

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

+12-12
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/src/DependencyInjection/HttpClientMappingRegistry.cs

-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ namespace Microsoft.Extensions.DependencyInjection
1313
// See: https://github.com/aspnet/Extensions/issues/960
1414
internal class HttpClientMappingRegistry
1515
{
16-
public Dictionary<Type, string> TypedClientRegistrations { get; } = new Dictionary<Type, string>();
17-
1816
public Dictionary<string, Type> NamedClientRegistrations { get; } = new Dictionary<string, Type>();
1917
}
2018
}

0 commit comments

Comments
 (0)