Handle failure to load content in Revision getSize, etc

The Revision class used to just return null if size or hsash were unknown
and could nto be determined. This patch restores this behavior by
catching any RevisionAccessExceptions raised by RevisionRecord when
failing to load content.

Bug: T184693
Bug: T184690
Change-Id: I393ea19b9fb48219583fc65ce81ea14d8d0a2357

diff --git a/includes/Revision.php b/includes/Revision.php
index 0844e89..6db3710 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -608,20 +608,27 @@ class Revision implements IDBAccessObject {
        /**
         * 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;
+               }
        }
 
        /**

diff --git a/includes/Storage/RevisionArchiveRecord.php b/includes/Storage/RevisionArchiveRecord.php
index 419cb95..9999179 100644 (file)
--- a/includes/Storage/RevisionArchiveRecord.php
+++ b/includes/Storage/RevisionArchiveRecord.php
@@ -107,6 +107,7 @@ class RevisionArchiveRecord extends RevisionRecord {
        }
 
        /**
+        * @throws RevisionAccessException if the size was unknown and could not be calculated.
         * @return int The nominal revision size, never null. May be computed on the fly.
         */
        public function getSize() {
@@ -120,6 +121,7 @@ class RevisionArchiveRecord extends RevisionRecord {
        }
 
        /**
+        * @throws RevisionAccessException if the hash was unknown and could not be calculated.
         * @return string The revision hash, never null. May be computed on the fly.
         */
        public function getSha1() {

diff --git a/includes/Storage/RevisionRecord.php b/includes/Storage/RevisionRecord.php
index f490f9b..8734f48 100644 (file)
--- a/includes/Storage/RevisionRecord.php
+++ b/includes/Storage/RevisionRecord.php
@@ -242,6 +242,7 @@ abstract class RevisionRecord {
         *
         * MCR migration note: this replaces Revision::getSize
         *
+        * @throws RevisionAccessException if the size was unknown and could not be calculated.
         * @return int
         */
        abstract public function getSize();
@@ -254,6 +255,7 @@ abstract class RevisionRecord {
         *
         * MCR migration note: this replaces Revision::getSha1
         *
+        * @throws RevisionAccessException if the hash was unknown and could not be calculated.
         * @return string
         */
        abstract public function getSha1();

diff --git a/includes/Storage/RevisionStoreRecord.php b/includes/Storage/RevisionStoreRecord.php
index 341855d..e8efcfa 100644 (file)
--- a/includes/Storage/RevisionStoreRecord.php
+++ b/includes/Storage/RevisionStoreRecord.php
@@ -150,6 +150,7 @@ class RevisionStoreRecord extends RevisionRecord {
        }
 
        /**
+        * @throws RevisionAccessException if the size was unknown and could not be calculated.
         * @return string The nominal revision size, never null. May be computed on the fly.
         */
        public function getSize() {
@@ -163,6 +164,7 @@ class RevisionStoreRecord extends RevisionRecord {
        }
 
        /**
+        * @throws RevisionAccessException if the hash was unknown and could not be calculated.
         * @return string The revision hash, never null. May be computed on the fly.
         */
        public function getSha1() {

diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php
index 0db76ff..a467346 100644 (file)
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -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;
@@ -1567,4 +1571,102 @@ class RevisionTest extends MediaWikiTestCase {
                );
        }
 
+       /**
+        * @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() );
+       }
+
 }