The deafult RateLimiterStrategyOptions instance is now valid (#1315)

Author: martintmk

src/Polly.RateLimiting/RateLimiterConstants.cs

@@ -2,6 +2,10 @@ namespace Polly.RateLimiting;
 
 internal static class RateLimiterConstants
 {
+    public const int DefaultPermitLimit = 1000;
+
+    public const int DefaultQueueLimit = 0;
+
     public const string StrategyType = "RateLimiter";
 
     public const string OnRateLimiterRejectedEvent = "OnRateLimiterRejected";

src/Polly.RateLimiting/RateLimiterResilienceStrategyBuilderExtensions.cs

@@ -20,6 +20,7 @@ public static class RateLimiterResilienceStrategyBuilderExtensions
     /// <returns>The builder instance with the concurrency limiter strategy added.</returns>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> is <see langword="null"/>.</exception>
     /// <exception cref="ValidationException">Thrown when the options constructed from the arguments are invalid.</exception>
+    /// <exception cref="ArgumentException">Thrown when <paramref name="permitLimit"/> or <paramref name="queueLimit"/> is invalid.</exception>
     public static TBuilder AddConcurrencyLimiter<TBuilder>(
         this TBuilder builder,
         int permitLimit,
@@ -44,6 +45,7 @@ public static TBuilder AddConcurrencyLimiter<TBuilder>(
     /// <returns>The builder instance with the concurrency limiter 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 the options constructed from the arguments are invalid.</exception>
+    /// <exception cref="ArgumentException">Thrown when <paramref name="options"/> are invalid.</exception>
     public static TBuilder AddConcurrencyLimiter<TBuilder>(
         this TBuilder builder,
         ConcurrencyLimiterOptions options)
@@ -54,7 +56,7 @@ public static TBuilder AddConcurrencyLimiter<TBuilder>(
 
         return builder.AddRateLimiter(new RateLimiterStrategyOptions
         {
-            RateLimiter = new ConcurrencyLimiter(options),
+            DefaultRateLimiterOptions = options
         });
     }
 
@@ -90,6 +92,7 @@ public static TBuilder AddRateLimiter<TBuilder>(
     /// <returns>The builder instance with the rate limiter 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>
+    /// <exception cref="ArgumentException">Thrown when <see cref="RateLimiterStrategyOptions.DefaultRateLimiterOptions"/> for <paramref name="options"/> are invalid.</exception>
     public static TBuilder AddRateLimiter<TBuilder>(
         this TBuilder builder,
         RateLimiterStrategyOptions options)
@@ -98,7 +101,15 @@ public static TBuilder AddRateLimiter<TBuilder>(
         Guard.NotNull(builder);
         Guard.NotNull(options);
 
-        builder.AddStrategy(context => new RateLimiterResilienceStrategy(options.RateLimiter!, options.OnRejected, context.Telemetry), options);
+        builder.AddStrategy(
+            context =>
+            {
+                return new RateLimiterResilienceStrategy(
+                    options.RateLimiter ?? new ConcurrencyLimiter(options.DefaultRateLimiterOptions),
+                    options.OnRejected,
+                    context.Telemetry);
+            },
+            options);
         return builder;
     }
 }

src/Polly.RateLimiting/RateLimiterStrategyOptions.cs

@@ -14,6 +14,23 @@ public class RateLimiterStrategyOptions : ResilienceStrategyOptions
     /// <remarks>Returns <c>RateLimiter</c> value.</remarks>
     public sealed override string StrategyType => RateLimiterConstants.StrategyType;
 
+    /// <summary>
+    /// Gets or sets the default rate limiter options.
+    /// </summary>
+    /// <remarks>
+    /// The options for the default limiter that will be used when <see cref="RateLimiter"/> is <see langword="null"/>.
+    /// <para>
+    /// <see cref="ConcurrencyLimiterOptions.PermitLimit"/> defaults to 1000.
+    /// <see cref="ConcurrencyLimiterOptions.QueueLimit"/> defaults to 0.
+    /// </para>
+    /// </remarks>
+    [Required]
+    public ConcurrencyLimiterOptions DefaultRateLimiterOptions { get; set; } = new()
+    {
+        QueueLimit = RateLimiterConstants.DefaultQueueLimit,
+        PermitLimit = RateLimiterConstants.DefaultPermitLimit,
+    };
+
     /// <summary>
     /// Gets or sets an event that is raised when the execution of user-provided callback is rejected by the rate limiter.
     /// </summary>
@@ -26,8 +43,8 @@ public class RateLimiterStrategyOptions : ResilienceStrategyOptions
     ///  Gets or sets the rate limiter that the strategy uses.
     /// </summary>
     /// <remarks>
-    /// This property is required and defaults to <see langword="null"/>.
+    /// Defaults to <see langword="null"/>. If this property is <see langword="null"/>,
+    /// then the strategy will use a <see cref="ConcurrencyLimiter"/> created using <see cref="DefaultRateLimiterOptions"/>.
     /// </remarks>
-    [Required]
     public RateLimiter? RateLimiter { get; set; }
 }

test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyBuilderExtensionsTests.cs

@@ -26,8 +26,9 @@ public class RateLimiterResilienceStrategyBuilderExtensionsTests
         },
         builder =>
         {
-            builder.AddRateLimiter(Mock.Of<RateLimiter>());
-            AssertRateLimiter(builder, hasEvents: false);
+            var expected = Mock.Of<RateLimiter>();
+            builder.AddRateLimiter(expected);
+            AssertRateLimiter(builder, hasEvents: false, limiter => limiter.Should().Be(expected));
         }
     };
 
@@ -42,6 +43,20 @@ public void AddRateLimiter_Extensions_Ok(Action<ResilienceStrategyBuilder<int>>
         builder.Build().Should().BeOfType<RateLimiterResilienceStrategy>();
     }
 
+    [Fact]
+    public void AddConcurrencyLimiter_InvalidOptions_Throws()
+    {
+        Assert.Throws<ArgumentException>(() =>
+        {
+            return new ResilienceStrategyBuilder().AddConcurrencyLimiter(new ConcurrencyLimiterOptions
+            {
+                PermitLimit = -10,
+                QueueLimit = -10
+            })
+            .Build();
+        });
+    }
+
     [Fact]
     public void AddRateLimiter_AllExtensions_Ok()
     {
@@ -71,28 +86,28 @@ public void AddRateLimiter_Ok()
     [Fact]
     public void AddRateLimiter_InvalidOptions_Throws()
     {
-        new ResilienceStrategyBuilder().Invoking(b => b.AddRateLimiter(new RateLimiterStrategyOptions()))
+        new ResilienceStrategyBuilder().Invoking(b => b.AddRateLimiter(new RateLimiterStrategyOptions { DefaultRateLimiterOptions = null! }))
             .Should()
             .Throw<ValidationException>()
             .WithMessage("""
             The 'RateLimiterStrategyOptions' are invalid.
 
             Validation Errors:
-            The RateLimiter field is required.
+            The DefaultRateLimiterOptions field is required.
             """);
     }
 
     [Fact]
     public void AddGenericRateLimiter_InvalidOptions_Throws()
     {
-        new ResilienceStrategyBuilder<int>().Invoking(b => b.AddRateLimiter(new RateLimiterStrategyOptions()))
+        new ResilienceStrategyBuilder<int>().Invoking(b => b.AddRateLimiter(new RateLimiterStrategyOptions { DefaultRateLimiterOptions = null! }))
             .Should()
             .Throw<ValidationException>()
             .WithMessage("""
             The 'RateLimiterStrategyOptions' are invalid.
 
             Validation Errors:
-            The RateLimiter field is required.
+            The DefaultRateLimiterOptions field is required.
             """);
     }
 
@@ -109,7 +124,7 @@ public void AddRateLimiter_Options_Ok()
         strategy.Should().BeOfType<RateLimiterResilienceStrategy>();
     }
 
-    private static void AssertRateLimiter(ResilienceStrategyBuilder<int> builder, bool hasEvents)
+    private static void AssertRateLimiter(ResilienceStrategyBuilder<int> builder, bool hasEvents, Action<RateLimiter>? assertLimiter = null)
     {
         var strategy = GetResilienceStrategy(builder.Build());
         strategy.Limiter.Should().NotBeNull();
@@ -125,6 +140,8 @@ private static void AssertRateLimiter(ResilienceStrategyBuilder<int> builder, bo
         {
             strategy.OnLeaseRejected.Should().BeNull();
         }
+
+        assertLimiter?.Invoke(strategy.Limiter);
     }
 
     private static void AssertConcurrencyLimiter(ResilienceStrategyBuilder<int> builder, bool hasEvents)

test/Polly.RateLimiting.Tests/RateLimiterStrategyOptionsTests.cs

@@ -10,5 +10,7 @@ public void Ctor_EnsureDefaults()
         options.StrategyType.Should().Be(RateLimiterConstants.StrategyType);
         options.RateLimiter.Should().BeNull();
         options.OnRejected.Should().BeNull();
+        options.DefaultRateLimiterOptions.PermitLimit.Should().Be(1000);
+        options.DefaultRateLimiterOptions.QueueLimit.Should().Be(0);
     }
 }