Make the ReactiveResilienceStrategy type-safe (#1462)

Author: martintmk

src/Polly.Core/CircuitBreaker/CircuitBreakerCompositeStrategyBuilderExtensions.cs

@@ -24,12 +24,17 @@ public static class CircuitBreakerCompositeStrategyBuilderExtensions
     /// </remarks>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when <paramref name="options"/> are invalid.</exception>
+    [UnconditionalSuppressMessage(
+        "Trimming",
+        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
+        Justification = "All options members preserved.")]
+    [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(CircuitBreakerStrategyOptions))]
     public static CompositeStrategyBuilder AddCircuitBreaker(this CompositeStrategyBuilder builder, CircuitBreakerStrategyOptions options)
     {
         Guard.NotNull(builder);
         Guard.NotNull(options);
 
-        return builder.AddCircuitBreakerCore(options);
+        return builder.AddStrategy(context => CreateStrategy(context, options), options);
     }
 
     /// <summary>
@@ -47,39 +52,29 @@ public static CompositeStrategyBuilder AddCircuitBreaker(this CompositeStrategyB
     /// </remarks>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when <paramref name="options"/> are invalid.</exception>
-    public static CompositeStrategyBuilder<TResult> AddCircuitBreaker<TResult>(this CompositeStrategyBuilder<TResult> builder, CircuitBreakerStrategyOptions<TResult> options)
-    {
-        Guard.NotNull(builder);
-        Guard.NotNull(options);
-
-        return builder.AddCircuitBreakerCore(options);
-    }
-
     [UnconditionalSuppressMessage(
         "Trimming",
         "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
         Justification = "All options members preserved.")]
-    private static TBuilder AddCircuitBreakerCore<TBuilder, TResult>(this TBuilder builder, CircuitBreakerStrategyOptions<TResult> options)
-        where TBuilder : CompositeStrategyBuilderBase
+    public static CompositeStrategyBuilder<TResult> AddCircuitBreaker<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TResult>(
+        this CompositeStrategyBuilder<TResult> builder,
+        CircuitBreakerStrategyOptions<TResult> options)
     {
-        return builder.AddStrategy(
-            context =>
-            {
-                var behavior = new AdvancedCircuitBehavior(
-                    options.FailureRatio,
-                    options.MinimumThroughput,
-                    HealthMetrics.Create(options.SamplingDuration, context.TimeProvider));
+        Guard.NotNull(builder);
+        Guard.NotNull(options);
 
-                return CreateStrategy<TResult, CircuitBreakerStrategyOptions<TResult>>(context, options, behavior);
-            },
-            options);
+        return builder.AddStrategy(context => CreateStrategy(context, options), options);
     }
 
-    internal static CircuitBreakerResilienceStrategy<TResult> CreateStrategy<TResult, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>(
+    internal static CircuitBreakerResilienceStrategy<TResult> CreateStrategy<TResult>(
         StrategyBuilderContext context,
-        CircuitBreakerStrategyOptions<TResult> options,
-        CircuitBehavior behavior)
+        CircuitBreakerStrategyOptions<TResult> options)
     {
+        var behavior = new AdvancedCircuitBehavior(
+            options.FailureRatio,
+            options.MinimumThroughput,
+            HealthMetrics.Create(options.SamplingDuration, context.TimeProvider));
+
         var controller = new CircuitStateController<TResult>(
             options.BreakDuration,
             options.OnOpened,

src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs

@@ -21,7 +21,7 @@ public CircuitBreakerResilienceStrategy(
             _controller.Dispose);
     }
 
-    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
+    protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
     {
         if (await _controller.OnActionPreExecuteAsync(context).ConfigureAwait(context.ContinueOnCapturedContext) is Outcome<T> outcome)
         {

src/Polly.Core/CompositeStrategyBuilderExtensions.cs

@@ -70,6 +70,53 @@ public static TBuilder AddStrategy<TBuilder>(this TBuilder builder, Func<Strateg
         return builder;
     }
 
+    /// <summary>
+    /// Adds a reactive strategy to the builder.
+    /// </summary>
+    /// <param name="builder">The builder instance.</param>
+    /// <param name="factory">The factory that creates a resilience strategy.</param>
+    /// <param name="options">The options associated with the strategy. If none are provided the default instance of <see cref="ResilienceStrategyOptions"/> is created.</param>
+    /// <returns>The same builder instance.</returns>
+    /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/>, <paramref name="factory"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
+    /// <exception cref="InvalidOperationException">Thrown when this builder was already used to create a strategy. The builder cannot be modified after it has been used.</exception>
+    /// <exception cref="ValidationException">Thrown when <paramref name="options"/> is invalid.</exception>
+    [RequiresUnreferencedCode(Constants.OptionsValidation)]
+    public static CompositeStrategyBuilder AddStrategy(
+        this CompositeStrategyBuilder builder, Func<StrategyBuilderContext, ReactiveResilienceStrategy<object>> factory,
+        ResilienceStrategyOptions options)
+    {
+        Guard.NotNull(builder);
+        Guard.NotNull(factory);
+        Guard.NotNull(options);
+
+        builder.AddStrategyCore(context => new ReactiveResilienceStrategyBridge<object>(factory(context)), options);
+        return builder;
+    }
+
+    /// <summary>
+    /// Adds a reactive strategy to the builder.
+    /// </summary>
+    /// <typeparam name="TResult">The type of the result.</typeparam>
+    /// <param name="builder">The builder instance.</param>
+    /// <param name="factory">The factory that creates a resilience strategy.</param>
+    /// <param name="options">The options associated with the strategy. If none are provided the default instance of <see cref="ResilienceStrategyOptions"/> is created.</param>
+    /// <returns>The same builder instance.</returns>
+    /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/>, <paramref name="factory"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
+    /// <exception cref="InvalidOperationException">Thrown when this builder was already used to create a strategy. The builder cannot be modified after it has been used.</exception>
+    /// <exception cref="ValidationException">Thrown when <paramref name="options"/> is invalid.</exception>
+    [RequiresUnreferencedCode(Constants.OptionsValidation)]
+    public static CompositeStrategyBuilder<TResult> AddStrategy<TResult>(
+        this CompositeStrategyBuilder<TResult> builder, Func<StrategyBuilderContext, ReactiveResilienceStrategy<TResult>> factory,
+        ResilienceStrategyOptions options)
+    {
+        Guard.NotNull(builder);
+        Guard.NotNull(factory);
+        Guard.NotNull(options);
+
+        builder.AddStrategyCore(context => new ReactiveResilienceStrategyBridge<TResult>(factory(context)), options);
+        return builder;
+    }
+
     internal sealed class EmptyOptions : ResilienceStrategyOptions
     {
         public static readonly EmptyOptions Instance = new();

src/Polly.Core/Fallback/FallbackCompositeStrategyBuilderExtensions.cs

@@ -18,13 +18,18 @@ public static class FallbackCompositeStrategyBuilderExtensions
     /// <returns>The builder instance with the fallback strategy added.</returns>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when <paramref name="options"/> are invalid.</exception>
-    public static CompositeStrategyBuilder<TResult> AddFallback<TResult>(this CompositeStrategyBuilder<TResult> builder, FallbackStrategyOptions<TResult> options)
+    [UnconditionalSuppressMessage(
+        "Trimming",
+        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
+        Justification = "All options members preserved.")]
+    public static CompositeStrategyBuilder<TResult> AddFallback<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TResult>(
+        this CompositeStrategyBuilder<TResult> builder,
+        FallbackStrategyOptions<TResult> options)
     {
         Guard.NotNull(builder);
         Guard.NotNull(options);
 
-        builder.AddFallbackCore<TResult, FallbackStrategyOptions<TResult>>(options);
-        return builder;
+        return builder.AddStrategy(context => CreateFallback(context, options), options);
     }
 
     /// <summary>
@@ -35,34 +40,30 @@ public static CompositeStrategyBuilder<TResult> AddFallback<TResult>(this Compos
     /// <returns>The builder instance with the fallback strategy added.</returns>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when <paramref name="options"/> are invalid.</exception>
+    [UnconditionalSuppressMessage(
+        "Trimming",
+        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
+        Justification = "All options members preserved.")]
+    [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(FallbackStrategyOptions))]
     internal static CompositeStrategyBuilder AddFallback(this CompositeStrategyBuilder builder, FallbackStrategyOptions options)
     {
         Guard.NotNull(builder);
         Guard.NotNull(options);
 
-        builder.AddFallbackCore<object, FallbackStrategyOptions>(options);
-        return builder;
+        return builder.AddStrategy(context => CreateFallback(context, options), options);
     }
 
-    [UnconditionalSuppressMessage(
-        "Trimming",
-        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
-        Justification = "All options members preserved.")]
-    internal static void AddFallbackCore<TResult, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] TOptions>(
-        this CompositeStrategyBuilderBase builder,
+    private static ReactiveResilienceStrategy<TResult> CreateFallback<TResult>(
+        StrategyBuilderContext context,
         FallbackStrategyOptions<TResult> options)
     {
-        builder.AddStrategy(context =>
-        {
-            var handler = new FallbackHandler<TResult>(
-                options.ShouldHandle!,
-                options.FallbackAction!);
+        var handler = new FallbackHandler<TResult>(
+            options.ShouldHandle!,
+            options.FallbackAction!);
 
-            return new FallbackResilienceStrategy<TResult>(
-                handler,
-                options.OnFallback,
-                context.Telemetry);
-        },
-        options);
+        return new FallbackResilienceStrategy<TResult>(
+            handler,
+            options.OnFallback,
+            context.Telemetry);
     }
 }

src/Polly.Core/Fallback/FallbackResilienceStrategy.cs

@@ -17,7 +17,7 @@ public FallbackResilienceStrategy(FallbackHandler<T> handler, Func<OutcomeArgume
         _telemetry = telemetry;
     }
 
-    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
+    protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
     {
         var outcome = await ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
         var handleFallbackArgs = new OutcomeArguments<T, FallbackPredicateArguments>(context, outcome, default);

src/Polly.Core/Hedging/HedgingCompositeStrategyBuilderExtensions.cs

@@ -19,13 +19,18 @@ public static class HedgingCompositeStrategyBuilderExtensions
     /// <returns>The builder instance with the hedging strategy added.</returns>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when <paramref name="options"/> are invalid.</exception>
-    public static CompositeStrategyBuilder<TResult> AddHedging<TResult>(this CompositeStrategyBuilder<TResult> builder, HedgingStrategyOptions<TResult> options)
+    [UnconditionalSuppressMessage(
+        "Trimming",
+        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
+        Justification = "All options members preserved.")]
+    public static CompositeStrategyBuilder<TResult> AddHedging<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TResult>(
+        this CompositeStrategyBuilder<TResult> builder,
+        HedgingStrategyOptions<TResult> options)
     {
         Guard.NotNull(builder);
         Guard.NotNull(options);
 
-        builder.AddHedgingCore<TResult, HedgingStrategyOptions<TResult>>(options);
-        return builder;
+        return builder.AddStrategy(context => CreateHedgingStrategy(context, options, isGeneric: true), options);
     }
 
     /// <summary>
@@ -36,39 +41,36 @@ public static CompositeStrategyBuilder<TResult> AddHedging<TResult>(this Composi
     /// <returns>The builder instance with the hedging strategy added.</returns>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> or <paramref name="options"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when <paramref name="options"/> are invalid.</exception>
+    [UnconditionalSuppressMessage(
+        "Trimming",
+        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
+        Justification = "All options members preserved.")]
+    [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(HedgingStrategyOptions))]
     internal static CompositeStrategyBuilder AddHedging(this CompositeStrategyBuilder builder, HedgingStrategyOptions options)
     {
         Guard.NotNull(builder);
         Guard.NotNull(options);
 
-        builder.AddHedgingCore<object, HedgingStrategyOptions>(options);
-        return builder;
+        return builder.AddStrategy(context => CreateHedgingStrategy(context, options, isGeneric: false), options);
     }
 
-    [UnconditionalSuppressMessage(
-        "Trimming",
-        "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
-        Justification = "All options members preserved.")]
-    internal static void AddHedgingCore<TResult, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>(
-        this CompositeStrategyBuilderBase builder,
-        HedgingStrategyOptions<TResult> options)
+    private static HedgingResilienceStrategy<TResult> CreateHedgingStrategy<TResult>(
+        StrategyBuilderContext context,
+        HedgingStrategyOptions<TResult> options,
+        bool isGeneric)
     {
-        builder.AddStrategy(context =>
-        {
-            var handler = new HedgingHandler<TResult>(
-                options.ShouldHandle!,
-                options.HedgingActionGenerator,
-                IsGeneric: builder is not CompositeStrategyBuilder);
+        var handler = new HedgingHandler<TResult>(
+                        options.ShouldHandle!,
+                        options.HedgingActionGenerator,
+                        IsGeneric: isGeneric);
 
-            return new HedgingResilienceStrategy<TResult>(
-                options.HedgingDelay,
-                options.MaxHedgedAttempts,
-                handler,
-                options.OnHedging,
-                options.HedgingDelayGenerator,
-                context.TimeProvider,
-                context.Telemetry);
-        },
-        options);
+        return new HedgingResilienceStrategy<TResult>(
+            options.HedgingDelay,
+            options.MaxHedgedAttempts,
+            handler,
+            options.OnHedging,
+            options.HedgingDelayGenerator,
+            context.TimeProvider,
+            context.Telemetry);
     }
 }

src/Polly.Core/Hedging/HedgingResilienceStrategy.cs

@@ -41,7 +41,7 @@ public HedgingResilienceStrategy(
     public Func<OutcomeArguments<T, OnHedgingArguments>, ValueTask>? OnHedging { get; }
 
     [ExcludeFromCodeCoverage] // coverlet issue
-    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(
+    protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(
         Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback,
         ResilienceContext context,
         TState state)