diff --git a/src/Cache/RateLimit/RateLimitConfig.php b/src/Cache/RateLimit/RateLimitConfig.php index 28ac7ad6..c5762892 100644 --- a/src/Cache/RateLimit/RateLimitConfig.php +++ b/src/Cache/RateLimit/RateLimitConfig.php @@ -25,6 +25,11 @@ public function connection(): string return $this->config['connection'] ?? 'default'; } + public function prefix(): string + { + return (string) Configuration::get('cache.prefix', ''); + } + public function ttl(): int { return 60; diff --git a/src/Cache/RateLimit/RateLimitFactory.php b/src/Cache/RateLimit/RateLimitFactory.php index 11f7c20d..d9458101 100644 --- a/src/Cache/RateLimit/RateLimitFactory.php +++ b/src/Cache/RateLimit/RateLimitFactory.php @@ -14,11 +14,12 @@ class RateLimitFactory { - public static function redis(int $ttl, string $connection = 'default'): RateLimit + public static function redis(int $ttl, string $connection = 'default', string $prefix = ''): RateLimit { $clientWrapper = Redis::connection($connection)->client(); + $rateLimit = new RedisRateLimit($clientWrapper->getClient(), $ttl); - return new RedisRateLimit($clientWrapper->getClient(), $ttl); + return self::withPrefix($rateLimit, $prefix); } public static function local(int $ttl): RateLimit @@ -31,6 +32,10 @@ public static function local(int $ttl): RateLimit public static function withPrefix(RateLimit $rateLimit, string $prefix): RateLimit { + if ($prefix === '') { + return $rateLimit; + } + return new PrefixRateLimit($rateLimit, $prefix); } } diff --git a/src/Cache/RateLimit/RateLimitManager.php b/src/Cache/RateLimit/RateLimitManager.php index dc53f36c..f354d770 100644 --- a/src/Cache/RateLimit/RateLimitManager.php +++ b/src/Cache/RateLimit/RateLimitManager.php @@ -47,7 +47,7 @@ public function prefixed(string $prefix): self protected function resolveStore(): RateLimit { return match ($this->config->default()) { - 'redis' => RateLimitFactory::redis($this->config->ttl(), $this->config->connection()), + 'redis' => RateLimitFactory::redis($this->config->ttl(), $this->config->connection(), $this->config->prefix()), 'local' => RateLimitFactory::local($this->config->ttl()), default => RateLimitFactory::local($this->config->ttl()), }; diff --git a/src/Cache/Stores/RedisStore.php b/src/Cache/Stores/RedisStore.php index 9d8e55ea..55c54fe7 100644 --- a/src/Cache/Stores/RedisStore.php +++ b/src/Cache/Stores/RedisStore.php @@ -59,7 +59,7 @@ public function clear(): void $iterator = null; do { - [$keys, $iterator] = $this->client->execute('SCAN', $iterator ?? 0, 'MATCH', $this->getPrefixedKey('*'), 'COUNT', 1000); + [$iterator, $keys] = $this->client->execute('SCAN', $iterator ?? 0, 'MATCH', $this->getPrefixedKey('*'), 'COUNT', 1000); if (! empty($keys)) { $this->client->execute('DEL', ...$keys); diff --git a/src/Testing/TestCase.php b/src/Testing/TestCase.php index 6897e539..91b7caea 100644 --- a/src/Testing/TestCase.php +++ b/src/Testing/TestCase.php @@ -12,6 +12,7 @@ use Phenix\Cache\Constants\Store; use Phenix\Console\Phenix; use Phenix\Facades\Cache; +use Phenix\Facades\Config; use Phenix\Facades\Event; use Phenix\Facades\Mail; use Phenix\Facades\Queue; @@ -19,6 +20,7 @@ use Phenix\Testing\Concerns\InteractWithDatabase; use Phenix\Testing\Concerns\InteractWithResponses; use Phenix\Testing\Concerns\RefreshDatabase; +use Phenix\Util\Str; use Symfony\Component\Console\Tester\CommandTester; use Throwable; @@ -45,6 +47,8 @@ protected function setUp(): void $this->app->enableTestingMode(); } + Config::set('cache.prefix', sprintf('phenix_test_%s_', Str::random(16))); + $uses = class_uses_recursive($this); if (in_array(RefreshDatabase::class, $uses, true) && method_exists($this, 'refreshDatabase')) { diff --git a/tests/Unit/Cache/RateLimit/RedisRateLimitTest.php b/tests/Unit/Cache/RateLimit/RedisRateLimitTest.php index c238bdca..97a40750 100644 --- a/tests/Unit/Cache/RateLimit/RedisRateLimitTest.php +++ b/tests/Unit/Cache/RateLimit/RedisRateLimitTest.php @@ -2,18 +2,79 @@ declare(strict_types=1); +use Amp\Redis\Connection\RedisLink; +use Amp\Redis\Protocol\RedisResponse; +use Amp\Redis\RedisClient; +use Kelunik\RateLimit\PrefixRateLimit; use Kelunik\RateLimit\RedisRateLimit; use Phenix\Cache\Constants\Store; use Phenix\Cache\RateLimit\RateLimitManager; +use Phenix\Database\Constants\Connection; use Phenix\Facades\Config; +use Phenix\Redis\ClientWrapper; beforeEach(function (): void { Config::set('cache.default', Store::REDIS->value); Config::set('cache.rate_limit.store', Store::REDIS->value); }); -it('call redis rate limit factory', function (): void { +it('prefixes redis rate limit keys with the cache namespace', function (): void { + Config::set('cache.prefix', 'cache-prefix:'); + $manager = new RateLimitManager(); + $limiter = $manager->limiter(); + + expect($limiter)->toBeInstanceOf(PrefixRateLimit::class); + + $reflection = new ReflectionClass($limiter); + + $prefix = $reflection->getProperty('prefix'); + $prefix->setAccessible(true); + + $rateLimit = $reflection->getProperty('rateLimit'); + $rateLimit->setAccessible(true); + + expect($prefix->getValue($limiter))->toBe('cache-prefix:'); + expect($rateLimit->getValue($limiter))->toBeInstanceOf(RedisRateLimit::class); +}); + +it('isolates redis rate limit state across cache prefixes', function (): void { + $incrementResponse = $this->createStub(RedisResponse::class); + $incrementResponse->method('unwrap')->willReturn(1); + + $expireResponse = $this->createStub(RedisResponse::class); + $expireResponse->method('unwrap')->willReturn(1); + + $getResponse = $this->createStub(RedisResponse::class); + $getResponse->method('unwrap')->willReturn(null); + + $link = $this->createMock(RedisLink::class); + + $link->expects($this->exactly(3)) + ->method('execute') + ->withConsecutive( + [ + $this->equalTo('incr'), + $this->equalTo(['first-prefix:route:client']), + ], + [ + $this->equalTo('expire'), + $this->equalTo(['first-prefix:route:client', 60]), + ], + [ + $this->equalTo('get'), + $this->equalTo(['second-prefix:route:client']), + ] + ) + ->willReturnOnConsecutiveCalls($incrementResponse, $expireResponse, $getResponse); + + $client = new RedisClient($link); + $this->app->swap(Connection::redis('default'), new ClientWrapper($client)); + + Config::set('cache.prefix', 'first-prefix:'); + (new RateLimitManager())->prefixed('route:')->increment('client'); + + Config::set('cache.prefix', 'second-prefix:'); - expect($manager->limiter())->toBeInstanceOf(RedisRateLimit::class); + expect((new RateLimitManager())->prefixed('route:')->get('client'))->toBe(0); }); diff --git a/tests/Unit/Cache/RedisStoreTest.php b/tests/Unit/Cache/RedisStoreTest.php index 4225320c..3f497e6b 100644 --- a/tests/Unit/Cache/RedisStoreTest.php +++ b/tests/Unit/Cache/RedisStoreTest.php @@ -165,7 +165,7 @@ expect(Cache::has('gamma'))->toBeFalse(); }); -it('clears all values', function (): void { +it('clears all values across scan iterations', function (): void { $client = $this->getMockBuilder(ClientWrapper::class) ->disableOriginalConstructor() ->getMock(); @@ -190,10 +190,21 @@ expect($args[4])->toBe('COUNT'); expect($args[5])->toBe(1000); - return [["{$prefix}a", "{$prefix}b"], '0']; + return ['12', []]; } if ($callCount === 4) { + expect($args[0])->toBe('SCAN'); + expect($args[1])->toBe('12'); + expect($args[2])->toBe('MATCH'); + expect($args[3])->toBe("{$prefix}*"); + expect($args[4])->toBe('COUNT'); + expect($args[5])->toBe(1000); + + return ['0', ["{$prefix}a", "{$prefix}b"]]; + } + + if ($callCount === 5) { expect($args[0])->toBe('DEL'); expect($args[1])->toBe("{$prefix}a"); expect($args[2])->toBe("{$prefix}b"); @@ -201,10 +212,6 @@ return 2; } - if ($callCount === 5) { - return 0; - } - return null; }); @@ -214,8 +221,6 @@ Cache::set('b', 2); Cache::clear(); - - expect(Cache::has('a'))->toBeFalse(); }); it('stores forever without expiration', function (): void {