diff --git a/extension.json b/extension.json index e574ba5b0..a25aa334c 100644 --- a/extension.json +++ b/extension.json @@ -241,7 +241,6 @@ "MediaWiki\\Extension\\AbuseFilter\\Maintenance\\": "maintenance/" }, "TestAutoloadClasses": { - "AbuseFilterConsequencesTest": "tests/phpunit/AbuseFilterConsequencesTest.php", "MediaWiki\\Extension\\AbuseFilter\\Tests\\Unit\\Parser\\ParserTestCase": "tests/phpunit/unit/Parser/ParserTestCase.php", "MediaWiki\\Extension\\AbuseFilter\\Tests\\Integration\\Api\\AbuseFilterApiTestTrait": "tests/phpunit/integration/Api/AbuseFilterApiTestTrait.php", "AbuseFilterUploadTestTrait": "tests/phpunit/AbuseFilterUploadTestTrait.php", @@ -351,9 +350,6 @@ "AbuseFilterChangeTagsManager" ] }, - "Tests": { - "class": "MediaWiki\\Extension\\AbuseFilter\\Hooks\\Handlers\\TestsHandler" - }, "SchemaChanges": { "class": "MediaWiki\\Extension\\AbuseFilter\\Hooks\\Handlers\\SchemaChangesHandler", "factory": "MediaWiki\\Extension\\AbuseFilter\\Hooks\\Handlers\\SchemaChangesHandler::newFromGlobalState" @@ -427,8 +423,6 @@ "UserMergeAccountFields": "UserMerge", "BeforeCreateEchoEvent": "Echo", "ParserOutputStashForEdit": "FilteredActions", - "UnitTestsAfterDatabaseSetup": "Tests", - "UnitTestsBeforeDatabaseTeardown": "Tests", "JsonValidateSave": "EditPermission" }, "ServiceWiringFiles": [ diff --git a/includes/FilterLookup.php b/includes/FilterLookup.php index c33b227d4..5f372047d 100644 --- a/includes/FilterLookup.php +++ b/includes/FilterLookup.php @@ -13,6 +13,7 @@ use MediaWiki\Extension\AbuseFilter\Filter\Flags; use MediaWiki\Extension\AbuseFilter\Filter\HistoryFilter; use MediaWiki\Extension\AbuseFilter\Filter\LastEditInfo; use MediaWiki\Extension\AbuseFilter\Filter\Specs; +use RuntimeException; use stdClass; use WANObjectCache; use Wikimedia\Rdbms\ILoadBalancer; @@ -71,6 +72,12 @@ class FilterLookup implements IDBAccessObject { /** @var ActorMigrationBase */ private $actorMigration; + /** + * @var bool Flag used in PHPUnit tests to "hide" local filters when testing global ones, so that we can use the + * local database pretending it's not local. + */ + private bool $localFiltersHiddenForTest = false; + /** * @param ILoadBalancer $loadBalancer * @param WANObjectCache $cache @@ -170,6 +177,10 @@ class FilterLookup implements IDBAccessObject { * @return ExistingFilter[] */ private function getAllActiveFiltersInGroupFromDB( string $group, bool $global, int $flags ): array { + if ( $this->localFiltersHiddenForTest && !$global ) { + return []; + } + [ $dbIndex, $dbOptions ] = DBAccessObjectUtils::getDBOptions( $flags ); $dbr = $this->getDBConnection( $dbIndex, $global ); @@ -532,4 +543,16 @@ class FilterLookup implements IDBAccessObject { private function getCacheKey( int $filterID, bool $global ): string { return GlobalNameUtils::buildGlobalName( $filterID, $global ); } + + /** + * "Hides" local filters when testing global ones, so that we can use the + * local database pretending it's not local. + * @codeCoverageIgnore + */ + public function hideLocalFiltersForTesting(): void { + if ( !defined( 'MW_PHPUNIT_TEST' ) ) { + throw new RuntimeException( 'Can only be called in tests' ); + } + $this->localFiltersHiddenForTest = true; + } } diff --git a/includes/Hooks/Handlers/TestsHandler.php b/includes/Hooks/Handlers/TestsHandler.php deleted file mode 100644 index 93596c4aa..000000000 --- a/includes/Hooks/Handlers/TestsHandler.php +++ /dev/null @@ -1,66 +0,0 @@ -tableExists( $externalPrefix . AbuseFilterConsequencesTest::$externalTables[0], __METHOD__ ) ) { - // Check a random table to avoid unnecessary table creations. See T155147. - return; - } - - foreach ( AbuseFilterConsequencesTest::$externalTables as $table ) { - // Don't create them as temporary, as we'll access the DB via another connection - if ( $db->getType() === 'sqlite' ) { - // SQLite definitions don't have the prefix, ref T251967 - $db->duplicateTableStructure( - $table, - "$prefix$externalPrefix$table", - true, - __METHOD__ - ); - $db->query( "INSERT INTO $prefix$externalPrefix$table SELECT * FROM $prefix$table" ); - } else { - $db->duplicateTableStructure( - "$prefix$table", - "$prefix$externalPrefix$table", - false, - __METHOD__ - ); - } - } - } - - /** - * Drop tables used for global filters in AbuseFilterConsequencesTest. - * Note: this has the same problem as T201290. - * - * @suppress PhanUndeclaredClassConstant AbuseFilterConsequencesTest is in AutoloadClasses - * @suppress PhanUndeclaredClassStaticProperty AbuseFilterConsequencesTest is in AutoloadClasses - */ - public function onUnitTestsBeforeDatabaseTeardown() { - $db = MediaWikiServices::getInstance()->getDBLoadBalancer() - ->getMaintenanceConnectionRef( DB_PRIMARY ); - foreach ( AbuseFilterConsequencesTest::$externalTables as $table ) { - $db->dropTable( AbuseFilterConsequencesTest::DB_EXTERNAL_PREFIX . $table ); - } - } -} diff --git a/tests/phpunit/AbuseFilterConsequencesTest.php b/tests/phpunit/AbuseFilterConsequencesTest.php index 81a8e7502..a8aaa195d 100644 --- a/tests/phpunit/AbuseFilterConsequencesTest.php +++ b/tests/phpunit/AbuseFilterConsequencesTest.php @@ -70,15 +70,6 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * @var User The user performing actions */ private $user; - /** Prefix for tables to emulate an external DB */ - public const DB_EXTERNAL_PREFIX = 'external_'; - /** @var string[] Tables to create in the external DB */ - public static $externalTables = [ - 'abuse_filter', - 'abuse_filter_action', - 'abuse_filter_log', - 'text', - ]; /** * @var array These tables will be deleted in parent::tearDown @@ -136,7 +127,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { ] ], 5 => [ - 'af_pattern' => 'user_name == "FilteredUser"', + // XXX Need to hardcode UTSysop here because this is a constant + 'af_pattern' => 'user_name == "UTSysop"', 'af_public_comments' => 'Mock filter', 'actions' => [ 'tag' => [ @@ -269,7 +261,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { ] ], 19 => [ - 'af_pattern' => 'user_name === "FilteredUser"', + // XXX Need to hardcode UTSysop here because this is a constant + 'af_pattern' => 'user_name === "UTSysop"', 'af_public_comments' => 'Another global filter', 'af_global' => 1, 'actions' => [ @@ -325,31 +318,13 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { ]; // phpcs:enable Generic.Files.LineLength - /** - * Add tables for global filters to the list of used tables - * - * @inheritDoc - */ - public function __construct( $name = null, array $data = [], $dataName = '' ) { - $prefixedTables = array_map( - static function ( $table ) { - return self::DB_EXTERNAL_PREFIX . $table; - }, - self::$externalTables - ); - $this->tablesUsed = array_merge( $this->tablesUsed, $prefixedTables ); - parent::__construct( $name, $data, $dataName ); - } - /** * @inheritDoc */ protected function setUp(): void { parent::setUp(); // Ensure that our user is a sysop - $this->user = $this->getServiceContainer()->getUserFactory()->newFromName( 'FilteredUser' ); - $this->user->addToDatabase(); - $this->getServiceContainer()->getUserGroupManager()->addUserToGroup( $this->user, 'sysop' ); + $this->user = $this->getTestSysop()->getUser(); // Pin time to avoid time shifts on relative block duration ConvertibleTimestamp::setFakeTime( time() ); @@ -384,13 +359,11 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * Creates new filters with the given ids, referred to self::FILTERS * * @param int[] $ids IDs of the filters to create - * @param bool $external Whether to create filters in the external table */ - private function createFilters( $ids, $external = false ) { + private function createFilters( $ids ) { global $wgAbuseFilterActions; $dbw = $this->getDb(); - $tablePrefix = $external ? self::DB_EXTERNAL_PREFIX : ''; $defaultRowSection = [ 'af_user' => 0, 'af_user_text' => 'FilterTester', @@ -414,7 +387,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { $filter[ 'af_id' ] = $id; $dbw->insert( - "{$tablePrefix}abuse_filter", + "abuse_filter", $filter, __METHOD__ ); @@ -434,7 +407,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { } $dbw->insert( - "{$tablePrefix}abuse_filter_action", + "abuse_filter_action", $actionsRows, __METHOD__ ); @@ -449,8 +422,9 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { */ private function stashEdit( Title $title, Content $content, string $summary ) { $services = $this->getServiceContainer(); + $pageUpdater = $services->getWikiPageFactory()->newFromTitle( $title )->newPageUpdater( $this->user ); return $services->getPageEditStash()->parseAndCache( - $services->getWikiPageFactory()->newFromTitle( $title ), + $pageUpdater, $content, $this->user, $summary @@ -476,7 +450,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { __METHOD__ . ' page creation' ); if ( !$status->isGood() ) { - throw new Exception( "Could not create test page. $status" ); + throw new RuntimeException( "Could not create test page. $status" ); } $page->clear(); $title->resetArticleID( -1 ); @@ -793,7 +767,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { $this->assertSame( $actionParams['username'], $row->afl_user_text, $dumpStr ); $this->assertSame( -1, (int)$row->afl_namespace, $dumpStr ); } else { - $this->assertSame( 'FilteredUser', $row->afl_user_text, $dumpStr ); + $this->assertSame( $this->user->getName(), $row->afl_user_text, $dumpStr ); $this->assertSame( $title->getNamespace(), (int)$row->afl_namespace, $dumpStr ); $this->assertSame( $title->getDBkey(), $row->afl_title, $dumpStr ); } @@ -847,6 +821,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * @return array */ public static function provideFilters() { + // XXX Need to hardcode the username of $this->user here. + $username = 'UTSysop'; return [ 'Basic test for "edit" action' => [ [ 1, 2 ], @@ -898,7 +874,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { [ 5 ], [ 'action' => 'edit', - 'target' => 'User:FilteredUser', + 'target' => "User:$username", 'oldText' => 'Hey.', 'newText' => 'I am a very nice user, really!', 'summary' => '' @@ -942,7 +918,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { [ 1, 3, 7, 12 ], [ 'action' => 'edit', - 'target' => 'User:FilteredUser', + 'target' => "User:$username", 'oldText' => '', 'newText' => 'A couple of lines about me...', 'summary' => 'My user page' @@ -1407,6 +1383,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * @return array */ public static function provideStashedEdits() { + // XXX Need to hardcode the username of $this->user here. + $username = 'UTSysop'; $sets = [ [ [ 1, 2 ], @@ -1421,7 +1399,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { [ [ 5 ], [ - 'target' => 'User:FilteredUser', + 'target' => "User:$username", 'oldText' => 'Hey.', 'newText' => 'I am a very nice user, really!', 'summary' => '' @@ -1471,7 +1449,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { [ [ 1, 3, 7, 12 ], [ - 'target' => 'User:FilteredUser', + 'target' => "User:$username", 'oldText' => '', 'newText' => 'A couple of lines about me...', 'summary' => 'My user page' @@ -1511,9 +1489,9 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { /** * Tests for global filters, defined on a central wiki and executed on another (e.g. a filter - * defined on meta but triggered on another wiki using meta's global filters). - * We emulate an external database by using different tables prefixed with - * self::DB_EXTERNAL_PREFIX + * defined on meta but triggered on another wiki using meta's global filters). + * For simplicity, this test creates filters in the local database but makes the extension believe that it's + * actually external. * * @param int[] $createIds IDs of the filters to create * @param array $actionParams Details of the action we need to execute to trigger filters @@ -1522,25 +1500,18 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * @covers \MediaWiki\Extension\AbuseFilter\AbuseLogger */ public function testGlobalFilters( $createIds, $actionParams, $consequences ) { - // cannot access temporary tables of other sessions - $this->markTestSkippedIfDbType( 'postgres' ); - // Read from wrong table: unittest_external_abuse_filter - $this->markTestSkippedIfDbType( 'sqlite' ); - $this->setMwGlobals( [ - 'wgAbuseFilterCentralDB' => - $this->getDb()->getDBname() . - '-' . - $this->dbPrefix() . self::DB_EXTERNAL_PREFIX, + 'wgAbuseFilterCentralDB' => WikiMap::getCurrentWikiId(), 'wgAbuseFilterIsCentral' => false, ] ); - $this->createFilters( $createIds, true ); + $this->createFilters( $createIds ); + AbuseFilterServices::getFilterLookup( $this->getServiceContainer() )->hideLocalFiltersForTesting(); $result = $this->doAction( $actionParams ); list( $expected, $actual ) = $this->checkConsequences( $result, $actionParams, $consequences ); - // First check that the filter work as expected + // First check that the filters work as expected $this->assertEquals( $expected, $actual, @@ -1548,9 +1519,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { ); // Check that the hits were logged on the "external" DB - $db = MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->getPrimaryDatabase(); - $loggedFilters = $db->selectFieldValues( - self::DB_EXTERNAL_PREFIX . 'abuse_filter_log', + $loggedFilters = $this->getDb()->selectFieldValues( + 'abuse_filter_log', 'afl_filter_id', [ 'afl_wiki IS NOT NULL' ], __METHOD__