Skip to content

Configure existing HttpClient using WebApplicationFactory (e.g. for named clients using IServiceCollection.AddHttpClient<T>) #38883

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

Open
christianrondeau opened this issue Dec 8, 2021 · 5 comments
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-mvc-testing MVC testing package
Milestone

Comments

@christianrondeau
Copy link

christianrondeau commented Dec 8, 2021

Is your feature request related to a problem? Please describe.

This is a duplicate of #5967 but because it was closed due to inactivity, I am creating a new one with more details and use cases.

I am trying to run tests against a Hot Chocolate (GraphQL) server and the Strawberry Shake GraphQL client, which uses HttpClientFactory with a named client.

I'd like to start my app in my tests using Microsoft.AspNetCore.Mvc.Testing, and use the code generation features of Strawberry Shake to generate the GraphQL client. I can't, because Strawberry Shake uses the HttpClientFactory, and all of the methods required to build or configure an HttpClient are internal in WebApplicationFactory.

Example of the code in my tests I'd like to have (simplified and inlined):

var services = new ServiceCollection();
// Here I cannot easily configure the client since it needs a custom HttpMessageHandler from TestServer
services
  .AddHttpClient(GraphClient.ClientName, client => client.BaseAddress = _factory.Server.BaseAddress)
  .ConfigurePrimaryHttpMessageHandler(_ => _factory.Server.CreateHandler());
services
  // This is a library that depends on HttpClientFactory
  .AddGraphClient()
// This is using HttpClientFactory internally
var graphClient = services.BuildServiceProvider().GetRequiredService<GraphClient>();

Describe the solution you'd like

It would be much simpler if instead of just having CreateClient(), I could also use ConfigureClient(HttpClient).

e.g.:

  .ConfigureHttpClient(client => _factory.ConfigureClient(client));

The code is mostly the same as is already inside WebApplicationFactory, however instead of making more properties and methods public, there would be a method to assist in cases where using HttpClientFactory is desirable.

Additional context

While my use case (and the one from #5967) are because of external libraries, I believe making use of a pooled HttpClient factory can make sense for reusing existing code from internal libraries.

@TanayParikh TanayParikh added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Dec 8, 2021
@mkArtakMSFT mkArtakMSFT added old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Dec 8, 2021
@captainsafia
Copy link
Member

We could sensibly add a ConfigureHttpClient overload here that takes an Action<HttpClient> for these kinds of customizations to the WebApplicationFactory.

Tagging this alongside #33846 as we approach addressing some of these issues the WAF for .NET 7.

@captainsafia captainsafia added the feature-mvc-testing MVC testing package label Jan 14, 2022
@ghost
Copy link

ghost commented Jan 14, 2022

Thanks for contacting us.
We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@ghost
Copy link

ghost commented Oct 11, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@berhir
Copy link

berhir commented Jan 9, 2023

I created a custom HttpMessageHandlerBuilder to solve this issue.

public class TestServerHttpMessageHandlerBuilder : HttpMessageHandlerBuilder
{
    public TestServerHttpMessageHandlerBuilder(TestServer testServer, IServiceProvider services)
    {
        Services = services;
        PrimaryHandler = testServer.CreateHandler();
    }

    private string? _name;

    [DisallowNull]
    public override string? Name
    {
        get => _name;
        set
        {
            ArgumentNullException.ThrowIfNull(value);
            _name = value;
        }
    }

    public override HttpMessageHandler PrimaryHandler { get; set; }

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

    public override IServiceProvider Services { get; }

    public override HttpMessageHandler Build()
    {
        if (PrimaryHandler == null)
        {
            throw new InvalidOperationException($"{nameof(PrimaryHandler)} must not be null");
        }

        return CreateHandlerPipeline(PrimaryHandler, AdditionalHandlers);
    }
}

Then in the startup code register the custom TestServerHttpMessageHandlerBuilder BEFORE calling AddHttpClient.

clientServices.AddTransient<HttpMessageHandlerBuilder>(sp => new TestServerHttpMessageHandlerBuilder(webAppFactory.Server, sp));

AddHttpClient checks if a HttpMessageHandlerBuilder was already registered and uses it instead of the default implementation.
Now ALL HttpClients created using the HttpClientFactory will use the TestServer. This is fine as long as you don't want to use the TestServer only for some HttpClients.

@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 20, 2023
@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
@jkone27
Copy link

jkone27 commented Apr 20, 2025

not sure if helpful but also such libraries do similar things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-mvc-testing MVC testing package
Projects
No open projects
Status: No status
Development

No branches or pull requests

7 participants