From 9f2906e34b3da5f5bbf8edc58c2d09979da5a0f3 Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Sat, 19 Sep 2020 17:14:31 +0200 Subject: [PATCH] Reduce dependencies of AbuseFilter::saveFilter This patch removes the dependency of saveFilter on the ContextSource kitchen sink. It also removes some unneded dependency, and adds $originalRow/$originalActions as parameter, rather than hacky properties in $newRow that are easy to forget. The related test can also be greatly simplified. This also introduces a behaviour change: checking $newRow instead of the Request allows us to account for values normalization done in AbuseFilterViewEdit::loadRequest, and to also work correctly for imports (and generally speaking, it makes the method suitable for an AbuseFilterEdit API module, too). Next step is moving this method to a service. Some signatures, indenting, name choices etc. are subpar, but this is just because these methods are temporary anyway. Bug: T213037 Change-Id: I235b928d7b9c2ef1c46ea0bf3e3ed212500b4161 --- includes/AbuseFilter.php | 72 ++--- includes/Views/AbuseFilterViewEdit.php | 12 +- tests/phpunit/AbuseFilterSaveTest.php | 389 ++++++++++--------------- 3 files changed, 200 insertions(+), 273 deletions(-) diff --git a/includes/AbuseFilter.php b/includes/AbuseFilter.php index 50cc1c2d7..21e111f20 100644 --- a/includes/AbuseFilter.php +++ b/includes/AbuseFilter.php @@ -912,47 +912,53 @@ class AbuseFilter { * - OK with errors if a validation error occurred * - Fatal in case of a permission-related error * - * @param ContextSource $context + * @param User $user * @param int|string $filter * @param stdClass $newRow * @param array $actions + * @param stdClass $originalRow + * @param array $originalActions * @param IDatabase $dbw DB_MASTER Where the filter should be saved + * @param Config $config * @return Status + * @internal */ public static function saveFilter( - ContextSource $context, + User $user, $filter, - $newRow, - $actions, - IDatabase $dbw + stdClass $newRow, + array $actions, + stdClass $originalRow, + array $originalActions, + IDatabase $dbw, + Config $config ) { $validationStatus = Status::newGood(); - $request = $context->getRequest(); - $user = $context->getUser(); // Check the syntax - $syntaxerr = self::getDefaultParser()->checkSyntax( $request->getVal( 'wpFilterRules' ) ); + $syntaxerr = self::getDefaultParser()->checkSyntax( $newRow->af_pattern ); if ( $syntaxerr !== true ) { $validationStatus->error( 'abusefilter-edit-badsyntax', $syntaxerr[0] ); return $validationStatus; } // Check for missing required fields (title and pattern) $missing = []; - if ( !$request->getVal( 'wpFilterRules' ) || - trim( $request->getVal( 'wpFilterRules' ) ) === '' ) { - $missing[] = $context->msg( 'abusefilter-edit-field-conditions' )->escaped(); + if ( !$newRow->af_pattern || trim( $newRow->af_pattern ) === '' ) { + $missing[] = new Message( 'abusefilter-edit-field-conditions' ); } - if ( !$request->getVal( 'wpFilterDescription' ) ) { - $missing[] = $context->msg( 'abusefilter-edit-field-description' )->escaped(); + if ( !$newRow->af_public_comments ) { + $missing[] = new Message( 'abusefilter-edit-field-description' ); } if ( count( $missing ) !== 0 ) { - $missing = $context->getLanguage()->commaList( $missing ); - $validationStatus->error( 'abusefilter-edit-missingfields', $missing ); + $validationStatus->error( + 'abusefilter-edit-missingfields', + Message::listParam( $missing, 'comma' ) + ); return $validationStatus; } // Don't allow setting as deleted an active filter - if ( $request->getCheck( 'wpFilterEnabled' ) && $request->getCheck( 'wpFilterDeleted' ) ) { + if ( $newRow->af_enabled && $newRow->af_deleted ) { $validationStatus->error( 'abusefilter-edit-deleting-enabled' ); return $validationStatus; } @@ -994,11 +1000,11 @@ class AbuseFilter { } $availableActions = array_keys( - array_filter( $context->getConfig()->get( 'AbuseFilterActions' ) ) + array_filter( $config->get( 'AbuseFilterActions' ) ) ); $differences = self::compareVersions( [ $newRow, $actions ], - [ $newRow->mOriginalRow, $newRow->mOriginalActions ], + [ $originalRow, $originalActions ], $availableActions ); @@ -1006,7 +1012,7 @@ class AbuseFilter { // rule that is currently global, without permissions. if ( !self::canEditFilter( $user, $newRow ) || - !self::canEditFilter( $user, $newRow->mOriginalRow ) + !self::canEditFilter( $user, $originalRow ) ) { $validationStatus->fatal( 'abusefilter-edit-notallowed-global' ); return $validationStatus; @@ -1014,18 +1020,14 @@ class AbuseFilter { // Don't allow custom messages on global rules if ( $newRow->af_global == 1 && ( - $request->getVal( 'wpFilterWarnMessage' ) !== 'abusefilter-warning' || - $request->getVal( 'wpFilterDisallowMessage' ) !== 'abusefilter-disallowed' + ( isset( $actions['warn'] ) && $actions['warn'][0] !== 'abusefilter-warning' ) || + ( isset( $actions['disallow'] ) && $actions['disallow'][0] !== 'abusefilter-disallowed' ) ) ) { $validationStatus->fatal( 'abusefilter-edit-notallowed-global-custom-msg' ); return $validationStatus; } - $origActions = $newRow->mOriginalActions; - $wasGlobal = (bool)$newRow->mOriginalRow->af_global; - - unset( $newRow->mOriginalRow ); - unset( $newRow->mOriginalActions ); + $wasGlobal = (bool)$originalRow->af_global; // Check for non-changes if ( !count( $differences ) ) { @@ -1034,10 +1036,10 @@ class AbuseFilter { } // Check for restricted actions - $restrictions = $context->getConfig()->get( 'AbuseFilterRestrictions' ); + $restrictions = $config->get( 'AbuseFilterRestrictions' ); if ( count( array_intersect_key( array_filter( $restrictions ), - array_merge( $actions, $origActions ) + array_merge( $actions, $originalActions ) ) ) && !MediaWikiServices::getInstance()->getPermissionManager() ->userHasRight( $user, 'abusefilter-modify-restricted' ) @@ -1048,7 +1050,7 @@ class AbuseFilter { // Everything went fine, so let's save the filter list( $new_id, $history_id ) = - self::doSaveFilter( $newRow, $differences, $filter, $actions, $wasGlobal, $context, $dbw ); + self::doSaveFilter( $user, $newRow, $differences, $filter, $actions, $wasGlobal, $dbw, $config ); $validationStatus->setResult( true, [ $new_id, $history_id ] ); return $validationStatus; } @@ -1056,26 +1058,26 @@ class AbuseFilter { /** * Saves new filter's info to DB * + * @param User $user * @param stdClass $newRow * @param array $differences * @param int|string $filter * @param array $actions * @param bool $wasGlobal - * @param ContextSource $context * @param IDatabase $dbw DB_MASTER where the filter will be saved + * @param Config $config * @return int[] first element is new ID, second is history ID */ private static function doSaveFilter( + User $user, $newRow, $differences, $filter, $actions, $wasGlobal, - ContextSource $context, - IDatabase $dbw + IDatabase $dbw, + Config $config ) { - $user = $context->getUser(); - // Convert from object to array $newRow = get_object_vars( $newRow ); @@ -1113,7 +1115,7 @@ class AbuseFilter { } '@phan-var int $new_id'; - $availableActions = $context->getConfig()->get( 'AbuseFilterActions' ); + $availableActions = $config->get( 'AbuseFilterActions' ); $actionsRows = []; foreach ( array_filter( $availableActions ) as $action => $_ ) { // Check if it's set diff --git a/includes/Views/AbuseFilterViewEdit.php b/includes/Views/AbuseFilterViewEdit.php index 23794631d..10acf1809 100644 --- a/includes/Views/AbuseFilterViewEdit.php +++ b/includes/Views/AbuseFilterViewEdit.php @@ -104,6 +104,7 @@ class AbuseFilterViewEdit extends AbuseFilterView { ); return; } + // Note, this is [ $row, $actions, $originalRow, $originalActions ] $data = $status->getValue(); } else { $data = $this->loadFromDatabase( $filter, $history_id ); @@ -128,9 +129,12 @@ class AbuseFilterViewEdit extends AbuseFilterView { // In the current implementation, this cannot happen. throw new LogicException( 'Should always be able to retrieve data for saving' ); } - list( $newRow, $actions ) = $reqStatus->getValue(); + [ $newRow, $actions, $origRow, $origActions ] = $reqStatus->getValue(); $dbw = wfGetDB( DB_MASTER ); - $status = AbuseFilter::saveFilter( $this, $filter, $newRow, $actions, $dbw ); + $status = AbuseFilter::saveFilter( + $this->getUser(), $filter, $newRow, $actions, + $origRow, $origActions, $dbw, $this->getConfig() + ); if ( !$status->isGood() ) { $err = $status->getErrors(); @@ -1209,8 +1213,6 @@ class AbuseFilterViewEdit extends AbuseFilterView { 'af_throttled' => $origRow->af_throttled, 'af_hit_count' => $origRow->af_hit_count, ]; - $row->mOriginalRow = $origRow; - $row->mOriginalActions = $origActions; // Check for importing $import = $request->getVal( 'wpImportText' ); @@ -1338,7 +1340,7 @@ class AbuseFilterViewEdit extends AbuseFilterView { $row->af_actions = implode( ',', array_keys( $actions ) ); - return Status::newGood( [ $row, $actions ] ); + return Status::newGood( [ $row, $actions, $origRow, $origActions ] ); } /** diff --git a/tests/phpunit/AbuseFilterSaveTest.php b/tests/phpunit/AbuseFilterSaveTest.php index e5e287c48..db30d1e5d 100644 --- a/tests/phpunit/AbuseFilterSaveTest.php +++ b/tests/phpunit/AbuseFilterSaveTest.php @@ -22,7 +22,6 @@ * @license GPL-2.0-or-later */ -use MediaWiki\Linker\LinkRenderer; use PHPUnit\Framework\MockObject\MockObject; use Wikimedia\Rdbms\IDatabase; @@ -30,12 +29,9 @@ use Wikimedia\Rdbms\IDatabase; * @group Test * @group AbuseFilter * @group AbuseFilterSave - * - * @covers AbuseFilter - * @covers AbuseFilterViewEdit */ class AbuseFilterSaveTest extends MediaWikiTestCase { - private static $defaultFilterRow = [ + private const DEFAULT_ABUSE_FILTER_ROW = [ 'af_pattern' => '/**/', 'af_user' => 0, 'af_user_text' => 'FilterTester', @@ -53,18 +49,9 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ]; /** - * Gets an instance of AbuseFilterViewEdit ready for creating or editing filter - * - * @param User $user - * @param array $params - * @param bool $existing Whether the filter already exists - * @return AbuseFilterViewEdit|MockObject + * @return HashConfig */ - private function getViewEdit( User $user, array $params, $existing ) { - $special = new SpecialAbuseFilter(); - $context = new RequestContext(); - $context->setRequest( $this->getRequest( $params ) ); - $context->setUser( $user ); + private function getConfig() : HashConfig { $cfgOpts = [ 'LanguageCode' => 'en', 'AbuseFilterActions' => [ @@ -86,113 +73,49 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'AbuseFilterIsCentral' => true, ]; - - $context->setConfig( new HashConfig( $cfgOpts ) ); - - $special->setContext( $context ); - $filter = $params['id']; - /** @var LinkRenderer|MockObject $lr */ - $lr = $this->getMockBuilder( LinkRenderer::class ) - ->disableOriginalConstructor() - ->getMock(); - $special->setLinkRenderer( $lr ); - /** @var AbuseFilterViewEdit|MockObject $viewEdit */ - $viewEdit = $this->getMockBuilder( AbuseFilterViewEdit::class ) - ->setConstructorArgs( [ $special, [ 'filter' => $filter ] ] ) - ->setMethods( [ 'loadFilterData' ] ) - ->getMock(); - - if ( $existing ) { - $origValues = [ (object)( self::$defaultFilterRow + [ 'af_id' => 1 ] ), [] ]; - } else { - $origValues = [ - (object)[ - 'af_pattern' => '', - 'af_enabled' => 1, - 'af_hidden' => 0, - 'af_global' => 0, - 'af_throttled' => 0, - 'af_hit_count' => 0, - ], - [] - ]; - } - $viewEdit->expects( $this->once() ) - ->method( 'loadFilterData' ) - ->willReturn( $origValues ); - - return $viewEdit; - } - - /** - * Creates a FauxRequest object - * - * @param array $params - * @return FauxRequest - */ - private function getRequest( array $params ) { - $reqParams = [ - 'wpFilterRules' => $params['rules'], - 'wpFilterDescription' => $params['description'], - 'wpFilterNotes' => $params['notes'] ?? '', - 'wpFilterGroup' => $params['group'] ?? 'default', - 'wpFilterEnabled' => $params['enabled'] ?? true, - 'wpFilterHidden' => $params['hidden'] ?? false, - 'wpFilterDeleted' => $params['deleted'] ?? false, - 'wpFilterGlobal' => $params['global'] ?? false, - 'wpFilterActionThrottle' => $params['throttleEnabled'] ?? false, - 'wpFilterThrottleCount' => $params['throttleCount'] ?? 0, - 'wpFilterThrottlePeriod' => $params['throttlePeriod'] ?? 0, - 'wpFilterThrottleGroups' => $params['throttleGroups'] ?? '', - 'wpFilterActionWarn' => $params['warnEnabled'] ?? false, - 'wpFilterWarnMessage' => $params['warnMessage'] ?? 'abusefilter-warning', - 'wpFilterWarnMessageOther' => $params['warnMessageOther'] ?? '', - 'wpFilterActionDisallow' => $params['disallowEnabled'] ?? false, - 'wpFilterDisallowMessage' => $params['disallowMessage'] ?? 'abusefilter-disallowed', - 'wpFilterDisallowMessageOther' => $params['disallowMessageOther'] ?? '', - 'wpFilterActionBlockautopromote' => $params['blockautopromoteEnabled'] ?? false, - 'wpFilterActionDegroup' => $params['degroupEnabled'] ?? false, - 'wpFilterActionBlock' => $params['blockEnabled'] ?? false, - 'wpFilterBlockTalk' => $params['blockTalk'] ?? false, - 'wpBlockAnonDuration' => $params['blockAnons'] ?? 'infinity', - 'wpBlockUserDuration' => $params['blockUsers'] ?? 'infinity', - 'wpFilterActionRangeblock' => $params['rangeblockEnabled'] ?? false, - 'wpFilterActionTag' => $params['tagEnabled'] ?? false, - 'wpFilterTags' => $params['tagTags'] ?? '', - ]; - - // Checkboxes aren't included at all if they aren't selected. We can remove them - // this way (instead of iterating a hardcoded list) since they're the only false values - $reqParams = array_filter( $reqParams, function ( $el ) { - return $el !== false; - } ); - - return new FauxRequest( $reqParams, true ); + return new HashConfig( $cfgOpts ); } /** * @param array $testPerms * @return User|MockObject */ - private function getUserMock( $testPerms ) { + private function getUserMock( array $testPerms ) { $perms = array_merge( $testPerms, [ 'abusefilter-modify' ] ); - $user = $this->getMockBuilder( User::class ) - ->setMethods( [ 'getBlock', 'getName', 'getId', 'getActorId' ] ) - ->getMock(); - - $user->expects( $this->any() ) - ->method( 'getName' ) - ->willReturn( 'FilterUser' ); - $user->expects( $this->any() ) - ->method( 'getId' ) - ->willReturn( 1 ); - $user->expects( $this->any() ) - ->method( 'getActorId' ) - ->willReturn( 1 ); + /** @var User|MockObject $user */ + $user = $this->createMock( User::class ); + $user->method( 'getName' )->willReturn( 'FilterUser' ); + $user->method( 'getId' )->willReturn( 1 ); + $user->method( 'getActorId' )->willReturn( 1 ); $this->overrideUserPermissions( $user, $perms ); return $user; } + /** + * @param array $args + * @return array + */ + private function getRowAndActionsFromTestSpecs( array $args ) : array { + $newRow = (object)( $args['row'] + self::DEFAULT_ABUSE_FILTER_ROW ); + $actions = $args['actions'] ?? []; + + $existing = isset( $args['testData']['existing'] ); + if ( $existing ) { + $origRow = (object)( self::DEFAULT_ABUSE_FILTER_ROW + [ 'af_id' => 1 ] ); + } else { + $origRow = (object)[ + 'af_pattern' => '', + 'af_enabled' => 1, + 'af_hidden' => 0, + 'af_global' => 0, + 'af_throttled' => 0, + 'af_hit_count' => 0, + ]; + } + + return [ $newRow, $actions, $origRow, [] ]; + } + /** * Validate and save a filter given its parameters * @@ -203,51 +126,39 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { public function testSaveFilter( $args ) { $user = $this->getUserMock( $args['testData']['userPerms'] ?? [] ); - $params = $args['filterParameters']; - $filter = $params['id'] = $params['id'] ?? 'new'; - $existing = isset( $args['testData']['existing'] ); - $viewEdit = $this->getViewEdit( $user, $params, $existing ); - - $reqStatus = $viewEdit->loadRequest( $filter ); - if ( !$reqStatus->isGood() ) { - $this->fail( 'Cannot retrieve request data correctly' ); - } - list( $newRow, $actions ) = $reqStatus->getValue(); + $filter = $args['row']['af_id'] = $args['row']['af_id'] ?? 'new'; + [ $newRow, $actions, $origRow, $origActions ] = $this->getRowAndActionsFromTestSpecs( $args ); /** @var IDatabase|MockObject $dbw */ - $dbw = $this->getMockForAbstractClass( IDatabase::class ); - $dbw->expects( $this->any() ) - ->method( 'insertId' ) - ->willReturn( 1 ); - $row = new stdClass(); - $row->actor_id = '1'; - $dbw->expects( $this->any() ) - ->method( 'selectRow' ) - ->willReturn( $row ); - $status = AbuseFilter::saveFilter( $viewEdit, $filter, $newRow, $actions, $dbw ); + $dbw = $this->createMock( IDatabase::class ); + $dbw->method( 'insertId' )->willReturn( 1 ); + // This is needed because of the ManualLogEntry usage + $dbw->method( 'selectRow' )->willReturn( (object)[ 'actor_id' => '1' ] ); + $status = AbuseFilter::saveFilter( + $user, $filter, $newRow, $actions, $origRow, + $origActions, $dbw, $this->getConfig() + ); if ( $args['testData']['shouldFail'] ) { $this->assertFalse( $status->isGood(), 'The filter validation returned a valid status.' ); $actual = $status->getErrors()[0]['message']; $expected = $args['testData']['expectedMessage']; $this->assertEquals( $expected, $actual ); + } elseif ( $args['testData']['shouldBeSaved'] ) { + $this->assertTrue( + $status->isGood(), + "Save failed with status: $status" + ); + $value = $status->getValue(); + $this->assertIsArray( $value ); + $this->assertCount( 2, $value ); + $this->assertContainsOnly( 'int', $value ); } else { - if ( $args['testData']['shouldBeSaved'] ) { - $this->assertTrue( - $status->isGood(), - "Save failed with status: $status" - ); - $value = $status->getValue(); - $this->assertIsArray( $value ); - $this->assertCount( 2, $value ); - $this->assertContainsOnly( 'int', $value ); - } else { - $this->assertTrue( - $status->isGood(), - "Got a non-good status: $status" - ); - $this->assertFalse( $status->getValue(), 'Status value should be false' ); - } + $this->assertTrue( + $status->isGood(), + "Got a non-good status: $status" + ); + $this->assertFalse( $status->getValue(), 'Status value should be false' ); } } @@ -255,14 +166,16 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { * Data provider for creating and editing filters. * @return array */ - public function provideFilters() { + public function provideFilters() : array { return [ 'Fail due to empty description and rules' => [ [ - 'filterParameters' => [ - 'rules' => '', - 'description' => '', - 'blockautopromoteEnabled' => true, + 'row' => [ + 'af_pattern' => '', + 'af_public_comments' => '', + ], + 'actions' => [ + 'blockautopromote' => [] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-missingfields', @@ -273,11 +186,11 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Success for only rules and description' => [ [ - 'filterParameters' => [ - 'rules' => '/* My rules */', - 'description' => 'Some new filter', - 'enabled' => false, - 'deleted' => true, + 'row' => [ + 'af_pattern' => '/* My rules */', + 'af_public_comments' => 'Some new filter', + 'af_enabled' => false, + 'af_deleted' => true ], 'testData' => [ 'shouldFail' => false, @@ -287,12 +200,12 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to syntax error' => [ [ - 'filterParameters' => [ - 'rules' => 'rlike', - 'description' => 'This syntax aint good', - 'blockEnabled' => true, - 'blockTalk' => true, - 'blockAnons' => '8 hours', + 'row' => [ + 'af_pattern' => 'rlike', + 'af_public_comments' => 'This syntax aint good', + ], + 'actions' => [ + 'block' => [ true, '8 hours', '8 hours' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-badsyntax', @@ -303,13 +216,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to both "enabled" and "deleted" selected' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Enabled and deleted', - 'deleted' => true, - 'blockEnabled' => true, - 'blockTalk' => true, - 'blockAnons' => '8 hours', + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Enabled and deleted', + 'af_deleted' => true + ], + 'actions' => [ + 'block' => [ true, '8 hours', '8 hours' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-deleting-enabled', @@ -320,13 +233,14 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to a reserved tag' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Reserved tag', - 'notes' => 'Some notes', - 'hidden' => true, - 'tagEnabled' => true, - 'tagTags' => 'mw-undo' + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Reserved tag', + 'af_comments' => 'Some notes', + 'af_hidden' => true + ], + 'actions' => [ + 'tag' => [ 'mw-undo' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-bad-tags', @@ -337,12 +251,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to an invalid tag' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Invalid tag', - 'notes' => 'Some notes', - 'tagEnabled' => true, - 'tagTags' => 'some|tag' + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Invalid tag', + 'af_comments' => 'Some notes', + ], + 'actions' => [ + 'tag' => [ 'invalid|tag' ] ], 'testData' => [ 'expectedMessage' => 'tags-create-invalid-chars', @@ -353,12 +268,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to an empty tag' => [ [ - 'filterParameters' => [ - 'rules' => '1!=0', - 'description' => 'Empty tag', - 'notes' => '', - 'tagEnabled' => true, - 'tagTags' => '' + 'row' => [ + 'af_pattern' => '1!=0', + 'af_public_comments' => 'Empty tag', + 'af_comments' => '', + ], + 'actions' => [ + 'tag' => [ '' ] ], 'testData' => [ 'expectedMessage' => 'tags-create-no-name', @@ -369,11 +285,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to lack of modify-global right' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Global without perms', - 'global' => true, - 'disallowEnabled' => true, + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Global without perms', + 'af_global' => true, + ], + 'actions' => [ + 'disallow' => [ 'abusefilter-disallowed' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-notallowed-global', @@ -384,12 +302,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to custom warn message on global filter' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Global with invalid warn message', - 'global' => true, - 'warnEnabled' => true, - 'warnMessage' => 'abusefilter-beautiful-warning', + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Global with invalid warn message', + 'af_global' => true, + ], + 'actions' => [ + 'warn' => [ 'abusefilter-beautiful-warning' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-notallowed-global-custom-msg', @@ -401,12 +320,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to custom disallow message on global filter' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Global with invalid disallow message', - 'global' => true, - 'disallowEnabled' => true, - 'disallowMessage' => 'abusefilter-disallowed-something', + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Global with invalid disallow message', + 'af_global' => true, + ], + 'actions' => [ + 'disallow' => [ 'abusefilter-disallowed-something' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-notallowed-global-custom-msg', @@ -418,10 +338,12 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to a restricted action' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Restricted action', - 'degroupEnabled' => true, + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Restricted action', + ], + 'actions' => [ + 'degroup' => [] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-restricted', @@ -432,10 +354,10 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Pass validation but do not save when there are no changes' => [ [ - 'filterParameters' => [ - 'id' => '1', - 'rules' => '/**/', - 'description' => 'Mock filter', + 'row' => [ + 'af_id' => '1', + 'af_pattern' => '/**/', + 'af_public_comments' => 'Mock filter' ], 'testData' => [ 'shouldFail' => false, @@ -446,14 +368,13 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to invalid throttle groups' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Invalid throttle groups', - 'notes' => 'Throttle... Again', - 'throttleEnabled' => true, - 'throttleCount' => 11, - 'throttlePeriod' => 111, - 'throttleGroups' => 'user\nfoo' + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Invalid throttle groups', + 'af_comments' => 'Throttle... Again', + ], + 'actions' => [ + 'throttle' => [ 'new', '11,111', "user\nfoo" ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-invalid-throttlegroups', @@ -464,11 +385,12 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to empty warning message' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Empty warning message', - 'warnEnabled' => true, - 'warnMessage' => '', + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Empty warning message', + ], + 'actions' => [ + 'warn' => [ '' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-invalid-warn-message', @@ -479,11 +401,12 @@ class AbuseFilterSaveTest extends MediaWikiTestCase { ], 'Fail due to empty disallow message' => [ [ - 'filterParameters' => [ - 'rules' => '1==1', - 'description' => 'Empty disallow message', - 'disallowEnabled' => true, - 'disallowMessage' => '', + 'row' => [ + 'af_pattern' => '1==1', + 'af_public_comments' => 'Empty disallow message', + ], + 'actions' => [ + 'disallow' => [ '' ] ], 'testData' => [ 'expectedMessage' => 'abusefilter-edit-invalid-disallow-message',