From cf004d524d1534e23443ded8d35a4b299fccaa46 Mon Sep 17 00:00:00 2001 From: Ammar Abdulhamid Date: Fri, 4 Sep 2020 13:31:40 +0100 Subject: [PATCH] Remove requirement for ApiWatchlistTrait to be in ApiBase. This trait is not needed in ApiBase and its presence here is proving to be problematic. See I795db12. In this patch, the trait usage (more precisely the 'use statement') has been removed from ApiBase and accordingly the signatures of ApiWatchlistTrait::getWatchlistValue() and ::setWatch() have been altered to now require User object. With these changes, the abstract getUser() method in the trait is no longer needed, so it has been removed also. All core usages of the affected functions are fixed in this patch. The trait is used in only one extension according to codesearch tool, the extension will be fixed in Ic22e163. Bug: T262175 Bug: T248512 Follow-up: Ia18627b9824dca81f44f0571e8420d89b7626cf6 Change-Id: Idabcea71edfca9e7ed42000a258c99ff407873d4 --- includes/api/ApiBase.php | 1 - includes/api/ApiDelete.php | 2 +- includes/api/ApiEditPage.php | 2 +- includes/api/ApiMove.php | 4 ++-- includes/api/ApiProtect.php | 2 +- includes/api/ApiRollback.php | 2 +- includes/api/ApiUndelete.php | 2 +- includes/api/ApiUpload.php | 10 ++++++---- includes/api/ApiWatchlistTrait.php | 31 +++++++++++++----------------- 9 files changed, 26 insertions(+), 30 deletions(-) diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index a2df1cc7b2a..63512f0c628 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -52,7 +52,6 @@ use Wikimedia\Rdbms\IDatabase; abstract class ApiBase extends ContextSource { use ApiBlockInfoTrait; - use ApiWatchlistTrait; /** @var HookContainer */ private $hookContainer; diff --git a/includes/api/ApiDelete.php b/includes/api/ApiDelete.php index f42396495ee..276529d0469 100644 --- a/includes/api/ApiDelete.php +++ b/includes/api/ApiDelete.php @@ -103,7 +103,7 @@ class ApiDelete extends ApiBase { } $watchlistExpiry = $this->getExpiryFromParams( $params ); - $this->setWatch( $watch, $titleObj, 'watchdeletion', $watchlistExpiry ); + $this->setWatch( $watch, $titleObj, $user, 'watchdeletion', $watchlistExpiry ); $r = [ 'title' => $titleObj->getPrefixedText(), diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php index 5cdc49a333f..1f970bdd330 100644 --- a/includes/api/ApiEditPage.php +++ b/includes/api/ApiEditPage.php @@ -368,7 +368,7 @@ class ApiEditPage extends ApiBase { $requestArray['wpSection'] = ''; } - $watch = $this->getWatchlistValue( $params['watchlist'], $titleObj ); + $watch = $this->getWatchlistValue( $params['watchlist'], $titleObj, $user ); $watchlistExpiry = $params['watchlistexpiry'] ?? null; // Deprecated parameters diff --git a/includes/api/ApiMove.php b/includes/api/ApiMove.php index cd94e149024..9afcbf6ab54 100644 --- a/includes/api/ApiMove.php +++ b/includes/api/ApiMove.php @@ -169,8 +169,8 @@ class ApiMove extends ApiBase { $watchlistExpiry = $this->getExpiryFromParams( $params ); // Watch pages - $this->setWatch( $watch, $fromTitle, 'watchmoves', $watchlistExpiry ); - $this->setWatch( $watch, $toTitle, 'watchmoves', $watchlistExpiry ); + $this->setWatch( $watch, $fromTitle, $user, 'watchmoves', $watchlistExpiry ); + $this->setWatch( $watch, $toTitle, $user, 'watchmoves', $watchlistExpiry ); $result->addValue( null, $this->getModuleName(), $r ); } diff --git a/includes/api/ApiProtect.php b/includes/api/ApiProtect.php index 935f0ab92bc..16f7a55a568 100644 --- a/includes/api/ApiProtect.php +++ b/includes/api/ApiProtect.php @@ -113,7 +113,7 @@ class ApiProtect extends ApiBase { $watch = $params['watch'] ? 'watch' : $params['watchlist']; $watchlistExpiry = $this->getExpiryFromParams( $params ); - $this->setWatch( $watch, $titleObj, 'watchdefault', $watchlistExpiry ); + $this->setWatch( $watch, $titleObj, $user, 'watchdefault', $watchlistExpiry ); $status = $pageObj->doUpdateRestrictions( $protections, diff --git a/includes/api/ApiRollback.php b/includes/api/ApiRollback.php index cf59660cb2f..d9f4e0cdbcd 100644 --- a/includes/api/ApiRollback.php +++ b/includes/api/ApiRollback.php @@ -94,7 +94,7 @@ class ApiRollback extends ApiBase { $watchlistExpiry = $this->getExpiryFromParams( $params ); // Watch pages - $this->setWatch( $watch, $titleObj, 'watchrollback', $watchlistExpiry ); + $this->setWatch( $watch, $titleObj, $user, 'watchrollback', $watchlistExpiry ); $currentRevisionRecord = $details['current-revision-record']; $targetRevisionRecord = $details['target-revision-record']; diff --git a/includes/api/ApiUndelete.php b/includes/api/ApiUndelete.php index 019a19a55a3..49b2d76c273 100644 --- a/includes/api/ApiUndelete.php +++ b/includes/api/ApiUndelete.php @@ -93,7 +93,7 @@ class ApiUndelete extends ApiBase { } $watchlistExpiry = $this->getExpiryFromParams( $params ); - $this->setWatch( $params['watchlist'], $titleObj, null, $watchlistExpiry ); + $this->setWatch( $params['watchlist'], $titleObj, $user, null, $watchlistExpiry ); $info = [ 'title' => $titleObj->getPrefixedText(), diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 21dc8f84e2b..fc971781a16 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -819,20 +819,22 @@ class ApiUpload extends ApiBase { /** @var LocalFile $file */ $file = $this->mUpload->getLocalFile(); + $user = $this->getUser(); + $title = $file->getTitle(); - // For preferences mode, we want to watch if 'watchdefault' is set, + // for preferences mode, we want to watch if 'watchdefault' is set, // or if the *file* doesn't exist, and either 'watchuploads' or // 'watchcreations' is set. But getWatchlistValue()'s automatic // handling checks if the *title* exists or not, so we need to check // all three preferences manually. $watch = $this->getWatchlistValue( - $this->mParams['watchlist'], $file->getTitle(), 'watchdefault' + $this->mParams['watchlist'], $title, $user, 'watchdefault' ); if ( !$watch && $this->mParams['watchlist'] == 'preferences' && !$file->exists() ) { $watch = ( - $this->getWatchlistValue( 'preferences', $file->getTitle(), 'watchuploads' ) || - $this->getWatchlistValue( 'preferences', $file->getTitle(), 'watchcreations' ) + $this->getWatchlistValue( 'preferences', $title, $user, 'watchuploads' ) || + $this->getWatchlistValue( 'preferences', $title, $user, 'watchcreations' ) ); } $watchlistExpiry = $this->getExpiryFromParams( $this->mParams ); diff --git a/includes/api/ApiWatchlistTrait.php b/includes/api/ApiWatchlistTrait.php index e20940fa8bb..31b3f185ab6 100644 --- a/includes/api/ApiWatchlistTrait.php +++ b/includes/api/ApiWatchlistTrait.php @@ -63,36 +63,39 @@ trait ApiWatchlistTrait { /** * Set a watch (or unwatch) based the based on a watchlist parameter. * @param string $watch Valid values: 'watch', 'unwatch', 'preferences', 'nochange' - * @param Title $titleObj The article's title to change + * @param Title $title The article's title to change + * @param User $user The user to set watch/unwatch for * @param string|null $userOption The user option to consider when $watch=preferences * @param string|null $expiry Optional expiry timestamp in any format acceptable to wfTimestamp(), * null will not create expiries, or leave them unchanged should they already exist. */ protected function setWatch( string $watch, - Title $titleObj, + Title $title, + User $user, ?string $userOption = null, ?string $expiry = null ): void { - $value = $this->getWatchlistValue( $watch, $titleObj, $userOption ); - WatchAction::doWatchOrUnwatch( $value, $titleObj, $this->getUser(), $expiry ); + $value = $this->getWatchlistValue( $watch, $title, $user, $userOption ); + WatchAction::doWatchOrUnwatch( $value, $title, $user, $expiry ); } /** * Return true if we're to watch the page, false if not. * @param string $watchlist Valid values: 'watch', 'unwatch', 'preferences', 'nochange' - * @param Title $titleObj The page under consideration + * @param Title $title The page under consideration + * @param User $user The user get the the value for. * @param string|null $userOption The user option to consider when $watchlist=preferences. - * If not set will use watchdefault always and watchcreations if $titleObj doesn't exist. + * If not set will use watchdefault always and watchcreations if $title doesn't exist. * @return bool */ protected function getWatchlistValue( string $watchlist, - Title $titleObj, + Title $title, + User $user, ?string $userOption = null ): bool { - $user = $this->getUser(); - $userWatching = $user->isWatched( $titleObj, User::IGNORE_USER_RIGHTS ); + $userWatching = $user->isWatched( $title, User::IGNORE_USER_RIGHTS ); switch ( $watchlist ) { case 'watch': @@ -110,7 +113,7 @@ trait ApiWatchlistTrait { if ( $userOption === null ) { return $user->getBoolOption( 'watchdefault' ) || $user->getBoolOption( 'watchcreations' ) && - !$titleObj->exists(); + !$title->exists(); } // Watch the article based on the user preference @@ -137,12 +140,4 @@ trait ApiWatchlistTrait { return $watchlistExpiry; } - - /** - * Implemented by ApiBase. We do this because otherwise Phan would complain - * about calls to $this->getUser() in this trait, since it doesn't infer that - * classes using the trait are also extending ApiBase. - * @return User - */ - abstract protected function getUser(); }