From ceda8ac60e3b7d29eae000f28d12a08c6a9a2bf5 Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Thu, 29 Jul 2021 17:19:22 +0300 Subject: [PATCH] Remove Title from signatures in CategoryViewer Co-authored-by: Daniel Kinzler Change-Id: Ie880482fe58b96809fdfd08ebf0825bdcf1d19d1 --- RELEASE-NOTES-1.37 | 1 + includes/Category.php | 77 +++++++------ includes/CategoryViewer.php | 101 ++++++++++++------ includes/page/CategoryPage.php | 2 +- .../ParserOutputSearchDataExtractor.php | 3 +- tests/phpunit/includes/CategoryTest.php | 2 + 6 files changed, 115 insertions(+), 71 deletions(-) diff --git a/RELEASE-NOTES-1.37 b/RELEASE-NOTES-1.37 index f1eae705811..2f82d84937e 100644 --- a/RELEASE-NOTES-1.37 +++ b/RELEASE-NOTES-1.37 @@ -618,6 +618,7 @@ because of Phabricator reports. use getPerformerIdentity() instead. * ContentHandler::cleanupHandlersCache(), deprecated since 1.35, now emits deprecation warnings. +* Category::getTitle() was deprecated in favor of Category::getPage() * File::getUser method was hard deprecated, along with overrides in LocalFile and ForeignApiFile in favor of ::getUploader. * SpecialBlock::checkUnblockSelf(), deprecated in 1.36, now emits deprecation diff --git a/includes/Category.php b/includes/Category.php index eb486fb87d5..1ce87d640b4 100644 --- a/includes/Category.php +++ b/includes/Category.php @@ -22,6 +22,7 @@ */ use MediaWiki\MediaWikiServices; +use MediaWiki\Page\PageIdentity; use Wikimedia\Rdbms\ILoadBalancer; /** @@ -35,9 +36,9 @@ class Category { private $mID = null; /** * Category page title - * @var Title + * @var PageIdentity */ - private $mTitle = null; + private $mPage = null; /** Counts of membership (cat_pages, cat_subcats, cat_files) */ private $mPages = null, $mSubcats = null, $mFiles = null; @@ -86,17 +87,17 @@ class Category { if ( !$row ) { # Okay, there were no contents. Nothing to initialize. - if ( $this->mTitle ) { - # If there is a title object but no record in the category table, + if ( $this->mPage ) { + # If there is a page object but no record in the category table, # treat this as an empty category. $this->mID = false; - $this->mName = $this->mTitle->getDBkey(); + $this->mName = $this->mPage->getDBkey(); $this->mPages = 0; $this->mSubcats = 0; $this->mFiles = 0; - # If the title exists, call refreshCounts to add a row for it. - if ( $mode === self::LAZY_INIT_ROW && $this->mTitle->exists() ) { + # If the page exists, call refreshCounts to add a row for it. + if ( $mode === self::LAZY_INIT_ROW && $this->mPage->exists() ) { DeferredUpdates::addCallableUpdate( [ $this, 'refreshCounts' ] ); } @@ -143,7 +144,7 @@ class Category { return false; } - $cat->mTitle = $title; + $cat->mPage = $title; $cat->mName = $title->getDBkey(); return $cat; @@ -152,14 +153,14 @@ class Category { /** * Factory function. * - * @param Title $title Title for the category page - * @return Category|bool On a totally invalid name + * @param PageIdentity $page Category page. Warning, no validation is performed! + * @return Category */ - public static function newFromTitle( $title ) { + public static function newFromTitle( PageIdentity $page ): self { $cat = new self(); - $cat->mTitle = $title; - $cat->mName = $title->getDBkey(); + $cat->mPage = $page; + $cat->mName = $page->getDBkey(); return $cat; } @@ -167,7 +168,7 @@ class Category { /** * Factory function. * - * @param int $id A category id + * @param int $id A category id. Warning, no validation is performed! * @return Category */ public static function newFromID( $id ) { @@ -179,18 +180,16 @@ class Category { /** * Factory function, for constructing a Category object from a result set * - * @param stdClass $row Result set row, must contain the cat_xxx fields. If the - * fields are null, the resulting Category object will represent an empty - * category if a title object was given. If the fields are null and no - * title was given, this method fails and returns false. - * @param Title|null $title Optional title object for the category represented by - * the given row. May be provided if it is already known, to avoid having - * to re-create a title object later. + * @param stdClass $row Result set row, must contain the cat_xxx fields. If the fields are + * null, the resulting Category object will represent an empty category if a page object was + * given. If the fields are null and no PageIdentity was given, this method fails and returns + * false. + * @param PageIdentity|null $page This must be provided if there is no cat_title field in $row. * @return Category|false */ - public static function newFromRow( $row, $title = null ) { + public static function newFromRow( stdClass $row, ?PageIdentity $page = null ) { $cat = new self(); - $cat->mTitle = $title; + $cat->mPage = $page; # NOTE: the row often results from a LEFT JOIN on categorylinks. This may result in # all the cat_xxx fields being null, if the category page exists, but nothing @@ -198,13 +197,13 @@ class Category { # category, if possible. if ( $row->cat_title === null ) { - if ( $title === null ) { + if ( $page === null ) { # the name is probably somewhere in the row, for example as page_title, # but we can't know that here... return false; } else { - # if we have a title object, fetch the category name from there - $cat->mName = $title->getDBkey(); + # if we have a PageIdentity object, fetch the category name from there + $cat->mName = $page->getDBkey(); } $cat->mID = false; @@ -258,19 +257,29 @@ class Category { } /** - * @return Title|bool Title for this category, or false on failure. + * @since 1.37 + * @return ?PageIdentity the page associated with this category, or null on failure. NOTE: This + * returns null on failure, unlike getTitle() which returns false. */ - public function getTitle() { - if ( $this->mTitle ) { - return $this->mTitle; + public function getPage(): ?PageIdentity { + if ( $this->mPage ) { + return $this->mPage; } if ( !$this->initialize( self::LAZY_INIT_ROW ) ) { - return false; + return null; } - $this->mTitle = Title::makeTitleSafe( NS_CATEGORY, $this->mName ); - return $this->mTitle; + $this->mPage = Title::makeTitleSafe( NS_CATEGORY, $this->mName ); + return $this->mPage; + } + + /** + * @deprecated since 1.37, use getPage() instead. + * @return Title|bool Title for this category, or false on failure. + */ + public function getTitle() { + return Title::castFromPageIdentity( $this->getPage() ) ?? false; } /** @@ -374,7 +383,7 @@ class Category { __METHOD__ ); - $shouldExist = $result->pages > 0 || $this->getTitle()->exists(); + $shouldExist = $result->pages > 0 || $this->getPage()->exists(); if ( $this->mID ) { if ( $shouldExist ) { diff --git a/includes/CategoryViewer.php b/includes/CategoryViewer.php index feab6f0b0e3..d1d5b23be11 100644 --- a/includes/CategoryViewer.php +++ b/includes/CategoryViewer.php @@ -21,10 +21,14 @@ */ use MediaWiki\HookContainer\ProtectedHookAccessorTrait; +use MediaWiki\Linker\LinkTarget; use MediaWiki\MediaWikiServices; +use MediaWiki\Page\PageIdentity; +use MediaWiki\Page\PageReference; class CategoryViewer extends ContextSource { use ProtectedHookAccessorTrait; + use DeprecationHelper; /** @var int */ public $limit; @@ -65,8 +69,8 @@ class CategoryViewer extends ContextSource { /** @var array */ public $flip; - /** @var Title */ - public $title; + /** @var PageIdentity */ + protected $page; /** @var Collation */ public $collation; @@ -85,17 +89,29 @@ class CategoryViewer extends ContextSource { /** * @since 1.19 $context is a second, required parameter - * @param Title $title + * @param PageIdentity $page * @param IContextSource $context * @param array $from An array with keys page, subcat, * and file for offset of results of each section (since 1.17) * @param array $until An array with 3 keys for until of each section (since 1.17) * @param array $query */ - public function __construct( $title, IContextSource $context, $from = [], - $until = [], $query = [] + public function __construct( PageIdentity $page, IContextSource $context, array $from = [], + array $until = [], array $query = [] ) { - $this->title = $title; + $this->page = $page; + + $this->deprecatePublicPropertyFallback( + 'title', + '1.37', + function (): Title { + return Title::castFromPageIdentity( $this->page ); + }, + function ( PageIdentity $page ) { + $this->page = $page; + } + ); + $this->setContext( $context ); $this->getOutput()->addModuleStyles( [ 'mediawiki.action.view.categoryPage.styles' @@ -103,7 +119,7 @@ class CategoryViewer extends ContextSource { $this->from = $from; $this->until = $until; $this->limit = $context->getConfig()->get( 'CategoryPagingLimit' ); - $this->cat = Category::newFromTitle( $title ); + $this->cat = Category::newFromTitle( $page ); $this->query = $query; $this->collation = MediaWikiServices::getInstance()->getCollationFactory()->getCategoryCollation(); $this->languageConverter = MediaWikiServices::getInstance() @@ -188,22 +204,23 @@ class CategoryViewer extends ContextSource { */ public function addSubcategoryObject( Category $cat, $sortkey, $pageLength ) { // Subcategory; strip the 'Category' namespace from the link text. - $title = $cat->getTitle(); + $pageRecord = MediaWikiServices::getInstance()->getPageStore() + ->getPageByReference( $cat->getPage() ); $this->children[] = $this->generateLink( 'subcat', - $title, - $title->isRedirect(), - htmlspecialchars( $title->getText() ) + $pageRecord, + $pageRecord->isRedirect(), + htmlspecialchars( str_replace( '_', ' ', $pageRecord->getDBkey() ) ) ); $this->children_start_char[] = - $this->getSubcategorySortChar( $cat->getTitle(), $sortkey ); + $this->getSubcategorySortChar( $cat->getPage(), $sortkey ); } /** * @param string $type - * @param Title $title + * @param PageReference $page * @param bool $isRedirect * @param string|null $html * @return string @@ -212,15 +229,19 @@ class CategoryViewer extends ContextSource { * @param-taint $html tainted * @return-taint escaped */ - private function generateLink( $type, Title $title, $isRedirect, $html = null ) { + private function generateLink( + string $type, PageReference $page, bool $isRedirect, ?string $html = null + ): string { $link = null; - $this->getHookRunner()->onCategoryViewer__generateLink( $type, $title, $html, $link ); + $legacyTitle = MediaWikiServices::getInstance()->getTitleFactory() + ->castFromPageReference( $page ); + $this->getHookRunner()->onCategoryViewer__generateLink( $type, $legacyTitle, $html, $link ); if ( $link === null ) { $linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer(); if ( $html !== null ) { $html = new HtmlArmor( $html ); } - $link = $linkRenderer->makeLink( $title, $html ); + $link = $linkRenderer->makeLink( $page, $html ); } if ( $isRedirect ) { $link = Html::rawElement( @@ -240,13 +261,15 @@ class CategoryViewer extends ContextSource { * Workaround: If sortkey == "Category:".$title, than use $title for sorting, * else use sortkey... * - * @param Title $title + * @param PageIdentity $page * @param string $sortkey The human-readable sortkey (before transforming to icu or whatever). * @return string */ - public function getSubcategorySortChar( $title, $sortkey ) { - if ( $title->getPrefixedText() == $sortkey ) { - $word = $title->getDBkey(); + public function getSubcategorySortChar( PageIdentity $page, string $sortkey ): string { + $titleText = MediaWikiServices::getInstance()->getTitleFormatter() + ->getPrefixedText( $page ); + if ( $titleText === $sortkey ) { + $word = $page->getDBkey(); } else { $word = $sortkey; } @@ -258,12 +281,16 @@ class CategoryViewer extends ContextSource { /** * Add a page in the image namespace - * @param Title $title + * @param PageReference $page * @param string $sortkey * @param int $pageLength * @param bool $isRedirect */ - public function addImage( Title $title, $sortkey, $pageLength, $isRedirect = false ) { + public function addImage( + PageReference $page, string $sortkey, int $pageLength, bool $isRedirect = false + ): void { + $title = MediaWikiServices::getInstance()->getTitleFactory() + ->castFromPageReference( $page ); if ( $this->showGallery ) { $flip = $this->flip['file']; if ( $flip ) { @@ -272,7 +299,7 @@ class CategoryViewer extends ContextSource { $this->gallery->add( $title ); } } else { - $this->imgsNoGallery[] = $this->generateLink( 'image', $title, $isRedirect ); + $this->imgsNoGallery[] = $this->generateLink( 'image', $page, $isRedirect ); $this->imgsNoGallery_start_char[] = $this->languageConverter->convert( $this->collation->getFirstLetter( $sortkey ) ); @@ -281,13 +308,15 @@ class CategoryViewer extends ContextSource { /** * Add a miscellaneous page - * @param Title $title + * @param PageReference $page * @param string $sortkey * @param int $pageLength * @param bool $isRedirect */ - public function addPage( $title, $sortkey, $pageLength, $isRedirect = false ) { - $this->articles[] = $this->generateLink( 'page', $title, $isRedirect ); + public function addPage( + PageReference $page, string $sortkey, int $pageLength, bool $isRedirect = false + ): void { + $this->articles[] = $this->generateLink( 'page', $page, $isRedirect ); $this->articles_start_char[] = $this->languageConverter->convert( $this->collation->getFirstLetter( $sortkey ) ); @@ -355,7 +384,7 @@ class CategoryViewer extends ContextSource { 'cl_collation' ] ), - array_merge( [ 'cl_to' => $this->title->getDBkey() ], $extraConds ), + array_merge( [ 'cl_to' => $this->page->getDBkey() ], $extraConds ), __METHOD__, [ 'USE INDEX' => [ 'categorylinks' => 'cl_sortkey' ], @@ -562,7 +591,9 @@ class CategoryViewer extends ContextSource { $list = self::shortList( $articles, $articles_start_char ); } - $pageLang = $this->title->getPageLanguage(); + $pageLang = MediaWikiServices::getInstance()->getTitleFactory() + ->castFromPageIdentity( $this->page ) + ->getPageLanguage(); $attribs = [ 'lang' => $pageLang->getHtmlCode(), 'dir' => $pageLang->getDir(), 'class' => 'mw-content-' . $pageLang->getDir() ]; $list = Html::rawElement( 'div', $attribs, $list ); @@ -665,7 +696,7 @@ class CategoryViewer extends ContextSource { $prevQuery["{$type}until"] = $first; unset( $prevQuery["{$type}from"] ); $prevLink = $linkRenderer->makeKnownLink( - $this->addFragmentToTitle( $this->title, $type ), + $this->addFragmentToTitle( $this->page, $type ), new HtmlArmor( $prevLink ), [], $prevQuery @@ -679,7 +710,7 @@ class CategoryViewer extends ContextSource { $lastQuery["{$type}from"] = $last; unset( $lastQuery["{$type}until"] ); $nextLink = $linkRenderer->makeKnownLink( - $this->addFragmentToTitle( $this->title, $type ), + $this->addFragmentToTitle( $this->page, $type ), new HtmlArmor( $nextLink ), [], $lastQuery @@ -693,12 +724,12 @@ class CategoryViewer extends ContextSource { * Takes a title, and adds the fragment identifier that * corresponds to the correct segment of the category. * - * @param Title $title The title (usually $this->title) + * @param PageReference $page The title (usually $this->title) * @param string $section Which section * @throws MWException - * @return Title + * @return LinkTarget */ - private function addFragmentToTitle( $title, $section ) { + private function addFragmentToTitle( PageReference $page, string $section ): LinkTarget { switch ( $section ) { case 'page': $fragment = 'mw-pages'; @@ -714,8 +745,8 @@ class CategoryViewer extends ContextSource { " Invalid section $section." ); } - return Title::makeTitle( $title->getNamespace(), - $title->getDBkey(), $fragment ); + return new TitleValue( $page->getNamespace(), + $page->getDBkey(), $fragment ); } /** diff --git a/includes/page/CategoryPage.php b/includes/page/CategoryPage.php index fcfaf4b88cb..9fc884c8c64 100644 --- a/includes/page/CategoryPage.php +++ b/includes/page/CategoryPage.php @@ -104,7 +104,7 @@ class CategoryPage extends Article { unset( $reqArray["to"] ); $viewer = new $this->mCategoryViewerClass( - $this->getContext()->getTitle(), + $this->getPage(), $this->getContext(), $from, $until, diff --git a/includes/search/ParserOutputSearchDataExtractor.php b/includes/search/ParserOutputSearchDataExtractor.php index 4b60a0c5337..89678f00d61 100644 --- a/includes/search/ParserOutputSearchDataExtractor.php +++ b/includes/search/ParserOutputSearchDataExtractor.php @@ -38,7 +38,8 @@ class ParserOutputSearchDataExtractor { $categories = []; foreach ( $parserOutput->getCategoryLinks() as $key ) { - $categories[] = Category::newFromName( $key )->getTitle()->getText(); + $name = Category::newFromName( $key )->getName(); + $categories[] = str_replace( '_', ' ', $name ); } return $categories; diff --git a/tests/phpunit/includes/CategoryTest.php b/tests/phpunit/includes/CategoryTest.php index e1aa563bd1b..37655eda539 100644 --- a/tests/phpunit/includes/CategoryTest.php +++ b/tests/phpunit/includes/CategoryTest.php @@ -101,6 +101,8 @@ class CategoryTest extends MediaWikiIntegrationTestCase { $title = Title::newFromText( 'Category:Example' ); $category = Category::newFromTitle( $title ); $this->assertSame( 'Example', $category->getName() ); + $this->assertTrue( $title->isSamePageAs( $category->getPage() ) ); + $this->assertTrue( $title->isSamePageAs( $category->getTitle() ) ); } /**