From bc42269dba9c682e9a2d544129061633ddad0c35 Mon Sep 17 00:00:00 2001 From: daniel Date: Mon, 10 Sep 2018 21:34:31 +0200 Subject: [PATCH] Introduce RevisionRecord::isReadForInsertion RevisionRecord::isReadForInsertion provides a concise way to check whether a revision is complete enough to be inserted into the database. Change-Id: I0c79f55c0b935bce9943163ed4c2cc8b5f9c82d5 --- includes/Storage/RevisionArchiveRecord.php | 9 ++++ includes/Storage/RevisionRecord.php | 25 ++++++++++ includes/Storage/RevisionStore.php | 6 +++ includes/Storage/RevisionStoreRecord.php | 9 ++++ .../Storage/MutableRevisionRecordTest.php | 50 +++++++++++++++++++ .../includes/Storage/RevisionRecordTests.php | 5 ++ 6 files changed, 104 insertions(+) diff --git a/includes/Storage/RevisionArchiveRecord.php b/includes/Storage/RevisionArchiveRecord.php index 213ee3cd1b..173da51907 100644 --- a/includes/Storage/RevisionArchiveRecord.php +++ b/includes/Storage/RevisionArchiveRecord.php @@ -167,4 +167,13 @@ class RevisionArchiveRecord extends RevisionRecord { return parent::getTimestamp(); } + /** + * @see RevisionStore::isComplete + * + * @return bool always true. + */ + public function isReadyForInsertion() { + return true; + } + } diff --git a/includes/Storage/RevisionRecord.php b/includes/Storage/RevisionRecord.php index 17c56ea0ea..8c31a3caf7 100644 --- a/includes/Storage/RevisionRecord.php +++ b/includes/Storage/RevisionRecord.php @@ -532,4 +532,29 @@ abstract class RevisionRecord { } } + /** + * Returns whether this RevisionRecord is ready for insertion, that is, whether it contains all + * information needed to save it to the database. This should trivially be true for + * RevisionRecords loaded from the database. + * + * Note that this may return true even if getId() or getPage() return null or 0, since these + * are generally assigned while the revision is saved to the database, and may not be available + * before. + * + * @return bool + */ + public function isReadyForInsertion() { + // NOTE: don't check getSize() and getSha1(), since that may cause the full content to + // be loaded in order to calculate the values. Just assume these methods will not return + // null if mSlots is not empty. + + // NOTE: getId() and getPageId() may return null before a revision is saved, so don't + //check them. + + return $this->getTimestamp() !== null + && $this->getComment( self::RAW ) !== null + && $this->getUser( self::RAW ) !== null + && $this->mSlots->getSlotRoles() !== []; + } + } diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php index d219267298..d74babae02 100644 --- a/includes/Storage/RevisionStore.php +++ b/includes/Storage/RevisionStore.php @@ -466,6 +466,12 @@ class RevisionStore $this->failOnNull( $user->getId(), 'user field' ); $this->failOnEmpty( $user->getName(), 'user_text field' ); + if ( !$rev->isReadyForInsertion() ) { + // This is here for future-proofing. At the time this check being added, it + // was redundant to the individual checks above. + throw new IncompleteRevisionException( 'Revision is incomplete' ); + } + // TODO: we shouldn't need an actual Title here. $title = Title::newFromLinkTarget( $rev->getPageAsLinkTarget() ); $pageId = $this->failOnEmpty( $rev->getPageId(), 'rev_page field' ); // check this early diff --git a/includes/Storage/RevisionStoreRecord.php b/includes/Storage/RevisionStoreRecord.php index d092f22ed9..6148c44366 100644 --- a/includes/Storage/RevisionStoreRecord.php +++ b/includes/Storage/RevisionStoreRecord.php @@ -207,4 +207,13 @@ class RevisionStoreRecord extends RevisionRecord { return parent::getTimestamp(); } + /** + * @see RevisionStore::isComplete + * + * @return bool always true. + */ + public function isReadyForInsertion() { + return true; + } + } diff --git a/tests/phpunit/includes/Storage/MutableRevisionRecordTest.php b/tests/phpunit/includes/Storage/MutableRevisionRecordTest.php index 43678f9d98..48bf4aa665 100644 --- a/tests/phpunit/includes/Storage/MutableRevisionRecordTest.php +++ b/tests/phpunit/includes/Storage/MutableRevisionRecordTest.php @@ -14,6 +14,7 @@ use MediaWiki\User\UserIdentityValue; use MediaWikiTestCase; use TextContent; use Title; +use User; use WikitextContent; /** @@ -53,6 +54,7 @@ class MutableRevisionRecordTest extends MediaWikiTestCase { $record->setContent( 'main', new TextContent( 'Lorem Ipsum' ) ); $record->setComment( $comment ); $record->setUser( $user ); + $record->setTimestamp( '20101010000000' ); return $record; } @@ -294,4 +296,52 @@ class MutableRevisionRecordTest extends MediaWikiTestCase { $this->assertFalse( $record->hasSlot( 'c' ) ); } + public function provideNotReadyForInsertion() { + /** @var Title $title */ + $title = $this->getMock( Title::class ); + + /** @var User $user */ + $user = $this->getMock( User::class ); + + /** @var CommentStoreComment $comment */ + $comment = $this->getMockBuilder( CommentStoreComment::class ) + ->disableOriginalConstructor() + ->getMock(); + + $content = new TextContent( 'Test' ); + + $rev = new MutableRevisionRecord( $title ); + yield 'empty' => [ $rev ]; + + $rev = new MutableRevisionRecord( $title ); + $rev->setContent( 'main', $content ); + $rev->setUser( $user ); + $rev->setComment( $comment ); + yield 'no timestamp' => [ $rev ]; + + $rev = new MutableRevisionRecord( $title ); + $rev->setUser( $user ); + $rev->setComment( $comment ); + $rev->setTimestamp( '20101010000000' ); + yield 'no content' => [ $rev ]; + + $rev = new MutableRevisionRecord( $title ); + $rev->setContent( 'main', $content ); + $rev->setComment( $comment ); + $rev->setTimestamp( '20101010000000' ); + yield 'no user' => [ $rev ]; + + $rev = new MutableRevisionRecord( $title ); + $rev->setUser( $user ); + $rev->setContent( 'main', $content ); + $rev->setTimestamp( '20101010000000' ); + yield 'no comment' => [ $rev ]; + } + + /** + * @dataProvider provideNotReadyForInsertion + */ + public function testNotReadyForInsertion( $rev ) { + $this->assertFalse( $rev->isReadyForInsertion() ); + } } diff --git a/tests/phpunit/includes/Storage/RevisionRecordTests.php b/tests/phpunit/includes/Storage/RevisionRecordTests.php index eb048a7edc..df7ee7276c 100644 --- a/tests/phpunit/includes/Storage/RevisionRecordTests.php +++ b/tests/phpunit/includes/Storage/RevisionRecordTests.php @@ -517,4 +517,9 @@ trait RevisionRecordTests { } } + public function testIsReadyForInsertion() { + $rev = $this->newRevision(); + $this->assertTrue( $rev->isReadyForInsertion() ); + } + } -- 2.20.1