Commit graph

16 commits

Author SHA1 Message Date
Timo Tijhof
2581453fac 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
2015-06-18 20:39:38 +00:00
umherirrender
d8821f2b0b Fixed spacing
- Removed space after casts
- Removed spaces in array index
- Added spaces around string concat
- Added space after words: switch, foreach
- else if -> elseif
- Removed parentheses around require_once, because it is not a function
- Added newline at end of file
- Removed double spaces
- Added spaces around operations
- Removed repeated newlines

Bug: T102609
Change-Id: Ib860222b24f8ad8e9062cd4dc42ec88dc63fb49e
2015-06-17 20:22:32 +00:00
Andrew Green
7e0d3369c2 resourceloader: Add context param to ResourceLoaderModule::getDependencies
By providing context as a parameter in getDependencies, we allow
modules to dyanamically determine dependencies based on context.
Note: To ease rollout, the parameter is optional in this patch. It is expected
that it will be made non-optional in the near future.

The use case is for CentralNotice campaigns to be able to add special
modules ahead of deciding which banner to show a user. The dynamically
chosen RL modules would replace ad-hoc JS currently sent with some banners.
A list of possible campaigns and banners is already sent as a PHP-
implemented RL module; that's the module that will dynamically choose other
modules as dependencies when appropriate. This approach will save a round
trip as compared to dynamically loading the modules client-side.

For compatibility, extensions that override
ResourceLoaderModule::getDependencies() should be updated with the new
method signature. Here are changes for extensions currently deployed on
Wikimedia wikis:

* CentralNotice: I816bffa3815e2eab7e88cb04d1b345070e6aa15f
* Gadgets: I0a10fb0cbf17d095ece493e744296caf13dcee02
* EventLogging: I67e957f74d6ca48cfb9a41fb5144bcc78f885e50
* PageTriage: Ica3ba32aa2fc76d11a44f391b6edfc871e7fbe0d
* UniversalLanguageSelector: Ic63e617f51702c27104e123d4bed91983a726b7f
* VisualEditor: I0ac775ca286e64825e31a9213b94648e41a5bc30

For more on the CentralNotice use case, please see I9f80edcbcacca2.

Bug: T98924
Change-Id: Iee61e5b527321d01287baa03ad9b4d4f526ff3ef
2015-06-09 03:10:20 +01:00
Timo Tijhof
1e4336d435 resourceloader: Add unit test for validateScriptFile()
Follows-up cd0dff5c00.

Change-Id: Ie208e58053048e932ef3f61f849148b1d88bc0be
2015-06-02 23:50:57 +01:00
Timo Tijhof
f37cee996e resourceloader: Replace timestamp system with version hashing
Modules now track their version via getVersionHash() instead of getModifiedTime().

== Background ==

While some resources have observeable timestamps (e.g. files stored on disk),
many other resources do not. E.g. config variables, and module definitions.

For static file modules, one can e.g. revert one of more files in a module to a
previous version and not affect the max timestamp.

Wiki modules include pages only if they exist. The user module supports common.js
and skin.js. By default neither exists. If a user has both, and then the
less-recently modified one is deleted, the max-timestamp remains unchanged.

For client-side caching, batch requests use "Math.max" on the relevant timestamps.
Again, if a module changes but another module is more recent (e.g. out-of-order
deployment, or out-of-order discovery), the change would not result in a cache miss.

More scenarios can be found in the associated Phabricator tasks.

== Version hash ==

Previously we virtually mapped these variables to a timestamp by storing the current
time alongside a hash of the value in ObjectCache. Considering the number of
possible request contexts (wikis * modules * users * skins * languages) this doesn't
work well. It results in needless cache invalidation when the first time observation
is purged due to LRU algorithms. It also has other minor bugs leading to fewer
cache hits.

All modules automatically get the benefits of version hashing with this change.
The old getDefinitionMtime() and getHashMtime() have been replaced with dummies
that return 1. These functions are often called from getModifiedTime() in subclasses.

For backward-compatibility, their respective values (definition summary and hash)
are now included in getVersionHash directly.

As examples, the following modules have been updated to use getVersionHash directly.
Other modules still work fine and can be updated later.

* ResourceLoaderFileModule
* ResourceLoaderEditToolbarModule
* ResourceLoaderStartUpModule
* ResourceLoaderWikiModule

The presence of hashes in place of timestamps increases the startup module size on
a default MediaWiki install from 4.4k to 5.8k (after gzip and minification).

== ETag ==

Since timestamps are no longer tracked, we need a different way to implement caching
for cache proxies (e.g. Varnish) and web browsers. Previously we used the
Last-Modified header (in combination with Cache-Control and Expires).

Instead of Last-Modified (and If-Modified-Since), we use ETag (and If-None-Match).

Entity tags (new in HTTP/1.1) are much stricter than Last-Modified by default.
They instruct browsers to allow usage of partial Range requests. Since our responses
are dynamically generated, we need to use the Weak version of ETag.

