From ced317e6630292f179f57c6756afc2343d33460d Mon Sep 17 00:00:00 2001 From: Jakob Givoni Date: Thu, 18 Jan 2024 10:55:43 +0100 Subject: [PATCH] 11-typeerror-assuming-fileattributes-but-could-be-directory (#12) --- src/CacheAdapter.php | 129 +++++++++++++++++++-------------- src/CacheItemsTrait.php | 5 ++ tests/GetChecksum_Test.php | 23 ++++++ tests/GetFileSize_Test.php | 24 ++++++ tests/GetLastModified_Test.php | 24 ++++++ tests/GetMimetype_Test.php | 24 ++++++ tests/GetVisibility_Test.php | 23 ++++++ 7 files changed, 199 insertions(+), 53 deletions(-) diff --git a/src/CacheAdapter.php b/src/CacheAdapter.php index d5b4dbd..26149e4 100644 --- a/src/CacheAdapter.php +++ b/src/CacheAdapter.php @@ -10,8 +10,11 @@ use League\Flysystem\FileAttributes; use League\Flysystem\FilesystemAdapter; use League\Flysystem\StorageAttributes; +use League\Flysystem\UnableToProvideChecksum; use League\Flysystem\UnableToReadFile; +use League\Flysystem\UnableToRetrieveMetadata; use Psr\Cache\CacheItemPoolInterface; +use RuntimeException; class CacheAdapter implements FilesystemAdapter, ChecksumProvider { @@ -245,15 +248,19 @@ public function setVisibility(string $path, string $visibility): void */ public function visibility(string $path): FileAttributes { - return $this->getFileAttributes( - path: $path, - loader: function () use ($path) { - return $this->adapter->visibility($path); - }, - attributeAccessor: function (FileAttributes $fileAttributes) { - return $fileAttributes->visibility(); - }, - ); + try { + return $this->getFileAttributes( + path: $path, + loader: function () use ($path) { + return $this->adapter->visibility($path); + }, + attributeAccessor: function (FileAttributes $fileAttributes) { + return $fileAttributes->visibility(); + }, + ); + } catch (RuntimeException $e) { + throw UnableToRetrieveMetadata::visibility($path, $e->getMessage(), $e); + } } /** @@ -261,15 +268,19 @@ public function visibility(string $path): FileAttributes */ public function mimeType(string $path): FileAttributes { - return $this->getFileAttributes( - path: $path, - loader: function () use ($path) { - return $this->adapter->mimeType($path); - }, - attributeAccessor: function (FileAttributes $fileAttributes) { - return $fileAttributes->mimeType(); - }, - ); + try { + return $this->getFileAttributes( + path: $path, + loader: function () use ($path) { + return $this->adapter->mimeType($path); + }, + attributeAccessor: function (FileAttributes $fileAttributes) { + return $fileAttributes->mimeType(); + }, + ); + } catch (RuntimeException $e) { + throw UnableToRetrieveMetadata::mimeType($path, $e->getMessage(), $e); + } } /** @@ -277,15 +288,19 @@ public function mimeType(string $path): FileAttributes */ public function lastModified(string $path): FileAttributes { - return $this->getFileAttributes( - path: $path, - loader: function () use ($path) { - return $this->adapter->lastModified($path); - }, - attributeAccessor: function (FileAttributes $fileAttributes) { - return $fileAttributes->lastModified(); - }, - ); + try { + return $this->getFileAttributes( + path: $path, + loader: function () use ($path) { + return $this->adapter->lastModified($path); + }, + attributeAccessor: function (FileAttributes $fileAttributes) { + return $fileAttributes->lastModified(); + }, + ); + } catch (RuntimeException $e) { + throw UnableToRetrieveMetadata::lastModified($path, $e->getMessage(), $e); + } } /** @@ -293,15 +308,19 @@ public function lastModified(string $path): FileAttributes */ public function fileSize(string $path): FileAttributes { - return $this->getFileAttributes( - path: $path, - loader: function () use ($path) { - return $this->adapter->fileSize($path); - }, - attributeAccessor: function (FileAttributes $fileAttributes) { - return $fileAttributes->fileSize(); - }, - ); + try { + return $this->getFileAttributes( + path: $path, + loader: function () use ($path) { + return $this->adapter->fileSize($path); + }, + attributeAccessor: function (FileAttributes $fileAttributes) { + return $fileAttributes->fileSize(); + }, + ); + } catch (RuntimeException $e) { + throw UnableToRetrieveMetadata::fileSize($path, $e->getMessage(), $e); + } } /** @@ -324,25 +343,29 @@ public function checksum(string $path, Config $config): string 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 - // and be able to cache the result - try { - if (!$this->adapter instanceof ChecksumProvider) { - throw new ChecksumAlgoIsNotSupported; + try { + $fileAttributes = $this->getFileAttributes( + path: $path, + loader: function () use ($path, $config, $metadataKey) { + // 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) { + throw new ChecksumAlgoIsNotSupported; + } + + $checksum = $this->adapter->checksum($path, $config); + } catch (ChecksumAlgoIsNotSupported) { + $checksum = $this->calculateChecksumFromStream($path, $config); } - $checksum = $this->adapter->checksum($path, $config); - } catch (ChecksumAlgoIsNotSupported) { - $checksum = $this->calculateChecksumFromStream($path, $config); - } - - return new FileAttributes($path, extraMetadata: [$metadataKey => $checksum]); - }, - attributeAccessor: $attributeAccessor - ); + return new FileAttributes($path, extraMetadata: [$metadataKey => $checksum]); + }, + attributeAccessor: $attributeAccessor + ); + } catch (RuntimeException $e) { + throw new UnableToProvideChecksum($e->getMessage(), $path, $e); + } return $attributeAccessor($fileAttributes); } diff --git a/src/CacheItemsTrait.php b/src/CacheItemsTrait.php index b1fe768..8d4014e 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 RuntimeException; /** * Trait for handling cache items in CacheAdapter @@ -119,6 +120,10 @@ protected function getFileAttributes( if ($item->isHit()) { /** @var FileAttributes $fileAttributes */ $fileAttributes = $item->get(); + + if (!$fileAttributes instanceof FileAttributes) { + throw new RuntimeException('Cached item is not a file'); + } } else { $fileAttributes = new FileAttributes( path: $path, diff --git a/tests/GetChecksum_Test.php b/tests/GetChecksum_Test.php index feacfc0..2e20e94 100644 --- a/tests/GetChecksum_Test.php +++ b/tests/GetChecksum_Test.php @@ -6,6 +6,7 @@ use League\Flysystem\AwsS3V3\AwsS3V3Adapter; use League\Flysystem\Config; use League\Flysystem\FileAttributes; +use League\Flysystem\UnableToProvideChecksum; use Mockery; use Mockery\MockInterface; @@ -72,4 +73,26 @@ public static function aws_dataProvider(): iterable yield 'checksum is not cached but aws ETag is' => ['partially-cached-file-with-aws-etag', 'my-cached-aws-etag']; yield 'file is not cached' => ['non-cached-file', 'my-aws-etag']; } + + /** + * @test + * @dataProvider errorDataProvider + */ + public function error(string $path): void + { + $this->expectException(UnableToProvideChecksum::class); + + $this->cacheAdapter->checksum($path, new Config); + } + + /** + * + * @return iterable> + */ + public static function errorDataProvider(): iterable + { + yield 'File not found' => ['nonexistingfile']; + yield 'Path is directory (cached)' => ['cached-directory']; + yield 'Path is directory (non-cached)' => ['non-cached-directory']; + } } diff --git a/tests/GetFileSize_Test.php b/tests/GetFileSize_Test.php index 78be0e6..3651492 100644 --- a/tests/GetFileSize_Test.php +++ b/tests/GetFileSize_Test.php @@ -2,6 +2,8 @@ namespace tests\jgivoni\Flysystem\Cache; +use League\Flysystem\UnableToRetrieveMetadata; + class GetFileSize_Test extends CacheTestCase { /** @@ -26,4 +28,26 @@ public static function dataProvider(): iterable yield 'cached file returns last known file size' => ['deleted-cached-file', 10]; yield 'overwritten file still returns old file size' => ['overwritten-file', 20]; } + + /** + * @test + * @dataProvider errorDataProvider + */ + public function error(string $path): void + { + $this->expectException(UnableToRetrieveMetadata::class); + + $this->cacheAdapter->fileSize($path); + } + + /** + * + * @return iterable> + */ + public static function errorDataProvider(): iterable + { + yield 'File not found' => ['nonexistingfile']; + yield 'Path is directory (cached)' => ['cached-directory']; + yield 'Path is directory (non-cached)' => ['non-cached-directory']; + } } diff --git a/tests/GetLastModified_Test.php b/tests/GetLastModified_Test.php index 9855c81..cbdaf10 100644 --- a/tests/GetLastModified_Test.php +++ b/tests/GetLastModified_Test.php @@ -2,6 +2,8 @@ namespace tests\jgivoni\Flysystem\Cache; +use League\Flysystem\UnableToRetrieveMetadata; + class GetLastModified_Test extends CacheTestCase { /** @@ -23,4 +25,26 @@ public static function dataProvider(): iterable { yield 'partially cached file was updated' => ['partially-cached-file']; } + + /** + * @test + * @dataProvider errorDataProvider + */ + public function error(string $path): void + { + $this->expectException(UnableToRetrieveMetadata::class); + + $this->cacheAdapter->lastModified($path); + } + + /** + * + * @return iterable> + */ + public static function errorDataProvider(): iterable + { + yield 'File not found' => ['nonexistingfile']; + yield 'Path is directory (cached)' => ['cached-directory']; + yield 'Path is directory (non-cached)' => ['non-cached-directory']; + } } diff --git a/tests/GetMimetype_Test.php b/tests/GetMimetype_Test.php index 96fe53a..e200380 100644 --- a/tests/GetMimetype_Test.php +++ b/tests/GetMimetype_Test.php @@ -4,7 +4,9 @@ use League\Flysystem\FileAttributes; use League\Flysystem\StorageAttributes; +use League\Flysystem\UnableToRetrieveMetadata; use League\Flysystem\Visibility; +use Psr\Cache\InvalidArgumentException; class GetMimetype_Test extends CacheTestCase { @@ -42,4 +44,26 @@ public function file_is_cached_after_checking_filesystem(): void $path => new FileAttributes($path, mimeType: 'text/plain'), ]); } + + /** + * @test + * @dataProvider errorDataProvider + */ + public function error(string $path): void + { + $this->expectException(UnableToRetrieveMetadata::class); + + $this->cacheAdapter->mimeType($path); + } + + /** + * + * @return iterable> + */ + public static function errorDataProvider(): iterable + { + yield 'File not found' => ['nonexistingfile']; + yield 'Path is directory (cached)' => ['cached-directory']; + yield 'Path is directory (non-cached)' => ['non-cached-directory']; + } } diff --git a/tests/GetVisibility_Test.php b/tests/GetVisibility_Test.php index b064a49..cc09502 100644 --- a/tests/GetVisibility_Test.php +++ b/tests/GetVisibility_Test.php @@ -4,6 +4,7 @@ use League\Flysystem\FileAttributes; use League\Flysystem\StorageAttributes; +use League\Flysystem\UnableToRetrieveMetadata; use League\Flysystem\Visibility; class GetVisibility_Test extends CacheTestCase @@ -42,4 +43,26 @@ public function file_is_cached_after_checking_filesystem(): void $path => new FileAttributes($path, visibility: Visibility::PUBLIC), ]); } + + /** + * @test + * @dataProvider errorDataProvider + */ + public function error(string $path): void + { + $this->expectException(UnableToRetrieveMetadata::class); + + $this->cacheAdapter->visibility($path); + } + + /** + * + * @return iterable> + */ + public static function errorDataProvider(): iterable + { + yield 'File not found' => ['nonexistingfile']; + yield 'Path is directory (cached)' => ['cached-directory']; + yield 'Path is directory (non-cached)' => ['non-cached-directory']; + } }