Merge branch 'Operation-Rate-Limiting' into Operation-Rate-Limiting-Test

docs/en/framework/infrastructure/operation-rate-limiting.md

@@ -283,6 +283,7 @@ The exception includes the following data properties:
 | `CurrentCount` | int | Current usage count |
 | `RemainingCount` | int | Remaining allowed count |
 | `RetryAfterSeconds` | int | Seconds until the window resets |
+| `RetryAfterMinutes` | int | Minutes until the window resets (rounded down) |
 | `RetryAfter` | string | Localized retry-after description (e.g., "5 minutes") |
 | `WindowDurationSeconds` | int | Total window duration in seconds |
 | `WindowDescription` | string | Localized window description |

framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Checker/OperationRateLimitingChecker.cs

@@ -65,16 +65,31 @@ public class OperationRateLimitingChecker : IOperationRateLimitingChecker, ITran
             ThrowRateLimitException(policy, aggregatedResult, context);
         }
 
-        // Phase 2: All rules pass - now increment all counters.
-        // Also guard against a concurrent race where another request consumed the last quota
+        // Phase 2: All rules passed in Phase 1 - now increment counters.
+        // Guard against concurrent races where another request consumed the last quota
         // between Phase 1 and Phase 2.
+        // Once any rule fails during increment, stop incrementing subsequent rules
+        // to minimize wasted quota. Remaining rules use read-only check instead.
         var incrementResults = new List<OperationRateLimitingRuleResult>();
+        var phase2Failed = false;
         foreach (var rule in rules)
         {
-            incrementResults.Add(await rule.AcquireAsync(context));
+            if (phase2Failed)
+            {
+                incrementResults.Add(await rule.CheckAsync(context));
+            }
+            else
+            {
+                var result = await rule.AcquireAsync(context);
+                incrementResults.Add(result);
+                if (!result.IsAllowed)
+                {
+                    phase2Failed = true;
+                }
+            }
         }
 
