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

Merge "Simplify AbuseFilterBlockTest"

Browse source
This commit is contained in:
jenkins-bot 2021-03-06 09:23:26 +00:00 committed by Gerrit Code Review
parent 51f37697dc 92ecccbdc7
commit 12f4e81964
3 changed files with 26 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
includes/Consequences/Consequence/Block.php
View file

@ -4,7 +4,6 @@ namespace MediaWiki\Extension\AbuseFilter\Consequences\Consequence;
use ManualLogEntry;
use MediaWiki\Block\BlockUserFactory;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Block\DatabaseBlockStore;
use MediaWiki\Extension\AbuseFilter\Consequences\Parameters;
use MediaWiki\Extension\AbuseFilter\FilterUser;
@ -21,6 +20,8 @@ class Block extends BlockingConsequence implements ReversibleConsequence {
private $preventsTalkEdit;
/** @var DatabaseBlockStore */
private $databaseBlockStore;
/** @var callable */
private $blockFactory;
/**
* @param Parameters $params
@ -28,6 +29,7 @@ class Block extends BlockingConsequence implements ReversibleConsequence {
* @param bool $preventTalkEdit
* @param BlockUserFactory $blockUserFactory
* @param DatabaseBlockStore $databaseBlockStore
* @param callable $blockFactory Should take a user name and return a DatabaseBlock or null.
* @param FilterUser $filterUser
* @param MessageLocalizer $messageLocalizer
*/
@ -37,12 +39,14 @@ class Block extends BlockingConsequence implements ReversibleConsequence {
bool $preventTalkEdit,
BlockUserFactory $blockUserFactory,
DatabaseBlockStore $databaseBlockStore,
callable $blockFactory,
FilterUser $filterUser,
MessageLocalizer $messageLocalizer
) {
parent::__construct( $params, $expiry, $blockUserFactory, $filterUser, $messageLocalizer );
$this->databaseBlockStore = $databaseBlockStore;
$this->preventsTalkEdit = $preventTalkEdit;
$this->blockFactory = $blockFactory;
}
/**
@ -67,8 +71,8 @@ class Block extends BlockingConsequence implements ReversibleConsequence {
* @todo This could use UnblockUser, but we need to check if the block was performed by the AF user
*/
public function revert( $info, UserIdentity $performer, string $reason ): bool {
// TODO: DI once T255433 is resolved
$block = DatabaseBlock::newFromTarget( $this->parameters->getUser()->getName() );
// TODO: Proper DI once T255433 is resolved
$block = ( $this->blockFactory )( $this->parameters->getUser()->getName() );
if ( !( $block && $block->getBy() === $this->filterUser->getUser()->getId() ) ) {
// Not blocked by abuse filter
return false;
@ -80,12 +84,11 @@ class Block extends BlockingConsequence implements ReversibleConsequence {
$logEntry->setTarget( new TitleValue( NS_USER, $this->parameters->getUser()->getName() ) );
$logEntry->setComment( $reason );
$logEntry->setPerformer( $performer );
$id = $logEntry->insert();
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
// This has a bazillion of static dependencies all around the place, and a nightmare to deal with in tests
// TODO: Remove this check once T253717 is resolved
// @codeCoverageIgnoreStart
$logEntry->publish( $id );
$logEntry->publish( $logEntry->insert() );
// @codeCoverageIgnoreEnd
}
return true;

3
includes/Consequences/ConsequencesFactory.php
View file

@ -4,6 +4,7 @@ namespace MediaWiki\Extension\AbuseFilter\Consequences;
use BagOStuff;
use MediaWiki\Block\BlockUserFactory;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Block\DatabaseBlockStore;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\AbuseFilter\BlockAutopromoteStore;
@ -143,6 +144,8 @@ class ConsequencesFactory {
$preventsTalk,
$this->blockUserFactory,
$this->databaseBlockStore,
// FIXME This is a hack until DI is possible here.
[ DatabaseBlock::class, 'newFromTarget' ],
$this->filterUser,
$this->messageLocalizer
);

68
tests/phpunit/AbuseFilterBlockTest.php
View file

@ -2,14 +2,13 @@
use MediaWiki\Block\BlockUser;
use MediaWiki\Block\BlockUserFactory;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Block\DatabaseBlockStore;
use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Block;
use MediaWiki\Extension\AbuseFilter\Consequences\Parameters;
use MediaWiki\Extension\AbuseFilter\FilterUser;
use MediaWiki\User\UserIdentity;
use MediaWiki\User\UserIdentityValue;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILoadBalancer;
/**
* @coversDefaultClass \MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Block
@ -77,6 +76,9 @@ class AbuseFilterBlockTest extends MediaWikiIntegrationTestCase {
$preventsTalkEdit = true,
$blockUserFactory,
$this->createMock( DatabaseBlockStore::class ),
function () {
return null;
},
$this->getFilterUser(),
$this->getMsgLocalizer()
);
@ -94,6 +96,9 @@ class AbuseFilterBlockTest extends MediaWikiIntegrationTestCase {
false,
$this->createMock( BlockUserFactory::class ),
$this->createMock( DatabaseBlockStore::class ),
function () {
return null;
},
$this->createMock( FilterUser::class ),
$this->getMsgLocalizer()
);
@ -103,70 +108,24 @@ class AbuseFilterBlockTest extends MediaWikiIntegrationTestCase {
public function provideRevert() {
yield 'no block to revert' => [ null, null, false ];
$getMockRow = function ( UserIdentity $performer ) {
return new class( $performer ) extends stdClass {
private $performer;
public function __construct( UserIdentity $performer ) {
$this->performer = $performer;
}
public function __get( string $prop ) {
switch ( $prop ) {
case 'ipb_by':
return $this->performer->getId();
case 'ipb_by_text':
return $this->performer->getName();
case 'ipb_by_actor':
return $this->performer->getActorId();
case 'ipb_auto':
return false;
default:
return 'foo';
}
}
};
};
$randomUser = new UserIdentityValue( 1234, 'Some other user', 3456 );
yield 'not blocked by AF user' => [ $getMockRow( $randomUser ), null, false ];
yield 'not blocked by AF user' => [ new DatabaseBlock( [ 'by' => $randomUser->getUserId() ] ), null, false ];
$filterUserIdentity = $this->getFilterUser()->getUser();
$blockByFilter = new DatabaseBlock( [ 'by' => $this->getFilterUser()->getUser()->getUserId() ] );
$failBlockStore = $this->createMock( DatabaseBlockStore::class );
$failBlockStore->expects( $this->once() )->method( 'deleteBlock' )->willReturn( false );
yield 'cannot delete block' => [ $getMockRow( $filterUserIdentity ), $failBlockStore, false ];
yield 'cannot delete block' => [ $blockByFilter, $failBlockStore, false ];
$succeedBlockStore = $this->createMock( DatabaseBlockStore::class );
$succeedBlockStore->expects( $this->once() )->method( 'deleteBlock' )->willReturn( true );
yield 'succeed' => [ $getMockRow( $filterUserIdentity ), $succeedBlockStore, true ];
yield 'succeed' => [ $blockByFilter, $succeedBlockStore, true ];
}
/**
* @covers ::revert
* @dataProvider provideRevert
* @todo This sucks. Clean it up once T255433 and T253717 are resolved
*/
public function testRevert( ?stdClass $blockRow, ?DatabaseBlockStore $blockStore, bool $expected ) {
// Unset all hook handlers per T272124
$this->setService( 'HookContainer', $this->createHookContainer() );
$db = $this->createMock( IDatabase::class );
$db->method( 'select' )->willReturnCallback( function ( $tables ) use ( $blockRow ) {
return in_array( 'ipblocks', $tables, true ) && $blockRow ? [ $blockRow ] : [];
} );
$lb = $this->createMock( ILoadBalancer::class );
$lb->method( 'getMaintenanceConnectionRef' )->willReturn( $db );
$this->setService( 'DBLoadBalancer', $lb );
$commentStore = $this->createMock( CommentStore::class );
$migrationData = [ 'tables' => [], 'fields' => [], 'joins' => [] ];
$commentStore->method( 'getJoin' )->willReturn( $migrationData );
$commentStore->method( 'insert' )->willReturn( [] );
$this->setService( 'CommentStore', $commentStore );
$actorMigration = $this->createMock( ActorMigration::class );
$actorMigration->method( 'getInsertValues' )->willReturn( [] );
$actorMigration->method( 'getJoin' )->willReturn( $migrationData );
$this->setService( 'ActorMigration', $actorMigration );
$this->setService( 'LinkCache', $this->createMock( LinkCache::class ) );
public function testRevert( ?DatabaseBlock $block, ?DatabaseBlockStore $blockStore, bool $expected ) {
$params = $this->createMock( Parameters::class );
$params->method( 'getUser' )->willReturn( new UserIdentityValue( 1, 'Foobar', 2 ) );
$block = new Block(
@ -175,6 +134,9 @@ class AbuseFilterBlockTest extends MediaWikiIntegrationTestCase {
false,
$this->createMock( BlockUserFactory::class ),
$blockStore ?? $this->createMock( DatabaseBlockStore::class ),
function () use ( $block ) {
return $block;
},
$this->getFilterUser(),
$this->getMsgLocalizer()
);