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

Remove array caching to prevent always returning miss from memory #3

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
8 changes: 4 additions & 4 deletions src/CacheAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -312,22 +312,22 @@ public function checksum(string $path, Config $config): string
$algo = $config->get('checksum_algo');
$metadataKey = isset($algo) ? 'checksum_' . $algo : 'checksum';

$attributeAccessor = function (FileAttributes $fileAttributes) use ($metadataKey) {
$attributeAccessor = function (StorageAttributes $storageAttributes) use ($metadataKey) {
if (\is_a($this->adapter, 'League\Flysystem\AwsS3V3\AwsS3V3Adapter')) {
// Special optimization for AWS S3, but won't break if adapter not installed
$etag = $fileAttributes->extraMetadata()['ETag'] ?? \null;
$etag = $storageAttributes->extraMetadata()['ETag'] ?? \null;
if (isset($etag)) {
$checksum = trim($etag, '" ');
}
}

return $checksum ?? $fileAttributes->extraMetadata()[$metadataKey] ?? \null;
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
// 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) {
Expand Down
33 changes: 6 additions & 27 deletions src/CacheItemsTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

/**
* Trait for handling cache items in CacheAdapter
*
*
* @property FilesystemAdapter $adapter
* @property CacheItemPoolInterface $cache
*/
Expand All @@ -21,42 +21,21 @@ trait CacheItemsTrait
const CACHE_KEY_PREFIX = 'flysystem_item_';
const CACHE_KEY_HASH_SALT = '563ce5132194441b';

/** @var array<CacheItemInterface> */
protected $cacheItems = [];

protected function getCacheItem(string $path): CacheItemInterface
{
if (!isset($this->cacheItems[$path])) {
$key = self::getCacheItemKey($path);

$this->cacheItems[$path] = $this->cache->getItem($key);
}
$key = self::getCacheItemKey($path);

return $this->cacheItems[$path];
return $this->cache->getItem($key);
}

protected function saveCacheItem(CacheItemInterface $cacheItem): void
{
$this->cache->save($cacheItem);

/** @var StorageAttributes $storageAttributes */
$storageAttributes = $cacheItem->get();

$path = $storageAttributes->path();

$this->cacheItems[$path] = $cacheItem;
}

protected function deleteCacheItem(CacheItemInterface $cacheItem): void
{
$this->cache->deleteItem($cacheItem->getKey());

/** @var StorageAttributes $storageAttributes */
$storageAttributes = $cacheItem->get();

$path = $storageAttributes->path();

unset($this->cacheItems[$path]);
}

public static function getCacheItemKey(string $path): string
Expand All @@ -76,7 +55,7 @@ protected function addCacheEntry(string $path, StorageAttributes $storageAttribu
/**
* Returns a new FileAttributes with all properties from $fileAttributesExtension
* overriding existing properties from $fileAttributesBase (with the exception of path)
*
*
* For extraMetadata, each individual element in the array is also merged
*/
protected static function mergeFileAttributes(
Expand All @@ -103,7 +82,7 @@ protected static function mergeFileAttributes(
/**
* Returns a new DirectoryAttributes with all properties from $directoryAttributesExtension
* overriding existing properties from $directoryAttributesBase (with the exception of path)
*
*
* For extraMetadata, each individual element in the array is also merged
*/
protected static function mergeDirectoryAttributes(
Expand All @@ -126,7 +105,7 @@ protected static function mergeDirectoryAttributes(
/**
* Returns FileAttributes from cache if desired attribute is found,
* or loads the desired missing attribute from the adapter and merges it with the cached attributes.
*
*
* @param Closure $loader Returns FileAttributes with the desired attribute loaded from adapter
* @param Closure $attributeAccessor Returns value of desired attribute from cached item
*/
Expand Down
52 changes: 52 additions & 0 deletions tests/CacheItemsTrait_Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

namespace tests\jgivoni\Flysystem\Cache;

use jgivoni\Flysystem\Cache\CacheItemsTrait;
use League\Flysystem\FileAttributes;
use PHPUnit\Framework\TestCase;
use Psr\Cache\CacheItemInterface;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\Adapter\ArrayAdapter;

class CacheItemsTrait_Test extends TestCase
{
/**
* @test
*/
public function return_hit_after_miss(): void
{
$adapter = new class {
use CacheItemsTrait;

public function __construct(
protected readonly CacheItemPoolInterface $cache = new ArrayAdapter()
) {
}

public function getItem(string $path): CacheItemInterface
{
return $this->getCacheItem($path);
}

public function saveItem(CacheItemInterface $item): void
{
$this->saveCacheItem($item);
}
};

$path = 'test.txt';
$item = $adapter->getItem($path);

self::assertFalse($item->isHit());

$item->set(new FileAttributes(
path: $path
));

$adapter->saveItem($item);

$freshItem = $adapter->getItem($path);
self::assertTrue($freshItem->isHit());
}
}
6 changes: 3 additions & 3 deletions tests/FileExists_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

class FileExists_Test extends CacheTestCase
{
/**
/**
* @test
* @dataProvider dataProvider
*/
Expand All @@ -18,7 +18,7 @@ public function file_exists_ok(string $path, bool $expectedResult): void
}

/**
*
*
* @return iterable<array<mixed>>
*/
public static function dataProvider(): iterable
Expand All @@ -30,7 +30,7 @@ public static function dataProvider(): iterable
yield 'directory is not a file' => ['cached-directory', false];
}

/**
/**
* @test
*/
public function file_is_cached_after_checking_filesystem(): void
Expand Down