Improvements to cache keys and time multiplier constants (#6)

* Replaced magic time multiplier numbers with constants

* Added helper methods for suffixed cache keys, changed const names to use suffix instead of prefix

Author: Feijs

src/Throttle/Throttler/ElasticWindowThrottler.php

@@ -142,6 +142,6 @@ public function getRetryTimeout()
             return 0;
         }
 
-        return 1e3 * $this->ttl;
+        return self::SECOND_TO_MILLISECOND_MULTIPLIER * $this->ttl;
     }
 }

src/Throttle/Throttler/FixedWindowThrottler.php

@@ -29,8 +29,8 @@
 
 final class FixedWindowThrottler extends AbstractWindowThrottler implements RetriableThrottlerInterface
 {
-    const TIME_CACHE_KEY = ':time';
-    const HITS_CACHE_KEY = ':hits';
+    const CACHE_KEY_TIME = ':time';
+    const CACHE_KEY_HITS = ':hits';
 
     /**
      * @var int|null
@@ -46,11 +46,11 @@ public function hit()
 
         // Update the window start time if the previous window has passed, or no cached window exists
         try {
-            if (($this->timeProvider->now() - $this->cache->get($this->key.self::TIME_CACHE_KEY)) > $this->timeLimit) {
-                $this->cache->set($this->key.self::TIME_CACHE_KEY, $this->timeProvider->now(), $this->cacheTtl);
+            if (($this->timeProvider->now() - $this->cache->get($this->getTimeCacheKey())) > $this->timeLimit) {
+                $this->cache->set($this->getTimeCacheKey(), $this->timeProvider->now(), $this->cacheTtl);
             }
         } catch (ItemNotFoundException $exception) {
-            $this->cache->set($this->key.self::TIME_CACHE_KEY, $this->timeProvider->now(), $this->cacheTtl);
+            $this->cache->set($this->getTimeCacheKey(), $this->timeProvider->now(), $this->cacheTtl);
         }
 
         return $this;
@@ -62,7 +62,7 @@ public function hit()
     public function count()
     {
         try {
-            if (($this->timeProvider->now() - $this->cache->get($this->key.self::TIME_CACHE_KEY)) > $this->timeLimit) {
+            if (($this->timeProvider->now() - $this->cache->get($this->getTimeCacheKey())) > $this->timeLimit) {
                 return 0;
             }
 
@@ -78,7 +78,7 @@ public function count()
     public function clear()
     {
         $this->setCachedHitCount(0);
-        $this->cache->set($this->key.self::TIME_CACHE_KEY, $this->timeProvider->now(), $this->cacheTtl);
+        $this->cache->set($this->getTimeCacheKey(), $this->timeProvider->now(), $this->cacheTtl);
     }
 
     /**
@@ -92,7 +92,9 @@ public function getRetryTimeout()
 
         // Return the time until the current window ends
         // Try/catch for the ItemNotFoundException is not required, in that case $this->check() will return true
-        return 1e3 * ($this->timeLimit - $this->timeProvider->now() + $this->cache->get($this->key.self::TIME_CACHE_KEY));
+        $cachedTime = $this->cache->get($this->getTimeCacheKey());
+
+        return self::SECOND_TO_MILLISECOND_MULTIPLIER * ($this->timeLimit - $this->timeProvider->now() + $cachedTime);
     }
 
     /**
@@ -106,7 +108,7 @@ private function getCachedHitCount()
             return $this->hitCount;
         }
 
-        return $this->cache->get($this->key.self::HITS_CACHE_KEY);
+        return $this->cache->get($this->getHitsCacheKey());
     }
 
     /**
@@ -115,6 +117,22 @@ private function getCachedHitCount()
     private function setCachedHitCount($hitCount)
     {
         $this->hitCount = $hitCount;
-        $this->cache->set($this->key.self::HITS_CACHE_KEY, $hitCount, $this->cacheTtl);
+        $this->cache->set($this->getHitsCacheKey(), $hitCount, $this->cacheTtl);
+    }
+
+    /**
+     * @return string
+     */
+    private function getHitsCacheKey()
+    {
+        return $this->key.self::CACHE_KEY_HITS;
+    }
+
+    /**
+     * @return string
+     */
+    private function getTimeCacheKey()
+    {
+        return $this->key.self::CACHE_KEY_TIME;
     }
 }

src/Throttle/Throttler/LeakyBucketThrottler.php

