From 4450dbd84a5fce4b9b8cc4eb3a48a10b52f2072d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:58:43 +0000 Subject: [PATCH 1/3] Fix WAN cache invalidation race --- README.md | 2 +- src/NamespaceRepository.php | 1 - .../integration/NamespaceRepositoryTest.php | 120 ++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) 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/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..9e97215 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,95 @@ public function testReplaceWithEmptyListDeletesAllRows(): void { $this->assertTrue( $this->repository->isEmpty() ); } + public function testInvalidationRejectsConcurrentStaleCacheFill(): void { + $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] ); + $repository = $this->newCacheOnlyRepository( $cache ); + $key = $cache->makeKey( 'namespacemanager', 'definitions' ); + $checkKey = $cache->makeKey( 'namespacemanager', 'definitions', 'check' ); + + $stale = $cache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static function () use ( $repository ): string { + $repository->invalidate(); + return 'stale'; + }, + [ 'checkKeys' => [ $checkKey ] ] + ); + $regenerations = 0; + $fresh = $cache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static function () use ( &$regenerations ): string { + $regenerations++; + return 'fresh'; + }, + [ 'checkKeys' => [ $checkKey ] ] + ); + + $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 = [ 'checkKeys' => [ $checkKey ] ]; + $regenerations = 0; + + $readerCache->getWithSetCallback( + $key, + WANObjectCache::TTL_WEEK, + static fn (): string => 'stale', + $options + ); + $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 ); + } + + private function newCacheOnlyRepository( WANObjectCache $cache ): NamespaceRepository { + return new NamespaceRepository( + $this->createMock( IConnectionProvider::class ), + $cache, + new NullLogger() + ); + } + /** * @return array> */ @@ -90,3 +183,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; + } +} From 1634540d2e21822f6b7535b795c11e7b568de2cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:14:35 +0000 Subject: [PATCH 2/3] Correct WAN cache concurrency tests --- .../integration/NamespaceRepositoryTest.php | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/phpunit/integration/NamespaceRepositoryTest.php b/tests/phpunit/integration/NamespaceRepositoryTest.php index 9e97215..042f22f 100644 --- a/tests/phpunit/integration/NamespaceRepositoryTest.php +++ b/tests/phpunit/integration/NamespaceRepositoryTest.php @@ -72,9 +72,12 @@ public function testReplaceWithEmptyListDeletesAllRows(): void { 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, @@ -83,8 +86,9 @@ static function () use ( $repository ): string { $repository->invalidate(); return 'stale'; }, - [ 'checkKeys' => [ $checkKey ] ] + $options ); + $cache->setMockTime( $mockTime + 20 ); $regenerations = 0; $fresh = $cache->getWithSetCallback( $key, @@ -93,7 +97,7 @@ static function () use ( &$regenerations ): string { $regenerations++; return 'fresh'; }, - [ 'checkKeys' => [ $checkKey ] ] + $options ); $this->assertSame( 'stale', $stale ); @@ -108,15 +112,9 @@ public function testInvalidationDoesNotPurgeConcurrentFreshCacheFill(): void { $repository = $this->newCacheOnlyRepository( $writerCache ); $key = $readerCache->makeKey( 'namespacemanager', 'definitions' ); $checkKey = $readerCache->makeKey( 'namespacemanager', 'definitions', 'check' ); - $options = [ 'checkKeys' => [ $checkKey ] ]; + $options = $this->getCacheOptions( $checkKey ); $regenerations = 0; - $readerCache->getWithSetCallback( - $key, - WANObjectCache::TTL_WEEK, - static fn (): string => 'stale', - $options - ); $cacheBag->afterNextSet( static function () use ( $readerCache, @@ -151,6 +149,18 @@ static function () use ( &$regenerations ): string { $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 ), From bfbfdb1c4e6e9a16bc244df4941a97cf9ecc2e8a Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Wed, 29 Jul 2026 11:09:01 -0700 Subject: [PATCH 3/3] Bump version from 2.0.0 to 2.0.1 --- extension.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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]" ],