Commit graph

1110 commits

Author SHA1 Message Date
jenkins-bot 524555c400 Merge "Add value objects to represent filters" 2020-11-05 15:08:53 +00:00
jenkins-bot 1d06f5fc4c Merge "Use HTMLForm features instead of mSubmit" 2020-11-04 13:07:47 +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 0f17e47d88 Use HTMLForm features instead of mSubmit
Rely on modern HTMLForm features instead of using a dedicated class
property. The form identifiers are necessary, because these forms are
GET forms, and HTMLForm will always think that the form was submitted,
if it doesn't have an identifier (see T238467 and related
tasks/patches).

Additionally, make the first form on ViewRevert a GET form, like on
Special:AbuseLog.

Bug: T263627
Change-Id: Ia6ca45896732742ef73e401b09663728b9e7dda2
2020-11-04 11:51:27 +00:00
jenkins-bot 3b9a79cabc Merge "Introduce AbuseFilterParserFactory service" 2020-11-04 10:34:43 +00:00
jenkins-bot 648c2f8001 Merge "Divide AbuseFilterPermissionManager::canSeeLogDetails" 2020-11-04 10:32:46 +00:00
jenkins-bot 94ef2b3ad4 Merge "Resolve/remove a few todos in AbuseFilterRunner" 2020-11-04 10:00:33 +00:00
jenkins-bot 16c6cba1f5 Merge "Remove exclusions for new PHPCS rules, bump PHPCS to 33.0.0" 2020-11-03 19:53:16 +00:00
Daimona Eaytoy 4cc3934a73 Remove exclusions for new PHPCS rules, bump PHPCS to 33.0.0
Change-Id: I346c5e41b76322c4bcbc6b2402f1316e73c45681
2020-11-03 19:26:11 +00:00
Huji 9c9d2885a4 Correct the documentation of publishEntry() method
The publish() method that it resembles is not a method defined in
the LogEntry class, and not even in LogEntryBase class. It is
defined in the ManualLogEntry class. Let's reference it correctly.

Change-Id: I60cfceac7c19047e299cf9f704dda8d8ef2f2ba6
2020-11-02 15:53:55 -05:00
jenkins-bot 8946ba54a2 Merge "Remove dead line of code from AbuseFilterExaminePager" 2020-11-01 14:44:12 +00:00
Daimona Eaytoy c1c3daa031 Resolve/remove a few todos in AbuseFilterRunner
Some were outdated/not doable, others were resolved.

Change-Id: Ice524a4d31f8d90ab507801562787b946c59d651
2020-11-01 14:08:25 +01:00
Daimona Eaytoy be75cf1c40 Introduce AbuseFilterParserFactory service
TODO For the future: the final directory for Parser-related classes
should be "Parser", not "ParserNS". However, moving all classes now
would make it harder to rebase changes etc.

Change-Id: Ice335f4723e74f4e5fbe8dcc76ff8ea16310962c
2020-10-31 21:19:00 +01:00
Matěj Suchánek 5efbf80034 Remove dead line of code from AbuseFilterExaminePager
Ordering is done by in IndexPager::buildQueryInfo. In fact,
this key is unconditionally overridden there and the query
is sorted by rc_id (specified in ::getIndexField). It would
probably deserve some performance analysis because
the ordering and filtering don't seem to use matching indices.

Change-Id: I9e73d44d868ddf5beba6dc6e4550e851a6df5119
2020-10-31 18:00:03 +01:00
jenkins-bot bec7c44d12 Merge "Move mCounter property from AbuseFilterViewExamine to AbuseFilterExaminePager" 2020-10-31 16:27:12 +00:00
Matěj Suchánek 3e8a4b63ab Move mCounter property from AbuseFilterViewExamine to AbuseFilterExaminePager
It is only used there. Reduce coupling.

