Merge "resources: Strip '$' and 'mw' from file closures"
[lhc/web/wiklou.git] / includes / Storage / RevisionStore.php
index 12bee1e..879f322 100644 (file)
@@ -56,7 +56,7 @@ use Wikimedia\Assert\Assert;
 use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\DBConnRef;
 use Wikimedia\Rdbms\IDatabase;
-use Wikimedia\Rdbms\LoadBalancer;
+use Wikimedia\Rdbms\ILoadBalancer;
 
 /**
  * Service for looking up page revisions.
@@ -88,7 +88,7 @@ class RevisionStore
        private $contentHandlerUseDB = true;
 
        /**
-        * @var LoadBalancer
+        * @var ILoadBalancer
         */
        private $loadBalancer;
 
@@ -122,35 +122,69 @@ class RevisionStore
         */
        private $slotRoleStore;
 
-       /** @var int One of the MIGRATION_* constants */
+       /** @var int An appropriate combination of SCHEMA_COMPAT_XXX flags. */
        private $mcrMigrationStage;
 
        /**
         * @todo $blobStore should be allowed to be any BlobStore!
         *
-        * @param LoadBalancer $loadBalancer
+        * @param ILoadBalancer $loadBalancer
         * @param SqlBlobStore $blobStore
-        * @param WANObjectCache $cache
+        * @param WANObjectCache $cache A cache for caching revision rows. This can be the local
+        *        wiki's default instance even if $wikiId refers to a different wiki, since
+        *        makeGlobalKey() is used to constructed a key that allows cached revision rows from
+        *        the same database to be re-used between wikis. For example, enwiki and frwiki will
+        *        use the same cache keys for revision rows from the wikidatawiki database, regardless
+        *        of the cache's default key space.
         * @param CommentStore $commentStore
         * @param NameTableStore $contentModelStore
         * @param NameTableStore $slotRoleStore
-        * @param int $migrationStage
+        * @param int $mcrMigrationStage An appropriate combination of SCHEMA_COMPAT_XXX flags
         * @param ActorMigration $actorMigration
         * @param bool|string $wikiId
+        *
+        * @throws MWException if $mcrMigrationStage or $wikiId is invalid.
         */
        public function __construct(
-               LoadBalancer $loadBalancer,
+               ILoadBalancer $loadBalancer,
                SqlBlobStore $blobStore,
                WANObjectCache $cache,
                CommentStore $commentStore,
                NameTableStore $contentModelStore,
                NameTableStore $slotRoleStore,
-               $migrationStage,
+               $mcrMigrationStage,
                ActorMigration $actorMigration,
                $wikiId = false
        ) {
                Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' );
-               Assert::parameterType( 'integer', $migrationStage, '$migrationStage' );
+               Assert::parameterType( 'integer', $mcrMigrationStage, '$mcrMigrationStage' );
+               Assert::parameter(
+                       ( $mcrMigrationStage & SCHEMA_COMPAT_READ_BOTH ) !== SCHEMA_COMPAT_READ_BOTH,
+                       '$mcrMigrationStage',
+                       'Reading from the old and the new schema at the same time is not supported.'
+               );
+               Assert::parameter(
+                       ( $mcrMigrationStage & SCHEMA_COMPAT_READ_BOTH ) !== 0,
+                       '$mcrMigrationStage',
+                       'Reading needs to be enabled for the old or the new schema.'
+               );
+               Assert::parameter(
+                       ( $mcrMigrationStage & SCHEMA_COMPAT_WRITE_BOTH ) !== 0,
+                       '$mcrMigrationStage',
+                       'Writing needs to be enabled for the old or the new schema.'
+               );
+               Assert::parameter(
+                       ( $mcrMigrationStage & SCHEMA_COMPAT_READ_OLD ) === 0
+                       || ( $mcrMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) !== 0,
+                       '$mcrMigrationStage',
+                       'Cannot read the old schema when not also writing it.'
+               );
+               Assert::parameter(
+                       ( $mcrMigrationStage & SCHEMA_COMPAT_READ_NEW ) === 0
+                       || ( $mcrMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) !== 0,
+                       '$mcrMigrationStage',
+                       'Cannot read the new schema when not also writing it.'
+               );
 
                $this->loadBalancer = $loadBalancer;
                $this->blobStore = $blobStore;
@@ -158,12 +192,35 @@ class RevisionStore
                $this->commentStore = $commentStore;
                $this->contentModelStore = $contentModelStore;
                $this->slotRoleStore = $slotRoleStore;
-               $this->mcrMigrationStage = $migrationStage;
+               $this->mcrMigrationStage = $mcrMigrationStage;
                $this->actorMigration = $actorMigration;
                $this->wikiId = $wikiId;
                $this->logger = new NullLogger();
        }
 
