From e202f651e2678f794b114a91a3952cce03dd1e4e Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 15 Apr 2020 22:05:02 +1200 Subject: [PATCH] NEW Add ReadOnlyCacheService to cache read-only calls to DBFile::exists(). Other small optimisation tweaks. --- src/Flysystem/FlysystemAssetStore.php | 2 +- src/Services/ReadOnlyCacheService.php | 70 +++++++++++++++++++ src/Storage/DBFile.php | 13 +++- src/Storage/Sha1FileHashingService.php | 17 ++++- .../php/Services/ReadOnlyCacheServiceTest.php | 40 +++++++++++ 5 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 src/Services/ReadOnlyCacheService.php create mode 100644 tests/php/Services/ReadOnlyCacheServiceTest.php diff --git a/src/Flysystem/FlysystemAssetStore.php b/src/Flysystem/FlysystemAssetStore.php index 082c22a7..bc8b006a 100644 --- a/src/Flysystem/FlysystemAssetStore.php +++ b/src/Flysystem/FlysystemAssetStore.php @@ -321,7 +321,7 @@ private function applyToFileOnFilesystem(callable $callable, ParsedFileID $parse if ($parsedFileID->getHash()) { $mainFileID = $strategy->buildFileID($strategy->stripVariant($parsedFileID)); - if (!$fs->has($mainFileID)) { + if ($mainFileID !== $fileID && !$fs->has($mainFileID)) { // The main file doesn't exists ... this is kind of weird. continue; } diff --git a/src/Services/ReadOnlyCacheService.php b/src/Services/ReadOnlyCacheService.php new file mode 100644 index 00000000..348d7d0b --- /dev/null +++ b/src/Services/ReadOnlyCacheService.php @@ -0,0 +1,70 @@ +enabled; + } + + public function setEnabled(bool $enabled): void + { + $this->enabled = $enabled; + } + + public function reset(?array $cacheNameComponents = null): void + { + if (is_null($cacheNameComponents)) { + $this->cache = []; + return; + } + $cacheName = $this->buildKey($cacheNameComponents); + if (isset($this->cache[$cacheName])) { + unset($this->cache[$cacheName]); + } + } + + public function setValue(array $cacheNameComponents, array $cacheKeyComponents, $value): void + { + $cacheName = $this->buildKey($cacheNameComponents); + $key = $this->buildKey($cacheKeyComponents); + if (!isset($this->cache[$cacheName])) { + $this->cache[$cacheName] = []; + } + $this->cache[$cacheName][$key] = $value; + } + + public function getValue(array $cacheNameComponents, array $cacheKeyComponents) + { + $cacheName = $this->buildKey($cacheNameComponents); + $key = $this->buildKey($cacheKeyComponents); + return $this->cache[$cacheName][$key] ?? null; + } + + public function hasValue(array $cacheNameComponents, array $cacheKeyComponents): bool + { + $cacheName = $this->buildKey($cacheNameComponents); + $key = $this->buildKey($cacheKeyComponents); + return isset($this->cache[$cacheName]); + } + + private function buildKey(array $components) + { + return implode('-', $components); + } +} diff --git a/src/Storage/DBFile.php b/src/Storage/DBFile.php index 07b34cdc..6788664d 100644 --- a/src/Storage/DBFile.php +++ b/src/Storage/DBFile.php @@ -4,6 +4,7 @@ use SilverStripe\Assets\File; use SilverStripe\Assets\ImageManipulation; +use SilverStripe\Assets\Services\ReadOnlyCacheService; use SilverStripe\Assets\Thumbnail; use SilverStripe\Control\Director; use SilverStripe\Core\Injector\Injector; @@ -359,12 +360,22 @@ public function getVisibility() public function exists() { + $cacheService = ReadOnlyCacheService::singleton(); + $cacheNameComponents = [__CLASS__, __FUNCTION__]; + $dbFileComponents = [$this->Filename, $this->Hash, $this->Variant]; + if ($cacheService->getEnabled() && $cacheService->hasValue($cacheNameComponents, $dbFileComponents)) { + return $cacheService->getValue($cacheNameComponents, $dbFileComponents); + } if (empty($this->Filename)) { return false; } - return $this + $exists = $this ->getStore() ->exists($this->Filename, $this->Hash, $this->Variant); + if ($cacheService->getEnabled()) { + $cacheService->setValue($cacheNameComponents, $dbFileComponents, $exists); + } + return $exists; } public function getFilename() diff --git a/src/Storage/Sha1FileHashingService.php b/src/Storage/Sha1FileHashingService.php index 48e6c14f..b0ce7140 100644 --- a/src/Storage/Sha1FileHashingService.php +++ b/src/Storage/Sha1FileHashingService.php @@ -5,6 +5,7 @@ use InvalidArgumentException; use League\Flysystem\Filesystem; use League\Flysystem\Util; +use League\Flysystem\FileNotFoundException; use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Flushable; @@ -189,10 +190,20 @@ private function buildCacheKey($fileID, $fs) */ private function getTimestamp($fileID, $fs) { + $timestamp = ''; $filesystem = $this->getFilesystem($fs); - return $filesystem->has($fileID) ? - $filesystem->getTimestamp($fileID) : - DBDatetime::now()->getTimestamp(); + // Using a try/catch block instead of Filesystem::has() because that's already + // called in Filesystem::assertPresent() + try { + // Filesystem::getTimestamp($path) will get a timestamp of the physical file + // if it fails for whatever reason, then it will either + // a) return false, or + // b) throw a FileNotFoundException from Filesystem::assertPresent() + $timestamp = $filesystem->getTimestamp($fileID); + } catch (FileNotFoundException $exception) { + // do nothing + } + return $timestamp ?: DBDatetime::now()->getTimestamp(); } public function invalidate($fileID, $fs) diff --git a/tests/php/Services/ReadOnlyCacheServiceTest.php b/tests/php/Services/ReadOnlyCacheServiceTest.php new file mode 100644 index 00000000..6cf31acc --- /dev/null +++ b/tests/php/Services/ReadOnlyCacheServiceTest.php @@ -0,0 +1,40 @@ +assertFalse($service->getEnabled()); + $service->setEnabled(true); + $this->assertTrue($service->getEnabled()); + } + + public function testGetSetHasValue() + { + $service = ReadOnlyCacheService::singleton(); + $this->assertFalse($service->hasValue(['A', 'B'], ['1', '2'])); + $service->setValue(['A', 'B'], ['1', '2'], 'xyz'); + $this->assertTrue($service->hasValue(['A', 'B'], ['1', '2'])); + $this->assertEquals('xyz', $service->getValue(['A', 'B'], ['1', '2'])); + } + + public function testReset() + { + $service = ReadOnlyCacheService::singleton(); + $service->setValue(['A', 'B'], ['1', '2'], 'xyz'); + $service->setValue(['C', 'D'], ['3', '4'], 'wvu'); + $this->assertTrue($service->hasValue(['A', 'B'], ['1', '2'])); + $service->reset(['A', 'B']); + $this->assertFalse($service->hasValue(['A', 'B'], ['1', '2'])); + $this->assertTrue($service->hasValue(['C', 'D'], ['3', '4'])); + $service->reset(); + $this->assertFalse($service->hasValue(['C', 'D'], ['3', '4'])); + } +}