Move most User::clearAllNotifications() logic to WatchedItemStore

Change-Id: Ib1b0c40e408f6fad6fc8257c5073fa1c3c264c3a

diff --git a/autoload.php b/autoload.php
index eb3d776..0b0c288 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -271,6 +271,7 @@ $wgAutoloadLocalClasses = [
        'CleanupUsersWithNoId' => __DIR__ . '/maintenance/cleanupUsersWithNoId.php',
        'ClearInterwikiCache' => __DIR__ . '/maintenance/clearInterwikiCache.php',
        'ClearUserWatchlistJob' => __DIR__ . '/includes/jobqueue/jobs/ClearUserWatchlistJob.php',
        'CleanupUsersWithNoId' => __DIR__ . '/maintenance/cleanupUsersWithNoId.php',
        'ClearInterwikiCache' => __DIR__ . '/maintenance/clearInterwikiCache.php',
        'ClearUserWatchlistJob' => __DIR__ . '/includes/jobqueue/jobs/ClearUserWatchlistJob.php',

+       'ClearWatchlistNotificationsJob' => __DIR__ . '/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php',

        'CliInstaller' => __DIR__ . '/includes/installer/CliInstaller.php',
        'CloneDatabase' => __DIR__ . '/includes/db/CloneDatabase.php',
        'CodeCleanerGlobalsPass' => __DIR__ . '/maintenance/CodeCleanerGlobalsPass.inc',
        'CliInstaller' => __DIR__ . '/includes/installer/CliInstaller.php',
        'CloneDatabase' => __DIR__ . '/includes/db/CloneDatabase.php',
        'CodeCleanerGlobalsPass' => __DIR__ . '/maintenance/CodeCleanerGlobalsPass.inc',

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 538c1b2..f473b3e 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -7454,8 +7454,9 @@ $wgJobClasses = [
        'categoryMembershipChange' => CategoryMembershipChangeJob::class,
        'clearUserWatchlist' => ClearUserWatchlistJob::class,
        'cdnPurge' => CdnPurgeJob::class,
        'categoryMembershipChange' => CategoryMembershipChangeJob::class,
        'clearUserWatchlist' => ClearUserWatchlistJob::class,
        'cdnPurge' => CdnPurgeJob::class,

-       'enqueue' => EnqueueJob::class, // local queue for multi-DC setups

        'userGroupExpiry' => UserGroupExpiryJob::class,
        'userGroupExpiry' => UserGroupExpiryJob::class,

+       'clearWatchlistNotifications' => ClearWatchlistNotificationsJob::class,
+       'enqueue' => EnqueueJob::class, // local queue for multi-DC setups

        'null' => NullJob::class,
 ];
 
        'null' => NullJob::class,
 ];
 

diff --git a/includes/jobqueue/jobs/ActivityUpdateJob.php b/includes/jobqueue/jobs/ActivityUpdateJob.php
index da4ec23..8cc14e5 100644 (file)
--- a/includes/jobqueue/jobs/ActivityUpdateJob.php
+++ b/includes/jobqueue/jobs/ActivityUpdateJob.php
@@ -22,6 +22,12 @@
 /**
  * Job for updating user activity like "last viewed" timestamps
  *
 /**
  * Job for updating user activity like "last viewed" timestamps
  *

+ * Job parameters include:
+ *   - type: one of (updateWatchlistNotification) [required]
+ *   - userid: affected user ID [required]
+ *   - notifTime: timestamp to set watchlist entries to [required]
+ *   - curTime: UNIX timestamp of the event that triggered this job [required]
+ *

  * @ingroup JobQueue
  * @since 1.26
  */
  * @ingroup JobQueue
  * @since 1.26
  */


