diff --git a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs index ebda07a8f77d1a..fcd10d2aa0cb36 100644 --- a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs @@ -645,16 +645,51 @@ public static IHttpClientBuilder ConfigureAdditionalHttpMessageHandlers(this IHt return builder; } -#pragma warning disable 1591 // TODO: Document this API. https://github.com/dotnet/runtime/issues/105974 + /// + /// Registers a named and the related handler pipeline as keyed + /// services with the client's name as the key, and a lifetime provided in the parameter. + /// By default, the lifetime is . + /// + /// The . + /// Lifetime of the keyed services registered. + /// An that can be used to configure the client. + /// + /// + /// A named client resolved from DI as a keyed service will behave similarly to a client you would create with . This + /// means that the client will continue reusing the same instance for the duration of , + /// and it will continue to use the separate, handler's DI scope instead of the scope it was resolved from. + /// + /// + /// WARNING: Registering the client as a keyed service will lead to the and + /// instances being captured by DI as both implement . This might lead to memory leaks if the client is resolved multiple times within a + /// service. + /// + /// + /// WARNING: In case of (1) a keyed registration, or (2) a keyed + /// injected into a service, or (3) long-running application scopes, + /// the instances will get captured by a singleton or a long-running scope, so they will NOT be able to participate in the handler rotation, + /// which can result in the loss of DNS changes. (This is a similar issue to the one with Typed Clients, that are registered as services.) + /// + /// + /// If called twice with for a builder with the same name, the lifetime of the keyed service will be updated to the latest used value. + /// + /// + /// If called for a typed client, only the related named client and handler will be registered as keyed. The typed client itself will continue to be registered as + /// a transient service. + /// + /// + /// If used in conjuction with , + /// the key is used, so any named instance will be resolvable as a keyed service (unless explicitly opted-out + /// from the keyed registration via ). + /// + /// public static IHttpClientBuilder AddAsKeyed(this IHttpClientBuilder builder, ServiceLifetime lifetime = ServiceLifetime.Scoped) { ThrowHelper.ThrowIfNull(builder); string? name = builder.Name; IServiceCollection services = builder.Services; - HttpClientMappingRegistry registry = GetMappingRegistry(services); - - UpdateEmptyNameHttpClient(services, registry); + HttpClientMappingRegistry registry = services.GetMappingRegistry(); if (name == null) { @@ -678,15 +713,25 @@ public static IHttpClientBuilder AddAsKeyed(this IHttpClientBuilder builder, Ser return builder; } + /// + /// Removes the keyed registrations for the named and . + /// + /// The . + /// An that can be used to configure the client. + /// + /// + /// If used in conjuction with , + /// it will only affect the previous "global" registration, and won't affect the clients registered for a specific name + /// with . + /// + /// public static IHttpClientBuilder RemoveAsKeyed(this IHttpClientBuilder builder) { ThrowHelper.ThrowIfNull(builder); string? name = builder.Name; IServiceCollection services = builder.Services; - HttpClientMappingRegistry registry = GetMappingRegistry(services); - - UpdateEmptyNameHttpClient(services, registry); + HttpClientMappingRegistry registry = services.GetMappingRegistry(); if (name == null) { @@ -704,32 +749,6 @@ public static IHttpClientBuilder RemoveAsKeyed(this IHttpClientBuilder builder) return builder; } -#pragma warning restore 1591 - - // workaround for https://github.com/dotnet/runtime/issues/102654 - private static void UpdateEmptyNameHttpClient(IServiceCollection services, HttpClientMappingRegistry registry) - { - if (registry.EmptyNameHttpClientDescriptor is not null) - { - bool removed = services.Remove(registry.EmptyNameHttpClientDescriptor); - - if (removed) - { - // trying to add it as keyed instead - if (!registry.KeyedLifetimeMap.ContainsKey(string.Empty)) - { - var clientLifetime = new HttpClientKeyedLifetime(string.Empty, ServiceLifetime.Transient); - registry.KeyedLifetimeMap[string.Empty] = clientLifetime; - clientLifetime.AddRegistration(services); - } - } - } - - if (services.Any(sd => sd.ServiceType == typeof(HttpClient) && sd.ServiceKey is null)) - { - throw new InvalidOperationException($"{nameof(AddAsKeyed)} isn't supported when {nameof(HttpClient)} is registered as a service."); - } - } // See comments on HttpClientMappingRegistry. private static void ReserveClient(IHttpClientBuilder builder, Type type, string name, bool validateSingleType) @@ -763,7 +782,11 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string } } - private static HttpClientMappingRegistry GetMappingRegistry(IServiceCollection services) - => HttpClientFactoryServiceCollectionExtensions.GetMappingRegistry(services); + internal static HttpClientMappingRegistry GetMappingRegistry(this IServiceCollection services) + { + var registry = (HttpClientMappingRegistry?)services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance; + Debug.Assert(registry != null); + return registry; + } } } diff --git a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs index 239a7bd85254d2..a7c6ba236c8c98 100644 --- a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs @@ -63,7 +63,10 @@ public static IServiceCollection AddHttpClient(this IServiceCollection services) services.TryAddSingleton(new DefaultHttpClientConfigurationTracker()); // Register default client as HttpClient - TryAddEmptyNameHttpClient(services); + services.TryAddTransient(s => + { + return s.GetRequiredService().CreateClient(string.Empty); + }); return services; } @@ -831,34 +834,5 @@ public static IHttpClientBuilder AddHttpClient(this IS builder.AddTypedClient(factory); return builder; } - - internal static HttpClientMappingRegistry GetMappingRegistry(IServiceCollection services) - { - var registry = (HttpClientMappingRegistry?)services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance; - Debug.Assert(registry != null); - return registry; - } - - private static void TryAddEmptyNameHttpClient(IServiceCollection services) - { - HttpClientMappingRegistry mappingRegistry = GetMappingRegistry(services); - - if (mappingRegistry.EmptyNameHttpClientDescriptor is not null) - { - return; - } - - if (services.Any(sd => sd.ServiceType == typeof(HttpClient) && sd.ServiceKey is null)) - { - return; - } - - mappingRegistry.EmptyNameHttpClientDescriptor = ServiceDescriptor.Transient(s => - { - return s.GetRequiredService().CreateClient(string.Empty); - }); - - services.Add(mappingRegistry.EmptyNameHttpClientDescriptor); - } } } diff --git a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientMappingRegistry.cs b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientMappingRegistry.cs index 73ca788cd810a6..3c47458430f28a 100644 --- a/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientMappingRegistry.cs +++ b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientMappingRegistry.cs @@ -18,7 +18,5 @@ internal sealed class HttpClientMappingRegistry public Dictionary KeyedLifetimeMap { get; } = new(); public HttpClientKeyedLifetime? DefaultKeyedLifetime { get; set; } - - public ServiceDescriptor? EmptyNameHttpClientDescriptor { get; set; } } } diff --git a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DependencyInjection/HttpClientKeyedRegistrationTest.Descriptors.cs b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DependencyInjection/HttpClientKeyedRegistrationTest.Descriptors.cs index c5971939ddc8f7..7e814843de949e 100644 --- a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DependencyInjection/HttpClientKeyedRegistrationTest.Descriptors.cs +++ b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DependencyInjection/HttpClientKeyedRegistrationTest.Descriptors.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Net.Http; using Xunit; @@ -9,43 +8,6 @@ namespace Microsoft.Extensions.DependencyInjection { public partial class HttpClientKeyedRegistrationTest { - [Fact] - public void AddAsKeyed_EmptyNameHttpClientUpdated() // test for workaround for https://github.com/dotnet/runtime/issues/102654 - { - var serviceCollection = new ServiceCollection(); - - serviceCollection.AddHttpClient(); - - var emptyNameDescriptor = Assert.Single(serviceCollection, d => d.ServiceType == typeof(HttpClient)); - Assert.False(emptyNameDescriptor.IsKeyedService); - - serviceCollection.AddHttpClient(Test).AddAsKeyed(); - - emptyNameDescriptor = Assert.Single(serviceCollection, d => d.ServiceType == typeof(HttpClient) && !(d.ServiceKey is string s && s == Test)); - Assert.True(emptyNameDescriptor.IsKeyedService); - Assert.Equal(string.Empty, emptyNameDescriptor.ServiceKey); - } - - [Fact] - public void AddAsKeyed_NonFactoryHttpClientAdded_Throws() // test for workaround for https://github.com/dotnet/runtime/issues/102654 - { - var serviceCollection = new ServiceCollection(); - - serviceCollection.AddSingleton(new HttpClient()); - - var emptyNameDescriptor = Assert.Single(serviceCollection, d => d.ServiceType == typeof(HttpClient)); - Assert.False(emptyNameDescriptor.IsKeyedService); - Assert.Equal(ServiceLifetime.Singleton, emptyNameDescriptor.Lifetime); - - var builder = serviceCollection.AddHttpClient(Test); - - emptyNameDescriptor = Assert.Single(serviceCollection, d => d.ServiceType == typeof(HttpClient)); - Assert.False(emptyNameDescriptor.IsKeyedService); - Assert.Equal(ServiceLifetime.Singleton, emptyNameDescriptor.Lifetime); - - Assert.Throws(() => builder.AddAsKeyed()); - } - [Fact] public void AddAsKeyed_ScopedLifetime() { @@ -150,10 +112,10 @@ public void RemoveAsKeyed_PerName_AnyKeyDescriptorRemains() } private static bool IsKeyedClientDescriptor(ServiceDescriptor descriptor) - => descriptor.ServiceType == typeof(HttpClient) && descriptor.IsKeyedService && (descriptor.ServiceKey is not string name || name.Length > 0); + => descriptor.IsKeyedService && descriptor.ServiceType == typeof(HttpClient); private static bool IsKeyedHandlerDescriptor(ServiceDescriptor descriptor) - => descriptor.ServiceType == typeof(HttpMessageHandler) && descriptor.IsKeyedService && (descriptor.ServiceKey is not string name || name.Length > 0); + => descriptor.IsKeyedService && descriptor.ServiceType == typeof(HttpMessageHandler); private static void AssertSingleKeyedClientDescriptor(IServiceCollection services, ServiceLifetime lifetime, object key) {