Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate keyed DI with HttpClientFactory #90272

Closed
wants to merge 5 commits into from
Closed

Integrate keyed DI with HttpClientFactory #90272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a4f650
Add keyed services support
CarnaViire Aug 3, 2023
3a7e53d
Separate handler builder actions
CarnaViire Aug 3, 2023
7709396
Add tests
CarnaViire Aug 8, 2023
ac5d1af
Add tests
CarnaViire Aug 9, 2023
cf9004b
Bring back default
CarnaViire Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/libraries/Microsoft.Extensions.Http/src/DefaultHttpClientFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,24 @@ internal ActiveHandlerTrackingEntry CreateHandlerEntry(string name)

void Configure(HttpMessageHandlerBuilder b)
{
for (int i = 0; i < options.HttpMessageHandlerBuilderActions.Count; i++)
if (options.MergedToHandlerBuilderActions)
{
options.HttpMessageHandlerBuilderActions[i](b);
for (int i = 0; i < options.HttpMessageHandlerBuilderActions.Count; i++)
{
options.HttpMessageHandlerBuilderActions[i](b);
}
}
else
{
for (int i = 0; i < options.PrimaryHandlerActions.Count; i++)
{
options.PrimaryHandlerActions[i](b);
}

for (int i = 0; i < options.AdditionalHandlersActions.Count; i++)
{
options.AdditionalHandlersActions[i](b);
}
Comment on lines +176 to +184
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change PrimaryHandlerActions / AdditionalHandlersActions to be List<T> instead of IReadOnlyList<T>, then you can foreach these, instead.

}

// Logging is added separately in the end. But for now it should be still possible to override it via filters...
Expand Down
21 changes: 20 additions & 1 deletion src/libraries/Microsoft.Extensions.Http/src/DefaultHttpMessageHandlerBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,26 @@ public override string? Name
}
}

public override HttpMessageHandler PrimaryHandler { get; set; } = new HttpClientHandler();
private HttpMessageHandler? _primaryHandler;
internal bool PrimaryHandlerIsSet { get; private set;}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Why can't _primaryHandler just be checked directly to see whether it's null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there are existing cases when primary handler is set explicitly to null. E.g. in gRPC. While I kinda consider this pattern erroneous, I still don't want to break the users...

public override HttpMessageHandler PrimaryHandler
{
get
{
if (_primaryHandler is null && !PrimaryHandlerIsSet)
{
_primaryHandler = new HttpClientHandler();
PrimaryHandlerIsSet = true;
}
return _primaryHandler!;
}

set
{
_primaryHandler = value;
PrimaryHandlerIsSet = true;
}
}

public override IList<DelegatingHandler> AdditionalHandlers { get; } = new List<DelegatingHandler>();

Expand Down
27 changes: 25 additions & 2 deletions .../Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.Logging.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ public static IHttpClientBuilder AddLogger(this IHttpClientBuilder builder, Func
options.LoggingBuilderActions.Add(b =>
{
IHttpClientLogger httpClientLogger = httpClientLoggerFactory(b.Services);
HttpClientLoggerHandler handler = new HttpClientLoggerHandler(httpClientLogger);

HttpClientLoggerHandler handler = new HttpClientLoggerHandler(httpClientLogger);
if (wrapHandlersPipeline)
{
b.AdditionalHandlers.Insert(0, handler);
Expand Down Expand Up @@ -110,7 +110,30 @@ public static IHttpClientBuilder AddLogger<TLogger>(this IHttpClientBuilder buil
{
ThrowHelper.ThrowIfNull(builder);

return AddLogger(builder, services => services.GetRequiredService<TLogger>(), wrapHandlersPipeline);
builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.LoggingBuilderActions.Add(b =>
{
IHttpClientLogger? httpClientLogger = null;
if (b.Services is IKeyedServiceProvider keyedProvider)
{
httpClientLogger = keyedProvider.GetKeyedService<TLogger>(b.Name);
}
httpClientLogger ??= b.Services.GetRequiredService<TLogger>();

HttpClientLoggerHandler handler = new HttpClientLoggerHandler(httpClientLogger);
if (wrapHandlersPipeline)
{
b.AdditionalHandlers.Insert(0, handler);
}
else
{
b.AdditionalHandlers.Add(handler);
}
});
});

return builder;
}

/// <summary>
Expand Down
78 changes: 66 additions & 12 deletions ...ibraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static IHttpClientBuilder AddHttpMessageHandler(this IHttpClientBuilder b

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => b.AdditionalHandlers.Add(configureHandler()));
options.AddAdditionalHandlersAction(b => b.AdditionalHandlers.Add(configureHandler()));
});

return builder;
Expand Down Expand Up @@ -106,7 +106,7 @@ public static IHttpClientBuilder AddHttpMessageHandler(this IHttpClientBuilder b

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => b.AdditionalHandlers.Add(configureHandler(b.Services)));
options.AddAdditionalHandlersAction(b => b.AdditionalHandlers.Add(configureHandler(b.Services)));
});

return builder;
Expand All @@ -133,7 +133,17 @@ public static IHttpClientBuilder AddHttpMessageHandler<THandler>(this IHttpClien

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => b.AdditionalHandlers.Add(b.Services.GetRequiredService<THandler>()));
options.AddAdditionalHandlersAction(b =>
{
THandler? handler = null;
if (b.Services is IKeyedServiceProvider keyedProvider)
{
handler = keyedProvider.GetKeyedService<THandler>(b.Name);
}
handler ??= b.Services.GetRequiredService<THandler>();

b.AdditionalHandlers.Add(handler);
});
});