+       /**
+        * @param int $flags A combination of SCHEMA_COMPAT_XXX flags.
+        * @return bool True if all the given flags were set in the $mcrMigrationStage
+        *         parameter passed to the constructor.
+        */
+       private function hasMcrSchemaFlags( $flags ) {
+               return ( $this->mcrMigrationStage & $flags ) === $flags;
+       }
+
+       /**
+        * Throws a RevisionAccessException if this RevisionStore is configured for cross-wiki loading
+        * and still reading from the old DB schema.
+        *
+        * @throws RevisionAccessException
+        */
+       private function assertCrossWikiContentLoadingIsSafe() {
+               if ( $this->wikiId !== false && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
+                       throw new RevisionAccessException(
+                               "Cross-wiki content loading is not supported by the pre-MCR schema"
+                       );
+               }
+       }
+
        public function setLogger( LoggerInterface $logger ) {
                $this->logger = $logger;
        }
@@ -188,16 +245,20 @@ class RevisionStore
         * @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.'
-                       );
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW )
+                       || $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_NEW )
+               ) {
+                       if ( !$contentHandlerUseDB ) {
+                               throw new MWException(
+                                       'Content model must be stored in the database for multi content revision migration.'
+                               );
+                       }
                }
                $this->contentHandlerUseDB = $contentHandlerUseDB;
        }
 
        /**
-        * @return LoadBalancer
+        * @return ILoadBalancer
         */
        private function getDBLoadBalancer() {
                return $this->loadBalancer;
@@ -213,6 +274,16 @@ class RevisionStore
                return $lb->getConnection( $mode, [], $this->wikiId );
        }
 
+       /**
+        * @param int $queryFlags a bit field composed of READ_XXX flags
+        *
+        * @return DBConnRef
+        */
+       private function getDBConnectionRefForQueryFlags( $queryFlags ) {
+               list( $mode, ) = DBAccessObjectUtils::getDBOptions( $queryFlags );
+               return $this->getDBConnectionRef( $mode );
+       }
+
        /**
         * @param IDatabase $connection
         */
@@ -372,11 +443,17 @@ class RevisionStore
                        );
                }
 
-               // While inserting into the old schema make sure only the main slot is allowed.
-               // TODO: support extra slots in MIGRATION_WRITE_BOTH mode!
-               if ( $this->mcrMigrationStage <= MIGRATION_WRITE_BOTH && $slotRoles !== [ 'main' ] ) {
+               // If we are not writing into the new schema, we can't support extra slots.
+               if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) && $slotRoles !== [ 'main' ] ) {
                        throw new InvalidArgumentException(
-                               'Only the main slot is supported with MCR migration mode <= MIGRATION_WRITE_BOTH!'
+                               'Only the main slot is supported when not writing to the MCR enabled schema!'
+                       );
+               }
+
+               // As long as we are not reading from the new schema, we don't want to write extra slots.
+               if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_NEW ) && $slotRoles !== [ 'main' ] ) {
+                       throw new InvalidArgumentException(
+                               'Only the main slot is supported when not reading from the MCR enabled schema!'
                        );
                }
 
@@ -389,6 +466,12 @@ class RevisionStore
                $this->failOnNull( $user->getId(), 'user field' );
                $this->failOnEmpty( $user->getName(), 'user_text field' );
 
+               if ( !$rev->isReadyForInsertion() ) {
+                       // This is here for future-proofing. At the time this check being added, it
+                       // was redundant to the individual checks above.
+                       throw new IncompleteRevisionException( 'Revision is incomplete' );
+               }
+
                // TODO: we shouldn't need an actual Title here.
                $title = Title::newFromLinkTarget( $rev->getPageAsLinkTarget() );
                $pageId = $this->failOnEmpty( $rev->getPageId(), 'rev_page field' ); // check this early
@@ -433,7 +516,7 @@ class RevisionStore
                );
 
                // Trigger exception if the main slot is missing.
-               // Technically, this could go away with MIGRATION_NEW: while
+               // Technically, this could go away after MCR migration: while
                // calling code may require a main slot to exist, RevisionStore
                // really should not know or care about that requirement.
                $rev->getSlot( 'main', RevisionRecord::RAW );
@@ -442,11 +525,11 @@ class RevisionStore
                        $slot = $rev->getSlot( $role, RevisionRecord::RAW );
                        Assert::postcondition(
                                $slot->getContent() !== null,
-                               $role .  ' slot must have content'
+                               $role . ' slot must have content'
                        );
                        Assert::postcondition(
                                $slot->hasRevision(),
-                               $role .  ' slot must have a revision associated'
+                               $role . ' slot must have a revision associated'
                        );
                }
 
