From: Kunal Mehta <legoktm@gmail.com>
Date: Fri, 13 Feb 2015 07:40:13 +0000 (-0800)
Subject: registration: Improve merging of arrays
X-Git-Tag: 1.31.0-rc.0~12393
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/operations/?a=commitdiff_plain;h=1b22e7783b262f19630c44471870b0f233ee7454;p=lhc%2Fweb%2Fwiklou.git

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 8541e319ef..5b18e72ddb 100644
--- 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
index 0000000000..421cab5b28
--- /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,
+						)
+					),
+				),
+			)
+		);
+	}
+}