Skip to content

Commit

Permalink
API Review Feedback (1) (#1420)
Browse files Browse the repository at this point in the history
  • Loading branch information
martintmk committed Jul 21, 2023
1 parent 284e3d1 commit 02a1944
Show file tree
Hide file tree
Showing 60 changed files with 69 additions and 321 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@

internal sealed class EmptyResilienceOptions : ResilienceStrategyOptions
{
public override string StrategyType => "Empty";
}
2 changes: 0 additions & 2 deletions src/Polly.Core/CircuitBreaker/CircuitBreakerConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ namespace Polly.CircuitBreaker;

internal static class CircuitBreakerConstants
{
public const string StrategyType = "CircuitBreaker";

public const string OnCircuitClosed = "OnCircuitClosed";

public const string OnHalfOpenEvent = "OnCircuitHalfOpened";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ namespace Polly.CircuitBreaker;
/// </remarks>
public abstract class CircuitBreakerStrategyOptions<TResult> : ResilienceStrategyOptions
{
/// <summary>
/// Gets the strategy type.
/// </summary>
/// <remarks>Returns <c>CircuitBreaker</c> value.</remarks>
public sealed override string StrategyType => CircuitBreakerConstants.StrategyType;

#pragma warning disable IL2026 // Addressed with DynamicDependency on ValidationHelper.Validate method
/// <summary>
/// Gets or sets the duration of break the circuit will stay open before resetting.
Expand Down
3 changes: 0 additions & 3 deletions src/Polly.Core/Fallback/FallbackConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,5 @@ namespace Polly.Fallback;
internal static class FallbackConstants
{
public const string OnFallback = "OnFallback";

public const string StrategyType = "Fallback";

}

6 changes: 0 additions & 6 deletions src/Polly.Core/Fallback/FallbackStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ namespace Polly.Fallback;
/// <typeparam name="TResult">The result type.</typeparam>
public class FallbackStrategyOptions<TResult> : ResilienceStrategyOptions
{
/// <summary>
/// Gets the strategy type.
/// </summary>
/// <remarks>Returns <c>Fallback</c> value.</remarks>
public sealed override string StrategyType => FallbackConstants.StrategyType;

/// <summary>
/// Gets or sets the outcome predicate for determining whether a fallback should be executed.
/// </summary>
Expand Down
2 changes: 0 additions & 2 deletions src/Polly.Core/Hedging/HedgingConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ namespace Polly.Hedging;

internal static class HedgingConstants
{
public const string StrategyType = "Hedging";

public const string OnHedgingEventName = "OnHedging";

public const int DefaultMaxHedgedAttempts = 2;
Expand Down
6 changes: 0 additions & 6 deletions src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ namespace Polly.Hedging;
/// <typeparam name="TResult">The type of result these hedging options handle.</typeparam>
public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
{
/// <summary>
/// Gets the strategy type.
/// </summary>
/// <remarks>Returns <c>Hedging</c> value.</remarks>
public sealed override string StrategyType => HedgingConstants.StrategyType;

/// <summary>
/// Gets or sets the minimal time of waiting before spawning a new hedged call.
/// </summary>
Expand Down
17 changes: 3 additions & 14 deletions src/Polly.Core/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,13 @@
abstract Polly.Registry.ResilienceStrategyProvider<TKey>.TryGetStrategy(TKey key, out Polly.ResilienceStrategy? strategy) -> bool
abstract Polly.Registry.ResilienceStrategyProvider<TKey>.TryGetStrategy<TResult>(TKey key, out Polly.ResilienceStrategy<TResult>? strategy) -> bool
abstract Polly.ResilienceStrategy.ExecuteCoreAsync<TResult, TState>(System.Func<Polly.ResilienceContext!, TState, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>>! callback, Polly.ResilienceContext! context, TState state) -> System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>
abstract Polly.ResilienceStrategyOptions.StrategyType.get -> string!
const Polly.Retry.RetryStrategyOptions.InfiniteRetryCount = -1 -> int
override Polly.Outcome<TResult>.ToString() -> string!
override Polly.Registry.ResilienceStrategyRegistry<TKey>.TryGetStrategy(TKey key, out Polly.ResilienceStrategy? strategy) -> bool
override Polly.Registry.ResilienceStrategyRegistry<TKey>.TryGetStrategy<TResult>(TKey key, out Polly.ResilienceStrategy<TResult>? strategy) -> bool
override Polly.ResiliencePropertyKey<TValue>.Equals(object? obj) -> bool
override Polly.ResiliencePropertyKey<TValue>.GetHashCode() -> int
override Polly.ResiliencePropertyKey<TValue>.ToString() -> string!
override Polly.Telemetry.ResilienceEvent.ToString() -> string!
override sealed Polly.CircuitBreaker.CircuitBreakerStrategyOptions<TResult>.StrategyType.get -> string!
override sealed Polly.Fallback.FallbackStrategyOptions<TResult>.StrategyType.get -> string!
override sealed Polly.Hedging.HedgingStrategyOptions<TResult>.StrategyType.get -> string!
override sealed Polly.Retry.RetryStrategyOptions<TResult>.StrategyType.get -> string!
override sealed Polly.Timeout.TimeoutStrategyOptions.StrategyType.get -> string!
Polly.CircuitBreaker.AdvancedCircuitBreakerStrategyOptions
Polly.CircuitBreaker.AdvancedCircuitBreakerStrategyOptions.AdvancedCircuitBreakerStrategyOptions() -> void
Polly.CircuitBreaker.AdvancedCircuitBreakerStrategyOptions<TResult>
Expand Down Expand Up @@ -220,7 +213,6 @@ Polly.Registry.ResilienceStrategyRegistryOptions<TKey>.StrategyComparer.get -> S
Polly.Registry.ResilienceStrategyRegistryOptions<TKey>.StrategyComparer.set -> void
Polly.ResilienceContext
Polly.ResilienceContext.CancellationToken.get -> System.Threading.CancellationToken
Polly.ResilienceContext.CancellationToken.set -> void
Polly.ResilienceContext.ContinueOnCapturedContext.get -> bool
Polly.ResilienceContext.ContinueOnCapturedContext.set -> void
Polly.ResilienceContext.IsSynchronous.get -> bool
Expand Down Expand Up @@ -298,13 +290,12 @@ Polly.ResilienceStrategyBuilderContext.BuilderInstanceName.get -> string?
Polly.ResilienceStrategyBuilderContext.BuilderName.get -> string?
Polly.ResilienceStrategyBuilderContext.BuilderProperties.get -> Polly.ResilienceProperties!
Polly.ResilienceStrategyBuilderContext.StrategyName.get -> string?
Polly.ResilienceStrategyBuilderContext.StrategyType.get -> string!
Polly.ResilienceStrategyBuilderContext.Telemetry.get -> Polly.Telemetry.ResilienceStrategyTelemetry!
Polly.ResilienceStrategyBuilderExtensions
Polly.ResilienceStrategyOptions
Polly.ResilienceStrategyOptions.ResilienceStrategyOptions() -> void
Polly.ResilienceStrategyOptions.StrategyName.get -> string?
Polly.ResilienceStrategyOptions.StrategyName.set -> void
Polly.ResilienceStrategyOptions.Name.get -> string?
Polly.ResilienceStrategyOptions.Name.set -> void
Polly.ResilienceValidationContext
Polly.ResilienceValidationContext.Instance.get -> object!
Polly.ResilienceValidationContext.PrimaryMessage.get -> string!
Expand Down Expand Up @@ -381,11 +372,9 @@ Polly.Telemetry.ResilienceTelemetrySource.BuilderName.get -> string?
Polly.Telemetry.ResilienceTelemetrySource.BuilderName.init -> void
Polly.Telemetry.ResilienceTelemetrySource.BuilderProperties.get -> Polly.ResilienceProperties!
Polly.Telemetry.ResilienceTelemetrySource.BuilderProperties.init -> void
Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? BuilderName, string? BuilderInstanceName, Polly.ResilienceProperties! BuilderProperties, string? StrategyName, string! StrategyType) -> void
Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? BuilderName, string? BuilderInstanceName, Polly.ResilienceProperties! BuilderProperties, string? StrategyName) -> void
Polly.Telemetry.ResilienceTelemetrySource.StrategyName.get -> string?
Polly.Telemetry.ResilienceTelemetrySource.StrategyName.init -> void
Polly.Telemetry.ResilienceTelemetrySource.StrategyType.get -> string!
Polly.Telemetry.ResilienceTelemetrySource.StrategyType.init -> void
Polly.Telemetry.TelemetryEventArguments
Polly.Telemetry.TelemetryEventArguments.Arguments.get -> object!
Polly.Telemetry.TelemetryEventArguments.Context.get -> Polly.ResilienceContext!
Expand Down
3 changes: 1 addition & 2 deletions src/Polly.Core/Registry/ResilienceStrategyRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,7 @@ private static ResilienceStrategy CreateStrategy<TBuilder>(
context.BuilderName,
context.BuilderInstanceName,
builder.Properties,
ReloadableResilienceStrategy.StrategyName,
ReloadableResilienceStrategy.StrategyType));
null));
}