return builder;
Expand All @@ -157,7 +167,7 @@ public static IHttpClientBuilder ConfigurePrimaryHttpMessageHandler(this IHttpCl

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => b.PrimaryHandler = configureHandler());
options.AddPrimaryHandlerAction(b => b.PrimaryHandler = configureHandler(), updatesExistingHandler: false);
});

return builder;
Expand Down Expand Up @@ -187,7 +197,7 @@ public static IHttpClientBuilder ConfigurePrimaryHttpMessageHandler(this IHttpCl

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => b.PrimaryHandler = configureHandler(b.Services));
options.AddPrimaryHandlerAction(b => b.PrimaryHandler = configureHandler(b.Services), updatesExistingHandler: false);
});

return builder;
Expand Down Expand Up @@ -215,7 +225,18 @@ public static IHttpClientBuilder ConfigurePrimaryHttpMessageHandler<THandler>(th

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => b.PrimaryHandler = b.Services.GetRequiredService<THandler>());
options.AddPrimaryHandlerAction(b =>
{
THandler? handler = null;
if (b.Services is IKeyedServiceProvider keyedProvider)
{
handler = keyedProvider.GetKeyedService<THandler>(b.Name);
}
handler ??= b.Services.GetRequiredService<THandler>();

b.PrimaryHandler = handler;
},
updatesExistingHandler: false);
});

return builder;
Expand All @@ -241,7 +262,7 @@ public static IHttpClientBuilder ConfigurePrimaryHttpMessageHandler(this IHttpCl

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => configureHandler(b.PrimaryHandler, b.Services));
options.AddPrimaryHandlerAction(b => configureHandler(b.PrimaryHandler, b.Services), updatesExistingHandler: true);
});

return builder;
Expand Down Expand Up @@ -288,15 +309,22 @@ public static IHttpClientBuilder UseSocketsHttpHandler(this IHttpClientBuilder b

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b =>
options.AddPrimaryHandlerAction(b =>
{
if (b.PrimaryHandler is not SocketsHttpHandler handler)
SocketsHttpHandler? handler = null;

// checking for PrimaryHandlerIsSet first avoids allocating a default handler instance on accessing PrimaryHandler property
bool canAccessPrimaryHandler = b is not DefaultHttpMessageHandlerBuilder dfb || dfb.PrimaryHandlerIsSet;
if (canAccessPrimaryHandler)
{
handler = new SocketsHttpHandler();
handler = b.PrimaryHandler as SocketsHttpHandler;
}
handler ??= new SocketsHttpHandler();

configureHandler?.Invoke(handler, b.Services);
b.PrimaryHandler = handler;
});
},
updatesExistingHandler: true);
});

return builder;
Expand Down Expand Up @@ -378,6 +406,13 @@ public static IHttpClientBuilder UseSocketsHttpHandler(this IHttpClientBuilder b

builder.Services.AddTransient(s => AddTransientHelper<TClient>(s, builder));

builder.Services.AddKeyedTransient<TClient>(builder.Name, (s, key) =>
{
HttpClient httpClient = s.GetRequiredKeyedService<HttpClient>(key);
ITypedHttpClientFactory<TClient> typedClientFactory = s.GetRequiredService<ITypedHttpClientFactory<TClient>>();
return typedClientFactory.CreateClient(httpClient);
});

return builder;
}

Expand Down Expand Up @@ -442,6 +477,13 @@ private static TClient AddTransientHelper<TClient>(IServiceProvider s, IHttpClie

builder.Services.AddTransient(s => AddTransientHelper<TClient, TImplementation>(s, builder));

builder.Services.AddKeyedTransient<TClient>(builder.Name, (s, key) =>
{
HttpClient httpClient = s.GetRequiredKeyedService<HttpClient>(key);
ITypedHttpClientFactory<TImplementation> typedClientFactory = s.GetRequiredService<ITypedHttpClientFactory<TImplementation>>();
return typedClientFactory.CreateClient(httpClient);
});

return builder;
}

Expand Down Expand Up @@ -499,6 +541,12 @@ internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientB
return factory(httpClient);
});

builder.Services.AddKeyedTransient<TClient>(builder.Name, (s, key) =>
{
HttpClient httpClient = s.GetRequiredKeyedService<HttpClient>(key);
return factory(httpClient);
});

return builder;
}

Expand Down Expand Up @@ -548,6 +596,12 @@ internal static IHttpClientBuilder AddTypedClientCore<TClient>(this IHttpClientB
return factory(httpClient, s);
});

builder.Services.AddKeyedTransient<TClient>(builder.Name, (s, key) =>
{
HttpClient httpClient = s.GetRequiredKeyedService<HttpClient>(key);
return factory(httpClient, s);
});

return builder;
}

Expand Down Expand Up @@ -644,7 +698,7 @@ public static IHttpClientBuilder ConfigureAdditionalHttpMessageHandlers(this IHt

builder.Services.Configure<HttpClientFactoryOptions>(builder.Name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b => configureAdditionalHandlers(b.AdditionalHandlers, b.Services));
options.AddAdditionalHandlersAction(b => configureAdditionalHandlers(b.AdditionalHandlers, b.Services));
});

return builder;
Expand Down
Loading