Extensions: Add an overload with a Func<IServiceProvider, T> to AddHttpClient<TClient, TImplementation>

Created on 19 Dec 2018  路  6Comments  路  Source: dotnet/extensions

We have a multi-tenant web app that calls to our multi-tenant Web Api. I'm changing the api client used in the web app to use IHttpClientFactory, using typed clients. However, as it's a multi-tenant software, we have logic to call to different URLs depending on where the request is coming from. Something like this:

public class MyApiClient : IMyApiClient
{
   public MyApiClient(HttpClient client, string baseUrl) {}
}

I used to do DI like this before:

services.AddScoped<IMyApiClient>(sp => 
{
   // decide which url to use in the client
   return new MyApiClient(url);
}

However, now with .AddHttpClient, since there's no overload that accepts a Func<IServiceCollection, T>. there's no way I can do that anymore, and I'm left with three non-optimal solutions:

1) Change to use named clients instead of typed clients. However, that's not good not only because well, it's named clients, but also because I already have plenty of dependencies to IMyApiClient in a lot of places.

2) Copy the source code from .AddHttpClient and do the base URL selection just like before.

3) Create a whole new class just to represent the API base URL so I can register it on the IoC container and ask for that instead of a string on the MyApiClient constructor.

Note that I can't just set the HttpClient base URL using the Action<HttpClient> configureClient overload for two reasons:

1) In order to select the base URL, I need another dependency. Thus, I would need an instance of IServiceProvider.
2) Even if I could change my logic to not need any dependency when selecting the base URL, it still wouldn't work, because the base URL can't be changed after the HttpClient has executed a request.

EDIT:

I ended up going with option 2. This is what I came up with:

services.AddHttpClient("MyApiClient")
    .AddTransientHttpErrorPolicy(p => p.WaitAndRetryAsync(4, attempt => TimeSpan.FromSeconds(Math.Pow(1.45, attempt)) + TimeSpan.FromMilliseconds(_xoroshiro.Next(0, 100))))
    .AddTransientHttpErrorPolicy(p => p.CircuitBreakerAsync(5, TimeSpan.FromSeconds(15)));

services.AddTransient<IMyApiClient>(sp =>
{
    // logic to select URL
    var context = sp.GetRequiredService<IHttpContextAccessor>().HttpContext;
    string[] host = context.Request.Host.Host.Replace("someurl", "").Split(new char[] { '.' });
    string subdomain= host.Length > 1 ? host[1] : host[0];
    string url = Configuration.GetSection("MyApiBaseUrl")?[subdomain] ?? Configuration.GetSection("MyApiBaseUrl")?["Default"];

    // actual DI logic
    var httpClientFactory = sp.GetRequiredService<IHttpClientFactory>();
    var httpClient = httpClientFactory.CreateClient("MyApiClient");

    return new MyApiClient(url, httpClient);
});

What's different from the ASP.NET implementation is that I don't use .GetRequiredService<ITypedHttpClientFactory<TClient>>(); and .Activator(_services, new object[] { httpClient });, instead just newing the api client up. Is there any practical difference to how this will behave?

3 - Done api-approved area-httpclientfactory enhancement

Most helpful comment

Here's the API proposal to review.

Summary

This change adds an accelerator for some functionality that's hard to discover. We've had a few bugs reported now to the effect of "HTTP Client Factory doesn't have what I need" people people didn't find these APIs where they are currently exposed.

Before

```C#
services.AddHttpClient("widget").AddTypedClient((httpClient, services) =>
{
var client = new WidgetClient(httpClient, services.GetRequiredService>());
client.SomeSetting = Configuration["MySetting"];
return client;
});

**After**

```C#
services.AddHttpClient<IWidgetClient, WidgetClient>((httpClient, services) =>
{
    var client = new WidgetClient(httpClient, services.GetRequiredService<ILogger<WidgetClient>>());
    client.SomeSetting = Configuration["MySetting"];
    return client;
});

APIs

