Merge "Fix field names and behavior in SlotRecord."
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 15 Mar 2018 16:07:28 +0000 (16:07 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 15 Mar 2018 16:07:28 +0000 (16:07 +0000)
includes/Storage/RevisionStore.php
includes/Storage/SlotRecord.php
tests/phpunit/includes/Storage/MutableRevisionRecordTest.php
tests/phpunit/includes/Storage/RevisionStoreDbTest.php
tests/phpunit/includes/Storage/SlotRecordTest.php

index e00deef..98ad287 100644 (file)
@@ -455,7 +455,7 @@ class RevisionStore
                        $dbw->insert( 'ip_changes', $ipcRow, __METHOD__ );
                }
 
-               $newSlot = SlotRecord::newSaved( $row['rev_id'], $blobAddress, $slot );
+               $newSlot = SlotRecord::newSaved( $row['rev_id'], $textId, $blobAddress, $slot );
                $slots = new RevisionSlots( [ 'main' => $newSlot ] );
 
                $rev = new RevisionStoreRecord(
@@ -751,6 +751,10 @@ class RevisionStore
        private function emulateMainSlot_1_29( $row, $queryFlags, Title $title ) {
                $mainSlotRow = new stdClass();
                $mainSlotRow->role_name = 'main';
+               $mainSlotRow->model_name = null;
+               $mainSlotRow->slot_revision_id = null;
+               $mainSlotRow->content_address = null;
+               $mainSlotRow->slot_content_id = null;
 
                $content = null;
                $blobData = null;
@@ -763,7 +767,8 @@ class RevisionStore
                        }
 
                        if ( isset( $row->rev_text_id ) && $row->rev_text_id > 0 ) {
-                               $mainSlotRow->cont_address = 'tt:' . $row->rev_text_id;
+                               $mainSlotRow->slot_content_id = $row->rev_text_id;
+                               $mainSlotRow->content_address = 'tt:' . $row->rev_text_id;
                        }
 
                        if ( isset( $row->old_text ) ) {
@@ -776,10 +781,10 @@ class RevisionStore
                                $blobFlags = ( $row->old_flags === null ) ? '' : $row->old_flags;
                        }
 
-                       $mainSlotRow->slot_revision = intval( $row->rev_id );
+                       $mainSlotRow->slot_revision_id = intval( $row->rev_id );
 
-                       $mainSlotRow->cont_size = isset( $row->rev_len ) ? intval( $row->rev_len ) : null;
-                       $mainSlotRow->cont_sha1 = isset( $row->rev_sha1 ) ? strval( $row->rev_sha1 ) : null;
+                       $mainSlotRow->content_size = isset( $row->rev_len ) ? intval( $row->rev_len ) : null;
+                       $mainSlotRow->content_sha1 = isset( $row->rev_sha1 ) ? strval( $row->rev_sha1 ) : null;
                        $mainSlotRow->model_name = isset( $row->rev_content_model )
                                ? strval( $row->rev_content_model )
                                : null;
@@ -788,13 +793,16 @@ class RevisionStore
                                ? strval( $row->rev_content_format )
                                : null;
                } elseif ( is_array( $row ) ) {
-                       $mainSlotRow->slot_revision = isset( $row['id'] ) ? intval( $row['id'] ) : null;
+                       $mainSlotRow->slot_revision_id = isset( $row['id'] ) ? intval( $row['id'] ) : null;
 
-                       $mainSlotRow->cont_address = isset( $row['text_id'] )
+                       $mainSlotRow->slot_content_id = isset( $row['text_id'] )
+                               ? intval( $row['text_id'] )
+                               : null;
+                       $mainSlotRow->content_address = isset( $row['text_id'] )
                                ? 'tt:' . intval( $row['text_id'] )
                                : null;
-                       $mainSlotRow->cont_size = isset( $row['len'] ) ? intval( $row['len'] ) : null;
-                       $mainSlotRow->cont_sha1 = isset( $row['sha1'] ) ? strval( $row['sha1'] ) : null;
+                       $mainSlotRow->content_size = isset( $row['len'] ) ? intval( $row['len'] ) : null;
+                       $mainSlotRow->content_sha1 = isset( $row['sha1'] ) ? strval( $row['sha1'] ) : null;
 
                        $mainSlotRow->model_name = isset( $row['content_model'] )
                                ? strval( $row['content_model'] ) : null;  // XXX: must be a string!
@@ -853,6 +861,7 @@ class RevisionStore
                        };
                }
 
