From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Tue, 17 Jan 2017 15:51:27 +0000 (-0500)
Subject: ApiErrorFormatter_BackCompat: Use first error, not last
X-Git-Tag: 1.31.0-rc.0~4265^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/operations/recherche.php?a=commitdiff_plain;h=382450573d494d955e7718fdb38bda35f87a6fb2;p=lhc%2Fweb%2Fwiklou.git

ApiErrorFormatter_BackCompat: Use first error, not last

Before Iae0e2ce3b, the only place in the API that had to deal with
choosing from multiple errors was ApiBase::dieStatus(), which chose the
first one in the Status object. Iae0e2ce3b changed this to choose the
last one instead, which is an unnecessary backwards compatibility break.

While we could make the change in ApiBase::dieStatus(), it's cleaner to
change ApiErrorFormatter_BackCompat's behavior instead since it seems
unlikely anything else was using that code path.

Bug: T155268
Change-Id: Ia06527f8480c3d4a689792ceb8671b0d399ffbe3
---

diff --git a/includes/api/ApiErrorFormatter.php b/includes/api/ApiErrorFormatter.php
index 814004a673..c52b731bbd 100644
--- a/includes/api/ApiErrorFormatter.php
+++ b/includes/api/ApiErrorFormatter.php
@@ -414,12 +414,17 @@ class ApiErrorFormatter_BackCompat extends ApiErrorFormatter {
 
 		if ( $tag === 'error' ) {
 			// In BC mode, only one error
-			$value = [
-				'code' => $msg->getApiCode(),
-				'info' => $value,
-			] + $msg->getApiData();
-			$this->result->addValue( null, 'error', $value,
-				ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP | ApiResult::NO_SIZE_CHECK );
+			$existingError = $this->result->getResultData( [ 'error' ] );
+			if ( !is_array( $existingError ) ||
+				!isset( $existingError['code'] ) || !isset( $existingError['info'] )
+			) {
+				$value = [
+					'code' => $msg->getApiCode(),
+					'info' => $value,
+				] + $msg->getApiData();
+				$this->result->addValue( null, 'error', $value,
+					ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP | ApiResult::NO_SIZE_CHECK );
+			}
 		} else {
 			if ( $modulePath === null ) {
 				$moduleName = 'unknown';
diff --git a/tests/phpunit/includes/api/ApiErrorFormatterTest.php b/tests/phpunit/includes/api/ApiErrorFormatterTest.php
index a40db24680..eaa4d178e7 100644
--- a/tests/phpunit/includes/api/ApiErrorFormatterTest.php
+++ b/tests/phpunit/includes/api/ApiErrorFormatterTest.php
@@ -439,8 +439,8 @@ class ApiErrorFormatterTest extends MediaWikiLangTestCase {
 		$formatter->addMessagesFromStatus( 'status', $status );
 		$this->assertSame( [
 			'error' => [
-				'code' => 'parentheses',
-				'info' => $parensPlain,
+				'code' => 'mainpage',
+				'info' => $mainpagePlain,
 			],
 			'warnings' => [
 				'status' => [
@@ -502,6 +502,17 @@ class ApiErrorFormatterTest extends MediaWikiLangTestCase {
 			$formatter->arrayFromStatus( $status, 'warning' ),
 			'arrayFromStatus test for warning'
 		);
+
+		$result->reset();
+		$result->addValue( null, 'error', [ 'bogus' ] );
+		$formatter->addError( 'err', 'mainpage' );
+		$this->assertSame( [
+			'error' => [
+				'code' => 'mainpage',
+				'info' => $mainpagePlain,
+			],
+			ApiResult::META_TYPE => 'assoc',
+		], $result->getResultData(), 'Overwrites bogus "error" value with real error' );
 	}
 
 	/**