Merge "Revision: Handle all return values of Title::newFromId"

diff --combined includes/Revision.php
index 4058c63,b8b56a3..1d7e62c
--- 1/includes/Revision.php
--- 2/includes/Revision.php
+++ b/includes/Revision.php
@@@ -94,11 -94,54 +94,11 @@@ class Revision implements IDBAccessObje
         *
         * @param int $id
         * @param int $flags (optional)
 -       * @param Title $title (optional) If known you can pass the Title in here.
 -       *  Passing no Title may result in another DB query if there are recent writes.
         * @return Revision|null
         */
 -      public static function newFromId( $id, $flags = 0, Title $title = null ) {
 -              /**
 -               * MCR RevisionStore Compat
 -               *
 -               * If the title is not passed in as a param (already known) then select it here.
 -               *
 -               * Do the selection with MASTER if $flags includes READ_LATEST or recent changes
 -               * have happened on our load balancer.
 -               *
 -               * If we select the title here and pass it down it will results in fewer queries
 -               * further down the stack.
 -               */
 -              if ( !$title ) {
 -                      if (
 -                              $flags & self::READ_LATEST ||
 -                              wfGetLB()->hasOrMadeRecentMasterChanges()
 -                      ) {
 -                              $dbr = wfGetDB( DB_MASTER );
 -                      } else {
 -                              $dbr = wfGetDB( DB_REPLICA );
 -                      }
 -                      $row = $dbr->selectRow(
 -                              [ 'revision', 'page' ],
 -                              [
 -                                      'page_namespace',
 -                                      'page_title',
 -                                      'page_id',
 -                                      'page_latest',
 -                                      'page_is_redirect',
 -                                      'page_len',
 -                              ],
 -                              [ 'rev_id' => $id ],
 -                              __METHOD__,
 -                              [],
 -                              [ 'page' => [ 'JOIN', 'page_id=rev_page' ] ]
 -                      );
 -                      if ( $row ) {
 -                              $title = Title::newFromRow( $row );
 -                      }
 -                      wfGetLB()->reuseConnection( $dbr );
 -              }
 -
 -              $rec = self::getRevisionStore()->getRevisionById( $id, $flags, $title );
 -              return $rec === null ? null : new Revision( $rec, $flags, $title );
 +      public static function newFromId( $id, $flags = 0 ) {
 +              $rec = self::getRevisionStore()->getRevisionById( $id, $flags );
 +              return $rec === null ? null : new Revision( $rec, $flags );
        }
  
        /**
@@@ -145,11 -188,12 +145,11 @@@
         *
         * @param object $row
         * @param array $overrides
 -       * @param Title $title (optional)
         *
         * @throws MWException
         * @return Revision
         */
 -      public static function newFromArchiveRow( $row, $overrides = [], Title $title = null ) {
 +      public static function newFromArchiveRow( $row, $overrides = [] ) {
                /**
                 * MCR Migration: https://phabricator.wikimedia.org/T183564
                 * This method used to overwrite attributes, then passed to Revision::__construct
@@@ -161,30 -205,7 +161,30 @@@
                        unset( $overrides['page'] );
                }
  
 -              $rec = self::getRevisionStore()->newRevisionFromArchiveRow( $row, 0, $title, $overrides );
 +              /**
 +               * We require a Title for both the Revision object and the RevisionRecord.
 +               * Below is duplicated logic from RevisionStore::newRevisionFromArchiveRow
 +               * to fetch a title in order pass it into the Revision object.
 +               */
 +              $title = null;
 +              if ( isset( $overrides['title'] ) ) {
 +                      if ( !( $overrides['title'] instanceof Title ) ) {
 +                              throw new MWException( 'title field override must contain a Title object.' );
 +                      }
 +
 +                      $title = $overrides['title'];
 +              }
 +              if ( $title !== null ) {
 +                      if ( isset( $row->ar_namespace ) && isset( $row->ar_title ) ) {
 +                              $title = Title::makeTitle( $row->ar_namespace, $row->ar_title );
 +                      } else {
 +                              throw new InvalidArgumentException(
 +                                      'A Title or ar_namespace and ar_title must be given'
 +                              );
 +                      }
 +              }
 +
 +              $rec = self::getRevisionStore()->newRevisionFromArchiveRow( $row, 0, null, $overrides );
                return new Revision( $rec, self::READ_NORMAL, $title );
        }
  
@@@ -917,8 -938,10 +917,8 @@@
         * @return Revision|null
         */
        public function getPrevious() {
 -              $rec = self::getRevisionStore()->getPreviousRevision( $this->mRecord, $this->getTitle() );
 -              return $rec === null
 -                      ? null
 -                      : new Revision( $rec, self::READ_NORMAL, $this->getTitle() );
 +              $rec = self::getRevisionStore()->getPreviousRevision( $this->mRecord );
 +              return $rec === null ? null : new Revision( $rec );
        }
  
        /**
@@@ -927,8 -950,10 +927,8 @@@
         * @return Revision|null
         */
        public function getNext() {
 -              $rec = self::getRevisionStore()->getNextRevision( $this->mRecord, $this->getTitle() );
 -              return $rec === null
 -                      ? null
 -                      : new Revision( $rec, self::READ_NORMAL, $this->getTitle() );
 +              $rec = self::getRevisionStore()->getNextRevision( $this->mRecord );
 +              return $rec === null ? null : new Revision( $rec );
        }
  
        /**
@@@ -1179,6 -1204,10 +1179,10 @@@
                        ? $pageIdOrTitle
                        : Title::newFromID( $pageIdOrTitle );
  
+               if ( !$title ) {
+                       return false;
+               }
+ 
                $record = self::getRevisionStore()->getKnownCurrentRevision( $title, $revId );
                return $record ? new Revision( $record ) : false;
        }

diff --combined tests/phpunit/includes/RevisionDbTestBase.php
index 94f8fba,bd0dea6..6139524
--- 1/tests/phpunit/includes/RevisionDbTestBase.php
--- 2/tests/phpunit/includes/RevisionDbTestBase.php
+++ b/tests/phpunit/includes/RevisionDbTestBase.php
@@@ -649,7 -649,6 +649,7 @@@ abstract class RevisionDbTestBase exten
        }
  
        /**
 +       * @covers Revision::userWasLastToEdit
         * @dataProvider provideUserWasLastToEdit
         */
        public function testUserWasLastToEdit( $sinceIdx, $expectedLast ) {
@@@ -1364,6 -1363,24 +1364,24 @@@
                $this->assertEquals( $rev->getId(), $cachedRow->rev_id );
        }
  
+       public function testNewKnownCurrent_withPageId() {
+               $db = wfGetDB( DB_MASTER );
+ 
+               $this->testPage->doEditContent( new WikitextContent( __METHOD__ ), __METHOD__ );
+               $rev = $this->testPage->getRevision();
+ 
+               $pageId = $this->testPage->getId();
+ 
+               $newRev = Revision::newKnownCurrent( $db, $pageId, $rev->getId() );
+               $this->assertRevEquals( $rev, $newRev );
+       }
+ 
+       public function testNewKnownCurrent_returnsFalseWhenTitleDoesntExist() {
+               $db = wfGetDB( DB_MASTER );
+ 
+               $this->assertFalse( Revision::newKnownCurrent( $db, 0 ) );
+       }
+ 
        public function provideUserCanBitfield() {
                yield [ 0, 0, [], null, true ];
                // Bitfields match, user has no permissions