14-purge-cache-on-unsuccessful-operations (#15)

jgivoni · Jan 30, 2024 · ff50d3e · ff50d3e
1 parent e29afec
commit ff50d3e
Show file tree

Hide file tree

Showing 8 changed files with 154 additions and 44 deletions.
diff --git a/src/CacheAdapter.php b/src/CacheAdapter.php
@@ -10,9 +10,13 @@
 use League\Flysystem\FileAttributes;
 use League\Flysystem\FilesystemAdapter;
 use League\Flysystem\StorageAttributes;
+use League\Flysystem\UnableToCopyFile;
+use League\Flysystem\UnableToDeleteFile;
+use League\Flysystem\UnableToMoveFile;
 use League\Flysystem\UnableToProvideChecksum;
 use League\Flysystem\UnableToReadFile;
 use League\Flysystem\UnableToRetrieveMetadata;
+use League\Flysystem\UnableToSetVisibility;
 use Psr\Cache\CacheItemPoolInterface;
 use RuntimeException;
 
@@ -100,18 +104,15 @@ public function writeStream(string $path, $contents, Config $config): void
      */
     public function read(string $path): string
     {
-        $item = $this->getCacheItem($path);
-
         try {
             $contents = $this->adapter->read($path);
         } catch (UnableToReadFile $e) {
-            if ($item->isHit()) {
-                $this->deleteCacheItem($item);
-            }
-
+            $this->purgeCacheItem($path);
             throw $e;
         }
 
+        $item = $this->getCacheItem($path);
+
         if (!$item->isHit()) {
             $fileAttributes = new FileAttributes(
                 path: $path,
@@ -130,18 +131,15 @@ public function read(string $path): string
      */
     public function readStream(string $path)
     {
-        $item = $this->getCacheItem($path);
-
         try {
             $resource = $this->adapter->readStream($path);
         } catch (UnableToReadFile $e) {
-            if ($item->isHit()) {
-                $this->deleteCacheItem($item);
-            }
-
+            $this->purgeCacheItem($path);
             throw $e;
         }
 
+        $item = $this->getCacheItem($path);
+
         if (!$item->isHit()) {
             $fileAttributes = new FileAttributes(
                 path: $path,
@@ -160,12 +158,10 @@ public function readStream(string $path)
      */
     public function delete(string $path): void
     {
-        $this->adapter->delete($path);
-
-        $item = $this->getCacheItem($path);
-
-        if ($item->isHit()) {
-            $this->deleteCacheItem($item);
+        try {
+            $this->adapter->delete($path);
+        } finally {
+            $this->purgeCacheItem($path);
         }
     }
 
@@ -174,21 +170,15 @@ public function delete(string $path): void
      */
     public function deleteDirectory(string $path): void
     {
-        foreach ($this->adapter->listContents($path, true) as $storageAttributes) {
-            /** @var StorageAttributes $storageAttributes */
-            $item = $this->getCacheItem($storageAttributes->path());
-
-            if ($item->isHit()) {
-                $this->deleteCacheItem($item);
+        try {
+            foreach ($this->adapter->listContents($path, true) as $storageAttributes) {
+                /** @var StorageAttributes $storageAttributes */
+                $this->purgeCacheItem($storageAttributes->path());
             }
-        }
 
-        $this->adapter->deleteDirectory($path);
-
-        $item = $this->getCacheItem($path);
-
-        if ($item->isHit()) {
-            $this->deleteCacheItem($item);
+            $this->adapter->deleteDirectory($path);
+        } finally {
+            $this->purgeCacheItem($path);
         }
     }
 
@@ -207,7 +197,12 @@ public function createDirectory(string $path, Config $config): void
      */
     public function setVisibility(string $path, string $visibility): void
     {
-        $this->adapter->setVisibility($path, $visibility);
+        try {
+            $this->adapter->setVisibility($path, $visibility);
+        } catch (UnableToSetVisibility $e) {
+            $this->purgeCacheItem($path);
+            throw $e;
+        }
 
         $item = $this->getCacheItem($path);
 
@@ -416,7 +411,13 @@ public function listContents(string $path, bool $deep): iterable
      */
     public function move(string $source, string $destination, Config $config): void
     {
-        $this->adapter->move($source, $destination, $config);
+        try {
+            $this->adapter->move($source, $destination, $config);
+        } catch (UnableToMoveFile $e) {
+            $this->purgeCacheItem($source);
+            $this->purgeCacheItem($destination);
+            throw $e;
+        }
 
         $itemSource = $this->getCacheItem($source);
         $itemDestination = $this->getCacheItem($destination);
@@ -442,7 +443,13 @@ public function move(string $source, string $destination, Config $config): void
      */
     public function copy(string $source, string $destination, Config $config): void
     {
-        $this->adapter->copy($source, $destination, $config);
+        try {
+            $this->adapter->copy($source, $destination, $config);
+        } catch (UnableToCopyFile $e) {
+            $this->purgeCacheItem($source);
+            $this->purgeCacheItem($destination);
+            throw $e;
+        }
 
         $itemSource = $this->getCacheItem($source);
         $itemDestination = $this->getCacheItem($destination);

diff --git a/src/CacheItemsTrait.php b/src/CacheItemsTrait.php
@@ -9,6 +9,7 @@
 use Psr\Cache\CacheItemInterface;
 use Psr\Cache\CacheItemPoolInterface;
 use League\Flysystem\FilesystemAdapter;
+use League\Flysystem\UnableToRetrieveMetadata;
 use RuntimeException;
 
 /**
@@ -39,6 +40,14 @@ protected function deleteCacheItem(CacheItemInterface $cacheItem): void
         $this->cache->deleteItem($cacheItem->getKey());
     }
 
+    protected function purgeCacheItem(string $path): void
+    {
+        $item = $this->getCacheItem($path);
+        if ($item->isHit()) {
+            $this->deleteCacheItem($item);
+        }
+    }
+
     public static function getCacheItemKey(string $path): string
     {
         return self::$CACHE_KEY_PREFIX . md5(self::$CACHE_KEY_HASH_SALT . $path);

diff --git a/tests/Copy_Test.php b/tests/Copy_Test.php
@@ -4,6 +4,8 @@
 
 use League\Flysystem\Config;
 use League\Flysystem\FileAttributes;
+use League\Flysystem\UnableToCopyFile;
+use League\Flysystem\UnableToReadFile;
 use League\Flysystem\Visibility;
 
 class Copy_Test extends CacheTestCase
@@ -33,4 +35,21 @@ public static function dataProvider(): iterable
         yield 'cache item is copied' => ['fully-cached-file', new FileAttributes('fully-cached-file', 10, Visibility::PUBLIC), new FileAttributes('destination', 10, Visibility::PUBLIC)];
         yield 'cache item is created' => ['non-cached-file', \null, new FileAttributes('destination')];
     }
+
+    /** 
+     * @test
+     */
+    public function cache_is_purged_after_unsuccessful_copy(): void
+    {
+        $path = 'deleted-cached-file';
+
+        try {
+            $this->cacheAdapter->copy($path, 'destination', new Config);
+        } catch (UnableToCopyFile $e) {
+        }
+
+        $this->assertCachedItems([
+            $path => \null,
+        ]);
+    }
 }
diff --git a/tests/DeleteDirectory_Test.php b/tests/DeleteDirectory_Test.php
@@ -2,6 +2,8 @@
 
 namespace tests\jgivoni\Flysystem\Cache;
 
+use League\Flysystem\UnableToDeleteDirectory;
+
 class DeleteDirectory_Test extends CacheTestCase
 {
     /** 
@@ -38,4 +40,21 @@ public function nested_files_are_purged_from_cache(): void
             'cached-directory/file' => \null,
         ]);
     }
+
+    /** 
+     * @test
+     */
+    public function cache_is_purged_after_unsuccessful_delete(): void
+    {
+        $path = 'deleted-cached-directory';
+
+        try {
+            $this->cacheAdapter->deleteDirectory($path);
+        } catch (UnableToDeleteDirectory $e) {
+        }
+
+        $this->assertCachedItems([
+            $path => \null,
+        ]);
+    }
 }
diff --git a/tests/Delete_Test.php b/tests/Delete_Test.php
@@ -2,6 +2,8 @@
 
 namespace tests\jgivoni\Flysystem\Cache;
 
+use League\Flysystem\UnableToDeleteFile;
+
 class Delete_Test extends CacheTestCase
 {
     /** 
@@ -26,4 +28,21 @@ public static function dataProvider(): iterable
         yield 'cache is purged after deleting' => ['fully-cached-file'];
         yield 'non cached file stays uncached' => ['non-cached-file'];
     }
+
+    /** 
+     * @test
+     */
+    public function cache_is_purged_after_unsuccessful_delete(): void
+    {
+        $path = 'deleted-cached-file';
+
+        try {
+            $this->cacheAdapter->delete($path);
+        } catch (UnableToDeleteFile $e) {
+        }
+
+        $this->assertCachedItems([
+            $path => \null,
+        ]);
+    }
 }
diff --git a/tests/Move_Test.php b/tests/Move_Test.php
@@ -4,6 +4,7 @@
 
 use League\Flysystem\Config;
 use League\Flysystem\FileAttributes;
+use League\Flysystem\UnableToMoveFile;
 use League\Flysystem\Visibility;
 
 class Move_Test extends CacheTestCase
@@ -33,4 +34,21 @@ public static function dataProvider(): iterable
         yield 'cache item is moved' => ['fully-cached-file', new FileAttributes('destination', 10, Visibility::PUBLIC)];
         yield 'cache item is created' => ['non-cached-file', new FileAttributes('destination')];
     }
+
+    /** 
+     * @test
+     */
+    public function cache_is_purged_after_unsuccessful_move(): void
+    {
+        $path = 'deleted-cached-file';
+
+        try {
+            $this->cacheAdapter->move($path, 'destination', new Config);
+        } catch (UnableToMoveFile $e) {
+        }
+
+        $this->assertCachedItems([
+            $path => \null,
+        ]);
+    }
 }
diff --git a/tests/Read_Test.php b/tests/Read_Test.php
@@ -52,13 +52,13 @@ public function cache_is_purged_after_unsuccessful_read(): void
     {
         $path = 'deleted-cached-file';
 
-        $this->expectException(UnableToReadFile::class);
-
-        $this->cacheAdapter->read($path);
-
-        $this->assertCachedItems([
-            $path => \null,
-        ]);
+        try {
+            $this->cacheAdapter->read($path);
+        } catch (UnableToReadFile $e) {
+            $this->assertCachedItems([
+                $path => \null,
+            ]);
+        }
     }
 
     /** 
@@ -68,9 +68,10 @@ public function cache_is_purged_after_unsuccessful_readStream(): void
     {
         $path = 'deleted-cached-file';
 
-        $this->expectException(UnableToReadFile::class);
-
-        $this->cacheAdapter->readStream($path);
+        try {
+            $this->cacheAdapter->readStream($path);
+        } catch (UnableToReadFile $e) {
+        }
 
         $this->assertCachedItems([
             $path => \null,

diff --git a/tests/SetVisibility_Test.php b/tests/SetVisibility_Test.php
@@ -4,6 +4,7 @@
 
 use League\Flysystem\FileAttributes;
 use League\Flysystem\StorageAttributes;
+use League\Flysystem\UnableToSetVisibility;
 use League\Flysystem\Visibility;
 
 class SetVisibility_Test extends CacheTestCase
@@ -34,4 +35,21 @@ public static function dataProvider(): iterable
         // yield 'cached directory' => ['cached-directory', Visibility::PUBLIC, new DirectoryAttributes('fully-cached-file', visibility: Visibility::PUBLIC)];
         // yield 'cached directory, set private' => ['cached-directory', Visibility::PRIVATE, new DirectoryAttributes('fully-cached-file', visibility: Visibility::PRIVATE)];
     }
+
+    /** 
+     * @test
+     */
+    public function cache_is_purged_after_unsuccessful_set(): void
+    {
+        $path = 'deleted-cached-file';
+
+        try {
+            $this->cacheAdapter->setVisibility($path, Visibility::PRIVATE);
+        } catch (UnableToSetVisibility $e) {
+        }
+
+        $this->assertCachedItems([
+            $path => \null,
+        ]);
+    }
 }