From 04bac0dee1c919c2f5c63527c90412b0b8fac081 Mon Sep 17 00:00:00 2001 From: daniel Date: Thu, 11 Jan 2018 13:40:53 +0100 Subject: [PATCH] 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 --- includes/Revision.php | 17 +++- includes/Storage/RevisionArchiveRecord.php | 2 + includes/Storage/RevisionRecord.php | 2 + includes/Storage/RevisionStoreRecord.php | 2 + tests/phpunit/includes/RevisionTest.php | 102 +++++++++++++++++++++ 5 files changed, 120 insertions(+), 5 deletions(-) diff --git a/includes/Revision.php b/includes/Revision.php index 0844e89286..6db3710b2d 100644 --- 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 419cb95b0b..9999179074 100644 --- 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 f490f9b40f..8734f48848 100644 --- 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 341855dbe3..e8efcfa1b0 100644 --- 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 0db76ff193..a467346efa 100644 --- a/tests/phpunit/includes/RevisionTest.php +++ b/tests/phpunit/includes/RevisionTest.php @@ -1,7 +1,11 @@ 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() ); + } + } -- 2.20.1