Commit graph

86 commits

Author SHA1 Message Date
Matěj Suchánek 352a207c70 Improve code coverage
Bug: T201193
Change-Id: Ie086fd525bec19c63c13f8710a27897229cc33c8
2020-12-19 16:28:34 +01:00
Matěj Suchánek dc7509811a Notify of a throttled filter
Use Echo for delivering the notification to the last
user who edited the filter.

Much boilerplate.

Change-Id: I7a46a03b4f15de20902ec70c62fb4fe750096842
Depends-On: If585b14a6dd6fb8c7d2c3bee1f20d9d08eaac706
2020-12-19 10:31:29 +01:00
jenkins-bot d386633103 Merge "Create a dedicated namespace for all consequences-related classes" 2020-12-18 22:57:00 +00:00
Daimona Eaytoy b394956c22 Create a dedicated namespace for all consequences-related classes
Change-Id: Ibc39593e34da36e57b640af0b5bbf2145f725e92
2020-12-18 19:27:33 +00:00
Daimona Eaytoy 7c1d1c6d7d Return warnings from the parser, add warning for catch-all regexps
This commit introduces some boilerplate for emitting warnings from the
AbuseFilter parser, and also code for showing these warnings in the ace
editor. Adding new warnings should be as simple as appending to
AbuseFilterParser::warnings (and adding the relevant i18n).

Bug: T264768
Bug: T269770
Change-Id: Ic11021b379f997a89f59c8c0572338d957e089a6
2020-12-18 18:22:41 +01:00
Matěj Suchánek 2da4cb4321 Implement GetAutoPromoteGroupsHook
Test coverage included.

Bug: T261067
Change-Id: Iba49f0518d2c1a72405498b2743e5ef300dd6d61
2020-12-15 18:01:04 +01: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
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 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
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 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
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
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
Matěj Suchánek 93556284a0 Inject ChangeTagsManager to ChangeTagger
We decided to have the tag name provided by ChangeTagsManager,
so make ChangeTagger depend on it.

Change-Id: If3cbfd992f45651f47477031befffc0fd30f4a28
2020-10-21 16:30:43 +02:00
Daimona Eaytoy 9bc885b6b3 Add a ChangeTagger class
The logic about action IDs and the persistent buffer is now encapsulated
inside a single service, which is a step towards getting rid of global
state in the AbuseFilter class, and reducing the responsibilities of the
Runner.

An important change made here is that we now require a LinkTarget rather
than a Title. This removes a dependency on the Title class (a monster
object), makes tests simpler, and denies the need to inject a
TitleFactory. This means living without some bits of context (e.g. we're
no longer using makeTitleSafe to ensure a valid title, and we have to
build a "prefixedtext" manually), but this shouldn't be a problem, given
that the titles are only used to create a cache key: invalid titles are
not a problem, and concatenating namespace + title should always be
sufficient.

Bug: T265370
Change-Id: Iff59cd3d889454a482a89c16691bfefcc5ec0a12
2020-10-21 13:19:30 +02:00
Matěj Suchánek d91ddd2169 Cleanup filter id handling on Special:AbuseFilter/history
In particular, the interface shouldn't generate links to
"Special:AbuseFilter/history/0" (AbuseFilterHistoryPager::getTitle,
can be seen when visiting "Special:AbuseFilter/history").

Change-Id: Id3dc1bb4fc3c5e853603bf0ec04a6b1751f7d862
2020-10-10 11:40:46 +00:00
Daimona Eaytoy f0539e0c1e Represent new filters with null instead of 'new'
PHP is not strongly typed, so it's not a good idea to use scalars of
different types (here it's an integer vs the string 'new') to represent
different possibilities. This can have bad effects when type juggling
occurs, and it's also harder to figure out what the type of the
parameter can be (because a numeric ID might have been passed as a
string). Using integer vs null avoids all of this, and also allows us to
use nullable typehints.

These changes were partly copied from
If981cb35bf19a8469aa6c43c907e107cf8c65bc2 and should help with the
migration to the Filter value objects.

Change-Id: I8837d46c3c33761fea53f67b530b721dc7bd49b0
2020-10-10 12:23:50 +02:00
Daimona Eaytoy 2e13d58c74 Add tests for retrieving RC variables
This was also long overdue. Also fix a bug that caused page creations to
not be shown when examining past edits (using rc_last_oldid doesn't work
for page creations).

Bug: T201193
Bug: T262903
Change-Id: I5f7a994add12332c950904146248c5de7c2beee5
2020-10-04 12:43:04 +00:00
Matěj Suchánek eb81b92c06 Refactor AbuseFilterView instantiation
- Make a separate method which determines the view
  to be shown from subpage syntax and test it.
- Reduce circular dependency between SpecialAbuseFilter
  and AbuseFilterView. Use params to transfer information
  to views.

Change-Id: Ib9442ea5f9990a5c48f9b9e04055aa22bf7e456e
2020-10-04 13:15:04 +02:00
Daimona Eaytoy 1bdf4e5351 Rewrite the VariableHolder code to translate deprecated variables
The current code was more of a subpar, temporary solution. However, we
need a stable solution in case more variables will be deprecated in the
future (T213006 fixes the problem for the past deprecation round). So,
instead of setting a hacky property, directly translate all variables
when loading the var dump. This is not only stable, but has a couple
micro-performance advantages:
 - Calling getDeprecatedVariables happens only once when loading the
   dump, and not every time a variable is accessed
 - No checks are needed when retrieving a variable,
   because names can always assumed to be new

Some simple benchmarks reveals a runtime reduction of 8-15% compared to
the old code (8% when it had varsVersion = 2, 15% for varsVersion = 1),
which comes at no cost together with increased readability and
stability. It ain't much, but it's honest work.

Change-Id: Ib32a92c4ad939790633aa63eb3ef8d4629488bea
2020-09-29 15:06:14 +00:00
jenkins-bot 7a684c487c Merge "Move some misplaced AbuseFilterParser entry points" 2020-09-29 13:51:17 +00:00
Daimona Eaytoy 55ba083b13 Introduce a KeywordsManager service
This will decouple a bit the huge and chaotic tangle of AF classes. Some
boilerplate code for AbuseFilter services is also added with this patch.

Note that this requires injecting a KeywordsManager in
AbuseFilterVariableHolder, or unit tests would fail. This is still
incomplete, and the Manager is only injected in tests, because
VariableHolder still has to be refactored.

The test for the UpdateVarDumps script had to be updated, because
serializing VHs in there was a bad choice. As pointed out in a comment,
the test is likely going to break again once we remove the BC code, but
I hope that we'll be able to remove the test at that point.

Change-Id: I12a656a310adb8c5f75cab63f6db9e121e109717
2020-09-28 23:03:52 +00:00
Daimona Eaytoy a1626a0d7f Move some misplaced AbuseFilterParser entry points
These methods had no reals reason to be static and belong to the
AbuseFilter class. Most of them were moved to Parser class as common
variations of the existing entry points. One was specific to the
EvalExpression API module and was moved there.

This change comes at no cost, and will make it possible to inject a
parser where needed.

Change-Id: Ifd169cfc99df8a5eb4ca94ac330f301ca28a2442
2020-09-29 00:36:08 +02:00