Skip to content

Commit c69d926

Browse files
brightbytelucaswerkmeister
brightbyte
authored and
lucaswerkmeister
committed
Avoid $wgHooks global
Hooks should be managed through HookContainer. Change-Id: I9d48c0e220baae71e936923773d2548071ee93d4
1 parent 5d7e5bc commit c69d926

File tree

8 files changed

+113
-91
lines changed

8 files changed

+113
-91
lines changed

‎client/includes/Hooks/ExtensionLoadHandler.php‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
use ApiMain;
66
use ExtensionRegistry;
7+
use MediaWiki\HookContainer\HookContainer;
8+
use MediaWiki\MediaWikiServices;
79
use Parser;
810
use Wikibase\Client\Api\ApiFormatReference;
911
use Wikibase\Client\DataAccess\ReferenceFormatterFactory;
@@ -25,24 +27,34 @@ class ExtensionLoadHandler {
2527
/** @var ExtensionRegistry */
2628
private $extensionRegistry;
2729

30+
/** @var HookContainer */
31+
private $hookContainer;
32+
33+
/**
34+
* @param ExtensionRegistry $extensionRegistry
35+
* @param HookContainer $hookContainer
36+
*/
2837
public function __construct(
29-
ExtensionRegistry $extensionRegistry
38+
ExtensionRegistry $extensionRegistry,
39+
HookContainer $hookContainer
3040
) {
3141
$this->extensionRegistry = $extensionRegistry;
42+
$this->hookContainer = $hookContainer;
3243
}
3344

3445
public static function factory(): self {
3546
return new self(
36-
ExtensionRegistry::getInstance()
47+
ExtensionRegistry::getInstance(),
48+
MediaWikiServices::getInstance()->getHookContainer()
3749
);
3850
}
3951

4052
public static function onExtensionLoad() {
41-
global $wgHooks, $wgWBClientSettings, $wgAPIModules;
53+
global $wgWBClientSettings, $wgAPIModules;
4254

4355
$handler = self::factory();
4456

45-
$wgHooks = array_merge_recursive( $wgHooks, $handler->getHooks() );
57+
$handler->registerHooks();
4658

4759
if ( $wgWBClientSettings === null ) {
4860
$wgWBClientSettings = [];
@@ -54,13 +66,14 @@ public static function onExtensionLoad() {
5466
}
5567
}
5668

57-
public function getHooks(): array {
58-
$hooks = [];
59-
69+
/**
70+
* Register the appropriate hooks in the HookContainer passed to the constructor.
71+
*/
72+
public function registerHooks(): void {
6073
// These hooks should only be run if we use the Echo extension
6174
if ( $this->extensionRegistry->isLoaded( 'Echo' ) ) {
62-
$hooks['LocalUserCreated'][] = EchoNotificationsHandlers::class . '::onLocalUserCreated';
63-
$hooks['WikibaseHandleChange'][] = EchoNotificationsHandlers::class . '::onWikibaseHandleChange';
75+
$this->hookContainer->register( 'LocalUserCreated', EchoNotificationsHandlers::class . '::onLocalUserCreated' );
76+
$this->hookContainer->register( 'WikibaseHandleChange', EchoNotificationsHandlers::class . '::onWikibaseHandleChange' );
6477
}
6578

6679
// This is in onExtensionLoad to ensure we register our
@@ -69,10 +82,8 @@ public function getHooks(): array {
6982
// However, ORES is not required.
7083
//
7184
// recent changes / watchlist hooks
72-
$hooks['ChangesListSpecialPageStructuredFilters'][] =
73-
ChangesListSpecialPageHookHandler::class . '::onChangesListSpecialPageStructuredFilters';
74-
75-
return $hooks;
85+
$this->hookContainer->register( 'ChangesListSpecialPageStructuredFilters',
86+
ChangesListSpecialPageHookHandler::class . '::onChangesListSpecialPageStructuredFilters' );
7687
}
7788

7889
public function getApiFormatReferenceSpec( array $clientSettings ): ?array {

‎client/tests/phpunit/unit/includes/Hooks/ExtensionLoadHandlerTest.php‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace Wikibase\Client\Tests\Unit\Hooks;
44

55
use ExtensionRegistry;
6+
use MediaWiki\HookContainer\HookContainer;
7+
use PHPUnit\Framework\MockObject\MockObject;
68
use PHPUnit\Framework\TestCase;
79
use Wikibase\Client\Api\ApiFormatReference;
810
use Wikibase\Client\Hooks\ChangesListSpecialPageHookHandler;
@@ -19,14 +21,33 @@
1921
*/
2022
class ExtensionLoadHandlerTest extends TestCase {
2123

24+
/**
25+
* @param &$hooks
26+
*
27+
* @return MockObject&HookContainer
28+
*/
29+
private function getFauxHookContainer( &$hooks ) {
30+
$container = $this->createMock( HookContainer::class );
31+
$container->method( 'register' )->willReturnCallback(
32+
function ( $name, $handler ) use ( &$hooks ) {
33+
$hooks[$name][] = $handler;
34+
}
35+
);
36+
37+
return $container;
38+
}
39+
2240
public function testGetHooks_withEcho() {
2341
$extensionRegistry = $this->createMock( ExtensionRegistry::class );
2442
$extensionRegistry->method( 'isLoaded' )
2543
->with( 'Echo' )
2644
->willReturn( true );
27-
$handler = new ExtensionLoadHandler( $extensionRegistry );
2845

29-
$actualHooks = $handler->getHooks();
46+
$actualHooks = [];
47+
$container = $this->getFauxHookContainer( $actualHooks );
48+
49+
$handler = new ExtensionLoadHandler( $extensionRegistry, $container );
50+
$handler->registerHooks();
3051

3152
$expectedHooks = [
3253
'LocalUserCreated' => [
@@ -47,9 +68,12 @@ public function testGetHooks_withoutEcho() {
4768
$extensionRegistry->method( 'isLoaded' )
4869
->with( 'Echo' )
4970
->willReturn( false );
50-
$handler = new ExtensionLoadHandler( $extensionRegistry );
5171

52-
$actualHooks = $handler->getHooks();
72+
$actualHooks = [];
73+
$container = $this->getFauxHookContainer( $actualHooks );
74+
75+
$handler = new ExtensionLoadHandler( $extensionRegistry, $container );
76+
$handler->registerHooks();
5377

5478
$expectedHooks = [
5579
'ChangesListSpecialPageStructuredFilters' => [
@@ -60,7 +84,7 @@ public function testGetHooks_withoutEcho() {
6084
}
6185

6286
public function testGetApiFormatReferenceSpec_settingTrue() {
63-
$handler = new ExtensionLoadHandler( $this->createMock( ExtensionRegistry::class ) );
87+
$handler = new ExtensionLoadHandler( $this->createMock( ExtensionRegistry::class ), $this->createMock( HookContainer::class ) );
6488

6589
$spec = $handler->getApiFormatReferenceSpec( [ 'dataBridgeEnabled' => true ] );
6690

@@ -70,7 +94,7 @@ public function testGetApiFormatReferenceSpec_settingTrue() {
7094

7195
/** @dataProvider provideNotTrueDataBridgeEnabledSettings */
7296
public function testGetApiFormatReferenceSpec_settingNotTrue( array $settings ) {
73-
$handler = new ExtensionLoadHandler( $this->createMock( ExtensionRegistry::class ) );
97+
$handler = new ExtensionLoadHandler( $this->createMock( ExtensionRegistry::class ), $this->createMock( HookContainer::class ) );
7498

7599
$spec = $handler->getApiFormatReferenceSpec( $settings );
76100

‎lib/tests/phpunit/WikibaseContentLanguagesTest.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,14 @@ public function testGetDefaultInstance_defaultContexts() {
4949

5050
public function testGetDefaultInstance_withHook() {
5151
$testLanguages = new StaticContentLanguages( [ 'test' ] );
52-
$this->mergeMwGlobalArrayValue( 'wgHooks', [
53-
'WikibaseContentLanguages' => [
54-
function ( array &$contentLanguages ) use ( $testLanguages ) {
55-
$contentLanguages['test'] = $testLanguages;
56-
},
57-
],
58-
] );
59-
$wcl = WikibaseContentLanguages::getDefaultInstance();
52+
$this->setTemporaryHook(
53+
'WikibaseContentLanguages',
54+
function ( array &$contentLanguages ) use ( $testLanguages ) {
55+
$contentLanguages['test'] = $testLanguages;
56+
}
57+
);
6058

59+
$wcl = WikibaseContentLanguages::getDefaultInstance();
6160
$this->assertSame( $testLanguages, $wcl->getContentLanguages( 'test' ) );
6261
}
6362

‎repo/tests/phpunit/includes/Actions/EditEntityActionTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ protected function setUp(): void {
3131
parent::setUp();
3232

3333
// Remove handlers for the "OutputPageParserOutput" hook
34-
$this->mergeMwGlobalArrayValue( 'wgHooks', [ 'OutputPageParserOutput' => [] ] );
34+
$this->clearHook( 'OutputPageParserOutput' );
3535
}
3636

3737
public function testActionForPage() {

‎repo/tests/phpunit/includes/Actions/ViewEntityActionTest.php‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,8 @@ protected function setUp(): void {
3333
$this->setUserLang( 'de' );
3434

3535
// Remove handlers for the "OutputPageParserOutput" and "DifferenceEngineViewHeader" hooks
36-
$this->mergeMwGlobalArrayValue(
37-
'wgHooks',
38-
[
39-
'OutputPageParserOutput' => [],
40-
'DifferenceEngineViewHeader' => []
41-
]
42-
);
36+
$this->clearHook( 'OutputPageParserOutput' );
37+
$this->clearHook( 'DifferenceEngineViewHeader' );
4338
}
4439

4540
public function testActionForPage() {

‎repo/tests/phpunit/includes/Content/EntityContentTestCase.php‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,17 +113,16 @@ public function testWikibaseTextForSearchIndex() {
113113
$entityContent = $this->newBlank();
114114
$this->setLabel( $entityContent->getEntity(), 'en', "cake" );
115115

116-
$this->mergeMwGlobalArrayValue( 'wgHooks', [
117-
'WikibaseTextForSearchIndex' => [
118-
function ( $actualEntityContent, &$text ) use ( $entityContent ) {
119-
$this->assertSame( $entityContent, $actualEntityContent );
120-
$this->assertSame( 'cake', $text );
121-
122-
$text .= "\nHOOK";
123-
return true;
124-
},
125-
],
126-
] );
116+
$this->setTemporaryHook(
117+
'WikibaseTextForSearchIndex',
118+
function ( $actualEntityContent, &$text ) use ( $entityContent ) {
119+
$this->assertSame( $entityContent, $actualEntityContent );
120+
$this->assertSame( 'cake', $text );
121+
122+
$text .= "\nHOOK";
123+
return true;
124+
}
125+
);
127126

128127
$text = $entityContent->getTextForSearchIndex();
129128
$this->assertSame( "cake\nHOOK", $text, 'Text for search index should be updated by the hook' );
@@ -133,13 +132,12 @@ public function testWikibaseTextForSearchIndex_abort() {
133132
$entityContent = $this->newBlank();
134133
$this->setLabel( $entityContent->getEntity(), 'en', "cake" );
135134

136-
$this->mergeMwGlobalArrayValue( 'wgHooks', [
137-
'WikibaseTextForSearchIndex' => [
138-
static function () {
139-
return false;
140-
},
141-
],
142-
] );
135+
$this->setTemporaryHook(
136+
'WikibaseTextForSearchIndex',
137+
static function () {
138+
return false;
139+
}
140+
);
143141

144142
$text = $entityContent->getTextForSearchIndex();
145143
$this->assertSame( '', $text, 'Text for search index should be empty if the hook returned false' );

‎repo/tests/phpunit/includes/Hooks/EditFilterHookRunnerTest.php‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function getEditFilterHookRunner(): MediawikiEditFilterHookRunner {
5959
}
6060

6161
public function testRun_noHooksRegisteredGoodStatus() {
62-
$this->mergeMwGlobalArrayValue( 'wgHooks', [ 'EditFilterMergedContent' => [] ] );
62+
$this->clearHook( 'EditFilterMergedContent' );
6363

6464
$context = new RequestContext();
6565
$context->setRequest( new FauxRequest() );
@@ -141,37 +141,33 @@ public function runData() {
141141
* @dataProvider runData
142142
*/
143143
public function testRun_hooksAreCalled( Status $inputStatus, $new, array $expected ) {
144-
$hooks = array_merge(
145-
$GLOBALS['wgHooks'],
146-
[ 'EditFilterMergedContent' => [] ]
144+
$this->clearHook( 'EditFilterMergedContent' );
145+
146+
$this->setTemporaryHook(
147+
'EditFilterMergedContent',
148+
function(
149+
IContextSource $context,
150+
Content $content,
151+
Status $status,
152+
$summary,
153+
User $user,
154+
$minoredit
155+
) use ( $expected, $inputStatus ) {
156+
$wikiPage = MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( $context->getTitle() );
157+
$this->assertSame( $expected['title'], $context->getTitle()->getFullText() );
158+
$this->assertSame( $context->getTitle(), $wikiPage->getTitle() );
159+
$this->assertSame( $expected['namespace'], $context->getTitle()->getNamespace() );
160+
$this->assertEquals( new ItemContent( new EntityInstanceHolder( new Item() ) ), $content );
161+
$this->assertTrue( $status->isGood() );
162+
$this->assertIsString( $summary );
163+
$this->assertSame( 'EditFilterHookRunnerTestUser', $user->getName() );
164+
$this->assertIsBool( $minoredit );
165+
166+
// Change the status
167+
$status->merge( $inputStatus );
168+
}
147169
);
148170

149-
$hooks['EditFilterMergedContent'][] = function(
150-
IContextSource $context,
151-
Content $content,
152-
Status $status,
153-
$summary,
154-
User $user,
155-
$minoredit
156-
) use ( $expected, $inputStatus ) {
157-
$wikiPage = MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( $context->getTitle() );
158-
$this->assertSame( $expected['title'], $context->getTitle()->getFullText() );
159-
$this->assertSame( $context->getTitle(), $wikiPage->getTitle() );
160-
$this->assertSame( $expected['namespace'], $context->getTitle()->getNamespace() );
161-
$this->assertEquals( new ItemContent( new EntityInstanceHolder( new Item() ) ), $content );
162-
$this->assertTrue( $status->isGood() );
163-
$this->assertIsString( $summary );
164-
$this->assertSame( 'EditFilterHookRunnerTestUser', $user->getName() );
165-
$this->assertIsBool( $minoredit );
166-
167-
// Change the status
168-
$status->merge( $inputStatus );
169-
};
170-
171-
$this->setMwGlobals( [
172-
'wgHooks' => $hooks
173-
] );
174-
175171
$context = new RequestContext();
176172
$context->setRequest( new FauxRequest() );
177173
$context->setUser( User::newFromName( 'EditFilterHookRunnerTestUser' ) );

‎repo/tests/phpunit/includes/Notifications/HookChangeTransmitterTest.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@ public function testTransmitChange() {
2121
$change = $this->createMock( EntityChange::class );
2222

2323
$called = false;
24-
$this->mergeMwGlobalArrayValue( 'wgHooks', [
25-
'HookChangeTransmitterTest' => [
26-
function ( $actualChange ) use ( $change, &$called ) {
27-
self::assertEquals( $change, $actualChange );
28-
$called = true;
29-
},
30-
],
31-
] );
24+
$this->setTemporaryHook(
25+
'HookChangeTransmitterTest',
26+
function ( $actualChange ) use ( $change, &$called ) {
27+
self::assertEquals( $change, $actualChange );
28+
$called = true;
29+
}
30+
);
3231

3332
$transmitter = new HookChangeTransmitter(
3433
$this->getServiceContainer()->getHookContainer(),

0 commit comments

Comments
 (0)