Problem statement

This proposal aims to solve the following problems with the existing hook system:

• There is no way to inject dependencies into a hook handler, so many handlers are obliged to access the global service container.
• The types of the hook arguments are undeclared and unvalidated.
• There is no place to put doc comments relating to hook functions; they are documented in a separate text file and on mediawiki.org. There is no editor integration with the separate documentation.
• The global hook runner is implemented in static functions rather than a service.

Proposal summary

• Hook interfaces: Have one interface per hook. Each interface has a single method with a name specific to the hook.
• Indirect registration: Instead of having extensions register the function name associated with a hook, they register a handler name. Extensions also register an array of handlers, each handler being described by an ObjectFactory spec.
• Handler dependency injection: As with the REST API, the "services" key in the ObjectFactory spec can be used to inject services into the hook handler constructor.
• HookRunner: A HookRunner class in core provides a concrete implementation of all core hook interfaces. That is, for each hook, the same interface is used for both calling the hook and handling it. Each method consists of approximately one line of code which proxies the call to HookContainer. Extensions may implement their own HookRunner style classes along the same lines.
• HookContainer: A service which is responsible for creating, caching and calling handler object instances.

Comparison with past work

• T212482 Filters and Actions: The current proposal is complementary to the filters and actions RFC. The current proposal allows existing hooks to be remediated, as well as providing a precedent for registration and strong typing that can be used when filters and actions are implemented.
• T154674 HookRunner: That proposal is conservative and leaves many of the problems with hooks unresolved. I agree with @Tgr's comment at T154674#2998357, and this proposal is very similar to the scheme he proposed there.

Tradeoffs and judgement calls

Having one interface per hook, with a unique method name for each hook, allows extensions to implement many different hooks in a single class, simplifying DI and allowing them to keep the same code structure as is currently typical. In smart code editors, adding an interface to a handler class automatically gives the developer the documentation and a method stub. The system makes it easy for extension developers to implement hooks.

One drawback of using interfaces is that it will no longer be possible to add parameters to existing hook interfaces. Instead, hook interfaces will need to be versioned.

Inevitably, some classes will have a very long "implements" clause. I think the clutter is a small price to pay. PHP might iterate over all parent interfaces in certain circumstances — we should verify that the performance is acceptable.

In T154674#2998357, @Tgr proposed that instead of interfaces, there would be one abstract class per module, for example, UploadHookRunner. This may be convenient for core callers of hooks, but it is not convenient to reuse such an abstract class to ensure strong typing in handlers. The point of using interfaces is to allow sharing of parameter signatures between callers and handlers.

On what level of modularity should HookRunner and hook interfaces be grouped? I am proposing one HookRunner class in core. I previously proposed grouping of all core interfaces under a single namespace, but the proposal has now been updated to place interfaces in the same namespace as the caller. I think it is convenient to keep masses of repetitive boilerplate code in one place. But it is not an essential point.

It's important that method names are globally unique. I suggest naming them after the hook name. If methods are not globally unique then the conflicting hooks cannot be implemented in the same handler class.

Registration

The registration system follows the way we did handler registration in the REST API, except that an extra level of indirection is added in the config, to avoid duplication of ObjectFactory specs. I think the "handler name" could be implicitly prefixed with the extension name, so that handler names would not need to be globally unique.

This sample code shows a very brief potential syntax in which handler names are brought into the same namespace as global functions. The registration system would search first for a hook handler with the corresponding name, and if one does not exist, it is assumed to be a global function. Alternatively, the namespaces could be separated by either changing the top-level "Hooks" key or by making the value be an object like {"handler":"*"}.

{
	"HookHandlers": {
		"*": {
			"class": "MyExtension\\Hooks",
			"services": [ "ActorMigration" ]
		}
	},
	"Hooks": {
		"UserLoginComplete" : "*"
	}
}

Deprecation and superseded hooks

Core will have a list of deprecated hooks, which extensions can add to via the DeprecatedHooks attribute. If an extension registers a handler for a deprecated hook, a deprecation warning will be raised when extension.json is loaded, unless the deprecation is acknowledged in extension.json:

{
	"Hooks": {
		"UserLoginComplete" : {
			"handler": "*",
			"deprecated": true
		},
		"UserLoginComplete2" : {
			"handler": "*"
		}
	}
}

