Page MenuHomePhabricator

Don’t load Repo settings in PHP entry point
Closed, ResolvedPublic

Event Timeline

I looked a bit into the Repo settings to see how this could be done. One thing that surprised me is that Repo barely uses the “config values can be callbacks to compute the real value on-demand” feature, which is used much more in Client – in Repo, the only three callbacks are:

'dataRightsUrl' => function() {
	return $GLOBALS['wgRightsUrl'];
},

'dataRightsText' => function() {
	return $GLOBALS['wgRightsText'];
},

'conceptBaseUri' => function() {
	$uri = preg_replace( '!^//!', 'http://', $GLOBALS['wgServer'] );
	return $uri . '/entity/';
},

This might make Repo a much more attractive candidate for porting to MediaWiki’s standard config system than Client.

Besides that, if we don’t load the default config in the PHP entry point, we’ll have the same problem as in Client (see T256252 and I193008a732): if the value from LocalSettings (written for a Wikibase where the default config was loaded in the PHP entry point) and the default value are both nonempty arrays, then we won’t know if the two should be merged, or if the LocalSettings value should overwrite the default value completely.

The default config has the following nonempty arrays:

  • string-limits: Associative array of arrays. I don’t think we even support unsetting any entries of this array (code like WikibaseRepo::getDefaultInstance()->getSettings()->getSetting( 'string-limits' )['VT:monolingualtext'] doesn’t seem to account for that option), so wfArrayPlus2d should be fine.
  • urlSchemes: List of strings. Probably the biggest problem – we can’t distinguish between $wgWBRepoSettings['urlSchemes'][] = 'gopher'; and $wgWBRepoSettings['urlSchemes'] = [ 'gopher' ];, and both of these seem to be valid things that one might want to do, at least in principle (Gopher is of course a bit silly as an example). I think here we’ll have to bite the bullet and introduce a new way to remove URL schemes from the list ($wgWBRepoSettings['mailto'] = false? though then the array has mixed string and numeric keys), and then send an email around and tell people to update their configs accordingly.
  • entityDataFormats. List of strings. I believe the default value is the full list of possible entity data formats, so the only useful change to this list is to remove entries from it – so the custom value should probably overwrite the default value, without merging.
  • canonicalLanguageCodes: Associative array of strings. I think it would be pointless for anyone to remove entries from this, so we can probably use a standard array_merge. But if we want to support removing canonical language codes, we can update our code to treat false values like absent keys, and then tell people to change LocalSettings code like unset( $wgWBRepoSettings['canonicalLanguageCodes']['simple'] ); to something like $wgWBRepoSettings['canonicalLanguageCodes']['simple'] = false;.
  • globeUris: Associative array of strings. Behaves like canonicalLanguageCodes, I’d say.
  • pagePropertiesRdf: Associative array of arrays. I’m not sure why anyone would configure this, to be honest. wfArrayPlus2d, I guess, and likewise use false to support removing a page property?

I looked a bit into the Repo settings to see how this could be done. One thing that surprised me is that Repo barely uses the “config values can be callbacks to compute the real value on-demand” feature, which is used much more in Client – in Repo, the only three callbacks are:

I think it's mostly because the client is being called way more often (around 100 times more) than repo. So performance issues in repo doesn't show itself in the big picture (but it doesn't mean we should ignore those TBH). Can we make sure using mediawiki standard config system would not decrease the performance?

This might make Repo a much more attractive candidate for porting to MediaWiki’s standard config system than Client.

I'd be a fan of changing to that but I feel it would be lots of changes. Maybe we can turn RepoSettings to a facade of the actual config? There are some issues like that the standard config doesn't support set method (which I think is good thing) but changing/fixing it might be quite fun (or hacky)

Change 615574 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Don’t load Repo settings in PHP entry point

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

Change 615579 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/WikibaseMediaInfo@master] Avoid using $wgWBRepoSettings directly

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

Change 615579 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Avoid using $wgWBRepoSettings directly

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

Change 615574 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Don’t load Repo settings in PHP entry point

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