Added CAS logic to User::addAutopromoteOnceGroups

* This should avoid duplicate logging events on races or when
  the cache fails to update.
* Also added getDBTouched() method to get user_touched itself.

Bug: T48834
Change-Id: Ib2cd0a2c72629fa4e13dcff4d2d6fbac8e690b32

diff --git a/includes/User.php b/includes/User.php
index 4ada6b1..6218ce2 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -1422,37 +1422,85 @@ class User implements IDBAccessObject {
        public function addAutopromoteOnceGroups( $event ) {
                global $wgAutopromoteOnceLogInRC, $wgAuth;
 
-               $toPromote = array();
-               if ( !wfReadOnly() && $this->getId() ) {
-                       $toPromote = Autopromote::getAutopromoteOnceGroups( $this, $event );
-                       if ( count( $toPromote ) ) {
-                               $oldGroups = $this->getGroups(); // previous groups
-
-                               foreach ( $toPromote as $group ) {
-                                       $this->addGroup( $group );
-                               }
-                               // update groups in external authentication database
-                               $wgAuth->updateExternalDBGroups( $this, $toPromote );
+               if ( wfReadOnly() || !$this->getId() ) {
+                       return array();
+               }
 
-                               $newGroups = array_merge( $oldGroups, $toPromote ); // all groups
+               $toPromote = Autopromote::getAutopromoteOnceGroups( $this, $event );
+               if ( !count( $toPromote ) ) {
+                       return array();
+               }
 
-                               $logEntry = new ManualLogEntry( 'rights', 'autopromote' );
-                               $logEntry->setPerformer( $this );
-                               $logEntry->setTarget( $this->getUserPage() );
-                               $logEntry->setParameters( array(
-                                       '4::oldgroups' => $oldGroups,
-                                       '5::newgroups' => $newGroups,
-                               ) );
-                               $logid = $logEntry->insert();
-                               if ( $wgAutopromoteOnceLogInRC ) {
-                                       $logEntry->publish( $logid );
-                               }
-                       }
+               if ( !$this->checkAndSetTouched() ) {
+                       return array(); // raced out (bug T48834)
+               }
+
+               $oldGroups = $this->getGroups(); // previous groups
+               foreach ( $toPromote as $group ) {
+                       $this->addGroup( $group );
+               }
+
+               // update groups in external authentication database
+               $wgAuth->updateExternalDBGroups( $this, $toPromote );
+
+               $newGroups = array_merge( $oldGroups, $toPromote ); // all groups
+
+               $logEntry = new ManualLogEntry( 'rights', 'autopromote' );
+               $logEntry->setPerformer( $this );
+               $logEntry->setTarget( $this->getUserPage() );
+               $logEntry->setParameters( array(
+                       '4::oldgroups' => $oldGroups,
+                       '5::newgroups' => $newGroups,
+               ) );
+               $logid = $logEntry->insert();
+               if ( $wgAutopromoteOnceLogInRC ) {
+                       $logEntry->publish( $logid );
                }
 
                return $toPromote;
        }
 
+       /**
+        * Bump user_touched if it didn't change since this object was loaded
+        *
+        * On success, the mTouched field is updated.
+        * The user serialization cache is always cleared.
+        *
+        * @return bool Whether user_touched was actually updated
+        * @since 1.26
+        */
+       protected function checkAndSetTouched() {
+               $this->load();
+
+               if ( !$this->mId ) {
+                       return false; // anon
+               }
+
+               // Get a new user_touched that is higher than the old one
+               $oldTouched = $this->mTouched;
+               $newTouched = $this->newTouchedTimestamp();
+
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->update( 'user',
+                       array( 'user_touched' => $dbw->timestamp( $newTouched ) ),
+                       array(
+                               'user_id' => $this->mId,
+                               'user_touched' => $dbw->timestamp( $oldTouched ) // CAS check
+                       ),
+                       __METHOD__
+               );
+               $success = ( $dbw->affectedRows() > 0 );
+
+               if ( $success ) {
+                       $this->mTouched = $newTouched;
+               }
+
+               // Clears on failure too since that is desired if the cache is stale
+               $this->clearSharedCache();
+
+               return $success;
+       }
+
        /**
         * Clear various cached data stored in this object. The cache of the user table
         * data (i.e. self::$mCacheVars) is not cleared unless $reloadFrom is given.
@@ -2355,6 +2403,17 @@ class User implements IDBAccessObject {
                return $this->mTouched;
        }
 
+       /**
+        * Get the user_touched timestamp field (time of last DB updates)
+        * @return string TS_MW Timestamp
+        * @since 1.26
+        */
+       protected function getDBTouched() {
+               $this->load();
+
+               return $this->mTouched;
+       }
+
        /**
         * @return Password
         * @since 1.24

diff --git a/tests/phpunit/includes/UserTest.php b/tests/phpunit/includes/UserTest.php
index b74a7ea..370b5b2 100644 (file)
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -425,4 +425,24 @@ class UserTest extends MediaWikiTestCase {
                $this->assertFalse( $user->isLoggedIn() );
                $this->assertTrue( $user->isAnon() );
        }
+
+       /**
+        * @covers User::checkAndSetTouched
+        */
+       public function testCheckAndSetTouched() {
+               $user = TestingAccessWrapper::newFromObject( User::newFromName( 'UTSysop' ) );
+               $this->assertTrue( $user->isLoggedIn() );
+
+               $touched = $user->getDBTouched();
+               $this->assertTrue(
+                       $user->checkAndSetTouched(), "checkAndSetTouched() succeded" );
+               $this->assertGreaterThan(
+                       $touched, $user->getDBTouched(), "user_touched increased with casOnTouched()" );
+
+               $touched = $user->getDBTouched();
+               $this->assertTrue(
+                       $user->checkAndSetTouched(), "checkAndSetTouched() succeded #2" );
+               $this->assertGreaterThan(
+                       $touched, $user->getDBTouched(), "user_touched increased with casOnTouched() #2" );
+       }
 }