Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "Improve DeletePage tests"

Browse source
This commit is contained in:
jenkins-bot 2021-09-09 21:10:39 +00:00 committed by Gerrit Code Review
commit 6ae92c0c73
4 changed files with 189 additions and 100 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
includes/page/DeletePage.php
View file

@ -10,7 +10,6 @@ use DeferrableUpdate;
use DeferredUpdates;
use DeletePageJob;
use Exception;
use InvalidArgumentException;
use JobQueueGroup;
use LinksDeletionUpdate;
use LinksUpdate;
@ -197,6 +196,7 @@ class DeletePage {
/**
* @internal FIXME: Hack used when running the DeletePage unit test to disable some legacy code.
* @codeCoverageIgnore
* @param bool $test
*/
public function setIsDeletePageUnitTest( bool $test ): void {
@ -450,7 +450,7 @@ class DeletePage {
$dbw->endAtomic( __METHOD__ );
$this->doDeleteUpdates( $id, $revisionRecord );
$this->doDeleteUpdates( $revisionRecord );
$legacyDeleter = $this->userFactory->newFromAuthority( $this->deleter );
// TODO Replace hook (replace typehints)
@ -592,16 +592,11 @@ class DeletePage {
* @private Public for BC only
* Do some database updates after deletion
*
* @param int $id The page_id value of the page being deleted
* @param RevisionRecord $revRecord The current page revision at the time of
* deletion, used when determining the required updates. This may be needed because
* $this->page->getRevisionRecord() may already return null when the page proper was deleted.
*/
public function doDeleteUpdates( int $id, RevisionRecord $revRecord ): void {
if ( $id !== $this->page->getId() ) {
throw new InvalidArgumentException( 'Mismatching page ID' );
}
public function doDeleteUpdates( RevisionRecord $revRecord ): void {
try {
$countable = $this->page->isCountable();
} catch ( Exception $ex ) {
@ -652,7 +647,7 @@ class DeletePage {
if ( !$this->isDeletePageUnitTest ) {
// TODO Remove conditional once DeferredUpdates is servicified (T265749)
// Search engine
DeferredUpdates::addUpdate( new SearchUpdate( $id, $this->page->getTitle() ) );
DeferredUpdates::addUpdate( new SearchUpdate( $this->page->getId(), $this->page->getTitle() ) );
}
}

6
includes/page/WikiPage.php
View file

@ -2776,6 +2776,10 @@ class WikiPage implements Page, IDBAccessObject, PageRecord {
if ( !$revRecord ) {
throw new BadMethodCallException( __METHOD__ . ' now requires a RevisionRecord' );
}
if ( $id !== $this->getId() ) {
throw new InvalidArgumentException( 'Mismatching page ID' );
}
$user = $user ?? new UserIdentityValue( 0, 'unknown' );
$services = MediaWikiServices::getInstance();
$deletePage = $services->getDeletePageFactory()->newDeletePage(
@ -2783,7 +2787,7 @@ class WikiPage implements Page, IDBAccessObject, PageRecord {
$services->getUserFactory()->newFromUserIdentity( $user )
);
$deletePage->doDeleteUpdates( $id, $revRecord );
$deletePage->doDeleteUpdates( $revRecord );
}
/**

24
tests/phpunit/includes/page/WikiPageDbTest.php
View file

@ -658,19 +658,31 @@ class WikiPageDbTest extends MediaWikiLangTestCase {
$commentQuery['joins']
);
$archive = new PageArchive( $page->getTitle(), MediaWikiServices::getInstance()->getMainConfig() );
$archivedRevs = $archive->listRevisions();
if ( !$archivedRevs || $archivedRevs->numRows() !== 1 ) {
$this->fail( 'Unexpected number of archived revisions' );
}
$archivedRev = MediaWikiServices::getInstance()->getRevisionStore()
->newRevisionFromArchiveRow( $archivedRevs->current() );
$this->assertNull(
$page->getContent( RevisionRecord::FOR_PUBLIC ),
"WikiPage::getContent should return null after the page was suppressed for general users"
$archivedRev->getContent( SlotRecord::MAIN, RevisionRecord::FOR_PUBLIC ),
"Archived content should be null after the page was suppressed for general users"
);
$this->assertNull(
$page->getContent( RevisionRecord::FOR_THIS_USER, $this->getTestUser()->getUser() ),
"WikiPage::getContent should return null after the page was suppressed for individual users"
$archivedRev->getContent(
SlotRecord::MAIN,
RevisionRecord::FOR_THIS_USER,
$this->getTestUser()->getUser()
),
"Archived content should be null after the page was suppressed for individual users"
);
$this->assertNull(
$page->getContent( RevisionRecord::FOR_THIS_USER, $user ),
"WikiPage::getContent should return null after the page was suppressed even for a sysop"
$archivedRev->getContent( SlotRecord::MAIN, RevisionRecord::FOR_THIS_USER, $user ),
"Archived content should be null after the page was suppressed even for a sysop"
);
}

246
tests/phpunit/integration/includes/page/DeletePageTest.php
View file

@ -3,6 +3,7 @@
namespace MediaWiki\Tests\Page;
use CommentStoreComment;
use Content;
use ContentHandler;
use MediaWiki\MediaWikiServices;
use MediaWiki\Page\DeletePage;
@ -10,15 +11,17 @@ use MediaWiki\Page\ProperPageIdentity;
use MediaWiki\Permissions\Authority;
use MediaWiki\Permissions\UltimateAuthority;
use MediaWiki\Revision\RevisionRecord;
use MediaWiki\Revision\SlotRecord;
use MediaWikiIntegrationTestCase;
use PageArchive;
use Title;
use User;
use WikiPage;
/**
* @covers \MediaWiki\Page\DeletePage
* @group Database
* @note Permission-related tests are in \MediaWiki\Tests\Unit\Page\DeletePageTest
* @todo Test all possible mutations
*/
class DeletePageTest extends MediaWikiIntegrationTestCase {
protected $tablesUsed = [
@ -34,6 +37,8 @@ class DeletePageTest extends MediaWikiIntegrationTestCase {
'recentchanges',
'logging',
'pagelinks',
'change_tag',
'change_tag_def',
];
private function getDeletePage( ProperPageIdentity $page, Authority $deleter ): DeletePage {
@ -68,20 +73,94 @@ class DeletePageTest extends MediaWikiIntegrationTestCase {
return $page;
}
public function testDeleteUnsafe() {
$deleterUser = static::getTestSysop()->getUser();
$deleter = new UltimateAuthority( $deleterUser );
$page = $this->createPage( __METHOD__, "[[original text]] foo" );
$id = $page->getId();
private function assertDeletionLogged(
ProperPageIdentity $title,
User $deleter,
string $reason,
bool $suppress,
string $logSubtype,
int $logID
): void {
$commentQuery = MediaWikiServices::getInstance()->getCommentStore()->getJoin( 'log_comment' );
$this->assertSelect(
[ 'logging' ] + $commentQuery['tables'],
[
'log_type',
'log_action',
'log_comment' => $commentQuery['fields']['log_comment_text'],
'log_actor',
'log_namespace',
'log_title',
],
[ 'log_id' => $logID ],
[ [
$suppress ? 'suppress' : 'delete',
$logSubtype,
$reason,
(string)$deleter->getActorId(),
(string)$title->getNamespace(),
$title->getDBkey(),
] ],
[],
$commentQuery['joins']
);
}
$reason = "testing deletion";
$status = $this->getDeletePage( $page, $deleter )->deleteUnsafe( $reason );
private function assertArchiveVisibility( Title $title, bool $suppression ): void {
if ( !$suppression ) {
// Archived revisions are considered "deleted" only when suppressed, so we'd always get a content
// in case of normal deletion.
return;
}
$archive = new PageArchive( $title, MediaWikiServices::getInstance()->getMainConfig() );
$archivedRevs = $archive->listRevisions();
if ( !$archivedRevs || $archivedRevs->numRows() !== 1 ) {
$this->fail( 'Unexpected number of archived revisions' );
}
$archivedRev = MediaWikiServices::getInstance()->getRevisionStore()
->newRevisionFromArchiveRow( $archivedRevs->current() );
$this->assertFalse(
$page->getTitle()->getArticleID() > 0,
$this->assertNotNull(
$archivedRev->getContent( SlotRecord::MAIN, RevisionRecord::RAW ),
"Archived content should be there"
);
$this->assertNull(
$archivedRev->getContent( SlotRecord::MAIN, RevisionRecord::FOR_PUBLIC ),
"Archived content should be null after the page was suppressed for general users"
);
$getContentForUser = static function ( Authority $user ) use ( $archivedRev ): ?Content {
return $archivedRev->getContent(
SlotRecord::MAIN,
RevisionRecord::FOR_THIS_USER,
$user
);
};
$this->assertNull(
$getContentForUser( static::getTestUser()->getUser() ),
"Archived content should be null after the page was suppressed for individual users"
);
$this->assertNull(
$getContentForUser( static::getTestSysop()->getUser() ),
"Archived content should be null after the page was suppressed even for a sysop"
);
$this->assertNotNull(
$getContentForUser( static::getTestUser( [ 'suppress' ] )->getUser() ),
"Archived content should be visible after the page was suppressed for an oversighter"
);
}
private function assertPageObjectsConsistency( WikiPage $page ): void {
$this->assertSame(
0,
$page->getTitle()->getArticleID(),
"Title object should now have page id 0"
);
$this->assertFalse( $page->getId() > 0, "WikiPage should now have page id 0" );
$this->assertSame( 0, $page->getId(), "WikiPage should now have page id 0" );
$this->assertFalse(
$page->exists(),
"WikiPage::exists should return false after page was deleted"
@ -96,92 +175,96 @@ class DeletePageTest extends MediaWikiIntegrationTestCase {
$t->exists(),
"Title::exists should return false after page was deleted"
);
}
$this->runJobs();
private function assertPageLinksUpdate( int $pageID, bool $shouldRunJobs ): void {
if ( $shouldRunJobs ) {
$this->runJobs();
}
$dbr = wfGetDB( DB_REPLICA );
$res = $dbr->select( 'pagelinks', '*', [ 'pl_from' => $id ] );
$res = $dbr->select( 'pagelinks', '*', [ 'pl_from' => $pageID ] );
$this->assertSame( 0, $res->numRows(), 'pagelinks should contain no more links from the page' );
}
// Test deletion logging
$logId = $status->getValue();
$commentQuery = MediaWikiServices::getInstance()->getCommentStore()->getJoin( 'log_comment' );
$this->assertSelect(
[ 'logging' ] + $commentQuery['tables'],
[
'log_type',
'log_action',
'log_comment' => $commentQuery['fields']['log_comment_text'],
'log_actor',
'log_namespace',
'log_title',
],
[ 'log_id' => $logId ],
[ [
'delete',
'delete',
$reason,
(string)$deleterUser->getActorId(),
(string)$page->getTitle()->getNamespace(),
$page->getTitle()->getDBkey(),
] ],
[],
$commentQuery['joins']
private function assertDeletionTags( int $logId, array $tags ): void {
if ( !$tags ) {
return;
}
$actualTags = wfGetDB( DB_REPLICA )->selectFieldValues(
'change_tag',
'ct_tag_id',
[ 'ct_log_id' => $logId ]
);
$changeTagDefStore = MediaWikiServices::getInstance()->getChangeTagDefStore();
$expectedTags = array_map( [ $changeTagDefStore, 'acquireId' ], $tags );
$this->assertArrayEquals( $expectedTags, array_map( 'intval', $actualTags ) );
}
/**
* @todo Merge in testDeleteUnsafe
* @dataProvider provideDeleteUnsafe
*/
public function testDeleteUnsafe__suppress() {
public function testDeleteUnsafe( bool $suppress, array $tags, bool $immediate, string $logSubtype ) {
$deleterUser = static::getTestSysop()->getUser();
$deleter = new UltimateAuthority( $deleterUser );
$page = $this->createPage( __METHOD__, "[[original text]] foo" );
$id = $page->getId();
if ( !$immediate ) {
// Ensure that the job queue can be used
$this->setMwGlobals( [
'wgDeleteRevisionsBatchSize' => 1
] );
$this->editPage( $page, "second revision" );
}
$reason = "testing deletion";
$status = $this->getDeletePage( $page, $deleter )->setSuppress( true )->deleteUnsafe( $reason );
$status = $this->getDeletePage( $page, $deleter )
->setSuppress( $suppress )
->setTags( $tags )
->forceImmediate( $immediate )
->setLogSubtype( $logSubtype )
->deleteUnsafe( $reason );
// Test suppression logging
$logId = $status->getValue();
$commentQuery = MediaWikiServices::getInstance()->getCommentStore()->getJoin( 'log_comment' );
$this->assertSelect(
[ 'logging' ] + $commentQuery['tables'],
[
'log_type',
'log_action',
'log_comment' => $commentQuery['fields']['log_comment_text'],
'log_actor',
'log_namespace',
'log_title',
],
[ 'log_id' => $logId ],
[ [
'suppress',
'delete',
$reason,
(string)$deleterUser->getActorId(),
(string)$page->getTitle()->getNamespace(),
$page->getTitle()->getDBkey(),
] ],
[],
$commentQuery['joins']
);
$this->assertTrue( $status->isGood(), 'Deletion should succeed' );
$this->assertNull(
$page->getContent( RevisionRecord::FOR_PUBLIC ),
"WikiPage::getContent should return null after the page was suppressed for general users"
);
if ( $immediate ) {
$this->assertIsInt( $status->getValue() );
$logID = $status->getValue();
} else {
$this->assertFalse( $status->getValue() );
$this->runJobs();
$logID = wfGetDB( DB_REPLICA )->selectField(
'logging',
'log_id',
[
'log_type' => $suppress ? 'suppress' : 'delete',
'log_namespace' => $page->getNamespace(),
'log_title' => $page->getDBkey()
]
);
$this->assertNotFalse( $logID, 'Should have a log ID now' );
$logID = (int)$logID;
// Clear caches.
$page->getTitle()->resetArticleID( false );
$page->clear();
}
$this->assertNull(
$page->getContent( RevisionRecord::FOR_THIS_USER, static::getTestUser()->getUser() ),
"WikiPage::getContent should return null after the page was suppressed for individual users"
);
$this->assertPageObjectsConsistency( $page );
$this->assertArchiveVisibility( $page->getTitle(), $suppress );
$this->assertDeletionLogged( $page, $deleterUser, $reason, $suppress, $logSubtype, $logID );
$this->assertDeletionTags( $logID, $tags );
$this->assertPageLinksUpdate( $id, $immediate );
}
$this->assertNull(
$page->getContent( RevisionRecord::FOR_THIS_USER, $deleterUser ),
"WikiPage::getContent should return null after the page was suppressed even for a sysop"
);
public function provideDeleteUnsafe(): iterable {
// Note that we're using immediate deletion as default
yield 'standard deletion' => [ false, [], true, 'delete' ];
yield 'suppression' => [ true, [], true, 'delete' ];
yield 'deletion with tags' => [ false, [ 'tag-foo', 'tag-bar' ], true, 'delete' ];
yield 'custom deletion log' => [ false, [], true, 'custom-del-log' ];
yield 'queued deletion' => [ false, [], false, 'delete' ];
}
/**
@ -196,12 +279,7 @@ class DeletePageTest extends MediaWikiIntegrationTestCase {
// Similar to MovePage logic
wfGetDB( DB_PRIMARY )->delete( 'page', [ 'page_id' => $id ], __METHOD__ );
$this->getDeletePage( $page, $user )->doDeleteUpdates( $id, $page->getRevisionRecord() );
$this->runJobs();
$dbr = wfGetDB( DB_REPLICA );
$res = $dbr->select( 'pagelinks', '*', [ 'pl_from' => $id ] );
$this->assertSame( 0, $res->numRows(), 'pagelinks should contain no more links from the page' );
$this->getDeletePage( $page, $user )->doDeleteUpdates( $page->getRevisionRecord() );
$this->assertPageLinksUpdate( $id, true );
}
}