private GenericRegistry<TResult> GetGenericRegistry<TResult>()
Expand Down
6 changes: 3 additions & 3 deletions src/Polly.Core/ResilienceContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ private ResilienceContext()
public string? OperationKey { get; private set; }

/// <summary>
/// Gets or sets the <see cref="CancellationToken"/> associated with the execution.
/// Gets the <see cref="CancellationToken"/> associated with the execution.
/// </summary>
public CancellationToken CancellationToken { get; set; }
public CancellationToken CancellationToken { get; internal set; }

/// <summary>
/// Gets a value indicating whether the execution is synchronous.
Expand Down Expand Up @@ -164,7 +164,7 @@ internal bool Reset()
ResultType = typeof(UnknownResult);
ContinueOnCapturedContext = false;
CancellationToken = default;
((IDictionary<string, object?>)Properties).Clear();
Properties.Options.Clear();
_resilienceEvents.Clear();
return true;
}
Expand Down
55 changes: 1 addition & 54 deletions src/Polly.Core/ResilienceProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Polly;
/// <summary>
/// Represents a collection of custom resilience properties.
/// </summary>
public sealed class ResilienceProperties : IDictionary<string, object?>
public sealed class ResilienceProperties
{
internal IDictionary<string, object?> Options { get; set; } = new Dictionary<string, object?>();

Expand Down Expand Up @@ -80,58 +80,5 @@ internal void Replace(ResilienceProperties other)
}

