diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php index 46b274476f3..08592c45ccb 100644 --- a/includes/Revision/RevisionStore.php +++ b/includes/Revision/RevisionStore.php @@ -1591,7 +1591,7 @@ class RevisionStore foreach ( $rows as $row ) { if ( isset( $rowsByRevId[$row->$revIdField] ) ) { $result->warning( - 'internalerror', + 'internalerror_info', "Duplicate rows in newRevisionsFromBatch, $revIdField {$row->$revIdField}" ); } @@ -1671,12 +1671,20 @@ class RevisionStore true, array_map( function ( $row ) - use ( $queryFlags, $titlesByPageKey, $result, $newRevisionRecord ) { + use ( $queryFlags, $titlesByPageKey, $result, $newRevisionRecord, $revIdField ) { try { + if ( !isset( $titlesByPageKey[$row->_page_key] ) ) { + $result->warning( + 'internalerror_info', + "Couldn't find title for rev {$row->$revIdField} " + . "(page key {$row->_page_key})" + ); + return null; + } return $newRevisionRecord( $row, null, $queryFlags, - $titlesByPageKey[ $row->_page_key ] ?? null ); + $titlesByPageKey[ $row->_page_key ] ); } catch ( MWException $e ) { - $result->warning( 'internalerror', $e->getMessage() ); + $result->warning( 'internalerror_info', $e->getMessage() ); return null; } }, @@ -1712,11 +1720,19 @@ class RevisionStore ) { if ( !isset( $slotRowsByRevId[$row->$revIdField] ) ) { $result->warning( - 'internalerror', + 'internalerror_info', "Couldn't find slots for rev {$row->$revIdField}" ); return null; } + if ( !isset( $titlesByPageKey[$row->_page_key] ) ) { + $result->warning( + 'internalerror_info', + "Couldn't find title for rev {$row->$revIdField} " + . "(page key {$row->_page_key})" + ); + return null; + } try { return $newRevisionRecord( $row, @@ -1725,14 +1741,14 @@ class RevisionStore $row->$revIdField, $slotRowsByRevId[$row->$revIdField], $queryFlags, - $titlesByPageKey[$row->_page_key] ?? null + $titlesByPageKey[$row->_page_key] ) ), $queryFlags, $titlesByPageKey[$row->_page_key] ); } catch ( MWException $e ) { - $result->warning( 'internalerror', $e->getMessage() ); + $result->warning( 'internalerror_info', $e->getMessage() ); return null; } }, @@ -1838,7 +1854,7 @@ class RevisionStore $slotRow->blob_data = $slotContents[$slotRow->content_address]; } else { $result->warning( - 'internalerror', + 'internalerror_info', "Couldn't find blob data for rev {$slotRow->slot_revision_id}" ); $slotRow->blob_data = null; diff --git a/maintenance/findBadBlobs.php b/maintenance/findBadBlobs.php index da23fe1cea1..3d23302f1f3 100644 --- a/maintenance/findBadBlobs.php +++ b/maintenance/findBadBlobs.php @@ -264,10 +264,12 @@ class FindBadBlobs extends Maintenance { $queryInfo['joins'] ); $result = $this->revisionStore->newRevisionsFromBatch( $rows, [ 'slots' => true ] ); - if ( !$result->isOK() ) { - $this->fatalError( Status::wrap( $result )->getMessage( false, false, 'en' )->text() ); - } - return $result->value; + $this->handleStatus( $result ); + + $records = array_filter( $result->value ); + + '@phan-var RevisionStoreRecord[] $records'; + return $records; } /** @@ -292,10 +294,12 @@ class FindBadBlobs extends Maintenance { $rows, [ 'archive' => true, 'slots' => true ] ); - if ( !$result->isOK() ) { - $this->fatalError( Status::wrap( $result )->getMessage( false, false, 'en' )->text() ); - } - return $result->value; + $this->handleStatus( $result ); + + $records = array_filter( $result->value ); + + '@phan-var RevisionArchiveRecord[] $records'; + return $records; } /** @@ -371,11 +375,10 @@ class FindBadBlobs extends Maintenance { $result = $this->revisionStore->newRevisionsFromBatch( $rows, [ 'slots' => true ] ); - if ( !$result->isOK() ) { - $this->fatalError( Status::wrap( $result )->getMessage( false, false, 'en' )->text() ); - } + $this->handleStatus( $result ); - $revisions = $result->value; + $revisions = array_filter( $result->value ); + '@phan-var RevisionArchiveRecord[] $revisions'; // if not all revisions were found, check the archive table. if ( count( $revisions ) < count( $ids ) ) { @@ -398,14 +401,10 @@ class FindBadBlobs extends Maintenance { [ 'slots' => true, 'archive' => true ] ); - if ( !$archiveResult->isOK() ) { - $this->fatalError( - Status::wrap( $archiveResult )->getMessage( false, false, 'en' )->text() - ); - } + $this->handleStatus( $archiveResult ); // don't use array_merge, since it will re-index - $revisions = $revisions + $archiveResult->value; + $revisions = $revisions + array_filter( $archiveResult->value ); } return $revisions; @@ -497,6 +496,19 @@ class FindBadBlobs extends Maintenance { return $this->lbFactory->waitForReplication(); } + private function handleStatus( StatusValue $status ) { + if ( !$status->isOK() ) { + $this->fatalError( + Status::wrap( $status )->getMessage( false, false, 'en' )->text() + ); + } + if ( !$status->isGood() ) { + $this->error( + "\t! " . Status::wrap( $status )->getMessage( false, false, 'en' )->text() + ); + } + } + } $maintClass = FindBadBlobs::class; diff --git a/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php b/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php index e7bd5eada02..700f0eb4915 100644 --- a/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php +++ b/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php @@ -205,7 +205,7 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase { $contentAddress = $rev1->getRevisionRecord()->getSlot( SlotRecord::MAIN )->getAddress(); $blobStatus = StatusValue::newGood( [] ); - $blobStatus->warning( 'internalerror', 'oops!' ); + $blobStatus->warning( 'internalerror_info', 'oops!' ); $mockBlobStore = $this->createMock( BlobStore::class ); $mockBlobStore->method( 'getBlobBatch' ) @@ -230,14 +230,14 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase { $this->assertSame( [ [ 'type' => 'warning', - 'message' => 'internalerror', + 'message' => 'internalerror_info', 'params' => [ "oops!" ] ], [ 'type' => 'warning', - 'message' => 'internalerror', + 'message' => 'internalerror_info', 'params' => [ "Couldn't find blob data for rev " . $rev1->getId() ] @@ -322,7 +322,7 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase { $this->assertNull( $records[$invalidRow->rev_id] ); $this->assertSame( [ [ 'type' => 'warning', - 'message' => 'internalerror', + 'message' => 'internalerror_info', 'params' => [ "Couldn't find slots for rev 100500" ] diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index 1dec89ed939..8b09bbd4c46 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -2398,7 +2398,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { ->newRevisionsFromBatch( [ $this->revisionToRow( $rev1 ), $this->revisionToRow( $rev1 ) ] ); $this->assertFalse( $status->isGood() ); - $this->assertTrue( $status->hasMessage( 'internalerror' ) ); + $this->assertTrue( $status->hasMessage( 'internalerror_info' ) ); } /**