Merge "RCFilters: Fix RC jumpiness due to expanded/collapsed community links"

diff --git a/autoload.php b/autoload.php
index abedffd..a489b8b 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -1497,6 +1497,7 @@ $wgAutoloadLocalClasses = [
        'TrackBlobs' => __DIR__ . '/maintenance/storage/trackBlobs.php',
        'TrackingCategories' => __DIR__ . '/includes/TrackingCategories.php',
        'TraditionalImageGallery' => __DIR__ . '/includes/gallery/TraditionalImageGallery.php',
+       'TransactionRoundDefiningUpdate' => __DIR__ . '/includes/deferred/TransactionRoundDefiningUpdate.php',
        'TransformParameterError' => __DIR__ . '/includes/media/MediaTransformOutput.php',
        'TransformTooBigImageAreaError' => __DIR__ . '/includes/media/MediaTransformOutput.php',
        'TransformationalImageHandler' => __DIR__ . '/includes/media/TransformationalImageHandler.php',

diff --git a/includes/deferred/DeferredUpdates.php b/includes/deferred/DeferredUpdates.php
index e8e250b..3c4833c 100644 (file)
--- a/includes/deferred/DeferredUpdates.php
+++ b/includes/deferred/DeferredUpdates.php
@@ -250,6 +250,8 @@ class DeferredUpdates {
                                // Run only the job enqueue logic to complete the update later
                                $spec = $update->getAsJobSpecification();
                                JobQueueGroup::singleton( $spec['wiki'] )->push( $spec['job'] );
+                       } elseif ( $update instanceof TransactionRoundDefiningUpdate ) {
+                               $update->doUpdate();
                        } else {
                                // Run the bulk of the update now
                                $fnameTrxOwner = get_class( $update ) . '::doUpdate';

diff --git a/includes/deferred/TransactionRoundDefiningUpdate.php b/includes/deferred/TransactionRoundDefiningUpdate.php
new file mode 100644 (file)
index 0000000..65baec5
--- /dev/null
+++ b/includes/deferred/TransactionRoundDefiningUpdate.php
@@ -0,0 +1,31 @@
+<?php
+
+/**
+ * Deferrable update for closure/callback updates that need LBFactory and Database
+ * to be outside any active transaction round.
+ *
+ * @since 1.31
+ */
+class TransactionRoundDefiningUpdate implements DeferrableUpdate, DeferrableCallback {
+       /** @var callable|null */
+       private $callback;
+       /** @var string */
+       private $fname;
+
+       /**
+        * @param callable $callback
+        * @param string $fname Calling method
+        */
+       public function __construct( callable $callback, $fname = 'unknown' ) {
+               $this->callback = $callback;
+               $this->fname = $fname;
+       }
+
+       public function doUpdate() {
+               call_user_func( $this->callback );
+       }
+
+       public function getOrigin() {
+               return $this->fname;
+       }
+}

diff --git a/includes/specials/SpecialRunJobs.php b/includes/specials/SpecialRunJobs.php
index 73fe70f..375694b 100644 (file)
--- a/includes/specials/SpecialRunJobs.php
+++ b/includes/specials/SpecialRunJobs.php
@@ -39,17 +39,21 @@ class SpecialRunJobs extends UnlistedSpecialPage {
 
        public function execute( $par = '' ) {
                $this->getOutput()->disable();
+
                if ( wfReadOnly() ) {
                        wfHttpError( 423, 'Locked', 'Wiki is in read-only mode.' );
                        return;
-               } elseif ( !$this->getRequest()->wasPosted() ) {
+               }
+
+               // Validate request method
+               if ( !$this->getRequest()->wasPosted() ) {
                        wfHttpError( 400, 'Bad Request', 'Request must be POSTed.' );
                        return;
                }
 
+               // Validate request parameters
                $optional = [ 'maxjobs' => 0, 'maxtime' => 30, 'type' => false, 'async' => true ];
                $required = array_flip( [ 'title', 'tasks', 'signature', 'sigexpiry' ] );
-
                $params = array_intersect_key( $this->getRequest()->getValues(), $required + $optional );
                $missing = array_diff_key( $required, $params );
                if ( count( $missing ) ) {
@@ -59,11 +63,11 @@ class SpecialRunJobs extends UnlistedSpecialPage {
                        return;
                }
 
+               // Validate request signature
                $squery = $params;
                unset( $squery['signature'] );
                $correctSignature = self::getQuerySignature( $squery, $this->getConfig()->get( 'SecretKey' ) );
                $providedSignature = $params['signature'];
-
                $verified = is_string( $providedSignature )
                        && hash_equals( $correctSignature, $providedSignature );
                if ( !$verified || $params['sigexpiry'] < time() ) {
@@ -75,39 +79,34 @@ class SpecialRunJobs extends UnlistedSpecialPage {
                $params += $optional;
 
                if ( $params['async'] ) {
-                       // Client will usually disconnect before checking the response,
-                       // but it needs to know when it is safe to disconnect. Until this
-                       // reaches ignore_user_abort(), it is not safe as the jobs won't run.
-                       ignore_user_abort( true ); // jobs may take a bit of time
                        // HTTP 202 Accepted
                        HttpStatus::header( 202 );
-                       ob_flush();
-                       flush();
-                       // Once the client receives this response, it can disconnect
-                       set_error_handler( function ( $errno, $errstr ) {
-                               if ( strpos( $errstr, 'Cannot modify header information' ) !== false ) {
-                                       return true; // bug T115413
-                               }
-                               // Delegate unhandled errors to the default MediaWiki handler
-                               // so that fatal errors get proper logging (T89169)
-                               return call_user_func_array(
-                                       'MWExceptionHandler::handleError', func_get_args()
-                               );
-                       } );
+                       // Clients are meant to disconnect without waiting for the full response.
+                       // Let the page output happen before the jobs start, so that clients know it's
+                       // safe to disconnect. MediaWiki::preOutputCommit() calls ignore_user_abort()
+                       // or similar to make sure we stay alive to run the deferred update.
+                       DeferredUpdates::addUpdate(
+                               new TransactionRoundDefiningUpdate(
+                                       function () use ( $params ) {
+                                               $this->doRun( $params );
+                                       },
+                                       __METHOD__
+                               ),
+                               DeferredUpdates::POSTSEND
+                       );
+               } else {
+                       $this->doRun( $params );
+                       print "Done\n";
                }
+       }
 
-               // Do all of the specified tasks...
-               if ( in_array( 'jobs', explode( '|', $params['tasks'] ) ) ) {
-                       $runner = new JobRunner( LoggerFactory::getInstance( 'runJobs' ) );
-                       $runner->run( [
-                               'type'     => $params['type'],
-                               'maxJobs'  => $params['maxjobs'] ? $params['maxjobs'] : 1,
-                               'maxTime'  => $params['maxtime'] ? $params['maxjobs'] : 30
-                       ] );
-                       if ( !$params['async'] ) {
-                               print "Done\n";
-                       }
-               }
+       protected function doRun( array $params ) {
+               $runner = new JobRunner( LoggerFactory::getInstance( 'runJobs' ) );
+               $runner->run( [
+                       'type'     => $params['type'],
+                       'maxJobs'  => $params['maxjobs'] ?: 1,
+                       'maxTime'  => $params['maxtime'] ?: 30
+               ] );
        }
 
        /**

diff --git a/tests/phpunit/includes/deferred/DeferredUpdatesTest.php b/tests/phpunit/includes/deferred/DeferredUpdatesTest.php
index 999ad03..8332d2c 100644 (file)
--- a/tests/phpunit/includes/deferred/DeferredUpdatesTest.php
+++ b/tests/phpunit/includes/deferred/DeferredUpdatesTest.php
@@ -1,5 +1,7 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
+
 class DeferredUpdatesTest extends MediaWikiTestCase {
 
        /**
@@ -289,4 +291,46 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
                $this->assertTrue( $x, "Outer POSTSEND update ran" );
                $this->assertTrue( $y, "Nested PRESEND update ran" );
        }
+
+       /**
+        * @covers DeferredUpdates::runUpdate
+        */
+       public function testRunUpdateTransactionScope() {
+               $this->setMwGlobals( 'wgCommandLineMode', false );
+
+               $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+               $this->assertFalse( $lbFactory->hasTransactionRound(), 'Initial state' );
+
+               $ran = 0;
+               DeferredUpdates::addCallableUpdate( function () use ( &$ran, $lbFactory ) {
+                       $ran++;
+                       $this->assertTrue( $lbFactory->hasTransactionRound(), 'Has transaction' );
+               } );
+               DeferredUpdates::doUpdates();
+
+               $this->assertSame( 1, $ran, 'Update ran' );
+               $this->assertFalse( $lbFactory->hasTransactionRound(), 'Final state' );
+       }
+
+       /**
+        * @covers DeferredUpdates::runUpdate
+        * @covers TransactionRoundDefiningUpdate::getOrigin
+        */
+       public function testRunOuterScopeUpdate() {
+               $this->setMwGlobals( 'wgCommandLineMode', false );
+
+               $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+               $this->assertFalse( $lbFactory->hasTransactionRound(), 'Initial state' );
+
+               $ran = 0;
+               DeferredUpdates::addUpdate( new TransactionRoundDefiningUpdate(
+                       function () use ( &$ran, $lbFactory ) {
+                               $ran++;
+                               $this->assertFalse( $lbFactory->hasTransactionRound(), 'No transaction' );
+                       } )
+               );
+               DeferredUpdates::doUpdates();
+
+               $this->assertSame( 1, $ran, 'Update ran' );
+       }
 }

diff --git a/tests/phpunit/includes/deferred/TransactionRoundDefiningUpdateTest.php b/tests/phpunit/includes/deferred/TransactionRoundDefiningUpdateTest.php
new file mode 100644 (file)
index 0000000..e6ad072
--- /dev/null
+++ b/tests/phpunit/includes/deferred/TransactionRoundDefiningUpdateTest.php
@@ -0,0 +1,17 @@
+<?php
+
+/**
+ * @covers TransactionRoundDefiningUpdate
+ */
+class TransactionRoundDefiningUpdateTest extends PHPUnit_Framework_TestCase {
+
+       public function testDoUpdate() {
+               $ran = 0;
+               $update = new TransactionRoundDefiningUpdate( function () use ( &$ran ) {
+                       $ran++;
+               } );
+               $this->assertSame( 0, $ran );
+               $update->doUpdate();
+               $this->assertSame( 1, $ran );
+       }
+}