Building on my last few changes to use UserIdentityLookup and
TitleParser, plus the recent addition of UserNameUtils to the
DummyServicesTrait, its now fairly simple to make this a unit
test and retrieve services from DummyServicesTrait instead of
MediaWikiServices.
Add a 'hookContainer' option to
DummyServicesTrait::getDummyUserNameUtils(), because
subclasses of TypeDefTestCase don't have a helper method
createHookContainer() (normally this is provided by
MediaWikiTestCaseTrait). Instead, create a manual mock
HookContainer, like we did previously at NamespaceDefTest.
Also add more options to DummyServicesTrait to allow
callers to provide service instances, needed to avoid
creating two MediaWikiTitleCodec objects and to use
a hook container in NamespaceInfo.
This also required replacing uses of createNoOpMock()
in DummyServicesTrait, because that is also not available
in UserDefTest (its another feature of MediaWikiTestCaseTrait).
It may be worth exploring splitting MediaWikiTestCaseTrait into
the parts that are specific to MediaWiki (like HookContainer
or the Message system) and parts that are useful generally
(like createNoOpMock).
Change-Id: I25b8f0256d222d994173eee66f379fb5422994b5
The User class sets the name to be the context ip when creating based
on id 0, and the switch to UserIdentity(Value) matched that behavior,
but its likely unexpected for callers - treat 0 the same as other
ids that don't correspond to existing users by returning a
UserIdentityValue with an id 0 and the name "Unknown user".
Bug: T288311
Change-Id: I9763e4c1e17de8930e0f982ea868405db20a8efd
Migrate away from the Title object, use
TitleParser::parseTitle() which returns a
TitleValue which is enough. Will be followed by
switching UserDefTest to a unit test, but in a separate
commit.
Change-Id: Ia756964861c4e0f3edea89f6beec2643243ca741
Maintain the existing behavior for creating based on a user
id or name that does not correspond to an existing user:
* if creating a User object based on the id 0, User::loadFromId()
will load the defaults with the name being set to false, and
User::getName() will convert that false to the ip of the current
request. We probably don't want UserDef to be doing that, but
this behavior should be changed separately - at the moment
we are just matching the existing behavior (see the prior commit
which adds tests to confirm the behavior isn't changing)
* if creating a User object based on an id that cannot be found
in the database, User::loadFromDatabase() will load the same
defaults, *except* that it will set the user name to "Unknown user"
* if creating a User object based on a name, User::getId() will,
if the id isn't already known, return 0 if the name matches
UserNameUtils::isIP() or ExternalUserNames::isExternal(). However,
the User object is currently only created based on a name after
passing an ExternalUserNames check, and the creation with
RIGOR_VALID prevents it from being an ip. Thus, the existing
code will call User::load(), which will check the database or
fallback to 0 if the user does not exist.
When creating a User from an id that does not exist, User::getId()
will return that id, until after the object tries to fully load,
after which it'll return 0 for the id. Since we cannot replicate
this with UserIdentity (switching the id to 0 only after getName()
is called for the first time) we set the id to always be 0, which
is more accurate. This is the only change in behavior.
We will remove the use of the context ip in a follow-up that will
switch to using "Unknown user" instead.
Accordingly, we will use a UserIdentityLookup as well as
manual construction of UserIdentityValue objects to match this
behavior.
Bug: T288311
Change-Id: Ida80c5d04d721fafa8d66f656dbd346c6cf643eb
Our basic support has been updated and Firefox supports replacement
`resolution` since version 8 or 16, depending which data source
one checks.
WebKit's support is a bit more complex and it seems like safest bet is
to leave its vendor prefix in for now.
Bug: T277803
Change-Id: I752f0b67672759824099b4b78892d143344e766f
Handle edge case of fast callbacks that happen to run just before
the transaction age reaches MAX_READ_LAG. Such callbacks will no
longer be considered "slow".
Distinguish slow and fast regenerations for the case when TTL
mitigation is triggered by the combination of snapshot and
replication lag.
Reduce the cases where TTL_UNCACHEABLE is used as the mitigation
TTL, to avoid operational risk.
Simplify the mitigation TTL code by reducing needless variation
between the "lockTSE" and non-"lockTSE" cases.
Track the key and time of the last 10 get() calls that returned
either a stale value or no value at all. Use this this estimate
the walltime in set(), if it was not provided.
Bug: T285070
Change-Id: Ie6c88fbf7dbfc831d67bfb6cde082c123555a8cf
Removes deprecated API endpoints and modules for dealing with
CSRF tokens.
Note: i18n messages are removed in a followup for ease of revert.
Bug: T280806
Depends-On: Ic83f44587db119ff2e3e6d5ff33a10894e0695e7
Change-Id: I58aedec6942ac5d3c21574cb0072f00ef365098c
Per [[mw:CC/PHP]], use of file existence check is an anti-pattern
because it is subject to race conditions (e.g. parallel tests), and
needlessly slower than just performing the operation directly.
If code needs to do something in response to the file not existing,
then one can check the return value instead. This matches how other
we do it in tests and other production code.
Follows-up I3b2bac837c.
Change-Id: I4944bead2629e9d1e89cf53fa3e710a617f63726
The software behind this is abandoned and we are migrating djvu metadata
to json instead.
This doesn't affect production as we already use djvudump
Bug: T275268
Change-Id: If45ae5746ba91ba305f93603dc1e3aafba80a369
* Move to separate file.
* Remove most of the verbose assert messages in favour of just
a few words that label the asserted value or the call that it
checks the value of. The actual/expected values are already in
the UI, and the class/method are already in the formatted test
name (which includes the module name).
Change-Id: I43ec038abd0946a92f222a859807e252696c33d4
This simplification effort is the result of a (failed) attempt to
micro-optimize this method. I submit it, despite it not being any
faster or slower, because I noticed that the simpler code actually
resulted in behaviours that I consider "less surprising".
* The method previously (I believe, unintentionally) treated all
colons as segment separators. It now only treats double-colon (::)
as segment separator.
Thus "foo.bar:baz" became "foo.bar.baz", now "foo.bar_baz".
* Leading and trailing underscores are no longer removed from
segments. This makes the former-presence of invalid characters
more obvious to consumers and seems helpful.
This also fixed two bugs:
- In some cases, a segment could become the empty string, which is
invalid for Graphite as each segment must be a directory name.
- In some cases, segments could dissappear entirely, thus changing
the hierarchy specified by the producer in ways they probably
didn't expect.
* Avoid use of empty().
Change-Id: I8601faf1fe45124370a92d43e854608e623c1214
Avoid the time cost of normalizeMetricKey (and other effects of
produceStatsdData, such as holding on to StatsdData during the request),
by deferring it until we are about to send the data to the StatsD server.
As of this commit, the sending of data still happens pre-send which
means this change does not remove its delay from the HTTP response time.
But, it does have two other effects:
* It makes the cost of calling $stats->timing() and ->increment()
much lower, which helps reduce the time attributed to calling
`WANObjectCache->getWithSetCallback`.
* It prepares the code for moving this to post-send in a future patch.
* It moves the scattered cost to a centralised place in the flame graph.
WMF's flame graphs are configued to crop the top of flames that are
smaller than 1px. This is important for making the flame graphs render
in a reasonable amount of time, but means that functions that have
many unique paths to them hidden from view. Centralising this will
make it easier to quantify how much time is spent here, which will
be of use even after it becomes post-send, since we have flame graphs
for post-send as well.
Also:
* Improve documentation.
* Add tests for normalizeMetricKey().
Bug: T288702
Change-Id: I1c57104590e48222b3e972589d49891b71afaee7
This structure test makes sure that some basic operations work peroperly
for all registered content models.
The idea is to make sure we don't break anything for extensions when we
make changes to COntent and ContentHandler classes.
Bug: T287158
Depends-On: I739be0e8b03bf2c7f2589a106e2fae46f7bb160e
Change-Id: I2fc864a201b929fcf97c02296f7e3c831d5cbea4
addGoodLinkObj() has many optional arguments, but omitting them actually
means corrupting the cache.
Nearly all existing callers are in tests.
So LinkCacheTestTrait::addGoodLinkObject() was created only
for testing. It is better to have this method in the
trait, because building the row directly in each test
would make these tests brittle against schema changes.
The only usage in WMF production code was in WikiPage and has been
fixed.
Bug: T284955
Change-Id: I03a2bd9ed64fcc0281ee29a286c8db395a9e03d9
This is no longer required now that the prefix of the proofreadpage
meta query is set to 'prpi'.
This reverts commit d6db8f4ad0.
Bug: T290585
Depends-On: If2041873eac35806ae14645064d1f544142186e1
Change-Id: Ie7092cf26c4e55d324834f3478188498012f48cb
As part of our wider work on modernising and making more welcoming the
language we use within and around MediaWiki, now is a good time for us
to rename these configuration variables:
- $wgFileBlacklist is now $wgProhibitedFileExtensions
- $wgMimeTypeBlacklist is now $wgMimeTypeExclusions
- $wgEnableUserEmailBlacklist is now $wgEnableUserEmailMuteList
- $wgShortPagesNamespaceBlacklist is now $wgShortPagesNamespaceExclusions
Bug: T277987
Depends-On: I91e065c58fda144a722a41cf532e717f962d7a64
Change-Id: I558a8b20d67d48edccce0d065aec2d22992e9dda
This reverts commit bc200b6a1a reapplying
the patch. It has been modified slightly to temporarily work
with the beta cluster's unsupported configuration.
Bug: T207038
Change-Id: Ie86a5b59fbf93a400796a4cac3724207830092b5
This reverts commit 33ac451d3d.
Reason for revert: /wiki/Special:Version TypeError: Argument 1 passed to ResourceLoaderSkinModule::getRelativeSizedLogo() must be of the type array, boolean given, called in includes/resourceloader/ResourceLoaderSkinModule.php on line 600
Change-Id: Iddeb33d6165169dc106e9beb3e4a703a4b97eeb6
-> Fixed TODOs such as adding test cases for namespaces that are
not in `$wgNamespacesWithSubpages` in `testGetSubpageText()`.
-> Re-arranged provider methods to come before the test case as
already in the files for consistency.
Change-Id: Ice2b3a07ea2ddd20417cbe87da93065e1812619e
This is shared between ProofreadPage's proofreadinfo and PageImages.
As these two extensions are not installed on the same wikis (PRP is
Wikisource-only, PageImages is disabled on Wikisource), there is not
urgent need to change one or the other, as that will break API
stability.
So, simply disable the API prefix uniqueness check for now (following
the example of T196962), and migrate the PRP info API as a follow up,
giving API clients time to update.
Bug: T290585
Change-Id: I7c35f911f254dcb7d2fa21bbac67416209229dcc
The behavior of getCascadeStrictionSources was changed by Ia73ea587586cb69eb5
to conform to the documented behavior: it would always return false as the
first element of the return value if there were no cascading restriction sources,
as specified in the documentation.
Hopwever, the previous behavior was to return an empty array in that
case, unless the $getPages parameter was false. That behavior seems more
senible, and there is existing code that relies on it.
This patch restores the previous behavior and updates the documentation
instead.
Bug: T218395
Needed-By: I31ca0a8987f9694bc3b312a48c2c111ceda6fa3e
Change-Id: I1f24703b80566220ac6fe8ee500e838ed7fd29af
- Switch do actual DI
- Add some more parameters for needed dependencies
- Create a factory interface, implement it in PageCommandFactory and add
wiring for it.
- Add new unit tests for DeletePage; unfortunately, this requires
conditionally disabling some code during tests due to dependencies on
legacy code that hasn't been migrated yet. I believe that these
temporary hacks are acceptable, since they allow us to use real unit
tests immediately.
- Adjust WikiPageDbTest: the two logging tests were identical as of
Ie0d9da2c8d273c93301921e1e108d9ffb381b8a5; and then the logging part
could just be part of the "main" test.
- Add integration tests for DeletePage. For now, these are all copying
their WikiPageDbTest counterpart. More tests will be added soon™.
Bug: T288758
Change-Id: I2fb79ed905ce621cb87f0658983d97148948da28
The methods that have been covered in this patch are listed below:
~ Title::hasFragment()
~ Title::getFragment()
~ Title::setFragment()
~ Title::isMainPage()
~ Title::equals()
~ Title::getPrefixedURL()
~ Title::prefix()
~ Title::__toString()
~ Title::getFullText()
Bug: T241406
Change-Id: I51ffadb52e7eef822e9e619e5132513dc1501f9b
See full rationale at I59068cfed10aabf6c6002f9e9312a6ef6e7e9441.
Using IDatabase for now instead of DBConnRef for better BC.
Change-Id: Ie75aaf46ba91779e8706b10efeefa9580857f489
The tests now work regardless of whether the underlying wiki or browser
would normally have the store enabled (via
wgResourceLoaderStorageEnabled, or in Firefox, or in debug mode).
Bug: T245693
Change-Id: Ib1f8867ae7ba585b7efd11e1a8692fa7da16eb28
This patch injects services into WikiExporter. It also adds a
WikiExporterFactory service for creating WikiExporter instances.
Change-Id: Ib1547defea54c309865c116bc83d617c21568843
Since MediaWiki 1.36, this method is provisioned to replace creating
new instances of the services object. If one is already created and
seen by the service locator, just use it.
Change-Id: I9509497a8380194aa93310343b1896521070fc31
== Backgrond ==
When working on an unrelated patch (Ib1f8867ae7ba585b), the tests for
mw.loader.store failed both locally and in CI, consistently, claiming
that the mw.inspect() size for this test case was now 371 instead of
372. It still passed on master where it adds up as 372.
How can this be? Well. Our tests don't run in debug mode, which means
the test suite code is minified. That shouldn't make a difference in
terms of how the code works since all statements, data, and function
calls perform the same either way. And indeed, the part of mw.inspect
that serializes arrays and objects, was unaffected.
It doesn't matter if `x = [ 'a', 'b' ]` or `x=['a','b']`. The data
seen at run-time is identical in every observable way. And thus it is
fully in the hands of mw.inspect() to choose how to measure its size
(e.g. it could call JSON.stringify with or without indentation, but
either way it's going to be consistent and unaffected by how the JS
code was formatted when the array is defined).
But.. it's different for functions. When we measure the size of a
function, we ask the JS engine to reflect back to use the source code
via Function#toString. This exposes details of minification. That's
still okay, in that the expected size of 372 is the size with this
minification taken into account.
== The problem ==
A good minifier should be deterministic given the same input, and our
minifier is no different. But, what kind of change do we expect when
the input does change? Well, in our case, we minify the code into
lines of upto around 1000 characters. This means every once in a while
a line break is let through.
In my unrelated patch, the amount of code above this test case
changed, so it changed where each compression chunk starts and ends,
and thus ended up no longer placing a line break in one of this test
case's functions.
== The fix ==
Define this function with a string literal and eval, so that it is
unaffected by minification.
== Long term ==
These kinds of subtle differences in optimisation are common to most
minifiers, and are thus unavoidable in practice. The better solution
I think is to run our tests in debug mode. I already intend to do
this after T250045, and this patch is another reason to do that,
albeit the first time in 11 years that we've run into this!
Change-Id: Ibab369ae193a7b99616da1ccef93ab4850b8686a
We will soon be converting $wgUser to be a stub
object wrapping a User, and we want to be able
to access the public properties of that object
(doing so will still trigger deprecation warnings,
but it won't break). This will also work for
non-existent properties that are handled via
the __get() and __set() methods of whatever
inner object the StubObject is wrapping.
Bug: T267861
Change-Id: I4c29c615bcb107d4ef8bf4b8e48db2ecf863e5f7
Use ObjectFactory specs for collation classes
Avoid the language construction in the factory class,
make it a detail of the implementation of each class
Follow-Up of Ifc96f851e6091ce834dbaf0e91695c648a42169c
Bug: T286079
Change-Id: Ib581f64aec8619986fb8dd49ceee0524d59a1b84
Remove Message::$format and the $format parameter of
Message::toString(), soft-deprecated in 1.26 and hard-deprecated
in 1.36. Also make the signature of toString() stricter.
Bug: T146416
Change-Id: I19aa3e482968dea5164afb93b04a55f27c644ce6
Previously, getMessages() only worked with fewer than 50 messages
(depending on user rights), and reject with API error "toomanyvalues"
if asking for more.
Bug: T290208
Change-Id: Icc5711a0017eff25788b4fb71b8ab7ec25ef0efe
