11-typeerror-assuming-fileattributes-but-could-be-directory (#12)

jgivoni · Jan 18, 2024 · ced317e · ced317e
1 parent 6eda71b
commit ced317e
Show file tree

Hide file tree

Showing 7 changed files with 199 additions and 53 deletions.
diff --git 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,63 +248,79 @@ 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);
+        }
     }
 
     /**
      * @inheritdoc
      */
     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);
+        }
     }
 
     /**
      * @inheritdoc
      */
     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);
+        }
     }
 
     /**
      * @inheritdoc
      */
     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
@@ -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
@@ -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<array<mixed>>
+     */
+    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
@@ -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<array<mixed>>
+     */
+    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
@@ -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<array<mixed>>
+     */
+    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
@@ -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<array<mixed>>
+     */
+    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
@@ -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<array<mixed>>
+     */
+    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'];
+    }
 }