internal void Clear() => Options.Clear();

/// <inheritdoc/>
object? IDictionary<string, object?>.this[string key]
{
get => Options[key];
set => Options[key] = value;
}

/// <inheritdoc/>
ICollection<string> IDictionary<string, object?>.Keys => Options.Keys;

/// <inheritdoc/>
ICollection<object?> IDictionary<string, object?>.Values => Options.Values;

/// <inheritdoc/>
int ICollection<KeyValuePair<string, object?>>.Count => Options.Count;

/// <inheritdoc/>
bool ICollection<KeyValuePair<string, object?>>.IsReadOnly => Options.IsReadOnly;

/// <inheritdoc/>
void IDictionary<string, object?>.Add(string key, object? value) => Options.Add(key, value);

/// <inheritdoc/>
void ICollection<KeyValuePair<string, object?>>.Add(KeyValuePair<string, object?> item) => Options.Add(item);

/// <inheritdoc/>
void ICollection<KeyValuePair<string, object?>>.Clear() => Options.Clear();

/// <inheritdoc/>
bool ICollection<KeyValuePair<string, object?>>.Contains(KeyValuePair<string, object?> item) => Options.Contains(item);

/// <inheritdoc/>
bool IDictionary<string, object?>.ContainsKey(string key) => Options.ContainsKey(key);

