RevisionStore::newRevisionFromBatch should use Title::newFromRow

If the rows were obtained using RevisionStore::getQueryInfo with
'page' flags, the revision row already contains the fields needed
to construct the Title without an additional database query.

Change-Id: Ie36c85871a8996a5706c80d286854a9c8363905f

diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php
index b1370dc..420fe65 100644 (file)
--- a/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@ -1924,7 +1924,7 @@ class RevisionStore
                $result = new StatusValue();
 
                $rowsByRevId = [];
                $result = new StatusValue();
 
                $rowsByRevId = [];

-               $pageIds = [];
+               $pageIdsToFetchTitles = [];

                $titlesByPageId = [];
                foreach ( $rows as $row ) {
                        if ( isset( $rowsByRevId[$row->rev_id] ) ) {
                $titlesByPageId = [];
                foreach ( $rows as $row ) {
                        if ( isset( $rowsByRevId[$row->rev_id] ) ) {


@@ -1937,8 +1937,17 @@ class RevisionStore
                                throw new InvalidArgumentException(
                                        "Revision {$row->rev_id} doesn't belong to page {$title->getArticleID()}"
                                );
                                throw new InvalidArgumentException(
                                        "Revision {$row->rev_id} doesn't belong to page {$title->getArticleID()}"
                                );

+                       } elseif ( !$title && !isset( $titlesByPageId[ $row->rev_page ] ) ) {
+                               if ( isset( $row->page_namespace ) && isset( $row->page_title ) &&
+                                       // This should not happen, but just in case we don't have a page_id
+                                       // set or it doesn't match rev_page, let's fetch the title again.
+                                       isset( $row->page_id ) && $row->rev_page === $row->page_id
+                               ) {
+                                       $titlesByPageId[ $row->rev_page ] = Title::newFromRow( $row );
+                               } else {
+                                       $pageIdsToFetchTitles[] = $row->rev_page;
+                               }

                        }
                        }

-                       $pageIds[] = $row->rev_page;

                        $rowsByRevId[$row->rev_id] = $row;
                }
 
                        $rowsByRevId[$row->rev_id] = $row;
                }
 


@@ -1950,9 +1959,9 @@ class RevisionStore
                // If the title is not supplied, batch-fetch Title objects.
                if ( $title ) {
                        $titlesByPageId[$title->getArticleID()] = $title;
                // If the title is not supplied, batch-fetch Title objects.
                if ( $title ) {
                        $titlesByPageId[$title->getArticleID()] = $title;

-               } else {
-                       $pageIds = array_unique( $pageIds );
-                       foreach ( Title::newFromIDs( $pageIds ) as $t ) {
+               } elseif ( !empty( $pageIdsToFetchTitles ) ) {
+                       $pageIdsToFetchTitles = array_unique( $pageIdsToFetchTitles );
+                       foreach ( Title::newFromIDs( $pageIdsToFetchTitles ) as $t ) {

                                $titlesByPageId[$t->getArticleID()] = $t;
                        }
                }
                                $titlesByPageId[$t->getArticleID()] = $t;
                        }
                }

diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
index daec8a2..4248650 100644 (file)
--- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
+++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
@@ -2028,10 +2028,12 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
 
        public function provideNewRevisionsFromBatchOptions() {
                yield 'No preload slots or content, single page' => [
 
        public function provideNewRevisionsFromBatchOptions() {
                yield 'No preload slots or content, single page' => [

+                       [ 'comment' ],

                        null,
                        []
                ];
                yield 'Preload slots and content, single page' => [
                        null,
                        []
                ];
                yield 'Preload slots and content, single page' => [

+                       [ 'comment' ],

                        null,
                        [
                                'slots' => [ SlotRecord::MAIN ],
                        null,
                        [
                                'slots' => [ SlotRecord::MAIN ],


@@ -2039,14 +2041,25 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                        ]
                ];
                yield 'Ask for no slots' => [
                        ]
                ];
                yield 'Ask for no slots' => [

+                       [ 'comment' ],

                        null,
                        [ 'slots' => [] ]
                ];
                yield 'No preload slots or content, multiple pages' => [
                        null,
                        [ 'slots' => [] ]
                ];
                yield 'No preload slots or content, multiple pages' => [

+                       [ 'comment' ],

                        'Other_Page',
                        []
                ];
                yield 'Preload slots and content, multiple pages' => [
                        'Other_Page',
                        []
                ];
                yield 'Preload slots and content, multiple pages' => [

+                       [ 'comment' ],
+                       'Other_Page',
+                       [
+                               'slots' => [ SlotRecord::MAIN ],
+                               'content' => true
+                       ]
+               ];
+               yield 'Preload slots and content, multiple pages, preload page fields' => [
+                       [ 'page', 'comment' ],

                        'Other_Page',
                        [
                                'slots' => [ SlotRecord::MAIN ],
                        'Other_Page',
                        [
                                'slots' => [ SlotRecord::MAIN ],


@@ -2057,12 +2070,14 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
 
        /**
         * @dataProvider provideNewRevisionsFromBatchOptions
 
        /**
         * @dataProvider provideNewRevisionsFromBatchOptions

-        * @covers \MediaWiki\Revision\RevisionStore::newRevisionsFromBatch
+        * @covers       \MediaWiki\Revision\RevisionStore::newRevisionsFromBatch
+        * @param array|null $queryOptions options to provide to revisionToRow

         * @param string|null $otherPageTitle
         * @param array|null $options
         * @throws \MWException
         */
        public function testNewRevisionsFromBatch_preloadContent(
         * @param string|null $otherPageTitle
         * @param array|null $options
         * @throws \MWException
         */
        public function testNewRevisionsFromBatch_preloadContent(

+               $queryOptions,

                $otherPageTitle = null,
                array $options = []
        ) {
                $otherPageTitle = null,
                array $options = []
        ) {


@@ -2081,7 +2096,10 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $result = $store->newRevisionsFromBatch(
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $result = $store->newRevisionsFromBatch(

-                       [ $this->revisionToRow( $rev1 ), $this->revisionToRow( $rev2 ) ],
+                       [
+                               $this->revisionToRow( $rev1, $queryOptions ),
+                               $this->revisionToRow( $rev2, $queryOptions )
+                       ],

                        $options
                );
                $this->assertTrue( $result->isGood() );
                        $options
                );
                $this->assertTrue( $result->isGood() );