From 32df3c3d26b0b361c7cb9ffbd6063d639b4d42fe Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 27 Oct 2023 10:02:49 -0500 Subject: [PATCH] redo of #769 to address #772 --- .../Configuration/FlurlClientBuilder.cs | 91 ++++++++++--------- .../Configuration/FlurlClientCache.cs | 5 +- .../Configuration/FlurlClientFactory.cs | 27 ++++-- src/Flurl.Http/FlurlHttp.cs | 5 +- src/Flurl.Http/FlurlHttpException.cs | 11 +++ .../Http/FlurlClientBuilderTests.cs | 27 +++++- 6 files changed, 105 insertions(+), 61 deletions(-) diff --git a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs index 1459f633..8c05ed54 100644 --- a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs +++ b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Net.Http; +using System.Runtime.Versioning; namespace Flurl.Http.Configuration { @@ -21,12 +22,19 @@ public interface IFlurlClientBuilder IFlurlClientBuilder ConfigureHttpClient(Action configure); /// - /// Configure the inner-most HttpMessageHandler associated with this IFlurlClient. + /// Configure the inner-most HttpMessageHandler (an instance of HttpClientHandler) associated with this IFlurlClient. /// -#if NETCOREAPP2_1_OR_GREATER - IFlurlClientBuilder ConfigureInnerHandler(Action configure); -#else IFlurlClientBuilder ConfigureInnerHandler(Action configure); + +#if NET + /// + /// Configure a SocketsHttpHandler instead of HttpClientHandler as the inner-most HttpMessageHandler. + /// Note that HttpClientHandler has broader platform support and defers its work to SocketsHttpHandler + /// on supported platforms. It is recommended to explicitly use SocketsHttpHandler ONLY if you + /// need to directly configure its properties that aren't available on HttpClientHandler. + /// + [UnsupportedOSPlatform("browser")] + IFlurlClientBuilder UseSocketsHttpHandler(Action configure); #endif /// @@ -45,33 +53,24 @@ public interface IFlurlClientBuilder /// public class FlurlClientBuilder : IFlurlClientBuilder { - private readonly IFlurlClientFactory _factory; + private IFlurlClientFactory _factory = new DefaultFlurlClientFactory(); + private readonly string _baseUrl; private readonly List> _addMiddleware = new(); - private readonly List> _configSettings = new(); - private readonly List> _configClient = new(); -#if NETCOREAPP2_1_OR_GREATER - private readonly HandlerBuilder _handlerBuilder = new(); -#else - private readonly HandlerBuilder _handlerBuilder = new(); -#endif + private readonly List> _settingsConfigs = new(); + private readonly List> _clientConfigs = new(); + private readonly List> _handlerConfigs = new(); /// /// Creates a new FlurlClientBuilder. /// - public FlurlClientBuilder(string baseUrl = null) : this(new DefaultFlurlClientFactory(), baseUrl) { } - - /// - /// Creates a new FlurlClientBuilder. - /// - internal FlurlClientBuilder(IFlurlClientFactory factory, string baseUrl) { - _factory = factory; + public FlurlClientBuilder(string baseUrl = null) { _baseUrl = baseUrl; } /// public IFlurlClientBuilder WithSettings(Action configure) { - _configSettings.Add(configure); + _settingsConfigs.Add(configure); return this; } @@ -83,23 +82,42 @@ public IFlurlClientBuilder AddMiddleware(Func create) { /// public IFlurlClientBuilder ConfigureHttpClient(Action configure) { - _configClient.Add(configure); + _clientConfigs.Add(configure); return this; } /// -#if NETCOREAPP2_1_OR_GREATER - public IFlurlClientBuilder ConfigureInnerHandler(Action configure) { -#else public IFlurlClientBuilder ConfigureInnerHandler(Action configure) { +#if NET + if (_factory is SocketsHandlerFlurlClientFactory && _handlerConfigs.Any()) + throw new FlurlConfigurationException("ConfigureInnerHandler and UseSocketsHttpHandler cannot be used together. The former configures and instance of HttpClientHandler and would be ignored when switching to SocketsHttpHandler."); #endif - _handlerBuilder.Configs.Add(configure); + _handlerConfigs.Add(h => configure(h as HttpClientHandler)); + return this; + } + +#if NET + /// + public IFlurlClientBuilder UseSocketsHttpHandler(Action configure) { + if (!SocketsHttpHandler.IsSupported) + throw new PlatformNotSupportedException("SocketsHttpHandler is not supported on one or more target platforms."); + + if (_factory is DefaultFlurlClientFactory && _handlerConfigs.Any()) + throw new FlurlConfigurationException("ConfigureInnerHandler and UseSocketsHttpHandler cannot be used together. The former configures and instance of HttpClientHandler and would be ignored when switching to SocketsHttpHandler."); + + if (!(_factory is SocketsHandlerFlurlClientFactory)) + _factory = new SocketsHandlerFlurlClientFactory(); + + _handlerConfigs.Add(h => configure(h as SocketsHttpHandler)); return this; } +#endif /// public IFlurlClient Build() { - var outerHandler = _handlerBuilder.Build(_factory); + var outerHandler = _factory.CreateInnerHandler(); + foreach (var config in _handlerConfigs) + config(outerHandler); foreach (var middleware in Enumerable.Reverse(_addMiddleware).Select(create => create())) { middleware.InnerHandler = outerHandler; @@ -107,31 +125,14 @@ public IFlurlClient Build() { } var httpCli = _factory.CreateHttpClient(outerHandler); - foreach (var config in _configClient) + foreach (var config in _clientConfigs) config(httpCli); var flurlCli = new FlurlClient(httpCli, _baseUrl); - foreach (var config in _configSettings) + foreach (var config in _settingsConfigs) config(flurlCli.Settings); return flurlCli; } - - // helper class to keep those compiler switches from getting too messy - private class HandlerBuilder where T : HttpMessageHandler - { - public List> Configs { get; } = new(); - - public HttpMessageHandler Build(IFlurlClientFactory fac) { - var handler = fac.CreateInnerHandler(); - foreach (var config in Configs) { - if (handler is T h) - config(h); - else - throw new Exception($"ConfigureInnerHandler expected an instance of {typeof(T).Name} but received an instance of {handler.GetType().Name}."); - } - return handler; - } - } } } diff --git a/src/Flurl.Http/Configuration/FlurlClientCache.cs b/src/Flurl.Http/Configuration/FlurlClientCache.cs index 5da8da86..8fac043a 100644 --- a/src/Flurl.Http/Configuration/FlurlClientCache.cs +++ b/src/Flurl.Http/Configuration/FlurlClientCache.cs @@ -45,7 +45,6 @@ public interface IFlurlClientCache /// public class FlurlClientCache : IFlurlClientCache { private readonly ConcurrentDictionary> _clients = new(); - private readonly IFlurlClientFactory _factory = new DefaultFlurlClientFactory(); private Action _configureAll; /// @@ -53,7 +52,7 @@ public IFlurlClientBuilder Add(string name, string baseUrl = null) { if (_clients.ContainsKey(name)) throw new ArgumentException($"A client named '{name}' was already registered with this factory. AddClient should be called just once per client at startup."); - var builder = new FlurlClientBuilder(_factory, baseUrl); + var builder = new FlurlClientBuilder(baseUrl); _clients[name] = CreateLazyInstance(builder); return builder; } @@ -63,7 +62,7 @@ public virtual IFlurlClient Get(string name) { if (name == null) throw new ArgumentNullException(nameof(name)); - Lazy Create() => CreateLazyInstance(new FlurlClientBuilder(_factory, null)); + Lazy Create() => CreateLazyInstance(new FlurlClientBuilder()); return _clients.AddOrUpdate(name, _ => Create(), (_, existing) => existing.Value.IsDisposed ? Create() : existing).Value; } diff --git a/src/Flurl.Http/Configuration/FlurlClientFactory.cs b/src/Flurl.Http/Configuration/FlurlClientFactory.cs index 2c27bac5..ab4c16c1 100644 --- a/src/Flurl.Http/Configuration/FlurlClientFactory.cs +++ b/src/Flurl.Http/Configuration/FlurlClientFactory.cs @@ -51,13 +51,6 @@ public virtual HttpClient CreateHttpClient(HttpMessageHandler handler) { /// public virtual HttpMessageHandler CreateInnerHandler() { // Flurl has its own mechanisms for managing cookies and redirects, so we need to disable them in the inner handler. -#if NETCOREAPP2_1_OR_GREATER - var handler = new SocketsHttpHandler { - UseCookies = false, - AllowAutoRedirect = false, - AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate - }; -#else var handler = new HttpClientHandler(); if (handler.SupportsRedirectConfiguration) @@ -70,8 +63,26 @@ public virtual HttpMessageHandler CreateInnerHandler() { try { handler.UseCookies = false; } catch (PlatformNotSupportedException) { } // look out for WASM platforms (#543) -#endif + return handler; } } + +#if NET + /// + /// An implementation of IFlurlClientFactory that uses SocketsHttpHandler on supported platforms. + /// + public class SocketsHandlerFlurlClientFactory : DefaultFlurlClientFactory + { + /// + /// Creates and configures a new SocketsHttpHandler as needed when a new IFlurlClient instance is created. + /// + public override HttpMessageHandler CreateInnerHandler() => new SocketsHttpHandler { + // Flurl has its own mechanisms for managing cookies and redirects, so we need to disable them in the inner handler. + UseCookies = false, + AllowAutoRedirect = false, + AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate + }; + } +#endif } diff --git a/src/Flurl.Http/FlurlHttp.cs b/src/Flurl.Http/FlurlHttp.cs index 937f5dd4..e7e048f5 100644 --- a/src/Flurl.Http/FlurlHttp.cs +++ b/src/Flurl.Http/FlurlHttp.cs @@ -26,7 +26,10 @@ public static class FlurlHttp /// /// Gets or creates the IFlurlClient that would be selected for sending the given IFlurlRequest when the clientless pattern is used. /// - public static IFlurlClient GetClientForRequest(IFlurlRequest req) => Clients.Get(_cachingStrategy(req)); + public static IFlurlClient GetClientForRequest(IFlurlRequest req) { + throw new Exception("here"); + //return Clients.Get(_cachingStrategy(req)); + } /// /// Sets a global caching strategy for getting or creating an IFlurlClient instance when the clientless pattern is used, e.g. url.GetAsync. diff --git a/src/Flurl.Http/FlurlHttpException.cs b/src/Flurl.Http/FlurlHttpException.cs index 8dbfbb45..2d6ff49b 100644 --- a/src/Flurl.Http/FlurlHttpException.cs +++ b/src/Flurl.Http/FlurlHttpException.cs @@ -106,4 +106,15 @@ private static string BuildMessage(FlurlCall call, string expectedFormat) { return msg + ((call == null) ? "." : $": {call}"); } } + + /// + /// An exception that is thrown when Flurl.Http has been misconfigured. + /// + public class FlurlConfigurationException : Exception + { + /// + /// Initializes a new instance of the class. + /// + public FlurlConfigurationException(string message) : base(message) { } + } } \ No newline at end of file diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs index 1fe97be5..fba59818 100644 --- a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -51,18 +51,37 @@ public async Task can_add_middleware() { } [Test] - public void inner_hanlder_is_SocketsHttpHandler_when_supported() { + public void uses_HttpClientHandler_by_default() { HttpMessageHandler handler = null; new FlurlClientBuilder() .ConfigureInnerHandler(h => handler = h) .Build(); + Assert.IsInstanceOf(handler); + } + #if NET + [Test] + public void can_use_SocketsHttpHandler() { + HttpMessageHandler handler = null; + new FlurlClientBuilder() + .UseSocketsHttpHandler(h => handler = h) + .Build(); Assert.IsInstanceOf(handler); -#else - Assert.IsInstanceOf(handler); -#endif } + [Test] + public void cannot_mix_handler_types() { + Assert.Throws(() => new FlurlClientBuilder() + .ConfigureInnerHandler(_ => { }) + .UseSocketsHttpHandler(_ => { })); + + // reverse + Assert.Throws(() => new FlurlClientBuilder() + .UseSocketsHttpHandler(_ => { }) + .ConfigureInnerHandler(_ => { })); + } +#endif + class BlockingHandler : DelegatingHandler { private readonly string _msg;