Commit graph

301 commits

Author SHA1 Message Date
DannyS712 7ccf758c4b AbuseFilterConsequencesTest: stop setting $wgUser
Shouldn't be needed anymore, not read by extension

Bug: T246733
Change-Id: I10dc21ad34402d83d57f23bc754a437b8a015af7
2020-12-14 20:53:00 +00:00
Daimona Eaytoy 9d288478fe Remove a bunch of deprecated methods
These are all unused.

Depends-On: Id99da02a98bb392cafed370768edcc8ac3d712ab
Change-Id: I9ff17714f94e1fbbd52da32ebf4f054a551edc1f
2020-12-13 18:31:27 +00:00
Daimona Eaytoy 5e609eb537 Add GlobalNameUtils class
This is just a temporary location for these two methods. Since they're
used a lot, having them in the AbuseFilter class means that the
dependency graph is unnecessarily complicated. Thus, since these methods
aren't doing much, they were moved to a dedicated class. Future todo is
finding an appropriate location, that might be either as part of another
service, or keep them in a Utilities class, perhaps a single class with
all util methods, rather than a specific class.

Change-Id: I52cc47a6b9a387cd1e68c5127f6598a4c43ca428
2020-12-12 17:49:48 +00:00
jenkins-bot 93c477d4b8 Merge "Move parser classes to a dedicated namespace" 2020-12-09 10:48:36 +00:00
Daimona Eaytoy da1c71ec4c Move parser classes to a dedicated namespace
Names were kept for now.

Change-Id: Ib2eb5d7b523a64f2a0f72fdcdde2043a76cc9a37
2020-12-09 01:30:20 +00:00
Daimona Eaytoy ca3f652cd7 Almost kill the last use of wgUser
This is the last use, and it was a bit harder to remove because it was
buried inside AFComputedVariable. Starting with
I4444cada720ab62d187f2dd0c4760697e465f2ff, we can freely change the
parameters to AFComputedVariable without breaking old log entries.

Note, we still need a fallback for other extensions calling this
method...

Bug: T246733
Depends-On: I4444cada720ab62d187f2dd0c4760697e465f2ff
Change-Id: I5d786a518ef88fad9c8d9c25ef4553a0bf30b2b2
2020-12-08 23:28:24 +01:00
Daimona Eaytoy 815ef6051c Split afl_filter in afl_filter_id and afl_global
Add a script to migrate the columns (which can also
be executed in dry run), and a config option with the migration stage
(defaults to SCHEMA_COMPAT_OLD).
Some of the script-related code is stolen from
Ic755526d5f989c4a66b1d37527cda235f61cb437.

Bug: T220791
Change-Id: I7460a2d63f60c2933b36f8383a8abdbba8649e12
2020-12-08 18:31:27 +00:00
Daimona Eaytoy 1c625eeae4 Drop back-compat code
This should be merged once T246539 is done.

Bug: T213006
Change-Id: I4444cada720ab62d187f2dd0c4760697e465f2ff
2020-12-08 17:15:47 +00:00
Daimona Eaytoy 600f1735f2 Add a hook for extensions to add custom actions
The global is now deprecated, and it will be removed soon.

Bug: T265794
Change-Id: I4e6c9b143744cb72c441017921bac9cd1960609c
2020-12-04 16:10:44 +00:00
Daimona Eaytoy d351d7150b Always take into account custom actions
$wgAbuseFilterActions shouldn't be used normally, as it excludes actions
registered by other extensions.

Note: mw:Extension:AbuseFilter#Integration_with_other_extensions should
be updated after merging.

Bug: T239348
Change-Id: I89b3f0228eacdf145e8f2dd2a5602d0c7ce75a86
2020-12-03 21:39:35 +00:00
Daimona Eaytoy c786c4adc6 Add ConsequencesRegistry
Change-Id: I91f4f28e09fa46b9ab2457b2a241b6f105320bdd
2020-12-03 22:39:25 +01:00
jenkins-bot d02b10db97 Merge "Improve type safety of filter ids" 2020-12-03 17:50:31 +00:00
Thiemo Kreuz 34968d783e Simplify a few pieces of code
… mostly by inlining pieces, instead of assigning them to
a variable first.

Change-Id: Ibc432ed05f7b853a44fc1a301ef820984facb067
2020-12-03 16:50:55 +01:00
Matěj Suchánek de997fe98e Improve type safety of filter ids
Also fix a bug in FilterProfiler. It would attempt to reset
stats for global filters but we do not record them (yet?).

