diff --git a/autoload.php b/autoload.php index e0e1ba3f842..eb2fcaf38bc 100644 --- a/autoload.php +++ b/autoload.php @@ -1684,6 +1684,8 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Rest\\Handler\\CreationHandler' => __DIR__ . '/includes/Rest/Handler/CreationHandler.php', 'MediaWiki\\Rest\\Handler\\EditHandler' => __DIR__ . '/includes/Rest/Handler/EditHandler.php', 'MediaWiki\\Rest\\Handler\\HtmlInputTransformHelper' => __DIR__ . '/includes/Rest/Handler/HtmlInputTransformHelper.php', + 'MediaWiki\\Rest\\Handler\\HtmlMessageOutputHelper' => __DIR__ . '/includes/Rest/Handler/HtmlMessageOutputHelper.php', + 'MediaWiki\\Rest\\Handler\\HtmlOutputHelper' => __DIR__ . '/includes/Rest/Handler/HtmlOutputHelper.php', 'MediaWiki\\Rest\\Handler\\HtmlOutputRendererHelper' => __DIR__ . '/includes/Rest/Handler/HtmlOutputRendererHelper.php', 'MediaWiki\\Rest\\Handler\\LanguageLinksHandler' => __DIR__ . '/includes/Rest/Handler/LanguageLinksHandler.php', 'MediaWiki\\Rest\\Handler\\MediaFileHandler' => __DIR__ . '/includes/Rest/Handler/MediaFileHandler.php', diff --git a/includes/Rest/Handler/HtmlMessageOutputHelper.php b/includes/Rest/Handler/HtmlMessageOutputHelper.php new file mode 100644 index 00000000000..997de56e9fd --- /dev/null +++ b/includes/Rest/Handler/HtmlMessageOutputHelper.php @@ -0,0 +1,123 @@ +page = $page; + } + + /** + * @return Message|null + */ + private function getDefaultSystemMessage(): ?Message { + $title = Title::castFromPageIdentity( $this->page ); + + return $title ? $title->getDefaultSystemMessage() : null; + } + + /** + * @inheritDoc + */ + public function getHtml(): ParserOutput { + $message = $this->getDefaultSystemMessage(); + + // NOTE: This class should be used only for system messages, + // so failing hard here is fine if we're not dealing with one. + $messageDom = DOMUtils::parseHTML( $message->parse() ); + DOMUtils::appendToHead( $messageDom, 'meta', [ + 'http-equiv' => 'content-language', + 'content' => LanguageCode::bcp47( $message->getLanguage()->getCode() ), + ] ); + + $messageDocHtml = ContentUtils::toXML( $messageDom ); + + return new ParserOutput( $messageDocHtml ); + } + + /** + * @inheritDoc + */ + public function getETag( string $suffix = '' ): ?string { + // XXX: We end up generating the HTML twice. Would be nice to avoid that. + // But messages are small, and not hit a lot... + $output = $this->getHtml(); + + return '"message/' . sha1( $output->getRawText() ) . '/' . $suffix . '"'; + } + + /** + * @inheritDoc + * + * @note This is guaranteed to always return NULL since + * proper system messages (with no DB entry) have no + * revision, so they should have no last modified time. + */ + public function getLastModified(): ?string { + return null; + } + + /** + * @inheritDoc + */ + public function getParamSettings(): array { + return []; + } + + /** + * @inheritDoc + */ + public function setVariantConversionLanguage( + string $targetLanguageCode, + ?string $sourceLanguageCode = null + ): void { + // TODO: Set language in the response headers. + } + + public function putHeaders( + ResponseInterface $response, + bool $forHtml = true + ): void { + // TODO: Set language in the response headers. + } + +} diff --git a/includes/Rest/Handler/HtmlOutputHelper.php b/includes/Rest/Handler/HtmlOutputHelper.php new file mode 100644 index 00000000000..9d5dec441a0 --- /dev/null +++ b/includes/Rest/Handler/HtmlOutputHelper.php @@ -0,0 +1,98 @@ +revisionOrId = $revisionOrId; if ( $this->getRevisionId() === null ) { @@ -335,11 +340,15 @@ class HtmlOutputRendererHelper { } /** + * Initializes the helper with the given parameters like the page + * we're dealing with, parameters gotten from the request inputs, + * and the revision if any is available. + * * @param PageIdentity $page * @param array $parameters * @param User $user - * @param RevisionRecord|int|null $revision DEPRECATED, use setRevision() - * @param Language|null $pageLanguage DEPRECATED, use setPageLanguage() + * @param RevisionRecord|int|null $revision + * @param Language|null $pageLanguage */ public function init( PageIdentity $page, @@ -350,10 +359,16 @@ class HtmlOutputRendererHelper { ) { $this->page = $page; $this->user = $user; - $this->revisionOrId = $revision; - $this->pageLanguage = $pageLanguage; $this->stash = $parameters['stash'] ?? false; + if ( $revision !== null ) { + $this->setRevision( $revision ); + } + + if ( $pageLanguage !== null ) { + $this->setPageLanguage( $pageLanguage ); + } + if ( $this->stash ) { $this->setFlavor( 'stash' ); } else { @@ -362,19 +377,18 @@ class HtmlOutputRendererHelper { } /** - * Set the language to be used for variant conversion - * @param string $targetLanguageCode - * @param null|string $sourceLanguageCode + * @inheritDoc */ - public function setVariantConversionLanguage( string $targetLanguageCode, ?string $sourceLanguageCode = null ) { + public function setVariantConversionLanguage( + string $targetLanguageCode, + ?string $sourceLanguageCode = null + ): void { $this->targetLanguageCode = $targetLanguageCode; $this->sourceLanguageCode = $sourceLanguageCode; } /** - * @return ParserOutput a tuple with html and content-type - * @throws LocalizedHttpException - * @throws ClientError + * @inheritDoc */ public function getHtml(): ParserOutput { if ( $this->processedParserOutput ) { @@ -442,11 +456,7 @@ class HtmlOutputRendererHelper { } /** - * Returns an ETag uniquely identifying the HTML output. - * - * @param string $suffix A suffix to attach to the etag. - * - * @return string|null + * @inheritDoc */ public function getETag( string $suffix = '' ): ?string { $parserOutput = $this->getParserOutput(); @@ -467,16 +477,14 @@ class HtmlOutputRendererHelper { } /** - * Returns the time at which the HTML was rendered. - * - * @return string|null + * @inheritDoc */ public function getLastModified(): ?string { return $this->getParserOutput()->getCacheTime(); } /** - * @return array + * @inheritDoc */ public function getParamSettings(): array { return [ @@ -566,14 +574,9 @@ class HtmlOutputRendererHelper { } /** - * Set the HTTP headers based on the response generated - * - * @param ResponseInterface $response - * @param bool $forHtml Whether the response will be HTML (rather than JSON) - * - * @return void + * @inheritDoc */ - public function putHeaders( ResponseInterface $response, bool $forHtml = true ) { + public function putHeaders( ResponseInterface $response, bool $forHtml = true ): void { if ( $forHtml ) { // For HTML we want to set the Content-Language. For JSON, we probably don't. $response->setHeader( 'Content-Language', $this->getHtmlOutputContentLanguage() ); diff --git a/includes/Rest/Handler/PageHTMLHandler.php b/includes/Rest/Handler/PageHTMLHandler.php index 725b812eecb..45704200a68 100644 --- a/includes/Rest/Handler/PageHTMLHandler.php +++ b/includes/Rest/Handler/PageHTMLHandler.php @@ -2,7 +2,6 @@ namespace MediaWiki\Rest\Handler; -use LanguageCode; use LogicException; use MediaWiki\MediaWikiServices; use MediaWiki\Page\RedirectStore; @@ -10,11 +9,8 @@ use MediaWiki\Rest\LocalizedHttpException; use MediaWiki\Rest\Response; use MediaWiki\Rest\SimpleHandler; use MediaWiki\Rest\StringStream; -use ParserOutput; use TitleFormatter; use Wikimedia\Assert\Assert; -use Wikimedia\Parsoid\Utils\ContentUtils; -use Wikimedia\Parsoid\Utils\DOMUtils; /** * A handler that returns Parsoid HTML for the following routes: @@ -26,7 +22,7 @@ use Wikimedia\Parsoid\Utils\DOMUtils; class PageHTMLHandler extends SimpleHandler { use PageRedirectHandlerTrait; - /** @var HtmlOutputRendererHelper */ + /** @var HtmlOutputHelper */ private $htmlHelper; /** @var PageContentHelper */ @@ -38,6 +34,8 @@ class PageHTMLHandler extends SimpleHandler { /** @var RedirectStore */ private $redirectStore; + private PageRestHelperFactory $helperFactory; + public function __construct( TitleFormatter $titleFormatter, RedirectStore $redirectStore, @@ -46,6 +44,7 @@ class PageHTMLHandler extends SimpleHandler { $this->titleFormatter = $titleFormatter; $this->redirectStore = $redirectStore; $this->contentHelper = $helperFactory->newPageContentHelper(); + $this->helperFactory = $helperFactory; $this->htmlHelper = $helperFactory->newHtmlOutputRendererHelper(); } @@ -57,19 +56,23 @@ class PageHTMLHandler extends SimpleHandler { $this->contentHelper->init( $user, $this->getValidatedParams() ); $page = $this->contentHelper->getPageIdentity(); + $isSystemMessage = $this->contentHelper->useDefaultSystemMessage(); - if ( $this->contentHelper->useDefaultSystemMessage() ) { - // We can't use the helper object with system messages. - // TODO: We should have an implementation of HtmlOutputRendererHelper - // for system messages in the future. - // Currently NO OP. - } elseif ( $page ) { - $this->htmlHelper->init( $page, $this->getValidatedParams(), $user ); - - $request = $this->getRequest(); - $acceptLanguage = $request->getHeaderLine( 'Accept-Language' ) ?: null; - if ( $acceptLanguage ) { - $this->htmlHelper->setVariantConversionLanguage( $acceptLanguage ); + if ( $page ) { + if ( $isSystemMessage ) { + $this->htmlHelper = $this->helperFactory->newHtmlMessageOutputHelper(); + $this->htmlHelper->init( $page ); + } else { + $revision = $this->contentHelper->getTargetRevision(); + // NOTE: We know that $this->htmlHelper is an instance of HtmlOutputRendererHelper + // because we set it in the constructor. + $this->htmlHelper->init( $page, $this->getValidatedParams(), $user, $revision ); + + $request = $this->getRequest(); + $acceptLanguage = $request->getHeaderLine( 'Accept-Language' ) ?: null; + if ( $acceptLanguage ) { + $this->htmlHelper->setVariantConversionLanguage( $acceptLanguage ); + } } } } @@ -80,8 +83,7 @@ class PageHTMLHandler extends SimpleHandler { */ public function run(): Response { $this->contentHelper->checkAccess(); - $page = $this->contentHelper->getPage(); - $isSystemMessage = $this->contentHelper->useDefaultSystemMessage(); + $page = $this->contentHelper->getPageIdentity(); $params = $this->getRequest()->getQueryParams(); if ( array_key_exists( 'redirect', $params ) ) { @@ -92,30 +94,22 @@ class PageHTMLHandler extends SimpleHandler { // The call to $this->contentHelper->getPage() should not return null if // $this->contentHelper->checkAccess() did not throw. - Assert::invariant( - $page !== null || $isSystemMessage, - 'Page should be known or be a valid system message page' + Assert::invariant( $page !== null, 'Page should be known' ); + + $redirectResponse = $this->createRedirectResponseIfNeeded( + $page, + $followWikiRedirects, + $this->contentHelper->getTitleText(), + $this->titleFormatter, + $this->redirectStore ); - if ( $isSystemMessage ) { - $parserOutput = $this->getSystemMessageOutput(); - } else { - '@phan-var \MediaWiki\Page\ExistingPageRecord $page'; - $redirectResponse = $this->createRedirectResponseIfNeeded( - $page, - $followWikiRedirects, - $this->contentHelper->getTitleText(), - $this->titleFormatter, - $this->redirectStore - ); - - if ( $redirectResponse !== null ) { - return $redirectResponse; - } - - $parserOutput = $this->htmlHelper->getHtml(); + if ( $redirectResponse !== null ) { + return $redirectResponse; } + $parserOutput = $this->htmlHelper->getHtml(); + // Do not de-duplicate styles, Parsoid already does it in a slightly different way (T300325) $parserOutputHtml = $parserOutput->getText( [ 'deduplicateStyles' => false ] ); @@ -130,17 +124,12 @@ class PageHTMLHandler extends SimpleHandler { $body = $this->contentHelper->constructMetadata(); $body['html'] = $parserOutputHtml; - if ( $page ) { - // If param redirect=no is present, that means this page can be a redirect - // check for a redirectTargetUrl and send it to the body as `redirect_target` - '@phan-var \MediaWiki\Page\ExistingPageRecord $page'; - $redirectTargetUrl = $this->getWikiRedirectTargetUrl( - $page, $this->redirectStore, $this->titleFormatter - ); - - if ( $redirectTargetUrl ) { - $body['redirect_target'] = $redirectTargetUrl; - } + $redirectTargetUrl = $this->getWikiRedirectTargetUrl( + $page, $this->redirectStore, $this->titleFormatter + ); + + if ( $redirectTargetUrl ) { + $body['redirect_target'] = $redirectTargetUrl; } $response = $this->getResponseFactory()->createJson( $body ); @@ -150,29 +139,12 @@ class PageHTMLHandler extends SimpleHandler { throw new LogicException( "Unknown HTML type $outputMode" ); } - if ( !$isSystemMessage ) { - $setContentLanguageHeader = ( $outputMode === 'html' ); - $this->htmlHelper->putHeaders( $response, $setContentLanguageHeader ); - } + $setContentLanguageHeader = ( $outputMode === 'html' ); + $this->htmlHelper->putHeaders( $response, $setContentLanguageHeader ); return $response; } - private function getSystemMessageOutput(): ParserOutput { - $message = $this->contentHelper->getDefaultSystemMessage(); - - $messageDom = DOMUtils::parseHTML( $message->parse() ); - DOMUtils::appendToHead( $messageDom, 'meta', [ - 'http-equiv' => 'content-language', - 'content' => LanguageCode::bcp47( $message->getLanguage()->getCode() ), - ] ); - - $messageDocHtml = ContentUtils::toXML( $messageDom ); - - // TODO: Set language in the response headers. - return new ParserOutput( $messageDocHtml ); - } - /** * Returns an ETag representing a page's source. The ETag assumes a page's source has changed * if the latest revision of a page has been made private, un-readable for another reason, @@ -184,13 +156,6 @@ class PageHTMLHandler extends SimpleHandler { return null; } - if ( $this->contentHelper->useDefaultSystemMessage() ) { - // XXX: We end up generating the HTML twice. Would be nice to avoid that. - // But messages are small, and not hit a lot... - $output = $this->getSystemMessageOutput(); - return '"message/' . sha1( $output->getRawText() ) . '/' . $this->getOutputMode() . '"'; - } - // Vary eTag based on output mode return $this->htmlHelper->getETag( $this->getOutputMode() ); } @@ -203,10 +168,6 @@ class PageHTMLHandler extends SimpleHandler { return null; } - if ( $this->contentHelper->useDefaultSystemMessage() ) { - return null; - } - return $this->htmlHelper->getLastModified(); } diff --git a/includes/Rest/Handler/PageRestHelperFactory.php b/includes/Rest/Handler/PageRestHelperFactory.php index 5967229fc71..d40df082252 100644 --- a/includes/Rest/Handler/PageRestHelperFactory.php +++ b/includes/Rest/Handler/PageRestHelperFactory.php @@ -99,6 +99,10 @@ class PageRestHelperFactory { ); } + public function newHtmlMessageOutputHelper(): HtmlMessageOutputHelper { + return new HtmlMessageOutputHelper(); + } + public function newHtmlInputTransformHelper( $envOptions = [] ): HtmlInputTransformHelper { return new HtmlInputTransformHelper( $this->stats, diff --git a/includes/Rest/Handler/RevisionHTMLHandler.php b/includes/Rest/Handler/RevisionHTMLHandler.php index 5c4b800fa16..bcd31589aa4 100644 --- a/includes/Rest/Handler/RevisionHTMLHandler.php +++ b/includes/Rest/Handler/RevisionHTMLHandler.php @@ -42,8 +42,7 @@ class RevisionHTMLHandler extends SimpleHandler { $revision = $this->contentHelper->getTargetRevision(); if ( $page && $revision ) { - $this->htmlHelper->init( $page, $this->getValidatedParams(), $user ); - $this->htmlHelper->setRevision( $revision ); + $this->htmlHelper->init( $page, $this->getValidatedParams(), $user, $revision ); $request = $this->getRequest(); $acceptLanguage = $request->getHeaderLine( 'Accept-Language' ) ?: null; diff --git a/tests/phpunit/integration/includes/Rest/Handler/HtmlMessageOutputHelperTest.php b/tests/phpunit/integration/includes/Rest/Handler/HtmlMessageOutputHelperTest.php new file mode 100644 index 00000000000..c0392f1109f --- /dev/null +++ b/tests/phpunit/integration/includes/Rest/Handler/HtmlMessageOutputHelperTest.php @@ -0,0 +1,71 @@ +getNonexistingTestPage( 'MediaWiki:Logouttext' ); + + $helper = $this->newHelper(); + $helper->init( $page ); + + $this->assertSame( 0, $page->getLatest() ); + + $htmlresult = $helper->getHtml()->getRawText(); + + $this->assertStringContainsString( 'You are now logged out', $htmlresult ); + // Check that we have a full HTML document in English + $this->assertStringContainsString( 'assertStringContainsString( 'content="en"', $htmlresult ); + } + + /** + * @covers \MediaWiki\Rest\Handler\HtmlMessageOutputHelper::init + * @covers \MediaWiki\Rest\Handler\HtmlMessageOutputHelper::getETag + */ + public function testGetETag() { + $page = $this->getNonexistingTestPage( 'MediaWiki:Logouttext' ); + + $helper = $this->newHelper(); + $helper->init( $page ); + + $etag = $helper->getETag(); + + $this->assertStringContainsString( '"message/', $etag ); + } + + /** + * @covers \MediaWiki\Rest\Handler\HtmlMessageOutputHelper::init + * @covers \MediaWiki\Rest\Handler\HtmlMessageOutputHelper::getHtml + */ + public function testGetHtmlWithLanguageCode() { + $page = $this->getNonexistingTestPage( 'MediaWiki:Logouttext/de' ); + + $helper = $this->newHelper(); + $helper->init( $page ); + + $this->assertSame( 0, $page->getLatest() ); + + $htmlresult = $helper->getHtml()->getRawText(); + + $this->assertStringContainsString( 'Du bist nun abgemeldet', $htmlresult ); + // Check that we have a full HTML document in English + $this->assertStringContainsString( 'assertStringContainsString( 'content="de"', $htmlresult ); + } +} diff --git a/tests/phpunit/integration/includes/Rest/Handler/PageRestHelperFactoryTest.php b/tests/phpunit/integration/includes/Rest/Handler/PageRestHelperFactoryTest.php new file mode 100644 index 00000000000..751c67f9bd9 --- /dev/null +++ b/tests/phpunit/integration/includes/Rest/Handler/PageRestHelperFactoryTest.php @@ -0,0 +1,33 @@ +getServiceContainer()->getPageRestHelperFactory(); + + $helper = $helperFactory->newHtmlMessageOutputHelper(); + + $this->assertInstanceOf( HtmlMessageOutputHelper::class, $helper ); + $this->assertInstanceOf( HtmlOutputHelper::class, $helper ); + + $helper = $helperFactory->newHtmlOutputRendererHelper(); + + $this->assertInstanceOf( HtmlOutputRendererHelper::class, $helper ); + $this->assertInstanceOf( HtmlOutputHelper::class, $helper ); + } + +} diff --git a/tests/phpunit/integration/includes/Rest/Handler/ParsoidHandlerTest.php b/tests/phpunit/integration/includes/Rest/Handler/ParsoidHandlerTest.php index 59df8513d4e..c7e158e074d 100644 --- a/tests/phpunit/integration/includes/Rest/Handler/ParsoidHandlerTest.php +++ b/tests/phpunit/integration/includes/Rest/Handler/ParsoidHandlerTest.php @@ -1965,7 +1965,7 @@ class ParsoidHandlerTest extends MediaWikiIntegrationTestCase { * @dataProvider provideWt2html * * @param array $attribs - * @param string $text + * @param string|null $text * @param string[] $expectedData * @param string[] $unexpectedHtml * @param string[] $expectedHeaders diff --git a/tests/phpunit/unit/includes/Rest/Handler/PageHandlerTestTrait.php b/tests/phpunit/unit/includes/Rest/Handler/PageHandlerTestTrait.php index c43efd23df2..b67afeacc21 100644 --- a/tests/phpunit/unit/includes/Rest/Handler/PageHandlerTestTrait.php +++ b/tests/phpunit/unit/includes/Rest/Handler/PageHandlerTestTrait.php @@ -8,6 +8,7 @@ use MediaWiki\MainConfigNames; use MediaWiki\MainConfigSchema; use MediaWiki\Parser\ParserCacheFactory; use MediaWiki\Parser\Parsoid\ParsoidOutputAccess; +use MediaWiki\Rest\Handler\HtmlMessageOutputHelper; use MediaWiki\Rest\Handler\HtmlOutputRendererHelper; use MediaWiki\Rest\Handler\LanguageLinksHandler; use MediaWiki\Rest\Handler\PageContentHelper; @@ -83,7 +84,7 @@ trait PageHandlerTestTrait { $helperFactory = $this->createNoOpMock( PageRestHelperFactory::class, - [ 'newPageContentHelper', 'newHtmlOutputRendererHelper' ] + [ 'newPageContentHelper', 'newHtmlOutputRendererHelper', 'newHtmlMessageOutputHelper' ] ); $helperFactory->method( 'newPageContentHelper' ) @@ -94,15 +95,20 @@ trait PageHandlerTestTrait { $services->getPageStore() ) ); + $parsoidOutputStash = $this->getParsoidOutputStash(); $helperFactory->method( 'newHtmlOutputRendererHelper' ) - ->willReturn( new HtmlOutputRendererHelper( - $this->getParsoidOutputStash(), - $services->getStatsdDataFactory(), - $parsoidOutputAccess, - $services->getHtmlTransformFactory(), - $services->getContentHandlerFactory(), - $services->getLanguageFactory() - ) ); + ->willReturn( + new HtmlOutputRendererHelper( + $parsoidOutputStash, + $services->getStatsdDataFactory(), + $parsoidOutputAccess, + $services->getHtmlTransformFactory(), + $services->getContentHandlerFactory(), + $services->getLanguageFactory() + ) + ); + $helperFactory->method( 'newHtmlMessageOutputHelper' ) + ->willReturn( new HtmlMessageOutputHelper() ); return new PageHTMLHandler( $services->getTitleFormatter(),