Skip to content

Commit

Permalink
#770 remove potentially confusing IFlurlClientCache.Add overload
Browse files Browse the repository at this point in the history
  • Loading branch information
Todd committed Dec 11, 2023
1 parent ad46151 commit 7314cf3
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 34 deletions.
31 changes: 6 additions & 25 deletions src/Flurl.Http/Configuration/FlurlClientCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ public interface IFlurlClientCache
/// </summary>
/// <param name="name">Name of the IFlurlClient. Serves as a cache key. Subsequent calls to Get will return this client.</param>
/// <param name="baseUrl">Optional. The base URL associated with the new client.</param>
/// <returns>A builder to further configure the new client.</returns>
IFlurlClientBuilder Add(string name, string baseUrl = null);
/// <param name="configure">Optional. Configure the builder associated with the added client.</param>
/// <returns>This IFlurlCache.</returns>
IFlurlClientCache Add(string name, string baseUrl = null, Action<IFlurlClientBuilder> configure = null);

/// <summary>
/// Gets a preconfigured named IFlurlClient.
Expand Down Expand Up @@ -54,27 +55,6 @@ public interface IFlurlClientCache
IFlurlClientCache Clear();
}

/// <summary>
/// Extension methods on IFlurlClientCache.
/// </summary>
public static class IFlurlClientCacheExtensions
{
/// <summary>
/// Adds a new IFlurlClient to this cache. Call once per client at startup to register and configure a named client.
/// Allows configuring via a nested lambda, rather than returning a builder, so multiple Add calls can be fluently chained.
/// </summary>
/// <param name="cache">This IFlurlCache</param>
/// <param name="name">Name of the IFlurlClient. Serves as a cache key. Subsequent calls to Get will return this client.</param>
/// <param name="baseUrl">The base URL associated with the new client.</param>
/// <param name="configure">Configure the builder associated with the added client.</param>
/// <returns>This IFlurlCache.</returns>
public static IFlurlClientCache Add(this IFlurlClientCache cache, string name, string baseUrl, Action<IFlurlClientBuilder> configure) {
var builder = cache.Add(name, baseUrl);
configure?.Invoke(builder);
return cache;
}
}

/// <summary>
/// Default implementation of IFlurlClientCache.
/// </summary>
Expand All @@ -84,15 +64,16 @@ public class FlurlClientCache : IFlurlClientCache
private readonly List<Action<IFlurlClientBuilder>> _defaultConfigs = new();

/// <inheritdoc />
public IFlurlClientBuilder Add(string name, string baseUrl = null) {
public IFlurlClientCache Add(string name, string baseUrl = null, Action<IFlurlClientBuilder> configure = null) {
if (name == null)
throw new ArgumentNullException(nameof(name));

var builder = CreateBuilder(baseUrl);
if (!_clients.TryAdd(name, new Lazy<IFlurlClient>(builder.Build)))
throw new ArgumentException($"A client named '{name}' was already registered. Add should be called just once per client at startup.");

return builder;
configure?.Invoke(builder);
return this;
}

/// <inheritdoc />
Expand Down
6 changes: 5 additions & 1 deletion src/Flurl.Http/FlurlHttp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ public static class FlurlHttp
/// 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.
/// </summary>
public static IFlurlClientBuilder ConfigureClientForUrl(string url) => Clients.Add(_cachingStrategy(new FlurlRequest(url)));
public static IFlurlClientBuilder ConfigureClientForUrl(string url) {
IFlurlClientBuilder builder = null;
Clients.Add(_cachingStrategy(new FlurlRequest(url)), null, b => builder = b);
return builder;
}

/// <summary>
/// Gets or creates the IFlurlClient that would be selected for sending the given IFlurlRequest when the clientless pattern is used.
Expand Down
15 changes: 7 additions & 8 deletions test/Flurl.Test/Http/FlurlClientCacheTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ 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));
cache.Add("github", "https://api.github.com", builder =>
builder.WithSettings(s => s.Timeout = TimeSpan.FromSeconds(123)));
cache.Add("google", "https://api.google.com", builder =>
builder.WithSettings(s => s.Timeout = TimeSpan.FromSeconds(234)));

var gh = cache.Get("github");
Assert.AreEqual("https://api.github.com", gh.BaseUrl);
Expand Down Expand Up @@ -68,12 +70,9 @@ public void can_configure_defaults() {

var cli1 = cache.GetOrAdd("foo");

cache.Add("bar").WithSettings(s => {
s.Timeout = TimeSpan.FromSeconds(456);
});
cache.WithDefaults(b => b.WithSettings(s => {
s.Timeout = TimeSpan.FromSeconds(789);
}));
cache
.Add("bar", null, builder => builder.WithSettings(s => s.Timeout = TimeSpan.FromSeconds(456)))
.WithDefaults(b => b.WithSettings(s => s.Timeout = TimeSpan.FromSeconds(789)));

var cli2 = cache.GetOrAdd("bar");
var cli3 = cache.GetOrAdd("buzz");
Expand Down

0 comments on commit 7314cf3

Please sign in to comment.