wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-11-23 21:53:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Clean up AbuseFilterConsequencesTest a bit

Browse source 
Most notably, make it not use additional DB tables to test global
filters. Instead, just pretend that the local database is not local (via
config) and "hide" local filters with a simple test-only flag in
FilterLookup.

Change-Id: Ib431dbf6c9d84978ee84e7f0671cfcbf8a54d7a2
This commit is contained in:
Daimona Eaytoy 2023-08-18 15:00:49 +02:00
parent a10765baec
commit 85f8f90531
4 changed files with 52 additions and 131 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
extension.json
View file

@ -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": [

23
includes/FilterLookup.php
View file

@ -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;
}
}

66
includes/Hooks/Handlers/TestsHandler.php
View file

@ -1,66 +0,0 @@
<?php
namespace MediaWiki\Extension\AbuseFilter\Hooks\Handlers;
use AbuseFilterConsequencesTest;
use MediaWiki\Hook\UnitTestsAfterDatabaseSetupHook;
use MediaWiki\Hook\UnitTestsBeforeDatabaseTeardownHook;
use MediaWiki\MediaWikiServices;
use Wikimedia\Rdbms\IMaintainableDatabase;
/**
* @codeCoverageIgnore This is test code
*/
class TestsHandler implements UnitTestsAfterDatabaseSetupHook, UnitTestsBeforeDatabaseTeardownHook {
/**
* Setup tables to emulate global filters, used in AbuseFilterConsequencesTest.
*
* @param IMaintainableDatabase $db
* @param string $prefix The prefix used in unit tests
* @suppress PhanUndeclaredClassConstant AbuseFilterConsequencesTest is in AutoloadClasses
* @suppress PhanUndeclaredClassStaticProperty AbuseFilterConsequencesTest is in AutoloadClasses
*/
public function onUnitTestsAfterDatabaseSetup( $db, $prefix ) {
$externalPrefix = AbuseFilterConsequencesTest::DB_EXTERNAL_PREFIX;
if ( $db->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 );
}
}
}

88
tests/phpunit/AbuseFilterConsequencesTest.php
View file

@ -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__