From: Aryeh Gregor Date: Sun, 28 Apr 2019 11:07:18 +0000 (+0300) Subject: Convert WatchedItem and friends to UserIdentity X-Git-Tag: 1.34.0-rc.0~1813 X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=f7201e3b9b8117582bdd29c52b0a8c637650fbc9;p=lhc%2Fweb%2Fwiklou.git Convert WatchedItem and friends to UserIdentity I wasn't able to port some places that rely on isAllowed, getOption, or related methods. This adds isRegistered() to UserIdentity, which works like User::isLoggedIn() but with a better name. I also cleaned up User mocks in WatchedItemQueryServiceUnitTest in the course of debugging test failures when switching them to UserIdentityValue instead of mock Users where possible. They now specify explicitly which methods are allowed to be called on their User objects, which I believe is good practice for mocks (and unfortunately PHPUnit makes it awkward). Bug: T207972 Depends-On: I883d506197a011fe4c102b72df4d9deb58ab5ca2 Change-Id: Iadbf7bc31a496899dbef44e49065ff89f37aea89 --- diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 5d46edd8c2..25888ccbd5 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -126,6 +126,7 @@ because of Phabricator reports. instead. * The Config argument to ChangesListSpecialPage::checkStructuredFilterUiEnabled is deprecated. Pass only the User argument. +* WatchedItem::getUser is deprecated. Use getUserIdentity. === Other changes in 1.34 === * … diff --git a/includes/jobqueue/jobs/ClearUserWatchlistJob.php b/includes/jobqueue/jobs/ClearUserWatchlistJob.php index 01fa46c002..0cb1a52d69 100644 --- a/includes/jobqueue/jobs/ClearUserWatchlistJob.php +++ b/includes/jobqueue/jobs/ClearUserWatchlistJob.php @@ -1,6 +1,7 @@ $user->getId(), 'maxWatchlistId' => $maxWatchlistId ] ); } diff --git a/includes/user/User.php b/includes/user/User.php index cdbbcc583b..d6e1081714 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -3665,12 +3665,25 @@ class User implements IDBAccessObject, UserIdentity { return true; } + /** + * Alias of isLoggedIn() with a name that describes its actual functionality. UserIdentity has + * only this new name and not the old isLoggedIn() variant. + * + * @return bool True if user is registered on this wiki, i.e., has a user ID. False if user is + * anonymous or has no local account (which can happen when importing). This is equivalent to + * getId() != 0 and is provided for code readability. + * @since 1.34 + */ + public function isRegistered() { + return $this->getId() != 0; + } + /** * Get whether the user is logged in * @return bool */ public function isLoggedIn() { - return $this->getId() != 0; + return $this->isRegistered(); } /** @@ -3678,7 +3691,7 @@ class User implements IDBAccessObject, UserIdentity { * @return bool */ public function isAnon() { - return !$this->isLoggedIn(); + return !$this->isRegistered(); } /** diff --git a/includes/user/UserIdentity.php b/includes/user/UserIdentity.php index ac9bbec3f1..64c61fe294 100644 --- a/includes/user/UserIdentity.php +++ b/includes/user/UserIdentity.php @@ -62,4 +62,12 @@ interface UserIdentity { */ public function equals( UserIdentity $user ); + /** + * @since 1.34 + * + * @return bool True if user is registered on this wiki, i.e., has a user ID. False if user is + * anonymous or has no local account (which can happen when importing). This must be + * equivalent to getId() != 0 and is provided for code readability. + */ + public function isRegistered(); } diff --git a/includes/user/UserIdentityValue.php b/includes/user/UserIdentityValue.php index d1fd19de52..800ac760a5 100644 --- a/includes/user/UserIdentityValue.php +++ b/includes/user/UserIdentityValue.php @@ -93,4 +93,14 @@ class UserIdentityValue implements UserIdentity { return $this->getName() === $user->getName(); } + /** + * @since 1.34 + * + * @return bool True if user is registered on this wiki, i.e., has a user ID. False if user is + * anonymous or has no local account (which can happen when importing). This is equivalent to + * getId() != 0 and is provided for code readability. + */ + public function isRegistered() { + return $this->getId() != 0; + } } diff --git a/includes/watcheditem/NoWriteWatchedItemStore.php b/includes/watcheditem/NoWriteWatchedItemStore.php index fc95ebc46e..d5d175cac5 100644 --- a/includes/watcheditem/NoWriteWatchedItemStore.php +++ b/includes/watcheditem/NoWriteWatchedItemStore.php @@ -18,7 +18,9 @@ * @file * @ingroup Watchlist */ + use MediaWiki\Linker\LinkTarget; +use MediaWiki\User\UserIdentity; use Wikimedia\Rdbms\DBReadOnlyError; /** @@ -42,7 +44,7 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface { $this->actualStore = $actualStore; } - public function countWatchedItems( User $user ) { + public function countWatchedItems( UserIdentity $user ) { return $this->actualStore->countWatchedItems( $user ); } @@ -68,27 +70,27 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface { ); } - public function getWatchedItem( User $user, LinkTarget $target ) { + public function getWatchedItem( UserIdentity $user, LinkTarget $target ) { return $this->actualStore->getWatchedItem( $user, $target ); } - public function loadWatchedItem( User $user, LinkTarget $target ) { + public function loadWatchedItem( UserIdentity $user, LinkTarget $target ) { return $this->actualStore->loadWatchedItem( $user, $target ); } - public function getWatchedItemsForUser( User $user, array $options = [] ) { + public function getWatchedItemsForUser( UserIdentity $user, array $options = [] ) { return $this->actualStore->getWatchedItemsForUser( $user, $options ); } - public function isWatched( User $user, LinkTarget $target ) { + public function isWatched( UserIdentity $user, LinkTarget $target ) { return $this->actualStore->isWatched( $user, $target ); } - public function getNotificationTimestampsBatch( User $user, array $targets ) { + public function getNotificationTimestampsBatch( UserIdentity $user, array $targets ) { return $this->actualStore->getNotificationTimestampsBatch( $user, $targets ); } - public function countUnreadNotifications( User $user, $unreadLimit = null ) { + public function countUnreadNotifications( UserIdentity $user, $unreadLimit = null ) { return $this->actualStore->countUnreadNotifications( $user, $unreadLimit ); } @@ -100,36 +102,38 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function addWatch( User $user, LinkTarget $target ) { + public function addWatch( UserIdentity $user, LinkTarget $target ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function addWatchBatchForUser( User $user, array $targets ) { + public function addWatchBatchForUser( UserIdentity $user, array $targets ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function removeWatch( User $user, LinkTarget $target ) { + public function removeWatch( UserIdentity $user, LinkTarget $target ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } public function setNotificationTimestampsForUser( - User $user, + UserIdentity $user, $timestamp, array $targets = [] ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) { + public function updateNotificationTimestamp( + UserIdentity $editor, LinkTarget $target, $timestamp + ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function resetAllNotificationTimestampsForUser( User $user ) { + public function resetAllNotificationTimestampsForUser( UserIdentity $user ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } public function resetNotificationTimestamp( - User $user, + UserIdentity $user, Title $title, $force = '', $oldid = 0 @@ -137,19 +141,21 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function clearUserWatchedItems( User $user ) { + public function clearUserWatchedItems( UserIdentity $user ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function clearUserWatchedItemsUsingJobQueue( User $user ) { + public function clearUserWatchedItemsUsingJobQueue( UserIdentity $user ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function removeWatchBatchForUser( User $user, array $titles ) { + public function removeWatchBatchForUser( UserIdentity $user, array $titles ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function getLatestNotificationTimestamp( $timestamp, User $user, LinkTarget $target ) { + public function getLatestNotificationTimestamp( + $timestamp, UserIdentity $user, LinkTarget $target + ) { return wfTimestampOrNull( TS_MW, $timestamp ); } } diff --git a/includes/watcheditem/WatchedItem.php b/includes/watcheditem/WatchedItem.php index 43a9c4e536..4bf7f0c300 100644 --- a/includes/watcheditem/WatchedItem.php +++ b/includes/watcheditem/WatchedItem.php @@ -20,6 +20,7 @@ */ use MediaWiki\Linker\LinkTarget; +use MediaWiki\User\UserIdentity; /** * Representation of a pair of user and title for watchlist entries. @@ -36,7 +37,7 @@ class WatchedItem { private $linkTarget; /** - * @var User + * @var UserIdentity */ private $user; @@ -46,12 +47,12 @@ class WatchedItem { private $notificationTimestamp; /** - * @param User $user + * @param UserIdentity $user * @param LinkTarget $linkTarget * @param null|string $notificationTimestamp the value of the wl_notificationtimestamp field */ public function __construct( - User $user, + UserIdentity $user, LinkTarget $linkTarget, $notificationTimestamp ) { @@ -61,9 +62,17 @@ class WatchedItem { } /** + * @deprecated since 1.34, use getUserIdentity() * @return User */ public function getUser() { + return User::newFromIdentity( $this->user ); + } + + /** + * @return UserIdentity + */ + public function getUserIdentity() { return $this->user; } diff --git a/includes/watcheditem/WatchedItemQueryService.php b/includes/watcheditem/WatchedItemQueryService.php index 6094f41721..30e3cbee0e 100644 --- a/includes/watcheditem/WatchedItemQueryService.php +++ b/includes/watcheditem/WatchedItemQueryService.php @@ -1,8 +1,9 @@ string (format accepted by wfTimestamp) requires 'dir' option, * timestamp to end enumerating * 'watchlistOwner' => User user whose watchlist items should be listed if different - * than the one specified with $user param, - * requires 'watchlistOwnerToken' option + * than the one specified with $user param, requires + * 'watchlistOwnerToken' option * 'watchlistOwnerToken' => string a watchlist token used to access another user's * watchlist, used with 'watchlistOwnerToken' option * 'limit' => int maximum numbers of items to return @@ -256,7 +257,7 @@ class WatchedItemQueryService { /** * For simple listing of user's watchlist items, see WatchedItemStore::getWatchedItemsForUser * - * @param User $user + * @param UserIdentity $user * @param array $options Allowed keys: * 'sort' => string optional sorting by namespace ID and title * one of the self::SORT_* constants @@ -272,8 +273,8 @@ class WatchedItemQueryService { * specified using the form option * @return WatchedItem[] */ - public function getWatchedItemsForUser( User $user, array $options = [] ) { - if ( $user->isAnon() ) { + public function getWatchedItemsForUser( UserIdentity $user, array $options = [] ) { + if ( !$user->isRegistered() ) { // TODO: should this just return an empty array or rather complain loud at this point // as e.g. ApiBase::getWatchlistUser does? return []; @@ -460,11 +461,12 @@ class WatchedItemQueryService { return $conds; } - private function getWatchlistOwnerId( User $user, array $options ) { + private function getWatchlistOwnerId( UserIdentity $user, array $options ) { if ( array_key_exists( 'watchlistOwner', $options ) ) { /** @var User $watchlistOwner */ $watchlistOwner = $options['watchlistOwner']; - $ownersToken = $watchlistOwner->getOption( 'watchlisttoken' ); + $ownersToken = + $watchlistOwner->getOption( 'watchlisttoken' ); $token = $options['watchlistOwnerToken']; if ( $ownersToken == '' || !hash_equals( $ownersToken, $token ) ) { throw ApiUsageException::newWithMessage( null, 'apierror-bad-watchlist-token', 'bad_wltoken' ); @@ -613,7 +615,9 @@ class WatchedItemQueryService { ); } - private function getWatchedItemsForUserQueryConds( IDatabase $db, User $user, array $options ) { + private function getWatchedItemsForUserQueryConds( + IDatabase $db, UserIdentity $user, array $options + ) { $conds = [ 'wl_user' => $user->getId() ]; if ( $options['namespaceIds'] ) { $conds['wl_namespace'] = array_map( 'intval', $options['namespaceIds'] ); diff --git a/includes/watcheditem/WatchedItemQueryServiceExtension.php b/includes/watcheditem/WatchedItemQueryServiceExtension.php index a0e64c5d12..00770ea79d 100644 --- a/includes/watcheditem/WatchedItemQueryServiceExtension.php +++ b/includes/watcheditem/WatchedItemQueryServiceExtension.php @@ -1,5 +1,6 @@ cache->makeKey( (string)$target->getNamespace(), $target->getDBkey(), @@ -176,7 +177,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } private function cache( WatchedItem $item ) { - $user = $item->getUser(); + $user = $item->getUserIdentity(); $target = $item->getLinkTarget(); $key = $this->getCacheKey( $user, $target ); $this->cache->set( $key, $item ); @@ -184,7 +185,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac $this->stats->increment( 'WatchedItemStore.cache' ); } - private function uncache( User $user, LinkTarget $target ) { + private function uncache( UserIdentity $user, LinkTarget $target ) { $this->cache->delete( $this->getCacheKey( $user, $target ) ); unset( $this->cacheIndex[$target->getNamespace()][$target->getDBkey()][$user->getId()] ); $this->stats->increment( 'WatchedItemStore.uncache' ); @@ -201,7 +202,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } } - private function uncacheUser( User $user ) { + private function uncacheUser( UserIdentity $user ) { $this->stats->increment( 'WatchedItemStore.uncacheUser' ); foreach ( $this->cacheIndex as $ns => $dbKeyArray ) { foreach ( $dbKeyArray as $dbKey => $userArray ) { @@ -218,12 +219,12 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } /** - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * * @return WatchedItem|false */ - private function getCached( User $user, LinkTarget $target ) { + private function getCached( UserIdentity $user, LinkTarget $target ) { return $this->cache->get( $this->getCacheKey( $user, $target ) ); } @@ -231,12 +232,12 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * Return an array of conditions to select or update the appropriate database * row. * - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * * @return array */ - private function dbCond( User $user, LinkTarget $target ) { + private function dbCond( UserIdentity $user, LinkTarget $target ) { return [ 'wl_user' => $user->getId(), 'wl_namespace' => $target->getNamespace(), @@ -260,11 +261,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * * @since 1.30 * - * @param User $user + * @param UserIdentity $user * * @return bool true on success, false when too many items are watched */ - public function clearUserWatchedItems( User $user ) { + public function clearUserWatchedItems( UserIdentity $user ) { if ( $this->countWatchedItems( $user ) > $this->updateRowsPerQuery ) { return false; } @@ -280,7 +281,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return true; } - private function uncacheAllItemsForUser( User $user ) { + private function uncacheAllItemsForUser( UserIdentity $user ) { $userId = $user->getId(); foreach ( $this->cacheIndex as $ns => $dbKeyIndex ) { foreach ( $dbKeyIndex as $dbKey => $userIndex ) { @@ -309,9 +310,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * * @since 1.31 * - * @param User $user + * @param UserIdentity $user */ - public function clearUserWatchedItemsUsingJobQueue( User $user ) { + public function clearUserWatchedItemsUsingJobQueue( UserIdentity $user ) { $job = ClearUserWatchlistJob::newForUser( $user, $this->getMaxId() ); $this->queueGroup->push( $job ); } @@ -332,10 +333,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.31 - * @param User $user + * @param UserIdentity $user * @return int */ - public function countWatchedItems( User $user ) { + public function countWatchedItems( UserIdentity $user ) { $dbr = $this->getConnectionRef( DB_REPLICA ); $return = (int)$dbr->selectField( 'watchlist', @@ -394,16 +395,16 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } /** - * @param User $user + * @param UserIdentity $user * @param TitleValue[] $titles * @return bool * @throws MWException */ - public function removeWatchBatchForUser( User $user, array $titles ) { + public function removeWatchBatchForUser( UserIdentity $user, array $titles ) { if ( $this->readOnlyMode->isReadOnly() ) { return false; } - if ( $user->isAnon() ) { + if ( !$user->isRegistered() ) { return false; } if ( !$titles ) { @@ -563,12 +564,12 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * @return bool */ - public function getWatchedItem( User $user, LinkTarget $target ) { - if ( $user->isAnon() ) { + public function getWatchedItem( UserIdentity $user, LinkTarget $target ) { + if ( !$user->isRegistered() ) { return false; } @@ -583,13 +584,13 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * @return WatchedItem|bool */ - public function loadWatchedItem( User $user, LinkTarget $target ) { - // Only loggedin user can have a watchlist - if ( $user->isAnon() ) { + public function loadWatchedItem( UserIdentity $user, LinkTarget $target ) { + // Only registered user can have a watchlist + if ( !$user->isRegistered() ) { return false; } @@ -618,11 +619,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param array $options * @return WatchedItem[] */ - public function getWatchedItemsForUser( User $user, array $options = [] ) { + public function getWatchedItemsForUser( UserIdentity $user, array $options = [] ) { $options += [ 'forWrite' => false ]; $dbOptions = []; @@ -664,27 +665,27 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * @return bool */ - public function isWatched( User $user, LinkTarget $target ) { + public function isWatched( UserIdentity $user, LinkTarget $target ) { return (bool)$this->getWatchedItem( $user, $target ); } /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget[] $targets * @return array */ - public function getNotificationTimestampsBatch( User $user, array $targets ) { + public function getNotificationTimestampsBatch( UserIdentity $user, array $targets ) { $timestamps = []; foreach ( $targets as $target ) { $timestamps[$target->getNamespace()][$target->getDBkey()] = false; } - if ( $user->isAnon() ) { + if ( !$user->isRegistered() ) { return $timestamps; } @@ -728,27 +729,27 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * @throws MWException */ - public function addWatch( User $user, LinkTarget $target ) { + public function addWatch( UserIdentity $user, LinkTarget $target ) { $this->addWatchBatchForUser( $user, [ $target ] ); } /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget[] $targets * @return bool * @throws MWException */ - public function addWatchBatchForUser( User $user, array $targets ) { + public function addWatchBatchForUser( UserIdentity $user, array $targets ) { if ( $this->readOnlyMode->isReadOnly() ) { return false; } - // Only logged-in user can have a watchlist - if ( $user->isAnon() ) { + // Only registered user can have a watchlist + if ( !$user->isRegistered() ) { return false; } @@ -799,12 +800,12 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * @return bool * @throws MWException */ - public function removeWatch( User $user, LinkTarget $target ) { + public function removeWatch( UserIdentity $user, LinkTarget $target ) { return $this->removeWatchBatchForUser( $user, [ $target ] ); } @@ -820,14 +821,16 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * only the specified titles will be updated, and this will be done immediately (not deferred). * * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param string|int $timestamp Value to set the "last viewed" timestamp to (null to clear) * @param LinkTarget[] $targets Titles to set the timestamp for; [] means the entire watchlist * @return bool */ - public function setNotificationTimestampsForUser( User $user, $timestamp, array $targets = [] ) { - // Only loggedin user can have a watchlist - if ( $user->isAnon() || $this->readOnlyMode->isReadOnly() ) { + public function setNotificationTimestampsForUser( + UserIdentity $user, $timestamp, array $targets = [] + ) { + // Only registered user can have a watchlist + if ( !$user->isRegistered() || $this->readOnlyMode->isReadOnly() ) { return false; } @@ -873,7 +876,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return true; } - public function getLatestNotificationTimestamp( $timestamp, User $user, LinkTarget $target ) { + public function getLatestNotificationTimestamp( + $timestamp, UserIdentity $user, LinkTarget $target + ) { $timestamp = wfTimestampOrNull( TS_MW, $timestamp ); if ( $timestamp === null ) { return null; // no notification @@ -894,12 +899,12 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * Schedule a DeferredUpdate that sets all of the "last viewed" timestamps for a given user * to the same value. - * @param User $user + * @param UserIdentity $user * @param string|int|null $timestamp Value to set all timestamps to, null to clear them */ - public function resetAllNotificationTimestampsForUser( User $user, $timestamp = null ) { - // Only loggedin user can have a watchlist - if ( $user->isAnon() ) { + public function resetAllNotificationTimestampsForUser( UserIdentity $user, $timestamp = null ) { + // Only registered user can have a watchlist + if ( !$user->isRegistered() ) { return; } @@ -920,12 +925,14 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $editor + * @param UserIdentity $editor * @param LinkTarget $target * @param string|int $timestamp * @return int[] */ - public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) { + public function updateNotificationTimestamp( + UserIdentity $editor, LinkTarget $target, $timestamp + ) { $dbw = $this->getConnectionRef( DB_MASTER ); $uids = $dbw->selectFieldValues( 'watchlist', @@ -977,23 +984,32 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param Title $title * @param string $force * @param int $oldid * @return bool */ - public function resetNotificationTimestamp( User $user, Title $title, $force = '', $oldid = 0 ) { + public function resetNotificationTimestamp( + UserIdentity $user, Title $title, $force = '', $oldid = 0 + ) { $time = time(); - // Only loggedin user can have a watchlist - if ( $this->readOnlyMode->isReadOnly() || $user->isAnon() ) { + // Only registered user can have a watchlist + if ( $this->readOnlyMode->isReadOnly() || !$user->isRegistered() ) { return false; } - if ( !Hooks::run( 'BeforeResetNotificationTimestamp', [ &$user, &$title, $force, &$oldid ] ) ) { + // Hook expects User, not UserIdentity + $userObj = User::newFromId( $user->getId() ); + if ( !Hooks::run( 'BeforeResetNotificationTimestamp', + [ &$userObj, &$title, $force, &$oldid ] ) + ) { return false; } + if ( !$userObj->equals( $user ) ) { + $user = $userObj; + } $item = null; if ( $force != 'force' ) { @@ -1053,10 +1069,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } /** - * @param User $user + * @param UserIdentity $user * @return MapCacheLRU|null The map contains prefixed title keys and TS_MW values */ - private function getPageSeenTimestamps( User $user ) { + private function getPageSeenTimestamps( UserIdentity $user ) { $key = $this->getPageSeenTimestampsKey( $user ); return $this->latestUpdateCache->getWithSetCallback( @@ -1069,10 +1085,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } /** - * @param User $user + * @param UserIdentity $user * @return string */ - private function getPageSeenTimestampsKey( User $user ) { + private function getPageSeenTimestampsKey( UserIdentity $user ) { return $this->stash->makeGlobalKey( 'watchlist-recent-updates', $this->lbFactory->getLocalDomainID(), @@ -1088,7 +1104,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return "{$target->getNamespace()}:{$target->getDBkey()}"; } - private function getNotificationTimestamp( User $user, Title $title, $item, $force, $oldid ) { + private function getNotificationTimestamp( + UserIdentity $user, Title $title, $item, $force, $oldid + ) { if ( !$oldid ) { // No oldid given, assuming latest revision; clear the timestamp. return null; @@ -1137,11 +1155,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac /** * @since 1.27 - * @param User $user + * @param UserIdentity $user * @param int|null $unreadLimit * @return int|bool */ - public function countUnreadNotifications( User $user, $unreadLimit = null ) { + public function countUnreadNotifications( UserIdentity $user, $unreadLimit = null ) { $dbr = $this->getConnectionRef( DB_REPLICA ); $queryOptions = []; @@ -1241,10 +1259,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } /** - * @param User $user + * @param UserIdentity $user * @param Title[] $titles */ - private function uncacheTitlesForUser( User $user, array $titles ) { + private function uncacheTitlesForUser( UserIdentity $user, array $titles ) { foreach ( $titles as $title ) { $this->uncache( $user, $title ); } diff --git a/includes/watcheditem/WatchedItemStoreInterface.php b/includes/watcheditem/WatchedItemStoreInterface.php index 349d98a231..299c22209b 100644 --- a/includes/watcheditem/WatchedItemStoreInterface.php +++ b/includes/watcheditem/WatchedItemStoreInterface.php @@ -18,7 +18,9 @@ * @file * @ingroup Watchlist */ + use MediaWiki\Linker\LinkTarget; +use MediaWiki\User\UserIdentity; use Wikimedia\Rdbms\DBUnexpectedError; /** @@ -43,11 +45,11 @@ interface WatchedItemStoreInterface { * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * * @return int */ - public function countWatchedItems( User $user ); + public function countWatchedItems( UserIdentity $user ); /** * @since 1.31 @@ -115,29 +117,29 @@ interface WatchedItemStoreInterface { * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * * @return WatchedItem|false */ - public function getWatchedItem( User $user, LinkTarget $target ); + public function getWatchedItem( UserIdentity $user, LinkTarget $target ); /** * Loads an item from the db * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * * @return WatchedItem|false */ - public function loadWatchedItem( User $user, LinkTarget $target ); + public function loadWatchedItem( UserIdentity $user, LinkTarget $target ); /** * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param array $options Allowed keys: * 'forWrite' => bool defaults to false * 'sort' => string optional sorting by namespace ID and title @@ -145,24 +147,24 @@ interface WatchedItemStoreInterface { * * @return WatchedItem[] */ - public function getWatchedItemsForUser( User $user, array $options = [] ); + public function getWatchedItemsForUser( UserIdentity $user, array $options = [] ); /** * Must be called separately for Subject & Talk namespaces * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * * @return bool */ - public function isWatched( User $user, LinkTarget $target ); + public function isWatched( UserIdentity $user, LinkTarget $target ); /** * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget[] $targets * * @return array multi-dimensional like $return[$namespaceId][$titleString] = $timestamp, @@ -170,54 +172,54 @@ interface WatchedItemStoreInterface { * - string|null value of wl_notificationtimestamp, * - false if $target is not watched by $user. */ - public function getNotificationTimestampsBatch( User $user, array $targets ); + public function getNotificationTimestampsBatch( UserIdentity $user, array $targets ); /** * Must be called separately for Subject & Talk namespaces * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target */ - public function addWatch( User $user, LinkTarget $target ); + public function addWatch( UserIdentity $user, LinkTarget $target ); /** * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget[] $targets * * @return bool success */ - public function addWatchBatchForUser( User $user, array $targets ); + public function addWatchBatchForUser( UserIdentity $user, array $targets ); /** - * Removes an entry for the User watching the LinkTarget + * Removes an entry for the UserIdentity watching the LinkTarget * Must be called separately for Subject & Talk namespaces * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * * @return bool success * @throws DBUnexpectedError * @throws MWException */ - public function removeWatch( User $user, LinkTarget $target ); + public function removeWatch( UserIdentity $user, LinkTarget $target ); /** * @since 1.31 * - * @param User $user The user to set the timestamps for + * @param UserIdentity $user The user to set the timestamps for * @param string|null $timestamp Set the update timestamp to this value * @param LinkTarget[] $targets List of targets to update. Default to all targets * * @return bool success */ public function setNotificationTimestampsForUser( - User $user, + UserIdentity $user, $timestamp, array $targets = [] ); @@ -227,28 +229,29 @@ interface WatchedItemStoreInterface { * * @since 1.31 * - * @param User $user The user to reset the timestamps for + * @param UserIdentity $user The user to reset the timestamps for */ - public function resetAllNotificationTimestampsForUser( User $user ); + public function resetAllNotificationTimestampsForUser( UserIdentity $user ); /** * @since 1.31 * - * @param User $editor The editor that triggered the update. Their notification + * @param UserIdentity $editor The editor that triggered the update. Their notification * timestamp will not be updated(they have already seen it) * @param LinkTarget $target The target to update timestamps for * @param string $timestamp Set the update timestamp to this value * * @return int[] Array of user IDs the timestamp has been updated for */ - public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ); + public function updateNotificationTimestamp( + UserIdentity $editor, LinkTarget $target, $timestamp ); /** * Reset the notification timestamp of this entry * * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param Title $title * @param string $force Whether to force the write query to be executed even if the * page is not watched or the notification timestamp is already NULL. @@ -258,18 +261,19 @@ interface WatchedItemStoreInterface { * * @return bool success Whether a job was enqueued */ - public function resetNotificationTimestamp( User $user, Title $title, $force = '', $oldid = 0 ); + public function resetNotificationTimestamp( + UserIdentity $user, Title $title, $force = '', $oldid = 0 ); /** * @since 1.31 * - * @param User $user + * @param UserIdentity $user * @param int|null $unreadLimit * * @return int|bool The number of unread notifications * true if greater than or equal to $unreadLimit */ - public function countUnreadNotifications( User $user, $unreadLimit = null ); + public function countUnreadNotifications( UserIdentity $user, $unreadLimit = null ); /** * Check if the given title already is watched by the user, and if so @@ -303,28 +307,28 @@ interface WatchedItemStoreInterface { * * @since 1.31 * - * @param User $user + * @param UserIdentity $user */ - public function clearUserWatchedItems( User $user ); + public function clearUserWatchedItems( UserIdentity $user ); /** * Queues a job that will clear the users watchlist using the Job Queue. * * @since 1.31 * - * @param User $user + * @param UserIdentity $user */ - public function clearUserWatchedItemsUsingJobQueue( User $user ); + public function clearUserWatchedItemsUsingJobQueue( UserIdentity $user ); /** * @since 1.32 * - * @param User $user + * @param UserIdentity $user * @param LinkTarget[] $targets * * @return bool success */ - public function removeWatchBatchForUser( User $user, array $targets ); + public function removeWatchBatchForUser( UserIdentity $user, array $targets ); /** * Convert $timestamp to TS_MW or return null if the page was visited since then by $user @@ -335,9 +339,10 @@ interface WatchedItemStoreInterface { * Usage of this method should be limited to WatchedItem* classes * * @param string|null $timestamp Value of wl_notificationtimestamp from the DB - * @param User $user + * @param UserIdentity $user * @param LinkTarget $target * @return string TS_MW timestamp or null */ - public function getLatestNotificationTimestamp( $timestamp, User $user, LinkTarget $target ); + public function getLatestNotificationTimestamp( + $timestamp, UserIdentity $user, LinkTarget $target ); } diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index ebc3b79c3f..fa5832e7ae 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -2408,4 +2408,18 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { 'comment' => $comment, ] ); } + + /** + * Returns a PHPUnit constraint that matches anything other than a fixed set of values. This can + * be used to whitelist values, e.g. + * $mock->expects( $this->never() )->method( $this->anythingBut( 'foo', 'bar' ) ); + * which will throw if any unexpected method is called. + * + * @param mixed ...$values Values that are not matched + */ + protected function anythingBut( ...$values ) { + return $this->logicalNot( $this->logicalOr( + ...array_map( [ $this, 'matches' ], $values ) + ) ); + } } diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php index 481da75af6..02b2f3ce28 100644 --- a/tests/phpunit/includes/user/UserTest.php +++ b/tests/phpunit/includes/user/UserTest.php @@ -504,20 +504,24 @@ class UserTest extends MediaWikiTestCase { } /** + * @covers User::isRegistered * @covers User::isLoggedIn * @covers User::isAnon */ public function testLoggedIn() { $user = $this->getMutableTestUser()->getUser(); + $this->assertTrue( $user->isRegistered() ); $this->assertTrue( $user->isLoggedIn() ); $this->assertFalse( $user->isAnon() ); // Non-existent users are perceived as anonymous $user = User::newFromName( 'UTNonexistent' ); + $this->assertFalse( $user->isRegistered() ); $this->assertFalse( $user->isLoggedIn() ); $this->assertTrue( $user->isAnon() ); $user = new User; + $this->assertFalse( $user->isRegistered() ); $this->assertFalse( $user->isLoggedIn() ); $this->assertTrue( $user->isAnon() ); } diff --git a/tests/phpunit/includes/watcheditem/NoWriteWatchedItemStoreUnitTest.php b/tests/phpunit/includes/watcheditem/NoWriteWatchedItemStoreUnitTest.php index a8761e39e9..cc60899153 100644 --- a/tests/phpunit/includes/watcheditem/NoWriteWatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/watcheditem/NoWriteWatchedItemStoreUnitTest.php @@ -1,5 +1,7 @@ setExpectedException( DBReadOnlyError::class ); - $noWriteService->addWatch( $this->getTestSysop()->getUser(), new TitleValue( 0, 'Foo' ) ); + $noWriteService->addWatch( + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'Foo' ) ); } public function testAddWatchBatchForUser() { @@ -24,7 +27,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $this->setExpectedException( DBReadOnlyError::class ); - $noWriteService->addWatchBatchForUser( $this->getTestSysop()->getUser(), [] ); + $noWriteService->addWatchBatchForUser( new UserIdentityValue( 1, 'MockUser', 0 ), [] ); } public function testRemoveWatch() { @@ -34,7 +37,8 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $this->setExpectedException( DBReadOnlyError::class ); - $noWriteService->removeWatch( $this->getTestSysop()->getUser(), new TitleValue( 0, 'Foo' ) ); + $noWriteService->removeWatch( + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'Foo' ) ); } public function testSetNotificationTimestampsForUser() { @@ -45,7 +49,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $this->setExpectedException( DBReadOnlyError::class ); $noWriteService->setNotificationTimestampsForUser( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), 'timestamp', [] ); @@ -59,7 +63,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $this->setExpectedException( DBReadOnlyError::class ); $noWriteService->updateNotificationTimestamp( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'Foo' ), 'timestamp' ); @@ -73,7 +77,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $this->setExpectedException( DBReadOnlyError::class ); $noWriteService->resetNotificationTimestamp( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), Title::newFromText( 'Foo' ) ); } @@ -85,7 +89,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->countWatchedItems( - $this->getTestSysop()->getUser() + new UserIdentityValue( 1, 'MockUser', 0 ) ); $this->assertEquals( __METHOD__, $return ); } @@ -154,7 +158,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->getWatchedItem( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'Foo' ) ); $this->assertEquals( __METHOD__, $return ); @@ -167,7 +171,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->loadWatchedItem( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'Foo' ) ); $this->assertEquals( __METHOD__, $return ); @@ -182,7 +186,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->getWatchedItemsForUser( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), [] ); $this->assertEquals( __METHOD__, $return ); @@ -195,7 +199,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->isWatched( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'Foo' ) ); $this->assertEquals( __METHOD__, $return ); @@ -210,7 +214,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->getNotificationTimestampsBatch( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), [ new TitleValue( 0, 'Foo' ) ] ); $this->assertEquals( __METHOD__, $return ); @@ -225,7 +229,7 @@ class NoWriteWatchedItemStoreUnitTest extends MediaWikiTestCase { $noWriteService = new NoWriteWatchedItemStore( $innerService ); $return = $noWriteService->countUnreadNotifications( - $this->getTestSysop()->getUser(), + new UserIdentityValue( 1, 'MockUser', 0 ), 88 ); $this->assertEquals( __METHOD__, $return ); diff --git a/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php index b22b7f8141..3ba8773c29 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php @@ -1,5 +1,6 @@ getMockBuilder( User::class )->getMock(); - $mock->expects( $this->any() ) - ->method( 'isAnon' ) - ->will( $this->returnValue( false ) ); - $mock->expects( $this->any() ) - ->method( 'getId' ) - ->will( $this->returnValue( $id ) ); + $mock->method( 'isRegistered' )->willReturn( true ); + $mock->method( 'getId' )->willReturn( $id ); + $methods = array_merge( [ + 'isRegistered', + 'getId', + ], $extraMethods ); + $mock->expects( $this->never() )->method( $this->anythingBut( ...$methods ) ); return $mock; } /** * @param int $id + * @param string[] $extraMethods Extra methods that are expected might be called * @return PHPUnit_Framework_MockObject_MockObject|User */ - private function getMockUnrestrictedNonAnonUserWithId( $id ) { - $mock = $this->getMockNonAnonUserWithId( $id ); - $mock->expects( $this->any() ) - ->method( 'isAllowed' ) - ->will( $this->returnValue( true ) ); - $mock->expects( $this->any() ) - ->method( 'isAllowedAny' ) - ->will( $this->returnValue( true ) ); - $mock->expects( $this->any() ) - ->method( 'useRCPatrol' ) - ->will( $this->returnValue( true ) ); + private function getMockUnrestrictedNonAnonUserWithId( $id, array $extraMethods = [] ) { + $mock = $this->getMockNonAnonUserWithId( $id, + array_merge( [ 'isAllowed', 'isAllowedAny', 'useRCPatrol' ], $extraMethods ) ); + $mock->method( 'isAllowed' )->willReturn( true ); + $mock->method( 'isAllowedAny' )->willReturn( true ); + $mock->method( 'useRCPatrol' )->willReturn( true ); return $mock; } @@ -193,18 +192,19 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase { * @return PHPUnit_Framework_MockObject_MockObject|User */ private function getMockNonAnonUserWithIdAndRestrictedPermissions( $id, $notAllowedAction ) { - $mock = $this->getMockNonAnonUserWithId( $id ); + $mock = $this->getMockNonAnonUserWithId( $id, + [ 'isAllowed', 'isAllowedAny', 'useRCPatrol', 'useNPPatrol' ] ); - $mock->expects( $this->any() ) - ->method( 'isAllowed' ) + $mock->method( 'isAllowed' ) ->will( $this->returnCallback( function ( $action ) use ( $notAllowedAction ) { return $action !== $notAllowedAction; } ) ); - $mock->expects( $this->any() ) - ->method( 'isAllowedAny' ) + $mock->method( 'isAllowedAny' ) ->will( $this->returnCallback( function ( ...$actions ) use ( $notAllowedAction ) { return !in_array( $notAllowedAction, $actions ); } ) ); + $mock->method( 'useRCPatrol' )->willReturn( false ); + $mock->method( 'useNPPatrol' )->willReturn( false ); return $mock; } @@ -214,7 +214,8 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase { * @return PHPUnit_Framework_MockObject_MockObject|User */ private function getMockNonAnonUserWithIdAndNoPatrolRights( $id ) { - $mock = $this->getMockNonAnonUserWithId( $id ); + $mock = $this->getMockNonAnonUserWithId( $id, + [ 'isAllowed', 'isAllowedAny', 'useRCPatrol', 'useNPPatrol' ] ); $mock->expects( $this->any() ) ->method( 'isAllowed' ) @@ -233,14 +234,6 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase { return $mock; } - private function getMockAnonUser() { - $mock = $this->getMockBuilder( User::class )->getMock(); - $mock->expects( $this->any() ) - ->method( 'isAnon' ) - ->will( $this->returnValue( true ) ); - return $mock; - } - private function getFakeRow( array $rowValues ) { $fakeRow = new stdClass(); foreach ( $rowValues as $valueName => $value ) { @@ -1382,7 +1375,7 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase { $queryService = $this->newService( $mockDb ); $user = $this->getMockUnrestrictedNonAnonUserWithId( 1 ); - $otherUser = $this->getMockUnrestrictedNonAnonUserWithId( 2 ); + $otherUser = $this->getMockUnrestrictedNonAnonUserWithId( 2, [ 'getOption' ] ); $otherUser->expects( $this->once() ) ->method( 'getOption' ) ->with( 'watchlisttoken' ) @@ -1413,7 +1406,7 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase { $queryService = $this->newService( $mockDb ); $user = $this->getMockUnrestrictedNonAnonUserWithId( 1 ); - $otherUser = $this->getMockUnrestrictedNonAnonUserWithId( 2 ); + $otherUser = $this->getMockUnrestrictedNonAnonUserWithId( 2, [ 'getOption' ] ); $otherUser->expects( $this->once() ) ->method( 'getOption' ) ->with( 'watchlisttoken' ) @@ -1713,7 +1706,8 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase { $queryService = $this->newService( $mockDb ); - $items = $queryService->getWatchedItemsForUser( $this->getMockAnonUser() ); + $items = $queryService->getWatchedItemsForUser( + new UserIdentityValue( 0, 'AnonUser', 0 ) ); $this->assertEmpty( $items ); } diff --git a/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php index 2f95688548..ca5ae3e4c8 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php @@ -1,5 +1,6 @@ createMock( User::class ); - $mock->expects( $this->any() ) - ->method( 'isAnon' ) - ->will( $this->returnValue( false ) ); - $mock->expects( $this->any() ) - ->method( 'getId' ) - ->will( $this->returnValue( $id ) ); - $mock->expects( $this->any() ) - ->method( 'getUserPage' ) - ->will( $this->returnValue( Title::makeTitle( NS_USER, 'MockUser' ) ) ); - return $mock; - } - - /** - * @return User - */ - private function getAnonUser() { - return User::newFromName( 'Anon_User' ); - } - private function getFakeRow( array $rowValues ) { $fakeRow = new stdClass(); foreach ( $rowValues as $valueName => $value ) { @@ -158,7 +134,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testClearWatchedItems() { - $user = $this->getMockNonAnonUserWithId( 7 ); + $user = new UserIdentityValue( 7, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) @@ -200,7 +176,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testClearWatchedItems_tooManyItemsWatched() { - $user = $this->getMockNonAnonUserWithId( 7 ); + $user = new UserIdentityValue( 7, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) @@ -231,7 +207,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testCountWatchedItems() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->exactly( 1 ) ) @@ -716,7 +692,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testCountUnreadNotifications() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->exactly( 1 ) ) @@ -751,7 +727,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { * @dataProvider provideIntWithDbUnsafeVersion */ public function testCountUnreadNotifications_withUnreadLimit_overLimit( $limit ) { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->exactly( 1 ) ) @@ -790,7 +766,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { * @dataProvider provideIntWithDbUnsafeVersion */ public function testCountUnreadNotifications_withUnreadLimit_underLimit( $limit ) { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->exactly( 1 ) ) @@ -852,8 +828,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $store->duplicateEntry( - Title::newFromText( 'Old_Title' ), - Title::newFromText( 'New_Title' ) + new TitleValue( 0, 'Old_Title' ), + new TitleValue( 0, 'New_Title' ) ); } @@ -912,8 +888,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $store->duplicateEntry( - Title::newFromText( 'Old_Title' ), - Title::newFromText( 'New_Title' ) + new TitleValue( 0, 'Old_Title' ), + new TitleValue( 0, 'New_Title' ) ); } @@ -960,14 +936,14 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $store->duplicateAllAssociatedEntries( - Title::newFromText( 'Old_Title' ), - Title::newFromText( 'New_Title' ) + new TitleValue( 0, 'Old_Title' ), + new TitleValue( 0, 'New_Title' ) ); } public function provideLinkTargetPairs() { return [ - [ Title::newFromText( 'Old_Title' ), Title::newFromText( 'New_Title' ) ], + [ new TitleValue( 0, 'Old_Title' ), new TitleValue( 0, 'New_Title' ) ], [ new TitleValue( 0, 'Old_Title' ), new TitleValue( 0, 'New_Title' ) ], ]; } @@ -1089,8 +1065,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $store->addWatch( - $this->getMockNonAnonUserWithId( 1 ), - Title::newFromText( 'Some_Page' ) + new UserIdentityValue( 1, 'MockUser', 0 ), + new TitleValue( 0, 'Some_Page' ) ); } @@ -1111,8 +1087,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $store->addWatch( - $this->getAnonUser(), - Title::newFromText( 'Some_Page' ) + new UserIdentityValue( 0, 'AnonUser', 0 ), + new TitleValue( 0, 'Some_Page' ) ); } @@ -1126,7 +1102,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->addWatchBatchForUser( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), [ new TitleValue( 0, 'Some_Page' ), new TitleValue( 1, 'Some_Page' ) ] ) ); @@ -1175,7 +1151,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->getMockReadOnlyMode() ); - $mockUser = $this->getMockNonAnonUserWithId( 1 ); + $mockUser = new UserIdentityValue( 1, 'MockUser', 0 ); $this->assertTrue( $store->addWatchBatchForUser( @@ -1203,14 +1179,14 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->addWatchBatchForUser( - $this->getAnonUser(), + new UserIdentityValue( 0, 'AnonUser', 0 ), [ new TitleValue( 0, 'Other_Page' ) ] ) ); } public function testAddWatchBatchReturnsTrue_whenGivenEmptyList() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->never() ) ->method( 'insert' ); @@ -1263,7 +1239,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $watchedItem = $store->loadWatchedItem( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ); $this->assertInstanceOf( WatchedItem::class, $watchedItem ); @@ -1300,7 +1276,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->loadWatchedItem( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1324,7 +1300,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->loadWatchedItem( - $this->getAnonUser(), + new UserIdentityValue( 0, 'AnonUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1372,11 +1348,10 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->getMockReadOnlyMode() ); - $titleValue = new TitleValue( 0, 'SomeDbKey' ); $this->assertTrue( $store->removeWatch( - $this->getMockNonAnonUserWithId( 1 ), - Title::newFromTitleValue( $titleValue ) + new UserIdentityValue( 1, 'MockUser', 0 ), + new TitleValue( 0, 'SomeDbKey' ) ) ); } @@ -1424,11 +1399,10 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->getMockReadOnlyMode() ); - $titleValue = new TitleValue( 0, 'SomeDbKey' ); $this->assertFalse( $store->removeWatch( - $this->getMockNonAnonUserWithId( 1 ), - Title::newFromTitleValue( $titleValue ) + new UserIdentityValue( 1, 'MockUser', 0 ), + new TitleValue( 0, 'SomeDbKey' ) ) ); } @@ -1452,7 +1426,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->removeWatch( - $this->getAnonUser(), + new UserIdentityValue( 0, 'AnonUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1497,7 +1471,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $watchedItem = $store->getWatchedItem( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ); $this->assertInstanceOf( WatchedItem::class, $watchedItem ); @@ -1511,7 +1485,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $mockDb->expects( $this->never() ) ->method( 'selectRow' ); - $mockUser = $this->getMockNonAnonUserWithId( 1 ); + $mockUser = new UserIdentityValue( 1, 'MockUser', 0 ); $linkTarget = new TitleValue( 0, 'SomeDbKey' ); $cachedItem = new WatchedItem( $mockUser, $linkTarget, '20151212010101' ); @@ -1573,7 +1547,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->getWatchedItem( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1598,7 +1572,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->getWatchedItem( - $this->getAnonUser(), + new UserIdentityValue( 0, 'AnonUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1637,7 +1611,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $mockCache, $this->getMockReadOnlyMode() ); - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $watchedItems = $store->getWatchedItemsForUser( $user ); @@ -1670,7 +1644,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $mockDb = $this->getMockDb(); $mockCache = $this->getMockCache(); $mockLoadBalancer = $this->getMockLBFactory( $mockDb, $dbType ); - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $mockDb->expects( $this->once() ) ->method( 'select' ) @@ -1707,7 +1681,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->setExpectedException( InvalidArgumentException::class ); $store->getWatchedItemsForUser( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), [ 'sort' => 'foo' ] ); } @@ -1750,7 +1724,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertTrue( $store->isWatched( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1788,7 +1762,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->isWatched( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1813,7 +1787,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->isWatched( - $this->getAnonUser(), + new UserIdentityValue( 0, 'AnonUser', 0 ), new TitleValue( 0, 'SomeDbKey' ) ) ); @@ -1885,7 +1859,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { 0 => [ 'SomeDbKey' => '20151212010101', ], 1 => [ 'AnotherDbKey' => null, ], ], - $store->getNotificationTimestampsBatch( $this->getMockNonAnonUserWithId( 1 ), $targets ) + $store->getNotificationTimestampsBatch( + new UserIdentityValue( 1, 'MockUser', 0 ), $targets ) ); } @@ -1936,7 +1911,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { [ 0 => [ 'OtherDbKey' => false, ], ], - $store->getNotificationTimestampsBatch( $this->getMockNonAnonUserWithId( 1 ), $targets ) + $store->getNotificationTimestampsBatch( + new UserIdentityValue( 1, 'MockUser', 0 ), $targets ) ); } @@ -1946,7 +1922,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { new TitleValue( 1, 'AnotherDbKey' ), ]; - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $cachedItem = new WatchedItem( $user, $targets[0], '20151212010101' ); $mockDb = $this->getMockDb(); @@ -2010,7 +1986,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { new TitleValue( 1, 'AnotherDbKey' ), ]; - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $cachedItems = [ new WatchedItem( $user, $targets[0], '20151212010101' ), new WatchedItem( $user, $targets[1], null ), @@ -2070,7 +2046,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { 0 => [ 'SomeDbKey' => false, ], 1 => [ 'AnotherDbKey' => false, ], ], - $store->getNotificationTimestampsBatch( $this->getAnonUser(), $targets ) + $store->getNotificationTimestampsBatch( + new UserIdentityValue( 0, 'AnonUser', 0 ), $targets ) ); } @@ -2093,7 +2070,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->resetNotificationTimestamp( - $this->getAnonUser(), + new UserIdentityValue( 0, 'AnonUser', 0 ), Title::newFromText( 'SomeDbKey' ) ) ); @@ -2128,14 +2105,14 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertFalse( $store->resetNotificationTimestamp( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), Title::newFromText( 'SomeDbKey' ) ) ); } public function testResetNotificationTimestamp_item() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $title = Title::newFromText( 'SomeDbKey' ); $mockDb = $this->getMockDb(); @@ -2189,7 +2166,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testResetNotificationTimestamp_noItemForced() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $title = Title::newFromText( 'SomeDbKey' ); $mockDb = $this->getMockDb(); @@ -2265,7 +2242,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testResetNotificationTimestamp_oldidSpecifiedLatestRevisionForced() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $oldid = 22; $title = $this->getMockTitle( 'SomeTitle' ); $title->expects( $this->once() ) @@ -2318,7 +2295,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testResetNotificationTimestamp_oldidSpecifiedNotLatestRevisionForced() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $oldid = 22; $title = $this->getMockTitle( 'SomeDbKey' ); $title->expects( $this->once() ) @@ -2397,7 +2374,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testResetNotificationTimestamp_notWatchedPageForced() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $oldid = 22; $title = $this->getMockTitle( 'SomeDbKey' ); $title->expects( $this->once() ) @@ -2460,7 +2437,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testResetNotificationTimestamp_futureNotificationTimestampForced() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $oldid = 22; $title = $this->getMockTitle( 'SomeDbKey' ); $title->expects( $this->once() ) @@ -2539,7 +2516,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testResetNotificationTimestamp_futureNotificationTimestampNotForced() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $oldid = 22; $title = $this->getMockTitle( 'SomeDbKey' ); $title->expects( $this->once() ) @@ -2624,11 +2601,12 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->getMockCache(), $this->getMockReadOnlyMode() ); - $this->assertFalse( $store->setNotificationTimestampsForUser( $this->getAnonUser(), '' ) ); + $this->assertFalse( $store->setNotificationTimestampsForUser( + new UserIdentityValue( 0, 'AnonUser', 0 ), '' ) ); } public function testSetNotificationTimestampsForUser_allRows() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $timestamp = '20100101010101'; $store = $this->newWatchedItemStore( @@ -2653,7 +2631,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testSetNotificationTimestampsForUser_nullTimestamp() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $timestamp = null; $store = $this->newWatchedItemStore( @@ -2677,7 +2655,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testSetNotificationTimestampsForUser_specificTargets() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $timestamp = '20100101010101'; $targets = [ new TitleValue( 0, 'Foo' ), new TitleValue( 0, 'Bar' ) ]; @@ -2753,7 +2731,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertEquals( [ 2, 3 ], $store->updateNotificationTimestamp( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ), '20151212010101' ) @@ -2793,7 +2771,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { ); $watchers = $store->updateNotificationTimestamp( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), new TitleValue( 0, 'SomeDbKey' ), '20151212010101' ); @@ -2802,7 +2780,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { } public function testUpdateNotificationTimestamp_clearsCachedItems() { - $user = $this->getMockNonAnonUserWithId( 1 ); + $user = new UserIdentityValue( 1, 'MockUser', 0 ); $titleValue = new TitleValue( 0, 'SomeDbKey' ); $mockDb = $this->getMockDb(); @@ -2841,7 +2819,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $store->getWatchedItem( $user, $titleValue ); $store->updateNotificationTimestamp( - $this->getMockNonAnonUserWithId( 1 ), + new UserIdentityValue( 1, 'MockUser', 0 ), $titleValue, '20151212010101' );