Make MergeableUpdate jobs avoid the sub-queue so they can always merge

Change-Id: I5b100fae29b785ab4524d165dad2e8ee46406b0c

diff --git a/includes/deferred/DeferredUpdates.php b/includes/deferred/DeferredUpdates.php
index e6a0e81..86feddb 100644 (file)
--- 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 f3c949d..2eaa5e2 100644 (file)
--- 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();
        }
 }