Don't pass Config to service constructors
authorAryeh Gregor <ayg@aryeh.name>
Wed, 10 Apr 2019 15:03:54 +0000 (18:03 +0300)
committerAryeh Gregor <ayg@aryeh.name>
Thu, 2 May 2019 08:33:56 +0000 (11:33 +0300)
We don't want to depend on the entire site configuration when we only
need a few specific settings.

This change additionally means that these services no longer see a live
version of the settings, but rather a copy. This means in tests you
really do have to call overrideMwServices() if you want services to pick
up your config changes.

ResourceLoader and SearchEngineConfig will need more work to port,
because they expose their member Config in a getter, and the getter is
actually used.

Parser and NamespaceInfo are also relatively complicated, so I split
them into separate patches.

Tested with 100% code coverage. \o/

Depends-On: If6534b18f6657ec1aba7327463f2661037f995b3
Change-Id: I1a3f358e8659b49de4502dc8216ecb6f35f4e02a

15 files changed:
RELEASE-NOTES-1.34
autoload.php
includes/ConfiguredReadOnlyMode.php
includes/ServiceWiring.php
includes/Storage/BlobStoreFactory.php
includes/config/ServiceOptions.php [new file with mode: 0644]
includes/db/MWLBFactory.php
includes/preferences/DefaultPreferencesFactory.php
includes/specialpage/ChangesListSpecialPage.php
includes/specialpage/SpecialPageFactory.php
includes/specials/SpecialWatchlist.php
tests/phpunit/includes/ReadOnlyModeTest.php
tests/phpunit/includes/api/ApiParseTest.php
tests/phpunit/includes/config/ServiceOptionsTest.php [new file with mode: 0644]
tests/phpunit/includes/preferences/DefaultPreferencesFactoryTest.php

index 3a9ea7c..5d46edd 100644 (file)
@@ -124,7 +124,8 @@ because of Phabricator reports.
   to the BlockManager as private helper methods.
 * User::isDnsBlacklisted is deprecated. Use BlockManager::isDnsBlacklisted
   instead.
-* …
+* The Config argument to ChangesListSpecialPage::checkStructuredFilterUiEnabled
+  is deprecated. Pass only the User argument.
 
 === Other changes in 1.34 ===
 * …