@@ -29,8 +35,10 @@ class ActivityUpdateJob extends Job {
        function __construct( Title $title, array $params ) {
                parent::__construct( 'activityUpdateJob', $title, $params );
 
        function __construct( Title $title, array $params ) {
                parent::__construct( 'activityUpdateJob', $title, $params );
 

-               if ( !isset( $params['type'] ) ) {
-                       throw new InvalidArgumentException( "Missing 'type' parameter." );
+               static $required = [ 'type', 'userid', 'notifTime', 'curTime' ];
+               $missing = implode( ', ', array_diff( $required, array_keys( $this->params ) ) );
+               if ( $missing != '' ) {
+                       throw new InvalidArgumentException( "Missing paramter(s) $missing" );

                }
 
                $this->removeDuplicates = true;
                }
 
                $this->removeDuplicates = true;


@@ -40,8 +48,7 @@ class ActivityUpdateJob extends Job {
                if ( $this->params['type'] === 'updateWatchlistNotification' ) {
                        $this->updateWatchlistNotification();
                } else {
                if ( $this->params['type'] === 'updateWatchlistNotification' ) {
                        $this->updateWatchlistNotification();
                } else {

-                       throw new InvalidArgumentException(
-                               "Invalid 'type' parameter '{$this->params['type']}'." );
+                       throw new InvalidArgumentException( "Invalid 'type' '{$this->params['type']}'." );

                }
 
                return true;
                }
 
                return true;

diff --git a/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php b/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
new file mode 100644 (file)
index 0000000..94c1351
--- /dev/null
+++ b/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
@@ -0,0 +1,79 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup JobQueue
+ */
+
+use MediaWiki\MediaWikiServices;
+
+/**
+ * Job for clearing all of the "last viewed" timestamps for a user's watchlist
+ *
+ * Job parameters include:
+ *   - userId: affected user ID [required]
+ *   - casTime: UNIX timestamp of the event that triggered this job [required]
+ *
+ * @ingroup JobQueue
+ * @since 1.31
+ */
+class ClearWatchlistNotificationsJob extends Job {
+       function __construct( Title $title, array $params ) {
+               parent::__construct( 'clearWatchlistNotifications', $title, $params );
+
+               static $required = [ 'userId', 'casTime' ];
+               $missing = implode( ', ', array_diff( $required, array_keys( $this->params ) ) );
+               if ( $missing != '' ) {
+                       throw new InvalidArgumentException( "Missing paramter(s) $missing" );
+               }
+
+               $this->removeDuplicates = true;
+       }
+
+       public function run() {
+               $services = MediaWikiServices::getInstance();
+               $lbFactory = $services->getDBLoadBalancerFactory();
+               $rowsPerQuery = $services->getMainConfig()->get( 'UpdateRowsPerQuery' );
+
+               $dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
+               $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
+
+               $asOfTimes = array_unique( $dbw->selectFieldValues(
+                       'watchlist',
+                       'wl_notificationtimestamp',
+                       [ 'wl_user' => $this->params['userId'], 'wl_notificationtimestamp IS NOT NULL' ],
+                       __METHOD__,
+                       [ 'ORDER BY' => 'wl_notificationtimestamp DESC' ]
+               ) );
+
+               foreach ( array_chunk( $asOfTimes, $rowsPerQuery ) as $asOfTimeBatch ) {
+                       $dbw->update(
+                               'watchlist',
+                               [ 'wl_notificationtimestamp' => null ],
+                               [
+                                       'wl_user' => $this->params['userId'],
+                                       'wl_notificationtimestamp' => $asOfTimeBatch,
+                                       // New notifications since the reset should not be cleared
+                                       'wl_notificationtimestamp < ' .
+                                               $dbw->addQuotes( $dbw->timestamp( $this->params['casTime'] ) )
+                               ],
+                               __METHOD__
+                       );
+                       $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket );
+               }
+       }
+}

diff --git a/includes/user/User.php b/includes/user/User.php
index fda49a5..9777148 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -3969,51 +3969,9 @@ class User implements IDBAccessObject, UserIdentity {
                        return;
                }
 
                        return;
                }
 

-               $dbw = wfGetDB( DB_MASTER );
-               $asOfTimes = array_unique( $dbw->selectFieldValues(
-                       'watchlist',
-                       'wl_notificationtimestamp',
-                       [ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ],
-                       __METHOD__,
-                       [ 'ORDER BY' => 'wl_notificationtimestamp DESC', 'LIMIT' => 500 ]
-               ) );
-               if ( !$asOfTimes ) {
-                       return;
-               }
-               // Immediately update the most recent touched rows, which hopefully covers what
-               // the user sees on the watchlist page before pressing "mark all pages visited"....
-               $dbw->update(
-                       'watchlist',
-                       [ 'wl_notificationtimestamp' => null ],
-                       [ 'wl_user' => $id, 'wl_notificationtimestamp' => $asOfTimes ],
-                       __METHOD__
-               );
-               // ...and finish the older ones in a post-send update with lag checks...
-               DeferredUpdates::addUpdate( new AutoCommitUpdate(
-                       $dbw,
-                       __METHOD__,
-                       function () use ( $dbw, $id ) {
-                               global $wgUpdateRowsPerQuery;
-
-                               $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
-                               $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
-                               $asOfTimes = array_unique( $dbw->selectFieldValues(
-                                       'watchlist',
-                                       'wl_notificationtimestamp',
-                                       [ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ],
-                                       __METHOD__
-                               ) );
-                               foreach ( array_chunk( $asOfTimes, $wgUpdateRowsPerQuery ) as $asOfTimeBatch ) {
-                                       $dbw->update(
-                                               'watchlist',
-                                               [ 'wl_notificationtimestamp' => null ],
-                                               [ 'wl_user' => $id, 'wl_notificationtimestamp' => $asOfTimeBatch ],
-                                               __METHOD__
-                                       );
-                                       $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket );
-                               }
-                       }
-               ) );
+               $watchedItemStore = MediaWikiServices::getInstance()->getWatchedItemStore();
+               $watchedItemStore->resetAllNotificationTimestampsForUser( $this );
+

                // We also need to clear here the "you have new message" notification for the own
                // user_talk page; it's cleared one page view later in WikiPage::doViewUpdates().
        }
                // We also need to clear here the "you have new message" notification for the own
                // user_talk page; it's cleared one page view later in WikiPage::doViewUpdates().
        }