@@ -31,8 +31,8 @@
 
 final class LeakyBucketThrottler implements RetriableThrottlerInterface
 {
-    const TIME_CACHE_KEY = ':time';
-    const TOKEN_CACHE_KEY = ':tokens';
+    const CACHE_KEY_TIME = ':time';
+    const CACHE_KEY_TOKEN = ':tokens';
 
     /**
      * @var CacheAdapterInterface
@@ -114,7 +114,7 @@ public function hit()
         $this->setUsedCapacity($tokenCount + 1);
 
         if (0 < $wait = $this->getWaitTime($tokenCount)) {
-            $this->timeProvider->usleep(1e3 * $wait);
+            $this->timeProvider->usleep(self::MILLISECOND_TO_MICROSECOND_MULTIPLIER * $wait);
         }
 
         return $wait;
@@ -134,13 +134,14 @@ public function clear()
     public function count()
     {
         try {
-            $timeSinceLastRequest = 1e3 * ($this->timeProvider->now() - $this->cache->get($this->key.self::TIME_CACHE_KEY));
+            $cachedTime = $this->cache->get($this->getTimeCacheKey());
+            $timeSinceLastRequest = self::SECOND_TO_MILLISECOND_MULTIPLIER * ($this->timeProvider->now() - $cachedTime);
 
             if ($timeSinceLastRequest > $this->timeLimit) {
                 return 0;
             }
 
-            $lastTokenCount = $this->cache->get($this->key.self::TOKEN_CACHE_KEY);
+            $lastTokenCount = $this->cache->get($this->getTokenCacheKey());
         } catch (ItemNotFoundException $exception) {
             $this->clear(); //Clear the bucket
 
@@ -206,7 +207,23 @@ private function getWaitTime($tokenCount)
      */
     private function setUsedCapacity($tokens)
     {
-        $this->cache->set($this->key.self::TOKEN_CACHE_KEY, $tokens, $this->cacheTtl);
-        $this->cache->set($this->key.self::TIME_CACHE_KEY, $this->timeProvider->now(), $this->cacheTtl);
+        $this->cache->set($this->getTokenCacheKey(), $tokens, $this->cacheTtl);
+        $this->cache->set($this->getTimeCacheKey(), $this->timeProvider->now(), $this->cacheTtl);
+    }
+
+    /**
+     * @return string
+     */
+    private function getTokenCacheKey()
+    {
+        return $this->key.self::CACHE_KEY_TOKEN;
+    }
+
+    /**
+     * @return string
+     */
+    private function getTimeCacheKey()
+    {
+        return $this->key.self::CACHE_KEY_TIME;
     }
 }

src/Throttle/Throttler/MovingWindowThrottler.php

@@ -78,11 +78,14 @@ public function getRetryTimeout()
         // Then return the time remaining for that timestamp to expire
         foreach ($this->hitCountMapping as $timestamp => $hitCount) {
             if ($this->hitLimit > $totalHitCount -= $hitCount) {
-                return 1e3 * max(0, $this->timeLimit - ((int) ceil($this->timeProvider->now()) - $timestamp));
+                return self::SECOND_TO_MILLISECOND_MULTIPLIER * max(
+                    0,
+                    $this->timeLimit - ((int) ceil($this->timeProvider->now()) - $timestamp)
+                );
             }
         }
 
