diff --git a/docs/hooks.txt b/docs/hooks.txt index 4fc876c0960..31645ab5dec 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -3921,6 +3921,7 @@ used to alter the SQL query which gets the list of wanted pages. [&]$user: user that will watch [&]$page: WikiPage object to be watched &$status: Status object to be returned if the hook returns false +$expiry: Optional expiry timestamp in any format acceptable to wfTimestamp() 'WatchArticleComplete': After a watch is added to an article. [&]$user: user that watched diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 165a66e7150..f41e2a98284 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -9095,6 +9095,14 @@ $wgNativeImageLazyLoading = false; */ $wgMainPageIsDomainRoot = false; +/** + * Whether to enable the watchlist expiry feature. + * + * @since 1.35 + * @var bool + */ +$wgWatchlistExpiry = false; + /** * For really cool vim folding this needs to be at the end: * vim: foldmarker=@{,@} foldmethod=marker diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 94cbff6c944..56eddeb2513 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -1076,7 +1076,8 @@ return [ $services->getReadOnlyMode(), $services->getMainConfig()->get( 'UpdateRowsPerQuery' ), $services->getNamespaceInfo(), - $services->getRevisionLookup() + $services->getRevisionLookup(), + $services->getMainConfig()->get( 'WatchlistExpiry' ) ); $store->setStatsdDataFactory( $services->getStatsdDataFactory() ); diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php index e88654ad2fa..a04928716f9 100644 --- a/includes/actions/WatchAction.php +++ b/includes/actions/WatchAction.php @@ -111,12 +111,15 @@ class WatchAction extends FormAction { * @param User $user User who is watching/unwatching * @param bool $checkRights Passed through to $user->addWatch() * Pass User::CHECK_USER_RIGHTS or User::IGNORE_USER_RIGHTS. + * @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. * @return Status */ public static function doWatch( Title $title, User $user, - $checkRights = User::CHECK_USER_RIGHTS + $checkRights = User::CHECK_USER_RIGHTS, + ?string $expiry = null ) { $permissionManager = MediaWikiServices::getInstance()->getPermissionManager(); if ( $checkRights && !$permissionManager->userHasRight( $user, 'editmywatchlist' ) ) { @@ -126,9 +129,9 @@ class WatchAction extends FormAction { $page = WikiPage::factory( $title ); $status = Status::newFatal( 'hookaborted' ); - if ( Hooks::run( 'WatchArticle', [ &$user, &$page, &$status ] ) ) { + if ( Hooks::run( 'WatchArticle', [ &$user, &$page, &$status, $expiry ] ) ) { $status = Status::newGood(); - $user->addWatch( $title, $checkRights ); + $user->addWatch( $title, $checkRights, $expiry ); Hooks::run( 'WatchArticleComplete', [ &$user, &$page ] ); } diff --git a/includes/user/User.php b/includes/user/User.php index 34ad0ecb791..31d3b5dc521 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -3735,12 +3735,19 @@ class User implements IDBAccessObject, UserIdentity { * @param Title $title Title of the article to look at * @param bool $checkRights Whether to check 'viewmywatchlist'/'editmywatchlist' rights. * Pass User::CHECK_USER_RIGHTS or User::IGNORE_USER_RIGHTS. + * @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. */ - public function addWatch( $title, $checkRights = self::CHECK_USER_RIGHTS ) { + public function addWatch( + $title, + $checkRights = self::CHECK_USER_RIGHTS, + ?string $expiry = null + ) { if ( !$checkRights || $this->isAllowed( 'editmywatchlist' ) ) { MediaWikiServices::getInstance()->getWatchedItemStore()->addWatchBatchForUser( $this, - [ $title->getSubjectPage(), $title->getTalkPage() ] + [ $title->getSubjectPage(), $title->getTalkPage() ], + $expiry ); } $this->invalidateCache(); diff --git a/includes/watcheditem/NoWriteWatchedItemStore.php b/includes/watcheditem/NoWriteWatchedItemStore.php index f4b5e89bd1f..30ee9c06e02 100644 --- a/includes/watcheditem/NoWriteWatchedItemStore.php +++ b/includes/watcheditem/NoWriteWatchedItemStore.php @@ -105,11 +105,15 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function addWatch( UserIdentity $user, LinkTarget $target ) { + public function addWatch( UserIdentity $user, LinkTarget $target, ?string $expiry = null ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } - public function addWatchBatchForUser( UserIdentity $user, array $targets ) { + public function addWatchBatchForUser( + UserIdentity $user, + array $targets, + ?string $expiry = null + ) { throw new DBReadOnlyError( null, self::DB_READONLY_ERROR ); } diff --git a/includes/watcheditem/WatchedItem.php b/includes/watcheditem/WatchedItem.php index 4bf7f0c3006..fa1372672df 100644 --- a/includes/watcheditem/WatchedItem.php +++ b/includes/watcheditem/WatchedItem.php @@ -46,19 +46,27 @@ class WatchedItem { */ private $notificationTimestamp; + /** + * @var string|null When to automatically unwatch the page + */ + private $expiry; + /** * @param UserIdentity $user * @param LinkTarget $linkTarget * @param null|string $notificationTimestamp the value of the wl_notificationtimestamp field + * @param null|string $expiry Optional expiry timestamp in any format acceptable to wfTimestamp() */ public function __construct( UserIdentity $user, LinkTarget $linkTarget, - $notificationTimestamp + $notificationTimestamp, + ?string $expiry = null ) { $this->user = $user; $this->linkTarget = $linkTarget; $this->notificationTimestamp = $notificationTimestamp; + $this->expiry = $expiry; } /** @@ -91,4 +99,31 @@ class WatchedItem { public function getNotificationTimestamp() { return $this->notificationTimestamp; } + + /** + * When the watched item will expire. + * + * @since 1.35 + * + * @return string|null null or in a format acceptable to wfTimestamp(). + */ + public function getExpiry(): ?string { + return $this->expiry; + } + + /** + * Has the watched item expired? + * + * @since 1.35 + * + * @return bool + */ + public function isExpired(): bool { + if ( null === $this->getExpiry() ) { + return false; + } + + $unix = MWTimestamp::convert( TS_UNIX, $this->getExpiry() ); + return $unix < wfTimestamp(); + } } diff --git a/includes/watcheditem/WatchedItemStore.php b/includes/watcheditem/WatchedItemStore.php index 2e5797d421d..10316fc4713 100644 --- a/includes/watcheditem/WatchedItemStore.php +++ b/includes/watcheditem/WatchedItemStore.php @@ -7,6 +7,7 @@ use MediaWiki\User\UserIdentity; use Wikimedia\Assert\Assert; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\ILBFactory; +use Wikimedia\Rdbms\IResultWrapper; use Wikimedia\Rdbms\LoadBalancer; use Wikimedia\ScopedCallback; @@ -89,6 +90,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac */ private $stats; + /** + * @var bool Correlates to $wgWatchlistExpiry feature flag. + */ + private $expiryEnabled; + /** * @param ILBFactory $lbFactory * @param JobQueueGroup $queueGroup @@ -98,6 +104,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * @param int $updateRowsPerQuery * @param NamespaceInfo $nsInfo * @param RevisionLookup $revisionLookup + * @param bool $expiryEnabled */ public function __construct( ILBFactory $lbFactory, @@ -107,7 +114,8 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac ReadOnlyMode $readOnlyMode, $updateRowsPerQuery, NamespaceInfo $nsInfo, - RevisionLookup $revisionLookup + RevisionLookup $revisionLookup, + bool $expiryEnabled ) { $this->lbFactory = $lbFactory; $this->loadBalancer = $lbFactory->getMainLB(); @@ -121,6 +129,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac $this->updateRowsPerQuery = $updateRowsPerQuery; $this->nsInfo = $nsInfo; $this->revisionLookup = $revisionLookup; + $this->expiryEnabled = $expiryEnabled; $this->latestUpdateCache = new HashBagOStuff( [ 'maxKeys' => 3 ] ); } @@ -216,23 +225,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return $this->cache->get( $this->getCacheKey( $user, $target ) ); } - /** - * Return an array of conditions to select or update the appropriate database - * row. - * - * @param UserIdentity $user - * @param LinkTarget $target - * - * @return array - */ - private function dbCond( UserIdentity $user, LinkTarget $target ) { - return [ - 'wl_user' => $user->getId(), - 'wl_namespace' => $target->getNamespace(), - 'wl_title' => $target->getDBkey(), - ]; - } - /** * @param int $dbIndex DB_MASTER or DB_REPLICA * @@ -413,12 +405,32 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac foreach ( $rows as $namespace => $namespaceTitles ) { $rowBatches = array_chunk( $namespaceTitles, $this->updateRowsPerQuery ); foreach ( $rowBatches as $toDelete ) { - $dbw->delete( 'watchlist', [ + // First fetch the wl_ids. + $wlIds = $dbw->selectField( 'watchlist', 'wl_id', [ 'wl_user' => $user->getId(), 'wl_namespace' => $namespace, 'wl_title' => $toDelete - ], __METHOD__ ); - $affectedRows += $dbw->affectedRows(); + ] ); + + if ( $wlIds ) { + // Delete rows from both the watchlist and watchlist_expiry tables. + $dbw->delete( + 'watchlist', + [ 'wl_id' => $wlIds ], + __METHOD__ + ); + $affectedRows += $dbw->affectedRows(); + + if ( $this->expiryEnabled ) { + $dbw->delete( + 'watchlist_expiry', + [ 'we_item' => $wlIds ], + __METHOD__ + ); + $affectedRows += $dbw->affectedRows(); + } + } + if ( $ticket ) { $this->lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); } @@ -564,7 +576,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } $cached = $this->getCached( $user, $target ); - if ( $cached ) { + if ( $cached && !$cached->isExpired() ) { $this->stats->increment( 'WatchedItemStore.getWatchedItem.cached' ); return $cached; } @@ -586,22 +598,19 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac $dbr = $this->getConnectionRef( DB_REPLICA ); - $row = $dbr->selectRow( - 'watchlist', - 'wl_notificationtimestamp', - $this->dbCond( $user, $target ), - __METHOD__ + $row = $this->fetchWatchedItems( + $dbr, + $user, + [ 'wl_notificationtimestamp' ], + [], + $target ); if ( !$row ) { return false; } - $item = new WatchedItem( - $user, - $target, - $this->getLatestNotificationTimestamp( $row->wl_notificationtimestamp, $user, $target ) - ); + $item = $this->getWatchedItemFromRow( $user, $target, $row ); $this->cache( $item ); return $item; @@ -630,11 +639,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } $db = $this->getConnectionRef( $options['forWrite'] ? DB_MASTER : DB_REPLICA ); - $res = $db->select( - 'watchlist', + $res = $this->fetchWatchedItems( + $db, + $user, [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], - [ 'wl_user' => $user->getId() ], - __METHOD__, $dbOptions ); @@ -642,17 +650,86 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac foreach ( $res as $row ) { $target = new TitleValue( (int)$row->wl_namespace, $row->wl_title ); // @todo: Should we add these to the process cache? - $watchedItems[] = new WatchedItem( - $user, - $target, - $this->getLatestNotificationTimestamp( - $row->wl_notificationtimestamp, $user, $target ) - ); + $watchedItems[] = $this->getWatchedItemFromRow( $user, $target, $row ); } return $watchedItems; } + /** + * Construct a new WatchedItem given a row from watchlist/watchlist_expiry. + * @param UserIdentity $user + * @param LinkTarget $target + * @param stdClass $row + * @return WatchedItem + */ + private function getWatchedItemFromRow( + UserIdentity $user, + LinkTarget $target, + stdClass $row + ): WatchedItem { + return new WatchedItem( + $user, + $target, + $this->getLatestNotificationTimestamp( + $row->wl_notificationtimestamp, $user, $target ), + wfTimestampOrNull( TS_MW, $row->we_expiry ?? null ) + ); + } + + /** + * Fetches either a single or all watched items for the given user. + * If a $target is given, IDatabase::selectRow() is called, otherwise select(). + * If $wgWatchlistExpiry is enabled, expired items are not returned. + * + * @param IDatabase $db + * @param UserIdentity $user + * @param array $vars we_expiry is added when $wgWatchlistExpiry is enabled. + * @param array $options + * @param LinkTarget|null $target null if selecting all watched items. + * @return IResultWrapper|stdClass|false + */ + private function fetchWatchedItems( + IDatabase $db, + UserIdentity $user, + array $vars, + array $options = [], + ?LinkTarget $target = null + ) { + $dbMethod = 'select'; + $conds = [ 'wl_user' => $user->getId() ]; + + if ( $target ) { + $dbMethod = 'selectRow'; + $conds = array_merge( $conds, [ + 'wl_namespace' => $target->getNamespace(), + 'wl_title' => $target->getDBkey(), + ] ); + } + + if ( $this->expiryEnabled ) { + $vars[] = 'we_expiry'; + $conds[] = 'we_expiry IS NULL OR we_expiry > ' . $db->addQuotes( $db->timestamp() ); + + return $db->{$dbMethod}( + [ 'watchlist', 'watchlist_expiry' ], + $vars, + $conds, + __METHOD__, + $options, + [ 'watchlist_expiry' => [ 'LEFT JOIN', [ 'wl_id = we_item' ] ] ] + ); + } + + return $db->{$dbMethod}( + 'watchlist', + $vars, + $conds, + __METHOD__, + $options + ); + } + /** * @since 1.27 * @param UserIdentity $user @@ -718,21 +795,34 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac } /** - * @since 1.27 + * @since 1.27 Method added. + * @since 1.35 Accepts $expiry parameter. * @param UserIdentity $user * @param LinkTarget $target + * @param string|null $expiry Optional expiry in any format acceptable to wfTimestamp(). + * null will not create an expiry, or leave it unchanged should one already exist. */ - public function addWatch( UserIdentity $user, LinkTarget $target ) { - $this->addWatchBatchForUser( $user, [ $target ] ); + public function addWatch( UserIdentity $user, LinkTarget $target, ?string $expiry = null ) { + $this->addWatchBatchForUser( $user, [ $target ], $expiry ); } /** - * @since 1.27 + * Add multiple items to the user's watchlist. + * If adding a single item, use self::addWatch() where you can optionally provide an expiry. + * + * @since 1.27 Method added. + * @since 1.35 Accepts $expiry parameter. * @param UserIdentity $user * @param LinkTarget[] $targets - * @return bool + * @param string|null $expiry Optional expiry in a format acceptable to wfTimestamp(), + * null will not create expiries, or leave them unchanged should they already exist. + * @return bool Whether database transactions were performed. */ - public function addWatchBatchForUser( UserIdentity $user, array $targets ) { + public function addWatchBatchForUser( + UserIdentity $user, + array $targets, + ?string $expiry = null + ) { if ( $this->readOnlyMode->isReadOnly() ) { return false; } @@ -757,7 +847,8 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac $items[] = new WatchedItem( $user, $target, - null + null, + $expiry ); $this->uncache( $user, $target ); } @@ -772,6 +863,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac // if there's already an entry for this page $dbw->insert( 'watchlist', $toInsert, __METHOD__, [ 'IGNORE' ] ); $affectedRows += $dbw->affectedRows(); + + if ( $this->expiryEnabled ) { + $affectedRows += $this->updateOrDeleteExpiries( $dbw, $user->getId(), $toInsert, $expiry ); + } + if ( $ticket ) { $this->lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); } @@ -786,6 +882,100 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return (bool)$affectedRows; } + /** + * Insert/update expiries, or delete them if the expiry is 'infinity'. + * + * @param IDatabase $dbw + * @param int $userId + * @param array $rows + * @param string|null $expiry + * @return int Number of affected rows. + */ + private function updateOrDeleteExpiries( + IDatabase $dbw, + int $userId, + array $rows, + ?string $expiry = null + ): int { + if ( null === $expiry ) { + // null expiry means do nothing, 0 rows affected. + return 0; + } + + // Build the giant `(...) OR (...)` part to be used with WHERE. + $conds = []; + foreach ( $rows as $row ) { + $conds[] = $dbw->makeList( + [ + 'wl_user' => $userId, + 'wl_namespace' => $row['wl_namespace'], + 'wl_title' => $row['wl_title'] + ], + $dbw::LIST_AND + ); + } + $cond = $dbw->makeList( $conds, $dbw::LIST_OR ); + + if ( wfIsInfinity( $expiry ) ) { + // Rows should be deleted rather than updated. + $dbw->deleteJoin( + 'watchlist_expiry', + 'watchlist', + 'we_item', + 'wl_id', + [ $cond ], + __METHOD__ + ); + + return $dbw->affectedRows(); + } + + // TODO: Refactor with Special:Userrights::expiryToTimestamp() ? + // Difference here is we need to accept null as being 'no changes'. + // Validation of the expiry should probably happen before this method is called, too. + $unix = strtotime( $expiry ); + if ( !$unix || $unix === -1 ) { + // Invalid expiry, 0 rows effected. + return 0; + } + $expiry = wfTimestamp( TS_MW, $expiry ); + + return $this->updateExpiries( $dbw, $expiry, $cond ); + } + + /** + * Update the expiries for items found with the given $cond. + * @param IDatabase $dbw + * @param string $expiry + * @param string $cond + * @return int Number of affected rows. + */ + private function updateExpiries( IDatabase $dbw, string $expiry, string $cond ): int { + // First fetch the wl_ids from the watchlist table. + // We'd prefer to do a INSERT/SELECT in the same query with IDatabase::insertSelect(), + // but it doesn't allow us to use the "ON DUPLICATE KEY UPDATE" clause. + $wlIds = (array)$dbw->selectFieldValues( 'watchlist', 'wl_id', $cond, __METHOD__ ); + + $expiry = $dbw->timestamp( $expiry ); + + $weRows = array_map( function ( $wlId ) use ( $expiry, $dbw ) { + return [ + 'we_item' => $wlId, + 'we_expiry' => $expiry + ]; + }, $wlIds ); + + // Insert into watchlist_expiry, updating the expiry for duplicate rows. + $dbw->upsert( + 'watchlist_expiry', + $weRows, + 'we_item', + [ 'we_expiry' => $expiry ] + ); + + return $dbw->affectedRows(); + } + /** * @since 1.27 * @param UserIdentity $user diff --git a/includes/watcheditem/WatchedItemStoreInterface.php b/includes/watcheditem/WatchedItemStoreInterface.php index 20244c39e75..551088c4884 100644 --- a/includes/watcheditem/WatchedItemStoreInterface.php +++ b/includes/watcheditem/WatchedItemStoreInterface.php @@ -177,22 +177,28 @@ interface WatchedItemStoreInterface { /** * Must be called separately for Subject & Talk namespaces * - * @since 1.31 + * @since 1.31 Method added. + * @since 1.35 Accepts $expiry parameter. * * @param UserIdentity $user * @param LinkTarget $target + * @param string|null $expiry Optional expiry timestamp in any format acceptable to wfTimestamp(). + * null will not create an expiry, or leave it unchanged should one already exist. */ - public function addWatch( UserIdentity $user, LinkTarget $target ); + public function addWatch( UserIdentity $user, LinkTarget $target, ?string $expiry = null ); /** - * @since 1.31 + * @since 1.31 Method added. + * @since 1.35 Accepts $expiry parameter. * * @param UserIdentity $user * @param LinkTarget[] $targets + * @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. * * @return bool success */ - public function addWatchBatchForUser( UserIdentity $user, array $targets ); + public function addWatchBatchForUser( UserIdentity $user, array $targets, ?string $expiry = null ); /** * Removes an entry for the UserIdentity watching the LinkTarget diff --git a/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php b/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php index 4bd54e6f60b..fb83629450a 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php @@ -16,6 +16,10 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase { parent::setUp(); self::$users['WatchedItemStoreIntegrationTestUser'] = new TestUser( 'WatchedItemStoreIntegrationTestUser' ); + + $this->setMwGlobals( [ + 'wgWatchlistExpiry' => true, + ] ); } private function getUser() { @@ -107,6 +111,76 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase { ); } + public function testWatchAndUnWatchItemWithExpiry(): void { + $user = $this->getUser(); + $title = Title::newFromText( 'WatchedItemStoreIntegrationTestPage' ); + $store = MediaWikiServices::getInstance()->getWatchedItemStore(); + + $store->addWatch( $user, $title, '20300101000000' ); + $this->assertSame( + '20300101000000', + $store->loadWatchedItem( $user, $title )->getExpiry() + ); + + // Invalid expiry, nothing should change. + $store->addWatch( $user, $title, 'invalid expiry' ); + $this->assertSame( + '20300101000000', + $store->loadWatchedItem( $user, $title )->getExpiry() + ); + + // Changed to infinity, so expiry row should be removed. + $store->addWatch( $user, $title, 'infinity' ); + $this->assertNull( + $store->loadWatchedItem( $user, $title )->getExpiry() + ); + + // Updating to a valid expiry. + $store->addWatch( $user, $title, '20500101000000' ); + $this->assertSame( + '20500101000000', + $store->loadWatchedItem( $user, $title )->getExpiry() + ); + + // Expiry in the past, should not be considered watched. + $store->addWatch( $user, $title, '20090101000000' ); + + // Test isWatch(), which would normally pull from the cache. In this case + // the cache should bust and return false since the item has expired. + $this->assertFalse( + $store->isWatched( $user, $title ) + ); + } + + public function testWatchAndUnwatchMultipleWithExpiry(): void { + $user = $this->getUser(); + $title1 = Title::newFromText( 'WatchedItemStoreIntegrationTestPage1' ); + $title2 = Title::newFromText( 'WatchedItemStoreIntegrationTestPage1' ); + $store = MediaWikiServices::getInstance()->getWatchedItemStore(); + + $timestamp = '20500101000000'; + $store->addWatchBatchForUser( $user, [ $title1, $title2 ], $timestamp ); + + $this->assertSame( + $timestamp, + $store->loadWatchedItem( $user, $title1 )->getExpiry() + ); + $this->assertSame( + $timestamp, + $store->loadWatchedItem( $user, $title2 )->getExpiry() + ); + + // Clear expiries. + $store->addWatchBatchForUser( $user, [ $title1, $title2 ], 'infinity' ); + + $this->assertNull( + $store->loadWatchedItem( $user, $title1 )->getExpiry() + ); + $this->assertNull( + $store->loadWatchedItem( $user, $title2 )->getExpiry() + ); + } + public function testWatchBatchAndClearItems() { $user = $this->getUser(); $title1 = Title::newFromText( 'WatchedItemStoreIntegrationTestPage1' ); diff --git a/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php index ef0c3a90a8a..7bfa775ee30 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php @@ -5,6 +5,7 @@ use MediaWiki\Revision\RevisionLookup; use MediaWiki\Revision\RevisionRecord; use MediaWiki\User\UserIdentityValue; use PHPUnit\Framework\MockObject\MockObject; +use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\LBFactory; use Wikimedia\Rdbms\LoadBalancer; use Wikimedia\TestingAccessWrapper; @@ -169,6 +170,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { * * readOnlyMode * * nsInfo * * revisionLookup + * * expiryEnabled + * @return WatchedItemStore */ private function newWatchedItemStore( array $mocks = [] ) : WatchedItemStore { return new WatchedItemStore( @@ -180,7 +183,8 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $mocks['readOnlyMode'] ?? $this->getMockReadOnlyMode(), 1000, $mocks['nsInfo'] ?? $this->getMockNsInfo(), - $mocks['revisionLookup'] ?? $this->getMockRevisionLookup() + $mocks['revisionLookup'] ?? $this->getMockRevisionLookup(), + $mocks['expiryEnabled'] ?? true ); } @@ -1146,19 +1150,26 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testLoadWatchedItem_existingItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( - $this->getFakeRow( [ 'wl_notificationtimestamp' => '20151212010101' ] ) + $this->getFakeRow( [ + 'wl_notificationtimestamp' => '20151212010101', + 'we_expiry' => '20300101000000' + ] ) ) ); $mockCache = $this->getMockCache(); @@ -1177,20 +1188,25 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertInstanceOf( WatchedItem::class, $watchedItem ); $this->assertEquals( 1, $watchedItem->getUser()->getId() ); $this->assertEquals( 'SomeDbKey', $watchedItem->getLinkTarget()->getDBkey() ); + $this->assertSame( '20300101000000', $watchedItem->getExpiry() ); $this->assertSame( 0, $watchedItem->getLinkTarget()->getNamespace() ); } public function testLoadWatchedItem_noItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( [] ) ); @@ -1231,26 +1247,21 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testRemoveWatch_existingItem() { $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) + ->method( 'selectField' ) + ->willReturn( [ 1, 2 ] ); + $mockDb->expects( $this->exactly( 2 ) ) ->method( 'delete' ) ->withConsecutive( [ 'watchlist', - [ - 'wl_user' => 1, - 'wl_namespace' => 0, - 'wl_title' => [ 'SomeDbKey' ], - ], + [ 'wl_id' => [ 1, 2 ] ] ], [ - 'watchlist', - [ - 'wl_user' => 1, - 'wl_namespace' => 1, - 'wl_title' => [ 'SomeDbKey' ], - ] + 'watchlist_expiry', + [ 'we_item' => [ 1, 2 ] ] ] ); - $mockDb->expects( $this->exactly( 1 ) ) + $mockDb->expects( $this->exactly( 2 ) ) ->method( 'affectedRows' ) ->willReturn( 2 ); @@ -1276,29 +1287,12 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testRemoveWatch_noItem() { $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) - ->method( 'delete' ) - ->withConsecutive( - [ - 'watchlist', - [ - 'wl_user' => 1, - 'wl_namespace' => 0, - 'wl_title' => [ 'SomeDbKey' ], - ] - ], - [ - 'watchlist', - [ - 'wl_user' => 1, - 'wl_namespace' => 1, - 'wl_title' => [ 'SomeDbKey' ], - ] - ] - ); - - $mockDb->expects( $this->once() ) - ->method( 'affectedRows' ) - ->willReturn( 0 ); + ->method( 'selectField' ) + ->willReturn( null ); + $mockDb->expects( $this->never() ) + ->method( 'delete' ); + $mockDb->expects( $this->never() ) + ->method( 'affectedRows' ); $mockCache = $this->getMockCache(); $mockCache->expects( $this->never() )->method( 'get' ); @@ -1341,19 +1335,26 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testGetWatchedItem_existingItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( - $this->getFakeRow( [ 'wl_notificationtimestamp' => '20151212010101' ] ) + $this->getFakeRow( [ + 'wl_notificationtimestamp' => '20151212010101', + 'we_expiry' => '20300101000000' + ] ) ) ); $mockCache = $this->getMockCache(); @@ -1379,6 +1380,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertInstanceOf( WatchedItem::class, $watchedItem ); $this->assertEquals( 1, $watchedItem->getUser()->getId() ); $this->assertEquals( 'SomeDbKey', $watchedItem->getLinkTarget()->getDBkey() ); + $this->assertSame( '20300101000000', $watchedItem->getExpiry() ); $this->assertSame( 0, $watchedItem->getLinkTarget()->getNamespace() ); } @@ -1414,15 +1416,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testGetWatchedItem_noItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( [] ) ); @@ -1467,18 +1473,22 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testGetWatchedItemsForUser() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'select' ) ->with( - 'watchlist', - [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], - [ 'wl_user' => 1 ] + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp', 'we_expiry' ], + [ 'wl_user' => 1, 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( [ $this->getFakeRow( [ 'wl_namespace' => 0, 'wl_title' => 'Foo1', 'wl_notificationtimestamp' => '20151212010101', + 'we_expiry' => '20300101000000' ] ), $this->getFakeRow( [ 'wl_namespace' => 1, @@ -1503,7 +1513,12 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->assertInstanceOf( WatchedItem::class, $watchedItem ); } $this->assertEquals( - new WatchedItem( $user, new TitleValue( 0, 'Foo1' ), '20151212010101' ), + new WatchedItem( + $user, + new TitleValue( 0, 'Foo1' ), + '20151212010101', + '20300101000000' + ), $watchedItems[0] ); $this->assertEquals( @@ -1528,12 +1543,15 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $mockLoadBalancer = $this->getMockLBFactory( $mockDb, $dbType ); $user = new UserIdentityValue( 1, 'MockUser', 0 ); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'select' ) ->with( - 'watchlist', - [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], - [ 'wl_user' => 1 ], + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp', 'we_expiry' ], + [ 'wl_user' => 1, 'we_expiry IS NULL OR we_expiry > 20200101000000' ], $this->isType( 'string' ), [ 'ORDER BY' => [ 'wl_namespace ASC', 'wl_title ASC' ] ] ) @@ -1561,15 +1579,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testIsWatchedItem_existingItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( @@ -1600,15 +1622,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testIsWatchedItem_noItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( [] ) ); @@ -1903,15 +1929,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { public function testResetNotificationTimestamp_noItem() { $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( [] ) ); @@ -1936,15 +1966,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $title = new TitleValue( 0, 'SomeDbKey' ); $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( @@ -2141,15 +2175,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $mockNextRevision = $this->createNoOpMock( RevisionRecord::class ); $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( @@ -2226,15 +2264,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $title = new TitleValue( 0, 'SomeDbKey' ); $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( false ) ); @@ -2311,15 +2353,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $title = new TitleValue( 0, 'SomeDbKey' ); $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( @@ -2400,15 +2446,19 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $title = new TitleValue( 0, 'SomeDbKey' ); $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'addQuotes' ) + ->willReturn( '20200101000000' ); $mockDb->expects( $this->once() ) ->method( 'selectRow' ) ->with( - 'watchlist', - 'wl_notificationtimestamp', + [ 'watchlist', 'watchlist_expiry' ], + [ 'wl_notificationtimestamp', 'we_expiry' ], [ 'wl_user' => 1, 'wl_namespace' => 0, 'wl_title' => 'SomeDbKey', + 'we_expiry IS NULL OR we_expiry > 20200101000000' ] ) ->will( $this->returnValue( diff --git a/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php new file mode 100644 index 00000000000..0c5f7d53382 --- /dev/null +++ b/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php @@ -0,0 +1,23 @@ +assertFalse( $notExpired1->isExpired() ); + + $notExpired2 = new WatchedItem( $user, $target, null ); + $this->assertFalse( $notExpired2->isExpired() ); + + $expired = new WatchedItem( $user, $target, null, '20010101000000' ); + $this->assertTrue( $expired->isExpired() ); + } +}