HttpClientFactory Keyed DI APIs

[CarnaViire]

Fixes #89755

UPD: Updated to the approved API shape

---

Included:

• Opt-in API for all lifetimes
• Opt-out API; after opting-out, can opt-in again, and vice versa
• Combination with ConfigureHttpClientDefaults, both for opt-in and opt-out, in any order
• UPD: more tests
• UPD: fix for handler DI lifetime
• UPD: workaround for Dependency injection: FromKeyedServiceAttribute falls back to last defined unkeyed service of the type -- unexpected behavior? #102654

Not included (yet?):

• Typed client lifetime adjustment
• IDisposable transients capture mitigation (do not allow transients?)
• scope mismatch fix integration (punted from 9.0)

cc @halter73 @JamesNK @davidfowl @stephentoub @ManickaP

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ManickaP]

LGTM, not an expert here though 😄

[CarnaViire]

@halter73 @JamesNK @rokonec @benjaminpetit PTAL, if possible 🙏

[halter73]

I would still prefer it if we could make keyed DI work by default. We should fix #102654 regardless and backport the fix to .NET 8 imo. This seems good to me though.

It's too bad we couldn't resolve #47091 at least for people that use keyed services, but I understand it's a hard problem. And it would probably be better to have a one-size-fits-all solution for typed clients and keyed clients.

This won't affect the performance of resolving non-keyed HttpClientFactory services, right? Either way, I expect the cost of DI to be extremely small compared to the costs of making HTTP requests.

[halter73]

Not included (yet?):

• Typed client lifetime adjustment
• IDisposable transients capture mitigation (do not allow transients?)
• scope mismatch fix integration (punted from 9.0)

I wonder if the simplest/best thing to do for now would be to throw a NotSupportedException for attempted transient registrations and include an aka.ms link to https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection-guidelines#disposable-transient-services-captured-by-container in the exception message.

[CarnaViire]

I wonder if the simplest/best thing to do for now would be to throw a NotSupportedException for attempted transient registrations

I've thought about it, but then decided that nothing really prevents the users to make the keyed registrations manually themselves, and still end up with the same problem (I've already seen folks doing that, as we didn't have the integration ready in 8.0). Also, the same problem is there for any transient IDisposable registrations now -- and we allow the users to register them still.

I was considering getting in touch with the Analyzer team and see maybe we could have some analyzer warnings as a middle ground.

And at least we have a scoped lifetime selected by default.

I'd also expect that the keyed clients would mostly get injected in constructors, rather than instantiated directly via ServiceProvider. (arguably, injecting HttpClientFactory and calling CreateClient looks better than injecting ServiceProvider and calling GetRequiredKeyedService). Singletons have a limited number of instances, and as for scoped services, the transient dependencies would be all cleaned up when the scope finishes.