Skip to content

refactor: extract shared rate limiter, standardize error codes, name constants#4

Merged
CybotTM merged 4 commits intomainfrom
fix/code-quality-cleanup
Feb 25, 2026
Merged

refactor: extract shared rate limiter, standardize error codes, name constants#4
CybotTM merged 4 commits intomainfrom
fix/code-quality-cleanup

Conversation

@CybotTM
Copy link
Copy Markdown
Owner

@CybotTM CybotTM commented Feb 25, 2026

Summary

Code quality cleanup addressing issues found by comprehensive codebase audit:

  • Extract SlidingWindowRateLimiter utility — consolidates 3 near-identical rate limiter implementations (ChallengeKeyRateLimiter, DeviceRegistrationRateLimiter, WebSocketConnectionRateLimiter) into a single reusable class with configurable maxRequests and windowMs. Reduces ~90 lines of duplicated synchronized sliding-window logic to a single 66-line utility.
  • Add 8 unit tests for SlidingWindowRateLimiter — boundary conditions, key independence, reset, instance isolation, cleanup safety, concurrent access.
  • Standardize ApiException error codes — replaces 10 instances of ApiException(HttpStatusCode.BadRequest.value, ...) with integer literals (400, 403, 404, 429) for consistency with 98 existing instances.
  • Name magic numbers — extracts CLEANUP_INTERVAL_MS (5 min), MAX_REQUEST_BODY_BYTES (10 MB), and HSTS_MAX_AGE_SECONDS (2 years) from inline expressions in Application.kt.
  • Update docs — test count 456→464, class count 40→41, add SlidingWindowRateLimiter to util listing and test table, fix stale comment references.
  • Remove unused importio.ktor.http.* from BucketService.kt.

Test plan

  • All 464 server tests pass (456 original + 8 new)
  • Detekt: zero violations
  • Conformance tests: pass
  • Android build: pass (881+ tests)
  • CI: all 4 checks green on every commit
  • No behavior changes — pure refactoring + docs
  • 3 review cycles completed, all findings addressed

…magic numbers

- Extract SlidingWindowRateLimiter utility from 3 duplicate implementations
  (ChallengeKeyRateLimiter, DeviceRegistrationRateLimiter, WebSocketConnectionRateLimiter)
- Standardize ApiException to use integer status codes (10 instances of
  HttpStatusCode.*.value replaced with literal ints for consistency with 98 existing)
- Extract magic numbers in Application.kt to named constants:
  CLEANUP_INTERVAL_MS, MAX_REQUEST_BODY_BYTES, HSTS_MAX_AGE_SECONDS
- Remove unused io.ktor.http.* import from BucketService.kt
8 tests covering boundary conditions, key independence, reset behavior,
instance isolation, cleanup safety, and concurrent access under contention.
…ter refactor

- Update server test count from 456 to 464 in AGENTS.md, CONTRIBUTING.md,
  and server/AGENTS.md (3 files, 5 occurrences)
- Add SlidingWindowRateLimiter to server/AGENTS.md util listing and test table
- Update stale ChallengeKeyRateLimiter reference in RateLimiterTest.kt comments
- Remove unused assertEquals import and lambda parameter in test file
- Root: thin with Commands, File Map, Golden Samples, Heuristics,
  Terminology, and working scope index links
- Server: add managed header, Setup, Code Style, Security, Checklist,
  Examples, and When Stuck sections
- Android: add managed header, Setup, Code Style, Security, Checklist,
  Examples, and When Stuck sections
- All 3 files pass validate-structure.sh (0 errors, 0 warnings)
@CybotTM CybotTM merged commit cadee5f into main Feb 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant