Merge "resourceloader: Warn on ResourceLoader::construct without Config"

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 418f532..515f287 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -236,15 +236,14 @@ class ResourceLoader implements LoggerAwareInterface {
 
        /**
         * Register core modules and runs registration hooks.
-        * @param Config|null $config [optional]
+        * @param Config|null $config
         * @param LoggerInterface|null $logger [optional]
         */
        public function __construct( Config $config = null, LoggerInterface $logger = null ) {
                $this->logger = $logger ?: new NullLogger();
 
                if ( !$config ) {
-                       // TODO: Deprecate and remove.
-                       $this->logger->debug( __METHOD__ . ' was called without providing a Config instance' );
+                       wfDeprecated( __METHOD__ . ' without a Config instance', '1.34' );
                        $config = MediaWikiServices::getInstance()->getMainConfig();
                }
                $this->config = $config;

diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index c834db1..d6cc646 100644 (file)
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -70,14 +70,14 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                // Build list of variables
                $skin = $context->getSkin();
                $vars = [
-                       'wgLoadScript' => wfScript( 'load' ),
+                       'wgLoadScript' => $conf->get( 'LoadScript' ),
                        'debug' => $context->getDebug(),
                        'skin' => $skin,
                        'stylepath' => $conf->get( 'StylePath' ),
                        'wgUrlProtocols' => wfUrlProtocols(),
                        'wgArticlePath' => $conf->get( 'ArticlePath' ),
                        'wgScriptPath' => $conf->get( 'ScriptPath' ),
-                       'wgScript' => wfScript(),
+                       'wgScript' => $conf->get( 'Script' ),
                        'wgSearchType' => $conf->get( 'SearchType' ),
                        'wgVariantArticlePath' => $conf->get( 'VariantArticlePath' ),
                        // Force object to avoid "empty" associative array from

diff --git a/tests/phpunit/ResourceLoaderTestCase.php b/tests/phpunit/ResourceLoaderTestCase.php
index e9a8a1f..bd6df5f 100644 (file)
--- a/tests/phpunit/ResourceLoaderTestCase.php
+++ b/tests/phpunit/ResourceLoaderTestCase.php
@@ -2,7 +2,6 @@
 
 use MediaWiki\MediaWikiServices;
 use Psr\Log\LoggerInterface;
-use Psr\Log\NullLogger;
 
 abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
        // Version hash for a blank file module.
@@ -34,7 +33,7 @@ abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
                        'only' => 'scripts',
                        'safemode' => null,
                ];
-               $resourceLoader = $rl ?: new ResourceLoader();
+               $resourceLoader = $rl ?: new ResourceLoader( MediaWikiServices::getInstance()->getMainConfig() );
                $request = new FauxRequest( [
                                'debug' => $options['debug'],
                                'lang' => $options['lang'],
@@ -57,16 +56,23 @@ abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
                        // For ResourceLoader::inDebugMode since it doesn't have context
                        'ResourceLoaderDebug' => true,
 
-                       // Avoid influence from wgInvalidateCacheOnLocalSettingsChange
-                       'CacheEpoch' => '20140101000000',
-
-                       // For wfScript()
+                       // For ResourceLoaderStartUpModule and ResourceLoader::__construct()
                        'ScriptPath' => '/w',
                        'Script' => '/w/index.php',
                        'LoadScript' => '/w/load.php',
+
+                       // For ResourceLoader::register() - TODO: Inject somehow T32956
+                       'ResourceModuleSkinStyles' => [],
+
+                       // For ResourceLoader::respond() - TODO: Inject somehow T32956
+                       'UseFileCache' => false,
                ];
        }
 
+       public static function getMinimalConfig() {
+               return new HashConfig( self::getSettings() );
+       }
+
        protected function setUp() {
                parent::setUp();
 
@@ -171,14 +177,8 @@ class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {
 }
 
 class EmptyResourceLoader extends ResourceLoader {
-       // TODO: This won't be needed once ResourceLoader is empty by default
-       // and default registrations are done from ServiceWiring instead.
        public function __construct( Config $config = null, LoggerInterface $logger = null ) {
-               $this->setLogger( $logger ?: new NullLogger() );
-               $this->config = $config ?: MediaWikiServices::getInstance()->getMainConfig();
-               // Source "local" is required by StartupModule
-               $this->addSource( 'local', $this->config->get( 'LoadScript' ) );
-               $this->setMessageBlobStore( new MessageBlobStore( $this, $this->getLogger() ) );
+               parent::__construct( $config ?: ResourceLoaderTestCase::getMinimalConfig(), $logger );
        }
 
        public function getErrors() {

diff --git a/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php b/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php
index df9ddee..e178e96 100644 (file)
--- a/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php
+++ b/tests/phpunit/includes/resourceloader/DerivativeResourceLoaderContextTest.php
@@ -16,7 +16,10 @@ class DerivativeResourceLoaderContextTest extends PHPUnit\Framework\TestCase {
                                'skin' => 'fallback',
                                'target' => 'test',
                ] );
-               return new ResourceLoaderContext( new ResourceLoader(), $request );
+               return new ResourceLoaderContext(
+                       new ResourceLoader( ResourceLoaderTestCase::getMinimalConfig() ),
+                       $request
+               );
        }
 
        public function testChangeModules() {

diff --git a/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php b/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
index 9afa232..e094d92 100644 (file)
--- a/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
+++ b/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
@@ -25,7 +25,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
 
        public function testBlobCreation() {
                $module = $this->makeModule( [ 'mainpage' ] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
 
                $blobStore = $this->makeBlobStore( null, $rl );
@@ -36,7 +36,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
 
        public function testBlobCreation_empty() {
                $module = $this->makeModule( [] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
 
                $blobStore = $this->makeBlobStore( null, $rl );
@@ -47,7 +47,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
 
        public function testBlobCreation_unknownMessage() {
                $module = $this->makeModule( [ 'i-dont-exist', 'mainpage', 'i-dont-exist2' ] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( null, $rl );
 
@@ -59,7 +59,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
 
        public function testMessageCachingAndPurging() {
                $module = $this->makeModule( [ 'example' ] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
 
@@ -104,7 +104,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
 
        public function testPurgeEverything() {
                $module = $this->makeModule( [ 'example' ] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
                // Advance this new WANObjectCache instance to a normal state.
@@ -138,7 +138,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        public function testValidateAgainstModuleRegistry() {
                // Arrange version 1 of a module
                $module = $this->makeModule( [ 'foo' ] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
                $blobStore->expects( $this->once() )
@@ -157,7 +157,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
                // must always match the set of message keys required by the module.
                // We do not receive purges for this because no messages were changed.
                $module = $this->makeModule( [ 'foo', 'bar' ] );
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
                $blobStore->expects( $this->exactly( 2 ) )

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php
index 60cd4a8..7f4d9a8 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderContextTest.php
@@ -15,6 +15,8 @@ class ResourceLoaderContextTest extends PHPUnit\Framework\TestCase {
                return new EmptyResourceLoader( new HashConfig( [
                        'ResourceLoaderDebug' => false,
                        'LoadScript' => '/w/load.php',
+                       // For ResourceLoader::register()
+                       'ResourceModuleSkinStyles' => [],
                ] ) );
        }
 

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index 85a47de..2e19e1c 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -111,7 +111,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
                $resourceLoader->register( 'test.foo', new ResourceLoaderTestModule() );
                $resourceLoader->register( 'test.bar', new ResourceLoaderTestModule() );
                $this->assertEquals(
-                       [ 'test.foo', 'test.bar' ],
+                       [ 'startup', 'test.foo', 'test.bar' ],
                        $resourceLoader->getModuleNames()
                );
        }
@@ -318,7 +318,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
         * @covers ResourceLoader::getSources
         */
        public function testAddSource( $name, $info, $expected ) {
-               $rl = new ResourceLoader;
+               $rl = new EmptyResourceLoader;
                $rl->addSource( $name, $info );
                if ( is_array( $expected ) ) {
                        foreach ( $expected as $source ) {
@@ -333,7 +333,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
         * @covers ResourceLoader::addSource
         */
        public function testAddSourceDupe() {
-               $rl = new ResourceLoader;
+               $rl = new EmptyResourceLoader;
                $this->setExpectedException(
                        MWException::class, 'ResourceLoader duplicate source addition error'
                );
@@ -345,7 +345,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
         * @covers ResourceLoader::addSource
         */
        public function testAddSourceInvalid() {
-               $rl = new ResourceLoader;
+               $rl = new EmptyResourceLoader;
                $this->setExpectedException( MWException::class, 'with no "loadScript" key' );
                $rl->addSource( 'foo',  [ 'x' => 'https://example.org/w/load.php' ] );
        }
@@ -623,7 +623,7 @@ END
         * @covers ResourceLoader::getLoadScript
         */
        public function testGetLoadScript() {
-               $rl = new ResourceLoader();
+               $rl = new EmptyResourceLoader();
                $sources = self::fakeSources();
                $rl->addSource( $sources );
                foreach ( [ 'examplewiki', 'example2wiki' ] as $name ) {
@@ -881,12 +881,13 @@ END
         * @covers ResourceLoader::makeModuleResponse
         */
        public function testMakeModuleResponseStartupError() {
-               $rl = new EmptyResourceLoader();
+               // This is an integration test that uses a lot of MediaWiki state,
+               // provide the full Config object here.
+               $rl = new EmptyResourceLoader( MediaWikiServices::getInstance()->getMainConfig() );
                $rl->register( [
                        'foo' => self::getSimpleModuleMock( 'foo();' ),
                        'ferry' => self::getFailFerryMock(),
                        'bar' => self::getSimpleModuleMock( 'bar();' ),
-                       'startup' => [ 'class' => ResourceLoaderStartUpModule::class ],
                ] );
                $context = $this->getResourceLoaderContext(
                        [
@@ -897,7 +898,7 @@ END
                );
 
                $this->assertEquals(
-                       [ 'foo', 'ferry', 'bar', 'startup' ],
+                       [ 'startup', 'foo', 'ferry', 'bar' ],
                        $rl->getModuleNames(),
                        'getModuleNames'
                );