Merge "Make Revision::__construct work with bad page ID"

diff --combined includes/Revision.php
index 66b4173,8849697..510c1ee
--- 1/includes/Revision.php
--- 2/includes/Revision.php
+++ b/includes/Revision.php
@@@ -495,13 -495,13 +495,13 @@@ class Revision implements IDBAccessObje
                        $this->mRecord = self::getRevisionStore()->newMutableRevisionFromArray(
                                $row,
                                $queryFlags,
-                               $title
+                               $this->ensureTitle( $row, $queryFlags, $title )
                        );
                } elseif ( is_object( $row ) ) {
                        $this->mRecord = self::getRevisionStore()->newRevisionFromRow(
                                $row,
                                $queryFlags,
-                               $title
+                               $this->ensureTitle( $row, $queryFlags, $title )
                        );
                } else {
                        throw new InvalidArgumentException(
@@@ -510,6 -510,51 +510,51 @@@
                }
        }
  
+       /**
+        * Make sure we have *some* Title object for use by the constructor.
+        * For B/C, the constructor shouldn't fail even for a bad page ID or bad revision ID.
+        *
+        * @param array|object $row
+        * @param int $queryFlags
+        * @param Title|null $title
+        *
+        * @return Title $title if not null, or a Title constructed from information in $row.
+        */
+       private function ensureTitle( $row, $queryFlags, $title = null ) {
+               if ( $title ) {
+                       return $title;
+               }
+ 
+               if ( is_array( $row ) ) {
+                       if ( isset( $row['title'] ) ) {
+                               if ( !( $row['title'] instanceof Title ) ) {
+                                       throw new MWException( 'title field must contain a Title object.' );
+                               }
+ 
+                               return $row['title'];
+                       }
+ 
+                       $pageId = isset( $row['page'] ) ? $row['page'] : 0;
+                       $revId = isset( $row['id'] ) ? $row['id'] : 0;
+               } else {
+                       $pageId = isset( $row->rev_page ) ? $row->rev_page : 0;
+                       $revId = isset( $row->rev_id ) ? $row->rev_id : 0;
+               }
+ 
+               try {
+                       $title = self::getRevisionStore()->getTitle( $pageId, $revId, $queryFlags );
+               } catch ( RevisionAccessException $ex ) {
+                       // construct a dummy title!
+                       wfLogWarning( __METHOD__ . ': ' . $ex->getMessage() );
+ 
+                       // NOTE: this Title will only be used inside RevisionRecord
+                       $title = Title::makeTitleSafe( NS_SPECIAL, "Badtitle/ID=$pageId" );
+                       $title->resetArticleID( $pageId );
+               }
+ 
+               return $title;
+       }
+ 
        /**
         * @return RevisionRecord
         */
