Page MenuHomePhabricator

Evaluate how to structure internal calls to TemplateData in PHP
Open, MediumPublic

Description

Background

This is a follow-up from T238954 and https://gerrit.wikimedia.org/r/552620 and parsoid:DataAccess.php

Parsoid
	$ret = [];
	Hooks::runWithoutAbort( 'ParserFetchTemplateData', [ [ $title ], &$ret ] );
TemplateData
class TemplateDataHooks {
	/** …*/
	public static function onParserFetchTemplateData( array $tplTitles, array &$tplData ): void {
		$tplData = [];
		// …
			$tplData[  ] = 
	}
}
Problem

The short-term approach (to unblock the Parsoid JS-PHP migration) was to have the Parsoid-PHP code run a hook that doesn't normally exist in MediaWiki, with a name specific to the TemplateData extension. And for TemplateData to listen to that hook and mutate the array that is by-ref passed in.

This means that Parsoid has no hard dependency on TemplateData. And that by itself is desirable and something we'll probably want to retain.

However, it also creates a fragile situation because:

  • The hook may have multiple listeners. It is currently unpredictable/undefined what will or should happen. Which one wins? Why? Can that be controlled? What about the wasted computations for values that end up losing?
  • The format used of the structure transferred between them by-ref is undocumented and untested.

Looking at the principles outlines in the (upcoming) RFC at T212482, this extension point we want to have here seems to fit the description of a "service" much better than the description of a filter or action hook. I think it would be reasonable to require that only one extension can provide this service at a given time. If we want other extension to influence how TemplateData behaves, we'll want to come up with smaller more granular hook points within TemplateDate, without exposing a hook like this directly.

If we do it as a PHP class service, that leaves us with (at least) two natural paths forward.

Optinos
1. Normal soft-dependendency (explicitly for Extension:TemplateData)

We would move the logic in TemplateData to separate class (or to an existing class, e.g. TemplateDataBlob).

Parsoid
if ( ExtensionRegistry->isLoaded( 'TemplateData' ) ) {
   $ret = TemplateDataSomething::getMultiFromDb( , $titles );
} else {
  $ret = [];
}
2. Standarised service interface (in core, or in Parsoid extension)

A php interface like IParsoidTemplateDataLookup with a null/no-op default implementation registered in ServiceWiring. The TemplateData extension would implement that service interface and overwrite the default service object if it is installed.

TemplateData
class MWTemplateData\ParsoidTemplateDataLookup implements MWParsoid\…\IParsoidTemplateDataLookup {
  
}
# ServiceWiring
'ParsoidTemplateDataLookup' => function ( $services ) : MWParsoid\…\IParsoidTemplateDataLookup {
   return new MWTemplateData\ParsoidTemplateDataLookup(  );
}
Parsoid
$ret = $services->getParsoidTemplateDataLookup()->getMulti( $titles );

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Note that while the code currently lives in a Parsoid "extension", that's intended as a temporary measure. The intention is that Parsoid will be used as a composer library from core in the near-ish future.

The hook may have multiple listeners. It is currently unpredictable/undefined what will or should happen. Which one wins? Why? Can that be controlled? What about the wasted computations for values that end up losing?

Not any more so than any other hook in MediaWiki.

  1. Normal soft-dependendency (explicitly for Extension:TemplateData)

This violates our normal practice of Core not knowing about arbitrary extensions.

  1. Standarised service interface (in core, or in Parsoid extension)

It seems a bit strange to me to be defining single-purpose no-op global public services in core. We already have the hook mechanism for this sort of thing.

ssastry triaged this task as Medium priority.Feb 14 2020, 4:40 PM
ssastry moved this task from Needs Triage to Tech Debt / Big changes on the Parsoid board.