@CCicalese_WMF and I are interested in having an IRC meeting on this to keep it moving. This is tied closely to work Core Platform is doing on APIs.

We should have at least a basic survey of the most-used hooks, and the way they are used, before we have an IRC meeting about this.

I like the idea of separation of concerns and having a codebase that is explicit and easy to use. From my understanding,

• the action hooks can be used to modify state (expensive calls like update user in DB, log stuff, add tags, etc.)
• the filter hooks can be used to build/filter things (like building UserDefaultOptions array, adding JS modules).

If I didn't understand it correctly, could you provide any examples?

At this moment I have a couple of concerns. Most of the time I develop extensions (Popups, TextExtracts, MobileFrontend, Minerva skin), and I can say that proposed rules fit MediaWiki Core land correctly, but when it comes to extensions - it looks troubling.

ActionHooks: "Instead, they are run from a deferred update or job."

If we decide to go with a deferred update, we need a way to specify if this is done POSTSEND or PRESEND. In some cases we can encounter a race condition, when the output (page redirect) is sent to the user and browser starts rendering a new page, but the script still processes the ActionHook. That leads into the state when the user sees a partially modified state. But sometimes we process stuff that is not visible to the user and it should be executed on POSTSEND.

FilterHooks: "Other instances and services should not be obtained" -> this might work excellent in the MediaWiki core, but extensions/skins need to access their services in some way. I'm slowly shifting old Singletons into Services (registered via ServiceWiring files).

During hooks execution, I need to access Services, so I can decide what to do next, as I cannot keep all logic inside one Hooks file. Without being able to get stuff via MediaWikiServices I need to keep things like old MobileContext::getSingleton() which I want to avoid. Singletons are difficult to test as there is no easy way to inject mocks. Additionally, it's very easy to create side effects when Singletons are in use.

Once we decide to start naming hooks "ActionHooks.XYZ", "FilterHooks.XYZ" - there will be no easy way to switch one to another. It's a good thing, but it may cause some troubles in the future. Each change in the core requires changes in all extensions/skins. If possible, I'd like to have a hook system that allows us small changes to the Hook signature without fixing all extensions/skins that use this hook.

If I understood properly what FilterHooks are, some of those Hooks could be safely replaced by service tagging. There is no need to listen to a hook when we need to build things. Example:
MobileFrontend has an interface called IMobileTransform. This interface represents a single, small transformation to the DOM structure and it's used during MobileFormatter execution. We have many transforms (like MoveLeadParagraphTransform, AddMobileTocTransform). Now, if we abandon hooks in favor of service tagging: instead of such structure:

$transforms = [];
Hooks::filter( 'MobileFrontend.RegisterTransform', &transforms );
/** @var IMobileTransform $transfor */
foreach( $transforms as $transform) {
   $transform->apply( $dom );
}

We could do something IMHO much cleaner:

$transforms = MediaWikiServices::getAllByTag('MobileFrontend.Transform');
/** @var IMobileTransform $transfor */
foreach( $transforms as $transform) {
   $transform->apply( $dom );
}

This reduces side effects to zero, as there is no Hook call and no arbitrary code execution, just a function that loads sets of services from MediaWikiServices.

My last comment is regarding the general approach. We need to get the control on what's going on in the Hooks world.
Passing variables, references to variables () and depending on which method from Hooks is called doesn't give us much flexibility nor any control. Some extensions will have to access MediaWikiServices::getService() or RequestContext::getMain() anyway to get required objects/services. Hooks cannot provide all dependencies for all extensions. Instead of passing a mutable/immutable arguments and depending on fact which Hooks method is called (action or filter). I'd love to see us passing an object that takes control on what can be done and whats not allowed. We should fully embrace an object-oriented approach to managing the execution flow of hooks. We have Status class, that IMHO fits here perfectly.

