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

diff --git a/includes/Storage/RevisionArchiveRecord.php b/includes/Storage/RevisionArchiveRecord.php
index 213ee3c..173da51 100644 (file)
--- 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 17c56ea..8c31a3c 100644 (file)
--- 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 d219267..d74baba 100644 (file)
--- 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 d092f22..6148c44 100644 (file)
--- 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 43678f9..48bf4aa 100644 (file)
--- 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 eb048a7..df7ee72 100644 (file)
--- 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() );
+       }
+
 }