Skip to content

Commit

Permalink
API feedback (#1327)
Browse files Browse the repository at this point in the history
  • Loading branch information
martintmk authored Jun 19, 2023
1 parent 6260f7e commit 91e5384
Show file tree
Hide file tree
Showing 22 changed files with 152 additions and 134 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public void Setup()
}

[Benchmark]
public void Get_Ok() => _provider!.Get("dummy");
public void Get_Ok() => _provider!.GetStrategy("dummy");

[Benchmark]
public void Get_Generic_Ok() => _provider!.Get<string>("dummy");
public void Get_Generic_Ok() => _provider!.GetStrategy<string>("dummy");
}
2 changes: 1 addition & 1 deletion bench/Polly.Core.Benchmarks/TelemetryBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private ResilienceStrategy Build(ResilienceStrategyBuilder builder)
});
}

builder.EnableTelemetry(options);
builder.ConfigureTelemetry(options);
}

return builder.Build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal static partial class Helper

if (telemetry)
{
builder.EnableTelemetry(NullLoggerFactory.Instance);
builder.ConfigureTelemetry(NullLoggerFactory.Instance);
}
}),
_ => throw new NotSupportedException()
Expand Down
2 changes: 1 addition & 1 deletion samples/GenericStrategies/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
var response = await strategy.ExecuteAsync(
async token =>
{
await Task.Delay(10);
await Task.Delay(10, token);
// This causes the action fail, thus using the fallback strategy above
return new HttpResponseMessage(HttpStatusCode.InternalServerError);
},
Expand Down
4 changes: 2 additions & 2 deletions samples/Intro/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
strategy.Execute(() => { });

// Asynchronously
await strategy.ExecuteAsync(async token => { await Task.Delay(10); }, CancellationToken.None);
await strategy.ExecuteAsync(async token => { await Task.Delay(10, token); }, CancellationToken.None);

// Synchronously with result
strategy.Execute(token => "some-result");

// Asynchronously with result
await strategy.ExecuteAsync(async token => { await Task.Delay(10); return "some-result"; }, CancellationToken.None);
await strategy.ExecuteAsync(async token => { await Task.Delay(10, token); return "some-result"; }, CancellationToken.None);

// Use state to avoid lambda allocation
strategy.Execute(static state => state, "my-state");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public static ResilienceStrategyBuilder AddSimpleCircuitBreaker(this ResilienceS
private static TBuilder AddAdvancedCircuitBreakerCore<TBuilder, TResult>(this TBuilder builder, AdvancedCircuitBreakerStrategyOptions<TResult> options)
where TBuilder : ResilienceStrategyBuilderBase
{
builder.AddStrategy(
return builder.AddStrategy(
context =>
{
var behavior = new AdvancedCircuitBehavior(
Expand All @@ -113,15 +113,12 @@ private static TBuilder AddAdvancedCircuitBreakerCore<TBuilder, TResult>(this TB
return CreateStrategy(context, options, behavior);
},
options);

return builder;
}

private static TBuilder AddSimpleCircuitBreakerCore<TBuilder, TResult>(this TBuilder builder, SimpleCircuitBreakerStrategyOptions<TResult> options)
where TBuilder : ResilienceStrategyBuilderBase
{
builder.AddStrategy(context => CreateStrategy(context, options, new ConsecutiveFailuresCircuitBehavior(options.FailureThreshold)), options);
return builder;
return builder.AddStrategy(context => CreateStrategy(context, options, new ConsecutiveFailuresCircuitBehavior(options.FailureThreshold)), options);
}

internal static CircuitBreakerResilienceStrategy CreateStrategy<TResult>(ResilienceStrategyBuilderContext context, CircuitBreakerStrategyOptions<TResult> options, CircuitBehavior behavior)
Expand Down
12 changes: 6 additions & 6 deletions src/Polly.Core/Registry/ResilienceStrategyProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ public abstract class ResilienceStrategyProvider<TKey>
/// <param name="key">The key used to identify the resilience strategy.</param>
/// <returns>The resilience strategy associated with the specified key.</returns>
/// <exception cref="KeyNotFoundException">Thrown when no resilience strategy is found for the specified key.</exception>
public virtual ResilienceStrategy Get(TKey key)
public virtual ResilienceStrategy GetStrategy(TKey key)
{
if (TryGet(key, out var strategy))
if (TryGetStrategy(key, out var strategy))
{
return strategy;
}
Expand All @@ -35,9 +35,9 @@ public virtual ResilienceStrategy Get(TKey key)
/// <param name="key">The key used to identify the resilience strategy.</param>
/// <returns>The resilience strategy associated with the specified key.</returns>
/// <exception cref="KeyNotFoundException">Thrown when no resilience strategy is found for the specified key.</exception>
public virtual ResilienceStrategy<TResult> Get<TResult>(TKey key)
public virtual ResilienceStrategy<TResult> GetStrategy<TResult>(TKey key)
{
if (TryGet<TResult>(key, out var strategy))
if (TryGetStrategy<TResult>(key, out var strategy))
{
return strategy;
}
Expand All @@ -52,7 +52,7 @@ public virtual ResilienceStrategy<TResult> Get<TResult>(TKey key)
/// <param name="key">The key used to identify the resilience strategy.</param>
/// <param name="strategy">The output resilience strategy if found, <see langword="null"/> otherwise.</param>
/// <returns><see langword="true"/> if the strategy was found, <see langword="false"/> otherwise.</returns>
public abstract bool TryGet(TKey key, [NotNullWhen(true)] out ResilienceStrategy? strategy);
public abstract bool TryGetStrategy(TKey key, [NotNullWhen(true)] out ResilienceStrategy? strategy);

/// <summary>
/// Tries to get a generic resilience strategy from the provider using the specified key.
Expand All @@ -61,5 +61,5 @@ public virtual ResilienceStrategy<TResult> Get<TResult>(TKey key)
/// <param name="key">The key used to identify the resilience strategy.</param>
/// <param name="strategy">The output resilience strategy if found, <see langword="null"/> otherwise.</param>
/// <returns><see langword="true"/> if the strategy was found, <see langword="false"/> otherwise.</returns>
public abstract bool TryGet<TResult>(TKey key, [NotNullWhen(true)] out ResilienceStrategy<TResult>? strategy);
public abstract bool TryGetStrategy<TResult>(TKey key, [NotNullWhen(true)] out ResilienceStrategy<TResult>? strategy);
}
16 changes: 8 additions & 8 deletions src/Polly.Core/Registry/ResilienceStrategyRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public ResilienceStrategyRegistry(ResilienceStrategyRegistryOptions<TKey> option
/// <param name="strategy">The resilience strategy instance.</param>
/// <returns><see langword="true"/> if the strategy was added successfully, <see langword="false"/> otherwise.</returns>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="strategy"/> is <see langword="null"/>.</exception>
public bool TryAdd(TKey key, ResilienceStrategy strategy)
public bool TryAddStrategy(TKey key, ResilienceStrategy strategy)
{
Guard.NotNull(strategy);

Expand All @@ -80,7 +80,7 @@ public bool TryAdd(TKey key, ResilienceStrategy strategy)
/// <param name="strategy">The resilience strategy instance.</param>
/// <returns><see langword="true"/> if the strategy was added successfully, <see langword="false"/> otherwise.</returns>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="strategy"/> is <see langword="null"/>.</exception>
public bool TryAdd<TResult>(TKey key, ResilienceStrategy<TResult> strategy)
public bool TryAddStrategy<TResult>(TKey key, ResilienceStrategy<TResult> strategy)
{
Guard.NotNull(strategy);

Expand All @@ -92,24 +92,24 @@ public bool TryAdd<TResult>(TKey key, ResilienceStrategy<TResult> strategy)
/// </summary>
/// <param name="key">The key used to identify the resilience strategy.</param>
/// <returns><see langword="true"/> if the strategy was removed successfully, <see langword="false"/> otherwise.</returns>
public bool Remove(TKey key) => _strategies.TryRemove(key, out _);
public bool RemoveStrategy(TKey key) => _strategies.TryRemove(key, out _);

/// <summary>
/// Removes a generic resilience strategy from the registry.
/// </summary>
/// <typeparam name="TResult">The type of result that the resilience strategy handles.</typeparam>
/// <param name="key">The key used to identify the resilience strategy.</param>
/// <returns><see langword="true"/> if the strategy was removed successfully, <see langword="false"/> otherwise.</returns>
public bool Remove<TResult>(TKey key) => GetGenericRegistry<TResult>().Remove(key);
public bool RemoveStrategy<TResult>(TKey key) => GetGenericRegistry<TResult>().Remove(key);

/// <inheritdoc/>
public override bool TryGet<TResult>(TKey key, [NotNullWhen(true)] out ResilienceStrategy<TResult>? strategy)
public override bool TryGetStrategy<TResult>(TKey key, [NotNullWhen(true)] out ResilienceStrategy<TResult>? strategy)
{
return GetGenericRegistry<TResult>().TryGet(key, out strategy);
}

/// <inheritdoc/>
public override bool TryGet(TKey key, [NotNullWhen(true)] out ResilienceStrategy? strategy)
public override bool TryGetStrategy(TKey key, [NotNullWhen(true)] out ResilienceStrategy? strategy)
{
if (_strategies.TryGetValue(key, out strategy))
{
Expand Down Expand Up @@ -192,7 +192,7 @@ public bool TryAddBuilder<TResult>(TKey key, Action<ResilienceStrategyBuilder<TR
/// <remarks>
/// This method only clears the cached strategies, the registered builders are kept unchanged.
/// </remarks>
public void Clear() => _strategies.Clear();
public void ClearStrategies() => _strategies.Clear();

/// <summary>
/// Clears all cached generic strategies.
Expand All @@ -201,7 +201,7 @@ public bool TryAddBuilder<TResult>(TKey key, Action<ResilienceStrategyBuilder<TR
/// <remarks>
/// This method only clears the cached strategies, the registered builders are kept unchanged.
/// </remarks>
public void Clear<TResult>() => GetGenericRegistry<TResult>().Clear();
public void ClearStrategies<TResult>() => GetGenericRegistry<TResult>().Clear();

private static ResilienceStrategy CreateStrategy<TBuilder>(
Func<TBuilder> activator,
Expand Down
10 changes: 1 addition & 9 deletions src/Polly.Core/ResilienceStrategyBuilderBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,7 @@ private protected ResilienceStrategyBuilderBase(ResilienceStrategyBuilderBase ot

internal abstract bool IsGenericBuilder { get; }

/// <summary>
/// Adds a strategy to the builder.
/// </summary>
/// <param name="factory">The factory that creates a resilience strategy.</param>
/// <param name="options">The options associated with the strategy. If none are provided the default instance of <see cref="ResilienceStrategyOptions"/> is created.</param>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="factory"/> is null.</exception>
/// <exception cref="InvalidOperationException">Thrown when this builder was already used to create a strategy. The builder cannot be modified after it has been used.</exception>
/// <exception cref="ValidationException">Thrown when the <paramref name="options"/> are invalid.</exception>
public void AddStrategy(Func<ResilienceStrategyBuilderContext, ResilienceStrategy> factory, ResilienceStrategyOptions options)
internal void AddStrategyCore(Func<ResilienceStrategyBuilderContext, ResilienceStrategy> factory, ResilienceStrategyOptions options)
{
Guard.NotNull(factory);
Guard.NotNull(options);
Expand Down
25 changes: 24 additions & 1 deletion src/Polly.Core/ResilienceStrategyBuilderExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.ComponentModel.DataAnnotations;

namespace Polly;

/// <summary>
Expand All @@ -20,7 +22,28 @@ public static TBuilder AddStrategy<TBuilder>(this TBuilder builder, ResilienceSt
Guard.NotNull(builder);
Guard.NotNull(strategy);

builder.AddStrategy(_ => strategy, EmptyOptions.Instance);
return builder.AddStrategy(_ => strategy, EmptyOptions.Instance);
}

/// <summary>
/// Adds a strategy to the builder.
/// </summary>
/// <typeparam name="TBuilder">The builder type.</typeparam>
/// <param name="builder">The builder instance.</param>
/// <param name="factory">The factory that creates a resilience strategy.</param>
/// <param name="options">The options associated with the strategy. If none are provided the default instance of <see cref="ResilienceStrategyOptions"/> is created.</param>
/// <returns>The same builder instance.</returns>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/>, <paramref name="factory"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
/// <exception cref="InvalidOperationException">Thrown when this builder was already used to create a strategy. The builder cannot be modified after it has been used.</exception>
/// <exception cref="ValidationException">Thrown when <paramref name="options"/> is invalid.</exception>
public static TBuilder AddStrategy<TBuilder>(this TBuilder builder, Func<ResilienceStrategyBuilderContext, ResilienceStrategy> factory, ResilienceStrategyOptions options)
where TBuilder : ResilienceStrategyBuilderBase
{
Guard.NotNull(builder);
Guard.NotNull(factory);
Guard.NotNull(options);

builder.AddStrategyCore(factory, options);
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static ResilienceStrategyBuilder<TResult> AddRetry<TResult>(this Resilien
private static TBuilder AddRetryCore<TBuilder, TResult>(this TBuilder builder, RetryStrategyOptions<TResult> options)
where TBuilder : ResilienceStrategyBuilderBase
{
builder.AddStrategy(context =>
return builder.AddStrategy(context =>
new RetryResilienceStrategy(
options.BaseDelay,
options.BackoffType,
Expand All @@ -90,8 +90,6 @@ private static TBuilder AddRetryCore<TBuilder, TResult>(this TBuilder builder, R
context.Telemetry,
RandomUtil.Instance),
options);

return builder;
}

private static void ConfigureShouldRetry<TResult>(Action<PredicateBuilder<TResult>> shouldRetry, RetryStrategyOptions<TResult> options)
Expand Down
9 changes: 9 additions & 0 deletions src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
/// Gets or sets the type of the back-off.
/// </summary>
/// <remarks>
/// <para>
/// This property is ignored when <see cref="RetryDelayGenerator"/> is set.
/// </para>
/// Defaults to <see cref="RetryBackoffType.Constant"/>.
/// </remarks>
public RetryBackoffType BackoffType { get; set; } = RetryConstants.DefaultBackoffType;
Expand All @@ -51,6 +54,9 @@ public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
/// </item>
/// </list>
/// <para>
/// This property is ignored when <see cref="RetryDelayGenerator"/> is set.
/// </para>
/// <para>
/// Defaults to 2 seconds.
/// </para>
/// </remarks>
Expand All @@ -71,6 +77,9 @@ public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
/// Gets or sets the generator instance that is used to calculate the time between retries.
/// </summary>
/// <remarks>
/// <para>
/// The generator has precedence over <see cref="BaseDelay"/> and <see cref="BackoffType"/>.
/// </para>
/// Defaults to <see langword="null"/>.
/// </remarks>
public Func<OutcomeArguments<TResult, RetryDelayArguments>, ValueTask<TimeSpan>>? RetryDelayGenerator { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ private static void AddResilienceStrategyBuilder(this IServiceCollection service
{
var builder = new ResilienceStrategyBuilder();
builder.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider);
builder.EnableTelemetry(serviceProvider.GetRequiredService<IOptions<TelemetryOptions>>().Value);
builder.ConfigureTelemetry(serviceProvider.GetRequiredService<IOptions<TelemetryOptions>>().Value);
return builder;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ public static class TelemetryResilienceStrategyBuilderExtensions
/// Additionally, the telemetry strategy that logs and meters the executions is added to the beginning of the strategy pipeline.
/// </remarks>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="loggerFactory"/> is <see langword="null"/>.</exception>
public static TBuilder EnableTelemetry<TBuilder>(this TBuilder builder, ILoggerFactory loggerFactory)
public static TBuilder ConfigureTelemetry<TBuilder>(this TBuilder builder, ILoggerFactory loggerFactory)
where TBuilder : ResilienceStrategyBuilderBase
{
Guard.NotNull(builder);
Guard.NotNull(loggerFactory);

return builder.EnableTelemetry(new TelemetryOptions { LoggerFactory = loggerFactory });
return builder.ConfigureTelemetry(new TelemetryOptions { LoggerFactory = loggerFactory });
}

/// <summary>
Expand All @@ -43,7 +43,7 @@ public static TBuilder EnableTelemetry<TBuilder>(this TBuilder builder, ILoggerF
/// Additionally, the telemetry strategy that logs and meters the executions is added to the beginning of the strategy pipeline.
/// </remarks>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
public static TBuilder EnableTelemetry<TBuilder>(this TBuilder builder, TelemetryOptions options)
public static TBuilder ConfigureTelemetry<TBuilder>(this TBuilder builder, TelemetryOptions options)
where TBuilder : ResilienceStrategyBuilderBase
{
Guard.NotNull(builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public static TBuilder AddRateLimiter<TBuilder>(
Guard.NotNull(builder);
Guard.NotNull(options);

builder.AddStrategy(
return builder.AddStrategy(
context =>
{
return new RateLimiterResilienceStrategy(
Expand All @@ -110,6 +110,5 @@ public static TBuilder AddRateLimiter<TBuilder>(
context.Telemetry);
},
options);
return builder;
}
}
Loading

0 comments on commit 91e5384

Please sign in to comment.