Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 76 additions & 53 deletions src/CacheAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
}
}

/**
Expand All @@ -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);
}
Expand Down
5 changes: 5 additions & 0 deletions src/CacheItemsTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions tests/GetChecksum_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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'];
}
}
24 changes: 24 additions & 0 deletions tests/GetFileSize_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace tests\jgivoni\Flysystem\Cache;

use League\Flysystem\UnableToRetrieveMetadata;

class GetFileSize_Test extends CacheTestCase
{
/**
Expand All @@ -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'];
}
}
24 changes: 24 additions & 0 deletions tests/GetLastModified_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace tests\jgivoni\Flysystem\Cache;

use League\Flysystem\UnableToRetrieveMetadata;

class GetLastModified_Test extends CacheTestCase
{
/**
Expand All @@ -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'];
}
}
24 changes: 24 additions & 0 deletions tests/GetMimetype_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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'];
}
}
23 changes: 23 additions & 0 deletions tests/GetVisibility_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'];
}
}
Loading