As a first step, we could come up with a straightforward interface (let's call it HookExcecution for easier reading) for hook execution with only essential methods. Then, we could do something like that:

$execution = new ActionHookExecution( $name, $data );
Hooks::run( $execution );

Hooks::run() can be a factory, and depending on the $execution type can fire filter or action methods. The HookExecution can have methods like: abort() (only filter), fail() (only action) or getService(). This provides us control over what users can do (they can access only methods from the
HookExecution, because HookExecution provides services there will be no need to access global variables/singletons). Inside the HookExecution we could freeze some things (like make some services read-only, when returning a service we can return a clone/frozen version, so we're sure it doesn't have side effects). Then, most important bits of our system could have concrete HookExecution classes, something like Hooks/SpecialPagesInit that exposes an API to modify set of SpecialPages. By default, the HookExecution class can have get( $key ) and set( $key, $value ). Also, it can implement ArrayAccess, therefore in some cases, no changes to extensions have to be done (when we pass an array as the first argument, for example, ChangeTagsListActive`).

Instead of passing references to variables, first we should organize the code in the way it exposes APIs on how to modify stuff, and then pass those APIs. Making things like adding 'FilterHook.' to the hook name, and passing things as $data = [ 'mutable' => &$variableA, 'notmutable' => $variableB ]
is just a short term improvement that will not fix the system. It makes HooksSystem feel better, but under the hood, IMHO it's still the same.

As final thought, I'd like to mention, that some time ago, I came up with the idea of T171515: Hooks system should support MediaWiki services. This is somewhat related to this task, in general, our Hooks system should utilize MediaWikiServices more often. That approach will help us reduce the number of static functions (at this moment most of the Hooks are static methods which are usually difficult to test), therefore it should minimize possible side effects.

moving back to "under discussion" until we have a better understanding of current hook usage.

Currently, passing values by reference makes it impossible for hook subscribers to know which types they are getting, making type hinting impossible. That causes issues like T214358.

Could the hook runner be extended do validate the types of mutable values after each subscriber returns?

The alternative would be that mutable values would need to be wrapped in container objects that expose interface to modify the value. I imagine something generic like new ModifiableValue( $type, $value ) that exposes get and set that enforce the type is possible, but being far from elegant and hiding the real type.

In T212482#5164991, @Nikerabbit wrote:

The alternative would be that mutable values would need to be wrapped in container objects that expose interface to modify the value. I imagine something generic like new ModifiableValue( $type, $value ) that exposes get and set that enforce the type is possible, but being far from elegant and hiding the real type.

@BPirkle suggested to use anonymous objects that expose setters tailored for just the hook, and document them along with the hook signature. E.g.

$foo an object that exposes setBar( $x, $y, $z ).

I really like this solution, because it allows us to define narrow, one-off interfaces that allow for readable code:

$foo->setBar( 1, 2, 3)

@daniel Given a filter only mutating one value (previously written as "taking one parameter", we since then loosened it up to allow any number of parameters, but only one of which should be mutated by the hook), this could also be solved by using the return value.

That's what WordPress do, for example. This means it can be validated by return type hints. And if we want to enforce it in the hook runner, this could be passed to the hook runner as option, but I'm not sure that added complexity and verboseness throughout is worth it. It would caught naturally soon enough by parameter type hints from other handlers, other methods called after the hook by the source code, or the source's return type hint etc.

We can progressively enforce with PHPCS that filter hook callbacks must have a return type hint, for example. Might be more DRY that way, then to have the source code also document in multiple places, and pass around, and validate in-process etc.

In T212482#5211044, @Krinkle wrote:

@daniel Given a filter only mutating one value (previously written as "taking one parameter", we since then loosened it up to allow any number of parameters, but only one of which should be mutated by the hook), this could also be solved by using the return value.

That would mean that we'd lose the ability to break the chain of filters by returning false. This simplifies the logic, but may make it difficult to port some existing use cases. How does WordPress do chaining of filters?

In T212482#5103934, @pmiazga wrote:

If I understood properly what FilterHooks are, some of those Hooks could be safely replaced by service tagging. There is no need to listen to a hook when we need to build things. Example:
[...]
This reduces side effects to zero, as there is no Hook call and no arbitrary code execution, just a function that loads sets of services from MediaWikiServices.

Your example seems to have just pushed the hook call one level up the stack (now MediaWikiServices calls a bunch of registered callbacks instead of your code calling them directly), and possibly also requires that more things be created on every request rather than being determined when they are needed. Either way, I'm not sure that's much of an improvement.

Hooks::run() can be a factory, and depending on the $execution type can fire filter or action methods. The HookExecution can have methods like: abort() (only filter), fail() (only action) or getService(). This provides us control over what users can do (they can access only methods from the
HookExecution, because HookExecution provides services there will be no need to access global variables/singletons). Inside the HookExecution we could freeze some things (like make some services read-only, when returning a service we can return a clone/frozen version, so we're sure it doesn't have side effects). Then, most important bits of our system could have concrete HookExecution classes, something like Hooks/SpecialPagesInit that exposes an API to modify set of SpecialPages. By default, the HookExecution class can have get( $key ) and set( $key, $value ). Also, it can implement ArrayAccess, therefore in some cases, no changes to extensions have to be done (when we pass an array as the first argument, for example, ChangeTagsListActive`).

Do we really need complicated accessors wrapping everything just to try to enforce a policy on which services can be accessed from a hook callback? We discussed the same thing with respect to trying to force hook calls to use immutable objects in T212482#4841064 and T212482#4867960.

As final thought, I'd like to mention, that some time ago, I came up with the idea of T171515: Hooks system should support MediaWiki services. This is somewhat related to this task, in general, our Hooks system should utilize MediaWikiServices more often. That approach will help us reduce the number of static functions (at this moment most of the Hooks are static methods which are usually difficult to test), therefore it should minimize possible side effects.

Static functions aren't always bad. They'd probably be a better approach to wiring than the current "array full of Closure objects" we have in ServiceWiring.php, as mentioned at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/508972/5/includes/Rest/Router.php#152.

While calling hooks on services in MediaWikiServices may be a good idea when a service already exists, at what point are we putting too much into it? Would it really be a win for every extension to register a global public "FooBarHooks" service just to be able to hook hooks? Would it be better to register "private" services by allowing use of ObjectFactory specs (see T222409) when registering hooks?

In T212482#5164991, @Nikerabbit wrote:

Currently, passing values by reference makes it impossible for hook subscribers to know which types they are getting, making type hinting impossible. That causes issues like T214358.

The problem in T214358 seems to have been that the parameter was defined as \Language|string, was really \Language|\StubUserLang|string, and in a patch someone assumed it was just \Language. None of that seems to have much to do with passing values by reference beyond making it slightly harder to find where the string a later "filter" received was coming from.

Could the hook runner be extended do validate the types of mutable values after each subscriber returns?

The alternative would be that mutable values would need to be wrapped in container objects that expose interface to modify the value. I imagine something generic like new ModifiableValue( $type, $value ) that exposes get and set that enforce the type is possible, but being far from elegant and hiding the real type.

Both of those require defining the type in the first place, and then a bunch of code run on every hook function call to check it. Does this sort of problem really come up often enough to justify all that complexity? Could we just enable some logging in such situations to log the types when they actually need to be investigated?

In T212482#5211044, @Krinkle wrote:

@daniel Given a filter only mutating one value

Is that a given, though? T212482#4869654 never received a reply.

I just came across T157402: Provide a reliable way to pass information between hook handlers, "hooked" objects and I'm wondering how the use cases described there fit into the model proposed here.

In T212482#5234908, @daniel wrote:

I just came across T157402: Provide a reliable way to pass information between hook handlers, "hooked" objects and I'm wondering how the use cases described there fit into the model proposed here.

Currently, it doesn't seem this proposal does anything more to address the issue than the current system does. But,

• As mentioned in T212482#4869654, this proposal would seem to require more such communication.
• The current workaround is adding properties on relevant high-level objects that are passed into the hooks. Changes that result in no persistent objects being passed would invalidate that workaround.

In T212482#4869654, @Anomie wrote:

In T212482#4867960, @Krinkle wrote:

The point is to avoid having filters like "outputpage_render( OutputPage )" which don't relate to a tangible value, but a point in time ("event") given a monolithic object. Instead, I think these should have more granular filters like "outputpage_htmltitle", "outputpage_bodyclass", ""outputpage_modules", which have a dedicated purpose and a limited set of factors by which we support their variance. [..]

[..] It seems pretty likely that an extension might need to manipulate the HTML [..], the modules [..], and the body class [..]. Having such granular hooks means would need to register and coordinate across them all, and [..] its only option seems to be depending on global state [..]

Yeah, that'd be nasty. We already see that a lot in extensions that hook into Parser, and that's not a pattern we'd want to encourage further. My unverified gut-feeling is that these "filter" hooks would not actually any sort of coordination, though.

By virtue of only being allowed to vary on the given parameters (as our proposed design states), that implicitly also rules out the outcome of other hooks. If such outcome is relevant and the maintainer of the code the hooks are subject to is okay with that side-effect being officially supported, that probably means the code needs some kind of primitive to support that which could then be passed as parameter to the relevant hook. For the most part, though, I expect filters to be deterministic given their parameters and to generally not rely on outcome of other hooks.

As mentioned by Brion already, I think it's time we look at a practical example to see how this would turn out. Continuing further in theory only I think would leave us vulnerable to either wasting time or considering cases that might work out the way we think they would.

I created a draft of a guideline for extension points, see https://www.mediawiki.org/wiki/Wikimedia_Technical_Committee/Extension_point_guidelines.

Flagging for PET roadmapping review, to see where this fits in with the decoupling "expedition".

Changing hook signatures is hard, due to how interfaces work in PHP. This problem can be avoided by having each hook take a single parameter, using a type specific to the hook. In case of action hooks, this parameter would model an event. Consider a signature like afterPageDeleted( PageDeletedEvent $event ).

In T212482#7406864, @daniel wrote:

Changing hook signatures is hard, due to how interfaces work in PHP. This problem can be avoided by having each hook take a single parameter, using a type specific to the hook. In case of action hooks, this parameter would model an event. Consider a signature like afterPageDeleted( PageDeletedEvent $event ).

I thought about that during the discussion of T240307: Hook container with strong types and DI; it would re-allow the same kind of signature changes that were possible with the old hook system, plus the the event object would be a nice place for issuing deprecation warnings and for converting a legacy signature into the new siganture. It would also allow grepping for usage, which used to be a common problem - one wants to remove the last argument of the hook signature, which is not useful anymore, but how to tell which of the many subscribers of that hook actually use it?

OTOH, just like the interface, it would be static coupling that's fragile when refactoring (a little less fragile, as method signature <-> caller signature has less strict match conditions than interface signature <-> implementation signature, but the difference is not that much). Instead of versioned interfaces, you'd need versioned factory methods for the event object, which is not much of an improvement. And the code for calling and implementing hook handlers becomes less readable. So on the net it didn't seem like an improvement.

Change 838910 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] EXPERIMENT: Filter hooks and action hooks.

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

