IMHO these can be considered unit tests; they were already fast, but now
they're executed in an instant.
This requires several changes: 1 - delay retrieving messages in
AFPUserVisibleException, to avoid having to deal with i18n whenever we
want to test exceptions; 2 - Use some DI for Parser and Tokenizer.
Equivset-dependend tests are also moved to a new class, thus helping to
fix the AF part of T189560.
Change-Id: If4585bf9bb696857005cf40a0d6985c36ac7e7a8
Some of these are transformed into real unit tests, while the
AbuseFilterSaveTest class is refactored to avoid using the DB and to use
a lot more of mocks and DI.
Depends-On: I22743557e162fd23b3b4e52951a649d8c21109c8
Change-Id: Id8412e2b8a4e873fd4821ecc1a3c95710be9a870
There are lots of cases where we can inject a User object without
additional efforts. Now $wgUser is only used inside AFComputedVariable,
which is a little bit harder to handle because some instances of that
class are serialized in the DB, and thus we cannot easily change the
constructor until T213006 is resolved.
This partly copies what Ia474f02dfeee8c7d067ee7e555c08cbfef08f6a6 tried
to do, but adopting a different approach for various can*() methods:
they're now static methods in the AbuseFilter class, so future callers
don't need to instantiate an AbuseFilterView class. This also allows to
re-use those methods in an API module for editing filters (T213037).
Bug: T213037
Bug: T159299
Change-Id: I22743557e162fd23b3b4e52951a649d8c21109c8
Problems fixed:
- Don't hardcode duration in the message
- Move duration to a constant
- Fix wrong parameter order for AbuseFilter::blockAutopromote
- Log a warning if we cannot block autopromotion
- Remove the $performer parameter, as it should only and always be the
filter user.
Bug: T230296
Change-Id: Ice9e4b21033c430cf1fd34182c63ca64ad2f5d3e
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
This allows us to extract yet another static method from the AbuseFilter
class. This class should be expanded in the future, and an example use
case could be Ia5fd4f0b35fcabf045a7b49fa40fa85b72c92544.
Depends-On: I7c0170167b508132cd16e566c654a6c98dd683e9
Change-Id: I1bb45e47c3b42c01388b99778ce833e4e44419e1
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
Instead of scattering the process all over the code (and doing it
together with checking if the key already exists).
Wrap the logic in new methods for better readability.
Depends-On: Ib12e072a245fcad93c6c6bd452041f3441f68bb7
Change-Id: I24fbd41ac188a9cf6a7d3ca33dce349aedc9faa6
The last step of the profiling overhaul. See T53294 for the original description by Dragons flight.
Note: Here I'm adding a FixMe for a problem which already exists in the code
and the child patch will fix it.
Bug: T53294
Depends-On: I2d8c8f8278073a9420e3eb373fb89a655925618a
Change-Id: Ib12e072a245fcad93c6c6bd452041f3441f68bb7
Instead of seconds, and round the average condition at 1dp instead of 0.
Split from child patch by Dragons flight.
Depends-On: I2d8c8f8278073a9420e3eb373fb89a655925618a
Change-Id: I339aed5f8c1d49714e7927ce49286f9ce6c839f5
They're currently stored separately, so move matches count together with
other per-filter data to keep it consistent. This also removes a
parameter from filterMatchesKey, as it's not needed anymore.
Split from child patch by Dragons flight.
Bug: T53294
Depends-On: I8f47beb73cfc1b63c4b3c809fc6d65a1e66ee334
Change-Id: I2d8c8f8278073a9420e3eb373fb89a655925618a
Instead of having three keys, one for total actions, one for time and
one for conditions. This has several benefits: first, it avoids race
conditions which could happen having different keys. Second, it's much
more performant. Third, the code is also clearer to understand,
and more uniform with the one for global stats.
Split from child patch by Dragons flight.
Bug: T53294
Depends-On: I1dc3be6da1cc9e03bc47e8f8c867089ad0100f6f
Change-Id: I8f47beb73cfc1b63c4b3c809fc6d65a1e66ee334
This is the proper solution to replace
Ia8e38ba25d1989fe71714d2b76891c4587921466, using a class member and an
additional method. Plus, change checkFilter not to accept a prefix, but a boolean indicating if the filter is global (as that's how it's used currently).
This change also fixes an issue which caused profiling data for local
filters to be mixed with profiling data for global filters with the same
ID.
Depends-On: Iafc142d2e5ba7aa0fb0d3265fa05cace27679738
Change-Id: I1dc3be6da1cc9e03bc47e8f8c867089ad0100f6f
Instead of duplicating code in several files.
Depends-On: I2eab2e50356eeb5224446ee2d0df9c787ae95b80
Change-Id: Iafc142d2e5ba7aa0fb0d3265fa05cace27679738
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
This property is meant to be private, since it has all kinds of
getters/setters, aside from one which is introduced in this patch.
Change-Id: I217b1e22cabd3c0468c84b1d6a69a6ed3c6fa8e6
Now we have the key old-text for the old_wikitext variable, and the key
old-text-stripped for old_text. This can be confusing (see I61b2d252333ca634eae560d824f740f0f947b3d3), so use i18n keys more similar to the variable name.
NOTE: the keys will have to be changed on translatewiki if we want to avoid
confusing people.
Change-Id: Ie612350642ac4afc76f18639d988e72b4016b1e2
The current form is awkward. They're all like
[ actionname => [ 'action' => actionname, 'parameters' => params ] ]
This is greatly confusing since adds a nesting level, and just
duplicates the actionname information (also, we actually never retrieve
it from the internal array). Instead, change all of them to be
[ actionname => params ]
which is a lot shorter and clearer (and easier to handle).
A similar case is handled in I8134ecc41fbecdbed99faf406e9e3ca91b6123b9
(see PS 8..10).
Change-Id: I34c040dbeb3ab01158fb3db22496def6ccaf72d9
Use a more explicit TTL_WEEK, and add the version to avoid breaking the
world if we change the format.
Bug: T227299
Change-Id: I22705496ed8541c3dd9b643d78dff8886f4ff070
The 'AbuseFilter-filterAction' hook is deprecated in favour of a new
'AbuseFilterAlterVariables' hook, which provides a User object and has a better name, since it reflects what it should be used for, and doesn't include the name of a function which will be removed. The hook will be hard deprecated in a subsequent patch, to avoid test failures.
Depends-On: Ic0ecc8746e2883c746bef815a0fee4131f1a0646
Change-Id: I212b1e09e9c05d487d96b2f4c28f2a613e6ff3cf
This is somehow a follow-up of
Ieb04f019453033c275e211cfc9fd68d5d7c392ef. A new method is introduced to
cache a filter, which checks that all fields are there.
Depends-On: I7c1ea17adf7f42cf9260d416906bfbf3b8a20688
Change-Id: Ic0ecc8746e2883c746bef815a0fee4131f1a0646
To make the switch to afl_filter_id and afl_global easier.
Bug: T227095
Depends-On: Ie550889495232b534c0f9aec31039cf21b2135b1
Change-Id: If557bad8f5c1a6d15e3556e4bfbd0330d7d49c59
Instead of relying on static methods and members in the AbuseFilter
class, move everything related to conditions inside the Parser, as the
amount of used conditions is something pertaining a single
AbuseFilter(Caching)Parser instance.
This change requires changing some signatures and adding parameters,
but will make introducing the new AbuseFilterRunner class easier (and
that will clean signatures, too).
Depends-On: I5b29ff556eca45fe59d15e2e3df4d06f1f6b3934
Change-Id: I7c1ea17adf7f42cf9260d416906bfbf3b8a20688
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
Fix the typehint, and use selectField instead of selectRow.
Follow-up of Ie550889495232b534c0f9aec31039cf21b2135b1 suggested by
Krinkle.
Change-Id: I7e74b7b484dfa487db96598ef7aef4895d7bf275
Now it returns an array with a bit more info, and has a different name
to reflect the fact that its input is now split in two parts. Plus, make
it throw whenever it gets an unexpected input, and add a bunch of test
cases for it.
Depends-On: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
Change-Id: Ie550889495232b534c0f9aec31039cf21b2135b1
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
Added cachingParser back to *all* the parser tests, fixed a couple of
differences with the normal parser, and added a couple of tests so that
any cachingParser-related file has 100% coverage. Also move the remaining
get_matches tests inside parserTests, and specify the parser used in case of failure.
This also adds a new base class for parser-related tests with a couple
of util methods.
Bug: T201193
Change-Id: I980aec3481a52ecc35f1811a366014a5581a7cdb
Yet another important part to have covered. While for normal edits it
already works, for stashed ones it doesn't. That's why we need the patch
for checkAllFilters. Since for stashed edits profiling stats are all
zeros, this may explain T201334.
Changed the timestamp variable to use wfTimestamp instead of time() so
that we can fake it inside unit tests.
In a subsequent patch we should add average runtime conditions to tests
(really tricky).
Bug: T201193
Depends-On: Ib17821240b25c972a187e6b5eae42c5ada6c65e7
Change-Id: I5ee7ba44a6cd82a5ddb24fb4127af04d96e647f4
