Merge pull request #123 from phenixphp/develop

Release v0.8.5

Author: barbosa89

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;

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);
     }
 }

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()),
         };

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);

src/Testing/TestCase.php

@@ -12,13 +12,15 @@
 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;
 use Phenix\Facades\View;
 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')) {

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);
 });

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,21 +190,28 @@
                 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");
 
                 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 {