-        if (incrementResults.Any(r => !r.IsAllowed))
+        if (phase2Failed)
         {
             var aggregatedResult = AggregateResults(incrementResults, policy);
             ThrowRateLimitException(policy, aggregatedResult, context);
@@ -132,6 +147,11 @@ public class OperationRateLimitingChecker : IOperationRateLimitingChecker, ITran
 
     public virtual async Task ResetAsync(string policyName, OperationRateLimitingContext? context = null)
     {
+        if (!Options.IsEnabled)
+        {
+            return;
+        }
+
         context = EnsureContext(context);
         var policy = await PolicyProvider.GetAsync(policyName);
         var rules = CreateRules(policy);
@@ -153,12 +173,11 @@ public class OperationRateLimitingChecker : IOperationRateLimitingChecker, ITran
     {
         var rules = new List<IOperationRateLimitingRule>();
 
-        for (var i = 0; i < policy.Rules.Count; i++)
+        foreach (var ruleDefinition in policy.Rules)
         {
             rules.Add(new FixedWindowOperationRateLimitingRule(
                 policy.Name,
-                i,
-                policy.Rules[i],
+                ruleDefinition,
                 Store,
                 CurrentUser,
                 CurrentTenant,
@@ -188,7 +207,7 @@ public class OperationRateLimitingChecker : IOperationRateLimitingChecker, ITran
             IsAllowed = isAllowed,
             RemainingCount = mostRestrictive.RemainingCount,
             MaxCount = mostRestrictive.MaxCount,
-            CurrentCount = mostRestrictive.MaxCount - mostRestrictive.RemainingCount,
+            CurrentCount = mostRestrictive.CurrentCount,
             RetryAfter = ruleResults.Any(r => !r.IsAllowed && r.RetryAfter.HasValue)
                 ? ruleResults
                     .Where(r => !r.IsAllowed && r.RetryAfter.HasValue)
@@ -233,7 +252,7 @@ public class OperationRateLimitingChecker : IOperationRateLimitingChecker, ITran
                     ["IsAllowed"] = ruleResult.IsAllowed,
                     ["MaxCount"] = ruleResult.MaxCount,
                     ["RemainingCount"] = ruleResult.RemainingCount,
-                    ["CurrentCount"] = ruleResult.MaxCount - ruleResult.RemainingCount,
+                    ["CurrentCount"] = ruleResult.CurrentCount,
                     ["WindowDurationSeconds"] = (int)ruleResult.WindowDuration.TotalSeconds,
                     ["WindowDescription"] = ruleResult.WindowDuration > TimeSpan.Zero
                         ? formatter.Format(ruleResult.WindowDuration)

framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Checker/OperationRateLimitingRuleResult.cs

@@ -8,6 +8,8 @@ public class OperationRateLimitingRuleResult
 
     public bool IsAllowed { get; set; }
 
+    public int CurrentCount { get; set; }
+
     public int RemainingCount { get; set; }
 
     public int MaxCount { get; set; }

framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Rules/FixedWindowOperationRateLimitingRule.cs

@@ -10,7 +10,6 @@ public class FixedWindowOperationRateLimitingRule : IOperationRateLimitingRule
     private const string HostTenantKey = "host";
 
     protected string PolicyName { get; }
-    protected int RuleIndex { get; }
     protected OperationRateLimitingRuleDefinition Definition { get; }
     protected IOperationRateLimitingStore Store { get; }
     protected ICurrentUser CurrentUser { get; }
@@ -19,7 +18,6 @@ public class FixedWindowOperationRateLimitingRule : IOperationRateLimitingRule
 
     public FixedWindowOperationRateLimitingRule(
         string policyName,
-        int ruleIndex,
         OperationRateLimitingRuleDefinition definition,
         IOperationRateLimitingStore store,
         ICurrentUser currentUser,
@@ -27,7 +25,6 @@ public class FixedWindowOperationRateLimitingRule : IOperationRateLimitingRule
         IWebClientInfoProvider webClientInfoProvider)
     {
         PolicyName = policyName;
-        RuleIndex = ruleIndex;
         Definition = definition;
         Store = store;
         CurrentUser = currentUser;
@@ -140,6 +137,7 @@ public class FixedWindowOperationRateLimitingRule : IOperationRateLimitingRule
         {
             RuleName = $"{PolicyName}:Rule[{(long)Definition.Duration.TotalSeconds}s,{Definition.MaxCount},{Definition.PartitionType}]",
             IsAllowed = storeResult.IsAllowed,
+            CurrentCount = storeResult.CurrentCount,
             RemainingCount = storeResult.MaxCount - storeResult.CurrentCount,
             MaxCount = storeResult.MaxCount,
             RetryAfter = storeResult.RetryAfter,

framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/AbpOperationRateLimitingPhase2EarlyBreakTestModule.cs

@@ -0,0 +1,102 @@
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Volo.Abp.Autofac;
+using Volo.Abp.ExceptionHandling;
+using Volo.Abp.Modularity;
+
+namespace Volo.Abp.OperationRateLimiting;
+
+/// <summary>
+/// A mock store that simulates a multi-rule Phase 2 race condition:
+/// - GetAsync always reports quota available (Phase 1 passes for all rules).
+/// - IncrementAsync succeeds for the first call, fails on the second call
+///   (simulating a concurrent race on Rule2), and tracks total increment calls
+///   so tests can verify that Rule3 was never incremented (early break).
+/// </summary>
+internal class MultiRuleRaceConditionSimulatorStore : IOperationRateLimitingStore
+{
+    private int _incrementCallCount;
+
+    /// <summary>
+    /// Total number of IncrementAsync calls made.
+    /// </summary>
+    public int IncrementCallCount => _incrementCallCount;
+
+    public Task<OperationRateLimitingStoreResult> GetAsync(string key, TimeSpan duration, int maxCount)
+    {
+        return Task.FromResult(new OperationRateLimitingStoreResult
+        {
+            IsAllowed = true,
+            CurrentCount = 0,
+            MaxCount = maxCount
+        });
+    }
+
+    public Task<OperationRateLimitingStoreResult> IncrementAsync(string key, TimeSpan duration, int maxCount)
+    {
+        var callIndex = Interlocked.Increment(ref _incrementCallCount);
+
+        if (callIndex == 2)
+        {
+            // Second rule: simulate concurrent race - another request consumed the last slot.
+            return Task.FromResult(new OperationRateLimitingStoreResult
+            {
+                IsAllowed = false,
+                CurrentCount = maxCount,
+                MaxCount = maxCount,
+                RetryAfter = duration
+            });
+        }
+
+        // First rule (and any others if early break fails): succeed.
+        return Task.FromResult(new OperationRateLimitingStoreResult
+        {
+            IsAllowed = true,
+            CurrentCount = 1,
+            MaxCount = maxCount
+        });
+    }
+
+    public Task ResetAsync(string key)
+    {
+        return Task.CompletedTask;
+    }
+}
+
+[DependsOn(
+    typeof(AbpOperationRateLimitingModule),
+    typeof(AbpExceptionHandlingModule),
+    typeof(AbpTestBaseModule),
+    typeof(AbpAutofacModule)
+)]
+public class AbpOperationRateLimitingPhase2EarlyBreakTestModule : AbpModule
+{
+    public override void ConfigureServices(ServiceConfigurationContext context)
+    {
+        context.Services.Replace(
+            ServiceDescriptor.Singleton<IOperationRateLimitingStore, MultiRuleRaceConditionSimulatorStore>());
+
+        Configure<AbpOperationRateLimitingOptions>(options =>
+        {
+            // 3-rule composite policy: all PartitionByParameter with different durations
+            // so they generate unique cache keys and don't trigger duplicate rule validation.
+            options.AddPolicy("TestMultiRuleRacePolicy", policy =>
+            {
+                policy.AddRule(rule => rule
+                    .WithFixedWindow(TimeSpan.FromHours(1), maxCount: 5)
+                    .PartitionByParameter());
+
+                policy.AddRule(rule => rule
+                    .WithFixedWindow(TimeSpan.FromHours(2), maxCount: 5)
+                    .PartitionByParameter());
+
+                policy.AddRule(rule => rule
+                    .WithFixedWindow(TimeSpan.FromHours(3), maxCount: 5)
+                    .PartitionByParameter());
+            });
+        });
+    }
+}

framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingCheckerFixes_Tests.cs

@@ -97,6 +97,59 @@ public class OperationRateLimitingCheckerPhase1_Tests : OperationRateLimitingTes
     }
 }
 
+/// <summary>
+/// Tests for Phase 2 early break: when a multi-rule policy encounters a race condition
+/// in Phase 2 (Rule2 fails), Rule3 should NOT be incremented.
+/// Uses a mock store where IncrementAsync fails on the 2nd call.
+/// </summary>
+public class OperationRateLimitingCheckerPhase2EarlyBreak_Tests
+    : AbpIntegratedTest<AbpOperationRateLimitingPhase2EarlyBreakTestModule>
+{
+    protected override void SetAbpApplicationCreationOptions(AbpApplicationCreationOptions options)
+    {
+        options.UseAutofac();
+    }
+
+    [Fact]
+    public async Task Should_Not_Increment_Remaining_Rules_After_Phase2_Failure()
+    {
+        // 3-rule policy. Mock store: Rule1 increment succeeds, Rule2 increment fails (race),
+        // Rule3 should NOT be incremented due to early break.
+        var checker = GetRequiredService<IOperationRateLimitingChecker>();
+        var store = (MultiRuleRaceConditionSimulatorStore)GetRequiredService<IOperationRateLimitingStore>();
+        var context = new OperationRateLimitingContext { Parameter = "early-break-test" };
+
+        var exception = await Assert.ThrowsAsync<AbpOperationRateLimitingException>(async () =>
+        {
+            await checker.CheckAsync("TestMultiRuleRacePolicy", context);
+        });
+
+        exception.PolicyName.ShouldBe("TestMultiRuleRacePolicy");
+        exception.Result.IsAllowed.ShouldBeFalse();
+
+        // Key assertion: only 2 IncrementAsync calls were made (Rule1 + Rule2).
+        // Rule3 was skipped (used CheckAsync instead) due to early break.
+        store.IncrementCallCount.ShouldBe(2);
+    }
+
+    [Fact]
+    public async Task Should_Include_All_Rule_Results_Despite_Early_Break()
+    {
+        // Even with early break, the aggregated result should contain all 3 rules
+        // (Rule3 via CheckAsync instead of AcquireAsync).
+        var checker = GetRequiredService<IOperationRateLimitingChecker>();
+        var context = new OperationRateLimitingContext { Parameter = $"all-results-{Guid.NewGuid()}" };
+
+        var exception = await Assert.ThrowsAsync<AbpOperationRateLimitingException>(async () =>
+        {
+            await checker.CheckAsync("TestMultiRuleRacePolicy", context);
+        });
+
+        exception.Result.RuleResults.ShouldNotBeNull();
+        exception.Result.RuleResults!.Count.ShouldBe(3);
+    }
+}
+
 /// <summary>
 /// Tests for Fix #1: Phase 2 in CheckAsync now checks the result of AcquireAsync.
 /// Uses a mock store that simulates a concurrent race condition:

framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingChecker_Tests.cs

@@ -731,6 +731,57 @@ public class OperationRateLimitingChecker_Tests : OperationRateLimitingTestBase
         });
     }
 
+
+    [Fact]
+    public async Task Should_Return_Correct_CurrentCount_In_RuleResults()
+    {
+        var param = $"current-count-{Guid.NewGuid()}";
+        var context = new OperationRateLimitingContext { Parameter = param };
+
+        await _checker.CheckAsync("TestSimple", context);
+        await _checker.CheckAsync("TestSimple", context);
+
+        var status = await _checker.GetStatusAsync("TestSimple", context);
+        status.RuleResults.ShouldNotBeNull();
+        status.RuleResults!.Count.ShouldBe(1);
+        status.RuleResults[0].CurrentCount.ShouldBe(2);
+        status.RuleResults[0].RemainingCount.ShouldBe(1);
+        status.RuleResults[0].MaxCount.ShouldBe(3);
+    }
+
+    [Fact]
+    public async Task ResetAsync_Should_Skip_When_Disabled()
+    {
+        var options = GetRequiredService<Microsoft.Extensions.Options.IOptions<AbpOperationRateLimitingOptions>>();
+        var originalValue = options.Value.IsEnabled;
+
+        try
+        {
+            var param = $"reset-disabled-{Guid.NewGuid()}";
+            var context = new OperationRateLimitingContext { Parameter = param };
+
+            // Exhaust the quota
+            await _checker.CheckAsync("TestSimple", context);
+            await _checker.CheckAsync("TestSimple", context);
+            await _checker.CheckAsync("TestSimple", context);
+
+            // Disable and call ResetAsync — should be a no-op (counter not actually reset)
+            options.Value.IsEnabled = false;
+            await _checker.ResetAsync("TestSimple", context);
+
+            // Re-enable: quota should still be exhausted because reset was skipped
+            options.Value.IsEnabled = true;
+            await Assert.ThrowsAsync<AbpOperationRateLimitingException>(async () =>
+            {
+                await _checker.CheckAsync("TestSimple", context);
+            });
+        }
+        finally
+        {
+            options.Value.IsEnabled = originalValue;
+        }
+    }
+
     private static ClaimsPrincipal CreateClaimsPrincipal(Guid userId)
     {
         return new ClaimsPrincipal(