From a3f309c43e11fb07e7a2b2b9ff4f901caa615708 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Mon, 14 Aug 2023 09:26:05 +0200 Subject: [PATCH 1/4] API Review Feedback (2) --- .../CompositeStrategyBuilderBase.cs | 26 ++----------------- src/Polly.Core/PublicAPI.Unshipped.txt | 9 +++---- .../Registry/ResilienceStrategyRegistry.cs | 1 - ...RetryCompositeStrategyBuilderExtensions.cs | 4 +-- .../Retry/RetryResilienceStrategy.cs | 5 ++-- .../Retry/RetryStrategyOptions.TResult.cs | 11 ++++++++ src/Polly.Core/StrategyBuilderContext.cs | 17 ++---------- .../Telemetry/ResilienceTelemetrySource.cs | 8 ------ src/Polly.Core/Telemetry/TelemetryUtil.cs | 3 +-- .../PollyDependencyInjectionKeys.cs | 12 --------- .../PollyServiceCollectionExtensions.cs | 1 - .../CompositeStrategyBuilderContextTests.cs | 6 +---- .../CompositeStrategyBuilderTests.cs | 10 ------- .../GenericCompositeStrategyBuilderTests.cs | 1 - .../ResilienceStrategyRegistryTests.cs | 6 ++--- ...CompositeStrategyBuilderExtensionsTests.cs | 5 ++-- .../Retry/RetryResilienceStrategyTests.cs | 6 ++--- .../Retry/RetryStrategyOptionsTests.cs | 1 + .../ResilienceStrategyTelemetryTests.cs | 5 +--- .../Telemetry/TelemetryEventArgumentsTests.cs | 2 +- .../Telemetry/TelemetryUtilTests.cs | 14 +--------- .../PollyDependencyInjectionKeysTests.cs | 12 --------- .../PollyServiceCollectionExtensionTests.cs | 6 +---- ...s.OnCircuitBreakWithServiceProvider_796.cs | 6 +++-- ...esilienceTelemetryDiagnosticSourceTests.cs | 7 ----- test/Polly.TestUtils/TestUtilities.cs | 7 +++-- 26 files changed, 43 insertions(+), 148 deletions(-) delete mode 100644 src/Polly.Extensions/DependencyInjection/PollyDependencyInjectionKeys.cs delete mode 100644 test/Polly.Extensions.Tests/DependencyInjection/PollyDependencyInjectionKeysTests.cs diff --git a/src/Polly.Core/CompositeStrategyBuilderBase.cs b/src/Polly.Core/CompositeStrategyBuilderBase.cs index a2017cc5eba..7203bca8103 100644 --- a/src/Polly.Core/CompositeStrategyBuilderBase.cs +++ b/src/Polly.Core/CompositeStrategyBuilderBase.cs @@ -26,9 +26,7 @@ private protected CompositeStrategyBuilderBase() private protected CompositeStrategyBuilderBase(CompositeStrategyBuilderBase other) { Name = other.Name; - Properties = other.Properties; TimeProvider = other.TimeProvider; - Randomizer = other.Randomizer; DiagnosticSource = other.DiagnosticSource; } @@ -55,14 +53,6 @@ private protected CompositeStrategyBuilderBase(CompositeStrategyBuilderBase othe /// public string? InstanceName { get; set; } - /// - /// Gets the custom properties attached to builder options. - /// - /// - /// The default value is the empty instance of . - /// - public ResilienceProperties Properties { get; } = new(); - /// /// Gets or sets a that is used by strategies that work with time. /// @@ -87,16 +77,6 @@ private protected CompositeStrategyBuilderBase(CompositeStrategyBuilderBase othe [EditorBrowsable(EditorBrowsableState.Never)] public DiagnosticSource? DiagnosticSource { get; set; } - /// - /// Gets or sets the randomizer that is used by strategies that need to generate random numbers. - /// - /// - /// The default value is thread-safe randomizer that returns values between 0.0 and 1.0. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - [Required] - public Func Randomizer { get; set; } = RandomUtil.Instance.NextDouble; - /// /// Gets the validator that is used for the validation. /// @@ -139,7 +119,7 @@ internal ResilienceStrategy BuildStrategy() return CompositeResilienceStrategy.Create( strategies, - TelemetryUtil.CreateTelemetry(DiagnosticSource, Name, InstanceName, Properties, null), + TelemetryUtil.CreateTelemetry(DiagnosticSource, Name, InstanceName, null), TimeProvider); } @@ -148,11 +128,9 @@ private ResilienceStrategy CreateResilienceStrategy(Entry entry) var context = new StrategyBuilderContext( builderName: Name, builderInstanceName: InstanceName, - builderProperties: Properties, strategyName: entry.Options.Name, timeProvider: TimeProvider, - diagnosticSource: DiagnosticSource, - randomizer: Randomizer); + diagnosticSource: DiagnosticSource); var strategy = entry.Factory(context); strategy.Options = entry.Options; diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index b150181940e..1666a78db41 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/src/Polly.Core/PublicAPI.Unshipped.txt @@ -88,9 +88,6 @@ Polly.CompositeStrategyBuilderBase.InstanceName.get -> string? Polly.CompositeStrategyBuilderBase.InstanceName.set -> void Polly.CompositeStrategyBuilderBase.Name.get -> string? Polly.CompositeStrategyBuilderBase.Name.set -> void -Polly.CompositeStrategyBuilderBase.Properties.get -> Polly.ResilienceProperties! -Polly.CompositeStrategyBuilderBase.Randomizer.get -> System.Func! -Polly.CompositeStrategyBuilderBase.Randomizer.set -> void Polly.CompositeStrategyBuilderBase.Validator.get -> System.Action! Polly.CompositeStrategyBuilderExtensions Polly.ExecutionRejectedException @@ -309,6 +306,8 @@ Polly.Retry.RetryStrategyOptions.BaseDelay.get -> System.TimeSpan Polly.Retry.RetryStrategyOptions.BaseDelay.set -> void Polly.Retry.RetryStrategyOptions.OnRetry.get -> System.Func, System.Threading.Tasks.ValueTask>? Polly.Retry.RetryStrategyOptions.OnRetry.set -> void +Polly.Retry.RetryStrategyOptions.Randomizer.get -> System.Func! +Polly.Retry.RetryStrategyOptions.Randomizer.set -> void Polly.Retry.RetryStrategyOptions.RetryCount.get -> int Polly.Retry.RetryStrategyOptions.RetryCount.set -> void Polly.Retry.RetryStrategyOptions.RetryDelayGenerator.get -> System.Func, System.Threading.Tasks.ValueTask>? @@ -322,7 +321,6 @@ Polly.RetryCompositeStrategyBuilderExtensions Polly.StrategyBuilderContext Polly.StrategyBuilderContext.BuilderInstanceName.get -> string? Polly.StrategyBuilderContext.BuilderName.get -> string? -Polly.StrategyBuilderContext.BuilderProperties.get -> Polly.ResilienceProperties! Polly.StrategyBuilderContext.StrategyName.get -> string? Polly.StrategyBuilderContext.Telemetry.get -> Polly.Telemetry.ResilienceStrategyTelemetry! Polly.Telemetry.ExecutionAttemptArguments @@ -354,8 +352,7 @@ Polly.Telemetry.ResilienceStrategyTelemetry.Report(Polly.Telemetry.Resili Polly.Telemetry.ResilienceTelemetrySource Polly.Telemetry.ResilienceTelemetrySource.BuilderInstanceName.get -> string? Polly.Telemetry.ResilienceTelemetrySource.BuilderName.get -> string? -Polly.Telemetry.ResilienceTelemetrySource.BuilderProperties.get -> Polly.ResilienceProperties! -Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? builderName, string? builderInstanceName, Polly.ResilienceProperties! builderProperties, string? strategyName) -> void +Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? builderName, string? builderInstanceName, string? strategyName) -> void Polly.Telemetry.ResilienceTelemetrySource.StrategyName.get -> string? Polly.Telemetry.TelemetryEventArguments Polly.Telemetry.TelemetryEventArguments.Arguments.get -> object! diff --git a/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs b/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs index 23f5c0d9791..9693fe5ca58 100644 --- a/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs +++ b/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs @@ -297,7 +297,6 @@ private static ResilienceStrategy CreateStrategy( diagnosticSource, context.BuilderName, context.BuilderInstanceName, - builder.Properties, null)); } diff --git a/src/Polly.Core/Retry/RetryCompositeStrategyBuilderExtensions.cs b/src/Polly.Core/Retry/RetryCompositeStrategyBuilderExtensions.cs index 716204d1bb3..72689deb417 100644 --- a/src/Polly.Core/Retry/RetryCompositeStrategyBuilderExtensions.cs +++ b/src/Polly.Core/Retry/RetryCompositeStrategyBuilderExtensions.cs @@ -28,7 +28,7 @@ public static CompositeStrategyBuilder AddRetry(this CompositeStrategyBuilder bu Guard.NotNull(options); return builder.AddStrategy( - context => new RetryResilienceStrategy(options, context.TimeProvider, context.Telemetry, context.Randomizer), + context => new RetryResilienceStrategy(options, context.TimeProvider, context.Telemetry), options); } @@ -53,7 +53,7 @@ public static CompositeStrategyBuilder AddRetry(this CompositeStrategyBuilder bu Guard.NotNull(options); return builder.AddStrategy( - context => new RetryResilienceStrategy(options, context.TimeProvider, context.Telemetry, context.Randomizer), + context => new RetryResilienceStrategy(options, context.TimeProvider, context.Telemetry), options); } } diff --git a/src/Polly.Core/Retry/RetryResilienceStrategy.cs b/src/Polly.Core/Retry/RetryResilienceStrategy.cs index 765b5843360..98decf1e910 100644 --- a/src/Polly.Core/Retry/RetryResilienceStrategy.cs +++ b/src/Polly.Core/Retry/RetryResilienceStrategy.cs @@ -11,8 +11,7 @@ internal sealed class RetryResilienceStrategy : ReactiveResilienceStrategy public RetryResilienceStrategy( RetryStrategyOptions options, TimeProvider timeProvider, - ResilienceStrategyTelemetry telemetry, - Func randomizer) + ResilienceStrategyTelemetry telemetry) { ShouldHandle = options.ShouldHandle; BaseDelay = options.BaseDelay; @@ -24,7 +23,7 @@ public RetryResilienceStrategy( _timeProvider = timeProvider; _telemetry = telemetry; - _randomizer = randomizer; + _randomizer = options.Randomizer; } public TimeSpan BaseDelay { get; } diff --git a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs index ad8cdd19f88..4d5b169af21 100644 --- a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs +++ b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; namespace Polly.Retry; @@ -103,4 +104,14 @@ public class RetryStrategyOptions : ResilienceStrategyOptions /// The default value is . /// public Func, ValueTask>? OnRetry { get; set; } + + /// + /// Gets or sets the randomizer that is used by the retry strategy to generate random numbers. + /// + /// + /// The default value is thread-safe randomizer that returns values between 0.0 and 1.0. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + [Required] + public Func Randomizer { get; set; } = RandomUtil.Instance.NextDouble; } diff --git a/src/Polly.Core/StrategyBuilderContext.cs b/src/Polly.Core/StrategyBuilderContext.cs index f0e65a2f608..7edb9cbd3f2 100644 --- a/src/Polly.Core/StrategyBuilderContext.cs +++ b/src/Polly.Core/StrategyBuilderContext.cs @@ -2,8 +2,6 @@ namespace Polly; -#pragma warning disable S107 - /// /// The context used for building an individual resilience strategy. /// @@ -12,19 +10,15 @@ public sealed class StrategyBuilderContext internal StrategyBuilderContext( string? builderName, string? builderInstanceName, - ResilienceProperties builderProperties, string? strategyName, TimeProvider timeProvider, - DiagnosticSource? diagnosticSource, - Func randomizer) + DiagnosticSource? diagnosticSource) { BuilderName = builderName; BuilderInstanceName = builderInstanceName; - BuilderProperties = builderProperties; StrategyName = strategyName; TimeProvider = timeProvider; - Telemetry = TelemetryUtil.CreateTelemetry(diagnosticSource, builderName, builderInstanceName, builderProperties, strategyName); - Randomizer = randomizer; + Telemetry = TelemetryUtil.CreateTelemetry(diagnosticSource, builderName, builderInstanceName, strategyName); } /// @@ -37,11 +31,6 @@ internal StrategyBuilderContext( /// public string? BuilderInstanceName { get; } - /// - /// Gets the custom properties attached to the builder. - /// - public ResilienceProperties BuilderProperties { get; } - /// /// Gets the name of the strategy. /// @@ -56,6 +45,4 @@ internal StrategyBuilderContext( /// Gets the used by this strategy. /// internal TimeProvider TimeProvider { get; } - - internal Func Randomizer { get; } } diff --git a/src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs b/src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs index 96b134fd002..e75579869fa 100644 --- a/src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs +++ b/src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs @@ -13,17 +13,14 @@ public sealed class ResilienceTelemetrySource /// /// The builder name. /// The builder instance name. - /// The builder properties. /// The strategy name. public ResilienceTelemetrySource( string? builderName, string? builderInstanceName, - ResilienceProperties builderProperties, string? strategyName) { BuilderName = builderName; BuilderInstanceName = builderInstanceName; - BuilderProperties = builderProperties; StrategyName = strategyName; } @@ -37,11 +34,6 @@ public ResilienceTelemetrySource( /// public string? BuilderInstanceName { get; } - /// - /// Gets the builder properties. - /// - public ResilienceProperties BuilderProperties { get; } - /// /// Gets the strategy name. /// diff --git a/src/Polly.Core/Telemetry/TelemetryUtil.cs b/src/Polly.Core/Telemetry/TelemetryUtil.cs index af44bed2d8e..2a140f447e5 100644 --- a/src/Polly.Core/Telemetry/TelemetryUtil.cs +++ b/src/Polly.Core/Telemetry/TelemetryUtil.cs @@ -14,10 +14,9 @@ public static ResilienceStrategyTelemetry CreateTelemetry( DiagnosticSource? diagnosticSource, string? builderName, string? builderInstanceName, - ResilienceProperties builderProperties, string? strategyName) { - var telemetrySource = new ResilienceTelemetrySource(builderName, builderInstanceName, builderProperties, strategyName); + var telemetrySource = new ResilienceTelemetrySource(builderName, builderInstanceName, strategyName); return new ResilienceStrategyTelemetry(telemetrySource, diagnosticSource); } diff --git a/src/Polly.Extensions/DependencyInjection/PollyDependencyInjectionKeys.cs b/src/Polly.Extensions/DependencyInjection/PollyDependencyInjectionKeys.cs deleted file mode 100644 index c4f12831f30..00000000000 --- a/src/Polly.Extensions/DependencyInjection/PollyDependencyInjectionKeys.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace Polly.DependencyInjection; - -/// -/// The resilience keys used in the dependency injection scenarios. -/// -internal static class PollyDependencyInjectionKeys -{ - /// - /// The key used to store and access the from . - /// - public static readonly ResiliencePropertyKey ServiceProvider = new("Polly.DependencyInjection.ServiceProvider"); -} diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index 9d64f847777..4b8ca1be45d 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -251,7 +251,6 @@ private static void AddCompositeStrategyBuilder(this IServiceCollection services services.TryAddTransient(serviceProvider => { var builder = new CompositeStrategyBuilder(); - builder.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider); builder.ConfigureTelemetry(serviceProvider.GetRequiredService>().Value); return builder; }); diff --git a/test/Polly.Core.Tests/CompositeStrategyBuilderContextTests.cs b/test/Polly.Core.Tests/CompositeStrategyBuilderContextTests.cs index 6cc38877fcb..ffb7946eb97 100644 --- a/test/Polly.Core.Tests/CompositeStrategyBuilderContextTests.cs +++ b/test/Polly.Core.Tests/CompositeStrategyBuilderContextTests.cs @@ -8,20 +8,16 @@ public class CompositeStrategyBuilderContextTests [Fact] public void Ctor_EnsureDefaults() { - var properties = new ResilienceProperties(); var timeProvider = new FakeTimeProvider(); - var context = new StrategyBuilderContext("builder-name", "instance", properties, "strategy-name", timeProvider, Substitute.For(), () => 1.0); + var context = new StrategyBuilderContext("builder-name", "instance", "strategy-name", timeProvider, Substitute.For()); context.BuilderName.Should().Be("builder-name"); context.BuilderInstanceName.Should().Be("instance"); - context.BuilderProperties.Should().BeSameAs(properties); context.StrategyName.Should().Be("strategy-name"); context.TimeProvider.Should().Be(timeProvider); context.Telemetry.Should().NotBeNull(); - context.Randomizer.Should().NotBeNull(); context.Telemetry.TelemetrySource.BuilderName.Should().Be("builder-name"); - context.Telemetry.TelemetrySource.BuilderProperties.Should().BeSameAs(properties); context.Telemetry.TelemetrySource.StrategyName.Should().Be("strategy-name"); } } diff --git a/test/Polly.Core.Tests/CompositeStrategyBuilderTests.cs b/test/Polly.Core.Tests/CompositeStrategyBuilderTests.cs index e6e50ba898c..8bcb272f14e 100644 --- a/test/Polly.Core.Tests/CompositeStrategyBuilderTests.cs +++ b/test/Polly.Core.Tests/CompositeStrategyBuilderTests.cs @@ -15,9 +15,7 @@ public void Ctor_EnsureDefaults() var builder = new CompositeStrategyBuilder(); builder.Name.Should().BeNull(); - builder.Properties.Should().NotBeNull(); builder.TimeProvider.Should().Be(TimeProvider.System); - builder.Randomizer.Should().NotBeNull(); } [Fact] @@ -27,18 +25,13 @@ public void CopyCtor_Ok() { TimeProvider = Substitute.For(), Name = "dummy", - Randomizer = () => 0.0, DiagnosticSource = Substitute.For(), }; - builder.Properties.Set(new ResiliencePropertyKey("dummy"), "dummy"); - var other = new CompositeStrategyBuilder(builder); other.Name.Should().Be(builder.Name); other.TimeProvider.Should().Be(builder.TimeProvider); - other.Randomizer.Should().BeSameAs(builder.Randomizer); other.DiagnosticSource.Should().BeSameAs(builder.DiagnosticSource); - other.Properties.GetValue(new ResiliencePropertyKey("dummy"), "").Should().Be("dummy"); } [Fact] @@ -305,10 +298,8 @@ public void BuildStrategy_EnsureCorrectContext() { context.BuilderName.Should().Be("builder-name"); context.StrategyName.Should().Be("strategy-name"); - context.BuilderProperties.Should().BeSameAs(builder.Properties); context.Telemetry.Should().NotBeNull(); context.TimeProvider.Should().Be(builder.TimeProvider); - context.Randomizer.Should().BeSameAs(builder.Randomizer); verified1 = true; return new TestResilienceStrategy(); @@ -320,7 +311,6 @@ public void BuildStrategy_EnsureCorrectContext() { context.BuilderName.Should().Be("builder-name"); context.StrategyName.Should().Be("strategy-name-2"); - context.BuilderProperties.Should().BeSameAs(builder.Properties); context.Telemetry.Should().NotBeNull(); context.TimeProvider.Should().Be(builder.TimeProvider); verified2 = true; diff --git a/test/Polly.Core.Tests/GenericCompositeStrategyBuilderTests.cs b/test/Polly.Core.Tests/GenericCompositeStrategyBuilderTests.cs index 6cc3b69bcac..d697b4fefdc 100644 --- a/test/Polly.Core.Tests/GenericCompositeStrategyBuilderTests.cs +++ b/test/Polly.Core.Tests/GenericCompositeStrategyBuilderTests.cs @@ -11,7 +11,6 @@ public class GenericCompositeStrategyBuilderTests public void Ctor_EnsureDefaults() { _builder.Name.Should().BeNull(); - _builder.Properties.Should().NotBeNull(); _builder.TimeProvider.Should().Be(TimeProvider.System); } diff --git a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs index da6b19f8dae..32f940d8f32 100644 --- a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs @@ -177,10 +177,9 @@ public void AddBuilder_GetStrategy_EnsureCalled() _callback = _ => activatorCalls++; var registry = CreateRegistry(); var called = 0; - registry.TryAddBuilder(StrategyId.Create("A"), (builder, context) => + registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => { builder.AddStrategy(new TestResilienceStrategy()); - builder.Properties.Set(StrategyId.ResilienceKey, context.StrategyKey); called++; }); @@ -206,10 +205,9 @@ public void AddBuilder_GenericGetStrategy_EnsureCalled() _callback = _ => activatorCalls++; var registry = CreateRegistry(); var called = 0; - registry.TryAddBuilder(StrategyId.Create("A"), (builder, context) => + registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => { builder.AddStrategy(new TestResilienceStrategy()); - builder.Properties.Set(StrategyId.ResilienceKey, context.StrategyKey); called++; }); diff --git a/test/Polly.Core.Tests/Retry/RetryCompositeStrategyBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Retry/RetryCompositeStrategyBuilderExtensionsTests.cs index 54f4aa8ea75..06e7571f597 100644 --- a/test/Polly.Core.Tests/Retry/RetryCompositeStrategyBuilderExtensionsTests.cs +++ b/test/Polly.Core.Tests/Retry/RetryCompositeStrategyBuilderExtensionsTests.cs @@ -131,7 +131,7 @@ private static TimeSpan GetAggregatedDelay(RetryStrategyOptions options) { var aggregatedDelay = TimeSpan.Zero; - var strategy = new CompositeStrategyBuilder { Randomizer = () => 1.0 }.AddRetry(new() + var strategy = new CompositeStrategyBuilder().AddRetry(new() { RetryCount = options.RetryCount, BaseDelay = options.BaseDelay, @@ -144,7 +144,8 @@ private static TimeSpan GetAggregatedDelay(RetryStrategyOptions options) // return zero delay, so no waiting return new ValueTask(TimeSpan.Zero); - } + }, + Randomizer = () => 1.0, }) .Build(); diff --git a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs index 1bbadbf297b..db9ff7fa053 100644 --- a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs +++ b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs @@ -18,6 +18,7 @@ public RetryResilienceStrategyTests() { _telemetry = TestUtilities.CreateResilienceTelemetry(_diagnosticSource); _options.ShouldHandle = _ => new ValueTask(false); + _options.Randomizer = () => 1; } @@ -352,8 +353,5 @@ private async ValueTask ExecuteAndAdvance(ReactiveResilienceStrategyBridge< } private ReactiveResilienceStrategyBridge CreateSut(TimeProvider? timeProvider = null) => - new(new RetryResilienceStrategy(_options, - timeProvider ?? _timeProvider, - _telemetry, - () => 1.0)); + new(new RetryResilienceStrategy(_options, timeProvider ?? _timeProvider, _telemetry)); } diff --git a/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs b/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs index 85be2996f9b..d95a47ec0fe 100644 --- a/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs +++ b/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs @@ -21,6 +21,7 @@ public void Ctor_Ok() options.BackoffType.Should().Be(RetryBackoffType.Constant); options.BaseDelay.Should().Be(TimeSpan.FromSeconds(2)); options.Name.Should().Be("Retry"); + options.Randomizer.Should().NotBeNull(); } [Fact] diff --git a/test/Polly.Core.Tests/Telemetry/ResilienceStrategyTelemetryTests.cs b/test/Polly.Core.Tests/Telemetry/ResilienceStrategyTelemetryTests.cs index 446418be4e9..e0814002710 100644 --- a/test/Polly.Core.Tests/Telemetry/ResilienceStrategyTelemetryTests.cs +++ b/test/Polly.Core.Tests/Telemetry/ResilienceStrategyTelemetryTests.cs @@ -10,7 +10,6 @@ public class ResilienceStrategyTelemetryTests public ResilienceStrategyTelemetryTests() => _sut = new(new ResilienceTelemetrySource( "builder", "instance", - new ResilienceProperties(), "strategy-name"), _diagnosticSource); private readonly ResilienceStrategyTelemetry _sut; @@ -30,7 +29,6 @@ public void Report_NoOutcome_OK() args.Event.Severity.Should().Be(ResilienceEventSeverity.Warning); args.Outcome.Should().BeNull(); args.Source.StrategyName.Should().Be("strategy-name"); - args.Source.BuilderProperties.Should().NotBeNull(); args.Arguments.Should().BeOfType(); args.Outcome.Should().BeNull(); args.Context.Should().NotBeNull(); @@ -54,7 +52,7 @@ public void Report_NoOutcomeWhenNotSubscribed_None() [Fact] public void ResilienceStrategyTelemetry_NoDiagnosticSource_Ok() { - var source = new ResilienceTelemetrySource("builder", "instance", new ResilienceProperties(), "strategy-name"); + var source = new ResilienceTelemetrySource("builder", "instance", "strategy-name"); var sut = new ResilienceStrategyTelemetry(source, null); var context = ResilienceContextPool.Shared.Get(); @@ -76,7 +74,6 @@ public void Report_Outcome_OK() args.Event.EventName.Should().Be("dummy-event"); args.Event.Severity.Should().Be(ResilienceEventSeverity.Warning); args.Source.StrategyName.Should().Be("strategy-name"); - args.Source.BuilderProperties.Should().NotBeNull(); args.Arguments.Should().BeOfType(); args.Outcome.Should().NotBeNull(); args.Outcome!.Value.Result.Should().Be(99); diff --git a/test/Polly.Core.Tests/Telemetry/TelemetryEventArgumentsTests.cs b/test/Polly.Core.Tests/Telemetry/TelemetryEventArgumentsTests.cs index 518c47236a2..b0701748942 100644 --- a/test/Polly.Core.Tests/Telemetry/TelemetryEventArgumentsTests.cs +++ b/test/Polly.Core.Tests/Telemetry/TelemetryEventArgumentsTests.cs @@ -4,7 +4,7 @@ namespace Polly.Extensions.Tests.Telemetry; public class TelemetryEventArgumentsTests { - private readonly ResilienceTelemetrySource _source = new("builder", "instance", new ResilienceProperties(), "strategy"); + private readonly ResilienceTelemetrySource _source = new("builder", "instance", "strategy"); [Fact] public void Get_Ok() diff --git a/test/Polly.Core.Tests/Telemetry/TelemetryUtilTests.cs b/test/Polly.Core.Tests/Telemetry/TelemetryUtilTests.cs index 67e13f4e3f4..bd642d6f418 100644 --- a/test/Polly.Core.Tests/Telemetry/TelemetryUtilTests.cs +++ b/test/Polly.Core.Tests/Telemetry/TelemetryUtilTests.cs @@ -1,4 +1,3 @@ -using NSubstitute; using Polly.Telemetry; namespace Polly.Core.Tests.Telemetry; @@ -8,7 +7,7 @@ public class TelemetryUtilTests [Fact] public void CreateResilienceTelemetry_Ok() { - var telemetry = TelemetryUtil.CreateTelemetry(null, "builder", "instance", new ResilienceProperties(), "strategy-name"); + var telemetry = TelemetryUtil.CreateTelemetry(null, "builder", "instance", "strategy-name"); telemetry.TelemetrySource.BuilderName.Should().Be("builder"); telemetry.TelemetrySource.BuilderInstanceName.Should().Be("instance"); @@ -16,17 +15,6 @@ public void CreateResilienceTelemetry_Ok() telemetry.DiagnosticSource.Should().BeNull(); } - [Fact] - public void CreateResilienceTelemetry_DiagnosticSourceFromProperties_Ok() - { - var props = new ResilienceProperties(); - var source = Substitute.For(); - - var telemetry = TelemetryUtil.CreateTelemetry(source, "builder", "instance", props, "strategy-name"); - - telemetry.DiagnosticSource.Should().Be(source); - } - [InlineData(true, ResilienceEventSeverity.Warning)] [InlineData(false, ResilienceEventSeverity.Information)] [Theory] diff --git a/test/Polly.Extensions.Tests/DependencyInjection/PollyDependencyInjectionKeysTests.cs b/test/Polly.Extensions.Tests/DependencyInjection/PollyDependencyInjectionKeysTests.cs deleted file mode 100644 index dbf68e1de70..00000000000 --- a/test/Polly.Extensions.Tests/DependencyInjection/PollyDependencyInjectionKeysTests.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Polly.DependencyInjection; - -namespace Polly.Extensions.Tests.DependencyInjection; - -public class PollyDependencyInjectionKeysTests -{ - [Fact] - public void ServiceProvider_Ok() - { - PollyDependencyInjectionKeys.ServiceProvider.Key.Should().Be("Polly.DependencyInjection.ServiceProvider"); - } -} diff --git a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs index 88d082229e5..6ea59488a33 100644 --- a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs +++ b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs @@ -165,11 +165,7 @@ public void AddResilienceStrategy_EnsureCompositeStrategyBuilderResolvedCorrectl var asserted = false; var key = new ResiliencePropertyKey("A"); - AddResilienceStrategy(Key, context => - { - context.BuilderProperties.TryGetValue(PollyDependencyInjectionKeys.ServiceProvider, out _).Should().BeTrue(); - asserted = true; - }); + AddResilienceStrategy(Key, context => asserted = true); CreateProvider().GetStrategy(Key); diff --git a/test/Polly.Extensions.Tests/Issues/IssuesTests.OnCircuitBreakWithServiceProvider_796.cs b/test/Polly.Extensions.Tests/Issues/IssuesTests.OnCircuitBreakWithServiceProvider_796.cs index 292002536dd..2497cb61afb 100644 --- a/test/Polly.Extensions.Tests/Issues/IssuesTests.OnCircuitBreakWithServiceProvider_796.cs +++ b/test/Polly.Extensions.Tests/Issues/IssuesTests.OnCircuitBreakWithServiceProvider_796.cs @@ -7,6 +7,8 @@ namespace Polly.Extensions.Tests.Issues; public partial class IssuesTests { + private static readonly ResiliencePropertyKey ServiceProviderKey = new("ServiceProvider"); + [Fact] public async Task OnCircuitBreakWithServiceProvider_796() { @@ -23,7 +25,7 @@ public async Task OnCircuitBreakWithServiceProvider_796() MinimumThroughput = 10, OnOpened = async args => { - args.Context.Properties.GetValue(PollyDependencyInjectionKeys.ServiceProvider, null!).Should().NotBeNull(); + args.Context.Properties.GetValue(ServiceProviderKey, null!).Should().NotBeNull(); contextChecked = true; // do asynchronous call @@ -65,7 +67,7 @@ protected override ValueTask> ExecuteCore( ResilienceContext context, TState state) { - context.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, _serviceProvider); + context.Properties.Set(ServiceProviderKey, _serviceProvider); return callback(context, state); } } diff --git a/test/Polly.Extensions.Tests/Telemetry/ResilienceTelemetryDiagnosticSourceTests.cs b/test/Polly.Extensions.Tests/Telemetry/ResilienceTelemetryDiagnosticSourceTests.cs index 8b96cb873c2..0816e573320 100644 --- a/test/Polly.Extensions.Tests/Telemetry/ResilienceTelemetryDiagnosticSourceTests.cs +++ b/test/Polly.Extensions.Tests/Telemetry/ResilienceTelemetryDiagnosticSourceTests.cs @@ -523,17 +523,10 @@ private static void ReportEvent( ResilienceEventSeverity severity = ResilienceEventSeverity.Warning) { context ??= ResilienceContextPool.Shared.Get("op-key"); - var props = new ResilienceProperties(); - if (!string.IsNullOrEmpty(instanceName)) - { - props.Set(new ResiliencePropertyKey("Polly.StrategyKey"), instanceName); - } - telemetry.ReportEvent( new ResilienceEvent(severity, "my-event"), "my-builder", instanceName, - props, "my-strategy", context, outcome, diff --git a/test/Polly.TestUtils/TestUtilities.cs b/test/Polly.TestUtils/TestUtilities.cs index 48db532993b..484ff494db6 100644 --- a/test/Polly.TestUtils/TestUtilities.cs +++ b/test/Polly.TestUtils/TestUtilities.cs @@ -38,10 +38,10 @@ public static async Task AssertWithTimeoutAsync(Func assertion, TimeSpan t } public static ResilienceStrategyTelemetry CreateResilienceTelemetry(DiagnosticSource source) - => new(new ResilienceTelemetrySource("dummy-builder", "dummy-instance", new ResilienceProperties(), "strategy-name"), source); + => new(new ResilienceTelemetrySource("dummy-builder", "dummy-instance", "strategy-name"), source); public static ResilienceStrategyTelemetry CreateResilienceTelemetry(Action callback) - => new(new ResilienceTelemetrySource("dummy-builder", "dummy-instance", new ResilienceProperties(), "strategy-name"), new CallbackDiagnosticSource(callback)); + => new(new ResilienceTelemetrySource("dummy-builder", "dummy-instance", "strategy-name"), new CallbackDiagnosticSource(callback)); public static ILoggerFactory CreateLoggerFactory(out FakeLogger logger) { @@ -91,7 +91,6 @@ public static void ReportEvent( ResilienceEvent resilienceEvent, string builderName, string? instanceName, - ResilienceProperties builderProperties, string? strategyName, ResilienceContext context, Outcome? outcome, @@ -99,7 +98,7 @@ public static void ReportEvent( #pragma warning restore S107 // Methods should not have too many parameters { source.Write(resilienceEvent.EventName, TelemetryEventArguments.Get( - new ResilienceTelemetrySource(builderName, instanceName, builderProperties, strategyName), + new ResilienceTelemetrySource(builderName, instanceName, strategyName), resilienceEvent, context, outcome, From 904b988c359ac9692eb843ef6acbcab1b589bb7c Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Mon, 14 Aug 2023 10:13:57 +0200 Subject: [PATCH 2/4] code coverage --- .../Registry/ResilienceStrategyRegistryTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs index 32f940d8f32..dfc7a09ba85 100644 --- a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs @@ -238,6 +238,8 @@ public void AddBuilder_EnsureStrategyKey() { context.BuilderName.Should().Be("A"); context.BuilderInstanceName.Should().Be("Instance1"); + context.StrategyKey.Should().Be(StrategyId.Create("A", "Instance1")); + builder.AddStrategy(new TestResilienceStrategy()); builder.Name.Should().Be("A"); called = true; From 0249281677016fbd64ecfb4a6e687c2dd0effb18 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Mon, 14 Aug 2023 11:56:18 +0200 Subject: [PATCH 3/4] fixes --- src/Polly.Core/PublicAPI.Unshipped.txt | 28 ++----------------- .../ResiliencePipelineRegistryTests.cs | 2 +- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index 5c139b3cb66..8516af72bb3 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/src/Polly.Core/PublicAPI.Unshipped.txt @@ -1,4 +1,5 @@ -abstract Polly.Registry.ResiliencePipelineProvider.TryGetPipeline(TKey key, out Polly.ResiliencePipeline? pipeline) -> bool +#nullable enable +abstract Polly.Registry.ResiliencePipelineProvider.TryGetPipeline(TKey key, out Polly.ResiliencePipeline? pipeline) -> bool abstract Polly.Registry.ResiliencePipelineProvider.TryGetPipeline(TKey key, out Polly.ResiliencePipeline? pipeline) -> bool abstract Polly.ResilienceContextPool.Get(string? operationKey, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Polly.ResilienceContext! abstract Polly.ResilienceContextPool.Return(Polly.ResilienceContext! context) -> void @@ -74,22 +75,6 @@ Polly.CircuitBreaker.OnCircuitOpenedArguments Polly.CircuitBreaker.OnCircuitOpenedArguments.BreakDuration.get -> System.TimeSpan Polly.CircuitBreaker.OnCircuitOpenedArguments.IsManual.get -> bool Polly.CircuitBreaker.OnCircuitOpenedArguments.OnCircuitOpenedArguments(System.TimeSpan breakDuration, bool isManual) -> void -Polly.CircuitBreakerCompositeStrategyBuilderExtensions -Polly.CompositeStrategyBuilder -Polly.CompositeStrategyBuilder.Build() -> Polly.ResilienceStrategy! -Polly.CompositeStrategyBuilder.CompositeStrategyBuilder() -> void -Polly.CompositeStrategyBuilder -Polly.CompositeStrategyBuilder.Build() -> Polly.ResilienceStrategy! -Polly.CompositeStrategyBuilder.CompositeStrategyBuilder() -> void -Polly.CompositeStrategyBuilderBase -Polly.CompositeStrategyBuilderBase.DiagnosticSource.get -> System.Diagnostics.DiagnosticSource? -Polly.CompositeStrategyBuilderBase.DiagnosticSource.set -> void -Polly.CompositeStrategyBuilderBase.InstanceName.get -> string? -Polly.CompositeStrategyBuilderBase.InstanceName.set -> void -Polly.CompositeStrategyBuilderBase.Name.get -> string? -Polly.CompositeStrategyBuilderBase.Name.set -> void -Polly.CompositeStrategyBuilderBase.Validator.get -> System.Action! -Polly.CompositeStrategyBuilderExtensions Polly.CircuitBreakerResiliencePipelineBuilderExtensions Polly.ExecutionRejectedException Polly.ExecutionRejectedException.ExecutionRejectedException() -> void @@ -271,9 +256,6 @@ Polly.ResiliencePipelineBuilderBase.InstanceName.get -> string? Polly.ResiliencePipelineBuilderBase.InstanceName.set -> void Polly.ResiliencePipelineBuilderBase.Name.get -> string? Polly.ResiliencePipelineBuilderBase.Name.set -> void -Polly.ResiliencePipelineBuilderBase.Properties.get -> Polly.ResilienceProperties! -Polly.ResiliencePipelineBuilderBase.Randomizer.get -> System.Func! -Polly.ResiliencePipelineBuilderBase.Randomizer.set -> void Polly.ResiliencePipelineBuilderBase.Validator.get -> System.Action! Polly.ResiliencePipelineBuilderExtensions Polly.ResilienceProperties @@ -369,13 +351,9 @@ Polly.Telemetry.ResilienceStrategyTelemetry.IsEnabled.get -> bool Polly.Telemetry.ResilienceStrategyTelemetry.Report(Polly.Telemetry.ResilienceEvent resilienceEvent, Polly.OutcomeArguments args) -> void Polly.Telemetry.ResilienceStrategyTelemetry.Report(Polly.Telemetry.ResilienceEvent resilienceEvent, Polly.ResilienceContext! context, TArgs args) -> void Polly.Telemetry.ResilienceTelemetrySource -Polly.Telemetry.ResilienceTelemetrySource.BuilderInstanceName.get -> string? -Polly.Telemetry.ResilienceTelemetrySource.BuilderName.get -> string? -Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? builderName, string? builderInstanceName, string? strategyName) -> void -Polly.Telemetry.ResilienceTelemetrySource.BuilderProperties.get -> Polly.ResilienceProperties! Polly.Telemetry.ResilienceTelemetrySource.PipelineInstanceName.get -> string? Polly.Telemetry.ResilienceTelemetrySource.PipelineName.get -> string? -Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? pipelineName, string? pipelineInstanceName, Polly.ResilienceProperties! builderProperties, string? strategyName) -> void +Polly.Telemetry.ResilienceTelemetrySource.ResilienceTelemetrySource(string? pipelineName, string? pipelineInstanceName, string? strategyName) -> void Polly.Telemetry.ResilienceTelemetrySource.StrategyName.get -> string? Polly.Telemetry.TelemetryEventArguments Polly.Telemetry.TelemetryEventArguments.Arguments.get -> object! diff --git a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs index 54715674983..1e50e7e7473 100644 --- a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs @@ -238,7 +238,7 @@ public void AddBuilder_EnsurePipelineKey() { context.BuilderName.Should().Be("A"); context.BuilderInstanceName.Should().Be("Instance1"); - context.StrategyKey.Should().Be(StrategyId.Create("A", "Instance1")); + context.PipelineKey.Should().Be(StrategyId.Create("A", "Instance1")); builder.AddStrategy(new TestResilienceStrategy()); builder.Name.Should().Be("A"); From 81ccdac03723d0e5e3ae532bc8bd02d1cf0f2fce Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Mon, 14 Aug 2023 12:02:05 +0200 Subject: [PATCH 4/4] cleanup --- src/Polly.RateLimiting/RateLimiterStrategyOptions.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Polly.RateLimiting/RateLimiterStrategyOptions.cs b/src/Polly.RateLimiting/RateLimiterStrategyOptions.cs index e64ffb49702..8bc850c2d51 100644 --- a/src/Polly.RateLimiting/RateLimiterStrategyOptions.cs +++ b/src/Polly.RateLimiting/RateLimiterStrategyOptions.cs @@ -41,11 +41,9 @@ public class RateLimiterStrategyOptions : ResilienceStrategyOptions /// /// Gets or sets the rate limiter that the strategy uses. /// - /// - /// If this property is , then the strategy will use a created using . - /// /// - /// The default value is . This property is required. + /// The default value is . If this property is , then the strategy + /// will use a created using . /// public ResilienceRateLimiter? RateLimiter { get; set; } }