@@ -489,9 +572,14 @@ class RevisionStore
                foreach ( $slotRoles as $role ) {
                        $slot = $rev->getSlot( $role, RevisionRecord::RAW );
 
-                       if ( $slot->hasRevision() ) {
-                               // If the SlotRecord already has a revision ID set, this means it already exists
-                               // in the database, and should already belong to the current revision.
+                       // If the SlotRecord already has a revision ID set, this means it already exists
+                       // in the database, and should already belong to the current revision.
+                       // However, a slot may already have a revision, but no content ID, if the slot
+                       // is emulated based on the archive table, because we are in SCHEMA_COMPAT_READ_OLD
+                       // mode, and the respective archive row was not yet migrated to the new schema.
+                       // In that case, a new slot row (and content row) must be inserted even during
+                       // undeletion.
+                       if ( $slot->hasRevision() && $slot->hasContentId() ) {
                                // TODO: properly abort transaction if the assertion fails!
                                Assert::parameter(
                                        $slot->getRevision() === $revisionId,
@@ -506,7 +594,9 @@ class RevisionStore
                                $newSlots[$role] = $slot;
 
                                // Write the main slot's text ID to the revision table for backwards compatibility
-                               if ( $slot->getRole() === 'main' && $this->mcrMigrationStage <= MIGRATION_WRITE_BOTH ) {
+                               if ( $slot->getRole() === 'main'
+                                       && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD )
+                               ) {
                                        $blobAddress = $slot->getAddress();
                                        $this->updateRevisionTextId( $dbw, $revisionId, $blobAddress );
                                }
@@ -533,6 +623,8 @@ class RevisionStore
         * @param IDatabase $dbw
         * @param int $revisionId
         * @param string &$blobAddress (may change!)
+        *
+        * @return int the text row id
         */
        private function updateRevisionTextId( IDatabase $dbw, $revisionId, &$blobAddress ) {
                $textId = $this->blobStore->getTextIdFromAddress( $blobAddress );
@@ -552,6 +644,8 @@ class RevisionStore
                        [ 'rev_id' => $revisionId ],
                        __METHOD__
                );
+
+               return $textId;
        }
 
        /**
@@ -575,12 +669,19 @@ class RevisionStore
                        $blobAddress = $this->storeContentBlob( $protoSlot, $title, $blobHints );
                }
 
+               $contentId = null;
+
                // Write the main slot's text ID to the revision table for backwards compatibility
-               if ( $protoSlot->getRole() === 'main' && $this->mcrMigrationStage <= MIGRATION_WRITE_BOTH ) {
-                       $this->updateRevisionTextId( $dbw, $revisionId, $blobAddress );
+               if ( $protoSlot->getRole() === 'main'
+                       && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD )
+               ) {
+                       // If SCHEMA_COMPAT_WRITE_NEW is also set, the fake content ID is overwritten
+                       // with the real content ID below.
+                       $textId = $this->updateRevisionTextId( $dbw, $revisionId, $blobAddress );
+                       $contentId = $this->emulateContentId( $textId );
                }
 
-               if ( $this->mcrMigrationStage >= MIGRATION_WRITE_BOTH ) {
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) ) {
                        if ( $protoSlot->hasContentId() ) {
                                $contentId = $protoSlot->getContentId();
                        } else {
@@ -588,8 +689,6 @@ class RevisionStore
                        }
 
                        $this->insertSlotRowOn( $protoSlot, $dbw, $revisionId, $contentId );
-               } else {
-                       $contentId = null;
                }
 
                $savedSlot = SlotRecord::newSaved(
@@ -665,6 +764,76 @@ class RevisionStore
                if ( !isset( $revisionRow['rev_id'] ) ) {
                        // only if auto-increment was used
                        $revisionRow['rev_id'] = intval( $dbw->insertId() );
+
+                       if ( $dbw->getType() === 'mysql' ) {
+                               // (T202032) MySQL until 8.0 and MariaDB until some version after 10.1.34 don't save the
+                               // auto-increment value to disk, so on server restart it might reuse IDs from deleted
+                               // revisions. We can fix that with an insert with an explicit rev_id value, if necessary.
+
+                               $maxRevId = intval( $dbw->selectField( 'archive', 'MAX(ar_rev_id)', '', __METHOD__ ) );
+                               $table = 'archive';
+                               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) ) {
+                                       $maxRevId2 = intval( $dbw->selectField( 'slots', 'MAX(slot_revision_id)', '', __METHOD__ ) );
+                                       if ( $maxRevId2 >= $maxRevId ) {
+                                               $maxRevId = $maxRevId2;
+                                               $table = 'slots';
+                                       }
+                               }
+
+                               if ( $maxRevId >= $revisionRow['rev_id'] ) {
+                                       $this->logger->debug(
+                                               '__METHOD__: Inserted revision {revid} but {table} has revisions up to {maxrevid}.'
+                                                       . ' Trying to fix it.',
+                                               [
+                                                       'revid' => $revisionRow['rev_id'],
+                                                       'table' => $table,
+                                                       'maxrevid' => $maxRevId,
+                                               ]
+                                       );
+
+                                       if ( !$dbw->lock( 'fix-for-T202032', __METHOD__ ) ) {
+                                               throw new MWException( 'Failed to get database lock for T202032' );
+                                       }
+                                       $fname = __METHOD__;
+                                       $dbw->onTransactionResolution( function ( $trigger, $dbw ) use ( $fname ) {
+                                               $dbw->unlock( 'fix-for-T202032', $fname );
+                                       } );
+
+                                       $dbw->delete( 'revision', [ 'rev_id' => $revisionRow['rev_id'] ], __METHOD__ );
+
+                                       // The locking here is mostly to make MySQL bypass the REPEATABLE-READ transaction
+                                       // isolation (weird MySQL "feature"). It does seem to block concurrent auto-incrementing
+                                       // inserts too, though, at least on MariaDB 10.1.29.
+                                       //
+                                       // Don't try to lock `revision` in this way, it'll deadlock if there are concurrent
+                                       // transactions in this code path thanks to the row lock from the original ->insert() above.
+                                       //
+                                       // And we have to use raw SQL to bypass the "aggregation used with a locking SELECT" warning
+                                       // that's for non-MySQL DBs.
+                                       $row1 = $dbw->query(
+                                               $dbw->selectSqlText( 'archive', [ 'v' => "MAX(ar_rev_id)" ], '', __METHOD__ ) . ' FOR UPDATE'
+                                       )->fetchObject();
+                                       if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) ) {
+                                               $row2 = $dbw->query(
+                                                       $dbw->selectSqlText( 'slots', [ 'v' => "MAX(slot_revision_id)" ], '', __METHOD__ )
+                                                               . ' FOR UPDATE'
+                                               )->fetchObject();
+                                       } else {
+                                               $row2 = null;
+                                       }
+                                       $maxRevId = max(
+                                               $maxRevId,
+                                               $row1 ? intval( $row1->v ) : 0,
+                                               $row2 ? intval( $row2->v ) : 0
+                                       );
+
+                                       // If we don't have SCHEMA_COMPAT_WRITE_NEW, all except the first of any concurrent
+                                       // transactions will throw a duplicate key error here. It doesn't seem worth trying
+                                       // to avoid that.
+                                       $revisionRow['rev_id'] = $maxRevId + 1;
+                                       $dbw->insert( 'revision', $revisionRow, __METHOD__ );
+                               }
+                       }
                }
 
                $commentCallback( $revisionRow['rev_id'] );
@@ -705,14 +874,16 @@ class RevisionStore
                        $revisionRow['rev_id'] = $rev->getId();
                }
 
-               if ( $this->mcrMigrationStage <= MIGRATION_WRITE_BOTH ) {
-                       // In non MCR more this IF section will relate to the main slot
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD ) ) {
+                       // In non MCR mode this IF section will relate to the main slot
                        $mainSlot = $rev->getSlot( 'main' );
                        $model = $mainSlot->getModel();
                        $format = $mainSlot->getFormat();
 
                        // MCR migration note: rev_content_model and rev_content_format will go away
                        if ( $this->contentHandlerUseDB ) {
+                               $this->assertCrossWikiContentLoadingIsSafe();
+
                                $defaultModel = ContentHandler::getDefaultModelFor( $title );
                                $defaultFormat = ContentHandler::getForModelID( $defaultModel )->getDefaultFormat();
 
@@ -821,6 +992,8 @@ class RevisionStore
                        // if $wgContentHandlerUseDB is not set,
                        // all revisions must use the default content model and format.
 
+                       $this->assertCrossWikiContentLoadingIsSafe();
+
                        $defaultModel = ContentHandler::getDefaultModelFor( $title );
                        $defaultHandler = ContentHandler::getForModelID( $defaultModel );
                        $defaultFormat = $defaultHandler->getDefaultFormat();
@@ -855,7 +1028,7 @@ class RevisionStore
         * Such revisions can for instance identify page rename
         * operations and other such meta-modifications.
         *
-        * @note: This method grabs a FOR UPDATE lock on the relevant row of the page table,
+        * @note This method grabs a FOR UPDATE lock on the relevant row of the page table,
         * to prevent a new revision from being inserted before the null revision has been written
         * to the database.
         *
@@ -896,8 +1069,13 @@ class RevisionStore
                        return null;
                }
 
-               // Fetch the actual revision row, without locking all extra tables.
-               $oldRevision = $this->loadRevisionFromId( $dbw, $pageLatest );
+               // Fetch the actual revision row from master, without locking all extra tables.
+               $oldRevision = $this->loadRevisionFromConds(
+                       $dbw,
+                       [ 'rev_id' => intval( $pageLatest ) ],
+                       self::READ_LATEST,
+                       $title
+               );
 
                // Construct the new revision
                $timestamp = wfTimestampNow(); // TODO: use a callback, so we can override it for testing.
@@ -943,9 +1121,8 @@ class RevisionStore
         * @return null|RecentChange
         */
        public function getRecentChange( RevisionRecord $rev, $flags = 0 ) {
-               $dbr = $this->getDBConnection( DB_REPLICA );
-
                list( $dbType, ) = DBAccessObjectUtils::getDBOptions( $flags );
+               $db = $this->getDBConnection( $dbType );
 
                $userIdentity = $rev->getUser( RevisionRecord::RAW );
 
@@ -956,18 +1133,18 @@ class RevisionStore
                }
 
                // TODO: Select by rc_this_oldid alone - but as of Nov 2017, there is no index on that!
-               $actorWhere = $this->actorMigration->getWhere( $dbr, 'rc_user', $rev->getUser(), false );
+               $actorWhere = $this->actorMigration->getWhere( $db, 'rc_user', $rev->getUser(), false );
                $rc = RecentChange::newFromConds(
                        [
                                $actorWhere['conds'],
-                               'rc_timestamp' => $dbr->timestamp( $rev->getTimestamp() ),
+                               'rc_timestamp' => $db->timestamp( $rev->getTimestamp() ),
                                'rc_this_oldid' => $rev->getId()
                        ],
                        __METHOD__,
                        $dbType
                );
 
-               $this->releaseDBConnection( $dbr );
+               $this->releaseDBConnection( $db );
 
                // XXX: cache this locally? Glue it to the RevisionRecord?
                return $rc;
@@ -1035,6 +1212,7 @@ class RevisionStore
                $mainSlotRow->role_name = 'main';
                $mainSlotRow->model_name = null;
                $mainSlotRow->slot_revision_id = null;
+               $mainSlotRow->slot_content_id = null;
                $mainSlotRow->content_address = null;
 
                $content = null;
@@ -1042,7 +1220,7 @@ class RevisionStore
                $blobFlags = null;
 
                if ( is_object( $row ) ) {
-                       if ( $this->mcrMigrationStage >= MIGRATION_NEW ) {
+                       if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_NEW ) ) {
                                // Don't emulate from a row when using the new schema.
                                // Emulating from an array is still OK.
                                throw new LogicException( 'Can\'t emulate the main slot when using MCR schema.' );
@@ -1071,7 +1249,7 @@ class RevisionStore
                                if ( !property_exists( $row, 'old_flags' ) ) {
                                        throw new InvalidArgumentException( 'old_flags was not set in $row' );
                                }
-                               $blobFlags = ( $row->old_flags === null ) ? '' : $row->old_flags;
+                               $blobFlags = $row->old_flags ?? '';
                        }
 
                        $mainSlotRow->slot_revision_id = intval( $row->rev_id );
@@ -1085,6 +1263,12 @@ class RevisionStore
                        $mainSlotRow->format_name = isset( $row->rev_content_format )
                                ? strval( $row->rev_content_format )
                                : null;
+
+                       if ( isset( $row->rev_text_id ) && intval( $row->rev_text_id ) > 0 ) {
+                               // Overwritten below for SCHEMA_COMPAT_WRITE_NEW
+                               $mainSlotRow->slot_content_id
+                                       = $this->emulateContentId( intval( $row->rev_text_id ) );
+                       }
                } elseif ( is_array( $row ) ) {
                        $mainSlotRow->slot_revision_id = isset( $row['id'] ) ? intval( $row['id'] ) : null;
 
@@ -1124,6 +1308,12 @@ class RevisionStore
                                        $mainSlotRow->format_name = $handler->getDefaultFormat();
                                }
                        }
+
+                       if ( isset( $row['text_id'] ) && intval( $row['text_id'] ) > 0 ) {
+                               // Overwritten below for SCHEMA_COMPAT_WRITE_NEW
+                               $mainSlotRow->slot_content_id
+                                       = $this->emulateContentId( intval( $row['text_id'] ) );
+                       }
                } else {
                        throw new MWException( 'Revision constructor passed invalid row format.' );
                }
