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.

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

Allow configuration of HttpClients used by DownstreamWebApi #1735

Closed
wants to merge 4 commits into from

Conversation

h3rmanj
Copy link
Contributor

@h3rmanj h3rmanj commented May 12, 2022

Addresses #1740

The MS docs recommends implementing resilient http requests using libraries like Polly. At the same time, the recommended way to make authenticated api calls is through the IDownstreamWebApi interface. This PR closes the gaps in the docs and allows the consumer access to the IHttpClientBuilder, allowing them to add Polly policies for their downstream apis etc.

Changed to add named HttpClients, which DownstreamWebApi then gets from IHttpClientFactory, allowing the consumer to optionally configure HttpClient per downstream service.

Example usage with this:

builder.Services.AddMicrosoftIdentityWebApi(builder.Configuration)
  .EnableTokenAcquisitionToCallDownstreamApi()
    .AddDownstreamWebApi(
      "DownstreamApi",
      builder.Configuration.GetSection("DownstreamApi"),
      httpClientBuilder => httpClientBuilder
        .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
        {
          TimeSpan.FromSeconds(1),
          TimeSpan.FromSeconds(5),
          TimeSpan.FromSeconds(10)
        })))
    .AddDownstreamWebApi(
      "AnotherApi",
      builder.Configuration.GetSection("AnotherApi"),
      httpClientBuilder => httpClientBuilder
        .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
        {
          TimeSpan.FromSeconds(1),
          TimeSpan.FromSeconds(2)
        })))
  .AddDistributedTokenCaches();

allow the consumer access to the IHttpClientBuilder, allowing them to things like Polly policies etc.

changed to add named clients, which DownstreamWebApi then gets from IHttpClientFactory, allowing the consumer to configure HttpClient per downstream service
@ghost
Copy link

ghost commented May 12, 2022

CLA assistant check
All CLA requirements met.

h3rmanj added 2 commits May 12, 2022 18:03
…pClient

It seems the typed HttpClient expects HttpClient present in the constructor. Register the IDownstreamWebApi as a Transient service like the AddHttpClient would, and only register the named HttpClient.
using TryAddTransient instead of AddTransient
@h3rmanj
Copy link
Contributor Author

h3rmanj commented Jun 10, 2022

Alternatively, if changing the API of AddDownstreamWebApi isn't desirable, I could just drop it. Multiple calls to AddHttpClient with the same client name applies all options to the same client. So we could add a named HttpClient with the serviceName internally, and the consumer could then AddHttpClient(serviceName) as well if they need it. It would be a little bit more of a hidden feature, but would work.

builder.Services.AddMicrosoftIdentityWebApi(builder.Configuration)
  .EnableTokenAcquisitionToCallDownstreamApi()
    .AddDownstreamWebApi("DownstreamApi", builder.Configuration.GetSection("DownstreamApi"))
    .AddDownstreamWebApi("AnotherApi", builder.Configuration.GetSection("AnotherApi"))
  .AddDistributedTokenCaches();

builder.Services.AddHttpClient("DownstreamApi")
  .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
  {
    TimeSpan.FromSeconds(1),
    TimeSpan.FromSeconds(5),
    TimeSpan.FromSeconds(10)
  }));
      
builder.Services.AddHttpClient("AnotherApi")
  .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
  {
    TimeSpan.FromSeconds(1),
    TimeSpan.FromSeconds(2)
  }));

@jmprieur
Copy link
Collaborator

@h3rmanj
Let's us get back to you on this one. We are considering having a new interface (IDownstreamRestApi) which therefore would not have breaking changes (as it would be new).

@jennyf19: let's work on collecting all the issues and PRs around IDownstreamWebApi and add the right design for IDownstreamRestApi

@h3rmanj
Copy link
Contributor Author

h3rmanj commented Nov 8, 2022

@jmprieur @jennyf19
I see some work has been done in the rel/v2 branch, but I'm not sure if it's complete. What is the status of this? Will there be a chance to review before a final release?

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 9, 2022

@h3rmanj : what you wrote in #1735 (comment) would work in Microsoft.Identity.Web 2.0.5-preview (with AddDownstrreamRestApi)

@h3rmanj
Copy link
Contributor Author

h3rmanj commented Feb 28, 2023

Implemented in 2.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants