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
This commit is contained in:
Aaron Schulz 2016-06-13 04:53:50 -07:00
parent 76ef672c2f
commit e91939fb3f
2 changed files with 136 additions and 34 deletions

View file

@ -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 @@ class AbuseFilter {
/**
* 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 @@ class AbuseFilter {
*
* @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 @@ class AbuseFilter {
* @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 @@ class AbuseFilter {
// 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 @@ class AbuseFilter {
$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 @@ class AbuseFilter {
}
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
@ -945,6 +969,33 @@ class AbuseFilter {
return $status;
}
/**
* @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

View file

@ -119,8 +119,6 @@ class AbuseFilterHooks {
*/
public static function filterEdit( IContextSource $context, $content, $text,
Status $status, $summary, $minoredit ) {
// Load vars
$vars = new AbuseFilterVariableHolder();
$title = $context->getTitle();
@ -165,24 +163,12 @@ class AbuseFilterHooks {
$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 );
@ -195,6 +181,36 @@ class AbuseFilterHooks {
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;
}
/**
* Common implementation for the APIEditBeforeSave and EditFilterMergedContent hooks.
*
@ -226,6 +242,18 @@ class AbuseFilterHooks {
);
}
/**
* @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 @@ class AbuseFilterHooks {
return true;
}
/** @var AbuseFilterVariableHolder $vars */
$vars = self::$successful_action_vars;
if ( $vars->getVar( 'article_prefixedtext' )->toString() !==
@ -372,7 +401,7 @@ class AbuseFilterHooks {
* @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 @@ class AbuseFilterHooks {
* @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' );
}
/**