@@@ -608,27 -653,20 +653,27 @@@
        /**
         * Returns the length of the text in this revision, or null if unknown.
         *
 -       * @return int
 +       * @return int|null
         */
        public function getSize() {
 -              return $this->mRecord->getSize();
 +              try {
 +                      return $this->mRecord->getSize();
 +              } catch ( RevisionAccessException $ex ) {
 +                      return null;
 +              }
        }
  
        /**
         * Returns the base36 sha1 of the content in this revision, or null if unknown.
         *
 -       * @return string
 +       * @return string|null
         */
        public function getSha1() {
 -              // XXX: we may want to drop all the hashing logic, it's not worth the overhead.
 -              return $this->mRecord->getSha1();
 +              try {
 +                      return $this->mRecord->getSha1();
 +              } catch ( RevisionAccessException $ex ) {
 +                      return null;
 +              }
        }
  
        /**
@@@ -1072,11 -1110,7 +1117,11 @@@
  
                $comment = CommentStoreComment::newUnsavedComment( $summary, null );
  
 -              $title = Title::newFromID( $pageId );
 +              $title = Title::newFromID( $pageId, Title::GAID_FOR_UPDATE );
 +              if ( $title === null ) {
 +                      return null;
 +              }
 +
                $rec = self::getRevisionStore()->newNullRevision( $dbw, $title, $comment, $minor, $user );
  
                return new Revision( $rec );

diff --combined tests/phpunit/includes/RevisionTest.php
index a467346,7ce43ed..73d69a5
--- 1/tests/phpunit/includes/RevisionTest.php
--- 2/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@@ -1,11 -1,7 +1,11 @@@
  <?php
  
  use MediaWiki\Storage\BlobStoreFactory;
 +use MediaWiki\Storage\MutableRevisionRecord;
 +use MediaWiki\Storage\RevisionAccessException;
 +use MediaWiki\Storage\RevisionRecord;
  use MediaWiki\Storage\RevisionStore;
 +use MediaWiki\Storage\SlotRecord;
  use MediaWiki\Storage\SqlBlobStore;
  use Wikimedia\Rdbms\IDatabase;
  use Wikimedia\Rdbms\LoadBalancer;
@@@ -80,6 -76,17 +80,17 @@@ class RevisionTest extends MediaWikiTes
                $this->assertNull( $rev->getContent(), 'no content object should be available' );
        }
  
+       /**
+        * @covers Revision::__construct
+        * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+        */
+       public function testConstructFromArrayWithBadPageId() {
+               MediaWiki\suppressWarnings();
+               $rev = new Revision( [ 'page' => 77777777 ] );
+               $this->assertSame( 77777777, $rev->getPage() );
+               MediaWiki\restoreWarnings();
+       }
+ 
        public function provideConstructFromArray_userSetAsExpected() {
                yield 'no user defaults to wgUser' => [
                        [
@@@ -301,6 -308,17 +312,17 @@@
                $assertions( $this, $rev );
        }
  
+       /**
+        * @covers Revision::__construct
+        * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+        */
+       public function testConstructFromRowWithBadPageId() {
+               MediaWiki\suppressWarnings();
+               $rev = new Revision( (object)[ 'rev_page' => 77777777 ] );
+               $this->assertSame( 77777777, $rev->getPage() );
+               MediaWiki\restoreWarnings();
+       }
+ 
        public function provideGetRevisionText() {
                yield 'Generic test' => [
                        'This is a goat of revision text.',
@@@ -1571,102 -1589,4 +1593,102 @@@
                );
        }
  
 +      /**
 +       * @covers Revision::getSize
 +       */
 +      public function testGetSize() {
 +              $title = $this->getMockTitle();
 +
 +              $rec = new MutableRevisionRecord( $title );
 +              $rev = new Revision( $rec, 0, $title );
 +
 +              $this->assertSame( 0, $rev->getSize(), 'Size of no slots is 0' );
 +
 +              $rec->setSize( 13 );
 +              $this->assertSame( 13, $rev->getSize() );
 +      }
 +
 +      /**
 +       * @covers Revision::getSize
 +       */
 +      public function testGetSize_failure() {
 +              $title = $this->getMockTitle();
 +
 +              $rec = $this->getMockBuilder( RevisionRecord::class )
 +                      ->disableOriginalConstructor()
 +                      ->getMock();
 +
 +              $rec->method( 'getSize' )
 +                      ->willThrowException( new RevisionAccessException( 'Oops!' ) );
 +
 +              $rev = new Revision( $rec, 0, $title );
 +              $this->assertNull( $rev->getSize() );
 +      }
 +
 +      /**
 +       * @covers Revision::getSha1
 +       */
 +      public function testGetSha1() {
 +              $title = $this->getMockTitle();
 +
 +              $rec = new MutableRevisionRecord( $title );
 +              $rev = new Revision( $rec, 0, $title );
 +
 +              $emptyHash = SlotRecord::base36Sha1( '' );
 +              $this->assertSame( $emptyHash, $rev->getSha1(), 'Sha1 of no slots is hash of empty string' );
 +
 +              $rec->setSha1( 'deadbeef' );
 +              $this->assertSame( 'deadbeef', $rev->getSha1() );
 +      }
 +
 +      /**
 +       * @covers Revision::getSha1
 +       */
 +      public function testGetSha1_failure() {
 +              $title = $this->getMockTitle();
 +
 +              $rec = $this->getMockBuilder( RevisionRecord::class )
 +                      ->disableOriginalConstructor()
 +                      ->getMock();
 +
 +              $rec->method( 'getSha1' )
 +                      ->willThrowException( new RevisionAccessException( 'Oops!' ) );
 +
 +              $rev = new Revision( $rec, 0, $title );
 +              $this->assertNull( $rev->getSha1() );
 +      }
 +
 +      /**
 +       * @covers Revision::getContent
 +       */
 +      public function testGetContent() {
 +              $title = $this->getMockTitle();
 +
 +              $rec = new MutableRevisionRecord( $title );
 +              $rev = new Revision( $rec, 0, $title );
 +
 +              $this->assertNull( $rev->getContent(), 'Content of no slots is null' );
 +
 +              $content = new TextContent( 'Hello Kittens!' );
 +              $rec->setContent( 'main', $content );
 +              $this->assertSame( $content, $rev->getContent() );
 +      }
 +
 +      /**
 +       * @covers Revision::getContent
 +       */
 +      public function testGetContent_failure() {
 +              $title = $this->getMockTitle();
 +
 +              $rec = $this->getMockBuilder( RevisionRecord::class )
 +                      ->disableOriginalConstructor()
 +                      ->getMock();
 +
 +              $rec->method( 'getContent' )
 +                      ->willThrowException( new RevisionAccessException( 'Oops!' ) );
 +
 +              $rev = new Revision( $rec, 0, $title );
 +              $this->assertNull( $rev->getContent() );
 +      }
 +
  }