From eb59f3b157552259c5eb3318eaa3f5c0ff585bc1 Mon Sep 17 00:00:00 2001 From: Jonathan Jaubart Date: Thu, 25 Feb 2021 16:43:08 +0000 Subject: [PATCH 1/2] Implement tag-based cache invalidation, and apply to Gedcom Records. --- app/Cache.php | 49 ++++-- app/Factories/AbstractGedcomRecordFactory.php | 2 +- app/Factories/CacheFactory.php | 11 +- app/Factories/FamilyFactory.php | 2 +- app/Factories/GedcomRecordFactory.php | 2 +- app/Factories/HeaderFactory.php | 2 +- app/Factories/ImageFactory.php | 2 +- app/Factories/IndividualFactory.php | 2 +- app/Factories/LocationFactory.php | 2 +- app/Factories/MediaFactory.php | 2 +- app/Factories/NoteFactory.php | 2 +- app/Factories/RepositoryFactory.php | 2 +- app/Factories/SourceFactory.php | 2 +- app/Factories/SubmissionFactory.php | 2 +- app/Factories/SubmitterFactory.php | 2 +- app/GedcomRecord.php | 20 ++- app/Http/Middleware/BadBotBlocker.php | 2 +- app/Http/RequestHandlers/SetupWizard.php | 3 +- app/Module/MediaTabModule.php | 29 ++-- app/Module/ModuleCustomTrait.php | 2 +- app/Module/SiteMapModule.php | 4 +- tests/app/CacheTest.php | 154 ++++++++++++++++++ tests/app/DefaultUserTest.php | 3 +- tests/app/Services/UserServiceTest.php | 3 +- tests/app/TreeTest.php | 3 +- tests/app/UserTest.php | 3 +- 26 files changed, 262 insertions(+), 50 deletions(-) create mode 100644 tests/app/CacheTest.php diff --git a/app/Cache.php b/app/Cache.php index 4ff92f884c2..76131c10f91 100644 --- a/app/Cache.php +++ b/app/Cache.php @@ -20,8 +20,8 @@ namespace Fisharebest\Webtrees; use Closure; -use Symfony\Contracts\Cache\CacheInterface; use Symfony\Contracts\Cache\ItemInterface; +use Symfony\Contracts\Cache\TagAwareCacheInterface; /** * Wrapper around the symfony PSR6 cache library. @@ -29,35 +29,64 @@ */ class Cache { - /** @var CacheInterface */ + /** @var TagAwareCacheInterface */ private $cache; /** * Cache constructor. * - * @param CacheInterface $cache + * @param TagAwareCacheInterface $cache */ - public function __construct(CacheInterface $cache) + public function __construct(TagAwareCacheInterface $cache) { $this->cache = $cache; } + /** + * Generate a key compatible with PSR-6 requirements + * @see https://www.php-fig.org/psr/psr-6/ + * + * @param string $key + * @return string + */ + public function safeKey(string $key): string + { + return md5($key); + } + /** * Fetch an item from the cache - or create it where it does not exist. * * @param string $key * @param Closure $closure + * @param string[] $tags * @param int|null $ttl * * @return mixed */ - public function remember(string $key, Closure $closure, int $ttl = null) + public function remember(string $key, Closure $closure, array $tags = [], int $ttl = null) { - return $this->cache->get(md5($key), static function (ItemInterface $item) use ($closure, $ttl) { - $item->expiresAfter($ttl); + $tags = array_map([$this, 'safeKey'], $tags); + return $this->cache->get( + $this->safeKey($key), + static function (ItemInterface $item) use ($closure, $tags, $ttl) { + $item->expiresAfter($ttl); + $item->tag($tags); - return $closure(); - }); + return $closure(); + } + ); + } + + /** + * Invalidate cache items based on tags. + * + * @param string[] $tags + * @return bool + */ + public function invalidateTags(array $tags): bool + { + return $this->cache->invalidateTags(array_map([$this, 'safeKey'], $tags)); } /** @@ -67,6 +96,6 @@ public function remember(string $key, Closure $closure, int $ttl = null) */ public function forget(string $key): void { - $this->cache->delete(md5($key)); + $this->cache->delete($this->safeKey($key)); } } diff --git a/app/Factories/AbstractGedcomRecordFactory.php b/app/Factories/AbstractGedcomRecordFactory.php index 43b2acdc2c3..9246bd1bd95 100644 --- a/app/Factories/AbstractGedcomRecordFactory.php +++ b/app/Factories/AbstractGedcomRecordFactory.php @@ -59,7 +59,7 @@ protected function pendingChanges(Tree $tree): Collection ->where('status', '=', 'pending') ->orderBy('change_id') ->pluck('new_gedcom', 'xref'); - }); + }, ['pending-t-' . $tree->id()]); } /** diff --git a/app/Factories/CacheFactory.php b/app/Factories/CacheFactory.php index e90c6492b72..858e71aed40 100644 --- a/app/Factories/CacheFactory.php +++ b/app/Factories/CacheFactory.php @@ -23,7 +23,8 @@ use Fisharebest\Webtrees\Contracts\CacheFactoryInterface; use Fisharebest\Webtrees\Webtrees; use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Adapter\FilesystemAdapter; +use Symfony\Component\Cache\Adapter\FilesystemTagAwareAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; use function random_int; @@ -39,10 +40,10 @@ class CacheFactory implements CacheFactoryInterface private const FILES_TTL = 8640000; private const FILES_DIR = Webtrees::DATA_DIR . 'cache/'; - /** @var ArrayAdapter */ + /** @var TagAwareAdapter */ private $array_adapter; - /** @var FilesystemAdapter */ + /** @var FilesystemTagAwareAdapter */ private $filesystem_adapter; /** @@ -50,8 +51,8 @@ class CacheFactory implements CacheFactoryInterface */ public function __construct() { - $this->array_adapter = new ArrayAdapter(0, false); - $this->filesystem_adapter = new FilesystemAdapter('', self::FILES_TTL, self::FILES_DIR); + $this->array_adapter = new TagAwareAdapter(new ArrayAdapter(0, false)); + $this->filesystem_adapter = new FilesystemTagAwareAdapter('', self::FILES_TTL, self::FILES_DIR); } /** diff --git a/app/Factories/FamilyFactory.php b/app/Factories/FamilyFactory.php index f5f064e00e1..1c3ab7cf47d 100644 --- a/app/Factories/FamilyFactory.php +++ b/app/Factories/FamilyFactory.php @@ -68,7 +68,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Family ->map(Registry::individualFactory()->mapper($tree)); return new Family($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/GedcomRecordFactory.php b/app/Factories/GedcomRecordFactory.php index 4a3fa85fc1b..2c751048748 100644 --- a/app/Factories/GedcomRecordFactory.php +++ b/app/Factories/GedcomRecordFactory.php @@ -108,7 +108,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?GedcomRe $type = $this->extractType($gedcom ?? $pending); return $this->newGedcomRecord($type, $xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/HeaderFactory.php b/app/Factories/HeaderFactory.php index 9e8484e9802..0622592d7de 100644 --- a/app/Factories/HeaderFactory.php +++ b/app/Factories/HeaderFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Header $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Header($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/ImageFactory.php b/app/Factories/ImageFactory.php index c8f5e82ec72..a59295819a1 100644 --- a/app/Factories/ImageFactory.php +++ b/app/Factories/ImageFactory.php @@ -239,7 +239,7 @@ public function mediaFileThumbnailResponse( // Images and Responses both contain resources - which cannot be serialized. // So cache the raw image data. - $data = Registry::cache()->file()->remember($key, $closure, static::THUMBNAIL_CACHE_TTL); + $data = Registry::cache()->file()->remember($key, $closure, [], static::THUMBNAIL_CACHE_TTL); return $this->imageResponse($data, $mime_type, ''); } catch (NotReadableException $ex) { diff --git a/app/Factories/IndividualFactory.php b/app/Factories/IndividualFactory.php index 1bbb58a8740..e08b90156f1 100644 --- a/app/Factories/IndividualFactory.php +++ b/app/Factories/IndividualFactory.php @@ -58,7 +58,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Individu $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Individual($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/LocationFactory.php b/app/Factories/LocationFactory.php index b3371b6c7e0..9c1e69cf777 100644 --- a/app/Factories/LocationFactory.php +++ b/app/Factories/LocationFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Location $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Location($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/MediaFactory.php b/app/Factories/MediaFactory.php index cca17a2f0be..e1cf520574a 100644 --- a/app/Factories/MediaFactory.php +++ b/app/Factories/MediaFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Media $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Media($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/NoteFactory.php b/app/Factories/NoteFactory.php index d63ce19457e..7c4c4df5419 100644 --- a/app/Factories/NoteFactory.php +++ b/app/Factories/NoteFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Note $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Note($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/RepositoryFactory.php b/app/Factories/RepositoryFactory.php index ab9e45c5b14..4c9000ba665 100644 --- a/app/Factories/RepositoryFactory.php +++ b/app/Factories/RepositoryFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Reposito $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Repository($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/SourceFactory.php b/app/Factories/SourceFactory.php index 5af95fe4ca3..3c14e0578de 100644 --- a/app/Factories/SourceFactory.php +++ b/app/Factories/SourceFactory.php @@ -60,7 +60,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Source $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Source($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/SubmissionFactory.php b/app/Factories/SubmissionFactory.php index ae3981405a1..de7e4f6c668 100644 --- a/app/Factories/SubmissionFactory.php +++ b/app/Factories/SubmissionFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Submissi $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Submission($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/SubmitterFactory.php b/app/Factories/SubmitterFactory.php index 0096b1f997c..f9d9177c48e 100644 --- a/app/Factories/SubmitterFactory.php +++ b/app/Factories/SubmitterFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Submitte $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Submitter($xref, $gedcom ?? '', $pending, $tree); - }); + }, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/GedcomRecord.php b/app/GedcomRecord.php index 5fcdc275fe5..d3598789fb8 100644 --- a/app/GedcomRecord.php +++ b/app/GedcomRecord.php @@ -315,7 +315,7 @@ public function canShow(int $access_level = null): bool return Registry::cache()->array()->remember($cache_key, function () use ($access_level) { return $this->canShowRecord($access_level); - }); + }, ['gedrec-' . $this->tree->id() . '-' . $this->xref]); } /** @@ -1005,6 +1005,7 @@ public function updateFact(string $fact_id, string $gedcom, bool $update_chan): ]); $this->pending = $new_gedcom; + $this->invalidateInCache(); if (Auth::user()->getPreference(UserInterface::PREF_AUTO_ACCEPT_EDITS) === '1') { app(PendingChangesService::class)->acceptRecord($this); @@ -1051,6 +1052,7 @@ public function updateRecord(string $gedcom, bool $update_chan): void // Clear the cache $this->pending = $gedcom; + $this->invalidateInCache(); // Accept this pending change if (Auth::user()->getPreference(UserInterface::PREF_AUTO_ACCEPT_EDITS) === '1') { @@ -1082,6 +1084,8 @@ public function deleteRecord(): void ]); } + $this->invalidateInCache(); + // Auto-accept this pending change if (Auth::user()->getPreference(UserInterface::PREF_AUTO_ACCEPT_EDITS) === '1') { app(PendingChangesService::class)->acceptRecord($this); @@ -1090,6 +1094,20 @@ public function deleteRecord(): void Log::addEditLog('Delete: ' . static::RECORD_TYPE . ' ' . $this->xref, $this->tree); } + /** + * Invalidate this record in the caches + * Only the array cache is invalidated + * + * @return bool + */ + public function invalidateInCache(): bool + { + return Registry::cache()->array()->invalidateTags([ + 'gedrec-' . $this->xref . '@' . $this->tree()->id() . '', + 'pending-t-' . $this->tree->id() + ]); + } + /** * Remove all links from this record to $xref * diff --git a/app/Http/Middleware/BadBotBlocker.php b/app/Http/Middleware/BadBotBlocker.php index eb70856ea30..cd98d533965 100644 --- a/app/Http/Middleware/BadBotBlocker.php +++ b/app/Http/Middleware/BadBotBlocker.php @@ -266,7 +266,7 @@ private function fetchIpRangesForAsn(string $asn): array } catch (Throwable $ex) { return []; } - }, random_int(self::WHOIS_TTL_MIN, self::WHOIS_TTL_MAX)); + }, [], random_int(self::WHOIS_TTL_MIN, self::WHOIS_TTL_MAX)); } /** diff --git a/app/Http/RequestHandlers/SetupWizard.php b/app/Http/RequestHandlers/SetupWizard.php index e202267f905..730c13e5b9c 100644 --- a/app/Http/RequestHandlers/SetupWizard.php +++ b/app/Http/RequestHandlers/SetupWizard.php @@ -42,6 +42,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Symfony\Component\Cache\Adapter\NullAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; use Throwable; use function app; @@ -140,7 +141,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $request = $request->withAttribute('client-ip', $ip_address); app()->instance(ServerRequestInterface::class, $request); - app()->instance('cache.array', new Cache(new NullAdapter())); + app()->instance('cache.array', new Cache(new TagAwareAdapter(new NullAdapter()))); $data = $this->userData($request); diff --git a/app/Module/MediaTabModule.php b/app/Module/MediaTabModule.php index 46ea22fc2fd..03e8c2ed0b7 100644 --- a/app/Module/MediaTabModule.php +++ b/app/Module/MediaTabModule.php @@ -132,21 +132,26 @@ public function getTabContent(Individual $individual): string */ protected function getFactsWithMedia(Individual $individual): Collection { - return Registry::cache()->array()->remember(__CLASS__ . ':' . __METHOD__, static function () use ($individual): Collection { - $facts = $individual->facts(); - - foreach ($individual->spouseFamilies() as $family) { - if ($family->canShow()) { - $facts = $facts->concat($family->facts()); + $cacheTag = $individual->xref() . '@' . $individual->tree()->id(); + return Registry::cache()->array()->remember( + __CLASS__ . ':' . __METHOD__ . '-' . $cacheTag, + static function () use ($individual): Collection { + $facts = $individual->facts(); + + foreach ($individual->spouseFamilies() as $family) { + if ($family->canShow()) { + $facts = $facts->concat($family->facts()); + } } - } - $facts = $facts->filter(static function (Fact $fact): bool { - return preg_match('/(?:^1|\n\d) OBJE @' . Gedcom::REGEX_XREF . '@/', $fact->gedcom()) === 1; - }); + $facts = $facts->filter(static function (Fact $fact): bool { + return preg_match('/(?:^1|\n\d) OBJE @' . Gedcom::REGEX_XREF . '@/', $fact->gedcom()) === 1; + }, []); - return Fact::sortFacts($facts); - }); + return Fact::sortFacts($facts); + }, + ['gedrec-' . $cacheTag] + ); } /** diff --git a/app/Module/ModuleCustomTrait.php b/app/Module/ModuleCustomTrait.php index 294e4fd06a8..19e1b83a8e8 100644 --- a/app/Module/ModuleCustomTrait.php +++ b/app/Module/ModuleCustomTrait.php @@ -101,7 +101,7 @@ public function customModuleLatestVersion(): string } return $this->customModuleVersion(); - }, 86400); + }, [], 86400); } /** diff --git a/app/Module/SiteMapModule.php b/app/Module/SiteMapModule.php index 3b76b5c332a..9cef15ef2aa 100644 --- a/app/Module/SiteMapModule.php +++ b/app/Module/SiteMapModule.php @@ -292,7 +292,7 @@ private function siteMapIndex(ServerRequestInterface $request): ResponseInterfac 'records_per_volume' => self::RECORDS_PER_VOLUME, 'sitemap_xsl' => route('sitemap-style'), ]); - }, self::CACHE_LIFE); + }, [], self::CACHE_LIFE); return response($content, StatusCodeInterface::STATUS_OK, [ 'Content-Type' => 'application/xml', @@ -327,7 +327,7 @@ private function siteMapFile(ServerRequestInterface $request): ResponseInterface 'sitemap_xsl' => route('sitemap-style'), 'tree' => $tree, ]); - }, self::CACHE_LIFE); + }, [], self::CACHE_LIFE); return response($content, StatusCodeInterface::STATUS_OK, [ 'Content-Type' => 'application/xml', diff --git a/tests/app/CacheTest.php b/tests/app/CacheTest.php new file mode 100644 index 00000000000..c8954883808 --- /dev/null +++ b/tests/app/CacheTest.php @@ -0,0 +1,154 @@ +. + */ + +declare(strict_types=1); + +namespace Fisharebest\Webtrees; + +use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; +use Symfony\Component\Cache\CacheItem; + +/** + * Class CacheTest. + */ +class CacheTest extends TestCase +{ + /** + * @var TagAwareAdapter $tagAwareAdapter + */ + private $tagAwareAdapter; + + /** + * @var Cache $cache + */ + private $cache; + + /** + * {@inheritDoc} + * @see \Fisharebest\Webtrees\TestCase::setUp() + */ + public function setUp(): void + { + parent::setUp(); + + $this->tagAwareAdapter = new TagAwareAdapter(new ArrayAdapter()); + $this->cache = new Cache($this->tagAwareAdapter); + } + + /** + * {@inheritDoc} + * @see \Fisharebest\Webtrees\TestCase::tearDown() + */ + public function tearDown(): void + { + parent::tearDown(); + + unset($this->tagAwareAdapter); + unset($this->cache); + } + + /** + * @covers \Fisharebest\Webtrees\Cache::safeKey + * @dataProvider keyProvider + * + * @param string $key + */ + public function testSafeKey(string $key): void + { + $safeKey = $this->cache->safeKey($key); + self::assertNotEmpty($safeKey); + self::assertSame($safeKey, CacheItem::validateKey($safeKey)); + } + + /** + * Data provider with example of keys + * + * @return string[][] + */ + public function keyProvider(): array + { + return [ ['test'], ['I1@3'], [str_repeat('a', 70)], [''] ]; + } + + /** + * @covers \Fisharebest\Webtrees\Cache::__construct + * @covers \Fisharebest\Webtrees\Cache::remember + * + * @return void + */ + public function testRemember(): void + { + self::assertEquals(10, $this->cache->remember('test', function () { + return 10; + })); + + self::assertTrue($this->tagAwareAdapter->hasItem($this->cache->safeKey('test'))); + self::assertEquals(10, $this->cache->remember('test', function () { + return 15; + })); + } + + /** + * @covers \Fisharebest\Webtrees\Cache::remember + * + * @return void + */ + public function testRememberWithTTL(): void + { + self::assertEquals(10, $this->cache->remember('test', function () { + return 10; + }, [], 1)); + self::assertTrue($this->tagAwareAdapter->hasItem($this->cache->safeKey('test'))); + + sleep(2); + self::assertFalse($this->tagAwareAdapter->hasItem($this->cache->safeKey('test'))); + } + + /** + * @covers \Fisharebest\Webtrees\Cache::invalidateTags + * + * @return void + */ + public function testInvalidateTags(): void + { + self::assertEquals(10, $this->cache->remember('test', function () { + return 10; + }, ['test-tag'])); + self::assertTrue($this->cache->invalidateTags(['test-tag'])); + self::assertEquals(15, $this->cache->remember('test', function () { + return 15; + })); + + self::assertTrue($this->cache->invalidateTags(['test-tag2'])); + } + + /** + * @covers \Fisharebest\Webtrees\Cache::forget + * + * @return void + */ + public function testForget(): void + { + self::assertEquals(10, $this->cache->remember('test', function () { + return 10; + })); + $this->cache->forget('test'); + + self::assertFalse($this->tagAwareAdapter->hasItem($this->cache->safeKey('test'))); + } +} diff --git a/tests/app/DefaultUserTest.php b/tests/app/DefaultUserTest.php index 02cc9071d81..708a0a688c3 100644 --- a/tests/app/DefaultUserTest.php +++ b/tests/app/DefaultUserTest.php @@ -22,6 +22,7 @@ use Fisharebest\Webtrees\Contracts\CacheFactoryInterface; use Fisharebest\Webtrees\Contracts\UserInterface; use Symfony\Component\Cache\Adapter\NullAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; /** * Test the DefaultUser class @@ -35,7 +36,7 @@ public function setUp(): void parent::setUp(); $cache_factory = self::createMock(CacheFactoryInterface::class); - $cache_factory->method('array')->willReturn(new Cache(new NullAdapter())); + $cache_factory->method('array')->willReturn(new Cache(new TagAwareAdapter(new NullAdapter()))); Registry::cache($cache_factory); } diff --git a/tests/app/Services/UserServiceTest.php b/tests/app/Services/UserServiceTest.php index b8317662d8f..b92f434674d 100644 --- a/tests/app/Services/UserServiceTest.php +++ b/tests/app/Services/UserServiceTest.php @@ -23,6 +23,7 @@ use Fisharebest\Webtrees\Contracts\UserInterface; use Fisharebest\Webtrees\Services\UserService; use Symfony\Component\Cache\Adapter\NullAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; /** * Test the UserService class @@ -36,7 +37,7 @@ public function setUp(): void parent::setUp(); $cache_factory = self::createMock(CacheFactoryInterface::class); - $cache_factory->method('array')->willReturn(new Cache(new NullAdapter())); + $cache_factory->method('array')->willReturn(new Cache(new TagAwareAdapter(new NullAdapter()))); Registry::cache($cache_factory); } diff --git a/tests/app/TreeTest.php b/tests/app/TreeTest.php index e03c27a8db6..272a72c6d3c 100644 --- a/tests/app/TreeTest.php +++ b/tests/app/TreeTest.php @@ -27,6 +27,7 @@ use Fisharebest\Webtrees\Services\UserService; use InvalidArgumentException; use Symfony\Component\Cache\Adapter\NullAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; use function stream_get_contents; @@ -42,7 +43,7 @@ public function setUp(): void parent::setUp(); $cache_factory = self::createMock(CacheFactoryInterface::class); - $cache_factory->method('array')->willReturn(new Cache(new NullAdapter())); + $cache_factory->method('array')->willReturn(new Cache(new TagAwareAdapter(new NullAdapter()))); Registry::cache($cache_factory); } diff --git a/tests/app/UserTest.php b/tests/app/UserTest.php index 51418651e4c..4bcd2d9bb3b 100644 --- a/tests/app/UserTest.php +++ b/tests/app/UserTest.php @@ -23,6 +23,7 @@ use Fisharebest\Webtrees\Contracts\UserInterface; use Fisharebest\Webtrees\Services\UserService; use Symfony\Component\Cache\Adapter\NullAdapter; +use Symfony\Component\Cache\Adapter\TagAwareAdapter; /** * Test the user functions @@ -36,7 +37,7 @@ public function setUp(): void parent::setUp(); $cache_factory = self::createMock(CacheFactoryInterface::class); - $cache_factory->method('array')->willReturn(new Cache(new NullAdapter())); + $cache_factory->method('array')->willReturn(new Cache(new TagAwareAdapter(new NullAdapter()))); Registry::cache($cache_factory); } From 210454254cd7d530cd9de96cd109772ea65e1c96 Mon Sep 17 00:00:00 2001 From: Jonathan Jaubart Date: Sat, 27 Feb 2021 13:42:01 +0000 Subject: [PATCH 2/2] Cache invalidation: Restore non-breaking signature on Cache remember method --- app/Cache.php | 4 ++-- app/Factories/AbstractGedcomRecordFactory.php | 2 +- app/Factories/FamilyFactory.php | 2 +- app/Factories/GedcomRecordFactory.php | 2 +- app/Factories/HeaderFactory.php | 2 +- app/Factories/ImageFactory.php | 2 +- app/Factories/IndividualFactory.php | 2 +- app/Factories/LocationFactory.php | 2 +- app/Factories/MediaFactory.php | 2 +- app/Factories/NoteFactory.php | 2 +- app/Factories/RepositoryFactory.php | 2 +- app/Factories/SourceFactory.php | 2 +- app/Factories/SubmissionFactory.php | 2 +- app/Factories/SubmitterFactory.php | 2 +- app/GedcomRecord.php | 2 +- app/Http/Middleware/BadBotBlocker.php | 2 +- app/Module/MediaTabModule.php | 3 ++- app/Module/ModuleCustomTrait.php | 2 +- app/Module/SiteMapModule.php | 4 ++-- tests/app/CacheTest.php | 4 ++-- 20 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/Cache.php b/app/Cache.php index 76131c10f91..6b2b041aa13 100644 --- a/app/Cache.php +++ b/app/Cache.php @@ -59,12 +59,12 @@ public function safeKey(string $key): string * * @param string $key * @param Closure $closure - * @param string[] $tags * @param int|null $ttl + * @param string[] $tags * * @return mixed */ - public function remember(string $key, Closure $closure, array $tags = [], int $ttl = null) + public function remember(string $key, Closure $closure, int $ttl = null, array $tags = []) { $tags = array_map([$this, 'safeKey'], $tags); return $this->cache->get( diff --git a/app/Factories/AbstractGedcomRecordFactory.php b/app/Factories/AbstractGedcomRecordFactory.php index 9246bd1bd95..171bc30255b 100644 --- a/app/Factories/AbstractGedcomRecordFactory.php +++ b/app/Factories/AbstractGedcomRecordFactory.php @@ -59,7 +59,7 @@ protected function pendingChanges(Tree $tree): Collection ->where('status', '=', 'pending') ->orderBy('change_id') ->pluck('new_gedcom', 'xref'); - }, ['pending-t-' . $tree->id()]); + }, null, ['pending-t-' . $tree->id()]); } /** diff --git a/app/Factories/FamilyFactory.php b/app/Factories/FamilyFactory.php index 1c3ab7cf47d..dff3d2fdba3 100644 --- a/app/Factories/FamilyFactory.php +++ b/app/Factories/FamilyFactory.php @@ -68,7 +68,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Family ->map(Registry::individualFactory()->mapper($tree)); return new Family($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/GedcomRecordFactory.php b/app/Factories/GedcomRecordFactory.php index 2c751048748..e33ae357072 100644 --- a/app/Factories/GedcomRecordFactory.php +++ b/app/Factories/GedcomRecordFactory.php @@ -108,7 +108,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?GedcomRe $type = $this->extractType($gedcom ?? $pending); return $this->newGedcomRecord($type, $xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/HeaderFactory.php b/app/Factories/HeaderFactory.php index 0622592d7de..4b70e2f5d38 100644 --- a/app/Factories/HeaderFactory.php +++ b/app/Factories/HeaderFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Header $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Header($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/ImageFactory.php b/app/Factories/ImageFactory.php index a59295819a1..c8f5e82ec72 100644 --- a/app/Factories/ImageFactory.php +++ b/app/Factories/ImageFactory.php @@ -239,7 +239,7 @@ public function mediaFileThumbnailResponse( // Images and Responses both contain resources - which cannot be serialized. // So cache the raw image data. - $data = Registry::cache()->file()->remember($key, $closure, [], static::THUMBNAIL_CACHE_TTL); + $data = Registry::cache()->file()->remember($key, $closure, static::THUMBNAIL_CACHE_TTL); return $this->imageResponse($data, $mime_type, ''); } catch (NotReadableException $ex) { diff --git a/app/Factories/IndividualFactory.php b/app/Factories/IndividualFactory.php index e08b90156f1..c36b38d7326 100644 --- a/app/Factories/IndividualFactory.php +++ b/app/Factories/IndividualFactory.php @@ -58,7 +58,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Individu $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Individual($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/LocationFactory.php b/app/Factories/LocationFactory.php index 9c1e69cf777..8df412dc71e 100644 --- a/app/Factories/LocationFactory.php +++ b/app/Factories/LocationFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Location $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Location($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/MediaFactory.php b/app/Factories/MediaFactory.php index e1cf520574a..d081c420300 100644 --- a/app/Factories/MediaFactory.php +++ b/app/Factories/MediaFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Media $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Media($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/NoteFactory.php b/app/Factories/NoteFactory.php index 7c4c4df5419..4a14b6bd489 100644 --- a/app/Factories/NoteFactory.php +++ b/app/Factories/NoteFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Note $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Note($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/RepositoryFactory.php b/app/Factories/RepositoryFactory.php index 4c9000ba665..d48198f19c5 100644 --- a/app/Factories/RepositoryFactory.php +++ b/app/Factories/RepositoryFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Reposito $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Repository($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/SourceFactory.php b/app/Factories/SourceFactory.php index 3c14e0578de..c89b7954f24 100644 --- a/app/Factories/SourceFactory.php +++ b/app/Factories/SourceFactory.php @@ -60,7 +60,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Source $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Source($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/SubmissionFactory.php b/app/Factories/SubmissionFactory.php index de7e4f6c668..16366ccaf45 100644 --- a/app/Factories/SubmissionFactory.php +++ b/app/Factories/SubmissionFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Submissi $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Submission($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/Factories/SubmitterFactory.php b/app/Factories/SubmitterFactory.php index f9d9177c48e..7d58960e2e7 100644 --- a/app/Factories/SubmitterFactory.php +++ b/app/Factories/SubmitterFactory.php @@ -59,7 +59,7 @@ public function make(string $xref, Tree $tree, string $gedcom = null): ?Submitte $xref = $this->extractXref($gedcom ?? $pending, $xref); return new Submitter($xref, $gedcom ?? '', $pending, $tree); - }, ['gedrec-' . $xref . '@' . $tree->id()]); + }, null, ['gedrec-' . $xref . '@' . $tree->id()]); } /** diff --git a/app/GedcomRecord.php b/app/GedcomRecord.php index d3598789fb8..9e66b1adbbe 100644 --- a/app/GedcomRecord.php +++ b/app/GedcomRecord.php @@ -315,7 +315,7 @@ public function canShow(int $access_level = null): bool return Registry::cache()->array()->remember($cache_key, function () use ($access_level) { return $this->canShowRecord($access_level); - }, ['gedrec-' . $this->tree->id() . '-' . $this->xref]); + }, null, ['gedrec-' . $this->tree->id() . '-' . $this->xref]); } /** diff --git a/app/Http/Middleware/BadBotBlocker.php b/app/Http/Middleware/BadBotBlocker.php index cd98d533965..eb70856ea30 100644 --- a/app/Http/Middleware/BadBotBlocker.php +++ b/app/Http/Middleware/BadBotBlocker.php @@ -266,7 +266,7 @@ private function fetchIpRangesForAsn(string $asn): array } catch (Throwable $ex) { return []; } - }, [], random_int(self::WHOIS_TTL_MIN, self::WHOIS_TTL_MAX)); + }, random_int(self::WHOIS_TTL_MIN, self::WHOIS_TTL_MAX)); } /** diff --git a/app/Module/MediaTabModule.php b/app/Module/MediaTabModule.php index 03e8c2ed0b7..b996e35639f 100644 --- a/app/Module/MediaTabModule.php +++ b/app/Module/MediaTabModule.php @@ -146,10 +146,11 @@ static function () use ($individual): Collection { $facts = $facts->filter(static function (Fact $fact): bool { return preg_match('/(?:^1|\n\d) OBJE @' . Gedcom::REGEX_XREF . '@/', $fact->gedcom()) === 1; - }, []); + }); return Fact::sortFacts($facts); }, + null, ['gedrec-' . $cacheTag] ); } diff --git a/app/Module/ModuleCustomTrait.php b/app/Module/ModuleCustomTrait.php index 19e1b83a8e8..294e4fd06a8 100644 --- a/app/Module/ModuleCustomTrait.php +++ b/app/Module/ModuleCustomTrait.php @@ -101,7 +101,7 @@ public function customModuleLatestVersion(): string } return $this->customModuleVersion(); - }, [], 86400); + }, 86400); } /** diff --git a/app/Module/SiteMapModule.php b/app/Module/SiteMapModule.php index 9cef15ef2aa..3b76b5c332a 100644 --- a/app/Module/SiteMapModule.php +++ b/app/Module/SiteMapModule.php @@ -292,7 +292,7 @@ private function siteMapIndex(ServerRequestInterface $request): ResponseInterfac 'records_per_volume' => self::RECORDS_PER_VOLUME, 'sitemap_xsl' => route('sitemap-style'), ]); - }, [], self::CACHE_LIFE); + }, self::CACHE_LIFE); return response($content, StatusCodeInterface::STATUS_OK, [ 'Content-Type' => 'application/xml', @@ -327,7 +327,7 @@ private function siteMapFile(ServerRequestInterface $request): ResponseInterface 'sitemap_xsl' => route('sitemap-style'), 'tree' => $tree, ]); - }, [], self::CACHE_LIFE); + }, self::CACHE_LIFE); return response($content, StatusCodeInterface::STATUS_OK, [ 'Content-Type' => 'application/xml', diff --git a/tests/app/CacheTest.php b/tests/app/CacheTest.php index c8954883808..db702293109 100644 --- a/tests/app/CacheTest.php +++ b/tests/app/CacheTest.php @@ -112,7 +112,7 @@ public function testRememberWithTTL(): void { self::assertEquals(10, $this->cache->remember('test', function () { return 10; - }, [], 1)); + }, 1, [])); self::assertTrue($this->tagAwareAdapter->hasItem($this->cache->safeKey('test'))); sleep(2); @@ -128,7 +128,7 @@ public function testInvalidateTags(): void { self::assertEquals(10, $this->cache->remember('test', function () { return 10; - }, ['test-tag'])); + }, null, ['test-tag'])); self::assertTrue($this->cache->invalidateTags(['test-tag'])); self::assertEquals(15, $this->cache->remember('test', function () { return 15;