Refactoring AbstractCache.php

composer.json

@@ -8,12 +8,12 @@
     },
     "require": {
         "league/flysystem": "~1.0",
-        "psr/cache": "^1.0.0"
+        "psr/cache": "^2.0|^3.0"
     },
     "require-dev": {
-        "phpspec/phpspec": "^3.4",
-        "phpunit/phpunit": "^5.7",
-        "mockery/mockery": "~0.9",
+        "phpspec/phpspec": "^7.4.0",
+        "phpunit/phpunit": "^8.5",
+        "mockery/mockery": "^1.6",
         "predis/predis": "~1.0",
         "tedivm/stash": "~0.12"
     },

phpunit.xml

@@ -8,7 +8,6 @@
          convertWarningsToExceptions="true"
          processIsolation="false"
          stopOnFailure="false"
-         syntaxCheck="true"
          verbose="true"
 >
     <testsuites>
@@ -21,9 +20,4 @@
             <directory suffix=".php">./src/</directory>
         </whitelist>
     </filter>
-    <logging>
-        <log type="coverage-text" target="php://stdout" showUncoveredFiles="true"/>
-        <log type="coverage-html" target="coverage" showUncoveredFiles="true"/>
-        <log type="coverage-clover" target="clover/phpunit.xml" showUncoveredFiles="true"/>
-    </logging>
 </phpunit>

src/Storage/AbstractCache.php

@@ -10,7 +10,7 @@ abstract class AbstractCache implements CacheInterface
     /**
      * @var bool
      */
-    protected $autosave = true;
+    protected bool $autosave = true;
 
     /**
      * @var array
@@ -22,22 +22,15 @@ abstract class AbstractCache implements CacheInterface
      */
     protected $complete = [];
 
-    /**
-     * Destructor.
-     */
+
     public function __destruct()
     {
         if (! $this->autosave) {
             $this->save();
         }
     }
 
-    /**
-     * Get the autosave setting.
-     *
-     * @return bool autosave
-     */
-    public function getAutosave()
+    public function getAutosave(): bool
     {
         return $this->autosave;
     }
@@ -47,11 +40,12 @@ public function getAutosave()
      *
      * @param bool $autosave
      */
-    public function setAutosave($autosave)
+    public function setAutosave(bool $autosave): void
     {
         $this->autosave = $autosave;
     }
 
