Make RepoGroup a service instead of singleton
authorAryeh Gregor <ayg@aryeh.name>
Wed, 1 May 2019 12:54:54 +0000 (15:54 +0300)
committerAryeh Gregor <ayg@aryeh.name>
Mon, 6 May 2019 10:57:40 +0000 (13:57 +0300)
Change-Id: Id1661bf992ee7b7a1822f52fdfefe8e045b9f280

RELEASE-NOTES-1.34
includes/GlobalFunctions.php
includes/MediaWikiServices.php
includes/ServiceWiring.php
includes/filerepo/RepoGroup.php
tests/parser/ParserTestRunner.php
tests/phpunit/includes/ContentSecurityPolicyTest.php
tests/phpunit/includes/filerepo/RepoGroupTest.php
tests/phpunit/suites/UploadFromUrlTestSuite.php

index 76ee2ef..98ed961 100644 (file)
@@ -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 ===
 * …
index c7a45c7..66a4d9a 100644 (file)
@@ -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 );
 }
 
 /**
index c374a62..d6f50bf 100644 (file)
@@ -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
index 44ab5e2..9836736 100644 (file)
@@ -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.
index b6c70ab..8047835 100644 (file)
@@ -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 );
        }
index 606bedb..df897d9 100644 (file)
@@ -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()
                );
        }
 
index 5f0200d..a758f99 100644 (file)
@@ -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();
index 5a343f6..67de698 100644 (file)
@@ -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();
        }
 }
index 3b6d6f2..d340221 100644 (file)
@@ -1,5 +1,7 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
+
 require_once dirname( __DIR__ ) . '/includes/upload/UploadFromUrlTest.php';
 
 class UploadFromUrlTestSuite extends PHPUnit_Framework_TestSuite {
@@ -71,7 +73,7 @@ class UploadFromUrlTestSuite extends PHPUnit_Framework_TestSuite {
                        $wgStyleDirectory = "$IP/skins";
                }
 
-               RepoGroup::destroySingleton();
+               MediaWikiServices::getInstance()->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();