Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API feedback #1327

Merged
merged 4 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 the <paramref name="options"/> are invalid.</exception>
martincostello marked this conversation as resolved.
Show resolved Hide resolved
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
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class ResilienceStrategyProviderTests
public void Get_DoesNotExist_Throws()
{
new Provider()
.Invoking(o => o.Get("not-exists"))
.Invoking(o => o.GetStrategy("not-exists"))
.Should()
.Throw<KeyNotFoundException>()
.WithMessage("Unable to find a resilience strategy associated with the key 'not-exists'. Please ensure that either the resilience strategy or the builder is registered.");
Expand All @@ -19,7 +19,7 @@ public void Get_DoesNotExist_Throws()
public void Get_GenericDoesNotExist_Throws()
{
new Provider()
.Invoking(o => o.Get<string>("not-exists"))
.Invoking(o => o.GetStrategy<string>("not-exists"))
.Should()
.Throw<KeyNotFoundException>()
.WithMessage("Unable to find a generic resilience strategy of 'String' associated with the key 'not-exists'. " +
Expand All @@ -31,15 +31,15 @@ public void Get_Exist_Ok()
{
var provider = new Provider { Strategy = new TestResilienceStrategy() };

provider.Get("exists").Should().Be(provider.Strategy);
provider.GetStrategy("exists").Should().Be(provider.Strategy);
}

[Fact]
public void Get_GenericExist_Ok()
{
var provider = new Provider { GenericStrategy = new TestResilienceStrategy<string>() };

provider.Get<string>("exists").Should().Be(provider.GenericStrategy);
provider.GetStrategy<string>("exists").Should().Be(provider.GenericStrategy);
}

private class Provider : ResilienceStrategyProvider<string>
Expand All @@ -48,13 +48,13 @@ private class Provider : ResilienceStrategyProvider<string>

public object? GenericStrategy { get; set; }

public override bool TryGet(string key, [NotNullWhen(true)] out ResilienceStrategy? strategy)
public override bool TryGetStrategy(string key, [NotNullWhen(true)] out ResilienceStrategy? strategy)
{
strategy = Strategy;
return Strategy != null;
}

public override bool TryGet<TResult>(string key, [NotNullWhen(true)] out ResilienceStrategy<TResult>? strategy)
public override bool TryGetStrategy<TResult>(string key, [NotNullWhen(true)] out ResilienceStrategy<TResult>? strategy)
{
strategy = (ResilienceStrategy<TResult>?)GenericStrategy;
return GenericStrategy != null;
Expand Down
Loading