Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ExtensionRegistry: Remove exporting and caching of wgExtensionCredits

Browse source 
This data is the same as the 'credits' data that is already compiled,
cached and made available via ExtensionRegistry.

Similar to various other configuration variables previously, the
$wgExtensionCredits variable is now also required to only be used
for providing input to the system (e.g. from LocalSettings.php,
or from legacy extension PHP entry points). It is no longer
supported to use this variable to reliably read out a full view
of all extension credits (specifically those registered via
extension.json).

Doing so had the downside of adding processing cost to every
web request, as well as taking one the single largest portion
of the ExtensionRegistry APCu cache key, which in PHP7+ incurs
a linear cost for every string value, string key, of every
(sub)array in this huge structure; and does to on every request
just in case something reads from $wgExtensionCredits.

The new method to access this information reliably is owned
by SpecialVersion for now (could be moved elsewhere). This
also makes the merging logic more testable and incurs it on-demand
rather than upfront.

Details:

* Move 'type' internally from NOT_ATTRIBS to CREDIT_ATTRIBS.
  These two arrays behave identically for most purposes (they are
  both used to mean "don't export this as a global attribute").

  By placing it in CREDIT_ATTRIBS it becomes complete and makes
  it easy to refer to in docs. Previously, extractCredits()
  read the 'type' key outside the loop for CREDIT_ATTRIBS.

* Remove redundant code in ApiBase.php, that is now more obviously
  redundant. Looks like a left-over or merge conflict mistake
  from when ExtensionRegistry was first introduced.

Bug: T187154
Change-Id: I6d66c58fbe57c530f9a43cae504b0d6aa4ffcd0d
This commit is contained in:
Timo Tijhof 2020-05-25 19:47:06 +01:00 committed by Krinkle
parent d4df7daa3b
commit 5d8f62bdd3
8 changed files with 62 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
includes/DefaultSettings.php
View file

@ -7760,19 +7760,23 @@ $wgAutoloadClasses = $wgAutoloadClasses ?? [];
$wgAutoloadAttemptLowercase = false;
/**
* An array of information about installed extensions keyed by their type.
* Add information about an installed extension, keyed by its type.
*
* All but 'name', 'path' and 'author' can be omitted.
* This is for use from LocalSettings.php and legacy PHP-entrypoint
* extensions. In general, extensions should (only) declare this
* information in their extension.json file.
*
* The 'name', 'path' and 'author' keys are required.
*
* @code
* $wgExtensionCredits[$type][] = [
* $wgExtensionCredits['other'][] = [
* 'path' => __FILE__,
* 'name' => 'Example extension',
* 'namemsg' => 'exampleextension-name',
* 'author' => [
* 'Foo Barstein',
* ],
* 'version' => '1.9.0',
* 'version' => '0.0.1',
* 'url' => 'https://example.org/example-extension/',
* 'descriptionmsg' => 'exampleextension-desc',
* 'license-name' => 'GPL-2.0-or-later',
@ -7810,6 +7814,8 @@ $wgAutoloadAttemptLowercase = false;
*
* - license-name: Short name of the license (used as label for the link), such
* as "GPL-2.0-or-later" or "MIT" (https://spdx.org/licenses/ for a list of identifiers).
*
* @see SpecialVersion::getCredits
*/
$wgExtensionCredits = [];

11
includes/api/ApiBase.php
View file

