From bc0de57e7522e09a995dec3795534da46ade74cc Mon Sep 17 00:00:00 2001 From: David Causse Date: Thu, 1 Aug 2019 12:13:45 +0200 Subject: [PATCH] Simplify SearchUpdate constructor and hard deprecate some param types Change-Id: I5677041169402014f1afc1a9012460c760ca24b6 --- RELEASE-NOTES-1.34 | 2 + includes/Storage/DerivedPageDataUpdater.php | 3 +- includes/deferred/SearchUpdate.php | 40 +++++++++---------- maintenance/Maintenance.php | 2 +- .../includes/deferred/SearchUpdateTest.php | 3 +- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 744dffac6f..b056551022 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -428,6 +428,8 @@ because of Phabricator reports. $contextlines and $contextchars to the SearchHighlighter methods, they will use proper defaults defined in SearchHighlighter::DEFAULT_CONTEXT_LINES and DEFAULT_CONTEXT_CHARS. +* SearchUpdate constructor: passing a string as the title param and or a boolean + or a string as the content will produce a deprecation warning. === Other changes in 1.34 === * … diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php index 5d847b6a83..68814ef3aa 100644 --- a/includes/Storage/DerivedPageDataUpdater.php +++ b/includes/Storage/DerivedPageDataUpdater.php @@ -1484,7 +1484,6 @@ class DerivedPageDataUpdater implements IDBAccessObject, LoggerAwareInterface { $id = $this->getPageId(); $title = $this->getTitle(); - $dbKey = $title->getPrefixedDBkey(); $shortTitle = $title->getDBkey(); if ( !$title->exists() ) { @@ -1522,7 +1521,7 @@ class DerivedPageDataUpdater implements IDBAccessObject, LoggerAwareInterface { // TODO: make search infrastructure aware of slots! $mainSlot = $this->revision->getSlot( SlotRecord::MAIN ); if ( !$mainSlot->isInherited() && !$this->isContentDeleted() ) { - DeferredUpdates::addUpdate( new SearchUpdate( $id, $dbKey, $mainSlot->getContent() ) ); + DeferredUpdates::addUpdate( new SearchUpdate( $id, $title, $mainSlot->getContent() ) ); } // If this is another user's talk page, update newtalk. diff --git a/includes/deferred/SearchUpdate.php b/includes/deferred/SearchUpdate.php index 3625476398..f37e9a50e5 100644 --- a/includes/deferred/SearchUpdate.php +++ b/includes/deferred/SearchUpdate.php @@ -37,7 +37,7 @@ class SearchUpdate implements DeferrableUpdate { /** @var Title Title we're updating */ private $title; - /** @var Content|bool Content of the page (not text) */ + /** @var Content|null Content of the page (not text) */ private $content; /** @var WikiPage **/ @@ -45,30 +45,30 @@ class SearchUpdate implements DeferrableUpdate { /** * @param int $id Page id to update - * @param Title|string $title Title of page to update - * @param Content|string|bool $c Content of the page to update. Default: false. - * If a Content object, text will be gotten from it. String is for back-compat. - * Passing false tells the backend to just update the title, not the content + * @param Title $title Title of page to update + * @param Content|null $c Content of the page to update. */ - public function __construct( $id, $title, $c = false ) { + public function __construct( $id, $title, $c = null ) { if ( is_string( $title ) ) { - $nt = Title::newFromText( $title ); + wfDeprecated( __METHOD__ . " with a string for the title", 1.34 ); + $this->title = Title::newFromText( $title ); + if ( $this->title === null ) { + throw new InvalidArgumentException( "Cannot construct the title: $title" ); + } } else { - $nt = $title; + $this->title = $title; } - if ( $nt ) { - $this->id = $id; - // is_string() check is back-compat for ApprovedRevs - if ( is_string( $c ) ) { - $this->content = new TextContent( $c ); - } else { - $this->content = $c ?: false; - } - $this->title = $nt; - } else { - wfDebug( "SearchUpdate object created with invalid title '$title'\n" ); + $this->id = $id; + // is_string() check is back-compat for ApprovedRevs + if ( is_string( $c ) ) { + wfDeprecated( __METHOD__ . " with a string for the content", 1.34 ); + $c = new TextContent( $c ); + } elseif ( is_bool( $c ) ) { + wfDeprecated( __METHOD__ . " with a boolean for the content", 1.34 ); + $c = null; } + $this->content = $c; } /** @@ -94,7 +94,7 @@ class SearchUpdate implements DeferrableUpdate { if ( $this->getLatestPage() === null ) { $search->delete( $this->id, $normalTitle ); continue; - } elseif ( $this->content === false ) { + } elseif ( $this->content === null ) { $search->updateTitle( $this->id, $normalTitle ); continue; } diff --git a/maintenance/Maintenance.php b/maintenance/Maintenance.php index 0263425508..b74ac40d49 100644 --- a/maintenance/Maintenance.php +++ b/maintenance/Maintenance.php @@ -1529,7 +1529,7 @@ abstract class Maintenance { $title = $titleObj->getPrefixedDBkey(); $this->output( "$title..." ); # Update searchindex - $u = new SearchUpdate( $pageId, $titleObj->getText(), $rev->getContent() ); + $u = new SearchUpdate( $pageId, $titleObj, $rev->getContent() ); $u->doUpdate(); $this->output( "\n" ); } diff --git a/tests/phpunit/includes/deferred/SearchUpdateTest.php b/tests/phpunit/includes/deferred/SearchUpdateTest.php index 8faaeda0d2..43fbee8007 100644 --- a/tests/phpunit/includes/deferred/SearchUpdateTest.php +++ b/tests/phpunit/includes/deferred/SearchUpdateTest.php @@ -12,8 +12,7 @@ class SearchUpdateTest extends MediaWikiTestCase { protected function setUp() { parent::setUp(); - $this->setMwGlobals( 'wgSearchType', 'MockSearch' ); - $this->su = new SearchUpdate( 0, "" ); + $this->su = new SearchUpdate( 0, Title::newMainPage() ); } public function updateText( $text ) { -- 2.20.1