Consistent handling of multiple keys in Message

Message objects may be constructed with a list of keys as a
simple fallback mechanism. This patch assures consistent
handling of this case.

Change-Id: I458c0af3114754ddf3d721f6c374e249f482e4cf

diff --git a/includes/Message.php b/includes/Message.php
index 931a3f9..4df0d80 100644 (file)
--- a/includes/Message.php
+++ b/includes/Message.php
@@ -175,10 +175,16 @@ class Message {
        protected $language = null;
 
        /**
-        * @var string|string[] The message key or array of keys.
+        * @var string The message key. If $keysToTry has more than one element,
+        * this may change to one of the keys to try when fetching the message text.
         */
        protected $key;
 
+       /**
+        * @var string[] List of keys to try when fetching the message.
+        */
+       protected $keysToTry;
+
        /**
         * @var array List of parameters which will be substituted into the message.
         */
@@ -224,30 +230,61 @@ class Message {
         * non-empty message for.
         * @param array $params Message parameters.
         * @param Language $language Optional language of the message, defaults to $wgLang.
+        *
+        * @throws InvalidArgumentException
         */
        public function __construct( $key, $params = array(), Language $language = null ) {
                global $wgLang;
 
-               $this->key = $key;
+               if ( !is_string( $key ) && !is_array( $key ) ) {
+                       throw new InvalidArgumentException( '$key must be a string or an array' );
+               }
+
+               $this->keysToTry = (array)$key;
+
+               if ( empty( $this->keysToTry ) ) {
+                       throw new InvalidArgumentException( '$key must not be an empty list' );
+               }
+
+               $this->key = reset( $this->keysToTry );
+
                $this->parameters = array_values( $params );
                $this->language = $language ? $language : $wgLang;
        }
 
        /**
-        * Returns the message key or the first from an array of message keys.
+        * @since 1.24
+        *
+        * @return bool True if this is a multi-key message, that is, if the key provided to the
+        * constructor was a fallback list of keys to try.
+        */
+       public function isMultiKey() {
+               return count( $this->keysToTry ) > 1;
+       }
+
+       /**
+        * @since 1.24
+        *
+        * @return string[] The list of keys to try when fetching the message text,
+        * in order of preference.
+        */
+       public function getKeysToTry() {
+               return $this->keysToTry;
+       }
+
+       /**
+        * Returns the message key.
+        *
+        * If a list of multiple possible keys was supplied to the constructor, this method may
+        * return any of these keys. After the message ahs been fetched, this method will return
+        * the key that was actually used to fetch the message.
         *
         * @since 1.21
         *
         * @return string
         */
        public function getKey() {
-               if ( is_array( $this->key ) ) {
-                       // May happen if some kind of fallback is applied.
-                       // For now, just use the first key. We really need a better solution.
-                       return $this->key[0];
-               } else {
-                       return $this->key;
-               }
+               return $this->key;
        }
 
        /**
@@ -637,7 +674,7 @@ class Message {
                $string = $this->fetchMessage();
 
                if ( $string === false ) {
-                       $key = htmlspecialchars( is_array( $this->key ) ? $this->key[0] : $this->key );
+                       $key = htmlspecialchars( $this->key );
                        if ( $this->format === 'plain' ) {
                                return '<' . $key . '>';
                        }
@@ -997,20 +1034,18 @@ class Message {
        protected function fetchMessage() {
                if ( $this->message === null ) {
                        $cache = MessageCache::singleton();
-                       if ( is_array( $this->key ) ) {
-                               if ( !count( $this->key ) ) {
-                                       throw new MWException( "Given empty message key array." );
-                               }
-                               foreach ( $this->key as $key ) {
-                                       $message = $cache->get( $key, $this->useDatabase, $this->language );
-                                       if ( $message !== false && $message !== '' ) {
-                                               break;
-                                       }
+
+                       foreach ( $this->keysToTry as $key ) {
+                               $message = $cache->get( $key, $this->useDatabase, $this->language );
+                               if ( $message !== false && $message !== '' ) {
+                                       break;
                                }
-                               $this->message = $message;
-                       } else {
-                               $this->message = $cache->get( $this->key, $this->useDatabase, $this->language );
                        }
+
+                       // NOTE: The constructor makes sure keysToTry isn't empty,
+                       //       so we know that $key and $message are initialized.
+                       $this->key = $key;
+                       $this->message = $message;
                }
                return $this->message;
        }
@@ -1038,13 +1073,20 @@ class RawMessage extends Message {
         *
         * @see Message::__construct
         *
-        * @param string|string[] $key Message to use.
+        * @param string $text Message to use.
         * @param array $params Parameters for the message.
+        *
+        * @throws InvalidArgumentException
         */
-       public function __construct( $key, $params = array() ) {
-               parent::__construct( $key, $params );
+       public function __construct( $text, $params = array() ) {
+               if ( !is_string( $text ) ) {
+                       throw new InvalidArgumentException( '$text must be a string' );
+               }
+
+               parent::__construct( $text, $params );
+
                // The key is the message.
-               $this->message = $key;
+               $this->message = $text;
        }
 
        /**
@@ -1057,6 +1099,7 @@ class RawMessage extends Message {
                if ( $this->message === null ) {
                        $this->message = $this->key;
                }
+
                return $this->message;
        }
 

diff --git a/tests/phpunit/includes/MessageTest.php b/tests/phpunit/includes/MessageTest.php
index 6a36b4b..c2acec0 100644 (file)
--- a/tests/phpunit/includes/MessageTest.php
+++ b/tests/phpunit/includes/MessageTest.php
@@ -317,4 +317,52 @@ class MessageTest extends MediaWikiLangTestCase {
        public function testInLanguageThrows() {
                wfMessage( 'foo' )->inLanguage( 123 );
        }
+
+       public function keyProvider() {
+               return array(
+                       'string' => array(
+                               'key' => 'mainpage',
+                               'expected' => array( 'mainpage' ),
+                       ),
+                       'single' => array(
+                               'key' => array( 'mainpage' ),
+                               'expected' => array( 'mainpage' ),
+                       ),
+                       'multi' => array(
+                               'key' => array( 'mainpage-foo', 'mainpage-bar', 'mainpage' ),
+                               'expected' => array( 'mainpage-foo', 'mainpage-bar', 'mainpage' ),
+                       ),
+                       'empty' => array(
+                               'key' => array(),
+                               'expected' => null,
+                               'exception' => 'InvalidArgumentException',
+                       ),
+                       'null' => array(
+                               'key' => null,
+                               'expected' => null,
+                               'exception' => 'InvalidArgumentException',
+                       ),
+                       'bad type' => array(
+                               'key' => 17,
+                               'expected' => null,
+                               'exception' => 'InvalidArgumentException',
+                       ),
+               );
+       }
+
+       /**
+        * @dataProvider keyProvider()
+        *
+        * @covers Message::getKey
+        */
+       public function testGetKey( $key, $expected, $exception = null ) {
+               if ( $exception ) {
+                       $this->setExpectedException( $exception );
+               }
+
+               $msg = new Message( $key );
+               $this->assertEquals( $expected, $msg->getKeysToTry() );
+               $this->assertEquals( count( $expected ) > 1, $msg->isMultiKey() );
+               $this->assertContains( $msg->getKey(), $expected );
+       }
 }