+               $mainSlotRow->slot_id = $mainSlotRow->slot_revision_id;
                return new SlotRecord( $mainSlotRow, $content );
        }
 
index 8769330..b59d92f 100644 (file)
@@ -23,6 +23,7 @@
 namespace MediaWiki\Storage;
 
 use Content;
+use InvalidArgumentException;
 use LogicException;
 use OutOfBoundsException;
 use Wikimedia\Assert\Assert;
@@ -72,7 +73,8 @@ class SlotRecord {
         * @return SlotRecord
         */
        private static function newDerived( SlotRecord $slot, array $overrides = [] ) {
-               $row = $slot->row;
+               $row = clone $slot->row;
+               $row->slot_id = null; // never copy the row ID!
 
                foreach ( $overrides as $key => $value ) {
                        $row->$key = $value;
@@ -85,6 +87,10 @@ class SlotRecord {
         * Constructs a new SlotRecord for a new revision, inheriting the content of the given SlotRecord
         * of a previous revision.
         *
+        * Note that a SlotRecord constructed this way are intended as prototypes,
+        * to be used wit newSaved(). They are incomplete, so some getters such as
+        * getRevision() will fail.
+        *
         * @param SlotRecord $slot
         *
         * @return SlotRecord
@@ -92,32 +98,35 @@ class SlotRecord {
        public static function newInherited( SlotRecord $slot ) {
                return self::newDerived( $slot, [
                        'slot_inherited' => true,
-                       'slot_revision' => null,
+                       'slot_revision_id' => null,
                ] );
        }
 
        /**
         * Constructs a new Slot from a Content object for a new revision.
         * This is the preferred way to construct a slot for storing Content that
-        * resulted from a user edit.
+        * resulted from a user edit. The slot is assumed to be not inherited.
+        *
+        * Note that a SlotRecord constructed this way are intended as prototypes,
+        * to be used wit newSaved(). They are incomplete, so some getters such as
+        * getAddress() will fail.
         *
         * @param string $role
         * @param Content $content
-        * @param bool $inherited
         *
-        * @return SlotRecord
+        * @return SlotRecord An incomplete proto-slot object, to be used with newSaved() later.
         */
-       public static function newUnsaved( $role, Content $content, $inherited = false ) {
-               Assert::parameterType( 'boolean', $inherited, '$inherited' );
+       public static function newUnsaved( $role, Content $content ) {
                Assert::parameterType( 'string', $role, '$role' );
 
                $row = [
                        'slot_id' => null, // not yet known
-                       'slot_address' => null, // not yet known. need setter?
-                       'slot_revision' => null, // not yet known
-                       'slot_inherited' => $inherited,
-                       'cont_size' => null, // compute later
-                       'cont_sha1' => null, // compute later
+                       'slot_revision_id' => null, // not yet known
+                       'slot_inherited' => 0, // not inherited
+                       'content_size' => null, // compute later
+                       'content_sha1' => null, // compute later
+                       'slot_content_id' => null, // not yet known, will be set in newSaved()
+                       'content_address' => null, // not yet known, will be set in newSaved()
                        'role_name' => $role,
                        'model_name' => $content->getModel(),
                ];
@@ -126,23 +135,64 @@ class SlotRecord {
        }
 
        /**
-        * Constructs a SlotRecord for a newly saved revision, based on the proto-slot that was
-        * supplied to the code that performed the save operation. This adds information that
-        * has only become available during saving, particularly the revision ID and blob address.
-        *
-        * @param int $revisionId
-        * @param string $blobAddress
-        * @param SlotRecord $protoSlot The proto-slot that was provided to the code that then
-        *
-        * @return SlotRecord
+        * Constructs a complete SlotRecord for a newly saved revision, based on the incomplete
+        * proto-slot. This adds information that has only become available during saving,
+        * particularly the revision ID and content address.
+        *
+        * @param int $revisionId the revision the slot is to be associated with (field slot_revision_id).
+        *        If $protoSlot already has a revision, it must be the same.
+        * @param int $contentId the ID of the row in the content table describing the content
+        *        referenced by $contentAddress (field slot_content_id).
+        *        If $protoSlot already has a content ID, it must be the same.
+        * @param string $contentAddress the slot's content address (field content_address).
+        *        If $protoSlot already has an address, it must be the same.
+        * @param SlotRecord $protoSlot The proto-slot that was provided as input for creating a new
+        *        revision. $protoSlot must have a content address if inherited.
+        *
+        * @return SlotRecord If the state of $protoSlot is inappropriate for saving a new revision.
         */
-       public static function newSaved( $revisionId, $blobAddress, SlotRecord $protoSlot ) {
+       public static function newSaved(
+               $revisionId,
+               $contentId,
+               $contentAddress,
+               SlotRecord $protoSlot
+       ) {
                Assert::parameterType( 'integer', $revisionId, '$revisionId' );
-               Assert::parameterType( 'string', $blobAddress, '$blobAddress' );
+               Assert::parameterType( 'integer', $contentId, '$contentId' );
+               Assert::parameterType( 'string', $contentAddress, '$contentAddress' );
+
+               if ( $protoSlot->hasRevision() && $protoSlot->getRevision() !== $revisionId ) {
+                       throw new LogicException(
+                               "Mismatching revision ID $revisionId: "
+                               . "The slot already belongs to revision {$protoSlot->getRevision()}. "
+                               . "Use SlotRecord::newInherited() to re-use content between revisions."
+                       );
+               }
+
+               if ( $protoSlot->hasAddress() && $protoSlot->getAddress() !== $contentAddress ) {
+                       throw new LogicException(
+                               "Mismatching blob address $contentAddress: "
+                               . "The slot already has content at {$protoSlot->getAddress()}."
+                       );
+               }
+
+               if ( $protoSlot->hasAddress() && $protoSlot->getContentId() !== $contentId ) {
+                       throw new LogicException(
+                               "Mismatching content ID $contentId: "
+                               . "The slot already has content row {$protoSlot->getContentId()} associated."
+                       );
+               }
+
+               if ( $protoSlot->isInherited() && !$protoSlot->hasAddress() ) {
+                       throw new InvalidArgumentException(
+                               "An inherited blob should have a content address!"
+                       );
+               }
 
                return self::newDerived( $protoSlot, [
-                       'slot_revision' => $revisionId,
-                       'cont_address' => $blobAddress,
+                       'slot_revision_id' => $revisionId,
+                       'slot_content_id' => $contentId,
+                       'content_address' => $contentAddress,
                ] );
        }
 
@@ -165,6 +215,37 @@ class SlotRecord {
                Assert::parameterType( 'object', $row, '$row' );
                Assert::parameterType( 'Content|callable', $content, '$content' );
 
+               Assert::parameter(
+                       property_exists( $row, 'slot_id' ),
+                       '$row->slot_id',
+                       'must exist'
+               );
+               Assert::parameter(
+                       property_exists( $row, 'slot_revision_id' ),
+                       '$row->slot_revision_id',
+                       'must exist'
+               );
+               Assert::parameter(
+                       property_exists( $row, 'slot_inherited' ),
+                       '$row->slot_inherited',
+                       'must exist'
+               );
+               Assert::parameter(
+                       property_exists( $row, 'slot_content_id' ),
+                       '$row->slot_content_id',
+                       'must exist'
+               );
+               Assert::parameter(
+                       property_exists( $row, 'content_address' ),
+                       '$row->content_address',
+                       'must exist'
+               );
+               Assert::parameter(
+                       property_exists( $row, 'model_name' ),
+                       '$row->model_name',
+                       'must exist'
+               );
+
                $this->row = $row;
                $this->content = $content;
        }
@@ -217,7 +298,8 @@ class SlotRecord {
         * @param string $name
         *
         * @throws OutOfBoundsException
-        * @return mixed Returns the field's value, or null if the field is NULL in the DB row.
+        * @throws IncompleteRevisionException
+        * @return mixed Returns the field's value, never null.
         */
        private function getField( $name ) {
                if ( !isset( $this->row->$name ) ) {
@@ -280,7 +362,7 @@ class SlotRecord {
         * @return int
         */
        public function getRevision() {
-               return $this->getIntField( 'slot_revision' );
+               return $this->getIntField( 'slot_revision_id' );
        }
 
        /**
@@ -300,7 +382,7 @@ class SlotRecord {
         * @return bool
         */
        public function hasAddress() {
-               return $this->hasField( 'cont_address' );
+               return $this->hasField( 'content_address' );
        }
 
        /**
@@ -311,7 +393,7 @@ class SlotRecord {
         * @return bool
         */
        public function hasRevision() {
-               return $this->hasField( 'slot_revision' );
+               return $this->hasField( 'slot_revision_id' );
        }
 
        /**
@@ -330,7 +412,18 @@ class SlotRecord {
         * @return string
         */
        public function getAddress() {
-               return $this->getStringField( 'cont_address' );
+               return $this->getStringField( 'content_address' );
+       }
+
+       /**
+        * Returns the ID of the content meta data row associated with the slot.
+        * This information should be irrelevant to application logic, it is here to allow
+        * the construction of a full row for the revision table.
+        *
+        * @return int
+        */
+       public function getContentId() {
+               return $this->getIntField( 'slot_content_id' );
        }
 
        /**
@@ -340,10 +433,10 @@ class SlotRecord {
         */
        public function getSize() {
                try {
-                       $size = $this->getIntField( 'cont_size' );
+                       $size = $this->getIntField( 'content_size' );
                } catch ( IncompleteRevisionException $ex ) {
                        $size = $this->getContent()->getSize();
-                       $this->setField( 'cont_size', $size );
+                       $this->setField( 'content_size', $size );
                }
 
                return $size;
@@ -356,7 +449,7 @@ class SlotRecord {
         */
        public function getSha1() {
                try {
-                       $sha1 = $this->getStringField( 'cont_sha1' );
+                       $sha1 = $this->getStringField( 'content_sha1' );
                } catch ( IncompleteRevisionException $ex ) {
                        $format = $this->hasField( 'format_name' )
                                ? $this->getStringField( 'format_name' )
@@ -364,7 +457,7 @@ class SlotRecord {
 
                        $data = $this->getContent()->serialize( $format );
                        $sha1 = self::base36Sha1( $data );
-                       $this->setField( 'cont_sha1', $sha1 );
+                       $this->setField( 'content_sha1', $sha1 );
                }
 
                return $sha1;
index 807099f..675201e 100644 (file)
@@ -68,8 +68,8 @@ class MutableRevisionRecordTest extends MediaWikiTestCase {
 
        public function testSimpleSetGetSlot() {
                $record = new MutableRevisionRecord( Title::newFromText( 'Foo' ) );
-               $slot = new SlotRecord(
-                       (object)[ 'role_name' => 'main' ],
+               $slot = SlotRecord::newUnsaved(
+                       'main',
                        new WikitextContent( 'x' )
                );
                $record->setSlot( $slot );
index e81f0af..c713e2c 100644 (file)
@@ -167,8 +167,8 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                $this->assertEquals( $r1->getWikiId(), $r2->getWikiId() );
                $this->assertEquals( $r1->isMinor(), $r2->isMinor() );
                foreach ( $r1->getSlotRoles() as $role ) {
-                       $this->assertEquals( $r1->getSlot( $role ), $r2->getSlot( $role ) );
-                       $this->assertEquals( $r1->getContent( $role ), $r2->getContent( $role ) );
+                       $this->assertSlotRecordsEqual( $r1->getSlot( $role ), $r2->getSlot( $role ) );
+                       $this->assertTrue( $r1->getContent( $role )->equals( $r2->getContent( $role ) ) );
                }
                foreach ( [
                        RevisionRecord::DELETED_TEXT,
@@ -180,6 +180,29 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                }
        }
 
+       private function assertSlotRecordsEqual( SlotRecord $s1, SlotRecord $s2 ) {
+               $this->assertSame( $s1->getRole(), $s2->getRole() );
+               $this->assertSame( $s1->getModel(), $s2->getModel() );
+               $this->assertSame( $s1->getFormat(), $s2->getFormat() );
+               $this->assertSame( $s1->getSha1(), $s2->getSha1() );
+               $this->assertSame( $s1->getSize(), $s2->getSize() );
+               $this->assertTrue( $s1->getContent()->equals( $s2->getContent() ) );
+
+               $s1->hasRevision() ? $this->assertSame( $s1->getRevision(), $s2->getRevision() ) : null;
+               $s1->hasAddress() ? $this->assertSame( $s1->hasAddress(), $s2->hasAddress() ) : null;
+       }
+
+       private function assertRevisionCompleteness( RevisionRecord $r ) {
+               foreach ( $r->getSlotRoles() as $role ) {
+                       $this->assertSlotCompleteness( $r, $r->getSlot( $role ) );
+               }
+       }
+
+       private function assertSlotCompleteness( RevisionRecord $r, SlotRecord $slot ) {
+               $this->assertTrue( $slot->hasAddress() );
+               $this->assertSame( $r->getId(), $slot->getRevision() );
+       }
+
        /**
         * @param mixed[] $details
         *
@@ -257,6 +280,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
 
                $this->assertLinkTargetsEqual( $title, $return->getPageAsLinkTarget() );
                $this->assertRevisionRecordsEqual( $rev, $return );
+               $this->assertRevisionCompleteness( $return );
        }
 
        /**
index 27fcd0c..ef31315 100644 (file)
@@ -2,10 +2,12 @@
 
 namespace MediaWiki\Tests\Storage;
 
+use InvalidArgumentException;
+use LogicException;
+use MediaWiki\Storage\IncompleteRevisionException;
 use MediaWiki\Storage\SlotRecord;
+use MediaWiki\Storage\SuppressedDataException;
 use MediaWikiTestCase;
-use RuntimeException;
-use Wikimedia\Assert\ParameterTypeException;
 use WikitextContent;
 
 /**
@@ -13,52 +15,88 @@ use WikitextContent;
  */
 class SlotRecordTest extends MediaWikiTestCase {
 
-       public function provideAContent() {
-               yield [ new WikitextContent( 'A' ) ];
-               yield [
-                       function ( SlotRecord $slotRecord ) {
-                               if ( $slotRecord->getAddress() === 'tt:456' ) {
-                                       return new WikitextContent( 'A' );
-                               }
-                               throw new RuntimeException( 'Got Wrong SlotRecord for callback' );
-                       },
-               ];
-       }
-
-       /**
-        * @dataProvider provideAContent
-        */
-       public function testValidConstruction( $content ) {
-               $row = (object)[
-                       'cont_size' => '1',
-                       'cont_sha1' => 'someHash',
-                       'cont_address' => 'tt:456',
-                       'model_name' => 'aModelname',
-                       'slot_revision' => '2',
-                       'format_name' => 'someFormatName',
+       private function makeRow( $data = [] ) {
+               $data = $data + [
+                       'slot_id' => 1234,
+                       'slot_content_id' => 33,
+                       'content_size' => '5',
+                       'content_sha1' => 'someHash',
+                       'content_address' => 'tt:456',
+                       'model_name' => CONTENT_MODEL_WIKITEXT,
+                       'format_name' => CONTENT_FORMAT_WIKITEXT,
+                       'slot_revision_id' => '2',
+                       'slot_inherited' => '1',
                        'role_name' => 'myRole',
-                       'slot_inherited' => '99'
                ];
+               return (object)$data;
+       }
 
-               $record = new SlotRecord( $row, $content );
+       public function testCompleteConstruction() {
+               $row = $this->makeRow();
+               $record = new SlotRecord( $row, new WikitextContent( 'A' ) );
 
+               $this->assertTrue( $record->hasAddress() );
+               $this->assertTrue( $record->hasRevision() );
+               $this->assertTrue( $record->isInherited() );
                $this->assertSame( 'A', $record->getContent()->getNativeData() );
-               $this->assertSame( 1, $record->getSize() );
+               $this->assertSame( 5, $record->getSize() );
                $this->assertSame( 'someHash', $record->getSha1() );
-               $this->assertSame( 'aModelname', $record->getModel() );
+               $this->assertSame( CONTENT_MODEL_WIKITEXT, $record->getModel() );
                $this->assertSame( 2, $record->getRevision() );
                $this->assertSame( 'tt:456', $record->getAddress() );
-               $this->assertSame( 'someFormatName', $record->getFormat() );
+               $this->assertSame( 33, $record->getContentId() );
+               $this->assertSame( CONTENT_FORMAT_WIKITEXT, $record->getFormat() );
                $this->assertSame( 'myRole', $record->getRole() );
+       }
+
+       public function testConstructionDeferred() {
+               $row = $this->makeRow( [
+                       'content_size' => null, // to be computed
+                       'content_sha1' => null, // to be computed
+                       'format_name' => function () {
+                               return CONTENT_FORMAT_WIKITEXT;
+                       },
+                       'slot_inherited' => '0'
+               ] );
+
+               $content = function () {
+                       return new WikitextContent( 'A' );
+               };
+
+               $record = new SlotRecord( $row, $content );
+
                $this->assertTrue( $record->hasAddress() );
                $this->assertTrue( $record->hasRevision() );
-               $this->assertTrue( $record->isInherited() );
+               $this->assertFalse( $record->isInherited() );
+               $this->assertSame( 'A', $record->getContent()->getNativeData() );
+               $this->assertSame( 1, $record->getSize() );
+               $this->assertNotNull( $record->getSha1() );
+               $this->assertSame( CONTENT_MODEL_WIKITEXT, $record->getModel() );
+               $this->assertSame( 2, $record->getRevision() );
+               $this->assertSame( 'tt:456', $record->getAddress() );
+               $this->assertSame( 33, $record->getContentId() );
+               $this->assertSame( CONTENT_FORMAT_WIKITEXT, $record->getFormat() );
+               $this->assertSame( 'myRole', $record->getRole() );
+       }
+
+       public function testNewUnsaved() {
+               $record = SlotRecord::newUnsaved( 'myRole', new WikitextContent( 'A' ) );
+
+               $this->assertFalse( $record->hasAddress() );
+               $this->assertFalse( $record->hasRevision() );
+               $this->assertFalse( $record->isInherited() );
+               $this->assertSame( 'A', $record->getContent()->getNativeData() );
+               $this->assertSame( 1, $record->getSize() );
+               $this->assertNotNull( $record->getSha1() );
+               $this->assertSame( CONTENT_MODEL_WIKITEXT, $record->getModel() );
+               $this->assertSame( 'myRole', $record->getRole() );
        }
 
        public function provideInvalidConstruction() {
                yield 'both null' => [ null, null ];
                yield 'null row' => [ null, new WikitextContent( 'A' ) ];
-               yield 'array row' => [ null, new WikitextContent( 'A' ) ];
+               yield 'array row' => [ [], new WikitextContent( 'A' ) ];
+               yield 'empty row' => [ (object)[], new WikitextContent( 'A' ) ];
                yield 'null content' => [ (object)[], null ];
        }
 
@@ -66,25 +104,168 @@ class SlotRecordTest extends MediaWikiTestCase {
         * @dataProvider provideInvalidConstruction
         */
        public function testInvalidConstruction( $row, $content ) {
-               $this->setExpectedException( ParameterTypeException::class );
+               $this->setExpectedException( InvalidArgumentException::class );
                new SlotRecord( $row, $content );
        }
 
-       public function testHasAddress_false() {
-               $record = new SlotRecord( (object)[], new WikitextContent( 'A' ) );
-               $this->assertFalse( $record->hasAddress() );
+       public function testGetContentId_fails() {
+               $record = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) );
+               $this->setExpectedException( IncompleteRevisionException::class );
+
+               $record->getContentId();
        }
 
-       public function testHasRevision_false() {
-               $record = new SlotRecord( (object)[], new WikitextContent( 'A' ) );
-               $this->assertFalse( $record->hasRevision() );
+       public function testGetAddress_fails() {
+               $record = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) );
+               $this->setExpectedException( IncompleteRevisionException::class );
+
+               $record->getAddress();
        }
 
-       public function testInInherited_false() {
-               // TODO unskip me once fixed.
-               $this->markTestSkipped( 'Should probably return false, needs fixing?' );
-               $record = new SlotRecord( (object)[], new WikitextContent( 'A' ) );
-               $this->assertFalse( $record->isInherited() );
+       public function testGetRevision_fails() {
+               $record = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) );
+               $this->setExpectedException( IncompleteRevisionException::class );
+
+               $record->getRevision();
+       }
+
+       public function provideHashStability() {
+               yield [ '', 'phoiac9h4m842xq45sp7s6u21eteeq1' ];
+               yield [ 'Lorem ipsum', 'hcr5u40uxr81d3nx89nvwzclfz6r9c5' ];
+       }
+
+       /**
+        * @dataProvider provideHashStability
+        */
+       public function testHashStability( $text, $hash ) {
+               // Changing the output of the hash function will break things horribly!
+
+               $this->assertSame( $hash, SlotRecord::base36Sha1( $text ) );
+
+               $record = SlotRecord::newUnsaved( 'main', new WikitextContent( $text ) );
+               $this->assertSame( $hash, $record->getSha1() );
+       }
+
+       public function testNewWithSuppressedContent() {
+               $input = new SlotRecord( $this->makeRow(), new WikitextContent( 'A' ) );
+               $output = SlotRecord::newWithSuppressedContent( $input );
+
+               $this->setExpectedException( SuppressedDataException::class );
+               $output->getContent();
+       }
+
+       public function testNewInherited() {
+               $row = $this->makeRow( [ 'slot_revision_id' => 7, 'slot_inherited' => 0 ] );
+               $parent = new SlotRecord( $row, new WikitextContent( 'A' ) );
+
+               // This would happen while doing an edit, before saving revision meta-data.
+               $inherited = SlotRecord::newInherited( $parent );
+
+               $this->assertSame( $parent->getContentId(), $inherited->getContentId() );
+               $this->assertSame( $parent->getAddress(), $inherited->getAddress() );
+               $this->assertSame( $parent->getContent(), $inherited->getContent() );
+               $this->assertTrue( $inherited->isInherited() );
+               $this->assertFalse( $inherited->hasRevision() );
+
+               // make sure we didn't mess with the internal state of $parent
+               $this->assertFalse( $parent->isInherited() );
+               $this->assertSame( 7, $parent->getRevision() );
+
+               // This would happen while doing an edit, after saving the revision meta-data
+               // and content meta-data.
+               $saved = SlotRecord::newSaved(
+                       10,
+                       $inherited->getContentId(),
+                       $inherited->getAddress(),
+                       $inherited
+               );
+               $this->assertSame( $parent->getContentId(), $saved->getContentId() );
+               $this->assertSame( $parent->getAddress(), $saved->getAddress() );
+               $this->assertSame( $parent->getContent(), $saved->getContent() );
+               $this->assertTrue( $saved->isInherited() );
+               $this->assertTrue( $saved->hasRevision() );
+               $this->assertSame( 10, $saved->getRevision() );
+
+               // make sure we didn't mess with the internal state of $parent or $inherited
+               $this->assertSame( 7, $parent->getRevision() );
+               $this->assertFalse( $inherited->hasRevision() );
+       }
+
+       public function testNewSaved() {
+               // This would happen while doing an edit, before saving revision meta-data.
+               $unsaved = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) );
+
+               // This would happen while doing an edit, after saving the revision meta-data
+               // and content meta-data.
+               $saved = SlotRecord::newSaved( 10, 20, 'theNewAddress', $unsaved );
+               $this->assertFalse( $saved->isInherited() );
+               $this->assertTrue( $saved->hasRevision() );
+               $this->assertTrue( $saved->hasAddress() );
+               $this->assertSame( 'theNewAddress', $saved->getAddress() );
+               $this->assertSame( 20, $saved->getContentId() );
+               $this->assertSame( 'A', $saved->getContent()->getNativeData() );
+               $this->assertSame( 10, $saved->getRevision() );
+
+               // make sure we didn't mess with the internal state of $unsaved
+               $this->assertFalse( $unsaved->hasAddress() );
+               $this->assertFalse( $unsaved->hasRevision() );
+       }
+
+       public function provideNewSaved_LogicException() {
+               $freshRow = $this->makeRow( [
+                       'content_id' => 10,
+                       'content_address' => 'address:1',
+                       'slot_inherited' => 0,
+                       'slot_revision_id' => 1,
+               ] );
+
+               $freshSlot = new SlotRecord( $freshRow, new WikitextContent( 'A' ) );
+               yield 'mismatching address' => [ 1, 10, 'address:BAD', $freshSlot ];
+               yield 'mismatching revision' => [ 5, 10, 'address:1', $freshSlot ];
+               yield 'mismatching content ID' => [ 1, 17, 'address:1', $freshSlot ];
+
+               $inheritedRow = $this->makeRow( [
+                       'content_id' => null,
+                       'content_address' => null,
+                       'slot_inherited' => 1
+               ] );
+
+               $inheritedSlot = new SlotRecord( $inheritedRow, new WikitextContent( 'A' ) );
+               yield 'inherited, but no address' => [ 1, 10, 'address:2', $inheritedSlot ];
+       }
+
+       /**
+        * @dataProvider provideNewSaved_LogicException
+        */
+       public function testNewSaved_LogicException(
+               $revisionId,
+               $contentId,
+               $contentAddress,
+               SlotRecord $protoSlot
+       ) {
+               $this->setExpectedException( LogicException::class );
+               SlotRecord::newSaved( $revisionId, $contentId, $contentAddress, $protoSlot );
+       }
+
+       public function provideNewSaved_InvalidArgumentException() {
+               $unsaved = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) );
+
+               yield 'bad revision id' => [ 'xyzzy', 5, 'address', $unsaved ];
+               yield 'bad content id' => [ 7, 'xyzzy', 'address', $unsaved ];
+               yield 'bad content address' => [ 7, 5, 77, $unsaved ];
+       }
+
+       /**
+        * @dataProvider provideNewSaved_InvalidArgumentException
+        */
+       public function testNewSaved_InvalidArgumentException(
+               $revisionId,
+               $contentId,
+               $contentAddress,
+               SlotRecord $protoSlot
+       ) {
+               $this->setExpectedException( InvalidArgumentException::class );
+               SlotRecord::newSaved( $revisionId, $contentId, $contentAddress, $protoSlot );
        }
 
 }