API Review Feedback (2) (#1485)

src/Polly.Core/PublicAPI.Unshipped.txt

@@ -1,4 +1,5 @@
-abstract Polly.Registry.ResiliencePipelineProvider<TKey>.TryGetPipeline(TKey key, out Polly.ResiliencePipeline? pipeline) -> bool
+#nullable enable
+abstract Polly.Registry.ResiliencePipelineProvider<TKey>.TryGetPipeline(TKey key, out Polly.ResiliencePipeline? pipeline) -> bool
 abstract Polly.Registry.ResiliencePipelineProvider<TKey>.TryGetPipeline<TResult>(TKey key, out Polly.ResiliencePipeline<TResult>? 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
@@ -255,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<double>!
-Polly.ResiliencePipelineBuilderBase.Randomizer.set -> void
 Polly.ResiliencePipelineBuilderBase.Validator.get -> System.Action<Polly.ResilienceValidationContext!>!
 Polly.ResiliencePipelineBuilderExtensions
 Polly.ResilienceProperties
@@ -309,6 +307,8 @@ Polly.Retry.RetryStrategyOptions<TResult>.BaseDelay.get -> System.TimeSpan
 Polly.Retry.RetryStrategyOptions<TResult>.BaseDelay.set -> void
 Polly.Retry.RetryStrategyOptions<TResult>.OnRetry.get -> System.Func<Polly.OutcomeArguments<TResult, Polly.Retry.OnRetryArguments!>, System.Threading.Tasks.ValueTask>?
 Polly.Retry.RetryStrategyOptions<TResult>.OnRetry.set -> void
+Polly.Retry.RetryStrategyOptions<TResult>.Randomizer.get -> System.Func<double>!
+Polly.Retry.RetryStrategyOptions<TResult>.Randomizer.set -> void
 Polly.Retry.RetryStrategyOptions<TResult>.RetryCount.get -> int
 Polly.Retry.RetryStrategyOptions<TResult>.RetryCount.set -> void
 Polly.Retry.RetryStrategyOptions<TResult>.RetryDelayGenerator.get -> System.Func<Polly.OutcomeArguments<TResult, Polly.Retry.RetryDelayArguments>, System.Threading.Tasks.ValueTask<System.TimeSpan>>?
@@ -322,7 +322,6 @@ Polly.RetryResiliencePipelineBuilderExtensions
 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
@@ -352,10 +351,9 @@ Polly.Telemetry.ResilienceStrategyTelemetry.IsEnabled.get -> bool
 Polly.Telemetry.ResilienceStrategyTelemetry.Report<TArgs, TResult>(Polly.Telemetry.ResilienceEvent resilienceEvent, Polly.OutcomeArguments<TResult, TArgs> args) -> void
 Polly.Telemetry.ResilienceStrategyTelemetry.Report<TArgs>(Polly.Telemetry.ResilienceEvent resilienceEvent, Polly.ResilienceContext! context, TArgs args) -> void
 Polly.Telemetry.ResilienceTelemetrySource
-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!

src/Polly.Core/Registry/ResiliencePipelineRegistry.cs

@@ -297,7 +297,6 @@ private static ResiliencePipeline CreatePipeline<TBuilder>(
                 diagnosticSource,
                 context.BuilderName,
                 context.BuilderInstanceName,
-                builder.Properties,
                 null));
     }
 

src/Polly.Core/ResiliencePipelineBuilderBase.cs

@@ -26,9 +26,7 @@ private protected ResiliencePipelineBuilderBase()
     private protected ResiliencePipelineBuilderBase(ResiliencePipelineBuilderBase other)
     {
         Name = other.Name;
-        Properties = other.Properties;
         TimeProvider = other.TimeProvider;
-        Randomizer = other.Randomizer;
         DiagnosticSource = other.DiagnosticSource;
     }
 