-        return 1e3 * $this->timeLimit;
+        return self::SECOND_TO_MILLISECOND_MULTIPLIER * $this->timeLimit;
     }
 
     /**

src/Throttle/Throttler/RetrialQueueThrottler.php

@@ -66,7 +66,7 @@ public function access()
     public function hit()
     {
         if (0 !== $waitTime = $this->internalThrottler->getRetryTimeout()) {
-            $this->timeProvider->usleep(1e3 * $waitTime);
+            $this->timeProvider->usleep(self::MILLISECOND_TO_MICROSECOND_MULTIPLIER * $waitTime);
         }
 
         return $this->internalThrottler->hit();

src/Throttle/Throttler/ThrottlerInterface.php

@@ -27,6 +27,9 @@
 
 interface ThrottlerInterface
 {
+    const SECOND_TO_MILLISECOND_MULTIPLIER = 1000;
+    const MILLISECOND_TO_MICROSECOND_MULTIPLIER = 1000;
+
     /**
      * Access the resource and return status
      *

tests/Functional/LeakyBucketTest.php

@@ -9,6 +9,7 @@
 use Sunspikes\Ratelimit\Throttle\Factory\TimeAwareThrottlerFactory;
 use Sunspikes\Ratelimit\Throttle\Hydrator\HydratorFactory;
 use Sunspikes\Ratelimit\Throttle\Settings\LeakyBucketSettings;
+use Sunspikes\Ratelimit\Throttle\Throttler\ThrottlerInterface;
 use Sunspikes\Ratelimit\Time\TimeAdapterInterface;
 
 class LeakyBucketTest extends AbstractThrottlerTestCase
@@ -35,7 +36,9 @@ protected function setUp()
     public function testThrottleAccess()
     {
         $expectedWaitTime = self::TIME_LIMIT / (self::TOKEN_LIMIT - $this->getMaxAttempts());
-        $this->timeAdapter->shouldReceive('usleep')->with(1e3 * $expectedWaitTime)->once();
+        $this->timeAdapter->shouldReceive('usleep')
+            ->with(ThrottlerInterface::SECOND_TO_MILLISECOND_MULTIPLIER * $expectedWaitTime)
+            ->once();
 
         parent::testThrottleAccess();
     }

tests/Functional/RetrialQueueTest.php

@@ -10,6 +10,7 @@
 use Sunspikes\Ratelimit\Throttle\Hydrator\HydratorFactory;
 use Sunspikes\Ratelimit\Throttle\Settings\FixedWindowSettings;
 use Sunspikes\Ratelimit\Throttle\Settings\RetrialQueueSettings;
+use Sunspikes\Ratelimit\Throttle\Throttler\ThrottlerInterface;
 use Sunspikes\Ratelimit\Time\TimeAdapterInterface;
 
 class RetrialQueueTest extends AbstractThrottlerTestCase
@@ -34,7 +35,12 @@ protected function setUp()
 
     public function testThrottleAccess()
     {
-        $this->timeAdapter->shouldReceive('usleep')->with(1e6 * self::TIME_LIMIT)->once();
+        $this->timeAdapter->shouldReceive('usleep')
+            ->with(
+                ThrottlerInterface::SECOND_TO_MILLISECOND_MULTIPLIER *
+                ThrottlerInterface::MILLISECOND_TO_MICROSECOND_MULTIPLIER *
+                self::TIME_LIMIT
+            )->once();
 
         parent::testThrottleAccess();
     }

tests/Throttle/Throttler/ElasticWindowThrottlerTest.php

@@ -5,6 +5,7 @@
 use Mockery as M;
 use Sunspikes\Ratelimit\Cache\Adapter\CacheAdapterInterface;
 use Sunspikes\Ratelimit\Throttle\Throttler\ElasticWindowThrottler;
+use Sunspikes\Ratelimit\Throttle\Throttler\ThrottlerInterface;
 
 class ElasticWindowThrottlerTest extends \PHPUnit_Framework_TestCase
 {
@@ -75,6 +76,9 @@ public function testGetRetryTimeout()
         $this->throttler->hit();
         $this->throttler->hit();
 
-        $this->assertEquals(1e3 * self::TTL, $this->throttler->getRetryTimeout());
+        $this->assertEquals(
+            ThrottlerInterface::SECOND_TO_MILLISECOND_MULTIPLIER * self::TTL,
+            $this->throttler->getRetryTimeout()
+        );
     }
 }

tests/Throttle/Throttler/FixedWindowThrottlerTest.php

@@ -4,19 +4,20 @@
 
 use Mockery as M;
 use Sunspikes\Ratelimit\Throttle\Throttler\FixedWindowThrottler;
+use Sunspikes\Ratelimit\Throttle\Throttler\ThrottlerInterface;
 
 class FixedWindowThrottlerTest extends AbstractWindowThrottlerTest
 {
     public function testAccess()
     {
         $this->cacheAdapter
             ->shouldReceive('set')
-            ->with('key'.FixedWindowThrottler::HITS_CACHE_KEY, 1, self::CACHE_TTL)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_HITS, 1, self::CACHE_TTL)
             ->once();
 
         $this->cacheAdapter
             ->shouldReceive('set')
-            ->with('key'.FixedWindowThrottler::TIME_CACHE_KEY, self::TIME_LIMIT + 2, self::CACHE_TTL)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_TIME, self::TIME_LIMIT + 2, self::CACHE_TTL)
             ->once();
 
         parent::testAccess();
@@ -28,12 +29,12 @@ public function testClear()
 
         $this->cacheAdapter
             ->shouldReceive('set')
-            ->with('key'.FixedWindowThrottler::TIME_CACHE_KEY, self::INITIAL_TIME + 3, self::CACHE_TTL)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_TIME, self::INITIAL_TIME + 3, self::CACHE_TTL)
             ->once();
 
         $this->cacheAdapter
             ->shouldReceive('set')
-            ->with('key'.FixedWindowThrottler::HITS_CACHE_KEY, 0, self::CACHE_TTL)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_HITS, 0, self::CACHE_TTL)
             ->once();
 
         $this->throttler->clear();
@@ -46,7 +47,7 @@ public function testCountWithLessTimePassedThanLimit()
 
         $this->cacheAdapter
             ->shouldReceive('get')
-            ->with('key'.FixedWindowThrottler::HITS_CACHE_KEY)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_HITS)
             ->andReturn(self::HIT_LIMIT / 3);
 
         $this->assertEquals(self::HIT_LIMIT / 3, $this->throttler->count());
@@ -66,10 +67,13 @@ public function testGetRetryTimeoutPostLimit()
         $this->mockTimePassed(self::TIME_LIMIT / 2);
         $this->cacheAdapter
             ->shouldReceive('get')
-            ->with('key'.FixedWindowThrottler::HITS_CACHE_KEY)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_HITS)
             ->andReturn(self::HIT_LIMIT + 1);
 
-        $this->assertEquals(5e2 * self::TIME_LIMIT, $this->throttler->getRetryTimeout());
+        $this->assertEquals(
+            ThrottlerInterface::SECOND_TO_MILLISECOND_MULTIPLIER / 2 * self::TIME_LIMIT,
+            $this->throttler->getRetryTimeout()
+        );
     }
 
     /**
@@ -96,7 +100,7 @@ protected function mockTimePassed($timeDiff)
 
         $this->cacheAdapter
             ->shouldReceive('get')
-            ->with('key'.FixedWindowThrottler::TIME_CACHE_KEY)
+            ->with('key'.FixedWindowThrottler::CACHE_KEY_TIME)
             ->andReturn(self::INITIAL_TIME);
     }
 }