From d3e4ba5c6fe7d4a9c7847c5d013811e0c7c44384 Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Tue, 14 Apr 2026 00:11:20 +0200 Subject: [PATCH 1/3] Fix PageDeleteComplete hook calling getTitle() on ProperPageIdentity Fixes https://github.com/ProfessionalWiki/SubPageList/issues/85 The PageDeleteComplete hook passes a ProperPageIdentity (concretely a PageStoreRecord) as its first parameter, but the code called $page->getTitle() which does not exist on that interface. This caused a fatal error when deleting pages. Use Title::newFromPageIdentity() instead, which correctly creates a Title from any PageIdentity. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/Setup.php | 2 +- tests/Unit/SubPageList/SetupTest.php | 31 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Setup.php b/src/Setup.php index c5ea59d..7a58f66 100644 --- a/src/Setup.php +++ b/src/Setup.php @@ -115,7 +115,7 @@ private function registerCacheInvalidator() { */ $this->hookContainer->register( 'PageDeleteComplete', function( $page ) use ( $extension ) { if ( $extension->getSettings()->get( Settings::AUTO_REFRESH ) ) { - $extension->getCacheInvalidator()->invalidateCaches( $page->getTitle() ); + $extension->getCacheInvalidator()->invalidateCaches( Title::newFromPageIdentity( $page ) ); } } ); diff --git a/tests/Unit/SubPageList/SetupTest.php b/tests/Unit/SubPageList/SetupTest.php index c984d04..1a56274 100644 --- a/tests/Unit/SubPageList/SetupTest.php +++ b/tests/Unit/SubPageList/SetupTest.php @@ -3,7 +3,12 @@ namespace Tests\Unit\SubPageList; use MediaWiki\MediaWikiServices; +use MediaWiki\Page\ProperPageIdentity; +use MediaWiki\Title\Title; use PHPUnit\Framework\TestCase; +use SubPageList\CacheInvalidator; +use SubPageList\Extension; +use SubPageList\Settings; use SubPageList\Setup; /** @@ -32,6 +37,32 @@ public function testRun() { $this->assertTrue( $hookContainer->isRegistered( 'ParserFirstCallInit' ) ); } + public function testDeletedPageInvalidatesCache() { + $cacheInvalidator = $this->createMock( CacheInvalidator::class ); + $cacheInvalidator->expects( $this->once() ) + ->method( 'invalidateCaches' ) + ->with( $this->callback( function ( Title $title ) { + return $title->getDBkey() === 'TestPage' && $title->getNamespace() === NS_MAIN; + } ) ); + + $extension = $this->createMock( Extension::class ); + $extension->method( 'getSettings' ) + ->willReturn( new Settings( [ Settings::AUTO_REFRESH => true ] ) ); + $extension->method( 'getCacheInvalidator' ) + ->willReturn( $cacheInvalidator ); + + $hookContainer = MediaWikiServices::getInstance()->getHookContainer(); + + $setup = new Setup( $extension, $hookContainer, __DIR__ . '/..' ); + $setup->run(); + + $page = $this->createMock( ProperPageIdentity::class ); + $page->method( 'getNamespace' )->willReturn( NS_MAIN ); + $page->method( 'getDBkey' )->willReturn( 'TestPage' ); + + $hookContainer->run( 'PageDeleteComplete', [ $page, null, null, 0, null, null, 0 ] ); + } + private function newExtension() { return new \SubPageList\Extension( \SubPageList\Settings::newFromGlobals( $GLOBALS ) ); } From 90e7088d7a24541e96e71d1cf65dfcdb376ca7de Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Tue, 14 Apr 2026 00:13:20 +0200 Subject: [PATCH 2/3] Use isolated HookContainer mock in test to avoid global state pollution Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/Unit/SubPageList/SetupTest.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/Unit/SubPageList/SetupTest.php b/tests/Unit/SubPageList/SetupTest.php index 1a56274..2d06c80 100644 --- a/tests/Unit/SubPageList/SetupTest.php +++ b/tests/Unit/SubPageList/SetupTest.php @@ -2,6 +2,7 @@ namespace Tests\Unit\SubPageList; +use MediaWiki\HookContainer\HookContainer; use MediaWiki\MediaWikiServices; use MediaWiki\Page\ProperPageIdentity; use MediaWiki\Title\Title; @@ -38,6 +39,14 @@ public function testRun() { } public function testDeletedPageInvalidatesCache() { + $registeredCallbacks = []; + + $hookContainer = $this->createMock( HookContainer::class ); + $hookContainer->method( 'register' ) + ->willReturnCallback( function ( $name, $callback ) use ( &$registeredCallbacks ) { + $registeredCallbacks[$name] = $callback; + } ); + $cacheInvalidator = $this->createMock( CacheInvalidator::class ); $cacheInvalidator->expects( $this->once() ) ->method( 'invalidateCaches' ) @@ -51,8 +60,6 @@ public function testDeletedPageInvalidatesCache() { $extension->method( 'getCacheInvalidator' ) ->willReturn( $cacheInvalidator ); - $hookContainer = MediaWikiServices::getInstance()->getHookContainer(); - $setup = new Setup( $extension, $hookContainer, __DIR__ . '/..' ); $setup->run(); @@ -60,7 +67,8 @@ public function testDeletedPageInvalidatesCache() { $page->method( 'getNamespace' )->willReturn( NS_MAIN ); $page->method( 'getDBkey' )->willReturn( 'TestPage' ); - $hookContainer->run( 'PageDeleteComplete', [ $page, null, null, 0, null, null, 0 ] ); + $this->assertArrayHasKey( 'PageDeleteComplete', $registeredCallbacks ); + $registeredCallbacks['PageDeleteComplete']( $page ); } private function newExtension() { From 3d7e4579de4d4defe2cf3b1cf3cb34d3944b5760 Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Tue, 14 Apr 2026 00:27:23 +0200 Subject: [PATCH 3/3] Replace mock expectation with spy and direct assertions Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/Unit/SubPageList/SetupTest.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/Unit/SubPageList/SetupTest.php b/tests/Unit/SubPageList/SetupTest.php index 2d06c80..198da57 100644 --- a/tests/Unit/SubPageList/SetupTest.php +++ b/tests/Unit/SubPageList/SetupTest.php @@ -47,12 +47,13 @@ public function testDeletedPageInvalidatesCache() { $registeredCallbacks[$name] = $callback; } ); - $cacheInvalidator = $this->createMock( CacheInvalidator::class ); - $cacheInvalidator->expects( $this->once() ) - ->method( 'invalidateCaches' ) - ->with( $this->callback( function ( Title $title ) { - return $title->getDBkey() === 'TestPage' && $title->getNamespace() === NS_MAIN; - } ) ); + $cacheInvalidator = new class implements CacheInvalidator { + public ?Title $invalidatedTitle = null; + + public function invalidateCaches( Title $title ): void { + $this->invalidatedTitle = $title; + } + }; $extension = $this->createMock( Extension::class ); $extension->method( 'getSettings' ) @@ -69,6 +70,9 @@ public function testDeletedPageInvalidatesCache() { $this->assertArrayHasKey( 'PageDeleteComplete', $registeredCallbacks ); $registeredCallbacks['PageDeleteComplete']( $page ); + + $this->assertSame( 'TestPage', $cacheInvalidator->invalidatedTitle->getDBkey() ); + $this->assertSame( NS_MAIN, $cacheInvalidator->invalidatedTitle->getNamespace() ); } private function newExtension() {