refactor: unify middleware application for API routes and tests#1074
Conversation
|
Warning Review limit reached
More reviews will be available in 58 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesMiddleware Chain Reconciliation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Performance Smoke Test ResultsStatus: PASSED
Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%. Full results uploaded as workflow artifact: k6-smoke-summary. |
![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 (1)
cmd/api/app.go (1)
178-178: ⚡ Quick winUpdate comment to reflect actual wrapping target.
The comment says "Apply global compression around the entire mux" but the code wraps
apiHandler(which is the mux with API-specific middleware already applied), not the baremux.📝 Suggested comment update
- // Apply global compression around the entire mux + // Apply compression to the API handler (mux with API-specific middleware) compressedMux := restapi.CompressionMiddleware(apiHandler)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/api/app.go` at line 178, The comment for the CompressionMiddleware call at the assignment to compressedMux incorrectly states that compression is being applied around "the entire mux", but the actual code wraps apiHandler (which is already the mux with API-specific middleware applied). Update the comment to accurately describe what is being wrapped, specifying that the compression middleware is being applied around apiHandler rather than the bare mux.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/restapi/routes_test.go`:
- Around line 15-20: The middleware chain in the test setup has
CompressionMiddleware and api.FreshnessMiddleware in the wrong order compared to
production. Swap the positions of these two middleware assignments so that
api.FreshnessMiddleware wraps CompressionMiddleware (making it the outer layer),
matching the production order documented in cmd/api/app.go. Additionally, update
the comment on line 6-7 to correctly reflect the actual production middleware
order with Freshness as the outermost layer wrapping Compression.
---
Nitpick comments:
In `@cmd/api/app.go`:
- Line 178: The comment for the CompressionMiddleware call at the assignment to
compressedMux incorrectly states that compression is being applied around "the
entire mux", but the actual code wraps apiHandler (which is already the mux with
API-specific middleware applied). Update the comment to accurately describe what
is being wrapped, specifying that the compression middleware is being applied
around apiHandler rather than the bare mux.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 311fbbbd-9568-4c02-ba43-ab02662a6ce5
📒 Files selected for processing (3)
cmd/api/app.gointernal/restapi/routes.gointernal/restapi/routes_test.go
💤 Files with no reviewable changes (1)
- internal/restapi/routes.go
|
Performance Smoke Test ResultsStatus: PASSED
Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%. Full results uploaded as workflow artifact: k6-smoke-summary. |
2 participants
Fixes: #1045
Summary
Production and test middleware stacks had drifted apart.
SetupAPIRouteswas only used in tests, which led toVersionValidationMiddlewarebeing tested but never running in production (#1004).Changes
GtfsExpiryMiddlewareandVersionValidationMiddlewaretocmd/api.CreateServer.SetupAPIRoutestoroutes_test.gosince it's only used by tests.Summary by CodeRabbit