Page MenuHomePhabricator
Create Task
Maniphest T323908

Refactor Hooks.php to use HookHandlers
Closed, ResolvedPublic
Actions

Description

Correct me if I'm wrong, but I think that placing a hook method in Hooks.php is discouraged, and the more modern way to do this is to use a HookHandler.

Example patch adding a HookHandler to PageTriage: https://github.com/wikimedia/mediawiki-extensions-PageTriage/commit/7b1f0eb1683d12484640983e3a50024c87258d35

If so, PageTriage's 20 or so hook methods located in Hooks.php can be refactored into individual PHP files in the folder includes/HookHandlers.

I believe the main advantage is the ability to dependency inject MediaWiki services/instances.

https://www.mediawiki.org/wiki/Manual:Hooks#Handling_hooks_in_MediaWiki_1.35_and_later

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Introduce PageTriageQueueManagermediawiki/extensions/PageTriagemaster+152 -42
PageSaveComplete: Minor simplificationsmediawiki/extensions/PageTriagemaster+4 -6
Convert LocalUserCreated to HookHandlermediawiki/extensions/PageTriagemaster+17 -13
Convert ResourceLoaderRegisterModules to HookHandlermediawiki/extensions/PageTriagemaster+7 -9
Convert BlockIpCompleteHook to HookHandlermediawiki/extensions/PageTriagemaster+7 -11
Convert ResourceLoaderGetConfigVars to HookHandlermediawiki/extensions/PageTriagemaster+11 -12
Hooks.php: Use injected TitleFactory instead of static Title methodsmediawiki/extensions/PageTriagemaster+2 -2
Convert MarkPatrolledComplete to HookHandlermediawiki/extensions/PageTriagemaster+8 -14
Hooks.php: Use PageIdentity for flushUserStatusCachemediawiki/extensions/PageTriagemaster+10 -17
Convert ArticleViewFooter to HookHandlermediawiki/extensions/PageTriagemaster+45 -22
Convert ArticleDeleteComplete to HookHandlermediawiki/extensions/PageTriagemaster+47 -25
Convert PageSaveComplete to HookHandlermediawiki/extensions/PageTriagemaster+14 -21
Convert LinksUpdateComplete to HookHandlermediawiki/extensions/PageTriagemaster+10 -14
Convert RevisionFromEditComplete to HookHandlermediawiki/extensions/PageTriagemaster+18 -19
Convert PageMoveComplete to HookHandlermediawiki/extensions/PageTriagemaster+13 -24
Convert LoadExtensionSchemaUpdates to HookHandlermediawiki/extensions/PageTriagemaster+97 -81
Convert ListDefinedTags, ChangeTagsListActive, ChangeTagsAllowedAdd to Hook Handlermediawiki/extensions/PageTriagemaster+28 -12
Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

Novem_Linguae created this task.Nov 28 2022, 12:54 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptNov 28 2022, 12:54 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Novem_Linguae moved this task from Backlog to Priority technical debt on the PageTriage board.Nov 28 2022, 12:57 PM
kostajh subscribed.Nov 28 2022, 12:58 PM

You can still call it "Hooks.php", indeed the "main" HookHandler for PageTriage currently points to that file:

"HookHandlers": {
		"undelete": {
			"class": "MediaWiki\\Extension\\PageTriage\\HookHandlers\\UndeleteHookHandler"
		},
		"main": {
			"class": "MediaWiki\\Extension\\PageTriage\\Hooks",
			"services": [
				"MainConfig"
			]
		}
	},

Creating a separate file for each handler seems kind of excessive to me. I would suggest trying to organize the hook handlers by the surface area of the relevant feature that the hook handler is used for. e.g. maybe there is a "curationtoolbar" hook handler, and a "newpagesfeed" hook handler file, that would register hook handlers for those two features specifically.

Novem_Linguae moved this task from Priority technical debt to Backlog on the PageTriage board.Nov 28 2022, 12:59 PM
taavi added a parent task: T263353: Update all Wikimedia production extensions to use the new HookContainer/HookRunner system.Nov 29 2022, 9:58 AM
Novem_Linguae updated the task description. (Show Details)Nov 30 2022, 9:27 AM
kostajh mentioned this in T324070: Refactor PageTriage static methods to be non-static.Nov 30 2022, 9:58 AM
Sgs moved this task from Inbox to Triaged on the Growth-Team board.Nov 30 2022, 11:23 AM
Novem_Linguae moved this task from Backlog to Priority technical debt on the PageTriage board.Dec 1 2022, 3:10 PM
TheDJ subscribed.EditedDec 1 2022, 3:16 PM

