Commit graph

612 commits

Author SHA1 Message Date
Daimona Eaytoy 39fc7c12af Restore unit tests for CachingParser and fix it
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
2019-05-25 10:55:24 +02:00
jenkins-bot 1cb80be0ad Merge "Add tests for various data type casts" 2019-05-24 19:19:20 +00:00
jenkins-bot 058e215882 Merge "Refactor tokenizer caching" 2019-05-24 19:09:03 +00:00
Daimona Eaytoy f56562f583 Add tests for global filters
Another crucial part to have covered. Also clarify that
AbuseFilterCentralDB can be of the form "dbname-prefix".

Remove a filter used for profiling and replace it with a global one:
we're still fine, and the list is kept shorter.

Bug: T201193
Depends-On: I5ee7ba44a6cd82a5ddb24fb4127af04d96e647f4
Change-Id: If6b91711534c0d60e1aa27bd5748c3023e29f376
2019-05-24 16:58:23 +02:00
Daimona Eaytoy a766e39ade Add unit tests for profiling
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
2019-05-23 08:47:40 +00:00
Daimona Eaytoy 33fca4e096 Ignore trailing commas in function calls
First step before removing this weird syntax. I'd love to add a unit
test for params count, but I couldn't find a way, since doLevelFunction
is protected, relies on class members, and the args count is local.

Bug: T153251
Change-Id: Ica3e49f5b00595a95513d9683732e490aa7aae17
2019-05-21 23:13:31 +00:00
jenkins-bot a9afdc1f80 Merge "Slightly improve degroup action, remove Phan suppressions" 2019-05-19 17:20:23 +00:00
jenkins-bot 48ac8c492b Merge "Temporarily catch another BadMethodCallException" 2019-05-19 17:13:10 +00:00
jenkins-bot e66d30d37c Merge "Don't send empty array to Database::makeList" 2019-05-18 12:27:50 +00:00
jenkins-bot 75e5c907fc Merge "Remove problematic array type hint from hook handler" 2019-05-18 09:01:48 +00:00
Daimona Eaytoy 291c35cea0 Don't send empty array to Database::makeList
Check that the provided param is not empty, as otherwise
Database::makeList will throw and the exception will bubble up to the
user.

Bug: T222531
Change-Id: Icf5db25037a0d0a7b4076f21e7f1c9a6ee1d5a87
2019-05-18 10:55:26 +02:00
Thiemo Kreuz 1c5accd90a Remove problematic array type hint from hook handler
It's possible this parameter is null, as demonstrated in Id2caa44.

Change-Id: I69bf0d70552fb049aa1c93bb12bcb5cc9e457c53
2019-05-18 08:50:22 +02:00
jenkins-bot 6250eaea39 Merge "Add missing type hint to SpecialPage::execute()" 2019-05-16 15:19:22 +00:00
Thiemo Kreuz aba489a1f4 Add missing type hint to SpecialPage::execute()
[Also make use of the list() feature in one case I forgot before in
If2b6c95.]
 -> Changed to use direct array access by Daimona per inline comment.

Change-Id: I708dff30b6e00ccab3257b2e6fa5995eb9e30e0f
2019-05-16 14:31:54 +00:00
Daimona Eaytoy 44632f21a4 Temporarily catch another BadMethodCallException
This is the same as line 224, and I forgot to include this code path in
the same patch.

Bug: T187153
Change-Id: I28074680760a7070eb423b5eada1e35f829ed10a
2019-05-16 15:49:17 +02:00
jenkins-bot 99e821b125 Merge "Upgrade PHPCS and phan" 2019-05-15 19:42:42 +00:00
Daimona Eaytoy 311f68f6e8 Upgrade PHPCS and phan
Change-Id: Ibfb737e4552133d1ddd388e7410c6e1bc3a72e12
2019-05-15 19:06:04 +02:00
jenkins-bot 915bea466e Merge "Make use of PHP's list() feature where possible" 2019-05-15 15:06:20 +00:00
jenkins-bot c52850aae7 Merge "Add missing limits to explode() calls" 2019-05-15 15:06:18 +00:00
Thiemo Kreuz c6f20a64dd Add missing limits to explode() calls
This is fixing potential bugs where invalid strings with more than one
comma have silently been accepted.