@@ -55,14 +53,6 @@ private protected ResiliencePipelineBuilderBase(ResiliencePipelineBuilderBase ot
     /// </value>
     public string? InstanceName { get; set; }
 
-    /// <summary>
-    /// Gets the custom properties attached to builder options.
-    /// </summary>
-    /// <value>
-    /// The default value is the empty instance of <see cref="ResilienceProperties"/>.
-    /// </value>
-    public ResilienceProperties Properties { get; } = new();
-
     /// <summary>
     /// Gets or sets a <see cref="TimeProvider"/> that is used by strategies that work with time.
     /// </summary>
@@ -87,16 +77,6 @@ private protected ResiliencePipelineBuilderBase(ResiliencePipelineBuilderBase ot
     [EditorBrowsable(EditorBrowsableState.Never)]
     public DiagnosticSource? DiagnosticSource { get; set; }
 
-    /// <summary>
-    /// Gets or sets the randomizer that is used by strategies that need to generate random numbers.
-    /// </summary>
-    /// <value>
-    /// The default value is thread-safe randomizer that returns values between 0.0 and 1.0.
-    /// </value>
-    [EditorBrowsable(EditorBrowsableState.Never)]
-    [Required]
-    public Func<double> Randomizer { get; set; } = RandomUtil.Instance.NextDouble;
-
     /// <summary>
     /// Gets the validator that is used for the validation.
     /// </summary>
@@ -139,7 +119,7 @@ internal ResiliencePipeline BuildPipeline()
 
         return CompositeResiliencePipeline.Create(
             strategies,
-            TelemetryUtil.CreateTelemetry(DiagnosticSource, Name, InstanceName, Properties, null),
+            TelemetryUtil.CreateTelemetry(DiagnosticSource, Name, InstanceName, null),
             TimeProvider);
     }
 
@@ -148,11 +128,9 @@ private ResiliencePipeline CreateResiliencePipeline(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;

src/Polly.Core/Retry/RetryResiliencePipelineBuilderExtensions.cs

@@ -28,7 +28,7 @@ public static ResiliencePipelineBuilder AddRetry(this ResiliencePipelineBuilder
         Guard.NotNull(options);
 
         return builder.AddStrategy(
-            context => new RetryResilienceStrategy<object>(options, context.TimeProvider, context.Telemetry, context.Randomizer),
+            context => new RetryResilienceStrategy<object>(options, context.TimeProvider, context.Telemetry),
             options);
     }
 
@@ -53,7 +53,7 @@ public static ResiliencePipelineBuilder AddRetry(this ResiliencePipelineBuilder
         Guard.NotNull(options);
 
         return builder.AddStrategy(
-            context => new RetryResilienceStrategy<TResult>(options, context.TimeProvider, context.Telemetry, context.Randomizer),
+            context => new RetryResilienceStrategy<TResult>(options, context.TimeProvider, context.Telemetry),
             options);
     }
 }

src/Polly.Core/Retry/RetryResilienceStrategy.cs

@@ -11,8 +11,7 @@ internal sealed class RetryResilienceStrategy<T> : ResilienceStrategy<T>
     public RetryResilienceStrategy(
         RetryStrategyOptions<T> options,
         TimeProvider timeProvider,
-        ResilienceStrategyTelemetry telemetry,
-        Func<double> 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; }

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<TResult> : ResilienceStrategyOptions
     /// The default value is <see langword="null"/>.
     /// </value>
     public Func<OutcomeArguments<TResult, OnRetryArguments>, ValueTask>? OnRetry { get; set; }
+
+    /// <summary>
+    /// Gets or sets the randomizer that is used by the retry strategy to generate random numbers.
+    /// </summary>
+    /// <value>
+    /// The default value is thread-safe randomizer that returns values between 0.0 and 1.0.
+    /// </value>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    [Required]
+    public Func<double> Randomizer { get; set; } = RandomUtil.Instance.NextDouble;
 }

src/Polly.Core/StrategyBuilderContext.cs

@@ -10,19 +10,15 @@ public sealed class StrategyBuilderContext
     internal StrategyBuilderContext(
         string? builderName,
         string? builderInstanceName,
-        ResilienceProperties builderProperties,
         string? strategyName,
         TimeProvider timeProvider,
-        DiagnosticSource? diagnosticSource,
-        Func<double> 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);
     }
 
     /// <summary>