@@ -1136,6 +1326,8 @@ class RevisionStore
 
                if ( $mainSlotRow->model_name === null ) {
                        $mainSlotRow->model_name = function ( SlotRecord $slot ) use ( $title ) {
+                               $this->assertCrossWikiContentLoadingIsSafe();
+
                                // TODO: MCR: consider slot role in getDefaultModelFor()! Use LinkTarget!
                                // TODO: MCR: deprecate $title->getModel().
                                return ContentHandler::getDefaultModelFor( $title );
@@ -1159,19 +1351,38 @@ class RevisionStore
                        };
                }
 
-               // 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' );
-                       };
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) ) {
+                       // 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 ) {
+                                       $db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
+                                       return $this->findSlotContentId( $db, $mainSlotRow->slot_revision_id, 'main' );
+                               };
+               }
 
                return new SlotRecord( $mainSlotRow, $content );
        }
 
+       /**
+        * Provides a content ID to use with emulated SlotRecords in SCHEMA_COMPAT_OLD mode,
+        * based on the revision's text ID (rev_text_id or ar_text_id, respectively).
+        * Note that in SCHEMA_COMPAT_WRITE_BOTH, a callback to findSlotContentId() should be used
+        * instead, since in that mode, some revision rows may already have a real content ID,
+        * while other's don't - and for the ones that don't, we should indicate that it
+        * is missing and cause SlotRecords::hasContentId() to return false.
+        *
+        * @param int $textId
+        * @return int The emulated content ID
+        */
+       private function emulateContentId( $textId ) {
+               // Return a negative number to ensure the ID is distinct from any real content IDs
+               // that will be assigned in SCHEMA_COMPAT_WRITE_NEW mode and read in SCHEMA_COMPAT_READ_NEW
+               // mode.
+               return -$textId;
+       }
+
        /**
         * Loads a Content object based on a slot row.
         *
@@ -1287,12 +1498,11 @@ class RevisionStore
                        // and fall back to master. The assumption is that we only want to force the fallback
                        // if we are quite sure the revision exists because the caller supplied a revision ID.
                        // If the page isn't found at all on a replica, it probably simply does not exist.
-                       $db = $this->getDBConnection( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_REPLICA );
+                       $db = $this->getDBConnectionRefForQueryFlags( $flags );
 
                        $conds[] = 'rev_id=page_latest';
                        $rev = $this->loadRevisionFromConds( $db, $conds, $flags );
 
-                       $this->releaseDBConnection( $db );
                        return $rev;
                }
        }
@@ -1329,12 +1539,11 @@ class RevisionStore
                        // and fall back to master. The assumption is that we only want to force the fallback
                        // if we are quite sure the revision exists because the caller supplied a revision ID.
                        // If the page isn't found at all on a replica, it probably simply does not exist.
-                       $db = $this->getDBConnection( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_REPLICA );
+                       $db = $this->getDBConnectionRefForQueryFlags( $flags );
 
                        $conds[] = 'rev_id=page_latest';
                        $rev = $this->loadRevisionFromConds( $db, $conds, $flags );
 
-                       $this->releaseDBConnection( $db );
                        return $rev;
                }
        }
@@ -1389,6 +1598,10 @@ class RevisionStore
                $slots = [];
 
                foreach ( $res as $row ) {
+                       // resolve role names and model names from in-memory cache, instead of joining.
+                       $row->role_name = $this->slotRoleStore->getName( (int)$row->slot_role_id );
+                       $row->model_name = $this->contentModelStore->getName( (int)$row->content_model );
+
                        $contentCallback = function ( SlotRecord $slot ) use ( $queryFlags, $row ) {
                                return $this->loadSlotContent( $slot, null, null, null, $queryFlags );
                        };
@@ -1425,10 +1638,7 @@ class RevisionStore
                $queryFlags,
                Title $title
        ) {
-               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
-                       // TODO: in MIGRATION_WRITE_BOTH, we could use the old and the new method:
-                       // e.g. call emulateMainSlot_1_29() if loadSlotRecords() fails.
-
+               if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_NEW ) ) {
                        $mainSlot = $this->emulateMainSlot_1_29( $revisionRow, $queryFlags, $title );
                        $slots = new RevisionSlots( [ 'main' => $mainSlot ] );
                } else {
@@ -1505,9 +1715,9 @@ class RevisionStore
                        $user = new UserIdentityValue( 0, '', 0 );
                }
 
-               $comment = $this->commentStore
-                       // Legacy because $row may have come from self::selectFields()
-                       ->getCommentLegacy( $this->getDBConnection( DB_REPLICA ), 'ar_comment', $row, true );
+               $db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
+               // Legacy because $row may have come from self::selectFields()
+               $comment = $this->commentStore->getCommentLegacy( $db, 'ar_comment', $row, true );
 
                $slots = $this->newRevisionSlots( $row->ar_rev_id, $row, $queryFlags, $title );
 
@@ -1553,9 +1763,9 @@ class RevisionStore
                        $user = new UserIdentityValue( 0, '', 0 );
                }
 
-               $comment = $this->commentStore
-                       // Legacy because $row may have come from self::selectFields()
-                       ->getCommentLegacy( $this->getDBConnection( DB_REPLICA ), 'rev_comment', $row, true );
+               $db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
+               // Legacy because $row may have come from self::selectFields()
+               $comment = $this->commentStore->getCommentLegacy( $db, 'rev_comment', $row, true );
 
                $slots = $this->newRevisionSlots( $row->rev_id, $row, $queryFlags, $title );
 
@@ -1610,8 +1820,8 @@ class RevisionStore
                }
 
                if ( !empty( $fields['text_id'] ) ) {
-                       if ( $this->mcrMigrationStage >= MIGRATION_NEW ) {
-                               throw new MWException( "Cannot use text_id field with MCR schema" );
+                       if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
+                               throw new MWException( "The text_id field is only available in the pre-MCR schema" );
                        }
 
                        if ( !empty( $fields['content'] ) ) {
@@ -1843,14 +2053,13 @@ class RevisionStore
         *
         * @param array $conditions
         * @param int $flags (optional)
-        * @param Title $title
+        * @param Title|null $title
         *
         * @return RevisionRecord|null
         */
        private function newRevisionFromConds( $conditions, $flags = 0, Title $title = null ) {
-               $db = $this->getDBConnection( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_REPLICA );
+               $db = $this->getDBConnectionRefForQueryFlags( $flags );
                $rev = $this->loadRevisionFromConds( $db, $conditions, $flags, $title );
-               $this->releaseDBConnection( $db );
 
                $lb = $this->getDBLoadBalancer();
 
@@ -1862,9 +2071,9 @@ class RevisionStore
                        && $lb->hasOrMadeRecentMasterChanges()
                ) {
                        $flags = self::READ_LATEST;
-                       $db = $this->getDBConnection( DB_MASTER );
-                       $rev = $this->loadRevisionFromConds( $db, $conditions, $flags, $title );
-                       $this->releaseDBConnection( $db );
+                       $dbw = $this->getDBConnection( DB_MASTER );
+                       $rev = $this->loadRevisionFromConds( $dbw, $conditions, $flags, $title );
+                       $this->releaseDBConnection( $dbw );
                }
 
                return $rev;
