Daniel Kinzler has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/403634 )

Change subject: Handle failure to load content in Revision getSize, etc
......................................................................

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
---
M includes/Revision.php
M includes/Storage/RevisionArchiveRecord.php
M includes/Storage/RevisionRecord.php
M includes/Storage/RevisionStoreRecord.php
M tests/phpunit/includes/RevisionTest.php
5 files changed, 102 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/34/403634/1

diff --git a/includes/Revision.php b/includes/Revision.php
index 0844e89..6db3710 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -608,20 +608,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;
+               }
        }
 
        /**
diff --git a/includes/Storage/RevisionArchiveRecord.php 
b/includes/Storage/RevisionArchiveRecord.php
index 419cb95..3e78b72 100644
--- a/includes/Storage/RevisionArchiveRecord.php
+++ b/includes/Storage/RevisionArchiveRecord.php
@@ -107,6 +107,7 @@
        }
 
        /**
+        * @throw 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 @@
        }
 
        /**
+        * @throw 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..161075d 100644
--- a/includes/Storage/RevisionRecord.php
+++ b/includes/Storage/RevisionRecord.php
@@ -242,6 +242,7 @@
         *
         * MCR migration note: this replaces Revision::getSize
         *
+        * @throw RevisionAccessException if the size was unknown and could not 
be calculated.
         * @return int
         */
        abstract public function getSize();
@@ -254,6 +255,7 @@
         *
         * MCR migration note: this replaces Revision::getSha1
         *
+        * @throw 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..d605bae 100644
--- a/includes/Storage/RevisionStoreRecord.php
+++ b/includes/Storage/RevisionStoreRecord.php
@@ -150,6 +150,7 @@
        }
 
        /**
+        * @throw 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 @@
        }
 
        /**
+        * @throw 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..c8f552a 100644
--- 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,84 @@
                );
        }
 
+       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() );
+       }
+
+       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() );
+       }
+
+       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() );
+       }
+
+       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() );
+       }
+
+       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() );
+       }
+
+       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() );
+       }
+
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/403634
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I393ea19b9fb48219583fc65ce81ea14d8d0a2357
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to