note: the TImplementation type parameter is needed to avoid a really nasty source-breaking change. This means that practically speaking, both type parameters will need to be specified. I think this still accomplishes the goal of making these APIs more discoverable because they will be part of the overload set of AddHttpClient (today they are hiding on AddTypedClient where folks don't look)

```c#
namespace Microsoft.Extensions.DependencyInjection
{
public static class HttpClientFactoryServiceCollectionExtensions
{
public static IHttpClientBuilder AddHttpClient(this IServiceCollection services, Func factory)
where TClient : class
where TImplementation : class, TClient
{
}

    public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, string name, Func<HttpClient, TImplementation> factory)
        where TClient : class
        where TImplementation : class, TClient
    {
    }

    public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, Func<HttpClient, IServiceProvider, TImplementation> factory)
        where TClient : class
        where TImplementation : class, TClient
    {
    }

    public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, string name, Func<HttpClient, IServiceProvider, TImplementation> factory)
        where TClient : class
        where TImplementation : class, TClient
    {
    }
}

}
```

All 6 comments

I've got a similar problem. I need to send requests authenticated with Digest Authentication. However, in order to do that with HttpClient(and not manually, which is a pain) I need to pass the credentials to the HttpClient constructor. Unfortunately though, there is no oveload of AddHttpClient currently that accepts a Func<HttpClient> (only Action<HttpClient> for configuration).

Not sure how I missed following up on this issue, but we definitely have the following API in 2.2 and 3.0:

C# public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, Action<HttpClient> configureClient) where TClient : class where TImplementation : class, TClient

@rynowak Well, you see, there are two issues with that existing API: 1) in the provided Action<HttpClient> you'll not have access to the IServiceProvider that is providing that client; 2) you cannot control the creation of the client.

Thinking about it though, I'm not sure if 2 would be possible to solve at all, since the underlying system pools HttpClients, right? 1 should be doable as I understand it, however.

@rynowak BTW, I know this is a meta discussion and I totally do not want to go deep in this and I can totally guess and understand the reasons you do this and it's certainly _not_ specific to this issue, but just leaving a suggestion: it would be nice if you could ask if your suggestion solves the op's issue instead of closing the issue prematurely.

More than two times already I have had issues closed with a short "smart" suggested "fix" only to have it reopened later and actually fixed.

Sorry, I was clearing out old issues and misunderstood what you were asking for. Thanks for pointing it out 馃憤

You can do this today using AddHttpClient("name").AddTypedClient(...) - https://github.com/aspnet/Extensions/blob/e580a9e1e86b52d55c8c5a084ecb3bd6106156d3/src/HttpClientFactory/Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs#L737

There's no reason not to have this functionality directly exposed on the IServiceCollection.

Here's the API proposal to review.

Summary

This change adds an accelerator for some functionality that's hard to discover. We've had a few bugs reported now to the effect of "HTTP Client Factory doesn't have what I need" people people didn't find these APIs where they are currently exposed.

Before

```C#
services.AddHttpClient("widget").AddTypedClient((httpClient, services) =>
{
var client = new WidgetClient(httpClient, services.GetRequiredService>());
client.SomeSetting = Configuration["MySetting"];
return client;
});

**After**

```C#
services.AddHttpClient<IWidgetClient, WidgetClient>((httpClient, services) =>
{
    var client = new WidgetClient(httpClient, services.GetRequiredService<ILogger<WidgetClient>>());
    client.SomeSetting = Configuration["MySetting"];
    return client;
});

APIs

note: the TImplementation type parameter is needed to avoid a really nasty source-breaking change. This means that practically speaking, both type parameters will need to be specified. I think this still accomplishes the goal of making these APIs more discoverable because they will be part of the overload set of AddHttpClient (today they are hiding on AddTypedClient where folks don't look)

```c#
namespace Microsoft.Extensions.DependencyInjection
{
public static class HttpClientFactoryServiceCollectionExtensions
{
public static IHttpClientBuilder AddHttpClient(this IServiceCollection services, Func factory)
where TClient : class
where TImplementation : class, TClient
{
}

    public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, string name, Func<HttpClient, TImplementation> factory)
        where TClient : class
        where TImplementation : class, TClient
    {
    }

    public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, Func<HttpClient, IServiceProvider, TImplementation> factory)
        where TClient : class
        where TImplementation : class, TClient
    {
    }

    public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, string name, Func<HttpClient, IServiceProvider, TImplementation> factory)
        where TClient : class
        where TImplementation : class, TClient
    {
    }
}

}
```

Was this page helpful?
0 / 5 - 0 ratings

Related issues

SnakyBeaky picture SnakyBeaky  路  6Comments

halter73 picture halter73  路  6Comments

davidfowl picture davidfowl  路  5Comments

vanbukin picture vanbukin  路  7Comments

grahamehorner picture grahamehorner  路  3Comments