@@ -1879,7 +2088,7 @@ class RevisionStore
         * @param IDatabase $db
         * @param array $conditions
         * @param int $flags (optional)
-        * @param Title $title
+        * @param Title|null $title
         *
         * @return RevisionRecord|null
         */
@@ -1967,7 +2176,8 @@ 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.
+        * is still missing from SlotRecords, when writing to both the old and
+        * the new schema during MCR schema migration.
         *
         * @todo remove after MCR schema migration is complete.
         *
@@ -1978,7 +2188,7 @@ class RevisionStore
         * @return int|null
         */
        private function findSlotContentId( IDatabase $db, $revId, $role ) {
-               if ( $this->mcrMigrationStage < MIGRATION_WRITE_BOTH ) {
+               if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) ) {
                        return null;
                }
 
@@ -2014,8 +2224,8 @@ class RevisionStore
         *  - 'page': Join with the page table, and select fields to identify the page
         *  - 'user': Join with the user table, and select the user name
         *  - 'text': Join with the text table, and select fields to load page text. This
-        *    option is deprecated in MW 1.32 with MCR migration stage MIGRATION_WRITE_BOTH,
-        *    and disallowed with MIGRATION_MEW.
+        *    option is deprecated in MW 1.32 when the MCR migration flag SCHEMA_COMPAT_WRITE_NEW
+        *    is set, and disallowed when SCHEMA_COMPAT_READ_OLD is not set.
         *
         * @return array With three keys:
         *  - tables: (string[]) to include in the `$table` to `IDatabase->select()`
