From 4c3448be325d904d85bcb4faaf45d9a678c23dca Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 6 Mar 2026 13:46:29 +0800 Subject: [PATCH] refactor: improve operation rate limit rule handling and add multi-tenancy support in policy builder tests --- .../FixedWindowOperationRateLimitRule.cs | 4 +- .../OperationRateLimitPolicyBuilder.cs | 15 ++++-- .../OperationRateLimitRuleBuilder.cs | 23 +++++---- .../OperationRateLimitPolicyBuilder_Tests.cs | 48 +++++++++++++++++++ 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/FixedWindowOperationRateLimitRule.cs b/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/FixedWindowOperationRateLimitRule.cs index 737e957788..a13d00c087 100644 --- a/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/FixedWindowOperationRateLimitRule.cs +++ b/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/FixedWindowOperationRateLimitRule.cs @@ -24,7 +24,7 @@ public class FixedWindowOperationRateLimitRule : IOperationRateLimitRule IOperationRateLimitStore store, ICurrentUser currentUser, ICurrentTenant currentTenant, - IClientIpAddressProvider clientInfoProvider) + IClientIpAddressProvider clientIpAddressProvider) { PolicyName = policyName; RuleIndex = ruleIndex; @@ -32,7 +32,7 @@ public class FixedWindowOperationRateLimitRule : IOperationRateLimitRule Store = store; CurrentUser = currentUser; CurrentTenant = currentTenant; - ClientIpAddressProvider = clientInfoProvider; + ClientIpAddressProvider = clientIpAddressProvider; } public virtual async Task AcquireAsync( diff --git a/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder.cs b/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder.cs index 173af66758..a420b088fb 100644 --- a/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder.cs +++ b/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder.cs @@ -22,9 +22,12 @@ public class OperationRateLimitPolicyBuilder public OperationRateLimitPolicyBuilder AddRule( Action configure) { - var builder = new OperationRateLimitRuleBuilder(); + var builder = new OperationRateLimitRuleBuilder(this); configure(builder); - _rules.Add(builder.Build()); + if (!builder.IsCommitted) + { + _rules.Add(builder.Build()); + } return this; } @@ -74,15 +77,17 @@ public class OperationRateLimitPolicyBuilder } var duplicate = _rules - .GroupBy(r => (r.Duration, r.MaxCount, r.PartitionType)) + .Where(r => r.PartitionType != OperationRateLimitPartitionType.Custom) + .GroupBy(r => (r.Duration, r.MaxCount, r.PartitionType, r.IsMultiTenant)) .FirstOrDefault(g => g.Count() > 1); if (duplicate != null) { - var (duration, maxCount, partitionType) = duplicate.Key; + var (duration, maxCount, partitionType, isMultiTenant) = duplicate.Key; throw new AbpException( $"Operation rate limit policy '{_name}' has duplicate rules with the same " + - $"Duration ({duration}), MaxCount ({maxCount}), and PartitionType ({partitionType}). " + + $"Duration ({duration}), MaxCount ({maxCount}), PartitionType ({partitionType}), " + + $"and IsMultiTenant ({isMultiTenant}). " + "Each rule in a policy must have a unique combination of these properties."); } diff --git a/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitRuleBuilder.cs b/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitRuleBuilder.cs index 98dfd65f92..6cf8a89921 100644 --- a/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitRuleBuilder.cs +++ b/framework/src/Volo.Abp.OperationRateLimit/Volo/Abp/OperationRateLimit/OperationRateLimitRuleBuilder.cs @@ -4,16 +4,14 @@ namespace Volo.Abp.OperationRateLimit; public class OperationRateLimitRuleBuilder { - private readonly OperationRateLimitPolicyBuilder? _policyBuilder; + private readonly OperationRateLimitPolicyBuilder _policyBuilder; private TimeSpan _duration; private int _maxCount; private OperationRateLimitPartitionType? _partitionType; private Func? _customPartitionKeyResolver; private bool _isMultiTenant; - public OperationRateLimitRuleBuilder() - { - } + internal bool IsCommitted { get; private set; } internal OperationRateLimitRuleBuilder(OperationRateLimitPolicyBuilder policyBuilder) { @@ -41,7 +39,7 @@ public class OperationRateLimitRuleBuilder { _partitionType = OperationRateLimitPartitionType.Parameter; CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } /// @@ -51,7 +49,7 @@ public class OperationRateLimitRuleBuilder { _partitionType = OperationRateLimitPartitionType.CurrentUser; CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } /// @@ -61,7 +59,7 @@ public class OperationRateLimitRuleBuilder { _partitionType = OperationRateLimitPartitionType.CurrentTenant; CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } /// @@ -71,7 +69,7 @@ public class OperationRateLimitRuleBuilder { _partitionType = OperationRateLimitPartitionType.ClientIp; CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } /// @@ -82,7 +80,7 @@ public class OperationRateLimitRuleBuilder { _partitionType = OperationRateLimitPartitionType.Email; CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } /// @@ -93,7 +91,7 @@ public class OperationRateLimitRuleBuilder { _partitionType = OperationRateLimitPartitionType.PhoneNumber; CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } /// @@ -105,12 +103,13 @@ public class OperationRateLimitRuleBuilder _partitionType = OperationRateLimitPartitionType.Custom; _customPartitionKeyResolver = Check.NotNull(keyResolver, nameof(keyResolver)); CommitToPolicyBuilder(); - return _policyBuilder!; + return _policyBuilder; } protected virtual void CommitToPolicyBuilder() { - _policyBuilder?.AddRuleDefinition(Build()); + _policyBuilder.AddRuleDefinition(Build()); + IsCommitted = true; } internal OperationRateLimitRuleDefinition Build() diff --git a/framework/test/Volo.Abp.OperationRateLimit.Tests/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder_Tests.cs b/framework/test/Volo.Abp.OperationRateLimit.Tests/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder_Tests.cs index 1be970f4f4..76dac315ae 100644 --- a/framework/test/Volo.Abp.OperationRateLimit.Tests/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder_Tests.cs +++ b/framework/test/Volo.Abp.OperationRateLimit.Tests/Volo/Abp/OperationRateLimit/OperationRateLimitPolicyBuilder_Tests.cs @@ -206,4 +206,52 @@ public class OperationRateLimitPolicyBuilder_Tests exception.Message.ShouldContain("maxCount >= 0"); } + + [Fact] + public void Should_Allow_Same_Rule_With_Different_MultiTenancy() + { + var options = new AbpOperationRateLimitOptions(); + + // Same Duration/MaxCount/PartitionType but different IsMultiTenant should be allowed + options.AddPolicy("MultiTenancyPolicy", policy => + { + policy.AddRule(rule => rule + .WithFixedWindow(TimeSpan.FromHours(1), maxCount: 5) + .PartitionByParameter()); + + policy.AddRule(rule => rule + .WithFixedWindow(TimeSpan.FromHours(1), maxCount: 5) + .WithMultiTenancy() + .PartitionByParameter()); + }); + + var policy = options.Policies["MultiTenancyPolicy"]; + policy.Rules.Count.ShouldBe(2); + policy.Rules[0].IsMultiTenant.ShouldBeFalse(); + policy.Rules[1].IsMultiTenant.ShouldBeTrue(); + } + + [Fact] + public void Should_Allow_Multiple_Custom_Partition_Rules() + { + var options = new AbpOperationRateLimitOptions(); + + // Multiple custom partition rules with same Duration/MaxCount should be allowed + // because they may use different key resolvers + options.AddPolicy("MultiCustomPolicy", policy => + { + policy.AddRule(rule => rule + .WithFixedWindow(TimeSpan.FromHours(1), maxCount: 5) + .PartitionBy(ctx => $"by-ip:{ctx.Parameter}")); + + policy.AddRule(rule => rule + .WithFixedWindow(TimeSpan.FromHours(1), maxCount: 5) + .PartitionBy(ctx => $"by-device:{ctx.Parameter}")); + }); + + var policy = options.Policies["MultiCustomPolicy"]; + policy.Rules.Count.ShouldBe(2); + policy.Rules[0].PartitionType.ShouldBe(OperationRateLimitPartitionType.Custom); + policy.Rules[1].PartitionType.ShouldBe(OperationRateLimitPartitionType.Custom); + } }