Merge pull request codeigniter4#8952 from paulbalandan/redis-delete-m…

…atching-prefix fix: `RedisHandler::deleteMatching()` not deleting matching keys if cache prefix is used
ddevsr · Jun 14, 2024 · b6c668c · b6c668c
2 parents a13f54d + cb23bdb
commit b6c668c
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 33 deletions.
diff --git a/system/Cache/Handlers/RedisHandler.php b/system/Cache/Handlers/RedisHandler.php
@@ -175,18 +175,17 @@ public function delete(string $key)
      */
     public function deleteMatching(string $pattern)
     {
+        /** @var list<string> $matchedKeys */
         $matchedKeys = [];
+        $pattern     = static::validateKey($pattern, $this->prefix);
         $iterator    = null;
 
         do {
-            // Scan for some keys
+            /** @var false|list<string>|Redis $keys */
             $keys = $this->redis->scan($iterator, $pattern);
 
-            // Redis may return empty results, so protect against that
-            if ($keys !== false) {
-                foreach ($keys as $key) {
-                    $matchedKeys[] = $key;
-                }
+            if (is_array($keys)) {
+                $matchedKeys = [...$matchedKeys, ...$keys];
             }
         } while ($iterator > 0);
 

diff --git a/tests/system/Cache/Handlers/RedisHandlerTest.php b/tests/system/Cache/Handlers/RedisHandlerTest.php
@@ -13,9 +13,11 @@
 
 namespace CodeIgniter\Cache\Handlers;
 
+use CodeIgniter\Cache\CacheFactory;
 use CodeIgniter\CLI\CLI;
 use CodeIgniter\I18n\Time;
 use Config\Cache;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
 
 /**
@@ -129,44 +131,59 @@ public function testDelete(): void
         $this->assertFalse($this->handler->delete(self::$dummy));
     }
 
-    public function testDeleteMatchingPrefix(): void
+    #[DataProvider('provideDeleteMatching')]
+    public function testDeleteMatching(string $pattern, int $expectedDeleteCount, string $prefix = ''): void
     {
-        // Save 101 items to match on
+        $cache = new Cache();
+
+        if ($prefix !== '') {
+            $cache->prefix = $prefix;
+        }
+
+        /** @var RedisHandler $handler */
+        $handler = CacheFactory::getHandler($cache, 'redis');
+
         for ($i = 1; $i <= 101; $i++) {
-            $this->handler->save('key_' . $i, 'value' . $i);
+            $handler->save('key_' . $i, 'value_' . $i);
         }
 
-        // check that there are 101 items is cache store
-        $dbInfo = explode(',', $this->handler->getCacheInfo()['db0']);
-        $this->assertSame('keys=101', $dbInfo[0]);
+        $cacheInfo = $handler->getCacheInfo();
+        $this->assertIsArray($cacheInfo);
+        $this->assertArrayHasKey('db0', $cacheInfo);
+        $this->assertIsString($cacheInfo['db0']);
+        $this->assertMatchesRegularExpression('/^keys=(?P<count>\d+)/', $cacheInfo['db0']);
+
+        preg_match('/^keys=(?P<count>\d+)/', $cacheInfo['db0'], $matches);
+        $this->assertSame(101, (int) $matches['count']);
+
+        $this->assertSame($expectedDeleteCount, $handler->deleteMatching($pattern));
 
-        // Checking that given the prefix "key_1", deleteMatching deletes 13 keys:
-        // (key_1, key_10, key_11, key_12, key_13, key_14, key_15, key_16, key_17, key_18, key_19, key_100, key_101)
-        $this->assertSame(13, $this->handler->deleteMatching('key_1*'));
+        $cacheInfo = $handler->getCacheInfo();
+        $this->assertIsArray($cacheInfo);
+        $this->assertArrayHasKey('db0', $cacheInfo);
+        $this->assertIsString($cacheInfo['db0']);
+        $this->assertMatchesRegularExpression('/^keys=(?P<count>\d+)/', $cacheInfo['db0']);
 
-        // check that there remains (101 - 13) = 88 items is cache store
-        $dbInfo = explode(',', $this->handler->getCacheInfo()['db0']);
-        $this->assertSame('keys=88', $dbInfo[0]);
+        preg_match('/^keys=(?P<count>\d+)/', $cacheInfo['db0'], $matches);
+        $this->assertSame(101 - $expectedDeleteCount, (int) $matches['count']);
+
+        $handler->deleteMatching('key_*');
     }
 
-    public function testDeleteMatchingSuffix(): void
+    /**
+     * @return iterable<string, array{0: string, 1: int, 2?: string}>
+     */
+    public static function provideDeleteMatching(): iterable
     {
-        // Save 101 items to match on
-        for ($i = 1; $i <= 101; $i++) {
-            $this->handler->save('key_' . $i, 'value' . $i);
-        }
-
-        // check that there are 101 items is cache store
-        $dbInfo = explode(',', $this->handler->getCacheInfo()['db0']);
-        $this->assertSame('keys=101', $dbInfo[0]);
+        // Given the key "key_1*", deleteMatching() should delete 13 keys:
+        // key_1, key_10, key_11, key_12, key_13, key_14, key_15, key_16, key_17, key_18, key_19, key_100, key_101
+        yield 'prefix' => ['key_1*', 13];
 
-        // Checking that given the suffix "1", deleteMatching deletes 11 keys:
-        // (key_1, key_11, key_21, key_31, key_41, key_51, key_61, key_71, key_81, key_91, key_101)
-        $this->assertSame(11, $this->handler->deleteMatching('*1'));
+        // Given the key "*1", deleteMatching() should delete 11 keys:
+        // key_1, key_11, key_21, key_31, key_41, key_51, key_61, key_71, key_81, key_91, key_101
+        yield 'suffix' => ['*1', 11];
 
-        // check that there remains (101 - 13) = 88 items is cache store
-        $dbInfo = explode(',', $this->handler->getCacheInfo()['db0']);
-        $this->assertSame('keys=90', $dbInfo[0]);
+        yield 'cache-prefix' => ['key_1*', 13, 'foo_'];
     }
 
     public function testIncrementAndDecrement(): void