Consider a signature like afterPageDeleted( PageDeletedEvent $event ).

Of course, I like this. :) Doing something like this I think would allow the 'hook' runner to be pluggable too, or maybe configurable too. Sync, or async DeferredUpdate, or JobQueue, or X.

I'd suggest a more unified data model though, something more like:
afterPageDeleted( PageChangeEvent $event )
where PageChangeEvent can be used for the param to many of the page change hooks.

Or, perhaps, if we don't want to represent the change event, you could just just provide a page state before and page state after objects as params, and allow the hook function to do whatever they need.
afterPageDeleted( PageState $before PageState $after )

Not sure which is better, but maybe ^ would be more flexible?

FWIW, I find 'filter' a strange name for the type of hook that can mutate state. I'd expect a filter to be a boolean filter operation. I guess yall are saying it can cancel the calling of future callbacks, which is like a filter...but mutating state in a filter sounds weird :)

In T212482#8290304, @Ottomata wrote:

FWIW, I find 'filter' a strange name for the type of hook that can mutate state. I'd expect a filter to be a boolean filter operation. I guess yall are saying it can cancel the calling of future callbacks, which is like a filter...but mutating state in a filter sounds weird :)

I think Timo originally borrowed the terminology from WordPress, compare https://kinsta.com/blog/wordpress-hooks/#hooks-vs-actions-vs-filters.

