Commit graph

57 commits

Author SHA1 Message Date
Tim Starling
c16af26ea9 ResourceLoader: Factor out the loop body of makeModuleResponse()
Bug: T47514
Change-Id: Ia33ce18812b31d26b6b7ab6e50639c01c9353389
2023-07-24 18:22:20 +10:00
Lucas Werkmeister
07eaaed7c7 Use Html::encodeJsVar() and Html::encodeJsCall()
The only remaining references to the Xml:: versions of those methods are
in doc comments and in HISTORY.

Bug: T341779
Change-Id: I004a925f80ae23eff2c078c95b8caa8ccf64ffd2
2023-07-20 16:53:52 +02:00
Lucas Werkmeister
3c5a0c862f Html: Move encodeJsVar() + encodeJsCall() from Xml
These methods really belong in the Html class, not Xml. Leave behind
soft-deprecated Xml methods that forward to the Html ones, as well as a
class alias for HtmlJsCode (renamed from XmlJsCode).

Bug: T341779
Change-Id: I99a5f9de1411d4eb5ee30226b4e8ace3ea8b2c3b
2023-07-14 13:42:02 -04:00
Umherirrender
11f6d906a9 ResourceLoader: Replace array_walk in makeLoaderRegisterScript
Reuse the existing loop over modules to trim each module definition
Also remove a by-ref which is not used (was added in c0c221bf)

Change-Id: Ia3560694ccee4ce95607809d51097b3fd10d79aa
2023-06-11 09:40:24 +02:00
James D. Forrester
fa55ec4c24 Replace some deprecated wfExpandUrl calls with UrlUtils::expand
Bug: T319340
Change-Id: I2d81c2d7fd31bb07a2d2057361f1670cdeb8b8d0
2023-05-30 19:48:01 +00:00
jenkins-bot
4bbf8a908d Merge "Move some hooks to ResourceLoader\HookRunner" 2023-05-15 18:38:21 +00:00
Umherirrender
d85f9bd2b0 Move some hooks to ResourceLoader\HookRunner
- ResourceLoaderGetConfigVarsHook
- ResourceLoaderJqueryMsgModuleMagicWordsHook

Change-Id: Ifd8fa776655b347cb539ac824426afb12463148a
2023-05-11 20:51:19 +02:00
Timo Tijhof
85cde7fa8f ResourceLoader: Fix "out of sync" message to ignore $errors case
The conditional here was handling two different cases, exceptions
thrown with $errors, and versions out of sync.

Bug: T321394
Change-Id: I0b861dbac4d497c974244da7e9f444e3683e9032
2023-05-11 18:54:05 +01:00
Umherirrender
e04d3a28f6 Replace internal Hooks::runner
The Hooks class contains deprecated functions and the whole class is
going to get removed, so remove the convenience function and inline the
code.

Bug: T335536
Change-Id: I8ef3468a64a0199996f26ef293543fcacdf2797f
2023-05-11 06:17:38 +00:00
Timo Tijhof
b2bd5d70df ResourceLoader: Log when MAXAGE_RECOVER is detected
Send a message to Logstash when this condition is reached.
It is expected that this happens for a portion of clients during the
5min after a deployment, such such use an informational message,
not a warning.

This level is already enabled in wmf-config for channel=resourceloader.

I'm adding this now as I'd like to rule out or confirm whether the
our own CDN caching plays a rule in the corruptions reported at
T321394. If so, we can dig deeper there. If not, then we're likely
looking at a problem relating to browser extensions, Wikipedia mirrors,
or user scripts; rather than something under our own control.

Bug: T321394
Change-Id: Iad2f5189da33551b59653c2a6783419d6ad955d0
2023-05-05 03:21:12 +01:00
jenkins-bot
5b4f212c5d Merge "ResourceLoader: Add structure test for ResourceLoader::respond()" 2023-04-13 04:15:57 +00:00
jenkins-bot
0856164ba6 Merge "ResourceLoader: Avoid new use of MWException" 2023-04-11 15:13:26 +00:00
jenkins-bot
222bcd8081 Merge "ResourceLoader: Descriptive error for Less imports from codex-design-tokens" 2023-04-11 02:04:01 +00:00
Timo Tijhof
6ef84a7218 ResourceLoader: Avoid new use of MWException
Follows-up I992ccde79a59ad51c.

* Soften the test to only asserting that an exception is thrown,
  the type isn't part of the API.

* Throw RuntimeException instead of MWException per T86704.

