Merge "Various cleanups to MediaWiki::preOutputCommit"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 19 Jun 2019 02:11:22 +0000 (02:11 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 19 Jun 2019 02:11:22 +0000 (02:11 +0000)
32 files changed:
autoload.php
includes/Permissions/PermissionManager.php
includes/Title.php
includes/actions/HistoryAction.php
includes/actions/RawAction.php
includes/api/ApiCSPReport.php
includes/api/ApiMain.php
includes/debug/logger/monolog/CeeFormatter.php
includes/installer/WebInstallerOutput.php
includes/libs/rdbms/database/DBConnRef.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMysqlBase.php
includes/libs/rdbms/database/DatabaseMysqli.php
includes/libs/rdbms/database/IDatabase.php
includes/resourceloader/ResourceLoaderCircularDependencyError.php [new file with mode: 0644]
includes/resourceloader/ResourceLoaderImageModule.php
includes/resourceloader/ResourceLoaderLessVarFileModule.php
includes/resourceloader/ResourceLoaderStartUpModule.php
includes/upload/UploadStash.php
includes/user/BotPassword.php
maintenance/generateSitemap.php
tests/common/TestsAutoLoader.php
tests/phpunit/MediaWikiUnitTestCase.php [new file with mode: 0644]
tests/phpunit/includes/Revision/RevisionStoreTest.php
tests/phpunit/includes/Storage/NameTableStoreTest.php
tests/phpunit/includes/libs/rdbms/database/DBConnRefTest.php
tests/phpunit/includes/password/PasswordFactoryTest.php [deleted file]
tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
tests/phpunit/suite.xml
tests/phpunit/unit-tests.xml [new file with mode: 0644]
tests/phpunit/unit/includes/password/PasswordFactoryTest.php [new file with mode: 0644]
tests/phpunit/unit/initUnitTests.php [new file with mode: 0644]

index b3900a8..43440f0 100644 (file)
@@ -1231,6 +1231,7 @@ $wgAutoloadLocalClasses = [
        'ResetUserTokens' => __DIR__ . '/maintenance/resetUserTokens.php',
        'ResourceFileCache' => __DIR__ . '/includes/cache/ResourceFileCache.php',
        'ResourceLoader' => __DIR__ . '/includes/resourceloader/ResourceLoader.php',
+       'ResourceLoaderCircularDependencyError' => __DIR__ . '/includes/resourceloader/ResourceLoaderCircularDependencyError.php',
        'ResourceLoaderClientHtml' => __DIR__ . '/includes/resourceloader/ResourceLoaderClientHtml.php',
        'ResourceLoaderContext' => __DIR__ . '/includes/resourceloader/ResourceLoaderContext.php',
        'ResourceLoaderFileModule' => __DIR__ . '/includes/resourceloader/ResourceLoaderFileModule.php',
index e443803..202014f 100644 (file)
@@ -324,7 +324,7 @@ class PermissionManager {
         * Add the resulting error code to the errors array
         *
         * @param array $errors List of current errors
-        * @param array $result Result of errors
+        * @param array|string|MessageSpecifier|false $result Result of errors
         *
         * @return array List of errors
         */
index cf81b98..f69f1a4 100644 (file)
@@ -2256,7 +2256,7 @@ class Title implements LinkTarget, IDBAccessObject {
         * Add the resulting error code to the errors array
         *
         * @param array $errors List of current errors
-        * @param array $result Result of errors
+        * @param array|string|MessageSpecifier|false $result Result of errors
         *
         * @return array List of errors
         */
index 538b0a1..b1d5a50 100644 (file)
@@ -142,6 +142,7 @@ class HistoryAction extends FormlessAction {
 
        /**
         * Print the history page for an article.
+        * @return string|null
         */
        function onView() {
                $out = $this->getOutput();
@@ -151,7 +152,7 @@ class HistoryAction extends FormlessAction {
                 * Allow client caching.
                 */
                if ( $out->checkLastModified( $this->page->getTouched() ) ) {
-                       return; // Client cache fresh and headers sent, nothing more to do.
+                       return null; // Client cache fresh and headers sent, nothing more to do.
                }
 
                $this->preCacheMessages();
@@ -185,7 +186,7 @@ class HistoryAction extends FormlessAction {
                $feedType = $request->getRawVal( 'feed' );
                if ( $feedType !== null ) {
                        $this->feed( $feedType );
-                       return;
+                       return null;
                }
 
                $this->addHelpLink(
@@ -216,7 +217,7 @@ class HistoryAction extends FormlessAction {
                                ]
                        );
 
-                       return;
+                       return null;
                }
 
                $ts = $this->getTimestampFromRequest( $request );
@@ -300,6 +301,8 @@ class HistoryAction extends FormlessAction {
                        $pager->getNavigationBar()
                );
                $out->preventClickjacking( $pager->getPreventClickjacking() );
+
+               return null;
        }
 
        /**
index 505c9d5..3e4e614 100644 (file)
@@ -50,6 +50,7 @@ class RawAction extends FormlessAction {
 
        /**
         * @suppress SecurityCheck-XSS Non html mime type
+        * @return string|null
         */
        function onView() {
                $this->getOutput()->disable();
@@ -58,11 +59,11 @@ class RawAction extends FormlessAction {
                $config = $this->context->getConfig();
 
                if ( !$request->checkUrlExtension() ) {
-                       return;
+                       return null;
                }
 
                if ( $this->getOutput()->checkLastModified( $this->page->getTouched() ) ) {
-                       return; // Client cache fresh and headers sent, nothing more to do.
+                       return null; // Client cache fresh and headers sent, nothing more to do.
                }
 
                $contentType = $this->getContentType();
@@ -173,6 +174,8 @@ class RawAction extends FormlessAction {
                }
 
                echo $text;
+
+               return null;
        }
 
        /**
index 6271128..f53d2b9 100644 (file)
@@ -54,7 +54,7 @@ class ApiCSPReport extends ApiBase {
                        // XXX Is it ok to put untrusted data into log??
                        'csp-report' => $report,
                        'method' => __METHOD__,
-                       'user' => $this->getUser()->getName(),
+                       'user_id' => $this->getUser()->getId() || 'logged-out',
                        'user-agent' => $userAgent,
                        'source' => $this->getParameter( 'source' ),
                ] );
@@ -104,11 +104,11 @@ class ApiCSPReport extends ApiBase {
                        ) ||
                        (
                                isset( $report['blocked-uri'] ) &&
-                               isset( $falsePositives[$report['blocked-uri']] )
+                               $this->matchUrlPattern( $report['blocked-uri'], $falsePositives )
                        ) ||
                        (
                                isset( $report['source-file'] ) &&
-                               isset( $falsePositives[$report['source-file']] )
+                               $this->matchUrlPattern( $report['source-file'], $falsePositives )
                        )
                ) {
                        // False positive due to:
@@ -119,6 +119,39 @@ class ApiCSPReport extends ApiBase {
                return $flags;
        }
 
+       /**
+        * @param string $url
+        * @param string[] $patterns
+        * @return bool
+        */
+       private function matchUrlPattern( $url, array $patterns ) {
+               if ( isset( $patterns[ $url ] ) ) {
+                       return true;
+               }
+
+               $bits = wfParseUrl( $url );
+               unset( $bits['user'], $bits['pass'], $bits['query'], $bits['fragment'] );
+               $bits['path'] = '';
+               $serverUrl = wfAssembleUrl( $bits );
+               if ( isset( $patterns[$serverUrl] ) ) {
+                       // The origin of the url matches a pattern,
+                       // e.g. "https://example.org" matches "https://example.org/foo/b?a#r"
+                       return true;
+               }
+               foreach ( $patterns as $pattern => $val ) {
+                       // We only use this pattern if it ends in a slash, this prevents
+                       // "/foos" from matching "/foo", and "https://good.combo.bad" matching
+                       // "https://good.com".
+                       if ( substr( $pattern, -1 ) === '/' && strpos( $url, $pattern ) === 0 ) {
+                               // The pattern starts with the same as the url
+                               // e.g. "https://example.org/foo/" matches "https://example.org/foo/b?a#r"
+                               return true;
+                       }
+               }
+
+               return false;
+       }
+
        /**
         * Output an api error if post body is obviously not OK.
         */
@@ -176,15 +209,32 @@ class ApiCSPReport extends ApiBase {
                        $flagText = '[' . implode( ', ', $flags ) . ']';
                }
 
-               $blockedFile = $report['blocked-uri'] ?? 'n/a';
+               $blockedOrigin = isset( $report['blocked-uri'] )
+                       ? $this->originFromUrl( $report['blocked-uri'] )
+                       : 'n/a';
                $page = $report['document-uri'] ?? 'n/a';
-               $line = isset( $report['line-number'] ) ? ':' . $report['line-number'] : '';
+               $line = isset( $report['line-number'] )
+                       ? ':' . $report['line-number']
+                       : '';
                $warningText = $flagText .
-                       ' Received CSP report: <' . $blockedFile .
-                       '> blocked from being loaded on <' . $page . '>' . $line;
+                       ' Received CSP report: <' . $blockedOrigin . '>' .
+                       ' blocked from being loaded on <' . $page . '>' . $line;
                return $warningText;
        }
 
+       /**
+        * @param string $url
+        * @return string
+        */
+       private function originFromUrl( $url ) {
+               $bits = wfParseUrl( $url );
+               unset( $bits['user'], $bits['pass'], $bits['query'], $bits['fragment'] );
+               $bits['path'] = '';
+               $serverUrl = wfAssembleUrl( $bits );
+               // e.g. "https://example.org" from "https://example.org/foo/b?a#r"
+               return $serverUrl;
+       }
+
        /**
         * Stop processing the request, and output/log an error
         *
index b845c57..ed17e07 100644 (file)
@@ -1644,7 +1644,7 @@ class ApiMain extends ApiBase {
                        '$schema' => '/mediawiki/api/request/0.0.1',
                        'meta' => [
                                'request_id' => WebRequest::getRequestId(),
-                               'id' => UIDGenerator::newUUIDv1(),
+                               'id' => UIDGenerator::newUUIDv4(),
                                'dt' => wfTimestamp( TS_ISO_8601 ),
                                'domain' => $this->getConfig()->get( 'ServerName' ),
                                'stream' => 'mediawiki.api-request'
index 4b0c6cb..dc50543 100644 (file)
@@ -15,7 +15,7 @@ class CeeFormatter extends LogstashFormatter {
        /**
         * Format records with a cee cookie
         * @param array $record
-        * @return array
+        * @return mixed
         */
        public function format( array $record ) {
                return "@cee: " . parent::format( $record );
index 914a69e..65e7457 100644 (file)
@@ -285,7 +285,7 @@ class WebInstallerOutput {
 <?php echo Html::openElement( 'body', [ 'class' => $this->getLanguage()->getDir() ] ) . "\n"; ?>
 <div id="mw-page-base"></div>
 <div id="mw-head-base"></div>
-<div id="content" class="mw-body">
+<div id="content" class="mw-body" role="main">
 <div id="bodyContent" class="mw-body-content">
 
 <h1><?php $this->outputTitle(); ?></h1>
index 841a7ba..8af6bb3 100644 (file)
@@ -35,7 +35,7 @@ class DBConnRef implements IDatabase {
        public function __construct( ILoadBalancer $lb, $conn, $role ) {
                $this->lb = $lb;
                $this->role = $role;
-               if ( $conn instanceof Database ) {
+               if ( $conn instanceof IDatabase && !( $conn instanceof DBConnRef ) ) {
                        $this->conn = $conn; // live handle
                } elseif ( is_array( $conn ) && count( $conn ) >= 4 && $conn[self::FLD_DOMAIN] !== false ) {
                        $this->params = $conn;
@@ -461,7 +461,7 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
-       public function buildLike() {
+       public function buildLike( $param ) {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
index b7b45bd..5451476 100644 (file)
@@ -2718,11 +2718,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $s );
        }
 
-       public function buildLike() {
-               $params = func_get_args();
-
-               if ( count( $params ) > 0 && is_array( $params[0] ) ) {
-                       $params = $params[0];
+       public function buildLike( $param, ...$params ) {
+               if ( is_array( $param ) ) {
+                       $params = $param;
+               } else {
+                       $params = func_get_args();
                }
 
                $s = '';
index e871ab9..b5f83da 100644 (file)
@@ -369,7 +369,7 @@ abstract class DatabaseMysqlBase extends Database {
         * Fetch a result row as an associative and numeric array
         *
         * @param resource $res Raw result
-        * @return array
+        * @return array|false
         */
        abstract protected function mysqlFetchArray( $res );
 
index 937f381..703c64d 100644 (file)
@@ -203,7 +203,7 @@ class DatabaseMysqli extends DatabaseMysqlBase {
 
        /**
         * @param mysqli_result $res
-        * @return bool
+        * @return array|false
         */
        protected function mysqlFetchArray( $res ) {
                $array = $res->fetch_array();
index 802af15..faed1bf 100644 (file)
@@ -1220,9 +1220,10 @@ interface IDatabase {
         *   $query .= $dbr->buildLike( $pattern );
         *
         * @since 1.16
+        * @param array[]|string|LikeMatch $param
         * @return string Fully built LIKE statement
         */
-       public function buildLike();
+       public function buildLike( $param );
 
        /**
         * Returns a token for buildLike() that denotes a '_' to be used in a LIKE query
diff --git a/includes/resourceloader/ResourceLoaderCircularDependencyError.php b/includes/resourceloader/ResourceLoaderCircularDependencyError.php
new file mode 100644 (file)
index 0000000..7cd53fe
--- /dev/null
@@ -0,0 +1,26 @@
+<?php
+/**
+ *
+ * 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
+ */
+
+/**
+ * @internal For use by ResourceLoaderStartUpModule only
+ */
+class ResourceLoaderCircularDependencyError extends Exception {
+}
index db292cc..90b18eb 100644 (file)
@@ -39,7 +39,7 @@ class ResourceLoaderImageModule extends ResourceLoaderModule {
 
        protected $origin = self::ORIGIN_CORE_SITEWIDE;
 
-       /** @var ResourceLoaderImage[]|null */
+       /** @var ResourceLoaderImage[][]|null */
        protected $imageObjects = null;
        /** @var array */
        protected $images = [];
index c4e517a..0269ec3 100644 (file)
@@ -33,7 +33,7 @@ class ResourceLoaderLessVarFileModule extends ResourceLoaderFileModule {
         *
         * @param string $blob
         * @param array $exclusions
-        * @return array $blob
+        * @return object $blob
         */
        protected function excludeMessagesFromBlob( $blob, $exclusions ) {
                $data = json_decode( $blob, true );
index efed418..4ce4498 100644 (file)
@@ -124,29 +124,50 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
         *
         * @param array $registryData
         * @param string $moduleName
+        * @param string[] $handled Internal parameter for recursion. (Optional)
         * @return array
+        * @throws ResourceLoaderCircularDependencyError
         */
-       protected static function getImplicitDependencies( array $registryData, $moduleName ) {
+       protected static function getImplicitDependencies(
+               array $registryData,
+               $moduleName,
+               array $handled = []
+       ) {
                static $dependencyCache = [];
 
-               // The list of implicit dependencies won't be altered, so we can
-               // cache them without having to worry.
+               // No modules will be added or changed server-side after this point,
+               // so we can safely cache parts of the tree for re-use.
                if ( !isset( $dependencyCache[$moduleName] ) ) {
                        if ( !isset( $registryData[$moduleName] ) ) {
-                               // Dependencies may not exist
-                               $dependencyCache[$moduleName] = [];
+                               // Unknown module names are allowed here, this is only an optimisation.
+                               // Checks for illegal and unknown dependencies happen as PHPUnit structure tests,
+                               // and also client-side at run-time.
+                               $flat = [];
                        } else {
                                $data = $registryData[$moduleName];
-                               $dependencyCache[$moduleName] = $data['dependencies'];
+                               $flat = $data['dependencies'];
 
+                               // Prevent recursion
+                               $handled[] = $moduleName;
                                foreach ( $data['dependencies'] as $dependency ) {
-                                       // Recursively get the dependencies of the dependencies
-                                       $dependencyCache[$moduleName] = array_merge(
-                                               $dependencyCache[$moduleName],
-                                               self::getImplicitDependencies( $registryData, $dependency )
-                                       );
+                                       if ( in_array( $dependency, $handled, true ) ) {
+                                               // If we encounter a circular dependency, then stop the optimiser and leave the
+                                               // original dependencies array unmodified. Circular dependencies are not
+                                               // supported in ResourceLoader. Awareness of them exists here so that we can
+                                               // optimise the registry when it isn't broken, and otherwise transport the
+                                               // registry unchanged. The client will handle this further.
+                                               throw new ResourceLoaderCircularDependencyError();
+                                       } else {
+                                               // Recursively add the dependencies of the dependencies
+                                               $flat = array_merge(
+                                                       $flat,
+                                                       self::getImplicitDependencies( $registryData, $dependency, $handled )
+                                               );
+                                       }
                                }
                        }
+
+                       $dependencyCache[$moduleName] = $flat;
                }
 
                return $dependencyCache[$moduleName];
@@ -173,10 +194,16 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
        public static function compileUnresolvedDependencies( array &$registryData ) {
                foreach ( $registryData as $name => &$data ) {
                        $dependencies = $data['dependencies'];
-                       foreach ( $data['dependencies'] as $dependency ) {
-                               $implicitDependencies = self::getImplicitDependencies( $registryData, $dependency );
-                               $dependencies = array_diff( $dependencies, $implicitDependencies );
+                       try {
+                               foreach ( $data['dependencies'] as $dependency ) {
+                                       $implicitDependencies = self::getImplicitDependencies( $registryData, $dependency );
+                                       $dependencies = array_diff( $dependencies, $implicitDependencies );
+                               }
+                       } catch ( ResourceLoaderCircularDependencyError $err ) {
+                               // Leave unchanged
+                               $dependencies = $data['dependencies'];
                        }
+
                        // Rebuild keys
                        $data['dependencies'] = array_values( $dependencies );
                }
index d39975d..215bd20 100644 (file)
@@ -433,7 +433,7 @@ class UploadStash {
         * List all files in the stash.
         *
         * @throws UploadStashNotLoggedInException
-        * @return array
+        * @return array|false
         */
        public function listFiles() {
                if ( !$this->isLoggedIn ) {
index 6db219d..df5edef 100644 (file)
@@ -21,7 +21,7 @@
 use MediaWiki\Auth\AuthenticationResponse;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Session\BotPasswordSessionProvider;
-use Wikimedia\Rdbms\IMaintainableDatabase;
+use Wikimedia\Rdbms\IDatabase;
 
 /**
  * Utility class for bot passwords
@@ -71,7 +71,7 @@ class BotPassword implements IDBAccessObject {
        /**
         * Get a database connection for the bot passwords database
         * @param int $db Index of the connection to get, e.g. DB_MASTER or DB_REPLICA.
-        * @return IMaintainableDatabase
+        * @return IDatabase
         */
        public static function getDB( $db ) {
                global $wgBotPasswordsCluster, $wgBotPasswordsDatabase;
index 7d43f21..05dd0d0 100644 (file)
@@ -27,6 +27,7 @@
  */
 
 use MediaWiki\MediaWikiServices;
+use Wikimedia\Rdbms\IResultWrapper;
 
 require_once __DIR__ . '/Maintenance.php';
 
@@ -299,7 +300,7 @@ class GenerateSitemap extends Maintenance {
         * Return a database resolution of all the pages in a given namespace
         *
         * @param int $namespace Limit the query to this namespace
-        * @return Resource
+        * @return IResultWrapper
         */
        function getPageRes( $namespace ) {
                return $this->dbr->select( 'page',
index 861111a..3b643a5 100644 (file)
@@ -60,6 +60,7 @@ $wgAutoloadClasses += [
        'MediaWikiPHPUnitResultPrinter' => "$testDir/phpunit/MediaWikiPHPUnitResultPrinter.php",
        'MediaWikiPHPUnitTestListener' => "$testDir/phpunit/MediaWikiPHPUnitTestListener.php",
        'MediaWikiTestCase' => "$testDir/phpunit/MediaWikiTestCase.php",
+       'MediaWikiUnitTestCase' => "$testDir/phpunit/MediaWikiUnitTestCase.php",
        'MediaWikiTestResult' => "$testDir/phpunit/MediaWikiTestResult.php",
        'MediaWikiTestRunner' => "$testDir/phpunit/MediaWikiTestRunner.php",
        'PHPUnit4And6Compat' => "$testDir/phpunit/PHPUnit4And6Compat.php",
diff --git a/tests/phpunit/MediaWikiUnitTestCase.php b/tests/phpunit/MediaWikiUnitTestCase.php
new file mode 100644 (file)
index 0000000..407be20
--- /dev/null
@@ -0,0 +1,29 @@
+<?php
+/**
+ * Base class for MediaWiki unit tests.
+ *
+ * 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
+ * @ingroup Testing
+ */
+
+use PHPUnit\Framework\TestCase;
+
+abstract class MediaWikiUnitTestCase extends TestCase {
+       use PHPUnit4And6Compat;
+       use MediaWikiCoversValidator;
+}
index 5246e36..a8c8581 100644 (file)
@@ -16,7 +16,7 @@ use MediaWikiTestCase;
 use MWException;
 use Title;
 use WANObjectCache;
-use Wikimedia\Rdbms\Database;
+use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\LoadBalancer;
 use Wikimedia\TestingAccessWrapper;
 use WikitextContent;
@@ -70,10 +70,10 @@ class RevisionStoreTest extends MediaWikiTestCase {
        }
 
        /**
-        * @return \PHPUnit_Framework_MockObject_MockObject|Database
+        * @return \PHPUnit_Framework_MockObject_MockObject|IDatabase
         */
        private function getMockDatabase() {
-               return $this->getMockBuilder( Database::class )
+               return $this->getMockBuilder( IDatabase::class )
                        ->disableOriginalConstructor()->getMock();
        }
 
index ca87b49..47d3b92 100644 (file)
@@ -10,7 +10,7 @@ use MediaWiki\Storage\NameTableStore;
 use MediaWikiTestCase;
 use Psr\Log\NullLogger;
 use WANObjectCache;
-use Wikimedia\Rdbms\Database;
+use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\LoadBalancer;
 use Wikimedia\TestingAccessWrapper;
 
@@ -57,37 +57,25 @@ class NameTableStoreTest extends MediaWikiTestCase {
        }
 
        private function getCallCheckingDb( $insertCalls, $selectCalls ) {
-               $mock = $this->getMockBuilder( Database::class )
+               $proxiedMethods = [
+                       'select' => $selectCalls,
+                       'insert' => $insertCalls,
+                       'affectedRows' => null,
+                       'insertId' => null,
+                       'getSessionLagStatus' => null,
+                       'writesPending' => null,
+                       'onTransactionPreCommitOrIdle' => null
+               ];
+               $mock = $this->getMockBuilder( IDatabase::class )
                        ->disableOriginalConstructor()
                        ->getMock();
-               $mock->expects( $this->exactly( $insertCalls ) )
-                       ->method( 'insert' )
-                       ->willReturnCallback( function ( ...$args ) {
-                               return call_user_func_array( [ $this->db, 'insert' ], $args );
-                       } );
-               $mock->expects( $this->exactly( $selectCalls ) )
-                       ->method( 'select' )
-                       ->willReturnCallback( function ( ...$args ) {
-                               return call_user_func_array( [ $this->db, 'select' ], $args );
-                       } );
-               $mock->expects( $this->exactly( $insertCalls ) )
-                       ->method( 'affectedRows' )
-                       ->willReturnCallback( function ( ...$args ) {
-                               return call_user_func_array( [ $this->db, 'affectedRows' ], $args );
-                       } );
-               $mock->expects( $this->any() )
-                       ->method( 'insertId' )
-                       ->willReturnCallback( function ( ...$args ) {
-                               return call_user_func_array( [ $this->db, 'insertId' ], $args );
-                       } );
-               $mock->expects( $this->any() )
-                       ->method( 'query' )
-                       ->willReturn( [] );
-               $mock->expects( $this->any() )
-                       ->method( 'isOpen' )
-                       ->willReturn( true );
-               $wrapper = TestingAccessWrapper::newFromObject( $mock );
-               $wrapper->queryLogger = new NullLogger();
+               foreach ( $proxiedMethods as $method => $count ) {
+                       $mock->expects( is_int( $count ) ? $this->exactly( $count ) : $this->any() )
+                               ->method( $method )
+                               ->willReturnCallback( function ( ...$args ) use ( $method ) {
+                                       return call_user_func_array( [ $this->db, $method ], $args );
+                               } );
+               }
                return $mock;
        }
 
index 33e5c3b..833ac2c 100644 (file)
@@ -1,9 +1,8 @@
 <?php
 
-use Wikimedia\Rdbms\Database;
+use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\DBConnRef;
 use Wikimedia\Rdbms\FakeResultWrapper;
-use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\ILoadBalancer;
 use Wikimedia\Rdbms\ResultWrapper;
 
@@ -40,7 +39,7 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
         * @return IDatabase
         */
        private function getDatabaseMock() {
-               $db = $this->getMockBuilder( Database::class )
+               $db = $this->getMockBuilder( IDatabase::class )
                        ->disableOriginalConstructor()
                        ->getMock();
 
@@ -60,12 +59,6 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
                $db->method( 'isOpen' )->willReturnCallback( function () use ( &$open ) {
                        return $open;
                } );
-               $db->method( 'open' )->willReturnCallback( function () use ( &$open ) {
-                       $open = true;
-
-                       return $open;
-               } );
-               $db->method( '__toString' )->willReturn( 'MOCK_DB' );
 
                return $db;
        }
diff --git a/tests/phpunit/includes/password/PasswordFactoryTest.php b/tests/phpunit/includes/password/PasswordFactoryTest.php
deleted file mode 100644 (file)
index a7b3557..0000000
+++ /dev/null
@@ -1,124 +0,0 @@
-<?php
-
-/**
- * @covers PasswordFactory
- */
-class PasswordFactoryTest extends MediaWikiTestCase {
-       public function testConstruct() {
-               $pf = new PasswordFactory();
-               $this->assertEquals( [ '' ], array_keys( $pf->getTypes() ) );
-               $this->assertEquals( '', $pf->getDefaultType() );
-
-               $pf = new PasswordFactory( [
-                       'foo' => [ 'class' => 'FooPassword' ],
-                       'bar' => [ 'class' => 'BarPassword', 'baz' => 'boom' ],
-               ], 'foo' );
-               $this->assertEquals( [ '', 'foo', 'bar' ], array_keys( $pf->getTypes() ) );
-               $this->assertArraySubset( [ 'class' => 'BarPassword', 'baz' => 'boom' ], $pf->getTypes()['bar'] );
-               $this->assertEquals( 'foo', $pf->getDefaultType() );
-       }
-
-       public function testRegister() {
-               $pf = new PasswordFactory;
-               $pf->register( 'foo', [ 'class' => InvalidPassword::class ] );
-               $this->assertArrayHasKey( 'foo', $pf->getTypes() );
-       }
-
-       public function testSetDefaultType() {
-               $pf = new PasswordFactory;
-               $pf->register( '1', [ 'class' => InvalidPassword::class ] );
-               $pf->register( '2', [ 'class' => InvalidPassword::class ] );
-               $pf->setDefaultType( '1' );
-               $this->assertSame( '1', $pf->getDefaultType() );
-               $pf->setDefaultType( '2' );
-               $this->assertSame( '2', $pf->getDefaultType() );
-       }
-
-       /**
-        * @expectedException Exception
-        */
-       public function testSetDefaultTypeError() {
-               $pf = new PasswordFactory;
-               $pf->setDefaultType( 'bogus' );
-       }
-
-       public function testInit() {
-               $config = new HashConfig( [
-                       'PasswordConfig' => [
-                               'foo' => [ 'class' => InvalidPassword::class ],
-                       ],
-                       'PasswordDefault' => 'foo'
-               ] );
-               $pf = new PasswordFactory;
-               $pf->init( $config );
-               $this->assertSame( 'foo', $pf->getDefaultType() );
-               $this->assertArrayHasKey( 'foo', $pf->getTypes() );
-       }
-
-       public function testNewFromCiphertext() {
-               $pf = new PasswordFactory;
-               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
-               $pw = $pf->newFromCiphertext( ':B:salt:d529e941509eb9e9b9cfaeae1fe7ca23' );
-               $this->assertInstanceOf( MWSaltedPassword::class, $pw );
-       }
-
-       public function provideNewFromCiphertextErrors() {
-               return [ [ 'blah' ], [ ':blah:' ] ];
-       }
-
-       /**
-        * @dataProvider provideNewFromCiphertextErrors
-        * @expectedException PasswordError
-        */
-       public function testNewFromCiphertextErrors( $hash ) {
-               $pf = new PasswordFactory;
-               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
-               $pf->newFromCiphertext( $hash );
-       }
-
-       public function testNewFromType() {
-               $pf = new PasswordFactory;
-               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
-               $pw = $pf->newFromType( 'B' );
-               $this->assertInstanceOf( MWSaltedPassword::class, $pw );
-       }
-
-       /**
-        * @expectedException PasswordError
-        */
-       public function testNewFromTypeError() {
-               $pf = new PasswordFactory;
-               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
-               $pf->newFromType( 'bogus' );
-       }
-
-       public function testNewFromPlaintext() {
-               $pf = new PasswordFactory;
-               $pf->register( 'A', [ 'class' => MWOldPassword::class ] );
-               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
-               $pf->setDefaultType( 'A' );
-
-               $this->assertInstanceOf( InvalidPassword::class, $pf->newFromPlaintext( null ) );
-               $this->assertInstanceOf( MWOldPassword::class, $pf->newFromPlaintext( 'password' ) );
-               $this->assertInstanceOf( MWSaltedPassword::class,
-                       $pf->newFromPlaintext( 'password', $pf->newFromType( 'B' ) ) );
-       }
-
-       public function testNeedsUpdate() {
-               $pf = new PasswordFactory;
-               $pf->register( 'A', [ 'class' => MWOldPassword::class ] );
-               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
-               $pf->setDefaultType( 'A' );
-
-               $this->assertFalse( $pf->needsUpdate( $pf->newFromType( 'A' ) ) );
-               $this->assertTrue( $pf->needsUpdate( $pf->newFromType( 'B' ) ) );
-       }
-
-       public function testGenerateRandomPasswordString() {
-               $this->assertSame( 13, strlen( PasswordFactory::generateRandomPasswordString( 13 ) ) );
-       }
-
-       public function testNewInvalidPassword() {
-               $this->assertInstanceOf( InvalidPassword::class, PasswordFactory::newInvalidPassword() );
-       }
-}
index b5dd008..99f5e1b 100644 (file)
@@ -105,6 +105,83 @@ mw.loader.register( [
         "c",
         "{blankVer}"
     ]
+] );',
+                       ] ],
+                       [ [
+                               // Regression test for T223402.
+                               'msg' => 'Optimise the dependency tree (indirect circular dependency)',
+                               'modules' => [
+                                       'top' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'middle1', 'util' ] ] ),
+                                       'middle1' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'middle2', 'util' ] ] ),
+                                       'middle2' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'bottom' ] ] ),
+                                       'bottom' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'top' ] ] ),
+                                       'util' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
+                               ],
+                               'out' => '
+mw.loader.addSource( {
+    "local": "/w/load.php"
+} );
+mw.loader.register( [
+    [
+        "top",
+        "{blankVer}",
+        [
+            1,
+            4
+        ]
+    ],
+    [
+        "middle1",
+        "{blankVer}",
+        [
+            2,
+            4
+        ]
+    ],
+    [
+        "middle2",
+        "{blankVer}",
+        [
+            3
+        ]
+    ],
+    [
+        "bottom",
+        "{blankVer}",
+        [
+            0
+        ]
+    ],
+    [
+        "util",
+        "{blankVer}"
+    ]
+] );',
+                       ] ],
+                       [ [
+                               // Regression test for T223402.
+                               'msg' => 'Optimise the dependency tree (direct circular dependency)',
+                               'modules' => [
+                                       'top' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'util', 'top' ] ] ),
+                                       'util' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
+                               ],
+                               'out' => '
+mw.loader.addSource( {
+    "local": "/w/load.php"
+} );
+mw.loader.register( [
+    [
+        "top",
+        "{blankVer}",
+        [
+            1,
+            0
+        ]
+    ],
+    [
+        "util",
+        "{blankVer}"
+    ]
 ] );',
                        ] ],
                        [ [
index e6d55d8..cc6ac31 100644 (file)
                <testsuite name="documentation">
                        <directory>documentation</directory>
                </testsuite>
+               <testsuite name="unit">
+                       <directory>unit</directory>
+               </testsuite>
        </testsuites>
        <groups>
                <exclude>
-                       <group>Utility</group>
                        <group>Broken</group>
-                       <group>Stub</group>
                </exclude>
        </groups>
        <filter>
diff --git a/tests/phpunit/unit-tests.xml b/tests/phpunit/unit-tests.xml
new file mode 100644 (file)
index 0000000..cd4118c
--- /dev/null
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit bootstrap="unit/initUnitTests.php"
+                xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/4.8/phpunit.xsd"
+
+                colors="true"
+                backupGlobals="false"
+                convertErrorsToExceptions="true"
+                convertNoticesToExceptions="true"
+                convertWarningsToExceptions="true"
+                forceCoversAnnotation="true"
+                stopOnFailure="false"
+                timeoutForSmallTests="10"
+                timeoutForMediumTests="30"
+                timeoutForLargeTests="60"
+                beStrictAboutTestsThatDoNotTestAnything="true"
+                beStrictAboutOutputDuringTests="true"
+                beStrictAboutTestSize="true"
+                verbose="false">
+       <testsuites>
+               <testsuite name="tests">
+                       <directory>unit</directory>
+               </testsuite>
+       </testsuites>
+       <groups>
+               <exclude>
+                       <group>Broken</group>
+               </exclude>
+       </groups>
+       <filter>
+               <whitelist addUncoveredFilesFromWhitelist="true">
+                       <directory suffix=".php">../../includes</directory>
+                       <directory suffix=".php">../../languages</directory>
+                       <directory suffix=".php">../../maintenance</directory>
+                       <exclude>
+                               <directory suffix=".php">../../languages/messages</directory>
+                               <file>../../languages/data/normalize-ar.php</file>
+                               <file>../../languages/data/normalize-ml.php</file>
+                       </exclude>
+               </whitelist>
+       </filter>
+</phpunit>
diff --git a/tests/phpunit/unit/includes/password/PasswordFactoryTest.php b/tests/phpunit/unit/includes/password/PasswordFactoryTest.php
new file mode 100644 (file)
index 0000000..cbfddd4
--- /dev/null
@@ -0,0 +1,124 @@
+<?php
+
+/**
+ * @covers PasswordFactory
+ */
+class PasswordFactoryTest extends MediaWikiUnitTestCase {
+       public function testConstruct() {
+               $pf = new PasswordFactory();
+               $this->assertEquals( [ '' ], array_keys( $pf->getTypes() ) );
+               $this->assertEquals( '', $pf->getDefaultType() );
+
+               $pf = new PasswordFactory( [
+                       'foo' => [ 'class' => 'FooPassword' ],
+                       'bar' => [ 'class' => 'BarPassword', 'baz' => 'boom' ],
+               ], 'foo' );
+               $this->assertEquals( [ '', 'foo', 'bar' ], array_keys( $pf->getTypes() ) );
+               $this->assertArraySubset( [ 'class' => 'BarPassword', 'baz' => 'boom' ], $pf->getTypes()['bar'] );
+               $this->assertEquals( 'foo', $pf->getDefaultType() );
+       }
+
+       public function testRegister() {
+               $pf = new PasswordFactory;
+               $pf->register( 'foo', [ 'class' => InvalidPassword::class ] );
+               $this->assertArrayHasKey( 'foo', $pf->getTypes() );
+       }
+
+       public function testSetDefaultType() {
+               $pf = new PasswordFactory;
+               $pf->register( '1', [ 'class' => InvalidPassword::class ] );
+               $pf->register( '2', [ 'class' => InvalidPassword::class ] );
+               $pf->setDefaultType( '1' );
+               $this->assertSame( '1', $pf->getDefaultType() );
+               $pf->setDefaultType( '2' );
+               $this->assertSame( '2', $pf->getDefaultType() );
+       }
+
+       /**
+        * @expectedException Exception
+        */
+       public function testSetDefaultTypeError() {
+               $pf = new PasswordFactory;
+               $pf->setDefaultType( 'bogus' );
+       }
+
+       public function testInit() {
+               $config = new HashConfig( [
+                       'PasswordConfig' => [
+                               'foo' => [ 'class' => InvalidPassword::class ],
+                       ],
+                       'PasswordDefault' => 'foo'
+               ] );
+               $pf = new PasswordFactory;
+               $pf->init( $config );
+               $this->assertSame( 'foo', $pf->getDefaultType() );
+               $this->assertArrayHasKey( 'foo', $pf->getTypes() );
+       }
+
+       public function testNewFromCiphertext() {
+               $pf = new PasswordFactory;
+               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
+               $pw = $pf->newFromCiphertext( ':B:salt:d529e941509eb9e9b9cfaeae1fe7ca23' );
+               $this->assertInstanceOf( MWSaltedPassword::class, $pw );
+       }
+
+       public function provideNewFromCiphertextErrors() {
+               return [ [ 'blah' ], [ ':blah:' ] ];
+       }
+
+       /**
+        * @dataProvider provideNewFromCiphertextErrors
+        * @expectedException PasswordError
+        */
+       public function testNewFromCiphertextErrors( $hash ) {
+               $pf = new PasswordFactory;
+               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
+               $pf->newFromCiphertext( $hash );
+       }
+
+       public function testNewFromType() {
+               $pf = new PasswordFactory;
+               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
+               $pw = $pf->newFromType( 'B' );
+               $this->assertInstanceOf( MWSaltedPassword::class, $pw );
+       }
+
+       /**
+        * @expectedException PasswordError
+        */
+       public function testNewFromTypeError() {
+               $pf = new PasswordFactory;
+               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
+               $pf->newFromType( 'bogus' );
+       }
+
+       public function testNewFromPlaintext() {
+               $pf = new PasswordFactory;
+               $pf->register( 'A', [ 'class' => MWOldPassword::class ] );
+               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
+               $pf->setDefaultType( 'A' );
+
+               $this->assertInstanceOf( InvalidPassword::class, $pf->newFromPlaintext( null ) );
+               $this->assertInstanceOf( MWOldPassword::class, $pf->newFromPlaintext( 'password' ) );
+               $this->assertInstanceOf( MWSaltedPassword::class,
+                       $pf->newFromPlaintext( 'password', $pf->newFromType( 'B' ) ) );
+       }
+
+       public function testNeedsUpdate() {
+               $pf = new PasswordFactory;
+               $pf->register( 'A', [ 'class' => MWOldPassword::class ] );
+               $pf->register( 'B', [ 'class' => MWSaltedPassword::class ] );
+               $pf->setDefaultType( 'A' );
+
+               $this->assertFalse( $pf->needsUpdate( $pf->newFromType( 'A' ) ) );
+               $this->assertTrue( $pf->needsUpdate( $pf->newFromType( 'B' ) ) );
+       }
+
+       public function testGenerateRandomPasswordString() {
+               $this->assertSame( 13, strlen( PasswordFactory::generateRandomPasswordString( 13 ) ) );
+       }
+
+       public function testNewInvalidPassword() {
+               $this->assertInstanceOf( InvalidPassword::class, PasswordFactory::newInvalidPassword() );
+       }
+}
diff --git a/tests/phpunit/unit/initUnitTests.php b/tests/phpunit/unit/initUnitTests.php
new file mode 100644 (file)
index 0000000..2121877
--- /dev/null
@@ -0,0 +1,69 @@
+<?php
+/**
+ * PHPUnit bootstrap file for the unit test suite.
+ *
+ * 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
+ * @ingroup Testing
+ */
+
+if ( PHP_SAPI !== 'cli' ) {
+       die( 'This file is only meant to be executed indirectly by PHPUnit\'s bootstrap process!' );
+}
+
+/**
+ * PHPUnit includes the bootstrap file inside a method body, while most MediaWiki startup files
+ * assume to be included in the global scope.
+ * This utility provides a way to include these files: it makes all globals available in the
+ * inclusion scope before including the file, then exports all new or changed globals.
+ *
+ * @param string $fileName the file to include
+ */
+function wfRequireOnceInGlobalScope( $fileName ) {
+       // phpcs:disable MediaWiki.Usage.ForbiddenFunctions.extract
+       extract( $GLOBALS, EXTR_REFS | EXTR_SKIP );
+       // phpcs:enable
+
+       require_once $fileName;
+
+       foreach ( get_defined_vars() as $varName => $value ) {
+               $GLOBALS[$varName] = $value;
+       }
+}
+
+define( 'MEDIAWIKI', true );
+define( 'MW_PHPUNIT_TEST', true );
+
+// We don't use a settings file here but some code still assumes that one exists
+define( 'MW_CONFIG_FILE', 'LocalSettings.php' );
+
+$IP = realpath( __DIR__ . '/../../..' );
+
+// these variables must be defined before setup runs
+$GLOBALS['IP'] = $IP;
+$GLOBALS['wgCommandLineMode'] = true;
+
+require_once "$IP/tests/common/TestSetup.php";
+
+wfRequireOnceInGlobalScope( "$IP/includes/AutoLoader.php" );
+wfRequireOnceInGlobalScope( "$IP/includes/Defines.php" );
+wfRequireOnceInGlobalScope( "$IP/includes/DefaultSettings.php" );
+wfRequireOnceInGlobalScope( "$IP/includes/GlobalFunctions.php" );
+
+require_once "$IP/tests/common/TestsAutoLoader.php";
+
+TestSetup::applyInitialConfig();