-
-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #44: Use PSR-20 ClockInterface instead of TimerInterface #47
Changes from all commits
f7cef39
34d4814
841734b
9aaddfd
d2de4b2
01a643c
3d9c254
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,9 @@ | |
namespace Yiisoft\Yii\RateLimiter; | ||
|
||
use InvalidArgumentException; | ||
use Psr\Clock\ClockInterface; | ||
use Yiisoft\Yii\RateLimiter\Storage\StorageInterface; | ||
use Yiisoft\Yii\RateLimiter\Time\MicrotimeTimer; | ||
use Yiisoft\Yii\RateLimiter\Time\TimerInterface; | ||
use Yiisoft\Yii\RateLimiter\Time\SystemClock; | ||
|
||
/** | ||
* Counter implements generic cell rate limit algorithm (GCRA) that ensures that after reaching the limit further | ||
|
@@ -28,19 +28,19 @@ | |
private int $periodInMilliseconds; | ||
|
||
/** | ||
* @var float Maximum interval before next increment. | ||
* In GCRA it is known as emission interval. | ||
* @var float Maximum interval before the next increment. | ||
* In GCRA it's known as an emission interval. | ||
*/ | ||
private float $incrementIntervalInMilliseconds; | ||
private TimerInterface $timer; | ||
private ClockInterface $timer; | ||
|
||
/** | ||
* @param StorageInterface $storage Storage to use for counter values. | ||
* @param int $limit Maximum number of increments that could be performed before increments are limited. | ||
* @param int $limit A maximum number of increments that could be performed before increments are limited. | ||
* @param int $periodInSeconds Period to apply limit to. | ||
* @param int $storageTtlInSeconds Storage TTL. Should be higher than `$periodInSeconds`. | ||
* @param string $storagePrefix Storage prefix. | ||
* @param TimerInterface|null $timer Timer instance to get current time from. | ||
* @param ClockInterface|null $timer Timer instance to get current time from. | ||
* @param int $maxCasAttempts Maximum number of times to retry saveIfNotExists/saveCompareAndSwap operations before returning an error. | ||
*/ | ||
public function __construct( | ||
|
@@ -49,7 +49,7 @@ | |
int $periodInSeconds, | ||
private int $storageTtlInSeconds = self::DEFAULT_TTL, | ||
private string $storagePrefix = self::ID_PREFIX, | ||
TimerInterface|null $timer = null, | ||
ClockInterface|null $timer = null, | ||
private int $maxCasAttempts = self::DEFAULT_MAX_CAS_ATTEMPTS, | ||
) { | ||
if ($limit < 1) { | ||
|
@@ -61,7 +61,7 @@ | |
} | ||
|
||
$this->periodInMilliseconds = $periodInSeconds * self::MILLISECONDS_PER_SECOND; | ||
$this->timer = $timer ?: new MicrotimeTimer(); | ||
$this->timer = $timer ?: new SystemClock(); | ||
$this->incrementIntervalInMilliseconds = $this->periodInMilliseconds / $this->limit; | ||
} | ||
|
||
|
@@ -71,11 +71,11 @@ | |
public function hit(string $id): CounterState | ||
{ | ||
$attempts = 0; | ||
$isFailStoreUpdatedData = false; | ||
Check warning on line 74 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 74 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
|
||
do { | ||
// Last increment time. | ||
// In GCRA it's known as arrival time. | ||
$lastIncrementTimeInMilliseconds = $this->timer->nowInMilliseconds(); | ||
$lastIncrementTimeInMilliseconds = round((float)$this->timer->now()->format('U.u') * 1000); | ||
Check warning on line 78 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 78 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 78 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 78 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 78 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 78 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
|
||
|
||
$lastStoredTheoreticalNextIncrementTime = $this->getLastStoredTheoreticalNextIncrementTime($id); | ||
|
||
|
@@ -103,7 +103,7 @@ | |
$attempts++; | ||
if ($attempts >= $this->maxCasAttempts) { | ||
$isFailStoreUpdatedData = true; | ||
break; | ||
Check warning on line 106 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 106 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
|
||
} | ||
} while (true); | ||
|
||
|
@@ -112,7 +112,7 @@ | |
|
||
/** | ||
* @return float Theoretical increment time that would be expected from equally spaced increments at exactly rate | ||
* limit. In GCRA it is known as TAT, theoretical arrival time. | ||
* limit. In GCRA it's known as TAT, theoretical arrival time. | ||
*/ | ||
private function calculateTheoreticalNextIncrementTime( | ||
float $lastIncrementTimeInMilliseconds, | ||
|
@@ -134,8 +134,8 @@ | |
): int { | ||
$incrementAllowedAt = $theoreticalNextIncrementTime - $this->periodInMilliseconds; | ||
|
||
$remainingTimeInMilliseconds = round($lastIncrementTimeInMilliseconds - $incrementAllowedAt); | ||
Check warning on line 137 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 137 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 137 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 137 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
|
||
if ($remainingTimeInMilliseconds > 0) { | ||
Check warning on line 138 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 138 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
|
||
return (int) ($remainingTimeInMilliseconds / $this->incrementIntervalInMilliseconds); | ||
} | ||
|
||
|
@@ -173,7 +173,7 @@ | |
*/ | ||
private function calculateResetAfter(float $theoreticalNextIncrementTime): int | ||
{ | ||
return (int) ($theoreticalNextIncrementTime / self::MILLISECONDS_PER_SECOND); | ||
Check warning on line 176 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
Check warning on line 176 in src/Counter.php GitHub Actions / mutation / PHP 8.1-ubuntu-latest
|
||
} | ||
|
||
/** | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Yiisoft\Yii\RateLimiter\Time; | ||
|
||
use DateTimeImmutable; | ||
use Psr\Clock\ClockInterface; | ||
|
||
final class SystemClock implements ClockInterface | ||
{ | ||
public function now(): DateTimeImmutable | ||
{ | ||
return new DateTimeImmutable(); | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
use Yiisoft\Yii\RateLimiter\Storage\SimpleCacheStorage; | ||
use Yiisoft\Yii\RateLimiter\Storage\StorageInterface; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FakeSimpleCacheStorage; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FrozenTimeTimer; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FrozenClock; | ||
|
||
final class CounterTest extends BaseCounterTest | ||
{ | ||
|
@@ -20,11 +20,11 @@ protected function getStorage(): StorageInterface | |
|
||
/** | ||
* Testing that in concurrent scenarios, when dirty reads occur, | ||
* the current limiter cannot be as expected By 'SimpleCacheStorage'. | ||
* the current limiter can't be as expected By 'SimpleCacheStorage'. | ||
*/ | ||
public function testConcurrentHitsWithDirtyReading(): void | ||
{ | ||
$timer = new FrozenTimeTimer(); | ||
$timer = new FrozenClock(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we can don't use specific timer and replace it to |
||
$storage = new FakeSimpleCacheStorage(new ArrayCache(), 5); | ||
$limitHits = 10; | ||
$counter = new Counter( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Yiisoft\Yii\RateLimiter\Tests\Fixtures; | ||
|
||
use DateTimeImmutable; | ||
use Psr\Clock\ClockInterface; | ||
|
||
/** | ||
* Frozen timer returns the same value for all calls. | ||
*/ | ||
final class FrozenClock implements ClockInterface | ||
{ | ||
private DateTimeImmutable $now; | ||
|
||
public function __construct() | ||
{ | ||
$this->now = new DateTimeImmutable(); | ||
} | ||
|
||
public function now(): DateTimeImmutable | ||
{ | ||
return $this->now; | ||
} | ||
|
||
public function modify(string $modifier): void | ||
{ | ||
$this->now = $this->now->modify($modifier); | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ | |
use Yiisoft\Yii\RateLimiter\Storage\SimpleCacheStorage; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FakeApcuStorage; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FakeCounter; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FrozenTimeTimer; | ||
use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FrozenClock; | ||
|
||
final class MiddlewareTest extends TestCase | ||
{ | ||
|
@@ -227,7 +227,7 @@ public function testWithLimitingFunction(): void | |
*/ | ||
public function testWithExceedingMaxAttempts(): void | ||
{ | ||
$timer = new FrozenTimeTimer(); | ||
$timer = new FrozenClock(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we can don't use specific timer and replace it to |
||
$dirtyReadCount = 2; | ||
$storage = new FakeApcuStorage($dirtyReadCount); | ||
$counter = new Counter( | ||
|
@@ -259,7 +259,7 @@ public function testWithExceedingMaxAttempts(): void | |
|
||
public function testFailStoreUpdatedDataMiddleware(): void | ||
{ | ||
$timer = new FrozenTimeTimer(); | ||
$timer = new FrozenClock(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we can don't use specific timer and replace it to |
||
$dirtyReadCount = 2; | ||
$storage = new FakeApcuStorage($dirtyReadCount); | ||
$counter = new Counter( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would make static analysis complain.