Migrate image descriptions from image_comment_temp
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 7 Mar 2018 16:40:56 +0000 (11:40 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Tue, 2 Oct 2018 14:30:00 +0000 (10:30 -0400)
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

13 files changed:
RELEASE-NOTES-1.32
autoload.php
includes/CommentStore.php
includes/filerepo/file/LocalFile.php
includes/installer/DatabaseUpdater.php
includes/installer/MssqlUpdater.php
includes/installer/MysqlUpdater.php
includes/installer/OracleUpdater.php
includes/installer/PostgresUpdater.php
includes/installer/SqliteUpdater.php
maintenance/migrateComments.php
maintenance/migrateImageCommentTemp.php [new file with mode: 0644]
tests/phpunit/includes/CommentStoreTest.php

index 6c200d9..934d07b 100644 (file)
@@ -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
   <https://www.mediawiki.org/wiki/Multi-Content_Revisions> 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
index 33ee128..a0f5056 100644 (file)
@@ -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',
index 9969b78..1be7951 100644 (file)
@@ -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 );
index 36a44dc..ec01869 100644 (file)
@@ -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 ) {
index 82ccce2..8634f89 100644 (file)
@@ -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
index 17b1d7e..4a12d4c 100644 (file)
@@ -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' ],
                ];
        }
 
index 6430ece..a9ca286 100644 (file)
@@ -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' ],
                ];
        }
 
index 5833299..78b53aa 100644 (file)
@@ -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' ],
index 8fd5370..71c1a52 100644 (file)
@@ -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' ],
                ];
        }
 
index eb2d8c2..cba6a8a 100644 (file)
@@ -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' ],
                ];
        }
 
index cdecab0..555f2d1 100644 (file)
@@ -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 (file)
index 0000000..6db9d06
--- /dev/null
@@ -0,0 +1,138 @@
+<?php
+/**
+ * Merge `image_comment_temp` into the `image` table
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Maintenance
+ */
+
+require_once __DIR__ . '/Maintenance.php';
+
+/**
+ * Maintenance script that merges `image_comment_temp` into the `image` table
+ *
+ * @ingroup Maintenance
+ * @since 1.32
+ */
+class MigrateImageCommentTemp extends LoggedUpdateMaintenance {
+       public function __construct() {
+               parent::__construct();
+               $this->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;
index 0c5d8e3..bc1f101 100644 (file)
@@ -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 ) );
        }