Add new Polly DI extension method for HttpClientFactory

[martincostello]

This pull request adds the following new overload to the Polly extensions for HttpClientFactory:

public static IServiceCollection AddPolicyRegistry(
    this IServiceCollection services,
    Action<IServiceProvider, IPolicyRegistry<string>> configureRegistry)

The existing two registrations allow a policy registry to be registered and then configured directly, but there isn't an overload that takes a delegate and provides the IServiceProvider to allow the policies to be configured based on other registered services, such as settings loaded from IConfiguration.

I've added a unit test that demonstrates the use case of using another service to configure the policies at the point the policy registry dependency is first resolved.

I've also tidied up a few minor trivial things I found while making the change, which I can split into a separate PR if necessary.

[ghost]

As per aspnet/Announcements#411, we are currently migrating components from this repository to other repositories (dotnet/runtime, dotnet/aspnetcore). We recommend waiting until that process is complete and opening your PR against the appropriate repository. See the original announcement for a list mapping packages to the new repository.

[martincostello]

Any update here @Pilchie? As far as I could tell from the docs, the library I've changed isn't up for migration and is staying here.

[Pilchie]

Tagging @karelz for HttpClientFactory.

[martincostello]

@karelz Would you be able to take a look at this PR please? It seems to have fallen into a limbo state with the repo reorganisation.

[karelz]

Sorry for the delay @martincostello . Sadly, we do not have anyone currently with knowledge of this code base. We should be able to look at it (incl. general code transfer) around mid July once we are sure we can deliver all critical features and bug fixes for 5.0.
Will that work for you?

[martincostello]

No problem - thanks!

[martincostello]

Is this too late for 5.0 now?

[karelz]

We are still finishing 5.0 this week and a bit next week. Then we will have to ramp up on the code base from scratch, so I don't expect we will be able to make it happen with high confidence in 5.0 :(

[karelz]

Sorry, we finally got our grip on the code base (thanks @CarnaViire!) and got to this PR.
It seems to be specifically about Polly, which should be still owned by ASP.NET team -- @Pilchie who should take a look at Polly part of HttpClientFactory?

@martincostello sorry for the delay, you have been caught unfortunately in the middle of ownership transition where things are a bit messy :(

[halter73]

Original PR that added support for policy registry: aspnet/HttpClientFactory#66

@martincostello Did you copy this pattern from anywhere? To me this seems like it's trying to work around the fact that the IPolicyRegistry isn't configured using the ASP.NET Core's Options pattern. If it was, you'd be able to configure policies based on other registered services using Configure.

I'm thinking maybe we should update AddPolicyRegistry to use the Options pattern instead.

[martincostello]

@halter73 Yeah, I based it on the overloads that the various methods for configuring IHttpClientFactory have.

[halter73]

@davidfowl @Tratcher What are your thoughts about using the Options pattern here?

[Tratcher]

Action<IServiceProvider, IPolicyRegistry<string>> does resemble an Options pattern. @halter73 do you have a rough idea of what an Options version of this would look like?

[halter73]

I haven't fully thought it through, but I figure the IPolicyRegistry​<​string​> could be a property in an option type.

[JunTaoLuo]

FYI, as part of https://github.com/dotnet/aspnetcore-internal/issues/3707, we are moving this component to https://github.com/dotnet/aspnetcore, please re-open this PR in that repo.

[martincostello]

Opened dotnet/aspnetcore#28283.