From 33258e04d1b84ce78cae64e97318c6f684adb987 Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 26 Jun 2018 19:26:33 +0200 Subject: [PATCH] Introduce new schema flags and use them in RevisionStore. NOTE: this changes the numeric values of the MIGRATION_XXX constants! Order is preserved. Bug: T197619 Change-Id: I16db7dd5799ab98c1cb12e7cd1e0b2da83b366fc --- includes/DefaultSettings.php | 14 +- includes/Defines.php | 33 ++- includes/Storage/RevisionStore.php | 118 ++++++---- includes/page/WikiPage.php | 28 ++- maintenance/populateContentTables.php | 5 +- tests/common/TestsAutoLoader.php | 1 + tests/phpunit/includes/ActorMigrationTest.php | 13 +- tests/phpunit/includes/CommentStoreTest.php | 26 ++- tests/phpunit/includes/PageArchiveTest.php | 2 + .../includes/RevisionMcrReadNewDbTest.php | 23 ++ .../Storage/McrReadNewRevisionStoreDbTest.php | 214 ++++++++++++++++++ .../Storage/McrReadNewSchemaOverride.php | 58 +++++ .../Storage/McrWriteBothSchemaOverride.php | 2 +- .../includes/Storage/RevisionStoreTest.php | 27 ++- .../page/WikiPageMcrReadNewDbTest.php | 23 ++ 15 files changed, 503 insertions(+), 84 deletions(-) create mode 100644 tests/phpunit/includes/RevisionMcrReadNewDbTest.php create mode 100644 tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php create mode 100644 tests/phpunit/includes/Storage/McrReadNewSchemaOverride.php create mode 100644 tests/phpunit/includes/page/WikiPageMcrReadNewDbTest.php diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 3771df14fe..e10561c483 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -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; diff --git a/includes/Defines.php b/includes/Defines.php index e5261e7931..72cddd2b7f 100644 --- a/includes/Defines.php +++ b/includes/Defines.php @@ -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 ); /**@}*/ diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php index 6c30d62e7a..07329c99cb 100644 --- a/includes/Storage/RevisionStore.php +++ b/includes/Storage/RevisionStore.php @@ -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 } 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 ) { diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index b97082084d..fa56ab0755 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -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; diff --git a/maintenance/populateContentTables.php b/maintenance/populateContentTables.php index eee534ff67..b550cc2a09 100644 --- a/maintenance/populateContentTables.php +++ b/maintenance/populateContentTables.php @@ -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; } diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 88c541e533..3911faa59f 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -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", diff --git a/tests/phpunit/includes/ActorMigrationTest.php b/tests/phpunit/includes/ActorMigrationTest.php index 1b0c848bb6..d28be7a53c 100644 --- a/tests/phpunit/includes/ActorMigrationTest.php +++ b/tests/phpunit/includes/ActorMigrationTest.php @@ -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 ); diff --git a/tests/phpunit/includes/CommentStoreTest.php b/tests/phpunit/includes/CommentStoreTest.php index a51089763a..c41361cd82 100644 --- a/tests/phpunit/includes/CommentStoreTest.php +++ b/tests/phpunit/includes/CommentStoreTest.php @@ -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( diff --git a/tests/phpunit/includes/PageArchiveTest.php b/tests/phpunit/includes/PageArchiveTest.php index 623d4a65f1..15a991e387 100644 --- a/tests/phpunit/includes/PageArchiveTest.php +++ b/tests/phpunit/includes/PageArchiveTest.php @@ -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 index 0000000000..1054b7d03b --- /dev/null +++ b/tests/phpunit/includes/RevisionMcrReadNewDbTest.php @@ -0,0 +1,23 @@ +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 index 0000000000..195833304e --- /dev/null +++ b/tests/phpunit/includes/Storage/McrReadNewSchemaOverride.php @@ -0,0 +1,58 @@ + [], + '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; + } + +} diff --git a/tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php b/tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php index 2a54dbe43c..7275f90e6f 100644 --- a/tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php +++ b/tests/phpunit/includes/Storage/McrWriteBothSchemaOverride.php @@ -17,7 +17,7 @@ trait McrWriteBothSchemaOverride { * @return int */ protected function getMcrMigrationStage() { - return MIGRATION_WRITE_BOTH; + return SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD; } /** diff --git a/tests/phpunit/includes/Storage/RevisionStoreTest.php b/tests/phpunit/includes/Storage/RevisionStoreTest.php index 727697cfde..90bd57aa6d 100644 --- a/tests/phpunit/includes/Storage/RevisionStoreTest.php +++ b/tests/phpunit/includes/Storage/RevisionStoreTest.php @@ -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 index 0000000000..9e2ff09e6f --- /dev/null +++ b/tests/phpunit/includes/page/WikiPageMcrReadNewDbTest.php @@ -0,0 +1,23 @@ +