Just moving code around. Without a unit test because DI
coverage of change tags in core isn't available yet.
Change-Id: Iac861e1e24dae13581b8d9173357a1d6c94be88a
It makes sense to look at this and Iedd7a5dca24 together,
as this patch itself doesn't really fix anything.
Change-Id: Ifef5266b1803d1a96489789b08d9beed044d908f
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
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
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
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
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
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
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
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
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
- 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
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
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
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
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
- 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
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
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
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
Also fix the test for _first_contributor vars to cover all variables,
use builtin methods to compute the var (rather than calling
setLazyLoadVar), and to be an integration test.
Change-Id: I2594439acc786e31bce1cd4373d3cbf434204eda
While checking a filter, if a variable is not set (e.g. added_lines for
an account creation), the VariableHolder will return a DNULL, rather
than a DUNDEFINED. This means that some filters will resume working, and
the WMF servers will stop getting AF warnings at a rate of 4 millions per
day. This also requires adjusting some tests to reflect the new
behaviour (which is actually the OLD behaviour, that filters had until
last year when we introduced the DUNDEFINED data type). It also requires
adjusting a check in the old parser, but that's not really relevant
because the plan is to remove the old parser before 1.36 is released
(see I0e75f334c7e0dfc1239f2e5f5f7d7452b0bbf29e).
Bug: T230256
Change-Id: I4d06303047397674c1edbfc32628f1bc83ac3340
The problem is explained at T250570#6068702; basically, the previous
check didn't account for DUNDEFINED nested deep inside arrays.
Bug: T250570
Change-Id: Iacee2db54ca00108de6339bb3dae70af7e2eeb56
Using var_export for better visual effect, especially for arrays.
The result from /tools is much clearer and the 'wrong syntax' message is
a bit more explicative than before.
Bug: T190653
Bug: T239972
Change-Id: I79a17305c7f19f7900f896f895e9365bb5f2fd58
This was used to dynamically generate *_restriction_* variables.
However, it had two big problems:
- We only have i18n for 'create', 'move', 'edit', and 'upload' (the
default value of the global); other restrictions would show missing
messages in various pages.
- We had to access the global state in various points.
This change also makes some code in AbuseFilterVariableHolder simpler,
and also allows us to make AbuseFilterTest a unit test.
Change-Id: I321ad6e07f8243200af67a581b6e485970efd3ce
Also fix a couple of broken tests in Consequences:
- For createaccount, $user->addToDatabase must be called before
testForAccountCreation, or it will throw a CannotCreateActorException.
- In testThrottleLimit, also set wgAbuseFilterEmergencyDisableThreshold
to avoid relying on the local config.
Bug: T201193
Change-Id: If1a50b0a729e4d554485f2e2225d5877510966b6
Even if the array is DUNDEFINED, we need to check the offset to ensure
that it's valid.
Bug: T237351
Change-Id: Ibfa360c4ae1d80abe14d9fdf66991b76cb5954df
For the new parser, xhgui shows that AbuseFilterParser::getVarValue is
taking up a lot of time; in turn, most of the time spent inside
getVarValue is used to log the use of deprecated variables. Hence, given
that:
- We should keep the new parser performant
- There are tons of deprecated variables out there and they likely
won't be replaced
- Having gazillions of debugLog entries doesn't help
log them only in the cached phase.
Bug: T234427
Change-Id: I2bfc692c829c3cbe889e5076f5205e2c99097087
This is especially useful for old patches, created before the
introduction of FUNC_ARG_COUNT, where a rebase may break the parser.
Change-Id: Ib142438626a7305f102dc3e4cc9cb07ad33902b8
This bumps the level to WARN, and makes it very clear that people should
fix the affected filters. It also removes the calling method, which was
mostly meant for debugging purposes, and changes the type to 'op_type'
to avoid conflicting with type:mediawiki in logstash.
Bug: T156096
Change-Id: Ie73f1604e8ed82bc2e1be9fc90fa065be37889a3