@@ -2051,7 +2261,7 @@ class RevisionStore
                $ret['fields'] = array_merge( $ret['fields'], $actorQuery['fields'] );
                $ret['joins'] = array_merge( $ret['joins'], $actorQuery['joins'] );
 
-               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
                        $ret['fields'][] = 'rev_text_id';
 
                        if ( $this->contentHandlerUseDB ) {
@@ -2083,10 +2293,15 @@ class RevisionStore
                }
 
                if ( in_array( 'text', $options, true ) ) {
-                       if ( $this->mcrMigrationStage === MIGRATION_NEW ) {
+                       if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD ) ) {
                                throw new InvalidArgumentException( 'text table can no longer be joined directly' );
-                       } elseif ( $this->mcrMigrationStage >= MIGRATION_WRITE_BOTH ) {
-                               wfDeprecated( __METHOD__ . ' with `text` option', '1.32' );
+                       } elseif ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
+                               // NOTE: even when this class is set to not read from the old schema, callers
+                               // should still be able to join against the text table, as long as we are still
+                               // writing the old schema for compatibility.
+                               // TODO: This should trigger a deprecation warning eventually (T200918), but not
+                               // before all known usages are removed (see T198341 and T201164).
+                               // wfDeprecated( __METHOD__ . ' with `text` option', '1.32' );
                        }
 
                        $ret['tables'][] = 'text';
