Legoktm has uploaded a new change for review.
https://gerrit.wikimedia.org/r/228450
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
* group_permissions - For merging arrays like $wgGroupPermissions
* array_plus - use the "+" operator to combine arrays, preserving
integer keys
* array_merge - call `array_merge` (default)
Included bug fixes:
* Fixed merging of $wgRevokePermissions
* Fixed merging of various namespaces related settings
Bug: T107646
Change-Id: I64cb0553864e3b78b0f203333f58bb73b86a6434
---
M includes/registration/ExtensionProcessor.php
M includes/registration/ExtensionRegistry.php
M tests/phpunit/includes/registration/ExtensionProcessorTest.php
M tests/phpunit/includes/registration/ExtensionRegistryTest.php
4 files changed, 131 insertions(+), 22 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/50/228450/1
diff --git a/includes/registration/ExtensionProcessor.php
b/includes/registration/ExtensionProcessor.php
index 273e9ef..e72c8d8 100644
--- a/includes/registration/ExtensionProcessor.php
+++ b/includes/registration/ExtensionProcessor.php
@@ -2,6 +2,8 @@
class ExtensionProcessor implements Processor {
+ const MERGE_STRATEGY = ExtensionRegistry::MERGE_STRATEGY;
+
/**
* Keys that should be set to $GLOBALS
*
@@ -44,6 +46,24 @@
'APIPropModules',
'APIListModules',
'ValidSkinNames',
+ );
+
+ /**
+ * Mapping of global settings to their specific merge strategies.
+ *
+ * @see ExtensionRegistry:;exportExtractedData
+ * @see getExtractedInfo
+ * @var array
+ */
+ protected static $mergeStrategies = array(
+ 'wgGroupPermissions' => 'group_permissions',
+ 'wgRevokePermissions' => 'group_permissions',
+ 'wgHooks' => 'array_merge_recursive',
+ 'wgExtensionCredits' => 'array_merge_recursive',
+ 'wgExtraNamespaces' => 'array_plus',
+ 'wgExtraGenderNamespaces' => 'array_plus',
+ 'wgNamespacesWithSubpages' => 'array_plus',
+ 'wgNamespaceContentModels' => 'array_plus',
);
/**
@@ -156,6 +176,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][self::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 c9df4b1..f337344 100644
--- a/includes/registration/ExtensionRegistry.php
+++ b/includes/registration/ExtensionRegistry.php
@@ -22,6 +22,13 @@
const OLDEST_MANIFEST_VERSION = 1;
/**
+ * Special key that defines the merge strategy
+ *
+ * @since 1.26
+ */
+ const MERGE_STRATEGY = '_merge_strategy';
+
+ /**
* @var BagOStuff
*/
protected $cache;
@@ -181,25 +188,52 @@
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.
+ if ( isset( $val[self::MERGE_STRATEGY ] ) ) {
+ $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 'group_permissions':
+ // First merge individual groups
+ 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 merge groups 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 a79c9a8..31c5492 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(),
+ 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 515ce11..bf0c730 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(
@@ -173,7 +219,8 @@
'right' => true,
'somethingtwo' => false,
'nonduplicated' => true,
- )
+ ),
+
ExtensionRegistry::MERGE_STRATEGY => 'group_permissions',
),
),
array(
--
To view, visit https://gerrit.wikimedia.org/r/228450
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: master
Gerrit-Owner: Legoktm <legoktm.wikipe...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
