Merge "Do move options checks before the move"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 13 Jun 2019 10:13:31 +0000 (10:13 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 13 Jun 2019 10:13:31 +0000 (10:13 +0000)
31 files changed:
includes/EditPage.php
includes/Revision/RevisionStore.php
includes/api/ApiLogin.php
includes/block/BlockManager.php
includes/block/CompositeBlock.php [new file with mode: 0644]
includes/block/DatabaseBlock.php
includes/export/WikiExporter.php
includes/installer/PostgresUpdater.php
includes/libs/StatusValue.php
includes/libs/mime/MimeAnalyzer.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMssql.php
includes/libs/rdbms/database/DatabaseMysqlBase.php
includes/libs/rdbms/database/IDatabase.php
includes/logging/DeleteLogFormatter.php
includes/page/ImagePage.php
includes/password/LayeredParameterizedPassword.php
includes/resourceloader/ResourceLoaderFileModule.php
includes/user/User.php
maintenance/importTextFiles.php
resources/src/mediawiki.rcfilters/styles/mw.rcfilters.ui.MenuSelectWidget.less
resources/src/mediawiki.rcfilters/ui/FilterTagMultiselectWidget.js
tests/phpunit/ResourceLoaderTestCase.php
tests/phpunit/includes/api/ApiQueryLanguageinfoTest.php
tests/phpunit/includes/block/CompositeBlockTest.php [new file with mode: 0644]
tests/phpunit/includes/jobqueue/JobQueueTest.php
tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
tests/phpunit/includes/logging/DeleteLogFormatterTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php
tests/phpunit/includes/user/PasswordResetTest.php
tests/phpunit/includes/user/UserTest.php

index e4adb48..d27ef9c 100644 (file)
@@ -4130,7 +4130,7 @@ ERROR;
 
                if ( !Hooks::run( 'EditPageBeforeEditToolbar', [ &$toolbar ] ) ) {
                        return null;
-               };
+               }
                // Don't add a pointless `<div>` to the page unless a hook caller populated it
                return ( $toolbar === $startingToolbar ) ? null : $toolbar;
        }
index 29d7848..00c9d04 100644 (file)
@@ -1644,7 +1644,7 @@ class RevisionStore
                        throw new RevisionAccessException(
                                'Main slot of revision ' . $revId . ' not found in database!'
                        );
-               };
+               }
 
                return $slots;
        }