@@ -2108,6 +2323,9 @@ class RevisionStore
         *
         * @param array $options Any combination of the following strings
         *  - 'content': Join with the content table, and select content meta-data fields
+        *  - 'model': Join with the content_models table, and select the model_name field.
+        *             Only applicable if 'content' is also set.
+        *  - 'role': Join with the slot_roles table, and select the role_name field
         *
         * @return array With three keys:
         *  - tables: (string[]) to include in the `$table` to `IDatabase->select()`
@@ -2121,7 +2339,7 @@ class RevisionStore
                        'joins'  => [],
                ];
 
-               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
                        $db = $this->getDBConnectionRef( DB_REPLICA );
                        $ret['tables']['slots'] = 'revision';
 
@@ -2142,32 +2360,41 @@ class RevisionStore
                                        $ret['fields']['model_name'] = 'NULL';
                                }
                        }
-
-                       // XXX: in MIGRATION_WRITE_BOTH mode, emulate *and* select - using a UNION?
-                       // See Anomie's idea at <https://gerrit.wikimedia.org/r/c/416465/
-                       // 8..10/includes/Storage/RevisionStore.php#2113>
                } else {
                        $ret['tables'][] = 'slots';
-                       $ret['tables'][] = 'slot_roles';
                        $ret['fields'] = array_merge( $ret['fields'], [
                                'slot_revision_id',
                                'slot_content_id',
                                'slot_origin',
-                               'role_name'
+                               'slot_role_id',
                        ] );
-                       $ret['joins']['slot_roles'] = [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ];
+
+                       if ( in_array( 'role', $options, true ) ) {
+                               // Use left join to attach role name, so we still find the revision row even
+                               // if the role name is missing. This triggers a more obvious failure mode.
+                               $ret['tables'][] = 'slot_roles';
+                               $ret['joins']['slot_roles'] = [ 'LEFT JOIN', [ 'slot_role_id = role_id' ] ];
+                               $ret['fields'][] = 'role_name';
+                       }
 
                        if ( in_array( 'content', $options, true ) ) {
                                $ret['tables'][] = 'content';
-                               $ret['tables'][] = 'content_models';
                                $ret['fields'] = array_merge( $ret['fields'], [
                                        'content_size',
                                        'content_sha1',
                                        'content_address',
-                                       'model_name'
+                                       'content_model',
                                ] );
                                $ret['joins']['content'] = [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ];
-                               $ret['joins']['content_models'] = [ 'INNER JOIN', [ 'content_model = model_id' ] ];
+
+                               if ( in_array( 'model', $options, true ) ) {
+                                       // Use left join to attach model name, so we still find the revision row even
+                                       // if the model name is missing. This triggers a more obvious failure mode.
+                                       $ret['tables'][] = 'content_models';
+                                       $ret['joins']['content_models'] = [ 'LEFT JOIN', [ 'content_model = model_id' ] ];
+                                       $ret['fields'][] = 'model_name';
+                               }
+
                        }
                }
 
