From 62e5509772ff0b7376d4686f59f0c40e334f99ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Fri, 15 Jul 2022 10:44:16 +0200 Subject: [PATCH] Add regression test for RunVariableGenerator Test that null edits do not trigger filters, but sole content model change does. Also do some cleanup in AbuseFilterConsequencesTest. For better isolation, do not access the service container and do not initialize objects in the constructor. Change-Id: I043ecb312226a69d1f485a8382d558ccb899a270 --- tests/phpunit/AbuseFilterConsequencesTest.php | 92 +++++++++++++------ 1 file changed, 62 insertions(+), 30 deletions(-) diff --git a/tests/phpunit/AbuseFilterConsequencesTest.php b/tests/phpunit/AbuseFilterConsequencesTest.php index c34190392..5acda75cb 100644 --- a/tests/phpunit/AbuseFilterConsequencesTest.php +++ b/tests/phpunit/AbuseFilterConsequencesTest.php @@ -1,8 +1,8 @@ tablesUsed = array_merge( $this->tablesUsed, $prefixedTables ); - $this->user = User::newFromName( 'FilteredUser' ); parent::__construct( $name, $data, $dataName ); } @@ -345,14 +344,10 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { */ protected function setUp(): void { parent::setUp(); - // Ensure that our user is not blocked and is a sysop (matched filters could block or - // degroup the user) + // Ensure that our user is a sysop + $this->user = $this->getServiceContainer()->getUserFactory()->newFromName( 'FilteredUser' ); $this->user->addToDatabase(); - MediaWikiServices::getInstance()->getUserGroupManager()->addUserToGroup( $this->user, 'sysop' ); - $block = DatabaseBlock::newFromTarget( $this->user ); - if ( $block ) { - $block->delete(); - } + $this->getServiceContainer()->getUserGroupManager()->addUserToGroup( $this->user, 'sysop' ); // Pin time to avoid time shifts on relative block duration ConvertibleTimestamp::setFakeTime( time() ); @@ -443,15 +438,15 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { /** * @param Title $title Title of the page to edit - * @param string $text The new content of the page + * @param Content $content The new content of the page * @param string $summary The summary of the edit * @return string The status of the operation, as returned by the API. */ - private function stashEdit( $title, $text, $summary ) { + private function stashEdit( Title $title, Content $content, string $summary ) { $services = $this->getServiceContainer(); return $services->getPageEditStash()->parseAndCache( $services->getWikiPageFactory()->newFromTitle( $title ), - new WikitextContent( $text ), + $content, $this->user, $summary ); @@ -459,8 +454,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { /** * @param Title $title Title of the page to edit - * @param string $oldText Old content of the page - * @param string $newText The new content of the page + * @param Content|string $oldText Old content of the page + * @param Content|string $newText The new content of the page * @param string $summary The summary of the edit * @param bool|null $fromStash Whether to stash the edit. Null means no stashing, false means * stash the edit but don't reuse it for saving, true means stash and reuse. @@ -478,6 +473,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { if ( !$status->isGood() ) { throw new Exception( "Could not create test page. $status" ); } + $page->clear(); $title->resetArticleID( -1 ); } @@ -488,16 +484,18 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { // Otherwise, stash some random text which won't match the actual edit $stashText = md5( uniqid( rand(), true ) ); } - $stashResult = $this->stashEdit( $title, $stashText, $summary ); + $stashContent = $stashText instanceof Content ? $stashText : new WikitextContent( $stashText ); + $stashResult = $this->stashEdit( $title, $stashContent, $summary ); if ( $stashResult !== PageEditStash::ERROR_NONE ) { throw new MWException( "The edit cannot be stashed, got the following error: $stashResult" ); } } - $content = ContentHandler::makeContent( $newText, $title ); + $content = $newText instanceof Content + ? $newText + : ContentHandler::makeContent( $newText, $title ); $status = Status::newGood(); $context = RequestContext::getMain(); - $context->setTitle( $title ); $context->setUser( $this->user ); $context->setWikiPage( $page ); @@ -519,6 +517,8 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { NS_MAIN, $this->user ); + $page->clear(); + $title->resetArticleID( -1 ); } return $status; @@ -531,9 +531,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * @return Status */ private function doAction( $params ) { - $target = Title::newFromText( $params['target'] ); - // Make sure that previous blocks don't affect the test - $this->user->clearInstanceCache(); + $target = Title::newFromTextThrow( $params['target'] ); switch ( $params['action'] ) { case 'edit': @@ -553,7 +551,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { // Ensure that the page exists $this->getExistingTestPage( $target ); $newTitle = isset( $params['newTitle'] ) - ? Title::newFromText( $params['newTitle'] ) + ? Title::newFromTextThrow( $params['newTitle'] ) : $this->getNonExistingTestPage()->getTitle(); $status = $this->getServiceContainer() ->getMovePageFactory() @@ -608,20 +606,18 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { * @return string[] The applied tags */ private function getActionTags( $actionParams ) { + $title = Title::newFromTextThrow( $actionParams['target'] ); if ( $actionParams['action'] === 'edit' || $actionParams['action'] === 'stashedit' ) { - $page = $this->getServiceContainer()->getWikiPageFactory()->newFromTitle( - Title::newFromText( $actionParams['target'] ) - ); - return ChangeTags::getTags( $this->db, null, $page->getLatest() ); + return ChangeTags::getTags( $this->db, null, $title->getLatestRevID() ); } $logType = $actionParams['action'] === 'createaccount' ? 'newusers' : $actionParams['action']; $logAction = $logType === 'newusers' ? 'create2' : $logType; - $title = Title::newFromText( $actionParams['target'] ); $id = $this->db->selectField( 'logging', 'log_id', [ + 'log_namespace' => $title->getNamespace(), 'log_title' => $title->getDBkey(), 'log_type' => $logType, 'log_action' => $logAction @@ -633,7 +629,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { if ( !$id ) { $this->fail( 'Could not find the action in the logging table.' ); } - return ChangeTags::getTags( $this->db, null, null, $id ); + return ChangeTags::getTags( $this->db, null, null, (int)$id ); } /** @@ -932,7 +928,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { [ 5 ], [ 'action' => 'upload', - 'target' => 'MyFile.svg', + 'target' => 'File:MyFile.svg', ], [ 'tag' => [ 5 ] ] ], @@ -940,7 +936,7 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { [ 22 ], [ 'action' => 'upload', - 'target' => 'MyFile.svg', + 'target' => 'File:MyFile.svg', 'newText' => 'Block me please!', 'summary' => 'Asking to be blocked' ], @@ -1597,4 +1593,40 @@ class AbuseFilterConsequencesTest extends MediaWikiIntegrationTestCase { "AbuseLog entry updated with the revision id (full filter hit: $filterHitStr)" ); } + + /** + * Test a null edit. + * @covers \MediaWiki\Extension\AbuseFilter\VariableGenerator\RunVariableGenerator + */ + public function testNullEdit() { + $this->setService( + FilterRunnerFactory::SERVICE_NAME, + $this->createNoOpMock( FilterRunnerFactory::class ) + ); + // Filter 24 has no actions and always matches + $this->createFilters( [ 24 ] ); + + $targetTitle = Title::makeTitle( NS_MAIN, 'TestNullEdit' ); + $text = 'Some text'; + $status = $this->doEdit( $targetTitle, $text, $text, 'Summary' ); + $this->assertStatusGood( $status ); + } + + /** + * Test a content model change. + * @covers \MediaWiki\Extension\AbuseFilter\VariableGenerator\RunVariableGenerator + */ + public function testContentModelChange() { + // Filter 23 always matches and disables + $this->createFilters( [ 23 ] ); + + $targetTitle = Title::makeTitle( NS_MAIN, 'TestContentModelChange' ); + + $text = FormatJson::encode( [ 'key' => 'value' ] ); + $oldContent = new JsonContent( $text ); + $newContent = new WikitextContent( $text ); + + $status = $this->doEdit( $targetTitle, $oldContent, $newContent, 'Summary' ); + $this->assertStatusNotOK( $status ); + } }