From 672ca8601686c75dee82ca8391f71c737af364ab Mon Sep 17 00:00:00 2001 From: Derick Alangi Date: Mon, 21 Nov 2022 12:00:08 +0100 Subject: [PATCH] ApiDiscussionToolsTrait: PageInfo & Compare don't need HTML for editing ApiDiscussionToolsPageInfo and ApiDiscussionToolsCompare in direct parsoid or VRS modes tries to fetch HTML using VisualEditor thus stashing the HTML gotten which we don't want, we only need it for viewing in these cases. This seems like something that was/is already happening in RESTBase. So for APIs in DiscussionTools that need the HTML for viewing, just get it from parser cache and not stash it. Bug: T323357 Change-Id: I101c1e84739a2ac1f562f2f7bdc4b8f53d9f3b23 --- includes/ApiDiscussionToolsTrait.php | 15 ++- .../ApiDiscussionToolsPageInfoTest.php | 52 ---------- .../ApiDiscussionToolsCompareTest.php | 45 +++++++++ .../ApiDiscussionToolsPageInfoTest.php | 95 +++++++++++++++++++ 4 files changed, 153 insertions(+), 54 deletions(-) delete mode 100644 tests/phpunit/ApiDiscussionToolsPageInfoTest.php create mode 100644 tests/phpunit/integration/ApiDiscussionToolsCompareTest.php create mode 100644 tests/phpunit/integration/ApiDiscussionToolsPageInfoTest.php diff --git a/includes/ApiDiscussionToolsTrait.php b/includes/ApiDiscussionToolsTrait.php index 8ffdae948..f16eb9d9d 100644 --- a/includes/ApiDiscussionToolsTrait.php +++ b/includes/ApiDiscussionToolsTrait.php @@ -9,7 +9,9 @@ use DerivativeRequest; use IContextSource; use MediaWiki\Extension\VisualEditor\ParsoidClient; use MediaWiki\Extension\VisualEditor\VisualEditorParsoidClientFactory; +use MediaWiki\MediaWikiServices; use MediaWiki\Revision\RevisionRecord; +use ParserOptions; use Title; use TitleValue; use Wikimedia\Parsoid\Utils\DOMCompat; @@ -27,9 +29,15 @@ trait ApiDiscussionToolsTrait { * @return ContentThreadItemSet */ protected function parseRevision( RevisionRecord $revision ): ContentThreadItemSet { - $response = $this->requestRestbasePageHtml( $revision ); + $parsoidOutputAccess = MediaWikiServices::getInstance()->getParsoidOutputAccess(); + $status = $parsoidOutputAccess->getParserOutput( + $revision->getPage(), + ParserOptions::newFromAnon(), + $revision + ); + $html = $status->getValue()->getText(); - $doc = DOMUtils::parseHTML( $response['body'] ); + $doc = DOMUtils::parseHTML( $html ); $container = DOMCompat::getBody( $doc ); CommentUtils::unwrapParsoidSections( $container ); @@ -146,6 +154,9 @@ trait ApiDiscussionToolsTrait { } /** + * @warning (T323357) - Calling this method writes to stash, so it should be called + * only when we are fetching page HTML for editing. + * * @param RevisionRecord $revision * @return array */ diff --git a/tests/phpunit/ApiDiscussionToolsPageInfoTest.php b/tests/phpunit/ApiDiscussionToolsPageInfoTest.php deleted file mode 100644 index d73670aef..000000000 --- a/tests/phpunit/ApiDiscussionToolsPageInfoTest.php +++ /dev/null @@ -1,52 +0,0 @@ -setupEnv( $config, $data ); - $title = MediaWikiServices::getInstance()->getTitleParser()->parseTitle( $title ); - $threadItemSet = static::createParser( $data )->parse( $container, $title ); - - $pageInfo = TestingAccessWrapper::newFromClass( ApiDiscussionToolsPageInfo::class ); - - $threadItemsHtml = $pageInfo->getThreadItemsHtml( $threadItemSet ); - - // Optionally write updated content to the JSON files - if ( getenv( 'DISCUSSIONTOOLS_OVERWRITE_TESTS' ) ) { - static::overwriteJsonFile( $expectedPath, $threadItemsHtml ); - } - - static::assertEquals( $expected, $threadItemsHtml, $name ); - - $processedThreads = []; - } - - public function provideGetThreadItemsHtml(): array { - return static::getJson( '../cases/threaditemshtml.json' ); - } - -} diff --git a/tests/phpunit/integration/ApiDiscussionToolsCompareTest.php b/tests/phpunit/integration/ApiDiscussionToolsCompareTest.php new file mode 100644 index 000000000..6bb6682b0 --- /dev/null +++ b/tests/phpunit/integration/ApiDiscussionToolsCompareTest.php @@ -0,0 +1,45 @@ +getNonexistingTestPage( $title ); + + $this->editPage( $page, "== Test ==\n\nadd DT pageinfo content\n" ); + $rev1 = $page->getLatest(); + + $this->editPage( $page, ':adding another edit' ); + $rev2 = $page->getLatest(); + + $params = [ + 'action' => 'discussiontoolscompare', + 'fromrev' => $rev1, + 'torev' => $rev2, + ]; + + $result = $this->doApiRequestWithToken( $params ); + + $this->assertNotEmpty( $result[0]['discussiontoolscompare'] ); + $this->assertArrayHasKey( 'fromrevid', $result[0]['discussiontoolscompare'] ); + $this->assertSame( $rev1, $result[0]['discussiontoolscompare']['fromrevid'] ); + $this->assertArrayHasKey( 'torevid', $result[0]['discussiontoolscompare'] ); + $this->assertSame( $rev2, $result[0]['discussiontoolscompare']['torevid'] ); + $this->assertArrayHasKey( 'removedcomments', $result[0]['discussiontoolscompare'] ); + $this->assertArrayHasKey( 'addedcomments', $result[0]['discussiontoolscompare'] ); + } + +} diff --git a/tests/phpunit/integration/ApiDiscussionToolsPageInfoTest.php b/tests/phpunit/integration/ApiDiscussionToolsPageInfoTest.php new file mode 100644 index 000000000..c89b91b18 --- /dev/null +++ b/tests/phpunit/integration/ApiDiscussionToolsPageInfoTest.php @@ -0,0 +1,95 @@ +setMwGlobals( $config ); + $this->setMwGlobals( [ + 'wgArticlePath' => $config['wgArticlePath'], + 'wgNamespaceAliases' => $config['wgNamespaceIds'], + 'wgMetaNamespace' => strtr( $config['wgFormattedNamespaces'][NS_PROJECT], ' ', '_' ), + 'wgMetaNamespaceTalk' => strtr( $config['wgFormattedNamespaces'][NS_PROJECT_TALK], ' ', '_' ), + // TODO: Move this to $config + 'wgLocaltimezone' => $data['localTimezone'], + // Data used for the tests assumes there are no variants for English. + // Language variants are tested using other languages. + 'wgUsePigLatinVariant' => false, + ] ); + $this->setUserLang( $config['wgContentLanguage'] ); + $this->setContentLang( $config['wgContentLanguage'] ); + } + + /** + * @dataProvider provideGetThreadItemsHtml + */ + public function testGetThreadItemsHtml( + string $name, string $title, string $dom, string $expected, string $config, string $data + ): void { + $dom = static::getHtml( $dom ); + $expectedPath = $expected; + $expected = static::getJson( $expected ); + $config = static::getJson( $config ); + $data = static::getJson( $data ); + + $doc = static::createDocument( $dom ); + $container = static::getThreadContainer( $doc ); + + $this->setupEnv( $config, $data ); + $title = MediaWikiServices::getInstance()->getTitleParser()->parseTitle( $title ); + $threadItemSet = static::createParser( $data )->parse( $container, $title ); + + $pageInfo = TestingAccessWrapper::newFromClass( ApiDiscussionToolsPageInfo::class ); + + $threadItemsHtml = $pageInfo->getThreadItemsHtml( $threadItemSet ); + + // Optionally write updated content to the JSON files + if ( getenv( 'DISCUSSIONTOOLS_OVERWRITE_TESTS' ) ) { + static::overwriteJsonFile( $expectedPath, $threadItemsHtml ); + } + + static::assertEquals( $expected, $threadItemsHtml, $name ); + } + + public function provideGetThreadItemsHtml(): array { + return static::getJson( '../cases/threaditemshtml.json' ); + } + + /** + * @covers \MediaWiki\Extension\DiscussionTools\ApiDiscussionToolsPageInfo::execute + */ + public function testExecuteApiDiscussionToolsPageInfo() { + $page = $this->getNonexistingTestPage( __METHOD__ ); + $this->editPage( $page, 'add DT pageinfo content' ); + + $params = [ + 'action' => 'discussiontoolspageinfo', + 'page' => $page->getTitle()->getText(), + ]; + + $result = $this->doApiRequestWithToken( $params ); + + $this->assertNotEmpty( $result[0]['discussiontoolspageinfo'] ); + $this->assertArrayHasKey( 'transcludedfrom', $result[0]['discussiontoolspageinfo'] ); + } + +}