Change-Id: I0228d8c85dab146deb877dfce506f1e8e7711a9f
2020-12-03 14:58:51 +01:00
Matěj Suchánek 0f062fca06 Move AbuseFilterView classes to separate namespace
Change-Id: I569281b13ec81d9f35038c7ef17a2d98f16f9b5c
2020-12-03 13:06:53 +01:00
Matěj Suchánek a1ba43f586 DI for AbuseFilterView and subclasses
Also add a test to ensure all views can be instantiated without problem.

Change-Id: Iedd7a5dca240efab1077fa51a3522c983b0ba4fa
2020-12-03 13:04:35 +01:00
Thiemo Kreuz e17e1b7e01 Remove comments that literally repeat what the code says
Such comments don't add anything.

Change-Id: I7530d6693293fbdd06ca3ee077c6e783fd9a4ac1
2020-12-03 09:50:56 +01:00
Daimona Eaytoy c957188866 Add ConsequencesLookup
The class is used to retrieve consequences from the Database.

Change-Id: I46b3925aac47554723649c076eff64707a2ea2e6
2020-11-27 16:43:44 +01:00
Matěj Suchánek d76affb1db Move ChangeTags stuff to separate namespace
Change-Id: I6d7bed0e62f001f82c00a3528cc0018388c9c70e
2020-11-27 15:13:34 +00:00
jenkins-bot a6e96ed915 Merge "Introduce ChangeTagValidator service" 2020-11-27 15:13:06 +00:00
Matěj Suchánek 872b6118f4 Introduce ChangeTagValidator service
Just moving code around. Without a unit test because DI
coverage of change tags in core isn't available yet.

Change-Id: Iac861e1e24dae13581b8d9173357a1d6c94be88a
2020-11-27 15:11:48 +01:00
Matěj Suchánek 1ad77dc9fb Introduce EditBoxBuilderFactory service and EditBoxBuilder
It makes sense to look at this and Iedd7a5dca24 together,
as this patch itself doesn't really fix anything.

Change-Id: Ifef5266b1803d1a96489789b08d9beed044d908f
2020-11-26 14:49:04 +01:00
Daimona Eaytoy 904d9cddbb Represent Consequences with command objects
The consequence-taking logic is moved away from AbuseFilterRunner, to
dedicated classes. There's now one class per consequence, encapsulating
everything it needs to take the consequence.

Several interfaces allow customizing different types of consequences.
Every "special check" in AbuseFilter was generalized to use these
interfaces, rather than knowing how to handle each consequence.

Adding more consequences from other extensions will also be easier, and
it should happen via a hook (not a global), returning a class that
implements Consequence. The BCConsequence class was temporarily added
for legacy custom consequences.

A ConsequenceFactory class is added to instantiate consequences; this
would possibly benefit from using ObjectFactory, but it doesn't because
it would also reduce readability (although we might do that in the
future).

These classes are still not covered by unit tests, and this is left to
do for later. The new unit tests should mostly replace
AbuseFilterConsequencesTest. @covers tag were added to keep the status
quo (i.e. code that was considered covered while in AbuseFilterRunner
will still be considered covered), although we'll have to adjust them.

Change-Id: Ia1a9a8bbf55ddd875dfd5bbc55fcd612cff568ef
2020-11-25 17:35:36 +00:00
jenkins-bot 26d3abfab4 Merge "Introduce a service for saving filters" 2020-11-25 14:47:21 +00:00
Daimona Eaytoy c368575af0 Create a base interface for watchers
This will ease adding new watchers, for instance to send Echo
notifications (see T179495 and T100892).

For now, this is just boilerplate, and converting EmergencyWatcher to
the new interface.

Change-Id: I18d62aba53471202b709cdb19033b1729c5c25b4
2020-11-20 23:34:20 +01:00
Daimona Eaytoy 9595bd9da5 Introduce a service for saving filters
Change-Id: I6b7d16ad7ea1124989ed67c74413979cfd0275c4
2020-11-20 22:33:21 +01:00
Daimona Eaytoy 3f7fff56e8 Adjust code coverage
-Exclude methods and classes that cannot be meaningfully covered
-Add a simple test for AbuseFilterServices
-Exclude ServiceWiring because there's no way to tell PHPUnit it's
covered

