Use TextFormatter in the REST API
authorTim Starling <tstarling@wikimedia.org>
Tue, 16 Jul 2019 22:43:43 +0000 (08:43 +1000)
committerTim Starling <tstarling@wikimedia.org>
Tue, 17 Sep 2019 06:03:14 +0000 (16:03 +1000)
* Add ResponseFactory::createLocalizedHttpError(), which generates a
  JSON response body from a MessageValue
* ResponseFactory::__construct() accepts an array of TextFormatter
  objects. For ease of testing, the array may be empty. The integrated
  ResponseFactory has a TextFormatter for English, and one for
  $wgContLang if that is different.
* Use createLocalizedHttpError() to show helpful error messages for
  errors generated by Router.

Change-Id: I897a0aee42227916c568333ab384966f1b87f599

includes/Rest/EntryPoint.php
includes/Rest/LocalizedHttpException.php
includes/Rest/ResponseFactory.php
includes/Rest/Router.php
languages/i18n/en.json
languages/i18n/qqq.json
tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php
tests/phpunit/includes/Rest/EntryPointTest.php
tests/phpunit/unit/includes/Rest/Handler/HelloHandlerTest.php
tests/phpunit/unit/includes/Rest/ResponseFactoryTest.php
tests/phpunit/unit/includes/Rest/RouterTest.php

index ee3441e..4fdd1f8 100644 (file)
@@ -10,6 +10,7 @@ use MediaWiki\Rest\Validator\Validator;
 use RequestContext;
 use Title;
 use WebResponse;
+use Wikimedia\Message\ITextFormatter;
 
 class EntryPoint {
        /** @var RequestInterface */
@@ -49,6 +50,8 @@ class EntryPoint {
                        'cookiePrefix' => $conf->get( 'CookiePrefix' )
                ] );
 
+               $responseFactory = new ResponseFactory( self::getTextFormatters( $services ) );
+
                // @phan-suppress-next-line PhanAccessMethodInternal
                $authorizer = new MWBasicAuthorizer( $context->getUser(),
                        $services->getPermissionManager() );
