fix extensions

ATI.Services.Consul/ATI.Services.Consul.csproj

@@ -17,7 +17,7 @@
   </PropertyGroup>
   <ItemGroup>
     <FrameworkReference Include="Microsoft.AspNetCore.App" />
-    <PackageReference Include="atisu.services.common" Version="16.0.0-httpClientHandlers-rc9" />
+    <PackageReference Include="atisu.services.common" Version="16.0.0-httpClientHandlers-rc10" />
     <PackageReference Include="Consul" Version="1.6.10.9" />
   </ItemGroup>
 </Project>

ATI.Services.Consul/Http/HttpClientBuilderExtensions.cs

@@ -1,10 +1,9 @@
 using System.Threading;
 using ATI.Services.Common.Options;
-using ATI.Services.Consul.Http;
 using JetBrains.Annotations;
 using Microsoft.Extensions.DependencyInjection;
 
-namespace PassportVerification.test;
+namespace ATI.Services.Consul.Http;
 
 [PublicAPI]
 public static class HttpClientBuilderExtensions
@@ -16,7 +15,9 @@ public static IHttpClientBuilder WithConsul<TServiceOptions>(this IHttpClientBui
 
         return httpClientBuilder
             .AddHttpMessageHandler<HttpConsulHandler<TServiceOptions>>()
-            // infinite handler because we don't want to wait until ConsulServiceAddress will recreated and cache objects every 2 minutes (default)
+            // By default, handlers are alive for 2 minutes
+            // If we don't set InfiniteTimeSpan, every 2 minutes HttpConsulHandler will be recreated
+            // And it will lead to new ConsulServiceAddress instances, which constructor is pretty expensive and will stop http requests for some time
             .SetHandlerLifetime(Timeout.InfiniteTimeSpan);
     }
 

ATI.Services.Consul/Http/ServiceCollectionHttpClientExtensions.cs

@@ -1,100 +1,64 @@
 using System;
-using System.Collections.Generic;
-using System.Linq;
 using ATI.Services.Common.Http.Extensions;
-using ATI.Services.Common.Logging;
 using ATI.Services.Common.Options;
 using ATI.Services.Common.Variables;
 using JetBrains.Annotations;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using NLog;
-using PassportVerification.test;
 using ConfigurationManager = ATI.Services.Common.Behaviors.ConfigurationManager;
 
