This is another step needed to reduce the size of the gigantic
AbuseFilter and AbuseFilterHooks classes. It also makes many methods
non-static, for more testability.
Note, this layout is still not final. We should somehow merge the
functionality of VariableGenerator and AFComputedVariable, for which
I already have plans.
Change-Id: I366d598b69ad866496b7cb0059e0835c02e54041
RunVariableGenerator is for generating variables based on the current
action;
RowVariableGenerator is for RC entries;
VariableGenerator is the generic one.
This patch only moves the methods to the new classes, to keep the diff
easier to read, and facilitate conflict resolution. These classes will
then be revamped in I366d598b69ad866496b7cb0059e0835c02e54041.
Note that these classes are now namespaced.
One method, AbuseFilter::getEditVars, was renamed to
AbuseFilterVariableGenerator::generateEditVars, because it would
otherwise conflict with an incompatible method in RunVariableGenerator.
Change-Id: Iff412e5492873d4fae55402939a51609e64d55a8
This provides various shortcuts for user, target, comment, etc.,
avoiding direct access to the row, and thus a dependency on the
schema.
Change-Id: I250f94e0ac6cade33441a31ae8a27093a4d937a0
This patch is mostly replacing Revision::* constants,
Wikimedia\(restore|suppress)Warnings, and wfWikiId.
Change-Id: I13544cc3e12955a9376ccce3c120e2cee1f2ee2e
Even if the Content objects are different, the normalized text contents
may be identical.
Also, stop misattributing null edits by adding the last revision of the
page as afl_rev_id.
Bug: T240115
Change-Id: I3fb7b36ab38ca1544889a4c233b8ffdfc6c80936
$summary and $user are always guaranteed to be passed, and $user is
guaranteed to be a User object. Hence, update the hook handler to
reflect that.
Change-Id: I3a7fcb074b460b77210de5a6bad43f500aff3249
Setting 'apiHookResult' results in a "successful" response; if we want
to report an error, we need to use ApiMessage. We already were doing
this for action=upload. Now our action=edit API responses will be
consistent with MediaWiki and other extensions, and will be able to
take advantage of errorformat=html.
Since this breaks compatibility anyway, also remove some redundant
backwards-compatibility values from the output.
To avoid user interface regressions in VisualEditor, the changes
I3b9c4fef (in VE) and I106dbd3c (in MediaWiki) should be merged first.
Before:
{
"edit": {
"code": "abusefilter-disallowed",
"message": {
"key": "abusefilter-disallowed",
"params": [ ... ]
},
"abusefilter": { ... },
"info": "Hit AbuseFilter: Test filter disallow",
"warning": "This action has been automatically identified ...",
"result": "Failure"
}
}
After:
{
"errors": [
{
"code": "abusefilter-disallowed",
"data": {
"abusefilter": { ... },
},
"module": "edit",
"*": "This action has been automatically identified ..."
}
],
"*": "See http://localhost:3080/w/api.php for API usage. ..."
}
For comparison, a 'readonly' error:
{
"errors": [
{
"code": "readonly",
"data": {
"readonlyreason": "foo bar"
},
"module": "main",
"*": "The wiki is currently in read-only mode."
}
],
"*": "See http://localhost:3080/w/api.php for API usage. ..."
}
Bug: T229539
Depends-On: I106dbd3cbdbf7082b1d1f1c1106ece6b19c22a86
Depends-On: I3b9c4fefc0869ef7999c21cef754434febd852ec
Change-Id: I5424de387cbbcc9c85026b8cfeaf01635eee34a0
Instead of returning a successful message, return null and log a
warning. Also, make autopromoteBlockKey public + internal and use it
from Hooks instead of duplicating the logic.
Follow-up: I03feb05218789a3b73a31c9a94216daafcb7c145
Change-Id: I8ce96d1bd0239003f8ee6a45f412b9502d542a18
To keep a clear distinction between "private details" (i.e. user's ip)
and "private filters" (i.e. not publicly viewable). This patch renames
rights, i18n keys and methods names.
The patch for renaming globals and rights in WMF config is
I7e6b3d4453403edb6aa602587374b4ff5b6d625f.
Bug: T211004
Change-Id: I613dbadb8f75c8c4116a362607563a436a73d321
As for all mostly unused consequences, blockautopromote has a couple of
major problems: first, it blocked the status for a random time between 3
and 7 days, which to me makes no sense at all (is it some sort of
casino?), and this patch fixes it to 5 days. Second, nothing was logged,
not the blocking nor the unblocking. Here I'm adding a LogHandler for
two new sub-actions of 'rights' to keep track of both action.
Bug: T49412
Change-Id: If48a48f5b8baaf9e77c0826466f5d03bb7f691d0
This adds a new get(Type)Vars method for every action type. The goal is
to 1-have shorter methods, which is always good; 2-try to make this code
a bit more testable.
I left as a todo moving all these methods to a separate class, the idea
being to make them non-static and thus easier to be tested.
Depends-On: I2eab2e50356eeb5224446ee2d0df9c787ae95b80
Change-Id: I6de2dd27a8f972b3f74c730a1516639f8c622166
Currently we strongly abuse (pardon the pun) the AbuseFilter class: its
purpose should be to hold static functions intended as generic utility
functions (e.g. to format messages, determine whether a filter is global
etc.), but we actually use it for all methods related to running filters.
This patch creates a new class, AbuseFilterRunner, containing all such
methods, which have been made non-static. This leads to several
improvements (also for related methods and the parser), and opens the
way to further improve the code.
Aside from making the code prettier, less global and easier to test,
this patch could also produce a performance improvement, although I
don't have tools to measure that.
Also note that many public methods have been removed, and almost any of
them has been made protected; a couple of them (the ones used from outside)
are left for back-compat, and will be removed in the future.
Change-Id: I2eab2e50356eeb5224446ee2d0df9c787ae95b80
Follow-up of 0b925da36e, somehow I forgot
to add the removal code for MySQL and SQLite to the Hooks.
Bug: T214592
Change-Id: If0d1d5430573273784ff6f6e338b0c2199f6d7bb
This is the second part of removing meta-variables. To achieve this, a
static property is added and another one removed.
Depends-On: I7f60df24dc8e706af289ebbbde7536c0baf8d5c3
Change-Id: I5b29ff556eca45fe59d15e2e3df4d06f1f6b3934
Avoid all those data types (i.e. use null instead of false), use camelcase, make them private. Also, remove some logic to handle $lastEditPage being Article, as it can only be WikiPage.
Depends-On: I5a9db6e7c4356c9662a0b0a51e66252555b3d998
Depends-On: I359a618ffc4e45ce1fb70f2d1aa99a6668609e36
Change-Id: I7f60df24dc8e706af289ebbbde7536c0baf8d5c3
As all title variables would be null, and the result pretty meaningless.
NOTE: Please vote V+2 and submit manually. I359a618ffc4e45ce1fb70f2d
should then be +2ed right after that. This way, there is no need to create
two more patches just for a handful of tests being broken for a minute.
Bug: T144265
Bug: T219030
Depends-On: If6b91711534c0d60e1aa27bd5748c3023e29f376
Change-Id: I5a9db6e7c4356c9662a0b0a51e66252555b3d998
Another crucial part to have covered. Also clarify that
AbuseFilterCentralDB can be of the form "dbname-prefix".
Remove a filter used for profiling and replace it with a global one:
we're still fine, and the list is kept shorter.
Bug: T201193
Depends-On: I5ee7ba44a6cd82a5ddb24fb4127af04d96e647f4
Change-Id: If6b91711534c0d60e1aa27bd5748c3023e29f376
The reasoning is similar to the one of the parent patch (Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb). Plus, it records runtime metrics on action different than edits, as there's no reason not to do it.
No performance issues in production.
Bug: T191039
Depends-On: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
Change-Id: Ib1112e2fefd0631550d386ba87e5f87db84c3036
This variable was introduced to selectively enable profiling because
stats recording was bad for performance. Nowadays, stats are recorded in
a deferredupdate and don't harm performance anymore. Thus, this variable
can be removed and profiling be enabled by default.
Bug: T191039
Depends-On: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
Change-Id: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
Since double-equals are evil. I left some of them in place where I
wasn't sure, but I may be changed some which were intended to be
doubles. It could be a good idea to delay merging this patch until we'll
have more code coverage.
Change-Id: I1721a3ba532d481e3ecf35f51099c1438b6b73b2
The uppercase is just a leftover from a long time ago. Currently,
variables are case-insensitive, and we already perform a strtolower when
saving them. Since most parts of the code already use lowercase, the
uppercase leftovers only make it harder to grep the code to find
variables. As a bonus, make Ace recognize variables in a
case-insensitive fashion.
Change-Id: I72933fcc9952fc1aabf6464b2fc0b04ec39c024b
This patch adds missing methods for testing a filter against uploads in
RC. Please note that (as discussed below) using wfFindFile could be
relatively expensive (as it will be executed for 0-100 RC rows).
If this is true, then we should either use another method (but I
couldn't find a suitable one), or simply reduce the amount of testable
uploads to a reasonable limit.
Bug: T170249
Change-Id: Id406d4e1571873f49bb11e69029311b24ececf49