While this sounds bad, it's no different than Last-Modified. As reassured by
RFC 2616 <http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.3.3> the
specified behaviour behind Last-Modified follows the same "Weak" caching logic as
Entity tags. It's just that entity tags are capable of a stricter mode (whereas
Last-Modified is inherently weak).

== File cache ==

If $wgUseFileCache is enabled, ResourceLoader uses ResourceFileCache to cache
load.php responses. While the blind TTL handling (during the allowed expiry period)
is still maxage/timestamp based, tryRespondNotModified() now requires the caller to
know the expected ETag.

For this to work, the FileCache handling had to be moved from the top of
ResoureLoader::respond() to after the expected ETag is computed.

This also allows us to remove the duplicate tryRespondNotModified() handling since
that's is already handled by ResourceLoader::respond() meanwhile.

== Misc ==

* Remove redundant modifiedTime cache in ResourceLoaderFileModule.

* Change bugzilla references to Phabricator.

* Centralised inclusion of wgCacheEpoch using getDefinitionSummary. Previously this
  logic was duplicated in each place the modified timestamp was used.

* It's easy to forget calling the parent class in getDefinitionSummary().
  Previously this method only tracked 'class' by default. As such, various
  extensions hardcoded that one value instead of calling the parent and extending
  the array. To better prevent this in the future, getVersionHash() now asserts
  that the '_cacheEpoch' property made it through.

* tests: Don't use getDefinitionSummary() as an API.
  Fix ResourceLoaderWikiModuleTest to call getPages properly.

* In tests, the default timestamp used to be 1388534400000 (which is the unix time
  of 20140101000000; the unit tests' CacheEpoch). The new version hash of these
  modules is "XyCC+PSK", which is the base64 encoded prefix of the SHA1 digest of:
  '{"_class":"ResourceLoaderTestModule","_cacheEpoch":"20140101000000"}'

* Add sha1.js library for client-side hash generation.
  Compared various different implementations for code size (after minfication/gzip),
  and speed (when used for short hexidecimal strings).
  https://jsperf.com/sha1-implementations
  - CryptoJS <https://code.google.com/p/crypto-js/#SHA-1> (min+gzip: 2.5k)
    http://crypto-js.googlecode.com/svn/tags/3.1.2/build/rollups/sha1.js
    Chrome: 45k, Firefox: 89k, Safari: 92k
  - jsSHA <https://github.com/Caligatio/jsSHA>
    https://github.com/Caligatio/jsSHA/blob/3c1d4f2e/src/sha1.js (min+gzip: 1.8k)
    Chrome: 65k, Firefox: 53k, Safari: 69k
  - phpjs-sha1 <https://github.com/kvz/phpjs> (RL min+gzip: 0.8k)
    https://github.com/kvz/phpjs/blob/1eaab15d/functions/strings/sha1.js
    Chrome: 200k, Firefox: 280k, Safari: 78k

  Modern browsers implement the HTML5 Crypto API. However, this API is asynchronous,
  only enabled when on HTTPS in Chromium, and is quite low-level. It requires boilerplate
  code to actually use with TextEncoder, ArrayBuffer and Uint32Array. Due this being
  needed in the module loader, we'd have to load the fallback regardless. Considering
  this is not used in a critical path for performance, it's not worth shipping two
  implementations for this optimisation.

May also resolve:
* T44094
* T90411
* T94810

Bug: T94074
Change-Id: Ibb292d2416839327d1807a66c78fd96dac0637d0
2015-05-19 22:28:17 +00:00
Kunal Mehta
870f50d45c resourceloader: Implement '$pages' parameter to ResourceLoaderWikiModule constructor
This makes it easier for subclasses to use ResourceLoaderWikiModule. Currently
many subclasses of this simply need to override the getPages() method.

UserModule and SiteModule keep their getPages override due to the set of pages
being dependent on context.

Change-Id: I388531398671afacfec36c6c5746d72267b5bdac
2015-03-03 17:17:02 +00:00
Kunal Mehta
0e05ec5b31 Don't create Language objects during ResourceLoader tests
Mock calls to ResourceLoaderContext::getDirection(), which creates
Language objects to get the directionality of a language.

Change-Id: Ibe6da3013e658aa7cf596c1da2f8ca1314b7cdd3
2014-12-18 16:52:28 -08:00
Timo Tijhof
9d390a09cd resourceloader: Condition-wrap the HTML tag instead of JS response
Follows-up 9272bc6c47, 03c503da22, 1e063f6078.

One can't wrap arbitrary JavaScript in an if-statement and have
its inner-body mean exactly the same.

Certain statements are only allowed in the top of a scope (such
as hoisted function declarations). These are not allowed inside
a block. They're fine in both global scope and local function
scope, but not inside an if-block of any scope.

The ECMAScript spec only describes what is an allowed token.
Any unexpected token should result in a SyntaxError.

Chrome's implementation (V8) allows function declarations in
blocks and hoists them to *outside* the condition. Firefox's
SpiderMonkey silently ignores the statement. Neither throw a
SyntaxError.

Rgular ResourceLoader responses only contain mw.loader.implement()
and mw.loader.state() call which could be wrapped without issues.
However such responses don't need wrapping as they're only made
by mediawiki.js (in which case mw is obviously loaded). The
wrapping is for legacy scripts that execute in the global scope.

For those, let's wrap the script tag itself (instead of the
response). That seems like the most water-tight and semantically
correct solution.

