Is there any chance to introduce IHttpClient form factory?

[ptrstpp950]

I know it is late for such thing, but it is probably the last chance to introduce IHttpClient interface.
If you introduce factory in 2.1 without interface it will always return a class. An interface is better for testing, mocking, etc. I'm sure that I don't have to prove this 🤓

I can implement this if you only agree 😄

[DamianEdwards]

@glennc @davidfowl @rynowak

[Kralizek]

@ptrstpp950 Isn't better to have a mock of IHttpClientFactory that returns an HttpClient with a testing HttpMessageHandler? so that you receive a properly valued HttpResponseMessage instead of an empty shell.

In this test I use WorldDomination.HttpClient.Helpers that offers a FakeHttpMessageHandler receiving a set of HttpMessageOptionss to shortcircuit the HTTP pipeline.

[Test, AutoData]
public async Task Request_is_correct(Contact contact)
{
    var properties = (from p in contact.Properties
                        select new ValuedProperty(p.Key, p.Value.Value)).ToArray();

    var option = new HttpMessageOptions
    {
        HttpMethod = HttpMethod.Post,
        HttpResponseMessage = new HttpResponseMessage(HttpStatusCode.OK)
        {
            Content = Object(contact)
        }
    };

    var sut = CreateSystemUnderTest(option);

    var response = await sut.CreateAsync(properties);

    Assert.That(option.HttpResponseMessage.RequestMessage.RequestUri.AbsolutePath, Contains.Substring("/contacts/v1/contact"));
}

[ptrstpp950]

@Kralizek I can easily provide implementation of IHttpClient with HttpResponseMessage if needed, but I agree that IHttpClientFactory will be useful also

[martincostello]

I’ve seen this discussed previously for the full framework. If memory serves, concrete types are preferred in this case because they’re easier to extend in future versions as extending an interface requires breaking changes from a binary compatibility point of view.

[AdamDotNet]

Is it sufficient that you can create an interface based custom/named HttpClient class? Seems a little easier to mock GetFoo() than the actual HTTP stuff going to actually do the GET /Foo?bar=true stuff on a theoretically IHttpClient, especially when you consider all the nice, automatic content negotiation of WebApi client package are all extension methods ultimately wrapping over the HttpClient's GetAsync method that just returns a byte array/stream.

[davidfowl]

We have no plans to introduce an abstraction over HttpClient. See this for an uber long discussion on adding an interface for the client https://github.com/dotnet/corefx/issues/1624.

[glennc]

If you use the typed client pattern then the majority of your business logic would only be operating against your own typed client, which can have an interface. We have an overload of AddHttpClient that accepts an interface and a type for exactly that purpose. So the code that actually has to interact with HttpClient is the code that is deserializing return types and possibly some error handling and caching.

It is far from what you have to do today. For example, I wrote this to test my PageModel:

[Fact]
public async Task GET_populates_values()
{
    IEnumerable<string> testValues = new List<string>() { "value1", "value2", "value3" };

    var valueService = new Mock<ValuesService>();
    valueService.Setup(x => x.GetValues()).Returns(Task.FromResult(testValues));

    var indexUnderTest = new IndexModel(valueService.Object);

    await indexUnderTest.OnGet();

    Assert.Equal(testValues, indexUnderTest.Values);
}

And when testing the ValuesService itself I mocked HttpMessageHandler without much problem:

[Fact]
public async Task will_return_last_known_good()
{
    var firstCall = true;
    var handler = new MockMessageHandler(req =>
    {
        if(firstCall)
        {
            firstCall = false;
            var resp = new HttpResponseMessage(HttpStatusCode.OK);
            resp.Content = new StringContent("[\"testval\"]");
            return resp;
        }
        return new HttpResponseMessage(HttpStatusCode.InternalServerError);
    });

    var client = new HttpClient(handler);
    client.BaseAddress = new Uri("http://test.local");
    var cache = new MemoryCache(Options.Create(new MemoryCacheOptions()));

    var service = new ValuesService(client, cache, _logger.Object);

    await service.GetValues();
    var values = await service.GetValues();

    Assert.False(firstCall);
    Assert.Equal("testval", values.First());
}

public class MockMessageHandler : HttpMessageHandler
{
    private Func<HttpRequestMessage, HttpResponseMessage> _handler;

    public MockMessageHandler(Func<HttpRequestMessage, HttpResponseMessage> handler)
    {
        _handler = handler;
    }

    protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        return Task.FromResult(_handler(request));
    }
}

I didn't really feel the need for mocking HttpClient itself in any of that testing. The typed client abstraction made the tests that actually interacted with the HTTP types fairly small. Am I alone or naive in that?