Bug: T86704
Bug: T328602
Change-Id: Ifaa5d659941d60789ca14f771648e5830bbef317
2023-04-10 18:47:45 -07:00
Tim Starling
05c54baa36 ResourceLoader: Add structure test for ResourceLoader::respond()
Confirm that load.php can respond for all registered modules without
errors (except private modules, which can't we don't serve there).

Bug: T47514
Change-Id: I44655b2c05529ae719d71622f57bfed8d632550e
2023-04-10 18:18:54 -07:00
Tim Starling
25d0d37adc ResourceLoader: tweak comments and error messages
Change-Id: Ifcaefedf65b090a87ec1417808277ae6451302dc
2023-04-03 18:21:30 +10:00
Timo Tijhof
3e2d1921aa ResourceLoader: Improve makeLoaderImplementScript() docs
* Document `$scripts` types in a list for clarity, and make the
  end-to-end connection to what these are for in practice (especially
  for debug mode, and for site/user scripts).

* Fix outdated `$styles` docs. This has changed long ago.

* Remove support for $messages array. This existed solely to pass
  an empty value for cases where messages are not needed.
  Change that to null instead.

Change-Id: I0c0ef94d830171a3dd8588de8c4a1f3d67607d41
2023-03-31 21:58:14 -07:00
Roan Kattouw
aaecb49e9b ResourceLoader: Descriptive error for Less imports from codex-design-tokens
Importing Less files from @wikimedia/codex-design-tokens/ doesn't work,
on purpose. Instead of letting these imports fail with a nondescript
"file not found" error that doesn't help the developer understand why it
didn't work, provide a descriptive error message that points them in the
right direction.

Bug: T328602
Change-Id: I992ccde79a59ad51c7ebfe3ac7548a6e531f4a59
2023-03-30 16:19:38 -07:00
Roan Kattouw
0d96067d46 ResourceLoader: Use str_starts_with() for Less import paths
Follow-up to I2df266cde90d1b8dad8d1d1367b67001e2b2984d

Change-Id: I983df7c558473519297d745694452ffde1a07139
2023-03-30 09:53:24 -07:00
Roan Kattouw
7492234847 ResourceLoader: Add path remapping for Less imports
This replaces the hacky wrapper files in
resources/src/mediawiki.less/mediawiki.skin.codex-design-tokens/ and
resources/src/medawiki.less/@wikimedia/codex-icons/dist/ with real
import path aliasing/remapping.