/// <inheritdoc/>
void ICollection<KeyValuePair<string, object?>>.CopyTo(KeyValuePair<string, object?>[] array, int arrayIndex) =>
Options.CopyTo(array, arrayIndex);

/// <inheritdoc/>
IEnumerator<KeyValuePair<string, object?>> IEnumerable<KeyValuePair<string, object?>>.GetEnumerator() => Options.GetEnumerator();

/// <inheritdoc/>
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)Options).GetEnumerator();

/// <inheritdoc/>
bool IDictionary<string, object?>.Remove(string key) => Options.Remove(key);

/// <inheritdoc/>
bool ICollection<KeyValuePair<string, object?>>.Remove(KeyValuePair<string, object?> item) => Options.Remove(item);

/// <inheritdoc/>
bool IDictionary<string, object?>.TryGetValue(string key, out object? value) => Options.TryGetValue(key, out value);
}

3 changes: 1 addition & 2 deletions src/Polly.Core/ResilienceStrategyBuilderBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ private ResilienceStrategy CreateResilienceStrategy(Entry entry)
builderName: BuilderName,
builderInstanceName: InstanceName,
builderProperties: Properties,
strategyName: entry.Options.StrategyName,
strategyType: entry.Options.StrategyType,
strategyName: entry.Options.Name,
timeProvider: TimeProvider,
isGenericBuilder: IsGenericBuilder,
diagnosticSource: DiagnosticSource,
Expand Down
9 changes: 1 addition & 8 deletions src/Polly.Core/ResilienceStrategyBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ internal ResilienceStrategyBuilderContext(
string? builderInstanceName,
ResilienceProperties builderProperties,
string? strategyName,
string strategyType,
TimeProvider timeProvider,
bool isGenericBuilder,
DiagnosticSource? diagnosticSource,
Expand All @@ -24,10 +23,9 @@ internal ResilienceStrategyBuilderContext(
BuilderInstanceName = builderInstanceName;
BuilderProperties = builderProperties;
StrategyName = strategyName;
StrategyType = strategyType;
TimeProvider = timeProvider;
IsGenericBuilder = isGenericBuilder;
Telemetry = TelemetryUtil.CreateTelemetry(diagnosticSource, builderName, builderInstanceName, builderProperties, strategyName, strategyType);
Telemetry = TelemetryUtil.CreateTelemetry(diagnosticSource, builderName, builderInstanceName, builderProperties, strategyName);
Randomizer = randomizer;
}

Expand All @@ -51,11 +49,6 @@ internal ResilienceStrategyBuilderContext(
/// </summary>
public string? StrategyName { get; }

/// <summary>
/// Gets the type of the strategy.
/// </summary>
public string StrategyType { get; }

/// <summary>
/// Gets the resilience telemetry used to report important events.
/// </summary>
Expand Down
2 changes: 0 additions & 2 deletions src/Polly.Core/ResilienceStrategyBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,5 @@ public static TBuilder AddStrategy<TBuilder>(this TBuilder builder, Func<Resilie
internal sealed class EmptyOptions : ResilienceStrategyOptions
{
public static readonly EmptyOptions Instance = new();

public override string StrategyType => "Empty";
}
}
13 changes: 3 additions & 10 deletions src/Polly.Core/ResilienceStrategyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,9 @@ public abstract class ResilienceStrategyOptions
/// Gets or sets the name of the strategy.
/// </summary>
/// <remarks>
/// This name uniquely identifies a particular instance of a specific strategy.
/// This property is also included in the telemetry that is produced by the individual resilience strategies.
/// This name uniquely identifies a particular instance of a specific strategy and is also included
/// in the telemetry that is produced by the individual resilience strategies.
/// </remarks>
/// <value>The default value is <see langword="null"/>.</value>
public string? StrategyName { get; set; }

/// <summary>
/// Gets the strategy type.
/// </summary>
/// <remarks>This property is also included in the telemetry that is produced by the individual resilience strategies.
/// The strategy type uniquely identifies the strategy in the telemetry. The name should be in PascalCase (i.e. Retry, CircuitBreaker, Timeout).</remarks>
public abstract string StrategyType { get; }
public string? Name { get; set; }
}
6 changes: 1 addition & 5 deletions src/Polly.Core/Retry/RetryConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,13 @@ namespace Polly.Retry;

internal static class RetryConstants
{
public const string StrategyType = "Retry";

public const string OnRetryEvent = "OnRetry";

public const RetryBackoffType DefaultBackoffType = RetryBackoffType.Constant;

public const int DefaultRetryCount = 3;

public const int MaxRetryCount = 100;

public const int InfiniteRetryCount = -1;
public const int MaxRetryCount = int.MaxValue;

public static readonly TimeSpan DefaultBaseDelay = TimeSpan.FromSeconds(2);
}
10 changes: 1 addition & 9 deletions src/Polly.Core/Retry/RetryResilienceStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,5 @@ protected override async ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(Func
}
}