@Novem_Linguae I'd prioritize this hooks.php -> hookhandlers rewrite if I were you. The static code of hooks is a big reason of why so much of MediaWiki is still static and global, and getting rid of that static entry point into an extension makes lots of things simpler, including writing testcases, and rewriting all the static code that is behind these static entry points. It's also generally relatively easy to do. There are many examples from other extensions, that have recently gone through this rewrite, that can look at for guidance.

kostajh added a parent task: T324211: Refactor PageTriage to use services with narrow scopes and value objects for passing data.Dec 1 2022, 4:08 PM
gerritbot added a comment.Dec 1 2022, 9:24 PM

Change 862934 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Introduce PageTriageQueueManager

https://gerrit.wikimedia.org/r/862934

gerritbot added a project: Patch-For-Review.Dec 1 2022, 9:24 PM
HouseBlaster moved this task from Unsorted to Needs refactor on the Technical-Debt board.Dec 3 2022, 12:40 AM
gerritbot added a comment.Dec 12 2022, 12:59 PM

Change 867157 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert ListDefinedTags, ChangeTagsListActive, ChangeTagsAllowedAdd to Hook Handler

https://gerrit.wikimedia.org/r/867157

gerritbot added a comment.Dec 12 2022, 1:01 PM

Change 867158 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert LoadExtensionSchemaUpdates to HookHandler

https://gerrit.wikimedia.org/r/867158

gerritbot added a comment.Dec 12 2022, 1:03 PM

Change 867159 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert PageMoveComplete to HookHandler

https://gerrit.wikimedia.org/r/867159

kostajh claimed this task.Dec 12 2022, 1:03 PM
gerritbot added a comment.Dec 12 2022, 1:28 PM

Change 867162 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert RevisionFromEditComplete to HookHandler

https://gerrit.wikimedia.org/r/867162

gerritbot added a comment.Dec 12 2022, 3:32 PM

Change 867189 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert PageSaveComplete to HookHandler

https://gerrit.wikimedia.org/r/867189

gerritbot added a comment.Dec 12 2022, 3:34 PM

Change 867190 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert LinksUpdateComplete to HookHandler

https://gerrit.wikimedia.org/r/867190

gerritbot added a comment.Dec 12 2022, 3:41 PM

Change 867157 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert ListDefinedTags, ChangeTagsListActive, ChangeTagsAllowedAdd to Hook Handler

https://gerrit.wikimedia.org/r/867157

gerritbot added a comment.Dec 12 2022, 3:43 PM

Change 867193 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert ArticleViewFooter to HookHandler

https://gerrit.wikimedia.org/r/867193

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.14; 2022-12-12).Dec 12 2022, 4:01 PM
gerritbot added a comment.Dec 12 2022, 9:08 PM

Change 867158 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert LoadExtensionSchemaUpdates to HookHandler

https://gerrit.wikimedia.org/r/867158

gerritbot added a comment.Dec 13 2022, 12:48 PM

Change 867577 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert ArticleDeleteComplete to HookHandler

https://gerrit.wikimedia.org/r/867577

gerritbot added a comment.Dec 13 2022, 12:54 PM

Change 867578 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Hooks.php: Use PageIdentity for flushUserStatusCache

https://gerrit.wikimedia.org/r/867578

gerritbot added a comment.Dec 13 2022, 12:58 PM

Change 867580 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Hooks.php: Use injected TitleFactory instead of static Title methods

https://gerrit.wikimedia.org/r/867580

gerritbot added a comment.Dec 13 2022, 12:59 PM

Change 867581 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert MarkPatrolledComplete to HookHandler

https://gerrit.wikimedia.org/r/867581

gerritbot added a comment.Dec 13 2022, 1:06 PM

Change 867583 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert BlockIpCompleteHook to HookHandler

https://gerrit.wikimedia.org/r/867583

gerritbot added a comment.Dec 13 2022, 1:09 PM

Change 867584 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert ResourceLoaderGetConfigVarsHook to HookHandler

https://gerrit.wikimedia.org/r/867584