Change-Id: Ib1e7d0c99973f243ef6faad6389bab688187c1cf
2019-05-15 16:14:12 +02:00
Thiemo Kreuz 3dece9ef8c Make use of PHP's list() feature where possible
Change-Id: If2b6c95a319800dd4944ecc0d7a8d3a819c846c1
2019-05-15 16:12:51 +02:00
Thiemo Kreuz fa3ce90851 Remove comments literally repeating what the code says
I find it obvious that a file called "AbuseFilterTokenizerTest" is a
"test for the AbuseFilterTokenizer class". A comment that is just
repeating this information is typicalls not helpful, but distracting
and a potential source of mistakes, e.g. when stuff is copy-pasted,
but the comment not adjusted.

Change-Id: I1d4cc06e9e5631955ff73bf675090cf9c33c9390
2019-05-15 16:04:32 +02:00
Thalia f23905c402 Remove call to deprecated User::isBlocked
Change-Id: Ibb7412f8aa08a745a211b9b0581ccb6b0ca9eff5
2019-05-14 13:14:57 +01:00
jenkins-bot 06cac3682e Merge "Replace deprecated cache-related functions" 2019-05-01 16:27:56 +00:00
Daimona Eaytoy 2276d8ed2a Refactor tokenizer caching
Split a method, use WAN cache so that we're enabled to use
getWithSetCallback, pass the "version" option there and adapt the test
to it.
Follow-up of I9b3bc36b552901bc6ca7609ee51e80be2979a9c4

Change-Id: I4dd81a723e2bdb828b90594ad66a3918d8ec5b6c
2019-04-23 19:38:10 +02:00
Aaron Schulz bc04dd93fe Avoid sending stashing statsd data for bots in AbuseFilter::filterAction
Change-Id: Ic06f64c22fc94665e58620a98e17264d48c97deb
2019-04-22 17:45:51 -07:00
jenkins-bot cdad0e1a14 Merge "Revert "Use string cast for Postgres compatibility"" 2019-04-18 15:31:51 +00:00
Daimona Eaytoy 9a315f2a6e Revert "Use string cast for Postgres compatibility"
This reverts commit 4ab12305f1.

Bug: T221357
Change-Id: Id0f26f48ad9904e73a8b65d76586957c2be93e82
2019-04-18 11:51:16 +00:00
jenkins-bot 968bd9b817 Merge "Add tests for tokenizer caching" 2019-04-17 23:27:19 +00:00
Daimona Eaytoy 4b10a544ab Add tests for tokenizer caching
Caching the result of the tokenization is pretty important
performance-wise, so this test ensures that caching works as expected.
I have also extracted the method used to generate the cache key for
easier testing, and moved the cache instance to a class member because
otherwise that piece of code can't be tested...

Bug: T201193
Change-Id: I9b3bc36b552901bc6ca7609ee51e80be2979a9c4
2019-04-15 16:59:55 +02:00
Daimona Eaytoy ec110c657b Add tests for various data type casts
These are the ones which other tests don't cover, mostly because no
filter syntax can trigger those cases. This patch should bring coverage
for AFPData to 100%.

Bug: T201193
Change-Id: I997576141943959d4602a9f839311108928ec766
2019-04-14 14:08:57 +02:00
Daimona Eaytoy 23fe973544 Remove pointless number cast
If the number is int there's not need to intval it, and if it's float
there's no need to floatval... Just use it to determine the internal
data type, like it happens for sum and sub.

Change-Id: Ie00c4bb4e67ad1eface0cea3eb42d04aa5fb39cc
2019-04-14 10:49:09 +02:00
Daimona Eaytoy 909eec6716 Tweak coverage part 2
Follow-up of Ic30883f7d261d974a2be46308d023e2714119e95, with two files
that I forgot to git-add and a repositioning of comments to avoid the
last bracket to be reported as uncovered.

Bug: T201193
Change-Id: I6bf7e5892a0f49f6a138792f0aedf230a70c18a8
2019-04-13 19:26:01 +02:00
Daimona Eaytoy 4bcb64b01a Increase code coverage a bit
This patch mostly adds coverageIgnore comments for intendedly
unreachable code etc. Some of them could be made testable by adding a new
filter function (e.g. array cast), but this patch is meant to be
comment-only (aside from the parser test).
Ignoring coverage for these lines makes some methods reach 100%
coverage, which in turn makes it easier to look at the coverage chart
and identify at a glance which parts of the code *really* need to be
covered.