To me, "actions" are events. And filters are... I dunno, mutators? Mutables? Mogrifiers?...

In T212482#8290286, @Ottomata wrote:

I'd suggest a more unified data model though, something more like:
afterPageDeleted( PageChangeEvent $event )
where PageChangeEvent can be used for the param to many of the page change hooks.

If we use the same event class to represent multiple types of changes, we run into the problem that some properies make sense for one kind of change, but not for another. E.g. and edit has an old revision and a new revision, a delete doesn't. A page move has an old tilte and a new title, but an edit doesn't. Etc.

We could of course have a hierarchy of event types, where PageDeletedEvent IS A PageChangeEvent.

Or, perhaps, if we don't want to represent the change event, you could just just provide a page state before and page state after objects as params, and allow the hook function to do whatever they need.
afterPageDeleted( PageState $before PageState $after )

Not sure which is better, but maybe ^ would be more flexible?

The system I have in mind relies on there bing a single object representing the change. But we could of course have a PageChangeEvent that has methods like getPageStateBefore() and getPageStateAfter().

Since event objects are expected to be serialized and queued, I'd expect them to be relatively lightweight, though. So in my mind, the page state would not include the HTML, and probably also not the wikitext. It would however contain sufficient information to retrieve them (by revision ID and render ID).

And filters are... I dunno, mutators? Mutables? Mogrifiers?...

