Use BlobStore::getBlobBatch for RevisionStore::newRevisionsFromBatch

Bug: T233173
Change-Id: I3ffcfabc3524ce137593e0dba32c83f5eb7e5763

diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php
index 735a212..3ecef76 100644 (file)
--- a/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@ -1623,10 +1623,18 @@ class RevisionStore
         * @param object[]|IResultWrapper $slotRows
         * @param int $queryFlags
         * @param Title $title
+        * @param array|null $slotContents a map from blobAddress to slot
+        *      content blob or Content object.
         *
         * @return SlotRecord[]
         */
-       private function constructSlotRecords( $revId, $slotRows, $queryFlags, Title $title ) {
+       private function constructSlotRecords(
+               $revId,
+               $slotRows,
+               $queryFlags,
+               Title $title,
+               $slotContents = null
+       ) {
                $slots = [];
 
                foreach ( $slotRows as $row ) {
@@ -1651,8 +1659,15 @@ class RevisionStore
                                        = $this->emulateContentId( intval( $row->rev_text_id ) );
                        }
 
-                       $contentCallback = function ( SlotRecord $slot ) use ( $queryFlags ) {
-                               return $this->loadSlotContent( $slot, null, null, null, $queryFlags );
+                       $contentCallback = function ( SlotRecord $slot ) use ( $slotContents, $queryFlags ) {
+                               $blob = null;
+                               if ( isset( $slotContents[$slot->getAddress()] ) ) {
+                                       $blob = $slotContents[$slot->getAddress()];
+                                       if ( $blob instanceof Content ) {
+                                               return $blob;
+                                       }
+                               }
+                               return $this->loadSlotContent( $slot, $blob, null, null, $queryFlags );
                        };
 
                        $slots[$row->role_name] = new SlotRecord( $row, $contentCallback );
@@ -1804,8 +1819,10 @@ class RevisionStore
 
        /**
         * @param object $row A database row generated from a query based on getQueryInfo()
-        * @param null|object[] $slotRows Database rows generated from a query based on
-        *        getSlotsQueryInfo with the 'content' flag set.
+        * @param null|object[]|RevisionSlots $slots
+        *      - Database rows generated from a query based on getSlotsQueryInfo
+        *        with the 'content' flag set. Or
+        *  - RevisionSlots instance
         * @param int $queryFlags
         * @param Title|null $title
         * @param bool $fromCache if true, the returned RevisionRecord will ensure that no stale
@@ -1816,11 +1833,10 @@ class RevisionStore
         * @see RevisionFactory::newRevisionFromRow
         *
         * MCR migration note: this replaces Revision::newFromRow
-        *
         */
        public function newRevisionFromRowAndSlots(
                $row,
-               $slotRows,
+               $slots,
                $queryFlags = 0,
                Title $title = null,
                $fromCache = false
@@ -1857,7 +1873,9 @@ class RevisionStore
                // Legacy because $row may have come from self::selectFields()
                $comment = $this->commentStore->getCommentLegacy( $db, 'rev_comment', $row, true );
 
-               $slots = $this->newRevisionSlots( $row->rev_id, $row, $slotRows, $queryFlags, $title );
+               if ( !( $slots instanceof RevisionSlots ) ) {
+                       $slots = $this->newRevisionSlots( $row->rev_id, $row, $slots, $queryFlags, $title );
+               }
 
                // If this is a cached row, instantiate a cache-aware revision class to avoid stale data.
                if ( $fromCache ) {
@@ -1887,7 +1905,7 @@ class RevisionStore
         *               loaded immediately. Supports falsy or truthy value as well
         *               as an explicit list of slot role names.
         *               'content'- whether the actual content of the slots should be
-        *               preloaded. TODO: no supported yet.
+        *               preloaded.
         * @param int $queryFlags
         * @param Title|null $title
         * @return StatusValue a status with a RevisionRecord[] of successfully fetched revisions
@@ -1957,24 +1975,40 @@ class RevisionStore
                        }, $options['slots'] );
                }
 
-               // TODO: Support optional fetching of the content
-               $queryInfo = self::getSlotsQueryInfo( [ 'content' ] );
+               // We need to set the `content` flag because newRevisionFromRowAndSlots requires content
+               // metadata to be loaded.
+               $slotQueryInfo = self::getSlotsQueryInfo( [ 'content' ] );
                $db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
                $slotRows = $db->select(
-                       $queryInfo['tables'],
-                       $queryInfo['fields'],
+                       $slotQueryInfo['tables'],
+                       $slotQueryInfo['fields'],
                        $slotQueryConds,
                        __METHOD__,
                        [],
-                       $queryInfo['joins']
+                       $slotQueryInfo['joins']
                );
 
                $slotRowsByRevId = [];
                foreach ( $slotRows as $slotRow ) {
                        $slotRowsByRevId[$slotRow->slot_revision_id][] = $slotRow;
                }
+
+               $slotContents = null;
+               if ( $options['content'] ?? false ) {
+                       $blobAddresses = [];
+                       foreach ( $slotRows as $slotRow ) {
+                               $blobAddresses[] = $slotRow->content_address;
+                       }
+                       $slotContentFetchStatus = $this->blobStore
+                               ->getBlobBatch( $blobAddresses, $queryFlags );
+                       foreach ( $slotContentFetchStatus->getErrors() as $error ) {
+                               $result->warning( $error['message'], ...$error['params'] );
+                       }
+                       $slotContents = $slotContentFetchStatus->getValue();
+               }
+
                $result->setResult( true, array_map( function ( $row ) use
-                       ( $slotRowsByRevId, $queryFlags, $titlesByPageId, $result ) {
+                       ( $slotRowsByRevId, $queryFlags, $titlesByPageId, $slotContents, $result ) {
                                if ( !isset( $slotRowsByRevId[$row->rev_id] ) ) {
                                        $result->warning(
                                                'internalerror',
@@ -1985,7 +2019,15 @@ class RevisionStore
                                try {
                                        return $this->newRevisionFromRowAndSlots(
                                                $row,
-                                               $slotRowsByRevId[$row->rev_id],
+                                               new RevisionSlots(
+                                                       $this->constructSlotRecords(
+                                                               $row->rev_id,
+                                                               $slotRowsByRevId[$row->rev_id],
+                                                               $queryFlags,
+                                                               $titlesByPageId[$row->rev_page],
+                                                               $slotContents
+                                                       )
+                                               ),
                                                $queryFlags,
                                                $titlesByPageId[$row->rev_page]
                                        );

diff --git a/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php b/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php
index c0ff44b..e4d6b54 100644 (file)
--- a/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Revision/McrRevisionStoreDbTest.php
@@ -3,12 +3,17 @@
 namespace MediaWiki\Tests\Revision;
 
 use CommentStoreComment;
+use ContentHandler;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Revision\SlotRecord;
+use MediaWiki\Storage\SqlBlobStore;
+use Revision;
+use StatusValue;
 use TextContent;
 use Title;
+use Wikimedia\TestingAccessWrapper;
 use WikitextContent;
 
 /**
@@ -239,4 +244,49 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase {
                        ]
                ] ], $result->getErrors() );
        }
+
+       /**
+        * @covers \MediaWiki\Revision\RevisionStore::newRevisionsFromBatch
+        */
+       public function testNewRevisionFromBatchUsesGetBlobBatch() {
+               $page1 = $this->getTestPage();
+               $text = __METHOD__ . 'b-ä';
+               $editStatus = $this->editPage( $page1->getTitle()->getPrefixedDBkey(), $text . '1' );
+               $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 1' );
+               /** @var Revision $rev1 */
+               $rev1 = $editStatus->getValue()['revision'];
+
+               $contentAddress = $rev1->getRevisionRecord()->getSlot( SlotRecord::MAIN )->getAddress();
+               $mockBlobStore = $this->getMockBuilder( SqlBlobStore::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $mockBlobStore
+                       ->expects( $this->once() )
+                       ->method( 'getBlobBatch' )
+                       ->with( [ $contentAddress ], $this->anything() )
+                       ->willReturn( StatusValue::newGood( [
+                               $contentAddress => 'Content_From_Mock'
+                       ] ) );
+               $mockBlobStore
+                       ->expects( $this->never() )
+                       ->method( 'getBlob' );
+
+               $revStore = MediaWikiServices::getInstance()
+                       ->getRevisionStoreFactory()
+                       ->getRevisionStore();
+               $wrappedRevStore = TestingAccessWrapper::newFromObject( $revStore );
+               $wrappedRevStore->blobStore = $mockBlobStore;
+
+               $result = $revStore->newRevisionsFromBatch(
+                       [ $this->revisionToRow( $rev1 ) ],
+                       [
+                               'slots' => [ SlotRecord::MAIN ],
+                               'content' => true
+                       ]
+               );
+               $this->assertTrue( $result->isGood() );
+               $this->assertSame( 'Content_From_Mock',
+                       ContentHandler::getContentText( $result->getValue()[$rev1->getId()]
+                               ->getContent( SlotRecord::MAIN ) ) );
+       }
 }

diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
index 76190e0..4040ffc 100644 (file)
--- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
+++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
@@ -4,8 +4,10 @@ namespace MediaWiki\Tests\Revision;
 
 use CommentStoreComment;
 use Content;
+use ContentHandler;
 use Exception;
 use HashBagOStuff;
+use IDBAccessObject;
 use InvalidArgumentException;
 use Language;
 use MediaWiki\Linker\LinkTarget;
@@ -105,7 +107,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
         * @return WikiPage
         */
        protected function getTestPage( $pageTitle = null ) {
-               if ( !is_null( $pageTitle ) && $this->testPage ) {
+               if ( is_null( $pageTitle ) && $this->testPage ) {
                        return $this->testPage;
                }
 
@@ -1992,23 +1994,17 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
        ) {
                $page1 = $this->getTestPage();
                $text = __METHOD__ . 'b-ä';
+               $editStatus = $this->editPage( $page1->getTitle()->getPrefixedDBkey(), $text . '1' );
+               $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 1' );
                /** @var Revision $rev1 */
-               $rev1 = $page1->doEditContent(
-                       new WikitextContent( $text . '1' ),
-                       __METHOD__ . 'b',
-                       0,
-                       false,
-                       $this->getTestUser()->getUser()
-               )->value['revision'];
+               $rev1 = $editStatus->getValue()['revision'];
+
                $page2 = $this->getTestPage( $otherPageTitle );
+               $editStatus = $this->editPage( $page2->getTitle()->getPrefixedDBkey(), $text . '2' );
+               $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 2' );
                /** @var Revision $rev2 */
-               $rev2 = $page2->doEditContent(
-                       new WikitextContent( $text . '2' ),
-                       __METHOD__ . 'b',
-                       0,
-                       false,
-                       $this->getTestUser()->getUser()
-               )->value['revision'];
+               $rev2 = $editStatus->getValue()['revision'];
+
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $result = $store->newRevisionsFromBatch(
                        [ $this->revisionToRow( $rev1 ), $this->revisionToRow( $rev2 ) ],
@@ -2016,14 +2012,15 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                );
                $this->assertTrue( $result->isGood() );
                $this->assertEmpty( $result->getErrors() );
+               /** @var RevisionRecord[] $records */
                $records = $result->getValue();
                $this->assertRevisionRecordMatchesRevision( $rev1, $records[$rev1->getId()] );
                $this->assertRevisionRecordMatchesRevision( $rev2, $records[$rev2->getId()] );
 
                $this->assertSame( $text . '1',
-                       $records[$rev1->getId()]->getContent( SlotRecord::MAIN )->serialize() );
+                       ContentHandler::getContentText( $records[$rev1->getId()]->getContent( SlotRecord::MAIN ) ) );
                $this->assertSame( $text . '2',
-                       $records[$rev2->getId()]->getContent( SlotRecord::MAIN )->serialize() );
+                       ContentHandler::getContentText( $records[$rev2->getId()]->getContent( SlotRecord::MAIN ) ) );
                $this->assertEquals( $page1->getTitle()->getDBkey(),
                        $records[$rev1->getId()]->getPageAsLinkTarget()->getDBkey() );
                $this->assertEquals( $page2->getTitle()->getDBkey(),
@@ -2046,4 +2043,41 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                $this->assertEmpty( $result->getValue() );
                $this->assertEmpty( $result->getErrors() );
        }
+
+       /**
+        * @covers \MediaWiki\Revision\RevisionStore::newRevisionsFromBatch
+        */
+       public function testNewRevisionsFromBatch_wrongTitle() {
+               $page1 = $this->getTestPage();
+               $text = __METHOD__ . 'b-ä';
+               $editStatus = $this->editPage( $page1->getTitle()->getPrefixedDBkey(), $text . '1' );
+               $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 1' );
+               /** @var Revision $rev1 */
+               $rev1 = $editStatus->getValue()['revision'];
+
+               $this->setExpectedException( InvalidArgumentException::class );
+               MediaWikiServices::getInstance()->getRevisionStore()
+                       ->newRevisionsFromBatch(
+                               [ $this->revisionToRow( $rev1 ) ],
+                               [],
+                               IDBAccessObject::READ_NORMAL,
+                               $this->getTestPage( 'Title_Other_Then_The_One_Revision_Belongs_To' )->getTitle()
+                       );
+       }
+
+       /**
+        * @covers \MediaWiki\Revision\RevisionStore::newRevisionsFromBatch
+        */
+       public function testNewRevisionsFromBatch_DuplicateRows() {
+               $page1 = $this->getTestPage();
+               $text = __METHOD__ . 'b-ä';
+               $editStatus = $this->editPage( $page1->getTitle()->getPrefixedDBkey(), $text . '1' );
+               $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 1' );
+               /** @var Revision $rev1 */
+               $rev1 = $editStatus->getValue()['revision'];
+
+               $this->setExpectedException( InvalidArgumentException::class );
+               MediaWikiServices::getInstance()->getRevisionStore()
+                       ->newRevisionsFromBatch( [ $this->revisionToRow( $rev1 ), $this->revisionToRow( $rev1 ) ] );
+       }
 }