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