From bff5c1617ea890b338cdb94e69a68c9f27bd060e Mon Sep 17 00:00:00 2001 From: tk Date: Thu, 20 Jul 2023 15:52:22 +0200 Subject: [PATCH] Refactoring AbstractCache.php Array Optimisation Add Compatibility with PHP8 & PSR 2/3 --- composer.json | 8 +-- phpunit.xml | 6 -- src/Storage/AbstractCache.php | 112 ++++++++++++++++++++-------------- src/Storage/Noop.php | 2 +- src/Storage/PhpRedis.php | 48 ++++----------- src/Storage/Psr6Cache.php | 44 ++++--------- tests/Psr6CacheTest.php | 3 +- 7 files changed, 96 insertions(+), 127 deletions(-) diff --git a/composer.json b/composer.json index df7fb7f..0a6f419 100644 --- a/composer.json +++ b/composer.json @@ -8,12 +8,12 @@ }, "require": { "league/flysystem": "~1.0", - "psr/cache": "^1.0.0" + "psr/cache": "^2.0|^3.0" }, "require-dev": { - "phpspec/phpspec": "^3.4", - "phpunit/phpunit": "^5.7", - "mockery/mockery": "~0.9", + "phpspec/phpspec": "^7.4.0", + "phpunit/phpunit": "^8.5", + "mockery/mockery": "^1.6", "predis/predis": "~1.0", "tedivm/stash": "~0.12" }, diff --git a/phpunit.xml b/phpunit.xml index 660dd3b..eca9d6e 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -8,7 +8,6 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false" - syntaxCheck="true" verbose="true" > @@ -21,9 +20,4 @@ ./src/ - - - - - diff --git a/src/Storage/AbstractCache.php b/src/Storage/AbstractCache.php index 141b468..090a72c 100644 --- a/src/Storage/AbstractCache.php +++ b/src/Storage/AbstractCache.php @@ -10,7 +10,7 @@ abstract class AbstractCache implements CacheInterface /** * @var bool */ - protected $autosave = true; + protected bool $autosave = true; /** * @var array @@ -22,9 +22,7 @@ abstract class AbstractCache implements CacheInterface */ protected $complete = []; - /** - * Destructor. - */ + public function __destruct() { if (! $this->autosave) { @@ -32,12 +30,7 @@ public function __destruct() } } - /** - * Get the autosave setting. - * - * @return bool autosave - */ - public function getAutosave() + public function getAutosave(): bool { return $this->autosave; } @@ -47,11 +40,12 @@ public function getAutosave() * * @param bool $autosave */ - public function setAutosave($autosave) + public function setAutosave(bool $autosave): void { $this->autosave = $autosave; } + /** * Store the contents listing. * @@ -63,24 +57,32 @@ public function setAutosave($autosave) */ public function storeContents($directory, array $contents, $recursive = false) { - $directories = [$directory]; + // Use associative array instead of indexed array + $directories = [$directory => true]; foreach ($contents as $object) { $this->updateObject($object['path'], $object); - $object = $this->cache[$object['path']]; - if ($recursive && $this->pathIsInDirectory($directory, $object['path'])) { - $directories[] = $object['dirname']; + // Assign it to a variable before the loop + $objectCachePath = $this->cache[$object['path']]; + + if ($recursive && $this->pathIsInDirectory($directory, $objectCachePath['path'])) { + // Check if the directory is already present using isset() + if (!isset($directories[$objectCachePath['dirname']])) { + $directories[$objectCachePath['dirname']] = true; + } } } - foreach (array_unique($directories) as $directory) { + // Since $directories is now an associative array, use array_keys() to get the keys + foreach (array_keys($directories) as $directory) { $this->setComplete($directory, $recursive); } $this->autosave(); } + /** * Update the metadata for an object. * @@ -91,11 +93,11 @@ public function storeContents($directory, array $contents, $recursive = false) public function updateObject($path, array $object, $autosave = false) { if (! $this->has($path)) { - $this->cache[$path] = Util::pathinfo($path); + $this->cache[$path] = Util::pathinfo($path) + $object; + } else { + $this->cache[$path] += $object; } - $this->cache[$path] = array_merge($this->cache[$path], $object); - if ($autosave) { $this->autosave(); } @@ -103,6 +105,7 @@ public function updateObject($path, array $object, $autosave = false) $this->ensureParentDirectories($path); } + /** * Store object hit miss. * @@ -130,9 +133,10 @@ public function listContents($dirname = '', $recursive = false) if ($object === false) { continue; } - if ($object['dirname'] === $dirname) { - $result[] = $object; - } elseif ($recursive && $this->pathIsInDirectory($dirname, $object['path'])) { + + $inDirectory = $object['dirname'] === $dirname || ($recursive && $this->pathIsInDirectory($dirname, $object['path'])); + + if ($inDirectory) { $result[] = $object; } } @@ -166,6 +170,8 @@ public function read($path) return false; } + + /** * {@inheritdoc} */ @@ -214,17 +220,22 @@ public function delete($path) */ public function deleteDir($dirname) { - foreach ($this->cache as $path => $object) { - if ($this->pathIsInDirectory($dirname, $path) || $path === $dirname) { - unset($this->cache[$path]); - } - } + // We filter out any elements of the cache that belong to the directory + // we are deleting, and assign the filtered array back to $this->cache. + $this->cache = array_filter( + $this->cache, + function($path) use ($dirname) { + return !($this->pathIsInDirectory($dirname, $path) || $path === $dirname); + }, + ARRAY_FILTER_USE_KEY + ); unset($this->complete[$dirname]); $this->autosave(); } + /** * {@inheritdoc} */ @@ -297,17 +308,11 @@ public function getMetadata($path) */ public function isComplete($dirname, $recursive) { - if (! array_key_exists($dirname, $this->complete)) { - return false; - } - - if ($recursive && $this->complete[$dirname] !== 'recursive') { - return false; - } - - return true; + return array_key_exists($dirname, $this->complete) + && (!$recursive || $this->complete[$dirname] === 'recursive'); } + /** * {@inheritdoc} */ @@ -325,21 +330,28 @@ public function setComplete($dirname, $recursive) */ public function cleanContents(array $contents) { - $cachedProperties = array_flip([ + // Defined properties array that needs to be cached + $properties = [ 'path', 'dirname', 'basename', 'extension', 'filename', 'size', 'mimetype', 'visibility', 'timestamp', 'type', 'md5', - ]); + ]; + + // Flipped the properties array once outside of the loop to avoid repeated computation + $cachedProperties = array_flip($properties); - foreach ($contents as $path => $object) { + // Using array_map instead of foreach for better readability and performance. + // array_map applies a callback to each element in the array and returns a new array with the results + return array_map(function($object) use ($cachedProperties) { if (is_array($object)) { - $contents[$path] = array_intersect_key($object, $cachedProperties); + // Intersect the keys of the object with the cached properties + return array_intersect_key($object, $cachedProperties); } - } - - return $contents; + return $object; + }, $contents); } + /** * {@inheritdoc} */ @@ -394,15 +406,20 @@ public function setFromStorage($json) */ public function ensureParentDirectories($path) { + // Getting the initial object from cache $object = $this->cache[$path]; + // We'll keep creating parent directories until the dirname is empty or the directory already exists while ($object['dirname'] !== '' && ! isset($this->cache[$object['dirname']])) { - $object = Util::pathinfo($object['dirname']); - $object['type'] = 'dir'; + $object = Util::pathinfo($object['dirname']); // updating the object to be the parent directory + $object['type'] = 'dir'; // it's a directory, so we'll set the type to 'dir' + + // The new object represents the parent directory, so we'll use the 'path' as the key $this->cache[$object['path']] = $object; } } + /** * Determines if the path is inside the directory. * @@ -411,8 +428,9 @@ public function ensureParentDirectories($path) * * @return bool */ - protected function pathIsInDirectory($directory, $path) + protected function pathIsInDirectory(string $directory, string $path): bool { - return $directory === '' || strpos($path, $directory . '/') === 0; + return $directory === '' || str_starts_with($path, $directory . '/'); } + } diff --git a/src/Storage/Noop.php b/src/Storage/Noop.php index ff996e0..fac77ed 100644 --- a/src/Storage/Noop.php +++ b/src/Storage/Noop.php @@ -7,7 +7,7 @@ class Noop extends AbstractCache /** * {@inheritdoc} */ - protected $autosave = false; + protected bool $autosave = false; /** * {@inheritdoc} diff --git a/src/Storage/PhpRedis.php b/src/Storage/PhpRedis.php index 4530b14..9f191de 100644 --- a/src/Storage/PhpRedis.php +++ b/src/Storage/PhpRedis.php @@ -1,44 +1,25 @@ client = $client ?: new Redis(); + $this->client = $client ?? new Redis(); $this->key = $key; - $this->expire = $expire; + $this->ttl = $ttl; } - /** - * {@inheritdoc} - */ - public function load() + public function load(): void { $contents = $this->client->get($this->key); @@ -47,16 +28,13 @@ public function load() } } - /** - * {@inheritdoc} - */ - public function save() + public function save(): void { $contents = $this->getForStorage(); $this->client->set($this->key, $contents); - if ($this->expire !== null) { - $this->client->expire($this->key, $this->expire); + if ($this->ttl !== null) { + $this->client->expire($this->key, $this->ttl); } } } diff --git a/src/Storage/Psr6Cache.php b/src/Storage/Psr6Cache.php index 43be87e..fb5addf 100644 --- a/src/Storage/Psr6Cache.php +++ b/src/Storage/Psr6Cache.php @@ -1,59 +1,37 @@ pool = $pool; $this->key = $key; - $this->expire = $expire; + $this->ttl = $ttl; } - /** - * {@inheritdoc} - */ - public function save() + public function save(): void { $item = $this->pool->getItem($this->key); $item->set($this->getForStorage()); - $item->expiresAfter($this->expire); + $item->expiresAfter($this->ttl); $this->pool->save($item); } - /** - * {@inheritdoc} - */ - public function load() + public function load(): void { $item = $this->pool->getItem($this->key); if ($item->isHit()) { $this->setFromStorage($item->get()); } } -} \ No newline at end of file +} diff --git a/tests/Psr6CacheTest.php b/tests/Psr6CacheTest.php index d5e5700..a207daf 100644 --- a/tests/Psr6CacheTest.php +++ b/tests/Psr6CacheTest.php @@ -36,10 +36,11 @@ public function testSave() $pool = Mockery::mock('Psr\Cache\CacheItemPoolInterface'); $item = Mockery::mock('Psr\Cache\CacheItemInterface'); $item->shouldReceive('expiresAfter')->once()->with($ttl); - $item->shouldReceive('set')->once()->andReturn($response); + $item->shouldReceive('set')->once()->andReturn($item); // Changed this line $pool->shouldReceive('getItem')->once()->andReturn($item); $pool->shouldReceive('save')->once()->with($item); $cache = new Psr6Cache($pool, 'foo', $ttl); $cache->save(); } + }