Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/245484
Change subject: Move singletons of MediaWikiTitleCodec and
MediaWikiPageLinkRenderer to MediaWikiServices.
......................................................................
Move singletons of MediaWikiTitleCodec and MediaWikiPageLinkRenderer to
MediaWikiServices.
Change-Id: Icda128d4b03901efdab9c0c6ced142b3a7fe6671
---
M includes/MediaWikiServices.php
M includes/Title.php
M includes/specials/SpecialCategories.php
M includes/specials/SpecialLinkSearch.php
M tests/phpunit/includes/MediaWikiServicesTest.php
5 files changed, 148 insertions(+), 38 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/84/245484/4
diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 3606ea3..c318d2d 100644
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -74,12 +74,27 @@
}
/**
+ * @return Language
+ */
+ public function getContentLanguage() {
+ return $this->getConfig()->get( 'ContLang' );
+ }
+
+ /**
+ * @return string language code
+ */
+ public function getContentLanguageCode() {
+ return $this->getConfig()->get( 'LanguageCode' );
+ }
+
+ /**
* @param string $name
+ * @param string $reset if 'reset', creation of a fresh instance is
forced.
*
* @return object
*/
- private function getService( $name ) {
- if ( !isset( $this->services[$name] ) ) {
+ private function getService( $name, $reset = '' ) {
+ if ( $reset === 'reset' || !isset( $this->services[$name] ) ) {
$this->services[$name] = $this->createService( $name );
}
@@ -120,4 +135,87 @@
return new CachingSiteStore( $siteStore, $cache );
}
+ private $titleCodecFingerprint = null;
+
+ /**
+ * Checks the global configuration variables that affect the TitleCodec
have
+ * been modified.
+ *
+ * @note This is a hack around the fact that a) the configuration
object may depend on mutable
+ * global state and b) the title codec is accessed via static methods
in Title. Once TitleParser
+ * and TitleFormatter instances are property getting injected in all
places that need them,
+ * there will no longer be need to purge the default instance when the
config changes.
+ * Once that is the case, this method should be removed.
+ *
+ * @return bool true if the global configuration variables that affect
the TitleCodec have
+ * been modified.
+ */
+ private function checkConfigForTitleCodecChanged() {
+ $localInterwikis = $this->getConfig()->get( 'LocalInterwikis' );
+ $contLang = $this->getConfig()->get( 'ContLang' );
+
+ // $wgContLang and $wgLocalInterwikis may change (especially
while testing),
+ // make sure we are using the right one. To detect changes over
the course
+ // of a request, we remember a fingerprint of the config used
to create the
+ // codec singleton, and re-create it if the fingerprint doesn't
match.
+ $fingerprint = spl_object_hash( $contLang ) . '|' . join( '+',
$localInterwikis );
+ $changed = ( $fingerprint !== $this->titleCodecFingerprint );
+
+ $this->titleCodecFingerprint = $fingerprint;
+ return $changed;
+ }
+
+ /**
+ * @return TitleParser
+ */
+ public function getTitleParser() {
+ return $this->getService( 'TitleCodec',
$this->checkConfigForTitleCodecChanged() ? 'reset' : '' );
+ }
+
+ /**
+ * @return TitleFormatter
+ */
+ public function getTitleFormatter() {
+ // XXX: we may want to manage this per language, or at least
content language vs user language.
+ return $this->getService( 'TitleCodec',
$this->checkConfigForTitleCodecChanged() ? 'reset' : '' );
+ }
+
+ /**
+ * @note should be called by createService() only!
+ */
+ private function newTitleCodec() {
+ $localInterwikis = $this->getConfig()->get( 'LocalInterwikis' );
+
+ return new MediaWikiTitleCodec(
+ $this->getContentLanguage(),
+ $this->getGenderCache(),
+ $localInterwikis
+ );
+ }
+
+ /**
+ * @return GenderCache
+ */
+ public function getGenderCache() {
+ // @todo: manage the GenderCache singleton here instead of in
the GenderCache class.
+ return GenderCache::singleton();
+ }
+
+ /**
+ * @return PageLinkRenderer
+ */
+ public function getPageLinkRenderer() {
+ // XXX: we may want to manage this per language, or at least
content language vs user language.
+ return $this->getService( 'PageLinkRenderer' );
+ }
+
+ /**
+ * @note should be called by createService() only!
+ */
+ private function newPageLinkRenderer() {
+ return new MediaWikiPageLinkRenderer(
+ $this->getTitleFormatter()
+ );
+ }
+
}
diff --git a/includes/Title.php b/includes/Title.php
index 9ada4f3..16b7d7c 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -170,31 +170,7 @@
* @return TitleParser
*/
private static function getTitleParser() {
- global $wgContLang, $wgLocalInterwikis;
-
- static $titleCodec = null;
- static $titleCodecFingerprint = null;
-
- // $wgContLang and $wgLocalInterwikis may change (especially
while testing),
- // make sure we are using the right one. To detect changes over
the course
- // of a request, we remember a fingerprint of the config used
to create the
- // codec singleton, and re-create it if the fingerprint doesn't
match.
- $fingerprint = spl_object_hash( $wgContLang ) . '|' . join(
'+', $wgLocalInterwikis );
-
- if ( $fingerprint !== $titleCodecFingerprint ) {
- $titleCodec = null;
- }
-
- if ( !$titleCodec ) {
- $titleCodec = new MediaWikiTitleCodec(
- $wgContLang,
- GenderCache::singleton(),
- $wgLocalInterwikis
- );
- $titleCodecFingerprint = $fingerprint;
- }
-
- return $titleCodec;
+ return MediaWikiServices::getInstance()->getTitleParser();
}
/**
@@ -206,9 +182,7 @@
* @return TitleFormatter
*/
private static function getTitleFormatter() {
- // NOTE: we know that getTitleParser() returns a
MediaWikiTitleCodec,
- // which implements TitleFormatter.
- return self::getTitleParser();
+ return MediaWikiServices::getInstance()->getTitleFormatter();
}
function __construct() {
diff --git a/includes/specials/SpecialCategories.php
b/includes/specials/SpecialCategories.php
index cea6ceb..257c490 100644
--- a/includes/specials/SpecialCategories.php
+++ b/includes/specials/SpecialCategories.php
@@ -59,9 +59,7 @@
*/
private function initServices() {
if ( !$this->linkRenderer ) {
- $lang = $this->getContext()->getLanguage();
- $titleFormatter = new MediaWikiTitleCodec( $lang,
GenderCache::singleton() );
- $this->linkRenderer = new MediaWikiPageLinkRenderer(
$titleFormatter );
+ $this->linkRenderer =
MediaWikiServices::getInstance()->getPageLinkRenderer();
}
}
diff --git a/includes/specials/SpecialLinkSearch.php
b/includes/specials/SpecialLinkSearch.php
index f474867..eb5ed78 100644
--- a/includes/specials/SpecialLinkSearch.php
+++ b/includes/specials/SpecialLinkSearch.php
@@ -68,11 +68,9 @@
* This allows for dependency injection even though we don't control
object creation.
*/
private function initServices() {
- global $wgLanguageCode;
if ( !$this->linkRenderer ) {
- $lang = Language::factory( $wgLanguageCode );
- $titleFormatter = new MediaWikiTitleCodec( $lang,
GenderCache::singleton() );
- $this->linkRenderer = new MediaWikiPageLinkRenderer(
$titleFormatter );
+ $this->linkRenderer = $this->linkRenderer =
+
MediaWikiServices::getInstance()->getPageLinkRenderer();
}
}
diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php
b/tests/phpunit/includes/MediaWikiServicesTest.php
index a776920..60810a7 100644
--- a/tests/phpunit/includes/MediaWikiServicesTest.php
+++ b/tests/phpunit/includes/MediaWikiServicesTest.php
@@ -5,7 +5,7 @@
*
* @group MediaWiki
*/
-class MediaWikiServicesTest extends PHPUnit_Framework_TestCase {
+class MediaWikiServicesTest extends MediaWikiTestCase {
/**
* @param string $expectedType expected class
@@ -34,4 +34,46 @@
$this->assertGetterReturnType( 'SiteLookup', 'getSiteLookup' );
}
+ public function testGetGenderCache() {
+ $this->assertGetterReturnType( 'GenderCache', 'getGenderCache'
);
+ }
+
+ public function testGetTitleFormatter() {
+ $this->assertGetterReturnType( 'TitleFormatter',
'getTitleFormatter' );
+ }
+
+ /**
+ * Test that the TitleFormatter instance gets automatically reset when
affected by
+ * config changes, but is re-used otherwise.
+ */
+ public function testGetTitleFormatter_reset() {
+ global $wgContLang;
+ $this->stashMwGlobals( 'wgContLang' );
+
+ $locator = MediaWikiServices::getInstance();
+ $originalFormatter = $locator->getTitleFormatter();
+
+ $this->assertSame(
+ $originalFormatter,
+ $locator->getTitleFormatter(),
+ 'service instance should be cached'
+ );
+
+ $wgContLang = Language::factory( 'qqxyz' );
+
+ $this->assertNotSame(
+ $originalFormatter,
+ $locator->getTitleFormatter(),
+ 'service instance should be reset when $wgLanguageCode
changes'
+ );
+ }
+
+ public function testGetTitleParser() {
+ $this->assertGetterReturnType( 'TitleParser', 'getTitleParser'
);
+ }
+
+ public function testGetPageLinkRenderer() {
+ $this->assertGetterReturnType( 'PageLinkRenderer',
'getPageLinkRenderer' );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/245484
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icda128d4b03901efdab9c0c6ced142b3a7fe6671
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
