From 8c5406a437aab8751801a24dd2fe8e4473466b32 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 7 Apr 2015 13:50:00 -0700
Subject: [PATCH] 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
---
 includes/User.php                   | 107 +++++++++++++++++++++-------
 tests/phpunit/includes/UserTest.php |  20 ++++++
 2 files changed, 103 insertions(+), 24 deletions(-)

diff --git a/includes/User.php b/includes/User.php
index 4ada6b1fff..6218ce2eb9 100644
--- 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 b74a7eadc0..370b5b2c5b 100644
--- 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" );
+	}
 }
-- 
2.20.1