Change-Id: I1fad101c4cd971914a031b08f10114cd7278cc66
2020-10-31 15:31:23 +00: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 6a081ade68 Merge "Little cleanup for AbuseFilterRunner" 2020-10-31 11:42:31 +00:00
jenkins-bot 4f30f4e188 Merge "Process 'throttle' action if object caching is disabled" 2020-10-30 22:24:02 +00:00
Daimona Eaytoy 04451d7bde Little cleanup for AbuseFilterRunner
Remove outdated/pointless comments, use already defined variables, etc.

Additionally, make it possible to disable throttling locally.

Change-Id: I98fd5f3eb47b32fc1013360e462a57d932174a95
2020-10-30 21:42:54 +00:00
jenkins-bot f0962ccd51 Merge "Use MainObjectStash for generating throttle keys" 2020-10-30 20:06:37 +00:00
Daimona Eaytoy 91f2cf9439 Process 'throttle' action if object caching is disabled
See a longer explanation on phabricator.

Bug: T265216
Change-Id: I8e0054ba523f993aeb48a7e1533bbb913b46c435
2020-10-30 20:20:58 +01:00
Matěj Suchánek 59f507b16c Use MainObjectStash for generating throttle keys
Keys should be generated for a cache that will use
them.

Change-Id: Ic634410e2521b02c1b50c798a7f2d5b96705af8c
2020-10-30 18:41:45 +01:00
Daimona Eaytoy d73a94ad30 Create helper methods for the 'warn' action
Change-Id: I62e752e0dbed4f723cc6f600085a1689f3962bd3
2020-10-29 11:10:47 +00:00
Daimona Eaytoy 7dd10ff348 Split checking vs setting throttle
This is still not very useful, but it's going to come up handy when
we'll be refactoring this code.

Additionally, fix a shortcircuit issue which caused additional throttle
types to not be processed if a type was already triggered.

Change-Id: Ied44d9300b3fa2ad00fe95c9c3da3c3f8faa650b
2020-10-29 10:17:43 +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
jenkins-bot 5f38ddd5cf Merge "Add typehints to hook handlers" 2020-10-28 12:43:10 +00:00
Daimona Eaytoy be4ef544c4 Merge "Simplify ViewEdit, last round" 2020-10-28 10:38:15 +00:00
Daimona Eaytoy ccf8afe75b Add typehints to hook handlers
Needed after core change I95bb47104ad3dc0a69c812c627ffa631c5dc6ace to
make phan pass on master.

Change-Id: I6202212493340064945a559799e248130f418d6e
2020-10-28 11:37:07 +01:00
Daimona Eaytoy e0b187a546 Divide AbuseFilterPermissionManager::canSeeLogDetails
This commit splits this method into a version that doesn't need a
filter, and another version which requires one. This latter version has
a single mandatory parameter, $filterHidden, and it's up to the callers
to retrieve the value to pass in.

As mentioned in a TODO, this should eventually be changed to take a
Filter object (still under review as
I5f33227887c035e301313bbe24d1c1fefb75bc6a), which is also why
AbuseFilter::filterHidden is not being used here.

Change-Id: Id47a80131e12a5f7e1e93676299641dbf1e2b0ad
2020-10-27 19:51:01 +00:00
Matěj Suchánek be0268f200 Unbreak EmergencyDisable
FilterProfiler::getFilterProfile returns data in a different
format than the data is really stored.

Bug: T266531
Change-Id: I0d961a1ae67769da61f841df2462d47f81849972
2020-10-27 10:07:15 +01:00
Daimona Eaytoy 916234598d Simplify ViewEdit, last round
This deals with data inconsistencies in buildFilterEditor. Every
property of $row was tested in all 5 scenarios (also using Selenium) to
check when it's set. The result is in the normalizeRow method, which
aims to remove any inconsistencies, so that buildFilterEditor always
receives a "complete" row with all defaults set.

The code in buildFilterEditor is now cleaner (because there are no
isset() checks), and it gives us a unique place where we can set
defaults (rather than partly doing that in
loadRequest/loadFilterData/loadImport, and partly relying on isset).

