= 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 listener interfaces:** Have one listener 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 listener name. Extensions also register an array of listeners, each listener being described by an ObjectFactory spec.
* **Listener dependency injection:** As with the REST API, the "services" key in the ObjectFactory spec can be used to inject services into the hook listener constructor.
* **HookRunner:** A HookRunner service in core provides a concrete implementation of all core hook listener 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 listener 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 listener interfaces be grouped? I am proposing one HookRunner class in core, and grouping of all core interfaces under a single namespace. Having a single HookRunner in core means that only one service needs to be added to MediaWikiServices, instead of a collection of services or a factory which delivers a module-specific HookRunner. I think it is convenient to keep masses of repetitive boilerplate code in one place. But it is not an essential point: the proposal would still work if listener interfaces were put in the namespaces of the modules they serve, with module-specific runners to implement them.
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.
I have tried changing the terminology from "handler" to "listener", to better separate it from REST handlers, and following proposed terminology from other hook-related RFCs. But it is not an essential detail. I think "listener" makes more sense as a term for the interfaces than as a term for the extension implementations of those interfaces.
= 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 "listener name" could be implicitly prefixed with the extension name, so that listener names would not need to be globally unique.
This sample code shows a very brief potential syntax in which listener names are brought into the same namespace as global functions. The registration system would search first for a hook listener 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 `{"listener":"*"}`.
```lang=json
{
"HookListeners": {
"*": {
"class": "MyExtension\\Hooks",
"services": [ "ActorMigration" ]
}
},
"Hooks": {
"UserLoginComplete" : "*"
}
}
```
= Sample listener code =
```
<?php
namespace MyExtension;
use ActorMigration;
use MediaWiki\HookListener\UserLoginComplete;
class Hooks implements UserLoginComplete {
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
namespace MediaWiki\HookListener;
interface UserLoginComplete {
/**
* @param \User $user
* @param string $injectedHtml
* @param bool $direct
* @return bool
*/
function onUserLoginComplete( \User $user, &$injectedHtml, $direct );
}
```
==HookRunner.php==
```
<?php
namespace MediaWiki\HookRunner;
use MediaWiki\HookListener\UserLoginComplete;
class HookRunner implements
// ... many interfaces would be added here ...
UserLoginComplete,
{
/** @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;
}
}
}
$listeners = $this->getListeners( $name );
if ( $listeners ) {
$funcName = 'on' . str_replace( ':', '_', $name );
foreach ( $listeners as $listener ) {
$success = $listener->$funcName( ...$args );
if ( !$success ) {
return false;
}
}
}
return true;
}
}
```