From: Aryeh Gregor Date: Wed, 1 May 2019 12:54:54 +0000 (+0300) Subject: Make RepoGroup a service instead of singleton X-Git-Tag: 1.34.0-rc.0~1798^2 X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=e6691999f42372577641802112c99724a29ae856;p=lhc%2Fweb%2Fwiklou.git Make RepoGroup a service instead of singleton Change-Id: Id1661bf992ee7b7a1822f52fdfefe8e045b9f280 --- diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 76ee2efff1..98ed961605 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -142,6 +142,9 @@ because of Phabricator reports. MultiHttpClient directly. * Http::$httpEngine is deprecated and has no replacement. The default 'guzzle' engine will eventually be made the only engine for HTTP requests. +* RepoGroup::singleton(), RepoGroup::destroySingleton(), + RepoGroup::setSingleton(), wfFindFile(), and wfLocalFile() are all + deprecated. Use MediaWikiServices instead. === Other changes in 1.34 === * … diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index c7a45c72b3..66a4d9a34f 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -2633,25 +2633,25 @@ function wfGetLBFactory() { /** * Find a file. - * Shortcut for RepoGroup::singleton()->findFile() - * + * @deprecated since 1.34, use MediaWikiServices * @param string|LinkTarget $title String or LinkTarget object * @param array $options Associative array of options (see RepoGroup::findFile) * @return File|bool File, or false if the file does not exist */ function wfFindFile( $title, $options = [] ) { - return RepoGroup::singleton()->findFile( $title, $options ); + return MediaWikiServices::getInstance()->getRepoGroup()->findFile( $title, $options ); } /** * Get an object referring to a locally registered file. * Returns a valid placeholder object if the file does not exist. * + * @deprecated since 1.34, use MediaWikiServices * @param Title|string $title * @return LocalFile|null A File, or null if passed an invalid Title */ function wfLocalFile( $title ) { - return RepoGroup::singleton()->getLocalRepo()->newFile( $title ); + return MediaWikiServices::getInstance()->getRepoGroup()->getLocalRepo()->newFile( $title ); } /** diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index c374a62523..d6f50bf8aa 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -48,6 +48,7 @@ use ParserCache; use ParserFactory; use PasswordFactory; use ProxyLookup; +use RepoGroup; use ResourceLoader; use SearchEngine; use SearchEngineConfig; @@ -789,6 +790,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'ReadOnlyMode' ); } + /** + * @since 1.34 + * @return RepoGroup + */ + public function getRepoGroup() : RepoGroup { + return $this->getService( 'RepoGroup' ); + } + /** * @since 1.33 * @return ResourceLoader diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 44ab5e28ea..9836736394 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -484,6 +484,15 @@ return [ ); }, + 'RepoGroup' => function ( MediaWikiServices $services ) : RepoGroup { + $config = $services->getMainConfig(); + return new RepoGroup( + $config->get( 'LocalFileRepo' ), + $config->get( 'ForeignFileRepos' ), + $services->getMainWANObjectCache() + ); + }, + '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. diff --git a/includes/filerepo/RepoGroup.php b/includes/filerepo/RepoGroup.php index b6c70ab65e..8047835212 100644 --- a/includes/filerepo/RepoGroup.php +++ b/includes/filerepo/RepoGroup.php @@ -35,6 +35,9 @@ class RepoGroup { /** @var FileRepo[] */ protected $foreignRepos; + /** @var WANObjectCache */ + protected $wanCache; + /** @var bool */ protected $reposInitialised = false; @@ -47,66 +50,60 @@ class RepoGroup { /** @var ProcessCacheLRU */ protected $cache; - /** @var RepoGroup */ - protected static $instance; - /** Maximum number of cache items */ const MAX_CACHE_SIZE = 500; /** - * Get a RepoGroup instance. At present only one instance of RepoGroup is - * needed in a MediaWiki invocation, this may change in the future. + * @deprecated since 1.34, use MediaWikiServices::getRepoGroup * @return RepoGroup */ static function singleton() { - if ( self::$instance ) { - return self::$instance; - } - global $wgLocalFileRepo, $wgForeignFileRepos; - /** @var array $wgLocalFileRepo */ - self::$instance = new RepoGroup( $wgLocalFileRepo, $wgForeignFileRepos ); - - return self::$instance; + return MediaWikiServices::getInstance()->getRepoGroup(); } /** - * Destroy the singleton instance, so that a new one will be created next - * time singleton() is called. + * @deprecated since 1.34, use MediaWikiTestCase::overrideMwServices() or similar. This will + * cause bugs if you don't reset all other services that depend on this one at the same time. */ static function destroySingleton() { - self::$instance = null; + MediaWikiServices::getInstance()->resetServiceForTesting( 'RepoGroup' ); } /** - * Set the singleton instance to a given object - * Used by extensions which hook into the Repo chain. - * It's not enough to just create a superclass ... you have - * to get people to call into it even though all they know is RepoGroup::singleton() - * + * @deprecated since 1.34, use MediaWikiTestCase::setService, this can mess up state of other + * tests * @param RepoGroup $instance */ static function setSingleton( $instance ) { - self::$instance = $instance; + $services = MediaWikiServices::getInstance(); + $services->disableService( 'RepoGroup' ); + $services->redefineService( 'RepoGroup', + function () use ( $instance ) { + return $instance; + } + ); } /** - * Construct a group of file repositories. + * Construct a group of file repositories. Do not call this -- use + * MediaWikiServices::getRepoGroup. * * @param array $localInfo Associative array for local repo's info * @param array $foreignInfo Array of repository info arrays. * Each info array is an associative array with the 'class' member * giving the class name. The entire array is passed to the repository * constructor as the first parameter. + * @param WANObjectCache $wanCache */ - function __construct( $localInfo, $foreignInfo ) { + function __construct( $localInfo, $foreignInfo, $wanCache ) { $this->localInfo = $localInfo; $this->foreignInfo = $foreignInfo; $this->cache = new MapCacheLRU( self::MAX_CACHE_SIZE ); + $this->wanCache = $wanCache; } /** * Search repositories for an image. - * You can also use wfFindFile() to do this. * * @param Title|string $title Title object or string * @param array $options Associative array of options: @@ -419,8 +416,7 @@ class RepoGroup { protected function newRepo( $info ) { $class = $info['class']; - $cache = MediaWikiServices::getInstance()->getMainWANObjectCache(); - $info['wanCache'] = $cache; + $info['wanCache'] = $this->wanCache; return new $class( $info ); } diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php index 606bedbb92..df897d9a1e 100644 --- a/tests/parser/ParserTestRunner.php +++ b/tests/parser/ParserTestRunner.php @@ -289,9 +289,14 @@ class ParserTestRunner { // All FileRepo changes should be done here by injecting services, // there should be no need to change global variables. - RepoGroup::setSingleton( $this->createRepoGroup() ); + MediaWikiServices::getInstance()->disableService( 'RepoGroup' ); + MediaWikiServices::getInstance()->redefineService( 'RepoGroup', + function () { + return $this->createRepoGroup(); + } + ); $teardown[] = function () { - RepoGroup::destroySingleton(); + MediaWikiServices::getInstance()->resetServiceForTesting( 'RepoGroup' ); }; // Set up null lock managers @@ -449,7 +454,8 @@ class ParserTestRunner { 'transformVia404' => false, 'backend' => $backend ], - [] + [], + MediaWikiServices::getInstance()->getMainWANObjectCache() ); } diff --git a/tests/phpunit/includes/ContentSecurityPolicyTest.php b/tests/phpunit/includes/ContentSecurityPolicyTest.php index 5f0200d09d..a758f990c9 100644 --- a/tests/phpunit/includes/ContentSecurityPolicyTest.php +++ b/tests/phpunit/includes/ContentSecurityPolicyTest.php @@ -37,7 +37,7 @@ class ContentSecurityPolicyTest extends MediaWikiTestCase { // Note, there are some obscure globals which // could affect the results which aren't included above. - RepoGroup::destroySingleton(); + $this->overrideMwServices(); $context = RequestContext::getMain(); $resp = $context->getRequest()->response(); $conf = $context->getConfig(); diff --git a/tests/phpunit/includes/filerepo/RepoGroupTest.php b/tests/phpunit/includes/filerepo/RepoGroupTest.php index 5a343f6529..67de6982e6 100644 --- a/tests/phpunit/includes/filerepo/RepoGroupTest.php +++ b/tests/phpunit/includes/filerepo/RepoGroupTest.php @@ -7,7 +7,7 @@ class RepoGroupTest extends MediaWikiTestCase { function testHasForeignRepoNegative() { $this->setMwGlobals( 'wgForeignFileRepos', [] ); - RepoGroup::destroySingleton(); + $this->overrideMwServices(); FileBackendGroup::destroySingleton(); $this->assertFalse( RepoGroup::singleton()->hasForeignRepos() ); } @@ -27,7 +27,7 @@ class RepoGroupTest extends MediaWikiTestCase { function testForEachForeignRepoNone() { $this->setMwGlobals( 'wgForeignFileRepos', [] ); - RepoGroup::destroySingleton(); + $this->overrideMwServices(); FileBackendGroup::destroySingleton(); $fakeCallback = $this->createMock( RepoGroupTestHelper::class ); $fakeCallback->expects( $this->never() )->method( 'callback' ); @@ -48,7 +48,7 @@ class RepoGroupTest extends MediaWikiTestCase { 'apiThumbCacheExpiry' => 86400, 'directory' => $wgUploadDirectory ] ] ); - RepoGroup::destroySingleton(); + $this->overrideMwServices(); FileBackendGroup::destroySingleton(); } } diff --git a/tests/phpunit/suites/UploadFromUrlTestSuite.php b/tests/phpunit/suites/UploadFromUrlTestSuite.php index 3b6d6f219b..d340221b8b 100644 --- a/tests/phpunit/suites/UploadFromUrlTestSuite.php +++ b/tests/phpunit/suites/UploadFromUrlTestSuite.php @@ -1,5 +1,7 @@ resetServiceForTesting( 'RepoGroup' ); FileBackendGroup::destroySingleton(); } @@ -80,7 +82,7 @@ class UploadFromUrlTestSuite extends PHPUnit_Framework_TestSuite { $GLOBALS[$var] = $val; } // Restore backends - RepoGroup::destroySingleton(); + MediaWikiServices::getInstance()->resetServiceForTesting( 'RepoGroup' ); FileBackendGroup::destroySingleton(); parent::tearDown();