This will be especially useful when introducing value objects to
represent filters, because now you just have to look at normalizeRow()
to tell which properties are allowed to be missing, and thus what "kind"
of filter object you need (see
I5f33227887c035e301313bbe24d1c1fefb75bc6a).

Additionally, reduce the properties that get passed around during
export/import, and make the selenium test try a roundtrip, rather than
relying on hardcoded data that may get outdated. A future patch will
refactor the import/export code.

Change-Id: Id52c466baaf6da18e2981f27a81ffdad3a509e78
2020-10-26 13:07:29 +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 711f949b95 Merge "Cleanup for AbuseFilter class" 2020-10-26 11:25:01 +00:00
Daimona Eaytoy 0d751dde04 Cleanup for AbuseFilter class
Remove unused property, move to AbuseFilterView a method that's only
used there.

Change-Id: I16658521e32eeaafc1d601528d52bef17e1bf3b5
2020-10-25 15:55:21 +01:00
Daimona Eaytoy 6c9fc516aa ViewRevert: avoid needless query
The previous code would call getUserGroups again once creating the log
entry, but this was slightly flawed: we're updating groups on master,
but the read happens on a replica that might be outdated, hence
resulting in broken logging. Instead of reading from master, we can just
keep a list of the groups that were actually added, and use that
afterwards.

Change-Id: I7cc282e15561de3a3d3e183808a65991aa27d2bb
2020-10-25 10:29:59 +01:00
jenkins-bot 8fe9902af3 Merge "Use UserGroupManager when reverting degroup action" 2020-10-25 09:24:15 +00:00
jenkins-bot 50ae561641 Merge "Simplify ViewEdit, round 2" 2020-10-25 09:10:11 +00: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
jenkins-bot d7770ad520 Merge "Introduce BlockAutopromoteStore service" 2020-10-24 13:16:57 +00:00
jenkins-bot ba9e461ed0 Merge "Deduplicate cache keys used to check blockautopromote" 2020-10-24 12:57:11 +00: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
jenkins-bot dfc9cc2a19 Merge "Code cleanup for FilterProfiler" 2020-10-23 14:43:26 +00:00
Daimona Eaytoy 5890dea4ff Deduplicate cache keys used to check blockautopromote
Previously, AbuseFilterHooks would proxy the data from a slower backend
(db-replicated) to a faster one (hash) reusing the same key. This change
makes it use a dedicated key, so that the "main" key can be kept
internal inside the upcoming BlockAutopromoteStore.

Change-Id: Id46a66991d0e994ee0a83b83b9c95e8951f3041c
2020-10-23 16:43:24 +02: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
Daimona Eaytoy 4de4ef358b Use UserGroupManager when reverting degroup action
This commit avoids direct queries on the DB, which is already an
improvement. It also adds some TODO comments for future improvements,
mostly things that depend on core changes.

Bug: T265224
Change-Id: I8eb76a0c463751976c2c5deedb3570305f1ab4f0
2020-10-23 12:07:45 +00:00
jenkins-bot cc7763f760 Merge "Add dedicated classes for more hook handlers" 2020-10-23 11:38:20 +00:00
Daimona Eaytoy 6724227182 Flatten the array returned by getConsequencesForFilters
There's no point in repeating the action name, because it's already used
as key. We can then flatten the array and just keep the parameters in
the third nesting level.

Change-Id: I54abcc49322f432cedd361abeedb72e067d3de41
2020-10-22 16:36:11 +00:00
Daimona Eaytoy b309c804fc Add dedicated classes for more hook handlers
The schema changes hook was chosen because the handler is very long. The
test ones were chosen to keep test things away from actual code.

Bug: T261067
Change-Id: Ie06bf62399f6353e3e268cccb3fe4b41bbf951c5
2020-10-22 18:23:09 +02:00