Transforms?

We could of course have a hierarchy of event types, where PageDeletedEvent IS A PageChangeEvent.

This could work!

But we could of course have a PageChangeEvent that has methods like getPageStateBefore() and getPageStateAfter().

Could also work!

Since event objects are expected to be serialized and queued, I'd expect them to be relatively lightweight, though. So in my mind, the page state would not include the HTML, and probably also not the wikitext.

Agree. And this is exactly the data modeling exercise we are doing now in T308017: Design Schema for page state and page state with content (enriched) streams: trying to figure what does and does no belong in a 'page state change' event. We're going to need the ability to have page_change event with enriched data in other streams (with html or wikitext or ORES scores T317768, etc.), but to do that we'll just extend the page_change model to I dunno, page_change_with_wikitext. We could probably do the same in these MediaWiki event value classes if we had to. (I don't think we'll have to :p )

I'd LOVE if these MW event classes could JSON serialize to the same (or very close to the same) event schema data model we'd produce to Kafka. This would be just...wow...amazing.

BTW, The reason we are moving to preferring a single entity change model rather than multiple models for each change, is that generally, a consumer of these events is trying to keep track of what is happening to the entity. In Kafka, work and state are distributed via keyed partitions, so you want all events of with the same entity id to go to the same worker, and you'd prefer if all the changes that happen to that entity to be in order. If there are different event data models for each kind of change, then those will be in different streams, and there will be no way to guarantee the order in which the changes will be consumed, or which workers they will be consumed by, because they are in different topic-partitions in Kafka. A big shuffle and order by has to be done before work can be distributed.

In T212482#8293384, @Ottomata wrote:

BTW, The reason we are moving to preferring a single entity change model rather than multiple models for each change, is that generally, a consumer of these events is trying to keep track of what is happening to the entity.

Modeling an event as change-of-state-of-entity is intruiging, but I see two prolems with it:

1. you may want additional meta-data about the change itself, things that do not update the resource. E.g. the IP and edit came from.
2. the state of an entity is potentially huge, e.g. a "page" could potentially be modeled as containing all its revisions, or maybe even all possible renderings... representing "before" and "after" fully in the event itself becomes impossible.

