jenkins-bot has submitted this change and it was merged.
Change subject: Cache AbuseFilter::checkAllFilters during edit stashing
......................................................................
Cache AbuseFilter::checkAllFilters during edit stashing
This should improve page save times when manual edit summaries are
not used (and in a few cases, where they are).
Also fix a few annoying IDEA errors with block comments.
Bug: T137698
Depends-On: I2e407a3ac8b74e77bf88b1e34c1519f4dea63b80
Change-Id: I972e9147a5e52a941f478eaf1e96dc3ef8bdfe94
---
M AbuseFilter.class.php
M AbuseFilter.hooks.php
2 files changed, 136 insertions(+), 34 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/AbuseFilter.class.php b/AbuseFilter.class.php
index 69715c1..032706e 100644
--- a/AbuseFilter.class.php
+++ b/AbuseFilter.class.php
@@ -1,5 +1,8 @@
<?php
+use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
+
/**
* This class contains most of the business logic of AbuseFilter. It consists
of mostly
* static functions that handle activities such as parsing edits, applying
filters,
@@ -440,7 +443,7 @@
/**
* Returns an associative array of filters which were tripped
*
- * @param $vars array
+ * @param $vars AbuseFilterVariableHolder
* @param string $group The filter's group (as defined in
$wgAbuseFilterValidGroups)
*
* @return array
@@ -730,7 +733,7 @@
*
* @param $filters array
* @param $title Title
- * @param $vars array
+ * @param $vars AbuseFilterVariableHolder
* @return Status returns the operation's status. $status->isOK() will
return true if
* there were no actions taken, false otherwise.
$status->getValue() will return
* an array listing the actions taken. $status-getErrors(),
etc, will provide
@@ -865,9 +868,12 @@
* @param $title Title
* @param string $group The filter's group (as defined in
$wgAbuseFilterValidGroups)
* @param User $user The user performing the action; defaults to $wgUser
+ * @param string $mode Use 'execute' to run filters and log or 'stash'
to only cache matches
* @return Status
*/
- public static function filterAction( $vars, $title, $group = 'default',
$user = null ) {
+ public static function filterAction(
+ $vars, $title, $group = 'default', $user = null, $mode =
'execute'
+ ) {
global $wgUser, $wgTitle, $wgRequest;
$context = RequestContext::getMain();
@@ -884,24 +890,44 @@
// Add vars from extensions
Hooks::run( 'AbuseFilter-filterAction', array( &$vars, $title )
);
-
- // Set context
$vars->setVar( 'context', 'filter' );
$vars->setVar( 'timestamp', time() );
+ // Get the stash key based on the relevant "input" variables
+ $cache = ObjectCache::getLocalClusterInstance();
+ $stashKey = self::getStashKey( $cache, $vars, $group );
- $dbr = wfGetDB( DB_SLAVE );
+ $filter_matched = false;
+ if ( $mode === 'execute' ) {
+ // Check the filter edit stash results first
+ $filter_matched = $cache->get( $stashKey );
+ }
- $filter_matched = self::checkAllFilters( $vars, $group );
+ $logger = LoggerFactory::getInstance( 'StashEdit' );
+ $statsd =
MediaWikiServices::getInstance()->getStatsdDataFactory();
+ if ( is_array( $filter_matched ) ) {
+ $logger->info( __METHOD__ . ": cache hit for '$title'
(key $stashKey)." );
+ $statsd->increment( 'abusefilter.check-stash.hit' );
+ } else {
+ $filter_matched = self::checkAllFilters( $vars, $group
);
+ $logger->info( __METHOD__ . ": cache miss for '$title'
(key $stashKey)." );
+ $statsd->increment( 'abusefilter.check-stash.miss' );
+ }
+
+ if ( $mode === 'stash' ) {
+ // Save the filter stash result and do nothing further
+ $cache->set( $stashKey, $filter_matched,
$cache::TTL_MINUTE );
+ $logger->info( __METHOD__ . ": cache store for '$title'
(key $stashKey)." );
+ $statsd->increment( 'abusefilter.check-stash.store' );
+
+ return Status::newGood();
+ }
$matched_filters = array_keys( array_filter( $filter_matched )
);
if ( count( $matched_filters ) == 0 ) {
$status = Status::newGood();
} else {
- wfProfileIn( __METHOD__ . '-block' );
-
- $status = self::executeFilterActions(
- $matched_filters, $title, $vars );
+ $status = self::executeFilterActions( $matched_filters,
$title, $vars );
$actions_taken = $status->value; // getValue() was
introduced only in 1.20
@@ -915,7 +941,7 @@
$log_template = array(
'afl_user' => $user->getId(),
'afl_user_text' => $user->getName(),
- 'afl_timestamp' => $dbr->timestamp(
wfTimestampNow() ),
+ 'afl_timestamp' => wfGetDB( DB_SLAVE
)->timestamp( wfTimestampNow() ),
'afl_namespace' => $title->getNamespace(),
'afl_title' => $title->getDBkey(),
'afl_ip' => $wgRequest->getIP()
@@ -927,8 +953,6 @@
}
self::addLogEntries( $actions_taken, $log_template,
$action, $vars, $group );
-
- wfProfileOut( __METHOD__ . '-block' );
}
// Bug 53498: If we screwed around with $wgTitle, reset it so
the title
@@ -946,6 +970,33 @@
}
/**
+ * @param BagOStuff $cache
+ * @param $vars AbuseFilterVariableHolder
+ * @param string $group The filter's group (as defined in
$wgAbuseFilterValidGroups)
+ *
+ * @return string
+ */
+ private static function getStashKey(
+ BagOStuff $cache, AbuseFilterVariableHolder $vars, $group
+ ) {
+ $inputVars = $vars->exportAllVars();
+ // Exclude noisy fields that have superficial changes
+ unset( $inputVars['old_html'] );
+ unset( $inputVars['new_html'] );
+ unset( $inputVars['user_age'] );
+ unset( $inputVars['timestamp'] );
+ ksort( $inputVars );
+ $hash = md5( serialize( $inputVars ) );
+
+ return ObjectCache::getLocalClusterInstance()->makeKey(
+ 'abusefilter',
+ 'check-stash',
+ $group,
+ $hash
+ );
+ }
+
+ /**
* @param $actions_taken
* @param $log_template
* @param $action
diff --git a/AbuseFilter.hooks.php b/AbuseFilter.hooks.php
index 13dd8ba..5b333f5 100644
--- a/AbuseFilter.hooks.php
+++ b/AbuseFilter.hooks.php
@@ -119,8 +119,6 @@
*/
public static function filterEdit( IContextSource $context, $content,
$text,
Status $status, $summary, $minoredit ) {
- // Load vars
- $vars = new AbuseFilterVariableHolder();
$title = $context->getTitle();
@@ -165,24 +163,12 @@
$page = null;
}
- $vars->addHolders(
- AbuseFilter::generateUserVars( $user ),
- AbuseFilter::generateTitleVars( $title, 'ARTICLE' )
+ // Load vars for filters to check
+ $vars = self::newVariableHolderForEdit(
+ $user, $title, $page, $summary, $minoredit, $oldtext,
$text
);
- $vars->setVar( 'action', 'edit' );
- $vars->setVar( 'summary', $summary );
- $vars->setVar( 'minor_edit', $minoredit );
-
- $vars->setVar( 'old_wikitext', $oldtext );
- $vars->setVar( 'new_wikitext', $text );
-
- // TODO: set old_content and new_content vars, use them
-
- $vars->addHolders( AbuseFilter::getEditVars( $title, $page ) );
-
$filter_result = AbuseFilter::filterAction( $vars, $title );
-
if ( !$filter_result->isOK() ) {
$status->merge( $filter_result );
@@ -193,6 +179,36 @@
self::$last_edit_page = $page;
return true;
+ }
+
+ /**
+ * @param User $user
+ * @param Title $title
+ * @param WikiPage|null $page
+ * @param string $summary
+ * @param bool $minoredit
+ * @param string $oldtext
+ * @param string $text
+ * @return AbuseFilterVariableHolder
+ * @throws MWException
+ */
+ private static function newVariableHolderForEdit(
+ User $user, Title $title, $page, $summary, $minoredit,
$oldtext, $text
+ ) {
+ $vars = new AbuseFilterVariableHolder();
+ $vars->addHolders(
+ AbuseFilter::generateUserVars( $user ),
+ AbuseFilter::generateTitleVars( $title, 'ARTICLE' )
+ );
+ $vars->setVar( 'action', 'edit' );
+ $vars->setVar( 'summary', $summary );
+ $vars->setVar( 'minor_edit', $minoredit );
+ $vars->setVar( 'old_wikitext', $oldtext );
+ $vars->setVar( 'new_wikitext', $text );
+ // TODO: set old_content and new_content vars, use them
+ $vars->addHolders( AbuseFilter::getEditVars( $title, $page ) );
+
+ return $vars;
}
/**
@@ -226,6 +242,18 @@
);
}
+ /**
+ * @param Article|WikiPage $article
+ * @param User $user
+ * @param string $text
+ * @param string $summary
+ * @param bool $minoredit
+ * @param bool $watchthis
+ * @param string $sectionanchor
+ * @param integer $flags
+ * @param Revision $revision
+ * @return bool
+ */
public static function onArticleSaveComplete(
&$article, &$user, $text, $summary, $minoredit, $watchthis,
$sectionanchor,
&$flags, $revision
@@ -236,6 +264,7 @@
return true;
}
+ /** @var AbuseFilterVariableHolder $vars */
$vars = self::$successful_action_vars;
if ( $vars->getVar( 'article_prefixedtext' )->toString() !==
@@ -372,7 +401,7 @@
* @param $user User
* @param $reason string
* @param $error
- * @param $status
+ * @param Status $status
* @return bool
*/
public static function onArticleDelete( &$article, &$user, &$reason,
&$error, &$status ) {
@@ -806,11 +835,33 @@
* @param WikiPage $page
* @param Content $content
* @param ParserOutput $output
+ * @param string $summary
+ * @param User $user
*/
public static function onParserOutputStashForEdit(
- WikiPage $page, Content $content, ParserOutput $output
+ WikiPage $page, Content $content, ParserOutput $output,
$summary = '', $user = null
) {
- AFComputedVariable::getLastPageAuthors( $page->getTitle() );
+ $revision = $page->getRevision();
+ if ( !$revision ) {
+ return;
+ }
+
+ $text = AbuseFilter::contentToString( $content );
+ $oldcontent = $revision->getContent( Revision::RAW );
+ $oldtext = AbuseFilter::contentToString( $oldcontent );
+ $user = $user ?: RequestContext::getMain()->getUser();
+
+ // Cache any resulting filter matches...
+ // Case A: if the edit turns out to be non-minor
+ $vars = self::newVariableHolderForEdit(
+ $user, $page->getTitle(), $page, $summary, false,
$oldtext, $text
+ );
+ AbuseFilter::filterAction( $vars, $page->getTitle(), 'default',
$user, 'stash' );
+ // Case B: if the edit turns out to be minor
+ $vars = self::newVariableHolderForEdit(
+ $user, $page->getTitle(), $page, $summary, true,
$oldtext, $text
+ );
+ AbuseFilter::filterAction( $vars, $page->getTitle(), 'default',
$user, 'stash' );
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/294051
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I972e9147a5e52a941f478eaf1e96dc3ef8bdfe94
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/AbuseFilter
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: Se4598 <se4...@gmx.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
