jenkins-bot has submitted this change and it was merged.
Change subject: resourceloader: Fix WikiModule preload to support localised
titles
......................................................................
resourceloader: Fix WikiModule preload to support localised titles
Follows-up 6f8dc27ca2 and dbd11e04aa. You'd expect a bug in preloading
to fallback to fetching it on-demand but due to a title format mismatch
the preload method did use the same title format for the cache key, but
not the same title format for discovering the results. As such, it set
the right cache key to an empty array.
* Make relevant methods testable and mockable.
* Add regression test.
* Change call to array_interect_key to use the same format as
fetchTitleInfo(): Normalised title keys from Title::getPrefixedText.
Previously, the intersect used the declared titles from getPages() which
are not localised - causing an empty array to be returned from the
intersect on wikis where the namespace name is localised.
Bug: T145673
Change-Id: Ibe788157724d73c727b9e2127b6828db32ca9420
---
M includes/resourceloader/ResourceLoaderWikiModule.php
M tests/phpunit/ResourceLoaderTestCase.php
M tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
3 files changed, 91 insertions(+), 7 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php
b/includes/resourceloader/ResourceLoaderWikiModule.php
index 5580306..4fdd86e 100644
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -296,12 +296,12 @@
sort( $pageNames );
$key = implode( '|', $pageNames );
if ( !isset( $this->titleInfo[$key] ) ) {
- $this->titleInfo[$key] = self::fetchTitleInfo( $dbr,
$pageNames, __METHOD__ );
+ $this->titleInfo[$key] = static::fetchTitleInfo( $dbr,
$pageNames, __METHOD__ );
}
return $this->titleInfo[$key];
}
- private static function fetchTitleInfo( IDatabase $db, array $pages,
$fname = __METHOD__ ) {
+ protected static function fetchTitleInfo( IDatabase $db, array $pages,
$fname = __METHOD__ ) {
$titleInfo = [];
$batch = new LinkBatch;
foreach ( $pages as $titleText ) {
@@ -353,10 +353,17 @@
}
}
}
- $allInfo = self::fetchTitleInfo( $db, array_keys( $allPages ),
__METHOD__ );
+ $allInfo = static::fetchTitleInfo( $db, array_keys( $allPages
), __METHOD__ );
foreach ( $wikiModules as $module ) {
$pages = $module->getPages( $context );
- $info = array_intersect_key( $allInfo, $pages );
+ // Before we intersect, map the names to canonical form
(T145673).
+ $intersect = [];
+ foreach ( $pages as $page => $unused ) {
+ $title = Title::newFromText( $page
)->getPrefixedText();
+ $intersect[$title] = 1;
+ }
+ $info = array_intersect_key( $allInfo, $intersect );
+
$pageNames = array_keys( $pages );
sort( $pageNames );
$key = implode( '|', $pageNames );
diff --git a/tests/phpunit/ResourceLoaderTestCase.php
b/tests/phpunit/ResourceLoaderTestCase.php
index 18a49f6..f0eb12e 100644
--- a/tests/phpunit/ResourceLoaderTestCase.php
+++ b/tests/phpunit/ResourceLoaderTestCase.php
@@ -22,9 +22,7 @@
->setConstructorArgs( [ $resourceLoader, $request ] )
->setMethods( [ 'getDirection' ] )
->getMock();
- $ctx->expects( $this->any() )->method( 'getDirection' )->will(
- $this->returnValue( $dir )
- );
+ $ctx->method( 'getDirection' )->willReturn( $dir );
return $ctx;
}
diff --git
a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
index 404fd97..b12d235 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
@@ -138,4 +138,83 @@
],
];
}
+
+ /**
+ * @covers ResourceLoaderWikiModule::getTitleInfo
+ */
+ public function testGetTitleInfo() {
+ $pages = [
+ 'MediaWiki:Common.css' => [ 'type' => 'styles' ],
+ 'mediawiki: fallback.css' => [ 'type' => 'styles' ],
+ ];
+ $titleInfo = [
+ 'MediaWiki:Common.css' => [ 'page_len' => 1234 ],
+ 'MediaWiki:Fallback.css' => [ 'page_len' => 0 ],
+ ];
+ $expected = $titleInfo;
+
+ $module = $this->getMockBuilder( 'TestResourceLoaderWikiModule'
)
+ ->setMethods( [ 'getPages' ] )
+ ->getMock();
+ $module->method( 'getPages' )->willReturn( $pages );
+ // Can't mock static methods
+ $module::$returnFetchTitleInfo = $titleInfo;
+
+ $context = $this->getMockBuilder( 'ResourceLoaderContext' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $module = TestingAccessWrapper::newFromObject( $module );
+ $this->assertEquals( $expected, $module->getTitleInfo( $context
), 'Title info' );
+ }
+
+ /**
+ * @covers ResourceLoaderWikiModule::getTitleInfo
+ * @covers ResourceLoaderWikiModule::setTitleInfo
+ * @covers ResourceLoaderWikiModule::preloadTitleInfo
+ */
+ public function testGetPreloadedTitleInfo() {
+ $pages = [
+ 'MediaWiki:Common.css' => [ 'type' => 'styles' ],
+ // Regression against T145673. It's impossible to
statically declare page names in
+ // a canonical way since the canonical prefix is
localised. As such, the preload
+ // cache computed the right cache key, but failed to
find the results when
+ // doing an intersect on the canonical result,
producing an empty array.
+ 'mediawiki: fallback.css' => [ 'type' => 'styles' ],
+ ];
+ $titleInfo = [
+ 'MediaWiki:Common.css' => [ 'page_len' => 1234 ],
+ 'MediaWiki:Fallback.css' => [ 'page_len' => 0 ],
+ ];
+ $expected = $titleInfo;
+
+ $module = $this->getMockBuilder( 'TestResourceLoaderWikiModule'
)
+ ->setMethods( [ 'getPages' ] )
+ ->getMock();
+ $module->method( 'getPages' )->willReturn( $pages );
+ // Can't mock static methods
+ $module::$returnFetchTitleInfo = $titleInfo;
+
+ $rl = new EmptyResourceLoader();
+ $rl->register( 'testmodule', $module );
+ $context = new ResourceLoaderContext( $rl, new FauxRequest() );
+
+ TestResourceLoaderWikiModule::preloadTitleInfo(
+ $context,
+ wfGetDB( DB_REPLICA ),
+ [ 'testmodule' ]
+ );
+
+ $module = TestingAccessWrapper::newFromObject( $module );
+ $this->assertEquals( $expected, $module->getTitleInfo( $context
), 'Title info' );
+ }
+}
+
+class TestResourceLoaderWikiModule extends ResourceLoaderWikiModule {
+ public static $returnFetchTitleInfo = null;
+ protected static function fetchTitleInfo( IDatabase $db, array $pages,
$fname = null ) {
+ $ret = self::$returnFetchTitleInfo;
+ self::$returnFetchTitleInfo = null;
+ return $ret;
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/310671
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibe788157724d73c727b9e2127b6828db32ca9420
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
