Don’t use WikibaseRepo when initializing Client pageterms API
Closed, ResolvedPublic
Actions

Description

Currently, when Wikibase Client initializes the pageterms API, it uses WikibaseRepo code if the Repo is enabled:

public static function newFromGlobalState( ApiQuery $apiQuery, string $moduleName ): self {
		// FIXME: HACK: make pageterms work directly on entity pages on the repo.
		// We should instead use an EntityIdLookup that combines the repo and the client
		// implementation, see T115117.
		// NOTE: when changing repo and/or client integration, remember to update the
		// self-documentation of the API module in the "apihelp-query+pageterms-description"
		// message and the PageTerms::getExamplesMessages() method.
		if ( ExtensionRegistry::getInstance()->isLoaded( 'WikibaseRepository' ) ) {
			$repo = WikibaseRepo::getDefaultInstance();
			$termBuffer = $repo->getTermBuffer();
			$entityIdLookup = $repo->getEntityContentFactory();
		} else {
			$client = WikibaseClient::getDefaultInstance();
			$termBuffer = $client->getTermBuffer();
			$entityIdLookup = $client->getEntityIdLookup();
		}

This is a hack dating back to 2015 (T115117#1719166). We should find some better solution for this, depending on what the expected behavior of that API on repo wikis is.

Related Objects
Search...

Status	Assigned	Task
Resolved	• Lucas_Werkmeister_WMDE	T255880 Remove uses of PHP classes from Wikibase Repo in Client and Lib
Resolved	• Lucas_Werkmeister_WMDE	T255882 Don’t use WikibaseRepo when initializing Client pageterms API
Resolved	Ladsgroup	T256255 Investigate how pageterms API is used on Wikidata itself
Resolved	• Lucas_Werkmeister_WMDE	T257658 Introduce new Repo entityterms API and remove hack from Client pageterms API

Event Timeline

• Lucas_Werkmeister_WMDE created this task.Jun 19 2020, 4:27 PM

If the API behavior should stay the same, then I imagine we could resolve the direct Client→Repo dependency by registering some kind of “term buffer + entity ID lookup” stash in Lib, and Repo fills it with its services upon initialization, and Client fills it with its services only if it wasn’t already initialized by Repo. So the two extensions would coordinate via Lib instead of talk directly to one another, so to speak. Something similar could probably also be done if Product says the API should first return the terms of the current entity and otherwise the terms of the entity linked to the current page, or something like that.

If the expected behavior is that the API always returns the terms of the entity connected to the current page, then I think we can simply remove the hack :)

• Lucas_Werkmeister_WMDE moved this task from Backlog to Sprint 2 on the Wikibase Extension Decoupling and Registration board.Jun 24 2020, 10:47 AM

• Lucas_Werkmeister_WMDE edited projects, added Wikibase Extension Decoupling and Registration (Sprint 2); removed Wikibase Extension Decoupling and Registration.

One solution would be to move the “page terms of this item” functionality to a separate API module.

In T255882#6240846, @Lucas_Werkmeister_WMDE wrote:

If the API behavior should stay the same, then I imagine we could resolve the direct Client→Repo dependency by registering some kind of “term buffer + entity ID lookup” stash in Lib, and Repo fills it with its services upon initialization, and Client fills it with its services only if it wasn’t already initialized by Repo. So the two extensions would coordinate via Lib instead of talk directly to one another, so to speak. Something similar could probably also be done if Product says the API should first return the terms of the current entity and otherwise the terms of the entity linked to the current page, or something like that.

I would be very much in favor of having simpler logic in our codebase, this seems complicated (meaning harder to maintain and more likely to have bugs and harder onboarding). Sometimes complicated logic is inevitable, like high concurrency, large scale caching, etc. But this case can be resolved by having other API modules (all boils down to what product decide of course) but I would say complicated logic is a negative point for having everything in one place.

Tarrow moved this task from Sprint 2 to Sprint 3 on the Wikibase Extension Decoupling and Registration board.Jul 8 2020, 12:33 PM

Tarrow edited projects, added Wikibase Extension Decoupling and Registration (Sprint 3); removed Wikibase Extension Decoupling and Registration (Sprint 2).