diff --git a/extension.json b/extension.json index 10774dec8..6cea6d56d 100644 --- a/extension.json +++ b/extension.json @@ -171,7 +171,7 @@ "ApiAbuseLogPrivateDetails": "includes/api/ApiAbuseLogPrivateDetails.php", "NormalizeThrottleParameters": "maintenance/normalizeThrottleParameters.php", "AbuseFilterConsequencesTest": "tests/phpunit/AbuseFilterConsequencesTest.php", - "AbuseFilterParserTestCase": "tests/phpunit/AbuseFilterParserTestCase.php", + "AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php", "FixOldLogEntries": "maintenance/fixOldLogEntries.php" }, "ResourceModules": { @@ -293,7 +293,7 @@ }, "AbuseFilterParserClass": { "value": "AbuseFilterParser", - "description": "Class of the parser to use" + "description": "Class of the parser to use. The only possible values are 'AbuseFilterParser' and 'AbuseFilterCachingParser' (experimental). The code should only use the wrapper AbuseFilter::getDefaultParser." }, "AbuseFilterEmergencyDisableThreshold": { "value": { diff --git a/includes/AbuseFilter.php b/includes/AbuseFilter.php index bb37aec3c..c6fdf9842 100644 --- a/includes/AbuseFilter.php +++ b/includes/AbuseFilter.php @@ -425,12 +425,7 @@ class AbuseFilter { * and character position of the syntax error */ public static function checkSyntax( $filter ) { - global $wgAbuseFilterParserClass; - - /** @var $parser AbuseFilterParser */ - $parser = new $wgAbuseFilterParserClass; - - return $parser->checkSyntax( $filter ); + return self::getDefaultParser()->checkSyntax( $filter ); } /** @@ -438,8 +433,6 @@ class AbuseFilter { * @return string */ public static function evaluateExpression( $expr ) { - global $wgAbuseFilterParserClass; - if ( self::checkSyntax( $expr ) !== true ) { return 'BADSYNTAX'; } @@ -447,8 +440,7 @@ class AbuseFilter { // Static vars are the only ones available $vars = self::generateStaticVars(); $vars->setVar( 'timestamp', wfTimestamp( TS_UNIX ) ); - /** @var $parser AbuseFilterParser */ - $parser = new $wgAbuseFilterParserClass( $vars ); + $parser = self::getDefaultParser( $vars ); return $parser->evaluateExpression( $expr ); } @@ -469,9 +461,16 @@ class AbuseFilter { } catch ( Exception $excep ) { $result = false; + if ( $excep instanceof AFPUserVisibleException ) { + $msg = $excep->getMessageForLogs(); + $excep->setLocalizedMessage(); + } else { + $msg = $excep->getMessage(); + } + $logger = LoggerFactory::getInstance( 'AbuseFilter' ); $extraInfo = $filter !== null ? " for filter $filter" : ''; - $logger->warning( "AbuseFilter parser error$extraInfo: " . $excep->getMessage() ); + $logger->warning( "AbuseFilter parser error$extraInfo: $msg" ); if ( !$ignoreError ) { throw $excep; @@ -497,10 +496,7 @@ class AbuseFilter { $group = 'default', $mode = 'execute' ) { - global $wgAbuseFilterParserClass; - - /** @var $parser AbuseFilterParser */ - $parser = new $wgAbuseFilterParserClass( $vars ); + $parser = self::getDefaultParser( $vars ); $user = RequestContext::getMain()->getUser(); $runner = new AbuseFilterRunner( $user, $title, $vars, $group ); @@ -2237,4 +2233,31 @@ class AbuseFilter { public static function canViewPrivate( User $user ) { return $user->isAllowedAny( 'abusefilter-modify', 'abusefilter-view-private' ); } + + /** + * Get a parser instance using default options. This should mostly be intended as a wrapper + * around $wgAbuseFilterParserClass and for choosing the right type of cache. It also has the + * benefit of typehinting the return value, thus making IDEs and static analysis tools happier. + * + * @param AbuseFilterVariableHolder|null $vars + * @return AbuseFilterParser + * @throws InvalidArgumentException if $wgAbuseFilterParserClass is not valid + */ + public static function getDefaultParser( + AbuseFilterVariableHolder $vars = null + ) : AbuseFilterParser { + global $wgAbuseFilterParserClass; + + $allowedValues = [ AbuseFilterParser::class, AbuseFilterCachingParser::class ]; + if ( !in_array( $wgAbuseFilterParserClass, $allowedValues ) ) { + throw new InvalidArgumentException( + "Invalid value $wgAbuseFilterParserClass for \$wgAbuseFilterParserClass." + ); + } + + $contLang = MediaWikiServices::getInstance()->getContentLanguage(); + $cache = ObjectCache::getLocalServerInstance( 'hash' ); + $logger = LoggerFactory::getInstance( 'AbuseFilter' ); + return new $wgAbuseFilterParserClass( $contLang, $cache, $logger, $vars ); + } } diff --git a/includes/AbuseFilterRunner.php b/includes/AbuseFilterRunner.php index 9ead4e9f3..085acff17 100644 --- a/includes/AbuseFilterRunner.php +++ b/includes/AbuseFilterRunner.php @@ -93,11 +93,11 @@ class AbuseFilterRunner { } /** + * Shortcut method, so that it can be overridden in mocks. * @return AbuseFilterParser */ protected function getParser() : AbuseFilterParser { - global $wgAbuseFilterParserClass; - return new $wgAbuseFilterParserClass( $this->vars ); + return AbuseFilter::getDefaultParser( $this->vars ); } /** diff --git a/includes/Views/AbuseFilterViewTestBatch.php b/includes/Views/AbuseFilterViewTestBatch.php index 79909660d..da985b2f4 100644 --- a/includes/Views/AbuseFilterViewTestBatch.php +++ b/includes/Views/AbuseFilterViewTestBatch.php @@ -215,9 +215,7 @@ class AbuseFilterViewTestBatch extends AbuseFilterView { continue; } - $parserClass = $this->getConfig()->get( 'AbuseFilterParserClass' ); - /** @var AbuseFilterParser $parser */ - $parser = new $parserClass( $vars ); + $parser = AbuseFilter::getDefaultParser( $vars ); $parser->toggleConditionLimit( false ); $result = AbuseFilter::checkConditions( $this->mFilter, $parser ); diff --git a/includes/api/ApiAbuseFilterCheckMatch.php b/includes/api/ApiAbuseFilterCheckMatch.php index a6bd53e69..1b777eb0b 100644 --- a/includes/api/ApiAbuseFilterCheckMatch.php +++ b/includes/api/ApiAbuseFilterCheckMatch.php @@ -54,9 +54,7 @@ class ApiAbuseFilterCheckMatch extends ApiBase { $this->dieWithError( 'apierror-abusefilter-badsyntax', 'badsyntax' ); } - $parserClass = $this->getConfig()->get( 'AbuseFilterParserClass' ); - /** @var AbuseFilterParser $parser */ - $parser = new $parserClass( $vars ); + $parser = AbuseFilter::getDefaultParser( $vars ); $result = [ ApiResult::META_BC_BOOLS => [ 'result' ], 'result' => AbuseFilter::checkConditions( $params['filter'], $parser ), diff --git a/includes/parser/AFPTreeParser.php b/includes/parser/AFPTreeParser.php index f32ac3f1b..11567ebeb 100644 --- a/includes/parser/AFPTreeParser.php +++ b/includes/parser/AFPTreeParser.php @@ -7,7 +7,7 @@ * @file */ -use MediaWiki\Logger\LoggerFactory; +use Psr\Log\LoggerInterface; /** * A parser that transforms the text of the filter into a parse tree. @@ -38,9 +38,22 @@ class AFPTreeParser { private $variablesNames; /** - * Create a new instance + * @var BagOStuff Used to cache tokens */ - public function __construct() { + protected $cache; + + /** + * @var LoggerInterface Used for debugging + */ + protected $logger; + + /** + * @param BagOStuff $cache + * @param LoggerInterface $logger Used for debugging + */ + public function __construct( BagOStuff $cache, LoggerInterface $logger ) { + $this->cache = $cache; + $this->logger = $logger; $this->resetState(); } @@ -107,7 +120,8 @@ class AFPTreeParser { * @return AFPSyntaxTree */ public function parse( $code ) : AFPSyntaxTree { - $this->mTokens = AbuseFilterTokenizer::getTokens( $code ); + $tokenizer = new AbuseFilterTokenizer( $this->cache ); + $this->mTokens = $tokenizer->getTokens( $code ); $this->mPos = 0; return $this->buildSyntaxTree(); @@ -703,7 +717,6 @@ class AFPTreeParser { * should be avoided when merging the parsers. */ protected function checkArgCount( $args, $func ) { - $logger = LoggerFactory::getInstance( 'AbuseFilter' ); if ( !array_key_exists( $func, AbuseFilterParser::$funcArgCount ) ) { throw new InvalidArgumentException( "$func is not a valid function." ); } @@ -715,7 +728,7 @@ class AFPTreeParser { [ $func, $min, count( $args ) ] ); } elseif ( count( $args ) > $max ) { - $logger->warning( + $this->logger->warning( "Too many params to $func for filter: " . ( $this->mFilter ?? 'unavailable' ) ); /* diff --git a/includes/parser/AFPUserVisibleException.php b/includes/parser/AFPUserVisibleException.php index 0f1af235b..fc88b6c72 100644 --- a/includes/parser/AFPUserVisibleException.php +++ b/includes/parser/AFPUserVisibleException.php @@ -20,9 +20,23 @@ class AFPUserVisibleException extends AFPException { $this->mPosition = $position; $this->mParams = $params; - // Exception message text for logs should be in English. - $msg = $this->getMessageObj()->inLanguage( 'en' )->useDatabase( false )->text(); - parent::__construct( $msg ); + parent::__construct( $exception_id ); + } + + /** + * Change the message of the exception to a localized version + */ + public function setLocalizedMessage() { + $this->message = $this->getMessageObj()->text(); + } + + /** + * Returns the error message in English for use in logs + * + * @return string + */ + public function getMessageForLogs() { + return $this->getMessageObj()->inLanguage( 'en' )->useDatabase( false )->text(); } /** diff --git a/includes/parser/AbuseFilterCachingParser.php b/includes/parser/AbuseFilterCachingParser.php index 682ffe90e..de8413108 100644 --- a/includes/parser/AbuseFilterCachingParser.php +++ b/includes/parser/AbuseFilterCachingParser.php @@ -67,20 +67,15 @@ class AbuseFilterCachingParser extends AbuseFilterParser { * @return AFPSyntaxTree */ private function getTree( $code ) : AFPSyntaxTree { - static $cache = null; - if ( !$cache ) { - $cache = ObjectCache::getLocalServerInstance( 'hash' ); - } - - return $cache->getWithSetCallback( - $cache->makeGlobalKey( + return $this->cache->getWithSetCallback( + $this->cache->makeGlobalKey( __CLASS__, self::getCacheVersion(), hash( 'sha256', $code ) ), - $cache::TTL_DAY, + BagOStuff::TTL_DAY, function () use ( $code ) { - $parser = new AFPTreeParser(); + $parser = new AFPTreeParser( $this->cache, $this->logger ); $parser->setFilter( $this->mFilter ); return $parser->parse( $code ); } diff --git a/includes/parser/AbuseFilterParser.php b/includes/parser/AbuseFilterParser.php index c55cb8dbf..ef3889c58 100644 --- a/includes/parser/AbuseFilterParser.php +++ b/includes/parser/AbuseFilterParser.php @@ -1,8 +1,7 @@ 'funcLc', 'ucase' => 'funcUc', @@ -144,10 +156,21 @@ class AbuseFilterParser { /** * Create a new instance * + * @param Language $contLang Content language, used for language-dependent function + * @param BagOStuff $cache Used to cache the AST (in CachingParser) and the tokens + * @param LoggerInterface $logger Used for debugging * @param AbuseFilterVariableHolder|null $vars */ - public function __construct( AbuseFilterVariableHolder $vars = null ) { + public function __construct( + Language $contLang, + BagOStuff $cache, + LoggerInterface $logger, + AbuseFilterVariableHolder $vars = null + ) { $this->resetState(); + $this->contLang = $contLang; + $this->cache = $cache; + $this->logger = $logger; if ( $vars ) { $this->mVariables = $vars; } @@ -160,6 +183,20 @@ class AbuseFilterParser { $this->mFilter = $filter; } + /** + * @param BagOStuff $cache + */ + public function setCache( BagOStuff $cache ) { + $this->cache = $cache; + } + + /** + * @param LoggerInterface $logger + */ + public function setLogger( LoggerInterface $logger ) { + $this->logger = $logger; + } + /** * @return int */ @@ -355,7 +392,8 @@ class AbuseFilterParser { */ public function intEval( $code ) { // Reset all class members to their default value - $this->mTokens = AbuseFilterTokenizer::getTokens( $code ); + $tokenizer = new AbuseFilterTokenizer( $this->cache ); + $this->mTokens = $tokenizer->getTokens( $code ); $this->mPos = 0; $this->mShortCircuit = false; @@ -1101,8 +1139,7 @@ class AbuseFilterParser { $deprecatedVars = AbuseFilter::getDeprecatedVariables(); if ( array_key_exists( $var, $deprecatedVars ) ) { - $logger = LoggerFactory::getInstance( 'AbuseFilter' ); - $logger->debug( "AbuseFilter: deprecated variable $var used." ); + $this->logger->debug( "AbuseFilter: deprecated variable $var used." ); $var = $deprecatedVars[$var]; } if ( !$this->varExists( $var ) ) { @@ -1159,7 +1196,6 @@ class AbuseFilterParser { * @throws AFPUserVisibleException */ protected function checkArgCount( $args, $func ) { - $logger = LoggerFactory::getInstance( 'AbuseFilter' ); if ( !array_key_exists( $func, self::$funcArgCount ) ) { throw new InvalidArgumentException( "$func is not a valid function." ); } @@ -1171,7 +1207,7 @@ class AbuseFilterParser { [ $func, $min, count( $args ) ] ); } elseif ( count( $args ) > $max ) { - $logger->warning( + $this->logger->warning( "Too many params to $func for filter: " . ( $this->mFilter ?? 'unavailable' ) ); /* @@ -1238,10 +1274,9 @@ class AbuseFilterParser { * @return AFPData */ protected function funcLc( $args ) { - $contLang = MediaWikiServices::getInstance()->getContentLanguage(); $s = $args[0]->toString(); - return new AFPData( AFPData::DSTRING, $contLang->lc( $s ) ); + return new AFPData( AFPData::DSTRING, $this->contLang->lc( $s ) ); } /** @@ -1249,10 +1284,9 @@ class AbuseFilterParser { * @return AFPData */ protected function funcUc( $args ) { - $contLang = MediaWikiServices::getInstance()->getContentLanguage(); $s = $args[0]->toString(); - return new AFPData( AFPData::DSTRING, $contLang->uc( $s ) ); + return new AFPData( AFPData::DSTRING, $this->contLang->uc( $s ) ); } /** @@ -1862,8 +1896,7 @@ class AbuseFilterParser { * @param string $fname Method where the empty operand is found */ protected function logEmptyOperand( $type, $fname ) { - $logger = LoggerFactory::getInstance( 'AbuseFilter' ); - $logger->info( + $this->logger->info( "Empty operand of type {type} at method {fname}. Filter: {filter}", [ 'type' => $type, diff --git a/includes/parser/AbuseFilterTokenizer.php b/includes/parser/AbuseFilterTokenizer.php index 7fa773ffe..a0e615d85 100644 --- a/includes/parser/AbuseFilterTokenizer.php +++ b/includes/parser/AbuseFilterTokenizer.php @@ -66,16 +66,27 @@ class AbuseFilterTokenizer { 'rlike', 'irlike', 'regex', 'if', 'then', 'else', 'end', ]; + /** + * @var BagOStuff + */ + private $cache; + + /** + * @param BagOStuff $cache + */ + public function __construct( BagOStuff $cache ) { + $this->cache = $cache; + } + /** * Get a cache key used to store the tokenized code * - * @param BagOStuff $cache * @param string $code Not yet tokenized * @return string * @internal */ - public static function getCacheKey( BagOStuff $cache, $code ) { - return $cache->makeGlobalKey( __CLASS__, self::CACHE_VERSION, crc32( $code ) ); + public function getCacheKey( $code ) { + return $this->cache->makeGlobalKey( __CLASS__, self::CACHE_VERSION, crc32( $code ) ); } /** @@ -84,12 +95,10 @@ class AbuseFilterTokenizer { * @param string $code * @return array[] */ - public static function getTokens( $code ) { - $cache = ObjectCache::getLocalServerInstance( 'hash' ); - - $tokens = $cache->getWithSetCallback( - self::getCacheKey( $cache, $code ), - $cache::TTL_DAY, + public function getTokens( $code ) { + $tokens = $this->cache->getWithSetCallback( + $this->getCacheKey( $code ), + BagOStuff::TTL_DAY, function () use ( $code ) { return self::tokenize( $code ); } @@ -122,7 +131,7 @@ class AbuseFilterTokenizer { * @throws AFPException * @throws AFPUserVisibleException */ - protected static function nextToken( $code, &$offset ) { + private static function nextToken( $code, &$offset ) { $matches = []; $start = $offset; @@ -219,7 +228,7 @@ class AbuseFilterTokenizer { * @throws AFPException * @throws AFPUserVisibleException */ - protected static function readStringLiteral( $code, &$offset, $start ) { + private static function readStringLiteral( $code, &$offset, $start ) { $type = $code[$offset]; $offset++; $length = strlen( $code ); diff --git a/tests/parserTests/ccnorm-contains-all.t b/tests/parserTestsEquivset/ccnorm-contains-all.t similarity index 100% rename from tests/parserTests/ccnorm-contains-all.t rename to tests/parserTestsEquivset/ccnorm-contains-all.t diff --git a/tests/parserTests/ccnorm-contains-any.t b/tests/parserTestsEquivset/ccnorm-contains-any.t similarity index 100% rename from tests/parserTests/ccnorm-contains-any.t rename to tests/parserTestsEquivset/ccnorm-contains-any.t diff --git a/tests/parserTests/ccnorm.t b/tests/parserTestsEquivset/ccnorm.t similarity index 100% rename from tests/parserTests/ccnorm.t rename to tests/parserTestsEquivset/ccnorm.t diff --git a/tests/parserTests/mwexamples-functions.t b/tests/parserTestsEquivset/mwexamples-functions.t similarity index 100% rename from tests/parserTests/mwexamples-functions.t rename to tests/parserTestsEquivset/mwexamples-functions.t diff --git a/tests/parserTests/norm.t b/tests/parserTestsEquivset/norm.t similarity index 100% rename from tests/parserTests/norm.t rename to tests/parserTestsEquivset/norm.t diff --git a/tests/phpunit/AbuseFilterParserEquivsetTest.php b/tests/phpunit/AbuseFilterParserEquivsetTest.php new file mode 100644 index 000000000..cab20bc4c --- /dev/null +++ b/tests/phpunit/AbuseFilterParserEquivsetTest.php @@ -0,0 +1,120 @@ +toggleConditionLimit( false ); + $cachingParser = new AbuseFilterCachingParser( $contLang, $cache, $logger ); + $cachingParser->toggleConditionLimit( false ); + $parsers = [ $parser, $cachingParser ]; + } else { + // Reset so that already executed tests don't influence new ones + $parsers[0]->resetState(); + $parsers[0]->clearFuncCache(); + $parsers[1]->resetState(); + $parsers[1]->clearFuncCache(); + } + return $parsers; + } + + /** + * @param string $rule The rule to parse + * @dataProvider provideGenericTests + */ + public function testGeneric( $rule ) { + if ( !class_exists( 'Wikimedia\Equivset\Equivset' ) ) { + $this->markTestSkipped( 'Equivset is not installed' ); + } + foreach ( $this->getParsers() as $parser ) { + $this->assertTrue( $parser->parse( $rule ), 'Parser used: ' . get_class( $parser ) ); + } + } + + /** + * @return Generator|array + */ + public function provideGenericTests() { + $testPath = __DIR__ . "/../parserTestsEquivset"; + $testFiles = glob( $testPath . "/*.t" ); + + foreach ( $testFiles as $testFile ) { + $testName = basename( substr( $testFile, 0, -2 ) ); + $rule = trim( file_get_contents( $testFile ) ); + + yield $testName => [ $rule ]; + } + } + + /** + * @param string $func + * @see AbuseFilterParserTest::testVariadicFuncsArbitraryArgsAllowed() + * @dataProvider variadicFuncs + */ + public function testVariadicFuncsArbitraryArgsAllowed( $func ) { + $argsList = str_repeat( ', "arg"', 50 ); + $code = "$func( 'arg' $argsList )"; + foreach ( self::getParsers() as $parser ) { + $pname = get_class( $parser ); + try { + $parser->parse( $code ); + $this->assertTrue( true ); + } catch ( AFPException $e ) { + $this->fail( "Got exception with parser $pname.\n$e" ); + } + } + } + + /** + * @return array + */ + public function variadicFuncs() { + return [ + [ 'ccnorm_contains_any' ], + [ 'ccnorm_contains_all' ], + ]; + } +} diff --git a/tests/phpunit/AFPDataTest.php b/tests/phpunit/unit/AFPDataTest.php similarity index 100% rename from tests/phpunit/AFPDataTest.php rename to tests/phpunit/unit/AFPDataTest.php diff --git a/tests/phpunit/AbuseFilterParserTest.php b/tests/phpunit/unit/AbuseFilterParserTest.php similarity index 96% rename from tests/phpunit/AbuseFilterParserTest.php rename to tests/phpunit/unit/AbuseFilterParserTest.php index 085f7e4d5..bee889ea6 100644 --- a/tests/phpunit/AbuseFilterParserTest.php +++ b/tests/phpunit/unit/AbuseFilterParserTest.php @@ -21,6 +21,8 @@ * @author Marius Hoch < hoo@online.de > */ +use Psr\Log\NullLogger; + /** * @group Test * @group AbuseFilter @@ -45,7 +47,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider readTests */ public function testParser( $rule ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $this->assertTrue( $parser->parse( $rule ), 'Parser used: ' . get_class( $parser ) ); } } @@ -54,7 +56,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @return Generator|array */ public function readTests() { - $testPath = __DIR__ . "/../parserTests"; + $testPath = __DIR__ . "/../../parserTests"; $testFiles = glob( $testPath . "/*.t" ); foreach ( $testFiles as $testFile ) { @@ -73,7 +75,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider provideExpressions */ public function testEvaluateExpression( $expr, $expected ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $actual = $parser->evaluateExpression( $expr ); $this->assertEquals( $expected, $actual ); } @@ -102,7 +104,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider provideEmptySyntax */ public function testEmptySyntax( $code ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $this->assertFalse( $parser->parse( $code ) ); } } @@ -153,7 +155,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider condCountCases */ public function testCondCount( $rule, $expected ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $parserClass = get_class( $parser ); $countBefore = $parser->getCondCount(); $parser->parse( $rule ); @@ -190,7 +192,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { public function testArrayShortcircuit() { $code = 'a := [false, false]; b := [false, false]; c := 42; d := [0,1];' . 'a[0] != false & b[1] != false & (b[5**2/(5*(4+1))] !== a[43-c] | a[d[0]] === b[d[c-41]])'; - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $this->assertFalse( $parser->parse( $code ), 'Parser: ' . get_class( $parser ) ); } } @@ -662,8 +664,6 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { [ 'contains_any' ], [ 'contains_all' ], [ 'equals_to_any' ], - [ 'ccnorm_contains_any' ], - [ 'ccnorm_contains_all' ], ]; } @@ -678,7 +678,12 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { public function testCheckArgCountInConditional( $funcCode, $exceptionCode ) { $code = "if ( 1==1 ) then ( 1 ) else ( $funcCode ) end;"; // AbuseFilterParser skips the parentheses altogether, so this is not supposed to work - $parser = new AbuseFilterCachingParser(); + $parser = new AbuseFilterCachingParser( + new LanguageEn(), + new EmptyBagOStuff(), + new NullLogger() + ); + $parser->toggleConditionLimit( false ); try { $parser->parse( $code ); $this->fail( 'No exception was thrown.' ); @@ -709,16 +714,15 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider provideDeprecatedVars */ public function testDeprecatedVars( $old, $new ) { - $loggerMock = new TestLogger(); - $loggerMock->setCollect( true ); - $this->setLogger( 'AbuseFilter', $loggerMock ); - - $vars = new AbuseFilterVariableHolder(); // Set it under the new name, and check that the old name points to it - $vars->setVar( $new, 'Some value' ); + $vars = AbuseFilterVariableHolder::newFromArray( [ $new => 'value' ] ); - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $pname = get_class( $parser ); + $loggerMock = new TestLogger(); + $loggerMock->setCollect( true ); + $parser->setLogger( $loggerMock ); + $parser->setVariables( $vars ); $actual = $parser->parse( "$old === $new" ); @@ -759,7 +763,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider provideConsecutiveComparisons */ public function testDisallowConsecutiveComparisons( $code, $valid ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $pname = get_class( $parser ); $actuallyValid = true; try { @@ -815,7 +819,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider provideVarDeclarationInSkippedBlock */ public function testVarDeclarationInSkippedBlock( $code ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $pname = get_class( $parser ); try { $this->assertFalse( @@ -862,7 +866,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { * @dataProvider provideDUNDEFINED */ public function testDUNDEFINED( $code ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $pname = get_class( $parser ); try { $this->assertFalse( @@ -920,6 +924,9 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { public function testEmptyOperands( $code, $operandType ) { /** @var PHPUnit\Framework\MockObject\MockObject|AbuseFilterParser $mock */ $mock = $this->getMockBuilder( AbuseFilterParser::class ) + ->setConstructorArgs( + [ new LanguageEn(), new EmptyBagOStuff(), new NullLogger() ] + ) ->setMethods( [ 'logEmptyOperand' ] ) ->getMock(); @@ -927,6 +934,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase { ->method( 'logEmptyOperand' ) ->with( $operandType ); + $mock->toggleConditionLimit( false ); $mock->parse( $code ); } diff --git a/tests/phpunit/AbuseFilterParserTestCase.php b/tests/phpunit/unit/AbuseFilterParserTestCase.php similarity index 74% rename from tests/phpunit/AbuseFilterParserTestCase.php rename to tests/phpunit/unit/AbuseFilterParserTestCase.php index 455bd7a10..af2bfd86b 100644 --- a/tests/phpunit/AbuseFilterParserTestCase.php +++ b/tests/phpunit/unit/AbuseFilterParserTestCase.php @@ -23,17 +23,24 @@ /** * Helper for parser-related tests */ -abstract class AbuseFilterParserTestCase extends MediaWikiTestCase { +abstract class AbuseFilterParserTestCase extends MediaWikiUnitTestCase { /** * @return AbuseFilterParser[] */ - public static function getParsers() { + protected function getParsers() { static $parsers = null; if ( !$parsers ) { - $parsers = [ - new AbuseFilterParser(), - new AbuseFilterCachingParser() - ]; + // We're not interested in caching or logging; tests should call respectively setCache + // and setLogger if they want to test any of those. + $contLang = new LanguageEn(); + $cache = new EmptyBagOStuff(); + $logger = new \Psr\Log\NullLogger(); + + $parser = new AbuseFilterParser( $contLang, $cache, $logger ); + $parser->toggleConditionLimit( false ); + $cachingParser = new AbuseFilterCachingParser( $contLang, $cache, $logger ); + $cachingParser->toggleConditionLimit( false ); + $parsers = [ $parser, $cachingParser ]; } else { // Reset so that already executed tests don't influence new ones $parsers[0]->resetState(); @@ -54,7 +61,7 @@ abstract class AbuseFilterParserTestCase extends MediaWikiTestCase { * just used for debugging purposes. */ protected function exceptionTest( $excep, $expr, $caller ) { - foreach ( self::getParsers() as $parser ) { + foreach ( $this->getParsers() as $parser ) { $pname = get_class( $parser ); try { $parser->parse( $expr ); diff --git a/tests/phpunit/AbuseFilterTokenizerTest.php b/tests/phpunit/unit/AbuseFilterTokenizerTest.php similarity index 92% rename from tests/phpunit/AbuseFilterTokenizerTest.php rename to tests/phpunit/unit/AbuseFilterTokenizerTest.php index 95aba017b..c2273f6ee 100644 --- a/tests/phpunit/AbuseFilterTokenizerTest.php +++ b/tests/phpunit/unit/AbuseFilterTokenizerTest.php @@ -111,20 +111,19 @@ class AbuseFilterTokenizerTest extends AbuseFilterParserTestCase { } /** - * Test that tokenized code is saved in cache + * Test that tokenized code is saved in cache. * * @param string $code To be tokenized * @dataProvider provideCode + * @covers AbuseFilterTokenizer::getTokens */ public function testCaching( $code ) { $cache = new HashBagOStuff(); - $this->setService( 'LocalServerObjectCache', $cache ); + $tokenizer = new AbuseFilterTokenizer( $cache ); - $key = AbuseFilterTokenizer::getCacheKey( $cache, $code ); + $key = $tokenizer->getCacheKey( $code ); - // Other tests may have already cached the same code. - $cache->delete( $key ); - AbuseFilterTokenizer::getTokens( $code ); + $tokenizer->getTokens( $code ); $this->assertNotFalse( $cache->get( $key ) ); }