From 3e0c30ff9238f7492194da6575fb0f81e7f26615 Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Sun, 15 Sep 2019 17:48:13 +0200 Subject: [PATCH] Allow the parsers to return extra info This is achieved by creating a new ParserStatus class. Aside from the result of parse(), it contains whether the cache was warm. This can be used to differentiate profiling data as part of T231112. Another use case is returning non-fatal warnings (T269770). Change-Id: Ifcbda861ce1a44bbe9bffba5b83cd9ef338a8dba --- includes/AbuseFilterRunner.php | 2 +- includes/Api/CheckMatch.php | 2 +- includes/Parser/AFPData.php | 6 +-- includes/Parser/AbuseFilterCachingParser.php | 2 + includes/Parser/AbuseFilterParser.php | 39 ++++++++++++----- includes/Parser/ParserStatus.php | 44 ++++++++++++++++++++ includes/View/AbuseFilterViewTestBatch.php | 2 +- 7 files changed, 80 insertions(+), 17 deletions(-) create mode 100644 includes/Parser/ParserStatus.php diff --git a/includes/AbuseFilterRunner.php b/includes/AbuseFilterRunner.php index cf4391d5d..627fa3cf8 100644 --- a/includes/AbuseFilterRunner.php +++ b/includes/AbuseFilterRunner.php @@ -417,7 +417,7 @@ class AbuseFilterRunner { $origExtraTime = AFComputedVariable::$profilingExtraTime; $this->parser->setFilter( $filterName ); - $result = $this->parser->checkConditions( $filter->getRules(), true, $filterName ); + $result = $this->parser->checkConditions( $filter->getRules(), true, $filterName )->getResult(); $actualExtra = AFComputedVariable::$profilingExtraTime - $origExtraTime; $timeTaken = 1000 * ( microtime( true ) - $startTime - $actualExtra ); diff --git a/includes/Api/CheckMatch.php b/includes/Api/CheckMatch.php index 77b955521..63bfdca5c 100644 --- a/includes/Api/CheckMatch.php +++ b/includes/Api/CheckMatch.php @@ -68,7 +68,7 @@ class CheckMatch extends ApiBase { $parser->setVariables( $vars ); $result = [ ApiResult::META_BC_BOOLS => [ 'result' ], - 'result' => $parser->checkConditions( $params['filter'] ), + 'result' => $parser->checkConditions( $params['filter'] )->getResult(), ]; $this->getResult()->addValue( diff --git a/includes/Parser/AFPData.php b/includes/Parser/AFPData.php index 9e2cb2bcc..140597c0d 100644 --- a/includes/Parser/AFPData.php +++ b/includes/Parser/AFPData.php @@ -3,7 +3,7 @@ namespace MediaWiki\Extension\AbuseFilter\Parser; use InvalidArgumentException; -use MWException; +use RuntimeException; class AFPData { // Datatypes @@ -420,7 +420,7 @@ class AFPData { /** Convert shorteners */ /** - * @throws MWException + * @throws RuntimeException * @return mixed */ public function toNative() { @@ -447,7 +447,7 @@ class AFPData { return null; default: // @codeCoverageIgnoreStart - throw new MWException( "Unknown type" ); + throw new RuntimeException( "Unknown type" ); // @codeCoverageIgnoreEnd } } diff --git a/includes/Parser/AbuseFilterCachingParser.php b/includes/Parser/AbuseFilterCachingParser.php index 5c82cb500..62e0b90e4 100644 --- a/includes/Parser/AbuseFilterCachingParser.php +++ b/includes/Parser/AbuseFilterCachingParser.php @@ -78,6 +78,7 @@ class AbuseFilterCachingParser extends AbuseFilterParser { * @return AFPSyntaxTree */ private function getTree( $code ) : AFPSyntaxTree { + $this->fromCache = true; return $this->cache->getWithSetCallback( $this->cache->makeGlobalKey( __CLASS__, @@ -86,6 +87,7 @@ class AbuseFilterCachingParser extends AbuseFilterParser { ), BagOStuff::TTL_DAY, function () use ( $code ) { + $this->fromCache = false; $parser = new AFPTreeParser( $this->cache, $this->logger, $this->statsd, $this->keywordsManager ); $parser->setFilter( $this->mFilter ); return $parser->parse( $code ); diff --git a/includes/Parser/AbuseFilterParser.php b/includes/Parser/AbuseFilterParser.php index f645700ad..85a48f974 100644 --- a/includes/Parser/AbuseFilterParser.php +++ b/includes/Parser/AbuseFilterParser.php @@ -9,7 +9,6 @@ use IBufferingStatsdDataFactory; use InvalidArgumentException; use Language; use MediaWiki\Extension\AbuseFilter\KeywordsManager; -use MWException; use NullStatsdDataFactory; use Psr\Log\LoggerInterface; use Sanitizer; @@ -62,6 +61,10 @@ class AbuseFilterParser extends AFPTransitionBase { * @var BagOStuff Used to cache the AST (in CachingParser) and the tokens */ protected $cache; + /** + * @var bool Whether the AST was retrieved from cache (CachingParser only) + */ + protected $fromCache = false; /** * @var LoggerInterface Used for debugging */ @@ -184,7 +187,7 @@ class AbuseFilterParser extends AFPTransitionBase { /** * @param int $val The amount to increase the conditions count of. - * @throws MWException + * @throws AFPException */ protected function raiseCondCount( $val = 1 ) { global $wgAbuseFilterConditionLimit; @@ -192,7 +195,7 @@ class AbuseFilterParser extends AFPTransitionBase { $this->mCondCount += $val; if ( $this->condLimitEnabled && $this->mCondCount > $wgAbuseFilterConditionLimit ) { - throw new MWException( 'Condition limit reached.' ); + throw new AFPException( 'Condition limit reached.' ); } } @@ -267,15 +270,13 @@ class AbuseFilterParser extends AFPTransitionBase { * @param string $conds * @param bool $ignoreError * @param string|null $filter The ID of the filter being parsed - * @return bool - * @throws Exception + * @return ParserStatus + * @throws AFPException */ - public function checkConditions( string $conds, $ignoreError = true, $filter = null ) : bool { - try { - $result = $this->parse( $conds ); - } catch ( Exception $excep ) { - $result = false; - + public function checkConditions( string $conds, $ignoreError = true, $filter = null ) : ParserStatus { + $result = $this->parseDetailed( $conds ); + $excep = $result->getException(); + if ( $excep !== null ) { if ( $excep instanceof AFPUserVisibleException ) { $msg = $excep->getMessageForLogs(); $excep->setLocalizedMessage(); @@ -391,6 +392,22 @@ class AbuseFilterParser extends AFPTransitionBase { return $this->intEval( $code )->toBool(); } + /** + * Like self::parse(), but returns an object with additional info + * @param string $code + * @return ParserStatus + */ + public function parseDetailed( string $code ) : ParserStatus { + $excep = null; + try { + $res = $this->parse( $code ); + } catch ( AFPException $excep ) { + $res = false; + } finally { + return new ParserStatus( $res, $this->fromCache, $excep ); + } + } + /** * @param string $filter * @return string diff --git a/includes/Parser/ParserStatus.php b/includes/Parser/ParserStatus.php new file mode 100644 index 000000000..dd610ba66 --- /dev/null +++ b/includes/Parser/ParserStatus.php @@ -0,0 +1,44 @@ +result = $result; + $this->warmCache = $warmCache; + $this->excep = $excep; + } + + /** + * @return bool + */ + public function getResult() : bool { + return $this->result; + } + + /** + * @return bool + */ + public function getWarmCache() : bool { + return $this->warmCache; + } + + /** + * @return AFPException|null + */ + public function getException() : ?AFPException { + return $this->excep; + } +} diff --git a/includes/View/AbuseFilterViewTestBatch.php b/includes/View/AbuseFilterViewTestBatch.php index e682c292d..c91f7f9a9 100644 --- a/includes/View/AbuseFilterViewTestBatch.php +++ b/includes/View/AbuseFilterViewTestBatch.php @@ -278,7 +278,7 @@ class AbuseFilterViewTestBatch extends AbuseFilterView { } $parser->setVariables( $vars ); - $result = $parser->checkConditions( $this->testPattern ); + $result = $parser->checkConditions( $this->testPattern )->getResult(); if ( $result || $this->mShowNegative ) { // Stash result in RC item