Change-Id: I4c67b0d3fea68c7a3b3cbe01b5608f87e1b492db
2020-11-19 22:40:26 +00:00
Daimona Eaytoy eab1f13696 Make VariableGeneratorTest an integration test
It's actually using MediaWikiServices.

Change-Id: I6ec1b4723ff3f187eccf44a8b4ac286572fdfbbe
2020-11-19 13:55:16 +01:00
jenkins-bot 31f4607790 Merge "Handle DUNDEFINED in array offsets" 2020-11-18 23:30:58 +00:00
jenkins-bot 8f47259285 Merge "Add an interface for exporting/importing filters" 2020-11-18 23:13:53 +00:00
Daimona Eaytoy 3fc30021d2 Handle DUNDEFINED in array offsets
The behaviour is:
- When assigning to an undefined offset, delete the whole array and turn
it into another DUNDEFINED
- When retrieving from an undefined offset, just return DUNDEFINED.

Bug: T237214
Change-Id: I621ee7a16c90bb86a57be04e7ce0a748ecdbfcc7
2020-11-18 14:20:49 -08:00
Daimona Eaytoy 210cf29658 Add an interface for exporting/importing filters
The main benefit of having a dedicated interface is that we can easily
change the output format. So we're now using a custom array without
references to the DB schema, thus making the import/export process
completely independent from the schema.

Change-Id: I4c0de41d914baf1e9a0e588bd31f95b3524a424b
2020-11-18 22:06:09 +00:00
Daimona Eaytoy 7a24c94d6e Evaluate left-to-right when adding elements to array
Bug: T237090
Change-Id: I5fb72dec0ea12240b6563e66b69e399edc4c72d6
2020-11-18 21:25:45 +00:00
Daimona Eaytoy df017d478c Factor out another method from AbuseFilterRunner::getFilteredConsequences
This is a no-op, moving code around, introducing another distinction re
"filtering actions", which now happens in 2 steps:

 - The first step only uses "generic" information available by looking
   at enabled actions as a "group". This includes keeping only the
   longest block, and removing 'disallow' if other blocking actions are
   enabled.
 - The second step uses information that is only available after having
   "partly executed" (named "pre-checked") a consequence. For instance,
   we need to pre-check 'throttle' to see if the throttle was hit, and
   remove any other actions if not.

Change-Id: I7be5cfaa61e942a06f97ed52f50e9c8c70a120e8
2020-11-18 16:49:26 +00:00
Daimona Eaytoy ef9e828fbe Filter out actions to execute before actually executing them
This way we don't have special cases in executeFilterActions, and instead, we execute
all actions in the same place. In turn, this is going to ease the
transition to a new consequences system: next step is refactoring this
code into a service with proper DI etc.

Bug: T204447
Change-Id: I8134ecc41fbecdbed99faf406e9e3ca91b6123b9
2020-11-18 16:49:01 +00:00
Matěj Suchánek e7813fbafb Introduce EmergencyWatcher service
Change-Id: I45477ca84a99f620d182ef95e5627d421d38f077
2020-11-18 14:20:18 +00:00
Daimona Eaytoy ae29451ab8 Introduce a FilterCompare service
The scope is still quite limited, but as noted in a todo, we might want
to make this completely independent from the database, and add the use
case of ViewDiff.

Change-Id: Ie980fff0983b3e86037265e85da04444c809a6e8
2020-11-18 11:52:44 +00:00
Daimona Eaytoy 1bcfdc3b13 Introduce a FilterValidator
This moves a lot of things away from the AbuseFilter class. There's a
nasty static dependency on ChangeTags, but it's very limited anyway, and
it's going to be fixed once T245964 is resolved.

Change-Id: Ia7df4b4d3289c2722323f59ceecf3fdd38277785
2020-11-18 01:41:31 +00:00
Daimona Eaytoy 725ec052ed Add a FilterLookup service
Some pieces of code were updated to use Filter objects, while other
places are still to be updated. We also need to change the history part
to exclude actions somehow, cleanup the ViewEdit, reduce direct DB
access or anything mentioning DB fields outside of FilterLookup, etc.

Change-Id: I42b7ded685db76eddd45e4b1336f9828cba811ce
2020-11-18 01:17:47 +00:00
Daimona Eaytoy bad5a9a29c Make AbuseFilterViewEdit work with Filter objects
This requires adjusting some methods to work with Filter objects. Some
methods and tests are left in an inconsistent/suboptimal state, plus some todos
were added, but all of this is going to be remediated in another commit.

