Merge "[MCR] RevisionStore, enable insertions for new schema"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 14 Jun 2018 14:11:27 +0000 (14:11 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 14 Jun 2018 14:11:27 +0000 (14:11 +0000)
16 files changed:
includes/DefaultSettings.php
includes/ServiceWiring.php
includes/Storage/RevisionStore.php
includes/Storage/SlotRecord.php
includes/page/WikiPage.php
tests/common/TestsAutoLoader.php
tests/phpunit/MediaWikiTestCase.php
tests/phpunit/includes/RevisionDbTestBase.php
tests/phpunit/includes/RevisionMcrWriteBothDbTest.php [new file with mode: 0644]
tests/phpunit/includes/RevisionTest.php
tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php [new file with mode: 0644]
tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php [new file with mode: 0644]
tests/phpunit/includes/Storage/RevisionStoreDbTestBase.php
tests/phpunit/includes/Storage/RevisionStoreTest.php
tests/phpunit/includes/Storage/SlotRecordTest.php
tests/phpunit/includes/page/WikiPageMcrWriteBothDbTest.php [new file with mode: 0644]

index 56fb534..02cbc2f 100644 (file)
@@ -8893,6 +8893,17 @@ $wgInterwikiPrefixDisplayTypes = [];
  */
 $wgCommentTableSchemaMigrationStage = MIGRATION_OLD;
 
+/**
+ * RevisionStore table schema migration stage (content, slots, content_models & slot_roles tables)
+ *
+ * @see Task: https://phabricator.wikimedia.org/T174028
+ * @see Commit: https://gerrit.wikimedia.org/r/#/c/378724/
+ *
+ * @since 1.32
+ * @var int One of the MIGRATION_* constants
+ */
+$wgMultiContentRevisionSchemaMigrationStage = MIGRATION_OLD;
+
 /**
  * Actor table schema migration stage.
  * @since 1.31
index 3f8ba18..379424c 100644 (file)
@@ -471,6 +471,9 @@ return [
                        $blobStore,
                        $services->getMainWANObjectCache(),
                        $services->getCommentStore(),
+                       $services->getContentModelStore(),
+                       $services->getSlotRoleStore(),
+                       $services->getMainConfig()->get( 'MultiContentRevisionSchemaMigrationStage' ),
                        $services->getActorMigration()
                );
 
index ce09a6e..3f852b0 100644 (file)
@@ -69,6 +69,8 @@ use Wikimedia\Rdbms\LoadBalancer;
 class RevisionStore
        implements IDBAccessObject, RevisionFactory, RevisionLookup, LoggerAwareInterface {
 
+       const ROW_CACHE_KEY = 'revision-row-1.29';
+
        /**
         * @var SqlBlobStore
         */
@@ -109,6 +111,19 @@ class RevisionStore
         */
        private $logger;
 
+       /**
+        * @var NameTableStore
+        */
+       private $contentModelStore;
+
+       /**
+        * @var NameTableStore
+        */
+       private $slotRoleStore;
+
+       /** @var int One of the MIGRATION_* constants */
+       private $mcrMigrationStage;
+
        /**
         * @todo $blobStore should be allowed to be any BlobStore!
         *
@@ -116,6 +131,9 @@ class RevisionStore
         * @param SqlBlobStore $blobStore
         * @param WANObjectCache $cache
         * @param CommentStore $commentStore
+        * @param NameTableStore $contentModelStore
+        * @param NameTableStore $slotRoleStore
+        * @param int $migrationStage
         * @param ActorMigration $actorMigration
         * @param bool|string $wikiId
         */
@@ -124,15 +142,26 @@ class RevisionStore
                SqlBlobStore $blobStore,
                WANObjectCache $cache,
                CommentStore $commentStore,
+               NameTableStore $contentModelStore,
+               NameTableStore $slotRoleStore,
+               $migrationStage,
                ActorMigration $actorMigration,
                $wikiId = false
        ) {
                Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' );
+               Assert::parameterType( 'integer', $migrationStage, '$migrationStage' );
+
+               if ( $migrationStage > MIGRATION_WRITE_BOTH ) {
+                       throw new InvalidArgumentException( 'New schema is not fully supported yet' );
+               }
 
                $this->loadBalancer = $loadBalancer;
                $this->blobStore = $blobStore;
                $this->cache = $cache;
                $this->commentStore = $commentStore;
+               $this->contentModelStore = $contentModelStore;
+               $this->slotRoleStore = $slotRoleStore;
+               $this->mcrMigrationStage = $migrationStage;
                $this->actorMigration = $actorMigration;
                $this->wikiId = $wikiId;
                $this->logger = new NullLogger();
@@ -158,8 +187,14 @@ class RevisionStore
 
        /**
         * @param bool $contentHandlerUseDB
+        * @throws MWException
         */
        public function setContentHandlerUseDB( $contentHandlerUseDB ) {
+               if ( !$contentHandlerUseDB && $this->mcrMigrationStage > MIGRATION_OLD ) {
+                       throw new MWException(
+                               'Content model must be stored in the database for multi content revision migration.'
+                       );
+               }
                $this->contentHandlerUseDB = $contentHandlerUseDB;
        }
 
@@ -334,6 +369,7 @@ class RevisionStore
                        throw new InvalidArgumentException( 'At least one slot needs to be defined!' );
                }
 
+               // RevisionStore currently only supports writing a single slot
                if ( $rev->getSlotRoles() !== [ 'main' ] ) {
                        throw new InvalidArgumentException( 'Only the main slot is supported for now!' );
                }
@@ -347,13 +383,15 @@ class RevisionStore
                        : $rev->getParentId();
 
                // Record the text (or external storage URL) to the blob store
-               $slot = $rev->getSlot( 'main', RevisionRecord::RAW );
+               $mainSlot = $rev->getSlot( 'main', RevisionRecord::RAW );
 
                $size = $this->failOnNull( $rev->getSize(), 'size field' );
                $sha1 = $this->failOnEmpty( $rev->getSha1(), 'sha1 field' );
 
