Merge pull request #7667 from paulbalandan/refactor-file-handler-cache

refactor: [Cache] simplify code of `FileHandler::getItem()`

Author: paulbalandan

phpstan-baseline.neon.dist

@@ -10,16 +10,6 @@ parameters:
 			count: 1
 			path: system/Autoloader/Autoloader.php
 
-		-
-			message: "#^Comparison operation \"\\>\" between int and \\(array\\|float\\|int\\) results in an error\\.$#"
-			count: 1
-			path: system/Cache/Handlers/FileHandler.php
-
-		-
-			message: "#^If condition is always true\\.$#"
-			count: 1
-			path: system/Cache/Handlers/FileHandler.php
-
 		-
 			message: "#^Property CodeIgniter\\\\Cache\\\\Handlers\\\\RedisHandler\\:\\:\\$redis \\(Redis\\) in isset\\(\\) is not nullable\\.$#"
 			count: 1

system/Cache/Handlers/FileHandler.php

@@ -144,43 +144,30 @@ public function deleteMatching(string $pattern)
      */
     public function increment(string $key, int $offset = 1)
     {
-        $key  = static::validateKey($key, $this->prefix);
-        $data = $this->getItem($key);
+        $key = static::validateKey($key, $this->prefix);
+        $tmp = $this->getItem($key);
 
-        if ($data === false) {
-            $data = [
-                'data' => 0,
-                'ttl'  => 60,
-            ];
-        } elseif (! is_int($data['data'])) {
+        if ($tmp === false) {
+            $tmp = ['data' => 0, 'ttl' => 60];
+        }
+
+        ['data' => $value, 'ttl' => $ttl] = $tmp;
+
+        if (! is_int($value)) {
             return false;
         }
 
-        $newValue = $data['data'] + $offset;
+        $value += $offset;
 
-        return $this->save($key, $newValue, $data['ttl']) ? $newValue : false;
+        return $this->save($key, $value, $ttl) ? $value : false;
     }
 
     /**
      * {@inheritDoc}
      */
     public function decrement(string $key, int $offset = 1)
     {
-        $key  = static::validateKey($key, $this->prefix);
-        $data = $this->getItem($key);
-
-        if ($data === false) {
-            $data = [
-                'data' => 0,
-                'ttl'  => 60,
-            ];
-        } elseif (! is_int($data['data'])) {
-            return false;
-        }
-
-        $newValue = $data['data'] - $offset;
-
-        return $this->save($key, $newValue, $data['ttl']) ? $newValue : false;
+        return $this->increment($key, -$offset);
     }
 
     /**
@@ -229,7 +216,8 @@ public function isSupported(): bool
      * Does the heavy lifting of actually retrieving the file and
      * verifying it's age.
      *
-     * @return array|bool|float|int|object|string|null
+     * @return array<string, mixed>|false
+     * @phpstan-return array{data: mixed, ttl: int, time: int}|false
      */
     protected function getItem(string $filename)
     {
@@ -238,15 +226,21 @@ protected function getItem(string $filename)
         }
 
         $data = @unserialize(file_get_contents($this->path . $filename));
-        if (! is_array($data) || ! isset($data['ttl'])) {
+
+        if (! is_array($data)) {
+            return false;
+        }
+
+        if (! isset($data['ttl']) || ! is_int($data['ttl'])) {
+            return false;
+        }
+
+        if (! isset($data['time']) || ! is_int($data['time'])) {
             return false;
         }
 
         if ($data['ttl'] > 0 && Time::now()->getTimestamp() > $data['time'] + $data['ttl']) {
-            // If the file is still there then try to remove it
-            if (is_file($this->path . $filename)) {
-                @unlink($this->path . $filename);
-            }
+            @unlink($this->path . $filename);
 
             return false;
         }

tests/system/Cache/Handlers/FileHandlerTest.php

@@ -78,6 +78,11 @@ public function testNew()
         $this->assertInstanceOf(FileHandler::class, $this->handler);
     }
 
+    /**
+     * chmod('path', 0444) does not work on Windows
+     *
+     * @requires OS Linux|Darwin
+     */
     public function testNewWithNonWritablePath()
     {
         $this->expectException(CacheException::class);
@@ -132,6 +137,11 @@ public function testRemember()
         $this->assertNull($this->handler->get(self::$key1));
     }
 
+    /**
+     * chmod('path', 0444) does not work on Windows
+     *
+     * @requires OS Linux|Darwin
+     */
     public function testSave()
     {
         $this->assertTrue($this->handler->save(self::$key1, 'value'));
@@ -265,10 +275,11 @@ public function testIsSupported()
     /**
      * @dataProvider modeProvider
      *
-     * @param mixed $int
-     * @param mixed $string
+     * permissions given on Windows are fixed to `0666`
+     *
+     * @requires OS Linux|Darwin
      */
-    public function testSaveMode($int, $string)
+    public function testSaveMode(int $int, string $string): void
     {
         // Initialize mode
         $config               = new Cache();