Change-Id: Id063ee73d97c7aef56323e1457d99704f77ab943
2020-11-18 00:52:37 +00:00
Daimona Eaytoy 71a61c2089 Add value objects to represent filters
This is just a start; next step is adding a factory/store method to
get/store these objects. And then use these value objects whenever
applicable.

Note: the actions-related code is still not fully implemented. This is
going to happen as part of the FilterLookup.

Change-Id: I5f33227887c035e301313bbe24d1c1fefb75bc6a
2020-11-04 12:56:14 +01:00
Daimona Eaytoy 1f8df50cb3 Add a service to retrieve the central DB
This is a thin wrapper around LBFactory and the global variable, that
can be injected in classes requiring it (no real class right now, but
that's going to change soon).

Also, remove some DWIM-style returns which made the code harder to
understand.

Change-Id: I1d28ad4a67f914103f3a17cda5f61b28070c7f1c
2020-10-31 12:32:46 +00:00
jenkins-bot ec5b9bef44 Merge "Add a service to retrieve the filter user" 2020-10-29 09:52:56 +00:00
Matěj Suchánek 77f6ecce13 Cleanup FilterProfiler API
Make FilterProfiler::getFilterProfile return stats unchanged,
in a structured way. Move computations to AbuseFilterViewEdit,
as they are only useful there. Don't return false on cache
misses, return arrays with zero values instead.

Bug: T266531
Change-Id: I8718cc31a5004340bf742315c7075e10a61fcbfd
2020-10-28 12:48:30 +00:00
Daimona Eaytoy cbea88f818 Add a service to retrieve the filter user
Unfortunately, this isn't using DI completely, because of the
User::newSystemUser call. I'm not even sure if we really need to call it
or we can just stick to new UserIdentityValue, but leaving like this for
now.
Also, the types were weakened to UserIdentity, so the transition is
going to be easy anyway.

Change-Id: I08f8fae0fcc622ff0ac3f86771476d06d1c18549
2020-10-26 14:06:53 +01:00
jenkins-bot 50ae561641 Merge "Simplify ViewEdit, round 2" 2020-10-25 09:10:11 +00:00
Daimona Eaytoy 18ade98339 tests: Move any profiling-related test to FilterProfilerTest
This commit removes several tests from AbuseFilterConsequences, thus
speeding it up a lot (especially because these tests were very slow,
with each test *case* taking up to 30s in the coverage job).

Everything is now covered by the new AbuseFilterFilterProfilerTest
which, although not being a pure unit test, is much much faster than
*Consequences.

Change-Id: Ic6b16d23ec99abee287f36093b8573505f9c613a
2020-10-24 18:09:26 +02:00
Matěj Suchánek 6d81fca76b Improve FilterProfiler coverage
Also improve documentation of some FilterProfiler methods.

Change-Id: I08198c643a7d2dac10e928914e8a5c7413f2543d
2020-10-24 16:23:47 +02:00
Matěj Suchánek 1445d5962a Introduce BlockAutopromoteStore service
This service is responsible for the blockautopromote feature:
(un)block autopromotion and check status.

The patch mostly moves code from static methods to the new class
and relaxes type hints (e.g. from User to UserIdentity).

Change-Id: I79a72377881cf06717931cd09af12f3b8e5f3e3f
2020-10-24 12:31:44 +00:00
Daimona Eaytoy 416dcd9ba3 Simplify ViewEdit, round 2
- Add a helper method to output an unrecoverable error, comprising a
   button to go back to the filters list;
- Move the token check to attemptSave, so to make the conditionals
  easier to read, and group errors together
- Make buildFilterEditor take an HTML parameter for the error, so the
  caller can specify whether it's error or warning
- Move the check for non-existing filters out of buildFilterEditor
- Add a bunch of typehints
- Don't set af_throttled and af_hit_count in the empty row template, but
  set af_deleted (these are only used in buildFilterEditor)
- Make AbuseFilter::translateFromHistory consistently include the af_global
  property (previously it would only be set for global filters; this error
  was introduced when first implementing global filters)
- The only user-facing change is that, when trying to use a custom
  warning/disallow message on a global filter, this is now considered a
  non-fatal error, so we now show the editing interface (and not just an
  unrecoverable error).

The next step is resolving the @todo in buildFilterEditor about null
checks.

Change-Id: I9d217dcac3f4cc0b26e53eca735cc327d5efc76d
2020-10-23 13:00:43 +00:00