-               if ( !$slot->hasAddress() ) {
-                       $content = $slot->getContent();
+               $dbw->startAtomic( __METHOD__ );
+
+               if ( !$mainSlot->hasAddress() ) {
+                       $content = $mainSlot->getContent();
                        $format = $content->getDefaultFormat();
                        $model = $content->getModel();
 
@@ -368,18 +406,18 @@ class RevisionStore
                        $blobHints = [
                                BlobStore::DESIGNATION_HINT => 'page-content', // BlobStore may be used for other things too.
                                BlobStore::PAGE_HINT => $pageId,
-                               BlobStore::ROLE_HINT => $slot->getRole(),
+                               BlobStore::ROLE_HINT => $mainSlot->getRole(),
                                BlobStore::PARENT_HINT => $parentId,
-                               BlobStore::SHA1_HINT => $slot->getSha1(),
+                               BlobStore::SHA1_HINT => $mainSlot->getSha1(),
                                BlobStore::MODEL_HINT => $model,
                                BlobStore::FORMAT_HINT => $format,
                        ];
 
                        $blobAddress = $this->blobStore->storeBlob( $data, $blobHints );
                } else {
-                       $blobAddress = $slot->getAddress();
-                       $model = $slot->getModel();
-                       $format = $slot->getFormat();
+                       $blobAddress = $mainSlot->getAddress();
+                       $model = $mainSlot->getModel();
+                       $format = $mainSlot->getFormat();
                }
 
                $textId = $this->blobStore->getTextIdFromAddress( $blobAddress );
@@ -402,11 +440,10 @@ class RevisionStore
                $this->failOnNull( $user->getId(), 'user field' );
                $this->failOnEmpty( $user->getName(), 'user_text field' );
 
-               # Record the edit in revisions
-               $row = [
+               // Record the edit in revisions
+               $revisionRow = [
                        'rev_page'       => $pageId,
                        'rev_parent_id'  => $parentId,
-                       'rev_text_id'    => $textId,
                        'rev_minor_edit' => $rev->isMinor() ? 1 : 0,
                        'rev_timestamp'  => $dbw->timestamp( $timestamp ),
                        'rev_deleted'    => $rev->getVisibility(),
@@ -416,54 +453,106 @@ class RevisionStore
 
                if ( $rev->getId() !== null ) {
                        // Needed to restore revisions with their original ID
-                       $row['rev_id'] = $rev->getId();
+                       $revisionRow['rev_id'] = $rev->getId();
                }
 
                list( $commentFields, $commentCallback ) =
                        $this->commentStore->insertWithTempTable( $dbw, 'rev_comment', $comment );
-               $row += $commentFields;
+               $revisionRow += $commentFields;
 
                list( $actorFields, $actorCallback ) =
                        $this->actorMigration->getInsertValuesWithTempTable( $dbw, 'rev_user', $user );
-               $row += $actorFields;
+               $revisionRow += $actorFields;
+
+               if ( $this->mcrMigrationStage <= MIGRATION_WRITE_BOTH ) {
+                       $revisionRow['rev_text_id'] = $textId;
 
-               if ( $this->contentHandlerUseDB ) {
                        // MCR migration note: rev_content_model and rev_content_format will go away
+                       if ( $this->contentHandlerUseDB ) {
+                               $defaultModel = ContentHandler::getDefaultModelFor( $title );
+                               $defaultFormat = ContentHandler::getForModelID( $defaultModel )->getDefaultFormat();
 
-                       $defaultModel = ContentHandler::getDefaultModelFor( $title );
-                       $defaultFormat = ContentHandler::getForModelID( $defaultModel )->getDefaultFormat();
+                               $revisionRow['rev_content_model'] = ( $model === $defaultModel ) ? null : $model;
+                               $revisionRow['rev_content_format'] = ( $format === $defaultFormat ) ? null : $format;
+                       }
+               } else {
+                       /**
+                        * rev_text_id has NOT NULL and no DEFAULT, so set to 0 when we are not writing to it.
+                        * WARNING: This should NOT be removed after migration until a schema change has been
+                        * made in WMF production giving rev_text_id a DEFAULT value of 0 (otherwise inserts
+                        * will fail)
+                        * Task: https://phabricator.wikimedia.org/T190148#4064625
+                        */
+                       $revisionRow['rev_text_id'] = 0;
+               }
+
+               $dbw->insert( 'revision', $revisionRow, __METHOD__ );
+
+               $hasSlots = false;
+               $contentId = false;
+
+               if ( isset( $revisionRow['rev_id'] ) ) {
+                       // Restoring a revision, slots should already exist,
+                       // unless the archive row wasn't migrated yet.
+                       if ( $this->mcrMigrationStage === MIGRATION_NEW ) {
+                               $hasSlots = true;
+                       } else {
+                               $contentId = $this->findSlotContentId( $dbw, $revisionRow['rev_id'], 'main' );
+                               $hasSlots = (bool)$contentId;
+                       }
+               } else {
+                       // not restoring a revision, use auto-increment value
+                       $revisionRow['rev_id'] = intval( $dbw->insertId() );
+               }
 
-                       $row['rev_content_model'] = ( $model === $defaultModel ) ? null : $model;
-                       $row['rev_content_format'] = ( $format === $defaultFormat ) ? null : $format;
+               if ( $this->mcrMigrationStage > MIGRATION_OLD && $mainSlot->hasContentId() ) {
+                       // re-use content row of inherited slot!
+                       $contentId = $mainSlot->getContentId();
                }
 
-               $dbw->insert( 'revision', $row, __METHOD__ );
+               $revisionId = $revisionRow['rev_id'];
 
-               if ( !isset( $row['rev_id'] ) ) {
-                       // only if auto-increment was used
-                       $row['rev_id'] = intval( $dbw->insertId() );
-               }
-               $commentCallback( $row['rev_id'] );
-               $actorCallback( $row['rev_id'], $row );
+               $commentCallback( $revisionId );
+               $actorCallback( $revisionId, $revisionRow );
 
                // Insert IP revision into ip_changes for use when querying for a range.
                if ( $user->getId() === 0 && IP::isValid( $user->getName() ) ) {
                        $ipcRow = [
-                               'ipc_rev_id'        => $row['rev_id'],
-                               'ipc_rev_timestamp' => $row['rev_timestamp'],
+                               'ipc_rev_id'        => $revisionId,
+                               'ipc_rev_timestamp' => $revisionRow['rev_timestamp'],
                                'ipc_hex'           => IP::toHex( $user->getName() ),
                        ];
                        $dbw->insert( 'ip_changes', $ipcRow, __METHOD__ );
                }
 
-               $newSlot = SlotRecord::newSaved( $row['rev_id'], $textId, $blobAddress, $slot );
+               if ( $this->mcrMigrationStage >= MIGRATION_WRITE_BOTH ) {
+
+                       // Only insert slot rows for new revisions (not restored revisions).
+                       // Also, never insert content rows if not inserting slot rows.
+                       if ( !$hasSlots ) {
+
+                               // Only insert content rows for new content (not inherited content)
+                               if ( !$contentId ) {
+                                       Assert::invariant( !$hasSlots, 'Re-using slots, but not content ID is known' );
+                                       $contentId = $this->insertContentRowOn( $mainSlot, $dbw, $blobAddress );
+                               }
+
+                               $this->insertSlotRowOn( $mainSlot, $dbw, $revisionId, $contentId );
+                       }
+               } else {
+                       $contentId = null;
+               }
+
+               $dbw->endAtomic( __METHOD__ );
+
+               $newSlot = SlotRecord::newSaved( $revisionId, $contentId, $blobAddress, $mainSlot );
                $slots = new RevisionSlots( [ 'main' => $newSlot ] );
 
                $rev = new RevisionStoreRecord(
                        $title,
                        $user,
                        $comment,
-                       (object)$row,
+                       (object)$revisionRow,
                        $slots,
                        $this->wikiId
                );
@@ -485,7 +574,7 @@ class RevisionStore
                Assert::postcondition( $newSlot !== null, 'revision must have a main slot' );
                Assert::postcondition(
                        $newSlot->getAddress() !== null,
-                       'main slot must have an addess'
+                       'main slot must have an address'
                );
 
                Hooks::run( 'RevisionRecordInserted', [ $rev ] );
@@ -497,6 +586,41 @@ class RevisionStore
                return $rev;
        }
 
+       /**
+        * @param SlotRecord $slot
+        * @param IDatabase $dbw
+        * @param int $revisionId
+        * @param int $contentId
+        */
+       private function insertSlotRowOn( SlotRecord $slot, IDatabase $dbw, $revisionId, $contentId ) {
+               $slotRow = [
+                       'slot_revision_id' => $revisionId,
+                       'slot_role_id' => $this->slotRoleStore->acquireId( $slot->getRole() ),
+                       'slot_content_id' => $contentId,
+                       // If the slot has a specific origin use that ID, otherwise use the ID of the revision
+                       // that we just inserted.
+                       'slot_origin' => $slot->hasOrigin() ? $slot->getOrigin() : $revisionId,
+               ];
+               $dbw->insert( 'slots', $slotRow, __METHOD__ );
+       }
+
+       /**
+        * @param SlotRecord $slot
+        * @param IDatabase $dbw
+        * @param string $blobAddress
+        * @return int content row ID
+        */
+       private function insertContentRowOn( SlotRecord $slot, IDatabase $dbw, $blobAddress ) {
+               $contentRow = [
+                       'content_size' => $slot->getSize(),
+                       'content_sha1' => $slot->getSha1(),
+                       'content_model' => $this->contentModelStore->acquireId( $slot->getModel() ),
+                       'content_address' => $blobAddress,
+               ];
+               $dbw->insert( 'content', $contentRow, __METHOD__ );
+               return intval( $dbw->insertId() );
+       }
+
        /**
         * MCR migration note: this corresponds to Revision::checkContentModel
         *
@@ -581,20 +705,25 @@ class RevisionStore
                $fields = [ 'page_latest', 'page_namespace', 'page_title',
                        'rev_id', 'rev_text_id', 'rev_len', 'rev_sha1' ];
 
-               if ( $this->contentHandlerUseDB ) {
-                       $fields[] = 'rev_content_model';
-                       $fields[] = 'rev_content_format';
+               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+                       $fields[] = 'rev_text_id';
+                       if ( $this->contentHandlerUseDB ) {
+                               $fields[] = 'rev_content_model';
+                               $fields[] = 'rev_content_format';
+                       }
                }
 
+               // Don't use getQueryInfo() top avoid locking extra tables, compare T191892.
+               // XXX: perhaps getQueryInfo() could support 'no-comment' and 'no-actor' as options.
                $current = $dbw->selectRow(
                        [ 'page', 'revision' ],
                        $fields,
                        [
                                'page_id' => $title->getArticleID(),
-                               'page_latest=rev_id',
                        ],
                        __METHOD__,
-                       [ 'FOR UPDATE' ] // T51581
+                       [ 'FOR UPDATE' ], // T51581
+                       [ 'page' => [ 'JOIN', 'page_latest=rev_id' ] ]
                );
 
                if ( $current ) {
@@ -605,19 +734,20 @@ class RevisionStore
                                'actor'       => $user->getActorId(),
                                'comment'     => $comment,
                                'minor_edit'  => $minor,
-                               'text_id'     => $current->rev_text_id,
                                'parent_id'   => $current->page_latest,
                                'slot_origin' => $current->page_latest,
                                'len'         => $current->rev_len,
                                'sha1'        => $current->rev_sha1
                        ];
 
-                       if ( $this->contentHandlerUseDB ) {
-                               $fields['content_model'] = $current->rev_content_model;
-                               $fields['content_format'] = $current->rev_content_format;
-                       }
+                       if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+                               $fields['text_id'] = $current->rev_text_id;
 
-                       $fields['title'] = Title::makeTitle( $current->page_namespace, $current->page_title );
+                               if ( $this->contentHandlerUseDB ) {
+                                       $fields['content_model'] = $current->rev_content_model;
+                                       $fields['content_format'] = $current->rev_content_format;
+                               }
+                       }
 
                        $mainSlot = $this->emulateMainSlot_1_29( $fields, self::READ_LATEST, $title );
                        $revision = new MutableRevisionRecord( $title, $this->wikiId );
@@ -755,7 +885,6 @@ class RevisionStore
                $mainSlotRow->model_name = null;
                $mainSlotRow->slot_revision_id = null;
                $mainSlotRow->content_address = null;
-               $mainSlotRow->slot_content_id = null;
 
                $content = null;
                $blobData = null;
@@ -768,7 +897,6 @@ class RevisionStore
                        }
 
                        if ( isset( $row->rev_text_id ) && $row->rev_text_id > 0 ) {
-                               $mainSlotRow->slot_content_id = $row->rev_text_id;
                                $mainSlotRow->content_address = SqlBlobStore::makeAddressFromTextId(
                                        $row->rev_text_id
                                );
@@ -803,9 +931,6 @@ class RevisionStore
                } elseif ( is_array( $row ) ) {
                        $mainSlotRow->slot_revision_id = isset( $row['id'] ) ? intval( $row['id'] ) : null;
 
-                       $mainSlotRow->slot_content_id = isset( $row['text_id'] )
-                               ? intval( $row['text_id'] )
-                               : null;
                        $mainSlotRow->slot_origin = isset( $row['slot_origin'] )
                                ? intval( $row['slot_origin'] )
                                : null;
@@ -874,7 +999,18 @@ class RevisionStore
                        };
                }
 
-               $mainSlotRow->slot_id = $mainSlotRow->slot_revision_id;
+               // NOTE: this callback will be looped through RevisionSlot::newInherited(), allowing
+               // the inherited slot to have the same content_id as the original slot. In that case,
+               // $slot will be the inherited slot, while $mainSlotRow still refers to the original slot.
+               $mainSlotRow->slot_content_id =
+                       function ( SlotRecord $slot ) use ( $queryFlags, $mainSlotRow ) {
+                               list( $dbMode, ) = DBAccessObjectUtils::getDBOptions( $queryFlags );
+                               $db = $this->getDBConnectionRef( $dbMode );
+                               return $this->findSlotContentId( $db, $mainSlotRow->slot_revision_id, 'main' );
+                       };
+
+               // use negative IDs for fake slot records.
+               $mainSlotRow->slot_id = -( $mainSlotRow->slot_revision_id );
                return new SlotRecord( $mainSlotRow, $content );
        }
 
@@ -1580,7 +1716,7 @@ class RevisionStore
        private function fetchRevisionRowFromConds( IDatabase $db, $conditions, $flags = 0 ) {
                $this->checkDatabaseWikiId( $db );
 
-               $revQuery = self::getQueryInfo( [ 'page', 'user' ] );
+               $revQuery = $this->getQueryInfo( [ 'page', 'user' ] );
                $options = [];
                if ( ( $flags & self::READ_LOCKING ) == self::READ_LOCKING ) {
                        $options[] = 'FOR UPDATE';
@@ -1595,12 +1731,50 @@ class RevisionStore
                );
        }
 
+       /**
+        * Finds the ID of a content row for a given revision and slot role.
+        * This can be used to re-use content rows even while the content ID
+        * is still missing from SlotRecords, in MIGRATION_WRITE_BOTH mode.
+        *
+        * @todo remove after MCR schema migration is complete.
+        *
+        * @param IDatabase $db
+        * @param int $revId
+        * @param string $role
+        *
+        * @return int|null
+        */
+       private function findSlotContentId( IDatabase $db, $revId, $role ) {
+               if ( $this->mcrMigrationStage < MIGRATION_WRITE_BOTH ) {
+                       return null;
+               }
+
+               try {
+                       $roleId = $this->slotRoleStore->getId( $role );
+                       $conditions = [
+                               'slot_revision_id' => $revId,
+                               'slot_role_id' => $roleId,
+                       ];
+
+                       $contentId = $db->selectField( 'slots', 'slot_content_id', $conditions, __METHOD__ );
+
+                       return $contentId ?: null;
+               } catch ( NameTableAccessException $ex ) {
+                       // If the role is missing from the slot_roles table,
+                       // the corresponding row in slots cannot exist.
+                       return null;
+               }
+       }
+
        /**
         * Return the tables, fields, and join conditions to be selected to create
         * a new revision object.
         *
         * MCR migration note: this replaces Revision::getQueryInfo
         *
+        * If the format of fields returned changes in any way then the cache key provided by
+        * self::getRevisionRowCacheKey should be updated.
+        *
         * @since 1.31
         *
         * @param array $options Any combination of the following strings
@@ -1624,7 +1798,6 @@ class RevisionStore
                $ret['fields'] = array_merge( $ret['fields'], [
                        'rev_id',
                        'rev_page',
-                       'rev_text_id',
                        'rev_timestamp',
                        'rev_minor_edit',
                        'rev_deleted',
@@ -1643,9 +1816,13 @@ class RevisionStore
                $ret['fields'] = array_merge( $ret['fields'], $actorQuery['fields'] );
                $ret['joins'] = array_merge( $ret['joins'], $actorQuery['joins'] );
 
-               if ( $this->contentHandlerUseDB ) {
-                       $ret['fields'][] = 'rev_content_format';
-                       $ret['fields'][] = 'rev_content_model';
+               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+                       $ret['fields'][] = 'rev_text_id';
+
+                       if ( $this->contentHandlerUseDB ) {
+                               $ret['fields'][] = 'rev_content_format';
+                               $ret['fields'][] = 'rev_content_model';
+                       }
                }
 
                if ( in_array( 'page', $options, true ) ) {
@@ -1671,6 +1848,10 @@ class RevisionStore
                }
 
                if ( in_array( 'text', $options, true ) ) {
+                       if ( $this->mcrMigrationStage === MIGRATION_NEW ) {
+                               throw new InvalidArgumentException( 'text table can no longer be joined directly' );
+                       }
+
                        $ret['tables'][] = 'text';
                        $ret['fields'] = array_merge( $ret['fields'], [
                                'old_text',
@@ -1706,7 +1887,6 @@ class RevisionStore
                                        'ar_namespace',
                                        'ar_title',
                                        'ar_rev_id',
-                                       'ar_text_id',
                                        'ar_timestamp',
                                        'ar_minor_edit',
                                        'ar_deleted',
@@ -1717,9 +1897,13 @@ class RevisionStore
                        'joins' => $commentQuery['joins'] + $actorQuery['joins'],
                ];
 
-               if ( $this->contentHandlerUseDB ) {
-                       $ret['fields'][] = 'ar_content_format';
-                       $ret['fields'][] = 'ar_content_model';
+               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+                       $ret['fields'][] = 'ar_text_id';
+
+                       if ( $this->contentHandlerUseDB ) {
+                               $ret['fields'][] = 'ar_content_format';
+                               $ret['fields'][] = 'ar_content_model';
+                       }
                }
 
                return $ret;
@@ -1937,7 +2121,7 @@ class RevisionStore
                        return false;
                }
 
-               $revQuery = self::getQueryInfo();
+               $revQuery = $this->getQueryInfo();
                $res = $db->select(
                        $revQuery['tables'],
                        [
@@ -1995,7 +2179,7 @@ class RevisionStore
 
                $row = $this->cache->getWithSetCallback(
                        // Page/rev IDs passed in from DB to reflect history merges
-                       $this->cache->makeGlobalKey( 'revision-row-1.29', $db->getDomainID(), $pageId, $revId ),
+                       $this->getRevisionRowCacheKey( $db, $pageId, $revId ),
                        WANObjectCache::TTL_WEEK,
                        function ( $curValue, &$ttl, array &$setOpts ) use ( $db, $pageId, $revId ) {
                                $setOpts += Database::getCacheSetOptions( $db );
@@ -2019,6 +2203,26 @@ class RevisionStore
                }
        }
 
+       /**
+        * Get a cache key for use with a row as selected with getQueryInfo( [ 'page', 'user' ] )
+        * Caching rows without 'page' or 'user' could lead to issues.
+        * If the format of the rows returned by the query provided by getQueryInfo changes the
+        * cache key should be updated to avoid conflicts.
+        *
+        * @param IDatabase $db
+        * @param int $pageId
+        * @param int $revId
+        * @return string
+        */
+       private function getRevisionRowCacheKey( IDatabase $db, $pageId, $revId ) {
+               return $this->cache->makeGlobalKey(
+                       self::ROW_CACHE_KEY,
+                       $db->getDomainID(),
+                       $pageId,
+                       $revId
+               );
+       }
+
        // TODO: move relevant methods from Title here, e.g. getFirstRevision, isBigDeletion, etc.
 
 }
index 9462518..e63dd3c 100644 (file)
@@ -38,7 +38,9 @@ use Wikimedia\Assert\Assert;
 class SlotRecord {
 
        /**
-        * @var object database result row, as a raw object
+        * @var object database result row, as a raw object. Callbacks are supported for field values,
+        *      to enable on-demand emulation of these values. This is primarily intended for use
+        *      during schema migration.
         */
        private $row;
 
@@ -142,11 +144,11 @@ class 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.
+        * particularly the revision ID, content 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
+        * @param int|null $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).
@@ -163,7 +165,8 @@ class SlotRecord {
                SlotRecord $protoSlot
        ) {
                Assert::parameterType( 'integer', $revisionId, '$revisionId' );
-               Assert::parameterType( 'integer', $contentId, '$contentId' );
+               // TODO once migration is over $contentId must be an integer
+               Assert::parameterType( 'integer|null', $contentId, '$contentId' );
                Assert::parameterType( 'string', $contentAddress, '$contentAddress' );
 
                if ( $protoSlot->hasRevision() && $protoSlot->getRevision() !== $revisionId ) {
@@ -181,7 +184,7 @@ class SlotRecord {
                        );
                }
 
-               if ( $protoSlot->hasAddress() && $protoSlot->getContentId() !== $contentId ) {
+               if ( $protoSlot->hasContentId() && $protoSlot->getContentId() !== $contentId ) {
                        throw new LogicException(
                                "Mismatching content ID $contentId: "
                                . "The slot already has content row {$protoSlot->getContentId()} associated."
@@ -379,6 +382,13 @@ class SlotRecord {
         * @return bool whether this record contains the given field
         */
        private function hasField( $name ) {
+               if ( isset( $this->row->$name ) ) {
+                       // if the field is a callback, resolve first, then re-check
+                       if ( !is_string( $this->row->$name ) && is_callable( $this->row->$name ) ) {
+                               $this->getField( $name );
+                       }
+               }
+
                return isset( $this->row->$name );
        }
 
@@ -430,6 +440,30 @@ class SlotRecord {
                return $this->hasField( 'content_address' );
        }
 
+       /**
+        * Whether this slot has an origin (revision ID that originated the slot's content.
+        *
+        * @since 1.32
+        *
+        * @return bool
+        */
+       public function hasOrigin() {
+               return $this->hasField( 'slot_origin' );
+       }
+
+       /**
+        * Whether this slot has a content ID. Slots will have a content ID if their
+        * content has been stored in the content table. While building a new revision,
+        * SlotRecords will not have an ID associated.
+        *
+        * @since 1.32
+        *
+        * @return bool
+        */
+       public function hasContentId() {
+               return $this->hasField( 'slot_content_id' );
+       }
+
        /**
         * Whether this slot has revision ID associated. Slots will have a revision ID associated
         * only if they were loaded as part of an existing revision. While building a new revision,
index 7aa1aad..65b3428 100644 (file)
@@ -2410,7 +2410,7 @@ class WikiPage implements Page, IDBAccessObject {
                $tags = [], $logsubtype = 'delete'
        ) {
                global $wgUser, $wgContentHandlerUseDB, $wgCommentTableSchemaMigrationStage,
-                       $wgActorTableSchemaMigrationStage;
+                       $wgActorTableSchemaMigrationStage, $wgMultiContentRevisionSchemaMigrationStage;
 
                wfDebug( __METHOD__ . "\n" );
 
@@ -2497,9 +2497,17 @@ class WikiPage implements Page, IDBAccessObject {
                // Note array_intersect() preserves keys from the first arg, and we're
                // assuming $revQuery has `revision` primary and isn't using subtables
                // for anything we care about.
+               $tablesFlat = [];
+               array_walk_recursive(
+                       $revQuery['tables'],
+                       function ( $a ) use ( &$tablesFlat ) {
+                               $tablesFlat[] = $a;
+                       }
+               );
+
                $res = $dbw->select(
                        array_intersect(
-                               $revQuery['tables'],
+                               $tablesFlat,
                                [ 'revision', 'revision_comment_temp', 'revision_actor_temp' ]
                        ),
                        '1',
@@ -2539,6 +2547,14 @@ class WikiPage implements Page, IDBAccessObject {
                                'ar_minor_edit' => $row->rev_minor_edit,
                                'ar_rev_id'     => $row->rev_id,
                                'ar_parent_id'  => $row->rev_parent_id,
+                                       /**
+                                        * ar_text_id should probably not be written to when the multi content schema has
+                                        * been migrated to (wgMultiContentRevisionSchemaMigrationStage) however there is no
+                                        * default for the field in WMF production currently so we must keep writing
+                                        * writing until a default of 0 is set.
+                                        * Task: https://phabricator.wikimedia.org/T190148
+                                        * Copying the value from the revision table should not lead to any issues for now.
+                                        */
                                'ar_text_id'    => $row->rev_text_id,
                                'ar_len'        => $row->rev_len,
                                'ar_page_id'    => $id,
@@ -2546,7 +2562,10 @@ class WikiPage implements Page, IDBAccessObject {
                                'ar_sha1'       => $row->rev_sha1,
                        ] + $commentStore->insert( $dbw, 'ar_comment', $comment )
                                + $actorMigration->getInsertValues( $dbw, 'ar_user', $user );
-                       if ( $wgContentHandlerUseDB ) {
+                       if (
+                               $wgContentHandlerUseDB &&
+                               $wgMultiContentRevisionSchemaMigrationStage <= MIGRATION_WRITE_BOTH
+                       ) {
                                $rowInsert['ar_content_model'] = $row->rev_content_model;
                                $rowInsert['ar_content_format'] = $row->rev_content_format;
                        }
index 2cc5641..f457a06 100644 (file)
@@ -150,6 +150,7 @@ $wgAutoloadClasses += [
 
        # tests/phpunit/includes/Storage
        'MediaWiki\Tests\Storage\McrSchemaDetection' => "$testDir/phpunit/includes/Storage/McrSchemaDetection.php",
+       'MediaWiki\Tests\Storage\McrWriteBothSchemaOverride' => "$testDir/phpunit/includes/Storage/McrWriteBothSchemaOverride.php",
        'MediaWiki\Tests\Storage\RevisionSlotsTest' => "$testDir/phpunit/includes/Storage/RevisionSlotsTest.php",
        'MediaWiki\Tests\Storage\RevisionRecordTests' => "$testDir/phpunit/includes/Storage/RevisionRecordTests.php",
        'MediaWiki\Tests\Storage\RevisionStoreDbTestBase' => "$testDir/phpunit/includes/Storage/RevisionStoreDbTestBase.php",
index de1dbfd..b6c569c 100644 (file)
@@ -1602,8 +1602,10 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        private function resetDB( $db, $tablesUsed ) {
                if ( $db ) {
                        $userTables = [ 'user', 'user_groups', 'user_properties', 'actor' ];
-                       $pageTables = [ 'page', 'revision', 'ip_changes', 'revision_comment_temp',
-                               'revision_actor_temp', 'comment', 'archive' ];
+                       $pageTables = [
+                               'page', 'revision', 'ip_changes', 'revision_comment_temp', 'comment', 'archive',
+                               'revision_actor_temp', 'slots', 'content', 'content_models', 'slot_roles',
+                       ];
                        $coreDBDataTables = array_merge( $userTables, $pageTables );
 
                        // If any of the user or page tables were marked as used, we should clear all of them.
index f8c6715..1ab78f4 100644 (file)
@@ -414,6 +414,9 @@ abstract class RevisionDbTestBase extends MediaWikiTestCase {
                        $services->getService( '_SqlBlobStore' ),
                        $services->getMainWANObjectCache(),
                        $services->getCommentStore(),
+                       $services->getContentModelStore(),
+                       $services->getSlotRoleStore(),
+                       $this->getMcrMigrationStage(),
                        $services->getActorMigration()
                );
 
@@ -1378,7 +1381,7 @@ abstract class RevisionDbTestBase extends MediaWikiTestCase {
                $rev = $this->testPage->getRevision();
 
                // Clear any previous cache for the revision during creation
-               $key = $cache->makeGlobalKey( 'revision-row-1.29',
+               $key = $cache->makeGlobalKey( RevisionStore::ROW_CACHE_KEY,
                        $db->getDomainID(),
                        $rev->getPage(),
                        $rev->getId()
diff --git a/tests/phpunit/includes/RevisionMcrWriteBothDbTest.php b/tests/phpunit/includes/RevisionMcrWriteBothDbTest.php
new file mode 100644 (file)
index 0000000..436b379
--- /dev/null
@@ -0,0 +1,23 @@
+<?php
+use MediaWiki\Tests\Storage\McrWriteBothSchemaOverride;
+
+/**
+ * Tests Revision against the intermediate MCR DB schema for use during schema migration.
+ *
+ * @covers Revision
+ *
+ * @group Revision
+ * @group Storage
+ * @group ContentHandler
+ * @group Database
+ * @group medium
+ */
+class RevisionMcrWriteBothDbTest extends RevisionDbTestBase {
+
+       use McrWriteBothSchemaOverride;
+
+       protected function getContentHandlerUseDB() {
+               return true;
+       }
+
+}
index ab067a4..761548c 100644 (file)
@@ -17,6 +17,11 @@ use Wikimedia\Rdbms\LoadBalancer;
  */
 class RevisionTest extends MediaWikiTestCase {
 
+       public function setUp() {
+               parent::setUp();
+               $this->setMwGlobals( 'wgMultiContentRevisionSchemaMigrationStage', MIGRATION_OLD );
+       }
+
        public function provideConstructFromArray() {
                yield 'with text' => [
                        [
@@ -488,6 +493,9 @@ class RevisionTest extends MediaWikiTestCase {
                        $this->getBlobStore(),
                        $cache,
                        MediaWikiServices::getInstance()->getCommentStore(),
+                       MediaWikiServices::getInstance()->getContentModelStore(),
+                       MediaWikiServices::getInstance()->getSlotRoleStore(),
+                       MIGRATION_OLD,
                        MediaWikiServices::getInstance()->getActorMigration()
                );
                return $blobStore;
@@ -1156,10 +1164,60 @@ class RevisionTest extends MediaWikiTestCase {
                $revisionStore = $this->getRevisionStore();
                $revisionStore->setContentHandlerUseDB( $globals['wgContentHandlerUseDB'] );
                $this->setService( 'RevisionStore', $revisionStore );
-               $this->assertEquals(
-                       $expected,
-                       Revision::getArchiveQueryInfo()
+
+               $queryInfo = Revision::getArchiveQueryInfo();
+
+               $this->assertArrayEqualsIgnoringIntKeyOrder(
+                       $expected['tables'],
+                       $queryInfo['tables']
                );
+               $this->assertArrayEqualsIgnoringIntKeyOrder(
+                       $expected['fields'],
+                       $queryInfo['fields']
+               );
+               $this->assertArrayEqualsIgnoringIntKeyOrder(
+                       $expected['joins'],
+                       $queryInfo['joins']
+               );
+       }
+
+       /**
+        * Assert that the two arrays passed are equal, ignoring the order of the values that integer
+        * keys.
+        *
+        * Note: Failures of this assertion can be slightly confusing as the arrays are actually
+        * split into a string key array and an int key array before assertions occur.
+        *
+        * @param array $expected
+        * @param array $actual
+        */
+       private function assertArrayEqualsIgnoringIntKeyOrder( array $expected, array $actual ) {
+               $this->objectAssociativeSort( $expected );
+               $this->objectAssociativeSort( $actual );
+
+               // Separate the int key values from the string key values so that assertion failures are
+               // easier to understand.
+               $expectedIntKeyValues = [];
+               $actualIntKeyValues = [];
+
+               // Remove all int keys and re add them at the end after sorting by value
+               // This will result in all int keys being in the same order with same ints at the end of
+               // the array
+               foreach ( $expected as $key => $value ) {
+                       if ( is_int( $key ) ) {
+                               unset( $expected[$key] );
+                               $expectedIntKeyValues[] = $value;
+                       }
+               }
+               foreach ( $actual as $key => $value ) {
+                       if ( is_int( $key ) ) {
+                               unset( $actual[$key] );
+                               $actualIntKeyValues[] = $value;
+                       }
+               }
+
+               $this->assertArrayEquals( $expected, $actual, false, true );
+               $this->assertArrayEquals( $expectedIntKeyValues, $actualIntKeyValues, false, true );
        }
 
        public function provideGetQueryInfo() {
@@ -1391,9 +1449,19 @@ class RevisionTest extends MediaWikiTestCase {
                $revisionStore->setContentHandlerUseDB( $globals['wgContentHandlerUseDB'] );
                $this->setService( 'RevisionStore', $revisionStore );
 
-               $this->assertEquals(
-                       $expected,
-                       Revision::getQueryInfo( $options )
+               $queryInfo = Revision::getQueryInfo( $options );
+
+               $this->assertArrayEqualsIgnoringIntKeyOrder(
+                       $expected['tables'],
+                       $queryInfo['tables']
+               );
+               $this->assertArrayEqualsIgnoringIntKeyOrder(
+                       $expected['fields'],
+                       $queryInfo['fields']
+               );
+               $this->assertArrayEqualsIgnoringIntKeyOrder(
+                       $expected['joins'],
+                       $queryInfo['joins']
                );
        }
 
diff --git a/tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php b/tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php
new file mode 100644 (file)
index 0000000..81c98c0
--- /dev/null
@@ -0,0 +1,114 @@
+<?php
+namespace MediaWiki\Tests\Storage;
+
+use MediaWiki\Storage\RevisionRecord;
+use MediaWiki\Storage\SlotRecord;
+
+/**
+ * Tests RevisionStore against the intermediate MCR DB schema for use during schema migration.
+ *
+ * @covers \MediaWiki\Storage\RevisionStore
+ *
+ * @group RevisionStore
+ * @group Storage
+ * @group Database
+ * @group medium
+ */
+class McrWriteBothRevisionStoreDbTest extends RevisionStoreDbTestBase {
+
+       use McrWriteBothSchemaOverride;
+
+       protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
+               parent::assertRevisionExistsInDatabase( $rev );
+
+               $this->assertSelect(
+                       'slots', [ 'count(*)' ], [ 'slot_revision_id' => $rev->getId() ], [ [ '1' ] ]
+               );
+               $this->assertSelect(
+                       'content',
+                       [ 'count(*)' ],
+                       [ 'content_address' => $rev->getSlot( 'main' )->getAddress() ],
+                       [ [ '1' ] ]
+               );
+       }
+
+       /**
+        * @param SlotRecord $a
+        * @param SlotRecord $b
+        */
+       protected function assertSameSlotContent( SlotRecord $a, SlotRecord $b ) {
+               parent::assertSameSlotContent( $a, $b );
+
+               // Assert that the same content ID has been used
+               if ( $a->hasContentId() && $b->hasContentId() ) {
+                       $this->assertSame( $a->getContentId(), $b->getContentId() );
+               }
+       }
+
+       public function provideGetArchiveQueryInfo() {
+               yield [
+                       [
+                               'tables' => [ 'archive' ],
+                               'fields' => array_merge(
+                                       $this->getDefaultArchiveFields(),
+                                       [
+                                               'ar_comment_text' => 'ar_comment',
+                                               'ar_comment_data' => 'NULL',
+                                               'ar_comment_cid' => 'NULL',
+                                               'ar_user_text' => 'ar_user_text',
+                                               'ar_user' => 'ar_user',
+                                               'ar_actor' => 'NULL',
+                                               'ar_content_format',
+                                               'ar_content_model',
+                                       ]
+                               ),
+                               'joins' => [],
+                       ]
+               ];
+       }
+
+       public function provideGetQueryInfo() {
+               yield [
+                       [],
+                       [
+                               'tables' => [ 'revision' ],
+                               'fields' => array_merge(
+                                       $this->getDefaultQueryFields(),
+                                       $this->getCommentQueryFields(),
+                                       $this->getActorQueryFields(),
+                                       $this->getContentHandlerQueryFields()
+                               ),
+                               'joins' => [],
+                       ]
+               ];
+               yield [
+                       [ 'page', 'user', 'text' ],
+                       [
+                               'tables' => [ 'revision', 'page', 'user', 'text' ],
+                               'fields' => array_merge(
+                                       $this->getDefaultQueryFields(),
+                                       $this->getCommentQueryFields(),
+                                       $this->getActorQueryFields(),
+                                       $this->getContentHandlerQueryFields(),
+                                       [
+                                               'page_namespace',
+                                               'page_title',
+                                               'page_id',
+                                               'page_latest',
+                                               'page_is_redirect',
+                                               'page_len',
+                                               'user_name',
+                                               'old_text',
+                                               'old_flags',
+                                       ]
+                               ),
+                               'joins' => [
+                                       'page' => [ 'INNER JOIN', [ 'page_id = rev_page' ] ],
+                                       'user' => [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ],
+                                       'text' => [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ],
+                               ],
+                       ]
+               ];
+       }
+
+}
diff --git a/tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php b/tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php
new file mode 100644 (file)
index 0000000..2a54dbe
--- /dev/null
@@ -0,0 +1,58 @@
+<?php
+namespace MediaWiki\Tests\Storage;
+
+use Wikimedia\Rdbms\IMaintainableDatabase;
+use MediaWiki\DB\PatchFileLocation;
+
+/**
+ * Trait providing schema overrides that allow tests to run against the intermediate MCR database
+ * schema for use during schema migration.
+ */
+trait McrWriteBothSchemaOverride {
+
+       use PatchFileLocation;
+       use McrSchemaDetection;
+
+       /**
+        * @return int
+        */
+       protected function getMcrMigrationStage() {
+               return MIGRATION_WRITE_BOTH;
+       }
+
+       /**
+        * @return string[]
+        */
+       protected function getMcrTablesToReset() {
+               return [ 'content', 'content_models', 'slots', 'slot_roles' ];
+       }
+
+       /**
+        * @override MediaWikiTestCase::getSchemaOverrides
+        * @return array[]
+        */
+       protected function getSchemaOverrides( IMaintainableDatabase $db ) {
+               $overrides = [
+                       'scripts' => [],
+                       'drop' => [],
+                       'create' => [],
+                       'alter' => [],
+               ];
+
+               if ( !$this->hasMcrTables( $db ) ) {
+                       $overrides['create'] = [ 'slots', 'content', 'slot_roles', 'content_models', ];
+                       $overrides['scripts'][] = $this->getSqlPatchPath( $db, 'patch-slot_roles' );
+                       $overrides['scripts'][] = $this->getSqlPatchPath( $db, 'patch-content_models' );
+                       $overrides['scripts'][] = $this->getSqlPatchPath( $db, 'patch-content' );
+                       $overrides['scripts'][] = $this->getSqlPatchPath( $db, 'patch-slots' );
+               }
+
+               if ( !$this->hasPreMcrFields( $db ) ) {
+                       $overrides['alter'][] = 'revision';
+                       $overrides['scripts'][] = $this->getSqlPatchPath( $db, 'create-pre-mcr-fields', __DIR__ );
+               }
+
+               return $overrides;
+       }
+
+}
index 9c113b5..5927cfc 100644 (file)
@@ -179,6 +179,9 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                        $blobStore,
                        new WANObjectCache( [ 'cache' => new HashBagOStuff() ] ),
                        MediaWikiServices::getInstance()->getCommentStore(),
+                       MediaWikiServices::getInstance()->getContentModelStore(),
+                       MediaWikiServices::getInstance()->getSlotRoleStore(),
+                       $this->getMcrMigrationStage(),
                        MediaWikiServices::getInstance()->getActorMigration(),
                        $wikiId
                );
@@ -316,6 +319,8 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
        /**
         * @dataProvider provideInsertRevisionOn_successes
         * @covers \MediaWiki\Storage\RevisionStore::insertRevisionOn
+        * @covers \MediaWiki\Storage\RevisionStore::insertSlotRowOn
+        * @covers \MediaWiki\Storage\RevisionStore::insertContentRowOn
         */
        public function testInsertRevisionOn_successes(
                Title $title,
@@ -480,6 +485,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
        /**
         * @dataProvider provideNewNullRevision
         * @covers \MediaWiki\Storage\RevisionStore::newNullRevision
+        * @covers \MediaWiki\Storage\RevisionStore::findSlotContentId
         */
        public function testNewNullRevision( Title $title, $comment, $minor ) {
                $this->overrideMwServices();
index 61d0512..a877f87 100644 (file)
@@ -2,17 +2,21 @@
 
 namespace MediaWiki\Tests\Storage;
 
+use CommentStore;
 use HashBagOStuff;
+use InvalidArgumentException;
 use Language;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Storage\RevisionAccessException;
 use MediaWiki\Storage\RevisionStore;
 use MediaWiki\Storage\SqlBlobStore;
 use MediaWikiTestCase;
+use MWException;
 use Title;
 use WANObjectCache;
 use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\LoadBalancer;
+use Wikimedia\TestingAccessWrapper;
 
 class RevisionStoreTest extends MediaWikiTestCase {
 
@@ -28,11 +32,18 @@ class RevisionStoreTest extends MediaWikiTestCase {
                $blobStore = null,
                $WANObjectCache = null
        ) {
+               global $wgMultiContentRevisionSchemaMigrationStage;
+               // the migration stage should be irrelevant, since all the tests that interact with
+               // the database are in RevisionStoreDbTest, not here.
+
                return new RevisionStore(
                        $loadBalancer ?: $this->getMockLoadBalancer(),
                        $blobStore ?: $this->getMockSqlBlobStore(),
                        $WANObjectCache ?: $this->getHashWANObjectCache(),
                        MediaWikiServices::getInstance()->getCommentStore(),
+                       MediaWikiServices::getInstance()->getContentModelStore(),
+                       MediaWikiServices::getInstance()->getSlotRoleStore(),
+                       $wgMultiContentRevisionSchemaMigrationStage,
                        MediaWikiServices::getInstance()->getActorMigration()
                );
        }
@@ -61,190 +72,53 @@ class RevisionStoreTest extends MediaWikiTestCase {
                        ->disableOriginalConstructor()->getMock();
        }
 
-       private function getHashWANObjectCache() {
-               return new WANObjectCache( [ 'cache' => new \HashBagOStuff() ] );
-       }
-
        /**
-        * @covers \MediaWiki\Storage\RevisionStore::getContentHandlerUseDB
-        * @covers \MediaWiki\Storage\RevisionStore::setContentHandlerUseDB
+        * @return \PHPUnit_Framework_MockObject_MockObject|CommentStore
         */
-       public function testGetSetContentHandlerDb() {
-               $store = $this->getRevisionStore();
-               $this->assertTrue( $store->getContentHandlerUseDB() );
-               $store->setContentHandlerUseDB( false );
-               $this->assertFalse( $store->getContentHandlerUseDB() );
-               $store->setContentHandlerUseDB( true );
-               $this->assertTrue( $store->getContentHandlerUseDB() );
-       }
-
-       private function getDefaultQueryFields() {
-               return [
-                       'rev_id',
-                       'rev_page',
-                       'rev_text_id',
-                       'rev_timestamp',
-                       'rev_minor_edit',
-                       'rev_deleted',
-                       'rev_len',
-                       'rev_parent_id',
-                       'rev_sha1',
-               ];
-       }
-
-       private function getCommentQueryFields() {
-               return [
-                       'rev_comment_text' => 'rev_comment',
-                       'rev_comment_data' => 'NULL',
-                       'rev_comment_cid' => 'NULL',
-               ];
+       private function getMockCommentStore() {
+               return $this->getMockBuilder( CommentStore::class )
+                       ->disableOriginalConstructor()->getMock();
        }
 
-       private function getActorQueryFields() {
-               return [
-                       'rev_user' => 'rev_user',
-                       'rev_user_text' => 'rev_user_text',
-                       'rev_actor' => 'NULL',
-               ];
+       private function getHashWANObjectCache() {
+               return new WANObjectCache( [ 'cache' => new \HashBagOStuff() ] );
        }
 
-       private function getContentHandlerQueryFields() {
+       public function provideSetContentHandlerUseDB() {
                return [
-                       'rev_content_format',
-                       'rev_content_model',
-               ];
-       }
-
-       public function provideGetQueryInfo() {
-               yield [
-                       true,
-                       [],
-                       [
-                               'tables' => [ 'revision' ],
-                               'fields' => array_merge(
-                                       $this->getDefaultQueryFields(),
-                                       $this->getCommentQueryFields(),
-                                       $this->getActorQueryFields(),
-                                       $this->getContentHandlerQueryFields()
-                               ),
-                               'joins' => [],
-                       ]
-               ];
-               yield [
-                       false,
-                       [],
-                       [
-                               'tables' => [ 'revision' ],
-                               'fields' => array_merge(
-                                       $this->getDefaultQueryFields(),
-                                       $this->getCommentQueryFields(),
-                                       $this->getActorQueryFields()
-                               ),
-                               'joins' => [],
-                       ]
-               ];
-               yield [
-                       false,
-                       [ 'page' ],
-                       [
-                               'tables' => [ 'revision', 'page' ],
-                               'fields' => array_merge(
-                                       $this->getDefaultQueryFields(),
-                                       $this->getCommentQueryFields(),
-                                       $this->getActorQueryFields(),
-                                       [
-                                               'page_namespace',
-                                               'page_title',
-                                               'page_id',
-                                               'page_latest',
-                                               'page_is_redirect',
-                                               'page_len',
-                                       ]
-                               ),
-                               'joins' => [
-                                       'page' => [ 'INNER JOIN', [ 'page_id = rev_page' ] ],
-                               ],
-                       ]
-               ];
-               yield [
-                       false,
-                       [ 'user' ],
-                       [
-                               'tables' => [ 'revision', 'user' ],
-                               'fields' => array_merge(
-                                       $this->getDefaultQueryFields(),
-                                       $this->getCommentQueryFields(),
-                                       $this->getActorQueryFields(),
-                                       [
-                                               'user_name',
-                                       ]
-                               ),
-                               'joins' => [
-                                       'user' => [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ],
-                               ],
-                       ]
-               ];
-               yield [
-                       false,
-                       [ 'text' ],
-                       [
-                               'tables' => [ 'revision', 'text' ],
-                               'fields' => array_merge(
-                                       $this->getDefaultQueryFields(),
-                                       $this->getCommentQueryFields(),
-                                       $this->getActorQueryFields(),
-                                       [
-                                               'old_text',
-                                               'old_flags',
-                                       ]
-                               ),
-                               'joins' => [
-                                       'text' => [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ],
-                               ],
-                       ]
-               ];
-               yield [
-                       true,
-                       [ 'page', 'user', 'text' ],
-                       [
-                               'tables' => [ 'revision', 'page', 'user', 'text' ],
-                               'fields' => array_merge(
-                                       $this->getDefaultQueryFields(),
-                                       $this->getCommentQueryFields(),
-                                       $this->getActorQueryFields(),
-                                       $this->getContentHandlerQueryFields(),
-                                       [
-                                               'page_namespace',
-                                               'page_title',
-                                               'page_id',
-                                               'page_latest',
-                                               'page_is_redirect',
-                                               'page_len',
-                                               'user_name',
-                                               'old_text',
-                                               'old_flags',
-                                       ]
-                               ),
-                               'joins' => [
-                                       'page' => [ 'INNER JOIN', [ 'page_id = rev_page' ] ],
-                                       'user' => [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ],
-                                       'text' => [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ],
-                               ],
-                       ]
+                       // ContentHandlerUseDB can be true of false pre migration
+                       [ false, MIGRATION_OLD, false ],
+                       [ true, MIGRATION_OLD, false ],
+                       // During migration it can not be false
+                       [ false, MIGRATION_WRITE_BOTH, true ],
+                       // But it can be true
+                       [ true, MIGRATION_WRITE_BOTH, false ],
                ];
        }
 
        /**
-        * @dataProvider provideGetQueryInfo
-        * @covers \MediaWiki\Storage\RevisionStore::getQueryInfo
+        * @dataProvider provideSetContentHandlerUseDB
+        * @covers \MediaWiki\Storage\RevisionStore::getContentHandlerUseDB
+        * @covers \MediaWiki\Storage\RevisionStore::setContentHandlerUseDB
         */
-       public function testGetQueryInfo( $contentHandlerUseDb, $options, $expected ) {
-               $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', MIGRATION_OLD );
-               $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_OLD );
-               $this->overrideMwServices();
-               $store = $this->getRevisionStore();
-               $store->setContentHandlerUseDB( $contentHandlerUseDb );
-               $this->assertEquals( $expected, $store->getQueryInfo( $options ) );
+       public function testSetContentHandlerUseDB( $contentHandlerDb, $migrationMode, $expectedFail ) {
+               if ( $expectedFail ) {
+                       $this->setExpectedException( MWException::class );
+               }
+
+               $store = new RevisionStore(
+                       $this->getMockLoadBalancer(),
+                       $this->getMockSqlBlobStore(),
+                       $this->getHashWANObjectCache(),
+                       $this->getMockCommentStore(),
+                       MediaWikiServices::getInstance()->getContentModelStore(),
+                       MediaWikiServices::getInstance()->getSlotRoleStore(),
+                       $migrationMode,
+                       MediaWikiServices::getInstance()->getActorMigration()
+               );
+
+               $store->setContentHandlerUseDB( $contentHandlerDb );
+               $this->assertSame( $contentHandlerDb, $store->getContentHandlerUseDB() );
        }
 
        public function testGetTitle_successFromPageId() {
@@ -608,4 +482,47 @@ class RevisionStoreTest extends MediaWikiTestCase {
                return (object)$row;
        }
 
+       public function provideMigrationConstruction() {
+               return [
+                       [ MIGRATION_OLD, false ],
+                       [ MIGRATION_WRITE_BOTH, false ],
+               ];
+       }
+
+       /**
+        * @covers \MediaWiki\Storage\RevisionStore::__construct
+        * @dataProvider provideMigrationConstruction
+        */
+       public function testMigrationConstruction( $migration, $expectException ) {
+               if ( $expectException ) {
+                       $this->setExpectedException( InvalidArgumentException::class );
+               }
+               $loadBalancer = $this->getMockLoadBalancer();
+               $blobStore = $this->getMockSqlBlobStore();
+               $cache = $this->getHashWANObjectCache();
+               $commentStore = $this->getMockCommentStore();
+               $contentModelStore = MediaWikiServices::getInstance()->getContentModelStore();
+               $slotRoleStore = MediaWikiServices::getInstance()->getSlotRoleStore();
+               $store = new RevisionStore(
+                       $loadBalancer,
+                       $blobStore,
+                       $cache,
+                       $commentStore,
+                       MediaWikiServices::getInstance()->getContentModelStore(),
+                       MediaWikiServices::getInstance()->getSlotRoleStore(),
+                       $migration,
+                       MediaWikiServices::getInstance()->getActorMigration()
+               );
+               if ( !$expectException ) {
+                       $store = TestingAccessWrapper::newFromObject( $store );
+                       $this->assertSame( $loadBalancer, $store->loadBalancer );
+                       $this->assertSame( $blobStore, $store->blobStore );
+                       $this->assertSame( $cache, $store->cache );
+                       $this->assertSame( $commentStore, $store->commentStore );
+                       $this->assertSame( $contentModelStore, $store->contentModelStore );
+                       $this->assertSame( $slotRoleStore, $store->slotRoleStore );
+                       $this->assertSame( $migration, $store->mcrMigrationStage );
+               }
+       }
+
 }
index feeb538..1aae16d 100644 (file)
@@ -36,6 +36,7 @@ class SlotRecordTest extends MediaWikiTestCase {
                $record = new SlotRecord( $row, new WikitextContent( 'A' ) );
 
                $this->assertTrue( $record->hasAddress() );
+               $this->assertTrue( $record->hasContentId() );
                $this->assertTrue( $record->hasRevision() );
                $this->assertTrue( $record->isInherited() );
                $this->assertSame( 'A', $record->getContent()->getNativeData() );
@@ -59,6 +60,9 @@ class SlotRecordTest extends MediaWikiTestCase {
                        },
                        'slot_revision_id' => '2',
                        'slot_origin' => '2',
+                       'slot_content_id' => function () {
+                               return null;
+                       },
                ] );
 
                $content = function () {
@@ -69,6 +73,7 @@ class SlotRecordTest extends MediaWikiTestCase {
 
                $this->assertTrue( $record->hasAddress() );
                $this->assertTrue( $record->hasRevision() );
+               $this->assertFalse( $record->hasContentId() );
                $this->assertFalse( $record->isInherited() );
                $this->assertSame( 'A', $record->getContent()->getNativeData() );
                $this->assertSame( 1, $record->getSize() );
@@ -77,7 +82,6 @@ class SlotRecordTest extends MediaWikiTestCase {
                $this->assertSame( 2, $record->getRevision() );
                $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() );
        }
@@ -86,8 +90,10 @@ class SlotRecordTest extends MediaWikiTestCase {
                $record = SlotRecord::newUnsaved( 'myRole', new WikitextContent( 'A' ) );
 
                $this->assertFalse( $record->hasAddress() );
+               $this->assertFalse( $record->hasContentId() );
                $this->assertFalse( $record->hasRevision() );
                $this->assertFalse( $record->isInherited() );
+               $this->assertFalse( $record->hasOrigin() );
                $this->assertSame( 'A', $record->getContent()->getNativeData() );
                $this->assertSame( 1, $record->getSize() );
                $this->assertNotNull( $record->getSha1() );
@@ -190,6 +196,7 @@ class SlotRecordTest extends MediaWikiTestCase {
                $this->assertSame( $parent->getAddress(), $inherited->getAddress() );
                $this->assertSame( $parent->getContent(), $inherited->getContent() );
                $this->assertTrue( $inherited->isInherited() );
+               $this->assertTrue( $inherited->hasOrigin() );
                $this->assertFalse( $inherited->hasRevision() );
 
                // make sure we didn't mess with the internal state of $parent
@@ -224,8 +231,10 @@ class SlotRecordTest extends MediaWikiTestCase {
                // and content meta-data.
                $saved = SlotRecord::newSaved( 10, 20, 'theNewAddress', $unsaved );
                $this->assertFalse( $saved->isInherited() );
+               $this->assertTrue( $saved->hasOrigin() );
                $this->assertTrue( $saved->hasRevision() );
                $this->assertTrue( $saved->hasAddress() );
+               $this->assertTrue( $saved->hasContentId() );
                $this->assertSame( 'theNewAddress', $saved->getAddress() );
                $this->assertSame( 20, $saved->getContentId() );
                $this->assertSame( 'A', $saved->getContent()->getNativeData() );
@@ -234,6 +243,7 @@ class SlotRecordTest extends MediaWikiTestCase {
 
                // make sure we didn't mess with the internal state of $unsaved
                $this->assertFalse( $unsaved->hasAddress() );
+               $this->assertFalse( $unsaved->hasContentId() );
                $this->assertFalse( $unsaved->hasRevision() );
        }
 
diff --git a/tests/phpunit/includes/page/WikiPageMcrWriteBothDbTest.php b/tests/phpunit/includes/page/WikiPageMcrWriteBothDbTest.php
new file mode 100644 (file)
index 0000000..78bbfa7
--- /dev/null
@@ -0,0 +1,23 @@
+<?php
+use MediaWiki\Tests\Storage\McrWriteBothSchemaOverride;
+
+/**
+ * Tests WikiPage against the intermediate MCR DB schema for use during schema migration.
+ *
+ * @covers WikiPage
+ *
+ * @group WikiPage
+ * @group Storage
+ * @group ContentHandler
+ * @group Database
+ * @group medium
+ */
+class WikiPageMcrWriteBothDbTest extends WikiPageDbTestBase {
+
+       use McrWriteBothSchemaOverride;
+
+       protected function getContentHandlerUseDB() {
+               return true;
+       }
+
+}