Acknowledging deprecation in this way causes the handler to not be called. In this example, UserLoginComplete2 replaces UserLoginComplete, and calls to UserLoginComplete will be filtered if core is aware that UserLoginComplete is a deprecated hook.

For forwards compatibility, if a handler acknowledges deprecation but core is not aware of the hook in question being deprecated, the call will not be filtered. So in this example, the UserLoginComplete handler will still be called as long as a version of core is used in which it is not deprecated.

Sample handler code

<?php

namespace MyExtension;

use ActorMigration;
use UserLoginCompleteHook;

class Hooks implements UserLoginCompleteHook {
	private $actorMigration;
	
	public function __construct( ActorMigration $actorMigration ) {
		$this->actorMigration = $actorMigration;
	}
	
	/**
	 * @param \User $user
	 * @param string $injectedHtml
	 * @param bool $direct
	 * @return bool
	 */
	function onUserLoginComplete( \User $user, &$injectedHtml, $direct ) {
		// ...
	}
}

Sample core caller

Here is some incomplete sample code showing how legacy reference arguments could be dealt with during migration to the new system. UserLoginComplete is an existing hook which passes $user as a reference, for legacy reasons.

UserLoginComplete.php

<?php

interface UserLoginCompleteHook {
	/**
	 * @param \User $user
	 * @param string $injectedHtml
	 * @param bool $direct
	 * @return bool
	 */
	function onUserLoginComplete( \User $user, &$injectedHtml, $direct );
}

HookRunner.php

<?php

namespace MediaWiki\HookRunner;

use UserLoginCompleteHook;

class HookRunner implements
	// ... many interfaces would be added here ...
	UserLoginCompleteHook,
{
	/** @var HookContainer */
	private $hookContainer;

	public function __construct( HookContainer $hookContainer ) {
		$this->hookContainer = $hookContainer;
	}

	/**
	 * @param \User $user
	 * @param string $injectedHtml
	 * @param bool $direct
	 * @return bool
	 */
	function onUserLoginComplete( \User $user, &$injectedHtml, $direct ) {
		return $this->hookContainer->run( 'UserLoginComplete',
			[ $user, &$injectedHtml, $direct ],
			[ 'legacyRefArgs' => [ 0 ] ]
		);
	}
}

HookContainer.php

For simplicity, I have left out a lot of details present in Hooks::run(). The point is to sketch out the call flow.

<?php
namespace MediaWiki\HookRunner;

class HookContainer {
	// ...

	public function run( $name, $args, $options = [] ) {
		$legacyHandlers = $this->getLegacyHandlers( $name );
		if ( $legacyHandlers ) {
			$legacyArgs = $args;
			if ( isset( $options['legacyRefArgs'] ) ) {
				foreach ( $args as $i => $arg ) {
					if ( in_array( $i, $options['legacyRefArgs'] ) ) {
						$legacyArgs[$i] =& $args[$i];
					}
				}
			}
			foreach ( $legacyHandlers as $handler ) {
				$success = $handler( ...$legacyArgs );
				if ( !$success ) {
					return false;
				}
			}
		}

		$handlers = $this->getHandlers( $name );
		if ( $handlers ) {
			$funcName = 'on' . str_replace( ':', '_', $name );
			foreach ( $handlers as $handler ) {
				 $success = $handler->$funcName( ...$args );
				 if ( !$success ) {
					return false;
				 }
			}
		}

		return true;
	}
}

SpecialCreateAccount.php

@@ -44,6 +48,8 @@ class SpecialCreateAccount extends LoginSignupSpecialPage {
 
 	public function __construct() {
 		parent::__construct( 'CreateAccount' );
+		$this->hookRunner = new HookRunner(
+			MediaWikiServices::getInstance()->getHookContainer() );
 	}

@@ -139,14 +145,14 @@ class SpecialCreateAccount extends LoginSignupSpecialPage {
 		# Run any hooks; display injected HTML
 		$injected_html = '';
 		$welcome_creation_msg = 'welcomecreation-msg';
-		Hooks::run( 'UserLoginComplete', [ &$user, &$injected_html, $direct ] );
+		$this->hookRunner->onUserLoginComplete( $user, $injected_html, $direct );