-namespace ATI.Services.Common.Http;
+namespace ATI.Services.Consul.Http;
 
 [PublicAPI]
 public static class ServiceCollectionHttpClientExtensions
 {
-    private static readonly HashSet<string> RegisteredServiceNames = new ();
-
-    /// <summary>
-    /// Dynamically add all inheritors of BaseServiceOptions as AddConsulHttpClient<T>
-    /// </summary>
-    /// <param name="services"></param>
-    /// <returns></returns>
-    public static IServiceCollection AddConsulHttpClients(this IServiceCollection services)
-    {
-        var servicesOptionsTypes = AppDomain.CurrentDomain.GetAssemblies()
-            .SelectMany(assembly => assembly.GetTypes())
-            .Where(type => type.IsSubclassOf(typeof(BaseServiceOptions)));
-
-        foreach (var serviceOptionType in servicesOptionsTypes)
-        {
-            var method = typeof(ServiceCollectionHttpClientExtensions)
-                .GetMethod(nameof(AddConsulHttpClient), new[] { typeof(IServiceCollection) });
-            var generic = method.MakeGenericMethod(serviceOptionType);
-            generic.Invoke(null, new[] { services });
-        }
-
-        return services;
-    }
-
     /// <summary>
     /// Add HttpClient to HttpClientFactory with retry/cb/timeout policy
     /// Will add only if UseHttpClientFactory == true
     /// </summary>
     /// <param name="services"></param>
-    /// <typeparam name="T"></typeparam>
+    /// <typeparam name="TAdapter">Type of the http adapter for typed HttpClient</typeparam>
+    /// <typeparam name="TServiceOptions"></typeparam>
     /// <returns></returns>s
-    public static IServiceCollection AddConsulHttpClient<T>(this IServiceCollection services) where T : BaseServiceOptions
+    public static IServiceCollection AddConsulHttpClient<TAdapter, TServiceOptions>(this IServiceCollection services) 
+        where TAdapter : class
+        where TServiceOptions : BaseServiceOptions
     {
-        var className = typeof(T).Name;
-        var settings = ConfigurationManager.GetSection(className).Get<T>();
+        var className = typeof(TServiceOptions).Name;
+        var settings = ConfigurationManager.GetSection(className).Get<TServiceOptions>();
         if (settings == null)
         {
             throw new Exception($"Cannot find section for {className}");
         }
-
-        var serviceName = settings.ServiceName;
-
-        var logger = LogManager.GetLogger(serviceName);
 
-        if (!settings.UseHttpClientFactory || string.IsNullOrEmpty(settings.ConsulName))
+        if (string.IsNullOrEmpty(settings.ConsulName))
         {
-            logger.WarnWithObject($"Class ${className} has UseHttpClientFactory == false OR ConsulName == null while AddCustomHttpClient");
-            return services;
+            throw new Exception($"Class {className} has ConsulName == null while AddConsulHttpClient");
         }
 
-        // Each HttpClient must be added only once, otherwise we will get exceptions like 	System.InvalidOperationException: The 'InnerHandler' property must be null. 'DelegatingHandler' instances provided to 'HttpMessageHandlerBuilder' must not be reused or cached.
-        // Handler: 'ATI.Services.Consul.Http.HttpConsulHandler
-        // Possible reason - because HttpConsulHandler is singleton (not transient)
-        // https://stackoverflow.com/questions/77542613/the-innerhandler-property-must-be-null-delegatinghandler-instances-provided
-        if (RegisteredServiceNames.Contains(serviceName))
-        {
-            logger.WarnWithObject($"Class ${className} is already registered");
-            return services;
-        }
-
+        var serviceName = settings.ServiceName;
+        var logger = LogManager.GetLogger(serviceName);
+
         var serviceVariablesOptions = ConfigurationManager.GetSection(nameof(ServiceVariablesOptions)).Get<ServiceVariablesOptions>();
 
-        services.AddHttpClient(serviceName, httpClient =>
+        services.AddHttpClient<TAdapter>(httpClient =>
             {
                 // We will override this url by consul, but we need to set it, otherwise we will get exception because HttpRequestMessage doesn't have baseUrl (only relative)
                 httpClient.BaseAddress = new Uri("http://localhost");
                 httpClient.SetBaseFields(serviceVariablesOptions.GetServiceAsClientName(), serviceVariablesOptions.GetServiceAsClientHeaderName(),  settings.AdditionalHeaders);
             })
-            .WithLogging<T>()
-            .WithProxyFields<T>()
+            .WithLogging<TServiceOptions>()
+            .WithProxyFields<TServiceOptions>()
             .AddRetryPolicy(settings, logger)
             // Get new instance url for each retry (because 1 instance can be down)
-            .WithConsul<T>()
+            .WithConsul<TServiceOptions>()
             .AddHostSpecificCircuitBreakerPolicy(settings, logger)
             .AddTimeoutPolicy(settings.TimeOut)
-            .WithMetrics<T>();
-
-        RegisteredServiceNames.Add(serviceName);
+            .WithMetrics<TServiceOptions>();
+        // we don't override PooledConnectionLifetime even we use HttpClient in static TAdapter
+        // because we are getting new host from consul for each request
+        // https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines
 
         return services;
     }

ATI.Services.Consul/README.md

@@ -39,8 +39,7 @@
 ### Http
 
 За основу взята работа с `HttClientFactory` из `atisu.services.common`, но добавлены следующие extensions:
-1. `services.AddConsulHttpClient<XServiceOptions>`
-2. `services.AddConsulHttpClients()` - он автоматически соберет из проекта всех наследников `BaseServiceOptions`, где `ConsulName не NULL и UseHttpClientFactory = true` и для них сделает вызов `services.AddConsulHttpClient<>()` 
+1. `services.AddConsulHttpClient<TAdapter, TServiceOptions>`
 
 Методы делают почти все то же самое, что и `services.AddCustomHttpClient<>`, но дополнительно:
 1. Добавляется `ServiceAsClientName` хедер во все запросы