From 2855210048d6056601b4afa6dc0a53cf861eb011 Mon Sep 17 00:00:00 2001
From: Mattflaschen <mflaschen@wikimedia.org>
Date: Thu, 30 Jan 2014 23:38:59 +0000
Subject: [PATCH] Revert "User::saveOptions() optimization"

The hooks are not actually being called multiple times,
since getDefaultOptions caches its return value
(except while unit testing).

However, this change caused a regression due to different
handling of saved values (e.g. false no longer saves
when the default is true).

This reverts commit ff355e87e2f7a41a87c37f43232a44113cb033e0.

Bug: 60653
Change-Id: Ibbd34dde5ec5fafbdf6097337cc0fa94614f0b85
---
 includes/User.php                   | 22 +++++++-------
 tests/phpunit/includes/UserTest.php | 45 -----------------------------
 2 files changed, 11 insertions(+), 56 deletions(-)

diff --git a/includes/User.php b/includes/User.php
index 7f7a22db9d..ca3f79b5de 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -4629,20 +4629,20 @@ class User {
 			return;
 		}
 
-		$defaultOptions = self::getDefaultOptions();
-
 		$userId = $this->getId();
 		$insert_rows = array();
-		$changedOptions = array_diff_assoc( $saveOptions, $defaultOptions );
-		foreach ( $changedOptions as $key => $value ) {
-			if ( $value === false || is_null( $value ) ) {
-				continue;
+		foreach ( $saveOptions as $key => $value ) {
+			// Don't bother storing default values
+			$defaultOption = self::getDefaultOption( $key );
+			if ( ( is_null( $defaultOption ) &&
+					!( $value === false || is_null( $value ) ) ) ||
+					$value != $defaultOption ) {
+				$insert_rows[] = array(
+						'up_user' => $userId,
+						'up_property' => $key,
+						'up_value' => $value,
+					);
 			}
-			$insert_rows[] = array(
-				'up_user' => $userId,
-				'up_property' => $key,
-				'up_value' => $value,
-			);
 		}
 
 		$dbw = wfGetDB( DB_MASTER );
diff --git a/tests/phpunit/includes/UserTest.php b/tests/phpunit/includes/UserTest.php
index b7df3f617d..ff33e825ee 100644
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -234,49 +234,4 @@ class UserTest extends MediaWikiTestCase {
 		$this->assertEquals( $wgDefaultUserOptions['cols'], $this->user->getOption( 'cols' ) );
 		$this->assertEquals( 'test', $this->user->getOption( 'someoption' ) );
 	}
-
-	/**
-	 * Helper, fetch user properties from the database.
-	 * @param int $userId
-	 */
-	function dbUserProperties( $userId ) {
-		$res = wfGetDB( DB_SLAVE )->select(
-			'user_properties',
-			array( 'up_property', 'up_value' ),
-			array( 'up_user' => $userId ),
-			__METHOD__
-		);
-		$ret = array();
-		foreach( $res as $row ) {
-			$ret[$row->up_property] = $row->up_value;
-		}
-		return $ret;
-	}
-
-	public function testOnlySaveChangedOptions() {
-		$user = User::newFromName( 'UnitTestUser2' );
-		$user->addToDatabase();
-
-		// Fresh user only has default, so nothing should be in the DB
-		$dbProps = $this->dbUserProperties( $user->getId() );
-		$this->assertEmpty( $dbProps,
-			"A new user should not have any user property saved in the DB" );
-
-		// Make sure we only save the altered option
-		$user->setOption( 'changed_opt', 'alix_20281' );
-		$user->setOption( 'switch', 1 );
-		$user->setOption( 'anotherswitch', 1 );
-		$user->saveSettings();
-
-		$expected = array (
-			'changed_opt' => 'alix_20281',
-			'switch' => '1',
-			'anotherswitch' => '1',
-		);
-		$dbProps = $this->dbUserProperties( $user->getId() );
-
-		$this->assertEquals( $expected, $dbProps,
-			"non default options should be saved, and default ones should not" );
-
-	}
 }
-- 
2.20.1