Perhaps it would make sense to model page changes a bit like git commits: they have meta-data, and contain a patch against a base state.

Buut we are getting off-topic for this ticket :)

Yaya, this discussion is for T308017: Design Schema for page state and page state with content (enriched) streams. I'll respond to your comment there.

But yeah, these are all the things we are balancing. Basically, we want a data model that can be used as a changelog stream, similar to what might be in a MySQL replication binlog, but at a higher (denormalized) domain level.

In T212482#8290395, @daniel wrote:

In T212482#8290304, @Ottomata wrote:

FWIW, I find 'filter' a strange name for the type of hook that can mutate state. I'd expect a filter to be a boolean filter operation. I guess yall are saying it can cancel the calling of future callbacks, which is like a filter...but mutating state in a filter sounds weird :)

I think Timo originally borrowed the terminology from WordPress, compare https://kinsta.com/blog/wordpress-hooks/#hooks-vs-actions-vs-filters.

To me, "actions" are events. And filters are... I dunno, mutators? Mutables? Mogrifiers?...

+1, "filter" does sound a bit odd. I like "mutate/mutator" vs "listen/listener".

I am in favor of the single-parameter hook id, where the param is some kind of data object.
Significant changes to the data would likely still be cumbersome to manage, but it would at least reduce some change synchronization:

Adding a new param right now would probably look something like this:

1. Change hook invocation to overload with new arg
2. Changing all subscribers, which must start to accept a new optional argument ahead of time (while interface doesn't yet have it)
3. Change the hook interface to accept the new argument
4. Change all subscribers to make new arg non-nullable

With a single-param system:

1. Change hook invocation to overload with new arg
2. Change hook params object to accept new "argument"
3. Update only the subscribers that have business with the new data

I also like the separation of filter/mutable and action/listener hooks:

• listeners could more easily be moved post-send (insofar applicable)
• listeners would receive reliable data (whereas right now they could be receiving data that a later subscriber will alter again, or may not persist)

That said, while I like the separation, I'm wary of it introducing too much additional work.
I believe the following is true in reality:

• Hooks are often only implemented on an as-needed basis. There often isn't time or a realization that a(nother) hook would be desirable.
• Most existing hooks are mutators.
  • A quick count of functions in HookRunner reveals 350+ with variables passed by ref (explicitly intended to mutate data) and 150+ without (although some of those are still used to mutate, either by altering an object arg, or by interrupting process by returning false). That's 350+ "mutable" hooks vs. 150+ (possibly lower) "listener" hooks.
• Given an existing hook with the required data is available, subscribers will tend to use that one.
  • If only a mutator hook is available, it will be subscribed to - even if the only intent is listening.

If these end up being totally separate instances, I fear it may often end up being too much work to provide both hook types.
For those creating/invoking new hooks, there usually is 0 incentive to provide more than what is required at the time.
Even more so because it can often take multiple years for a use case to emerge where "the other, not provided type of hook" would be useful.
Unless creating a separate listener hook requires (almost) no additional work, or unless there's a mechanism that heavily discourages mutator-only hooks, I would expect we end up with the same pattern of mostly-mutable hooks that even pure listeners subscribe to.

And from a subscriber's POV, they'll often use whatever is available, rather than implementing that other kind.
And often for good reason (e.g. lack of time, lack of permissions to alter code base where the hook lives, compatibility with older code desirable, ...)

I think we should strive for having all types (filter/mutable + actions/listener) available by default any time a hook is called.
I realize that having multiple hook variations is probably overkill for 90% of them, but that's how we ended up where we are in the first place: we only build what is needed, then find them being used for different purposes years later.

About action/listeners hooks: thus far I've mostly seen this being discussed in the context of moving heavier workloads post-send, but I think we also have a slew of pre-send listeners that need to perform related side-effects immediately (note: I haven't actually gone over them, the actual number may anywhere from tiny to massive)
While this type could also subscribe to a mutable hook, they have no guarantee that data doesn't change later on, or even ends up persisting at all.