Bug: T201193
Change-Id: Ic30883f7d261d974a2be46308d023e2714119e95
2019-04-13 18:30:14 +02:00
jenkins-bot caeaac9e7d Merge "Add tests for storing and loading the variables dump" 2019-04-12 14:09:57 +00:00
jenkins-bot ed1c996f65 Merge "Temporarily catch BadMethodCallException when computing _links vars" 2019-04-12 08:27:19 +00:00
Daimona Eaytoy 8293ec176f Add tests for storing and loading the variables dump
These are specific tests for storeVarDump and loadVarDump, both alone
and in the context of running filters.
Also, include disabled variables in the VariableHolder object if they're
saved in the DB.

Bug: T201193
Depends-On: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
Change-Id: I5e35d773904a62105767ce6d7d962ab5525c2d12
2019-04-12 08:03:33 +00:00
jenkins-bot ca6ef32a69 Merge "Use string cast for Postgres compatibility" 2019-04-11 21:50:06 +00:00
Daimona Eaytoy e5ab8483fc Temporarily catch BadMethodCallException when computing _links vars
The root cause is database rows holding a serialized revision object
(awful, right?), and to properly fix it we need a maintenance script,
still WIP (T213006).
This temp fix is to avoid flooding the exception channel.

Bug: T187153
Change-Id: I062934091fbd6213cf9bc10e8ad6864ce6a58254
2019-04-11 09:33:16 +02:00
jenkins-bot 903f3db8fe Merge "Beautify old, broken abuse_filter_history rows" 2019-04-10 05:11:38 +00:00
Daimona Eaytoy 25ed009518 Beautify old, broken abuse_filter_history rows
And right when the throttle script seemed complete... Here is another
function! So, this change splits the logic in new functions called
sequentially, and the only actual change is adding the beautifyHistory
function. Its purpose is to search ANY row in abuse_filter_history with
empty/missing parameters and normalize it. More specifically, missing
period and count are inserted as 0, and for missing groups we add
"none", used by a newly introduced message. This way, messages shown on
Special:AbuseFilter/history will be clearer and won't have gaps.

Bug:T209565
Bug:T215787
Change-Id: I38395f4df9d83badfd26cdf584ffba743b6417a9
2019-04-10 04:51:58 +00:00
jenkins-bot efe32b7c93 Merge "Add doc for every class member" 2019-04-06 14:37:19 +00:00
jenkins-bot d53c84da36 Merge "Restore check for dividebyzero" 2019-04-06 12:35:23 +00:00
jenkins-bot e03488b66a Merge "Overhaul tag selector" 2019-04-06 12:35:20 +00:00
jenkins-bot cf5df265b0 Merge "Allow filtering AbuseLog for filter group" 2019-04-06 12:24:10 +00:00
jenkins-bot 8ac0dda62d Merge "Don't publish LogEntries without ID" 2019-04-06 12:24:07 +00:00
Daimona Eaytoy a777b681e2 Don't publish LogEntries without ID
Mimic what publish() does, for the part that we need.

Bug: T219951
Bug: T218940
Change-Id: I16dd7071837a6965934d08b770f455f45cd02a6b
2019-04-06 09:46:09 +02:00
Daimona Eaytoy 451666272e Slightly improve degroup action, remove Phan suppressions
Try to get the groups from the var first, and compute them if they don't
exist. Use getEffectiveGroups instead of getGroups as it's done when
setting the lazy var loader. Avoid a pointless array_intersect within an
array_diff. Remove Phan @suppress and add docblock to make it pass.

Change-Id: I49ec6a1264b767cefea55df66ef3b02d4f443b57
2019-03-30 09:53:51 +00:00
Daimona Eaytoy 7fb3ea9002 Reduce the amount of returns
Having a single return statement inside a function isn't always the
best, but having 5 is probably worse. This patch changes three long
if-return/if-return/... to a single if/elseif + return.

Change-Id: I5f4603627c61cf1b93859fe6bcd952eac8e82359
2019-03-30 09:52:56 +00:00
Daimona Eaytoy 5b4ea18045 Don't escape abusefilter-edit-status
This was sort of missed in Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb.

Bug: T157235
Change-Id: Id952119a89df05a20c964eea8d4fe332c67f9086
2019-03-29 09:54:30 +01:00