index 13037ff..be07cea 100644 (file)
@@ -872,6 +872,7 @@ $wgAutoloadLocalClasses = [
        'MediaWikiVersionFetcher' => __DIR__ . '/includes/MediaWikiVersionFetcher.php',
        'MediaWiki\\ChangeTags\\Taggable' => __DIR__ . '/includes/changetags/Taggable.php',
        'MediaWiki\\Config\\ConfigRepository' => __DIR__ . '/includes/config/ConfigRepository.php',
+       'MediaWiki\\Config\\ServiceOptions' => __DIR__ . '/includes/config/ServiceOptions.php',
        'MediaWiki\\DB\\PatchFileLocation' => __DIR__ . '/includes/db/PatchFileLocation.php',
        'MediaWiki\\Diff\\ComplexityException' => __DIR__ . '/includes/diff/ComplexityException.php',
        'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/diff/WordAccumulator.php',
index 7df2aed..f8ba5b1 100644 (file)
@@ -7,17 +7,28 @@
  * @since 1.29
  */
 class ConfiguredReadOnlyMode {
-       /** @var Config */
-       private $config;
-
-       /** @var string|bool|null */
-       private $fileReason;
+       /** @var string|boolean|null */
+       private $reason;
 
        /** @var string|null */
-       private $overrideReason;
+       private $reasonFile;
 
-       public function __construct( Config $config ) {
-               $this->config = $config;
+       /**
+        * @param string|bool|null $reason Current reason for read-only mode, if known. null means look
+        *   in $reasonFile instead.
+        * @param string|null $reasonFile A file to look in for a reason, if $reason is null. If it
+        *   exists and is non-empty, its contents are treated as the reason for read-only mode.
+        *   Otherwise, the wiki is not read-only.
+        */
+       public function __construct( $reason, $reasonFile = null ) {
+               if ( $reason instanceof Config ) {
+                       // Before 1.34 we passed a whole Config object, which was overkill
+                       wfDeprecated( __METHOD__ . ' with Config passed to constructor', '1.34' );
+                       $reason = $reason->get( 'ReadOnly' );
+                       $reasonFile = $reason->get( 'ReadOnlyFile' );
+               }
+               $this->reason = $reason;
+               $this->reasonFile = $reasonFile;
        }
 
        /**
@@ -35,23 +46,19 @@ class ConfiguredReadOnlyMode {
         * @return string|bool String when in read-only mode; false otherwise
         */
        public function getReason() {
-               if ( $this->overrideReason !== null ) {
-                       return $this->overrideReason;
+               if ( $this->reason !== null ) {
+                       return $this->reason;
                }
-               $confReason = $this->config->get( 'ReadOnly' );
-               if ( $confReason !== null ) {
-                       return $confReason;
+               if ( $this->reasonFile === null ) {
+                       return false;
                }
-               if ( $this->fileReason === null ) {
-                       // Cache for faster access next time
-                       $readOnlyFile = $this->config->get( 'ReadOnlyFile' );
-                       if ( is_file( $readOnlyFile ) && filesize( $readOnlyFile ) > 0 ) {
-                               $this->fileReason = file_get_contents( $readOnlyFile );
-                       } else {
-                               $this->fileReason = false;
-                       }
+               // Try the reason file
+               if ( is_file( $this->reasonFile ) && filesize( $this->reasonFile ) > 0 ) {
+                       $this->reason = file_get_contents( $this->reasonFile );
                }
-               return $this->fileReason;
+               // No need to try the reason file again
+               $this->reasonFile = null;
+               return $this->reason ?? false;
        }
 
        /**
@@ -61,6 +68,6 @@ class ConfiguredReadOnlyMode {
         * @param string|null $msg
         */
        public function setReason( $msg ) {
-               $this->overrideReason = $msg;
+               $this->reason = $msg;
        }
 }
index 93ddee9..bf722c3 100644 (file)
@@ -42,6 +42,7 @@ use MediaWiki\Auth\AuthManager;
 use MediaWiki\Block\BlockManager;
 use MediaWiki\Block\BlockRestrictionStore;
 use MediaWiki\Config\ConfigRepository;
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Interwiki\ClassicInterwikiLookup;
 use MediaWiki\Interwiki\InterwikiLookup;
 use MediaWiki\Linker\LinkRenderer;
@@ -81,7 +82,8 @@ return [
                return new BlobStoreFactory(
                        $services->getDBLoadBalancerFactory(),
                        $services->getMainWANObjectCache(),
-                       $services->getMainConfig(),
+                       new ServiceOptions( BlobStoreFactory::$constructorOptions,
+                               $services->getMainConfig() ),
                        $services->getContentLanguage()
                );
        },
@@ -132,7 +134,11 @@ return [
        },
 
        'ConfiguredReadOnlyMode' => function ( MediaWikiServices $services ) : ConfiguredReadOnlyMode {
-               return new ConfiguredReadOnlyMode( $services->getMainConfig() );
+               $config = $services->getMainConfig();
+               return new ConfiguredReadOnlyMode(
+                       $config->get( 'ReadOnly' ),
+                       $config->get( 'ReadOnlyFile' )
+               );
        },
 
        'ContentLanguage' => function ( MediaWikiServices $services ) : Language {
@@ -175,7 +181,7 @@ return [
 
                $lbConf = MWLBFactory::applyDefaultConfig(
                        $mainConfig->get( 'LBFactoryConf' ),
-                       $mainConfig,
+                       new ServiceOptions( MWLBFactory::$applyDefaultConfigOptions, $mainConfig ),
                        $services->getConfiguredReadOnlyMode(),
                        $services->getLocalServerObjectCache(),
                        $services->getMainObjectStash(),
@@ -184,7 +190,7 @@ return [
                $class = MWLBFactory::getLBFactoryClass( $lbConf );
 
                $instance = new $class( $lbConf );
-               MWLBFactory::setSchemaAliases( $instance, $mainConfig );
+               MWLBFactory::setSchemaAliases( $instance, $mainConfig->get( 'DBtype' ) );
 
                return $instance;
        },
@@ -450,7 +456,8 @@ return [
 
        'PreferencesFactory' => function ( MediaWikiServices $services ) : PreferencesFactory {
                $factory = new DefaultPreferencesFactory(
-                       $services->getMainConfig(),
+                       new ServiceOptions(
+                               DefaultPreferencesFactory::$constructorOptions, $services->getMainConfig() ),
                        $services->getContentLanguage(),
                        AuthManager::singleton(),
                        $services->getLinkRendererFactory()->create()
@@ -476,6 +483,8 @@ return [
        },
 
        'ResourceLoader' => function ( MediaWikiServices $services ) : ResourceLoader {
+               // @todo This should not take a Config object, but it's not so easy to remove because it
+               // exposes it in a getter, which is actually used.
                global $IP;
                $config = $services->getMainConfig();
 
@@ -539,6 +548,8 @@ return [
        },
 
        'SearchEngineConfig' => function ( MediaWikiServices $services ) : SearchEngineConfig {
+               // @todo This should not take a Config object, but it's not so easy to remove because it
+               // exposes it in a getter, which is actually used.
                return new SearchEngineConfig( $services->getMainConfig(),
                        $services->getContentLanguage() );
        },
@@ -623,13 +634,9 @@ return [
        },
 
        'SpecialPageFactory' => function ( MediaWikiServices $services ) : SpecialPageFactory {
-               $config = $services->getMainConfig();
-               $options = [];
-               foreach ( SpecialPageFactory::$constructorOptions as $key ) {
-                       $options[$key] = $config->get( $key );
-               }
                return new SpecialPageFactory(
-                       $options,
+                       new ServiceOptions(
+                               SpecialPageFactory::$constructorOptions, $services->getMainConfig() ),
                        $services->getContentLanguage()
                );
        },
index 4e1f97f..5e99454 100644 (file)
@@ -20,8 +20,8 @@
 
 namespace MediaWiki\Storage;
 
-use Config;
 use Language;
+use MediaWiki\Config\ServiceOptions;
 use WANObjectCache;
 use Wikimedia\Rdbms\LBFactory;
 
@@ -45,24 +45,39 @@ class BlobStoreFactory {
        private $cache;
 
        /**
-        * @var Config
+        * @var ServiceOptions
         */
-       private $config;
+       private $options;
 
        /**
         * @var Language
         */
        private $contLang;
 
+       /**
+        * TODO Make this a const when HHVM support is dropped (T192166)
+        *
+        * @var array
+        * @since 1.34
+        */
+       public static $constructorOptions = [
+               'CompressRevisions',
+               'DefaultExternalStore',
+               'LegacyEncoding',
+               'RevisionCacheExpiry',
+       ];
+
        public function __construct(
                LBFactory $lbFactory,
                WANObjectCache $cache,
-               Config $mainConfig,
+               ServiceOptions $options,
                Language $contLang
        ) {
+               $options->assertRequiredOptions( self::$constructorOptions );
+
                $this->lbFactory = $lbFactory;
                $this->cache = $cache;
-               $this->config = $mainConfig;
+               $this->options = $options;
                $this->contLang = $contLang;
        }
 
@@ -92,12 +107,12 @@ class BlobStoreFactory {
                        $wikiId
                );
 
-               $store->setCompressBlobs( $this->config->get( 'CompressRevisions' ) );
-               $store->setCacheExpiry( $this->config->get( 'RevisionCacheExpiry' ) );
-               $store->setUseExternalStore( $this->config->get( 'DefaultExternalStore' ) !== false );
+               $store->setCompressBlobs( $this->options->get( 'CompressRevisions' ) );
+               $store->setCacheExpiry( $this->options->get( 'RevisionCacheExpiry' ) );
+               $store->setUseExternalStore( $this->options->get( 'DefaultExternalStore' ) !== false );
 
-               if ( $this->config->get( 'LegacyEncoding' ) ) {
-                       $store->setLegacyEncoding( $this->config->get( 'LegacyEncoding' ), $this->contLang );
+               if ( $this->options->get( 'LegacyEncoding' ) ) {
+                       $store->setLegacyEncoding( $this->options->get( 'LegacyEncoding' ), $this->contLang );
                }
 
                return $store;
diff --git a/includes/config/ServiceOptions.php b/includes/config/ServiceOptions.php
new file mode 100644 (file)
index 0000000..0f3743f
--- /dev/null
@@ -0,0 +1,87 @@
+<?php
+
+namespace MediaWiki\Config;
+
+use Config;
+use InvalidArgumentException;
+use Wikimedia\Assert\Assert;
+
+/**
+ * A class for passing options to services. It can be constructed from a Config, and in practice
+ * most options will be taken from site configuration, but they don't have to be. The options passed
+ * are copied and will not reflect subsequent updates to site configuration (assuming they're not
+ * objects).
+ *
+ * Services that take this type as a parameter to their constructor should specify a list of the
+ * keys they expect to receive in an array. The convention is to make it a public static variable
+ * called $constructorOptions. (When we drop HHVM support -- see T192166 -- it should become a
+ * const.) In the constructor, they should call assertRequiredOptions() to make sure that they
+ * weren't passed too few or too many options. This way it's clear what each class depends on, and
+ * that it's getting passed the correct set of options. (This means there are no optional options.
+ * This makes sense for services, since they shouldn't be constructed by outside code.)
+ *
+ * @since 1.34
+ */
+class ServiceOptions {
+       private $options = [];
+
+       /**
+        * @param string[] $keys Which keys to extract from $sources
+        * @param Config|array ...$sources Each source is either a Config object or an array. If the
+        *  same key is present in two sources, the first one takes precedence. Keys that are not in
+        *  $keys are ignored.
+        * @throws InvalidArgumentException if one of $keys is not found in any of $sources
+        */
+       public function __construct( array $keys, ...$sources ) {
+               foreach ( $keys as $key ) {
+                       foreach ( $sources as $source ) {
+                               if ( $source instanceof Config ) {
+                                       if ( $source->has( $key ) ) {
+                                               $this->options[$key] = $source->get( $key );
+                                               continue 2;
+                                       }
+                               } else {
+                                       if ( array_key_exists( $key, $source ) ) {
+                                               $this->options[$key] = $source[$key];
+                                               continue 2;
+                                       }
+                               }
+                       }
+                       throw new InvalidArgumentException( "Key \"$key\" not found in input sources" );
+               }
+       }
+
+       /**
+        * Assert that the list of options provided in this instance exactly match $expectedKeys,
+        * without regard for order.
+        *
+        * @param string[] $expectedKeys
+        */
+       public function assertRequiredOptions( array $expectedKeys ) {
+               $actualKeys = array_keys( $this->options );
+               $extraKeys = array_diff( $actualKeys, $expectedKeys );
+               $missingKeys = array_diff( $expectedKeys, $actualKeys );
+               Assert::precondition( !$extraKeys && !$missingKeys,
+                       (
+                       $extraKeys
+                               ? 'Unsupported options passed: ' . implode( ', ', $extraKeys ) . '!'
+                               : ''
+                       ) . ( $extraKeys && $missingKeys ? ' ' : '' ) . (
+                       $missingKeys
+                               ? 'Required options missing: ' . implode( ', ', $missingKeys ) . '!'
+                               : ''
+                       )
+               );
+       }
+
+       /**
+        * @param string $key
+        * @return mixed
+        */
+       public function get( $key ) {
+               if ( !array_key_exists( $key, $this->options ) ) {
+                       throw new InvalidArgumentException( "Unrecognized option \"$key\"" );
+               }
+               return $this->options[$key];
+       }
+}
index 6633fba..be4f6ba 100644 (file)
@@ -21,6 +21,7 @@
  * @ingroup Database
  */
 
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Logger\LoggerFactory;
 use Wikimedia\Rdbms\LBFactory;
 use Wikimedia\Rdbms\DatabaseDomain;
@@ -34,9 +35,35 @@ abstract class MWLBFactory {
        /** @var array Cache of already-logged deprecation messages */
        private static $loggedDeprecations = [];
 
+       /**
+        * TODO Make this a const when HHVM support is dropped (T192166)
+        *
+        * @var array
+        * @since 1.34
+        */
+       public static $applyDefaultConfigOptions = [
+               'DBcompress',
+               'DBDefaultGroup',
+               'DBmwschema',
+               'DBname',
+               'DBpassword',
+               'DBport',
+               'DBprefix',
+               'DBserver',
+               'DBservers',
+               'DBssl',
+               'DBtype',
+               'DBuser',
+               'DBWindowsAuthentication',
+               'DebugDumpSql',
+               'ExternalServers',
+               'SQLiteDataDir',
+               'SQLMode',
+       ];
+
        /**
         * @param array $lbConf Config for LBFactory::__construct()
-        * @param Config $mainConfig Main config object from MediaWikiServices
+        * @param ServiceOptions $options
         * @param ConfiguredReadOnlyMode $readOnlyMode
         * @param BagOStuff $srvCace
         * @param BagOStuff $mainStash
@@ -45,21 +72,23 @@ abstract class MWLBFactory {
         */
        public static function applyDefaultConfig(
                array $lbConf,
-               Config $mainConfig,
+               ServiceOptions $options,
                ConfiguredReadOnlyMode $readOnlyMode,
                BagOStuff $srvCace,
                BagOStuff $mainStash,
                WANObjectCache $wanCache
        ) {
+               $options->assertRequiredOptions( self::$applyDefaultConfigOptions );
+
                global $wgCommandLineMode;
 
                $typesWithSchema = self::getDbTypesWithSchemas();
 
                $lbConf += [
                        'localDomain' => new DatabaseDomain(
-                               $mainConfig->get( 'DBname' ),
-                               $mainConfig->get( 'DBmwschema' ),
-                               $mainConfig->get( 'DBprefix' )
+                               $options->get( 'DBname' ),
+                               $options->get( 'DBmwschema' ),
+                               $options->get( 'DBprefix' )
                        ),
                        'profiler' => function ( $section ) {
                                return Profiler::instance()->scopedProfileIn( $section );
@@ -74,7 +103,7 @@ abstract class MWLBFactory {
                        'cliMode' => $wgCommandLineMode,
                        'hostname' => wfHostname(),
                        'readOnlyReason' => $readOnlyMode->getReason(),
-                       'defaultGroup' => $mainConfig->get( 'DBDefaultGroup' ),
+                       'defaultGroup' => $options->get( 'DBDefaultGroup' ),
                ];
 
                $serversCheck = [];
@@ -84,45 +113,46 @@ abstract class MWLBFactory {
                if ( $lbConf['class'] === Wikimedia\Rdbms\LBFactorySimple::class ) {
                        if ( isset( $lbConf['servers'] ) ) {
                                // Server array is already explicitly configured
-                       } elseif ( is_array( $mainConfig->get( 'DBservers' ) ) ) {
+                       } elseif ( is_array( $options->get( 'DBservers' ) ) ) {
                                $lbConf['servers'] = [];
-                               foreach ( $mainConfig->get( 'DBservers' ) as $i => $server ) {
-                                       $lbConf['servers'][$i] = self::initServerInfo( $server, $mainConfig );
+                               foreach ( $options->get( 'DBservers' ) as $i => $server ) {
+                                       $lbConf['servers'][$i] = self::initServerInfo( $server, $options );
                                }
                        } else {
                                $server = self::initServerInfo(
                                        [
-                                               'host' => $mainConfig->get( 'DBserver' ),
-                                               'user' => $mainConfig->get( 'DBuser' ),
-                                               'password' => $mainConfig->get( 'DBpassword' ),
-                                               'dbname' => $mainConfig->get( 'DBname' ),
-                                               'type' => $mainConfig->get( 'DBtype' ),
+                                               'host' => $options->get( 'DBserver' ),
+                                               'user' => $options->get( 'DBuser' ),
+                                               'password' => $options->get( 'DBpassword' ),
+                                               'dbname' => $options->get( 'DBname' ),
+                                               'type' => $options->get( 'DBtype' ),
                                                'load' => 1
                                        ],
-                                       $mainConfig
+                                       $options
                                );
 
-                               $server['flags'] |= $mainConfig->get( 'DBssl' ) ? DBO_SSL : 0;
-                               $server['flags'] |= $mainConfig->get( 'DBcompress' ) ? DBO_COMPRESS : 0;
+                               $server['flags'] |= $options->get( 'DBssl' ) ? DBO_SSL : 0;
+                               $server['flags'] |= $options->get( 'DBcompress' ) ? DBO_COMPRESS : 0;
 
                                $lbConf['servers'] = [ $server ];
                        }
                        if ( !isset( $lbConf['externalClusters'] ) ) {
-                               $lbConf['externalClusters'] = $mainConfig->get( 'ExternalServers' );
+                               $lbConf['externalClusters'] = $options->get( 'ExternalServers' );
                        }
 
                        $serversCheck = $lbConf['servers'];
                } elseif ( $lbConf['class'] === Wikimedia\Rdbms\LBFactoryMulti::class ) {
                        if ( isset( $lbConf['serverTemplate'] ) ) {
                                if ( in_array( $lbConf['serverTemplate']['type'], $typesWithSchema, true ) ) {
-                                       $lbConf['serverTemplate']['schema'] = $mainConfig->get( 'DBmwschema' );
+                                       $lbConf['serverTemplate']['schema'] = $options->get( 'DBmwschema' );
                                }
-                               $lbConf['serverTemplate']['sqlMode'] = $mainConfig->get( 'SQLMode' );
+                               $lbConf['serverTemplate']['sqlMode'] = $options->get( 'SQLMode' );
                        }
                        $serversCheck = [ $lbConf['serverTemplate'] ] ?? [];
                }
 
-               self::assertValidServerConfigs( $serversCheck, $mainConfig );
+               self::assertValidServerConfigs( $serversCheck, $options->get( 'DBname' ),
+                       $options->get( 'DBprefix' ) );
 
                $lbConf = self::injectObjectCaches( $lbConf, $srvCace, $mainStash, $wanCache );
 
@@ -138,10 +168,10 @@ abstract class MWLBFactory {
 
        /**
         * @param array $server
-        * @param Config $mainConfig
+        * @param ServiceOptions $options
         * @return array
         */
-       private static function initServerInfo( array $server, Config $mainConfig ) {
+       private static function initServerInfo( array $server, ServiceOptions $options ) {
                if ( $server['type'] === 'sqlite' ) {
                        $httpMethod = $_SERVER['REQUEST_METHOD'] ?? null;
                        // T93097: hint for how file-based databases (e.g. sqlite) should go about locking.
@@ -149,12 +179,12 @@ abstract class MWLBFactory {
                        // See https://www.sqlite.org/lockingv3.html#shared_lock
                        $isHttpRead = in_array( $httpMethod, [ 'GET', 'HEAD', 'OPTIONS', 'TRACE' ] );
                        $server += [
-                               'dbDirectory' => $mainConfig->get( 'SQLiteDataDir' ),
+                               'dbDirectory' => $options->get( 'SQLiteDataDir' ),
                                'trxMode' => $isHttpRead ? 'DEFERRED' : 'IMMEDIATE'
                        ];
                } elseif ( $server['type'] === 'postgres' ) {
                        $server += [
-                               'port' => $mainConfig->get( 'DBport' ),
+                               'port' => $options->get( 'DBport' ),
                                // Work around the reserved word usage in MediaWiki schema
                                'keywordTableMap' => [ 'user' => 'mwuser', 'text' => 'pagecontent' ]
                        ];
@@ -165,25 +195,25 @@ abstract class MWLBFactory {
                        ];
                } elseif ( $server['type'] === 'mssql' ) {
                        $server += [
-                               'port' => $mainConfig->get( 'DBport' ),
-                               'useWindowsAuth' => $mainConfig->get( 'DBWindowsAuthentication' )
+                               'port' => $options->get( 'DBport' ),
+                               'useWindowsAuth' => $options->get( 'DBWindowsAuthentication' )
                        ];
                }
 
                if ( in_array( $server['type'], self::getDbTypesWithSchemas(), true ) ) {
-                       $server += [ 'schema' => $mainConfig->get( 'DBmwschema' ) ];
+                       $server += [ 'schema' => $options->get( 'DBmwschema' ) ];
                }
 
                $flags = DBO_DEFAULT;
-               $flags |= $mainConfig->get( 'DebugDumpSql' ) ? DBO_DEBUG : 0;
+               $flags |= $options->get( 'DebugDumpSql' ) ? DBO_DEBUG : 0;
                if ( $server['type'] === 'oracle' ) {
-                       $flags |= $mainConfig->get( 'DBOracleDRCP' ) ? DBO_PERSISTENT : 0;
+                       $flags |= $options->get( 'DBOracleDRCP' ) ? DBO_PERSISTENT : 0;
                }
 
                $server += [
-                       'tablePrefix' => $mainConfig->get( 'DBprefix' ),
+                       'tablePrefix' => $options->get( 'DBprefix' ),
                        'flags' => $flags,
-                       'sqlMode' => $mainConfig->get( 'SQLMode' ),
+                       'sqlMode' => $options->get( 'SQLMode' ),
                ];
 
                return $server;
@@ -215,12 +245,10 @@ abstract class MWLBFactory {
 
        /**
         * @param array $servers
-        * @param Config $mainConfig
+        * @param string $lbDB Local domain database name
+        * @param string $lbTP Local domain prefix
         */
-       private static function assertValidServerConfigs( array $servers, Config $mainConfig ) {
-               $ldDB = $mainConfig->get( 'DBname' ); // local domain DB
-               $ldTP = $mainConfig->get( 'DBprefix' ); // local domain prefix
-
+       private static function assertValidServerConfigs( array $servers, $ldDB, $ldTP ) {
                foreach ( $servers as $server ) {
                        $type = $server['type'] ?? null;
                        $srvDB = $server['dbname'] ?? null; // server DB
@@ -332,8 +360,17 @@ abstract class MWLBFactory {
                return $class;
        }
 
-       public static function setSchemaAliases( LBFactory $lbFactory, Config $config ) {
-               if ( $config->get( 'DBtype' ) === 'mysql' ) {
+       /**
+        * @param LBFactory $lbFactory
+        * @param string $dbType 'mysql', 'sqlite', etc.
+        */
+       public static function setSchemaAliases( LBFactory $lbFactory, $dbType ) {
+               if ( $dbType instanceof Config ) {
+                       // Before 1.34 this took a whole Config just to get $dbType
+                       wfDeprecated( __METHOD__ . ' with Config argument', '1.34' );
+                       $dbType = $dbType->get( 'DBtype' );
+               }
+               if ( $dbType === 'mysql' ) {
                        /**
                         * When SQLite indexes were introduced in r45764, it was noted that
                         * SQLite requires index names to be unique within the whole database,
index e354d55..a5c8064 100644 (file)
@@ -34,6 +34,7 @@ use LanguageCode;
 use LanguageConverter;
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Auth\PasswordAuthenticationRequest;
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\MediaWikiServices;
 use MessageLocalizer;
@@ -61,8 +62,8 @@ use Xml;
 class DefaultPreferencesFactory implements PreferencesFactory {
        use LoggerAwareTrait;
 
-       /** @var Config */
-       protected $config;
+       /** @var ServiceOptions */
+       protected $options;
 
        /** @var Language The wiki's content language. */
        protected $contLang;
@@ -74,18 +75,58 @@ class DefaultPreferencesFactory implements PreferencesFactory {
        protected $linkRenderer;
 
        /**
-        * @param Config $config
+        * TODO Make this a const when we drop HHVM support (T192166)
+        *
+        * @var array
+        * @since 1.34
+        */
+       public static $constructorOptions = [
+               'AllowUserCss',
+               'AllowUserCssPrefs',
+               'AllowUserJs',
+               'DefaultSkin',
+               'DisableLangConversion',
+               'EmailAuthentication',
+               'EmailConfirmToEdit',
+               'EnableEmail',
+               'EnableUserEmail',
+               'EnableUserEmailBlacklist',
+               'EnotifMinorEdits',
+               'EnotifRevealEditorAddress',
+               'EnotifUserTalk',
+               'EnotifWatchlist',
+               'HiddenPrefs',
+               'ImageLimits',
+               'LanguageCode',
+               'LocalTZoffset',
+               'MaxSigChars',
+               'RCMaxAge',
+               'RCShowWatchingUsers',
+               'RCWatchCategoryMembership',
+               'SecureLogin',
+               'ThumbLimits',
+       ];
+
+       /**
+        * @param array|Config $options Config accepted for backwards compatibility
         * @param Language $contLang
         * @param AuthManager $authManager
         * @param LinkRenderer $linkRenderer
         */
        public function __construct(
-               Config $config,
+               $options,
                Language $contLang,
                AuthManager $authManager,
                LinkRenderer $linkRenderer
        ) {
-               $this->config = $config;
+               if ( $options instanceof Config ) {
+                       wfDeprecated( __METHOD__ . ' with Config parameter', '1.34' );
+                       $options = new ServiceOptions( self::$constructorOptions, $options );
+               }
+
+               $options->assertRequiredOptions( self::$constructorOptions );
+
+               $this->options = $options;
                $this->contLang = $contLang;
                $this->authManager = $authManager;
                $this->linkRenderer = $linkRenderer;
@@ -146,7 +187,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                User $user, IContextSource $context, &$defaultPreferences
        ) {
                # # Remove preferences that wikis don't want to use
-               foreach ( $this->config->get( 'HiddenPrefs' ) as $pref ) {
+               foreach ( $this->options->get( 'HiddenPrefs' ) as $pref ) {
                        if ( isset( $defaultPreferences[$pref] ) ) {
                                unset( $defaultPreferences[$pref] );
                        }
@@ -364,7 +405,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        ];
                }
                // Only show prefershttps if secure login is turned on
-               if ( $this->config->get( 'SecureLogin' ) && $canIPUseHTTPS ) {
+               if ( $this->options->get( 'SecureLogin' ) && $canIPUseHTTPS ) {
                        $defaultPreferences['prefershttps'] = [
                                'type' => 'toggle',
                                'label-message' => 'tog-prefershttps',
@@ -374,7 +415,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                }
 
                $languages = Language::fetchLanguageNames( null, 'mwfile' );
-               $languageCode = $this->config->get( 'LanguageCode' );
+               $languageCode = $this->options->get( 'LanguageCode' );
                if ( !array_key_exists( $languageCode, $languages ) ) {
                        $languages[$languageCode] = $languageCode;
                        // Sort the array again
@@ -408,7 +449,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                ];
 
                // see if there are multiple language variants to choose from
-               if ( !$this->config->get( 'DisableLangConversion' ) ) {
+               if ( !$this->options->get( 'DisableLangConversion' ) ) {
                        foreach ( LanguageConverter::$languagesWithVariants as $langCode ) {
                                if ( $langCode == $this->contLang->getCode() ) {
                                        if ( !$this->contLang->hasVariants() ) {
@@ -474,7 +515,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                ];
                $defaultPreferences['nickname'] = [
                        'type' => $this->authManager->allowsPropertyChange( 'nickname' ) ? 'text' : 'info',
-                       'maxlength' => $this->config->get( 'MaxSigChars' ),
+                       'maxlength' => $this->options->get( 'MaxSigChars' ),
                        'label-message' => 'yournick',
                        'validation-callback' => function ( $signature, $alldata, HTMLForm $form ) {
                                return $this->validateSignature( $signature, $alldata, $form );
@@ -494,13 +535,13 @@ class DefaultPreferencesFactory implements PreferencesFactory {
 
                # # Email stuff
 
-               if ( $this->config->get( 'EnableEmail' ) ) {
+               if ( $this->options->get( 'EnableEmail' ) ) {
                        if ( $canViewPrivateInfo ) {
-                               $helpMessages[] = $this->config->get( 'EmailConfirmToEdit' )
+                               $helpMessages[] = $this->options->get( 'EmailConfirmToEdit' )
                                                ? 'prefs-help-email-required'
                                                : 'prefs-help-email';
 
-                               if ( $this->config->get( 'EnableUserEmail' ) ) {
+                               if ( $this->options->get( 'EnableUserEmail' ) ) {
                                        // additional messages when users can send email to each other
                                        $helpMessages[] = 'prefs-help-email-others';
                                }
@@ -531,7 +572,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
 
                        $disableEmailPrefs = false;
 
-                       if ( $this->config->get( 'EmailAuthentication' ) ) {
+                       if ( $this->options->get( 'EmailAuthentication' ) ) {
                                $emailauthenticationclass = 'mw-email-not-authenticated';
                                if ( $user->getEmail() ) {
                                        if ( $user->getEmailAuthenticationTimestamp() ) {
@@ -575,7 +616,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                }
                        }
 
-                       if ( $this->config->get( 'EnableUserEmail' ) && $user->isAllowed( 'sendemail' ) ) {
+                       if ( $this->options->get( 'EnableUserEmail' ) && $user->isAllowed( 'sendemail' ) ) {
                                $defaultPreferences['disablemail'] = [
                                        'id' => 'wpAllowEmail',
                                        'type' => 'toggle',
@@ -600,7 +641,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                        'disabled' => $disableEmailPrefs,
                                ];
 
-                               if ( $this->config->get( 'EnableUserEmailBlacklist' ) ) {
+                               if ( $this->options->get( 'EnableUserEmailBlacklist' ) ) {
                                        $defaultPreferences['email-blacklist'] = [
                                                'type' => 'usersmultiselect',
                                                'label-message' => 'email-blacklist-label',
@@ -611,7 +652,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                }
                        }
 
-                       if ( $this->config->get( 'EnotifWatchlist' ) ) {
+                       if ( $this->options->get( 'EnotifWatchlist' ) ) {
                                $defaultPreferences['enotifwatchlistpages'] = [
                                        'type' => 'toggle',
                                        'section' => 'personal/email',
@@ -619,7 +660,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                        'disabled' => $disableEmailPrefs,
                                ];
                        }
-                       if ( $this->config->get( 'EnotifUserTalk' ) ) {
+                       if ( $this->options->get( 'EnotifUserTalk' ) ) {
                                $defaultPreferences['enotifusertalkpages'] = [
                                        'type' => 'toggle',
                                        'section' => 'personal/email',
@@ -627,8 +668,9 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                        'disabled' => $disableEmailPrefs,
                                ];
                        }
-                       if ( $this->config->get( 'EnotifUserTalk' ) || $this->config->get( 'EnotifWatchlist' ) ) {
-                               if ( $this->config->get( 'EnotifMinorEdits' ) ) {
+                       if ( $this->options->get( 'EnotifUserTalk' ) ||
+                       $this->options->get( 'EnotifWatchlist' ) ) {
+                               if ( $this->options->get( 'EnotifMinorEdits' ) ) {
                                        $defaultPreferences['enotifminoredits'] = [
                                                'type' => 'toggle',
                                                'section' => 'personal/email',
@@ -637,7 +679,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                        ];
                                }
 
-                               if ( $this->config->get( 'EnotifRevealEditorAddress' ) ) {
+                               if ( $this->options->get( 'EnotifRevealEditorAddress' ) ) {
                                        $defaultPreferences['enotifrevealaddr'] = [
                                                'type' => 'toggle',
                                                'section' => 'personal/email',
@@ -668,8 +710,8 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        ];
                }
 
-               $allowUserCss = $this->config->get( 'AllowUserCss' );
-               $allowUserJs = $this->config->get( 'AllowUserJs' );
+               $allowUserCss = $this->options->get( 'AllowUserCss' );
+               $allowUserJs = $this->options->get( 'AllowUserJs' );
                # Create links to user CSS/JS pages for all skins
                # This code is basically copied from generateSkinOptions().  It'd
                # be nice to somehow merge this back in there to avoid redundancy.
@@ -822,7 +864,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                ];
 
                # # Page Rendering ##############################
-               if ( $this->config->get( 'AllowUserCssPrefs' ) ) {
+               if ( $this->options->get( 'AllowUserCssPrefs' ) ) {
                        $defaultPreferences['underline'] = [
                                'type' => 'select',
                                'options' => [
@@ -891,7 +933,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        'label-message' => 'tog-editondblclick',
                ];
 
-               if ( $this->config->get( 'AllowUserCssPrefs' ) ) {
+               if ( $this->options->get( 'AllowUserCssPrefs' ) ) {
                        $defaultPreferences['editfont'] = [
                                'type' => 'select',
                                'section' => 'editing/editor',
@@ -946,7 +988,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
         * @param array &$defaultPreferences
         */
        protected function rcPreferences( User $user, MessageLocalizer $l10n, &$defaultPreferences ) {
-               $rcMaxAge = $this->config->get( 'RCMaxAge' );
+               $rcMaxAge = $this->options->get( 'RCMaxAge' );
                # # RecentChanges #####################################
                $defaultPreferences['rcdays'] = [
                        'type' => 'float',
@@ -999,7 +1041,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        'type' => 'api',
                ];
 
-               if ( $this->config->get( 'RCWatchCategoryMembership' ) ) {
+               if ( $this->options->get( 'RCWatchCategoryMembership' ) ) {
                        $defaultPreferences['hidecategorization'] = [
                                'type' => 'toggle',
                                'label-message' => 'tog-hidecategorization',
@@ -1023,7 +1065,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        ];
                }
 
-               if ( $this->config->get( 'RCShowWatchingUsers' ) ) {
+               if ( $this->options->get( 'RCShowWatchingUsers' ) ) {
                        $defaultPreferences['shownumberswatching'] = [
                                'type' => 'toggle',
                                'section' => 'rc/advancedrc',
@@ -1047,7 +1089,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
        protected function watchlistPreferences(
                User $user, IContextSource $context, &$defaultPreferences
        ) {
-               $watchlistdaysMax = ceil( $this->config->get( 'RCMaxAge' ) / ( 3600 * 24 ) );
+               $watchlistdaysMax = ceil( $this->options->get( 'RCMaxAge' ) / ( 3600 * 24 ) );
 
                # # Watchlist #####################################
                if ( $user->isAllowed( 'editmywatchlist' ) ) {
@@ -1127,10 +1169,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        'label-message' => 'tog-watchlisthideliu',
                ];
 
-               if ( !\SpecialWatchlist::checkStructuredFilterUiEnabled(
-                       $this->config,
-                       $user
-               ) ) {
+               if ( !\SpecialWatchlist::checkStructuredFilterUiEnabled( $user ) ) {
                        $defaultPreferences['watchlistreloadautomatically'] = [
                                'type' => 'toggle',
                                'section' => 'watchlist/advancedwatchlist',
@@ -1144,7 +1183,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        'label-message' => 'tog-watchlistunwatchlinks',
                ];
 
-               if ( $this->config->get( 'RCWatchCategoryMembership' ) ) {
+               if ( $this->options->get( 'RCWatchCategoryMembership' ) ) {
                        $defaultPreferences['watchlisthidecategorization'] = [
                                'type' => 'toggle',
                                'section' => 'watchlist/changeswatchlist',
@@ -1251,9 +1290,9 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        }
                }
 
-               $defaultSkin = $this->config->get( 'DefaultSkin' );
-               $allowUserCss = $this->config->get( 'AllowUserCss' );
-               $allowUserJs = $this->config->get( 'AllowUserJs' );
+               $defaultSkin = $this->options->get( 'DefaultSkin' );
+               $allowUserCss = $this->options->get( 'AllowUserCss' );
+               $allowUserJs = $this->options->get( 'AllowUserJs' );
 
                # Sort by the internal name, so that the ordering is the same for each display language,
                # especially if some skin names are translated to use a different alphabet and some are not.
@@ -1352,7 +1391,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                $ret = [];
                $pixels = $l10n->msg( 'unit-pixel' )->text();
 
-               foreach ( $this->config->get( 'ImageLimits' ) as $index => $limits ) {
+               foreach ( $this->options->get( 'ImageLimits' ) as $index => $limits ) {
                        // Note: A left-to-right marker (U+200E) is inserted, see T144386
                        $display = "{$limits[0]}\u{200E}×{$limits[1]}$pixels";
                        $ret[$display] = $index;
@@ -1369,7 +1408,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                $ret = [];
                $pixels = $l10n->msg( 'unit-pixel' )->text();
 
-               foreach ( $this->config->get( 'ThumbLimits' ) as $index => $size ) {
+               foreach ( $this->options->get( 'ThumbLimits' ) as $index => $size ) {
                        $display = $size . $pixels;
                        $ret[$display] = $index;
                }
@@ -1384,7 +1423,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
         * @return bool|string
         */
        protected function validateSignature( $signature, $alldata, HTMLForm $form ) {
-               $maxSigChars = $this->config->get( 'MaxSigChars' );
+               $maxSigChars = $this->options->get( 'MaxSigChars' );
                if ( mb_strlen( $signature ) > $maxSigChars ) {
                        return Xml::element( 'span', [ 'class' => 'error' ],
                                $form->msg( 'badsiglength' )->numParams( $maxSigChars )->text() );
@@ -1477,7 +1516,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
        protected function getTimezoneOptions( IContextSource $context ) {
                $opt = [];
 
-               $localTZoffset = $this->config->get( 'LocalTZoffset' );
+               $localTZoffset = $this->options->get( 'LocalTZoffset' );
                $timeZoneList = $this->getTimeZoneList( $context->getLanguage() );
 
                $timestamp = MWTimestamp::getLocalInstance();
@@ -1525,7 +1564,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
        protected function saveFormData( $formData, HTMLForm $form, array $formDescriptor ) {
                /** @var \User $user */
                $user = $form->getModifiedUser();
-               $hiddenPrefs = $this->config->get( 'HiddenPrefs' );
+               $hiddenPrefs = $this->options->get( 'HiddenPrefs' );
                $result = true;
 
                if ( !$user->isAllowedAny( 'editmyprivateinfo', 'editmyoptions' ) ) {
index 1b43a42..dee31b2 100644 (file)
@@ -1847,21 +1847,21 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                        return true;
                }
 
-               return static::checkStructuredFilterUiEnabled(
-                       $this->getConfig(),
-                       $this->getUser()
-               );
+               return static::checkStructuredFilterUiEnabled( $this->getUser() );
        }
 
        /**
         * Static method to check whether StructuredFilter UI is enabled for the given user
         *
         * @since 1.31
-        * @param Config $config
         * @param User $user
         * @return bool
         */
-       public static function checkStructuredFilterUiEnabled( Config $config, User $user ) {
+       public static function checkStructuredFilterUiEnabled( $user ) {
+               if ( $user instanceof Config ) {
+                       wfDeprecated( __METHOD__ . ' with Config argument', '1.34' );
+                       $user = func_get_arg( 1 );
+               }
                return !$user->getOption( 'rcenhancedfilters-disable' );
        }
 
index a3b7296..1053bda 100644 (file)
 
 namespace MediaWiki\Special;
 
-use Config;
 use Hooks;
 use IContextSource;
 use Language;
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Linker\LinkRenderer;
 use Profiler;
 use RequestContext;
 use SpecialPage;
 use Title;
 use User;
-use Wikimedia\Assert\Assert;
 
 /**
  * Factory for handling the special page list and generating SpecialPage objects.
@@ -215,7 +214,7 @@ class SpecialPageFactory {
        /** @var array */
        private $aliases;
 
-       /** @var Config */
+       /** @var ServiceOptions */
        private $options;
 
        /** @var Language */
@@ -238,13 +237,11 @@ class SpecialPageFactory {
        ];
 
        /**
-        * @param array $options
+        * @param ServiceOptions $options
         * @param Language $contLang
         */
-       public function __construct( array $options, Language $contLang ) {
-               Assert::parameter( count( $options ) === count( self::$constructorOptions ) &&
-                       !array_diff( self::$constructorOptions, array_keys( $options ) ),
-                       '$options', 'Wrong set of options present' );
+       public function __construct( ServiceOptions $options, Language $contLang ) {
+               $options->assertRequiredOptions( self::$constructorOptions );
                $this->options = $options;
                $this->contLang = $contLang;
        }
@@ -268,32 +265,32 @@ class SpecialPageFactory {
                if ( !is_array( $this->list ) ) {
                        $this->list = self::$coreList;
 
-                       if ( !$this->options['DisableInternalSearch'] ) {
+                       if ( !$this->options->get( 'DisableInternalSearch' ) ) {
                                $this->list['Search'] = \SpecialSearch::class;
                        }
 
-                       if ( $this->options['EmailAuthentication'] ) {
+                       if ( $this->options->get( 'EmailAuthentication' ) ) {
                                $this->list['Confirmemail'] = \EmailConfirmation::class;
                                $this->list['Invalidateemail'] = \EmailInvalidation::class;
                        }
 
-                       if ( $this->options['EnableEmail'] ) {
+                       if ( $this->options->get( 'EnableEmail' ) ) {
                                $this->list['ChangeEmail'] = \SpecialChangeEmail::class;
                        }
 
-                       if ( $this->options['EnableJavaScriptTest'] ) {
+                       if ( $this->options->get( 'EnableJavaScriptTest' ) ) {
                                $this->list['JavaScriptTest'] = \SpecialJavaScriptTest::class;
                        }
 
-                       if ( $this->options['PageLanguageUseDB'] ) {
+                       if ( $this->options->get( 'PageLanguageUseDB' ) ) {
                                $this->list['PageLanguage'] = \SpecialPageLanguage::class;
                        }
-                       if ( $this->options['ContentHandlerUseDB'] ) {
+                       if ( $this->options->get( 'ContentHandlerUseDB' ) ) {
                                $this->list['ChangeContentModel'] = \SpecialChangeContentModel::class;
                        }
 
                        // Add extension special pages
-                       $this->list = array_merge( $this->list, $this->options['SpecialPages'] );
+                       $this->list = array_merge( $this->list, $this->options->get( 'SpecialPages' ) );
 
                        // This hook can be used to disable unwanted core special pages
                        // or conditionally register special pages.
index c326257..bac059d 100644 (file)
@@ -110,7 +110,14 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                }
        }
 
-       public static function checkStructuredFilterUiEnabled( Config $config, User $user ) {
+       /**
+        * @see ChangesListSpecialPage::checkStructuredFilterUiEnabled
+        */
+       public static function checkStructuredFilterUiEnabled( $user ) {
+               if ( $user instanceof Config ) {
+                       wfDeprecated( __METHOD__ . ' with Config argument', '1.34' );
+                       $user = func_get_arg( 1 );
+               }
                return !$user->getOption( 'wlenhancedfilters-disable' );
        }
 
index 2d91d4d..8a0bfad 100644 (file)
@@ -98,12 +98,7 @@ class ReadOnlyModeTest extends MediaWikiTestCase {
        }
 
        private function createMode( $params, $makeLB ) {
-               $config = new HashConfig( [
-                       'ReadOnly' => $params['confMessage'],
-                       'ReadOnlyFile' => $this->createFile( $params ),
-               ] );
-
-               $rom = new ConfiguredReadOnlyMode( $config );
+               $rom = new ConfiguredReadOnlyMode( $params['confMessage'], $this->createFile( $params ) );
 
                if ( $makeLB ) {
                        $lb = $this->createLB( $params );
index f8399a3..0011d7a 100644 (file)
@@ -121,6 +121,7 @@ class ApiParseTest extends ApiTestCase {
 
                $this->setMwGlobals( 'wgExtraInterlanguageLinkPrefixes', [ 'madeuplanguage' ] );
                $this->tablesUsed[] = 'interwiki';
+               $this->overrideMwServices();
        }
 
        /**
@@ -581,8 +582,6 @@ class ApiParseTest extends ApiTestCase {
         * @param array $arr Extra params to add to API request
         */
        private function doTestLangLinks( array $arr = [] ) {
-               $this->setupInterwiki();
-
                $res = $this->doApiRequest( array_merge( [
                        'action' => 'parse',
                        'title' => 'Omelette',
@@ -600,10 +599,12 @@ class ApiParseTest extends ApiTestCase {
        }
 
        public function testLangLinks() {
+               $this->setupInterwiki();
                $this->doTestLangLinks();
        }
 
        public function testLangLinksWithSkin() {
+               $this->setupInterwiki();
                $this->setupSkin();
                $this->doTestLangLinks( [ 'useskin' => 'testing' ] );
        }
diff --git a/tests/phpunit/includes/config/ServiceOptionsTest.php b/tests/phpunit/includes/config/ServiceOptionsTest.php
new file mode 100644 (file)
index 0000000..966cf41
--- /dev/null
@@ -0,0 +1,149 @@
+<?php
+
+use MediaWiki\Config\ServiceOptions;
+
+/**
+ * @coversDefaultClass \MediaWiki\Config\ServiceOptions
+ */
+class ServiceOptionsTest extends MediaWikiTestCase {
+       public static $testObj;
+
+       public static function setUpBeforeClass() {
+               parent::setUpBeforeClass();
+
+               self::$testObj = new stdclass();
+       }
+
+       /**
+        * @dataProvider provideConstructor
+        * @covers ::__construct
+        * @covers ::assertRequiredOptions
+        * @covers ::get
+        */
+       public function testConstructor( $expected, $keys, ...$sources ) {
+               $options = new ServiceOptions( $keys, ...$sources );
+
+               foreach ( $expected as $key => $val ) {
+                       $this->assertSame( $val, $options->get( $key ) );
+               }
+
+               // This is lumped in the same test because there's no support for depending on a test that
+               // has a data provider.
+               $options->assertRequiredOptions( array_keys( $expected ) );
+
+               // Suppress warning if no assertions were run. This is expected for empty arguments.
+               $this->assertTrue( true );
+       }
+
+       public function provideConstructor() {
+               return [
+                       'No keys' => [ [], [], [ 'a' => 'aval' ] ],
+                       'Simple array source' => [
+                               [ 'a' => 'aval', 'b' => 'bval' ],
+                               [ 'a', 'b' ],
+                               [ 'a' => 'aval', 'b' => 'bval', 'c' => 'cval' ],
+                       ],
+                       'Simple HashConfig source' => [
+                               [ 'a' => 'aval', 'b' => 'bval' ],
+                               [ 'a', 'b' ],
+                               new HashConfig( [ 'a' => 'aval', 'b' => 'bval', 'c' => 'cval' ] ),
+                       ],
+                       'Three different sources' => [
+                               [ 'a' => 'aval', 'b' => 'bval' ],
+                               [ 'a', 'b' ],
+                               [ 'z' => 'zval' ],
+                               new HashConfig( [ 'a' => 'aval', 'c' => 'cval' ] ),
+                               [ 'b' => 'bval', 'd' => 'dval' ],
+                       ],
+                       'null key' => [
+                               [ 'a' => null ],
+                               [ 'a' ],
+                               [ 'a' => null ],
+                       ],
+                       'Numeric option name' => [
+                               [ '0' => 'nothing' ],
+                               [ '0' ],
+                               [ '0' => 'nothing' ],
+                       ],
+                       'Multiple sources for one key' => [
+                               [ 'a' => 'winner' ],
+                               [ 'a' ],
+                               [ 'a' => 'winner' ],
+                               [ 'a' => 'second place' ],
+                       ],
+                       'Object value is passed by reference' => [
+                               [ 'a' => self::$testObj ],
+                               [ 'a' ],
+                               [ 'a' => self::$testObj ],
+                       ],
+               ];
+       }
+
+       /**
+        * @covers ::__construct
+        */
+       public function testKeyNotFound() {
+               $this->setExpectedException( InvalidArgumentException::class,
+                       'Key "a" not found in input sources' );
+
+               new ServiceOptions( [ 'a' ], [ 'b' => 'bval' ], [ 'c' => 'cval' ] );
+       }
+
+       /**
+        * @covers ::__construct
+        * @covers ::assertRequiredOptions
+        */
+       public function testOutOfOrderAssertRequiredOptions() {
+               $options = new ServiceOptions( [ 'a', 'b' ], [ 'a' => '', 'b' => '' ] );
+               $options->assertRequiredOptions( [ 'b', 'a' ] );
+               $this->assertTrue( true, 'No exception thrown' );
+       }
+
+       /**
+        * @covers ::__construct
+        * @covers ::get
+        */
+       public function testGetUnrecognized() {
+               $this->setExpectedException( InvalidArgumentException::class,
+                       'Unrecognized option "b"' );
+
+               $options = new ServiceOptions( [ 'a' ], [ 'a' => '' ] );
+               $options->get( 'b' );
+       }
+
+       /**
+        * @covers ::__construct
+        * @covers ::assertRequiredOptions
+        */
+       public function testExtraKeys() {
+               $this->setExpectedException( Wikimedia\Assert\PreconditionException::class,
+                       'Precondition failed: Unsupported options passed: b, c!' );
+
+               $options = new ServiceOptions( [ 'a', 'b', 'c' ], [ 'a' => '', 'b' => '', 'c' => '' ] );
+               $options->assertRequiredOptions( [ 'a' ] );
+       }
+
+       /**
+        * @covers ::__construct
+        * @covers ::assertRequiredOptions
+        */
+       public function testMissingKeys() {
+               $this->setExpectedException( Wikimedia\Assert\PreconditionException::class,
+                       'Precondition failed: Required options missing: a, b!' );
+
+               $options = new ServiceOptions( [ 'c' ], [ 'c' => '' ] );
+               $options->assertRequiredOptions( [ 'a', 'b', 'c' ] );
+       }
+
+       /**
+        * @covers ::__construct
+        * @covers ::assertRequiredOptions
+        */
+       public function testExtraAndMissingKeys() {
+               $this->setExpectedException( Wikimedia\Assert\PreconditionException::class,
+                       'Precondition failed: Unsupported options passed: b! Required options missing: c!' );
+
+               $options = new ServiceOptions( [ 'a', 'b' ], [ 'a' => '', 'b' => '' ] );
+               $options->assertRequiredOptions( [ 'a', 'c' ] );
+       }
+}
index 48a9ecd..bcd5c37 100644 (file)
@@ -1,6 +1,7 @@
 <?php
 
 use MediaWiki\Auth\AuthManager;
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Preferences\DefaultPreferencesFactory;
 use Wikimedia\TestingAccessWrapper;
@@ -52,7 +53,7 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
         */
        protected function getPreferencesFactory() {
                return new DefaultPreferencesFactory(
-                       $this->config,
+                       new ServiceOptions( DefaultPreferencesFactory::$constructorOptions, $this->config ),
                        new Language(),
                        AuthManager::singleton(),
                        MediaWikiServices::getInstance()->getLinkRenderer()