Commit graph

30 commits

Author SHA1 Message Date
Daimona Eaytoy ed49f86b74 Make User::get* calls explicit in LazyVariableComputer
With explicit calls it's easier to see what method is being used,
whether it's deprecated, etc. Some methods here are in fact deprecated
or already have a proper replacement, but this is left for a follow-up.

Change-Id: Iee3154855f86c76aab98e7c14250c14e8b9ee939
2021-01-17 00:35:40 +00:00
Daimona Eaytoy 66928eda89 Remove deprecated param
Depends-On: Ie6abd2df5cf1b09c35f0a9e53b0d559e887de09b
Depends-On: Id99b94806095a974a65dd892b2200e59c475802f
Change-Id: I9a33cadb9903461038aa1095be18b68a60dd726d
2021-01-14 23:43:51 +01:00
Daimona Eaytoy 9afc968523 Refactor VariableGenerator and LazyVariableComputer tests
Additionally, avoid building Title objects in LazyVariableComputer, it
just adds a dependency on TitleFactory and creating mocks is more
complicated, but it's pointless because the caller already has a Title
object.

And also stop using Title::getEarliestRevTime(), since the replacement
is easy (we already have a RevisionLookup).

Note for reviewers about renames:
- Code VariableGeneratorDBTest was moved to LazyVariableComputerDBTest,
  RCVariableGeneratorTest, and AbuseFilterVariableGeneratorTest
- AbuseFilterVariableGenerator test was moved into a dedicated
  directory, methods were changed not to test the var values

Change-Id: I3dff8739a9b79f33321d836449b082c3ce63f277
2021-01-09 11:26:24 +00:00
Daimona Eaytoy 4cc608e320 Make HookRunner parameter mandatory in VariableGenerator
Depends-On: Icae3c7cd00bd9be62a46f9e85c311e46157ccabf
Depends-On: Ie5031528593ea28f3cdc3169336aa0e4337306f7
Depends-On: I7ff2f90d890a74fa14f40535c4a567fb3124920e
Change-Id: I4d629e26e31517aa06e6215499cc3422f0fe6c72
2021-01-04 12:47:26 +01:00
Daimona Eaytoy 6081bf90c4 Introduce a VariableGeneratorFactory service
So we can use DI in all generators. Some improvements were deliberately
omitted, e.g. injecting more services and relaxing User/Title to
UserIdentity/LinkTarget, and they'll be included in a subsequent commit.

Depends-On: I1f351071ef2b0b7c80e91407a9c3bb17be293044
Depends-On: Ie71740fac35a86f8fe03023080ae8ca08671243d
Depends-On: I589a0e1c2c5891070ab82cd5adfd9cedec19e67d
Change-Id: I92ef0abd5e45b672e6f297a71b3c2c345d56f136
2021-01-03 14:17:39 +01:00
Daimona Eaytoy 1beb405bc7 Make VariableHolder param optional in VariableGenerator
So we can remove it from callers and then use DI in this class.

Change-Id: I9055f54397279870740a7ff9567635ee4f17e4d2
2021-01-03 13:25:59 +01:00
Daimona Eaytoy 6e27a9ddb3 Cleanup variables-related classes
Change-Id: I20a7fe1a40255043ed0d125dee61ea6052dda69c
2021-01-02 18:19:38 +01:00
Daimona Eaytoy 762d71c51d Create a dedicated namespace for variables-related classes
Some cleanup is left for later to keep the diff easier to read.

Change-Id: Ife445b5e47e707ab77ec867ac3b005866aa74ef2
2021-01-02 18:16:48 +01:00
Daimona Eaytoy fad9a11f7a Add a TextExtractor service
This is an important step towards removing the AbuseFilter class. Note:
proposals for the name of the new service are welcome.

Change-Id: Ib4632173f728b1bdafadef96e01645a833bfceaa
2021-01-01 18:25:32 +01:00
Daimona Eaytoy c7f06750d6 Add a LazyVariableComputer service
See task for a description of the plan. Also note that
AFComputedVariable should be renamed and its properties made private.

This commit includes some adjustments for taint-check in
AbuseFilter::buildVarDumpTable and ::revisionToString.

There's some space for improvement in the new LazyVariableComputer, but
that's left for another commit.

Bug: T261069
Change-Id: Ia44f6e079d39f44cf0122dec5ddb5513ab54f0c6
2020-12-31 14:05:52 +01:00
James D. Forrester 7109c954a2 Use User->isRegistered(), not deprecated isLoggedIn()
Bug: T270450
Change-Id: I6ebf2f8040b6ac53025b5ccf503e5e221341eb09
2020-12-17 18:37:14 -08:00
Daimona Eaytoy f41fe76df7 Kill $wgUser
Depends-On: Iadbce7501e42971901f6d9efcb2810ae42be51d8
Depends-On: I624610cb2372db200995c8d01d62b1d74efca19e
Change-Id: I51d99c30fbc0e87c038013bf5b8c27b1c735e977
2020-12-08 23:23:13 +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 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 6305746de3 Use a WikiPage object when filtering edits on a non-existing Title
Remove $title->exists() from the check, so we have the following
changes:
 - The AbuseLog will add a diff link for page creations
 - Searching the AbuseLog for impact:saved will include page creations
 - We don't have to recreate the WikiPage again in RunVariableGenerator

Also remove an old reference to "bug 31656": that comment was added in
rEABFefecf8b2441ae2f31f924ff33103f5affe5d1d62, which changed
Article::getContent() to Article::getRevision()->getRawText(). Nowadays
we don't even use Article anymore, and that conditional isn't even for
retrieving the page content, so the comment is wrong.

