jenkins-bot has submitted this change and it was merged.
Change subject: resourceloader: Enable module content version for data modules
......................................................................
resourceloader: Enable module content version for data modules
This greatly simplifies logic required to compute module versions.
It also makes it significantly less error-prone.
Since f37cee996e, we support hashes as versions (instead of timestamps).
This means we can build a hash of the content directly, instead of compiling a
large array with all values that may influence the module content somehow.
Benefits:
* Remove all methods and logic related to querying database and disk for
timestamps, revision numbers, definition summaries, cache epochs, and more.
* No longer needlessly invalidate cache as a result of no-op changes to
implementation datails. Due to inclusion of absolute file paths in the
definition summary, cache was always invalidated when moving wikis to newer
MediaWiki branches; even if the module observed no actual changes.
* When changes are reverted within a certain period of time, old caches can now
be re-used. The module would produce the same version hash as before.
Previously when a change was deployed and then reverted, all web clients (even
those that never saw the bad version) would have re-fetch modules because the
version increased.
Updated unit tests to account for the change in version. New default version of
empty test modules is: "mvgTPvXh". For the record, this comes from the base64
encoding of the SHA1 digest of the JSON serialised form of the module content:
> $str = '{"scripts":"","styles":{"css":[]},"messagesBlob":"{}"}';
> echo base64_encode(sha1($str, true));
> FEb3+VuiUm/fOMfod1bjw/te+AQ=
Enabled content versioning for the data modules in MediaWiki core:
* EditToolbarModule
* JqueryMsgModule
* LanguageDataModule
* LanguageNamesModule
* SpecialCharacterDataModule
* UserCSSPrefsModule
* UserDefaultsModule
* UserOptionsModule
The FileModule and base class explicitly disable it for now and keep their
current behaviour of using the definition summary. We may remove it later, but
that requires more performance testing first.
Explicitly disable it in the WikiModule class to avoid breakage when the
default changes.
Ref T98087.
Change-Id: I782df43c50dfcfb7d7592f744e13a3a0430b0dc6
---
M includes/resourceloader/ResourceLoaderEditToolbarModule.php
M includes/resourceloader/ResourceLoaderFileModule.php
M includes/resourceloader/ResourceLoaderJqueryMsgModule.php
M includes/resourceloader/ResourceLoaderLanguageDataModule.php
M includes/resourceloader/ResourceLoaderLanguageNamesModule.php
M includes/resourceloader/ResourceLoaderModule.php
M includes/resourceloader/ResourceLoaderSpecialCharacterDataModule.php
M includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php
M includes/resourceloader/ResourceLoaderUserDefaultsModule.php
M includes/resourceloader/ResourceLoaderUserOptionsModule.php
M includes/resourceloader/ResourceLoaderWikiModule.php
M tests/phpunit/ResourceLoaderTestCase.php
M tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
13 files changed, 111 insertions(+), 83 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/resourceloader/ResourceLoaderEditToolbarModule.php
b/includes/resourceloader/ResourceLoaderEditToolbarModule.php
index d0273c2..f3fae0e 100644
--- a/includes/resourceloader/ResourceLoaderEditToolbarModule.php
+++ b/includes/resourceloader/ResourceLoaderEditToolbarModule.php
@@ -65,15 +65,10 @@
}
/**
- * @param ResourceLoaderContext $context
- * @return array
+ * @return bool
*/
- public function getDefinitionSummary( ResourceLoaderContext $context ) {
- $summary = parent::getDefinitionSummary( $context );
- $summary[] = array(
- 'lessVars' => $this->getLessVars( $context ),
- );
- return $summary;
+ public function enableModuleContentVersion() {
+ return true;
}
/**
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php
b/includes/resourceloader/ResourceLoaderFileModule.php
index e6c9bd0..b734def 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -514,6 +514,18 @@
}
/**
+ * Disable module content versioning.
+ *
+ * This class uses getDefinitionSummary() instead, to avoid filesystem
overhead
+ * involved with building the full module content inside a startup
request.
+ *
+ * @return bool
+ */
+ public function enableModuleContentVersion() {
+ return false;
+ }
+
+ /**
* Helper method to gather file mtimes for getDefinitionSummary.
*
* Last modified timestamps are calculated from the highest last
modified
diff --git a/includes/resourceloader/ResourceLoaderJqueryMsgModule.php
b/includes/resourceloader/ResourceLoaderJqueryMsgModule.php
index b905781..d159284 100644
--- a/includes/resourceloader/ResourceLoaderJqueryMsgModule.php
+++ b/includes/resourceloader/ResourceLoaderJqueryMsgModule.php
@@ -49,14 +49,9 @@
}
/**
- * @param ResourceLoaderContext $context
- * @return array|null
+ * @return bool
*/
- public function getDefinitionSummary( ResourceLoaderContext $context ) {
- $summary = parent::getDefinitionSummary( $context );
- $summary[] = array(
- 'sanitizerData' => Sanitizer::getRecognizedTagData()
- );
- return $summary;
+ public function enableModuleContentVersion() {
+ return true;
}
}
diff --git a/includes/resourceloader/ResourceLoaderLanguageDataModule.php
b/includes/resourceloader/ResourceLoaderLanguageDataModule.php
index be15008..27c74d7 100644
--- a/includes/resourceloader/ResourceLoaderLanguageDataModule.php
+++ b/includes/resourceloader/ResourceLoaderLanguageDataModule.php
@@ -63,11 +63,10 @@
}
/**
- * @param ResourceLoaderContext $context
- * @return string Hash
+ * @return bool
*/
- public function getModifiedHash( ResourceLoaderContext $context ) {
- return md5( serialize( $this->getData( $context ) ) );
+ public function enableModuleContentVersion() {
+ return true;
}
/**
diff --git a/includes/resourceloader/ResourceLoaderLanguageNamesModule.php
b/includes/resourceloader/ResourceLoaderLanguageNamesModule.php
index 827a573..081c728 100644
--- a/includes/resourceloader/ResourceLoaderLanguageNamesModule.php
+++ b/includes/resourceloader/ResourceLoaderLanguageNamesModule.php
@@ -32,7 +32,6 @@
protected $targets = array( 'desktop', 'mobile' );
-
/**
* @param ResourceLoaderContext $context
* @return array
@@ -69,11 +68,10 @@
}
/**
- * @param ResourceLoaderContext $context
- * @return string Hash
+ * @return bool
*/
- public function getModifiedHash( ResourceLoaderContext $context ) {
- return md5( serialize( $this->getData( $context ) ) );
+ public function enableModuleContentVersion() {
+ return true;
}
}
diff --git a/includes/resourceloader/ResourceLoaderModule.php
b/includes/resourceloader/ResourceLoaderModule.php
index d58c01d..3322eff 100644
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -600,22 +600,28 @@
$contextHash = $context->getHash();
if ( !array_key_exists( $contextHash, $this->versionHash ) ) {
- $summary = $this->getDefinitionSummary( $context );
- if ( !isset( $summary['_cacheEpoch'] ) ) {
- throw new Exception( 'getDefinitionSummary must
call parent method' );
- }
- $str = json_encode( $summary );
+ if ( $this->enableModuleContentVersion() ) {
+ // Detect changes directly
+ $str = json_encode( $this->getModuleContent(
$context ) );
+ } else {
+ // Infer changes based on definition and other
metrics
+ $summary = $this->getDefinitionSummary(
$context );
+ if ( !isset( $summary['_cacheEpoch'] ) ) {
+ throw new Exception(
'getDefinitionSummary must call parent method' );
+ }
+ $str = json_encode( $summary );
- $mtime = $this->getModifiedTime( $context );
- if ( $mtime !== null ) {
- // Support: MediaWiki 1.25 and earlier
- $str .= strval( $mtime );
- }
+ $mtime = $this->getModifiedTime( $context );
+ if ( $mtime !== null ) {
+ // Support: MediaWiki 1.25 and earlier
+ $str .= strval( $mtime );
+ }
- $mhash = $this->getModifiedHash( $context );
- if ( $mhash !== null ) {
- // Support: MediaWiki 1.25 and earlier
- $str .= strval( $mhash );
+ $mhash = $this->getModifiedHash( $context );
+ if ( $mhash !== null ) {
+ // Support: MediaWiki 1.25 and earlier
+ $str .= strval( $mhash );
+ }
}
$this->versionHash[$contextHash] =
ResourceLoader::makeHash( $str );
@@ -624,6 +630,19 @@
}
/**
+ * Whether to generate version hash based on module content.
+ *
+ * If a module requires database or file system access to build the
module
+ * content, consider disabling this in favour of manually tracking
relevant
+ * aspects in getDefinitionSummary(). See getVersionHash() for how this
is used.
+ *
+ * @return bool
+ */
+ public function enableModuleContentVersion() {
+ return false;
+ }
+
+ /**
* Get the definition summary for this module.
*
* This is the method subclasses are recommended to use to track values
in their
diff --git
a/includes/resourceloader/ResourceLoaderSpecialCharacterDataModule.php
b/includes/resourceloader/ResourceLoaderSpecialCharacterDataModule.php
index 03f2124..8170cb1 100644
--- a/includes/resourceloader/ResourceLoaderSpecialCharacterDataModule.php
+++ b/includes/resourceloader/ResourceLoaderSpecialCharacterDataModule.php
@@ -54,11 +54,10 @@
}
/**
- * @param ResourceLoaderContext $context
- * @return string Hash
+ * @return bool
*/
- public function getModifiedHash( ResourceLoaderContext $context ) {
- return md5( serialize( $this->getData() ) );
+ public function enableModuleContentVersion() {
+ return true;
}
/**
diff --git a/includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php
b/includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php
index d0f7d44..65d770e 100644
--- a/includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php
+++ b/includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php
@@ -30,11 +30,10 @@
protected $origin = self::ORIGIN_CORE_INDIVIDUAL;
/**
- * @param ResourceLoaderContext $context
- * @return array|int|mixed
+ * @return bool
*/
- public function getModifiedTime( ResourceLoaderContext $context ) {
- return wfTimestamp( TS_UNIX,
$context->getUserObj()->getTouched() );
+ public function enableModuleContentVersion() {
+ return true;
}
/**
diff --git a/includes/resourceloader/ResourceLoaderUserDefaultsModule.php
b/includes/resourceloader/ResourceLoaderUserDefaultsModule.php
index 2fd35ad..eba61ed 100644
--- a/includes/resourceloader/ResourceLoaderUserDefaultsModule.php
+++ b/includes/resourceloader/ResourceLoaderUserDefaultsModule.php
@@ -26,18 +26,13 @@
*/
class ResourceLoaderUserDefaultsModule extends ResourceLoaderModule {
- /* Protected Members */
-
protected $targets = array( 'desktop', 'mobile' );
- /* Methods */
-
/**
- * @param ResourceLoaderContext $context
- * @return string Hash
+ * @return bool
*/
- public function getModifiedHash( ResourceLoaderContext $context ) {
- return md5( serialize( User::getDefaultOptions() ) );
+ public function enableModuleContentVersion() {
+ return true;
}
/**
diff --git a/includes/resourceloader/ResourceLoaderUserOptionsModule.php
b/includes/resourceloader/ResourceLoaderUserOptionsModule.php
index aba0fa6..0847109 100644
--- a/includes/resourceloader/ResourceLoaderUserOptionsModule.php
+++ b/includes/resourceloader/ResourceLoaderUserOptionsModule.php
@@ -40,11 +40,10 @@
}
/**
- * @param ResourceLoaderContext $context
- * @return int
+ * @return bool
*/
- public function getModifiedTime( ResourceLoaderContext $context ) {
- return wfTimestamp( TS_UNIX,
$context->getUserObj()->getTouched() );
+ public function enableModuleContentVersion() {
+ return true;
}
/**
diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php
b/includes/resourceloader/ResourceLoaderWikiModule.php
index 264af5b..1561dae 100644
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -225,6 +225,20 @@
}
/**
+ * Disable module content versioning.
+ *
+ * This class does not support generating content outside of a module
+ * request due to foreign database support.
+ *
+ * See getDefinitionSummary() for meta-data versioning.
+ *
+ * @return bool
+ */
+ public function enableModuleContentVersion() {
+ return false;
+ }
+
+ /**
* @param ResourceLoaderContext $context
* @return array
*/
diff --git a/tests/phpunit/ResourceLoaderTestCase.php
b/tests/phpunit/ResourceLoaderTestCase.php
index 3165bb8..325b20e 100644
--- a/tests/phpunit/ResourceLoaderTestCase.php
+++ b/tests/phpunit/ResourceLoaderTestCase.php
@@ -102,6 +102,10 @@
public function isRaw() {
return $this->isRaw;
}
+
+ public function enableModuleContentVersion() {
+ return true;
+ }
}
class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {
diff --git
a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
index 72a2d6a..490f5c6 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
@@ -23,7 +23,7 @@
} );mw.loader.register( [
[
"test.blank",
- "XyCC+PSK"
+ "wvTifjse"
]
] );',
) ),
@@ -40,17 +40,17 @@
} );mw.loader.register( [
[
"test.blank",
- "XyCC+PSK"
+ "wvTifjse"
],
[
"test.group.foo",
- "XyCC+PSK",
+ "wvTifjse",
[],
"x-foo"
],
[
"test.group.bar",
- "XyCC+PSK",
+ "wvTifjse",
[],
"x-bar"
]
@@ -68,7 +68,7 @@
} );mw.loader.register( [
[
"test.blank",
- "XyCC+PSK"
+ "wvTifjse"
]
] );'
) ),
@@ -90,7 +90,7 @@
} );mw.loader.register( [
[
"test.blank",
- "XyCC+PSK",
+ "wvTifjse",
[],
null,
"example"
@@ -126,11 +126,11 @@
} );mw.loader.register( [
[
"test.x.core",
- "XyCC+PSK"
+ "wvTifjse"
],
[
"test.x.polyfill",
- "XyCC+PSK",
+ "wvTifjse",
[],
null,
null,
@@ -138,7 +138,7 @@
],
[
"test.y.polyfill",
- "XyCC+PSK",
+ "wvTifjse",
[],
null,
null,
@@ -146,7 +146,7 @@
],
[
"test.z.foo",
- "XyCC+PSK",
+ "wvTifjse",
[
0,
1,
@@ -222,36 +222,36 @@
} );mw.loader.register( [
[
"test.blank",
- "XyCC+PSK"
+ "wvTifjse"
],
[
"test.x.core",
- "XyCC+PSK"
+ "wvTifjse"
],
[
"test.x.util",
- "XyCC+PSK",
+ "wvTifjse",
[
1
]
],
[
"test.x.foo",
- "XyCC+PSK",
+ "wvTifjse",
[
1
]
],
[
"test.x.bar",
- "XyCC+PSK",
+ "wvTifjse",
[
2
]
],
[
"test.x.quux",
- "XyCC+PSK",
+ "wvTifjse",
[
3,
4,
@@ -260,25 +260,25 @@
],
[
"test.group.foo.1",
- "XyCC+PSK",
+ "wvTifjse",
[],
"x-foo"
],
[
"test.group.foo.2",
- "XyCC+PSK",
+ "wvTifjse",
[],
"x-foo"
],
[
"test.group.bar.1",
- "XyCC+PSK",
+ "wvTifjse",
[],
"x-bar"
],
[
"test.group.bar.2",
- "XyCC+PSK",
+ "wvTifjse",
[],
"x-bar",
"example"
@@ -344,8 +344,8 @@
$this->assertEquals(
'mw.loader.addSource({"local":"/w/load.php"});'
. 'mw.loader.register(['
-. '["test.blank","XyCC+PSK"],'
-. '["test.min","XyCC+PSK",[0],null,null,'
+. '["test.blank","wvTifjse"],'
+. '["test.min","wvTifjse",[0],null,null,'
. '"return!!(window.JSON\u0026\u0026JSON.parse\u0026\u0026JSON.stringify);"'
. ']]);',
$module->getModuleRegistrations( $context ),
@@ -367,11 +367,11 @@
} );mw.loader.register( [
[
"test.blank",
- "XyCC+PSK"
+ "wvTifjse"
],
[
"test.min",
- "XyCC+PSK",
+ "wvTifjse",
[
0
],
--
To view, visit https://gerrit.wikimedia.org/r/215364
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I782df43c50dfcfb7d7592f744e13a3a0430b0dc6
Gerrit-PatchSet: 18
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
