From a186dc62fd5ac334488f9566748f6fd65bdcea35 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sat, 16 Feb 2019 23:16:09 +0000 Subject: [PATCH] resourceloader: Instantiate main class via ServiceWiring It also removes some code duplication which is nice. This unlocks various future changes, including: * Making the `$config` parameter mandatory for the ResourceLoader class constructor, which currently falls back to global state. This should be deprecated and removed. * Making it possible to instantiate the ResourceLoader class without all the default MW modules being registered from global state. E.g. move MW module registration from main class constructor to ServiceWiring, and remove the 'EmptyResourceLoader' class hack from unit tests, and use regular 'new ResourceLoader' instead. * Making ResourceLoader a standalone library (some day), e.g. allowing it to be instantiated from a basic PHP script, in a way that is still useful and perhaps able to serve (most) RL modules without MW itself. Bug: T32956 Change-Id: I4939f296c705b268e9cf8de635e923a739410470 --- includes/MediaWikiServices.php | 9 +++++++++ includes/OutputPage.php | 7 ++----- includes/ServiceWiring.php | 7 +++++++ includes/cache/MessageBlobStore.php | 3 ++- includes/installer/WebInstallerOutput.php | 4 +++- includes/resourceloader/ResourceLoader.php | 1 + includes/resourceloader/ResourceLoaderContext.php | 11 +++++++++++ load.php | 7 +------ maintenance/cleanupRemovedModules.php | 2 +- tests/phpunit/structure/ResourcesTest.php | 8 +++++--- tests/phpunit/suites/LessTestSuite.php | 4 +++- 11 files changed, 45 insertions(+), 18 deletions(-) diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 4abd729d45..292e8dfa14 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -44,6 +44,7 @@ use ParserCache; use ParserFactory; use PasswordFactory; use ProxyLookup; +use ResourceLoader; use SearchEngine; use SearchEngineConfig; use SearchEngineFactory; @@ -744,6 +745,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'ReadOnlyMode' ); } + /** + * @since 1.33 + * @return ResourceLoader + */ + public function getResourceLoader() { + return $this->getService( 'ResourceLoader' ); + } + /** * @since 1.31 * @return RevisionFactory diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 61a1ef2d87..9b766bb2d4 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -21,7 +21,6 @@ */ use MediaWiki\Linker\LinkTarget; -use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; use MediaWiki\Session\SessionManager; use Wikimedia\Rdbms\IResultWrapper; @@ -3276,10 +3275,8 @@ class OutputPage extends ContextSource { */ public function getResourceLoader() { if ( is_null( $this->mResourceLoader ) ) { - $this->mResourceLoader = new ResourceLoader( - $this->getConfig(), - LoggerFactory::getInstance( 'resourceloader' ) - ); + // Lazy-initialise as needed + $this->mResourceLoader = MediaWikiServices::getInstance()->getResourceLoader(); } return $this->mResourceLoader; } diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 44ca5025dd..46dd9133b1 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -413,6 +413,13 @@ return [ ); }, + 'ResourceLoader' => function ( MediaWikiServices $services ) : ResourceLoader { + return new ResourceLoader( + $services->getMainConfig(), + LoggerFactory::getInstance( 'resourceloader' ) + ); + }, + 'RevisionFactory' => function ( MediaWikiServices $services ) : RevisionFactory { return $services->getRevisionStore(); }, diff --git a/includes/cache/MessageBlobStore.php b/includes/cache/MessageBlobStore.php index 65e659d39c..19c49972f7 100644 --- a/includes/cache/MessageBlobStore.php +++ b/includes/cache/MessageBlobStore.php @@ -23,6 +23,7 @@ * @author Timo Tijhof */ +use MediaWiki\MediaWikiServices; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -194,7 +195,7 @@ class MessageBlobStore implements LoggerAwareInterface { // Lazy-initialise this property because most callers don't need it. if ( $this->resourceloader === null ) { $this->logger->warning( __CLASS__ . ' created without a ResourceLoader instance' ); - $this->resourceloader = new ResourceLoader(); + $this->resourceloader = MediaWikiServices::getInstance()->getResourceLoader(); } return $this->resourceloader; } diff --git a/includes/installer/WebInstallerOutput.php b/includes/installer/WebInstallerOutput.php index ae07d0cbda..b061d0da36 100644 --- a/includes/installer/WebInstallerOutput.php +++ b/includes/installer/WebInstallerOutput.php @@ -21,6 +21,8 @@ * @ingroup Deployment */ +use MediaWiki\MediaWikiServices; + /** * Output class modelled on OutputPage. * @@ -147,7 +149,7 @@ class WebInstallerOutput { 'mediawiki.skinning.interface', ]; - $resourceLoader = new ResourceLoader(); + $resourceLoader = MediaWikiServices::getInstance()->getResourceLoader(); if ( file_exists( "$wgStyleDirectory/Vector/skin.json" ) ) { // Force loading Vector skin if available as a fallback skin diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index b64826063c..1a232583f6 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -245,6 +245,7 @@ class ResourceLoader implements LoggerAwareInterface { $this->logger = $logger ?: new NullLogger(); if ( !$config ) { + // TODO: Deprecate and remove. $this->logger->debug( __METHOD__ . ' was called without providing a Config instance' ); $config = MediaWikiServices::getInstance()->getMainConfig(); } diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php index 57392b97f8..d6573afc46 100644 --- a/includes/resourceloader/ResourceLoaderContext.php +++ b/includes/resourceloader/ResourceLoaderContext.php @@ -133,9 +133,20 @@ class ResourceLoaderContext implements MessageLocalizer { /** * Return a dummy ResourceLoaderContext object suitable for passing into * things that don't "really" need a context. + * + * Use cases: + * - Creating html5shiv script tag in OutputPage. + * - FileModule::readStyleFiles (deprecated, to be removed). + * - Unit tests (deprecated, create empty instance directly or use RLTestCase). + * * @return ResourceLoaderContext */ public static function newDummyContext() { + // This currently creates a non-empty instance of ResourceLoader (all modules registered), + // but that's probably not needed. So once that moves into ServiceWiring, this'll + // become more like the EmptyResourceLoader class we have in PHPUnit tests, which + // is what this should've had originally. If this turns out to be untrue, change to: + // `MediaWikiServices::getInstance()->getResourceLoader()` instead. return new self( new ResourceLoader( MediaWikiServices::getInstance()->getMainConfig(), LoggerFactory::getInstance( 'resourceloader' ) diff --git a/load.php b/load.php index 1997fe73fc..4d34e5ddca 100644 --- a/load.php +++ b/load.php @@ -22,7 +22,6 @@ * @author Trevor Parscal */ -use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; // This endpoint is supposed to be independent of request cookies and other @@ -40,11 +39,7 @@ if ( !$wgRequest->checkUrlExtension() ) { // writes when getting database connections for ResourceLoader. (T192611) MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->disableChronologyProtection(); -// Set up ResourceLoader -$resourceLoader = new ResourceLoader( - ConfigFactory::getDefaultInstance()->makeConfig( 'main' ), - LoggerFactory::getInstance( 'resourceloader' ) -); +$resourceLoader = MediaWikiServices::getInstance()->getResourceLoader(); $context = new ResourceLoaderContext( $resourceLoader, $wgRequest ); // Respond to ResourceLoader request diff --git a/maintenance/cleanupRemovedModules.php b/maintenance/cleanupRemovedModules.php index 63838d2591..cec640bf78 100644 --- a/maintenance/cleanupRemovedModules.php +++ b/maintenance/cleanupRemovedModules.php @@ -46,7 +46,7 @@ class CleanupRemovedModules extends Maintenance { $this->output( "Cleaning up module_deps table...\n" ); $dbw = $this->getDB( DB_MASTER ); - $rl = new ResourceLoader( MediaWikiServices::getInstance()->getMainConfig() ); + $rl = MediaWikiServices::getInstance()->getResourceLoader(); $moduleNames = $rl->getModuleNames(); $res = $dbw->select( 'module_deps', [ 'md_module', 'md_skin' ], diff --git a/tests/phpunit/structure/ResourcesTest.php b/tests/phpunit/structure/ResourcesTest.php index 8a08181c1e..e08ffd77eb 100644 --- a/tests/phpunit/structure/ResourcesTest.php +++ b/tests/phpunit/structure/ResourcesTest.php @@ -1,8 +1,10 @@ getResourceLoader(); $modules = []; diff --git a/tests/phpunit/suites/LessTestSuite.php b/tests/phpunit/suites/LessTestSuite.php index 26a784adca..b5bd882d50 100644 --- a/tests/phpunit/suites/LessTestSuite.php +++ b/tests/phpunit/suites/LessTestSuite.php @@ -1,5 +1,7 @@ */ @@ -7,7 +9,7 @@ class LessTestSuite extends PHPUnit_Framework_TestSuite { public function __construct() { parent::__construct(); - $resourceLoader = new ResourceLoader(); + $resourceLoader = MediaWikiServices::getInstance()->getResourceLoader(); foreach ( $resourceLoader->getModuleNames() as $name ) { $module = $resourceLoader->getModule( $name ); -- 2.20.1