Merge "Add test traits for uploads and account creation"

This commit is contained in:
jenkins-bot 2020-10-05 07:49:38 +00:00 committed by Gerrit Code Review
commit 2e1afd5a51
6 changed files with 167 additions and 98 deletions

View file

@ -189,7 +189,9 @@
},
"TestAutoloadClasses": {
"AbuseFilterConsequencesTest": "tests/phpunit/AbuseFilterConsequencesTest.php",
"AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php"
"AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php",
"AbuseFilterUploadTestTrait": "tests/phpunit/AbuseFilterUploadTestTrait.php",
"AbuseFilterCreateAccountTestTrait": "tests/phpunit/AbuseFilterCreateAccountTestTrait.php"
},
"ResourceModules": {
"ext.abuseFilter": {

View file

@ -40,29 +40,6 @@ class RCVariableGenerator extends VariableGenerator {
$this->contextUser = $contextUser;
}
/**
* Get an instance for a given rc_id.
*
* @todo FIXME this method doesn't appear to have any uses
*
* @param int $id
* @param AbuseFilterVariableHolder $vars
* @param User $contextUser
* @return self|null
*/
public static function newFromId(
int $id,
AbuseFilterVariableHolder $vars,
User $contextUser
) : ?self {
$rc = RecentChange::newFromId( $id );
if ( !$rc ) {
return null;
}
return new self( $vars, $rc, $contextUser );
}
/**
* @return AbuseFilterVariableHolder|null
*/
@ -89,8 +66,10 @@ class RCVariableGenerator extends VariableGenerator {
$this->addEditVarsForRow();
} else {
// @todo Ensure this cannot happen, and throw if it does
// @codeCoverageIgnoreStart
wfLogWarning( 'Cannot understand the given recentchanges row!' );
return null;
// @codeCoverageIgnoreEnd
}
$this->addGenericVars();
@ -178,10 +157,12 @@ class RCVariableGenerator extends VariableGenerator {
$title, [ 'time' => $time, 'private' => $this->contextUser ]
);
if ( !$file ) {
// FixMe This shouldn't happen!
// @fixme Ensure this cannot happen!
// @codeCoverageIgnoreStart
$logger = LoggerFactory::getInstance( 'AbuseFilter' );
$logger->debug( "Cannot find file from RC row with title $title" );
$logger->warning( "Cannot find file from RC row with title $title" );
return $this;
// @codeCoverageIgnoreEnd
}
// This is the same as AbuseFilterHooks::filterUpload, but from a different source

View file

@ -45,6 +45,9 @@ use PHPUnit\Framework\MockObject\MockObject;
* @covers AbuseFilterParser
*/
class AbuseFilterConsequencesTest extends MediaWikiTestCase {
use AbuseFilterCreateAccountTestTrait;
use AbuseFilterUploadTestTrait;
/**
* @var User The user performing actions
*/
@ -74,7 +77,9 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
'logging',
'change_tag',
'user',
'text'
'text',
'image',
'oldimage',
];
// phpcs:disable Generic.Files.LineLength
@ -370,8 +375,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
protected function tearDown() : void {
// Paranoia: ensure no fake timestamp leftover
MWTimestamp::setFakeTime( false );
// Clear any upload
$_FILES = [];
$this->clearUploads();
parent::tearDown();
}
@ -518,35 +522,6 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
return $status;
}
/**
* Upload a file. This is based on ApiUploadTestCase::fakeUploadFile
* @param string $fileName
* @param string $pageText
* @param string $summary
* @return Status
*/
private function doUpload( string $fileName, string $pageText, string $summary ) : Status {
$imgGen = new RandomImageGenerator();
// Use SVG, since the ImageGenerator doesn't need anything special to create it
$format = 'svg';
$mime = 'image/svg+xml';
$filePath = $imgGen->writeImages( 1, $format, $this->getNewTempDirectory() )[0];
clearstatcache();
$_FILES[ 'wpUploadFile' ] = [
'name' => $fileName,
'type' => $mime,
'tmp_name' => $filePath,
'error' => UPLOAD_ERR_OK,
'size' => filesize( $filePath ),
];
$request = new FauxRequest( [
'wpDestFile' => $fileName
] );
$ub = UploadBase::createFromRequest( $request );
$ub->verifyUpload();
return $ub->performUpload( $summary, $pageText, false, $this->user );
}
/**
* Executes an action to filter
*
@ -596,22 +571,11 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
);
break;
case 'createaccount':
$user = User::newFromName( $params['username'] );
// A creatable username must exist to be passed to $logEntry->setPerformer(),
// so create the account.
$user->addToDatabase();
$provider = new AbuseFilterPreAuthenticationProvider();
$status = $provider->testForAccountCreation( $user, $user, [] );
$logEntry = new ManualLogEntry( 'newusers', 'create' );
$logEntry->setPerformer( $user );
$logEntry->setTarget( $user->getUserPage() );
$logid = $logEntry->insert();
$logEntry->publish( $logid );
$status = $this->createAccount( $params['username'] );
break;
case 'upload':
$status = $this->doUpload(
[ $status, $this->clearPath ] = $this->doUpload(
$this->user,
$params['target'],
$params['newText'] ?? 'AbuseFilter test upload',
$params['summary'] ?? 'Test'
@ -653,7 +617,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
}
$logType = $actionParams['action'] === 'createaccount' ? 'newusers' : $actionParams['action'];
$logAction = $logType === 'newusers' ? 'create' : $logType;
$logAction = $logType === 'newusers' ? 'create2' : $logType;
$title = Title::newFromText( $actionParams['target'] );
$id = $this->db->selectField(
'logging',
@ -1019,7 +983,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
[ 5 ],
[
'action' => 'upload',
'target' => 'MyFile.jpg',
'target' => 'MyFile.svg',
],
[ 'tag' => [ 5 ] ]
],
@ -1027,7 +991,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
[ 22 ],
[
'action' => 'upload',
'target' => 'MyFile.jpg',
'target' => 'MyFile.svg',
'newText' => 'Block me please!',
'summary' => 'Asking to be blocked'
],

View file

@ -0,0 +1,43 @@
<?php
/**
* This trait can be used to create accounts in integration tests.
* NOTE: The implementing classes MUST extend MediaWikiIntegrationTestCase
* @todo This might be moved to MediaWikiIntegrationTestCase
*/
trait AbuseFilterCreateAccountTestTrait {
/**
* @param string $accountName
* @param User|null $creator Defaults to the newly created user
* @param bool $autocreate
* @return StatusValue
*/
protected function createAccount(
string $accountName,
User $creator = null,
bool $autocreate = false
) : StatusValue {
$user = User::newFromName( $accountName );
// A creatable username must exist to be passed to $logEntry->setPerformer(),
// so create the account.
$user->addToDatabase();
$creator = $creator ?? $user;
$provider = new AbuseFilterPreAuthenticationProvider();
$status = $provider->testForAccountCreation( $user, $creator, [] );
// FIXME This is a bit hacky, but AuthManager doesn't expose any methods for logging
$subType = $autocreate ? 'autocreate' : 'create2';
$logEntry = new \ManualLogEntry( 'newusers', $subType );
$logEntry->setPerformer( $creator );
$logEntry->setTarget( Title::makeTitle( NS_USER, $accountName ) );
$logEntry->setComment( 'Fooobarcomment' );
$logEntry->setParameters( [
'4::userid' => $user->getId(),
] );
$logid = $logEntry->insert();
$logEntry->publish( $logid );
return $status;
}
}

View file

@ -0,0 +1,67 @@
<?php
use MediaWiki\MediaWikiServices;
/**
* This trait can be used to perform uploads in integration tests.
* NOTE: The implementing classes MUST extend MediaWikiIntegrationTestCase
* The tearDown method must clear $_FILES and the local file
* @todo This might be moved to MediaWikiIntegrationTestCase
*
* @method string getNewTempDirectory()
* @method setMwGlobals($pairs)
*/
trait AbuseFilterUploadTestTrait {
/**
* @var string|null The path represented by this variable will be cleared in tearDown
*/
protected $clearPath;
/**
* Clear any temporary uploads, should be called from tearDown
*/
protected function clearUploads() : void {
$_FILES = [];
if ( $this->clearPath ) {
$backend = MediaWikiServices::getInstance()->getRepoGroup()->getLocalRepo()->getBackend();
$backend->delete( [ 'src' => $this->clearPath ], [ 'force' => 1 ] );
$this->clearPath = null;
}
}
/**
* This is based on ApiUploadTestCase::fakeUploadFile
*
* @param User $user
* @param string $fileName
* @param string $pageText
* @param string $summary
* @return array [ Status, file path ]
*/
protected function doUpload( User $user, string $fileName, string $pageText, string $summary ) : array {
global $wgFileExtensions;
$this->setMwGlobals( [ 'wgFileExtensions' => array_merge( $wgFileExtensions, [ 'svg' ] ) ] );
$imgGen = new RandomImageGenerator();
// Use SVG, since the ImageGenerator doesn't need anything special to create it
$format = 'svg';
$mime = 'image/svg+xml';
$filePath = $imgGen->writeImages( 1, $format, $this->getNewTempDirectory() )[0];
clearstatcache();
$_FILES[ 'wpUploadFile' ] = [
'name' => $fileName,
'type' => $mime,
'tmp_name' => $filePath,
'error' => UPLOAD_ERR_OK,
'size' => filesize( $filePath ),
];
$request = new FauxRequest( [
'wpDestFile' => $fileName
] );
/** @var UploadFromFile $ub */
$ub = UploadBase::createFromRequest( $request );
$ub->verifyUpload();
$status = $ub->performUpload( $summary, $pageText, false, $user );
return [ $status, $ub->getLocalFile()->getPath() ];
}
}

View file

@ -12,12 +12,17 @@ use MediaWiki\MediaWikiServices;
* @group Database
*/
class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
use AbuseFilterCreateAccountTestTrait;
use AbuseFilterUploadTestTrait;
protected $tablesUsed = [
'page',
'text',
'page_restrictions',
'user',
'recentchanges',
'image',
'oldimage',
];
/**
@ -25,6 +30,7 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
*/
protected function tearDown() : void {
MWTimestamp::setFakeTime( false );
$this->clearUploads();
parent::tearDown();
}
@ -259,7 +265,8 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
$expectedValues = [
'user_name' => $user->getName(),
'action' => $action,
'summary' => $summary
'summary' => $summary,
'timestamp' => $timestamp
];
switch ( $type ) {
@ -269,13 +276,11 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
case 'edit':
$newText = 'Some new text for testing RC vars.';
$this->editPage( $title->getText(), $newText, $summary, $title->getNamespace(), $user );
$expectedValues += [
'page_id' => $page->getId(),
'page_namespace' => $title->getNamespace(),
'page_title' => $title->getText(),
'page_prefixedtitle' => $title->getPrefixedText(),
'timestamp' => $timestamp
'page_prefixedtitle' => $title->getPrefixedText()
];
break;
case 'move':
@ -293,8 +298,7 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
'moved_to_id' => $newID,
'moved_to_namespace' => $newTitle->getNamespace(),
'moved_to_title' => $newTitle->getText(),
'moved_to_prefixedtitle' => $newTitle->getPrefixedText(),
'timestamp' => $timestamp
'moved_to_prefixedtitle' => $newTitle->getPrefixedText()
];
break;
case 'delete':
@ -303,24 +307,12 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
'page_id' => $page->getId(),
'page_namespace' => $title->getNamespace(),
'page_title' => $title->getText(),
'page_prefixedtitle' => $title->getPrefixedText(),
'timestamp' => $timestamp
'page_prefixedtitle' => $title->getPrefixedText()
];
break;
case 'newusers':
// FIXME AuthManager doesn't expose the method for doing this. And mocking AuthManager
// and everything here seems overkill.
$accountName = 'AbuseFilter dummy user';
$subType = $action === 'createaccount' ? 'create2' : 'autocreate';
$logEntry = new \ManualLogEntry( 'newusers', $subType );
$logEntry->setPerformer( $user );
$logEntry->setTarget( Title::makeTitle( NS_USER, $accountName ) );
$logEntry->setComment( 'Fooobarcomment' );
$logEntry->setParameters( [
'4::userid' => $user->getId(),
] );
$logid = $logEntry->insert();
$logEntry->publish( $logid );
$this->createAccount( $accountName, $user, $action === 'autocreateaccount' );
$expectedValues = [
'action' => $action,
@ -330,9 +322,29 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
];
break;
case 'upload':
// TODO Same problem as AuthManager, but worse because here we'd really have to
// upload a file (addUploadVars immediately tries reading the file)
$this->markTestIncomplete( 'Implement "upload" case!' );
$fileName = 'My File.svg';
$destTitle = Title::makeTitle( NS_FILE, $fileName );
$page = WikiPage::factory( $destTitle );
[ $status, $this->clearPath ] = $this->doUpload( $user, $fileName, 'Some text', $summary );
if ( !$status->isGood() ) {
throw new LogicException( "Cannot upload file:\n$status" );
}
// Since the SVG is randomly generated, we need to read some properties live
$file = MediaWikiServices::getInstance()->getRepoGroup()->getLocalRepo()->newFile( $destTitle );
$expectedValues += [
'page_id' => $page->getId(),
'page_namespace' => $destTitle->getNamespace(),
'page_title' => $destTitle->getText(),
'page_prefixedtitle' => $destTitle->getPrefixedText(),
'file_sha1' => \Wikimedia\base_convert( $file->getSha1(), 36, 16, 40 ),
'file_size' => $file->getSize(),
'file_mime' => 'image/svg+xml',
'file_mediatype' => 'DRAWING',
'file_width' => $file->getWidth(),
'file_height' => $file->getHeight(),
'file_bits_per_channel' => $file->getBitDepth(),
];
break;
default:
throw new LogicException( "Type $type not recognized!" );