RecentChange: Avoid duplicate patrol log entry after patrol conflict

Check the affected rows count to detect a patrol conflict, which is
when the database query succeeds, but it was a no-op because another
request (possibly from the same user, or someone else) marked it as
patrolled already.

We detect it using the `rc_patrolled` condition because without it,
affectedRows() would always return 1, as the previous update condition
could match either the patrolled or patrolled version of the row.

Bug: T196182
Change-Id: I2aa4f2f752a40657883e0a6714dbb175c9d2fdcb

1 files changed, 23 insertions, 5 deletions

diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php
index 05d172208778..81a22d0cdcce 100644
--- a/includes/changes/RecentChange.php
+++ b/includes/changes/RecentChange.php
@@ -631,8 +631,16 @@ class RecentChange implements Taggable {
 		if ( $this->getAttribute( 'rc_patrolled' ) ) {
 			return [];
 		}
-		// Actually set the 'patrolled' flag in RC
-		$this->reallyMarkPatrolled();
+		// Attempt to set the 'patrolled' flag in RC database
+		$affectedRowCount = $this->reallyMarkPatrolled();
+
+		if ( $affectedRowCount === 0 ) {
+			// Query succeeded but no rows change, e.g. another request
+			// patrolled the same change just before us.
+			// Avoid duplicate log entry (T196182).
+			return [];
+		}
+
 		// Log this patrol event
 		PatrolLog::record( $this, false, $performer->getUser(), $tags );
 
@@ -644,15 +652,25 @@ class RecentChange implements Taggable {
 
 	/**
 	 * Mark this RecentChange patrolled, without error checking
-	 * @return int Number of affected rows
+	 *
+	 * @return int Number of database rows changed, usually 1, but 0 if
+	 * another request already patrolled it in the mean time.
 	 */
 	public function reallyMarkPatrolled() {
 		$dbw = MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->getPrimaryDatabase();
 		$dbw->newUpdateQueryBuilder()
 			->update( 'recentchanges' )
 			->set( [ 'rc_patrolled' => self::PRC_PATROLLED ] )
-			->where( [ 'rc_id' => $this->getAttribute( 'rc_id' ) ] )
+			->where( [
+				'rc_id' => $this->getAttribute( 'rc_id' ),
+				'rc_patrolled' => self::PRC_UNPATROLLED,
+			] )
 			->caller( __METHOD__ )->execute();
+		$affectedRowCount = $dbw->affectedRows();
+		// The change was patrolled already, do nothing
+		if ( $affectedRowCount === 0 ) {
+			return 0;
+		}
 		// Invalidate the page cache after the page has been patrolled
 		// to make sure that the Patrol link isn't visible any longer!
 		$this->getTitle()->invalidateCache();
@@ -665,7 +683,7 @@ class RecentChange implements Taggable {
 			$revertedTagUpdateManager->approveRevertedTagForRevision( $revisionId );
 		}
 
-		return $dbw->affectedRows();
+		return $affectedRowCount;
 	}
 
 	/**