Had to bring in $isRaw from ResourceLoader.php, else the startup
module would have been wrapped as well (added regression test).

Bug: 69924
Change-Id: Iedda0464f734ba5f7a884726487f6c7e07d444f1
2014-09-09 15:54:16 +00:00
Kunal Mehta
8968d8787f Check page_len in ResourceLoaderWikiModule::isKnownEmpty() for 'user' modules
In most cases, we just check whether the pages exist before saying
the module is not empty to avoid generating cached HTML without
the appropriate <script> or <link> tags.

However, for modules in the 'user' group, normal users cannot
delete their personal JavaScript/CSS pages, causing needless
extra requests, even though we know the pages are empty.

ResourceLoader::isKnownEmpty() now checks the page_len field
for modules in the 'user' group to check that there is
some actual content.

Bug: 68488
Change-Id: I0570f62887fd4642fd60367ae0b51d7dc19488ca
2014-08-30 00:24:37 +00:00
Bartosz Dziewoński
90c13419f0 Add a test for mixed /*@noflip*/ and /*@embed*/ CSS annotations
Given that we have entirely separate code for handling one and the
other, and given the nature of code comments stuffed inside other
structures, this isn't really obvious that they work.

And indeed, "/*@noflip*/ /*@embed*/" doesn't work (filed bug 69698).
Amazingly, all other combinations do.

Change-Id: Ie30bab251eb4abee122c783d057de4102e53d1fc
2014-08-18 16:40:25 +02:00
Kunal Mehta
1e301b1550 Add tests for OutputPage::makeResourceLoaderLink()
Change-Id: I22dc7fd1003f07ab0be61bb4645b45a9db9f2548
2014-07-05 04:25:58 -07:00
Timo Tijhof
75c08916b0 resourceloader: Implement "skip function" feature
A module can be registered with a skip function. Such function,
if provided, will be invoked by the client when a module is
queued for loading. If the function returns true, the client will
bypass any further loading action and mark the module as 'ready'.

This can be used to implement a feature test for a module
providing a shim or polyfill.

* Change visibility of method ResourceLoader::filter to public.

So that it can be invoked by ResourceLoaderStartupModule.

* Add option to suppress the cache key report in ResourceLoader::filter.

We usually only call the minifier once on an entire request
reponse (because it's all concatenated javascript or embedded
javascript in various different closures, still valid as one
large script) and only add a little bottom line for the cache
key. When embedding the skip function we have to run the minifier
on them separately as they're output as strings (not actual
functions). These strings are typically quite small and blowing
up the response with loads of cache keys is not desirable in
production.

* Add method to clear the static cache of ResourceLoader::inDebugMode.

Global static state is evil but, as long as we have it, we at
least need to clear it after switching contexts in the test suite.

Also:
* Remove obsolete setting of 'debug=true' in the FauxRequest in
  ResourceLoaderTestCase. It already sets global wgResourceLoaderDebug
  in the setUp() method.

Bug: 66390
Change-Id: I87a0ea888d791ad39f114380c42e2daeca470961
2014-06-12 03:48:26 +02:00
Siebrand Mazeland
1fb1fa4601 Pass phpcs-strict on some test files (1/x)
Change-Id: I7f8dee09ac734cbc369441431841f2d4aa5d7f51
2014-04-24 17:05:32 +02:00
Timo Tijhof
1a67f2dd5f tests: Add unit tests for ResourceLoaderStartupModule
Change-Id: I7671813e1d7b4ea75265608c22d8efe8805560e4
2014-03-07 20:15:35 +01:00
Timo Tijhof
99a4c31425 resourceloader: Consistently pass inDebugMode to encodeJsCall() in load.php
Still quite a few were being minified unconditinally. This is in
preparation for adding unit tests for the startup module where
it'll make reading the unit tests (and looking at the diff in case
of a failure) a lot easier if the strings aren't minified.

Change-Id: Ia06787e0ce608fcafac4596c980606d06107f517
2014-03-07 19:11:19 +00:00
Timo Tijhof
9976cef4ed tests: Add ResourceLoaderTestCase and abstract context creation
Change-Id: Ib4b265256e60a2f2109da73dc7edba6a75587ce2
2014-03-07 20:09:59 +01:00