gerritbot added a comment.Dec 13 2022, 1:13 PM

Change 867586 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert LocalUserCreated to HookHandler

https://gerrit.wikimedia.org/r/867586

gerritbot added a comment.Dec 13 2022, 1:15 PM

Change 867587 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] Convert ResourceLoaderRegisterModules to HookHandler

https://gerrit.wikimedia.org/r/867587

gerritbot added a comment.Dec 13 2022, 1:28 PM

Change 867590 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/PageTriage@master] PageSaveComplete: Minor simplifications

https://gerrit.wikimedia.org/r/867590

gerritbot added a comment.Dec 14 2022, 7:06 PM

Change 867159 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert PageMoveComplete to HookHandler

https://gerrit.wikimedia.org/r/867159

gerritbot added a comment.Dec 14 2022, 7:15 PM

Change 867162 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert RevisionFromEditComplete to HookHandler

https://gerrit.wikimedia.org/r/867162

gerritbot added a comment.Dec 14 2022, 7:15 PM

Change 867189 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert PageSaveComplete to HookHandler

https://gerrit.wikimedia.org/r/867189

gerritbot added a comment.Dec 14 2022, 7:15 PM

Change 867190 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert LinksUpdateComplete to HookHandler

https://gerrit.wikimedia.org/r/867190

gerritbot added a comment.Dec 14 2022, 7:16 PM

Change 867193 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert ArticleViewFooter to HookHandler

https://gerrit.wikimedia.org/r/867193

gerritbot added a comment.Dec 14 2022, 7:16 PM

Change 867577 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert ArticleDeleteComplete to HookHandler

https://gerrit.wikimedia.org/r/867577

TheresNoTime subscribed.Dec 14 2022, 7:22 PM

Really sorry for the noise there... I +2'd from the wrong end(?) of the relation chain...

ReleaseTaggerBot edited projects, added: MW-1.40-notes (1.40.0-wmf.17; 2023-01-02); removed: MW-1.40-notes (1.40.0-wmf.14; 2022-12-12).Dec 14 2022, 8:01 PM
gerritbot added a comment.Dec 14 2022, 9:31 PM

Change 867578 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Hooks.php: Use PageIdentity for flushUserStatusCache

https://gerrit.wikimedia.org/r/867578

gerritbot added a comment.Dec 14 2022, 9:31 PM

Change 867580 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Hooks.php: Use injected TitleFactory instead of static Title methods

https://gerrit.wikimedia.org/r/867580

gerritbot added a comment.Dec 14 2022, 9:31 PM

Change 867581 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert MarkPatrolledComplete to HookHandler

https://gerrit.wikimedia.org/r/867581

gerritbot added a comment.Dec 14 2022, 10:07 PM

Change 867583 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert BlockIpCompleteHook to HookHandler

https://gerrit.wikimedia.org/r/867583

gerritbot added a comment.Dec 14 2022, 10:07 PM

Change 867584 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert ResourceLoaderGetConfigVars to HookHandler

https://gerrit.wikimedia.org/r/867584

gerritbot added a comment.Dec 14 2022, 10:07 PM

Change 867586 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert LocalUserCreated to HookHandler

https://gerrit.wikimedia.org/r/867586

gerritbot added a comment.Dec 14 2022, 10:07 PM

Change 867587 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Convert ResourceLoaderRegisterModules to HookHandler

https://gerrit.wikimedia.org/r/867587

gerritbot added a comment.Dec 14 2022, 10:50 PM

Change 867590 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] PageSaveComplete: Minor simplifications

https://gerrit.wikimedia.org/r/867590

gerritbot added a comment.Dec 17 2022, 3:21 AM

Change 862934 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Introduce PageTriageQueueManager

https://gerrit.wikimedia.org/r/862934

Jdlrobson subscribed.Mar 31 2023, 7:14 PM

Is this done? The remaining hooks relate to extensions (Echo and ORES).

Maintenance_bot removed a project: Patch-For-Review.Mar 31 2023, 7:30 PM
kostajh closed this task as Resolved.Mar 31 2023, 7:37 PM
In T323908#8746789, @Jdlrobson wrote:

Is this done? The remaining hooks relate to extensions (Echo and ORES).

Yep!

Umherirrender added a project: Core Platform Team Initiatives (New Hook System).Jun 9 2023, 8:42 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits