diff --git a/README.md b/README.md index 71b126a..ec95944 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ of truth. Definitions are memoized within the request and cached through `WANObjectCache`, including process caching, hot refresh, stampede protection, and a cross-server check key. Saves replace the complete definition set in one -database transaction and invalidate both the check key and cached value. +database transaction and invalidate the check key. Primary-database reads are used when regenerating the cache because replica lag must not temporarily change title interpretation. diff --git a/extension.json b/extension.json index 1ff4ecb..6d55508 100644 --- a/extension.json +++ b/extension.json @@ -1,6 +1,6 @@ { "name": "NamespaceManager", - "version": "2.0.0", + "version": "2.0.1", "author": [ "[https://github.com/mywikis MyWikis LLC]" ], diff --git a/src/NamespaceRepository.php b/src/NamespaceRepository.php index df1249c..8224194 100644 --- a/src/NamespaceRepository.php +++ b/src/NamespaceRepository.php @@ -113,7 +113,6 @@ public function isEmpty(): bool { public function invalidate(): void { $this->cache->touchCheckKey( $this->getCheckKey() ); - $this->cache->delete( $this->getCacheKey() ); $this->loaded = false; $this->memoizedDefinitions = []; } diff --git a/tests/phpunit/integration/NamespaceRepositoryTest.php b/tests/phpunit/integration/NamespaceRepositoryTest.php index af3ab95..042f22f 100644 --- a/tests/phpunit/integration/NamespaceRepositoryTest.php +++ b/tests/phpunit/integration/NamespaceRepositoryTest.php @@ -4,6 +4,10 @@ use MediaWiki\Extension\NamespaceManager\NamespaceRepository; use MediaWikiIntegrationTestCase; +use Psr\Log\NullLogger; +use Wikimedia\ObjectCache\HashBagOStuff; +use Wikimedia\ObjectCache\WANObjectCache; +use Wikimedia\Rdbms\IConnectionProvider; use Wikimedia\Rdbms\IDatabase; /** @@ -66,6 +70,105 @@ public function testReplaceWithEmptyListDeletesAllRows(): void { $this->assertTrue( $this->repository->isEmpty() ); } + public function testInvalidationRejectsConcurrentStaleCacheFill(): void { + $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] ); + $mockTime = 1_700_000_000.0; + $cache->setMockTime( $mockTime ); + $repository = $this->newCacheOnlyRepository( $cache ); + $key = $cache->makeKey( 'namespacemanager', 'definitions' ); + $checkKey = $cache->makeKey( 'namespacemanager', 'definitions', 'check' ); + $options = $this->getCacheOptions( $checkKey ); + + $stale = $cache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static function () use ( $repository ): string { + $repository->invalidate(); + return 'stale'; + }, + $options + ); + $cache->setMockTime( $mockTime + 20 ); + $regenerations = 0; + $fresh = $cache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static function () use ( &$regenerations ): string { + $regenerations++; + return 'fresh'; + }, + $options + ); + + $this->assertSame( 'stale', $stale ); + $this->assertSame( 'fresh', $fresh ); + $this->assertSame( 1, $regenerations ); + } + + public function testInvalidationDoesNotPurgeConcurrentFreshCacheFill(): void { + $cacheBag = new CallbackHashBagOStuff(); + $writerCache = new WANObjectCache( [ 'cache' => $cacheBag ] ); + $readerCache = new WANObjectCache( [ 'cache' => $cacheBag ] ); + $repository = $this->newCacheOnlyRepository( $writerCache ); + $key = $readerCache->makeKey( 'namespacemanager', 'definitions' ); + $checkKey = $readerCache->makeKey( 'namespacemanager', 'definitions', 'check' ); + $options = $this->getCacheOptions( $checkKey ); + $regenerations = 0; + + $cacheBag->afterNextSet( + static function () use ( + $readerCache, + $key, + $options, + &$regenerations + ): void { + $readerCache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static function () use ( &$regenerations ): string { + $regenerations++; + return 'fresh'; + }, + $options + ); + } + ); + + $repository->invalidate(); + $value = $readerCache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static function () use ( &$regenerations ): string { + $regenerations++; + return 'unexpected'; + }, + $options + ); + + $this->assertSame( 'fresh', $value ); + $this->assertSame( 1, $regenerations ); + } + + /** + * @return array + */ + private function getCacheOptions( string $checkKey ): array { + return [ + 'checkKeys' => [ $checkKey ], + 'hotTTR' => WANObjectCache::TTL_HOUR, + 'lockTSE' => 30, + 'version' => 1, + ]; + } + + private function newCacheOnlyRepository( WANObjectCache $cache ): NamespaceRepository { + return new NamespaceRepository( + $this->createMock( IConnectionProvider::class ), + $cache, + new NullLogger() + ); + } + /** * @return array> */ @@ -90,3 +193,30 @@ private function getDefinitions(): array { ]; } } + +class CallbackHashBagOStuff extends HashBagOStuff { + + /** @var callable|null */ + private $afterNextSet; + + public function afterNextSet( callable $callback ): void { + $this->afterNextSet = $callback; + } + + /** + * @param string $key + * @param mixed $value + * @param int $exptime + * @param int $flags + * @return bool + */ + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { + $result = parent::doSet( $key, $value, $exptime, $flags ); + if ( $this->afterNextSet !== null ) { + $callback = $this->afterNextSet; + $this->afterNextSet = null; + $callback(); + } + return $result; + } +}