Add logging for when the Title object cannot exist, as this should never
happen in the context of the EditFilterMergedContent hook, and always
create a WikiPage. Some signatures were changed to require a WikiPage
object now, and every caller updated to provide it.

Bug: T263104
Bug: T62179
Depends-On: Ic238eaa529ef6bfba06b4dd03924a8e0111d8259
Change-Id: Ibf3bf4f68328ba4a5616ab8f26a8b44d27a25cd7
2020-11-18 20:13:46 +00:00
Daimona Eaytoy 6c8a29698b Add test traits for uploads and account creation
Ideally, this might live in MediaWikiIntegrationTestCase. For the
createaccount one, AuthManager should also provide a method to log the
creation, because currently we are forced to copypaste that code here.

 - Add the missing tests for 'upload' in RCVariableGenerator, and adjust
the existing ones (delete file afterwards, more tablesUsed, use the
right extension).

 - Exclude from the coverage report a couple of lines which should
theoretically be unreachable. Escalate logging to WARN level, where it's
more likely to be spotted.

 - Remove an unused method (RCVariableGenerator::newFromID). This denies
   the need to maintain and cover it. We also don't want this generator
   to act as a factory.

Overall, this change brings the coverage for RCVariableGenerator to 100%

Bug: T201193
Change-Id: I425c3d9f6800f74eb6e4eda483b90cfb3bbbcb51
2020-10-04 13:16:58 +00: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
Daimona Eaytoy 8fa9e6625a Add tests for 'upload' action
This adds some coverage for the *VariableGenerator classes. It's still
not perfect, but something to start with in sight of future
refactorings.

Bug: T201193
Change-Id: Iafa85fb8623ea278ce6e42118df72751806382c2
2020-09-28 11:53:53 +00:00
DannyS712 801e1f57e5 Inject a user into RCVariableGenerator
Needed in ::addUploadVars

Bug: T263033
Change-Id: Iedde4a39dcc3192616e36a45690a0619efeb7309
2020-09-21 16:15:21 +00:00
DannyS712 9c1868d55e Update hook calling to use new HookContainer
Bug: T254306
Change-Id: Ic5c82a367e34135bbc0f00ece5aeef4f2d92881b
2020-09-17 10:05:45 +00:00
Umherirrender 82f549bed9 Do not abuse RCDatabaseLogEntry
When using RecentChange::getQueryInfo() it should be used to instance a
RecentChange class
RCDatabaseLogEntry is only useful in context of LogFormatter

This is a breaking change for the hook variable,
but "RC entry" refers more to the RecentChange class than to the
RCDatabaseLogEntry class

Change-Id: I3af1e42594f8235be815ce38e3411c762ae01092
2020-06-04 21:28:23 +00:00
jenkins-bot 22adab7159 Merge "Stop using the Revision class" 2020-02-26 16:47:37 +00:00
jenkins-bot 8de3ecdb89 Merge "Replace usage of deprecated Page in favor of WikiPage/Article" 2020-02-26 15:49:19 +00:00
Daimona Eaytoy 518c176754 Stop using the Revision class
Change-Id: Ie257c9b1ea94dcadce59f4541d5947465262bd75
2020-02-26 15:39:12 +00:00
Daimona Eaytoy 1bac110205 Remove dependency on $wgRestrictionTypes
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
2020-02-25 23:17:54 +00:00
ArtBaltai 22925c5344 Replace usage of deprecated Page in favor of WikiPage/Article
Complete WikiPage/Article split and deprecate Page interface
Using actual WikiPage/Article contract

Bug: T239975
Change-Id: I343c3ca2e30715656950cab49c6470061c72b9a0
2020-02-26 01:03:49 +03:00
Daimona Eaytoy d9ae71f578 Add site name and language variables
In T43172 it was told that adding the site name could increase the risk of
attracting more spam, but I don't see how this variable could cause that.

Bug: T240948
Bug: T97933
Change-Id: I1d2aeabaf008ac06798b8d7e4af7d61ae1702776
2020-02-09 14:32:02 +01:00
Daimona Eaytoy 661a77f0eb Rename addStaticVars and related hook
This code was introduced with Iba59fe8d190dd338ecc8cfd682205bce33c9738b
and is unused since then. The name should highlight that those variables are not
supposed to be "static", i.e. immutable. Examples are: timestamp, spam
blacklist, site name, site language. These are not immutable, but rather
"generic", and they're known even without an ongoing action.

Also add an RC row param and update docs.

Change-Id: I402f04585e9154059fc413e527e39dcb8e6b3d7c
2020-02-09 14:29:08 +01:00
Daimona Eaytoy 3f83e57ad7 Factor out variables-related methods
This is another step needed to reduce the size of the gigantic
AbuseFilter and AbuseFilterHooks classes. It also makes many methods
non-static, for more testability.

Note, this layout is still not final. We should somehow merge the
functionality of VariableGenerator and AFComputedVariable, for which
I already have plans.

Change-Id: I366d598b69ad866496b7cb0059e0835c02e54041
2020-02-07 20:27:26 +00:00
Daimona Eaytoy 1686042a91 Move variable generators to new classes
RunVariableGenerator is for generating variables based on the current
action;
RowVariableGenerator is for RC entries;
VariableGenerator is the generic one.

This patch only moves the methods to the new classes, to keep the diff
easier to read, and facilitate conflict resolution. These classes will
then be revamped in I366d598b69ad866496b7cb0059e0835c02e54041.

Note that these classes are now namespaced.

One method, AbuseFilter::getEditVars, was renamed to
AbuseFilterVariableGenerator::generateEditVars, because it would
otherwise conflict with an incompatible method in RunVariableGenerator.

Change-Id: Iff412e5492873d4fae55402939a51609e64d55a8
2020-02-07 19:44:31 +00:00