Introduce new schema flags and use them in RevisionStore.
authordaniel <daniel.kinzler@wikimedia.de>
Tue, 26 Jun 2018 17:26:33 +0000 (19:26 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Mon, 2 Jul 2018 15:20:14 +0000 (17:20 +0200)
NOTE: this changes the numeric values of the MIGRATION_XXX constants!
Order is preserved.

Bug: T197619
Change-Id: I16db7dd5799ab98c1cb12e7cd1e0b2da83b366fc

15 files changed:
includes/DefaultSettings.php
includes/Defines.php
includes/Storage/RevisionStore.php
includes/page/WikiPage.php
maintenance/populateContentTables.php
tests/common/TestsAutoLoader.php
tests/phpunit/includes/ActorMigrationTest.php
tests/phpunit/includes/CommentStoreTest.php
tests/phpunit/includes/PageArchiveTest.php
tests/phpunit/includes/RevisionMcrReadNewDbTest.php [new file with mode: 0644]
tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php [new file with mode: 0644]
tests/phpunit/includes/Storage/McrReadNewSchemaOverride.php [new file with mode: 0644]
tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php
tests/phpunit/includes/Storage/RevisionStoreTest.php
tests/phpunit/includes/page/WikiPageMcrReadNewDbTest.php [new file with mode: 0644]

index 3771df1..e10561c 100644 (file)
@@ -8897,13 +8897,23 @@ $wgInterwikiPrefixDisplayTypes = [];
 $wgCommentTableSchemaMigrationStage = MIGRATION_OLD;
 
 /**
- * RevisionStore table schema migration stage (content, slots, content_models & slot_roles tables)
+ * RevisionStore table schema migration stage (content, slots, content_models & slot_roles tables).
+ * Use the SCHEMA_COMPAT_XXX flags. Supported values:
+ *
+ * - SCHEMA_COMPAT_OLD
+ * - SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD
+ * - SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW
+ * - SCHEMA_COMPAT_OLD
+ *
+ * Note that reading the old and new schema at the same time is not supported.
+ * Attempting to set both read bits in $wgMultiContentRevisionSchemaMigrationStage
+ * will result in an InvalidArgumentException.
  *
  * @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
+ * @var int An appropriate combination of SCHEMA_COMPAT_XXX flags.
  */
 $wgMultiContentRevisionSchemaMigrationStage = MIGRATION_OLD;
 
index e5261e7..72cddd2 100644 (file)
@@ -270,11 +270,34 @@ define( 'CONTENT_FORMAT_XML', 'application/xml' );
 define( 'SHELL_MAX_ARG_STRLEN', '100000' );
 /**@}*/
 
+/**@{
+ * Schema compatibility flags.
+ *
+ * Used as flags in a bit field that indicates whether the old or new schema (or both)
+ * are read or written.
+ *
+ * - SCHEMA_COMPAT_WRITE_OLD: Whether information is written to the old schema.
+ * - SCHEMA_COMPAT_READ_OLD: Whether information stored in the old schema is read.
+ * - SCHEMA_COMPAT_WRITE_NEW: Whether information is written to the new schema.
+ * - SCHEMA_COMPAT_READ_NEW: Whether information stored in the new schema is read.
+ */
+define( 'SCHEMA_COMPAT_WRITE_OLD', 0x01 );
+define( 'SCHEMA_COMPAT_READ_OLD', 0x02 );
+define( 'SCHEMA_COMPAT_WRITE_NEW', 0x10 );
+define( 'SCHEMA_COMPAT_READ_NEW', 0x20 );
+define( 'SCHEMA_COMPAT_WRITE_BOTH', SCHEMA_COMPAT_WRITE_OLD | SCHEMA_COMPAT_WRITE_NEW );
+define( 'SCHEMA_COMPAT_READ_BOTH', SCHEMA_COMPAT_READ_OLD | SCHEMA_COMPAT_READ_NEW );
+define( 'SCHEMA_COMPAT_OLD', SCHEMA_COMPAT_WRITE_OLD | SCHEMA_COMPAT_READ_OLD );
+define( 'SCHEMA_COMPAT_NEW', SCHEMA_COMPAT_WRITE_NEW | SCHEMA_COMPAT_READ_NEW );
+/**@}*/
+
 /**@{
  * Schema change migration flags.
  *
  * Used as values of a feature flag for an orderly transition from an old
- * schema to a new schema.
+ * schema to a new schema. The numeric values of these constants are compatible with the
+ * SCHEMA_COMPAT_XXX bitfield semantics. High bits are used to ensure that the numeric
+ * ordering follows the order in which the migration stages should be used.
  *
  * - MIGRATION_OLD: Only read and write the old schema. The new schema need not
  *   even exist. This is used from when the patch is merged until the schema
@@ -289,8 +312,8 @@ define( 'SHELL_MAX_ARG_STRLEN', '100000' );
  * - MIGRATION_NEW: Only read and write the new schema. The old schema (and the
  *   feature flag) may now be removed.
  */
-define( 'MIGRATION_OLD', 0 );
-define( 'MIGRATION_WRITE_BOTH', 1 );
-define( 'MIGRATION_WRITE_NEW', 2 );
-define( 'MIGRATION_NEW', 3 );
+define( 'MIGRATION_OLD', 0x00000000 | SCHEMA_COMPAT_OLD );
+define( 'MIGRATION_WRITE_BOTH', 0x10000000 | SCHEMA_COMPAT_READ_BOTH | SCHEMA_COMPAT_WRITE_BOTH );
+define( 'MIGRATION_WRITE_NEW', 0x20000000 | SCHEMA_COMPAT_READ_BOTH | SCHEMA_COMPAT_WRITE_NEW );
+define( 'MIGRATION_NEW', 0x30000000 | SCHEMA_COMPAT_NEW );
 /**@}*/
index 6c30d62..07329c9 100644 (file)
@@ -121,7 +121,7 @@ class RevisionStore
         */
        private $slotRoleStore;
 
-       /** @var int One of the MIGRATION_* constants */
+       /** @var int An appropriate combination of SCHEMA_COMPAT_XXX flags. */
        private $mcrMigrationStage;
 
        /**
@@ -133,9 +133,11 @@ class RevisionStore
         * @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,
@@ -144,12 +146,39 @@ class RevisionStore
                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;
@@ -157,12 +186,21 @@ 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;
+       }
+
        public function setLogger( LoggerInterface $logger ) {
                $this->logger = $logger;
        }
@@ -186,10 +224,14 @@ 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;
        }
@@ -371,10 +413,9 @@ 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 ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD ) && $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 writing to the pre-MCR schema!'
                        );
                }
 
@@ -431,7 +472,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 );
@@ -504,7 +545,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 );
                                }
@@ -574,11 +617,13 @@ class RevisionStore
                }
 
                // Write the main slot's text ID to the revision table for backwards compatibility
-               if ( $protoSlot->getRole() === 'main' && $this->mcrMigrationStage <= MIGRATION_WRITE_BOTH ) {
+               if ( $protoSlot->getRole() === 'main'
+                       && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD )
+               ) {
                        $this->updateRevisionTextId( $dbw, $revisionId, $blobAddress );
                }
 
-               if ( $this->mcrMigrationStage >= MIGRATION_WRITE_BOTH ) {
+               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) ) {
                        if ( $protoSlot->hasContentId() ) {
                                $contentId = $protoSlot->getContentId();
                        } else {
@@ -703,8 +748,8 @@ 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();
@@ -1040,7 +1085,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.' );
@@ -1423,10 +1468,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 {
@@ -1608,8 +1650,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'] ) ) {
@@ -1965,7 +2007,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.
         *
@@ -1976,7 +2019,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;
                }
 
@@ -2012,8 +2055,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()`
@@ -2049,7 +2092,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 ) {
@@ -2081,9 +2124,12 @@ 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 ) {
+                       } 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.
                                wfDeprecated( __METHOD__ . ' with `text` option', '1.32' );
                        }
 
@@ -2119,7 +2165,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';
 
@@ -2140,10 +2186,6 @@ 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';
@@ -2206,7 +2248,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 ) {
index b970820..fa56ab0 100644 (file)
@@ -2550,7 +2550,19 @@ class WikiPage implements Page, IDBAccessObject {
                        // Fetch all rows in case the DB needs that to properly lock them.
                }
 
-               // Get all of the page revisions
+               // If SCHEMA_COMPAT_WRITE_OLD is set, also select all extra fields we still write,
+               // so we can copy it to the archive table.
+               // We know the fields exist, otherwise SCHEMA_COMPAT_WRITE_OLD could not function.
+               if ( $wgMultiContentRevisionSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) {
+                       $revQuery['fields'][] = 'rev_text_id';
+
+                       if ( $wgContentHandlerUseDB ) {
+                               $revQuery['fields'][] = 'rev_content_model';
+                               $revQuery['fields'][] = 'rev_content_format';
+                       }
+               }
+
+                       // Get all of the page revisions
                $res = $dbw->select(
                        $revQuery['tables'],
                        $revQuery['fields'],
@@ -2592,17 +2604,15 @@ class WikiPage implements Page, IDBAccessObject {
                        ] + $commentStore->insert( $dbw, 'ar_comment', $comment )
                                + $actorMigration->getInsertValues( $dbw, 'ar_user', $user );
 
-                       if ( $wgMultiContentRevisionSchemaMigrationStage < MIGRATION_NEW ) {
+                       if ( $wgMultiContentRevisionSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) {
                                $rowInsert['ar_text_id'] = $row->rev_text_id;
-                       }
 
-                       if (
-                               $wgContentHandlerUseDB &&
-                               $wgMultiContentRevisionSchemaMigrationStage <= MIGRATION_WRITE_BOTH
-                       ) {
-                               $rowInsert['ar_content_model'] = $row->rev_content_model;
-                               $rowInsert['ar_content_format'] = $row->rev_content_format;
+                               if ( $wgContentHandlerUseDB ) {
+                                       $rowInsert['ar_content_model'] = $row->rev_content_model;
+                                       $rowInsert['ar_content_format'] = $row->rev_content_format;
+                               }
                        }
+
                        $rowsInsert[] = $rowInsert;
                        $revids[] = $row->rev_id;
 
index eee534f..b550cc2 100644 (file)
@@ -72,9 +72,10 @@ class PopulateContentTables extends Maintenance {
 
                $t0 = microtime( true );
 
-               if ( $wgMultiContentRevisionSchemaMigrationStage < MIGRATION_WRITE_BOTH ) {
+               if ( ( $wgMultiContentRevisionSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) === 0 ) {
                        $this->writeln(
-                               "...cannot update while \$wgMultiContentRevisionSchemaMigrationStage < MIGRATION_WRITE_BOTH"
+                               '...cannot update while \$wgMultiContentRevisionSchemaMigrationStage '
+                               . 'does not have the SCHEMA_COMPAT_WRITE_NEW bit set.'
                        );
                        return false;
                }
index 88c541e..3911faa 100644 (file)
@@ -152,6 +152,7 @@ $wgAutoloadClasses += [
        'MediaWiki\Tests\Storage\McrSchemaDetection' => "$testDir/phpunit/includes/Storage/McrSchemaDetection.php",
        'MediaWiki\Tests\Storage\McrSchemaOverride' => "$testDir/phpunit/includes/Storage/McrSchemaOverride.php",
        'MediaWiki\Tests\Storage\McrWriteBothSchemaOverride' => "$testDir/phpunit/includes/Storage/McrWriteBothSchemaOverride.php",
+       'MediaWiki\Tests\Storage\McrReadNewSchemaOverride' => "$testDir/phpunit/includes/Storage/McrReadNewSchemaOverride.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 1b0c848..d28be7a 100644 (file)
@@ -470,16 +470,17 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                }
 
                $stages = [
-                       MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_NEW ],
-                       MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_NEW ],
-                       MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ],
-                       MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ],
+                       MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW ],
+                       MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW,
+                               MIGRATION_NEW ],
+                       MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW, MIGRATION_NEW ],
+                       MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW, MIGRATION_NEW ],
                ];
 
                $nameKey = $key . '_text';
                $actorKey = $key === 'ipb_by' ? 'ipb_by_actor' : substr( $key, 0, -5 ) . '_actor';
 
-               foreach ( $stages as $writeStage => $readRange ) {
+               foreach ( $stages as $writeStage => $possibleReadStages ) {
                        if ( $key === 'ipb_by' ) {
                                $extraFields['ipb_address'] = __CLASS__ . "#$writeStage";
                        }
@@ -512,7 +513,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                                $callback( $id, $extraFields );
                        }
 
-                       for ( $readStage = $readRange[0]; $readStage <= $readRange[1]; $readStage++ ) {
+                       foreach ( $possibleReadStages as $readStage ) {
                                $r = $this->makeMigration( $readStage );
 
                                $queryInfo = $r->getJoin( $key );
index a510897..c41361c 100644 (file)
@@ -367,13 +367,14 @@ class CommentStoreTest extends MediaWikiLangTestCase {
                ];
 
                $stages = [
-                       MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_NEW ],
-                       MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_NEW ],
-                       MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ],
-                       MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ],
+                       MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW ],
+                       MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW,
+                               MIGRATION_NEW ],
+                       MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW, MIGRATION_NEW ],
+                       MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW, MIGRATION_NEW ],
                ];
 
-               foreach ( $stages as $writeStage => $readRange ) {
+               foreach ( $stages as $writeStage => $possibleReadStages ) {
                        if ( $key === 'ipb_reason' ) {
                                $extraFields['ipb_address'] = __CLASS__ . "#$writeStage";
                        }
@@ -406,7 +407,7 @@ class CommentStoreTest extends MediaWikiLangTestCase {
                                $callback( $id );
                        }
 
-                       for ( $readStage = $readRange[0]; $readStage <= $readRange[1]; $readStage++ ) {
+                       foreach ( $possibleReadStages as $readStage ) {
                                $rstore = $this->makeStore( $readStage );
 
                                $fieldRow = $this->db->selectRow(
@@ -460,13 +461,14 @@ class CommentStoreTest extends MediaWikiLangTestCase {
                ];
 
                $stages = [
-                       MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_NEW ],
-                       MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_NEW ],
-                       MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ],
-                       MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ],
+                       MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW ],
+                       MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW,
+                               MIGRATION_NEW ],
+                       MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW, MIGRATION_NEW ],
+                       MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW, MIGRATION_NEW ],
                ];
 
-               foreach ( $stages as $writeStage => $readRange ) {
+               foreach ( $stages as $writeStage => $possibleReadStages ) {
                        if ( $key === 'ipb_reason' ) {
                                $extraFields['ipb_address'] = __CLASS__ . "#$writeStage";
                        }
@@ -499,7 +501,7 @@ class CommentStoreTest extends MediaWikiLangTestCase {
                                $callback( $id );
                        }
 
-                       for ( $readStage = $readRange[0]; $readStage <= $readRange[1]; $readStage++ ) {
+                       foreach ( $possibleReadStages as $readStage ) {
                                $rstore = $this->makeStoreWithKey( $readStage, $key );
 
                                $fieldRow = $this->db->selectRow(
index 623d4a6..15a991e 100644 (file)
@@ -52,6 +52,7 @@ class PageArchiveTest extends MediaWikiTestCase {
 
                $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', MIGRATION_OLD );
                $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_OLD );
+               $this->setMwGlobals( 'wgMultiContentRevisionSchemaMigrationStage', SCHEMA_COMPAT_OLD );
                $this->overrideMwServices();
 
                // First create our dummy page
@@ -135,6 +136,7 @@ class PageArchiveTest extends MediaWikiTestCase {
         */
        public function testListRevisions() {
                $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', MIGRATION_OLD );
+               $this->setMwGlobals( 'wgMultiContentRevisionSchemaMigrationStage', SCHEMA_COMPAT_OLD );
                $this->overrideMwServices();
 
                $revisions = $this->archivedPage->listRevisions();
diff --git a/tests/phpunit/includes/RevisionMcrReadNewDbTest.php b/tests/phpunit/includes/RevisionMcrReadNewDbTest.php
new file mode 100644 (file)
index 0000000..1054b7d
--- /dev/null
@@ -0,0 +1,23 @@
+<?php
+use MediaWiki\Tests\Storage\McrReadNewSchemaOverride;
+
+/**
+ * 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 RevisionMcrReadNewDbTest extends RevisionDbTestBase {
+
+       use McrReadNewSchemaOverride;
+
+       protected function getContentHandlerUseDB() {
+               return true;
+       }
+
+}
diff --git a/tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php b/tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php
new file mode 100644 (file)
index 0000000..0c8094c
--- /dev/null
@@ -0,0 +1,214 @@
+<?php
+namespace MediaWiki\Tests\Storage;
+
+use InvalidArgumentException;
+use MediaWiki\MediaWikiServices;
+use MediaWiki\Storage\RevisionRecord;
+use MediaWiki\Storage\SlotRecord;
+use TextContent;
+use WikitextContent;
+
+/**
+ * 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 McrReadNewRevisionStoreDbTest extends RevisionStoreDbTestBase {
+
+       use McrReadNewSchemaOverride;
+
+       protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
+               $this->assertSelect(
+                       'slots',
+                       [ 'count(*)' ],
+                       [ 'slot_revision_id' => $rev->getId() ],
+                       [ [ '1' ] ]
+               );
+
+               $this->assertSelect(
+                       'content',
+                       [ 'count(*)' ],
+                       [ 'content_address' => $rev->getSlot( 'main' )->getAddress() ],
+                       [ [ '1' ] ]
+               );
+
+               parent::assertRevisionExistsInDatabase( $rev );
+       }
+
+       /**
+        * @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
+               $this->assertSame( $a->getContentId(), $b->getContentId() );
+       }
+
+       public function testGetQueryInfo_NoSlotDataJoin() {
+               $store = MediaWikiServices::getInstance()->getRevisionStore();
+               $queryInfo = $store->getQueryInfo();
+
+               // with the new schema enabled, query info should not join the main slot info
+               $this->assertFalse( array_key_exists( 'a_slot_data', $queryInfo['tables'] ) );
+               $this->assertFalse( array_key_exists( 'a_slot_data', $queryInfo['joins'] ) );
+       }
+
+       public function provideGetArchiveQueryInfo() {
+               yield [
+                       [
+                               'tables' => [
+                                       'archive',
+                               ],
+                               'fields' => array_merge(
+                                       $this->getDefaultArchiveFields( false ),
+                                       [
+                                               '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',
+                                       ]
+                               ),
+                               'joins' => [
+                               ],
+                       ]
+               ];
+       }
+
+       public function provideGetQueryInfo() {
+               // TODO: more option variations
+               yield [
+                       [ 'page', 'user' ],
+                       [
+                               'tables' => [
+                                       'revision',
+                                       'page',
+                                       'user',
+                               ],
+                               'fields' => array_merge(
+                                       $this->getDefaultQueryFields( false ),
+                                       $this->getCommentQueryFields(),
+                                       $this->getActorQueryFields(),
+                                       [
+                                               'page_namespace',
+                                               'page_title',
+                                               'page_id',
+                                               'page_latest',
+                                               'page_is_redirect',
+                                               'page_len',
+                                               'user_name',
+                                       ]
+                               ),
+                               'joins' => [
+                                       'page' => [ 'INNER JOIN', [ 'page_id = rev_page' ] ],
+                                       'user' => [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ],
+                               ],
+                       ]
+               ];
+       }
+
+       public function provideGetSlotsQueryInfo() {
+               yield [
+                       [],
+                       [
+                               'tables' => [
+                                       'slots',
+                                       'slot_roles',
+                               ],
+                               'fields' => array_merge(
+                                       [
+                                               'slot_revision_id',
+                                               'slot_content_id',
+                                               'slot_origin',
+                                               'role_name',
+                                       ]
+                               ),
+                               'joins' => [
+                                       'slot_roles' => [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ],
+                               ],
+                       ]
+               ];
+               yield [
+                       [ 'content' ],
+                       [
+                               'tables' => [
+                                       'slots',
+                                       'slot_roles',
+                                       'content',
+                                       'content_models',
+                               ],
+                               'fields' => array_merge(
+                                       [
+                                               'slot_revision_id',
+                                               'slot_content_id',
+                                               'slot_origin',
+                                               'role_name',
+                                               'content_size',
+                                               'content_sha1',
+                                               'content_address',
+                                               'model_name',
+                                       ]
+                               ),
+                               'joins' => [
+                                       'slot_roles' => [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ],
+                                       'content' => [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ],
+                                       'content_models' => [ 'INNER JOIN', [ 'content_model = model_id' ] ],
+                               ],
+                       ]
+               ];
+       }
+
+       public function provideInsertRevisionOn_failures() {
+               foreach ( parent::provideInsertRevisionOn_failures() as $case ) {
+                       yield $case;
+               }
+
+               yield 'slot that is not main slot' => [
+                       [
+                               'content' => [
+                                       'main' => new WikitextContent( 'Chicken' ),
+                                       'lalala' => new WikitextContent( 'Duck' ),
+                               ],
+                               'comment' => $this->getRandomCommentStoreComment(),
+                               'timestamp' => '20171117010101',
+                               'user' => true,
+                       ],
+                       new InvalidArgumentException( 'Only the main slot is supported' )
+               ];
+       }
+
+       public function provideNewMutableRevisionFromArray() {
+               foreach ( parent::provideNewMutableRevisionFromArray() as $case ) {
+                       yield $case;
+               }
+
+               yield 'Basic array, multiple roles' => [
+                       [
+                               'id' => 2,
+                               'page' => 1,
+                               'timestamp' => '20171017114835',
+                               'user_text' => '111.0.1.2',
+                               'user' => 0,
+                               'minor_edit' => false,
+                               'deleted' => 0,
+                               'len' => 29,
+                               'parent_id' => 1,
+                               'sha1' => '89qs83keq9c9ccw9olvvm4oc9oq50ii',
+                               'comment' => 'Goat Comment!',
+                               'content' => [
+                                       'main' => new WikitextContent( 'Söme Cöntent' ),
+                                       'aux' => new TextContent( 'Öther Cöntent' ),
+                               ]
+                       ]
+               ];
+       }
+
+}
diff --git a/tests/phpunit/includes/Storage/McrReadNewSchemaOverride.php b/tests/phpunit/includes/Storage/McrReadNewSchemaOverride.php
new file mode 100644 (file)
index 0000000..1958333
--- /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 McrReadNewSchemaOverride {
+
+       use PatchFileLocation;
+       use McrSchemaDetection;
+
+       /**
+        * @return int
+        */
+       protected function getMcrMigrationStage() {
+               return SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW;
+       }
+
+       /**
+        * @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 2a54dbe..7275f90 100644 (file)
@@ -17,7 +17,7 @@ trait McrWriteBothSchemaOverride {
         * @return int
         */
        protected function getMcrMigrationStage() {
-               return MIGRATION_WRITE_BOTH;
+               return SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD;
        }
 
        /**
index 727697c..90bd57a 100644 (file)
@@ -86,13 +86,17 @@ class RevisionStoreTest extends MediaWikiTestCase {
 
        public function provideSetContentHandlerUseDB() {
                return [
-                       // 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 ],
+                       // ContentHandlerUseDB can be true of false pre migration.
+                       [ false, SCHEMA_COMPAT_OLD, false ],
+                       [ true, SCHEMA_COMPAT_OLD, false ],
+                       // During and after migration it can not be false...
+                       [ false, SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, true ],
+                       [ false, SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, true ],
+                       [ false, SCHEMA_COMPAT_NEW, true ],
+                       // ...but it can be true.
+                       [ true, SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, false ],
+                       [ true, SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, false ],
+                       [ true, SCHEMA_COMPAT_NEW, false ],
                ];
        }
 
@@ -482,8 +486,13 @@ class RevisionStoreTest extends MediaWikiTestCase {
 
        public function provideMigrationConstruction() {
                return [
-                       [ MIGRATION_OLD, false ],
-                       [ MIGRATION_WRITE_BOTH, false ],
+                       [ SCHEMA_COMPAT_OLD, false ],
+                       [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, false ],
+                       [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, false ],
+                       [ SCHEMA_COMPAT_NEW, false ],
+                       [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_BOTH, true ],
+                       [ SCHEMA_COMPAT_WRITE_OLD | SCHEMA_COMPAT_READ_BOTH, true ],
+                       [ SCHEMA_COMPAT_WRITE_NEW | SCHEMA_COMPAT_READ_BOTH, true ],
                ];
        }
 
diff --git a/tests/phpunit/includes/page/WikiPageMcrReadNewDbTest.php b/tests/phpunit/includes/page/WikiPageMcrReadNewDbTest.php
new file mode 100644 (file)
index 0000000..9e2ff09
--- /dev/null
@@ -0,0 +1,23 @@
+<?php
+use MediaWiki\Tests\Storage\McrReadNewSchemaOverride;
+
+/**
+ * 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 WikiPageMcrReadNewDbTest extends WikiPageDbTestBase {
+
+       use McrReadNewSchemaOverride;
+
+       protected function getContentHandlerUseDB() {
+               return true;
+       }
+
+}