Page MenuHomePhabricator

Stop injecting LanguageNameLookup into WikibaseValueFormatterBuilders
Closed, ResolvedPublic

Description

Currently, we inject the LanguageNameLookup service into the WikibaseValueFormatterBuilders class (WikibaseRepo.DefaultValueFormatterBuilders service). Since that service is deprecated, we should avoid that.

I think one option would be to use the LanguageNameLookupFactory service introduced in T281712. Most (all?) of the WikibaseValueFormatterBuilders methods take a FormatterOptions argument, which should(?) contain a language option (ValueFormatter::OPT_LANG); if we inject a language name lookup factory into the formatter builders, then each time we actually build a specific formatter from a set of options, we can use that factory to create a specific language name lookup.

That said, I’m not sure that the options actually contain a language each time. I suggest something like this (sketch code):

private function getLanguageNameLookup( FormatterOptions $options ) {
	if ( $options->hasOption( ValueFormatters::OPT_LANG ) ) {
		return $this->languageNameLookupFactory->getForLanguageCode( $options->getOption( ValueFormatters::OPT_LANG ) );
	} else {
		// TODO wfDeprecated() probably?
		return $this->languageNameLookupFactory->getForLanguage( WikibaseRepo::getUserLanguage() );
	}
}

Event Timeline

Change 943573 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders

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

I decided against the wfDeprecated() – I’m fairly sure all the paths that reach this code will have the $options populated with a default language already (because some other code called there, notably FormatterLabelDescriptionLookupFactory::getLabelDescriptionLookup(), also assumes the options contain a language), and if CI passes that seems safe enough for me.

Change 943573 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders

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

Task remains open; next week (or perhaps this week, if we backport), we can check if there are any “FormatterOptions without OPT_LANG or OPT_LANGUAGE_FALLBACK_CHAIN” Wikibase warnings in Logstash. If there aren’t, we can clean up the code a bit further.

Change 944848 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@wmf/1.41.0-wmf.20] Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders

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

Change 944848 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@wmf/1.41.0-wmf.20] Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders

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

Mentioned in SAL (#wikimedia-operations) [2023-08-02T13:26:50Z] <lucaswerkmeister-wmde@deploy1002> Started scap: Backport for [[gerrit:944848|Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders (T281726)]]

Mentioned in SAL (#wikimedia-operations) [2023-08-02T13:28:26Z] <lucaswerkmeister-wmde@deploy1002> lucaswerkmeister-wmde: Backport for [[gerrit:944848|Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders (T281726)]] synced to the testservers mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-08-02T13:35:29Z] <lucaswerkmeister-wmde@deploy1002> Finished scap: Backport for [[gerrit:944848|Inject LanguageNameLookupFactory into WikibaseValueFormatterBuilders (T281726)]] (duration: 08m 39s)

Alright, I think we can check back on logstash in a few days (mediawiki-warnings, channel:Wikibase AND FormatterOptions).

Alright, I think we can check back on logstash in a few days (mediawiki-warnings, channel:Wikibase AND FormatterOptions).

No results in the past 30 days other than the manual test message I did and a random timeout that happened to have FormatterOptions in the stack trace. I think we can go ahead and just throw an error.

Change 947858 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Require language in options in WikibaseValueFormatterBuilders

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

Change 947858 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Require language in options in WikibaseValueFormatterBuilders

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