Bug: T328602
Change-Id: I2df266cde90d1b8dad8d1d1367b67001e2b2984d
2023-03-28 16:18:44 -07:00
Tim Starling
be3018b268 Just another 80 or so PHPStorm inspection fixes (#4)
* Unnecessary regex modifier. I agree with this inspection which flags
  /s modifiers on regexes that don't use a dot.
* Property declared dynamically.
* Unused local variable. But it's acceptable for an unused local
  variable to take the return value of a method under test, when it is
  being tested for its side-effects. And it's acceptable for an unused
  local variable to document unused list expansion elements, or the
  nature of array keys in a foreach.

Change-Id: I067b5b45dd1138c00e7269b66d3d1385f202fe7f
2023-03-25 00:39:06 +00:00
Timo Tijhof
89c17acbf6 ResourceLoader: Silence failure of encodeJson() for $modules param
The $modules param is user controlled. We already catch invalid UTF-8
in server configurations. If something like that makes it here, the
only place it can come from is the client doing it intentionally,
such as the result of scanning tools.

Follows-up I89832142b, which silenced $modules formatting in a CSS
response. This patch also covers the case of a JS response, and adds a
test for both.

Bug: T331641
Change-Id: I03a0ec262203509f069654e7fa6fd8d8debd25d5
2023-03-23 11:20:59 -07:00
Aaron Schulz
29bab859fc profiler: Add ProfilingContext class
Use this class to track the entry point and handler used for requests,
making it available for use in profiling, stats, and logging code.

This makes it possible for periodic and/or shutdown profiling callbacks
to know the basic action handler that applies to the request (if any).
Metric names can easily include this string along with MW_ENTRY_POINT
to create per-action profiling dashboards.

This info cannot otherwise be acquired from things like excimer stack
traces since the router and handler classes do not appear in the stack
during PRESEND deferred updates and variations like ApiMain/SpecialPage
"inclusion mode" would have to be detected somehow.

Bug: T330810
Change-Id: Icca5a7a343faeeb18652994c96752acb61a61fd1
2023-03-23 00:08:49 +00:00
Timo Tijhof
02b4671dbf ResourceLoader: Silence new encodeJson warning for $states array
Follows-up I89832142b55ec0f for T329330. encodeJson() is almost always
called with data that a core component or extension was responsible
for processing or generating. There is one exception that I know of,
namely within ResourceLoader itself, where we report back the name of
a load.php?modules parameter value if it doesn't exist or is invalid.

In that case, silence the newly added warning from encodeJson() since
the audience for that error is not the developers working on the code,
but the user making a badly formed URL. And the warning was coming
from the exact code formulating that same error message.

Bug: T331641
Change-Id: Idcccb979c89ef837546892ab786d803d429ecda5
2023-03-10 19:00:05 +00:00
jenkins-bot
a37c1d0b4a Merge "ResourceLoader: Make internal encodeJsonForScript private" 2023-03-07 20:43:41 +00:00
Timo Tijhof
1ece7ee097 ResourceLoader: Make internal encodeJsonForScript private
Has been marked `@internal` since initial commit, with no references
outside this class in Codesearch Everywhere. The method relies on global
state in the form of $wgRequest, via ::inDebugMode.

New code can (and does) uses RL\Context::encodeJson() instead.

Bug: T32956
Change-Id: Ia5f3da26f4ee6dd5f49eb1869653fa7c3ce90e3a
2023-03-06 23:49:23 +00:00
Timo Tijhof
7ac1439211 ResourceLoader: Remove makeMessageSetScript() method
This was originally an internal method for ResourceLoader to create
the load.php?only=messages response. This was removed in 2015 with
change Ia6c87d687c6 (9b6ee1da59) in MW 1.26. After that it was
briefly used by RCFilters until change I2d58f55701 (2dd6342b64)
in 2018.

It has no known use in Codesearch Everywhere, no test coverage,
and relies on global state (including $wgRequest, via ::inDebugMode).

Bug: T32956
Change-Id: I49d71aa319c571cfefc9bcb459cd97047eb6d839
2023-03-06 01:53:13 +00:00
James D. Forrester
ad06527fb4 Reorg: Namespace the Title class
This is moderately messy.

Process was principally:

* xargs rg --files-with-matches '^use Title;' | grep 'php$' | \
  xargs -P 1 -n 1 sed -i -z 's/use Title;/use MediaWiki\\Title\\Title;/1'
* rg --files-without-match 'MediaWiki\\Title\\Title;' . | grep 'php$' | \
  xargs rg --files-with-matches 'Title\b' | \
  xargs -P 1 -n 1 sed -i -z 's/\nuse /\nuse MediaWiki\\Title\\Title;\nuse /1'
* composer fix

Then manual fix-ups for a few files that don't have any use statements.

Bug: T166010
Follows-Up: Ia5d8cb759dc3bc9e9bbe217d0fb109e2f8c4101a
Change-Id: If8fc9d0d95fc1a114021e282a706fc3e7da3524b
2023-03-02 08:46:53 -05:00
Amir Sarabadani
4bb2886562 Reorg: Migrate WikiMap to WikiMap/ out of includes
And WikiReference

Bug: T321882
Change-Id: I60cf4b9ef02b9d58118caa39172677ddfe03d787
2023-02-27 05:19:46 +01:00
Amir Sarabadani
7d8768e931 Reorg: Move HTML-related classes out of includes/ to Html/
Bug: T321882
Change-Id: I5dc1f7e9c303cd3f5b9dd7010d6bb470d8400a18
2023-02-16 20:40:01 +01:00
Umherirrender
ed169d991e Remove unused arguments to private functions
Found by phan dead detection

Change-Id: I93379b7b9a733206d0e53add04fcdb9478c58755
2023-02-08 19:00:47 +00:00
Alexander Vorwerk
f6bd18d6c2 Split a base class out of CommentStore
so that extensions (i.e. CheckUser) can implement their own comment
store without having a lot of code duplication

basically the comment store version of I3a6486532f2ef36

Bug: T233004
Change-Id: Ib40f99e00a514d41776ce521baf113e46d37e9cd
2023-01-01 22:34:36 +00:00
thiemowmde
8b49357b25 Replace some tivial ??= with even more trivial ??
Patch Ifa7a9bc replaced some longer `=== null` constructs with the
new ??= operator we have since PHP 7.4. However, some of these can be
simplified even more with the ?? operator we have since PHP 7.0.

Follow-Up: Ifa7a9bc7b2ec415ad7ecb23f4c1776f51f58fd6b
Change-Id: I7b05e723810558bb5437adc97eab54ca04d38c06
2022-12-23 08:35:44 +01:00
DannyS712
c1db64b808 Make use of ??= in more places
New feature from PHP 7.4

Change-Id: Ifa7a9bc7b2ec415ad7ecb23f4c1776f51f58fd6b
2022-12-17 01:10:13 +00:00
jenkins-bot
3173f253fb Merge "ResourceLoader: Add 1min grace via stale-while-revalidate Cache-Control" 2022-12-07 22:10:30 +00:00
Amir Sarabadani
02e5a33057 Drop more unused hard deprecated hooks
None are used in WMF-deployed extensions and have been hard deprecated
for multiple releases as well.

Change-Id: I62cfa22291f81295b4908192de8657a750c6716d
2022-12-01 03:36:48 +01:00
Tim Starling
f94dfb6e0b ResourceLoader: Clean up PHP 7.2 hacks
Remove workarounds for the lack of proper autoloading for aliased
parameter type declarations which was fixed in PHP 7.4.

Change-Id: Ic4485d06e7ad8d02859dc3e5c67867c4b084386f
2022-11-17 09:51:33 +00:00
Amir Sarabadani
7690ab4e33 Reorg: Move HeaderCallback to Request directory
Cleaning root of includes/

Bug: T321882
Change-Id: I1844da95d4fd79824646fdf4b6063cb771ca3000
2022-11-08 10:53:27 +01:00
Timo Tijhof
5cf4b7c3e2 ResourceLoader: Add 1min grace via stale-while-revalidate Cache-Control
Only enable it if maxage is above 1 minute in the first place, e.g.
not for debug/error responses or when otherwise having configured
things during development with a very low maxage.

This gives blocking resources like the startup manifest and stylesheets
a chance to never block page rendering other than during the first
view of a browsing session. Once a browsing session reaches the 5min
mark, instead of blocking the next page load, it could then be renewed
in the background if the pageview starts during the 6th minute.

CDNs and web proxies known to support the stale-while-revalidate
directive:
* Varnish 4.1.0
* NGINX 1.11.10
* Squid 2.7

Browsers known to support it:
* Firefox 68 (2019)
* Chrome 75 (2019)

Bug: T132418
Change-Id: Ifae1f09722be4abf1c4dfe895122a3503010a422
2022-10-27 23:38:51 +00:00
Zabe
f6b9381d7f Revert "Reorg: Move some of request related classes to MediaWiki/Request"
This reverts commit 2bdc0b2b72.

Reason for revert: T166010#8349431

Bug: T166010
Change-Id: Idcd3025647aec99532f5d69b9c1718c531761283
2022-10-27 13:14:16 +00:00
Amir Sarabadani
2bdc0b2b72 Reorg: Move some of request related classes to MediaWiki/Request
Moving:
 - DerivativeRequest
 - FauxRequest
 - FauxRequestUpload
 - PathRouter
 - WebRequest
 - WebRequestUpload

Bug: T166010
Change-Id: I5ea70120d745f2876ae31d039f3f8a51e49e9ad8
2022-10-26 16:49:10 +02:00
Umherirrender
5c5498a202 Remove unused key variable from foreach loops
Change-Id: Id2d91e30a6f7cc4eb93427b50efc1c5c77f14b75
2022-09-21 21:18:43 +02:00
Timo Tijhof
5965ae141a ResourceLoader: Remove redundant getCombinedVersion() try-catch
The getCombinedVersion() method itself already catches errors
in its loop. There are no known exception that we know can happen
and would want to tolerate within the loop logic itself.

Empirically, there are indeed zero messages in WMF Logstash for the
past 30 days that contain "Calculating version hash failed". Other
diagnostic messages in channel:resourceloader do show up from time
to time, e.g. once a week or so, indicating that the other
conditions and this log method generally do work and are enabled.

Bug: T312806
Change-Id: I2fe5ad104a6404c53c1b73b259468a6ed1071936
2022-08-03 01:05:23 +00:00
Timo Tijhof
584d5dd163 ResourceLoader: Remove outer TimeoutException try-catch in respond()
Follows-up I4c3770b9ef. While timeouts in load.php are extremely
rare (one in 90 days?), we did find one in the perf-team Logstash
dashboard and it coincided with this runtime error as result of the
added try-catch:

> PHP Notice: Undefined variable: etag

Remove the try-catch and let these fatal the same way as other very
early or very late errors with HTTP 500 instead.

Bug: T312806
Change-Id: Iffb238de2243d0885dcf1d78fcce7827b09cb84d
2022-07-31 18:39:48 -07:00
jenkins-bot
82df7b7b63 Merge "tests: Remove intermediary suites concept from /tests/qunit" 2022-07-13 20:00:00 +00:00
jenkins-bot
352c68b793 Merge "ResourceLoader: Remove DependencyStore::renew" 2022-07-13 05:03:50 +00:00
Timo Tijhof
68d4fe68b9 tests: Remove intermediary suites concept from /tests/qunit
I don't recall why I added this. Possibly in a confused effort
to match /tests/phpunit, except /tests/phpunit/suites is not
where test cases live, they live under /tests/phpunit/* directly,
mostly /tests/phpunit/includes named after the source directory.
The correct equivalent to that is /tests/qunit/resources for JS.

While at it, also remove mention of this concept from various other
places where it doesn't add value. It's one more word/concept to
learn, process, understand, or translate mentally. They're just tests,
or for the one or two places where we care about how they are
internally transmitted, a "test module".

Bug: T250045
Change-Id: I5ea22e4965d190357aa69883f29f9049ee8ebf13
2022-07-13 01:52:57 +00:00
Timo Tijhof
1d66a22805 ResourceLoader: Remove DependencyStore::renew
== Background

When file dependency information is lost, the startup module computes
a hash that is based on an incomplete summary of bundled resources.
This means it arrives at a "wrong" hash. Once a browser actually asks
for that version of the module, though, we rediscover the dependency
information, and subsequent startup responses will include arrive once
again at the same correct hash. These 5-minute windows of time where
the browser cache of anyone visiting is churned over are not great,
and so we try to avoid them.

The status quo is the dedicated module_deps table in core with no
expiry. This means a potential concern is building up gargage over
time for modules and extensions that no longer exist or are no longer
deployed on that wiki. In practice this has not been much of an issue,
we haven't run the cleanupRemovedModules.php or purgeModuleDeps.php
scripts in years. Once in 2017 to fix corrupt rows (T158105), and
once in 2020 to estimate needed space if we had expiries
<https://phabricator.wikimedia.org/T113916#6142457>.

Hence we're moving to mainstash via KeyValueDepStore, and not to
memcached. But for that we might as well start using experies.

To not compromise on losing dep info regularly and causing avoidable
browser cache for modules that are hot and very much still existing,
we adopted `renew()` in 5282a0296 when drafting KeyValueDepStore, so that
we keep moving the TTL of active rows forward and let the rest naturally
expire.

== Problem

The changeTTL writes are so heavy and undebounced, that it fully
saturates the hardware disk, unable to keep up simply with the amount
of streaming append-only writes to disk.

https://phabricator.wikimedia.org/T312902

== Future

Perhaps we can make this work if SqlBagOStuff in "MainStash" mode
was more efficient and lenient around changeTTL. E.g. rather than
simultanously ensure presence of the row itself for perfect eventual
consistency, maybe it could just be a light "touch" to ensure the
TTL of any such row has a given minimum TTL.

Alternatively, if we don't make it part of the generalised
SqlBag/MainStash interface but something speciifc to KeyValueDepStore,
we could also do something several orders of magnitudes more efficient,
such as only touching it once a day or once a week, instead of several
hundred times a second after every read performing a write that
amplifies the read back into a full row write, with thus a very large
and repetative binlog.

== This change

As interim measure, I propose we remove renew() and instead increase
the TTL from 1 week to 1 year. This is still shorter than "indefinite"
which is what the module_deps table does in the status quo, and that
was never an issue in practice in terms of space. This is because
the list of modules modules is quite stable. It's limited to modules
that are both file-backed (so no gadgets) and also have non-trivial
file dependencies (such as styles.less -> foo.css -> bar.svg).

== Impact

The installer and update.php (DatabaseUpdater) already clear
`module_deps` and `objectcache` so this is a non-issue for third
parties.

For WMF, it means that the maintenance script we never ran, can
be removed as it will now automatically clean up this stuff after
a year of inactivity, with a small cache churn cost to pay at that
time.

Bug: T113916
Bug: T312902
Change-Id: Ie11bdfdcf5e6724bc19ac24e4353aaea316029fd
2022-07-12 15:25:39 -07:00
Timo Tijhof
e06081bbfd ResourceLoader: Restore 5min startup cache-control (was 60s)
Follows-up I81eb79cc5b398d95b6, which accidentally shortened this
to 60s.

Bug: T312796
Change-Id: I73b3144b9e461541b0c5c820dbdcaf90d8d1fa1a
2022-07-11 10:34:29 -07:00
Derick Alangi
ae22031299 ResourceLoader: Inject HookContainer & UserOptionsLookup to getUserDefaults
Change-Id: I328ee0f959f0898a7e2632dfcfa3e3bbb579106e
2022-07-01 00:53:37 +00:00