index c3c5318..de5257e 100644 (file)
@@ -287,7 +287,7 @@ class ApiLogin extends ApiBase {
                ];
                if ( $response->message ) {
                        $ret['message'] = $response->message->inLanguage( 'en' )->plain();
-               };
+               }
                $reqs = [
                        'neededRequests' => $response->neededRequests,
                        'createRequest' => $response->createRequest,
index be240ca..60ae2f8 100644 (file)
@@ -110,7 +110,9 @@ class BlockManager {
        }
 
        /**
-        * Get the blocks that apply to a user and return the most relevant one.
+        * Get the blocks that apply to a user. If there is only one, return that, otherwise
+        * return a composite block that combines the strictest features of the applicable
+        * blocks.
         *
         * TODO: $user should be UserIdentity instead of User
         *
@@ -143,29 +145,28 @@ class BlockManager {
                }
 
                // User/IP blocking
+               // After this, $blocks is an array of blocks or an empty array
                // TODO: remove dependency on DatabaseBlock
-               $block = DatabaseBlock::newFromTarget( $user, $ip, !$fromReplica );
+               $blocks = DatabaseBlock::newListFromTarget( $user, $ip, !$fromReplica );
 
                // Cookie blocking
-               if ( !$block instanceof AbstractBlock ) {
-                       $block = $this->getBlockFromCookieValue( $user, $request );
+               $cookieBlock = $this->getBlockFromCookieValue( $user, $request );
+               if ( $cookieBlock instanceof AbstractBlock ) {
+                       $blocks[] = $cookieBlock;
                }
 
                // Proxy blocking
-               if ( !$block instanceof AbstractBlock
-                       && $ip !== null
-                       && !in_array( $ip, $this->proxyWhitelist )
-               ) {
+               if ( $ip !== null && !in_array( $ip, $this->proxyWhitelist ) ) {
                        // Local list
                        if ( $this->isLocallyBlockedProxy( $ip ) ) {
-                               $block = new SystemBlock( [
+                               $blocks[] = new SystemBlock( [
                                        'byText' => wfMessage( 'proxyblocker' )->text(),
                                        'reason' => wfMessage( 'proxyblockreason' )->plain(),
                                        'address' => $ip,
                                        'systemBlock' => 'proxy',
                                ] );
                        } elseif ( $isAnon && $this->isDnsBlacklisted( $ip ) ) {
-                               $block = new SystemBlock( [
+                               $blocks[] = new SystemBlock( [
                                        'byText' => wfMessage( 'sorbs' )->text(),
                                        'reason' => wfMessage( 'sorbsreason' )->plain(),
                                        'address' => $ip,
@@ -175,8 +176,7 @@ class BlockManager {
                }
 
                // (T25343) Apply IP blocks to the contents of XFF headers, if enabled
-               if ( !$block instanceof AbstractBlock
-                       && $this->applyIpBlocksToXff
+               if ( $this->applyIpBlocksToXff
                        && $ip !== null
                        && !in_array( $ip, $this->proxyWhitelist )
                ) {
@@ -185,21 +185,15 @@ class BlockManager {
                        $xff = array_diff( $xff, [ $ip ] );
                        // TODO: remove dependency on DatabaseBlock
                        $xffblocks = DatabaseBlock::getBlocksForIPList( $xff, $isAnon, !$fromReplica );
-                       // TODO: remove dependency on DatabaseBlock
-                       $block = DatabaseBlock::chooseBlock( $xffblocks, $xff );
-                       if ( $block instanceof AbstractBlock ) {
-                               # Mangle the reason to alert the user that the block
-                               # originated from matching the X-Forwarded-For header.
-                               $block->setReason( wfMessage( 'xffblockreason', $block->getReason() )->plain() );
-                       }
+                       $blocks = array_merge( $blocks, $xffblocks );
                }
 
-               if ( !$block instanceof AbstractBlock
-                       && $ip !== null
+               // Soft blocking
+               if ( $ip !== null
                        && $isAnon
                        && IP::isInRanges( $ip, $this->softBlockRanges )
                ) {
-                       $block = new SystemBlock( [
+                       $blocks[] = new SystemBlock( [
                                'address' => $ip,
                                'byText' => 'MediaWiki default',
                                'reason' => wfMessage( 'softblockrangesreason', $ip )->plain(),
@@ -208,7 +202,19 @@ class BlockManager {
                        ] );
                }
 
-               return $block;
+               if ( count( $blocks ) > 0 ) {
+                       if ( count( $blocks ) === 1 ) {
+                               $block = $blocks[ 0 ];
+                       } else {
+                               $block = new CompositeBlock( [
+                                       'address' => $ip,
+                                       'originalBlocks' => $blocks,
+                               ] );
+                       }
+                       return $block;
+               }
+
+               return null;
        }
 
        /**
@@ -393,13 +399,23 @@ class BlockManager {
        public function trackBlockWithCookie( User $user ) {
                $block = $user->getBlock();
                $request = $user->getRequest();
-
-               if (
-                       $block &&
-                       $request->getCookie( 'BlockID' ) === null &&
-                       $this->shouldTrackBlockWithCookie( $block, $user->isAnon() )
-               ) {
-                       $this->setBlockCookie( $block, $request->response() );
+               $response = $request->response();
+               $isAnon = $user->isAnon();
+
+               if ( $block && $request->getCookie( 'BlockID' ) === null ) {
+                       if ( $block instanceof CompositeBlock ) {
+                               // TODO: Improve on simply tracking the first trackable block (T225654)
+                               foreach ( $block->getOriginalBlocks() as $originalBlock ) {
+                                       if ( $this->shouldTrackBlockWithCookie( $originalBlock, $isAnon ) ) {
+                                               $this->setBlockCookie( $originalBlock, $response );
+                                               return;
+                                       }
+                               }
+                       } else {
+                               if ( $this->shouldTrackBlockWithCookie( $block, $isAnon ) ) {
+                                       $this->setBlockCookie( $block, $response );
+                               }
+                       }
                }
        }
 
diff --git a/includes/block/CompositeBlock.php b/includes/block/CompositeBlock.php
new file mode 100644 (file)
index 0000000..fda1505
--- /dev/null
@@ -0,0 +1,164 @@
+<?php
+/**
+ * Class for blocks composed from multiple blocks.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Block;
+
+use IContextSource;
+use Title;
+
+/**
+ * Multiple Block class.
+ *
+ * Multiple blocks exist to enforce restrictions from more than one block, if several
+ * blocks apply to a user/IP. Multiple blocks are created temporarily on enforcement.
+ *
+ * @since 1.34
+ */
+class CompositeBlock extends AbstractBlock {
+       /** @var AbstractBlock[] */
+       private $originalBlocks;
+
+       /**
+        * Create a new block with specified parameters on a user, IP or IP range.
+        *
+        * @param array $options Parameters of the block:
+        *     originalBlocks Block[] Blocks that this block is composed from
+        */
+       function __construct( $options = [] ) {
+               parent::__construct( $options );
+
+               $defaults = [
+                       'originalBlocks' => [],
+               ];
+
+               $options += $defaults;
+
+               $this->originalBlocks = $options[ 'originalBlocks' ];
+
+               $this->setHideName( $this->propHasValue( 'mHideName', true ) );
+               $this->isSitewide( $this->propHasValue( 'isSitewide', true ) );
+               $this->isEmailBlocked( $this->propHasValue( 'mBlockEmail', true ) );
+               $this->isCreateAccountBlocked( $this->propHasValue( 'blockCreateAccount', true ) );
+               $this->isUsertalkEditAllowed( !$this->propHasValue( 'allowUsertalk', false ) );
+       }
+
+       /**
+        * Determine whether any original blocks have a particular property set to a
+        * particular value.
+        *
+        * @param string $prop
+        * @param mixed $value
+        * @return bool At least one block has the property set to the value
+        */
+       private function propHasValue( $prop, $value ) {
+               foreach ( $this->originalBlocks as $block ) {
+                       if ( $block->$prop == $value ) {
+                               return true;
+                       }
+               }
+               return false;
+       }
+
+       /**
+        * Determine whether any original blocks have a particular method returning a
+        * particular value.
+        *
+        * @param string $method
+        * @param mixed $value
+        * @param mixed ...$params
+        * @return bool At least one block has the method returning the value
+        */
+       private function methodReturnsValue( $method, $value, ...$params ) {
+               foreach ( $this->originalBlocks as $block ) {
+                       if ( $block->$method( ...$params ) == $value ) {
+                               return true;
+                       }
+               }
+               return false;
+       }
+
+       /**
+        * Get the original blocks from which this block is composed
+        *
+        * @since 1.34
+        * @return AbstractBlock[]
+        */
+       public function getOriginalBlocks() {
+               return $this->originalBlocks;
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function getPermissionsError( IContextSource $context ) {
+               $params = $this->getBlockErrorParams( $context );
+
+               $msg = $this->isSitewide() ? 'blockedtext' : 'blockedtext-partial';
+
+               array_unshift( $params, $msg );
+
+               return $params;
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function appliesToRight( $right ) {
+               return $this->methodReturnsValue( __FUNCTION__, true, $right );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function appliesToUsertalk( Title $usertalk = null ) {
+               return $this->methodReturnsValue( __FUNCTION__, true, $usertalk );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function appliesToTitle( Title $title ) {
+               return $this->methodReturnsValue( __FUNCTION__, true, $title );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function appliesToNamespace( $ns ) {
+               return $this->methodReturnsValue( __FUNCTION__, true, $ns );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function appliesToPage( $pageId ) {
+               return $this->methodReturnsValue( __FUNCTION__, true, $pageId );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function appliesToPasswordReset() {
+               return $this->methodReturnsValue( __FUNCTION__, true );
+       }
+
+}
index 876a81f..ba08d54 100644 (file)
@@ -1159,26 +1159,40 @@ class DatabaseBlock extends AbstractBlock {
         *     not be the same as the target you gave if you used $vagueTarget!
         */
        public static function newFromTarget( $specificTarget, $vagueTarget = null, $fromMaster = false ) {
+               $blocks = self::newListFromTarget( $specificTarget, $vagueTarget, $fromMaster );
+               return self::chooseMostSpecificBlock( $blocks );
+       }
+
+       /**
+        * This is similar to DatabaseBlock::newFromTarget, but it returns all the relevant blocks.
+        *
+        * @since 1.34
+        * @param string|User|int|null $specificTarget
+        * @param string|User|int|null $vagueTarget
+        * @param bool $fromMaster
+        * @return DatabaseBlock[] Any relevant blocks
+        */
+       public static function newListFromTarget(
+               $specificTarget,
+               $vagueTarget = null,
+               $fromMaster = false
+       ) {
                list( $target, $type ) = self::parseTarget( $specificTarget );
                if ( $type == self::TYPE_ID || $type == self::TYPE_AUTO ) {
-                       return self::newFromID( $target );
-
+                       $block = self::newFromID( $target );
+                       return $block ? [ $block ] : [];
                } elseif ( $target === null && $vagueTarget == '' ) {
                        # We're not going to find anything useful here
                        # Be aware that the == '' check is explicit, since empty values will be
                        # passed by some callers (T31116)
-                       return null;
-
+                       return [];
                } elseif ( in_array(
                        $type,
                        [ self::TYPE_USER, self::TYPE_IP, self::TYPE_RANGE, null ] )
                ) {
-                       $blocks = self::newLoad( $target, $type, $fromMaster, $vagueTarget );
-                       if ( !empty( $blocks ) ) {
-                               return self::chooseMostSpecificBlock( $blocks );
-                       }
+                       return self::newLoad( $target, $type, $fromMaster, $vagueTarget );
                }
-               return null;
+               return [];
        }
 
        /**
index e8044af..8b42be1 100644 (file)
@@ -320,7 +320,7 @@ class WikiExporter {
                        }
 
                        $lastLogId = $this->outputLogStream( $result );
-               };
+               }
        }
 
        /**
index 008240a..31827a1 100644 (file)
@@ -1071,7 +1071,7 @@ END;
                        $this->db->query( $command );
                } else {
                        $this->output( "...foreign key constraint on '$table.$field' already does not exist\n" );
-               };
+               }
        }
 
        protected function changeFkeyDeferrable( $table, $field, $clause ) {
@@ -1235,7 +1235,7 @@ END;
                if ( $this->updateRowExists( 'patch-textsearch_bug66650.sql' ) ) {
                        $this->output( "...T68650 already fixed or not applicable.\n" );
                        return;
-               };
+               }
                $this->applyPatch( 'patch-textsearch_bug66650.sql', false,
                        'Rebuilding text search for T68650' );
        }
index 16cb1ed..71a0e34 100644 (file)
@@ -107,7 +107,7 @@ class StatusValue {
                        } else {
                                $errorsOnlyStatusValue->errors[] = $item;
                        }
-               };
+               }
 
                return [ $errorsOnlyStatusValue, $warningsOnlyStatusValue ];
        }
index a326df2..e7dc926 100644 (file)
@@ -849,7 +849,7 @@ EOT;
                $callback = $this->guessCallback;
                if ( $callback ) {
                        $callback( $this, $head, $tail, $file, $mime /* by reference */ );
-               };
+               }
 
                return $mime;
        }
index fe23a38..a2caa96 100644 (file)
@@ -1020,7 +1020,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *
         * @throws DBUnexpectedError
         */
-       protected function assertHasConnectionHandle() {
+       final protected function assertHasConnectionHandle() {
                if ( !$this->isOpen() ) {
                        throw new DBUnexpectedError( $this, "DB connection was already closed." );
                }
@@ -1029,7 +1029,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        /**
         * Make sure that this server is not marked as a replica nor read-only as a sanity check
         *
-        * @throws DBUnexpectedError
+        * @throws DBReadOnlyRoleError
+        * @throws DBReadOnlyError
         */
        protected function assertIsWritableMaster() {
                if ( $this->getLBInfo( 'replica' ) === true ) {
@@ -1064,6 +1065,17 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        /**
         * Run a query and return a DBMS-dependent wrapper or boolean
         *
+        * This is meant to handle the basic command of actually sending a query to the
+        * server via the driver. No implicit transaction, reconnection, nor retry logic
+        * should happen here. The higher level query() method is designed to handle those
+        * sorts of concerns. This method should not trigger such higher level methods.
+        *
+        * The lastError() and lastErrno() methods should meaningfully reflect what error,
+        * if any, occured during the last call to this method. Methods like executeQuery(),
+        * query(), select(), insert(), update(), delete(), and upsert() implement their calls
+        * to doQuery() such that an immediately subsequent call to lastError()/lastErrno()
+        * meaningfully reflects any error that occured during that public query method call.
+        *
         * For SELECT queries, this returns either:
         *   - a) A driver-specific value/resource, only on success. This can be iterated
         *        over by calling fetchObject()/fetchRow() until there are no more rows.
@@ -1108,11 +1120,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                // for all queries within a request. Use cases:
                // - Treating these as writes would trigger ChronologyProtector (see method doc).
                // - We use this method to reject writes to replicas, but we need to allow
-               //   use of transactions on replicas for read snapshots. This fine given
+               //   use of transactions on replicas for read snapshots. This is fine given
                //   that transactions by themselves don't make changes, only actual writes
                //   within the transaction matter, which we still detect.
                return !preg_match(
-                       '/^(?:SELECT|BEGIN|ROLLBACK|COMMIT|SAVEPOINT|RELEASE|SET|SHOW|EXPLAIN|\(SELECT)\b/i',
+                       '/^(?:SELECT|BEGIN|ROLLBACK|COMMIT|SAVEPOINT|RELEASE|SET|SHOW|EXPLAIN|USE|\(SELECT)\b/i',
                        $sql
                );
        }
@@ -1141,7 +1153,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected function isTransactableQuery( $sql ) {
                return !in_array(
                        $this->getQueryVerb( $sql ),
-                       [ 'BEGIN', 'ROLLBACK', 'COMMIT', 'SET', 'SHOW', 'CREATE', 'ALTER' ],
+                       [ 'BEGIN', 'ROLLBACK', 'COMMIT', 'SET', 'SHOW', 'CREATE', 'ALTER', 'USE' ],
                        true
                );
        }
@@ -1190,108 +1202,132 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function query( $sql, $fname = __METHOD__, $flags = 0 ) {
-               $this->assertTransactionStatus( $sql, $fname );
-               $this->assertHasConnectionHandle();
-
                $flags = (int)$flags; // b/c; this field used to be a bool
-               $ignoreErrors = $this->hasFlags( $flags, self::QUERY_SILENCE_ERRORS );
+               // Sanity check that the SQL query is appropriate in the current context and is
+               // allowed for an outside caller (e.g. does not break transaction/session tracking).
+               $this->assertQueryIsCurrentlyAllowed( $sql, $fname );
+
+               // Send the query to the server and fetch any corresponding errors
+               list( $ret, $err, $errno, $unignorable ) = $this->executeQuery( $sql, $fname, $flags );
+               if ( $ret === false ) {
+                       $ignoreErrors = $this->hasFlags( $flags, self::QUERY_SILENCE_ERRORS );
+                       // Throw an error unless both the ignore flag was set and a rollback is not needed
+                       $this->reportQueryError( $err, $errno, $sql, $fname, $ignoreErrors && !$unignorable );
+               }
+
+               return $this->resultObject( $ret );
+       }
+
+       /**
+        * Execute a query, retrying it if there is a recoverable connection loss
+        *
+        * This is similar to query() except:
+        *   - It does not prevent all non-ROLLBACK queries if there is a corrupted transaction
+        *   - It does not disallow raw queries that are supposed to use dedicated IDatabase methods
+        *   - It does not throw exceptions for common error cases
+        *
+        * This is meant for internal use with Database subclasses.
+        *
+        * @param string $sql Original SQL query
+        * @param string $fname Name of the calling function
+        * @param int $flags Bitfield of class QUERY_* constants
+        * @return array An n-tuple of:
+        *   - mixed|bool: An object, resource, or true on success; false on failure
+        *   - string: The result of calling lastError()
+        *   - int: The result of calling lastErrno()
+        *   - bool: Whether a rollback is needed to allow future non-rollback queries
+        * @throws DBUnexpectedError
+        */
+       final protected function executeQuery( $sql, $fname, $flags ) {
+               $this->assertHasConnectionHandle();
 
                $priorTransaction = $this->trxLevel;
-               $priorWritesPending = $this->writesOrCallbacksPending();
 
                if ( $this->isWriteQuery( $sql ) ) {
                        # In theory, non-persistent writes are allowed in read-only mode, but due to things
                        # like https://bugs.mysql.com/bug.php?id=33669 that might not work anyway...
                        $this->assertIsWritableMaster();
-                       # Do not treat temporary table writes as "meaningful writes" that need committing.
-                       # Profile them as reads. Integration tests can override this behavior via $flags.
+                       # Do not treat temporary table writes as "meaningful writes" since they are only
+                       # visible to one session and are not permanent. Profile them as reads. Integration
+                       # tests can override this behavior via $flags.
                        $pseudoPermanent = $this->hasFlags( $flags, self::QUERY_PSEUDO_PERMANENT );
                        $tableType = $this->registerTempTableWrite( $sql, $pseudoPermanent );
-                       $isEffectiveWrite = ( $tableType !== self::TEMP_NORMAL );
+                       $isPermWrite = ( $tableType !== self::TEMP_NORMAL );
                        # DBConnRef uses QUERY_REPLICA_ROLE to enforce the replica role for raw SQL queries
-                       if ( $isEffectiveWrite && $this->hasFlags( $flags, self::QUERY_REPLICA_ROLE ) ) {
+                       if ( $isPermWrite && $this->hasFlags( $flags, self::QUERY_REPLICA_ROLE ) ) {
                                throw new DBReadOnlyRoleError( $this, "Cannot write; target role is DB_REPLICA" );
                        }
                } else {
-                       $isEffectiveWrite = false;
+                       $isPermWrite = false;
                }
 
-               # Add trace comment to the begin of the sql string, right after the operator.
-               # Or, for one-word queries (like "BEGIN" or COMMIT") add it to the end (T44598)
+               // Add trace comment to the begin of the sql string, right after the operator.
+               // Or, for one-word queries (like "BEGIN" or COMMIT") add it to the end (T44598)
                $commentedSql = preg_replace( '/\s|$/', " /* $fname {$this->agent} */ ", $sql, 1 );
 
-               # Send the query to the server and fetch any corresponding errors
-               $ret = $this->attemptQuery( $sql, $commentedSql, $isEffectiveWrite, $fname );
-               $lastError = $this->lastError();
-               $lastErrno = $this->lastErrno();
-
-               $recoverableSR = false; // recoverable statement rollback?
-               $recoverableCL = false; // recoverable connection loss?
-
-               if ( $ret === false && $this->wasConnectionLoss() ) {
-                       # Check if no meaningful session state was lost
-                       $recoverableCL = $this->canRecoverFromDisconnect( $sql, $priorWritesPending );
-                       # Update session state tracking and try to restore the connection
-                       $reconnected = $this->replaceLostConnection( __METHOD__ );
-                       # Silently resend the query to the server if it is safe and possible
-                       if ( $recoverableCL && $reconnected ) {
-                               $ret = $this->attemptQuery( $sql, $commentedSql, $isEffectiveWrite, $fname );
-                               $lastError = $this->lastError();
-                               $lastErrno = $this->lastErrno();
-
-                               if ( $ret === false && $this->wasConnectionLoss() ) {
-                                       # Query probably causes disconnects; reconnect and do not re-run it
-                                       $this->replaceLostConnection( __METHOD__ );
-                               } else {
-                                       $recoverableCL = false; // connection does not need recovering
-                                       $recoverableSR = $this->wasKnownStatementRollbackError();
-                               }
-                       }
-               } else {
-                       $recoverableSR = $this->wasKnownStatementRollbackError();
+               // Send the query to the server and fetch any corresponding errors
+               list( $ret, $err, $errno, $recoverableSR, $recoverableCL, $reconnected ) =
+                       $this->executeQueryAttempt( $sql, $commentedSql, $isPermWrite, $fname, $flags );
+               // Check if the query failed due to a recoverable connection loss
+               if ( $ret === false && $recoverableCL && $reconnected ) {
+                       // Silently resend the query to the server since it is safe and possible
+                       list( $ret, $err, $errno, $recoverableSR, $recoverableCL ) =
+                               $this->executeQueryAttempt( $sql, $commentedSql, $isPermWrite, $fname, $flags );
                }
 
+               $corruptedTrx = false;
+
                if ( $ret === false ) {
                        if ( $priorTransaction ) {
                                if ( $recoverableSR ) {
                                        # We're ignoring an error that caused just the current query to be aborted.
                                        # But log the cause so we can log a deprecation notice if a caller actually
                                        # does ignore it.
-                                       $this->trxStatusIgnoredCause = [ $lastError, $lastErrno, $fname ];
+                                       $this->trxStatusIgnoredCause = [ $err, $errno, $fname ];
                                } elseif ( !$recoverableCL ) {
                                        # Either the query was aborted or all queries after BEGIN where aborted.
                                        # In the first case, the only options going forward are (a) ROLLBACK, or
                                        # (b) ROLLBACK TO SAVEPOINT (if one was set). If the later case, the only
                                        # option is ROLLBACK, since the snapshots would have been released.
+                                       $corruptedTrx = true; // cannot recover
                                        $this->trxStatus = self::STATUS_TRX_ERROR;
                                        $this->trxStatusCause =
-                                               $this->getQueryExceptionAndLog( $lastError, $lastErrno, $sql, $fname );
-                                       $ignoreErrors = false; // cannot recover
+                                               $this->getQueryExceptionAndLog( $err, $errno, $sql, $fname );
                                        $this->trxStatusIgnoredCause = null;
                                }
                        }
-
-                       $this->reportQueryError( $lastError, $lastErrno, $sql, $fname, $ignoreErrors );
                }
 
-               return $this->resultObject( $ret );
+               return [ $ret, $err, $errno, $corruptedTrx ];
        }
 
        /**
-        * Wrapper for query() that also handles profiling, logging, and affected row count updates
+        * Wrapper for doQuery() that handles DBO_TRX, profiling, logging, affected row count
+        * tracking, and reconnects (without retry) on query failure due to connection loss
         *
         * @param string $sql Original SQL query
         * @param string $commentedSql SQL query with debugging/trace comment
-        * @param bool $isEffectiveWrite Whether the query is a (non-temporary table) write
+        * @param bool $isPermWrite Whether the query is a (non-temporary table) write
         * @param string $fname Name of the calling function
-        * @return bool|IResultWrapper True for a successful write query, ResultWrapper
-        *     object for a successful read query, or false on failure
+        * @param int $flags Bitfield of class QUERY_* constants
+        * @return array An n-tuple of:
+        *   - mixed|bool: An object, resource, or true on success; false on failure
+        *   - string: The result of calling lastError()
+        *   - int: The result of calling lastErrno()
+        *       - bool: Whether a statement rollback error occured
+        *   - bool: Whether a disconnect *both* happened *and* was recoverable
+        *   - bool: Whether a reconnection attempt was *both* made *and* succeeded
+        * @throws DBUnexpectedError
         */
-       private function attemptQuery( $sql, $commentedSql, $isEffectiveWrite, $fname ) {
-               $this->beginIfImplied( $sql, $fname );
+       private function executeQueryAttempt( $sql, $commentedSql, $isPermWrite, $fname, $flags ) {
+               $priorWritesPending = $this->writesOrCallbacksPending();
+
+               if ( ( $flags & self::QUERY_IGNORE_DBO_TRX ) == 0 ) {
+                       $this->beginIfImplied( $sql, $fname );
+               }
 
                // Keep track of whether the transaction has write queries pending
-               if ( $isEffectiveWrite ) {
+               if ( $isPermWrite ) {
                        $this->lastWriteTime = microtime( true );
                        if ( $this->trxLevel && !$this->trxDoneWrites ) {
                                $this->trxDoneWrites = true;
@@ -1310,16 +1346,31 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->affectedRowCount = null;
                $this->lastQuery = $sql;
                $ret = $this->doQuery( $commentedSql );
+               $lastError = $this->lastError();
+               $lastErrno = $this->lastErrno();
+
                $this->affectedRowCount = $this->affectedRows();
                unset( $ps ); // profile out (if set)
                $queryRuntime = max( microtime( true ) - $startTime, 0.0 );
 
+               $recoverableSR = false; // recoverable statement rollback?
+               $recoverableCL = false; // recoverable connection loss?
+               $reconnected = false; // reconnection both attempted and succeeded?
+
                if ( $ret !== false ) {
                        $this->lastPing = $startTime;
-                       if ( $isEffectiveWrite && $this->trxLevel ) {
+                       if ( $isPermWrite && $this->trxLevel ) {
                                $this->updateTrxWriteQueryTime( $sql, $queryRuntime, $this->affectedRows() );
                                $this->trxWriteCallers[] = $fname;
                        }
+               } elseif ( $this->wasConnectionError( $lastErrno ) ) {
+                       # Check if no meaningful session state was lost
+                       $recoverableCL = $this->canRecoverFromDisconnect( $sql, $priorWritesPending );
+                       # Update session state tracking and try to restore the connection
+                       $reconnected = $this->replaceLostConnection( __METHOD__ );
+               } else {
+                       # Check if only the last query was rolled back
+                       $recoverableSR = $this->wasKnownStatementRollbackError();
                }
 
                if ( $sql === self::PING_QUERY ) {
@@ -1329,8 +1380,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxProfiler->recordQueryCompletion(
                        $generalizedSql,
                        $startTime,
-                       $isEffectiveWrite,
-                       $isEffectiveWrite ? $this->affectedRows() : $this->numRows( $ret )
+                       $isPermWrite,
+                       $isPermWrite ? $this->affectedRows() : $this->numRows( $ret )
                );
 
                // Avoid the overhead of logging calls unless debug mode is enabled
@@ -1346,7 +1397,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        );
                }
 
-               return $ret;
+               return [ $ret, $lastError, $lastErrno, $recoverableSR, $recoverableCL, $reconnected ];
        }
 
        /**
@@ -1407,7 +1458,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @param string $fname
         * @throws DBTransactionStateError
         */
-       private function assertTransactionStatus( $sql, $fname ) {
+       private function assertQueryIsCurrentlyAllowed( $sql, $fname ) {
                $verb = $this->getQueryVerb( $sql );
                if ( $verb === 'USE' ) {
                        throw new DBUnexpectedError( $this, "Got USE query; use selectDomain() instead." );
index a532ec2..5632027 100644 (file)
@@ -1167,10 +1167,13 @@ class DatabaseMssql extends Database {
 
                $database = $domain->getDatabase();
                if ( $database !== $this->getDBname() ) {
-                       $encDatabase = $this->addIdentifierQuotes( $database );
-                       $res = $this->doQuery( "USE $encDatabase" );
-                       if ( !$res ) {
-                               throw new DBExpectedError( $this, "Could not select database '$database'." );
+                       $sql = 'USE ' . $this->addIdentifierQuotes( $database );
+                       list( $res, $err, $errno ) =
+                               $this->executeQuery( $sql, __METHOD__, self::QUERY_IGNORE_DBO_TRX );
+
+                       if ( $res === false ) {
+                               $this->reportQueryError( $err, $errno, $sql, __METHOD__ );
+                               return false; // unreachable
                        }
                }
                // Update that domain fields on success (no exception thrown)
index 6d28717..b9d1df0 100644 (file)
@@ -244,11 +244,12 @@ abstract class DatabaseMysqlBase extends Database {
 
                if ( $database !== $this->getDBname() ) {
                        $sql = 'USE ' . $this->addIdentifierQuotes( $database );
-                       $ret = $this->doQuery( $sql );
-                       if ( $ret === false ) {
-                               $error = $this->lastError();
-                               $errno = $this->lastErrno();
-                               $this->reportQueryError( $error, $errno, $sql, __METHOD__ );
+                       list( $res, $err, $errno ) =
+                               $this->executeQuery( $sql, __METHOD__, self::QUERY_IGNORE_DBO_TRX );
+
+                       if ( $res === false ) {
+                               $this->reportQueryError( $err, $errno, $sql, __METHOD__ );
+                               return false; // unreachable
                        }
                }
 
@@ -952,21 +953,22 @@ abstract class DatabaseMysqlBase extends Database {
                        $gtidArg = $this->addQuotes( implode( ',', $gtidsWait ) );
                        if ( strpos( $gtidArg, ':' ) !== false ) {
                                // MySQL GTIDs, e.g "source_id:transaction_id"
-                               $res = $this->doQuery( "SELECT WAIT_FOR_EXECUTED_GTID_SET($gtidArg, $timeout)" );
+                               $sql = "SELECT WAIT_FOR_EXECUTED_GTID_SET($gtidArg, $timeout)";
                        } else {
                                // MariaDB GTIDs, e.g."domain:server:sequence"
-                               $res = $this->doQuery( "SELECT MASTER_GTID_WAIT($gtidArg, $timeout)" );
+                               $sql = "SELECT MASTER_GTID_WAIT($gtidArg, $timeout)";
                        }
                } else {
                        // Wait on the binlog coordinates
                        $encFile = $this->addQuotes( $pos->getLogFile() );
                        $encPos = intval( $pos->getLogPosition()[$pos::CORD_EVENT] );
-                       $res = $this->doQuery( "SELECT MASTER_POS_WAIT($encFile, $encPos, $timeout)" );
+                       $sql = "SELECT MASTER_POS_WAIT($encFile, $encPos, $timeout)";
                }
 
+               list( $res, $err ) = $this->executeQuery( $sql, __METHOD__, self::QUERY_IGNORE_DBO_TRX );
                $row = $res ? $this->fetchRow( $res ) : false;
                if ( !$row ) {
-                       throw new DBExpectedError( $this, "Replication wait failed: {$this->lastError()}" );
+                       throw new DBExpectedError( $this, "Replication wait failed: {$err}" );
                }
 
                // Result can be NULL (error), -1 (timeout), or 0+ per the MySQL manual
index 90e30fa..09a8090 100644 (file)
@@ -115,6 +115,8 @@ interface IDatabase {
        const QUERY_PSEUDO_PERMANENT = 2;
        /** @var int Enforce that a query does not make effective writes */
        const QUERY_REPLICA_ROLE = 4;
+       /** @var int Ignore the current presence of any DBO_TRX flag */
+       const QUERY_IGNORE_DBO_TRX = 8;
 
        /** @var bool Parameter to unionQueries() for UNION ALL */
        const UNION_ALL = true;
index 8078e2e..048b567 100644 (file)
@@ -270,8 +270,6 @@ class DeleteLogFormatter extends LogFormatter {
                                }
                        }
 
-                       $old = $this->parseBitField( $rawParams['6::ofield'] );
-                       $new = $this->parseBitField( $rawParams['7::nfield'] );
                        if ( !is_array( $rawParams['5::ids'] ) ) {
                                $rawParams['5::ids'] = explode( ',', $rawParams['5::ids'] );
                        }
@@ -279,8 +277,6 @@ class DeleteLogFormatter extends LogFormatter {
                        $params = [
                                '::type' => $rawParams['4::type'],
                                ':array:ids' => $rawParams['5::ids'],
-                               ':assoc:old' => [ 'bitmask' => $old ],
-                               ':assoc:new' => [ 'bitmask' => $new ],
                        ];
 
                        static $fields = [
@@ -289,9 +285,20 @@ class DeleteLogFormatter extends LogFormatter {
                                Revision::DELETED_USER => 'user',
                                Revision::DELETED_RESTRICTED => 'restricted',
                        ];
-                       foreach ( $fields as $bit => $key ) {
-                               $params[':assoc:old'][$key] = (bool)( $old & $bit );
-                               $params[':assoc:new'][$key] = (bool)( $new & $bit );
+
+                       if ( isset( $rawParams['6::ofield'] ) ) {
+                               $old = $this->parseBitField( $rawParams['6::ofield'] );
+                               $params[':assoc:old'] = [ 'bitmask' => $old ];
+                               foreach ( $fields as $bit => $key ) {
+                                       $params[':assoc:old'][$key] = (bool)( $old & $bit );
+                               }
+                       }
+                       if ( isset( $rawParams['7::nfield'] ) ) {
+                               $new = $this->parseBitField( $rawParams['7::nfield'] );
+                               $params[':assoc:new'] = [ 'bitmask' => $new ];
+                               foreach ( $fields as $bit => $key ) {
+                                       $params[':assoc:new'][$key] = (bool)( $new & $bit );
+                               }
                        }
                } elseif ( $subtype === 'restore' ) {
                        $rawParams = $entry->getParameters();
index e929ed8..12cfe83 100644 (file)
@@ -935,7 +935,7 @@ EOT
                                ) . "\n"
                        );
 
-               };
+               }
                $out->addHTML( Html::closeElement( 'ul' ) . "\n" );
                $res->free();
 
index 8413054..f3d8d03 100644 (file)
@@ -109,7 +109,7 @@ class LayeredParameterizedPassword extends ParameterizedPassword {
                foreach ( $this->config['types'] as $i => $type ) {
                        if ( $i == 0 ) {
                                continue;
-                       };
+                       }
 
                        // Construct pseudo-hash based on params and arguments
                        /** @var ParameterizedPassword $passObj */
index 031541b..015c828 100644 (file)
@@ -617,7 +617,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        'raw',
                ] as $member ) {
                        $options[$member] = $this->{$member};
-               };
+               }
 
                $summary[] = [
                        'options' => $options,
index bdcb17b..e5dfceb 100644 (file)
@@ -1822,8 +1822,7 @@ class User implements IDBAccessObject, UserIdentity {
                        $fromReplica
                );
 
-               if ( $block instanceof AbstractBlock ) {
-                       wfDebug( __METHOD__ . ": Found block.\n" );
+               if ( $block ) {
                        $this->mBlock = $block;
                        $this->mBlockedby = $block->getByName();
                        $this->mBlockreason = $block->getReason();
index c99aa15..0b5cdf9 100644 (file)
@@ -76,7 +76,7 @@ class ImportTextFiles extends Maintenance {
                                        $this->fatalError( "Fatal error: The file '$arg' does not exist!" );
                                }
                        }
-               };
+               }
 
                $count = count( $files );
                $this->output( "Importing $count pages...\n" );
index 198c820..70a8163 100644 (file)
@@ -17,6 +17,7 @@
 
        &-body {
                max-height: 70vh;
+               min-width: 100%;
        }
 
        &-footer {
index 085e22b..ab75653 100644 (file)
@@ -267,11 +267,6 @@ OO.inheritClass( FilterTagMultiselectWidget, OO.ui.MenuTagMultiselectWidget );
 
 /* Methods */
 
-/**
- * Override parent method to avoid unnecessary resize events.
- */
-FilterTagMultiselectWidget.prototype.updateIfHeightChanged = function () { };
-
 /**
  * Respond to view select widget choose event
  *
index bd6df5f..3e4531c 100644 (file)
@@ -160,12 +160,13 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
 class ResourceLoaderFileTestModule extends ResourceLoaderFileModule {
        protected $lessVars = [];
 
-       public function __construct( $options = [], $test = [] ) {
-               parent::__construct( $options );
-
-               foreach ( $test as $key => $value ) {
-                       $this->$key = $value;
+       public function __construct( $options = [] ) {
+               if ( isset( $options['lessVars'] ) ) {
+                       $this->lessVars = $options['lessVars'];
+                       unset( $options['lessVars'] );
                }
+
+               parent::__construct( $options );
        }
 
        public function getLessVars( ResourceLoaderContext $context ) {
index f4bab02..6bbdd3b 100644 (file)
@@ -1,10 +1,8 @@
 <?php
 
 /**
- * FIXME Temporary disabled per T225244
  * @group API
  * @group medium
- * @group Broken
  *
  * @covers ApiQueryLanguageinfo
  */
@@ -27,6 +25,7 @@ class ApiQueryLanguageinfoTest extends ApiTestCase {
                                }
                        }
                );
+               Language::clearCaches();
        }
 
        private function doQuery( array $params, $microtimeFunction = null ): array {
diff --git a/tests/phpunit/includes/block/CompositeBlockTest.php b/tests/phpunit/includes/block/CompositeBlockTest.php
new file mode 100644 (file)
index 0000000..5cd86b8
--- /dev/null
@@ -0,0 +1,254 @@
+<?php
+
+use MediaWiki\Block\BlockRestrictionStore;
+use MediaWiki\Block\CompositeBlock;
+use MediaWiki\Block\Restriction\PageRestriction;
+use MediaWiki\Block\Restriction\NamespaceRestriction;
+use MediaWiki\Block\SystemBlock;
+use MediaWiki\MediaWikiServices;
+
+/**
+ * @group Database
+ * @group Blocking
+ * @coversDefaultClass \MediaWiki\Block\CompositeBlock
+ */
+class CompositeBlockTest extends MediaWikiLangTestCase {
+       private function getPartialBlocks() {
+               $sysopId = $this->getTestSysop()->getUser()->getId();
+
+               $userBlock = new Block( [
+                       'address' => $this->getTestUser()->getUser(),
+                       'by' => $sysopId,
+                       'sitewide' => false,
+               ] );
+               $ipBlock = new Block( [
+                       'address' => '127.0.0.1',
+                       'by' => $sysopId,
+                       'sitewide' => false,
+               ] );
+
+               $userBlock->insert();
+               $ipBlock->insert();
+
+               return [
+                       'user' => $userBlock,
+                       'ip' => $ipBlock,
+               ];
+       }
+
+       private function deleteBlocks( $blocks ) {
+               foreach ( $blocks as $block ) {
+                       $block->delete();
+               }
+       }
+
+       /**
+        * @covers ::__construct
+        * @dataProvider provideTestStrictestParametersApplied
+        */
+       public function testStrictestParametersApplied( $blocks, $expected ) {
+               $this->setMwGlobals( [
+                       'wgBlockDisablesLogin' => false,
+                       'wgBlockAllowsUTEdit' => true,
+               ] );
+
+               $block = new CompositeBlock( [
+                       'originalBlocks' => $blocks,
+               ] );
+
+               $this->assertSame( $expected[ 'hideName' ], $block->getHideName() );
+               $this->assertSame( $expected[ 'sitewide' ], $block->isSitewide() );
+               $this->assertSame( $expected[ 'blockEmail' ], $block->isEmailBlocked() );
+               $this->assertSame( $expected[ 'allowUsertalk' ], $block->isUsertalkEditAllowed() );
+       }
+
+       public static function provideTestStrictestParametersApplied() {
+               return [
+                       'Sitewide block and partial block' => [
+                               [
+                                       new Block( [
+                                               'sitewide' => false,
+                                               'blockEmail' => true,
+                                               'allowUsertalk' => true,
+                                       ] ),
+                                       new Block( [
+                                               'sitewide' => true,
+                                               'blockEmail' => false,
+                                               'allowUsertalk' => false,
+                                       ] ),
+                               ],
+                               [
+                                       'hideName' => false,
+                                       'sitewide' => true,
+                                       'blockEmail' => true,
+                                       'allowUsertalk' => false,
+                               ],
+                       ],
+                       'Partial block and system block' => [
+                               [
+                                       new Block( [
+                                               'sitewide' => false,
+                                               'blockEmail' => true,
+                                               'allowUsertalk' => false,
+                                       ] ),
+                                       new SystemBlock( [
+                                               'systemBlock' => 'proxy',
+                                       ] ),
+                               ],
+                               [
+                                       'hideName' => false,
+                                       'sitewide' => true,
+                                       'blockEmail' => true,
+                                       'allowUsertalk' => false,
+                               ],
+                       ],
+                       'System block and user name hiding block' => [
+                               [
+                                       new Block( [
+                                               'hideName' => true,
+                                               'sitewide' => true,
+                                               'blockEmail' => true,
+                                               'allowUsertalk' => false,
+                                       ] ),
+                                       new SystemBlock( [
+                                               'systemBlock' => 'proxy',
+                                       ] ),
+                               ],
+                               [
+                                       'hideName' => true,
+                                       'sitewide' => true,
+                                       'blockEmail' => true,
+                                       'allowUsertalk' => false,
+                               ],
+                       ],
+                       'Two lenient partial blocks' => [
+                               [
+                                       new Block( [
+                                               'sitewide' => false,
+                                               'blockEmail' => false,
+                                               'allowUsertalk' => true,
+                                       ] ),
+                                       new Block( [
+                                               'sitewide' => false,
+                                               'blockEmail' => false,
+                                               'allowUsertalk' => true,
+                                       ] ),
+                               ],
+                               [
+                                       'hideName' => false,
+                                       'sitewide' => false,
+                                       'blockEmail' => false,
+                                       'allowUsertalk' => true,
+                               ],
+                       ],
+               ];
+       }
+
+       /**
+        * @covers ::appliesToTitle
+        */
+       public function testBlockAppliesToTitle() {
+               $this->setMwGlobals( [
+                       'wgBlockDisablesLogin' => false,
+               ] );
+
+               $blocks = $this->getPartialBlocks();
+
+               $block = new CompositeBlock( [
+                       'originalBlocks' => $blocks,
+               ] );
+
+               $pageFoo = $this->getExistingTestPage( 'Foo' );
+               $pageBar = $this->getExistingTestPage( 'User:Bar' );
+
+               $this->getBlockRestrictionStore()->insert( [
+                       new PageRestriction( $blocks[ 'user' ]->getId(), $pageFoo->getId() ),
+                       new NamespaceRestriction( $blocks[ 'ip' ]->getId(), NS_USER ),
+               ] );
+
+               $this->assertTrue( $block->appliesToTitle( $pageFoo->getTitle() ) );
+               $this->assertTrue( $block->appliesToTitle( $pageBar->getTitle() ) );
+
+               $this->deleteBlocks( $blocks );
+       }
+
+       /**
+        * @covers ::appliesToUsertalk
+        * @covers ::appliesToPage
+        * @covers ::appliesToNamespace
+        */
+       public function testBlockAppliesToUsertalk() {
+               $this->setMwGlobals( [
+                       'wgBlockAllowsUTEdit' => true,
+                       'wgBlockDisablesLogin' => false,
+               ] );
+
+               $blocks = $this->getPartialBlocks();
+
+               $block = new CompositeBlock( [
+                       'originalBlocks' => $blocks,
+               ] );
+
+               $title = $blocks[ 'user' ]->getTarget()->getTalkPage();
+               $page = $this->getExistingTestPage( 'User talk:' . $title->getText() );
+
+               $this->getBlockRestrictionStore()->insert( [
+                       new PageRestriction( $blocks[ 'user' ]->getId(), $page->getId() ),
+                       new NamespaceRestriction( $blocks[ 'ip' ]->getId(), NS_USER ),
+               ] );
+
+               $this->assertTrue( $block->appliesToUsertalk( $blocks[ 'user' ]->getTarget()->getTalkPage() ) );
+
+               $this->deleteBlocks( $blocks );
+       }
+
+       /**
+        * @covers ::appliesToRight
+        * @dataProvider provideTestBlockAppliesToRight
+        */
+       public function testBlockAppliesToRight( $blocks, $right, $expected ) {
+               $this->setMwGlobals( [
+                       'wgBlockDisablesLogin' => false,
+               ] );
+
+               $block = new CompositeBlock( [
+                       'originalBlocks' => $blocks,
+               ] );
+
+               $this->assertSame( $block->appliesToRight( $right ), $expected );
+       }
+
+       public static function provideTestBlockAppliesToRight() {
+               return [
+                       'Read is not blocked' => [
+                               [
+                                       new Block(),
+                                       new Block(),
+                               ],
+                               'read',
+                               false,
+                       ],
+                       'Email is blocked if blocked by any blocks' => [
+                               [
+                                       new Block( [
+                                               'blockEmail' => true,
+                                       ] ),
+                                       new Block( [
+                                               'blockEmail' => false,
+                                       ] ),
+                               ],
+                               'sendemail',
+                               true,
+                       ],
+               ];
+       }
+
+       /**
+        * Get an instance of BlockRestrictionStore
+        *
+        * @return BlockRestrictionStore
+        */
+       protected function getBlockRestrictionStore() : BlockRestrictionStore {
+               return MediaWikiServices::getInstance()->getBlockRestrictionStore();
+       }
+}
index ce07f78..8f8dde5 100644 (file)
@@ -48,7 +48,7 @@ class JobQueueTest extends MediaWikiTestCase {
                        } catch ( MWException $e ) {
                                // unsupported?
                                // @todo What if it was another error?
-                       };
+                       }
                }
        }
 
index c0d2555..0e133d8 100644 (file)
@@ -1878,7 +1878,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 
        /**
         * @expectedException \Wikimedia\Rdbms\DBTransactionStateError
-        * @covers \Wikimedia\Rdbms\Database::assertTransactionStatus
+        * @covers \Wikimedia\Rdbms\Database::assertQueryIsCurrentlyAllowed
         */
        public function testTransactionErrorState1() {
                $wrapper = TestingAccessWrapper::newFromObject( $this->database );
index 0e6855d..6648c31 100644 (file)
@@ -457,7 +457,7 @@ class DeleteLogFormatterTest extends LogFormatterTestCase {
                                ],
                        ],
 
-                       // Legacy format
+                       // Legacy formats
                        [
                                [
                                        'type' => 'suppress',
@@ -495,6 +495,27 @@ class DeleteLogFormatterTest extends LogFormatterTestCase {
                                        ],
                                ],
                        ],
+                       [
+                               [
+                                       'type' => 'delete',
+                                       'action' => 'revision',
+                                       'comment' => 'Old rows might lack ofield/nfield (T224815)',
+                                       'namespace' => NS_MAIN,
+                                       'title' => 'Page',
+                                       'params' => [
+                                               'oldid',
+                                               '1234',
+                                       ],
+                               ],
+                               [
+                                       'legacy' => true,
+                                       'text' => 'User changed visibility of revisions on page Page',
+                                       'api' => [
+                                               'type' => 'oldid',
+                                               'ids' => [ '1234' ],
+                                       ],
+                               ],
+                       ]
                ];
        }
 
index fbef12e..2aa0d27 100644 (file)
@@ -347,7 +347,6 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
                $module = new ResourceLoaderFileTestModule( [
                        'localBasePath' => $basePath,
                        'styles' => [ 'styles.less' ],
-               ], [
                        'lessVars' => [ 'foo' => '2px', 'Foo' => '#eeeeee' ]
                ] );
                $module->setName( 'test.less' );
@@ -355,27 +354,48 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
                $this->assertStringEqualsFile( $basePath . '/styles.css', $styles['all'] );
        }
 
+       public function provideGetVersionHash() {
+               $a = [];
+               $b = [
+                       'lessVars' => [ 'key' => 'value' ],
+               ];
+               yield 'with and without Less variables' => [ $a, $b, false ];
+
+               $a = [
+                       'lessVars' => [ 'key' => 'value1' ],
+               ];
+               $b = [
+                       'lessVars' => [ 'key' => 'value2' ],
+               ];
+               yield 'different Less variables' => [ $a, $b, false ];
+
+               $x = [
+                       'lessVars' => [ 'key' => 'value' ],
+               ];
+               yield 'identical Less variables' => [ $x, $x, true ];
+       }
+
        /**
+        * @dataProvider provideGetVersionHash
         * @covers ResourceLoaderFileModule::getDefinitionSummary
         * @covers ResourceLoaderFileModule::getFileHashes
         */
-       public function testGetVersionHash() {
+       public function testGetVersionHash( $a, $b, $isEqual ) {
                $context = $this->getResourceLoaderContext();
 
-               // Less variables
-               $module = new ResourceLoaderFileTestModule();
-               $version = $module->getVersionHash( $context );
-               $module = new ResourceLoaderFileTestModule( [], [
-                       'lessVars' => [ 'key' => 'value' ],
-               ] );
-               $this->assertNotEquals(
-                       $version,
-                       $module->getVersionHash( $context ),
-                       'Using less variables is significant'
+               $moduleA = new ResourceLoaderFileTestModule( $a );
+               $versionA = $moduleA->getVersionHash( $context );
+               $moduleB = new ResourceLoaderFileTestModule( $b );
+               $versionB = $moduleB->getVersionHash( $context );
+
+               $this->assertSame(
+                       $isEqual,
+                       ( $versionA === $versionB ),
+                       'Whether versions hashes are equal'
                );
        }
 
-       public function providerGetScriptPackageFiles() {
+       public function provideGetScriptPackageFiles() {
                $basePath = __DIR__ . '/../../data/resourceloader';
                $base = [ 'localBasePath' => $basePath ];
                $commentScript = file_get_contents( "$basePath/script-comment.js" );
@@ -559,7 +579,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
        }
 
        /**
-        * @dataProvider providerGetScriptPackageFiles
+        * @dataProvider provideGetScriptPackageFiles
         * @covers ResourceLoaderFileModule::getScript
         * @covers ResourceLoaderFileModule::getPackageFiles
         * @covers ResourceLoaderFileModule::expandPackageFiles
index 55a29e3..b0c0fec 100644 (file)
@@ -2,6 +2,7 @@
 
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Block\DatabaseBlock;
+use MediaWiki\Block\CompositeBlock;
 use MediaWiki\Block\SystemBlock;
 
 /**
@@ -141,6 +142,34 @@ class PasswordResetTest extends MediaWikiTestCase {
                                'globalBlock' => null,
                                'isAllowed' => false,
                        ],
+                       'blocked with multiple blocks, all allowing password reset' => [
+                               'passwordResetRoutes' => [ 'username' => true ],
+                               'enableEmail' => true,
+                               'allowsAuthenticationDataChange' => true,
+                               'canEditPrivate' => true,
+                               'block' => new CompositeBlock( [
+                                       'originalBlocks' => [
+                                               new SystemBlock( [ 'systemBlock' => 'wgSoftBlockRanges', 'anonOnly' => true ] ),
+                                               new Block( [] ),
+                                       ]
+                               ] ),
+                               'globalBlock' => null,
+                               'isAllowed' => true,
+                       ],
+                       'blocked with multiple blocks, not all allowing password reset' => [
+                               'passwordResetRoutes' => [ 'username' => true ],
+                               'enableEmail' => true,
+                               'allowsAuthenticationDataChange' => true,
+                               'canEditPrivate' => true,
+                               'block' => new CompositeBlock( [
+                                       'originalBlocks' => [
+                                               new SystemBlock( [ 'systemBlock' => 'wgSoftBlockRanges', 'anonOnly' => true ] ),
+                                               new SystemBlock( [ 'systemBlock' => 'proxy' ] ),
+                                       ]
+                               ] ),
+                               'globalBlock' => null,
+                               'isAllowed' => false,
+                       ],
                        'all OK' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => true,
index 14ddd9f..79c6e96 100644 (file)
@@ -4,6 +4,7 @@ define( 'NS_UNITTEST', 5600 );
 define( 'NS_UNITTEST_TALK', 5601 );
 
 use MediaWiki\Block\DatabaseBlock;
+use MediaWiki\Block\CompositeBlock;
 use MediaWiki\Block\Restriction\PageRestriction;
 use MediaWiki\Block\Restriction\NamespaceRestriction;
 use MediaWiki\Block\SystemBlock;
@@ -66,6 +67,15 @@ class UserTest extends MediaWikiTestCase {
                ];
        }
 
+       private function setSessionUser( User $user, WebRequest $request ) {
+               $this->setMwGlobals( 'wgUser', $user );
+               RequestContext::getMain()->setUser( $user );
+               RequestContext::getMain()->setRequest( $request );
+               TestingAccessWrapper::newFromObject( $user )->mRequest = $request;
+               $request->getSession()->setUser( $user );
+               $this->overrideMwServices();
+       }
+
        /**
         * @covers User::getGroupPermissions
         */
@@ -779,28 +789,20 @@ class UserTest extends MediaWikiTestCase {
         * @covers User::getBlockedStatus
         */
        public function testSoftBlockRanges() {
-               $setSessionUser = function ( User $user, WebRequest $request ) {
-                       $this->setMwGlobals( 'wgUser', $user );
-                       RequestContext::getMain()->setUser( $user );
-                       RequestContext::getMain()->setRequest( $request );
-                       TestingAccessWrapper::newFromObject( $user )->mRequest = $request;
-                       $request->getSession()->setUser( $user );
-                       $this->overrideMwServices();
-               };
                $this->setMwGlobals( 'wgSoftBlockRanges', [ '10.0.0.0/8' ] );
 
                // IP isn't in $wgSoftBlockRanges
                $wgUser = new User();
                $request = new FauxRequest();
                $request->setIP( '192.168.0.1' );
-               $setSessionUser( $wgUser, $request );
+               $this->setSessionUser( $wgUser, $request );
                $this->assertNull( $wgUser->getBlock() );
 
                // IP is in $wgSoftBlockRanges
                $wgUser = new User();
                $request = new FauxRequest();
                $request->setIP( '10.20.30.40' );
-               $setSessionUser( $wgUser, $request );
+               $this->setSessionUser( $wgUser, $request );
                $block = $wgUser->getBlock();
                $this->assertInstanceOf( SystemBlock::class, $block );
                $this->assertSame( 'wgSoftBlockRanges', $block->getSystemBlockType() );
@@ -809,7 +811,7 @@ class UserTest extends MediaWikiTestCase {
                $wgUser = $this->getTestUser()->getUser();
                $request = new FauxRequest();
                $request->setIP( '10.20.30.40' );
-               $setSessionUser( $wgUser, $request );
+               $this->setSessionUser( $wgUser, $request );
                $this->assertFalse( $wgUser->isAnon(), 'sanity check' );
                $this->assertNull( $wgUser->getBlock() );
        }
@@ -1316,6 +1318,35 @@ class UserTest extends MediaWikiTestCase {
                $this->assertFalse( $user->isBlockedFrom( $ut ) );
        }
 
+       /**
+        * @covers User::getBlockedStatus
+        */
+       public function testCompositeBlocks() {
+               $user = $this->getMutableTestUser()->getUser();
+               $request = $user->getRequest();
+               $this->setSessionUser( $user, $request );
+
+               $ipBlock = new Block( [
+                       'address' => $user->getRequest()->getIP(),
+                       'by' => $this->getTestSysop()->getUser()->getId(),
+                       'createAccount' => true,
+               ] );
+               $ipBlock->insert();
+
+               $userBlock = new Block( [
+                       'address' => $user,
+                       'by' => $this->getTestSysop()->getUser()->getId(),
+                       'createAccount' => false,
+               ] );
+               $userBlock->insert();
+
+               $block = $user->getBlock();
+               $this->assertInstanceOf( CompositeBlock::class, $block );
+               $this->assertTrue( $block->isCreateAccountBlocked() );
+               $this->assertTrue( $block->appliesToPasswordReset() );
+               $this->assertTrue( $block->appliesToNamespace( NS_MAIN ) );
+       }
+
        /**
         * @covers User::isBlockedFrom
         * @dataProvider provideIsBlockedFrom