Legoktm has uploaded a new change for review.
https://gerrit.wikimedia.org/r/237971
Change subject: registration: Overhaul merging of globals
......................................................................
registration: Overhaul merging of globals
Instead of hardcoding specific global settings in ExtensionRegistry,
create specific "merge strategies" that are used to merge globals.
Merge strategies are set for core properties in the ExtensionProcessor,
and extensions can set them for their own configuration settings using
the magic "_merge_strategy" key.
The following merge strategies are included:
* array_merge_recursive - call `array_merge_recursive` on the two arrays
* array_plus - use the "+" operator to combine arrays, preserving
integer keys
* array_plus_2d - A version of array_plus that works on 2d arrays, used
for merging arrays like $wgGroupPermissions
* array_merge - call `array_merge` (default)
This changes the merging of various namespaces related settings to use
array_plus so they actually work.
Bug: T107646
Change-Id: I64cb0553864e3b78b0f203333f58bb73b86a6434
---
M docs/extension.schema.json
M includes/registration/ExtensionProcessor.php
M includes/registration/ExtensionRegistry.php
M tests/phpunit/includes/registration/ExtensionProcessorTest.php
M tests/phpunit/includes/registration/ExtensionRegistryTest.php
5 files changed, 159 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/71/237971/1
diff --git a/docs/extension.schema.json b/docs/extension.schema.json
index d5c17a1..14d7c22 100644
--- a/docs/extension.schema.json
+++ b/docs/extension.schema.json
@@ -627,7 +627,24 @@
},
"config": {
"type": "object",
- "description": "Configuration options for this
extension"
+ "description": "Configuration options for this
extension",
+ "patternProperties": {
+
"^[a-zA-Z_\u007f-\u00ff][a-zA-Z0-9_\u007f-\u00ff]*$": {
+ "type": ["object", "array", "string",
"integer", "null", "boolean"],
+ "properties": {
+ "_merge_strategy": {
+ "type": "string",
+ "enum": [
+
"array_merge_recursive",
+ "array_plus_2d",
+ "array_plus",
+ "array_merge"
+ ],
+ "default": "array_merge"
+ }
+ }
+ }
+ }
},
"ParserTestFiles": {
"type": "array",
diff --git a/includes/registration/ExtensionProcessor.php
b/includes/registration/ExtensionProcessor.php
index a11f1aa..81ea969 100644
--- a/includes/registration/ExtensionProcessor.php
+++ b/includes/registration/ExtensionProcessor.php
@@ -47,6 +47,24 @@
);
/**
+ * Mapping of global settings to their specific merge strategies.
+ *
+ * @see ExtensionRegistry::exportExtractedData
+ * @see getExtractedInfo
+ * @var array
+ */
+ protected static $mergeStrategies = array(
+ 'wgGroupPermissions' => 'array_plus_2d',
+ 'wgRevokePermissions' => 'array_plus_2d',
+ 'wgHooks' => 'array_merge_recursive',
+ 'wgExtensionCredits' => 'array_merge_recursive',
+ 'wgExtraNamespaces' => 'array_plus',
+ 'wgExtraGenderNamespaces' => 'array_plus',
+ 'wgNamespacesWithSubpages' => 'array_plus',
+ 'wgNamespaceContentModels' => 'array_plus',
+ );
+
+ /**
* Keys that are part of the extension credits
*
* @var array
@@ -155,6 +173,13 @@
}
public function getExtractedInfo() {
+ // Make sure the merge strategies are set
+ foreach ( $this->globals as $key => $val ) {
+ if ( isset( self::$mergeStrategies[$key] ) ) {
+
$this->globals[$key][ExtensionRegistry::MERGE_STRATEGY] =
self::$mergeStrategies[$key];
+ }
+ }
+
return array(
'globals' => $this->globals,
'defines' => $this->defines,
diff --git a/includes/registration/ExtensionRegistry.php
b/includes/registration/ExtensionRegistry.php
index 8ea12ee..271b3a9 100644
--- a/includes/registration/ExtensionRegistry.php
+++ b/includes/registration/ExtensionRegistry.php
@@ -12,6 +12,23 @@
class ExtensionRegistry {
/**
+ * Version of the highest supported manifest version
+ */
+ const MANIFEST_VERSION = 1;
+
+ /**
+ * Version of the oldest supported manifest version
+ */
+ const OLDEST_MANIFEST_VERSION = 1;
+
+ /**
+ * Special key that defines the merge strategy
+ *
+ * @since 1.26
+ */
+ const MERGE_STRATEGY = '_merge_strategy';
+
+ /**
* @var BagOStuff
*/
protected $cache;
@@ -161,25 +178,54 @@
protected function exportExtractedData( array $info ) {
foreach ( $info['globals'] as $key => $val ) {
+ // If a merge strategy is set, read it and remove it
from the value
+ // so it doesn't accidentally end up getting set.
+ // Need to check $val is an array for PHP 5.3 which
will return
+ // true on isset( 'string'['foo'] ).
+ if ( isset( $val[self::MERGE_STRATEGY] ) && is_array(
$val ) ) {
+ $mergeStrategy = $val[self::MERGE_STRATEGY];
+ unset( $val[self::MERGE_STRATEGY] );
+ } else {
+ $mergeStrategy = 'array_merge';
+ }
+
+ // Optimistic: If the global is not set, or is an empty
array, replace it entirely.
+ // Will be O(1) performance.
if ( !isset( $GLOBALS[$key] ) || ( is_array(
$GLOBALS[$key] ) && !$GLOBALS[$key] ) ) {
$GLOBALS[$key] = $val;
- } elseif ( $key === 'wgHooks' || $key ===
'wgExtensionCredits' ) {
- // Special case $wgHooks and
$wgExtensionCredits, which require a recursive merge.
- // Ideally it would have been taken care of in
the first if block though.
- $GLOBALS[$key] = array_merge_recursive(
$GLOBALS[$key], $val );
- } elseif ( $key === 'wgGroupPermissions' || $key ===
'wgRevokePermissions' ) {
- // First merge individual groups
- foreach ( $GLOBALS[$key] as $name => &$groupVal
) {
- if ( isset( $val[$name] ) ) {
- $groupVal += $val[$name];
+ continue;
+ }
+
+ if ( !is_array( $GLOBALS[$key] ) || !is_array( $val ) )
{
+ // config setting that has already been
overridden, don't set it
+ continue;
+ }
+
+ switch ( $mergeStrategy ) {
+ case 'array_merge_recursive':
+ $GLOBALS[$key] = array_merge_recursive(
$GLOBALS[$key], $val );
+ break;
+ case 'array_plus_2d':
+ // First merge items that are in both
arrays
+ foreach ( $GLOBALS[$key] as $name =>
&$groupVal ) {
+ if ( isset( $val[$name] ) ) {
+ $groupVal +=
$val[$name];
+ }
}
- }
- // Now merge groups that didn't exist yet
- $GLOBALS[$key] += $val;
- } elseif ( is_array( $GLOBALS[$key] ) && is_array( $val
) ) {
- $GLOBALS[$key] = array_merge( $val,
$GLOBALS[$key] );
- } // else case is a config setting where it has already
been overriden, so don't set it
+ // Now add items that didn't exist yet
+ $GLOBALS[$key] += $val;
+ break;
+ case 'array_plus':
+ $GLOBALS[$key] = $val + $GLOBALS[$key];
+ break;
+ case 'array_merge':
+ $GLOBALS[$key] = array_merge( $val,
$GLOBALS[$key] );
+ break;
+ default:
+ throw new UnexpectedValueException(
"Unknown merge strategy '$mergeStrategy'" );
+ }
}
+
foreach ( $info['defines'] as $name => $val ) {
define( $name, $val );
}
diff --git a/tests/phpunit/includes/registration/ExtensionProcessorTest.php
b/tests/phpunit/includes/registration/ExtensionProcessorTest.php
index 9474496..b86279d 100644
--- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php
+++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php
@@ -38,6 +38,7 @@
}
public static function provideRegisterHooks() {
+ $merge = array( ExtensionRegistry::MERGE_STRATEGY =>
'array_merge_recursive' );
// Format:
// Current $wgHooks
// Content in extension.json
@@ -47,19 +48,19 @@
array(
array(),
self::$default,
- array(),
+ $merge,
),
// No current hooks, adding one for "FooBaz"
array(
array(),
array( 'Hooks' => array( 'FooBaz' =>
'FooBazCallback' ) ) + self::$default,
- array( 'FooBaz' => array( 'FooBazCallback' ) ),
+ array( 'FooBaz' => array( 'FooBazCallback' ) )
+ $merge,
),
// Hook for "FooBaz", adding another one
array(
array( 'FooBaz' => array( 'PriorCallback' ) ),
array( 'Hooks' => array( 'FooBaz' =>
'FooBazCallback' ) ) + self::$default,
- array( 'FooBaz' => array( 'PriorCallback',
'FooBazCallback' ) ),
+ array( 'FooBaz' => array( 'PriorCallback',
'FooBazCallback' ) ) + $merge,
),
// Hook for "BarBaz", adding one for "FooBaz"
array(
@@ -68,7 +69,7 @@
array(
'BarBaz' => array( 'BarBazCallback' ),
'FooBaz' => array( 'FooBazCallback' ),
- ),
+ ) + $merge,
),
// Callbacks for FooBaz wrapped in an array
array(
@@ -76,7 +77,7 @@
array( 'Hooks' => array( 'FooBaz' => array(
'Callback1' ) ) ) + self::$default,
array(
'FooBaz' => array( 'Callback1' ),
- ),
+ ) + $merge,
),
// Multiple callbacks for FooBaz hook
array(
@@ -84,7 +85,7 @@
array( 'Hooks' => array( 'FooBaz' => array(
'Callback1', 'Callback2' ) ) ) + self::$default,
array(
'FooBaz' => array( 'Callback1',
'Callback2' ),
- ),
+ ) + $merge,
),
);
}
diff --git a/tests/phpunit/includes/registration/ExtensionRegistryTest.php
b/tests/phpunit/includes/registration/ExtensionRegistryTest.php
index d7d4f19..55824bc 100644
--- a/tests/phpunit/includes/registration/ExtensionRegistryTest.php
+++ b/tests/phpunit/includes/registration/ExtensionRegistryTest.php
@@ -102,6 +102,50 @@
)
),
array(
+ 'Global already set, 1d array that appends',
+ array(
+ 'mwAvailableRights' => array(
+ 'foobar',
+ 'foo'
+ ),
+ ),
+ array(
+ 'mwAvailableRights' => array(
+ 'barbaz',
+ ),
+ ),
+ array(
+ 'mwAvailableRights' => array(
+ 'barbaz',
+ 'foobar',
+ 'foo',
+ ),
+ )
+ ),
+ array(
+ 'Global already set, 2d array with integer
keys',
+ array(
+ 'mwNamespacesFoo' => array(
+ 100 => true,
+ 102 => false
+ ),
+ ),
+ array(
+ 'mwNamespacesFoo' => array(
+ 100 => false,
+ 500 => true,
+
ExtensionRegistry::MERGE_STRATEGY => 'array_plus',
+ ),
+ ),
+ array(
+ 'mwNamespacesFoo' => array(
+ 100 => false,
+ 102 => false,
+ 500 => true,
+ ),
+ )
+ ),
+ array(
'No global already set, $wgHooks',
array(
'wgHooks' => array(),
@@ -111,6 +155,7 @@
'FooBarEvent' => array(
'FooBarClass::onFooBarEvent'
),
+
ExtensionRegistry::MERGE_STRATEGY => 'array_merge_recursive'
),
),
array(
@@ -138,6 +183,7 @@
'FooBarEvent' => array(
'BazBarClass::onFooBarEvent',
),
+
ExtensionRegistry::MERGE_STRATEGY => 'array_merge_recursive',
),
),
array(
@@ -172,7 +218,8 @@
'user' => array(
'right' => true,
'somethingtwo' => false,
- )
+ ),
+
ExtensionRegistry::MERGE_STRATEGY => 'array_plus_2d',
),
),
array(
--
To view, visit https://gerrit.wikimedia.org/r/237971
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I64cb0553864e3b78b0f203333f58bb73b86a6434
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_25
Gerrit-Owner: Legoktm <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
