From c95583a56041bea191fdd2fe9a7667c91d0aee4d Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 11 Aug 2016 09:19:09 -0700
Subject: [PATCH] Defer maybeUpgradeRow() post-send since they can trigger on
 non-POST

Change-Id: I791e7133d49ed9cd6dd40bb3fa35ea38d1ceba10
---
 includes/filerepo/file/LocalFile.php | 37 +++++++++++++++++-----------
 maintenance/refreshImageMetadata.php |  1 +
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index b8be4eaa5e..6eed22214c 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -117,6 +117,9 @@ class LocalFile extends File {
 	/** @var bool Whether the row was upgraded on load */
 	private $upgraded;
 
+	/** @var bool Whether the row was scheduled to upgrade on load */
+	private $upgrading;
+
 	/** @var bool True if the image row is locked */
 	private $locked;
 
@@ -559,37 +562,43 @@ class LocalFile extends File {
 	 */
 	function maybeUpgradeRow() {
 		global $wgUpdateCompatibleMetadata;
-		if ( wfReadOnly() ) {
+
+		if ( wfReadOnly() || $this->upgrading ) {
 			return;
 		}
 
 		$upgrade = false;
-		if ( is_null( $this->media_type ) ||
-			$this->mime == 'image/svg'
-		) {
+		if ( is_null( $this->media_type ) || $this->mime == 'image/svg' ) {
 			$upgrade = true;
 		} else {
 			$handler = $this->getHandler();
 			if ( $handler ) {
 				$validity = $handler->isMetadataValid( $this, $this->getMetadata() );
-				if ( $validity === MediaHandler::METADATA_BAD
-					|| ( $validity === MediaHandler::METADATA_COMPATIBLE && $wgUpdateCompatibleMetadata )
-				) {
+				if ( $validity === MediaHandler::METADATA_BAD ) {
 					$upgrade = true;
+				} elseif ( $validity === MediaHandler::METADATA_COMPATIBLE ) {
+					$upgrade = $wgUpdateCompatibleMetadata;
 				}
 			}
 		}
 
 		if ( $upgrade ) {
-			try {
-				$this->upgradeRow();
-			} catch ( LocalFileLockError $e ) {
-				// let the other process handle it (or do it next time)
-			}
-			$this->upgraded = true; // avoid rework/retries
+			$this->upgrading = true;
+			// Defer updates unless in auto-commit CLI mode
+			DeferredUpdates::addCallableUpdate( function() {
+				$this->upgrading = false; // avoid duplicate updates
+				try {
+					$this->upgradeRow();
+				} catch ( LocalFileLockError $e ) {
+					// let the other process handle it (or do it next time)
+				}
+			} );
 		}
 	}
 
+	/**
+	 * @return bool Whether upgradeRow() ran for this object
+	 */
 	function getUpgraded() {
 		return $this->upgraded;
 	}
@@ -639,7 +648,7 @@ class LocalFile extends File {
 		$this->invalidateCache();
 
 		$this->unlock(); // done
-
+		$this->upgraded = true; // avoid rework/retries
 	}
 
 	/**
diff --git a/maintenance/refreshImageMetadata.php b/maintenance/refreshImageMetadata.php
index bdbb347899..aea966ffb2 100644
--- a/maintenance/refreshImageMetadata.php
+++ b/maintenance/refreshImageMetadata.php
@@ -139,6 +139,7 @@ class RefreshImageMetadata extends Maintenance {
 			}
 
 			foreach ( $res as $row ) {
+				// LocalFile will upgrade immediately here if obsolete
 				$file = $repo->newFileFromRow( $row );
 				if ( $file->getUpgraded() ) {
 					// File was upgraded.
-- 
2.20.1