From 845b36c7bb8295ee348d0ab1bbdbcde6a306192f Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 12 Oct 2021 11:39:32 +0200 Subject: [PATCH] ParserOutput: remove Title from public interface Usages of Title in the public interface of ParsrOutput are being replaced with LinkTarget or PageReference, as appropriate. Change-Id: I571cf18fd7e666a59ef3947b09c2e75357bcac04 --- includes/parser/ParserOutput.php | 53 ++++++++++-------- .../includes/parser/ParserOutputTest.php | 55 ++++++++++++++++--- 2 files changed, 79 insertions(+), 29 deletions(-) diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php index ce1170973fc..d5926493b13 100644 --- a/includes/parser/ParserOutput.php +++ b/includes/parser/ParserOutput.php @@ -4,7 +4,10 @@ use HtmlFormatter\HtmlFormatter; use MediaWiki\Json\JsonUnserializable; use MediaWiki\Json\JsonUnserializableTrait; use MediaWiki\Json\JsonUnserializer; +use MediaWiki\Linker\LinkTarget; use MediaWiki\Logger\LoggerFactory; +use MediaWiki\MediaWikiServices; +use MediaWiki\Page\PageReference; use Wikimedia\Reflection\GhostFieldAccessTrait; /** @@ -874,17 +877,17 @@ class ParserOutput extends CacheTime { /** * Record a local or interwiki inline link for saving in future link tables. * - * @param Title $title + * @param LinkTarget $link (used to require Title until 1.38) * @param int|null $id Optional known page_id so we can skip the lookup */ - public function addLink( Title $title, $id = null ) { - if ( $title->isExternal() ) { + public function addLink( LinkTarget $link, $id = null ) { + if ( $link->isExternal() ) { // Don't record interwikis in pagelinks - $this->addInterwikiLink( $title ); + $this->addInterwikiLink( $link ); return; } - $ns = $title->getNamespace(); - $dbk = $title->getDBkey(); + $ns = $link->getNamespace(); + $dbk = $link->getDBkey(); if ( $ns === NS_MEDIA ) { // Normalize this pseudo-alias if it makes it down here... $ns = NS_FILE; @@ -901,7 +904,8 @@ class ParserOutput extends CacheTime { $this->mLinks[$ns] = []; } if ( $id === null ) { - $id = $title->getArticleID(); + $page = MediaWikiServices::getInstance()->getPageStore()->getPageForLink( $link ); + $id = $page->getId(); } $this->mLinks[$ns][$dbk] = $id; } @@ -921,13 +925,14 @@ class ParserOutput extends CacheTime { /** * Register a template dependency for this output - * @param Title $title + * + * @param LinkTarget $link (used to require Title until 1.38) * @param int $page_id * @param int $rev_id */ - public function addTemplate( $title, $page_id, $rev_id ) { - $ns = $title->getNamespace(); - $dbk = $title->getDBkey(); + public function addTemplate( $link, $page_id, $rev_id ) { + $ns = $link->getNamespace(); + $dbk = $link->getDBkey(); if ( !isset( $this->mTemplates[$ns] ) ) { $this->mTemplates[$ns] = []; } @@ -939,18 +944,20 @@ class ParserOutput extends CacheTime { } /** - * @param Title $title Title object, must be an interwiki link + * @param LinkTarget $link LinkTarget object, must be an interwiki link + * (used to require Title until 1.38). + * * @throws MWException If given invalid input */ - public function addInterwikiLink( $title ) { - if ( !$title->isExternal() ) { + public function addInterwikiLink( $link ) { + if ( !$link->isExternal() ) { throw new MWException( 'Non-interwiki link passed, internal parser error.' ); } - $prefix = $title->getInterwiki(); + $prefix = $link->getInterwiki(); if ( !isset( $this->mInterwikiLinks[$prefix] ) ) { $this->mInterwikiLinks[$prefix] = []; } - $this->mInterwikiLinks[$prefix][$title->getDBkey()] = 1; + $this->mInterwikiLinks[$prefix][$link->getDBkey()] = 1; } /** @@ -1028,28 +1035,30 @@ class ParserOutput extends CacheTime { * @todo Migrate some code to TrackingCategories * * @param string $msg Message key - * @param Title $title title of the page which is being tracked + * @param PageReference $page the page which is being tracked + * (used to require a Title until 1.38) * @return bool Whether the addition was successful * @since 1.25 */ - public function addTrackingCategory( $msg, $title ) { - if ( $title->isSpecialPage() ) { + public function addTrackingCategory( $msg, PageReference $page ) { + if ( $page->getNamespace() === NS_SPECIAL ) { wfDebug( __METHOD__ . ": Not adding tracking category $msg to special page!" ); return false; } // Important to parse with correct title (T33469) $cat = wfMessage( $msg ) - ->page( $title ) + ->page( $page ) ->inContentLanguage() ->text(); - # Allow tracking categories to be disabled by setting them to "-" + // Allow tracking categories to be disabled by setting them to "-" if ( $cat === '-' ) { return false; } - $containerCategory = Title::makeTitleSafe( NS_CATEGORY, $cat ); + $titleParser = MediaWikiServices::getInstance()->getTitleParser(); + $containerCategory = $titleParser->makeTitleValueSafe( NS_CATEGORY, $cat ); if ( $containerCategory ) { $this->addCategory( $containerCategory->getDBkey(), $this->getPageProperty( 'defaultsort' ) ?: '' ); return true; diff --git a/tests/phpunit/includes/parser/ParserOutputTest.php b/tests/phpunit/includes/parser/ParserOutputTest.php index eeeda508cbc..de5cfbf6da0 100644 --- a/tests/phpunit/includes/parser/ParserOutputTest.php +++ b/tests/phpunit/includes/parser/ParserOutputTest.php @@ -1,5 +1,6 @@ addLink( Title::makeTitle( NS_MAIN, 'Kittens' ), 6 ); + $a->addLink( new TitleValue( NS_TALK, 'Kittens' ), 16 ); + $a->addLink( new TitleValue( NS_MAIN, 'Goats_786827346' ) ); + + $expected = [ + NS_MAIN => [ 'Kittens' => 6, 'Goats_786827346' => 0 ], + NS_TALK => [ 'Kittens' => 16 ] + ]; + $this->assertSame( $expected, $a->getLinks() ); + } + public function provideMergeTrackingMetaDataFrom() { // links ------------ $a = new ParserOutput(); $a->addLink( Title::makeTitle( NS_MAIN, 'Kittens' ), 6 ); - $a->addLink( Title::makeTitle( NS_TALK, 'Kittens' ), 16 ); - $a->addLink( Title::makeTitle( NS_MAIN, 'Goats' ), 7 ); + $a->addLink( new TitleValue( NS_TALK, 'Kittens' ), 16 ); + $a->addLink( new TitleValue( NS_MAIN, 'Goats' ), 7 ); $a->addTemplate( Title::makeTitle( NS_TEMPLATE, 'Goats' ), 107, 1107 ); $a->addLanguageLink( 'de' ); $a->addLanguageLink( 'ru' ); $a->addInterwikiLink( Title::makeTitle( NS_MAIN, 'Kittens DE', '', 'de' ) ); - $a->addInterwikiLink( Title::makeTitle( NS_MAIN, 'Kittens RU', '', 'ru' ) ); + $a->addInterwikiLink( new TitleValue( NS_MAIN, 'Kittens RU', '', 'ru' ) ); $a->addExternalLink( 'https://kittens.wikimedia.test' ); $a->addExternalLink( 'https://goats.wikimedia.test' ); @@ -655,16 +673,16 @@ EOF $b = new ParserOutput(); $b->addLink( Title::makeTitle( NS_MAIN, 'Goats' ), 7 ); $b->addLink( Title::makeTitle( NS_TALK, 'Goats' ), 17 ); - $b->addLink( Title::makeTitle( NS_MAIN, 'Dragons' ), 8 ); - $b->addLink( Title::makeTitle( NS_FILE, 'Dragons.jpg' ), 28 ); + $b->addLink( new TitleValue( NS_MAIN, 'Dragons' ), 8 ); + $b->addLink( new TitleValue( NS_FILE, 'Dragons.jpg' ), 28 ); $b->addTemplate( Title::makeTitle( NS_TEMPLATE, 'Dragons' ), 108, 1108 ); - $a->addTemplate( Title::makeTitle( NS_MAIN, 'Dragons' ), 118, 1118 ); + $a->addTemplate( new TitleValue( NS_MAIN, 'Dragons' ), 118, 1118 ); $b->addLanguageLink( 'fr' ); $b->addLanguageLink( 'ru' ); $b->addInterwikiLink( Title::makeTitle( NS_MAIN, 'Kittens FR', '', 'fr' ) ); - $b->addInterwikiLink( Title::makeTitle( NS_MAIN, 'Dragons RU', '', 'ru' ) ); + $b->addInterwikiLink( new TitleValue( NS_MAIN, 'Dragons RU', '', 'ru' ) ); $b->addExternalLink( 'https://dragons.wikimedia.test' ); $b->addExternalLink( 'https://goats.wikimedia.test' ); @@ -1060,4 +1078,27 @@ EOF $this->assertEquals( $po->getExtraCSPDefaultSrcs(), [ 'baz.com' ], 'Default' ); $this->assertEquals( $po->getExtraCSPStyleSrcs(), [ 'fred.com', 'xyzzy.com' ], 'Style' ); } + + /** + * @covers ParserOutput::addTrackingCategory + */ + public function testAddTrackingCategory() { + $po = new ParserOutput; + $po->setPageProperty( 'defaultsort', 'foobar' ); + + $page = PageReferenceValue::localReference( NS_USER, 'Testing' ); + + $po->addTrackingCategory( 'index-category', $page ); // from CORE_TRACKING_CATEGORIES + $po->addTrackingCategory( 'sitenotice', $page ); // should be "-", which is ignored + $po->addTrackingCategory( 'brackets-start', $page ); // invalid text + // TODO: assert proper handling of non-existing messages + + $expected = wfMessage( 'index-category' ) + ->page( $page ) + ->inContentLanguage() + ->text(); + + $expected = strtr( $expected, ' ', '_' ); + $this->assertSame( [ $expected => 'foobar' ], $po->getCategories() ); + } }