Merge "mediawiki.util: Drop escapeId(), deprecated since 1.30 and unused"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Mon, 7 May 2018 22:24:42 +0000 (22:24 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 7 May 2018 22:24:42 +0000 (22:24 +0000)
includes/libs/rdbms/database/DBConnRef.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/IDatabase.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
includes/specials/SpecialActiveusers.php
includes/specials/SpecialAutoblockList.php
includes/specials/SpecialBlockList.php
resources/src/jquery/jquery.tablesorter.js
tests/phpunit/includes/db/LBFactoryTest.php

index c94f62f..3432bff 100644 (file)
@@ -113,6 +113,10 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
+       public function preCommitCallbacksPending() {
+               return $this->__call( __FUNCTION__, func_get_args() );
+       }
+
        public function writesOrCallbacksPending() {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
index 8da1ca9..1517bd9 100644 (file)
@@ -677,6 +677,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                );
        }
 
+       public function preCommitCallbacksPending() {
+               return $this->trxLevel && $this->trxPreCommitCallbacks;
+       }
+
        /**
         * @return string|null
         */
index bfaa950..43e9751 100644 (file)
@@ -254,8 +254,15 @@ interface IDatabase {
        public function writesPending();
 
        /**
-        * Returns true if there is a transaction/round open with possible write
-        * queries or transaction pre-commit/idle callbacks waiting on it to finish.
+        * @return bool Whether there is a transaction open with pre-commit callbacks pending
+        * @since 1.32
+        */
+       public function preCommitCallbacksPending();
+
+       /**
+        * Whether there is a transaction open with either possible write queries
+        * or unresolved pre-commit/commit/resolution callbacks pending
+        *
         * This does *not* count recurring callbacks, e.g. from setTransactionListener().
         *
         * @return bool
index fbc413e..fe18536 100644 (file)
@@ -246,7 +246,12 @@ abstract class LBFactory implements ILBFactory {
                /** @noinspection PhpUnusedLocalVariableInspection */
                $scope = $this->getScopedPHPBehaviorForCommit(); // try to ignore client aborts
                // Run pre-commit callbacks and suppress post-commit callbacks, aborting on failure
-               $this->forEachLBCallMethod( 'finalizeMasterChanges' );
+               do {
+                       $count = 0; // number of callbacks executed this iteration
+                       $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$count ) {
+                               $count += $lb->finalizeMasterChanges();
+                       } );
+               } while ( $count > 0 );
                $this->trxRoundId = false;
                // Perform pre-commit checks, aborting on failure
                $this->forEachLBCallMethod( 'approveMasterChanges', [ $options ] );
