feat: add IS_REPEATABLE support to RateLimit annotation #1015
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enable Attribute::IS_REPEATABLE flag on RateLimit annotation. Refactor RateLimitAspect to handle multiple annotations. Maintain backward compatibility with single annotation usage. Add comprehensive tests for new functionality. This change allows developers to apply multiple rate limit rules on a single method, enabling sophisticated rate limiting strategies. BREAKING CHANGE: None - fully backward compatible
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
概览将RateLimit注解改造为支持重复应用，允许在单个方法上添加多个RateLimit配置。通过将RateLimit注解转为可重复属性，并更新Aspect处理逻辑以迭代处理所有注解实例，实现对多个速率限制规则的支持。 变更
时序图sequenceDiagram
participant Client
participant Aspect as RateLimitAspect
participant MultiAnno as MultipleAnnotation
participant Factory as RateLimiterFactory
participant Limiter as RateLimiter
Client->>Aspect: 触发受保护方法
Aspect->>MultiAnno: 获取所有RateLimit注解
loop 遍历每个注解配置
MultiAnno->>Aspect: 返回单个RateLimit注解
Aspect->>Aspect: 解析限制键值
Aspect->>Factory: 创建或获取限制器
Factory->>Limiter: 初始化限制器
Aspect->>Limiter: 检查请求是否超限
alt 超限
Limiter-->>Aspect: 触发限制
Aspect-->>Client: 抛出异常
else 未超限
Limiter-->>Aspect: 允许继续
end
end
Aspect->>Client: 执行目标方法/返回结果
代码审查工作量评估🎯 2 (简单) | ⏱️ ~12分钟
诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/rate-limit/src/Aspect/RateLimitAspect.php (1)
50-65: 考虑多规则违规时的用户体验当多个速率限制规则同时被触发时，当前实现只会抛出第一个违规规则的异常。这可能导致以下问题：
- 用户无法了解是哪个具体规则被触发（如果有多个规则同时超限）
- 注解的声明顺序会影响错误消息，这可能不够直观
- 调试时难以识别所有被违反的限制规则
建议考虑以下改进方案之一：
- 在异常消息中明确指出是哪个规则被触发（例如包含规则的 key）
- 在文档中说明多规则的顺序评估行为
- 考虑收集所有违规规则并在异常中提供更详细的信息
可以应用此差异在异常消息中包含规则标识：
throw new RateLimitException( - $message, + "[{$annotation->key}] {$message}", $annotation->responseCode, $availableIn );tests/RateLimit/RepeatableAnnotationTest.php (1)
28-41: 考虑添加集成测试以验证实际限流行为当前测试验证了元数据结构的处理，但缺少对实际速率限制执行逻辑的测试。建议添加集成测试来验证：
- 多个速率限制规则是否都被正确执行
- 当某个规则被触发时是否抛出正确的异常
- 当所有规则都通过时请求是否能正常处理
您希望我帮助生成集成测试代码，或创建一个新的 issue 来跟踪此任务吗？
tests/RateLimit/MultipleRateLimitExample.php (1)
30-50: 补充占位符解析的说明示例中使用了
{ip}和{user_id}占位符，但没有说明这些值如何从请求中提取和解析。建议在注释或文档中补充：
- 占位符值的来源（例如从哪些参数或上下文中获取）
- 如何确保这些值在运行时可用
- 如果占位符无法解析时的回退行为
这将帮助用户更好地理解和使用此功能。
基于学习内容（从
resolveKey方法的实现可以看出，占位符从$proceedingJoinPoint->arguments['keys']中解析）
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/rate-limit/src/Annotation/RateLimit.php(1 hunks)src/rate-limit/src/Aspect/RateLimitAspect.php(1 hunks)tests/RateLimit/MultipleRateLimitExample.php(1 hunks)tests/RateLimit/RepeatableAnnotationTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/rate-limit/src/Aspect/RateLimitAspect.php (5)
tests/RateLimit/MultipleRateLimitExample.php (4)
RateLimit(30-57)RateLimit(64-85)RateLimit(92-113)RateLimit(119-128)src/rate-limit/src/Middleware/RateLimitMiddleware.php (2)
process(64-81)resolveKey(86-90)src/rate-limit/src/RateLimiterFactory.php (1)
make(34-51)src/rate-limit/src/Contract/RateLimiterInterface.php (2)
tooManyAttempts(34-34)availableIn(66-66)src/rate-limit/src/Exception/RateLimitException.php (1)
RateLimitException(17-27)
tests/RateLimit/RepeatableAnnotationTest.php (2)
tests/RateLimit/MultipleRateLimitExample.php (4)
RateLimit(30-57)RateLimit(64-85)RateLimit(92-113)RateLimit(119-128)src/rate-limit/src/Annotation/RateLimit.php (1)
Attribute(18-40)
🪛 PHPMD (2.15.0)
tests/RateLimit/MultipleRateLimitExample.php
78-78: Avoid unused parameters such as '$username'. (undefined)
(UnusedFormalParameter)
78-78: Avoid unused parameters such as '$password'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
🔇 Additional comments (8)
src/rate-limit/src/Aspect/RateLimitAspect.php (2)
38-48: 向后兼容性处理得当！将单个注解包装为数组的逻辑确保了新功能与现有代码的兼容性，实现清晰简洁。
50-67: 整体逻辑正确，实现了"所有规则必须通过"的语义循环处理所有注解并在任一规则触发时拒绝请求的设计是合理的，符合速率限制的预期行为。这种实现保持了与单注解场景的一致性。
src/rate-limit/src/Annotation/RateLimit.php (1)
18-18: IS_REPEATABLE 标志添加正确！使用标准的 PHP 8 Attribute 语法添加 IS_REPEATABLE 支持，实现简洁且符合最佳实践。
tests/RateLimit/RepeatableAnnotationTest.php (3)
14-26: 反射测试实现正确！通过反射验证 IS_REPEATABLE 标志的测试方法恰当，能够确保注解声明的正确性。
43-57: 向后兼容性测试完善！验证单注解场景的测试确保了现有代码不会受到影响，这对于保持 API 稳定性很重要。
59-105: 配置差异化测试覆盖全面！测试验证了多个速率限制规则可以使用不同的算法和配置参数，覆盖了主要使用场景。
tests/RateLimit/MultipleRateLimitExample.php (2)
64-84: 登录限流示例设计合理！使用不同的响应码（429 vs 503）来区分不同类型的限流（IP 限流 vs 全局限流）是一个很好的实践。静态分析工具关于未使用参数的警告可以忽略，因为这只是示例代码。
119-127: 向后兼容性示例清晰明了！单注解的使用示例很好地展示了新功能不会破坏现有代码，这对于渐进式采用新特性很重要。
…tation for better handling of repeatable annotations
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/rate-limit/src/Aspect/RateLimitAspect.php (1)
30-56: 多注解处理逻辑清晰，但建议增强对单注解场景的防御性兼容
- 正向看，目前通过
MultipleAnnotation::toAnnotations()循环每个RateLimit，并按每条规则分别 resolve key、创建 limiter、检测tooManyAttempts()，只要任一规则超限就抛RateLimitException，满足“多层限流全部必须通过”的预期语义，这点设计很好。- 但这里直接假定
$metadata->method[RateLimit::class]一定是MultipleAnnotation实例，如果在某些场景下（例如旧版本缓存或第三方扩展）仍返回单个RateLimit实例，将在调用$annotations?->toAnnotations()时触发Call to undefined method RateLimit::toAnnotations()的致命错误。- 建议稍作防御性处理，同时兼容
MultipleAnnotation与单个RateLimit，增强稳健性且不影响当前新行为，例如：- /** @var null|MultipleAnnotation $annotations */ - $annotations = $metadata->method[RateLimit::class] ?? null; - - foreach ($annotations?->toAnnotations() ?? [] as $annotation) { - /** @var RateLimit $annotation */ + $raw = $metadata->method[RateLimit::class] ?? null; + + if ($raw instanceof MultipleAnnotation) { + $annotations = $raw->toAnnotations(); + } elseif ($raw instanceof RateLimit) { + $annotations = [$raw]; + } else { + $annotations = []; + } + + foreach ($annotations as $annotation) { + /** @var RateLimit $annotation */ $key = $this->resolveKey($annotation->key, $proceedingJoinPoint); $limiter = $this->factory->make($annotation->algorithm, $annotation->pool);
- 这样既保持了当前多注解支持，又对可能出现的单注解返回值更容错。
tests/RateLimit/MultipleAnnotationTest.php (1)
15-113: 测试覆盖整体不错，但可考虑补充直接针对 RateLimitAspect::process 的用例
- 用
ReflectionClass + Attribute验证IS_REPEATABLEflag、用MultipleAnnotation验证多注解与单注解的转换，以及多组RateLimit配置的字段与算法值，整体把核心结构都跑了一遍，这对回归很有价值。- 测试名中多次提到 “RateLimit aspect can handle ...”，但当前这些测试实际上只在注解对象和
MultipleAnnotation层面断言属性，并未真正构造ProceedingJoinPoint去触达RateLimitAspect::process()的分支逻辑。- 如希望进一步锁死行为回归，建议补充一两个针对
RateLimitAspect::process的单元/集成用例：构造带有多条RateLimit注解的虚拟 metadata（包含不同 key、algorithm），注入一个假RateLimiterFactory/RateLimiterInterface，分别断言“全部通过时调用了process()”以及“任一规则超限时抛出RateLimitException且 message/availableIn 正确”。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/rate-limit/src/Annotation/RateLimit.php(1 hunks)src/rate-limit/src/Aspect/RateLimitAspect.php(2 hunks)tests/RateLimit/MultipleAnnotationTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/rate-limit/src/Aspect/RateLimitAspect.php (4)
src/rate-limit/src/Annotation/RateLimit.php (1)
__construct(30-39)src/rate-limit/src/Exception/RateLimitException.php (2)
__construct(19-26)RateLimitException(17-27)src/rate-limit/src/RateLimiterFactory.php (3)
__construct(30-32)RateLimiterFactory(23-62)make(34-51)src/rate-limit/src/Contract/RateLimiterInterface.php (2)
tooManyAttempts(34-34)availableIn(66-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
🔇 Additional comments (1)
src/rate-limit/src/Annotation/RateLimit.php (1)
16-19: RateLimit 可重复注解实现合理，保持了向后兼容
- 从
AbstractAnnotation切换到AbstractMultipleAnnotation，并在#[Attribute(... | Attribute::IS_REPEATABLE)]中加入IS_REPEATABLE，与 Hyperf 多注解（MultipleAnnotation）的用法一致，同时构造函数签名未变，对现有代码基本是向后兼容的。- 当前仅允许
TARGET_METHOD，符合本组件的主要用法场景；如果后续要在类或其他目标上复用限流规则，可再按需扩展 target flag，而不影响当前实现。
huangdijia
added a commit
that referenced
this pull request
Nov 18, 2025
* feat: add IS_REPEATABLE support to RateLimit annotation Enable Attribute::IS_REPEATABLE flag on RateLimit annotation. Refactor RateLimitAspect to handle multiple annotations. Maintain backward compatibility with single annotation usage. Add comprehensive tests for new functionality. This change allows developers to apply multiple rate limit rules on a single method, enabling sophisticated rate limiting strategies. BREAKING CHANGE: None - fully backward compatible * refactor: simplify constructor syntax in RateLimitAspect * fix: update RateLimit and RateLimitAspect to use AbstractMultipleAnnotation for better handling of repeatable annotations * test: enhance RepeatableAnnotationTest for IS_REPEATABLE support in RateLimit * test: add MultipleAnnotationTest for RateLimit functionality * chore: remove unnecessary blank lines in MultipleAnnotationTest * fix: improve key resolution logic in RateLimitAspect for better default handling --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds \Attribute::IS_REPEATABLE\ support to the RateLimit annotation, allowing developers to apply multiple rate limit rules to a single method.
Changes
Features
Multiple Rate Limit Rules
Backward Compatibility
Single annotation usage continues to work exactly as before:
Test Coverage
Benefits
Breaking Changes
None - fully backward compatible.
Summary by CodeRabbit
版本说明
新功能
测试