Allow Message::newFromSpecifier to handle ApiMessages

Instead of constructing a new Message from the Message as
a MessageSpecifier, just clone the existing Message which will preserve
subclass data.

Also, make use of this to simplify the logic in ApiBase::parseMsg().

Change-Id: I9545acb8da752c0c21e16d8b1d37d8802fcb329d

diff --git a/includes/Message.php b/includes/Message.php
index 712d3f1..d0325d7 100644 (file)
--- a/includes/Message.php
+++ b/includes/Message.php
@@ -402,8 +402,8 @@ class Message implements MessageSpecifier, Serializable {
                        $value = array_shift( $params );
                }
 
-               if ( $value instanceof RawMessage ) {
-                       $message = new RawMessage( $value->getKey(), $value->getParams() );
+               if ( $value instanceof Message ) { // Message, RawMessage, ApiMessage, etc
+                       $message = clone( $value );
                } elseif ( $value instanceof MessageSpecifier ) {
                        $message = new Message( $value );
                } elseif ( is_string( $value ) ) {

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 639f6be..3e57e89 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -2093,7 +2093,7 @@ abstract class ApiBase extends ContextSource {
 
        /**
         * Output the error message related to a certain array
-        * @param array|string $error Element of a getUserPermissionsErrors()-style array
+        * @param array|string|MessageSpecifier $error Element of a getUserPermissionsErrors()-style array
         * @throws UsageException always
         */
        public function dieUsageMsg( $error ) {
@@ -2110,7 +2110,7 @@ abstract class ApiBase extends ContextSource {
        /**
         * Will only set a warning instead of failing if the global $wgDebugAPI
         * is set to true. Otherwise behaves exactly as dieUsageMsg().
-        * @param array|string $error Element of a getUserPermissionsErrors()-style array
+        * @param array|string|MessageSpecifier $error Element of a getUserPermissionsErrors()-style array
         * @throws UsageException
         * @since 1.21
         */
@@ -2143,32 +2143,38 @@ abstract class ApiBase extends ContextSource {
 
        /**
         * Return the error message related to a certain array
-        * @param array $error Element of a getUserPermissionsErrors()-style array
+        * @param array|string|MessageSpecifier $error Element of a getUserPermissionsErrors()-style array
         * @return array('code' => code, 'info' => info)
         */
        public function parseMsg( $error ) {
-               $error = (array)$error; // It seems strings sometimes make their way in here
-               $key = array_shift( $error );
-
-               // Check whether the error array was nested
-               // array( array( <code>, <params> ), array( <another_code>, <params> ) )
-               if ( is_array( $key ) ) {
-                       $error = $key;
-                       $key = array_shift( $error );
+               // Check whether someone passed the whole array, instead of one element as
+               // documented. This breaks if it's actually an array of fallback keys, but
+               // that's long-standing misbehavior introduced in r87627 to incorrectly
+               // fix T30797.
+               if ( is_array( $error ) ) {
+                       $first = reset( $error );
+                       if ( is_array( $first ) ) {
+                               wfDebug( __METHOD__ . ' was passed an array of arrays. ' . wfGetAllCallers( 5 ) );
+                               $error = $first;
+                       }
                }
 
-               if ( $key instanceof IApiMessage ) {
+               $msg = Message::newFromSpecifier( $error );
+
+               if ( $msg instanceof IApiMessage ) {
                        return [
-                               'code' => $key->getApiCode(),
-                               'info' => $key->inLanguage( 'en' )->useDatabase( false )->text(),
-                               'data' => $key->getApiData()
+                               'code' => $msg->getApiCode(),
+                               'info' => $msg->inLanguage( 'en' )->useDatabase( false )->text(),
+                               'data' => $msg->getApiData()
                        ];
                }
 
+               $key = $msg->getKey();
                if ( isset( self::$messageMap[$key] ) ) {
+                       $params = $msg->getParams();
                        return [
-                               'code' => wfMsgReplaceArgs( self::$messageMap[$key]['code'], $error ),
-                               'info' => wfMsgReplaceArgs( self::$messageMap[$key]['info'], $error )
+                               'code' => wfMsgReplaceArgs( self::$messageMap[$key]['code'], $params ),
+                               'info' => wfMsgReplaceArgs( self::$messageMap[$key]['info'], $params )
                        ];
                }
 

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 0a79aa4..15c1e39 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -347,7 +347,7 @@ class ApiUpload extends ApiBase {
         * Throw an error that the user can recover from by providing a better
         * value for $parameter
         *
-        * @param array $error Error array suitable for passing to dieUsageMsg()
+        * @param array|string|MessageSpecifier $error Error suitable for passing to dieUsageMsg()
         * @param string $parameter Parameter that needs revising
         * @param array $data Optional extra data to pass to the user
         * @throws UsageException

diff --git a/tests/phpunit/includes/MessageTest.php b/tests/phpunit/includes/MessageTest.php
index 8aa1361..c4f3fb1 100644 (file)
--- a/tests/phpunit/includes/MessageTest.php
+++ b/tests/phpunit/includes/MessageTest.php
@@ -589,6 +589,10 @@ class MessageTest extends MediaWikiLangTestCase {
        public function testNewFromSpecifier( $value, $expectedText ) {
                $message = Message::newFromSpecifier( $value );
                $this->assertInstanceOf( Message::class, $message );
+               if ( $value instanceof Message ) {
+                       $this->assertInstanceOf( get_class( $value ), $message );
+                       $this->assertEquals( $value, $message );
+               }
                $this->assertSame( $expectedText, $message->text() );
        }
 
@@ -602,6 +606,7 @@ class MessageTest extends MediaWikiLangTestCase {
                        'array' => [ [ 'youhavenewmessages', 'foo', 'bar' ], 'You have foo (bar).' ],
                        'Message' => [ new Message( 'youhavenewmessages', [ 'foo', 'bar' ] ), 'You have foo (bar).' ],
                        'RawMessage' => [ new RawMessage( 'foo ($1)', [ 'bar' ] ), 'foo (bar)' ],
+                       'ApiMessage' => [ new ApiMessage( [ 'mainpage' ], 'code', [ 'data' ] ), 'Main Page' ],
                        'MessageSpecifier' => [ $messageSpecifier, 'Main Page' ],
                        'nested RawMessage' => [ [ new RawMessage( 'foo ($1)', [ 'bar' ] ) ], 'foo (bar)' ],
                ];