From f61868f13d71ab50ffd206e06f2c04d3a608d9d7 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 11 Oct 2018 15:11:50 -0700
Subject: [PATCH] Make MergeableUpdate jobs avoid the sub-queue so they can
 always merge

Change-Id: I5b100fae29b785ab4524d165dad2e8ee46406b0c
---
 includes/deferred/DeferredUpdates.php         | 23 +++++++++-----
 .../includes/deferred/CdnCacheUpdateTest.php  | 31 +++++++++++++++++--
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/includes/deferred/DeferredUpdates.php b/includes/deferred/DeferredUpdates.php
index e6a0e81779..86feddb28c 100644
--- a/includes/deferred/DeferredUpdates.php
+++ b/includes/deferred/DeferredUpdates.php
@@ -79,7 +79,11 @@ class DeferredUpdates {
 	public static function addUpdate( DeferrableUpdate $update, $stage = self::POSTSEND ) {
 		global $wgCommandLineMode;
 
-		if ( self::$executeContext && self::$executeContext['stage'] >= $stage ) {
+		if (
+			self::$executeContext &&
+			self::$executeContext['stage'] >= $stage &&
+			!( $update instanceof MergeableUpdate )
+		) {
 			// This is a sub-DeferredUpdate; run it right after its parent update.
 			// Also, while post-send updates are running, push any "pre-send" jobs to the
 			// active post-send queue to make sure they get run this round (or at all).
@@ -125,14 +129,17 @@ class DeferredUpdates {
 	 */
 	public static function doUpdates( $mode = 'run', $stage = self::ALL ) {
 		$stageEffective = ( $stage === self::ALL ) ? self::POSTSEND : $stage;
+		// For ALL mode, make sure that any PRESEND updates added along the way get run.
+		// Normally, these use the subqueue, but that isn't true for MergeableUpdate items.
+		do {
+			if ( $stage === self::ALL || $stage === self::PRESEND ) {
+				self::execute( self::$preSendUpdates, $mode, $stageEffective );
+			}
 
-		if ( $stage === self::ALL || $stage === self::PRESEND ) {
-			self::execute( self::$preSendUpdates, $mode, $stageEffective );
-		}
-
-		if ( $stage === self::ALL || $stage == self::POSTSEND ) {
-			self::execute( self::$postSendUpdates, $mode, $stageEffective );
-		}
+			if ( $stage === self::ALL || $stage == self::POSTSEND ) {
+				self::execute( self::$postSendUpdates, $mode, $stageEffective );
+			}
+		} while ( $stage === self::ALL && self::$preSendUpdates );
 	}
 
 	/**
diff --git a/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php b/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php
index f3c949d3d0..2eaa5e2f49 100644
--- a/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php
+++ b/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php
@@ -15,17 +15,44 @@ class CdnCacheUpdateTest extends MediaWikiTestCase {
 		$urls1[] = $title->getCanonicalURL( '?x=1' );
 		$urls1[] = $title->getCanonicalURL( '?x=2' );
 		$urls1[] = $title->getCanonicalURL( '?x=3' );
-		$update1 = new CdnCacheUpdate( $urls1 );
+		$update1 = $this->newCdnCacheUpdate( $urls1 );
 		DeferredUpdates::addUpdate( $update1 );
 
 		$urls2 = [];
 		$urls2[] = $title->getCanonicalURL( '?x=2' );
 		$urls2[] = $title->getCanonicalURL( '?x=3' );
 		$urls2[] = $title->getCanonicalURL( '?x=4' );
-		$update2 = new CdnCacheUpdate( $urls2 );
+		$update2 = $this->newCdnCacheUpdate( $urls2 );
 		DeferredUpdates::addUpdate( $update2 );
 
 		$wrapper = TestingAccessWrapper::newFromObject( $update1 );
 		$this->assertEquals( array_merge( $urls1, $urls2 ), $wrapper->urls );
+
+		$update = null;
+		DeferredUpdates::clearPendingUpdates();
+		DeferredUpdates::addCallableUpdate( function () use ( $urls1, $urls2, &$update ) {
+			$update = $this->newCdnCacheUpdate( $urls1 );
+			DeferredUpdates::addUpdate( $update );
+			DeferredUpdates::addUpdate( $this->newCdnCacheUpdate( $urls2 ) );
+			DeferredUpdates::addUpdate(
+				$this->newCdnCacheUpdate( $urls2 ), DeferredUpdates::PRESEND );
+		} );
+		DeferredUpdates::doUpdates();
+
+		$wrapper = TestingAccessWrapper::newFromObject( $update );
+		$this->assertEquals( array_merge( $urls1, $urls2 ), $wrapper->urls );
+
+		$this->assertEquals( DeferredUpdates::pendingUpdatesCount(), 0, 'PRESEND update run' );
+	}
+
+	/**
+	 * @param array $urls
+	 * @return CdnCacheUpdate
+	 */
+	private function newCdnCacheUpdate( array $urls ) {
+		return $this->getMockBuilder( CdnCacheUpdate::class )
+			->setConstructorArgs( [ $urls ] )
+			->setMethods( [ 'doUpdate' ] )
+			->getMock();
 	}
 }
-- 
2.20.1