diff --git a/.editorconfig b/.editorconfig index 5e9a93e..8802775 100644 --- a/.editorconfig +++ b/.editorconfig @@ -10,6 +10,10 @@ indent_style = space indent_size = 4 trim_trailing_whitespace = true +[*.php] +ij_php_space_before_short_closure_left_parenthesis = false +ij_php_space_after_type_cast = true + [*.md] trim_trailing_whitespace = false diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8506ea1..6890736 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -25,6 +25,8 @@ jobs: phpunit: uses: yiisoft/actions/.github/workflows/phpunit.yml@master with: + extensions: apcu + ini-values: apc.enabled=1,apc.shm_size=32M,apc.enable_cli=1 os: >- ['ubuntu-latest', 'windows-latest'] php: >- diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index c1aca98..aa1b231 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -23,6 +23,8 @@ jobs: mutation: uses: yiisoft/actions/.github/workflows/roave-infection.yml@master with: + extensions: apcu + ini-values: apc.enabled=1,apc.shm_size=32M,apc.enable_cli=1 os: >- ['ubuntu-latest'] php: >- diff --git a/.github/workflows/rector.yml b/.github/workflows/rector.yml index adacd73..b5a5649 100644 --- a/.github/workflows/rector.yml +++ b/.github/workflows/rector.yml @@ -14,6 +14,8 @@ name: rector jobs: rector: uses: yiisoft/actions/.github/workflows/rector.yml@master + secrets: + token: ${{ secrets.YIISOFT_GITHUB_TOKEN }} with: os: >- ['ubuntu-latest'] diff --git a/CHANGELOG.md b/CHANGELOG.md index 5213532..15981ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,17 +1,21 @@ # Yii Rate Limiter Change Log -## 2.0.1 under development +## 3.0.0 under development +- New #43: Add APCu counters storage (@jiaweipan) +- Enh #41: Adapt package to concurrent requests, for this `StorageInterface` method `save()` split to + `saveIfNotExists()` and `saveCompareAndSwap()` (@jiaweipan) - Enh #25: Raise minimum `PHP` version to `8.0` (@terabytesoftw) -- Chg #21: Update `yiisoft/http` dependency (devanych) +- Chg #21: Update `yiisoft/http` dependency (@devanych) ## 2.0.0 August 15, 2021 -- Enh #19: Introduce `LimitPolicyInterface`, `StorageInterface`, `TimerInterface`. Rename `Middleware` to `LimitRequestsMiddleware` (kafkiansky, samdark) +- Enh #19: Introduce `LimitPolicyInterface`, `StorageInterface`, `TimerInterface`. Rename `Middleware` to + `LimitRequestsMiddleware` (@kafkiansky, @samdark) ## 1.0.1 June 08, 2021 -- Bug #14: Throw exception on call `getCacheKey()` in counter without the specified ID (vjik) +- Bug #14: Throw exception on call `getCacheKey()` in counter without the specified ID (@vjik) ## 1.0.0 June 07, 2021 diff --git a/README.md b/README.md index feda99c..8230963 100644 --- a/README.md +++ b/README.md @@ -92,9 +92,11 @@ Another way it to implement `Yiisoft\Yii\RateLimiter\Policy\LimitPolicyInterface ### Implementing your own counter storage -By default, the package provides `\Yiisoft\Yii\RateLimiter\Storage\SimpleCacheStorage` that stores counters -in any [PSR-16](https://www.php-fig.org/psr/psr-16/) cache. To have your own storage -implement `Yiisoft\Yii\RateLimiter\Storage\StorageInterface`. +There are two ready to use counter storages available in the package: +- `\Yiisoft\Yii\RateLimiter\Storage\SimpleCacheStorage` - stores counters in any [PSR-16](https://www.php-fig.org/psr/psr-16/) cache. +- `\Yiisoft\Yii\RateLimiter\Storage\ApcuStorage` - stores counters by using the [APCu PHP extension](http://www.php.net/apcu) while taking concurrency into account. + +To use your own storage implement `Yiisoft\Yii\RateLimiter\Storage\StorageInterface`. ## Testing diff --git a/composer.json b/composer.json index a8d4d3c..4fa1b4f 100644 --- a/composer.json +++ b/composer.json @@ -31,6 +31,7 @@ "yiisoft/http": "^1.2" }, "require-dev": { + "ext-apcu": "*", "nyholm/psr7": "^1.0", "phpunit/phpunit": "^9.5", "rector/rector": "^0.15.2", @@ -39,6 +40,9 @@ "vimeo/psalm": "^4.18", "yiisoft/cache": "^2.0" }, + "suggest": { + "ext-apcu": "To use APCu storage" + }, "autoload": { "psr-4": { "Yiisoft\\Yii\\RateLimiter\\": "src" diff --git a/rector.php b/rector.php index 63713ce..20acccd 100644 --- a/rector.php +++ b/rector.php @@ -4,6 +4,8 @@ use Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector; use Rector\Config\RectorConfig; +use Rector\Php56\Rector\FunctionLike\AddDefaultValueForUndefinedVariableRector; +use Rector\Php74\Rector\Closure\ClosureToArrowFunctionRector; use Rector\Set\ValueObject\LevelSetList; return static function (RectorConfig $rectorConfig): void { @@ -19,4 +21,9 @@ $rectorConfig->sets([ LevelSetList::UP_TO_PHP_80, ]); + + $rectorConfig->skip([ + ClosureToArrowFunctionRector::class, + AddDefaultValueForUndefinedVariableRector::class, + ]); }; diff --git a/src/Counter.php b/src/Counter.php index 5cfeb02..7658010 100644 --- a/src/Counter.php +++ b/src/Counter.php @@ -20,6 +20,7 @@ final class Counter implements CounterInterface private const DEFAULT_TTL = 86400; private const ID_PREFIX = 'rate-limiter-'; private const MILLISECONDS_PER_SECOND = 1000; + private const DEFAULT_MAX_CAS_ATTEMPTS = 10; /** * @var int Period to apply limit to. @@ -40,14 +41,16 @@ final class Counter implements CounterInterface * @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 int $maxCasAttempts Maximum number of times to retry saveIfNotExists/saveCompareAndSwap operations before returning an error. */ public function __construct( private StorageInterface $storage, private int $limit, - private int $periodInSeconds, + int $periodInSeconds, private int $storageTtlInSeconds = self::DEFAULT_TTL, private string $storagePrefix = self::ID_PREFIX, - TimerInterface|null $timer = null + TimerInterface|null $timer = null, + private int $maxCasAttempts = self::DEFAULT_MAX_CAS_ATTEMPTS, ) { if ($limit < 1) { throw new InvalidArgumentException('The limit must be a positive value.'); @@ -57,7 +60,6 @@ public function __construct( throw new InvalidArgumentException('The period must be a positive value.'); } - $this->limit = $limit; $this->periodInMilliseconds = $periodInSeconds * self::MILLISECONDS_PER_SECOND; $this->timer = $timer ?: new MicrotimeTimer(); $this->incrementIntervalInMilliseconds = $this->periodInMilliseconds / $this->limit; @@ -68,23 +70,44 @@ public function __construct( */ public function hit(string $id): CounterState { - // Last increment time. - // In GCRA it's known as arrival time. - $lastIncrementTimeInMilliseconds = $this->timer->nowInMilliseconds(); - - $theoreticalNextIncrementTime = $this->calculateTheoreticalNextIncrementTime( - $lastIncrementTimeInMilliseconds, - $this->getLastStoredTheoreticalNextIncrementTime($id, $lastIncrementTimeInMilliseconds) - ); - - $remaining = $this->calculateRemaining($lastIncrementTimeInMilliseconds, $theoreticalNextIncrementTime); - $resetAfter = $this->calculateResetAfter($theoreticalNextIncrementTime); - - if ($remaining >= 1) { - $this->storeTheoreticalNextIncrementTime($id, $theoreticalNextIncrementTime); - } - - return new CounterState($this->limit, $remaining, $resetAfter); + $attempts = 0; + $isFailStoreUpdatedData = false; + do { + // Last increment time. + // In GCRA it's known as arrival time. + $lastIncrementTimeInMilliseconds = $this->timer->nowInMilliseconds(); + + $lastStoredTheoreticalNextIncrementTime = $this->getLastStoredTheoreticalNextIncrementTime($id); + + $theoreticalNextIncrementTime = $this->calculateTheoreticalNextIncrementTime( + $lastIncrementTimeInMilliseconds, + $lastStoredTheoreticalNextIncrementTime + ); + + $remaining = $this->calculateRemaining($lastIncrementTimeInMilliseconds, $theoreticalNextIncrementTime); + $resetAfter = $this->calculateResetAfter($theoreticalNextIncrementTime); + + if ($remaining === 0) { + break; + } + + $isStored = $this->storeTheoreticalNextIncrementTime( + $id, + $theoreticalNextIncrementTime, + $lastStoredTheoreticalNextIncrementTime + ); + if ($isStored) { + break; + } + + $attempts++; + if ($attempts >= $this->maxCasAttempts) { + $isFailStoreUpdatedData = true; + break; + } + } while (true); + + return new CounterState($this->limit, $remaining, $resetAfter, $isFailStoreUpdatedData); } /** @@ -92,34 +115,57 @@ public function hit(string $id): CounterState * limit. In GCRA it is known as TAT, theoretical arrival time. */ private function calculateTheoreticalNextIncrementTime( - int $lastIncrementTimeInMilliseconds, - float $storedTheoreticalNextIncrementTime + float $lastIncrementTimeInMilliseconds, + ?float $storedTheoreticalNextIncrementTime ): float { - return max($lastIncrementTimeInMilliseconds, $storedTheoreticalNextIncrementTime) + - $this->incrementIntervalInMilliseconds; + return ( + $storedTheoreticalNextIncrementTime === null + ? $lastIncrementTimeInMilliseconds + : max($lastIncrementTimeInMilliseconds, $storedTheoreticalNextIncrementTime) + ) + $this->incrementIntervalInMilliseconds; } /** * @return int The number of remaining requests in the current time period. */ - private function calculateRemaining(int $lastIncrementTimeInMilliseconds, float $theoreticalNextIncrementTime): int - { + private function calculateRemaining( + float $lastIncrementTimeInMilliseconds, + float $theoreticalNextIncrementTime + ): int { $incrementAllowedAt = $theoreticalNextIncrementTime - $this->periodInMilliseconds; - return (int) ( - round($lastIncrementTimeInMilliseconds - $incrementAllowedAt) / - $this->incrementIntervalInMilliseconds - ); + $remainingTimeInMilliseconds = round($lastIncrementTimeInMilliseconds - $incrementAllowedAt); + if ($remainingTimeInMilliseconds > 0) { + return (int) ($remainingTimeInMilliseconds / $this->incrementIntervalInMilliseconds); + } + + return 0; } - private function getLastStoredTheoreticalNextIncrementTime(string $id, int $lastIncrementTimeInMilliseconds): float + private function getLastStoredTheoreticalNextIncrementTime(string $id): ?float { - return (float) $this->storage->get($this->getStorageKey($id), $lastIncrementTimeInMilliseconds); + return $this->storage->get($this->getStorageKey($id)); } - private function storeTheoreticalNextIncrementTime(string $id, float $theoreticalNextIncrementTime): void - { - $this->storage->save($this->getStorageKey($id), $theoreticalNextIncrementTime, $this->storageTtlInSeconds); + private function storeTheoreticalNextIncrementTime( + string $id, + float $theoreticalNextIncrementTime, + ?float $lastStoredTheoreticalNextIncrementTime + ): bool { + if ($lastStoredTheoreticalNextIncrementTime !== null) { + return $this->storage->saveCompareAndSwap( + $this->getStorageKey($id), + $lastStoredTheoreticalNextIncrementTime, + $theoreticalNextIncrementTime, + $this->storageTtlInSeconds + ); + } + + return $this->storage->saveIfNotExists( + $this->getStorageKey($id), + $theoreticalNextIncrementTime, + $this->storageTtlInSeconds + ); } /** diff --git a/src/CounterState.php b/src/CounterState.php index 0da036c..1e2a973 100644 --- a/src/CounterState.php +++ b/src/CounterState.php @@ -13,9 +13,14 @@ final class CounterState * @param int $limit The maximum number of requests allowed with a time period. * @param int $remaining The number of remaining requests in the current time period. * @param int $resetTime Timestamp to wait until the rate limit resets. + * @param bool $isFailStoreUpdatedData If fail to store updated the rate limit data. */ - public function __construct(private int $limit, private int $remaining, private int $resetTime) - { + public function __construct( + private int $limit, + private int $remaining, + private int $resetTime, + private bool $isFailStoreUpdatedData = false + ) { } /** @@ -49,4 +54,12 @@ public function isLimitReached(): bool { return $this->remaining === 0; } + + /** + * @return bool If fail to store updated the rate limit data. + */ + public function isFailStoreUpdatedData(): bool + { + return $this->isFailStoreUpdatedData; + } } diff --git a/src/LimitRequestsMiddleware.php b/src/LimitRequestsMiddleware.php index 76d3792..200f9c9 100644 --- a/src/LimitRequestsMiddleware.php +++ b/src/LimitRequestsMiddleware.php @@ -29,7 +29,8 @@ final class LimitRequestsMiddleware implements MiddlewareInterface public function __construct( private CounterInterface $counter, private ResponseFactoryInterface $responseFactory, - LimitPolicyInterface|null $limitingPolicy = null + LimitPolicyInterface|null $limitingPolicy = null, + private ?MiddlewareInterface $failStoreUpdatedDataMiddleware = null, ) { $this->limitingPolicy = $limitingPolicy ?: new LimitPerIp(); } @@ -40,6 +41,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface if ($state->isLimitReached()) { $response = $this->createErrorResponse(); + } elseif ($state->isFailStoreUpdatedData() && $this->failStoreUpdatedDataMiddleware !== null) { + $response = $this->failStoreUpdatedDataMiddleware->process($request, $handler); } else { $response = $handler->handle($request); } @@ -58,8 +61,8 @@ private function createErrorResponse(): ResponseInterface private function addHeaders(ResponseInterface $response, CounterState $result): ResponseInterface { return $response - ->withHeader('X-Rate-Limit-Limit', (string)$result->getLimit()) - ->withHeader('X-Rate-Limit-Remaining', (string)$result->getRemaining()) - ->withHeader('X-Rate-Limit-Reset', (string)$result->getResetTime()); + ->withHeader('X-Rate-Limit-Limit', (string) $result->getLimit()) + ->withHeader('X-Rate-Limit-Remaining', (string) $result->getRemaining()) + ->withHeader('X-Rate-Limit-Reset', (string) $result->getResetTime()); } } diff --git a/src/Storage/ApcuStorage.php b/src/Storage/ApcuStorage.php new file mode 100644 index 0000000..e769a4d --- /dev/null +++ b/src/Storage/ApcuStorage.php @@ -0,0 +1,55 @@ +fixPrecisionRate; + + return apcu_add($key, (int) $value, $ttl); + } + + public function saveCompareAndSwap(string $key, float $oldValue, float $newValue, int $ttl): bool + { + $oldValue *= $this->fixPrecisionRate; + $newValue *= $this->fixPrecisionRate; + + return apcu_cas($key, (int) $oldValue, (int) $newValue); + } + + public function get(string $key): ?float + { + /** @psalm-suppress MixedAssignment */ + $value = apcu_fetch($key); + + return (is_int($value) || is_float($value)) + ? (float) $value / $this->fixPrecisionRate + : null; + } +} diff --git a/src/Storage/SimpleCacheStorage.php b/src/Storage/SimpleCacheStorage.php index 7261059..2e9f1d2 100644 --- a/src/Storage/SimpleCacheStorage.php +++ b/src/Storage/SimpleCacheStorage.php @@ -6,19 +6,30 @@ use Psr\SimpleCache\CacheInterface; +use function is_float; +use function is_int; + final class SimpleCacheStorage implements StorageInterface { public function __construct(private CacheInterface $cache) { } - public function save(string $key, mixed $value, int $ttl): void + public function saveIfNotExists(string $key, float $value, int $ttl): bool { - $this->cache->set($key, $value, $ttl); + return $this->cache->set($key, $value, $ttl); } - public function get(string $key, mixed $default = null): mixed + public function saveCompareAndSwap(string $key, float $oldValue, float $newValue, int $ttl): bool { - return $this->cache->get($key, $default); + return $this->cache->set($key, $newValue, $ttl); + } + + public function get(string $key): ?float + { + /** @psalm-suppress MixedAssignment */ + $value = $this->cache->get($key); + + return (is_int($value) || is_float($value)) ? (float) $value : null; } } diff --git a/src/Storage/StorageInterface.php b/src/Storage/StorageInterface.php index 73ede05..32d4bed 100644 --- a/src/Storage/StorageInterface.php +++ b/src/Storage/StorageInterface.php @@ -10,21 +10,39 @@ interface StorageInterface { /** - * Persists counter value in the storage, uniquely referenced by a key with an optional expiration TTL time. + * Saves the value of key only if it was not already saved in the store. + * It returns `false` if a value was saved. Otherwise, it returns `true` + * and saves the value. + * If the storage supports expiring keys, the key will expire after the provided TTL. * * @param string $key The ID of the counter to store. - * @param mixed $value The value of the counter. + * @param float $value The value of the counter. * @param int $ttl The TTL value of this counter. + * @return bool */ - public function save(string $key, mixed $value, int $ttl): void; + public function saveIfNotExists(string $key, float $value, int $ttl): bool; + + /** + * Compares the old value of key to the value which was saved in the store. + * If it matches, method saves it to the new value and returns `true`. + * Otherwise, it returns `false`. + * If the key does not exist in the storage, it returns `false`. + * If the storage supports expiring keys, the key will expire after the provided TTL. + * + * @param string $key The ID of the counter to store. + * @param float $oldValue The old value of the counter. + * @param float $newValue The new value of the counter. + * @param int $ttl The TTL value of this counter. + * @return bool + */ + public function saveCompareAndSwap(string $key, float $oldValue, float $newValue, int $ttl): bool; /** * Fetches a counter value from the storage. * * @param string $key The unique key of this counter in the storage. - * @param mixed $default Default value to return if the counter does not exist. * - * @return mixed The value of the counter from the storage, or $default in case of no counter with such key present. + * @return ?float It returns `float` if a value was saved. Otherwise, it returns `null` */ - public function get(string $key, mixed $default = null): mixed; + public function get(string $key): ?float; } diff --git a/src/Time/MicrotimeTimer.php b/src/Time/MicrotimeTimer.php index 6115a1c..cbae9af 100644 --- a/src/Time/MicrotimeTimer.php +++ b/src/Time/MicrotimeTimer.php @@ -8,8 +8,8 @@ final class MicrotimeTimer implements TimerInterface { private const MILLISECONDS_PER_SECOND = 1000; - public function nowInMilliseconds(): int + public function nowInMilliseconds(): float { - return (int) round(microtime(true) * self::MILLISECONDS_PER_SECOND); + return round(microtime(true) * self::MILLISECONDS_PER_SECOND); } } diff --git a/src/Time/TimerInterface.php b/src/Time/TimerInterface.php index 2d6005b..6aa38b5 100644 --- a/src/Time/TimerInterface.php +++ b/src/Time/TimerInterface.php @@ -6,5 +6,5 @@ interface TimerInterface { - public function nowInMilliseconds(): int; + public function nowInMilliseconds(): float; } diff --git a/tests/ApcuCounterTest.php b/tests/ApcuCounterTest.php new file mode 100644 index 0000000..b2b2c6c --- /dev/null +++ b/tests/ApcuCounterTest.php @@ -0,0 +1,86 @@ +clearStorage(); + } + + /** + * Testing that in concurrent scenarios, when dirty reads occur, + * the current limiter still performs as expected By 'ApcuStorage'. + */ + public function testConcurrentHitsWithDirtyReading(): void + { + $timer = new FrozenTimeTimer(); + $storage = new FakeApcuStorage(5); + $limitHits = 10; + $counter = new Counter( + $storage, + $limitHits, + 1, + 86400, + 'rate-limiter-', + $timer + ); + + $totalHits = 0; + do { + ++$totalHits; + + $statistics = $counter->hit('key'); + + $remaining = $statistics->getRemaining(); + if ($remaining === 0) { + break; + } + } while (true); + + $this->assertEquals($limitHits, $totalHits); + } + + public function testIsExceedingMaxAttempts(): void + { + $timer = new FrozenTimeTimer(); + $dirtyReadCount = 2; + $storage = new FakeApcuStorage($dirtyReadCount); + $counter = new Counter( + $storage, + 10, + 1, + 86400, + 'rate-limiter-', + $timer, + 1 + ); + + $counter->hit('key'); + $counter->hit('key'); + + $counterState = $counter->hit('key'); + $this->assertTrue($counterState->isFailStoreUpdatedData()); + } +} diff --git a/tests/BaseCounterTest.php b/tests/BaseCounterTest.php new file mode 100644 index 0000000..52360f3 --- /dev/null +++ b/tests/BaseCounterTest.php @@ -0,0 +1,107 @@ +getStorage(), 2, 5); + + $statistics = $counter->hit('key'); + $this->assertEquals(2, $statistics->getLimit()); + $this->assertEquals(1, $statistics->getRemaining()); + $this->assertGreaterThanOrEqual(time(), $statistics->getResetTime()); + $this->assertFalse($statistics->isLimitReached()); + } + + public function testStatisticsShouldBeCorrectWhenLimitIsReached(): void + { + $counter = new Counter($this->getStorage(), 2, 4); + + $counter->hit('key'); + $statistics = $counter->hit('key'); + + $this->assertEquals(2, $statistics->getLimit()); + $this->assertEquals(0, $statistics->getRemaining()); + $this->assertGreaterThanOrEqual(time(), $statistics->getResetTime()); + $this->assertTrue($statistics->isLimitReached()); + } + + public function testShouldNotBeAbleToSetInvalidLimit(): void + { + $this->expectException(InvalidArgumentException::class); + new Counter($this->getStorage(), 0, 60); + } + + public function testShouldNotBeAbleToSetInvalidPeriod(): void + { + $this->expectException(InvalidArgumentException::class); + new Counter($this->getStorage(), 10, 0); + } + + public function testIncrementMustBeUniformAfterLimitIsReached(): void + { + $timer = new FrozenTimeTimer(); + $counter = new Counter( + $this->getStorage(), + 10, + 1, + 86400, + 'rate-limiter-', + $timer + ); + + // Start with the limit reached. + for ($i = 0; $i < 10; $i++) { + $counter->hit('key'); + } + + for ($i = 0; $i < 5; $i++) { + // Move timer forward for (period in milliseconds / limit) + // i.e. once in period / limit remaining allowance should be increased by 1. + FrozenTimeTimer::setTimeMark($timer->nowInMilliseconds() + 100); + $statistics = $counter->hit('key'); + $this->assertEquals(1, $statistics->getRemaining()); + } + } + + public function testCustomTtl(): void + { + $storage = $this->getStorage(); + + $counter = new Counter( + $storage, + 1, + 1, + 1, + 'rate-limiter-', + new FrozenTimeTimer() + ); + + $counter->hit('test'); + + FrozenTimeTimer::setTimeMark((new MicrotimeTimer())->nowInMilliseconds() + 2); + + self::assertNull($storage->get('rate-limiter-test')); + } + + public function testGetKey(): void + { + $counter = new Counter($this->getStorage(), 1, 1, 1, 'rate-limiter-'); + + $this->assertSame('rate-limiter-key', Assert::invokeMethod($counter, 'getStorageKey', ['key'])); + } +} diff --git a/tests/CounterTest.php b/tests/CounterTest.php index 41f8ce1..a30cc88 100644 --- a/tests/CounterTest.php +++ b/tests/CounterTest.php @@ -4,106 +4,50 @@ namespace Yiisoft\Yii\RateLimiter\Tests; -use InvalidArgumentException; -use PHPUnit\Framework\TestCase; use Yiisoft\Cache\ArrayCache; use Yiisoft\Yii\RateLimiter\Counter; use Yiisoft\Yii\RateLimiter\Storage\SimpleCacheStorage; -use Yiisoft\Yii\RateLimiter\Time\MicrotimeTimer; -use Yiisoft\Yii\RateLimiter\Tests\Support\Assert; +use Yiisoft\Yii\RateLimiter\Storage\StorageInterface; +use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FakeSimpleCacheStorage; +use Yiisoft\Yii\RateLimiter\Tests\Fixtures\FrozenTimeTimer; -final class CounterTest extends TestCase +final class CounterTest extends BaseCounterTest { - public function testStatisticsShouldBeCorrectWhenLimitIsNotReached(): void + protected function getStorage(): StorageInterface { - $counter = new Counter(new SimpleCacheStorage(new ArrayCache()), 2, 5); - - $statistics = $counter->hit('key'); - $this->assertEquals(2, $statistics->getLimit()); - $this->assertEquals(1, $statistics->getRemaining()); - $this->assertGreaterThanOrEqual(time(), $statistics->getResetTime()); - $this->assertFalse($statistics->isLimitReached()); - } - - public function testStatisticsShouldBeCorrectWhenLimitIsReached(): void - { - $counter = new Counter(new SimpleCacheStorage(new ArrayCache()), 2, 4); - - $statistics = $counter->hit('key'); - $this->assertEquals(2, $statistics->getLimit()); - $this->assertEquals(1, $statistics->getRemaining()); - $this->assertGreaterThanOrEqual(time(), $statistics->getResetTime()); - $this->assertFalse($statistics->isLimitReached()); - - $statistics = $counter->hit('key'); - $this->assertEquals(2, $statistics->getLimit()); - $this->assertEquals(0, $statistics->getRemaining()); - $this->assertGreaterThanOrEqual(time(), $statistics->getResetTime()); - $this->assertTrue($statistics->isLimitReached()); + return new SimpleCacheStorage(new ArrayCache()); } - public function testShouldNotBeAbleToSetInvalidLimit(): void - { - $this->expectException(InvalidArgumentException::class); - new Counter(new SimpleCacheStorage(new ArrayCache()), 0, 60); - } - - public function testShouldNotBeAbleToSetInvalidPeriod(): void - { - $this->expectException(InvalidArgumentException::class); - new Counter(new SimpleCacheStorage(new ArrayCache()), 10, 0); - } - - public function testIncrementMustBeUniformAfterLimitIsReached(): void + /** + * Testing that in concurrent scenarios, when dirty reads occur, + * the current limiter cannot be as expected By 'SimpleCacheStorage'. + */ + public function testConcurrentHitsWithDirtyReading(): void { $timer = new FrozenTimeTimer(); + $storage = new FakeSimpleCacheStorage(new ArrayCache(), 5); + $limitHits = 10; $counter = new Counter( - new SimpleCacheStorage(new ArrayCache()), - 10, + $storage, + $limitHits, 1, 86400, 'rate-limiter-', $timer ); - // Start with the limit reached. - for ($i = 0; $i < 10; $i++) { - $counter->hit('key'); - } + $totalHits = 0; + do { + ++$totalHits; - for ($i = 0; $i < 5; $i++) { - // Move timer forward for (period in milliseconds / limit) - // i.e. once in period / limit remaining allowance should be increased by 1. - FrozenTimeTimer::setTimeMark($timer->nowInMilliseconds() + 100); $statistics = $counter->hit('key'); - $this->assertEquals(1, $statistics->getRemaining()); - } - } - public function testCustomTtl(): void - { - $storage = new SimpleCacheStorage(new ArrayCache()); - - $counter = new Counter( - $storage, - 1, - 1, - 1, - 'rate-limiter-', - new FrozenTimeTimer() - ); - - $counter->hit('test'); - - FrozenTimeTimer::setTimeMark((new MicrotimeTimer())->nowInMilliseconds() + 2); - - self::assertNull($storage->get('rate-limiter-test')); - } - - public function testGetKey(): void - { - $counter = new Counter(new SimpleCacheStorage(new ArrayCache()), 1, 1, 1, 'rate-limiter-'); + $remaining = $statistics->getRemaining(); + if ($remaining === 0) { + break; + } + } while (true); - $this->assertSame('rate-limiter-key', Assert::invokeMethod($counter, 'getStorageKey', ['key'])); + $this->assertGreaterThan($limitHits, $totalHits); } } diff --git a/tests/Fixtures/FakeApcuStorage.php b/tests/Fixtures/FakeApcuStorage.php new file mode 100644 index 0000000..73f9543 --- /dev/null +++ b/tests/Fixtures/FakeApcuStorage.php @@ -0,0 +1,55 @@ +fixPrecisionRate); + return (bool)apcu_add($key, $value, $ttl); + } + + public function saveCompareAndSwap(string $key, float $oldValue, float $newValue, int $ttl): bool + { + $oldValue = (int) ($oldValue * $this->fixPrecisionRate); + $newValue = (int) ($newValue * $this->fixPrecisionRate); + return (bool)apcu_cas($key, $oldValue, $newValue); + } + + public function get(string $key): ?float + { + // Simulate dirty reading scenarios in this ApcuStorage class + if ($this->remainingDirtyReadCount > 0 && $this->dirtyReadValue !== null) { + $this->remainingDirtyReadCount--; + return $this->dirtyReadValue; + } + + $readValue = apcu_fetch($key); + if ($readValue === false) { + return null; + } + + $readValue = (float) ($readValue / $this->fixPrecisionRate); + $this->dirtyReadValue = $readValue; + $this->remainingDirtyReadCount = $this->dirtyReadCount; + + return $readValue; + } +} diff --git a/tests/FakeCounter.php b/tests/Fixtures/FakeCounter.php similarity index 90% rename from tests/FakeCounter.php rename to tests/Fixtures/FakeCounter.php index d184e39..cf94576 100644 --- a/tests/FakeCounter.php +++ b/tests/Fixtures/FakeCounter.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Yiisoft\Yii\RateLimiter\Tests; +namespace Yiisoft\Yii\RateLimiter\Tests\Fixtures; use Yiisoft\Yii\RateLimiter\CounterInterface; use Yiisoft\Yii\RateLimiter\CounterState; diff --git a/tests/Fixtures/FakeSimpleCacheStorage.php b/tests/Fixtures/FakeSimpleCacheStorage.php new file mode 100644 index 0000000..0acda4e --- /dev/null +++ b/tests/Fixtures/FakeSimpleCacheStorage.php @@ -0,0 +1,52 @@ +cache->set($key, $value, $ttl); + } + + public function saveCompareAndSwap(string $key, mixed $oldValue, mixed $newValue, int $ttl): bool + { + return $this->cache->set($key, $newValue, $ttl); + } + + public function get(string $key): ?float + { + // Simulate dirty reading scenarios in this SimpleCacheStorage class + if ($this->remainingDirtyReadCount > 0 && $this->dirtyReadValue !== null) { + $this->remainingDirtyReadCount--; + return $this->dirtyReadValue; + } + + $readValue = $this->cache->get($key, false); + if ($readValue === false) { + return null; + } + + $readValue = (float)$readValue; + $this->dirtyReadValue = $readValue; + $this->remainingDirtyReadCount = $this->dirtyReadCount; + + return $readValue; + } +} diff --git a/tests/Fixtures/FrozenTimeTimer.php b/tests/Fixtures/FrozenTimeTimer.php new file mode 100644 index 0000000..3f31c33 --- /dev/null +++ b/tests/Fixtures/FrozenTimeTimer.php @@ -0,0 +1,37 @@ +createRateLimiter($counter, new LimitAlways()); + $middleware->process( + $this->createRequest(Method::GET, '/', ['REMOTE_ADDR' => '193.186.62.12']), + $this->createRequestHandler() + ); + $middleware->process( + $this->createRequest(Method::GET, '/', ['REMOTE_ADDR' => '193.186.62.12']), + $this->createRequestHandler() + ); + + $response = $middleware->process( + $this->createRequest(Method::GET, '/', ['REMOTE_ADDR' => '193.186.62.12']), + $this->createRequestHandler() + ); + + $this->assertEquals(Status::OK, $response->getStatusCode()); + } + + public function testFailStoreUpdatedDataMiddleware(): void + { + $timer = new FrozenTimeTimer(); + $dirtyReadCount = 2; + $storage = new FakeApcuStorage($dirtyReadCount); + $counter = new Counter( + $storage, + 10, + 1, + 86400, + 'rate-limiter-', + $timer, + 1 + ); + $middleware = $this->createRateLimiter( + $counter, + new LimitAlways(), + new class () implements MiddlewareInterface { + public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): ResponseInterface { + $response = $handler->handle($request); + return $response->withHeader('test', 'hello'); + } + } + ); + $middleware->process( + $this->createRequest(Method::GET, '/', ['REMOTE_ADDR' => '193.186.62.12']), + $this->createRequestHandler() + ); + $middleware->process( + $this->createRequest(Method::GET, '/', ['REMOTE_ADDR' => '193.186.62.12']), + $this->createRequestHandler() + ); + + $response = $middleware->process( + $this->createRequest(Method::GET, '/', ['REMOTE_ADDR' => '193.186.62.12']), + $this->createRequestHandler() + ); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('hello', $response->getHeaderLine('test')); + } + private function createRequestHandler(): RequestHandlerInterface { $requestHandler = $this->createMock(RequestHandlerInterface::class); @@ -237,8 +322,14 @@ private function createRequest( private function createRateLimiter( CounterInterface $counter, - LimitPolicyInterface $limitingPolicy = null + ?LimitPolicyInterface $limitingPolicy = null, + ?MiddlewareInterface $failStoreUpdatedDataMiddleware = null ): LimitRequestsMiddleware { - return new LimitRequestsMiddleware($counter, new Psr17Factory(), $limitingPolicy); + return new LimitRequestsMiddleware( + $counter, + new Psr17Factory(), + $limitingPolicy, + $failStoreUpdatedDataMiddleware + ); } } diff --git a/tests/LimitAlwaysTest.php b/tests/Policy/LimitAlwaysTest.php similarity index 93% rename from tests/LimitAlwaysTest.php rename to tests/Policy/LimitAlwaysTest.php index 95d82bf..fddd86f 100644 --- a/tests/LimitAlwaysTest.php +++ b/tests/Policy/LimitAlwaysTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Yiisoft\Yii\RateLimiter\Tests; +namespace Yiisoft\Yii\RateLimiter\Tests\Policy; use Nyholm\Psr7\ServerRequest; use PHPUnit\Framework\TestCase; diff --git a/tests/LimitPerIpTest.php b/tests/Policy/LimitPerIpTest.php similarity index 94% rename from tests/LimitPerIpTest.php rename to tests/Policy/LimitPerIpTest.php index 4d8fa76..8ba1d85 100644 --- a/tests/LimitPerIpTest.php +++ b/tests/Policy/LimitPerIpTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Yiisoft\Yii\RateLimiter\Tests; +namespace Yiisoft\Yii\RateLimiter\Tests\Policy; use Nyholm\Psr7\ServerRequest; use PHPUnit\Framework\TestCase; diff --git a/tests/LimitingFunctionTest.php b/tests/Policy/LimitingFunctionTest.php similarity index 93% rename from tests/LimitingFunctionTest.php rename to tests/Policy/LimitingFunctionTest.php index 210dd5d..e9d13a5 100644 --- a/tests/LimitingFunctionTest.php +++ b/tests/Policy/LimitingFunctionTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Yiisoft\Yii\RateLimiter\Tests; +namespace Yiisoft\Yii\RateLimiter\Tests\Policy; use InvalidArgumentException; use Nyholm\Psr7\ServerRequest; diff --git a/tests/Storage/ApcuStorageTest.php b/tests/Storage/ApcuStorageTest.php new file mode 100644 index 0000000..02e8ac3 --- /dev/null +++ b/tests/Storage/ApcuStorageTest.php @@ -0,0 +1,93 @@ +getStorage(); + + $value = (new FrozenTimeTimer())->nowInMilliseconds(); + $storage->saveIfNotExists('exists_key', $value, self::DEFAULT_TTL); + + $result = $storage->saveIfNotExists('exists_key', $value, self::DEFAULT_TTL); + + $this->assertFalse($result); + } + + public function testSaveCompareAndSwapWithNewKey(): void + { + $storage = $this->getStorage(); + + $newValue = (new FrozenTimeTimer())->nowInMilliseconds(); + $oldValue = (int) $storage->get('new_key'); + + $result = $storage->saveCompareAndSwap( + 'new_key', + $oldValue, + $newValue, + self::DEFAULT_TTL + ); + + $this->assertFalse($result); + } + + public function testSaveCompareAndSwapWithExistsKeyButOldValueDiffrent(): void + { + $storage = $this->getStorage(); + + $oldValue = (new FrozenTimeTimer())->nowInMilliseconds(); + $storage->saveIfNotExists('exists_key', $oldValue, self::DEFAULT_TTL); + + $oldValue = $oldValue + 200; + + $newValue = $oldValue + 100; + + $result = $storage->saveCompareAndSwap( + 'exists_key', + $oldValue, + $newValue, + self::DEFAULT_TTL + ); + + $this->assertFalse($result); + } + + public function testStringValueInCache(): void + { + apcu_add('key', 'string_value', parent::DEFAULT_TTL); + $storage = $this->getStorage(); + + $value = $storage->get('key'); + + $this->assertNull($value); + } +} diff --git a/tests/Storage/SimpleCacheStorageTest.php b/tests/Storage/SimpleCacheStorageTest.php new file mode 100644 index 0000000..255cdbf --- /dev/null +++ b/tests/Storage/SimpleCacheStorageTest.php @@ -0,0 +1,22 @@ +clearStorage(); + } + + public function testGetKeyWithMissingKey(): void + { + $storage = $this->getStorage(); + + $this->assertNull($storage->get('missing_key')); + } + + public function testGetKeyWithExistsKey(): void + { + $storage = $this->getStorage(); + + $want = (new FrozenTimeTimer())->nowInMilliseconds(); + + $storage->saveIfNotExists('exists_key', $want, self::DEFAULT_TTL); + + $result = $storage->get('exists_key'); + + $this->assertEquals($result, $want); + } + + public function testSaveIfNotExistsWithNewKey(): void + { + $storage = $this->getStorage(); + + $value = (new FrozenTimeTimer())->nowInMilliseconds(); + + $result = $storage->saveIfNotExists('new_key', $value, self::DEFAULT_TTL); + + $this->assertTrue($result); + } + + public function testSaveCompareAndSwapWithExistsKeyAndOldValueSame(): void + { + $storage = $this->getStorage(); + + $oldValue = (new FrozenTimeTimer())->nowInMilliseconds(); + $storage->saveIfNotExists('exists_key', $oldValue, self::DEFAULT_TTL); + + $newValue = $oldValue + 100; + + $result = $storage->saveCompareAndSwap( + 'exists_key', + $oldValue, + $newValue, + self::DEFAULT_TTL + ); + + $this->assertTrue($result); + } +}