registration: Improve merging of arrays

Currently we use array_merge_recursive when merging any array, which is really
only needed for merging $wgHooks entries, and causes issues when trying to
merge default settings if the config is already set.

$wgHooks and $wgGroupPermissions are now special cased when merging, and all
other arrays are just +='d.

Bug: T88665
Bug: T89364
Change-Id: I773a9463d4428aa618c17f848c01b24e04610e95

diff --git a/includes/registration/ExtensionRegistry.php b/includes/registration/ExtensionRegistry.php
index 8541e31..5b18e72 100644 (file)
--- a/includes/registration/ExtensionRegistry.php
+++ b/includes/registration/ExtensionRegistry.php
@@ -142,8 +142,21 @@ class ExtensionRegistry {
                foreach ( $info['globals'] as $key => $val ) {
                        if ( !isset( $GLOBALS[$key] ) || !$GLOBALS[$key] ) {
                                $GLOBALS[$key] = $val;
-                       } elseif ( is_array( $GLOBALS[$key] ) && is_array( $val ) ) {
+                       } elseif ( $key === 'wgHooks' ) {
+                               // Special case $wgHooks, which requires 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' ) {
+                               // 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] += $val;
                        } // else case is a config setting where it has already been overriden, so don't set it
                }
                foreach ( $info['defines'] as $name => $val ) {

diff --git a/tests/phpunit/includes/registration/ExtensionRegistryTest.php b/tests/phpunit/includes/registration/ExtensionRegistryTest.php
new file mode 100644 (file)
index 0000000..421cab5
--- /dev/null
+++ b/tests/phpunit/includes/registration/ExtensionRegistryTest.php
@@ -0,0 +1,180 @@
+<?php
+
+class ExtensionRegistryTest extends MediaWikiTestCase {
+
+       /**
+        * @covers ExtensionRegistry::exportExtractedData
+        * @dataProvider provideExportExtractedDataGlobals
+        * @@backupGlobals enabled
+        */
+       public function testExportExtractedDataGlobals( $desc, $before, $globals, $expected ) {
+               if ( $before ) {
+                       foreach ( $before as $key => $value ) {
+                               $GLOBALS[$key] = $value;
+                       }
+               }
+               $info = array(
+                       'globals' => $globals,
+                       'callbacks' => array(),
+                       'defines' => array(),
+                       'credits' => array(),
+                       'attributes' => array(),
+               );
+               $registry = new ExtensionRegistry();
+               $class = new ReflectionClass( 'ExtensionRegistry' );
+               $method = $class->getMethod( 'exportExtractedData' );
+               $method->setAccessible( true );
+               $method->invokeArgs( $registry, array( $info ) );
+               foreach ( $expected as $name => $value ) {
+                       $this->assertArrayHasKey( $name, $GLOBALS, $desc );
+                       $this->assertEquals( $value, $GLOBALS[$name], $desc );
+               }
+       }
+
+       public static function provideExportExtractedDataGlobals() {
+               // "mwtest" prefix used instead of "$wg" to avoid potential conflicts
+               return array(
+                       array(
+                               'Simple non-array values',
+                               array(
+                                       'mwtestFooBarConfig' => true,
+                                       'mwtestFooBarConfig2' => 'string',
+                               ),
+                               array(
+                                       'mwtestFooBarDefault' => 1234,
+                                       'mwtestFooBarConfig' => false,
+                               ),
+                               array(
+                                       'mwtestFooBarConfig' => true,
+                                       'mwtestFooBarConfig2' => 'string',
+                                       'mwtestFooBarDefault' => 1234,
+                               ),
+                       ),
+                       array(
+                               'No global already set, simple array',
+                               null,
+                               array(
+                                       'mwtestDefaultOptions' => array(
+                                               'foobar' => true,
+                                       )
+                               ),
+                               array(
+                                       'mwtestDefaultOptions' => array(
+                                               'foobar' => true,
+                                       )
+                               ),
+                       ),
+                       array(
+                               'Global already set, simple array',
+                               array(
+                                       'mwtestDefaultOptions' => array(
+                                               'foobar' => true,
+                                               'foo' => 'string'
+                                       ),
+                               ),
+                               array(
+                                       'mwtestDefaultOptions' => array(
+                                               'barbaz' => 12345,
+                                               'foobar' => false,
+                                       ),
+                               ),
+                               array(
+                                       'mwtestDefaultOptions' => array(
+                                               'barbaz' => 12345,
+                                               'foo' => 'string',
+                                               'foobar' => true,
+                                       ),
+                               )
+                       ),
+                       array(
+                               'No global already set, $wgHooks',
+                               array(
+                                       'wgHooks' => array(),
+                               ),
+                               array(
+                                       'wgHooks' => array(
+                                               'FooBarEvent' => array(
+                                                       'FooBarClass::onFooBarEvent'
+                                               ),
+                                       ),
+                               ),
+                               array(
+                                       'wgHooks' => array(
+                                               'FooBarEvent' => array(
+                                                       'FooBarClass::onFooBarEvent'
+                                               ),
+                                       ),
+                               ),
+                       ),
+                       array(
+                               'Global already set, $wgHooks',
+                               array(
+                                       'wgHooks' => array(
+                                               'FooBarEvent' => array(
+                                                       'FooBarClass::onFooBarEvent'
+                                               ),
+                                               'BazBarEvent' => array(
+                                                       'FooBarClass::onBazBarEvent',
+                                               ),
+                                       ),
+                               ),
+                               array(
+                                       'wgHooks' => array(
+                                               'FooBarEvent' => array(
+                                                       'BazBarClass::onFooBarEvent',
+                                               ),
+                                       ),
+                               ),
+                               array(
+                                       'wgHooks' => array(
+                                               'FooBarEvent' => array(
+                                                       'FooBarClass::onFooBarEvent',
+                                                       'BazBarClass::onFooBarEvent',
+                                               ),
+                                               'BazBarEvent' => array(
+                                                       'FooBarClass::onBazBarEvent',
+                                               ),
+                                       ),
+                               ),
+                       ),
+                       array(
+                               'Global already set, $wgGroupPermissions',
+                               array(
+                                       'wgGroupPermissions' => array(
+                                               'sysop' => array(
+                                                       'something' => true,
+                                               ),
+                                               'user' => array(
+                                                       'somethingtwo' => true,
+                                               )
+                                       ),
+                               ),
+                               array(
+                                       'wgGroupPermissions' => array(
+                                               'customgroup' => array(
+                                                       'right' => true,
+                                               ),
+                                               'user' => array(
+                                                       'right' => true,
+                                                       'somethingtwo' => false,
+                                               )
+                                       ),
+                               ),
+                               array(
+                                       'wgGroupPermissions' => array(
+                                               'customgroup' => array(
+                                                       'right' => true,
+                                               ),
+                                               'sysop' => array(
+                                                       'something' => true,
+                                               ),
+                                               'user' => array(
+                                                       'somethingtwo' => true,
+                                                       'right' => true,
+                                               )
+                                       ),
+                               ),
+                       )
+               );
+       }
+}