Merge "registration: Overhaul merging of globals"

diff --git a/docs/extension.schema.json b/docs/extension.schema.json
index 8e6cafd..eef2c0a 100644 (file)
--- a/docs/extension.schema.json
+++ b/docs/extension.schema.json
@@ -626,7 +626,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 273e9ef..da8600c 100644 (file)
--- a/includes/registration/ExtensionProcessor.php
+++ b/includes/registration/ExtensionProcessor.php
@@ -46,6 +46,24 @@ class ExtensionProcessor implements Processor {
                'ValidSkinNames',
        );
 
+       /**
+        * 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
         *
@@ -156,6 +174,13 @@ class ExtensionProcessor implements Processor {
        }
 
        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 c9df4b1..7e113fc 100644 (file)
--- a/includes/registration/ExtensionRegistry.php
+++ b/includes/registration/ExtensionRegistry.php
@@ -21,6 +21,13 @@ class ExtensionRegistry {
         */
        const OLDEST_MANIFEST_VERSION = 1;
 
+       /**
+        * Special key that defines the merge strategy
+        *
+        * @since 1.26
+        */
+       const MERGE_STRATEGY = '_merge_strategy';
+
        /**
         * @var BagOStuff
         */
@@ -181,25 +188,54 @@ class ExtensionRegistry {
 
        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 a79c9a8..0964137 100644 (file)
--- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php
+++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php
@@ -38,6 +38,7 @@ class ExtensionProcessorTest extends MediaWikiTestCase {
        }
 
        public static function provideRegisterHooks() {
+               $merge = array( ExtensionRegistry::MERGE_STRATEGY => 'array_merge_recursive' );
                // Format:
                // Current $wgHooks
                // Content in extension.json
@@ -47,19 +48,19 @@ class ExtensionProcessorTest extends MediaWikiTestCase {
                        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 @@ class ExtensionProcessorTest extends MediaWikiTestCase {
                                array(
                                        'BarBaz' => array( 'BarBazCallback' ),
                                        'FooBaz' => array( 'FooBazCallback' ),
-                               ),
+                               ) + $merge,
                        ),
                        // Callbacks for FooBaz wrapped in an array
                        array(
@@ -76,7 +77,7 @@ class ExtensionProcessorTest extends MediaWikiTestCase {
                                array( 'Hooks' => array( 'FooBaz' => array( 'Callback1' ) ) ) + self::$default,
                                array(
                                        'FooBaz' => array( 'Callback1' ),
-                               ),
+                               ) + $merge,
                        ),
                        // Multiple callbacks for FooBaz hook
                        array(
@@ -84,7 +85,7 @@ class ExtensionProcessorTest extends MediaWikiTestCase {
                                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..b8b1b06 100644 (file)
--- a/tests/phpunit/includes/registration/ExtensionRegistryTest.php
+++ b/tests/phpunit/includes/registration/ExtensionRegistryTest.php
@@ -101,6 +101,50 @@ class ExtensionRegistryTest extends MediaWikiTestCase {
                                        ),
                                )
                        ),
+                       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(
@@ -111,6 +155,7 @@ class ExtensionRegistryTest extends MediaWikiTestCase {
                                                'FooBarEvent' => array(
                                                        'FooBarClass::onFooBarEvent'
                                                ),
+                                               ExtensionRegistry::MERGE_STRATEGY => 'array_merge_recursive'
                                        ),
                                ),
                                array(
@@ -138,6 +183,7 @@ class ExtensionRegistryTest extends MediaWikiTestCase {
                                                'FooBarEvent' => array(
                                                        'BazBarClass::onFooBarEvent',
                                                ),
+                                               ExtensionRegistry::MERGE_STRATEGY => 'array_merge_recursive',
                                        ),
                                ),
                                array(
@@ -173,7 +219,8 @@ class ExtensionRegistryTest extends MediaWikiTestCase {
                                                        'right' => true,
                                                        'somethingtwo' => false,
                                                        'nonduplicated' => true,
-                                               )
+                                               ),
+                                               ExtensionRegistry::MERGE_STRATEGY => 'array_plus_2d',
                                        ),
                                ),
                                array(