Broadly speaking, we probably have 3 kinds of subscribers:

1. ones that need to mutate data, or alter execution flow (e.g. when returning false prevents other code from running)
2. ones that listen to data to perform related side-effects immediately
3. ones that listen to data for later consumption/processing

#1 & #2 are pre-send by nature; #3 can be post-send.
#1 would have to happen prior to whatever action the hook precedes (e.g. storing data); #2 & #3 ideally happen afterwards, once data is final.
#1 would have to be mutable; #2 & #3 immutable.

This would probably make the ideal scenario something like:

$hookContainer->run( 'MutateMyHook', [ $param ], [ 'abortable' => true ] );

// execute code, persist data

$hookContainer->run( 'ListenMyHook', [ $param ], [ 'abortable' => false ] );

DeferredUpdates::addCallableUpdate(
	function () use ( $event ) {
		$hookContainer->run( 'DeferredListenMyHook', [ $param ], [ 'abortable' => false ] );
	},
	DeferredUpdates::POSTSEND,
	$dbw
);

This, too, adds a bunch of extra boilerplate. It would be great if this too could end up being "magic"-ed away for the most part and become standard implementation (while still being possible to deviate from this standard - for some hooks, not all of these may make sense)

TL;DR: while I think the separation of types of hooks has multiple benefits, I don't think we'd reach them unless we break the pattern. Invoking multiple types of hooks should require minimal extra work; ideally, it would be the default.
But since these types of hooks would likely mostly operate on the same data & follow a similar structure, I'm hopeful a solution can be found where no extra work (or at worst only a negligible amount) would be required to create/invoke them.

But since these types of hooks would likely mostly operate on the same data & follow a similar structure, I'm hopeful a solution can be found where no extra work (or at worst only a negligible amount) would be required to create/invoke them.

Perhaps some interface where the when/how the hook is called can be parameterized, I suppose at hook registration time?

In T212482#8305686, @Ottomata wrote:

But since these types of hooks would likely mostly operate on the same data & follow a similar structure, I'm hopeful a solution can be found where no extra work (or at worst only a negligible amount) would be required to create/invoke them.

Perhaps some interface where the when/how the hook is called can be parameterized, I suppose at hook registration time?

One way to do this would be by a convention around the handler method's name:

• modifyFooBarResult would be called immediately
• afterFooBarResult would be called eventually (ideally, with an immutable version of the parameter)

About action/listeners hooks: thus far I've mostly seen this being discussed in the context of moving heavier workloads post-send, but I think we also have a slew of pre-send listeners that need to perform related side-effects immediately (note: I haven't actually gone over them, the actual number may anywhere from tiny to massive)

I'm very curious about this one - so far, I have been operating on the assumption that this should not be needed. And I can't think of a use case were this is actually a requirement. Even updating links tables for core happens post-send, outside the primary transaction of an edit...

Unless creating a separate listener hook requires (almost) no additional work, or unless there's a mechanism that heavily discourages mutator-only hooks, I would expect we end up with the same pattern of mostly-mutable hooks that even pure listeners subscribe to.

Perhaps there should be two hook types, and two listener types, but with a mapping that isn't quite 1:1. Hook types:

• ParameterHook called before the operation, to allow the parameters to be modified
• ResultHook called after an operation, to allow the result to be modified and to notify listeners of the outcome.

Handler types:

• Modifier: called immediately, calls setters, must not update persistent state
• Listener: called eventually, calls only getters, may update persistent state

ParameterHooks would call only modifier handlers, using the "modify" prefix on the method name.

ResultHooks would also call modifier handlers, but would also schedule a call to listeners to be executed post-send, using the "after" method prefix.

I'd still like to avoid listeners calling setters - that would screw with other listeners that look at the same object. Perhaps the implementation of a ResultData object could be split in two, an immutable base class, and a subclass adding setters?

It's not yet clear to me how to distinguish listeners from result modifiers during registration, if the hook name is the same. I'll think about it.

In T212482#8307043, @daniel wrote:

About action/listeners hooks: thus far I've mostly seen this being discussed in the context of moving heavier workloads post-send, but I think we also have a slew of pre-send listeners that need to perform related side-effects immediately (note: I haven't actually gone over them, the actual number may anywhere from tiny to massive)

I'm very curious about this one - so far, I have been operating on the assumption that this should not be needed. And I can't think of a use case were this is actually a requirement. Even updating links tables for core happens post-send, outside the primary transaction of an edit...

I've looked around just a little bit, and examples were fairly hard to come by, so it looks as if this is a rather small number.

I did find a couple in Flow, though (a great source for hooks abuse):

This CategoryViewer::doCategoryQuery hook subscriber is basically a listener - it'll subscribe to this hook's data, and use it to mutate data in another hook: CategoryViewer::generateLink.

Something similar happens with RC onChangesListInitRows where it listens to a hook exposing a full dataset, preload a batch of related stuff, and then mutates other hooks' (onChangesListInsertArticleLink, onOldChangesListRecentChangesLine & EnhancedChangesList::getLogText) data.

Wikibase repo seems to subscribe to that same hook onChangesListInitRows in order to prefetch data to be reused elsewhere.

Then there are a couple of other listeners (onRevisionUndeleted + onArticleUndelete and onTitleMoveStarting + onPageMoveCompleting) where it looks like handling the changes deferred would've led to rendering errors (I could be misinterpreting the comments)

I would imagine there may be a few more handlers here and there that only intend to listen to (final) data in order to alter output elsewhere (in another hook or some other component, ...)

These above examples usually aren't shining examples of solid code architecture and could probably also have been dealt with in different, more elegant ways.
Making sure that certain hooks expose more data, or function differently, could've solved some of these cases in a different way (although that does raise the point of "not knowing use cases ahead of time" and "potentially not being able to make those changes")

Note also that these examples are 7+ years old; a lot of MW's internal mechanics have greatly changed/improved since then.
Perhaps a more modern codebase like Wikibase would be worth a closer look to check whether similar examples continue to be a thing noawadays.

I concede that so far, these instances appear to be relatively small in number, and perhaps tend form a pattern that we may not want to encourage further anyway.

@matthiasmullie thank you for the investigation!

I have some unfinished thoughts around listeners signalling when and how they want to be called using the methdo name prefix. E.g. an async listener would implement afterPageMove(), while a listener that wants to be called synchronously while the transaction is in progress may implement duringPageMove(). Or something like that.

I imagine this would be detected while registering the hook, so we don't have to do anything with reflection when calling handlers. As Tim pointed out to me the other day, that code is "super hot", and needs to be optimized for performance.

Note to self (mostly) about naming conventions for parameter classes and handler methods:

• filter hooks (alter hook, value hook): MediaWiki\FooBarValue -> alterFooBarValues( FooBarValue ). May also be plural ("XyzValues"). Base class is MediaWiki\Hooks\MutableValue.
• event hooks: MediaWiki\FooBarEvent -> afterFooBarEvent( FooBarEvent ). Base class is MediaWiki\Hooks\Event. We could also use "handle"...
• all other hook names use "on": FooBarCompleted -> onFooBarCompleted( ... )

Should hook names be FQNs?

It seems to me that Symfony's event system is close to what we want to aim for: https://symfony.com/doc/current/components/event_dispatcher.html. Symfony also has mechanism to integrate "events" with async message queues: https://github.com/php-enqueue/enqueue-bundle

Interesting!

Also, from the convo in December's tech leadership CoP meeting, I started thinking about how what we want for T291120: MediaWiki Event Carried State Transfer - Problem Statement is pretty similar to what is in MW's logging table, except we need the data to be structured, comprehensive and consistent (meaning no missing state changes). In T120242: Eventually Consistent MediaWiki State Change Events, one of the solutions outlined is the 'Transactional Outbox' pattern, which is kinda similar to a comprehensive+structured logging table from which we can generate and externalize state change events. I betcha we could tie these ideas together somehow.