private bool IsLastAttempt(int attempt)
{
if (RetryCount == RetryStrategyOptions.InfiniteRetryCount)
{
return false;
}

return attempt >= RetryCount;
}
private bool IsLastAttempt(int attempt) => attempt >= RetryCount;
}
10 changes: 2 additions & 8 deletions src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,13 @@ namespace Polly.Retry;
/// <typeparam name="TResult">The type of result the retry strategy handles.</typeparam>
public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
{
/// <summary>
/// Gets the strategy type.
/// </summary>
/// <remarks>Returns <c>Retry</c> value.</remarks>
public sealed override string StrategyType => RetryConstants.StrategyType;

/// <summary>
/// Gets or sets the maximum number of retries to use, in addition to the original call.
/// </summary>
/// <value>
/// The default value is 3 retries. For infinite retries use <see cref="RetryStrategyOptions.InfiniteRetryCount"/> (-1).
/// The default value is 3 retries.
/// </value>
[Range(RetryStrategyOptions.InfiniteRetryCount, RetryConstants.MaxRetryCount)]
[Range(1, RetryConstants.MaxRetryCount)]
public int RetryCount { get; set; } = RetryConstants.DefaultRetryCount;

/// <summary>
Expand Down
4 changes: 0 additions & 4 deletions src/Polly.Core/Retry/RetryStrategyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,4 @@ namespace Polly.Retry;
/// <inheritdoc/>
public class RetryStrategyOptions : RetryStrategyOptions<object>
{
/// <summary>
/// Value that represents infinite retries.
/// </summary>
public const int InfiniteRetryCount = RetryConstants.InfiniteRetryCount;
}
4 changes: 1 addition & 3 deletions src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ namespace Polly.Telemetry;
/// <param name="BuilderInstanceName">The builder instance name.</param>
/// <param name="BuilderProperties">The builder properties.</param>
/// <param name="StrategyName">The strategy name. </param>
/// <param name="StrategyType">The strategy type.</param>
/// <remarks>
/// This class is used by the telemetry infrastructure and should not be used directly by user code.
/// </remarks>
public sealed record class ResilienceTelemetrySource(
string? BuilderName,
string? BuilderInstanceName,
ResilienceProperties BuilderProperties,
string? StrategyName,
string StrategyType);
string? StrategyName);

5 changes: 2 additions & 3 deletions src/Polly.Core/Telemetry/TelemetryUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ public static ResilienceStrategyTelemetry CreateTelemetry(
string? builderName,
string? builderInstanceName,
ResilienceProperties builderProperties,
string? strategyName,
string strategyType)
string? strategyName)
{
var telemetrySource = new ResilienceTelemetrySource(builderName, builderInstanceName, builderProperties, strategyName, strategyType);
var telemetrySource = new ResilienceTelemetrySource(builderName, builderInstanceName, builderProperties, strategyName);

return new ResilienceStrategyTelemetry(telemetrySource, diagnosticSource);
}
Expand Down
Loading

0 comments on commit 02a1944

Please sign in to comment.