Follows the amespace-ification of ResourceLoader (Id08a220e1d608,
3e2653f8), which implicitly modified the cache keygroup from
"MessageBlobStore" to "MediaWiki_ResourceLoader_MessageBlobStore".
Bring it more in line with other key groups by adopting the same
naming convention as other memcached keys in ResourceLoader and
more generally in MediaWiki code.
Also, factor out the cache key formula so that it is naturally correct
instead of requiring each of the three pairs to be tested and reviewed
to be identical to its counter-part, and yet different from the two
other very similar pairs.
Bug: T308718
Change-Id: I6b0b87e1c5ddf147788b0c61f401e8ff671b843c
While working on the site notice in Vector we've noticed these
styles are proving more problematic than helpful so splitting these
up allows us more granularity in the styles we ship.
The core interface styles are now enabled by default as they contain
important logic relating to empty portlets and the print footer
Bug: T316027
Change-Id: Ib5ae640bb260cee46e654228b785c776722c7003
The third argument to VueComponentParser::element is null for
void elements. Trying to run preg_replace on a null string causes
a warning in php8.1. Add a check for null.
This fixes a unit failure test in php8.1:
RuntimeException: Error parsing file 'resources/skins.vector.search/App.vue'
in module 'skins.vector.search': preg_replace(): Passing null to parameter #3
($subject) of type array|string is deprecated
Bug: T313663
Change-Id: I6904b0279ecc7bada3f8203a165307c17e5ee795
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
After the roll-out of $wgResourceLoaderUseObjectCacheForDeps on
WMF wikis, there was an unrelated database spike that caused some
error messages:
> DependencyStoreException:
> Cannot access the database: Too many connections (db1132)
I found this confusing, because servers shouldn't be using old
DepStore class any more (ref T311788?). And because the new store
is x2.mainstash, whereas the reported hostname is an s1.enwiki host.
I wasted some time not trusting the code path as there was no Rdbms
trace available to confirm for sure that this isn't an unrelated query
that happens to be caught during the DepStore interaction (e.g. some
generic MW code running from a hook, or Rdbms internal query from
LoadMonitor etc).
Improve telemetry by preserving the original trace.
I considered passing `$e` as third parameter to
DependencyStoreException, but since the new implementation doesn't
actually use this class, it's effectively going to remain unused in the
future and would not reliably indicate anything in particular to callers
unaware of which implementation is in use. There's also some benefit
to being able to aggregate and filter out specific db issues, which
is made harder by the same issue being reported multiple different
ways through wrapped errors.
The old implementation will remain for one release as default, and
probably one release after that as option for any third parties that
encounter an issue during upgrade so as to not block their upgrade
while we find/address the issue in question.
Bug: T113916
Change-Id: Iaa3907fc3aa0622daa9648eabfdd7efabdd4f2a9
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
The "${var}" and "${expr}" style string interpolations are deprecated in
PHP 8.2. Migrate usages in core to "{$var}" as appropriate.
Bug: T314096
Change-Id: I269bad3d4a68c2b251b3e71a066289d4ad9fd496
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
== 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
Avoid having to look at and query multiple channels for the same
core component. Use different log levels and messages instead.
E.g. avoid Foo and FooError or FooBar, use only "Foo".
This also switched to the injected Logger object at the same time.
This also avoids global wfDebugLog() at the same time.
Bug: T32956
Change-Id: I3e43b10d26858c5b3851476c8bbd27282316dd32
Use early returns to reduce nesting, unhoist variables,
should be a no-op in terms of functionality.
Change-Id: Ic6a63dccb982615b8b3e70aabd059dec70244374
Per Codesearch:
https://codesearch.wmcloud.org/search/?q=%5B%27%22%5Dexcludepage%5B%27%22%5D
There are no uses of the "excludepage" query parameter in our codebases
left. These were migrated in 2018 (change Ib9d2ce42931c1de8372e2) to
the parent class (WikiModule) via RL\Context::getContentOverrideCallback
and RL\DerivativeContext::setContentOverrideCallback.
Bug: T112474
Change-Id: I1cb4426308f4ebb1dfbd0ee1b2cd58512006ed99
Use early returns to reduce nesting in getImplicitDependencies(),
and unhoist some variables for clarity in getModuleRegistrations().
Should be a no-op in terms of functionality.
Change-Id: Ib999ecceca1f30eb289936ca3ae493e192cd0f43
All callers provide 'media' for $option and 'all' for $default, just use
those values directly. Class is not stable to extend so this protected
method is not part of the stable interface, codesearch shows no
code on gerrit outside of core that calls the method. Since the $option
parameter is removed, the name no longer makes sense, renamed and
made private.
Change-Id: I875942d5b8c339031fd302f547c9cf16328c9bf2
Use `continue` to reduce nesting, put `catch` on the same line
as the closing } from `try` (or a prior `catch` block), unhoist
variables, should be a no-op in terms of functionality.
Change-Id: Ibd3fe0708ecb25caaa72b189b9cfc68a3f2f23a0
Only need to determine $remoteDir once rather than each
time through the loop, avoid determining $media unless
needed, should be a no-op in terms of functionality.
Change-Id: Ie5a0a5ffa71197c242400c0f4b72697da0d8f26e
This was added as a workaround for the invalid values committed to
Beta Cluster config in I021e7b4003a. As I wrote then, boolean false
is not a valid value there for SkinModule. Instead of fixing this,
it was worked around in core with 4de725083 (Ie86a5b59).
Thiemo and Tim fixed the invalid Beta config last week with
164945371a (Id2712e2a), which was done as part of fixing outstanding
PHP 7.4 warnings, because despite the core workaround above, another
change to SkinModule later on introduced the same reasonable
assumption in another part of the code, that it would not be
invalid/false, but only null or string.
See also CR at:
* <https://gerrit.wikimedia.org/r/663263> (invalid config)
* <https://gerrit.wikimedia.org/r/719308> (original core change)
* <https://gerrit.wikimedia.org/r/719500> (revert core change)
Bug: T207038
Bug: T310767
Change-Id: Ieeff6198dff34de16dadb5c0347dbdae7bcf3a08
Fix notice on PHP 7.4+ when the wordmark is false. It is false on the
beta cluster and Mustache doesn't mind if it is false. Update docs.
Use !empty($x) as a shortcut for isset($x) && $x in two nearby places.
Bug: T310767
Change-Id: Ie33f8edf075f1216881428ec6aa29cae1dac4e64
5min seems like a long time to wait for a deployment race to
self-correct, likewise for internal errors that caused (part of)
a bundle to not be available.
I've shortened WMF's /w/static.php mismatch and error response to
1min caching for the same reason in e4468a4c942.
Change-Id: I81eb79cc5b398d95b6c3d5b52716dd74e62fa5eb
Move config defaults to ResourceLoader class, so that the defaults
reside within the component responsibility, and for future standalone
use and unit testing with the same set of defaults.
Bug: T32956
Change-Id: I4a268e11686e526c4377542d45e198a72e57f182
Follows-up I56a0ee74595404e1, I38a0761ae4633 and I6c3d186de1877f73d4.
Remove most "Usage" examples as these are internal and
mostly-normalized class fields. This is not where end-users should
look for documentation. Besides, many of the examples were wrong or
outdated, and almost all were incomplete. It also had an inherent
dilimma between describing all possible valid input and describing
what we actually store in that instance variable after normalization
in the class constructor. The usage docs have long been moved
to the MW config file, so we can omit that.
In its stead, place more complete and accurate type hints for Phan
to understand the possible code paths, in particular to reflect
the use of FilePath objects.
Change-Id: I1cc002f350785d3f46f79be5defb7b3cadbebf34
ResourceLoader's FilePath is designed to allow a FileModule
to include files that exist outside of the module's localBasePath,
to allow skins and extensions to extend core MediaWiki modules.
This is accomplished by having the FileModule use the FilePath's
localBasePath instead, in FileModule::getLocalPath.
(Similarly for remoteBasePath, but these are going out of fashion.)
However, the code processing 'packageFiles' did not handle this right:
it used FilePath's localBasePath when it appeared as a 'file',
but not when it was returned by a 'callback' or 'versionCallback',
using the FileModule's localBasePath instead in that case. Most
existing uses of FilePath in 'packageFiles' required the same base
path as the module, so it was convenient to avoid providing it twice.
To keep that convenience (already relied on by some extensions too)
while also allowing skins and extensions to add files from their own
directories to existing modules, the code processing 'packageFiles'
now uses FilePath's base paths in all cases, but they are optional,
and will fall back to the FileModule's paths when not provided.
Follow-up to 2890bca27d.
Change-Id: I38a0761ae4633a567b685b52c1d73b6ce280ffb7
Move ResourceLoader classes to their own namespace. Strip the
"ResourceLoader" prefix from all except ResourceLoader itself.
Move the tests by analogy.
I used a namespace alias "RL" in some callers since RL\Module is less
ambiguous at the call site than just "Module".
I did not address DependencyStore which continues to have a non-standard
location and namespace.
Revert of a241d83e0a.
Bug: T308718
Change-Id: Id08a220e1d6085e2b33f3f6c9d0e3935a4204659
This reverts commit e08ea8ccb9.
Reason for revert: Breaks Phan in extensions, and as far as I’m aware,
this change isn’t urgently needed for anything, so the simplest fix is
to revert it again for now. After PHP 7.4 it should be safer to try this
again (we hopefully won’t need the two “hack” classes by then).
Bug: T308443
Change-Id: Iff3318cbf97a67f821f78e60da62a583f63e389e
Move ResourceLoader classes to their own namespace. Strip the
"ResourceLoader" prefix from all except ResourceLoader and
ResourceLoaderContext.
Move the tests by analogy.
I used a namespace alias "RL" in some callers since RL\Module is less
ambiguous at the call site than just "Module".
I did not address DependencyStore which continues to have a non-standard
location and namespace.
Change-Id: I92998ae6a82e0b935c13e02a183e7c324fa410a3
