foreach ( $modules as $name => $module ) {
try {
$content = $module->getModuleContent( $context );
+ $implementKey = $name . '@' . $module->getVersionHash( $context );
$strContent = '';
// Append output
$strContent = $scripts;
} elseif ( is_array( $scripts ) ) {
// ...except when $scripts is an array of URLs
- $strContent = self::makeLoaderImplementScript( $name, $scripts, [], [], [] );
+ $strContent = self::makeLoaderImplementScript( $implementKey, $scripts, [], [], [] );
}
break;
case 'styles':
}
}
$strContent = self::makeLoaderImplementScript(
- $name,
+ $implementKey,
$scripts,
isset( $content['styles'] ) ? $content['styles'] : [],
isset( $content['messagesBlob'] ) ? new XmlJsCode( $content['messagesBlob'] ) : [],
/**
* Return JS code that calls mw.loader.implement with given module properties.
*
- * @param string $name Module name
+ * @param string $name Module name or implement key (format "`[name]@[version]`")
* @param XmlJsCode|array|string $scripts Code as XmlJsCode (to be wrapped in a closure),
* list of URLs to JavaScript files, or a string of JavaScript for `$.globalEval`.
* @param mixed $styles Array of CSS strings keyed by media type, or an array of lists of URLs
}
}
+ /**
+ * Make a versioned key for a specific module.
+ *
+ * @private
+ * @param {string} module Module name
+ * @return {string|null} Module key in format '`[name]@[version]`',
+ * or null if the module does not exist
+ */
+ function getModuleKey( module ) {
+ return hasOwn.call( registry, module ) ?
+ ( module + '@' + registry[ module ].version ) : null;
+ }
+
+ /**
+ * @private
+ * @param {string} key Module name or '`[name]@[version]`'
+ * @return {Object}
+ */
+ function splitModuleKey( key ) {
+ var index = key.indexOf( '@' );
+ if ( index === -1 ) {
+ return { name: key };
+ }
+ return {
+ name: key.slice( 0, index ),
+ version: key.slice( index )
+ };
+ }
+
/* Public Members */
return {
/**
* When #load() or #using() requests one or more modules, the server
* response contain calls to this function.
*
- * @param {string} module Name of module
+ * @param {string} module Name of module and current module version. Formatted
+ * as '`[name]@[version]`". This version should match the requested version
+ * (from #batchRequest and #registry). This avoids race conditions (T117587).
+ * For back-compat with MediaWiki 1.27 and earlier, the version may be omitted.
* @param {Function|Array|string} [script] Function with module code, list of URLs
* to load via `<script src>`, or string of module code for `$.globalEval()`.
* @param {Object} [style] Should follow one of the following patterns:
* @param {Object} [templates] List of key/value pairs to be added to mw#templates.
*/
implement: function ( module, script, style, messages, templates ) {
+ var split = splitModuleKey( module ),
+ name = split.name,
+ version = split.version;
// Automatically register module
- if ( !hasOwn.call( registry, module ) ) {
- mw.loader.register( module );
+ if ( !hasOwn.call( registry, name ) ) {
+ mw.loader.register( name );
}
// Check for duplicate implementation
- if ( hasOwn.call( registry, module ) && registry[ module ].script !== undefined ) {
- throw new Error( 'module already implemented: ' + module );
+ if ( hasOwn.call( registry, name ) && registry[ name ].script !== undefined ) {
+ throw new Error( 'module already implemented: ' + name );
+ }
+ if ( version ) {
+ // Without this reset, if there is a version mismatch between the
+ // requested and received module version, then mw.loader.store would
+ // cache the response under the requested key. Thus poisoning the cache
+ // indefinitely with a stale value. (T117587)
+ registry[ name ].version = version;
}
// Attach components
- registry[ module ].script = script || null;
- registry[ module ].style = style || null;
- registry[ module ].messages = messages || null;
- registry[ module ].templates = templates || null;
+ registry[ name ].script = script || null;
+ registry[ name ].style = style || null;
+ registry[ name ].messages = messages || null;
+ registry[ name ].templates = templates || null;
// The module may already have been marked as erroneous
- if ( $.inArray( registry[ module ].state, [ 'error', 'missing' ] ) === -1 ) {
- registry[ module ].state = 'loaded';
- if ( allReady( registry[ module ].dependencies ) ) {
- execute( module );
+ if ( $.inArray( registry[ name ].state, [ 'error', 'missing' ] ) === -1 ) {
+ registry[ name ].state = 'loaded';
+ if ( allReady( registry[ name ].dependencies ) ) {
+ execute( name );
}
}
},
MODULE_SIZE_MAX: 100 * 1000,
- // The contents of the store, mapping '[module name]@[version]' keys
+ // The contents of the store, mapping '[name]@[version]' keys
// to module implementations.
items: {},
].join( ':' );
},
- /**
- * Get a key for a specific module. The key format is '[name]@[version]'.
- *
- * @param {string} module Module name
- * @return {string|null} Module key or null if module does not exist
- */
- getModuleKey: function ( module ) {
- return hasOwn.call( registry, module ) ?
- ( module + '@' + registry[ module ].version ) : null;
- },
-
/**
* Initialize the store.
*
return false;
}
- key = mw.loader.store.getModuleKey( module );
+ key = getModuleKey( module );
if ( key in mw.loader.store.items ) {
mw.loader.store.stats.hits++;
return mw.loader.store.items[ key ];
return false;
}
- key = mw.loader.store.getModuleKey( module );
+ key = getModuleKey( module );
if (
// Already stored a copy of this exact version
try {
args = [
- JSON.stringify( module ),
+ JSON.stringify( key ),
typeof descriptor.script === 'function' ?
String( descriptor.script ) :
JSON.stringify( descriptor.script ),
for ( key in mw.loader.store.items ) {
module = key.slice( 0, key.indexOf( '@' ) );
- if ( mw.loader.store.getModuleKey( module ) !== key ) {
+ if ( getModuleKey( module ) !== key ) {
mw.loader.store.stats.expired++;
delete mw.loader.store.items[ key ];
} else if ( mw.loader.store.items[ key ].length > mw.loader.store.MODULE_SIZE_MAX ) {
use Psr\Log\NullLogger;
abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
+ // Version hash for a blank file module.
+ // Result of ResourceLoader::makeHash(), ResourceLoaderTestModule
+ // and ResourceLoaderFileModule::getDefinitionSummary().
+ const BLANK_VERSION = '09p30q0';
+
/**
* @param string $lang
* @param string $dir
*/
class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase {
+ protected static function expandVariables( $text ) {
+ return strtr( $text, [
+ '{blankVer}' => ResourceLoaderTestCase::BLANK_VERSION
+ ] );
+ }
+
protected static function makeContext( $extraQuery = [] ) {
$conf = new HashConfig( [
'ResourceLoaderSources' => [],
. '<script>(window.RLQ=window.RLQ||[]).push(function(){'
. 'mw.config.set({"key":"value"});'
. 'mw.loader.state({"test.exempt":"ready","test.private.top":"loading","test.styles.pure":"ready","test.styles.private":"ready","test.scripts.top":"loading"});'
- . 'mw.loader.implement("test.private.top",function($,jQuery,require,module){},{"css":[]});'
+ . 'mw.loader.implement("test.private.top@{blankVer}",function($,jQuery,require,module){},{"css":[]});'
. 'mw.loader.load(["test.top"]);'
. 'mw.loader.load("/w/load.php?debug=false\u0026lang=nl\u0026modules=test.scripts.top\u0026only=scripts\u0026skin=fallback");'
. '});</script>' . "\n"
. '<style>.private{}</style>' . "\n"
. '<script async="" src="/w/load.php?debug=false&lang=nl&modules=startup&only=scripts&skin=fallback"></script>';
// @codingStandardsIgnoreEnd
+ $expected = self::expandVariables( $expected );
$this->assertEquals( $expected, $client->getHeadHtml() );
}
// @codingStandardsIgnoreStart Generic.Files.LineLength
$expected = '<script>(window.RLQ=window.RLQ||[]).push(function(){'
- . 'mw.loader.implement("test.private.bottom",function($,jQuery,require,module){},{"css":[]});'
+ . 'mw.loader.implement("test.private.bottom@{blankVer}",function($,jQuery,require,module){},{"css":[]});'
. 'mw.loader.load("/w/load.php?debug=false\u0026lang=nl\u0026modules=test.scripts\u0026only=scripts\u0026skin=fallback");'
. 'mw.loader.load(["test"]);'
. '});</script>';
// @codingStandardsIgnoreEnd
+ $expected = self::expandVariables( $expected );
$this->assertEquals( $expected, $client->getBodyHtml() );
}
'context' => [],
'modules' => [ 'test.private.top' ],
'only' => ResourceLoaderModule::TYPE_COMBINED,
- 'output' => '<script>(window.RLQ=window.RLQ||[]).push(function(){mw.loader.implement("test.private.top",function($,jQuery,require,module){},{"css":[]});});</script>',
+ 'output' => '<script>(window.RLQ=window.RLQ||[]).push(function(){mw.loader.implement("test.private.top@{blankVer}",function($,jQuery,require,module){},{"css":[]});});</script>',
],
[
'context' => [],
$context = self::makeContext( $extraQuery );
$context->getResourceLoader()->register( self::makeSampleModules() );
$actual = ResourceLoaderClientHtml::makeLoad( $context, $modules, $type );
+ $expected = self::expandVariables( $expected );
$this->assertEquals( $expected, (string)$actual );
}
}
class ResourceLoaderStartUpModuleTest extends ResourceLoaderTestCase {
- // Version hash for a blank file module.
- // Result of ResourceLoader::makeHash(), ResourceLoaderTestModule
- // and ResourceLoaderFileModule::getDefinitionSummary().
- protected static $blankVersion = '09p30q0';
-
protected static function expandPlaceholders( $text ) {
return strtr( $text, [
- '{blankVer}' => self::$blankVersion
+ '{blankVer}' => self::BLANK_VERSION
] );
}
( function ( mw, $ ) {
- QUnit.module( 'mediawiki (mw.loader)' );
+ QUnit.module( 'mediawiki (mw.loader)', QUnit.newMwEnvironment( {
+ setup: function () {
+ mw.loader.store.enabled = false;
+ },
+ teardown: function () {
+ mw.loader.store.enabled = false;
+ }
+ } ) );
mw.loader.addSource(
'testloader',
} );
} );
+ QUnit.test( 'Stale response caching - T117587', function ( assert ) {
+ var count = 0;
+ mw.loader.store.enabled = true;
+ mw.loader.register( 'test.stale', 'v2' );
+ assert.strictEqual( mw.loader.store.get( 'test.stale' ), false, 'Not in store' );
+
+ mw.loader.implement( 'test.stale@v1', function () {
+ count++;
+ } );
+
+ return mw.loader.using( 'test.stale' )
+ .then( function () {
+ assert.strictEqual( count, 1 );
+ assert.strictEqual( mw.loader.getState( 'test.stale' ), 'ready' );
+ assert.ok( mw.loader.store.get( 'test.stale' ), 'In store' );
+ } )
+ .then( function () {
+ // Reset run time, but keep mw.loader.store
+ mw.loader.moduleRegistry[ 'test.stale' ].script = undefined;
+ mw.loader.moduleRegistry[ 'test.stale' ].state = 'registered';
+ mw.loader.moduleRegistry[ 'test.stale' ].version = 'v2';
+
+ // Module was stored correctly as v1
+ // On future navigations, it will be ignored until evicted
+ assert.strictEqual( mw.loader.store.get( 'test.stale' ), false, 'Not in store' );
+ } );
+ } );
+
+ QUnit.test( 'Stale response caching - backcompat', function ( assert ) {
+ var count = 0;
+ mw.loader.store.enabled = true;
+ mw.loader.register( 'test.stalebc', 'v2' );
+ assert.strictEqual( mw.loader.store.get( 'test.stalebc' ), false, 'Not in store' );
+
+ mw.loader.implement( 'test.stalebc', function () {
+ count++;
+ } );
+
+ return mw.loader.using( 'test.stalebc' )
+ .then( function () {
+ assert.strictEqual( count, 1 );
+ assert.strictEqual( mw.loader.getState( 'test.stalebc' ), 'ready' );
+ assert.ok( mw.loader.store.get( 'test.stalebc' ), 'In store' );
+ } )
+ .then( function () {
+ // Reset run time, but keep mw.loader.store
+ mw.loader.moduleRegistry[ 'test.stalebc' ].script = undefined;
+ mw.loader.moduleRegistry[ 'test.stalebc' ].state = 'registered';
+ mw.loader.moduleRegistry[ 'test.stalebc' ].version = 'v2';
+
+ // Legacy behaviour is storing under the expected version,
+ // which woudl lead to whitewashing and stale values (T117587).
+ assert.ok( mw.loader.store.get( 'test.stalebc' ), 'In store' );
+ } );
+ } );
+
QUnit.test( 'require()', 6, function ( assert ) {
mw.loader.register( [
[ 'test.require1', '0' ],