+
     /**
      * Store the contents listing.
      *
@@ -63,24 +57,32 @@ public function setAutosave($autosave)
      */
     public function storeContents($directory, array $contents, $recursive = false)
     {
-        $directories = [$directory];
+        // Use associative array instead of indexed array
+        $directories = [$directory => true];
 
         foreach ($contents as $object) {
             $this->updateObject($object['path'], $object);
-            $object = $this->cache[$object['path']];
 
-            if ($recursive && $this->pathIsInDirectory($directory, $object['path'])) {
-                $directories[] = $object['dirname'];
+            // Assign it to a variable before the loop
+            $objectCachePath = $this->cache[$object['path']];
+
+            if ($recursive && $this->pathIsInDirectory($directory, $objectCachePath['path'])) {
+                // Check if the directory is already present using isset()
+                if (!isset($directories[$objectCachePath['dirname']])) {
+                    $directories[$objectCachePath['dirname']] = true;
+                }
             }
         }
 
-        foreach (array_unique($directories) as $directory) {
+        // Since $directories is now an associative array, use array_keys() to get the keys
+        foreach (array_keys($directories) as $directory) {
             $this->setComplete($directory, $recursive);
         }
 
         $this->autosave();
     }
 
+
     /**
      * Update the metadata for an object.
      *
@@ -91,18 +93,19 @@ public function storeContents($directory, array $contents, $recursive = false)
     public function updateObject($path, array $object, $autosave = false)
     {
         if (! $this->has($path)) {
-            $this->cache[$path] = Util::pathinfo($path);
+            $this->cache[$path] = Util::pathinfo($path) + $object;
+        } else {
+            $this->cache[$path] += $object;
         }
 
-        $this->cache[$path] = array_merge($this->cache[$path], $object);
-
         if ($autosave) {
             $this->autosave();
         }
 
         $this->ensureParentDirectories($path);
     }
 
+
     /**
      * Store object hit miss.
      *
@@ -130,9 +133,10 @@ public function listContents($dirname = '', $recursive = false)
             if ($object === false) {
                 continue;
             }
-            if ($object['dirname'] === $dirname) {
-                $result[] = $object;
-            } elseif ($recursive && $this->pathIsInDirectory($dirname, $object['path'])) {
+
+            $inDirectory = $object['dirname'] === $dirname || ($recursive && $this->pathIsInDirectory($dirname, $object['path']));
+
+            if ($inDirectory) {
                 $result[] = $object;
             }
         }
@@ -166,6 +170,8 @@ public function read($path)
         return false;
     }
 
+
+
     /**
      * {@inheritdoc}
      */
@@ -214,17 +220,22 @@ public function delete($path)
      */
     public function deleteDir($dirname)
     {
-        foreach ($this->cache as $path => $object) {
-            if ($this->pathIsInDirectory($dirname, $path) || $path === $dirname) {
-                unset($this->cache[$path]);
-            }
-        }
+        // We filter out any elements of the cache that belong to the directory
+        // we are deleting, and assign the filtered array back to $this->cache.
+        $this->cache = array_filter(
+            $this->cache,
+            function($path) use ($dirname) {
+                return !($this->pathIsInDirectory($dirname, $path) || $path === $dirname);
+            },
+            ARRAY_FILTER_USE_KEY
+        );
 
         unset($this->complete[$dirname]);
 
         $this->autosave();
     }
 
+
     /**
      * {@inheritdoc}
      */
@@ -297,17 +308,11 @@ public function getMetadata($path)
      */
     public function isComplete($dirname, $recursive)
     {
-        if (! array_key_exists($dirname, $this->complete)) {
-            return false;
-        }
-
-        if ($recursive && $this->complete[$dirname] !== 'recursive') {
-            return false;
-        }
-
-        return true;
+        return array_key_exists($dirname, $this->complete)
+            && (!$recursive || $this->complete[$dirname] === 'recursive');
     }
 
+
     /**
      * {@inheritdoc}
      */
@@ -325,21 +330,28 @@ public function setComplete($dirname, $recursive)
      */
     public function cleanContents(array $contents)
     {
-        $cachedProperties = array_flip([
+        // Defined properties array that needs to be cached
+        $properties = [
             'path', 'dirname', 'basename', 'extension', 'filename',
             'size', 'mimetype', 'visibility', 'timestamp', 'type',
             'md5',
-        ]);
+        ];
+
+        // Flipped the properties array once outside of the loop to avoid repeated computation
+        $cachedProperties = array_flip($properties);
 
-        foreach ($contents as $path => $object) {
+        // Using array_map instead of foreach for better readability and performance.
+        // array_map applies a callback to each element in the array and returns a new array with the results
+        return array_map(function($object) use ($cachedProperties) {
             if (is_array($object)) {
-                $contents[$path] = array_intersect_key($object, $cachedProperties);
+                // Intersect the keys of the object with the cached properties
+                return array_intersect_key($object, $cachedProperties);
             }
-        }
-
-        return $contents;
+            return $object;
+        }, $contents);
     }
 
+
     /**
      * {@inheritdoc}
      */
@@ -394,15 +406,20 @@ public function setFromStorage($json)
      */
     public function ensureParentDirectories($path)
     {
+        // Getting the initial object from cache
         $object = $this->cache[$path];
 
+        // We'll keep creating parent directories until the dirname is empty or the directory already exists
         while ($object['dirname'] !== '' && ! isset($this->cache[$object['dirname']])) {
-            $object = Util::pathinfo($object['dirname']);
-            $object['type'] = 'dir';
+            $object = Util::pathinfo($object['dirname']); // updating the object to be the parent directory
+            $object['type'] = 'dir'; // it's a directory, so we'll set the type to 'dir'
+
+            // The new object represents the parent directory, so we'll use the 'path' as the key
             $this->cache[$object['path']] = $object;
         }
     }
 
+
     /**
      * Determines if the path is inside the directory.
      *
@@ -411,8 +428,9 @@ public function ensureParentDirectories($path)
      *
      * @return bool
      */
-    protected function pathIsInDirectory($directory, $path)
+    protected function pathIsInDirectory(string $directory, string $path): bool
     {
-        return $directory === '' || strpos($path, $directory . '/') === 0;
+        return $directory === '' || str_starts_with($path, $directory . '/');
     }
+
 }

src/Storage/Noop.php

@@ -7,7 +7,7 @@ class Noop extends AbstractCache
     /**
      * {@inheritdoc}
      */
-    protected $autosave = false;
+    protected bool $autosave = false;
 
     /**
      * {@inheritdoc}

src/Storage/PhpRedis.php

@@ -1,44 +1,25 @@
 <?php
 
+declare(strict_types=1);
+
 namespace League\Flysystem\Cached\Storage;
 
 use Redis;
 
 class PhpRedis extends AbstractCache
 {
-    /**
-     * @var Redis PhpRedis Client
-     */
-    protected $client;
-
-    /**
-     * @var string storage key
-     */
-    protected $key;
-
-    /**
-     * @var int|null seconds until cache expiration
-     */
-    protected $expire;
-
-    /**
-     * Constructor.
-     *
-     * @param Redis|null $client phpredis client
-     * @param string     $key    storage key
-     * @param int|null   $expire seconds until cache expiration
-     */
-    public function __construct(Redis $client = null, $key = 'flysystem', $expire = null)
+    protected Redis $client;
+    protected string $key;
+    protected ?int $ttl;
+
+    public function __construct(?Redis $client = null, string $key = 'flysystem', ?int $ttl = null)
     {
-        $this->client = $client ?: new Redis();
+        $this->client = $client ?? new Redis();
         $this->key = $key;
-        $this->expire = $expire;
+        $this->ttl = $ttl;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function load()
+    public function load(): void
     {
         $contents = $this->client->get($this->key);
 
@@ -47,16 +28,13 @@ public function load()
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function save()
+    public function save(): void
     {
         $contents = $this->getForStorage();
         $this->client->set($this->key, $contents);
 
-        if ($this->expire !== null) {
-            $this->client->expire($this->key, $this->expire);
+        if ($this->ttl !== null) {
+            $this->client->expire($this->key, $this->ttl);
         }
     }
 }