From 993baa3493f17161847ce4545927d42e7861aae9 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 18 Sep 2018 14:21:20 -0400 Subject: [PATCH] ActorMigration: Remove possibility of read-both When this was originally written, the plan was to read both the old and new fields during the transition period, while stopping writes to them midway through. It turns out that the WHERE conditions to do read-both correctly are generally not handled well by the database and working around that would require a lot of complicated code (see what's being removed from ApiQueryUserContribs here, for example). We can simplify things greatly by instead having it write both fields during the transition period, reading from the old for the first part and the new for the second part, as is being done for MCR. Bug: T204669 Change-Id: I4764c1c7883dc1003cb12729455c8107319f70b1 Depends-On: I845f6ae462f2539ebd35cbb5f2ca8b5714e2c1fb Depends-On: I88b31b977543fdbdf69f8c1158e77e448df94e11 --- RELEASE-NOTES-1.32 | 4 + includes/ActorMigration.php | 109 +++--- includes/Block.php | 9 +- includes/DefaultSettings.php | 15 +- includes/Revision.php | 21 +- includes/actions/InfoAction.php | 13 +- includes/api/ApiQueryAllUsers.php | 13 +- includes/api/ApiQueryContributors.php | 11 +- includes/api/ApiQueryUserContribs.php | 188 ++------- includes/cache/UserCache.php | 8 +- includes/changes/RecentChange.php | 7 +- includes/db/DatabaseOracle.php | 4 +- includes/filerepo/file/ArchivedFile.php | 9 +- includes/filerepo/file/LocalFile.php | 25 +- includes/filerepo/file/OldLocalFile.php | 9 +- includes/installer/DatabaseUpdater.php | 2 +- includes/logging/LogEntry.php | 4 +- includes/page/WikiPage.php | 2 +- includes/revisiondelete/RevDelList.php | 8 +- .../revisiondelete/RevisionDeleteUser.php | 4 +- includes/specials/SpecialLog.php | 26 +- includes/specials/pagers/ContribsPager.php | 14 +- includes/specials/pagers/NewFilesPager.php | 2 +- includes/user/User.php | 36 +- maintenance/initEditCount.php | 112 +----- maintenance/migrateActors.php | 6 +- maintenance/populateLogSearch.php | 22 +- maintenance/reassignEdits.php | 15 +- maintenance/removeUnusedAccounts.php | 8 +- maintenance/rollbackEdits.php | 22 +- tests/parser/ParserTestRunner.php | 2 +- tests/phpunit/includes/ActorMigrationTest.php | 364 ++++++++++-------- .../Revision/RevisionQueryInfoTest.php | 94 ++--- .../Revision/RevisionStoreDbTestBase.php | 2 +- tests/phpunit/includes/RevisionDbTestBase.php | 2 +- tests/phpunit/includes/RevisionTest.php | 2 +- .../api/query/ApiQueryUserContribsTest.php | 29 +- .../includes/logging/DatabaseLogEntryTest.php | 13 +- .../includes/page/PageArchiveTestBase.php | 2 +- .../ChangesListSpecialPageTest.php | 48 ++- tests/phpunit/includes/user/UserTest.php | 2 +- 41 files changed, 564 insertions(+), 724 deletions(-) diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 2375b9014c..efc3a2e338 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -40,6 +40,10 @@ production. old and the new schema, but reading the new schema, so Multi-Content Revisions (MCR) are now functional per default. The new default value of the setting is SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW. +* $wgActorTableSchemaMigrationStage no longer accepts MIGRATION_WRITE_BOTH or + MIGRATION_WRITE_NEW. It instead uses SCHEMA_COMPAT_WRITE_BOTH | + SCHEMA_COMPAT_READ_OLD and SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW + for intermediate stages of migration. ==== Removed configuration ==== * $wgEnableAPI and $wgEnableWriteAPI – These settings, deprecated in 1.31, diff --git a/includes/ActorMigration.php b/includes/ActorMigration.php index 51dfa60a0b..0c33eb9bfc 100644 --- a/includes/ActorMigration.php +++ b/includes/ActorMigration.php @@ -34,6 +34,12 @@ use Wikimedia\Rdbms\IDatabase; */ class ActorMigration { + /** + * Constant for extensions to feature-test whether $wgActorTableSchemaMigrationStage + * expects MIGRATION_* or SCHEMA_COMPAT_* + */ + const MIGRATION_STAGE_SCHEMA_COMPAT = 1; + /** * Define fields that use temporary tables for transitional purposes * @var array Keys are '$key', values are arrays with four fields: @@ -74,11 +80,27 @@ class ActorMigration { /** @var array|null Cache for `self::getJoin()` */ private $joinCache = null; - /** @var int One of the MIGRATION_* constants */ + /** @var int Combination of SCHEMA_COMPAT_* constants */ private $stage; /** @private */ public function __construct( $stage ) { + if ( ( $stage & SCHEMA_COMPAT_WRITE_BOTH ) === 0 ) { + throw new InvalidArgumentException( '$stage must include a write mode' ); + } + if ( ( $stage & SCHEMA_COMPAT_READ_BOTH ) === 0 ) { + throw new InvalidArgumentException( '$stage must include a read mode' ); + } + if ( ( $stage & SCHEMA_COMPAT_READ_BOTH ) === SCHEMA_COMPAT_READ_BOTH ) { + throw new InvalidArgumentException( 'Cannot read both schemas' ); + } + if ( ( $stage & SCHEMA_COMPAT_READ_OLD ) && !( $stage & SCHEMA_COMPAT_WRITE_OLD ) ) { + throw new InvalidArgumentException( 'Cannot read the old schema without also writing it' ); + } + if ( ( $stage & SCHEMA_COMPAT_READ_NEW ) && !( $stage & SCHEMA_COMPAT_WRITE_NEW ) ) { + throw new InvalidArgumentException( 'Cannot read the new schema without also writing it' ); + } + $this->stage = $stage; } @@ -96,7 +118,7 @@ class ActorMigration { * @return string */ public function isAnon( $field ) { - return $this->stage === MIGRATION_NEW ? "$field IS NULL" : "$field = 0"; + return ( $this->stage & SCHEMA_COMPAT_READ_NEW ) ? "$field IS NULL" : "$field = 0"; } /** @@ -105,7 +127,7 @@ class ActorMigration { * @return string */ public function isNotAnon( $field ) { - return $this->stage === MIGRATION_NEW ? "$field IS NOT NULL" : "$field != 0"; + return ( $this->stage & SCHEMA_COMPAT_READ_NEW ) ? "$field IS NOT NULL" : "$field != 0"; } /** @@ -140,18 +162,16 @@ class ActorMigration { list( $text, $actor ) = self::getFieldNames( $key ); - if ( $this->stage === MIGRATION_OLD ) { + if ( $this->stage & SCHEMA_COMPAT_READ_OLD ) { $fields[$key] = $key; $fields[$text] = $text; $fields[$actor] = 'NULL'; } else { - $join = $this->stage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN'; - if ( isset( self::$tempTables[$key] ) ) { $t = self::$tempTables[$key]; $alias = "temp_$key"; $tables[$alias] = $t['table']; - $joins[$alias] = [ $join, "{$alias}.{$t['pk']} = {$t['joinPK']}" ]; + $joins[$alias] = [ 'JOIN', "{$alias}.{$t['pk']} = {$t['joinPK']}" ]; $joinField = "{$alias}.{$t['field']}"; } else { $joinField = $actor; @@ -159,15 +179,10 @@ class ActorMigration { $alias = "actor_$key"; $tables[$alias] = 'actor'; - $joins[$alias] = [ $join, "{$alias}.actor_id = {$joinField}" ]; + $joins[$alias] = [ 'JOIN', "{$alias}.actor_id = {$joinField}" ]; - if ( $this->stage === MIGRATION_NEW ) { - $fields[$key] = "{$alias}.actor_user"; - $fields[$text] = "{$alias}.actor_name"; - } else { - $fields[$key] = "COALESCE( {$alias}.actor_user, $key )"; - $fields[$text] = "COALESCE( {$alias}.actor_name, $text )"; - } + $fields[$key] = "{$alias}.actor_user"; + $fields[$text] = "{$alias}.actor_name"; $fields[$actor] = $joinField; } @@ -197,11 +212,11 @@ class ActorMigration { list( $text, $actor ) = self::getFieldNames( $key ); $ret = []; - if ( $this->stage <= MIGRATION_WRITE_BOTH ) { + if ( $this->stage & SCHEMA_COMPAT_WRITE_OLD ) { $ret[$key] = $user->getId(); $ret[$text] = $user->getName(); } - if ( $this->stage >= MIGRATION_WRITE_BOTH ) { + if ( $this->stage & SCHEMA_COMPAT_WRITE_NEW ) { // We need to be able to assign an actor ID if none exists if ( !$user instanceof User && !$user->getActorId() ) { $user = User::newFromAnyId( $user->getId(), $user->getName(), null ); @@ -233,11 +248,11 @@ class ActorMigration { list( $text, $actor ) = self::getFieldNames( $key ); $ret = []; $callback = null; - if ( $this->stage <= MIGRATION_WRITE_BOTH ) { + if ( $this->stage & SCHEMA_COMPAT_WRITE_OLD ) { $ret[$key] = $user->getId(); $ret[$text] = $user->getName(); } - if ( $this->stage >= MIGRATION_WRITE_BOTH ) { + if ( $this->stage & SCHEMA_COMPAT_WRITE_NEW ) { // We need to be able to assign an actor ID if none exists if ( !$user instanceof User && !$user->getActorId() ) { $user = User::newFromAnyId( $user->getId(), $user->getName(), null ); @@ -301,6 +316,8 @@ class ActorMigration { * - orconds: (array[]) array of alternatives in case a union of multiple * queries would be more efficient than a query with OR. May have keys * 'actor', 'userid', 'username'. + * Since 1.32, this is guaranteed to contain just one alternative if + * $users contains a single user. * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` * All tables and joins are aliased, so `+` is safe to use. */ @@ -332,44 +349,26 @@ class ActorMigration { list( $text, $actor ) = self::getFieldNames( $key ); // Combine data into conditions to be ORed together - $actorNotEmpty = []; - if ( $this->stage === MIGRATION_OLD ) { - $actors = []; - $actorEmpty = []; - } elseif ( isset( self::$tempTables[$key] ) ) { - $t = self::$tempTables[$key]; - $alias = "temp_$key"; - $tables[$alias] = $t['table']; - $joins[$alias] = [ - $this->stage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', - "{$alias}.{$t['pk']} = {$t['joinPK']}" - ]; - $joinField = "{$alias}.{$t['field']}"; - $actorEmpty = [ $joinField => null ]; - if ( $this->stage !== MIGRATION_NEW ) { - // Otherwise the resulting test can evaluate to NULL, and - // NOT(NULL) is NULL rather than true. - $actorNotEmpty = [ "$joinField IS NOT NULL" ]; + if ( $this->stage & SCHEMA_COMPAT_READ_NEW ) { + if ( $actors ) { + if ( isset( self::$tempTables[$key] ) ) { + $t = self::$tempTables[$key]; + $alias = "temp_$key"; + $tables[$alias] = $t['table']; + $joins[$alias] = [ 'JOIN', "{$alias}.{$t['pk']} = {$t['joinPK']}" ]; + $joinField = "{$alias}.{$t['field']}"; + } else { + $joinField = $actor; + } + $conds['actor'] = $db->makeList( [ $joinField => $actors ], IDatabase::LIST_AND ); } } else { - $joinField = $actor; - $actorEmpty = [ $joinField => 0 ]; - } - - if ( $actors ) { - $conds['actor'] = $db->makeList( - $actorNotEmpty + [ $joinField => $actors ], IDatabase::LIST_AND - ); - } - if ( $this->stage < MIGRATION_NEW && $ids ) { - $conds['userid'] = $db->makeList( - $actorEmpty + [ $key => $ids ], IDatabase::LIST_AND - ); - } - if ( $this->stage < MIGRATION_NEW && $names ) { - $conds['username'] = $db->makeList( - $actorEmpty + [ $text => $names ], IDatabase::LIST_AND - ); + if ( $ids ) { + $conds['userid'] = $db->makeList( [ $key => $ids ], IDatabase::LIST_AND ); + } + if ( $names ) { + $conds['username'] = $db->makeList( [ $text => $names ], IDatabase::LIST_AND ); + } } return [ diff --git a/includes/Block.php b/includes/Block.php index 913aeb93ce..bf8bad169f 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -208,13 +208,14 @@ class Block { public static function selectFields() { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->ipb_by or $row->ipb_by_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } @@ -224,7 +225,7 @@ class Block { 'ipb_address', 'ipb_by', 'ipb_by_text', - 'ipb_by_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'ipb_by_actor' : 'NULL', + 'ipb_by_actor' => 'NULL', 'ipb_timestamp', 'ipb_auto', 'ipb_anon_only', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 4ae3fe5430..4ed17074d4 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -8984,10 +8984,21 @@ $wgMultiContentRevisionSchemaMigrationStage = SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_ /** * Actor table schema migration stage. + * + * 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_NEW + * + * Note that reading the old and new schema at the same time is not supported + * in 1.32, but was (with significant query performance issues) in 1.31. + * * @since 1.31 - * @var int One of the MIGRATION_* constants + * @since 1.32 changed allowed flags + * @var int An appropriate combination of SCHEMA_COMPAT_XXX flags. */ -$wgActorTableSchemaMigrationStage = MIGRATION_OLD; +$wgActorTableSchemaMigrationStage = SCHEMA_COMPAT_OLD; /** * Temporary option to disable the date picker from the Expiry Widget. diff --git a/includes/Revision.php b/includes/Revision.php index 5a6afd8dad..e8fe8bd688 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -318,12 +318,13 @@ class Revision implements IDBAccessObject { global $wgActorTableSchemaMigrationStage; wfDeprecated( __METHOD__, '1.31' ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's // no way the join it's trying to do can work once the old fields - // aren't being written anymore. + // aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } @@ -352,13 +353,14 @@ class Revision implements IDBAccessObject { global $wgContentHandlerUseDB, $wgActorTableSchemaMigrationStage; global $wgMultiContentRevisionSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->rev_user or $row->rev_user_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } @@ -411,13 +413,14 @@ class Revision implements IDBAccessObject { global $wgContentHandlerUseDB, $wgActorTableSchemaMigrationStage; global $wgMultiContentRevisionSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->ar_user or $row->ar_user_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } diff --git a/includes/actions/InfoAction.php b/includes/actions/InfoAction.php index 1cc4288cd5..d0145034c8 100644 --- a/includes/actions/InfoAction.php +++ b/includes/actions/InfoAction.php @@ -739,27 +739,18 @@ class InfoAction extends FormlessAction { $dbrWatchlist = wfGetDB( DB_REPLICA, 'watchlist' ); $setOpts += Database::getCacheSetOptions( $dbr, $dbrWatchlist ); - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { $tables = [ 'revision_actor_temp' ]; $field = 'revactor_actor'; $pageField = 'revactor_page'; $tsField = 'revactor_timestamp'; $joins = []; - } elseif ( $wgActorTableSchemaMigrationStage === MIGRATION_OLD ) { + } else { $tables = [ 'revision' ]; $field = 'rev_user_text'; $pageField = 'rev_page'; $tsField = 'rev_timestamp'; $joins = []; - } else { - $tables = [ 'revision', 'revision_actor_temp', 'actor' ]; - $field = 'COALESCE( actor_name, rev_user_text)'; - $pageField = 'rev_page'; - $tsField = 'rev_timestamp'; - $joins = [ - 'revision_actor_temp' => [ 'LEFT JOIN', 'revactor_rev = rev_id' ], - 'actor' => [ 'LEFT JOIN', 'revactor_actor = actor_id' ], - ]; } $watchedItemStore = MediaWikiServices::getInstance()->getWatchedItemStore(); diff --git a/includes/api/ApiQueryAllUsers.php b/includes/api/ApiQueryAllUsers.php index 9652f810f7..7d5f6e2a59 100644 --- a/includes/api/ApiQueryAllUsers.php +++ b/includes/api/ApiQueryAllUsers.php @@ -182,19 +182,12 @@ class ApiQueryAllUsers extends ApiQueryBase { // Actually count the actions using a subquery (T66505 and T66507) $tables = [ 'recentchanges' ]; $joins = []; - if ( $wgActorTableSchemaMigrationStage === MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_OLD ) { $userCond = 'rc_user_text = user_name'; } else { $tables[] = 'actor'; - $joins['actor'] = [ - $wgActorTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', - 'rc_actor = actor_id' - ]; - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { - $userCond = 'actor_user = user_id'; - } else { - $userCond = 'actor_user = user_id OR (rc_actor = 0 AND rc_user_text = user_name)'; - } + $joins['actor'] = [ 'JOIN', 'rc_actor = actor_id' ]; + $userCond = 'actor_user = user_id'; } $timestamp = $db->timestamp( wfTimestamp( TS_UNIX ) - $activeUserSeconds ); $this->addFields( [ diff --git a/includes/api/ApiQueryContributors.php b/includes/api/ApiQueryContributors.php index 85f3e4bd2b..642c9ac3e5 100644 --- a/includes/api/ApiQueryContributors.php +++ b/includes/api/ApiQueryContributors.php @@ -80,12 +80,13 @@ class ApiQueryContributors extends ApiQueryBase { $result = $this->getResult(); $revQuery = MediaWikiServices::getInstance()->getRevisionStore()->getQueryInfo(); - // For MIGRATION_NEW, target indexes on the revision_actor_temp table. - // Otherwise, revision is fine because it'll have to check all revision rows anyway. - $pageField = $wgActorTableSchemaMigrationStage === MIGRATION_NEW ? 'revactor_page' : 'rev_page'; - $idField = $wgActorTableSchemaMigrationStage === MIGRATION_NEW + // For SCHEMA_COMPAT_READ_NEW, target indexes on the + // revision_actor_temp table, otherwise on the revision table. + $pageField = ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) + ? 'revactor_page' : 'rev_page'; + $idField = ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) ? 'revactor_actor' : $revQuery['fields']['rev_user']; - $countField = $wgActorTableSchemaMigrationStage === MIGRATION_NEW + $countField = ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) ? 'revactor_actor' : $revQuery['fields']['rev_user_text']; // First, count anons diff --git a/includes/api/ApiQueryUserContribs.php b/includes/api/ApiQueryUserContribs.php index 36d4d341e2..f16f958eb1 100644 --- a/includes/api/ApiQueryUserContribs.php +++ b/includes/api/ApiQueryUserContribs.php @@ -102,10 +102,8 @@ class ApiQueryUserContribs extends ApiQueryBase { $from = $fromName ? "$op= " . $dbSecondary->addQuotes( $fromName ) : false; // For the new schema, pull from the actor table. For the - // old, pull from rev_user. For migration a FULL [OUTER] - // JOIN would be what we want, except MySQL doesn't support - // that so we have to UNION instead. - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + // old, pull from rev_user. + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { $res = $dbSecondary->select( 'actor', [ 'actor_id', 'user_id' => 'COALESCE(actor_user,0)', 'user_name' => 'actor_name' ], @@ -113,7 +111,7 @@ class ApiQueryUserContribs extends ApiQueryBase { $fname, [ 'ORDER BY' => [ "user_name $sort" ], 'LIMIT' => $limit ] ); - } elseif ( $wgActorTableSchemaMigrationStage === MIGRATION_OLD ) { + } else { $res = $dbSecondary->select( 'revision', [ 'actor_id' => 'NULL', 'user_id' => 'rev_user', 'user_name' => 'rev_user_text' ], @@ -121,46 +119,6 @@ class ApiQueryUserContribs extends ApiQueryBase { $fname, [ 'DISTINCT', 'ORDER BY' => [ "rev_user_text $sort" ], 'LIMIT' => $limit ] ); - } else { - // There are three queries we have to combine to be sure of getting all results: - // - actor table (any rows that have been migrated will have empty rev_user_text) - // - revision+actor by user id - // - revision+actor by name for anons - $options = $dbSecondary->unionSupportsOrderAndLimit() - ? [ 'ORDER BY' => [ "user_name $sort" ], 'LIMIT' => $limit ] : []; - $subsql = []; - $subsql[] = $dbSecondary->selectSQLText( - 'actor', - [ 'actor_id', 'user_id' => 'COALESCE(actor_user,0)', 'user_name' => 'actor_name' ], - array_merge( [ "actor_name$like" ], $from ? [ "actor_name $from" ] : [] ), - $fname, - $options - ); - $subsql[] = $dbSecondary->selectSQLText( - [ 'revision', 'actor' ], - [ 'actor_id', 'user_id' => 'rev_user', 'user_name' => 'rev_user_text' ], - array_merge( - [ "rev_user_text$like", 'rev_user != 0' ], - $from ? [ "rev_user_text $from" ] : [] - ), - $fname, - array_merge( [ 'DISTINCT' ], $options ), - [ 'actor' => [ 'LEFT JOIN', 'rev_user = actor_user' ] ] - ); - $subsql[] = $dbSecondary->selectSQLText( - [ 'revision', 'actor' ], - [ 'actor_id', 'user_id' => 'rev_user', 'user_name' => 'rev_user_text' ], - array_merge( - [ "rev_user_text$like", 'rev_user = 0' ], - $from ? [ "rev_user_text $from" ] : [] - ), - $fname, - array_merge( [ 'DISTINCT' ], $options ), - [ 'actor' => [ 'LEFT JOIN', 'rev_user_text = actor_name' ] ] - ); - $sql = $dbSecondary->unionQueries( $subsql, false ) . " ORDER BY user_name $sort"; - $sql = $dbSecondary->limitResult( $sql, $limit ); - $res = $dbSecondary->query( $sql, $fname ); } $count = 0; @@ -205,9 +163,8 @@ class ApiQueryUserContribs extends ApiQueryBase { } // For the new schema, just select from the actor table. For the - // old and transitional schemas, select from user and left join - // actor if it exists. - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + // old, select from user. + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { $res = $dbSecondary->select( 'actor', [ 'actor_id', 'user_id' => 'actor_user', 'user_name' => 'actor_name' ], @@ -215,7 +172,7 @@ class ApiQueryUserContribs extends ApiQueryBase { __METHOD__, [ 'ORDER BY' => "user_id $sort" ] ); - } elseif ( $wgActorTableSchemaMigrationStage === MIGRATION_OLD ) { + } else { $res = $dbSecondary->select( 'user', [ 'actor_id' => 'NULL', 'user_id' => 'user_id', 'user_name' => 'user_name' ], @@ -223,15 +180,6 @@ class ApiQueryUserContribs extends ApiQueryBase { __METHOD__, [ 'ORDER BY' => "user_id $sort" ] ); - } else { - $res = $dbSecondary->select( - [ 'user', 'actor' ], - [ 'actor_id', 'user_id', 'user_name' ], - array_merge( [ 'user_id' => $ids ], $from ? [ "user_id $from" ] : [] ), - __METHOD__, - [ 'ORDER BY' => "user_id $sort" ], - [ 'actor' => [ 'LEFT JOIN', 'actor_user = user_id' ] ] - ); } $userIter = UserArray::newFromResult( $res ); $batchSize = count( $ids ); @@ -278,9 +226,8 @@ class ApiQueryUserContribs extends ApiQueryBase { } // For the new schema, just select from the actor table. For the - // old and transitional schemas, select from user and left join - // actor if it exists then merge in any unknown users (IPs and imports). - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + // old, select from user then merge in any unknown users (IPs and imports). + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { $res = $dbSecondary->select( 'actor', [ 'actor_id', 'user_id' => 'actor_user', 'user_name' => 'actor_name' ], @@ -290,23 +237,12 @@ class ApiQueryUserContribs extends ApiQueryBase { ); $userIter = UserArray::newFromResult( $res ); } else { - if ( $wgActorTableSchemaMigrationStage === MIGRATION_OLD ) { - $res = $dbSecondary->select( - 'user', - [ 'actor_id' => 'NULL', 'user_id', 'user_name' ], - array_merge( [ 'user_name' => array_keys( $names ) ], $from ? [ "user_name $from" ] : [] ), - __METHOD__ - ); - } else { - $res = $dbSecondary->select( - [ 'user', 'actor' ], - [ 'actor_id', 'user_id', 'user_name' ], - array_merge( [ 'user_name' => array_keys( $names ) ], $from ? [ "user_name $from" ] : [] ), - __METHOD__, - [], - [ 'actor' => [ 'LEFT JOIN', 'actor_user = user_id' ] ] - ); - } + $res = $dbSecondary->select( + 'user', + [ 'actor_id' => 'NULL', 'user_id', 'user_name' ], + array_merge( [ 'user_name' => array_keys( $names ) ], $from ? [ "user_name $from" ] : [] ), + __METHOD__ + ); foreach ( $res as $row ) { $names[$row->user_name] = $row; } @@ -328,17 +264,8 @@ class ApiQueryUserContribs extends ApiQueryBase { $batchSize = count( $names ); } - // During migration, force ordering on the client side because we're - // having to combine multiple queries that would otherwise have - // different sort orders. - if ( $wgActorTableSchemaMigrationStage === MIGRATION_WRITE_BOTH || - $wgActorTableSchemaMigrationStage === MIGRATION_WRITE_NEW - ) { - $batchSize = 1; - } - // With the new schema, the DB query will order by actor so update $this->orderBy to match. - if ( $batchSize > 1 && $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + if ( $batchSize > 1 && ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) ) { $this->orderBy = 'actor'; } @@ -352,64 +279,22 @@ class ApiQueryUserContribs extends ApiQueryBase { $userIter->next(); } - // Ugh. We have to run the query three times, once for each - // possible 'orcond' from ActorMigration, and then merge them all - // together in the proper order. And preserving the correct - // $hookData for each one. - // @todo When ActorMigration is removed, this can go back to a - // single prepare and select. - $merged = []; - foreach ( [ 'actor', 'userid', 'username' ] as $which ) { - if ( $this->prepareQuery( $users, $limit - $count, $which ) ) { - $hookData = []; - $res = $this->select( __METHOD__, [], $hookData ); - foreach ( $res as $row ) { - $merged[] = [ $row, &$hookData ]; - } - } - } - $neg = $this->params['dir'] == 'newer' ? 1 : -1; - usort( $merged, function ( $a, $b ) use ( $neg, $batchSize ) { - if ( $batchSize === 1 ) { // One user, can't be different - $ret = 0; - } elseif ( $this->orderBy === 'id' ) { - $ret = $a[0]->rev_user <=> $b[0]->rev_user; - } elseif ( $this->orderBy === 'name' ) { - $ret = strcmp( $a[0]->rev_user_text, $b[0]->rev_user_text ); - } else { - $ret = $a[0]->rev_actor <=> $b[0]->rev_actor; - } - - if ( !$ret ) { - $ret = strcmp( - wfTimestamp( TS_MW, $a[0]->rev_timestamp ), - wfTimestamp( TS_MW, $b[0]->rev_timestamp ) - ); - } - - if ( !$ret ) { - $ret = $a[0]->rev_id <=> $b[0]->rev_id; - } - - return $neg * $ret; - } ); - $merged = array_slice( $merged, 0, $limit - $count + 1 ); - // (end "Ugh") + $hookData = []; + $this->prepareQuery( $users, $limit - $count ); + $res = $this->select( __METHOD__, [], $hookData ); if ( $this->fld_sizediff ) { $revIds = []; - foreach ( $merged as $data ) { - if ( $data[0]->rev_parent_id ) { - $revIds[] = $data[0]->rev_parent_id; + foreach ( $res as $row ) { + if ( $row->rev_parent_id ) { + $revIds[] = $row->rev_parent_id; } } $this->parentLens = MediaWikiServices::getInstance()->getRevisionStore() ->listRevisionSizes( $dbSecondary, $revIds ); } - foreach ( $merged as $data ) { - $row = $data[0]; - $hookData = &$data[1]; + foreach ( $res as $row ) { if ( ++$count > $limit ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... @@ -434,10 +319,8 @@ class ApiQueryUserContribs extends ApiQueryBase { * Prepares the query and returns the limit of rows requested * @param User[] $users * @param int $limit - * @param string $which 'actor', 'userid', or 'username' - * @return bool */ - private function prepareQuery( array $users, $limit, $which ) { + private function prepareQuery( array $users, $limit ) { global $wgActorTableSchemaMigrationStage, $wgChangeTagsSchemaMigrationStage; $this->resetQueryParams(); @@ -448,27 +331,26 @@ class ApiQueryUserContribs extends ApiQueryBase { $this->addJoinConds( $revQuery['joins'] ); $this->addFields( $revQuery['fields'] ); - $revWhere = ActorMigration::newMigration()->getWhere( $db, 'rev_user', $users ); - if ( !isset( $revWhere['orconds'][$which] ) ) { - return false; - } - $this->addWhere( $revWhere['orconds'][$which] ); - - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { + $revWhere = ActorMigration::newMigration()->getWhere( $db, 'rev_user', $users ); $orderUserField = 'rev_actor'; $userField = $this->orderBy === 'actor' ? 'revactor_actor' : 'actor_name'; - } else { - $orderUserField = $this->orderBy === 'id' ? 'rev_user' : 'rev_user_text'; - $userField = $revQuery['fields'][$orderUserField]; - } - if ( $which === 'actor' ) { $tsField = 'revactor_timestamp'; $idField = 'revactor_rev'; } else { + // If we're dealing with user names (rather than IDs) in read-old mode, + // pass false for ActorMigration::getWhere()'s $useId parameter so + // $revWhere['conds'] isn't an OR. + $revWhere = ActorMigration::newMigration() + ->getWhere( $db, 'rev_user', $users, $this->orderBy === 'id' ); + $orderUserField = $this->orderBy === 'id' ? 'rev_user' : 'rev_user_text'; + $userField = $revQuery['fields'][$orderUserField]; $tsField = 'rev_timestamp'; $idField = 'rev_id'; } + $this->addWhere( $revWhere['conds'] ); + // Handle continue parameter if ( !is_null( $this->params['continue'] ) ) { $continue = explode( '|', $this->params['continue'] ); @@ -620,8 +502,6 @@ class ApiQueryUserContribs extends ApiQueryBase { $this->addWhereFld( 'ct_tag', $this->params['tag'] ); } } - - return true; } /** diff --git a/includes/cache/UserCache.php b/includes/cache/UserCache.php index 2828b9a71f..8f816d9013 100644 --- a/includes/cache/UserCache.php +++ b/includes/cache/UserCache.php @@ -105,11 +105,13 @@ class UserCache { $fields = [ 'user_name', 'user_real_name', 'user_registration', 'user_id' ]; $joinConds = []; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + // Technically we shouldn't allow this without SCHEMA_COMPAT_READ_NEW, + // but it does little harm and might be needed for write callers loading a User. + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_NEW ) { $tables[] = 'actor'; $fields[] = 'actor_id'; $joinConds['actor'] = [ - $wgActorTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', + ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) ? 'JOIN' : 'LEFT JOIN', [ 'actor_user = user_id' ] ]; } @@ -125,7 +127,7 @@ class UserCache { $this->cache[$userId]['name'] = $row->user_name; $this->cache[$userId]['real_name'] = $row->user_real_name; $this->cache[$userId]['registration'] = $row->user_registration; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_NEW ) { $this->cache[$userId]['actor'] = $row->actor_id; } $usersToCheck[$userId] = $row->user_name; diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php index 819f1701dc..7f7d77d947 100644 --- a/includes/changes/RecentChange.php +++ b/includes/changes/RecentChange.php @@ -228,13 +228,14 @@ class RecentChange { global $wgActorTableSchemaMigrationStage; wfDeprecated( __METHOD__, '1.31' ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->rc_user or $row->rc_user_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index 343c7a60d7..6d921b9724 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -1193,7 +1193,9 @@ class DatabaseOracle extends Database { // all deletions on these tables have transactions so final failure rollbacks these updates // @todo: Normalize the schema to match MySQL, no special FKs and such $table = $this->tableName( $table ); - if ( $table == $this->tableName( 'user' ) && $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $table == $this->tableName( 'user' ) && + ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) + ) { $this->update( 'archive', [ 'ar_user' => 0 ], [ 'ar_user' => $conds['user_id'] ], $fname ); $this->update( 'ipblocks', [ 'ipb_user' => 0 ], diff --git a/includes/filerepo/file/ArchivedFile.php b/includes/filerepo/file/ArchivedFile.php index 5589c687e5..4a84cff172 100644 --- a/includes/filerepo/file/ArchivedFile.php +++ b/includes/filerepo/file/ArchivedFile.php @@ -221,13 +221,14 @@ class ArchivedFile { static function selectFields() { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->fa_user or $row->fa_user_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } @@ -248,7 +249,7 @@ class ArchivedFile { 'fa_minor_mime', 'fa_user', 'fa_user_text', - 'fa_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'fa_actor' : 'NULL', + 'fa_actor' => 'NULL', 'fa_timestamp', 'fa_deleted', 'fa_deleted_timestamp', /* Used by LocalFileRestoreBatch */ diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index ec01869c32..254ceff1b0 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -201,13 +201,14 @@ class LocalFile extends File { global $wgActorTableSchemaMigrationStage; wfDeprecated( __METHOD__, '1.31' ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->img_user or $row->img_user_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } @@ -223,7 +224,7 @@ class LocalFile extends File { 'img_minor_mime', 'img_user', 'img_user_text', - 'img_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'img_actor' : 'NULL', + 'img_actor' => 'NULL', 'img_timestamp', 'img_sha1', ] + MediaWikiServices::getInstance()->getCommentStore()->getFields( 'img_description' ); @@ -1573,16 +1574,16 @@ class LocalFile extends File { } } - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $fields['oi_user'] = 'img_user'; $fields['oi_user_text'] = 'img_user_text'; } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $fields['oi_actor'] = 'img_actor'; } - if ( $wgActorTableSchemaMigrationStage !== MIGRATION_OLD && - $wgActorTableSchemaMigrationStage !== MIGRATION_NEW + if ( + ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_BOTH ) === SCHEMA_COMPAT_WRITE_BOTH ) { // Upgrade any rows that are still old-style. Otherwise an upgrade // might be missed if a deletion happens while the migration script @@ -2569,16 +2570,16 @@ class LocalFileDeleteBatch { } } - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $fields['fa_user'] = 'img_user'; $fields['fa_user_text'] = 'img_user_text'; } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $fields['fa_actor'] = 'img_actor'; } - if ( $wgActorTableSchemaMigrationStage !== MIGRATION_OLD && - $wgActorTableSchemaMigrationStage !== MIGRATION_NEW + if ( + ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_BOTH ) === SCHEMA_COMPAT_WRITE_BOTH ) { // Upgrade any rows that are still old-style. Otherwise an upgrade // might be missed if a deletion happens while the migration script diff --git a/includes/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php index 9759b79289..f103afabc6 100644 --- a/includes/filerepo/file/OldLocalFile.php +++ b/includes/filerepo/file/OldLocalFile.php @@ -115,13 +115,14 @@ class OldLocalFile extends LocalFile { global $wgActorTableSchemaMigrationStage; wfDeprecated( __METHOD__, '1.31' ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { // If code is using this instead of self::getQueryInfo(), there's a // decent chance it's going to try to directly access // $row->oi_user or $row->oi_user_text and we can't give it - // useful values here once those aren't being written anymore. + // useful values here once those aren't being used anymore. throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage has SCHEMA_COMPAT_READ_NEW' ); } @@ -138,7 +139,7 @@ class OldLocalFile extends LocalFile { 'oi_minor_mime', 'oi_user', 'oi_user_text', - 'oi_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'oi_actor' : 'NULL', + 'oi_actor' => 'NULL', 'oi_timestamp', 'oi_deleted', 'oi_sha1', diff --git a/includes/installer/DatabaseUpdater.php b/includes/installer/DatabaseUpdater.php index 8634f89273..019482227e 100644 --- a/includes/installer/DatabaseUpdater.php +++ b/includes/installer/DatabaseUpdater.php @@ -1300,7 +1300,7 @@ abstract class DatabaseUpdater { */ protected function migrateActors() { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_NEW && + if ( ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) && !$this->updateRowExists( 'MigrateActors' ) ) { $this->output( diff --git a/includes/logging/LogEntry.php b/includes/logging/LogEntry.php index b53a486bae..5706f2d080 100644 --- a/includes/logging/LogEntry.php +++ b/includes/logging/LogEntry.php @@ -645,7 +645,7 @@ class ManualLogEntry extends LogEntryBase { $relations = $this->relations; // Ensure actor relations are set - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH && + if ( ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) && empty( $relations['target_author_actor'] ) ) { $actorIds = []; @@ -664,7 +664,7 @@ class ManualLogEntry extends LogEntryBase { $params['authorActors'] = $actorIds; } } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_NEW ) { + if ( !( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) ) { unset( $relations['target_author_id'], $relations['target_author_ip'] ); unset( $params['authorIds'], $params['authorIPs'] ); } diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 5793966d94..b79eb9251a 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -2914,7 +2914,7 @@ class WikiPage implements Page, IDBAccessObject { if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { $dbw->delete( 'revision_comment_temp', [ 'revcomment_rev' => $revids ], __METHOD__ ); } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $dbw->delete( 'revision_actor_temp', [ 'revactor_rev' => $revids ], __METHOD__ ); } diff --git a/includes/revisiondelete/RevDelList.php b/includes/revisiondelete/RevDelList.php index c5cffea505..7ddf587c9b 100644 --- a/includes/revisiondelete/RevDelList.php +++ b/includes/revisiondelete/RevDelList.php @@ -218,14 +218,14 @@ abstract class RevDelList extends RevisionListBase { $virtualOldBits |= $removedBits; $status->successCount++; - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { if ( $item->getAuthorId() > 0 ) { $authorIds[] = $item->getAuthorId(); } elseif ( IP::isIPAddress( $item->getAuthorName() ) ) { $authorIPs[] = $item->getAuthorName(); } } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $authorActors[] = $item->getAuthorActor(); } @@ -271,11 +271,11 @@ abstract class RevDelList extends RevisionListBase { // Log it $authorFields = []; - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $authorFields['authorIds'] = $authorIds; $authorFields['authorIPs'] = $authorIPs; } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $authorFields['authorActors'] = $authorActors; } $this->updateLog( diff --git a/includes/revisiondelete/RevisionDeleteUser.php b/includes/revisiondelete/RevisionDeleteUser.php index 6291e8d91e..a8bf81498f 100644 --- a/includes/revisiondelete/RevisionDeleteUser.php +++ b/includes/revisiondelete/RevisionDeleteUser.php @@ -67,7 +67,7 @@ class RevisionDeleteUser { $userTitle = Title::makeTitleSafe( NS_USER, $name ); $userDbKey = $userTitle->getDBkey(); - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { # Hide name from live edits $dbw->update( 'revision', @@ -116,7 +116,7 @@ class RevisionDeleteUser { ); } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $actorId = $dbw->selectField( 'actor', 'actor_id', [ 'actor_name' => $name ], __METHOD__ ); if ( $actorId ) { # Hide name from live edits diff --git a/includes/specials/SpecialLog.php b/includes/specials/SpecialLog.php index 54afde182d..9931614458 100644 --- a/includes/specials/SpecialLog.php +++ b/includes/specials/SpecialLog.php @@ -104,30 +104,12 @@ class SpecialLog extends SpecialPage { $offenderName = $opts->getValue( 'offender' ); $offender = empty( $offenderName ) ? null : User::newFromName( $offenderName, false ); if ( $offender ) { - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { $qc = [ 'ls_field' => 'target_author_actor', 'ls_value' => $offender->getActorId() ]; + } elseif ( $offender->getId() > 0 ) { + $qc = [ 'ls_field' => 'target_author_id', 'ls_value' => $offender->getId() ]; } else { - if ( $offender->getId() > 0 ) { - $field = 'target_author_id'; - $value = $offender->getId(); - } else { - $field = 'target_author_ip'; - $value = $offender->getName(); - } - if ( !$offender->getActorId() ) { - $qc = [ 'ls_field' => $field, 'ls_value' => $value ]; - } else { - $db = wfGetDB( DB_REPLICA ); - $qc = [ - 'ls_field' => [ 'target_author_actor', $field ], // So LogPager::getQueryInfo() works right - $db->makeList( [ - $db->makeList( - [ 'ls_field' => 'target_author_actor', 'ls_value' => $offender->getActorId() ], LIST_AND - ), - $db->makeList( [ 'ls_field' => $field, 'ls_value' => $value ], LIST_AND ), - ], LIST_OR ), - ]; - } + $qc = [ 'ls_field' => 'target_author_ip', 'ls_value' => $offender->getName() ]; } } } else { diff --git a/includes/specials/pagers/ContribsPager.php b/includes/specials/pagers/ContribsPager.php index 5b50f0a681..9900340957 100644 --- a/includes/specials/pagers/ContribsPager.php +++ b/includes/specials/pagers/ContribsPager.php @@ -221,14 +221,12 @@ class ContribsPager extends RangeChronologicalPager { $conds = ActorMigration::newMigration()->getWhere( $this->mDb, 'rev_user', $user ); $queryInfo['conds'][] = $conds['conds']; // Force the appropriate index to avoid bad query plans (T189026) - if ( count( $conds['orconds'] ) === 1 ) { - if ( isset( $conds['orconds']['actor'] ) ) { - // @todo: This will need changing when revision_comment_temp goes away - $queryInfo['options']['USE INDEX']['temp_rev_user'] = 'actor_timestamp'; - } else { - $queryInfo['options']['USE INDEX']['revision'] = - isset( $conds['orconds']['userid'] ) ? 'user_timestamp' : 'usertext_timestamp'; - } + if ( isset( $conds['orconds']['actor'] ) ) { + // @todo: This will need changing when revision_comment_temp goes away + $queryInfo['options']['USE INDEX']['temp_rev_user'] = 'actor_timestamp'; + } else { + $queryInfo['options']['USE INDEX']['revision'] = + isset( $conds['orconds']['userid'] ) ? 'user_timestamp' : 'usertext_timestamp'; } } } diff --git a/includes/specials/pagers/NewFilesPager.php b/includes/specials/pagers/NewFilesPager.php index c214f1f77b..6b7e4b8b31 100644 --- a/includes/specials/pagers/NewFilesPager.php +++ b/includes/specials/pagers/NewFilesPager.php @@ -113,7 +113,7 @@ class NewFilesPager extends RangeChronologicalPager { $conds['rc_patrolled'] = RecentChange::PRC_UNPATROLLED; $conds['rc_namespace'] = NS_FILE; - if ( $wgActorTableSchemaMigrationStage === MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) { $jcond = 'rc_actor = ' . $imgQuery['fields']['img_actor']; } else { $rcQuery = ActorMigration::newMigration()->getJoin( 'rc_user' ); diff --git a/includes/user/User.php b/includes/user/User.php index 12623e89fa..ada5dc9ce5 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -627,9 +627,12 @@ class User implements IDBAccessObject, UserIdentity { public static function newFromActorId( $id ) { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_OLD ) { + // Technically we shouldn't allow this without SCHEMA_COMPAT_READ_NEW, + // but it does little harm and might be needed for write callers loading a User. + if ( !( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_NEW ) ) { throw new BadMethodCallException( - 'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage is MIGRATION_OLD' + 'Cannot use ' . __METHOD__ + . ' when $wgActorTableSchemaMigrationStage lacks SCHEMA_COMPAT_NEW' ); } @@ -679,7 +682,9 @@ class User implements IDBAccessObject, UserIdentity { $user = new User; $user->mFrom = 'defaults'; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD && $actorId !== null ) { + // Technically we shouldn't allow this without SCHEMA_COMPAT_READ_NEW, + // but it does little harm and might be needed for write callers loading a User. + if ( ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_NEW ) && $actorId !== null ) { $user->mActorId = (int)$actorId; if ( $user->mActorId !== 0 ) { $user->mFrom = 'actor'; @@ -1488,7 +1493,9 @@ class User implements IDBAccessObject, UserIdentity { $this->mGroupMemberships = null; // deferred - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + // Technically we shouldn't allow this without SCHEMA_COMPAT_READ_NEW, + // but it does little harm and might be needed for write callers loading a User. + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_NEW ) { if ( isset( $row->actor_id ) ) { $this->mActorId = (int)$row->actor_id; if ( $this->mActorId !== 0 ) { @@ -2491,7 +2498,9 @@ class User implements IDBAccessObject, UserIdentity { public function getActorId( IDatabase $dbw = null ) { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_OLD ) { + // Technically we should always return 0 without SCHEMA_COMPAT_READ_NEW, + // but it does little harm and might be needed for write callers loading a User. + if ( !( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) ) { return 0; } @@ -2501,7 +2510,7 @@ class User implements IDBAccessObject, UserIdentity { // Currently $this->mActorId might be null if $this was loaded from a // cache entry that was written when $wgActorTableSchemaMigrationStage - // was MIGRATION_OLD. Once that is no longer a possibility (i.e. when + // was SCHEMA_COMPAT_OLD. Once that is no longer a possibility (i.e. when // User::VERSION is incremented after $wgActorTableSchemaMigrationStage // has been removed), that condition may be removed. if ( $this->mActorId === null || !$this->mActorId && $dbw ) { @@ -4222,7 +4231,7 @@ class User implements IDBAccessObject, UserIdentity { ); } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $dbw->update( 'actor', [ 'actor_name' => $this->mName ], @@ -4320,9 +4329,10 @@ class User implements IDBAccessObject, UserIdentity { $dbw->insert( 'user', $fields, $fname, [ 'IGNORE' ] ); if ( $dbw->affectedRows() ) { $newUser = self::newFromId( $dbw->insertId() ); + $newUser->mName = $fields['user_name']; + $newUser->updateActorId( $dbw ); // Load the user from master to avoid replica lag $newUser->load( self::READ_LATEST ); - $newUser->updateActorId( $dbw ); } else { $newUser = null; } @@ -4431,7 +4441,7 @@ class User implements IDBAccessObject, UserIdentity { private function updateActorId( IDatabase $dbw ) { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $dbw->insert( 'actor', [ 'actor_user' => $this->mId, 'actor_name' => $this->mName ], @@ -5656,14 +5666,18 @@ class User implements IDBAccessObject, UserIdentity { ], 'joins' => [], ]; - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + + // Technically we shouldn't allow this without SCHEMA_COMPAT_READ_NEW, + // but it does little harm and might be needed for write callers loading a User. + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_NEW ) { $ret['tables']['user_actor'] = 'actor'; $ret['fields'][] = 'user_actor.actor_id'; $ret['joins']['user_actor'] = [ - $wgActorTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', + ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_READ_NEW ) ? 'JOIN' : 'LEFT JOIN', [ 'user_actor.actor_user = user_id' ] ]; } + return $ret; } diff --git a/maintenance/initEditCount.php b/maintenance/initEditCount.php index 1b9dac068d..938503c74f 100644 --- a/maintenance/initEditCount.php +++ b/maintenance/initEditCount.php @@ -40,8 +40,6 @@ in the load balancer, usually indicating a replication environment.' ); } public function execute() { - global $wgActorTableSchemaMigrationStage; - $dbw = $this->getDB( DB_MASTER ); // Autodetect mode... @@ -56,15 +54,6 @@ in the load balancer, usually indicating a replication environment.' ); $actorQuery = ActorMigration::newMigration()->getJoin( 'rev_user' ); - $needSpecialQuery = ( $wgActorTableSchemaMigrationStage !== MIGRATION_OLD && - $wgActorTableSchemaMigrationStage !== MIGRATION_NEW ); - if ( $needSpecialQuery ) { - foreach ( $actorQuery['joins'] as &$j ) { - $j[0] = 'JOIN'; // replace LEFT JOIN - } - unset( $j ); - } - if ( $backgroundMode ) { $this->output( "Using replication-friendly background mode...\n" ); @@ -77,54 +66,15 @@ in the load balancer, usually indicating a replication environment.' ); for ( $min = 0; $min <= $lastUser; $min += $chunkSize ) { $max = $min + $chunkSize; - if ( $needSpecialQuery ) { - // Use separate subqueries to collect counts with the old - // and new schemas, to avoid having to do whole-table scans. - $result = $dbr->select( - [ - 'user', - 'rev1' => '(' - . $dbr->selectSQLText( - [ 'revision', 'revision_actor_temp' ], - [ 'rev_user', 'ct' => 'COUNT(*)' ], - [ - "rev_user > $min AND rev_user <= $max", - 'revactor_rev' => null, - ], - __METHOD__, - [ 'GROUP BY' => 'rev_user' ], - [ 'revision_actor_temp' => [ 'LEFT JOIN', 'revactor_rev = rev_id' ] ] - ) . ')', - 'rev2' => '(' - . $dbr->selectSQLText( - [ 'revision' ] + $actorQuery['tables'], - [ 'actor_user', 'ct' => 'COUNT(*)' ], - "actor_user > $min AND actor_user <= $max", - __METHOD__, - [ 'GROUP BY' => 'actor_user' ], - $actorQuery['joins'] - ) . ')', - ], - [ 'user_id', 'user_editcount' => 'COALESCE(rev1.ct,0) + COALESCE(rev2.ct,0)' ], - "user_id > $min AND user_id <= $max", - __METHOD__, - [], - [ - 'rev1' => [ 'LEFT JOIN', 'user_id = rev_user' ], - 'rev2' => [ 'LEFT JOIN', 'user_id = actor_user' ], - ] - ); - } else { - $revUser = $actorQuery['fields']['rev_user']; - $result = $dbr->select( - [ 'user', 'rev' => [ 'revision' ] + $actorQuery['tables'] ], - [ 'user_id', 'user_editcount' => "COUNT($revUser)" ], - "user_id > $min AND user_id <= $max", - __METHOD__, - [ 'GROUP BY' => 'user_id' ], - [ 'rev' => [ 'LEFT JOIN', "user_id = $revUser" ] ] + $actorQuery['joins'] - ); - } + $revUser = $actorQuery['fields']['rev_user']; + $result = $dbr->select( + [ 'user', 'rev' => [ 'revision' ] + $actorQuery['tables'] ], + [ 'user_id', 'user_editcount' => "COUNT($revUser)" ], + "user_id > $min AND user_id <= $max", + __METHOD__, + [ 'GROUP BY' => 'user_id' ], + [ 'rev' => [ 'LEFT JOIN', "user_id = $revUser" ] ] + $actorQuery['joins'] + ); foreach ( $result as $row ) { $dbw->update( 'user', @@ -149,41 +99,15 @@ in the load balancer, usually indicating a replication environment.' ); $this->output( "Using single-query mode...\n" ); $user = $dbw->tableName( 'user' ); - if ( $needSpecialQuery ) { - $subquery1 = $dbw->selectSQLText( - [ 'revision', 'revision_actor_temp' ], - [ 'COUNT(*)' ], - [ - 'user_id = rev_user', - 'revactor_rev' => null, - ], - __METHOD__, - [], - [ 'revision_actor_temp' => [ 'LEFT JOIN', 'revactor_rev = rev_id' ] ] - ); - $subquery2 = $dbw->selectSQLText( - [ 'revision' ] + $actorQuery['tables'], - [ 'COUNT(*)' ], - 'user_id = actor_user', - __METHOD__, - [], - $actorQuery['joins'] - ); - $dbw->query( - "UPDATE $user SET user_editcount=($subquery1) + ($subquery2)", - __METHOD__ - ); - } else { - $subquery = $dbw->selectSQLText( - [ 'revision' ] + $actorQuery['tables'], - [ 'COUNT(*)' ], - [ 'user_id = ' . $actorQuery['fields']['rev_user'] ], - __METHOD__, - [], - $actorQuery['joins'] - ); - $dbw->query( "UPDATE $user SET user_editcount=($subquery)", __METHOD__ ); - } + $subquery = $dbw->selectSQLText( + [ 'revision' ] + $actorQuery['tables'], + [ 'COUNT(*)' ], + [ 'user_id = ' . $actorQuery['fields']['rev_user'] ], + __METHOD__, + [], + $actorQuery['joins'] + ); + $dbw->query( "UPDATE $user SET user_editcount=($subquery)", __METHOD__ ); } $this->output( "Done!\n" ); diff --git a/maintenance/migrateActors.php b/maintenance/migrateActors.php index edd5dda002..5e27ac8e8f 100644 --- a/maintenance/migrateActors.php +++ b/maintenance/migrateActors.php @@ -45,9 +45,9 @@ class MigrateActors extends LoggedUpdateMaintenance { protected function doDBUpdates() { global $wgActorTableSchemaMigrationStage; - if ( $wgActorTableSchemaMigrationStage < MIGRATION_WRITE_NEW ) { + if ( !( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) ) { $this->output( - "...cannot update while \$wgActorTableSchemaMigrationStage < MIGRATION_WRITE_NEW\n" + "...cannot update while \$wgActorTableSchemaMigrationStage lacks SCHEMA_COMPAT_WRITE_NEW\n" ); return false; } @@ -266,7 +266,6 @@ class MigrateActors extends LoggedUpdateMaintenance { $table, [ $actorField => $row->actor_id, - $nameField => '', ], array_intersect_key( (array)$row, $pkFilter ) + [ $actorField => 0 @@ -377,7 +376,6 @@ class MigrateActors extends LoggedUpdateMaintenance { } $this->beginTransaction( $dbw, __METHOD__ ); $dbw->insert( $newTable, $inserts, __METHOD__ ); - $dbw->update( $table, [ $nameField => '' ], [ $primaryKey => $updates ], __METHOD__ ); $countUpdated += $dbw->affectedRows(); $this->commitTransaction( $dbw, __METHOD__ ); } diff --git a/maintenance/populateLogSearch.php b/maintenance/populateLogSearch.php index 589be48fa7..e80b6f6665 100644 --- a/maintenance/populateLogSearch.php +++ b/maintenance/populateLogSearch.php @@ -117,15 +117,17 @@ class PopulateLogSearch extends LoggedUpdateMaintenance { $tables = [ self::$tableMap[$prefix] ]; $fields = []; $joins = []; - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { + // Read the old fields if we're still writing them regardless of read mode, to handle upgrades $fields['userid'] = $prefix . '_user'; $fields['username'] = $prefix . '_user_text'; } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { + // Read the new fields if we're writing them regardless of read mode, to handle upgrades if ( $prefix === 'rev' ) { $tables[] = 'revision_actor_temp'; $joins['revision_actor_temp'] = [ - $wgActorTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', + ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) ? 'LEFT JOIN' : 'JOIN', 'rev_id = revactor_rev', ]; $fields['actorid'] = 'revactor_actor'; @@ -147,11 +149,13 @@ class PopulateLogSearch extends LoggedUpdateMaintenance { $log->addRelations( 'log_id', $items, $row->log_id ); // Query item author relations... $fields = []; - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { + // Read the old fields if we're still writing them regardless of read mode, to handle upgrades $fields['userid'] = 'log_user'; $fields['username'] = 'log_user_text'; } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { + // Read the new fields if we're writing them regardless of read mode, to handle upgrades $fields['actorid'] = 'log_actor'; } @@ -163,14 +167,14 @@ class PopulateLogSearch extends LoggedUpdateMaintenance { // Add item author relations... $userIds = $userIPs = $userActors = []; foreach ( $sres as $srow ) { - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { if ( $srow->userid > 0 ) { $userIds[] = intval( $srow->userid ); } elseif ( $srow->username != '' ) { $userIPs[] = $srow->username; } } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { if ( $srow->actorid ) { $userActors[] = intval( $srow->actorid ); } elseif ( $srow->userid > 0 ) { @@ -181,11 +185,11 @@ class PopulateLogSearch extends LoggedUpdateMaintenance { } } // Add item author relations... - if ( $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $log->addRelations( 'target_author_id', $userIds, $row->log_id ); $log->addRelations( 'target_author_ip', $userIPs, $row->log_id ); } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $log->addRelations( 'target_author_actor', $userActors, $row->log_id ); } } diff --git a/maintenance/reassignEdits.php b/maintenance/reassignEdits.php index d90a4a75eb..98025d1d90 100644 --- a/maintenance/reassignEdits.php +++ b/maintenance/reassignEdits.php @@ -136,20 +136,19 @@ class ReassignEdits extends Maintenance { if ( $total ) { # Reassign edits $this->output( "\nReassigning current edits..." ); - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $dbw->update( 'revision', [ 'rev_user' => $to->getId(), - 'rev_user_text' => - $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ? $to->getName() : '' + 'rev_user_text' => $to->getName(), ], $from->isLoggedIn() ? [ 'rev_user' => $from->getId() ] : [ 'rev_user_text' => $from->getName() ], __METHOD__ ); } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $dbw->update( 'revision_actor_temp', [ 'revactor_actor' => $to->getActorId( $dbw ) ], @@ -179,7 +178,7 @@ class ReassignEdits extends Maintenance { } /** - * Return user specifications + * Return user specifications for an UPDATE * i.e. user => id, user_text => text * * @param IDatabase $dbw Database handle @@ -193,13 +192,13 @@ class ReassignEdits extends Maintenance { global $wgActorTableSchemaMigrationStage; $ret = []; - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $ret += [ $idfield => $user->getId(), - $utfield => $wgActorTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ? $user->getName() : '', + $utfield => $user->getName(), ]; } - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $ret += [ $acfield => $user->getActorId( $dbw ) ]; } return $ret; diff --git a/maintenance/removeUnusedAccounts.php b/maintenance/removeUnusedAccounts.php index 3fa30cbd39..6b2f4886c6 100644 --- a/maintenance/removeUnusedAccounts.php +++ b/maintenance/removeUnusedAccounts.php @@ -48,7 +48,7 @@ class RemoveUnusedAccounts extends Maintenance { $delUser = []; $delActor = []; $dbr = $this->getDB( DB_REPLICA ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $res = $dbr->select( [ 'user', 'actor' ], [ 'user_id', 'user_name', 'user_touched', 'actor_id' ], @@ -94,7 +94,7 @@ class RemoveUnusedAccounts extends Maintenance { $this->output( "\nDeleting unused accounts..." ); $dbw = $this->getDB( DB_MASTER ); $dbw->delete( 'user', [ 'user_id' => $delUser ], __METHOD__ ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { # Keep actor rows referenced from ipblocks $keep = $dbw->selectFieldValues( 'ipblocks', 'ipb_by_actor', [ 'ipb_by_actor' => $delActor ], __METHOD__ @@ -110,11 +110,11 @@ class RemoveUnusedAccounts extends Maintenance { $dbw->delete( 'user_groups', [ 'ug_user' => $delUser ], __METHOD__ ); $dbw->delete( 'user_former_groups', [ 'ufg_user' => $delUser ], __METHOD__ ); $dbw->delete( 'user_properties', [ 'up_user' => $delUser ], __METHOD__ ); - if ( $wgActorTableSchemaMigrationStage > MIGRATION_OLD ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { $dbw->delete( 'logging', [ 'log_actor' => $delActor ], __METHOD__ ); $dbw->delete( 'recentchanges', [ 'rc_actor' => $delActor ], __METHOD__ ); } - if ( $wgActorTableSchemaMigrationStage < MIGRATION_NEW ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { $dbw->delete( 'logging', [ 'log_user' => $delUser ], __METHOD__ ); $dbw->delete( 'recentchanges', [ 'rc_user' => $delUser ], __METHOD__ ); } diff --git a/maintenance/rollbackEdits.php b/maintenance/rollbackEdits.php index 878eb9b09d..0ea5db506e 100644 --- a/maintenance/rollbackEdits.php +++ b/maintenance/rollbackEdits.php @@ -99,18 +99,16 @@ class RollbackEdits extends Maintenance { $titles = []; $actorQuery = ActorMigration::newMigration() ->getWhere( $dbr, 'rev_user', User::newFromName( $user, false ) ); - foreach ( $actorQuery['orconds'] as $cond ) { - $results = $dbr->select( - [ 'page', 'revision' ] + $actorQuery['tables'], - [ 'page_namespace', 'page_title' ], - [ $cond ], - __METHOD__, - [], - [ 'revision' => [ 'JOIN', 'page_latest = rev_id' ] ] + $actorQuery['joins'] - ); - foreach ( $results as $row ) { - $titles[] = Title::makeTitle( $row->page_namespace, $row->page_title ); - } + $results = $dbr->select( + [ 'page', 'revision' ] + $actorQuery['tables'], + [ 'page_namespace', 'page_title' ], + $actorQuery['conds'], + __METHOD__, + [], + [ 'revision' => [ 'JOIN', 'page_latest = rev_id' ] ] + $actorQuery['joins'] + ); + foreach ( $results as $row ) { + $titles[] = Title::makeTitle( $row->page_namespace, $row->page_title ); } return $titles; diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php index cc3ef1cf06..1f3183f3ed 100644 --- a/tests/parser/ParserTestRunner.php +++ b/tests/parser/ParserTestRunner.php @@ -1233,7 +1233,7 @@ class ParserTestRunner { $tables[] = 'image_comment_temp'; } - if ( $wgActorTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { // The new tables for actors are in use $tables[] = 'actor'; $tables[] = 'revision_actor_temp'; diff --git a/tests/phpunit/includes/ActorMigrationTest.php b/tests/phpunit/includes/ActorMigrationTest.php index c9387507c2..b761d29758 100644 --- a/tests/phpunit/includes/ActorMigrationTest.php +++ b/tests/phpunit/includes/ActorMigrationTest.php @@ -27,6 +27,53 @@ class ActorMigrationTest extends MediaWikiLangTestCase { return new ActorMigration( $stage ); } + /** + * @dataProvider provideConstructor + * @param int $stage + * @param string|null $exceptionMsg + */ + public function testConstructor( $stage, $exceptionMsg ) { + try { + $m = new ActorMigration( $stage ); + if ( $exceptionMsg !== null ) { + $this->fail( 'Expected exception not thrown' ); + } + $this->assertInstanceOf( ActorMigration::class, $m ); + } catch ( InvalidArgumentException $ex ) { + $this->assertSame( $exceptionMsg, $ex->getMessage() ); + } + } + + public static function provideConstructor() { + return [ + [ 0, '$stage must include a write mode' ], + [ SCHEMA_COMPAT_READ_OLD, '$stage must include a write mode' ], + [ SCHEMA_COMPAT_READ_NEW, '$stage must include a write mode' ], + [ SCHEMA_COMPAT_READ_BOTH, '$stage must include a write mode' ], + + [ SCHEMA_COMPAT_WRITE_OLD, '$stage must include a read mode' ], + [ SCHEMA_COMPAT_WRITE_OLD | SCHEMA_COMPAT_READ_OLD, null ], + [ + SCHEMA_COMPAT_WRITE_OLD | SCHEMA_COMPAT_READ_NEW, + 'Cannot read the new schema without also writing it' + ], + [ SCHEMA_COMPAT_WRITE_OLD | SCHEMA_COMPAT_READ_BOTH, 'Cannot read both schemas' ], + + [ SCHEMA_COMPAT_WRITE_NEW, '$stage must include a read mode' ], + [ + SCHEMA_COMPAT_WRITE_NEW | SCHEMA_COMPAT_READ_OLD, + 'Cannot read the old schema without also writing it' + ], + [ SCHEMA_COMPAT_WRITE_NEW | SCHEMA_COMPAT_READ_NEW, null ], + [ SCHEMA_COMPAT_WRITE_NEW | SCHEMA_COMPAT_READ_BOTH, 'Cannot read both schemas' ], + + [ SCHEMA_COMPAT_WRITE_BOTH, '$stage must include a read mode' ], + [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, null ], + [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, null ], + [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_BOTH, 'Cannot read both schemas' ], + ]; + } + /** * @dataProvider provideGetJoin * @param int $stage @@ -42,7 +89,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { public static function provideGetJoin() { return [ 'Simple table, old' => [ - MIGRATION_OLD, 'rc_user', [ + SCHEMA_COMPAT_OLD, 'rc_user', [ 'tables' => [], 'fields' => [ 'rc_user' => 'rc_user', @@ -52,34 +99,32 @@ class ActorMigrationTest extends MediaWikiLangTestCase { 'joins' => [], ], ], - 'Simple table, write-both' => [ - MIGRATION_WRITE_BOTH, 'rc_user', [ - 'tables' => [ 'actor_rc_user' => 'actor' ], + 'Simple table, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'rc_user', [ + 'tables' => [], 'fields' => [ - 'rc_user' => 'COALESCE( actor_rc_user.actor_user, rc_user )', - 'rc_user_text' => 'COALESCE( actor_rc_user.actor_name, rc_user_text )', - 'rc_actor' => 'rc_actor', - ], - 'joins' => [ - 'actor_rc_user' => [ 'LEFT JOIN', 'actor_rc_user.actor_id = rc_actor' ], + 'rc_user' => 'rc_user', + 'rc_user_text' => 'rc_user_text', + 'rc_actor' => 'NULL', ], + 'joins' => [], ], ], - 'Simple table, write-new' => [ - MIGRATION_WRITE_NEW, 'rc_user', [ + 'Simple table, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'rc_user', [ 'tables' => [ 'actor_rc_user' => 'actor' ], 'fields' => [ - 'rc_user' => 'COALESCE( actor_rc_user.actor_user, rc_user )', - 'rc_user_text' => 'COALESCE( actor_rc_user.actor_name, rc_user_text )', + 'rc_user' => 'actor_rc_user.actor_user', + 'rc_user_text' => 'actor_rc_user.actor_name', 'rc_actor' => 'rc_actor', ], 'joins' => [ - 'actor_rc_user' => [ 'LEFT JOIN', 'actor_rc_user.actor_id = rc_actor' ], + 'actor_rc_user' => [ 'JOIN', 'actor_rc_user.actor_id = rc_actor' ], ], ], ], 'Simple table, new' => [ - MIGRATION_NEW, 'rc_user', [ + SCHEMA_COMPAT_NEW, 'rc_user', [ 'tables' => [ 'actor_rc_user' => 'actor' ], 'fields' => [ 'rc_user' => 'actor_rc_user.actor_user', @@ -93,7 +138,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { ], 'ipblocks, old' => [ - MIGRATION_OLD, 'ipb_by', [ + SCHEMA_COMPAT_OLD, 'ipb_by', [ 'tables' => [], 'fields' => [ 'ipb_by' => 'ipb_by', @@ -103,34 +148,32 @@ class ActorMigrationTest extends MediaWikiLangTestCase { 'joins' => [], ], ], - 'ipblocks, write-both' => [ - MIGRATION_WRITE_BOTH, 'ipb_by', [ - 'tables' => [ 'actor_ipb_by' => 'actor' ], + 'ipblocks, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'ipb_by', [ + 'tables' => [], 'fields' => [ - 'ipb_by' => 'COALESCE( actor_ipb_by.actor_user, ipb_by )', - 'ipb_by_text' => 'COALESCE( actor_ipb_by.actor_name, ipb_by_text )', - 'ipb_by_actor' => 'ipb_by_actor', - ], - 'joins' => [ - 'actor_ipb_by' => [ 'LEFT JOIN', 'actor_ipb_by.actor_id = ipb_by_actor' ], + 'ipb_by' => 'ipb_by', + 'ipb_by_text' => 'ipb_by_text', + 'ipb_by_actor' => 'NULL', ], + 'joins' => [], ], ], - 'ipblocks, write-new' => [ - MIGRATION_WRITE_NEW, 'ipb_by', [ + 'ipblocks, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'ipb_by', [ 'tables' => [ 'actor_ipb_by' => 'actor' ], 'fields' => [ - 'ipb_by' => 'COALESCE( actor_ipb_by.actor_user, ipb_by )', - 'ipb_by_text' => 'COALESCE( actor_ipb_by.actor_name, ipb_by_text )', + 'ipb_by' => 'actor_ipb_by.actor_user', + 'ipb_by_text' => 'actor_ipb_by.actor_name', 'ipb_by_actor' => 'ipb_by_actor', ], 'joins' => [ - 'actor_ipb_by' => [ 'LEFT JOIN', 'actor_ipb_by.actor_id = ipb_by_actor' ], + 'actor_ipb_by' => [ 'JOIN', 'actor_ipb_by.actor_id = ipb_by_actor' ], ], ], ], 'ipblocks, new' => [ - MIGRATION_NEW, 'ipb_by', [ + SCHEMA_COMPAT_NEW, 'ipb_by', [ 'tables' => [ 'actor_ipb_by' => 'actor' ], 'fields' => [ 'ipb_by' => 'actor_ipb_by.actor_user', @@ -144,7 +187,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { ], 'Revision, old' => [ - MIGRATION_OLD, 'rev_user', [ + SCHEMA_COMPAT_OLD, 'rev_user', [ 'tables' => [], 'fields' => [ 'rev_user' => 'rev_user', @@ -154,42 +197,36 @@ class ActorMigrationTest extends MediaWikiLangTestCase { 'joins' => [], ], ], - 'Revision, write-both' => [ - MIGRATION_WRITE_BOTH, 'rev_user', [ - 'tables' => [ - 'temp_rev_user' => 'revision_actor_temp', - 'actor_rev_user' => 'actor', - ], + 'Revision, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'rev_user', [ + 'tables' => [], 'fields' => [ - 'rev_user' => 'COALESCE( actor_rev_user.actor_user, rev_user )', - 'rev_user_text' => 'COALESCE( actor_rev_user.actor_name, rev_user_text )', - 'rev_actor' => 'temp_rev_user.revactor_actor', - ], - 'joins' => [ - 'temp_rev_user' => [ 'LEFT JOIN', 'temp_rev_user.revactor_rev = rev_id' ], - 'actor_rev_user' => [ 'LEFT JOIN', 'actor_rev_user.actor_id = temp_rev_user.revactor_actor' ], + 'rev_user' => 'rev_user', + 'rev_user_text' => 'rev_user_text', + 'rev_actor' => 'NULL', ], + 'joins' => [], ], ], - 'Revision, write-new' => [ - MIGRATION_WRITE_NEW, 'rev_user', [ + 'Revision, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'rev_user', [ 'tables' => [ 'temp_rev_user' => 'revision_actor_temp', 'actor_rev_user' => 'actor', ], 'fields' => [ - 'rev_user' => 'COALESCE( actor_rev_user.actor_user, rev_user )', - 'rev_user_text' => 'COALESCE( actor_rev_user.actor_name, rev_user_text )', + 'rev_user' => 'actor_rev_user.actor_user', + 'rev_user_text' => 'actor_rev_user.actor_name', 'rev_actor' => 'temp_rev_user.revactor_actor', ], 'joins' => [ - 'temp_rev_user' => [ 'LEFT JOIN', 'temp_rev_user.revactor_rev = rev_id' ], - 'actor_rev_user' => [ 'LEFT JOIN', 'actor_rev_user.actor_id = temp_rev_user.revactor_actor' ], + 'temp_rev_user' => [ 'JOIN', 'temp_rev_user.revactor_rev = rev_id' ], + 'actor_rev_user' => [ 'JOIN', 'actor_rev_user.actor_id = temp_rev_user.revactor_actor' ], ], ], ], 'Revision, new' => [ - MIGRATION_NEW, 'rev_user', [ + SCHEMA_COMPAT_NEW, 'rev_user', [ 'tables' => [ 'temp_rev_user' => 'revision_actor_temp', 'actor_rev_user' => 'actor', @@ -248,34 +285,28 @@ class ActorMigrationTest extends MediaWikiLangTestCase { return [ 'Simple table, old' => [ - MIGRATION_OLD, 'rc_user', $genericUser, true, [ + SCHEMA_COMPAT_OLD, 'rc_user', $genericUser, true, [ 'tables' => [], 'orconds' => [ 'userid' => "rc_user = '1'" ], 'joins' => [], ], ], - 'Simple table, write-both' => [ - MIGRATION_WRITE_BOTH, 'rc_user', $genericUser, true, [ + 'Simple table, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'rc_user', $genericUser, true, [ 'tables' => [], - 'orconds' => [ - 'actor' => "rc_actor = '11'", - 'userid' => "rc_actor = '0' AND rc_user = '1'" - ], + 'orconds' => [ 'userid' => "rc_user = '1'" ], 'joins' => [], ], ], - 'Simple table, write-new' => [ - MIGRATION_WRITE_NEW, 'rc_user', $genericUser, true, [ + 'Simple table, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'rc_user', $genericUser, true, [ 'tables' => [], - 'orconds' => [ - 'actor' => "rc_actor = '11'", - 'userid' => "rc_actor = '0' AND rc_user = '1'" - ], + 'orconds' => [ 'actor' => "rc_actor = '11'" ], 'joins' => [], ], ], 'Simple table, new' => [ - MIGRATION_NEW, 'rc_user', $genericUser, true, [ + SCHEMA_COMPAT_NEW, 'rc_user', $genericUser, true, [ 'tables' => [], 'orconds' => [ 'actor' => "rc_actor = '11'" ], 'joins' => [], @@ -283,34 +314,28 @@ class ActorMigrationTest extends MediaWikiLangTestCase { ], 'ipblocks, old' => [ - MIGRATION_OLD, 'ipb_by', $genericUser, true, [ + SCHEMA_COMPAT_OLD, 'ipb_by', $genericUser, true, [ 'tables' => [], 'orconds' => [ 'userid' => "ipb_by = '1'" ], 'joins' => [], ], ], - 'ipblocks, write-both' => [ - MIGRATION_WRITE_BOTH, 'ipb_by', $genericUser, true, [ + 'ipblocks, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'ipb_by', $genericUser, true, [ 'tables' => [], - 'orconds' => [ - 'actor' => "ipb_by_actor = '11'", - 'userid' => "ipb_by_actor = '0' AND ipb_by = '1'" - ], + 'orconds' => [ 'userid' => "ipb_by = '1'" ], 'joins' => [], ], ], - 'ipblocks, write-new' => [ - MIGRATION_WRITE_NEW, 'ipb_by', $genericUser, true, [ + 'ipblocks, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'ipb_by', $genericUser, true, [ 'tables' => [], - 'orconds' => [ - 'actor' => "ipb_by_actor = '11'", - 'userid' => "ipb_by_actor = '0' AND ipb_by = '1'" - ], + 'orconds' => [ 'actor' => "ipb_by_actor = '11'" ], 'joins' => [], ], ], 'ipblocks, new' => [ - MIGRATION_NEW, 'ipb_by', $genericUser, true, [ + SCHEMA_COMPAT_NEW, 'ipb_by', $genericUser, true, [ 'tables' => [], 'orconds' => [ 'actor' => "ipb_by_actor = '11'" ], 'joins' => [], @@ -318,44 +343,32 @@ class ActorMigrationTest extends MediaWikiLangTestCase { ], 'Revision, old' => [ - MIGRATION_OLD, 'rev_user', $genericUser, true, [ + SCHEMA_COMPAT_OLD, 'rev_user', $genericUser, true, [ 'tables' => [], 'orconds' => [ 'userid' => "rev_user = '1'" ], 'joins' => [], ], ], - 'Revision, write-both' => [ - MIGRATION_WRITE_BOTH, 'rev_user', $genericUser, true, [ - 'tables' => [ - 'temp_rev_user' => 'revision_actor_temp', - ], - 'orconds' => [ - 'actor' => - "(temp_rev_user.revactor_actor IS NOT NULL) AND temp_rev_user.revactor_actor = '11'", - 'userid' => "temp_rev_user.revactor_actor IS NULL AND rev_user = '1'" - ], - 'joins' => [ - 'temp_rev_user' => [ 'LEFT JOIN', 'temp_rev_user.revactor_rev = rev_id' ], - ], + 'Revision, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'rev_user', $genericUser, true, [ + 'tables' => [], + 'orconds' => [ 'userid' => "rev_user = '1'" ], + 'joins' => [], ], ], - 'Revision, write-new' => [ - MIGRATION_WRITE_NEW, 'rev_user', $genericUser, true, [ + 'Revision, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'rev_user', $genericUser, true, [ 'tables' => [ 'temp_rev_user' => 'revision_actor_temp', ], - 'orconds' => [ - 'actor' => - "(temp_rev_user.revactor_actor IS NOT NULL) AND temp_rev_user.revactor_actor = '11'", - 'userid' => "temp_rev_user.revactor_actor IS NULL AND rev_user = '1'" - ], + 'orconds' => [ 'actor' => "temp_rev_user.revactor_actor = '11'" ], 'joins' => [ - 'temp_rev_user' => [ 'LEFT JOIN', 'temp_rev_user.revactor_rev = rev_id' ], + 'temp_rev_user' => [ 'JOIN', 'temp_rev_user.revactor_rev = rev_id' ], ], ], ], 'Revision, new' => [ - MIGRATION_NEW, 'rev_user', $genericUser, true, [ + SCHEMA_COMPAT_NEW, 'rev_user', $genericUser, true, [ 'tables' => [ 'temp_rev_user' => 'revision_actor_temp', ], @@ -367,7 +380,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { ], 'Multiple users, old' => [ - MIGRATION_OLD, 'rc_user', $complicatedUsers, true, [ + SCHEMA_COMPAT_OLD, 'rc_user', $complicatedUsers, true, [ 'tables' => [], 'orconds' => [ 'userid' => "rc_user IN ('1','2','3') ", @@ -376,30 +389,25 @@ class ActorMigrationTest extends MediaWikiLangTestCase { 'joins' => [], ], ], - 'Multiple users, write-both' => [ - MIGRATION_WRITE_BOTH, 'rc_user', $complicatedUsers, true, [ + 'Multiple users, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'rc_user', $complicatedUsers, true, [ 'tables' => [], 'orconds' => [ - 'actor' => "rc_actor IN ('11','12','34') ", - 'userid' => "rc_actor = '0' AND rc_user IN ('1','2','3') ", - 'username' => "rc_actor = '0' AND rc_user_text IN ('192.168.12.34','192.168.12.35') " + 'userid' => "rc_user IN ('1','2','3') ", + 'username' => "rc_user_text IN ('192.168.12.34','192.168.12.35') " ], 'joins' => [], ], ], - 'Multiple users, write-new' => [ - MIGRATION_WRITE_NEW, 'rc_user', $complicatedUsers, true, [ + 'Multiple users, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'rc_user', $complicatedUsers, true, [ 'tables' => [], - 'orconds' => [ - 'actor' => "rc_actor IN ('11','12','34') ", - 'userid' => "rc_actor = '0' AND rc_user IN ('1','2','3') ", - 'username' => "rc_actor = '0' AND rc_user_text IN ('192.168.12.34','192.168.12.35') " - ], + 'orconds' => [ 'actor' => "rc_actor IN ('11','12','34') " ], 'joins' => [], ], ], 'Multiple users, new' => [ - MIGRATION_NEW, 'rc_user', $complicatedUsers, true, [ + SCHEMA_COMPAT_NEW, 'rc_user', $complicatedUsers, true, [ 'tables' => [], 'orconds' => [ 'actor' => "rc_actor IN ('11','12','34') " ], 'joins' => [], @@ -407,7 +415,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { ], 'Multiple users, no use ID, old' => [ - MIGRATION_OLD, 'rc_user', $complicatedUsers, false, [ + SCHEMA_COMPAT_OLD, 'rc_user', $complicatedUsers, false, [ 'tables' => [], 'orconds' => [ 'username' => "rc_user_text IN ('User1','User2','User3','192.168.12.34','192.168.12.35') " @@ -415,30 +423,24 @@ class ActorMigrationTest extends MediaWikiLangTestCase { 'joins' => [], ], ], - 'Multiple users, write-both' => [ - MIGRATION_WRITE_BOTH, 'rc_user', $complicatedUsers, false, [ + 'Multiple users, read-old' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'rc_user', $complicatedUsers, false, [ 'tables' => [], 'orconds' => [ - 'actor' => "rc_actor IN ('11','12','34') ", - 'username' => "rc_actor = '0' AND " - . "rc_user_text IN ('User1','User2','User3','192.168.12.34','192.168.12.35') " + 'username' => "rc_user_text IN ('User1','User2','User3','192.168.12.34','192.168.12.35') " ], 'joins' => [], ], ], - 'Multiple users, write-new' => [ - MIGRATION_WRITE_NEW, 'rc_user', $complicatedUsers, false, [ + 'Multiple users, read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'rc_user', $complicatedUsers, false, [ 'tables' => [], - 'orconds' => [ - 'actor' => "rc_actor IN ('11','12','34') ", - 'username' => "rc_actor = '0' AND " - . "rc_user_text IN ('User1','User2','User3','192.168.12.34','192.168.12.35') " - ], + 'orconds' => [ 'actor' => "rc_actor IN ('11','12','34') " ], 'joins' => [], ], ], 'Multiple users, new' => [ - MIGRATION_NEW, 'rc_user', $complicatedUsers, false, [ + SCHEMA_COMPAT_NEW, 'rc_user', $complicatedUsers, false, [ 'tables' => [], 'orconds' => [ 'actor' => "rc_actor IN ('11','12','34') " ], 'joins' => [], @@ -470,12 +472,34 @@ class ActorMigrationTest extends MediaWikiLangTestCase { $user->method( 'getActorId' )->willReturn( $this->db->insertId() ); } + $stageNames = [ + SCHEMA_COMPAT_OLD => 'old', + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD => 'write-both-read-old', + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW => 'write-both-read-new', + SCHEMA_COMPAT_NEW => 'new', + ]; + $stages = [ - 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 ], + SCHEMA_COMPAT_OLD => [ + SCHEMA_COMPAT_OLD, + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, + ], + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD => [ + SCHEMA_COMPAT_OLD, + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, + SCHEMA_COMPAT_NEW + ], + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW => [ + SCHEMA_COMPAT_OLD, + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, + SCHEMA_COMPAT_NEW + ], + SCHEMA_COMPAT_NEW => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, + SCHEMA_COMPAT_NEW + ], ]; $nameKey = $key . '_text'; @@ -483,7 +507,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { foreach ( $stages as $writeStage => $possibleReadStages ) { if ( $key === 'ipb_by' ) { - $extraFields['ipb_address'] = __CLASS__ . "#$writeStage"; + $extraFields['ipb_address'] = __CLASS__ . "#{$stageNames[$writeStage]}"; } $w = $this->makeMigration( $writeStage ); @@ -495,17 +519,21 @@ class ActorMigrationTest extends MediaWikiLangTestCase { $fields = $w->getInsertValues( $this->db, $key, $user ); } - if ( $writeStage <= MIGRATION_WRITE_BOTH ) { - $this->assertSame( $user->getId(), $fields[$key], "old field, stage=$writeStage" ); - $this->assertSame( $user->getName(), $fields[$nameKey], "old field, stage=$writeStage" ); + if ( $writeStage & SCHEMA_COMPAT_WRITE_OLD ) { + $this->assertSame( $user->getId(), $fields[$key], + "old field, stage={$stageNames[$writeStage]}" ); + $this->assertSame( $user->getName(), $fields[$nameKey], + "old field, stage={$stageNames[$writeStage]}" ); } else { - $this->assertArrayNotHasKey( $key, $fields, "old field, stage=$writeStage" ); - $this->assertArrayNotHasKey( $nameKey, $fields, "old field, stage=$writeStage" ); + $this->assertArrayNotHasKey( $key, $fields, "old field, stage={$stageNames[$writeStage]}" ); + $this->assertArrayNotHasKey( $nameKey, $fields, "old field, stage={$stageNames[$writeStage]}" ); } - if ( $writeStage >= MIGRATION_WRITE_BOTH && !$usesTemp ) { - $this->assertSame( $user->getActorId(), $fields[$actorKey], "new field, stage=$writeStage" ); + if ( ( $writeStage & SCHEMA_COMPAT_WRITE_NEW ) && !$usesTemp ) { + $this->assertSame( $user->getActorId(), $fields[$actorKey], + "new field, stage={$stageNames[$writeStage]}" ); } else { - $this->assertArrayNotHasKey( $actorKey, $fields, "new field, stage=$writeStage" ); + $this->assertArrayNotHasKey( $actorKey, $fields, + "new field, stage={$stageNames[$writeStage]}" ); } $this->db->insert( $table, $extraFields + $fields, __METHOD__ ); @@ -527,12 +555,14 @@ class ActorMigrationTest extends MediaWikiLangTestCase { $queryInfo['joins'] ); - $this->assertSame( $user->getId(), (int)$row->$key, "w=$writeStage, r=$readStage, id" ); - $this->assertSame( $user->getName(), $row->$nameKey, "w=$writeStage, r=$readStage, name" ); + $this->assertSame( $user->getId(), (int)$row->$key, + "w={$stageNames[$writeStage]}, r={$stageNames[$readStage]}, id" ); + $this->assertSame( $user->getName(), $row->$nameKey, + "w={$stageNames[$writeStage]}, r={$stageNames[$readStage]}, name" ); $this->assertSame( - $readStage === MIGRATION_OLD || $writeStage === MIGRATION_OLD ? 0 : $user->getActorId(), + ( $readStage & SCHEMA_COMPAT_READ_OLD ) ? 0 : $user->getActorId(), (int)$row->$actorKey, - "w=$writeStage, r=$readStage, actor" + "w={$stageNames[$writeStage]}, r={$stageNames[$readStage]}, actor" ); } } @@ -572,10 +602,10 @@ class ActorMigrationTest extends MediaWikiLangTestCase { public static function provideStages() { return [ - 'MIGRATION_OLD' => [ MIGRATION_OLD ], - 'MIGRATION_WRITE_BOTH' => [ MIGRATION_WRITE_BOTH ], - 'MIGRATION_WRITE_NEW' => [ MIGRATION_WRITE_NEW ], - 'MIGRATION_NEW' => [ MIGRATION_NEW ], + 'old' => [ SCHEMA_COMPAT_OLD ], + 'read-old' => [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD ], + 'read-new' => [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW ], + 'new' => [ SCHEMA_COMPAT_NEW ], ]; } @@ -630,6 +660,12 @@ class ActorMigrationTest extends MediaWikiLangTestCase { } public function testInsertUserIdentity() { + $this->setMwGlobals( [ + // for User::getActorId() + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ] ); + $this->overrideMwServices(); + $user = $this->getTestUser()->getUser(); $userIdentity = $this->getMock( UserIdentity::class ); $userIdentity->method( 'getId' )->willReturn( $user->getId() ); @@ -638,7 +674,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase { list( $cFields, $cCallback ) = MediaWikiServices::getInstance()->getCommentStore() ->insertWithTempTable( $this->db, 'rev_comment', '' ); - $m = $this->makeMigration( MIGRATION_WRITE_BOTH ); + $m = $this->makeMigration( SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW ); list( $fields, $callback ) = $m->getInsertValuesWithTempTable( $this->db, 'rev_user', $userIdentity ); $extraFields = [ @@ -652,22 +688,22 @@ class ActorMigrationTest extends MediaWikiLangTestCase { $callback( $id, $extraFields ); $cCallback( $id ); - $qi = Revision::getQueryInfo(); + $qi = $m->getJoin( 'rev_user' ); $row = $this->db->selectRow( - $qi['tables'], $qi['fields'], [ 'rev_id' => $id ], __METHOD__, [], $qi['joins'] + [ 'revision' ] + $qi['tables'], $qi['fields'], [ 'rev_id' => $id ], __METHOD__, [], $qi['joins'] ); $this->assertSame( $user->getId(), (int)$row->rev_user ); $this->assertSame( $user->getName(), $row->rev_user_text ); $this->assertSame( $user->getActorId(), (int)$row->rev_actor ); - $m = $this->makeMigration( MIGRATION_WRITE_BOTH ); + $m = $this->makeMigration( SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW ); $fields = $m->getInsertValues( $this->db, 'dummy_user', $userIdentity ); $this->assertSame( $user->getId(), $fields['dummy_user'] ); $this->assertSame( $user->getName(), $fields['dummy_user_text'] ); $this->assertSame( $user->getActorId(), $fields['dummy_actor'] ); } - public function testConstructor() { + public function testNewMigration() { $m = ActorMigration::newMigration(); $this->assertInstanceOf( ActorMigration::class, $m ); $this->assertSame( $m, ActorMigration::newMigration() ); @@ -687,10 +723,12 @@ class ActorMigrationTest extends MediaWikiLangTestCase { public static function provideIsAnon() { return [ - 'MIGRATION_OLD' => [ MIGRATION_OLD, 'foo = 0', 'foo != 0' ], - 'MIGRATION_WRITE_BOTH' => [ MIGRATION_WRITE_BOTH, 'foo = 0', 'foo != 0' ], - 'MIGRATION_WRITE_NEW' => [ MIGRATION_WRITE_NEW, 'foo = 0', 'foo != 0' ], - 'MIGRATION_NEW' => [ MIGRATION_NEW, 'foo IS NULL', 'foo IS NOT NULL' ], + 'old' => [ SCHEMA_COMPAT_OLD, 'foo = 0', 'foo != 0' ], + 'read-old' => [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'foo = 0', 'foo != 0' ], + 'read-new' => [ + SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'foo IS NULL', 'foo IS NOT NULL' + ], + 'new' => [ SCHEMA_COMPAT_NEW, 'foo IS NULL', 'foo IS NOT NULL' ], ]; } diff --git a/tests/phpunit/includes/Revision/RevisionQueryInfoTest.php b/tests/phpunit/includes/Revision/RevisionQueryInfoTest.php index e852bec1f4..7188cf50a7 100644 --- a/tests/phpunit/includes/Revision/RevisionQueryInfoTest.php +++ b/tests/phpunit/includes/Revision/RevisionQueryInfoTest.php @@ -93,22 +93,14 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { ]; } - protected function getCompatActorQueryFields( $prefix, $tmp = false ) { - return [ - "{$prefix}_user" => "COALESCE( actor_{$prefix}_user.actor_user, {$prefix}_user )", - "{$prefix}_user_text" => "COALESCE( actor_{$prefix}_user.actor_name, {$prefix}_user_text )", - "{$prefix}_actor" => $tmp ?: "{$prefix}_actor", - ]; - } - - protected function getCompatActorJoins( $prefix ) { + protected function getNewActorJoins( $prefix ) { return [ "temp_{$prefix}_user" => [ - "LEFT JOIN", + "JOIN", "temp_{$prefix}_user.revactor_{$prefix} = {$prefix}_id", ], "actor_{$prefix}_user" => [ - "LEFT JOIN", + "JOIN", "actor_{$prefix}_user.actor_id = temp_{$prefix}_user.revactor_actor", ], ]; @@ -163,7 +155,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { [ 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_NEW, 'wgCommentTableSchemaMigrationStage' => MIGRATION_NEW, - 'wgActorTableSchemaMigrationStage' => MIGRATION_NEW, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_NEW, ], [ 'tables' => [ @@ -189,7 +181,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_NEW, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_NEW, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_NEW, ], [ 'tables' => [ @@ -199,13 +191,13 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { ], 'fields' => array_merge( $this->getArchiveQueryFields( false ), - $this->getCompatActorQueryFields( 'ar' ), + $this->getNewActorQueryFields( 'ar' ), $this->getCompatCommentQueryFields( 'ar' ) ), 'joins' => [ 'comment_ar_comment' => [ 'LEFT JOIN', 'comment_ar_comment.comment_id = ar_comment_id' ], - 'actor_ar_user' => [ 'LEFT JOIN', 'actor_ar_user.actor_id = ar_actor' ], + 'actor_ar_user' => [ 'JOIN', 'actor_ar_user.actor_id = ar_actor' ], ], ] ]; @@ -215,24 +207,22 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, ], [ 'tables' => [ 'archive', - 'actor_ar_user' => 'actor', 'comment_ar_comment' => 'comment', ], 'fields' => array_merge( $this->getArchiveQueryFields( true ), $this->getContentHandlerQueryFields( 'ar' ), - $this->getCompatActorQueryFields( 'ar' ), + $this->getOldActorQueryFields( 'ar' ), $this->getCompatCommentQueryFields( 'ar' ) ), 'joins' => [ 'comment_ar_comment' => [ 'LEFT JOIN', 'comment_ar_comment.comment_id = ar_comment_id' ], - 'actor_ar_user' => [ 'LEFT JOIN', 'actor_ar_user.actor_id = ar_actor' ], ], ] ]; @@ -241,7 +231,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => false, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [ 'tables' => [ @@ -264,7 +254,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => true, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_NEW, 'wgCommentTableSchemaMigrationStage' => MIGRATION_NEW, - 'wgActorTableSchemaMigrationStage' => MIGRATION_NEW, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_NEW, ], [ 'page', 'user' ], [ @@ -309,7 +299,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_NEW, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_NEW, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, ], [ 'page', 'user' ], [ @@ -326,7 +316,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { $this->getRevisionQueryFields( false ), $this->getPageQueryFields(), $this->getUserQueryFields(), - $this->getCompatActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), + $this->getNewActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), $this->getCompatCommentQueryFields( 'rev' ) ), 'joins' => array_merge( @@ -335,12 +325,12 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'user' => [ 'LEFT JOIN', [ - 'COALESCE( actor_rev_user.actor_user, rev_user ) != 0', - 'user_id = COALESCE( actor_rev_user.actor_user, rev_user )' + 'actor_rev_user.actor_user != 0', + 'user_id = actor_rev_user.actor_user', ] ], ], - $this->getCompatActorJoins( 'rev' ), + $this->getNewActorJoins( 'rev' ), $this->getCompatCommentJoins( 'rev' ) ), ] @@ -351,7 +341,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_NEW, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_NEW, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, ], [ 'page', 'user' ], [ @@ -368,7 +358,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { $this->getRevisionQueryFields( false ), $this->getPageQueryFields(), $this->getUserQueryFields(), - $this->getCompatActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), + $this->getNewActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), $this->getCompatCommentQueryFields( 'rev' ) ), 'joins' => array_merge( @@ -377,12 +367,12 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'user' => [ 'LEFT JOIN', [ - 'COALESCE( actor_rev_user.actor_user, rev_user ) != 0', - 'user_id = COALESCE( actor_rev_user.actor_user, rev_user )' + 'actor_rev_user.actor_user != 0', + 'user_id = actor_rev_user.actor_user' ] ], ], - $this->getCompatActorJoins( 'rev' ), + $this->getNewActorJoins( 'rev' ), $this->getCompatCommentJoins( 'rev' ) ), ] @@ -393,25 +383,22 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, ], [], [ 'tables' => [ 'revision', - 'temp_rev_user' => 'revision_actor_temp', 'temp_rev_comment' => 'revision_comment_temp', - 'actor_rev_user' => 'actor', 'comment_rev_comment' => 'comment', ], 'fields' => array_merge( $this->getRevisionQueryFields( true ), $this->getContentHandlerQueryFields( 'rev' ), - $this->getCompatActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), + $this->getOldActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), $this->getCompatCommentQueryFields( 'rev' ) ), 'joins' => array_merge( - $this->getCompatActorJoins( 'rev' ), $this->getCompatCommentJoins( 'rev' ) ), ] @@ -422,7 +409,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, ], [ 'page', 'user' ], [ @@ -430,9 +417,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'revision', 'page', 'user', - 'temp_rev_user' => 'revision_actor_temp', 'temp_rev_comment' => 'revision_comment_temp', - 'actor_rev_user' => 'actor', 'comment_rev_comment' => 'comment', ], 'fields' => array_merge( @@ -440,7 +425,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { $this->getContentHandlerQueryFields( 'rev' ), $this->getUserQueryFields(), $this->getPageQueryFields(), - $this->getCompatActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), + $this->getOldActorQueryFields( 'rev', 'temp_rev_user.revactor_actor' ), $this->getCompatCommentQueryFields( 'rev' ) ), 'joins' => array_merge( @@ -449,12 +434,11 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'user' => [ 'LEFT JOIN', [ - 'COALESCE( actor_rev_user.actor_user, rev_user ) != 0', - 'user_id = COALESCE( actor_rev_user.actor_user, rev_user )' + 'rev_user != 0', + 'user_id = rev_user' ] ], ], - $this->getCompatActorJoins( 'rev' ), $this->getCompatCommentJoins( 'rev' ) ), ] @@ -464,7 +448,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => true, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [], [ @@ -483,7 +467,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => true, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [ 'page', 'user' ], [ @@ -507,7 +491,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => false, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [], [ @@ -525,7 +509,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => false, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [ 'page' ], [ @@ -546,7 +530,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => false, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [ 'user' ], [ @@ -567,7 +551,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => false, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [ 'text' ], [ @@ -588,7 +572,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { 'wgContentHandlerUseDB' => false, 'wgMultiContentRevisionSchemaMigrationStage' => SCHEMA_COMPAT_OLD, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], [ 'text', 'page', 'user' ], [ @@ -850,7 +834,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { [ 'wgContentHandlerUseDB' => true, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, ], 'fields' => array_merge( [ @@ -878,7 +862,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { [ 'wgContentHandlerUseDB' => false, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], 'fields' => array_merge( [ @@ -905,7 +889,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { [ 'wgContentHandlerUseDB' => true, 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, ], 'fields' => array_merge( [ @@ -934,7 +918,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { [ 'wgContentHandlerUseDB' => false, 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ], 'fields' => array_merge( [ @@ -986,7 +970,7 @@ class RevisionQueryInfoTest extends MediaWikiTestCase { */ public function testRevisionUserJoinCond() { $this->hideDeprecated( 'Revision::userJoinCond' ); - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_OLD ); + $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_OLD ); $this->overrideMwServices(); $this->assertEquals( [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ], diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index 9e6d054ae8..0d6a439dc3 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -79,7 +79,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => $this->getMcrMigrationStage(), 'wgContentHandlerUseDB' => $this->getContentHandlerUseDB(), 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ] ); $this->overrideMwServices(); diff --git a/tests/phpunit/includes/RevisionDbTestBase.php b/tests/phpunit/includes/RevisionDbTestBase.php index 4b444084f3..cc166a39fb 100644 --- a/tests/phpunit/includes/RevisionDbTestBase.php +++ b/tests/phpunit/includes/RevisionDbTestBase.php @@ -91,7 +91,7 @@ abstract class RevisionDbTestBase extends MediaWikiTestCase { 'wgMultiContentRevisionSchemaMigrationStage' => $this->getMcrMigrationStage(), 'wgContentHandlerUseDB' => $this->getContentHandlerUseDB(), 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD, - 'wgActorTableSchemaMigrationStage' => MIGRATION_OLD, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_OLD, ] ); $this->overrideMwServices(); diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php index 1dbb298ed3..5868b8d38e 100644 --- a/tests/phpunit/includes/RevisionTest.php +++ b/tests/phpunit/includes/RevisionTest.php @@ -594,7 +594,7 @@ class RevisionTest extends MediaWikiTestCase { */ public function testLoadFromTitle() { $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', MIGRATION_OLD ); - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_OLD ); + $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_OLD ); $this->overrideMwServices(); $title = $this->getMockTitle(); diff --git a/tests/phpunit/includes/api/query/ApiQueryUserContribsTest.php b/tests/phpunit/includes/api/query/ApiQueryUserContribsTest.php index ac8637781a..924a1a5cd2 100644 --- a/tests/phpunit/includes/api/query/ApiQueryUserContribsTest.php +++ b/tests/phpunit/includes/api/query/ApiQueryUserContribsTest.php @@ -15,7 +15,7 @@ class ApiQueryUserContribsTest extends ApiTestCase { $wgActorTableSchemaMigrationStage = $v; $this->overrideMwServices(); }, [ $wgActorTableSchemaMigrationStage ] ); - $wgActorTableSchemaMigrationStage = MIGRATION_WRITE_BOTH; + $wgActorTableSchemaMigrationStage = SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD; $this->overrideMwServices(); $users = [ @@ -44,19 +44,12 @@ class ApiQueryUserContribsTest extends ApiTestCase { /** * @dataProvider provideSorting - * @param int $stage One of the MIGRATION_* constants for $wgActorTableSchemaMigrationStage + * @param int $stage SCHEMA_COMPAT contants for $wgActorTableSchemaMigrationStage * @param array $params Extra parameters for the query * @param bool $reverse Reverse order? * @param int $revs Number of revisions to expect */ public function testSorting( $stage, $params, $reverse, $revs ) { - if ( isset( $params['ucuserprefix'] ) && - ( $stage === MIGRATION_WRITE_BOTH || $stage === MIGRATION_WRITE_NEW ) && - $this->db->getType() === 'mysql' && $this->usesTemporaryTables() - ) { - // https://bugs.mysql.com/bug.php?id=10327 - $this->markTestSkipped( 'MySQL bug 10327 - can\'t reopen temporary tables' ); - } // FIXME: fails under sqlite $this->markTestSkippedIfDbType( 'sqlite' ); @@ -127,10 +120,10 @@ class ApiQueryUserContribsTest extends ApiTestCase { foreach ( [ - 'old' => MIGRATION_OLD, - 'write both' => MIGRATION_WRITE_BOTH, - 'write new' => MIGRATION_WRITE_NEW, - 'new' => MIGRATION_NEW, + 'old' => SCHEMA_COMPAT_OLD, + 'read old' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, + 'read new' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW, + 'new' => SCHEMA_COMPAT_NEW, ] as $stageName => $stage ) { foreach ( [ false, true ] as $reverse ) { @@ -152,7 +145,7 @@ class ApiQueryUserContribsTest extends ApiTestCase { /** * @dataProvider provideInterwikiUser - * @param int $stage One of the MIGRATION_* constants for $wgActorTableSchemaMigrationStage + * @param int $stage SCHEMA_COMPAT constants for $wgActorTableSchemaMigrationStage */ public function testInterwikiUser( $stage ) { $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', $stage ); @@ -186,10 +179,10 @@ class ApiQueryUserContribsTest extends ApiTestCase { public static function provideInterwikiUser() { return [ - 'old' => [ MIGRATION_OLD ], - 'write both' => [ MIGRATION_WRITE_BOTH ], - 'write new' => [ MIGRATION_WRITE_NEW ], - 'new' => [ MIGRATION_NEW ], + 'old' => [ SCHEMA_COMPAT_OLD ], + 'read old' => [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD ], + 'read new' => [ SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_NEW ], + 'new' => [ SCHEMA_COMPAT_NEW ], ]; } diff --git a/tests/phpunit/includes/logging/DatabaseLogEntryTest.php b/tests/phpunit/includes/logging/DatabaseLogEntryTest.php index 4af1742e1a..e75b1739d2 100644 --- a/tests/phpunit/includes/logging/DatabaseLogEntryTest.php +++ b/tests/phpunit/includes/logging/DatabaseLogEntryTest.php @@ -29,17 +29,19 @@ class DatabaseLogEntryTest extends MediaWikiTestCase { * @param array $selectFields * @param string[]|null $row * @param string[]|null $expectedFields - * @param string $migration + * @param int $commentMigration + * @param int $actorMigration */ public function testNewFromId( $id, array $selectFields, array $row = null, array $expectedFields = null, - $migration + $commentMigration, + $actorMigration ) { $this->setMwGlobals( [ - 'wgCommentTableSchemaMigrationStage' => $migration, - 'wgActorTableSchemaMigrationStage' => $migration, + 'wgCommentTableSchemaMigrationStage' => $commentMigration, + 'wgActorTableSchemaMigrationStage' => $actorMigration, ] ); $row = $row ? (object)$row : null; @@ -132,6 +134,7 @@ class DatabaseLogEntryTest extends MediaWikiTestCase { null, null, MIGRATION_OLD, + SCHEMA_COMPAT_OLD, ], [ 123, @@ -144,6 +147,7 @@ class DatabaseLogEntryTest extends MediaWikiTestCase { ], [ 'type' => 'foobarize', 'comment' => 'test!' ], MIGRATION_OLD, + SCHEMA_COMPAT_OLD, ], [ 567, @@ -156,6 +160,7 @@ class DatabaseLogEntryTest extends MediaWikiTestCase { ], [ 'type' => 'foobarize', 'comment' => 'test!' ], MIGRATION_NEW, + SCHEMA_COMPAT_NEW, ], ]; } diff --git a/tests/phpunit/includes/page/PageArchiveTestBase.php b/tests/phpunit/includes/page/PageArchiveTestBase.php index 6be19772de..ade8efd5c3 100644 --- a/tests/phpunit/includes/page/PageArchiveTestBase.php +++ b/tests/phpunit/includes/page/PageArchiveTestBase.php @@ -81,7 +81,7 @@ abstract class PageArchiveTestBase extends MediaWikiTestCase { $this->tablesUsed += $this->getMcrTablesToReset(); $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', MIGRATION_OLD ); - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_OLD ); + $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_OLD ); $this->setMwGlobals( 'wgContentHandlerUseDB', $this->getContentHandlerUseDB() ); $this->setMwGlobals( 'wgMultiContentRevisionSchemaMigrationStage', diff --git a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php index b8742158b8..b8cee67a00 100644 --- a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php +++ b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php @@ -197,15 +197,16 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testRcHidemyselfFilter() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $user = $this->getTestUser()->getUser(); $user->getActorId( wfGetDB( DB_MASTER ) ); $this->assertConditions( [ # expected - "NOT((rc_actor = '{$user->getActorId()}') OR " - . "(rc_actor = '0' AND rc_user = '{$user->getId()}'))", + "NOT((rc_user = '{$user->getId()}'))", ], [ 'hidemyself' => 1, @@ -218,7 +219,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase $id = $user->getActorId( wfGetDB( DB_MASTER ) ); $this->assertConditions( [ # expected - "NOT((rc_actor = '$id') OR (rc_actor = '0' AND rc_user_text = '10.11.12.13'))", + "NOT((rc_user_text = '10.11.12.13'))", ], [ 'hidemyself' => 1, @@ -229,15 +230,16 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testRcHidebyothersFilter() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $user = $this->getTestUser()->getUser(); $user->getActorId( wfGetDB( DB_MASTER ) ); $this->assertConditions( [ # expected - "(rc_actor = '{$user->getActorId()}') OR " - . "(rc_actor = '0' AND rc_user_text = '{$user->getName()}')", + "(rc_user_text = '{$user->getName()}')", ], [ 'hidebyothers' => 1, @@ -250,7 +252,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase $id = $user->getActorId( wfGetDB( DB_MASTER ) ); $this->assertConditions( [ # expected - "(rc_actor = '$id') OR (rc_actor = '0' AND rc_user_text = '10.11.12.13')", + "(rc_user_text = '10.11.12.13')", ], [ 'hidebyothers' => 1, @@ -462,13 +464,15 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testFilterUserExpLevelAllExperienceLevels() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $this->assertConditions( [ # expected - 'COALESCE( actor_rc_user.actor_user, rc_user ) != 0', + 'rc_user != 0', ], [ 'userExpLevel' => 'newcomer;learner;experienced', @@ -478,13 +482,15 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testFilterUserExpLevelRegistrered() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $this->assertConditions( [ # expected - 'COALESCE( actor_rc_user.actor_user, rc_user ) != 0', + 'rc_user != 0', ], [ 'userExpLevel' => 'registered', @@ -494,13 +500,15 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testFilterUserExpLevelUnregistrered() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $this->assertConditions( [ # expected - 'COALESCE( actor_rc_user.actor_user, rc_user ) = 0', + 'rc_user = 0', ], [ 'userExpLevel' => 'unregistered', @@ -510,13 +518,15 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testFilterUserExpLevelRegistreredOrLearner() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $this->assertConditions( [ # expected - 'COALESCE( actor_rc_user.actor_user, rc_user ) != 0', + 'rc_user != 0', ], [ 'userExpLevel' => 'registered;learner', @@ -526,13 +536,15 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase } public function testFilterUserExpLevelUnregistreredOrExperienced() { - $this->setMwGlobals( 'wgActorTableSchemaMigrationStage', MIGRATION_WRITE_BOTH ); + $this->setMwGlobals( + 'wgActorTableSchemaMigrationStage', SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD + ); $this->overrideMwServices(); $conds = $this->buildQuery( [ 'userExpLevel' => 'unregistered;experienced' ] ); $this->assertRegExp( - '/\(COALESCE\( actor_rc_user.actor_user, rc_user \) = 0\) OR ' + '/\(rc_user = 0\) OR ' . '\(\(user_editcount >= 500\) AND \(user_registration <= \'[^\']+\'\)\)/', reset( $conds ), "rc conditions: userExpLevel=unregistered;experienced" diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php index f86987a56f..cee15e82c8 100644 --- a/tests/phpunit/includes/user/UserTest.php +++ b/tests/phpunit/includes/user/UserTest.php @@ -22,7 +22,7 @@ class UserTest extends MediaWikiTestCase { $this->setMwGlobals( [ 'wgGroupPermissions' => [], 'wgRevokePermissions' => [], - 'wgActorTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH, + 'wgActorTableSchemaMigrationStage' => SCHEMA_COMPAT_WRITE_BOTH | SCHEMA_COMPAT_READ_OLD, ] ); $this->overrideMwServices(); -- 2.20.1