@@ -35,11 +31,6 @@ internal StrategyBuilderContext(
     /// </summary>
     public string? BuilderInstanceName { get; }
 
-    /// <summary>
-    /// Gets the custom properties attached to the builder.
-    /// </summary>
-    public ResilienceProperties BuilderProperties { get; }
-
     /// <summary>
     /// Gets the name of the strategy.
     /// </summary>
@@ -54,6 +45,4 @@ internal StrategyBuilderContext(
     /// Gets the <see cref="TimeProvider"/> used by this strategy.
     /// </summary>
     internal TimeProvider TimeProvider { get; }
-
-    internal Func<double> Randomizer { get; }
 }

src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs

@@ -13,17 +13,14 @@ public sealed class ResilienceTelemetrySource
     /// </summary>
     /// <param name="pipelineName">The pipeline name.</param>
     /// <param name="pipelineInstanceName">The pipeline instance name.</param>
-    /// <param name="builderProperties">The builder properties.</param>
     /// <param name="strategyName">The strategy name.</param>
     public ResilienceTelemetrySource(
         string? pipelineName,
         string? pipelineInstanceName,
-        ResilienceProperties builderProperties,
         string? strategyName)
     {
         PipelineName = pipelineName;
         PipelineInstanceName = pipelineInstanceName;
-        BuilderProperties = builderProperties;
         StrategyName = strategyName;
     }
 
@@ -37,11 +34,6 @@ public ResilienceTelemetrySource(
     /// </summary>
     public string? PipelineInstanceName { get; }
 
-    /// <summary>
-    /// Gets the builder properties.
-    /// </summary>
-    public ResilienceProperties BuilderProperties { get; }
-
     /// <summary>
     /// Gets the strategy name.
     /// </summary>

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);
     }

src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs

@@ -251,7 +251,6 @@ private static void AddResiliencePipelineBuilder(this IServiceCollection service
         services.TryAddTransient(serviceProvider =>
         {
             var builder = new ResiliencePipelineBuilder();
-            builder.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider);
             builder.ConfigureTelemetry(serviceProvider.GetRequiredService<IOptions<TelemetryOptions>>().Value);
             return builder;
         });

src/Polly.RateLimiting/RateLimiterStrategyOptions.cs

@@ -41,11 +41,9 @@ public class RateLimiterStrategyOptions : ResilienceStrategyOptions
     /// <summary>
     ///  Gets or sets the rate limiter that the strategy uses.
     /// </summary>
-    /// <remarks>
-    /// If this property is <see langword="null"/>, then the strategy will use a <see cref="ConcurrencyLimiter"/> created using <see cref="DefaultRateLimiterOptions"/>.
-    /// </remarks>
     /// <value>
-    /// The default value is <see langword="null"/>. This property is required.
+    /// The default value is <see langword="null"/>. If this property is <see langword="null"/>, then the strategy
+    /// will use a <see cref="ConcurrencyLimiter"/> created using <see cref="DefaultRateLimiterOptions"/>.
     /// </value>
     public ResilienceRateLimiter? RateLimiter { get; set; }
 }

test/Polly.Core.Tests/GenericResiliencePipelineBuilderTests.cs

@@ -11,7 +11,6 @@ public class GenericResiliencePipelineBuilderTests
     public void Ctor_EnsureDefaults()
     {
         _builder.Name.Should().BeNull();
-        _builder.Properties.Should().NotBeNull();
         _builder.TimeProvider.Should().Be(TimeProvider.System);
     }
 

test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs

@@ -177,10 +177,9 @@ public void AddBuilder_GetPipeline_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.PipelineKey);
             called++;
         });
 
@@ -206,10 +205,9 @@ public void AddBuilder_GenericGetPipeline_EnsureCalled()
         _callback = _ => activatorCalls++;
         var registry = CreateRegistry();
         var called = 0;
-        registry.TryAddBuilder<string>(StrategyId.Create("A"), (builder, context) =>
+        registry.TryAddBuilder<string>(StrategyId.Create("A"), (builder, _) =>
         {
             builder.AddStrategy(new TestResilienceStrategy());
-            builder.Properties.Set(StrategyId.ResilienceKey, context.PipelineKey);
             called++;
         });
 
@@ -240,6 +238,8 @@ public void AddBuilder_EnsurePipelineKey()
         {
             context.BuilderName.Should().Be("A");
             context.BuilderInstanceName.Should().Be("Instance1");
+            context.PipelineKey.Should().Be(StrategyId.Create("A", "Instance1"));
+
             builder.AddStrategy(new TestResilienceStrategy());
             builder.Name.Should().Be("A");
             called = true;

test/Polly.Core.Tests/ResiliencePipelineBuilderTests.cs

@@ -15,9 +15,7 @@ public void Ctor_EnsureDefaults()
         var builder = new ResiliencePipelineBuilder();
 
         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<TimeProvider>(),
             Name = "dummy",
-            Randomizer = () => 0.0,
             DiagnosticSource = Substitute.For<DiagnosticSource>(),
         };
 
-        builder.Properties.Set(new ResiliencePropertyKey<string>("dummy"), "dummy");
-
         var other = new ResiliencePipelineBuilder<double>(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<string>("dummy"), "").Should().Be("dummy");
     }
 
     [Fact]
@@ -305,10 +298,8 @@ public void Build_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 Build_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;