diff --git a/maintenance/deleteOldRevisions.php b/maintenance/deleteOldRevisions.php index cbdcaee3af5..4785ed49a02 100644 --- a/maintenance/deleteOldRevisions.php +++ b/maintenance/deleteOldRevisions.php @@ -36,12 +36,12 @@ class DeleteOldRevisions extends Maintenance { parent::__construct(); $this->addDescription( 'Delete old (non-current) revisions from the database' ); $this->addOption( 'delete', 'Actually perform the deletion' ); - $this->addOption( 'page_id', 'List of page ids to work on', false ); + $this->addArg( 'page_id', 'List of page ids to work on', false, true ); } public function execute() { $this->output( "Delete old revisions\n\n" ); - $this->doDelete( $this->hasOption( 'delete' ), $this->getArgs() ); + $this->doDelete( $this->hasOption( 'delete' ), $this->getArgs( 'page_id' ) ); } private function doDelete( $delete = false, $pageIds = [] ) { diff --git a/maintenance/includes/Maintenance.php b/maintenance/includes/Maintenance.php index afb9f74db4e..6b15a8f1a69 100644 --- a/maintenance/includes/Maintenance.php +++ b/maintenance/includes/Maintenance.php @@ -1034,12 +1034,16 @@ abstract class Maintenance { # Get the IDs of all text records not in these sets $this->output( 'Searching for inactive text records...' ); - $res = $dbw->newSelectQueryBuilder() + $textTableQueryBuilder = $dbw->newSelectQueryBuilder() ->select( 'old_id' ) ->distinct() - ->from( 'text' ) - ->where( $dbw->expr( 'old_id', '!=', $cur ) ) - ->caller( __METHOD__ )->fetchResultSet(); + ->from( 'text' ); + if ( count( $cur ) ) { + $textTableQueryBuilder->where( $dbw->expr( 'old_id', '!=', $cur ) ); + } + $res = $textTableQueryBuilder + ->caller( __METHOD__ ) + ->fetchResultSet(); $old = []; foreach ( $res as $row ) { $old[] = $row->old_id; diff --git a/tests/phpunit/maintenance/DeleteOldRevisionsTest.php b/tests/phpunit/maintenance/DeleteOldRevisionsTest.php new file mode 100644 index 00000000000..83d2d8e18aa --- /dev/null +++ b/tests/phpunit/maintenance/DeleteOldRevisionsTest.php @@ -0,0 +1,110 @@ +expectOutputRegex( $expectedOutputRegex ); + } + + /** @dataProvider provideExecuteForNoPages */ + public function testExecuteForNoPages( $deleteOptionProvided ) { + if ( $deleteOptionProvided ) { + $this->maintenance->setOption( 'delete', 1 ); + } + $this->maintenance->execute(); + $this->expectTheExpectedOutputRegex( false, 0, 0 ); + } + + public static function provideExecuteForNoPages() { + return [ + 'Delete option provided' => [ true ], + 'Delete option not provided' => [ false ], + ]; + } + + private function verifyRevisionCountForPage( WikiPage $page, int $expectedCount ) { + $this->newSelectQueryBuilder() + ->select( 'COUNT(*)' ) + ->from( 'revision' ) + ->where( [ 'rev_page' => $page->getId() ] ) + ->assertFieldValue( $expectedCount ); + } + + /** @dataProvider provideExecute */ + public function testExecute( $shouldDelete, $shouldSpecifyPageId, $expectedOldRevisionsCount ) { + $firstExistingTestPage = $this->getExistingTestPage(); + $secondExistingTestPage = $this->getExistingTestPage(); + $thirdExistingTestPage = $this->getExistingTestPage(); + // Make an edit to both the first and second existing test pages, so that they have old revisions + $this->editPage( $firstExistingTestPage, 'abcdef' ); + $this->editPage( $secondExistingTestPage, 'abcef' ); + // Check that the revision count for each page is correct for the test. + $this->verifyRevisionCountForPage( $thirdExistingTestPage, 1 ); + $this->verifyRevisionCountForPage( $secondExistingTestPage, 2 ); + $this->verifyRevisionCountForPage( $firstExistingTestPage, 2 ); + // Set the options for the maintenance script + if ( $shouldDelete ) { + $this->maintenance->setOption( 'delete', 1 ); + } + if ( $shouldSpecifyPageId ) { + $this->maintenance->setArg( 'page_id', $firstExistingTestPage->getId() ); + } + // Run the maintenance script and then verify the number of revisions for each test page is as expected + $this->maintenance->execute(); + $this->verifyRevisionCountForPage( $thirdExistingTestPage, 1 ); + $this->verifyRevisionCountForPage( + $secondExistingTestPage, + // Should not have been touched, unless --delete was specified and --page_id was not specified + $shouldDelete && !$shouldSpecifyPageId ? 1 : 2 + ); + $this->verifyRevisionCountForPage( + $firstExistingTestPage, + // Should not be touched, unless --delete is specified. + $shouldDelete ? 1 : 2 + ); + $this->expectTheExpectedOutputRegex( + $shouldDelete, + $shouldSpecifyPageId ? $firstExistingTestPage->getId() : 0, + $expectedOldRevisionsCount + ); + } + + public static function provideExecute() { + return [ + 'No options provided' => [ false, false, 2 ], + 'Not deleting, but specified page ID' => [ false, true, 1 ], + 'Deleting without specifying page IDs' => [ true, false, 2 ], + 'Deleting while specifying page ID' => [ true, true, 1 ], + ]; + } +} diff --git a/tests/phpunit/maintenance/MaintenanceTest.php b/tests/phpunit/maintenance/MaintenanceTest.php index f47e127dc6c..ad1462e8541 100644 --- a/tests/phpunit/maintenance/MaintenanceTest.php +++ b/tests/phpunit/maintenance/MaintenanceTest.php @@ -11,6 +11,7 @@ use Wikimedia\TestingAccessWrapper; /** * @covers \Maintenance + * @covers \MediaWiki\Maintenance\MaintenanceFatalError * @group Database */ class MaintenanceTest extends MaintenanceBaseTestCase { @@ -673,4 +674,10 @@ class MaintenanceTest extends MaintenanceBaseTestCase { 'Batch size as 150' => [ 150, true ], ]; } + + public function testPurgeRedundantTextWhenNoPagesExist() { + // Regression test for the method breaking if no rows exist in the content_address table. + $this->maintenance->purgeRedundantText(); + $this->expectOutputRegex( '/0 inactive items found[\s\S]*(?!Deleting)/' ); + } }