@ -1942,7 +1942,8 @@ abstract class ApiBase extends ContextSource {
'namemsg' => null,
'license-name' => null,
];
foreach ( $this->getConfig()->get( 'ExtensionCredits' ) as $group ) {
$credits = SpecialVersion::getCredits( ExtensionRegistry::getInstance(), $this->getConfig() );
foreach ( $credits as $group ) {
foreach ( $group as $ext ) {
if ( !isset( $ext['path'] ) || !isset( $ext['name'] ) ) {
// This shouldn't happen, but does anyway.
@ -1957,14 +1958,6 @@ abstract class ApiBase extends ContextSource {
array_intersect_key( $ext, $keep );
}
}
foreach ( ExtensionRegistry::getInstance()->getAllThings() as $ext ) {
$extpath = $ext['path'];
if ( !is_dir( $extpath ) ) {
$extpath = dirname( $extpath );
}
self::$extensionInfo[realpath( $extpath ) ?: $extpath] =
array_intersect_key( $ext, $keep );
}
}
// Now traverse parent directories until we find a match or run out of

6
includes/api/ApiQuerySiteinfo.php
View file

@ -599,7 +599,11 @@ class ApiQuerySiteinfo extends ApiQueryBase {
protected function appendExtensions( $property ) {
$data = [];
foreach ( $this->getConfig()->get( 'ExtensionCredits' ) as $type => $extensions ) {
$credits = SpecialVersion::getCredits(
ExtensionRegistry::getInstance(),
$this->getConfig()
);
foreach ( $credits as $type => $extensions ) {
foreach ( $extensions as $ext ) {
$ret = [];
$ret['type'] = $type;

6
includes/registration/ExtensionProcessor.php
View file

@ -79,7 +79,6 @@ class ExtensionProcessor implements Processor {
protected const MERGE_STRATEGIES = [
'wgAuthManagerAutoConfig' => 'array_plus_2d',
'wgCapitalLinkOverrides' => 'array_plus',
'wgExtensionCredits' => 'array_merge_recursive',
'wgExtraGenderNamespaces' => 'array_plus',
'wgGrantPermissions' => 'array_plus_2d',
'wgGroupPermissions' => 'array_plus_2d',
@ -98,6 +97,7 @@ class ExtensionProcessor implements Processor {
* @var array
*/
protected const CREDIT_ATTRIBS = [
'type',
'author',
'description',
'descriptionmsg',
@ -122,7 +122,6 @@ class ExtensionProcessor implements Processor {
'manifest_version',
'namespaces',
'requires',
'type',
'AutoloadClasses',
'ExtensionMessagesFiles',
'Hooks',
@ -620,7 +619,7 @@ class ExtensionProcessor implements Processor {
protected function extractCredits( $path, array $info ) {
$credits = [
'path' => $path,
'type' => $info['type'] ?? 'other',
'type' => 'other',
];
foreach ( self::CREDIT_ATTRIBS as $attr ) {
if ( isset( $info[$attr] ) ) {
@ -639,7 +638,6 @@ class ExtensionProcessor implements Processor {
}
$this->credits[$name] = $credits;
$this->globals['wgExtensionCredits'][$credits['type']][] = $credits;
return $name;
}

12
includes/registration/ExtensionRegistry.php
View file

@ -62,7 +62,13 @@ class ExtensionRegistry {
];
/**
* Array of loaded things, keyed by name, values are credits information
* Array of loaded things, keyed by name, values are credits information.
*
* The keys that the credit info arrays may have is defined
* by ExtensionProcessor::CREDIT_ATTRIBS (plus a 'path' key that
* points to the skin or extension JSON file).
*
* This info may be accessed via via ExtensionRegistry::getAllThings.
*
* @var array[]
*/
@ -631,9 +637,9 @@ class ExtensionRegistry {
}
/**
* Get information about all things
* Get credits information about all installed extensions and skins.
*
* @return array[]
* @return array[] Keyed by component name.
*/
public function getAllThings() {
return $this->loaded;

2
includes/skins/SkinFactory.php
View file

@ -66,7 +66,7 @@ class SkinFactory {
* to be, but doing so would change the case of i18n message keys).
* @param string $displayName For backwards-compatibility with old skin loading system. This is
* the text used as skin's human-readable name when the 'skinname-<skin>' message is not
* available. It should be the same as the skin name provided in $wgExtensionCredits.
* available.
* @param array|callable $spec Callback that takes the skin name as an argument, or
* object factory spec specifying how to create the skin
* @throws InvalidArgumentException If an invalid callback is provided

17
includes/specials/SpecialVersion.php
View file

@ -52,6 +52,21 @@ class SpecialVersion extends SpecialPage {
parent::__construct( 'Version' );
}
/**
* @since 1.35
* @param ExtensionRegistry $reg
* @param Config $conf For additional entries from $wgExtensionCredits.
* @return array[]
* @see $wgExtensionCredits
*/
public static function getCredits( ExtensionRegistry $reg, Config $conf ) : array {
$credits = $conf->get( 'ExtensionCredits' );
foreach ( $reg->getAllThings() as $name => $credit ) {
$credits[$credit['type']][] = $credit;
}
return $credits;
}
/**
* main()
* @param string|null $par
@ -59,7 +74,7 @@ class SpecialVersion extends SpecialPage {
public function execute( $par ) {
global $IP;
$config = $this->getConfig();
$credits = $config->get( 'ExtensionCredits' );
$credits = self::getCredits( ExtensionRegistry::getInstance(), $config );
$this->setHeaders();
$this->outputHeader();

18
tests/phpunit/includes/api/ApiQuerySiteinfoTest.php
View file

@ -1,6 +1,7 @@
<?php
use MediaWiki\MediaWikiServices;
use Wikimedia\TestingAccessWrapper;
/**
* @group API
@ -10,8 +11,18 @@ use MediaWiki\MediaWikiServices;
* @covers ApiQuerySiteinfo
*/
class ApiQuerySiteinfoTest extends ApiTestCase {
// We don't try to test every single thing for every category, just a sample
private $originalRegistryLoaded = null;
protected function tearDown() : void {
if ( $this->originalRegistryLoaded !== null ) {
$reg = TestingAccessWrapper::newFromObject( ExtensionRegistry::getInstance() );
$reg->loaded = $this->originalRegistryLoaded;
$this->originalRegistryLoaded = null;
}
parent::tearDown();
}
// We don't try to test every single thing for every category, just a sample
protected function doQuery( $siprop = null, $extraParams = [] ) {
$params = [ 'action' => 'query', 'meta' => 'siteinfo' ];
if ( $siprop !== null ) {
@ -426,6 +437,11 @@ class ApiQuerySiteinfoTest extends ApiTestCase {
'author' => [ 'John Smith', 'John Smith Jr.', '...' ],
'descriptionmsg' => [ 'another-extension-desc', 'param' ] ],
] ] );
// Make the main registry empty
// TODO: Make ExtensionRegistry an injected service?
$reg = TestingAccessWrapper::newFromObject( ExtensionRegistry::getInstance() );
$this->originalRegistryLoaded = $reg->loaded;
$reg->loaded = [];
$data = $this->doQuery( 'extensions' );