From 58852d882752b46a1c2b79d2476489dfbc978f5f Mon Sep 17 00:00:00 2001 From: Alexandre Emsenhuber Date: Sat, 9 Jun 2012 23:16:15 +0200 Subject: [PATCH] Deprecated Title::userIsWatching(); use User::isWatched() instead. * The problem is that Title::userIsWatching() relies on $wgUser, which is not suitable on every case. Instead User::isWatched() requires both an User and a Title object. * Replaced all core calls from the former to the latter * Added a cache in User for the WatchedItem instances so we do not need to do a database request every time something want to know whether a page is watched or not, which can happen several times per request. Change-Id: Ifa9c55b7ffb487ce6893c74df233eedc7654dc5e --- includes/Article.php | 12 ++++---- includes/EditPage.php | 4 +-- includes/FileDeleteForm.php | 12 ++++---- includes/ProtectionForm.php | 10 ++++--- includes/SkinLegacy.php | 2 +- includes/SkinTemplate.php | 4 +-- includes/Title.php | 1 + includes/User.php | 40 +++++++++++++++++++++++---- includes/api/ApiBase.php | 2 +- includes/specials/SpecialMovepage.php | 2 +- includes/specials/SpecialUpload.php | 2 +- skins/Vector.php | 2 +- 12 files changed, 64 insertions(+), 29 deletions(-) diff --git a/includes/Article.php b/includes/Article.php index dcde2cfba2..03a5cfca86 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -1369,10 +1369,12 @@ class Article extends Page { $this->doDelete( $reason, $suppress ); - if ( $request->getCheck( 'wpWatch' ) && $user->isLoggedIn() ) { - WatchAction::doWatch( $title, $user ); - } elseif ( $title->userIsWatching() ) { - WatchAction::doUnwatch( $title, $user ); + if ( $user->isLoggedIn() && $request->getCheck( 'wpWatch' ) != $user->isWatched( $title ) ) { + if ( $request->getCheck( 'wpWatch' ) ) { + WatchAction::doWatch( $title, $user ); + } else { + WatchAction::doUnwatch( $title, $user ); + } } return; @@ -1436,7 +1438,7 @@ class Article extends Page { } else { $suppress = ''; } - $checkWatch = $user->getBoolOption( 'watchdeletion' ) || $this->getTitle()->userIsWatching(); + $checkWatch = $user->getBoolOption( 'watchdeletion' ) || $user->isWatched( $this->getTitle() ); $form = Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getTitle()->getLocalURL( 'action=delete' ), 'id' => 'deleteconfirm' ) ) . diff --git a/includes/EditPage.php b/includes/EditPage.php index f533016284..1f526cce3e 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -756,7 +756,7 @@ class EditPage { } elseif ( $wgUser->getOption( 'watchcreations' ) && !$this->mTitle->exists() ) { # Watch creations $this->watchthis = true; - } elseif ( $this->mTitle->userIsWatching() ) { + } elseif ( $wgUser->isWatched( $this->mTitle ) ) { # Already watched $this->watchthis = true; } @@ -1470,7 +1470,7 @@ class EditPage { */ protected function commitWatch() { global $wgUser; - if ( $this->watchthis xor $this->mTitle->userIsWatching() ) { + if ( $wgUser->isLoggedIn() && $this->watchthis != $wgUser->isWatched( $this->mTitle ) ) { $dbw = wfGetDB( DB_MASTER ); $dbw->begin( __METHOD__ ); if ( $this->watchthis ) { diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php index 9d6ab6509a..18343f3642 100644 --- a/includes/FileDeleteForm.php +++ b/includes/FileDeleteForm.php @@ -119,10 +119,12 @@ class FileDeleteForm { // file, otherwise go back to the description page $wgOut->addReturnTo( $this->oldimage ? $this->title : Title::newMainPage() ); - if ( $wgRequest->getCheck( 'wpWatch' ) && $wgUser->isLoggedIn() ) { - WatchAction::doWatch( $this->title, $wgUser ); - } elseif ( $this->title->userIsWatching() ) { - WatchAction::doUnwatch( $this->title, $wgUser ); + if ( $wgUser->isLoggedIn() && $wgRequest->getCheck( 'wpWatch' ) != $wgUser->isWatched( $this->title ) ) { + if ( $wgRequest->getCheck( 'wpWatch' ) ) { + WatchAction::doWatch( $this->title, $wgUser ); + } else { + WatchAction::doUnwatch( $this->title, $wgUser ); + } } } return; @@ -210,7 +212,7 @@ class FileDeleteForm { $suppress = ''; } - $checkWatch = $wgUser->getBoolOption( 'watchdeletion' ) || $this->title->userIsWatching(); + $checkWatch = $wgUser->getBoolOption( 'watchdeletion' ) || $wgUser->isWatched( $this->title ); $form = Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getAction(), 'id' => 'mw-img-deleteconfirm' ) ) . Xml::openElement( 'fieldset' ) . diff --git a/includes/ProtectionForm.php b/includes/ProtectionForm.php index dbe06d49c6..31ed87f6be 100644 --- a/includes/ProtectionForm.php +++ b/includes/ProtectionForm.php @@ -318,10 +318,12 @@ class ProtectionForm { return false; } - if ( $wgRequest->getCheck( 'mwProtectWatch' ) && $wgUser->isLoggedIn() ) { - WatchAction::doWatch( $this->mTitle, $wgUser ); - } elseif ( $this->mTitle->userIsWatching() ) { - WatchAction::doUnwatch( $this->mTitle, $wgUser ); + if ( $wgUser->isLoggedIn() && $wgRequest->getCheck( 'mwProtectWatch' ) != $wgUser->isWatched( $this->mTitle ) ) { + if ( $wgRequest->getCheck( 'mwProtectWatch' ) ) { + WatchAction::doWatch( $this->mTitle, $wgUser ); + } else { + WatchAction::doUnwatch( $this->mTitle, $wgUser ); + } } return true; } diff --git a/includes/SkinLegacy.php b/includes/SkinLegacy.php index e1ec897d85..18bc29f3e2 100644 --- a/includes/SkinLegacy.php +++ b/includes/SkinLegacy.php @@ -638,7 +638,7 @@ class LegacyTemplate extends BaseTemplate { $title = $this->getSkin()->getTitle(); if ( $wgOut->isArticleRelated() ) { - if ( $title->userIsWatching() ) { + if ( $wgUser->isWatched( $title ) ) { $text = wfMsg( 'unwatchthispage' ); $query = array( 'action' => 'unwatch', diff --git a/includes/SkinTemplate.php b/includes/SkinTemplate.php index b3dfc9bd49..4028e78b70 100644 --- a/includes/SkinTemplate.php +++ b/includes/SkinTemplate.php @@ -258,7 +258,7 @@ class SkinTemplate extends Skin { /* XXX currently unused, might get useful later $tpl->set( 'editable', ( !$title->isSpecialPage() ) ); $tpl->set( 'exists', $title->getArticleID() != 0 ); - $tpl->set( 'watch', $title->userIsWatching() ? 'unwatch' : 'watch' ); + $tpl->set( 'watch', $user->isWatched( $title ) ? 'unwatch' : 'watch' ); $tpl->set( 'protect', count( $title->isProtected() ) ? 'unprotect' : 'protect' ); $tpl->set( 'helppage', $this->msg( 'helppage' )->text() ); */ @@ -974,7 +974,7 @@ class SkinTemplate extends Skin { * a change to that procedure these messages will have to remain as * the global versions. */ - $mode = $title->userIsWatching() ? 'unwatch' : 'watch'; + $mode = $user->isWatched( $title ) ? 'unwatch' : 'watch'; $token = WatchAction::getWatchToken( $title, $user, $mode ); $content_navigation['actions'][$mode] = array( 'class' => $onPage && ( $action == 'watch' || $action == 'unwatch' ) ? 'selected' : false, diff --git a/includes/Title.php b/includes/Title.php index 439d5afb5f..40a4e6a979 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1516,6 +1516,7 @@ class Title { /** * Is $wgUser watching this page? * + * @deprecated in 1.20; use User::isWatched() instead. * @return Bool */ public function userIsWatching() { diff --git a/includes/User.php b/includes/User.php index 01407b1d8e..5fc0773d6e 100644 --- a/includes/User.php +++ b/includes/User.php @@ -65,6 +65,11 @@ class User { const MW_USER_VERSION = MW_USER_VERSION; const EDIT_TOKEN_SUFFIX = EDIT_TOKEN_SUFFIX; + /** + * Maximum items in $mWatchedItems + */ + const MAX_WATCHED_ITEMS_CACHE = 100; + /** * Array of Strings List of member variables which are saved to the * shared cache (memcached). Any operation which changes the @@ -219,6 +224,11 @@ class User { */ private $mBlockedFromCreateAccount = false; + /** + * @var Array + */ + private $mWatchedItems = array(); + static $idCacheByName = array(); /** @@ -2613,14 +2623,34 @@ class User { return RequestContext::getMain()->getSkin(); } + /** + * Get a WatchedItem for this user and $title. + * + * @param $title Title + * @return WatchedItem + */ + public function getWatchedItem( $title ) { + $key = $title->getNamespace() . ':' . $title->getDBkey(); + + if ( isset( $this->mWatchedItems[$key] ) ) { + return $this->mWatchedItems[$key]; + } + + if ( count( $this->mWatchedItems ) >= self::MAX_WATCHED_ITEMS_CACHE ) { + $this->mWatchedItems = array(); + } + + $this->mWatchedItems[$key] = WatchedItem::fromUserTitle( $this, $title ); + return $this->mWatchedItems[$key]; + } + /** * Check the watched status of an article. * @param $title Title of the article to look at * @return Bool */ public function isWatched( $title ) { - $wl = WatchedItem::fromUserTitle( $this, $title ); - return $wl->isWatched(); + return $this->getWatchedItem( $title )->isWatched(); } /** @@ -2628,8 +2658,7 @@ class User { * @param $title Title of the article to look at */ public function addWatch( $title ) { - $wl = WatchedItem::fromUserTitle( $this, $title ); - $wl->addWatch(); + $this->getWatchedItem( $title )->addWatch(); $this->invalidateCache(); } @@ -2638,8 +2667,7 @@ class User { * @param $title Title of the article to look at */ public function removeWatch( $title ) { - $wl = WatchedItem::fromUserTitle( $this, $title ); - $wl->removeWatch(); + $this->getWatchedItem( $title )->removeWatch(); $this->invalidateCache(); } diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 6a9be24f08..883a3c085d 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -825,7 +825,7 @@ abstract class ApiBase extends ContextSource { */ protected function getWatchlistValue ( $watchlist, $titleObj, $userOption = null ) { - $userWatching = $titleObj->userIsWatching(); + $userWatching = $this->getUser()->isWatched( $titleObj ); switch ( $watchlist ) { case 'watch': diff --git a/includes/specials/SpecialMovepage.php b/includes/specials/SpecialMovepage.php index a512ee6387..ea73763597 100644 --- a/includes/specials/SpecialMovepage.php +++ b/includes/specials/SpecialMovepage.php @@ -358,7 +358,7 @@ class MovePageForm extends UnlistedSpecialPage { } $watchChecked = $user->isLoggedIn() && ($this->watch || $user->getBoolOption( 'watchmoves' ) - || $this->oldTitle->userIsWatching()); + || $user->isWatched( $this->oldTitle ) ); # Don't allow watching if user is not logged in if( $user->isLoggedIn() ) { $out->addHTML( " diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php index 91631f8aed..3072408d3c 100644 --- a/includes/specials/SpecialUpload.php +++ b/includes/specials/SpecialUpload.php @@ -516,7 +516,7 @@ class SpecialUpload extends SpecialPage { if( $local && $local->exists() ) { // We're uploading a new version of an existing file. // No creation, so don't watch it if we're not already. - return $local->getTitle()->userIsWatching(); + return $this->getUser()->isWatched( $local->getTitle() ); } else { // New page should get watched if that's our option. return $this->getUser()->getOption( 'watchcreations' ); diff --git a/skins/Vector.php b/skins/Vector.php index 6ac3f08744..be07ba8c99 100644 --- a/skins/Vector.php +++ b/skins/Vector.php @@ -103,7 +103,7 @@ class VectorTemplate extends BaseTemplate { $nav = $this->data['content_navigation']; if ( $wgVectorUseIconWatch ) { - $mode = $this->getSkin()->getRelevantTitle()->userIsWatching() ? 'unwatch' : 'watch'; + $mode = $this->getSkin()->getUser()->isWatched( $this->getSkin()->getRelevantTitle() ) ? 'unwatch' : 'watch'; if ( isset( $nav['actions'][$mode] ) ) { $nav['views'][$mode] = $nav['actions'][$mode]; $nav['views'][$mode]['class'] = rtrim( 'icon ' . $nav['views'][$mode]['class'], ' ' ); -- 2.20.1