From fdb7e9410465fd8f2370c182dc18221cb7bd8470 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Thu, 1 Dec 2016 22:02:28 -0800 Subject: [PATCH] registration: Move attributes out of the top level This moves attributes out of the top level, and namespaces them under each extension. If the extension that it belongs to is not installed, the attribute is not exported and dropped. The full name of the attribute is the name of the extension plus the name of the attribute key. This enforces the recommendation that the attribute name start with the extension's name. Add test coverage for attributes under manifest_version 1 and 2. Bug: T133627 Depends-On: I5a148763f68989c8da313a4fb1d0213658ee4495 Depends-On: I5a148763f68989c8da313a4fb1d0213658ee4459 Change-Id: I8613a027c56e2c9d2c6a83ca14749eb1c8fc23be --- docs/extension.schema.v2.json | 15 ++++ includes/registration/ExtensionProcessor.php | 75 ++++++++++++++++++- includes/registration/ExtensionRegistry.php | 2 +- .../registration/ExtensionProcessorTest.php | 66 ++++++++++++++++ 4 files changed, 153 insertions(+), 5 deletions(-) diff --git a/docs/extension.schema.v2.json b/docs/extension.schema.v2.json index a2fdf65aad..0c476b00cb 100644 --- a/docs/extension.schema.v2.json +++ b/docs/extension.schema.v2.json @@ -2,6 +2,7 @@ "$schema": "http://json-schema.org/schema#", "description": "MediaWiki extension.json schema", "type": "object", + "additionalProperties": false, "properties": { "manifest_version": { "type": "integer", @@ -729,6 +730,20 @@ "type": "array", "description": "List of service wiring files to be loaded by the default instance of MediaWikiServices" }, + "attributes": { + "description":"Registration information for other extensions", + "type": "object", + "patternProperties": { + ".*": { + "type": "object", + "patternProperties": { + ".*": { + "type": ["array", "object"] + } + } + } + } + }, "load_composer_autoloader": { "type": "boolean", "description": "Load the composer autoloader for this extension, if one is present" diff --git a/includes/registration/ExtensionProcessor.php b/includes/registration/ExtensionProcessor.php index 1212f9972c..d14be3ff8d 100644 --- a/includes/registration/ExtensionProcessor.php +++ b/includes/registration/ExtensionProcessor.php @@ -56,6 +56,16 @@ class ExtensionProcessor implements Processor { 'ValidSkinNames', ]; + /** + * Top-level attributes that come from MW core + * + * @var string[] + */ + protected static $coreAttributes = [ + 'SkinOOUIThemes', + 'TrackingCategories', + ]; + /** * Mapping of global settings to their specific merge strategies. * @@ -160,6 +170,14 @@ class ExtensionProcessor implements Processor { */ protected $attributes = []; + /** + * Extension attributes, keyed by name => + * settings. + * + * @var array + */ + protected $extAttributes = []; + /** * @param string $path * @param array $info @@ -186,14 +204,47 @@ class ExtensionProcessor implements Processor { $this->callbacks[$name] = $info['callback']; } + if ( $version === 2 ) { + $this->extractAttributes( $path, $info ); + } + foreach ( $info as $key => $val ) { + // If it's a global setting, if ( in_array( $key, self::$globalSettings ) ) { $this->storeToArray( $path, "wg$key", $val, $this->globals ); + continue; + } // Ignore anything that starts with a @ - } elseif ( $key[0] !== '@' && !in_array( $key, self::$notAttributes ) - && !in_array( $key, self::$creditsAttributes ) - ) { - $this->storeToArray( $path, $key, $val, $this->attributes ); + if ( $key[0] === '@' ) { + continue; + } + + if ( $version === 2 ) { + // Only whitelisted attributes are set + if ( in_array( $key, self::$coreAttributes ) ) { + $this->storeToArray( $path, $key, $val, $this->attributes ); + } + } else { + // version === 1 + if ( !in_array( $key, self::$notAttributes ) + && !in_array( $key, self::$creditsAttributes ) + ) { + // If it's not blacklisted, it's an attribute + $this->storeToArray( $path, $key, $val, $this->attributes ); + } + } + + } + } + + /** + * @param string $path + * @param array $info + */ + protected function extractAttributes( $path, array $info ) { + if ( isset( $info['attributes'] ) ) { + foreach ( $info['attributes'] as $extName => $value ) { + $this->storeToArray( $path, $extName, $value, $this->extAttributes ); } } } @@ -206,6 +257,22 @@ class ExtensionProcessor implements Processor { } } + // Merge $this->extAttributes into $this->attributes depending on what is loaded + foreach ( $this->extAttributes as $extName => $value ) { + // Only set the attribute if $extName is loaded (and hence present in credits) + if ( isset( $this->credits[$extName] ) ) { + foreach ( $value as $attrName => $attrValue ) { + $this->storeToArray( + '', // Don't provide a path since it's impossible to generate an error here + $extName . $attrName, + $attrValue, + $this->attributes + ); + } + unset( $this->extAttributes[$extName] ); + } + } + return [ 'globals' => $this->globals, 'defines' => $this->defines, diff --git a/includes/registration/ExtensionRegistry.php b/includes/registration/ExtensionRegistry.php index 344dd8f5c0..0c832feef7 100644 --- a/includes/registration/ExtensionRegistry.php +++ b/includes/registration/ExtensionRegistry.php @@ -31,7 +31,7 @@ class ExtensionRegistry { /** * Bump whenever the registration cache needs resetting */ - const CACHE_VERSION = 5; + const CACHE_VERSION = 6; /** * Special key that defines the merge strategy diff --git a/tests/phpunit/includes/registration/ExtensionProcessorTest.php b/tests/phpunit/includes/registration/ExtensionProcessorTest.php index d15725d2a4..ebe0bdede9 100644 --- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php +++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php @@ -447,6 +447,72 @@ class ExtensionProcessorTest extends MediaWikiTestCase { ]; } + /** + * Attributes under manifest_version 2 + * + * @covers ExtensionProcessor::extractAttributes + * @covers ExtensionProcessor::getExtractedInfo + */ + public function testExtractAttributes() { + $processor = new ExtensionProcessor(); + // Load FooBar extension + $processor->extractInfo( $this->dir, [ 'name' => 'FooBar' ], 2 ); + $processor->extractInfo( + $this->dir, + [ + 'name' => 'Baz', + 'attributes' => [ + // Loaded + 'FooBar' => [ + 'Plugins' => [ + 'ext.baz.foobar', + ], + ], + // Not loaded + 'FizzBuzz' => [ + 'MorePlugins' => [ + 'ext.baz.fizzbuzz', + ], + ], + ], + ], + 2 + ); + + $info = $processor->getExtractedInfo(); + $this->assertArrayHasKey( 'FooBarPlugins', $info['attributes'] ); + $this->assertSame( [ 'ext.baz.foobar' ], $info['attributes']['FooBarPlugins'] ); + $this->assertArrayNotHasKey( 'FizzBuzzMorePlugins', $info['attributes'] ); + } + + /** + * Attributes under manifest_version 1 + * + * @covers ExtensionProcessor::extractInfo + */ + public function testAttributes1() { + $processor = new ExtensionProcessor(); + $processor->extractInfo( + $this->dir, + [ + 'name' => 'FooBar', + 'FooBarPlugins' => [ + 'ext.baz.foobar', + ], + 'FizzBuzzMorePlugins' => [ + 'ext.baz.fizzbuzz', + ], + ], + 1 + ); + + $info = $processor->getExtractedInfo(); + $this->assertArrayHasKey( 'FooBarPlugins', $info['attributes'] ); + $this->assertSame( [ 'ext.baz.foobar' ], $info['attributes']['FooBarPlugins'] ); + $this->assertArrayHasKey( 'FizzBuzzMorePlugins', $info['attributes'] ); + $this->assertSame( [ 'ext.baz.fizzbuzz' ], $info['attributes']['FizzBuzzMorePlugins'] ); + } + public function testGlobalSettingsDocumentedInSchema() { global $IP; $globalSettings = TestingAccessWrapper::newFromClass( -- 2.20.1