From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Tue, 20 Dec 2016 18:59:05 +0000 (-0500)
Subject: Fixes and tests for ApiErrorFormatter ILocalizedException handling
X-Git-Tag: 1.31.0-rc.0~4499^2
X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22brouteur%22%2C%28%24id_rubrique%20?a=commitdiff_plain;h=0402b23040fecb1f3d86af8891d4300d959d8135;p=lhc%2Fweb%2Fwiklou.git

Fixes and tests for ApiErrorFormatter ILocalizedException handling

Change-Id: I9449ea5886e27dfb9e54b91cdb50a6a6a2c9a4ed
---

diff --git a/includes/api/ApiErrorFormatter.php b/includes/api/ApiErrorFormatter.php
index f246203a3a..814004a673 100644
--- a/includes/api/ApiErrorFormatter.php
+++ b/includes/api/ApiErrorFormatter.php
@@ -148,10 +148,11 @@ class ApiErrorFormatter {
 	 * @param Exception|Throwable $exception
 	 * @param array $options
 	 *  - wrap: (string|array|MessageSpecifier) Used to wrap the exception's
-	 *    message. The exception's message will be added as the final parameter.
+	 *    message if it's not an ILocalizedException. The exception's message
+	 *    will be added as the final parameter.
 	 *  - code: (string) Default code
-	 *  - data: (array) Extra data
-	 * @return ApiMessage
+	 *  - data: (array) Default extra data
+	 * @return IApiMessage
 	 */
 	public function getMessageFromException( $exception, array $options = [] ) {
 		$options += [ 'code' => null, 'data' => [] ];
@@ -163,11 +164,11 @@ class ApiErrorFormatter {
 			// Extract code and data from the exception, if applicable
 			if ( $exception instanceof UsageException ) {
 				$data = $exception->getMessageArray();
-				if ( !isset( $options['code'] ) ) {
+				if ( !$options['code'] ) {
 					$options['code'] = $data['code'];
 				}
 				unset( $data['code'], $data['info'] );
-				$options['data'] = array_merge( $data['code'], $options['data'] );
+				$options['data'] = array_merge( $data, $options['data'] );
 			}
 
 			if ( isset( $options['wrap'] ) ) {
diff --git a/tests/phpunit/includes/api/ApiErrorFormatterTest.php b/tests/phpunit/includes/api/ApiErrorFormatterTest.php
index 1b7f6bff57..a40db24680 100644
--- a/tests/phpunit/includes/api/ApiErrorFormatterTest.php
+++ b/tests/phpunit/includes/api/ApiErrorFormatterTest.php
@@ -504,4 +504,116 @@ class ApiErrorFormatterTest extends MediaWikiLangTestCase {
 		);
 	}
 
+	/**
+	 * @dataProvider provideGetMessageFromException
+	 * @covers ApiErrorFormatter::getMessageFromException
+	 * @covers ApiErrorFormatter::formatException
+	 * @param Exception $exception
+	 * @param array $options
+	 * @param array $expect
+	 */
+	public function testGetMessageFromException( $exception, $options, $expect ) {
+		$result = new ApiResult( 8388608 );
+		$formatter = new ApiErrorFormatter( $result, Language::factory( 'en' ), 'html', false );
+
+		$msg = $formatter->getMessageFromException( $exception, $options );
+		$this->assertInstanceOf( Message::class, $msg );
+		$this->assertInstanceOf( IApiMessage::class, $msg );
+		$this->assertSame( $expect, [
+			'text' => $msg->parse(),
+			'code' => $msg->getApiCode(),
+			'data' => $msg->getApiData(),
+		] );
+
+		$expectFormatted = $formatter->formatMessage( $msg );
+		$formatted = $formatter->formatException( $exception, $options );
+		$this->assertSame( $expectFormatted, $formatted );
+	}
+
+	/**
+	 * @dataProvider provideGetMessageFromException
+	 * @covers ApiErrorFormatter_BackCompat::formatException
+	 * @param Exception $exception
+	 * @param array $options
+	 * @param array $expect
+	 */
+	public function testGetMessageFromException_BC( $exception, $options, $expect ) {
+		$result = new ApiResult( 8388608 );
+		$formatter = new ApiErrorFormatter_BackCompat( $result );
+
+		$msg = $formatter->getMessageFromException( $exception, $options );
+		$this->assertInstanceOf( Message::class, $msg );
+		$this->assertInstanceOf( IApiMessage::class, $msg );
+		$this->assertSame( $expect, [
+			'text' => $msg->parse(),
+			'code' => $msg->getApiCode(),
+			'data' => $msg->getApiData(),
+		] );
+
+		$expectFormatted = $formatter->formatMessage( $msg );
+		$formatted = $formatter->formatException( $exception, $options );
+		$this->assertSame( $expectFormatted, $formatted );
+		$formatted = $formatter->formatException( $exception, $options + [ 'bc' => true ] );
+		$this->assertSame( $expectFormatted['info'], $formatted );
+	}
+
+	public static function provideGetMessageFromException() {
+		return [
+			'Normal exception' => [
+				new RuntimeException( '<b>Something broke!</b>' ),
+				[],
+				[
+					'text' => '&#60;b&#62;Something broke!&#60;/b&#62;',
+					'code' => 'internal_api_error_RuntimeException',
+					'data' => [],
+				]
+			],
+			'Normal exception, wrapped' => [
+				new RuntimeException( '<b>Something broke!</b>' ),
+				[ 'wrap' => 'parentheses', 'code' => 'some-code', 'data' => [ 'foo' => 'bar', 'baz' => 42 ] ],
+				[
+					'text' => '(&#60;b&#62;Something broke!&#60;/b&#62;)',
+					'code' => 'some-code',
+					'data' => [ 'foo' => 'bar', 'baz' => 42 ],
+				]
+			],
+			'UsageException' => [
+				new UsageException( '<b>Something broke!</b>', 'ue-code', 0, [ 'xxx' => 'yyy', 'baz' => 23 ] ),
+				[],
+				[
+					'text' => '&#60;b&#62;Something broke!&#60;/b&#62;',
+					'code' => 'ue-code',
+					'data' => [ 'xxx' => 'yyy', 'baz' => 23 ],
+				]
+			],
+			'UsageException, wrapped' => [
+				new UsageException( '<b>Something broke!</b>', 'ue-code', 0, [ 'xxx' => 'yyy', 'baz' => 23 ] ),
+				[ 'wrap' => 'parentheses', 'code' => 'some-code', 'data' => [ 'foo' => 'bar', 'baz' => 42 ] ],
+				[
+					'text' => '(&#60;b&#62;Something broke!&#60;/b&#62;)',
+					'code' => 'some-code',
+					'data' => [ 'xxx' => 'yyy', 'baz' => 42, 'foo' => 'bar' ],
+				]
+			],
+			'LocalizedException' => [
+				new LocalizedException( [ 'returnto', '<b>FooBar</b>' ] ),
+				[],
+				[
+					'text' => 'Return to <b>FooBar</b>.',
+					'code' => 'returnto',
+					'data' => [],
+				]
+			],
+			'LocalizedException, wrapped' => [
+				new LocalizedException( [ 'returnto', '<b>FooBar</b>' ] ),
+				[ 'wrap' => 'parentheses', 'code' => 'some-code', 'data' => [ 'foo' => 'bar', 'baz' => 42 ] ],
+				[
+					'text' => 'Return to <b>FooBar</b>.',
+					'code' => 'some-code',
+					'data' => [ 'foo' => 'bar', 'baz' => 42 ],
+				]
+			],
+		];
+	}
+
 }