Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

resourceloader: Make timestamp handling more consistent

Browse source 
* Use time() instead of:
  - wfTimestamp()
  - wfTimestamp( TS_UNIX )
  - wfTimestamp( TS_UNIX, 0 )
  - wfTimestamp( TS_UNIX, time() )
  - intval( wfTimestamp( TS_UNIX, time() ) )

* Consistently use 1 as default instead of 0. Previously the
  unwritten convention was that anything "final" max()'ed with 1,
  and any internal method would use 0, but this wasn't applied
  consistently made it fragile. There doesn't seem to be any
  value in returning 0 only to have it maxed up to 1 (because if
  the 0 would ever make it out alive, we'd be in trouble).

* wfTimestamp returns a string for TS_UNIX. In PHP this doesn't
  matter much. In fact, max() takes number-like integers so
  transparently, it even preserves it:
  > max( 1, 3, '2' );
  < 3
  > max( 1, '3', 2 );
  < "3"
  Just cast it in one place at the very end (StartupModule)
  instead of doing intval( wfTimestamp() ).

* Fix weird documentation claiming getModifiedTime can return
  an array, or mixed.

* Remove 'version > 1 ? version : 1' logic in
  ResourceLoader::makeLoaderRegisterScript. The client doesn't
  have "0 means now" behaviour so this isn't needed. And the method
  was only doing it for variadic argument calls.

Removal of quotes around timestamps reduced the size of the startup
module from 26.8KB to 25.9KB before gzip. After gzip the size was
and still is 5.7KB, though. (From 5456 bytes to 5415 bytes.)

Change-Id: If92ca3e7511e78fa779f2f2701e2ab24db78c8a8
This commit is contained in:
Timo Tijhof 2014-12-05 20:28:54 +00:00
parent 9c785e9158
commit 6a1ec17e79
8 changed files with 66 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
includes/resourceloader/ResourceLoader.php
View file

@ -140,7 +140,7 @@ class ResourceLoader {
foreach ( array_keys( $modulesWithoutMessages ) as $name ) {
$module = $this->getModule( $name );
if ( $module ) {
$module->setMsgBlobMtime( $lang, 0 );
$module->setMsgBlobMtime( $lang, 1 );
}
}
}
@ -1225,7 +1225,7 @@ class ResourceLoader {
ResourceLoader::inDebugMode()
);
} else {
$version = (int)$version > 1 ? (int)$version : 1;
$version = (int) $version;
return Xml::encodeJsCall(
'mw.loader.register',
array( $name, $version, $dependencies, $group, $source, $skip ),

29
includes/resourceloader/ResourceLoaderModule.php
View file

@ -388,12 +388,12 @@ abstract class ResourceLoaderModule {
* Get the last modification timestamp of the message blob for this
* module in a given language.
* @param string $lang Language code
* @return int UNIX timestamp, or 0 if the module doesn't have messages
* @return int UNIX timestamp
*/
public function getMsgBlobMtime( $lang ) {
if ( !isset( $this->msgBlobMtime[$lang] ) ) {
if ( !count( $this->getMessages() ) ) {
return 0;
return 1;
}
$dbr = wfGetDB( DB_SLAVE );
@ -416,7 +416,7 @@ abstract class ResourceLoaderModule {
* Set a preloaded message blob last modification timestamp. Used so we
* can load this information for all modules at once.
* @param string $lang Language code
* @param int $mtime UNIX timestamp or 0 if there is no such blob
* @param int $mtime UNIX timestamp
*/
public function setMsgBlobMtime( $lang, $mtime ) {
$this->msgBlobMtime[$lang] = $mtime;
@ -443,7 +443,6 @@ abstract class ResourceLoaderModule {
* @return int UNIX timestamp
*/
public function getModifiedTime( ResourceLoaderContext $context ) {
// 0 would mean now
return 1;
}
@ -451,13 +450,12 @@ abstract class ResourceLoaderModule {
* Helper method for calculating when the module's hash (if it has one) changed.
*
* @param ResourceLoaderContext $context
* @return int UNIX timestamp or 0 if no hash was provided
* by getModifiedHash()
* @return int UNIX timestamp
*/
public function getHashMtime( ResourceLoaderContext $context ) {
$hash = $this->getModifiedHash( $context );
if ( !is_string( $hash ) ) {
return 0;
return 1;
}
$cache = wfGetCache( CACHE_ANYTHING );
@ -469,7 +467,7 @@ abstract class ResourceLoaderModule {
return $data['timestamp'];
}
$timestamp = wfTimestamp();
$timestamp = time();
$cache->set( $key, array(
'hash' => $hash,
'timestamp' => $timestamp,
@ -497,15 +495,14 @@ abstract class ResourceLoaderModule {
* @since 1.23
*
* @param ResourceLoaderContext $context
* @return int UNIX timestamp or 0 if no definition summary was provided
* by getDefinitionSummary()
* @return int UNIX timestamp
*/
public function getDefinitionMtime( ResourceLoaderContext $context ) {
wfProfileIn( __METHOD__ );
$summary = $this->getDefinitionSummary( $context );
if ( $summary === null ) {
wfProfileOut( __METHOD__ );
return 0;
return 1;
}
$hash = md5( json_encode( $summary ) );
@ -640,16 +637,12 @@ abstract class ResourceLoaderModule {
* Safe version of filemtime(), which doesn't throw a PHP warning if the file doesn't exist
* but returns 1 instead.
* @param string $filename File name
* @return int UNIX timestamp, or 1 if the file doesn't exist
* @return int UNIX timestamp
*/
protected static function safeFilemtime( $filename ) {
if ( file_exists( $filename ) ) {
return filemtime( $filename );
} else {
// We only ever map this function on an array if we're gonna call max() after,
// so return our standard minimum timestamps here. This is 1, not 0, because
// wfTimestamp(0) == NOW
if ( !file_exists( $filename ) ) {
return 1;
}
return filemtime( $filename );
}
}

18
includes/resourceloader/ResourceLoaderStartUpModule.php
View file

@ -211,12 +211,10 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
continue;
}
// getModifiedTime() is supposed to return a UNIX timestamp, but it doesn't always
// seem to do that, and custom implementations might forget. Coerce it to TS_UNIX
// Coerce module timestamp to UNIX timestamp.
// getModifiedTime() is supposed to return a UNIX timestamp, but custom implementations
// might forget. TODO: Maybe emit warning?
$moduleMtime = wfTimestamp( TS_UNIX, $module->getModifiedTime( $context ) );
$mtime = max( $moduleMtime, wfTimestamp( TS_UNIX, $this->getConfig()->get( 'CacheEpoch' ) ) );
// FIXME: Convert to numbers, wfTimestamp always gives us stings, even for TS_UNIX
$skipFunction = $module->getSkipFunction();
if ( $skipFunction !== null && !ResourceLoader::inDebugMode() ) {
@ -229,8 +227,14 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
);
}
$mtime = max(
$moduleMtime,
wfTimestamp( TS_UNIX, $this->getConfig()->get( 'CacheEpoch' ) )
);
$registryData[$name] = array(
'version' => $mtime,
// Convert to numbers as wfTimestamp always returns a string, even for TS_UNIX
'version' => (int) $mtime,
'dependencies' => $module->getDependencies(),
'group' => $module->getGroup(),
'source' => $module->getSource(),
@ -352,7 +356,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
// Get the latest version
$loader = $context->getResourceLoader();
$version = 0;
$version = 1;
foreach ( $moduleNames as $moduleName ) {
$version = max( $version,
$loader->getModule( $moduleName )->getModifiedTime( $context )

2
includes/resourceloader/ResourceLoaderUserDefaultsModule.php
View file

@ -42,7 +42,7 @@ class ResourceLoaderUserDefaultsModule extends ResourceLoaderModule {
/**
* @param ResourceLoaderContext $context
* @return array|int|mixed
* @return int
*/
public function getModifiedTime( ResourceLoaderContext $context ) {
return $this->getHashMtime( $context );

2
includes/resourceloader/ResourceLoaderUserOptionsModule.php
View file

@ -46,7 +46,7 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule {
/**
* @param ResourceLoaderContext $context
* @return array|int|mixed
* @return int
*/
public function getModifiedTime( ResourceLoaderContext $context ) {
$hash = $context->getHash();

8
includes/resourceloader/ResourceLoaderWikiModule.php
View file

@ -164,10 +164,10 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
/**
* @param ResourceLoaderContext $context
* @return int|mixed
* @return int
*/
public function getModifiedTime( ResourceLoaderContext $context ) {
$modifiedTime = 1; // wfTimestamp() interprets 0 as "now"
$modifiedTime = 1;
$titleInfo = $this->getTitleInfo( $context );
if ( count( $titleInfo ) ) {
$mtimes = array_map( function ( $value ) {
@ -226,8 +226,8 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
* Get the modification times of all titles that would be loaded for
* a given context.
* @param ResourceLoaderContext $context Context object
* @return array keyed by page dbkey, with value is an array with 'length' and 'timestamp'
* keys, where the timestamp is a unix one
* @return array Keyed by page dbkey. Value is an array with 'length' and 'timestamp'
* keys, where the timestamp is a UNIX timestamp
*/
protected function getTitleInfo( ResourceLoaderContext $context ) {
$dbr = $this->getDB();

21
resources/src/mediawiki/mediawiki.js
View file

@ -807,7 +807,8 @@
}
/**
* Generates an ISO8601 "basic" string from a UNIX timestamp
* Convert UNIX timestamp to ISO8601 format
* @param {number} timestamp UNIX timestamp
* @private
*/
function formatVersionNumber( timestamp ) {
@ -1877,27 +1878,29 @@
/**
* Get the version of a module.
*
* @param {string} module Name of module to get version for
* @param {string} module Name of module
* @return {string|null} The version, or null if the module (or its version) is not
* in the registry.
*/
getVersion: function ( module ) {
if ( registry[module] !== undefined && registry[module].version !== undefined ) {
return formatVersionNumber( registry[module].version );
if ( !registry[module] || registry[module].version === undefined ) {
return null;
}
return null;
return formatVersionNumber( registry[module].version );
},
/**
* Get the state of a module.
*
* @param {string} module Name of module to get state for
* @param {string} module Name of module
* @return {string|null} The state, or null if the module (or its version) is not
* in the registry.
*/
getState: function ( module ) {
if ( registry[module] !== undefined && registry[module].state !== undefined ) {
return registry[module].state;
if ( !registry[module] || registry[module].state === undefined ) {
return null;
}
return null;
return registry[module].state;
},
/**

48
tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php
View file

@ -23,7 +23,7 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.blank",
"1388534400"
1388534400
]
] );',
) ),
@ -40,17 +40,17 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.blank",
"1388534400"
1388534400
],
[
"test.group.foo",
"1388534400",
1388534400,
[],
"x-foo"
],
[
"test.group.bar",
"1388534400",
1388534400,
[],
"x-bar"
]
@ -68,7 +68,7 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.blank",
"1388534400"
1388534400
]
] );'
) ),
@ -90,7 +90,7 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.blank",
"1388534400",
1388534400,
[],
null,
"example"
@ -126,11 +126,11 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.x.core",
"1388534400"
1388534400
],
[
"test.x.polyfill",
"1388534400",
1388534400,
[],
null,
"local",
@ -138,7 +138,7 @@ mw.loader.addSource( {
],
[
"test.y.polyfill",
"1388534400",
1388534400,
[],
null,
"local",
@ -146,7 +146,7 @@ mw.loader.addSource( {
],
[
"test.z.foo",
"1388534400",
1388534400,
[
"test.x.core",
"test.x.polyfil",
@ -222,36 +222,36 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.blank",
"1388534400"
1388534400
],
[
"test.x.core",
"1388534400"
1388534400
],
[
"test.x.util",
"1388534400",
1388534400,
[
"test.x.core"
]
],
[
"test.x.foo",
"1388534400",
1388534400,
[
"test.x.core"
]
],
[
"test.x.bar",
"1388534400",
1388534400,
[
"test.x.util"
]
],
[
"test.x.quux",
"1388534400",
1388534400,
[
"test.x.foo",
"test.x.bar",
@ -260,25 +260,25 @@ mw.loader.addSource( {
],
[
"test.group.foo.1",
"1388534400",
1388534400,
[],
"x-foo"
],
[
"test.group.foo.2",
"1388534400",
1388534400,
[],
"x-foo"
],
[
"test.group.bar.1",
"1388534400",
1388534400,
[],
"x-bar"
],
[
"test.group.bar.2",
"1388534400",
1388534400,
[],
"x-bar",
"example"
@ -344,8 +344,8 @@ mw.loader.addSource( {
$this->assertEquals(
'mw.loader.addSource({"local":"/w/load.php"});'
. 'mw.loader.register(['
. '["test.blank","1388534400"],'
. '["test.min","1388534400",["test.blank"],null,"local",'
. '["test.blank",1388534400],'
. '["test.min",1388534400,["test.blank"],null,"local",'
. '"return!!(window.JSON\u0026\u0026JSON.parse\u0026\u0026JSON.stringify);"'
. ']]);',
$module->getModuleRegistrations( $context ),
@ -367,11 +367,11 @@ mw.loader.addSource( {
} );mw.loader.register( [
[
"test.blank",
"1388534400"
1388534400
],
[
"test.min",
"1388534400",
1388534400,
[
"test.blank"
],