Multiple functions in RateLimitingUtil.scala read counter data from Redis independently. This creates potential inconsistency and code duplication.
- Uses:
EXISTS+GET - Returns: Boolean (are we under limit?)
- Handles missing key: Returns
true(under limit) - Purpose: Enforcement - check if request should be allowed
- Uses:
TTL+ (SETorINCR) - Returns: (ttl, count) as tuple
- Handles missing key (TTL=-2): Creates new key with value 1
- Purpose: Tracking - increment counter after allowed request
- Uses:
TTLonly - Returns: Long (normalized TTL)
- Handles missing key (TTL=-2): Returns 0
- Purpose: Helper - get remaining time for a period
- Uses:
TTL+GET - Returns: ((Option[Long], Option[Long]), period)
- Handles missing key (TTL=-2): Returns (Some(0), Some(0))
- Purpose: Reporting - display current usage to API consumers
-2: Key does not exist-1: Key exists with no expiry (shouldn't happen in our rate limiting)>0: Seconds until key expires
- Code duplication: Redis interaction logic repeated across functions
- Inconsistency risk: Each function interprets Redis state independently
- Multiple sources of truth: No single canonical way to read counter state
Refactor to have ONE canonical function that reads and normalizes counter state from Redis:
private def getCounterState(consumerKey: String, period: LimitCallPeriod): (Long, Long) = {
// Single place to read and normalize Redis counter data
// Returns (calls, ttl) with -2 handled as 0
}All other functions should use this single source of truth.
- Enforcement functions work correctly
- Reporting improved (returns 0 instead of None for missing keys)
- Refactoring to single read function: Not yet implemented