diff --git a/includes/watcheditem/NoWriteWatchedItemStore.php b/includes/watcheditem/NoWriteWatchedItemStore.php
index 1a0f504..86e7be8 100644 (file)
--- a/includes/watcheditem/NoWriteWatchedItemStore.php
+++ b/includes/watcheditem/NoWriteWatchedItemStore.php
@@ -122,6 +122,10 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface {
                throw new DBReadOnlyError( null, 'The watchlist is currently readonly.' );
        }
 
                throw new DBReadOnlyError( null, 'The watchlist is currently readonly.' );
        }
 

+       public function resetAllNotificationTimestampsForUser( User $user ) {
+               throw new DBReadOnlyError( null, 'The watchlist is currently readonly.' );
+       }
+

        public function resetNotificationTimestamp(
                User $user,
                Title $title,
        public function resetNotificationTimestamp(
                User $user,
                Title $title,

diff --git a/includes/watcheditem/WatchedItemStore.php b/includes/watcheditem/WatchedItemStore.php
index 1b37968..6e907de 100644 (file)
--- a/includes/watcheditem/WatchedItemStore.php
+++ b/includes/watcheditem/WatchedItemStore.php
@@ -504,7 +504,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
         * @since 1.27
         * @param User $user
         * @param LinkTarget $target
         * @since 1.27
         * @param User $user
         * @param LinkTarget $target

-        * @return bool
+        * @return WatchedItem|bool

         */
        public function loadWatchedItem( User $user, LinkTarget $target ) {
                // Only loggedin user can have a watchlist
         */
        public function loadWatchedItem( User $user, LinkTarget $target ) {
                // Only loggedin user can have a watchlist


@@ -765,12 +765,34 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
                return $success;
        }
 
                return $success;
        }
 

+       public function resetAllNotificationTimestampsForUser( User $user ) {
+               // Only loggedin user can have a watchlist
+               if ( $user->isAnon() ) {
+                       return;
+               }
+
+               // If the page is watched by the user (or may be watched), update the timestamp
+               $job = new ClearWatchlistNotificationsJob(
+                       $user->getUserPage(),
+                       [ 'userId'  => $user->getId(), 'casTime' => time() ]
+               );
+
+               // Try to run this post-send
+               // Calls DeferredUpdates::addCallableUpdate in normal operation
+               call_user_func(
+                       $this->deferredUpdatesAddCallableUpdateCallback,
+                       function () use ( $job ) {
+                               $job->run();
+                       }
+               );
+       }
+

        /**
         * @since 1.27
         * @param User $editor
         * @param LinkTarget $target
         * @param string|int $timestamp
        /**
         * @since 1.27
         * @param User $editor
         * @param LinkTarget $target
         * @param string|int $timestamp

-        * @return int
+        * @return int[]

         */
        public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) {
                $dbw = $this->getConnectionRef( DB_MASTER );
         */
        public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) {
                $dbw = $this->getConnectionRef( DB_MASTER );

diff --git a/includes/watcheditem/WatchedItemStoreInterface.php b/includes/watcheditem/WatchedItemStoreInterface.php
index 133f480..a450ae5 100644 (file)
--- a/includes/watcheditem/WatchedItemStoreInterface.php
+++ b/includes/watcheditem/WatchedItemStoreInterface.php
@@ -209,7 +209,7 @@ interface WatchedItemStoreInterface {
        /**
         * @since 1.31
         *
        /**
         * @since 1.31
         *

-        * @param User $user The user to set the timestamp for
+        * @param User $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
         *
         * @param string|null $timestamp Set the update timestamp to this value
         * @param LinkTarget[] $targets List of targets to update. Default to all targets
         *


@@ -221,6 +221,15 @@ interface WatchedItemStoreInterface {
                array $targets = []
        );
 
                array $targets = []
        );
 

+       /**
+        * Reset all watchlist notificaton timestamps for a user using the job queue
+        *
+        * @since 1.31
+        *
+        * @param User $user The user to reset the timestamps for
+        */
+       public function resetAllNotificationTimestampsForUser( User $user );
+

        /**
         * @since 1.31
         *
        /**
         * @since 1.31
         *


@@ -246,7 +255,7 @@ interface WatchedItemStoreInterface {
         * @param int $oldid The revision id being viewed. If not given or 0, latest revision is
         *     assumed.
         *
         * @param int $oldid The revision id being viewed. If not given or 0, latest revision is
         *     assumed.
         *

-        * @return bool success
+        * @return bool success Whether a job was enqueued

         */
        public function resetNotificationTimestamp( User $user, Title $title, $force = '', $oldid = 0 );
 
         */
        public function resetNotificationTimestamp( User $user, Title $title, $force = '', $oldid = 0 );