Skip to content

Commit

Permalink
Merge pull request #2710 from dotnet/rynowak/http-client-factory-fixes
Browse files Browse the repository at this point in the history 
Fixes for some servicing-proposed HttpClient Factory issues
  • Loading branch information
@mkArtakMSFT
mkArtakMSFT committed Jan 16, 2020
2 parents 46214d8 + edcf5f2 commit 08a9b86
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 64 deletions.
69 changes: 51 additions & 18 deletions src/HttpClientFactory/Http/src/DependencyInjection/HttpClientBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,13 @@ public static IHttpClientBuilder AddTypedClient<TClient>(this IHttpClientBuilder
throw new ArgumentNullException(nameof(builder));
}

ReserveClient(builder, typeof(TClient), builder.Name);
return AddTypedClientCore<TClient>(builder, validateSingleType: false);
}

internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientBuilder builder, bool validateSingleType)
where TClient : class
{
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);

builder.Services.AddTransient<TClient>(s =>
{
Expand Down Expand Up @@ -377,7 +383,14 @@ public static IHttpClientBuilder AddTypedClient<TClient>(this IHttpClientBuilder
throw new ArgumentNullException(nameof(builder));
}

ReserveClient(builder, typeof(TClient), builder.Name);
return AddTypedClientCore<TClient, TImplementation>(builder, validateSingleType: false);
}

internal static IHttpClientBuilder AddTypedClientCore<TClient, TImplementation>(this IHttpClientBuilder builder, bool validateSingleType)
where TClient : class
where TImplementation : class, TClient
{
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);

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

ReserveClient(builder, typeof(TClient), builder.Name);
return AddTypedClientCore<TClient>(builder, factory, validateSingleType: false);
}

internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientBuilder builder, Func<HttpClient, TClient> factory, bool validateSingleType)
where TClient : class
{
ReserveClient(builder, typeof(TClient), builder.Name, validateSingleType);

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

ReserveClient(builder, typeof(TClient), builder.Name);
return AddTypedClientCore<TClient>(builder, factory, validateSingleType: false);
}

internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientBuilder builder, Func<HttpClient, IServiceProvider, TClient> 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<TClient>(s =>
{
Expand Down Expand Up @@ -526,23 +561,19 @@ 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);

// 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))
{
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))
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.
type != otherType)
{
var message =
$"The HttpClient factory already has a registered client with the name '{name}', bound to the type '{otherType.FullName}'. " +
Expand All @@ -551,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;
}
}
}
}
24 changes: 12 additions & 12 deletions ...lientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection

var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
var builder = new DefaultHttpClientBuilder(services, name);
builder.AddTypedClient<TClient>();
builder.AddTypedClientCore<TClient>(validateSingleType: true);
return builder;
}

Expand Down Expand Up @@ -247,7 +247,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection

var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
var builder = new DefaultHttpClientBuilder(services, name);
builder.AddTypedClient<TClient, TImplementation>();
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: true);
return builder;
}

Expand Down Expand Up @@ -292,7 +292,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
AddHttpClient(services);

var builder = new DefaultHttpClientBuilder(services, name);
builder.AddTypedClient<TClient>();
builder.AddTypedClientCore<TClient>(validateSingleType: false); // Name was explicitly provided.
return builder;
}

Expand Down Expand Up @@ -343,7 +343,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
AddHttpClient(services);

var builder = new DefaultHttpClientBuilder(services, name);
builder.AddTypedClient<TClient, TImplementation>();
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: false); // name was explicitly provided
return builder;
}

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

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

Expand Down Expand Up @@ -483,7 +483,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClient<TClient, TImplementation>();
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: true);
return builder;
}

Expand Down Expand Up @@ -533,7 +533,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection
var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClient<TClient, TImplementation>();
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: true);
return builder;
}

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

var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClient<TClient>();
builder.AddTypedClientCore<TClient>(validateSingleType: false); // name was explicitly provided
return builder;
}

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

var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClient<TClient>();
builder.AddTypedClientCore<TClient>(validateSingleType: false); // name was explictly provided
return builder;
}

Expand Down Expand Up @@ -694,7 +694,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection

var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClient<TClient, TImplementation>();
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: false); // name was explicitly provided
return builder;
}

Expand Down Expand Up @@ -751,7 +751,7 @@ public static IHttpClientBuilder AddHttpClient<TClient>(this IServiceCollection

var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClient<TClient, TImplementation>();
builder.AddTypedClientCore<TClient, TImplementation>(validateSingleType: false); // name was explicitly provided
return builder;
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/HttpClientFactory/Http/src/DependencyInjection/HttpClientMappingRegistry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ namespace Microsoft.Extensions.DependencyInjection
// See: https://github.com/aspnet/Extensions/issues/960
internal class HttpClientMappingRegistry
{
public Dictionary<Type, string> TypedClientRegistrations { get; } = new Dictionary<Type, string>();

public Dictionary<string, Type> NamedClientRegistrations { get; } = new Dictionary<string, Type>();
}
}

0 comments on commit 08a9b86

Please sign in to comment.