From a52ca82f379c55d51909a037edc9079b01034bd3 Mon Sep 17 00:00:00 2001 From: Todd Date: Wed, 18 Oct 2023 13:14:27 -0500 Subject: [PATCH 1/9] #770 configuration overhaul --- .../DefaultFlurlClientFactory.cs | 23 ---- .../Configuration/FlurlClientBuilder.cs | 117 ++++++++++++++++++ .../Configuration/FlurlClientCache.cs | 94 ++++++++++++++ .../Configuration/FlurlClientFactory.cs | 79 ++++++++++++ .../Configuration/FlurlClientFactoryBase.cs | 96 -------------- .../Configuration/FlurlHttpSettings.cs | 89 +++++-------- .../Configuration/IFlurlClientFactory.cs | 66 ---------- .../PerBaseUrlFlurlClientFactory.cs | 31 ----- .../Content/CapturedMultipartContent.cs | 22 ++-- src/Flurl.Http/FlurlClient.cs | 18 +-- src/Flurl.Http/FlurlHttp.cs | 47 ++++--- src/Flurl.Http/FlurlRequest.cs | 6 +- src/Flurl.Http/SettingsExtensions.cs | 2 +- src/Flurl.Http/Testing/HttpCallAssertion.cs | 12 +- src/Flurl.Http/Testing/HttpTest.cs | 14 +-- test/Flurl.Test/Http/CookieTests.cs | 2 +- .../Http/FlurlClientBuilderTests.cs | 68 ++++++++++ test/Flurl.Test/Http/FlurlClientCacheTests.cs | 90 ++++++++++++++ .../Http/FlurlClientFactoryTests.cs | 80 ------------ test/Flurl.Test/Http/GlobalConfigTests.cs | 78 ++++++++++++ test/Flurl.Test/Http/MultipartTests.cs | 5 +- .../Http/SettingsExtensionsTests.cs | 2 +- test/Flurl.Test/Http/SettingsTests.cs | 113 +---------------- test/Flurl.Test/Http/TestingTests.cs | 2 +- 24 files changed, 633 insertions(+), 523 deletions(-) delete mode 100644 src/Flurl.Http/Configuration/DefaultFlurlClientFactory.cs create mode 100644 src/Flurl.Http/Configuration/FlurlClientBuilder.cs create mode 100644 src/Flurl.Http/Configuration/FlurlClientCache.cs create mode 100644 src/Flurl.Http/Configuration/FlurlClientFactory.cs delete mode 100644 src/Flurl.Http/Configuration/FlurlClientFactoryBase.cs delete mode 100644 src/Flurl.Http/Configuration/IFlurlClientFactory.cs delete mode 100644 src/Flurl.Http/Configuration/PerBaseUrlFlurlClientFactory.cs create mode 100644 test/Flurl.Test/Http/FlurlClientBuilderTests.cs create mode 100644 test/Flurl.Test/Http/FlurlClientCacheTests.cs delete mode 100644 test/Flurl.Test/Http/FlurlClientFactoryTests.cs create mode 100644 test/Flurl.Test/Http/GlobalConfigTests.cs diff --git a/src/Flurl.Http/Configuration/DefaultFlurlClientFactory.cs b/src/Flurl.Http/Configuration/DefaultFlurlClientFactory.cs deleted file mode 100644 index f7899157..00000000 --- a/src/Flurl.Http/Configuration/DefaultFlurlClientFactory.cs +++ /dev/null @@ -1,23 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Flurl.Http.Configuration -{ - /// - /// An IFlurlClientFactory implementation that caches and reuses the same one instance of - /// FlurlClient per combination of scheme, host, and port. This is the default - /// implementation used when calls are made fluently off Urls/strings. - /// - public class DefaultFlurlClientFactory : FlurlClientFactoryBase - { - /// - /// Returns a unique cache key based on scheme, host, and port of the given URL. - /// - /// The URL. - /// The cache key - protected override string GetCacheKey(Url url) => $"{url.Scheme}|{url.Host}|{url.Port}"; - } -} \ No newline at end of file diff --git a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs new file mode 100644 index 00000000..a80a9605 --- /dev/null +++ b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs @@ -0,0 +1,117 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net.Http; + +namespace Flurl.Http.Configuration +{ + /// + /// A builder for configuring IFlurlClient instances. + /// + public interface IFlurlClientBuilder + { + /// + /// Configure the IFlurlClient's Settings. + /// + IFlurlClientBuilder WithSettings(Action configAction); + + /// + /// Configure the HttpClient wrapped by this IFlurlClient. + /// + IFlurlClientBuilder ConfigureHttpClient(Action configAction); + + /// + /// Configure the inner-most HttpMessageHandler associated with this IFlurlClient. + /// + IFlurlClientBuilder ConfigureInnerHandler(Action configAction); + + /// + /// Add a provided DelegatingHandler to the IFlurlClient. + /// + IFlurlClientBuilder AddMiddleware(Func create); + + /// + /// Builds an instance of IFlurlClient based on configurations specified. + /// + IFlurlClient Build(); + } + + /// + /// Default implementation of IFlurlClientBuilder. + /// + public class FlurlClientBuilder : IFlurlClientBuilder + { + private readonly IFlurlClientFactory _factory; + private readonly string _baseUrl; + private readonly List> _addMiddleware = new(); + private readonly List> _configClient = new(); + private readonly List> _configHandler = new(); + private readonly List> _configSettings = 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; + _baseUrl = baseUrl; + } + + /// + public IFlurlClientBuilder WithSettings(Action configAction) { + _configSettings.Add(configAction); + return this; + } + + /// + public IFlurlClientBuilder AddMiddleware(Func create) { + _addMiddleware.Add(create); + return this; + } + + /// + public IFlurlClientBuilder ConfigureHttpClient(Action configAction) { + _configClient.Add(configAction); + return this; + } + + /// + public IFlurlClientBuilder ConfigureInnerHandler(Action configAction) { + _configHandler.Add(configAction); + return this; + } + + /// + public IFlurlClient Build() { + var innerHandler = _factory.CreateInnerHandler(); + foreach (var config in _configHandler) { + if (innerHandler is HttpClientHandler hch) + config(hch); + else + throw new Exception("ConfigureInnerHandler can only be used when IFlurlClientFactory.CreateInnerHandler returns an instance of HttpClientFactory."); + } + + HttpMessageHandler outerHandler = innerHandler; + foreach (var createMW in Enumerable.Reverse(_addMiddleware)) { + var middleware = createMW(); + middleware.InnerHandler = outerHandler; + outerHandler = middleware; + } + + var httpCli = _factory.CreateHttpClient(outerHandler); + foreach (var config in _configClient) + config(httpCli); + + var flurlCli = new FlurlClient(httpCli, _baseUrl); + foreach (var config in _configSettings) { + config(flurlCli.Settings); + } + + return flurlCli; + } + } +} diff --git a/src/Flurl.Http/Configuration/FlurlClientCache.cs b/src/Flurl.Http/Configuration/FlurlClientCache.cs new file mode 100644 index 00000000..5da8da86 --- /dev/null +++ b/src/Flurl.Http/Configuration/FlurlClientCache.cs @@ -0,0 +1,94 @@ +using System; +using System.Collections.Concurrent; + +namespace Flurl.Http.Configuration +{ + /// + /// Interface for a cache of IFlurlClient instances. + /// + public interface IFlurlClientCache + { + /// + /// Adds and returns a new IFlurlClient. Call once per client at startup to register and configure a named client. + /// + /// Name of the IFlurlClient. Serves as a cache key. Subsequent calls to Get will return this client. + /// Optional. The base URL associated with the new client. + /// + IFlurlClientBuilder Add(string name, string baseUrl = null); + + /// + /// Gets a named IFlurlClient, creating one if it doesn't exist or has been disposed. + /// + /// The client name. + /// + IFlurlClient Get(string name); + + /// + /// Configuration logic that gets executed for every new IFlurlClient added this case. Good place for things like default + /// settings. Executes before client-specific builder logic. + /// + IFlurlClientCache ConfigureAll(Action configure); + + /// + /// Removes a named client from this cache. + /// + void Remove(string name); + + /// + /// Disposes and removes all cached IFlurlClient instances. + /// + void Clear(); + } + + /// + /// Default implementation of IFlurlClientCache. + /// + public class FlurlClientCache : IFlurlClientCache { + private readonly ConcurrentDictionary> _clients = new(); + private readonly IFlurlClientFactory _factory = new DefaultFlurlClientFactory(); + private Action _configureAll; + + /// + 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); + _clients[name] = CreateLazyInstance(builder); + return builder; + } + + /// + public virtual IFlurlClient Get(string name) { + if (name == null) + throw new ArgumentNullException(nameof(name)); + + Lazy Create() => CreateLazyInstance(new FlurlClientBuilder(_factory, null)); + return _clients.AddOrUpdate(name, _ => Create(), (_, existing) => existing.Value.IsDisposed ? Create() : existing).Value; + } + + private Lazy CreateLazyInstance(FlurlClientBuilder builder) { + _configureAll?.Invoke(builder); + return new Lazy(builder.Build); + } + + /// + public IFlurlClientCache ConfigureAll(Action configure) { + _configureAll = configure; + return this; + } + + /// + public void Remove(string name) { + if (_clients.TryRemove(name, out var cli) && cli.IsValueCreated && !cli.Value.IsDisposed) + cli.Value.Dispose(); + } + + /// + public void Clear() { + // Remove takes care of disposing too, which is why we don't simply call _clients.Clear + foreach (var key in _clients.Keys) + Remove(key); + } + } +} diff --git a/src/Flurl.Http/Configuration/FlurlClientFactory.cs b/src/Flurl.Http/Configuration/FlurlClientFactory.cs new file mode 100644 index 00000000..bae9faf7 --- /dev/null +++ b/src/Flurl.Http/Configuration/FlurlClientFactory.cs @@ -0,0 +1,79 @@ +using System; +using System.Net; +using System.Net.Http; + +namespace Flurl.Http.Configuration +{ + /// + /// Interface for methods used to build and cache IFlurlClient instances. + /// + public interface IFlurlClientFactory + { + /// + /// Defines how HttpClient should be instantiated and configured by default. Do NOT attempt + /// to cache/reuse HttpClient instances here - that should be done at the FlurlClient level + /// via a custom FlurlClientFactory that gets registered globally. + /// + /// The HttpMessageHandler used to construct the HttpClient. + /// + HttpClient CreateHttpClient(HttpMessageHandler handler); + + /// + /// Defines how the HttpMessageHandler used by HttpClients that are created by + /// this factory should be instantiated and configured. + /// + /// + HttpMessageHandler CreateInnerHandler(); + } + + /// + /// Extension methods on IFlurlClientFactory + /// + public static class FlurlClientFactoryExtensions + { + /// + /// Creates an HttpClient with the HttpMessageHandler returned from this factory's CreateInnerHandler method. + /// + public static HttpClient CreateHttpClient(this IFlurlClientFactory fac) => fac.CreateHttpClient(fac.CreateInnerHandler()); + } + + /// + /// Default implementation of IFlurlClientFactory, used to build and cache IFlurlClient instances. + /// + public class DefaultFlurlClientFactory : IFlurlClientFactory + { + /// + /// Override in custom factory to customize the creation of HttpClient used in all Flurl HTTP calls + /// (except when one is passed explicitly to the FlurlClient constructor). In order not to lose + /// Flurl.Http functionality, it is recommended to call base.CreateClient and customize the result. + /// + public virtual HttpClient CreateHttpClient(HttpMessageHandler handler) { + return new HttpClient(handler); + } + + /// + /// Override in custom factory to customize the creation of the top-level HttpMessageHandler used in all + /// Flurl HTTP calls (except when an HttpClient is passed explicitly to the FlurlClient constructor). + /// In order not to lose Flurl.Http functionality, it is recommended to call base.CreateMessageHandler, and + /// either customize the returned HttpClientHandler, or set it as the InnerHandler of a DelegatingHandler. + /// + public virtual HttpMessageHandler CreateInnerHandler() { + var httpClientHandler = new HttpClientHandler(); + + // flurl has its own mechanisms for managing cookies and redirects + + try { httpClientHandler.UseCookies = false; } + catch (PlatformNotSupportedException) { } // look out for WASM platforms (#543) + + if (httpClientHandler.SupportsRedirectConfiguration) + httpClientHandler.AllowAutoRedirect = false; + + if (httpClientHandler.SupportsAutomaticDecompression) { + // #266 + // deflate not working? see #474 + httpClientHandler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; + } + return httpClientHandler; + } + } +} diff --git a/src/Flurl.Http/Configuration/FlurlClientFactoryBase.cs b/src/Flurl.Http/Configuration/FlurlClientFactoryBase.cs deleted file mode 100644 index ab158d0d..00000000 --- a/src/Flurl.Http/Configuration/FlurlClientFactoryBase.cs +++ /dev/null @@ -1,96 +0,0 @@ -using System; -using System.Collections.Concurrent; -using System.Net.Http; -using System.Net; - -namespace Flurl.Http.Configuration -{ - /// - /// Encapsulates a creation/caching strategy for IFlurlClient instances. Custom factories looking to extend - /// Flurl's behavior should inherit from this class, rather than implementing IFlurlClientFactory directly. - /// - public abstract class FlurlClientFactoryBase : IFlurlClientFactory - { - private readonly ConcurrentDictionary _clients = new ConcurrentDictionary(); - - /// - /// By default, uses a caching strategy of one FlurlClient per host. This maximizes reuse of - /// underlying HttpClient/Handler while allowing things like cookies to be host-specific. - /// - /// The URL. - /// The FlurlClient instance. - public virtual IFlurlClient Get(Url url) { - if (url == null) - throw new ArgumentNullException(nameof(url)); - - return _clients.AddOrUpdate( - GetCacheKey(url), - u => Create(u), - (u, client) => client.IsDisposed ? Create(u) : client); - } - - /// - /// Defines a strategy for getting a cache key based on a Url. Default implementation - /// returns the host part (i.e www.api.com) so that all calls to the same host use the - /// same FlurlClient (and HttpClient/HttpMessageHandler) instance. - /// - /// The URL. - /// The cache key - protected abstract string GetCacheKey(Url url); - - /// - /// Creates a new FlurlClient - /// - /// The URL (not used) - /// - protected virtual IFlurlClient Create(Url url) => new FlurlClient(); - - /// - /// Disposes all cached IFlurlClient instances and clears the cache. - /// - public void Dispose() { - foreach (var kv in _clients) { - if (!kv.Value.IsDisposed) - kv.Value.Dispose(); - } - _clients.Clear(); - } - - /// - /// Override in custom factory to customize the creation of HttpClient used in all Flurl HTTP calls - /// (except when one is passed explicitly to the FlurlClient constructor). In order not to lose - /// Flurl.Http functionality, it is recommended to call base.CreateClient and customize the result. - /// - public virtual HttpClient CreateHttpClient(HttpMessageHandler handler) { - return new HttpClient(handler) { - // Timeouts handled per request via FlurlHttpSettings.Timeout - Timeout = System.Threading.Timeout.InfiniteTimeSpan - }; - } - - /// - /// Override in custom factory to customize the creation of the top-level HttpMessageHandler used in all - /// Flurl HTTP calls (except when an HttpClient is passed explicitly to the FlurlClient constructor). - /// In order not to lose Flurl.Http functionality, it is recommended to call base.CreateMessageHandler, and - /// either customize the returned HttpClientHandler, or set it as the InnerHandler of a DelegatingHandler. - /// - public virtual HttpMessageHandler CreateMessageHandler() { - var httpClientHandler = new HttpClientHandler(); - - // flurl has its own mechanisms for managing cookies and redirects - - try { httpClientHandler.UseCookies = false; } - catch (PlatformNotSupportedException) { } // look out for WASM platforms (#543) - - if (httpClientHandler.SupportsRedirectConfiguration) - httpClientHandler.AllowAutoRedirect = false; - - if (httpClientHandler.SupportsAutomaticDecompression) { - // #266 - // deflate not working? see #474 - httpClientHandler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; - } - return httpClientHandler; - } - } -} diff --git a/src/Flurl.Http/Configuration/FlurlHttpSettings.cs b/src/Flurl.Http/Configuration/FlurlHttpSettings.cs index f763284f..6ac730b9 100644 --- a/src/Flurl.Http/Configuration/FlurlHttpSettings.cs +++ b/src/Flurl.Http/Configuration/FlurlHttpSettings.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Threading.Tasks; using System.Runtime.CompilerServices; @@ -11,11 +11,23 @@ namespace Flurl.Http.Configuration /// public class FlurlHttpSettings { + private static readonly FlurlHttpSettings Defaults = new() { + Timeout = TimeSpan.FromSeconds(100), // same as HttpClient + HttpVersion = "1.1", + JsonSerializer = new DefaultJsonSerializer(), + UrlEncodedSerializer = new DefaultUrlEncodedSerializer(), + Redirects = { + Enabled = true, + AllowSecureToInsecure = false, + ForwardHeaders = false, + ForwardAuthorizationHeader = false, + MaxAutoRedirects = 10 + } + }; + // Values are dictionary-backed so we can check for key existence. Can't do null-coalescing // because if a setting is set to null at the request level, that should stick. - private readonly IDictionary _vals = new Dictionary(); - - private FlurlHttpSettings _defaults; + private IDictionary _vals = new Dictionary(); /// /// Creates a new FlurlHttpSettings object. @@ -28,10 +40,7 @@ public FlurlHttpSettings() { /// /// Gets or sets the default values to fall back on when values are not explicitly set on this instance. /// - public virtual FlurlHttpSettings Defaults { - get => _defaults ?? FlurlHttp.GlobalSettings; - set => _defaults = value; - } + internal FlurlHttpSettings Parent { get; set; } /// /// Gets or sets the HTTP request timeout. @@ -154,7 +163,7 @@ public Func OnRedirectAsync { /// Resets all overridden settings to their default values. For example, on a FlurlRequest, /// all settings are reset to FlurlClient-level settings. /// - public virtual void ResetDefaults() { + public void ResetDefaults() { _vals.Clear(); } @@ -162,12 +171,18 @@ public virtual void ResetDefaults() { /// Gets a settings value from this instance if explicitly set, otherwise from the default settings that back this instance. /// internal T Get([CallerMemberName]string propName = null) { - var testVals = HttpTest.Current?.Settings._vals; - return - testVals?.ContainsKey(propName) == true ? (T)testVals[propName] : - _vals.ContainsKey(propName) ? (T)_vals[propName] : - Defaults != null ? (T)Defaults.Get(propName) : - default; + IEnumerable prioritize() { + yield return HttpTest.Current?.Settings; + yield return this; + yield return Parent; + yield return Defaults; + } + + foreach (var settings in prioritize()) + if (settings?._vals?.TryGetValue(propName, out var val) == true) + return (T)val; + + return default; // should never get this far assuming Defaults is fully populated } /// @@ -177,48 +192,4 @@ internal void Set(T value, [CallerMemberName]string propName = null) { _vals[propName] = value; } } - - /// - /// Global default settings for Flurl.Http - /// - public class GlobalFlurlHttpSettings : FlurlHttpSettings - { - internal GlobalFlurlHttpSettings() { - ResetDefaults(); - } - - /// - /// Defaults at the global level do not make sense and will always be null. - /// - public override FlurlHttpSettings Defaults { - get => null; - set => throw new Exception("Global settings cannot be backed by any higher-level defaults."); - } - - /// - /// Gets or sets the factory that defines creating, caching, and reusing FlurlClient instances and, - /// by proxy, HttpClient instances. - /// - public IFlurlClientFactory FlurlClientFactory { - get => Get(); - set => Set(value); - } - - /// - /// Resets all global settings to their default values. - /// - public override void ResetDefaults() { - base.ResetDefaults(); - Timeout = TimeSpan.FromSeconds(100); // same as HttpClient - HttpVersion = "1.1"; - JsonSerializer = new DefaultJsonSerializer(); - UrlEncodedSerializer = new DefaultUrlEncodedSerializer(); - FlurlClientFactory = new DefaultFlurlClientFactory(); - Redirects.Enabled = true; - Redirects.AllowSecureToInsecure = false; - Redirects.ForwardHeaders = false; - Redirects.ForwardAuthorizationHeader = false; - Redirects.MaxAutoRedirects = 10; - } - } } diff --git a/src/Flurl.Http/Configuration/IFlurlClientFactory.cs b/src/Flurl.Http/Configuration/IFlurlClientFactory.cs deleted file mode 100644 index 49805848..00000000 --- a/src/Flurl.Http/Configuration/IFlurlClientFactory.cs +++ /dev/null @@ -1,66 +0,0 @@ -using System; -using System.Net.Http; -using System.Runtime.CompilerServices; - -namespace Flurl.Http.Configuration -{ - /// - /// Interface for defining a strategy for creating, caching, and reusing IFlurlClient instances and - /// their underlying HttpClient instances. It is generally preferable to derive from FlurlClientFactoryBase - /// and only override methods as needed, rather than implementing this interface from scratch. - /// - public interface IFlurlClientFactory : IDisposable - { - /// - /// Strategy to create a FlurlClient or reuse an existing one, based on the URL being called. - /// - /// The URL being called. - /// - IFlurlClient Get(Url url); - - /// - /// Defines how HttpClient should be instantiated and configured by default. Do NOT attempt - /// to cache/reuse HttpClient instances here - that should be done at the FlurlClient level - /// via a custom FlurlClientFactory that gets registered globally. - /// - /// The HttpMessageHandler used to construct the HttpClient. - /// - HttpClient CreateHttpClient(HttpMessageHandler handler); - - /// - /// Defines how the HttpMessageHandler used by HttpClients that are created by - /// this factory should be instantiated and configured. - /// - /// - HttpMessageHandler CreateMessageHandler(); - } - - /// - /// Extension methods on IFlurlClientFactory - /// - public static class FlurlClientFactoryExtensions - { - // https://stackoverflow.com/questions/51563732/how-do-i-lock-when-the-ideal-scope-of-the-lock-object-is-known-only-at-runtime - private static readonly ConditionalWeakTable _clientLocks = new ConditionalWeakTable(); - - /// - /// Provides thread-safe access to a specific IFlurlClient, typically to configure settings and default headers. - /// The URL is used to find the client, but keep in mind that the same client will be used in all calls to the same host by default. - /// - /// This IFlurlClientFactory. - /// the URL used to find the IFlurlClient. - /// the action to perform against the IFlurlClient. - public static IFlurlClientFactory ConfigureClient(this IFlurlClientFactory factory, string url, Action configAction) { - var client = factory.Get(url); - lock (_clientLocks.GetOrCreateValue(client)) { - configAction(client); - } - return factory; - } - - /// - /// Creates an HttpClient with the HttpMessageHandler returned from this factory's CreateMessageHandler method. - /// - public static HttpClient CreateHttpClient(this IFlurlClientFactory fac) => fac.CreateHttpClient(fac.CreateMessageHandler()); - } -} \ No newline at end of file diff --git a/src/Flurl.Http/Configuration/PerBaseUrlFlurlClientFactory.cs b/src/Flurl.Http/Configuration/PerBaseUrlFlurlClientFactory.cs deleted file mode 100644 index b599b940..00000000 --- a/src/Flurl.Http/Configuration/PerBaseUrlFlurlClientFactory.cs +++ /dev/null @@ -1,31 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Flurl.Http.Configuration -{ - /// - /// An IFlurlClientFactory implementation that caches and reuses the same IFlurlClient instance - /// per URL requested, which it assumes is a "base" URL, and sets the IFlurlClient.BaseUrl property - /// to that value. Ideal for use with IoC containers - register as a singleton, inject into a service - /// that wraps some web service, and use to set a private IFlurlClient field in the constructor. - /// - public class PerBaseUrlFlurlClientFactory : FlurlClientFactoryBase - { - /// - /// Returns the entire URL, which is assumed to be some "base" URL for a service. - /// - /// The URL. - /// The cache key - protected override string GetCacheKey(Url url) => url.ToString(); - - /// - /// Returns a new new FlurlClient with BaseUrl set to the URL passed. - /// - /// The URL - /// - protected override IFlurlClient Create(Url url) => new FlurlClient(url); - } -} diff --git a/src/Flurl.Http/Content/CapturedMultipartContent.cs b/src/Flurl.Http/Content/CapturedMultipartContent.cs index 827308ba..44b76827 100644 --- a/src/Flurl.Http/Content/CapturedMultipartContent.cs +++ b/src/Flurl.Http/Content/CapturedMultipartContent.cs @@ -1,10 +1,8 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Net.Http; using System.Net.Http.Headers; -using System.Text; using Flurl.Http.Configuration; using Flurl.Util; @@ -16,7 +14,7 @@ namespace Flurl.Http.Content public class CapturedMultipartContent : MultipartContent { private readonly FlurlHttpSettings _settings; - private readonly List _capturedParts = new List(); + private readonly List _capturedParts = new(); /// /// Gets an array of HttpContent objects that make up the parts of the multipart request. @@ -26,18 +24,18 @@ public class CapturedMultipartContent : MultipartContent /// /// Initializes a new instance of the class. /// - /// The FlurlHttpSettings used to serialize each content part. (Defaults to FlurlHttp.GlobalSettings.) - public CapturedMultipartContent(FlurlHttpSettings settings = null) : base("form-data") { - _settings = settings ?? FlurlHttp.GlobalSettings; + /// The FlurlHttpSettings used to serialize each content part. + public CapturedMultipartContent(FlurlHttpSettings settings) : base("form-data") { + _settings = settings; } /// /// Initializes a new instance of the class. /// /// The subtype of the multipart content. - /// The FlurlHttpSettings used to serialize each content part. (Defaults to FlurlHttp.GlobalSettings.) - public CapturedMultipartContent(string subtype, FlurlHttpSettings settings = null) : base(subtype) { - _settings = settings ?? FlurlHttp.GlobalSettings; + /// The FlurlHttpSettings used to serialize each content part. + public CapturedMultipartContent(string subtype, FlurlHttpSettings settings) : base(subtype) { + _settings = settings; } /// @@ -45,9 +43,9 @@ public CapturedMultipartContent(string subtype, FlurlHttpSettings settings = nul /// /// The subtype of the multipart content. /// The boundary string for the multipart content. - /// The FlurlHttpSettings used to serialize each content part. (Defaults to FlurlHttp.GlobalSettings.) - public CapturedMultipartContent(string subtype, string boundary, FlurlHttpSettings settings = null) : base(subtype, boundary) { - _settings = settings ?? FlurlHttp.GlobalSettings; + /// The FlurlHttpSettings used to serialize each content part. + public CapturedMultipartContent(string subtype, string boundary, FlurlHttpSettings settings) : base(subtype, boundary) { + _settings = settings; } /// diff --git a/src/Flurl.Http/FlurlClient.cs b/src/Flurl.Http/FlurlClient.cs index 7cd57abd..d2b2843f 100644 --- a/src/Flurl.Http/FlurlClient.cs +++ b/src/Flurl.Http/FlurlClient.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Linq; using System.Net.Http; using System.Threading.Tasks; @@ -14,8 +14,7 @@ namespace Flurl.Http /// public interface IFlurlClient : IHttpSettingsContainer, IDisposable { /// - /// Gets the HttpClient to be used in subsequent HTTP calls. Creation (when necessary) is delegated - /// to FlurlHttp.FlurlClientFactory. Reused for the life of the FlurlClient. + /// Gets the HttpClient that this IFlurlClient wraps. /// HttpClient HttpClient { get; } @@ -51,15 +50,16 @@ public interface IFlurlClient : IHttpSettingsContainer, IDisposable { /// public class FlurlClient : IFlurlClient { + private static readonly Lazy _defaultFactory = new(() => new DefaultFlurlClientFactory()); + /// - /// Initializes a new instance of . + /// Creates a new instance of . /// /// The base URL associated with this client. - public FlurlClient(string baseUrl = null) : - this(FlurlHttp.GlobalSettings.FlurlClientFactory.CreateHttpClient(), baseUrl) { } + public FlurlClient(string baseUrl = null) : this(_defaultFactory.Value.CreateHttpClient(), baseUrl) { } /// - /// Initializes a new instance of , wrapping an existing HttpClient. + /// Creates a new instance of , wrapping an existing HttpClient. /// Generally, you should let Flurl create and manage HttpClient instances for you, but you might, for /// example, have an HttpClient instance that was created by a 3rd-party library and you want to use /// Flurl to build and send calls with it. Be aware that if the HttpClient has an underlying @@ -83,7 +83,7 @@ public FlurlClient(HttpClient httpClient, string baseUrl = null) { public string BaseUrl { get; set; } /// - public FlurlHttpSettings Settings { get; } = new FlurlHttpSettings(); + public FlurlHttpSettings Settings { get; } = new(); /// public INameValueList Headers { get; } = new NameValueList(false); // header names are case-insensitive https://stackoverflow.com/a/5259004/62600 @@ -192,7 +192,7 @@ private async Task ProcessRedirectAsync(FlurlCall call, HttpComp Verb = changeToGet ? HttpMethod.Get : call.HttpRequestMessage.Method, Content = changeToGet ? null : call.Request.Content, RedirectedFrom = call, - Settings = { Defaults = settings } + Settings = { Parent = settings } }; if (call.Request.CookieJar != null) diff --git a/src/Flurl.Http/FlurlHttp.cs b/src/Flurl.Http/FlurlHttp.cs index bd24346b..937f5dd4 100644 --- a/src/Flurl.Http/FlurlHttp.cs +++ b/src/Flurl.Http/FlurlHttp.cs @@ -4,37 +4,46 @@ namespace Flurl.Http { /// - /// A static container for global configuration settings affecting Flurl.Http behavior. + /// A static object for configuring Flurl for "clientless" usage. Provides a default IFlurlClientCache instance primarily + /// for clientless support, but can be used directly, as an alternative to a DI-managed singleton cache. /// public static class FlurlHttp { - private static readonly object _configLock = new object(); + private static Func _cachingStrategy = BuildClientNameByHost; - private static Lazy _settings = - new Lazy(() => new GlobalFlurlHttpSettings()); + /// + /// A global collection of cached IFlurlClient instances. + /// + public static IFlurlClientCache Clients { get; } = new FlurlClientCache(); + + /// + /// Gets a builder for configuring the IFlurlClient that would be selected for calling the given URL when the clientless pattern is used. + /// Note that if you've overridden the caching strategy to vary clients by request properties other than Url, you should instead use + /// FlurlHttp.Clients.Add(name) to ensure you are configuring the correct client. + /// + public static IFlurlClientBuilder ConfigureClientForUrl(string url) => Clients.Add(_cachingStrategy(new FlurlRequest(url))); + + /// + /// 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)); /// - /// Globally configured Flurl.Http settings. Should normally be written to by calling FlurlHttp.Configure once application at startup. + /// Sets a global caching strategy for getting or creating an IFlurlClient instance when the clientless pattern is used, e.g. url.GetAsync. /// - public static GlobalFlurlHttpSettings GlobalSettings => _settings.Value; + /// A delegate that returns a cache key used to store and retrieve a client instance based on properties of the request. + public static void UseClientCachingStrategy(Func buildClientName) => _cachingStrategy = buildClientName; /// - /// Provides thread-safe access to Flurl.Http's global configuration settings. Should only be called once at application startup. + /// Sets a global caching strategy of one IFlurlClient per scheme/host/port combination when the clientless pattern is used, + /// e.g. url.GetAsync. This is the default strategy, so you shouldn't need to call this except to revert a previous call to + /// UseClientCachingStrategy, which would be rare. /// - /// the action to perform against the GlobalSettings. - public static void Configure(Action configAction) { - lock (_configLock) { - configAction(GlobalSettings); - } - } + public static void UseClientPerHostStrategy() => _cachingStrategy = BuildClientNameByHost; /// - /// Provides thread-safe access to a specific IFlurlClient, typically to configure settings and default headers. - /// The URL is used to find the client, but keep in mind that the same client will be used in all calls to the same host by default. + /// Builds a cache key consisting of URL scheme, host, and port. This is the default client caching strategy. /// - /// the URL used to find the IFlurlClient. - /// the action to perform against the IFlurlClient. - public static void ConfigureClient(string url, Action configAction) => - GlobalSettings.FlurlClientFactory.ConfigureClient(url, configAction); + public static string BuildClientNameByHost(IFlurlRequest req) => $"{req.Url.Scheme}|{req.Url.Host}|{req.Url.Port}"; } } \ No newline at end of file diff --git a/src/Flurl.Http/FlurlRequest.cs b/src/Flurl.Http/FlurlRequest.cs index 558e0908..6d52f2e1 100644 --- a/src/Flurl.Http/FlurlRequest.cs +++ b/src/Flurl.Http/FlurlRequest.cs @@ -98,14 +98,14 @@ internal FlurlRequest(string baseUrl, params object[] urlSegments) { } /// - public FlurlHttpSettings Settings { get; } = new FlurlHttpSettings(); + public FlurlHttpSettings Settings { get; } = new(); /// public IFlurlClient Client { get => _client; set { _client = value; - Settings.Defaults = _client?.Settings; + Settings.Parent = _client?.Settings; } } @@ -138,7 +138,7 @@ public CookieJar CookieJar { public Task SendAsync(HttpMethod verb, HttpContent content = null, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead, CancellationToken cancellationToken = default) { Verb = verb; Content = content; - Client ??= FlurlHttp.GlobalSettings.FlurlClientFactory.Get(Url); + Client ??= FlurlHttp.GetClientForRequest(this); return Client.SendAsync(this, completionOption, cancellationToken); } diff --git a/src/Flurl.Http/SettingsExtensions.cs b/src/Flurl.Http/SettingsExtensions.cs index 24489200..b5c3a06f 100644 --- a/src/Flurl.Http/SettingsExtensions.cs +++ b/src/Flurl.Http/SettingsExtensions.cs @@ -17,7 +17,7 @@ public static class SettingsExtensions /// The IFlurlClient. /// Action defining the settings changes. /// The IFlurlClient with the modified Settings - public static IFlurlClient Configure(this IFlurlClient client, Action action) { + public static IFlurlClient WithSettings(this IFlurlClient client, Action action) { action(client.Settings); return client; } diff --git a/src/Flurl.Http/Testing/HttpCallAssertion.cs b/src/Flurl.Http/Testing/HttpCallAssertion.cs index e96b0f6a..150f8bb9 100644 --- a/src/Flurl.Http/Testing/HttpCallAssertion.cs +++ b/src/Flurl.Http/Testing/HttpCallAssertion.cs @@ -13,6 +13,7 @@ namespace Flurl.Http.Testing /// public class HttpCallAssertion { + private readonly HttpTest _test; private readonly bool _negate; private readonly IList _expectedConditions = new List(); @@ -21,10 +22,11 @@ public class HttpCallAssertion /// /// Constructs a new instance of HttpCallAssertion. /// - /// Set of calls (usually from HttpTest.CallLog) to assert against. + /// The HttpTest containing calls being asserted. /// If true, assertions pass when calls matching criteria were NOT made. - public HttpCallAssertion(IEnumerable loggedCalls, bool negate = false) { - _calls = loggedCalls.ToList(); + public HttpCallAssertion(HttpTest test, bool negate = false) { + _test = test; + _calls = test.CallLog.ToList(); _negate = negate; } @@ -99,7 +101,7 @@ public HttpCallAssertion WithRequestBody(string bodyPattern) { /// Asserts whether calls were made containing given JSON-encoded request body. body may contain * wildcard. /// public HttpCallAssertion WithRequestJson(object body) { - var serializedBody = FlurlHttp.GlobalSettings.JsonSerializer.Serialize(body); + var serializedBody = _test.Settings.JsonSerializer.Serialize(body); return WithRequestBody(serializedBody); } @@ -107,7 +109,7 @@ public HttpCallAssertion WithRequestJson(object body) { /// Asserts whether calls were made containing given URL-encoded request body. body may contain * wildcard. /// public HttpCallAssertion WithRequestUrlEncoded(object body) { - var serializedBody = FlurlHttp.GlobalSettings.UrlEncodedSerializer.Serialize(body); + var serializedBody = _test.Settings.UrlEncodedSerializer.Serialize(body); return WithRequestBody(serializedBody); } #endregion diff --git a/src/Flurl.Http/Testing/HttpTest.cs b/src/Flurl.Http/Testing/HttpTest.cs index c241826d..43a0c0a6 100644 --- a/src/Flurl.Http/Testing/HttpTest.cs +++ b/src/Flurl.Http/Testing/HttpTest.cs @@ -15,8 +15,8 @@ namespace Flurl.Http.Testing [Serializable] public class HttpTest : HttpTestSetup, IDisposable { - private readonly ConcurrentQueue _calls = new ConcurrentQueue(); - private readonly List _filteredSetups = new List(); + private readonly ConcurrentQueue _calls = new(); + private readonly List _filteredSetups = new(); /// /// Initializes a new instance of the class. @@ -43,7 +43,7 @@ public HttpTest() : base(new FlurlHttpSettings()) { /// /// Action defining the settings changes. /// This HttpTest - public HttpTest Configure(Action action) { + public HttpTest WithSettings(Action action) { action(Settings); return this; } @@ -66,7 +66,7 @@ internal HttpTestSetup FindSetup(FlurlCall call) { /// /// URL that should have been called. Can include * wildcard character. public HttpCallAssertion ShouldHaveCalled(string urlPattern) { - return new HttpCallAssertion(CallLog).WithUrlPattern(urlPattern); + return new HttpCallAssertion(this).WithUrlPattern(urlPattern); } /// @@ -74,21 +74,21 @@ public HttpCallAssertion ShouldHaveCalled(string urlPattern) { /// /// URL that should not have been called. Can include * wildcard character. public void ShouldNotHaveCalled(string urlPattern) { - new HttpCallAssertion(CallLog, true).WithUrlPattern(urlPattern); + new HttpCallAssertion(this, true).WithUrlPattern(urlPattern); } /// /// Asserts whether any HTTP call was made, throwing HttpCallAssertException if none were. /// public HttpCallAssertion ShouldHaveMadeACall() { - return new HttpCallAssertion(CallLog).WithUrlPattern("*"); + return new HttpCallAssertion(this).WithUrlPattern("*"); } /// /// Asserts whether no HTTP calls were made, throwing HttpCallAssertException if any were. /// public void ShouldNotHaveMadeACall() { - new HttpCallAssertion(CallLog, true).WithUrlPattern("*"); + new HttpCallAssertion(this, true).WithUrlPattern("*"); } /// diff --git a/test/Flurl.Test/Http/CookieTests.cs b/test/Flurl.Test/Http/CookieTests.cs index 049e7f3e..6f363241 100644 --- a/test/Flurl.Test/Http/CookieTests.cs +++ b/test/Flurl.Test/Http/CookieTests.cs @@ -107,7 +107,7 @@ public void can_build_response_header() { } [Test] - public void can_perist_and_load_jar() { + public void can_persist_and_load_jar() { var jar1 = new CookieJar() .AddOrReplace("x", "foo", "https://site1.com", DateTimeOffset.UtcNow) .AddOrReplace("y", "bar", "https://site2.com", DateTimeOffset.UtcNow.AddMinutes(-10)); diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs new file mode 100644 index 00000000..f30e37a6 --- /dev/null +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net.Http; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Flurl.Http; +using Flurl.Http.Configuration; +using NUnit.Framework; + +namespace Flurl.Test.Http +{ + [TestFixture] + public class FlurlClientBuilderTests + { + [Test] + public void can_configure_settings() { + var builder = new FlurlClientBuilder(); + var cli = builder.WithSettings(s => s.HttpVersion = "3.0").Build(); + Assert.AreEqual("3.0", cli.Settings.HttpVersion); + } + + [Test] + public void can_configure_HttpClient() { + var builder = new FlurlClientBuilder(); + var cli = builder.ConfigureHttpClient(c => c.BaseAddress = new Uri("https://flurl.dev/docs/fluent-http")).Build(); + Assert.AreEqual("https://flurl.dev/docs/fluent-http", cli.HttpClient.BaseAddress.ToString()); + Assert.AreEqual("https://flurl.dev/docs/fluent-http", cli.BaseUrl); + } + + [Test] + public async Task can_configure_inner_handler() { + var builder = new FlurlClientBuilder(); + // Handlers are not accessible beyond HttpClient constructor, making them hard to assert against! + var cli = builder.ConfigureInnerHandler(h => h.Dispose()).Build(); + try { + await cli.Request("https://www.google.com").GetAsync(); + Assert.Fail("Should have failed since the inner handler was disposed."); + } + catch (FlurlHttpException ex) { + Assert.IsInstanceOf(ex.InnerException); + StringAssert.EndsWith("Handler", (ex.InnerException as ObjectDisposedException).ObjectName); + } + } + + [Test] + public async Task can_add_middleware() { + var builder = new FlurlClientBuilder(); + var cli = builder.AddMiddleware(() => new BlockingHandler("blocked by flurl!")).Build(); + var resp = await cli.Request("https://www.google.com").GetStringAsync(); + Assert.AreEqual("blocked by flurl!", resp); + } + + class BlockingHandler : DelegatingHandler + { + private readonly string _msg; + + public BlockingHandler(string msg) { + _msg = msg; + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + return Task.FromResult(new HttpResponseMessage { Content = new StringContent(_msg) }); + } + } + } +} diff --git a/test/Flurl.Test/Http/FlurlClientCacheTests.cs b/test/Flurl.Test/Http/FlurlClientCacheTests.cs new file mode 100644 index 00000000..b44ce4ed --- /dev/null +++ b/test/Flurl.Test/Http/FlurlClientCacheTests.cs @@ -0,0 +1,90 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Flurl.Http.Configuration; +using NUnit.Framework; + +namespace Flurl.Test.Http +{ + [TestFixture] + public class FlurlClientCacheTests + { + [Test] + public void can_add_and_get_client() { + var cache = new FlurlClientCache(); + cache.Add("github", "https://api.github.com").WithSettings(s => s.Timeout = TimeSpan.FromSeconds(123)); + cache.Add("google", "https://api.google.com").WithSettings(s => s.Timeout = TimeSpan.FromSeconds(234)); + + var gh = cache.Get("github"); + Assert.AreEqual("https://api.github.com", gh.BaseUrl); + Assert.AreEqual(123, gh.Settings.Timeout.Value.TotalSeconds); + Assert.AreSame(gh, cache.Get("github"), "should reuse same-named clients."); + + var goog = cache.Get("google"); + Assert.AreEqual("https://api.google.com", goog.BaseUrl); + Assert.AreEqual(234, goog.Settings.Timeout.Value.TotalSeconds); + Assert.AreSame(goog, cache.Get("google"), "should reuse same-named clients."); + } + + [Test] + public void can_get_unconfigured_client() { + var cache = new FlurlClientCache(); + var cli = cache.Get("foo"); + Assert.IsNull(cli.BaseUrl); + Assert.AreEqual(100, cli.Settings.Timeout.Value.TotalSeconds); + + Assert.AreSame(cli, cache.Get("foo"), "should reuse same-named clients."); + Assert.AreNotSame(cli, cache.Get("bar"), "different-named clients should be different instances."); + } + + [Test] + public void cannot_add_same_name_twice() { + var cache = new FlurlClientCache(); + cache.Add("foo", "https://api.github.com"); + Assert.Throws(() => cache.Add("foo", "https://api.google.com")); + } + + [Test] + public void can_configure_all() { + var cache = new FlurlClientCache() + .ConfigureAll(b => b.WithSettings(s => s.Timeout = TimeSpan.FromSeconds(123))); + + var cli1 = cache.Get("foo"); + + cache.Add("bar").WithSettings(s => { + s.Timeout = TimeSpan.FromSeconds(456); + }); + cache.ConfigureAll(b => b.WithSettings(s => { + s.Timeout = TimeSpan.FromSeconds(789); + })); + + var cli2 = cache.Get("bar"); + var cli3 = cache.Get("buzz"); + + Assert.AreEqual(123, cli1.Settings.Timeout.Value.TotalSeconds, "fetched the client before changing the defaults, so original should stick"); + Assert.AreEqual(456, cli2.Settings.Timeout.Value.TotalSeconds, "client-specific settings should always win, even if defaults were changed after"); + Assert.AreEqual(789, cli3.Settings.Timeout.Value.TotalSeconds, "newer defaults should squash older"); + } + + [Test] + public void can_remove() { + // hard to test directly, but if we get a new instance after deleting, safe to assume it works. + var cache = new FlurlClientCache(); + cache.Add("foo"); + + var cli1 = cache.Get("foo"); + var cli2 = cache.Get("foo"); + cache.Remove("foo"); + var cli3 = cache.Get("foo"); + + Assert.AreSame(cli1, cli2); + Assert.AreNotSame(cli1, cli3); + + Assert.IsTrue(cli1.IsDisposed); + Assert.IsTrue(cli2.IsDisposed); + Assert.IsFalse(cli3.IsDisposed); + } + } +} diff --git a/test/Flurl.Test/Http/FlurlClientFactoryTests.cs b/test/Flurl.Test/Http/FlurlClientFactoryTests.cs deleted file mode 100644 index ce4de4ef..00000000 --- a/test/Flurl.Test/Http/FlurlClientFactoryTests.cs +++ /dev/null @@ -1,80 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using Flurl.Http.Configuration; -using NUnit.Framework; - -namespace Flurl.Test.Http -{ - [TestFixture] - public class FlurlClientFactoryTests - { - [Test] - public void default_factory_provides_same_client_per_host_scheme_port() { - var fac = new DefaultFlurlClientFactory(); - var cli1 = fac.Get("http://api.com/foo"); - var cli2 = fac.Get("http://api.com/bar"); - var cli3 = fac.Get("https://api.com/foo"); - var cli4 = fac.Get("https://api.com/bar"); - var cli5 = fac.Get("https://api.com:1234/foo"); - var cli6 = fac.Get("https://api.com:1234/bar"); - - Assert.AreSame(cli1, cli2); - Assert.AreSame(cli3, cli4); - Assert.AreSame(cli5, cli6); - - Assert.AreNotSame(cli1, cli3); - Assert.AreNotSame(cli3, cli5); - } - - [Test] - public void per_base_url_factory_provides_same_client_per_provided_url() { - var fac = new PerBaseUrlFlurlClientFactory(); - var cli1 = fac.Get("http://api.com/foo"); - var cli2 = fac.Get("http://api.com/bar"); - var cli3 = fac.Get("http://api.com/foo"); - Assert.AreNotSame(cli1, cli2); - Assert.AreSame(cli1, cli3); - } - - [Test] - public void can_configure_client_from_factory() { - var fac = new DefaultFlurlClientFactory() - .ConfigureClient("http://api.com/foo", c => c.Settings.Timeout = TimeSpan.FromSeconds(123)); - Assert.AreEqual(TimeSpan.FromSeconds(123), fac.Get("http://api.com/bar").Settings.Timeout); - Assert.AreNotEqual(TimeSpan.FromSeconds(123), fac.Get("http://api2.com/foo").Settings.Timeout); - } - - [Test] - public async Task ConfigureClient_is_thread_safe() { - var fac = new DefaultFlurlClientFactory(); - var sequence = new List(); - - var task1 = Task.Run(() => fac.ConfigureClient("http://api.com", c => { - sequence.Add(1); - Thread.Sleep(5000); - sequence.Add(3); - })); - - await Task.Delay(200); - - // modifies same client as task1, should get blocked until task1 is done - var task2 = Task.Run(() => fac.ConfigureClient("http://api.com", c => { - sequence.Add(4); - })); - - await Task.Delay(200); - - // modifies different client, should run immediately - var task3 = Task.Run(() => fac.ConfigureClient("http://api2.com", c => { - sequence.Add(2); - })); - - await Task.WhenAll(task1, task2, task3); - Assert.AreEqual("1,2,3,4", string.Join(",", sequence)); - } - } -} diff --git a/test/Flurl.Test/Http/GlobalConfigTests.cs b/test/Flurl.Test/Http/GlobalConfigTests.cs new file mode 100644 index 00000000..fecd033b --- /dev/null +++ b/test/Flurl.Test/Http/GlobalConfigTests.cs @@ -0,0 +1,78 @@ +using System; +using System.Threading.Tasks; +using Flurl.Http; +using NUnit.Framework; + +namespace Flurl.Test.Http +{ + [TestFixture, NonParallelizable] + public class GlobalConfigTests : HttpTestFixtureBase + { + [SetUp, TearDown] + public void ResetGlobalDefaults() { + FlurlHttp.UseClientPerHostStrategy(); + FlurlHttp.Clients.Clear(); + } + + // different path - share client + [TestCase("https://api.com/some/path", "https://api.com/other/path", true)] + // different userinfo - share client + [TestCase("https://api.com", "https://user:pass@api.com", true)] + // different host - don't share + [TestCase("https://api.com", "https://www.api.com", false)] + // different port - don't share + [TestCase("https://api.com", "https://api.com:3333", false)] + // different scheme - don't share + [TestCase("http://api.com", "https://api.com", false)] + public void default_caching_strategy_provides_same_client_per_host_scheme_port(string url1, string url2, bool sameClient) { + var cli1 = FlurlHttp.GetClientForRequest(new FlurlRequest(url1)); + var cli2 = FlurlHttp.GetClientForRequest(new FlurlRequest(url2)); + + if (sameClient) + Assert.AreSame(cli1, cli2); + else + Assert.AreNotSame(cli1, cli2); + } + + [Test] + public async Task can_replace_caching_strategy() { + FlurlHttp.UseClientCachingStrategy(req => "shared"); + Assert.AreSame( + FlurlHttp.GetClientForRequest(new FlurlRequest("https://a.com")), + FlurlHttp.GetClientForRequest(new FlurlRequest("https://b.com"))); + + var req1 = new FlurlRequest("https://a.com"); + await req1.GetAsync(); + var req2 = new FlurlRequest("https://b.com"); + await req2.GetAsync(); + + Assert.AreSame(req1.Client, req2.Client, "custom strategy uses same client for all calls"); + + FlurlHttp.UseClientPerHostStrategy(); + Assert.AreNotSame( + FlurlHttp.GetClientForRequest(new FlurlRequest("https://a.com")), + FlurlHttp.GetClientForRequest(new FlurlRequest("https://b.com"))); + + req1 = new FlurlRequest("https://a.com"); + await req1.GetAsync(); + req2 = new FlurlRequest("https://b.com"); + await req2.GetAsync(); + + Assert.AreNotSame(req1.Client, req2.Client, "default strategy uses different client per host"); + } + + [Test] + public void can_configure_client_for_url() { + FlurlHttp.ConfigureClientForUrl("https://api.com/some/path") + .WithSettings(s => s.Timeout = TimeSpan.FromSeconds(123)); + + var cli1 = FlurlHttp.GetClientForRequest(new FlurlRequest("https://api.com")); + var cli2 = FlurlHttp.GetClientForRequest(new FlurlRequest("https://api.com/differernt/path")); + var cli3 = FlurlHttp.GetClientForRequest(new FlurlRequest("https://api.com:3333")); + + Assert.AreEqual(123, cli1.Settings.Timeout.Value.TotalSeconds); + Assert.AreEqual(123, cli2.Settings.Timeout.Value.TotalSeconds); + Assert.AreNotEqual(123, cli3.Settings.Timeout.Value.TotalSeconds); + } + } +} diff --git a/test/Flurl.Test/Http/MultipartTests.cs b/test/Flurl.Test/Http/MultipartTests.cs index 500d4266..396d6c4e 100644 --- a/test/Flurl.Test/Http/MultipartTests.cs +++ b/test/Flurl.Test/Http/MultipartTests.cs @@ -6,6 +6,7 @@ using System.Text; using System.Threading.Tasks; using Flurl.Http; +using Flurl.Http.Configuration; using Flurl.Http.Content; using Flurl.Http.Testing; using NUnit.Framework; @@ -17,7 +18,7 @@ public class MultipartTests { [Test] public async Task can_build_and_send_multipart_content() { - var content = new CapturedMultipartContent() + var content = new CapturedMultipartContent(new FlurlHttpSettings()) .AddString("string", "foo") .AddString("string2", "bar", "text/blah") .AddStringParts(new { part1 = 1, part2 = 2, part3 = (string)null }) // part3 should be excluded @@ -67,7 +68,7 @@ private void AssertFilePart(HttpContent part, string name, string fileName, stri [Test] public void must_provide_required_args_to_builder() { - var content = new CapturedMultipartContent(); + var content = new CapturedMultipartContent(new FlurlHttpSettings()); Assert.Throws(() => content.AddStringParts(null)); Assert.Throws(() => content.AddString("other", null)); Assert.Throws(() => content.AddString(null, "hello!")); diff --git a/test/Flurl.Test/Http/SettingsExtensionsTests.cs b/test/Flurl.Test/Http/SettingsExtensionsTests.cs index a2340fd0..96cb170e 100644 --- a/test/Flurl.Test/Http/SettingsExtensionsTests.cs +++ b/test/Flurl.Test/Http/SettingsExtensionsTests.cs @@ -192,7 +192,7 @@ public void can_use_uri_with_WithUrl() { [Test] public void can_override_settings_fluently() { using (var test = new HttpTest()) { - var cli = new FlurlClient().Configure(s => s.AllowedHttpStatusRange = "*"); + var cli = new FlurlClient().WithSettings(s => s.AllowedHttpStatusRange = "*"); test.RespondWith("epic fail", 500); var req = "http://www.api.com".WithSettings(c => c.AllowedHttpStatusRange = "2xx"); req.Client = cli; // client-level settings shouldn't win diff --git a/test/Flurl.Test/Http/SettingsTests.cs b/test/Flurl.Test/Http/SettingsTests.cs index 547892f5..64e2f7cd 100644 --- a/test/Flurl.Test/Http/SettingsTests.cs +++ b/test/Flurl.Test/Http/SettingsTests.cs @@ -1,6 +1,5 @@ using System; using System.IO; -using System.Net.Http; using System.Threading.Tasks; using Flurl.Http; using Flurl.Http.Configuration; @@ -10,7 +9,7 @@ namespace Flurl.Test.Http { /// - /// FlurlHttpSettings are available at the global, test, client, and request level. This abstract class + /// FlurlHttpSettings are available at the test, client, and request level. This abstract class /// allows the same tests to be run against settings at all 4 levels. /// public abstract class SettingsTestsBase @@ -163,100 +162,7 @@ public async Task explicit_content_type_header_is_not_overridden() { } } - [TestFixture, NonParallelizable] // touches global settings, so can't run in parallel - public class GlobalSettingsTests : SettingsTestsBase - { - protected override FlurlHttpSettings GetSettings() => FlurlHttp.GlobalSettings; - protected override IFlurlRequest GetRequest() => new FlurlRequest("http://api.com"); - - [TearDown] - public void ResetDefaults() => FlurlHttp.GlobalSettings.ResetDefaults(); - - [Test] - public void settings_propagate_correctly() { - FlurlHttp.GlobalSettings.Redirects.Enabled = false; - FlurlHttp.GlobalSettings.AllowedHttpStatusRange = "4xx"; - FlurlHttp.GlobalSettings.Redirects.MaxAutoRedirects = 123; - - var client1 = new FlurlClient(); - client1.Settings.Redirects.Enabled = true; - Assert.AreEqual("4xx", client1.Settings.AllowedHttpStatusRange); - Assert.AreEqual(123, client1.Settings.Redirects.MaxAutoRedirects); - client1.Settings.AllowedHttpStatusRange = "5xx"; - client1.Settings.Redirects.MaxAutoRedirects = 456; - - var req = client1.Request("http://myapi.com"); - Assert.IsTrue(req.Settings.Redirects.Enabled, "request should inherit client settings when not set at request level"); - Assert.AreEqual("5xx", req.Settings.AllowedHttpStatusRange, "request should inherit client settings when not set at request level"); - Assert.AreEqual(456, req.Settings.Redirects.MaxAutoRedirects, "request should inherit client settings when not set at request level"); - - var client2 = new FlurlClient(); - client2.Settings.Redirects.Enabled = false; - - req.Client = client2; - Assert.IsFalse(req.Settings.Redirects.Enabled, "request should inherit client settings when not set at request level"); - Assert.AreEqual("4xx", req.Settings.AllowedHttpStatusRange, "request should inherit global settings when not set at request or client level"); - Assert.AreEqual(123, req.Settings.Redirects.MaxAutoRedirects, "request should inherit global settings when not set at request or client level"); - - client2.Settings.Redirects.Enabled = true; - client2.Settings.AllowedHttpStatusRange = "3xx"; - client2.Settings.Redirects.MaxAutoRedirects = 789; - Assert.IsTrue(req.Settings.Redirects.Enabled, "request should inherit client settings when not set at request level"); - Assert.AreEqual("3xx", req.Settings.AllowedHttpStatusRange, "request should inherit client settings when not set at request level"); - Assert.AreEqual(789, req.Settings.Redirects.MaxAutoRedirects, "request should inherit client settings when not set at request level"); - - req.Settings.Redirects.Enabled = false; - req.Settings.AllowedHttpStatusRange = "6xx"; - req.Settings.Redirects.MaxAutoRedirects = 2; - Assert.IsFalse(req.Settings.Redirects.Enabled, "request-level settings should override any defaults"); - Assert.AreEqual("6xx", req.Settings.AllowedHttpStatusRange, "request-level settings should override any defaults"); - Assert.AreEqual(2, req.Settings.Redirects.MaxAutoRedirects, "request-level settings should override any defaults"); - - req.Settings.ResetDefaults(); - Assert.IsTrue(req.Settings.Redirects.Enabled, "request should inherit client settings when cleared at request level"); - Assert.AreEqual("3xx", req.Settings.AllowedHttpStatusRange, "request should inherit client settings when cleared request level"); - Assert.AreEqual(789, req.Settings.Redirects.MaxAutoRedirects, "request should inherit client settings when cleared request level"); - - client2.Settings.ResetDefaults(); - Assert.IsFalse(req.Settings.Redirects.Enabled, "request should inherit global settings when cleared at request and client level"); - Assert.AreEqual("4xx", req.Settings.AllowedHttpStatusRange, "request should inherit global settings when cleared at request and client level"); - Assert.AreEqual(123, req.Settings.Redirects.MaxAutoRedirects, "request should inherit global settings when cleared at request and client level"); - } - - [Test] - public async Task can_provide_custom_client_factory() { - FlurlHttp.GlobalSettings.FlurlClientFactory = new MyCustomClientFactory(); - var req = GetRequest(); - - // client not assigned until request is sent - using var test = new HttpTest(); - await req.GetAsync(); - - Assert.IsInstanceOf(req.Client.HttpClient); - } - - [Test] - public void can_configure_global_from_FlurlHttp_object() { - FlurlHttp.Configure(settings => settings.Redirects.Enabled = false); - Assert.IsFalse(FlurlHttp.GlobalSettings.Redirects.Enabled); - } - - [Test] - public async Task can_configure_client_from_FlurlHttp_object() { - FlurlHttp.ConfigureClient("http://host1.com/foo", cli => cli.Settings.Redirects.Enabled = false); - var req1 = new FlurlRequest("http://host1.com/bar"); // different URL but same host, should use above client - var req2 = new FlurlRequest("http://host2.com"); // different host, should use new client - - // client not assigned until request is sent - using var test = new HttpTest(); - await Task.WhenAll(req1.GetAsync(), req2.GetAsync()); - - Assert.IsFalse(req1.Client.Settings.Redirects.Enabled); - Assert.IsTrue(req2.Client.Settings.Redirects.Enabled); - } - } - - [TestFixture, Parallelizable] + [TestFixture] public class HttpTestSettingsTests : SettingsTestsBase { private HttpTest _test; @@ -296,7 +202,7 @@ private class FakeSerializer : ISerializer } } - [TestFixture, Parallelizable] + [TestFixture] public class ClientSettingsTests : SettingsTestsBase { private readonly Lazy _client = new Lazy(() => new FlurlClient()); @@ -305,7 +211,7 @@ public class ClientSettingsTests : SettingsTestsBase protected override IFlurlRequest GetRequest() => _client.Value.Request("http://api.com"); } - [TestFixture, Parallelizable] + [TestFixture] public class RequestSettingsTests : SettingsTestsBase { private readonly Lazy _req = new Lazy(() => new FlurlRequest("http://api.com")); @@ -314,18 +220,11 @@ public class RequestSettingsTests : SettingsTestsBase protected override IFlurlRequest GetRequest() => _req.Value; [Test] - public void request_gets_global_settings_when_no_client() { + public void request_gets_default_settings_when_no_client() { var req = new FlurlRequest(); Assert.IsNull(req.Client); Assert.IsNull(req.Url); - Assert.AreEqual(FlurlHttp.GlobalSettings.JsonSerializer, req.Settings.JsonSerializer); + Assert.IsInstanceOf(req.Settings.JsonSerializer); } } - - class MyCustomClientFactory : DefaultFlurlClientFactory - { - public override HttpClient CreateHttpClient(HttpMessageHandler handler) => new MyCustomHttpClient(); - } - - class MyCustomHttpClient : HttpClient { } } diff --git a/test/Flurl.Test/Http/TestingTests.cs b/test/Flurl.Test/Http/TestingTests.cs index 616cb168..a1f40f29 100644 --- a/test/Flurl.Test/Http/TestingTests.cs +++ b/test/Flurl.Test/Http/TestingTests.cs @@ -439,7 +439,7 @@ public async Task can_deserialize_default_response_more_than_once() { [Test] public void can_configure_settings_for_test() { Assert.IsTrue(new FlurlRequest().Settings.Redirects.Enabled); - using (new HttpTest().Configure(settings => settings.Redirects.Enabled = false)) { + using (new HttpTest().WithSettings(settings => settings.Redirects.Enabled = false)) { Assert.IsFalse(new FlurlRequest().Settings.Redirects.Enabled); } // test disposed, should revert back to global settings From e9fbfbfcfc708233ddf9b2ec390a55b30ed372ab Mon Sep 17 00:00:00 2001 From: Todd Date: Wed, 18 Oct 2023 13:55:00 -0500 Subject: [PATCH 2/9] bug with redirects + settings hierarchy --- src/Flurl.Http/Configuration/FlurlHttpSettings.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Flurl.Http/Configuration/FlurlHttpSettings.cs b/src/Flurl.Http/Configuration/FlurlHttpSettings.cs index 6ac730b9..760bbc4b 100644 --- a/src/Flurl.Http/Configuration/FlurlHttpSettings.cs +++ b/src/Flurl.Http/Configuration/FlurlHttpSettings.cs @@ -173,8 +173,8 @@ public void ResetDefaults() { internal T Get([CallerMemberName]string propName = null) { IEnumerable prioritize() { yield return HttpTest.Current?.Settings; - yield return this; - yield return Parent; + for (var settings = this; settings != null; settings = settings.Parent) + yield return settings; yield return Defaults; } From d5ff66e42dd5f0d55cc23c22bf4bdc00e1484295 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 20 Oct 2023 12:20:12 -0500 Subject: [PATCH 3/9] #769 use SocketsHttpHandler on supported platforms --- .../Configuration/FlurlClientBuilder.cs | 50 +++++++++++----- .../Configuration/FlurlClientFactory.cs | 60 +++++++++---------- .../Http/FlurlClientBuilderTests.cs | 17 +++++- 3 files changed, 79 insertions(+), 48 deletions(-) diff --git a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs index a80a9605..7e01bb36 100644 --- a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs +++ b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs @@ -23,7 +23,11 @@ public interface IFlurlClientBuilder /// /// Configure the inner-most HttpMessageHandler associated with this IFlurlClient. /// +#if NETCOREAPP2_1_OR_GREATER + IFlurlClientBuilder ConfigureInnerHandler(Action configAction); +#else IFlurlClientBuilder ConfigureInnerHandler(Action configAction); +#endif /// /// Add a provided DelegatingHandler to the IFlurlClient. @@ -44,9 +48,13 @@ public class FlurlClientBuilder : IFlurlClientBuilder private readonly IFlurlClientFactory _factory; private readonly string _baseUrl; private readonly List> _addMiddleware = new(); - private readonly List> _configClient = new(); - private readonly List> _configHandler = 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 /// /// Creates a new FlurlClientBuilder. @@ -80,24 +88,20 @@ public IFlurlClientBuilder ConfigureHttpClient(Action configAction) } /// +#if NETCOREAPP2_1_OR_GREATER + public IFlurlClientBuilder ConfigureInnerHandler(Action configAction) { +#else public IFlurlClientBuilder ConfigureInnerHandler(Action configAction) { - _configHandler.Add(configAction); +#endif + _handlerBuilder.Configs.Add(configAction); return this; } /// public IFlurlClient Build() { - var innerHandler = _factory.CreateInnerHandler(); - foreach (var config in _configHandler) { - if (innerHandler is HttpClientHandler hch) - config(hch); - else - throw new Exception("ConfigureInnerHandler can only be used when IFlurlClientFactory.CreateInnerHandler returns an instance of HttpClientFactory."); - } + var outerHandler = _handlerBuilder.Build(_factory); - HttpMessageHandler outerHandler = innerHandler; - foreach (var createMW in Enumerable.Reverse(_addMiddleware)) { - var middleware = createMW(); + foreach (var middleware in Enumerable.Reverse(_addMiddleware).Select(create => create())) { middleware.InnerHandler = outerHandler; outerHandler = middleware; } @@ -107,11 +111,27 @@ public IFlurlClient Build() { config(httpCli); var flurlCli = new FlurlClient(httpCli, _baseUrl); - foreach (var config in _configSettings) { + foreach (var config in _configSettings) 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/FlurlClientFactory.cs b/src/Flurl.Http/Configuration/FlurlClientFactory.cs index bae9faf7..2c27bac5 100644 --- a/src/Flurl.Http/Configuration/FlurlClientFactory.cs +++ b/src/Flurl.Http/Configuration/FlurlClientFactory.cs @@ -5,24 +5,23 @@ namespace Flurl.Http.Configuration { /// - /// Interface for methods used to build and cache IFlurlClient instances. + /// Interface for helper methods used to construct IFlurlClient instances. /// public interface IFlurlClientFactory { /// - /// Defines how HttpClient should be instantiated and configured by default. Do NOT attempt - /// to cache/reuse HttpClient instances here - that should be done at the FlurlClient level - /// via a custom FlurlClientFactory that gets registered globally. + /// Creates and configures a new HttpClient as needed when a new IFlurlClient instance is created. + /// Implementors should NOT attempt to cache or reuse HttpClient instances here - their lifetime is + /// bound one-to-one with an IFlurlClient, whose caching and reuse is managed by IFlurlClientCache. /// - /// The HttpMessageHandler used to construct the HttpClient. - /// + /// The HttpMessageHandler passed to the constructor of the HttpClient. HttpClient CreateHttpClient(HttpMessageHandler handler); /// - /// Defines how the HttpMessageHandler used by HttpClients that are created by - /// this factory should be instantiated and configured. + /// Creates and configures a new HttpMessageHandler as needed when a new IFlurlClient instance is created. + /// The default implementation creates an instance of SocketsHttpHandler for platforms that support it, + /// otherwise HttpClientHandler. /// - /// HttpMessageHandler CreateInnerHandler(); } @@ -42,38 +41,37 @@ public static class FlurlClientFactoryExtensions /// public class DefaultFlurlClientFactory : IFlurlClientFactory { - /// - /// Override in custom factory to customize the creation of HttpClient used in all Flurl HTTP calls - /// (except when one is passed explicitly to the FlurlClient constructor). In order not to lose - /// Flurl.Http functionality, it is recommended to call base.CreateClient and customize the result. - /// + /// public virtual HttpClient CreateHttpClient(HttpMessageHandler handler) { return new HttpClient(handler); } /// - /// Override in custom factory to customize the creation of the top-level HttpMessageHandler used in all - /// Flurl HTTP calls (except when an HttpClient is passed explicitly to the FlurlClient constructor). - /// In order not to lose Flurl.Http functionality, it is recommended to call base.CreateMessageHandler, and - /// either customize the returned HttpClientHandler, or set it as the InnerHandler of a DelegatingHandler. + /// Creates and configures a new HttpMessageHandler as needed when a new IFlurlClient instance is created. /// public virtual HttpMessageHandler CreateInnerHandler() { - var httpClientHandler = new HttpClientHandler(); + // 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(); - // flurl has its own mechanisms for managing cookies and redirects + if (handler.SupportsRedirectConfiguration) + handler.AllowAutoRedirect = false; - try { httpClientHandler.UseCookies = false; } - catch (PlatformNotSupportedException) { } // look out for WASM platforms (#543) + // #266 + // deflate not working? see #474 + if (handler.SupportsAutomaticDecompression) + handler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; - if (httpClientHandler.SupportsRedirectConfiguration) - httpClientHandler.AllowAutoRedirect = false; - - if (httpClientHandler.SupportsAutomaticDecompression) { - // #266 - // deflate not working? see #474 - httpClientHandler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; - } - return httpClientHandler; + try { handler.UseCookies = false; } + catch (PlatformNotSupportedException) { } // look out for WASM platforms (#543) +#endif + return handler; } } } diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs index f30e37a6..527fdd42 100644 --- a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -1,8 +1,6 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Net.Http; -using System.Text; using System.Threading; using System.Threading.Tasks; using Flurl.Http; @@ -52,6 +50,21 @@ public async Task can_add_middleware() { Assert.AreEqual("blocked by flurl!", resp); } + [Test] + public void inner_hanlder_is_SocketsHttpHandler_when_supported() { + var supported = typeof(HttpClientHandler).Assembly.DefinedTypes.Any(t => t.Name == "SocketsHttpHandler"); +#if NET + Assert.IsTrue(supported, "SocketsHttpHandler should be defined"); // sanity check (tests the test, basically) +#endif + HttpMessageHandler handler = null; + new FlurlClientBuilder() + .ConfigureInnerHandler(h => handler = h) + .Build(); + + var expected = supported ? "SocketsHttpHandler" : "HttpClientHandler"; + Assert.AreEqual(expected, handler?.GetType().Name); + } + class BlockingHandler : DelegatingHandler { private readonly string _msg; From d1836f92ca8b0b0fc34ffe356e38c60afdc9e8b1 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 20 Oct 2023 12:49:58 -0500 Subject: [PATCH 4/9] troubleshooting gh build --- test/Flurl.Test/Http/FlurlClientBuilderTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs index 527fdd42..03782320 100644 --- a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -52,10 +52,15 @@ public async Task can_add_middleware() { [Test] public void inner_hanlder_is_SocketsHttpHandler_when_supported() { - var supported = typeof(HttpClientHandler).Assembly.DefinedTypes.Any(t => t.Name == "SocketsHttpHandler"); + var shh = typeof(HttpClientHandler).Assembly.DefinedTypes.FirstOrDefault(t => t.Name == "SocketsHttpHandler"); + var supported = (shh != null); #if NET Assert.IsTrue(supported, "SocketsHttpHandler should be defined"); // sanity check (tests the test, basically) #endif + if (supported) { + Console.WriteLine($"{shh.FullName} Found in {typeof(HttpClientHandler).Assembly.FullName}"); + } + HttpMessageHandler handler = null; new FlurlClientBuilder() .ConfigureInnerHandler(h => handler = h) From b8e328d5ee8e68d0f559b5e0c220e41423c7b3b3 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 20 Oct 2023 13:19:38 -0500 Subject: [PATCH 5/9] exists but not supported? --- test/Flurl.Test/Http/FlurlClientBuilderTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs index 03782320..8e0a0312 100644 --- a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -59,6 +59,8 @@ public void inner_hanlder_is_SocketsHttpHandler_when_supported() { #endif if (supported) { Console.WriteLine($"{shh.FullName} Found in {typeof(HttpClientHandler).Assembly.FullName}"); + supported = (bool)shh.GetProperty("IsSupported").GetValue(Activator.CreateInstance(shh)); + Console.WriteLine($"IsSupported = {supported}"); } HttpMessageHandler handler = null; From c25e83518c7f338b81c76abb2c3c226261936aac Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 20 Oct 2023 13:34:28 -0500 Subject: [PATCH 6/9] trying again --- test/Flurl.Test/Http/FlurlClientBuilderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs index 8e0a0312..a2fa0667 100644 --- a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -59,7 +59,7 @@ public void inner_hanlder_is_SocketsHttpHandler_when_supported() { #endif if (supported) { Console.WriteLine($"{shh.FullName} Found in {typeof(HttpClientHandler).Assembly.FullName}"); - supported = (bool)shh.GetProperty("IsSupported").GetValue(Activator.CreateInstance(shh)); + supported = shh.GetProperty("IsSupported")?.GetValue(Activator.CreateInstance(shh)) as bool? == true; Console.WriteLine($"IsSupported = {supported}"); } From b513532aae733894af81785941e485f656f51919 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 20 Oct 2023 15:54:40 -0500 Subject: [PATCH 7/9] test got a bit too convoluted for my tastes, this is good enough --- .../Http/FlurlClientBuilderTests.cs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs index a2fa0667..1fe97be5 100644 --- a/test/Flurl.Test/Http/FlurlClientBuilderTests.cs +++ b/test/Flurl.Test/Http/FlurlClientBuilderTests.cs @@ -52,24 +52,15 @@ public async Task can_add_middleware() { [Test] public void inner_hanlder_is_SocketsHttpHandler_when_supported() { - var shh = typeof(HttpClientHandler).Assembly.DefinedTypes.FirstOrDefault(t => t.Name == "SocketsHttpHandler"); - var supported = (shh != null); -#if NET - Assert.IsTrue(supported, "SocketsHttpHandler should be defined"); // sanity check (tests the test, basically) -#endif - if (supported) { - Console.WriteLine($"{shh.FullName} Found in {typeof(HttpClientHandler).Assembly.FullName}"); - supported = shh.GetProperty("IsSupported")?.GetValue(Activator.CreateInstance(shh)) as bool? == true; - Console.WriteLine($"IsSupported = {supported}"); - } - HttpMessageHandler handler = null; new FlurlClientBuilder() .ConfigureInnerHandler(h => handler = h) .Build(); - - var expected = supported ? "SocketsHttpHandler" : "HttpClientHandler"; - Assert.AreEqual(expected, handler?.GetType().Name); +#if NET + Assert.IsInstanceOf(handler); +#else + Assert.IsInstanceOf(handler); +#endif } class BlockingHandler : DelegatingHandler From fe2515af71eea497668823247c4d18599460bbba Mon Sep 17 00:00:00 2001 From: Todd Date: Sun, 22 Oct 2023 11:14:50 -0500 Subject: [PATCH 8/9] minor renames related to #770 --- .../Configuration/FlurlClientBuilder.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs index 7e01bb36..1459f633 100644 --- a/src/Flurl.Http/Configuration/FlurlClientBuilder.cs +++ b/src/Flurl.Http/Configuration/FlurlClientBuilder.cs @@ -13,20 +13,20 @@ public interface IFlurlClientBuilder /// /// Configure the IFlurlClient's Settings. /// - IFlurlClientBuilder WithSettings(Action configAction); + IFlurlClientBuilder WithSettings(Action configure); /// /// Configure the HttpClient wrapped by this IFlurlClient. /// - IFlurlClientBuilder ConfigureHttpClient(Action configAction); + IFlurlClientBuilder ConfigureHttpClient(Action configure); /// /// Configure the inner-most HttpMessageHandler associated with this IFlurlClient. /// #if NETCOREAPP2_1_OR_GREATER - IFlurlClientBuilder ConfigureInnerHandler(Action configAction); + IFlurlClientBuilder ConfigureInnerHandler(Action configure); #else - IFlurlClientBuilder ConfigureInnerHandler(Action configAction); + IFlurlClientBuilder ConfigureInnerHandler(Action configure); #endif /// @@ -70,8 +70,8 @@ internal FlurlClientBuilder(IFlurlClientFactory factory, string baseUrl) { } /// - public IFlurlClientBuilder WithSettings(Action configAction) { - _configSettings.Add(configAction); + public IFlurlClientBuilder WithSettings(Action configure) { + _configSettings.Add(configure); return this; } @@ -82,18 +82,18 @@ public IFlurlClientBuilder AddMiddleware(Func create) { } /// - public IFlurlClientBuilder ConfigureHttpClient(Action configAction) { - _configClient.Add(configAction); + public IFlurlClientBuilder ConfigureHttpClient(Action configure) { + _configClient.Add(configure); return this; } /// #if NETCOREAPP2_1_OR_GREATER - public IFlurlClientBuilder ConfigureInnerHandler(Action configAction) { + public IFlurlClientBuilder ConfigureInnerHandler(Action configure) { #else - public IFlurlClientBuilder ConfigureInnerHandler(Action configAction) { + public IFlurlClientBuilder ConfigureInnerHandler(Action configure) { #endif - _handlerBuilder.Configs.Add(configAction); + _handlerBuilder.Configs.Add(configure); return this; } From 8dbd2b933dbbd8999e92c29f0a859ae1712d7357 Mon Sep 17 00:00:00 2001 From: Todd Date: Sun, 22 Oct 2023 11:16:57 -0500 Subject: [PATCH 9/9] version bump - 4.0-pre5 --- src/Flurl.Http/Flurl.Http.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Flurl.Http/Flurl.Http.csproj b/src/Flurl.Http/Flurl.Http.csproj index 906bd5fe..607cdcfe 100644 --- a/src/Flurl.Http/Flurl.Http.csproj +++ b/src/Flurl.Http/Flurl.Http.csproj @@ -4,7 +4,7 @@ 9.0 True Flurl.Http - 4.0.0-pre4 + 4.0.0-pre5 Todd Menier A fluent, portable, testable HTTP client library. https://flurl.dev