Merge pull request #4 from MyOnlineStore/MWW-4056

Fixes inconsistencies in (milli)seconds in LeakyBucketThrottler.
MyOnlineStore · Nov 8, 2016 · faaf14e · faaf14e
2 parents 5acc059 + 2788819
commit faaf14e
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 16 deletions.
diff --git a/src/Throttle/Settings/LeakyBucketSettings.php b/src/Throttle/Settings/LeakyBucketSettings.php
@@ -88,8 +88,7 @@ public function isValid()
         return
             null !== $this->tokenLimit &&
             null !== $this->timeLimit &&
-            0 !== $this->timeLimit &&
-            $this->tokenLimit !== $this->threshold;
+            0 !== $this->timeLimit;
     }
 
     /**

diff --git a/src/Throttle/Throttler/LeakyBucketThrottler.php b/src/Throttle/Throttler/LeakyBucketThrottler.php
@@ -111,8 +111,8 @@ public function hit()
     {
         $tokenCount = $this->count();
 
-        if (0 !== $wait = $this->getWaitTime($tokenCount)) {
-            $this->timeProvider->usleep($wait * 1e3);
+        if (0 < $wait = $this->getWaitTime($tokenCount)) {
+            $this->timeProvider->usleep($wait);
         }
 
         $this->setUsedCapacity($tokenCount + 1);
@@ -134,7 +134,7 @@ public function clear()
     public function count()
     {
         try {
-            $timeSinceLastRequest = $this->timeProvider->now() - $this->cache->get($this->key.self::TIME_CACHE_KEY);
+            $timeSinceLastRequest = 1e3 * ($this->timeProvider->now() - $this->cache->get($this->key.self::TIME_CACHE_KEY));
 
             if ($timeSinceLastRequest > $this->timeLimit) {
                 return 0;
@@ -147,7 +147,7 @@ public function count()
             return 0;
         }
 
-        return  (int) ceil($lastTokenCount - ($this->tokenlimit * $timeSinceLastRequest / ($this->timeLimit)));
+        return  (int) max(0, ceil($lastTokenCount - ($this->tokenlimit * $timeSinceLastRequest / ($this->timeLimit))));
     }
 
     /**
@@ -185,7 +185,7 @@ private function getWaitTime($tokenCount)
             return 0;
         }
 
-        return ceil($this->timeLimit / ($this->tokenlimit - $this->threshold));
+        return ceil($this->timeLimit / max(1, ($this->tokenlimit - $this->threshold)));
     }
 
     /**

diff --git a/src/Time/PhpTimeAdapter.php b/src/Time/PhpTimeAdapter.php
@@ -28,11 +28,13 @@
 final class PhpTimeAdapter implements TimeAdapterInterface
 {
     /**
-     * @return int
+     * @return float
      */
     public function now()
     {
-        return time();
+        list($usec, $sec) = explode(" ", microtime());
+
+        return ((float) $usec + (float) $sec);
     }
 
     /**

diff --git a/src/Time/TimeAdapterInterface.php b/src/Time/TimeAdapterInterface.php
@@ -28,7 +28,7 @@
 interface TimeAdapterInterface
 {
     /**
-     * @return int
+     * @return float
      */
     public function now();
 

diff --git a/tests/Functional/LeakyBucketTest.php b/tests/Functional/LeakyBucketTest.php
@@ -69,7 +69,7 @@ public function testThrottlePostLimit()
     public function testThrottleAccess()
     {
         $expectedWaitTime = self::TIME_LIMIT / (self::TOKEN_LIMIT - self::THRESHOLD);
-        $this->timeAdapter->shouldReceive('usleep')->with(1e3 * $expectedWaitTime)->once();
+        $this->timeAdapter->shouldReceive('usleep')->with($expectedWaitTime)->once();
 
         $throttle = $this->ratelimiter->get('access-test');
         $throttle->access();

diff --git a/tests/Throttle/Settings/LeakyBucketSettingsTest.php b/tests/Throttle/Settings/LeakyBucketSettingsTest.php
@@ -54,7 +54,7 @@ public function inputProvider()
             [null, 600, null, false],
             [3, null, null, false],
             [3, 0, null, false],
-            [3, 600, 3, false],
+            [3, 600, 3, true],
             [30, 600, 15, true],
         ];
     }

diff --git a/tests/Throttle/Throttler/LeakyBucketThrottlerTest.php b/tests/Throttle/Throttler/LeakyBucketThrottlerTest.php
@@ -55,7 +55,7 @@ public function testAccess()
     {
         //More time has passed than the given window
         $this->mockTimePassed(self::TIME_LIMIT + 1, 2);
-        $this->mockSetUsedCapacity(1, self::INITIAL_TIME + self::TIME_LIMIT + 1);
+        $this->mockSetUsedCapacity(1, (self::INITIAL_TIME + self::TIME_LIMIT + 1) / 1e3);
 
         $this->assertEquals(true, $this->throttler->access());
     }
@@ -90,7 +90,7 @@ public function testHitOnThreshold()
         $this->mockSetUsedCapacity(self::THRESHOLD + 1, self::INITIAL_TIME);
 
         $expectedWaitTime = self::TIME_LIMIT / (self::TOKEN_LIMIT - self::THRESHOLD);
-        $this->timeAdapter->shouldReceive('usleep')->with(1e3 * $expectedWaitTime)->once();
+        $this->timeAdapter->shouldReceive('usleep')->with($expectedWaitTime)->once();
 
         $this->assertEquals($expectedWaitTime, $this->throttler->hit());
     }
@@ -167,11 +167,11 @@ private function mockSetUsedCapacity($tokens, $time)
      */
     private function mockTimePassed($timeDiff, $numCalls)
     {
-        $this->timeAdapter->shouldReceive('now')->times($numCalls)->andReturn(self::INITIAL_TIME + $timeDiff);
+        $this->timeAdapter->shouldReceive('now')->times($numCalls)->andReturn((self::INITIAL_TIME + $timeDiff) / 1e3);
 
         $this->cacheAdapter
             ->shouldReceive('get')
             ->with('key'.LeakyBucketThrottler::TIME_CACHE_KEY)
-            ->andReturn(self::INITIAL_TIME);
+            ->andReturn(self::INITIAL_TIME / 1e3);
     }
 }