Remove array caching to prevent always returning miss from memory (#3)

Merging into local branch for testing.
jgivoni · Oct 27, 2023 · 19cc7fd · 19cc7fd
1 parent 68ad938
commit 19cc7fd
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 30 deletions.
diff --git a/src/CacheAdapter.php b/src/CacheAdapter.php
@@ -312,22 +312,22 @@ public function checksum(string $path, Config $config): string
         $algo = $config->get('checksum_algo');
         $metadataKey = isset($algo) ? 'checksum_' . $algo : 'checksum';
 
-        $attributeAccessor = function (FileAttributes $fileAttributes) use ($metadataKey) {
+        $attributeAccessor = function (StorageAttributes $storageAttributes) use ($metadataKey) {
             if (\is_a($this->adapter, 'League\Flysystem\AwsS3V3\AwsS3V3Adapter')) {
                 // Special optimization for AWS S3, but won't break if adapter not installed
-                $etag = $fileAttributes->extraMetadata()['ETag'] ?? \null;
+                $etag = $storageAttributes->extraMetadata()['ETag'] ?? \null;
                 if (isset($etag)) {
                     $checksum = trim($etag, '" ');
                 }
             }
 
-            return $checksum ?? $fileAttributes->extraMetadata()[$metadataKey] ?? \null;
+            return $checksum ?? $storageAttributes->extraMetadata()[$metadataKey] ?? \null;
         };
 
         $fileAttributes = $this->getFileAttributes(
             path: $path,
             loader: function () use ($path, $config, $metadataKey) {
-                // This part is "mirrored" from FileSystem class to provide the fallback mechanism 
+                // This part is "mirrored" from FileSystem class to provide the fallback mechanism
                 // and be able to cache the result
                 try {
                     if (!$this->adapter instanceof ChecksumProvider) {

diff --git a/src/CacheItemsTrait.php b/src/CacheItemsTrait.php
@@ -21,42 +21,21 @@ trait CacheItemsTrait
     static string $CACHE_KEY_PREFIX = 'flysystem_item_';
     static string $CACHE_KEY_HASH_SALT = '563ce5132194441b';
 
-    /** @var array<CacheItemInterface> */
-    protected $cacheItems = [];
-
     protected function getCacheItem(string $path): CacheItemInterface
     {
-        if (!isset($this->cacheItems[$path])) {
-            $key = self::getCacheItemKey($path);
-
-            $this->cacheItems[$path] = $this->cache->getItem($key);
-        }
+        $key = self::getCacheItemKey($path);
 
-        return $this->cacheItems[$path];
+        return $this->cache->getItem($key);
     }
 
     protected function saveCacheItem(CacheItemInterface $cacheItem): void
     {
         $this->cache->save($cacheItem);
-
-        /** @var StorageAttributes $storageAttributes */
-        $storageAttributes = $cacheItem->get();
-
-        $path = $storageAttributes->path();
-
-        $this->cacheItems[$path] = $cacheItem;
     }
 
     protected function deleteCacheItem(CacheItemInterface $cacheItem): void
     {
         $this->cache->deleteItem($cacheItem->getKey());
-
-        /** @var StorageAttributes $storageAttributes */
-        $storageAttributes = $cacheItem->get();
-
-        $path = $storageAttributes->path();
-
-        unset($this->cacheItems[$path]);
     }
 
     public static function getCacheItemKey(string $path): string

diff --git a/tests/CacheItemsTrait_Test.php b/tests/CacheItemsTrait_Test.php
@@ -0,0 +1,52 @@
+<?php
+
+namespace tests\jgivoni\Flysystem\Cache;
+
+use jgivoni\Flysystem\Cache\CacheItemsTrait;
+use League\Flysystem\FileAttributes;
+use PHPUnit\Framework\TestCase;
+use Psr\Cache\CacheItemInterface;
+use Psr\Cache\CacheItemPoolInterface;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+
+class CacheItemsTrait_Test extends TestCase
+{
+    /**
+     * @test
+     */
+    public function return_hit_after_miss(): void
+    {
+        $adapter = new class {
+            use CacheItemsTrait;
+
+            public function __construct(
+                protected readonly CacheItemPoolInterface $cache = new ArrayAdapter()
+            ) {
+            }
+
+            public function getItem(string $path): CacheItemInterface
+            {
+                return $this->getCacheItem($path);
+            }
+
+            public function saveItem(CacheItemInterface $item): void
+            {
+                $this->saveCacheItem($item);
+            }
+        };
+
+        $path = 'test.txt';
+        $item = $adapter->getItem($path);
+
+        self::assertFalse($item->isHit());
+
+        $item->set(new FileAttributes(
+            path: $path
+        ));
+
+        $adapter->saveItem($item);
+
+        $freshItem = $adapter->getItem($path);
+        self::assertTrue($freshItem->isHit());
+    }
+}
diff --git a/tests/FileExists_Test.php b/tests/FileExists_Test.php
@@ -6,7 +6,7 @@
 
 class FileExists_Test extends CacheTestCase
 {
-    /** 
+    /**
      * @test
      * @dataProvider dataProvider
      */
@@ -18,7 +18,7 @@ public function file_exists_ok(string $path, bool $expectedResult): void
     }
 
     /**
-     * 
+     *
      * @return iterable<array<mixed>>
      */
     public static function dataProvider(): iterable
@@ -30,7 +30,7 @@ public static function dataProvider(): iterable
         yield 'directory is not a file' => ['cached-directory', false];
     }
 
-    /** 
+    /**
      * @test
      */
     public function file_is_cached_after_checking_filesystem(): void