index e69ec26..5d217e2 100644 (file)
@@ -380,6 +380,8 @@ interface ILoadBalancer {
         * Run pre-commit callbacks and defer execution of post-commit callbacks
         *
         * Use this only for mutli-database commits
+        *
+        * @return int Number of pre-commit callbacks run (since 1.32)
         */
        public function finalizeMasterChanges();
 
@@ -449,7 +451,7 @@ interface ILoadBalancer {
        public function hasMasterConnection();
 
        /**
-        * Determine if there are pending changes in a transaction by this thread
+        * Whether there are pending changes or callbacks in a transaction by this thread
         * @return bool
         */
        public function hasMasterChanges();
index ddc4277..cb6e4f4 100644 (file)
@@ -1263,10 +1263,11 @@ class LoadBalancer implements ILoadBalancer {
        }
 
        public function finalizeMasterChanges() {
-               $this->assertTransactionRoundStage( self::ROUND_CURSORY );
+               $this->assertTransactionRoundStage( [ self::ROUND_CURSORY, self::ROUND_FINALIZED ] );
 
                $this->trxRoundStage = self::ROUND_ERROR; // "failed" until proven otherwise
                // Loop until callbacks stop adding callbacks on other connections
+               $total = 0;
                do {
                        $count = 0; // callbacks execution attempts
                        $this->forEachOpenMasterConnection( function ( Database $conn ) use ( &$count ) {
@@ -1274,12 +1275,15 @@ class LoadBalancer implements ILoadBalancer {
                                // Any error should cause all (peer) transactions to be rolled back together.
                                $count += $conn->runOnTransactionPreCommitCallbacks();
                        } );
+                       $total += $count;
                } while ( $count > 0 );
                // Defer post-commit callbacks until after COMMIT/ROLLBACK happens on all handles
                $this->forEachOpenMasterConnection( function ( Database $conn ) {
                        $conn->setTrxEndCallbackSuppression( true );
                } );
                $this->trxRoundStage = self::ROUND_FINALIZED;
+
+               return $total;
        }
 
        public function approveMasterChanges( array $options ) {
@@ -1494,13 +1498,21 @@ class LoadBalancer implements ILoadBalancer {
        }
 
        /**
-        * @param string $stage
+        * @param string|string[] $stage
         */
        private function assertTransactionRoundStage( $stage ) {
-               if ( $this->trxRoundStage !== $stage ) {
+               $stages = (array)$stage;
+
+               if ( !in_array( $this->trxRoundStage, $stages, true ) ) {
+                       $stageList = implode(
+                               '/',
+                               array_map( function ( $v ) {
+                                       return "'$v'";
+                               }, $stages )
+                       );
                        throw new DBTransactionError(
                                null,
-                               "Transaction round stage must be '$stage' (not '{$this->trxRoundStage}')"
+                               "Transaction round stage must be $stageList (not '{$this->trxRoundStage}')"
                        );
                }
        }
index 9028787..0c709af 100644 (file)
@@ -80,10 +80,12 @@ class SpecialActiveUsers extends SpecialPage {
        protected function buildForm() {
                $groups = User::getAllGroups();
 
+               $options = [];
                foreach ( $groups as $group ) {
                        $msg = htmlspecialchars( UserGroupMembership::getGroupName( $group ) );
                        $options[$msg] = $group;
                }
+               asort( $options );
 
                // Backwards-compatibility with old URLs
                $req = $this->getRequest();
index 4d2d1b9..e1909f5 100644 (file)
@@ -42,7 +42,6 @@ class SpecialAutoblockList extends SpecialPage {
                $this->setHeaders();
                $this->outputHeader();
                $out = $this->getOutput();
-               $lang = $this->getLanguage();
                $out->setPageTitle( $this->msg( 'autoblocklist' ) );
                $this->addHelpLink( 'Autoblock' );
                $out->addModuleStyles( [ 'mediawiki.special' ] );
@@ -55,13 +54,7 @@ class SpecialAutoblockList extends SpecialPage {
                        'Limit' => [
                                'type' => 'limitselect',
                                'label-message' => 'table_pager_limit_label',
-                               'options' => [
-                                       $lang->formatNum( 20 ) => 20,
-                                       $lang->formatNum( 50 ) => 50,
-                                       $lang->formatNum( 100 ) => 100,
-                                       $lang->formatNum( 250 ) => 250,
-                                       $lang->formatNum( 500 ) => 500,
-                               ],
+                               'options' => $pager->getLimitSelectList(),
                                'name' => 'limit',
                                'default' => $pager->getLimit(),
                        ]
index 667b814..186e5ad 100644 (file)
@@ -44,7 +44,6 @@ class SpecialBlockList extends SpecialPage {
                $this->setHeaders();
                $this->outputHeader();
                $out = $this->getOutput();
-               $lang = $this->getLanguage();
                $out->setPageTitle( $this->msg( 'ipblocklist' ) );
                $out->addModuleStyles( [ 'mediawiki.special' ] );
 
@@ -89,13 +88,7 @@ class SpecialBlockList extends SpecialPage {
                        'Limit' => [
                                'type' => 'limitselect',
                                'label-message' => 'table_pager_limit_label',
-                               'options' => [
-                                       $lang->formatNum( 20 ) => 20,
-                                       $lang->formatNum( 50 ) => 50,
-                                       $lang->formatNum( 100 ) => 100,
-                                       $lang->formatNum( 250 ) => 250,
-                                       $lang->formatNum( 500 ) => 500,
-                               ],
+                               'options' => $pager->getLimitSelectList(),
                                'name' => 'limit',
                                'default' => $pager->getLimit(),
                        ],
index e9d5a91..552c0c3 100644 (file)
                                                return;
                                        }
                                }
-                               $table.addClass( 'jquery-tablesorter' );
+                               // The `sortable` class is used to identify tables which will become sortable
+                               // If not used it will create a FOUC but it should be added since the sortable class
+                               // is responsible for certain crucial style elements. If the class is already present
+                               // this action will be harmless.
+                               $table.addClass( 'jquery-tablesorter sortable' );
 
                                // Merge and extend
                                config = $.extend( {}, $.tablesorter.defaultOptions, settings );
index ed4c977..b715b1f 100644 (file)
@@ -157,12 +157,18 @@ class LBFactoryTest extends MediaWikiTestCase {
                global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir;
 
                $factory = new LBFactoryMulti( [
-                       'sectionsByDB' => [],
+                       'sectionsByDB' => [
+                               's1wiki' => 's1',
+                       ],
                        'sectionLoads' => [
+                               's1' => [
+                                       'test-db3' => 0,
+                                       'test-db4' => 100,
+                               ],
                                'DEFAULT' => [
                                        'test-db1' => 0,
                                        'test-db2' => 100,
-                               ],
+                               ]
                        ],
                        'serverTemplate' => [
                                'dbname'      => $wgDBname,
@@ -174,7 +180,9 @@ class LBFactoryTest extends MediaWikiTestCase {
                        ],
                        'hostsByName' => [
                                'test-db1'  => $wgDBserver,
-                               'test-db2'  => $wgDBserver
+                               'test-db2'  => $wgDBserver,
+                               'test-db3'  => $wgDBserver,
+                               'test-db4'  => $wgDBserver
                        ],
                        'loadMonitorClass' => LoadMonitorNull::class
                ] );
@@ -186,8 +194,25 @@ class LBFactoryTest extends MediaWikiTestCase {
                $dbr = $lb->getConnection( DB_REPLICA );
                $this->assertTrue( $dbr->getLBInfo( 'replica' ), 'slave shows as slave' );
 
+               // Test that LoadBalancer instances made during commitMasterChanges() do not throw
+               // DBTransactionError due to transaction ROUND_* stages being mismatched.
+               $factory->beginMasterChanges( __METHOD__ );
+               $dbw->onTransactionPreCommitOrIdle( function () use ( $factory ) {
+                       // Trigger s1 LoadBalancer instantiation during "finalize" stage.
+                       // There is no s1wiki DB to select so it is not in getConnection(),
+                       // but this fools getMainLB() at least.
+                       $factory->getMainLB( 's1wiki' )->getConnection( DB_MASTER );
+               } );
+               $factory->commitMasterChanges( __METHOD__ );
+
+               $count = 0;
+               $factory->forEachLB( function () use ( &$count ) {
+                       ++$count;
+               } );
+               $this->assertEquals( 2, $count );
+
                $factory->shutdown();
-               $lb->closeAll();
+               $factory->closeAll();
        }
 
        /**