From ff50d3ecc51812cf04e05901a30e4c8389c82927 Mon Sep 17 00:00:00 2001 From: Jakob Givoni Date: Tue, 30 Jan 2024 17:15:10 +0100 Subject: [PATCH] 14-purge-cache-on-unsuccessful-operations (#15) --- src/CacheAdapter.php | 75 +++++++++++++++++++--------------- src/CacheItemsTrait.php | 9 ++++ tests/Copy_Test.php | 19 +++++++++ tests/DeleteDirectory_Test.php | 19 +++++++++ tests/Delete_Test.php | 19 +++++++++ tests/Move_Test.php | 18 ++++++++ tests/Read_Test.php | 21 +++++----- tests/SetVisibility_Test.php | 18 ++++++++ 8 files changed, 154 insertions(+), 44 deletions(-) diff --git a/src/CacheAdapter.php b/src/CacheAdapter.php index 26149e4..e7af0a7 100644 --- 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 index 8d4014e..d2d45f8 100644 --- 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 index 2732703..75dcc78 100644 --- 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 index d1abfba..a3e4104 100644 --- 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 index 6903324..0fa41f8 100644 --- 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 index 28d9b64..220e713 100644 --- 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 index c81247c..9969f9d 100644 --- 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 index 1863a53..0c9e120 100644 --- 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, + ]); + } }