@@ -62,7 +65,7 @@ class EntryPoint {
                        ExtensionRegistry::getInstance()->getAttribute( 'RestRoutes' ),
                        $conf->get( 'RestPath' ),
                        $services->getLocalServerObjectCache(),
-                       new ResponseFactory,
+                       $responseFactory,
                        $authorizer,
                        $objectFactory,
                        $restValidator
@@ -76,6 +79,25 @@ class EntryPoint {
                $entryPoint->execute();
        }
 
+       /**
+        * Get a TextFormatter array from MediaWikiServices
+        *
+        * @param MediaWikiServices $services
+        * @return ITextFormatter[]
+        */
+       public static function getTextFormatters( MediaWikiServices $services ) {
+               $langs = array_unique( [
+                       $services->getMainConfig()->get( 'ContLang' )->getCode(),
+                       'en'
+               ] );
+               $textFormatters = [];
+               $factory = $services->getMessageFormatterFactory();
+               foreach ( $langs as $lang ) {
+                       $textFormatters[] = $factory->getTextFormatter( $lang );
+               }
+               return $textFormatters;
+       }
+
        public function __construct( RequestContext $context, RequestInterface $request,
                WebResponse $webResponse, Router $router
        ) {
index 10d3a40..184fe16 100644 (file)
@@ -5,7 +5,14 @@ namespace MediaWiki\Rest;
 use Wikimedia\Message\MessageValue;
 
 class LocalizedHttpException extends HttpException {
-       public function __construct( MessageValue $message, $code = 500 ) {
-               parent::__construct( 'Localized exception with key ' . $message->getKey(), $code );
+       private $messageValue;
+
+       public function __construct( MessageValue $messageValue, $code = 500 ) {
+               parent::__construct( 'Localized exception with key ' . $messageValue->getKey(), $code );
+               $this->messageValue = $messageValue;
+       }
+
+       public function getMessageValue() {
+               return $this->messageValue;
        }
 }
index 5e5a198..fd0f3c7 100644 (file)
@@ -5,19 +5,31 @@ namespace MediaWiki\Rest;
 use Exception;
 use HttpStatus;
 use InvalidArgumentException;
+use LanguageCode;
 use MWExceptionHandler;
 use stdClass;
 use Throwable;
+use Wikimedia\Message\ITextFormatter;
+use Wikimedia\Message\MessageValue;
 
 /**
  * Generates standardized response objects.
  */
 class ResponseFactory {
-
        const CT_PLAIN = 'text/plain; charset=utf-8';
        const CT_HTML = 'text/html; charset=utf-8';
        const CT_JSON = 'application/json';
 
+       /** @var ITextFormatter[] */
+       private $textFormatters;
+
+       /**
+        * @param ITextFormatter[] $textFormatters
+        */
+       public function __construct( $textFormatters ) {
+               $this->textFormatters = $textFormatters;
+       }
+
        /**
         * Encode a stdClass object or array to a JSON string
         *
@@ -167,13 +179,23 @@ class ResponseFactory {
                return $response;
        }
 
+       /**
+        * Create an HTTP 4xx or 5xx response with error message localisation
+        */
+       public function createLocalizedHttpError( $errorCode, MessageValue $messageValue ) {
+               return $this->createHttpError( $errorCode, $this->formatMessage( $messageValue ) );
+       }
+
        /**
         * Turn an exception into a JSON error response.
         * @param Exception|Throwable $exception
         * @return Response
         */
        public function createFromException( $exception ) {
-               if ( $exception instanceof HttpException ) {
+               if ( $exception instanceof LocalizedHttpException ) {
+                       $response = $this->createLocalizedHttpError( $exception->getCode(),
+                               $exception->getMessageValue() );
+               } elseif ( $exception instanceof HttpException ) {
                        // FIXME can HttpException represent 2xx or 3xx responses?
                        $response = $this->createHttpError(
                                $exception->getCode(),
@@ -240,4 +262,18 @@ class ResponseFactory {
                return "<!doctype html><title>Redirect</title><a href=\"$url\">$url</a>";
        }
 
+       public function formatMessage( MessageValue $messageValue ) {
+               if ( !$this->textFormatters ) {
+                       // For unit tests
+                       return [];
+               }
+               $translations = [];
+               foreach ( $this->textFormatters as $formatter ) {
+                       $lang = LanguageCode::bcp47( $formatter->getLangCode() );
+                       $messageText = $formatter->format( $messageValue );
+                       $translations[$lang] = $messageText;
+               }
+               return [ 'messageTranslations' => $translations ];
+       }
+
 }
index a520130..1c5f66b 100644 (file)
@@ -4,6 +4,7 @@ namespace MediaWiki\Rest;
 
 use AppendIterator;
 use BagOStuff;
+use Wikimedia\Message\MessageValue;
 use MediaWiki\Rest\BasicAccess\BasicAuthorizerInterface;
 use MediaWiki\Rest\PathTemplateMatcher\PathMatcher;
 use MediaWiki\Rest\Validator\Validator;
@@ -226,7 +227,10 @@ class Router {
                $path = $request->getUri()->getPath();
                $relPath = $this->getRelativePath( $path );
                if ( $relPath === false ) {
-                       return $this->responseFactory->createHttpError( 404 );
+                       return $this->responseFactory->createLocalizedHttpError( 404,
+                               ( new MessageValue( 'rest-prefix-mismatch' ) )
+                                       ->plaintextParams( $path, $this->rootPath )
+                       );
                }
 
                $matchers = $this->getMatchers();
@@ -245,12 +249,20 @@ class Router {
                                }
                        }
                        if ( $allowed ) {
-                               $response = $this->responseFactory->createHttpError( 405 );
+                               $response = $this->responseFactory->createLocalizedHttpError( 405,
+                                       ( new MessageValue( 'rest-wrong-method' ) )
+                                               ->textParams( $request->getMethod() )
+                                               ->commaListParams( $allowed )
+                                               ->numParams( count( $allowed ) )
+                               );
                                $response->setHeader( 'Allow', $allowed );
                                return $response;
                        } else {
                                // Did not match with any other method, must be 404
-                               return $this->responseFactory->createHttpError( 404 );
+                               return $this->responseFactory->createLocalizedHttpError( 404,
+                                       ( new MessageValue( 'rest-no-match' ) )
+                                               ->plaintextParams( $relPath )
+                               );
                        }
                }
 
@@ -272,6 +284,7 @@ class Router {
 
        /**
         * Execute a fully-constructed handler
+        *
         * @param Handler $handler
         * @return ResponseInterface
         */
index f5af2c5..69d4fe2 100644 (file)
        "mycustomjsredirectprotected": "You do not have permission to edit this JavaScript page because it is a redirect and it does not point inside your userspace.",
        "easydeflate-invaliddeflate": "Content provided is not properly deflated",
        "unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage",
-       "userlogout-continue": "Do you want to log out?"
+       "userlogout-continue": "Do you want to log out?",
+       "rest-prefix-mismatch": "The requested path ($1) was not inside the REST API root path ($2)",
+       "rest-wrong-method": "The request method ($1) was not {{PLURAL:$3|the allowed method for this path|one of the allowed methods for this path}} ($2)",
+       "rest-no-match": "The requested relative path ($1) did not match any known handler"
 }
index a33608f..436c11b 100644 (file)
        "mycustomjsredirectprotected": "Error message shown when user tries to edit their own JS page that is a foreign redirect without the 'mycustomjsredirectprotected' right. See also {{msg-mw|mycustomjsprotected}}.",
        "easydeflate-invaliddeflate": "Error message if the content passed to easydeflate was not deflated (compressed) properly",
        "unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected",
-       "userlogout-continue": "Shown if user attempted to log out without a token specified. Probably the user clicked on an old link that hasn't been updated to use the new system. $1 - url that user should click on in order to log out."
+       "userlogout-continue": "Shown if user attempted to log out without a token specified. Probably the user clicked on an old link that hasn't been updated to use the new system. $1 - url that user should click on in order to log out.",
+       "rest-prefix-mismatch": "Error message for REST API debugging, shown if $wgRestPath is incorrect or otherwise not matched. Parameters:\n* $1: The requested path.\n* $2: The configured root path ($wgRestPath).",
+       "rest-wrong-method": "Error message for REST API debugging, shown if the HTTP method is incorrect. Parameters:\n* $1: The received request method.\n* $2: A comma-separated list of allowed methods for this path.\n* $3: The number of items in the list $2",
+       "rest-no-match": "Error message for REST API debugging, shown if the path has the correct prefix but did not match any registered handler. Parameters:\n* $1: The received request path, relative to $wgRestPath."
 }
index 2d1fd98..a310242 100644 (file)
@@ -46,7 +46,7 @@ class MWBasicRequestAuthorizerTest extends MediaWikiTestCase {
                        [],
                        '/rest',
                        new \EmptyBagOStuff(),
-                       new ResponseFactory(),
+                       new ResponseFactory( [] ),
                        new MWBasicAuthorizer( $user, MediaWikiServices::getInstance()->getPermissionManager() ),
                        $objectFactory,
                        new Validator( $objectFactory, $request, $user )
index b984895..1c9bc41 100644 (file)
@@ -38,7 +38,7 @@ class EntryPointTest extends \MediaWikiTestCase {
                        [],
                        '/rest',
                        new EmptyBagOStuff(),
-                       new ResponseFactory(),
+                       new ResponseFactory( [] ),
                        new StaticBasicAuthorizer(),
                        $objectFactory,
                        new Validator( $objectFactory, $request, new User )
index 91652a2..7d682fd 100644 (file)
@@ -62,7 +62,7 @@ class HelloHandlerTest extends \MediaWikiUnitTestCase {
                        [],
                        '/rest',
                        new EmptyBagOStuff(),
-                       new ResponseFactory(),
+                       new ResponseFactory( [] ),
                        new StaticBasicAuthorizer(),
                        $objectFactory,
                        new Validator( $objectFactory, $request, new User )
index 04d54de..0a98686 100644 (file)
@@ -6,6 +6,8 @@ use ArrayIterator;
 use MediaWiki\Rest\HttpException;
 use MediaWiki\Rest\ResponseFactory;
 use MediaWikiUnitTestCase;
+use Wikimedia\Message\ITextFormatter;
+use Wikimedia\Message\MessageValue;
 
 /** @covers \MediaWiki\Rest\ResponseFactory */
 class ResponseFactoryTest extends MediaWikiUnitTestCase {
@@ -18,14 +20,27 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
                ];
        }
 
+       private function createResponseFactory() {
+               $fakeTextFormatter = new class implements ITextFormatter {
+                       function getLangCode() {
+                               return 'qqx';
+                       }
+
+                       function format( MessageValue $message ) {
+                               return $message->getKey();
+                       }
+               };
+               return new ResponseFactory( [ $fakeTextFormatter ] );
+       }
+
        /** @dataProvider provideEncodeJson */
        public function testEncodeJson( $input, $expected ) {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $this->assertSame( $expected, $rf->encodeJson( $input ) );
        }
 
        public function testCreateJson() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createJson( [] );
                $response->getBody()->rewind();
                $this->assertSame( 'application/json', $response->getHeaderLine( 'Content-Type' ) );
@@ -35,7 +50,7 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
        }
 
        public function testCreateNoContent() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createNoContent();
                $this->assertSame( [], $response->getHeader( 'Content-Type' ) );
                $this->assertSame( 0, $response->getBody()->getSize() );
@@ -43,35 +58,35 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
        }
 
        public function testCreatePermanentRedirect() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createPermanentRedirect( 'http://www.example.com/' );
                $this->assertSame( [ 'http://www.example.com/' ], $response->getHeader( 'Location' ) );
                $this->assertSame( 301, $response->getStatusCode() );
        }
 
        public function testCreateLegacyTemporaryRedirect() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createLegacyTemporaryRedirect( 'http://www.example.com/' );
                $this->assertSame( [ 'http://www.example.com/' ], $response->getHeader( 'Location' ) );
                $this->assertSame( 302, $response->getStatusCode() );
        }
 
        public function testCreateTemporaryRedirect() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createTemporaryRedirect( 'http://www.example.com/' );
                $this->assertSame( [ 'http://www.example.com/' ], $response->getHeader( 'Location' ) );
                $this->assertSame( 307, $response->getStatusCode() );
        }
 
        public function testCreateSeeOther() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createSeeOther( 'http://www.example.com/' );
                $this->assertSame( [ 'http://www.example.com/' ], $response->getHeader( 'Location' ) );
                $this->assertSame( 303, $response->getStatusCode() );
        }
 
        public function testCreateNotModified() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createNotModified();
                $this->assertSame( 0, $response->getBody()->getSize() );
                $this->assertSame( 304, $response->getStatusCode() );
@@ -79,12 +94,12 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
 
        /** @expectedException \InvalidArgumentException */
        public function testCreateHttpErrorInvalid() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $rf->createHttpError( 200 );
        }
 
        public function testCreateHttpError() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createHttpError( 415, [ 'message' => '...' ] );
                $this->assertSame( 415, $response->getStatusCode() );
                $body = $response->getBody();
