Page MenuHomePhabricator

Provide a reliable way to pass information between hook handlers, "hooked" objects
Closed, DeclinedPublic

Description

Extensions use hooks to insert functionality at different places of mediawiki, but there is no proper way to share non-core data between hook handlers. Yet this is often needed, for example the Cite extension needs to use the same Cite object between its different parser hooks, FlaggedRevs needs the same FlaggableWikiPage object between its various hooks, etc.

Workarounds against this limitation consist mostly in adding a custom field to the shared core object or making a static instance of the needed extension object. The former is a hack, even if allowed by PHP it should be avoided, and this can require cloning the objects or checking if they are the same. The latter is unsatisfactory, there may be several instances of the underlying core object or it may change in unanticipated ways (when anticipated, this requires clearing and recreating the instance).

An additional issue is that all hook handlers are forced to be bundled together in a huge class of static methods. This has significant drawbacks: the code is hard to follow, the dependencies are obscure and it can't be unit tested. The underlying problem is the same: hook handlers are static, not linked to the objects they're meant to extend the functionality of.

As such, we need to provide extensions with a unified and reliable way to pass information between hooks, and the flow of it should be clearer.

One possibility is to "hook" an extension object to a core object, for example Cite to Parser and FlaggableWikiPage to WikPage.
Then a hook run from the core object has its handler as a method of the hooked extension object.

Event Timeline

Cenarium renamed this task from Provide a way for passing information between hook handlers to Provide a reliable way to pass information between hook handlers, "hooked" objects.Feb 8 2017, 2:15 PM
Cenarium updated the task description. (Show Details)
daniel added a subscriber: daniel.

This RFC should be updated to clearly propose one or more solutions to the stated problem, and discuss them in detail. It should ideally also discuss alternative approaches.

Krinkle added a subscriber: Krinkle.

See also T212482, which outlines a proposal based on an architecture direction presented at TechConf 2018 (notes).

In a nut shell: We'd like to reduce the amount of state around hooks and make them more deterministic. As such, I think the specific proposal described here for hooks in general is not something TechCom is likely to approve.

This task, though, mentions two very specific problems that I think many developers have faced, and is an example where our current hook system does not work well. As such, I think it's worth discussing further in a reduced scope:

  • Parser: What problems are extensions like Cite currently facing? What is the best practice for doing that right now? Should the way Parser hooks work be changed? If so, what should that look like.

I didn't understand the issues around FlaggedRevs/WikiPage very well currently, but that's also an area I'm less experienced with. The same questions could apply to that as well.

I'd very much like your input about T212482. After that RFC has settled, I recommend TechCom revisit revisit this RFC and/or to invite the stewards of the Parser and WikiPage code bases to also think about the above questions.

Krinkle removed a project: TechCom-RFC.
Krinkle added a project: TechCom-RFC.
kchapman changed the task status from Open to Stalled.Jul 22 2019, 5:05 PM
kchapman added a subscriber: kchapman.

Stalled and in conflict of the hook system overhaul plan

Declining per my previous comment.

Isn't this actually resolved with the new hooks system? Hook handlers are now object instances with a service injection mechanism, so shared information can be exposed by a shared service.

Isn't this actually resolved with the new hooks system? Hook handlers are now object instances with a service injection mechanism, so shared information can be exposed by a shared service.

Even better, just use member fields on a handler object that implements multiple hook interfaces. I'm not a big fan of information flow via members, but it seems acceptable for this use case. Especially since other options seem worse.