-
Notifications
You must be signed in to change notification settings - Fork 149
extend: Add rate-limiting examples and update tests for validation #2533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughA new rate-limiting example YAML configuration file is introduced with global and per-endpoint rate limiter scenarios. Corresponding test methods are refactored with renamed methods, consolidated imports, and updated assertion styles from explicit to generic patterns. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/RateLimiterExampleTest.java (1)
17-21: Consider explicit imports over wildcards.While wildcard imports work fine here, explicit imports can improve code clarity by making it obvious where each class and method originates. This is particularly helpful for readers unfamiliar with the codebase and can prevent potential naming conflicts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
distribution/examples/rate-limiting/apis.yamldistribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/RateLimiterExampleTest.java
⏰ 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). (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (6)
distribution/examples/rate-limiting/apis.yaml (4)
3-6: LGTM!The global rate limiter configuration is correctly structured with a valid ISO 8601 duration format (PT1M = 1 minute).
8-14: LGTM!The first API definition is clean and correctly demonstrates an endpoint subject to the global rate limiter.
17-30: LGTM!The endpoint-specific rate limiter configuration correctly demonstrates per-user rate limiting using JSONPath. The keyExpression
$.userappropriately extracts the user identifier from the JSON request body, and PT30S (30 seconds) is a valid ISO 8601 duration format suitable for password reset scenarios.
1-1: Align schema version with codebase standard and fix outlier.The schema reference to v7.0.5 is consistent across nearly all configuration files in the repository. However,
distribution/examples/extending-membrane/global-interceptor/apis.yamluses v7.0.4, creating an inconsistency. Update that file to use v7.0.5 to match the rest of the codebase.distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/RateLimiterExampleTest.java (2)
31-43: LGTM!The test correctly validates the global rate limiter behavior:
- Sends exactly 10 requests (matching the limit in apis.yaml)
- Verifies the 11th request is rate-limited (429)
- Confirms appropriate log entries
The method rename and assertion style updates align well with JUnit 5 conventions.
46-76: LGTM!The test thoroughly validates endpoint-specific rate limiting:
- Correctly sends 3 requests for Alice (matching the limit in apis.yaml)
- Verifies Alice's 4th request is rate-limited (429)
- Confirms Bob can still make requests (validates per-user rate limiting via
$.userkeyExpression)- Validates appropriate log entries including the user identifier and duration
The method rename and assertion style updates align well with JUnit 5 conventions.
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.