aspnet / HttpClientFactory

[Archived] Contains an opinionated factory for creating HttpClient instances. Project moved to https://github.com/aspnet/Extensions
Apache License 2.0
251 stars 72 forks source link

Rename HttpClientFactory -> IHttpClientFactory #11

Closed rynowak closed 7 years ago

rynowak commented 7 years ago

At first I was like: 😞 😢 😭 😠

And then I was like: 😕 🤔 😃 👍

https://msdn.microsoft.com/en-us/library/system.net.http.httpclientfactory(v=vs.118).aspx


The fqn System.Net.Http.HttpClientFactory already exists in the old WebAPI client library. This is where we we also provide things like ReadAsAsync<T> methods and the general support for formatters and deserialization. We own this library and control it's destiny 👍

Basically we expect users to use this package and the name HttpClientFactory will conflict between Microsoft.Extensions.Http.HttpClientFactory and and System.Net.Http.HttpClientFactory. This will be broken for a lot of people.

:canada: Fortunately, we Canadians have the same word for crisis as we do for opportunity :canada:.

Please note that most of the types we need to interact with for HttpClient are in System.Net.Http. We were going to add Microsoft.Extensions.Http as a companion namespace. We expected that users will have to sprinkle some Microsoft.Extensions.Http around their code - but this was a non-goal.

Please also note that it is not a breaking change to change a type from static to abstract, in fact a static class is declared as sealed abstract at the IL level - and unsealing is not a breaking change.


At this point a plan starts forming in my mind. Don't reply until you get to the end. I know some of you are going to type a response before you read all of the steps. Here's what I think this would look like:

  1. We create a new package in the WSR repo System.Net.Http.XYZ - pick whatever name you want
  2. We type-forward HttpClientFactory into this new package
  3. System.Net.Http.Formatting now depends on this new package
  4. We unseal System.Net.Http.HttpClientFactory and add whatever we decide is required.
  5. We ship 5.2.4 of WSR including the new package (already planned)
  6. We ship Microsoft.Extensions.Http as the implementation of HttpClientFactory including the wire-ups for DI, logging, and options

Pros

Cons

Why another package This solves an issue with coupling.

Right now HttpClientFactory lives in the Microsoft.AspNet.WebApi.Client package, where it also has a whole bunch of infrastructure for formatters, content negotiation, and also a reference to JSON.NET

We anticipate that Microsoft.Extensions.Http to have a bunch of references to other Microsoft.Extensions.* packages. We really don't want to introduce coupling either way. You don't want things like DI and options showing up in your old ASP.NET WebAPI 2 project, and you don't necessary want WebAPI2 formatters in your ASP.NET Core project.

If we have to live with the coupling one way we'd probably want to reference Microsoft.AspNet.WebApi.Client from Microsoft.Extensions.Http. Most of what's there is still pretty relevant, and it's not coupled to any server-side abstractions.

glennc commented 7 years ago

Is your statement about the existing API not being that bad based upon an assumption that people don't really consume it anyway? Since I would've assumed that a new HTTPClientFactory would maintain the lists of delegating handlers and I would just give it a name. No?

rynowak commented 7 years ago

bad based upon an assumption that people don't really consume it anyway?

Yes, these are static methods that are basically infrastructure.

Since I would've assumed that a new HTTPClientFactory would maintain the lists of delegating handlers

Yes, but this is an implementation concern not part of the API.

I expect that we'd add:

public abstract HttpClient CreateClient(string name);

// Maybe
public virtual HttpClient CreateClient();
rynowak commented 7 years ago

Conclusions

We will use:

namespace System.Net.Http
{
public interface IHttpClientFactory
{
  HttpClient CreateClient(string name);
}
}

This will ship in the Microsoft.Extensions.Http package.

We are not concerned about confusion between HttpClientFactory and IHttpClientFactory.

We're comfortable making this an interface instead of a class because we think we're got the API nailed down and it's minimal.

PureKrome commented 7 years ago

@rynowak Quick question:

Will there be official documentation about this class AND how to use it in code?

I'm asking this about the lack of information (or discovery of this information) with respect to the current status quo with HttpClient and how many people still enclose this in a using statement ... until recent times (q3/q4 last year) when AspMonsters dropped that blog post which made a heap (nearly all?) of us go 'what the heck?! really?? /me face palms'.

Not trolling/hating - just want to make sure everyone gets onto the same page, as early as possible.

👍 🍰

rynowak commented 7 years ago

Will there be official documentation about this class AND how to use it in code?

Yes, what we're doing in this repo is building a library and a pattern for using HttpClient with DI in a configurable way. This is planned as part of the ASP.NET Core 2.1 release but will be usable outside of ASP.NET Core.

The .NET team is also looking at some of the foundational issues that have made using HttpClient difficult.

PureKrome commented 7 years ago

thank you heaps @rynowak for the info! Sounds great :) Can't wait to see and use this :)

P.S. Please don't forget UNIT TESTING scenario's 👍 🍰

rynowak commented 7 years ago

See also: https://github.com/aspnet/HttpClientFactory/issues/9

rynowak commented 7 years ago

8a64334