From fb7c95f567735d6297701e1783457879adc6a749 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Sun, 2 Dec 2012 22:40:55 -0500
Subject: [PATCH] (bug 42638) Fix API action=options&reset=1 & unit tests

Change I98df55f2 broke action=options&reset=1, causing it to return an
error "No changes were requested" rather than resetting the options as
it should. Unfortunately, that change also broke the unit test that
would have caught this regression.

This changeset fixes the bug and the unit tests.

Change-Id: I7fe63640d54efab4572538e9d08f5b75c61243a4
---
 includes/api/ApiOptions.php                   |  2 +-
 tests/phpunit/includes/api/ApiOptionsTest.php | 74 ++++++++++++++-----
 2 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/includes/api/ApiOptions.php b/includes/api/ApiOptions.php
index f21bbc0000..265c2ccb67 100644
--- a/includes/api/ApiOptions.php
+++ b/includes/api/ApiOptions.php
@@ -69,7 +69,7 @@ class ApiOptions extends ApiBase {
 			$newValue = isset( $params['optionvalue'] ) ? $params['optionvalue'] : null;
 			$changes[$params['optionname']] = $newValue;
 		}
-		if ( !count( $changes ) ) {
+		if ( !$changed && !count( $changes ) ) {
 			$this->dieUsage( 'No changes were requested', 'nochanges' );
 		}
 
diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php b/tests/phpunit/includes/api/ApiOptionsTest.php
index 5469ccf332..ac18afe7aa 100644
--- a/tests/phpunit/includes/api/ApiOptionsTest.php
+++ b/tests/phpunit/includes/api/ApiOptionsTest.php
@@ -2,11 +2,14 @@
 
 /**
  * @group API
+ * @group Database
  */
 class ApiOptionsTest extends MediaWikiLangTestCase {
 
 	private $mTested, $mUserMock, $mContext, $mSession;
 
+	private $mOldGetPreferencesHooks = false;
+
 	private static $Success = array( 'options' => 'success' );
 
 	protected function setUp() {
@@ -16,8 +19,13 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
+		// Set up groups
+		$this->mUserMock->expects( $this->any() )
+			->method( 'getEffectiveGroups' )->will( $this->returnValue( array( '*', 'user')) );
+
 		// Create a new context
 		$this->mContext = new DerivativeContext( new RequestContext() );
+		$this->mContext->getContext()->setTitle( Title::newFromText( 'Test' ) );
 		$this->mContext->setUser( $this->mUserMock );
 
 		$main = new ApiMain( $this->mContext );
@@ -26,6 +34,36 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->mSession = array();
 
 		$this->mTested = new ApiOptions( $main, 'options' );
+
+		global $wgHooks;
+		if ( !isset( $wgHooks['GetPreferences'] ) ) {
+			$wgHooks['GetPreferences'] = array();
+		}
+		$this->mOldGetPreferencesHooks = $wgHooks['GetPreferences'];
+		$wgHooks['GetPreferences'][] = array( $this, 'hookGetPreferences' );
+	}
+
+	protected function tearDown() {
+		global $wgHooks;
+
+		if ( $this->mOldGetPreferencesHooks !== false ) {
+			$wgHooks['GetPreferences'] = $this->mOldGetPreferencesHooks;
+			$this->mOldGetPreferencesHooks = false;
+		}
+
+		parent::tearDown();
+	}
+
+	public function hookGetPreferences( $user, &$preferences ) {
+		foreach ( array( 'name', 'willBeNull', 'willBeEmpty', 'willBeHappy' ) as $k ) {
+			$preferences[$k] = array(
+				'type' => 'text',
+				'section' => 'test',
+				'label' => '&#160;',
+			);
+		}
+
+		return true;
 	}
 
 	private function getSampleRequest( $custom = array() ) {
@@ -105,9 +143,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->fail( "UsageException was not thrown" );
 	}
 
-	/**
-	 * @group Broken
-	 */
 	public function testReset() {
 		$this->mUserMock->expects( $this->once() )
 			->method( 'resetOptions' );
@@ -125,9 +160,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->assertEquals( self::$Success, $response );
 	}
 
-	/**
-	 * @group Broken
-	 */
 	public function testOptionWithValue() {
 		$this->mUserMock->expects( $this->never() )
 			->method( 'resetOptions' );
@@ -146,9 +178,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->assertEquals( self::$Success, $response );
 	}
 
-	/**
-	 * @group Broken
-	 */
 	public function testOptionResetValue() {
 		$this->mUserMock->expects( $this->never() )
 			->method( 'resetOptions' );
@@ -166,22 +195,28 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->assertEquals( self::$Success, $response );
 	}
 
-	/**
-	 * @group Broken
-	 */
 	public function testChange() {
 		$this->mUserMock->expects( $this->never() )
 			->method( 'resetOptions' );
 
 		$this->mUserMock->expects( $this->at( 1 ) )
+			->method( 'getOptions' );
+
+		$this->mUserMock->expects( $this->at( 2 ) )
 			->method( 'setOption' )
 			->with( $this->equalTo( 'willBeNull' ), $this->equalTo( null ) );
 
-		$this->mUserMock->expects( $this->at( 2 ) )
+		$this->mUserMock->expects( $this->at( 3 ) )
+			->method( 'getOptions' );
+
+		$this->mUserMock->expects( $this->at( 4 ) )
 			->method( 'setOption' )
 			->with( $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) );
 
-		$this->mUserMock->expects( $this->at( 3 ) )
+		$this->mUserMock->expects( $this->at( 5 ) )
+			->method( 'getOptions' );
+
+		$this->mUserMock->expects( $this->at( 6 ) )
 			->method( 'setOption' )
 			->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
 
@@ -195,18 +230,21 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->assertEquals( self::$Success, $response );
 	}
 
-	/**
-	 * @group Broken
-	 */
 	public function testResetChangeOption() {
 		$this->mUserMock->expects( $this->once() )
 			->method( 'resetOptions' );
 
 		$this->mUserMock->expects( $this->at( 2 ) )
+			->method( 'getOptions' );
+
+		$this->mUserMock->expects( $this->at( 3 ) )
 			->method( 'setOption' )
 			->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
 
-		$this->mUserMock->expects( $this->at( 3 ) )
+		$this->mUserMock->expects( $this->at( 4 ) )
+			->method( 'getOptions' );
+
+		$this->mUserMock->expects( $this->at( 5 ) )
 			->method( 'setOption' )
 			->with( $this->equalTo( 'name' ), $this->equalTo( 'value' ) );
 
-- 
2.20.1