@@ -2208,7 +2435,7 @@ class RevisionStore
                        'joins' => $commentQuery['joins'] + $actorQuery['joins'],
                ];
 
-               if ( $this->mcrMigrationStage < MIGRATION_NEW ) {
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
                        $ret['fields'][] = 'ar_text_id';
 
                        if ( $this->contentHandlerUseDB ) {
@@ -2273,7 +2500,7 @@ class RevisionStore
         * MCR migration note: this replaces Revision::getPrevious
         *
         * @param RevisionRecord $rev
-        * @param Title $title if known (optional)
+        * @param Title|null $title if known (optional)
         *
         * @return RevisionRecord|null
         */
@@ -2294,7 +2521,7 @@ class RevisionStore
         * MCR migration note: this replaces Revision::getNext
         *
         * @param RevisionRecord $rev
-        * @param Title $title if known (optional)
+        * @param Title|null $title if known (optional)
         *
         * @return RevisionRecord|null
         */
@@ -2355,15 +2582,12 @@ class RevisionStore
         * @return string|bool False if not found
         */
        public function getTimestampFromId( $title, $id, $flags = 0 ) {
-               $db = $this->getDBConnection(
-                       ( $flags & IDBAccessObject::READ_LATEST ) ? DB_MASTER : DB_REPLICA
-               );
+               $db = $this->getDBConnectionRefForQueryFlags( $flags );
 
                $conds = [ 'rev_id' => $id ];
                $conds['rev_page'] = $title->getArticleID();
                $timestamp = $db->selectField( 'revision', 'rev_timestamp', $conds, __METHOD__ );
 
-               $this->releaseDBConnection( $db );
                return ( $timestamp !== false ) ? wfTimestamp( TS_MW, $timestamp ) : false;
        }