@@ -95,7 +110,7 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
        }
 
        public function testCreateFromExceptionUnlogged() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createFromException( new HttpException( 'hello', 415 ) );
                $this->assertSame( 415, $response->getStatusCode() );
                $body = $response->getBody();
@@ -106,7 +121,7 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
        }
 
        public function testCreateFromExceptionLogged() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createFromException( new \Exception( "hello", 415 ) );
                $this->assertSame( 500, $response->getStatusCode() );
                $body = $response->getBody();
@@ -131,7 +146,7 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
 
        /** @dataProvider provideCreateFromReturnValue */
        public function testCreateFromReturnValue( $input, $expected ) {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $response = $rf->createFromReturnValue( $input );
                $body = $response->getBody();
                $body->rewind();
@@ -140,7 +155,17 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
 
        /** @expectedException \InvalidArgumentException */
        public function testCreateFromReturnValueInvalid() {
-               $rf = new ResponseFactory;
+               $rf = $this->createResponseFactory();
                $rf->createFromReturnValue( new ArrayIterator );
        }
+
+       public function testCreateLocalizedHttpError() {
+               $rf = $this->createResponseFactory();
+               $response = $rf->createLocalizedHttpError( 404, new MessageValue( 'rftest' ) );
+               $body = $response->getBody();
+               $body->rewind();
+               $this->assertSame(
+                       '{"messageTranslations":{"qqx":"rftest"},"httpCode":404,"httpReason":"Not Found"}',
+                       $body->getContents() );
+       }
 }
index e16ea25..9f51371 100644 (file)
@@ -29,7 +29,7 @@ class RouterTest extends \MediaWikiUnitTestCase {
                        [],
                        '/rest',
                        new \EmptyBagOStuff(),
-                       new ResponseFactory(),
+                       new ResponseFactory( [] ),
                        new StaticBasicAuthorizer( $authError ),
                        $objectFactory,
                        new Validator( $objectFactory, $request, new User )