justeattakeaway / httpclient-interception

A .NET library for intercepting server-side HTTP requests
https://tech.just-eat.com/2017/10/02/reliably-testing-http-integrations-in-a-dotnet-application/
Apache License 2.0
346 stars 27 forks source link

Revise documentation covering usage together with IHttpClientFactory #750

Closed silkfire closed 4 months ago

silkfire commented 4 months ago

I was reading through the documentation that describes how to use HttpClientInterception together with IHttpClientFactory, and I found the recommended solution to be a bit misleading and quite over-engineered, especially having to handroll your own implementation of IHttpMessageHandlerBuilderFilter.

I was able to make it work just by doing this:

var builder = new HttpRequestInterceptionBuilder();

builder.Requests()
       .ForGet()
       .ForAnyHost()
       .Responds()
       .WithStatus(HttpStatusCode.OK)
       .RegisterWith(options);

var services = new ServiceCollection();
services.AddHttpClient("myclient")
        .AddHttpMessageHandler(options.CreateHttpMessageHandler));    // 👈🏻 This line is all you need

var serviceProvider = services.BuildServiceProvider();

var httpClientFactory = serviceProvider.GetRequiredService<IHttpClientFactory>();
var client = httpClientFactory.CreateClient("myclient");

Perhaps the documentation could be updated to reflect this?

hwoodiwiss commented 4 months ago

Thanks for raising this!

I can see your point, but I think your example here applies to a different use-case than the example in the readme.

In your case, you'd apply the message handler to your one client called "myclient". In the example IHttpMessageHandlerBuilderFilter is used to ensure that the interception is applied to all clients instantiated throughout the application via the client factory.

In .NET 8, a more similar example to what we're currently showing might be

var builder = new HttpRequestInterceptionBuilder();

builder.Requests()
       .ForGet()
       .ForAnyHost()
       .Responds()
       .WithStatus(HttpStatusCode.OK)
       .RegisterWith(options);

var services = new ServiceCollection();
services.ConfigureHttpClientDefaults()
    .ConfigureAdditionalHttpMessageHandlers((handlerList, _) => handlerList.Add(options.CreateHttpMessageHandler()));
services.AddHttpClient("myclient");

var serviceProvider = services.BuildServiceProvider();

var httpClientFactory = serviceProvider.GetRequiredService<IHttpClientFactory>();
var client = httpClientFactory.CreateClient("myclient");

However, we currently target older TFM's that wouldn't support this, so I think our documentation should reflect that for now.

martincostello commented 4 months ago

The advantage of IHttpMessageHandlerBuilderFilter is that it can be registered once and it will hook-up interception for all usages of IHttpClientFactory without the tests having to know any of the implementation details of how you use it in the application under test. It also plays nicely with the case where there are lots of other delegating handlers configured, as you can (mostly) ensure that the interception runs last, so your tests still cover the other handlers, rather than causing them to be short-circuited by the interception.

If you have an application where you have multiple named HTTP clients and wish to integration test them, using the approach in your original comment requires you to repeat the setup for each of those clients in order to intercept their calls, as well as knowing (and then repeating) all of the client registration calls in the tests.

For a concrete example of the above case, in one of my open source projects I configure HTTP clients for each OAuth provider I have, which in this case is 5-7 depending on the application's configuration. All of that detail is hidden from the integration test, which intercept the calls for all of them (and other usages of HttpClient not to do with OAuth) in these 3 lines of code using the IHttpMessageHandlerBuilderFilter approach. For a more extreme example, there's this project - we maintain over 90 OAuth providers, and all their HTTP calls are intercepted with these 2 lines of code.

In a simple application though, you're right, it could be done the other way if you were doing some unit testing where you just want a simple HttpClient that intercepts the calls.

We'd accept a pull request that suggests the approach you give as an alternative for a simple case like you describe, but I still feel that the current approach is the best overall recommendation as for just a few lines of code to add to your tests, it's the simplest way to integrate the library into an existing test suite.

silkfire commented 4 months ago

Thanks both of you for providing your opinions and knowledge on the subject, I feel much wiser now on how everything works together and the advantages of using the filter when working with a collection of tests.

silkfire commented 4 months ago

@martincostello Just a question. When using a shared fixture/collection like shown in the example, how does HttpClientInterceptor keep the registrations apart so that each test-specific registration doesn't conflict with the others? Especially as tests are run in parallell in each test class?

martincostello commented 4 months ago

It doesn't by default, but you can use a scope to constrain the lifetime of the matches where the matches can't otherwise be discriminated by more specific matches (e.g. looking at a header, a part of the path, etc.).