From 7a422138a791565d9bf8431991b2b0a18d99ba66 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Wed, 7 Mar 2018 11:40:56 -0500 Subject: [PATCH] Migrate image descriptions from image_comment_temp image_comment_temp was always intended to be temporary, until an expensive schema change on Wikimedia Commons's image table could be done. Now that that has been done, stop writing image_comment_temp and add a migration script to copy existing data into img_description_id. Ic8efeddc will remove the reads from image_comment_temp and drop the image_comment_temp table. Bug: T188132 Change-Id: Iab5f521577a415b2dc213b517ee8a0dca4fdd0aa --- RELEASE-NOTES-1.32 | 6 +- autoload.php | 1 + includes/CommentStore.php | 138 +++++++++++--------- includes/filerepo/file/LocalFile.php | 39 +++--- includes/installer/DatabaseUpdater.php | 16 +++ includes/installer/MssqlUpdater.php | 1 + includes/installer/MysqlUpdater.php | 1 + includes/installer/OracleUpdater.php | 1 + includes/installer/PostgresUpdater.php | 1 + includes/installer/SqliteUpdater.php | 1 + maintenance/migrateComments.php | 4 +- maintenance/migrateImageCommentTemp.php | 138 ++++++++++++++++++++ tests/phpunit/includes/CommentStoreTest.php | 40 ++++-- 13 files changed, 291 insertions(+), 96 deletions(-) create mode 100644 maintenance/migrateImageCommentTemp.php diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 6c200d900b..934d07bb67 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -475,6 +475,8 @@ because of Phabricator reports. * Skin::getDynamicStylesheetQuery() has been deprecated. It always returns action=raw&ctype=text/css which callers should use directly. * Class LegacyFormatter is deprecated. +* Use of CommentStore::insertWithTempTable() with 'img_description' is + deprecated. Use CommentStore::insert() instead. === Other changes in 1.32 === * (T198811) The following tables have had their UNIQUE indexes turned into @@ -490,7 +492,9 @@ because of Phabricator reports. yet for creating or managing content in slots beides the main slot. See for more information. -* … +* The image_comment_temp database table is merged into the image table and + deprecated. Since access should be mediated by the CommentStore class, this + change shouldn't affect external code. == Compatibility == MediaWiki 1.32 requires PHP 7.0.0 or later. Although HHVM 3.18.5 or later is diff --git a/autoload.php b/autoload.php index 33ee128db0..a0f505623a 100644 --- a/autoload.php +++ b/autoload.php @@ -950,6 +950,7 @@ $wgAutoloadLocalClasses = [ 'MigrateArchiveText' => __DIR__ . '/maintenance/migrateArchiveText.php', 'MigrateComments' => __DIR__ . '/maintenance/migrateComments.php', 'MigrateFileRepoLayout' => __DIR__ . '/maintenance/migrateFileRepoLayout.php', + 'MigrateImageCommentTemp' => __DIR__ . '/maintenance/migrateImageCommentTemp.php', 'MigrateUserGroup' => __DIR__ . '/maintenance/migrateUserGroup.php', 'MimeAnalyzer' => __DIR__ . '/includes/libs/mime/MimeAnalyzer.php', 'MinifyScript' => __DIR__ . '/maintenance/minify.php', diff --git a/includes/CommentStore.php b/includes/CommentStore.php index 9969b78782..1be79510cb 100644 --- a/includes/CommentStore.php +++ b/includes/CommentStore.php @@ -52,34 +52,33 @@ class CommentStore { /** * Define fields that use temporary tables for transitional purposes - * @var array Keys are '$key', values are arrays with four fields: + * @var array Keys are '$key', values are arrays with these possible fields: * - table: Temporary table name * - pk: Temporary table column referring to the main table's primary key * - field: Temporary table column referring comment.comment_id * - joinPK: Main table's primary key + * - stage: Migration stage + * - deprecatedIn: Version when using insertWithTempTable() was deprecated */ - protected static $tempTables = [ + protected $tempTables = [ 'rev_comment' => [ 'table' => 'revision_comment_temp', 'pk' => 'revcomment_rev', 'field' => 'revcomment_comment_id', 'joinPK' => 'rev_id', + 'stage' => MIGRATION_OLD, + 'deprecatedIn' => null, ], 'img_description' => [ 'table' => 'image_comment_temp', 'pk' => 'imgcomment_name', 'field' => 'imgcomment_description_id', 'joinPK' => 'img_name', + 'stage' => MIGRATION_WRITE_NEW, + 'deprecatedIn' => '1.32', ], ]; - /** - * Fields that formerly used $tempTables - * @var array Key is '$key', value is the MediaWiki version in which it was - * removed from $tempTables. - */ - protected static $formerTempTables = []; - /** * @since 1.30 * @deprecated in 1.31 @@ -174,9 +173,13 @@ class CommentStore { if ( $this->stage < MIGRATION_NEW ) { $fields["{$key}_old"] = $key; } - if ( isset( self::$tempTables[$key] ) ) { - $fields["{$key}_pk"] = self::$tempTables[$key]['joinPK']; - } else { + + $tempTableStage = isset( $this->tempTables[$key] ) + ? $this->tempTables[$key]['stage'] : MIGRATION_NEW; + if ( $tempTableStage < MIGRATION_NEW ) { + $fields["{$key}_pk"] = $this->tempTables[$key]['joinPK']; + } + if ( $tempTableStage > MIGRATION_OLD ) { $fields["{$key}_id"] = "{$key}_id"; } } @@ -213,12 +216,19 @@ class CommentStore { } else { $join = $this->stage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN'; - if ( isset( self::$tempTables[$key] ) ) { - $t = self::$tempTables[$key]; + $tempTableStage = isset( $this->tempTables[$key] ) + ? $this->tempTables[$key]['stage'] : MIGRATION_NEW; + if ( $tempTableStage < MIGRATION_NEW ) { + $t = $this->tempTables[$key]; $alias = "temp_$key"; $tables[$alias] = $t['table']; $joins[$alias] = [ $join, "{$alias}.{$t['pk']} = {$t['joinPK']}" ]; - $joinField = "{$alias}.{$t['field']}"; + if ( $tempTableStage === MIGRATION_OLD ) { + $joinField = "{$alias}.{$t['field']}"; + } else { + $joins[$alias][0] = 'LEFT JOIN'; + $joinField = "(CASE WHEN {$key}_id != 0 THEN {$key}_id ELSE {$alias}.{$t['field']} END)"; + } } else { $joinField = "{$key}_id"; } @@ -275,51 +285,48 @@ class CommentStore { } $data = null; } else { - if ( isset( self::$tempTables[$key] ) ) { - if ( array_key_exists( "{$key}_pk", $row ) ) { - if ( !$db ) { - throw new InvalidArgumentException( - "\$row does not contain fields needed for comment $key and getComment(), but " - . "does have fields for getCommentLegacy()" - ); - } - $t = self::$tempTables[$key]; - $id = $row["{$key}_pk"]; - $row2 = $db->selectRow( - [ $t['table'], 'comment' ], - [ 'comment_id', 'comment_text', 'comment_data' ], - [ $t['pk'] => $id ], - __METHOD__, - [], - [ 'comment' => [ 'JOIN', [ "comment_id = {$t['field']}" ] ] ] + $tempTableStage = isset( $this->tempTables[$key] ) + ? $this->tempTables[$key]['stage'] : MIGRATION_NEW; + $row2 = null; + if ( $tempTableStage > MIGRATION_OLD && array_key_exists( "{$key}_id", $row ) ) { + if ( !$db ) { + throw new InvalidArgumentException( + "\$row does not contain fields needed for comment $key and getComment(), but " + . "does have fields for getCommentLegacy()" ); - } elseif ( $fallback && isset( $row[$key] ) ) { - wfLogWarning( "Using deprecated fallback handling for comment $key" ); - $row2 = (object)[ 'comment_text' => $row[$key], 'comment_data' => null ]; - } else { - throw new InvalidArgumentException( "\$row does not contain fields needed for comment $key" ); } - } else { - if ( array_key_exists( "{$key}_id", $row ) ) { - if ( !$db ) { - throw new InvalidArgumentException( - "\$row does not contain fields needed for comment $key and getComment(), but " - . "does have fields for getCommentLegacy()" - ); - } - $id = $row["{$key}_id"]; - $row2 = $db->selectRow( - 'comment', - [ 'comment_id', 'comment_text', 'comment_data' ], - [ 'comment_id' => $id ], - __METHOD__ + $id = $row["{$key}_id"]; + $row2 = $db->selectRow( + 'comment', + [ 'comment_id', 'comment_text', 'comment_data' ], + [ 'comment_id' => $id ], + __METHOD__ + ); + } + if ( !$row2 && $tempTableStage < MIGRATION_NEW && array_key_exists( "{$key}_pk", $row ) ) { + if ( !$db ) { + throw new InvalidArgumentException( + "\$row does not contain fields needed for comment $key and getComment(), but " + . "does have fields for getCommentLegacy()" ); - } elseif ( $fallback && isset( $row[$key] ) ) { - wfLogWarning( "Using deprecated fallback handling for comment $key" ); - $row2 = (object)[ 'comment_text' => $row[$key], 'comment_data' => null ]; - } else { - throw new InvalidArgumentException( "\$row does not contain fields needed for comment $key" ); } + $t = $this->tempTables[$key]; + $id = $row["{$key}_pk"]; + $row2 = $db->selectRow( + [ $t['table'], 'comment' ], + [ 'comment_id', 'comment_text', 'comment_data' ], + [ $t['pk'] => $id ], + __METHOD__, + [], + [ 'comment' => [ 'JOIN', [ "comment_id = {$t['field']}" ] ] ] + ); + } + if ( $row2 === null && $fallback && isset( $row[$key] ) ) { + wfLogWarning( "Using deprecated fallback handling for comment $key" ); + $row2 = (object)[ 'comment_text' => $row[$key], 'comment_data' => null ]; + } + if ( $row2 === null ) { + throw new InvalidArgumentException( "\$row does not contain fields needed for comment $key" ); } if ( $row2 ) { @@ -525,8 +532,10 @@ class CommentStore { } if ( $this->stage >= MIGRATION_WRITE_BOTH ) { - if ( isset( self::$tempTables[$key] ) ) { - $t = self::$tempTables[$key]; + $tempTableStage = isset( $this->tempTables[$key] ) + ? $this->tempTables[$key]['stage'] : MIGRATION_NEW; + if ( $tempTableStage <= MIGRATION_WRITE_BOTH ) { + $t = $this->tempTables[$key]; $func = __METHOD__; $commentId = $comment->id; $callback = function ( $id ) use ( $dbw, $commentId, $t, $func ) { @@ -539,7 +548,8 @@ class CommentStore { $func ); }; - } else { + } + if ( $tempTableStage >= MIGRATION_WRITE_BOTH ) { $fields["{$key}_id"] = $comment->id; } } @@ -575,7 +585,9 @@ class CommentStore { // @codeCoverageIgnoreEnd } - if ( isset( self::$tempTables[$key] ) ) { + $tempTableStage = isset( $this->tempTables[$key] ) + ? $this->tempTables[$key]['stage'] : MIGRATION_NEW; + if ( $tempTableStage < MIGRATION_WRITE_NEW ) { throw new InvalidArgumentException( "Must use insertWithTempTable() for $key" ); } @@ -617,10 +629,10 @@ class CommentStore { // @codeCoverageIgnoreEnd } - if ( isset( self::$formerTempTables[$key] ) ) { - wfDeprecated( __METHOD__ . " for $key", self::$formerTempTables[$key] ); - } elseif ( !isset( self::$tempTables[$key] ) ) { + if ( !isset( $this->tempTables[$key] ) ) { throw new InvalidArgumentException( "Must use insert() for $key" ); + } elseif ( isset( $this->tempTables[$key]['deprecatedIn'] ) ) { + wfDeprecated( __METHOD__ . " for $key", $this->tempTables[$key]['deprecatedIn'] ); } list( $fields, $callback ) = $this->insertInternal( $dbw, $key, $comment, $data ); diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 36a44dcf7c..ec01869c32 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1470,8 +1470,7 @@ class LocalFile extends File { # This avoids race conditions by locking the row until the commit, and also # doesn't deadlock. SELECT FOR UPDATE causes a deadlock for every race condition. $commentStore = MediaWikiServices::getInstance()->getCommentStore(); - list( $commentFields, $commentCallback ) = - $commentStore->insertWithTempTable( $dbw, 'img_description', $comment ); + $commentFields = $commentStore->insert( $dbw, 'img_description', $comment ); $actorMigration = ActorMigration::newMigration(); $actorFields = $actorMigration->getInsertValues( $dbw, 'img_user', $user ); $dbw->insert( 'image', @@ -1543,7 +1542,8 @@ class LocalFile extends File { } if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { $tables[] = 'image_comment_temp'; - $fields['oi_description_id'] = 'imgcomment_description_id'; + $fields['oi_description_id'] = + 'CASE WHEN img_description_id = 0 THEN imgcomment_description_id ELSE img_description_id END'; $joins['image_comment_temp'] = [ $wgCommentTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', [ 'imgcomment_name = img_name' ] @@ -1559,16 +1559,17 @@ class LocalFile extends File { $res = $dbw->select( [ 'image', 'image_comment_temp' ], [ 'img_name', 'img_description' ], - [ 'img_name' => $this->getName(), 'imgcomment_name' => null ], + [ + 'img_name' => $this->getName(), + 'imgcomment_name' => null, + 'img_description_id' => 0, + ], __METHOD__, [], [ 'image_comment_temp' => [ 'LEFT JOIN', [ 'imgcomment_name = img_name' ] ] ] ); foreach ( $res as $row ) { - list( , $callback ) = $commentStore->insertWithTempTable( - $dbw, 'img_description', $row->img_description - ); - $callback( $row->img_name ); + $commentStore->insert( $dbw, 'img_description', $row->img_description ); } } @@ -1630,11 +1631,10 @@ class LocalFile extends File { __METHOD__ ); if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { - // So $commentCallback can insert the new row + // Clear deprecated table row $dbw->delete( 'image_comment_temp', [ 'imgcomment_name' => $this->getName() ], __METHOD__ ); } } - $commentCallback( $this->getName() ); $descTitle = $this->getTitle(); $descId = $descTitle->getArticleID(); @@ -2538,7 +2538,8 @@ class LocalFileDeleteBatch { } if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { $tables[] = 'image_comment_temp'; - $fields['fa_description_id'] = 'imgcomment_description_id'; + $fields['fa_description_id'] = + 'CASE WHEN img_description_id = 0 THEN imgcomment_description_id ELSE img_description_id END'; $joins['image_comment_temp'] = [ $wgCommentTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', [ 'imgcomment_name = img_name' ] @@ -2554,16 +2555,17 @@ class LocalFileDeleteBatch { $res = $dbw->select( [ 'image', 'image_comment_temp' ], [ 'img_name', 'img_description' ], - [ 'img_name' => $this->file->getName(), 'imgcomment_name' => null ], + [ + 'img_name' => $this->file->getName(), + 'imgcomment_name' => null, + 'img_description_id' => 0, + ], __METHOD__, [], [ 'image_comment_temp' => [ 'LEFT JOIN', [ 'imgcomment_name = img_name' ] ] ] ); foreach ( $res as $row ) { - list( , $callback ) = $commentStore->insertWithTempTable( - $dbw, 'img_description', $row->img_description - ); - $callback( $row->img_name ); + $commentStore->insert( $dbw, 'img_description', $row->img_description ); } } @@ -2670,6 +2672,7 @@ class LocalFileDeleteBatch { if ( $deleteCurrent ) { $dbw->delete( 'image', [ 'img_name' => $this->file->getName() ], __METHOD__ ); if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { + // Clear deprecated table row $dbw->delete( 'image_comment_temp', [ 'imgcomment_name' => $this->file->getName() ], __METHOD__ ); @@ -2937,8 +2940,7 @@ class LocalFileRestoreBatch { if ( $first && !$exists ) { // This revision will be published as the new current version $destRel = $this->file->getRel(); - list( $commentFields, $commentCallback ) = - $commentStore->insertWithTempTable( $dbw, 'img_description', $comment ); + $commentFields = $commentStore->insert( $dbw, 'img_description', $comment ); $actorFields = $actorMigration->getInsertValues( $dbw, 'img_user', $user ); $insertCurrent = [ 'img_name' => $row->fa_name, @@ -3050,7 +3052,6 @@ class LocalFileRestoreBatch { // This is not ideal, which is why it's important to lock the image row. if ( $insertCurrent ) { $dbw->insert( 'image', $insertCurrent, __METHOD__ ); - $commentCallback( $insertCurrent['img_name'] ); } if ( $insertBatch ) { diff --git a/includes/installer/DatabaseUpdater.php b/includes/installer/DatabaseUpdater.php index 82ccce2129..8634f89273 100644 --- a/includes/installer/DatabaseUpdater.php +++ b/includes/installer/DatabaseUpdater.php @@ -1278,6 +1278,22 @@ abstract class DatabaseUpdater { } } + /** + * Merge `image_comment_temp` into the `image` table + * @since 1.32 + */ + protected function migrateImageCommentTemp() { + global $wgCommentTableSchemaMigrationStage; + if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { + $this->output( "Merging image_comment_temp into the image table\n" ); + $task = $this->maintenance->runChild( + MigrateImageCommentTemp::class, 'migrateImageCommentTemp.php' + ); + $ok = $task->execute(); + $this->output( $ok ? "done.\n" : "errors were encountered.\n" ); + } + } + /** * Migrate actors to the new 'actor' table * @since 1.31 diff --git a/includes/installer/MssqlUpdater.php b/includes/installer/MssqlUpdater.php index 17b1d7e533..4a12d4cd99 100644 --- a/includes/installer/MssqlUpdater.php +++ b/includes/installer/MssqlUpdater.php @@ -151,6 +151,7 @@ class MssqlUpdater extends DatabaseUpdater { 'patch-change_tag-change_tag_rc_tag_id.sql' ], [ 'addField', 'ipblocks', 'ipb_sitewide', 'patch-ipb_sitewide.sql' ], [ 'addTable', 'ipblocks_restrictions', 'patch-ipblocks_restrictions-table.sql' ], + [ 'migrateImageCommentTemp' ], ]; } diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php index 6430ecee5b..a9ca28614d 100644 --- a/includes/installer/MysqlUpdater.php +++ b/includes/installer/MysqlUpdater.php @@ -371,6 +371,7 @@ class MysqlUpdater extends DatabaseUpdater { 'patch-change_tag-change_tag_rc_tag_id.sql' ], [ 'addField', 'ipblocks', 'ipb_sitewide', 'patch-ipb_sitewide.sql' ], [ 'addTable', 'ipblocks_restrictions', 'patch-ipblocks_restrictions-table.sql' ], + [ 'migrateImageCommentTemp' ], ]; } diff --git a/includes/installer/OracleUpdater.php b/includes/installer/OracleUpdater.php index 5833299e3f..78b53aab49 100644 --- a/includes/installer/OracleUpdater.php +++ b/includes/installer/OracleUpdater.php @@ -162,6 +162,7 @@ class OracleUpdater extends DatabaseUpdater { 'patch-change_tag-change_tag_rc_tag_id.sql' ], [ 'addField', 'ipblocks', 'ipb_sitewide', 'patch-ipb_sitewide.sql' ], [ 'addTable', 'ipblocks_restrictions', 'patch-ipblocks_restrictions-table.sql' ], + [ 'migrateImageCommentTemp' ], // KEEP THIS AT THE BOTTOM!! [ 'doRebuildDuplicateFunction' ], diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index 8fd5370367..71c1a52286 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -597,6 +597,7 @@ class PostgresUpdater extends DatabaseUpdater { 'patch-change_tag-change_tag_rc_tag_id.sql' ], [ 'addPgField', 'ipblocks', 'ipb_sitewide', 'SMALLINT NOT NULL DEFAULT 1' ], [ 'addTable', 'ipblocks_restrictions', 'patch-ipblocks_restrictions-table.sql' ], + [ 'migrateImageCommentTemp' ], ]; } diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php index eb2d8c2040..cba6a8a132 100644 --- a/includes/installer/SqliteUpdater.php +++ b/includes/installer/SqliteUpdater.php @@ -236,6 +236,7 @@ class SqliteUpdater extends DatabaseUpdater { 'patch-change_tag-change_tag_rc_tag_id.sql' ], [ 'addField', 'ipblocks', 'ipb_sitewide', 'patch-ipb_sitewide.sql' ], [ 'addTable', 'ipblocks_restrictions', 'patch-ipblocks_restrictions-table.sql' ], + [ 'migrateImageCommentTemp' ], ]; } diff --git a/maintenance/migrateComments.php b/maintenance/migrateComments.php index cdecab03c3..555f2d13f0 100644 --- a/maintenance/migrateComments.php +++ b/maintenance/migrateComments.php @@ -61,9 +61,7 @@ class MigrateComments extends LoggedUpdateMaintenance { ); $this->migrate( 'archive', 'ar_id', 'ar_comment' ); $this->migrate( 'ipblocks', 'ipb_id', 'ipb_reason' ); - $this->migrateToTemp( - 'image', 'img_name', 'img_description', 'imgcomment_name', 'imgcomment_description_id' - ); + $this->migrate( 'image', 'img_name', 'img_description' ); $this->migrate( 'oldimage', [ 'oi_name', 'oi_timestamp' ], 'oi_description' ); $this->migrate( 'filearchive', 'fa_id', 'fa_deleted_reason' ); $this->migrate( 'filearchive', 'fa_id', 'fa_description' ); diff --git a/maintenance/migrateImageCommentTemp.php b/maintenance/migrateImageCommentTemp.php new file mode 100644 index 0000000000..6db9d06d1e --- /dev/null +++ b/maintenance/migrateImageCommentTemp.php @@ -0,0 +1,138 @@ +addDescription( + 'Merges image_comment_temp into the image table' + ); + } + + /** + * Sets whether a run of this maintenance script has the force parameter set + * @param bool $forced + */ + public function setForce( $forced = true ) { + $this->mOptions['force'] = $forced; + } + + protected function getUpdateKey() { + return __CLASS__; + } + + protected function doDBUpdates() { + $batchSize = $this->getBatchSize(); + + $dbw = $this->getDB( DB_MASTER ); + if ( !$dbw->fieldExists( 'image', 'img_description_id', __METHOD__ ) ) { + $this->output( "Run update.php to create img_description_id.\n" ); + return false; + } + if ( !$dbw->tableExists( 'image_comment_temp', __METHOD__ ) ) { + $this->output( "image_comment_temp does not exist, so nothing to do.\n" ); + return true; + } + + $this->output( "Merging image_comment_temp into the image table...\n" ); + $conds = []; + $updated = 0; + $deleted = 0; + while ( true ) { + $this->beginTransaction( $dbw, __METHOD__ ); + + $res = $dbw->select( + [ 'image_comment_temp', 'image' ], + [ + 'name' => 'imgcomment_name', + 'oldid' => 'imgcomment_description_id', + 'newid' => 'img_description_id', + ], + $conds, + __METHOD__, + [ 'LIMIT' => $batchSize, 'ORDER BY' => [ 'name' ] ], + [ 'image' => [ 'JOIN', 'img_name = imgcomment_name' ] ] + ); + $numRows = $res->numRows(); + + $toDelete = []; + $last = null; + foreach ( $res as $row ) { + $last = $row->name; + $toDelete[] = $row->name; + if ( !$row->newid ) { + $dbw->update( + 'image', + [ 'img_description_id' => $row->oldid ], + [ 'img_name' => $row->name ], + __METHOD__ + ); + $updated++; + } elseif ( $row->newid !== $row->oldid ) { + $this->error( + "Image \"$row->name\" has img_description_id = $row->newid and " + . "imgcomment_description_id = $row->oldid. Ignoring the latter." + ); + } + } + if ( $toDelete ) { + $dbw->delete( 'image_comment_temp', [ 'imgcomment_name' => $toDelete ], __METHOD__ ); + $deleted += count( $toDelete ); + } + + $this->commitTransaction( $dbw, __METHOD__ ); + + if ( $numRows < $batchSize ) { + // We must have reached the end + break; + } + + $this->output( "... $last, updated $updated, deleted $deleted\n" ); + $conds = [ 'imgcomment_name > ' . $dbw->addQuotes( $last ) ]; + } + + // This should be 0, so it should be very fast + $count = (int)$dbw->selectField( 'image_comment_temp', 'COUNT(*)', [], __METHOD__ ); + if ( $count !== 0 ) { + $this->error( "Ignoring $count orphaned image_comment_temp row(s)." ); + } + + $this->output( + "Completed merge of image_comment_temp into the image table, " + . "$updated image rows updated, $deleted image_comment_temp rows deleted.\n" + ); + + return true; + } +} + +$maintClass = MigrateImageCommentTemp::class; +require_once RUN_MAINTENANCE_IF_MAIN; diff --git a/tests/phpunit/includes/CommentStoreTest.php b/tests/phpunit/includes/CommentStoreTest.php index 0c5d8e3143..bc1f10140b 100644 --- a/tests/phpunit/includes/CommentStoreTest.php +++ b/tests/phpunit/includes/CommentStoreTest.php @@ -115,15 +115,26 @@ class CommentStoreTest extends MediaWikiLangTestCase { ], 'Image, write-both' => [ MIGRATION_WRITE_BOTH, 'img_description', - [ 'img_description_old' => 'img_description', 'img_description_pk' => 'img_name' ], + [ + 'img_description_old' => 'img_description', + 'img_description_pk' => 'img_name', + 'img_description_id' => 'img_description_id' + ], ], 'Image, write-new' => [ MIGRATION_WRITE_NEW, 'img_description', - [ 'img_description_old' => 'img_description', 'img_description_pk' => 'img_name' ], + [ + 'img_description_old' => 'img_description', + 'img_description_pk' => 'img_name', + 'img_description_id' => 'img_description_id' + ], ], 'Image, new' => [ MIGRATION_NEW, 'img_description', - [ 'img_description_pk' => 'img_name' ], + [ + 'img_description_pk' => 'img_name', + 'img_description_id' => 'img_description_id' + ], ], ]; } @@ -296,7 +307,9 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'joins' => [ 'temp_img_description' => [ 'LEFT JOIN', 'temp_img_description.imgcomment_name = img_name' ], 'comment_img_description' => [ 'LEFT JOIN', - 'comment_img_description.comment_id = temp_img_description.imgcomment_description_id' ], + // phpcs:ignore Generic.Files.LineLength + 'comment_img_description.comment_id = (CASE WHEN img_description_id != 0 THEN img_description_id ELSE temp_img_description.imgcomment_description_id END)', + ], ], ], ], @@ -314,7 +327,9 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'joins' => [ 'temp_img_description' => [ 'LEFT JOIN', 'temp_img_description.imgcomment_name = img_name' ], 'comment_img_description' => [ 'LEFT JOIN', - 'comment_img_description.comment_id = temp_img_description.imgcomment_description_id' ], + // phpcs:ignore Generic.Files.LineLength + 'comment_img_description.comment_id = (CASE WHEN img_description_id != 0 THEN img_description_id ELSE temp_img_description.imgcomment_description_id END)', + ], ], ], ], @@ -330,9 +345,11 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'img_description_cid' => 'comment_img_description.comment_id', ], 'joins' => [ - 'temp_img_description' => [ 'JOIN', 'temp_img_description.imgcomment_name = img_name' ], + 'temp_img_description' => [ 'LEFT JOIN', 'temp_img_description.imgcomment_name = img_name' ], 'comment_img_description' => [ 'JOIN', - 'comment_img_description.comment_id = temp_img_description.imgcomment_description_id' ], + // phpcs:ignore Generic.Files.LineLength + 'comment_img_description.comment_id = (CASE WHEN img_description_id != 0 THEN img_description_id ELSE temp_img_description.imgcomment_description_id END)', + ], ], ], ], @@ -736,11 +753,14 @@ class CommentStoreTest extends MediaWikiLangTestCase { * @param int $stage */ public function testInsertWithTempTableDeprecated( $stage ) { - $wrap = TestingAccessWrapper::newFromClass( CommentStore::class ); - $wrap->formerTempTables += [ 'ipb_reason' => '1.30' ]; + $store = $this->makeStore( $stage ); + $wrap = TestingAccessWrapper::newFromObject( $store ); + $wrap->tempTables += [ 'ipb_reason' => [ + 'stage' => MIGRATION_NEW, + 'deprecatedIn' => '1.30', + ] ]; $this->hideDeprecated( 'CommentStore::insertWithTempTable for ipb_reason' ); - $store = $this->makeStore( $stage ); list( $fields, $callback ) = $store->insertWithTempTable( $this->db, 'ipb_reason', 'foo' ); $this->assertTrue( is_callable( $callback ) ); } -- 2.20.1