From 11cf01dd9a8512ad4d9bded43cf22ebd38af8818 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 6 Jun 2017 13:39:14 -0400 Subject: [PATCH] Add `comment` table and code to start using it A subsequent patch will remove the old columns. Bug: T166732 Change-Id: Ic3a434c061ed6e443ea072bc62dda09acbeeed7f --- RELEASE-NOTES-1.30 | 5 + autoload.php | 3 + includes/Block.php | 36 +- includes/CommentStore.php | 567 ++++++++++++++++ includes/CommentStoreComment.php | 57 ++ includes/DefaultSettings.php | 7 + includes/EditPage.php | 16 +- includes/FeedUtils.php | 7 +- includes/Revision.php | 24 +- includes/Title.php | 21 +- includes/WatchedItemQueryService.php | 33 +- includes/api/ApiQueryAllUsers.php | 3 +- includes/api/ApiQueryBase.php | 5 +- includes/api/ApiQueryBlocks.php | 11 +- includes/api/ApiQueryDeletedrevs.php | 14 +- includes/api/ApiQueryFilearchive.php | 13 +- includes/api/ApiQueryLogEvents.php | 18 +- includes/api/ApiQueryProtectedTitles.php | 15 +- includes/api/ApiQueryRecentChanges.php | 20 +- includes/api/ApiQueryUserContributions.php | 19 +- includes/api/ApiQueryUsers.php | 3 +- includes/api/ApiQueryWatchlist.php | 15 +- includes/changes/RecentChange.php | 43 +- includes/export/WikiExporter.php | 23 +- includes/export/XmlDumpWriter.php | 14 +- includes/filerepo/file/ArchivedFile.php | 9 +- includes/filerepo/file/LocalFile.php | 282 +++++--- includes/filerepo/file/OldLocalFile.php | 9 +- includes/import/WikiImporter.php | 5 +- includes/import/WikiRevision.php | 9 +- includes/installer/DatabaseUpdater.php | 21 + includes/installer/MysqlUpdater.php | 2 + includes/installer/PostgresUpdater.php | 31 +- includes/installer/SqliteUpdater.php | 2 + includes/logging/LogEntry.php | 18 +- includes/logging/LogPage.php | 2 +- includes/page/PageArchive.php | 48 +- includes/page/WikiPage.php | 26 +- .../rcfeed/IRCColourfulRCFeedFormatter.php | 4 +- includes/revisiondelete/RevDelLogItem.php | 5 +- includes/revisiondelete/RevDelLogList.php | 12 +- includes/specials/SpecialNewpages.php | 2 +- includes/specials/pagers/BlockListPager.php | 10 +- .../specials/pagers/DeletedContribsPager.php | 13 +- includes/specials/pagers/ImageListPager.php | 13 +- includes/specials/pagers/NewPagesPager.php | 10 +- .../specials/pagers/ProtectedPagesPager.php | 10 +- maintenance/archives/patch-comment-table.sql | 59 ++ maintenance/migrateComments.php | 291 +++++++++ maintenance/orphans.php | 25 +- .../postgres/archives/patch-comment-table.sql | 27 + maintenance/postgres/tables.sql | 52 +- maintenance/rebuildrecentchanges.php | 38 +- .../sqlite/archives/patch-comment-table.sql | 332 ++++++++++ maintenance/tables.sql | 109 +++- tests/phpunit/MediaWikiTestCase.php | 8 +- tests/phpunit/includes/CommentStoreTest.php | 614 ++++++++++++++++++ .../phpunit/includes/RevisionStorageTest.php | 8 +- .../WatchedItemQueryServiceUnitTest.php | 188 +++++- .../api/ApiQueryWatchlistIntegrationTest.php | 2 + .../includes/changes/RecentChangeTest.php | 25 + .../changes/TestRecentChangesHelper.php | 4 + .../includes/deferred/LinksUpdateTest.php | 37 +- .../includes/logging/LogFormatterTestCase.php | 3 +- tests/phpunit/includes/page/WikiPageTest.php | 81 +++ 65 files changed, 3096 insertions(+), 342 deletions(-) create mode 100644 includes/CommentStore.php create mode 100644 includes/CommentStoreComment.php create mode 100644 maintenance/archives/patch-comment-table.sql create mode 100644 maintenance/migrateComments.php create mode 100644 maintenance/postgres/archives/patch-comment-table.sql create mode 100644 maintenance/sqlite/archives/patch-comment-table.sql create mode 100644 tests/phpunit/includes/CommentStoreTest.php diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30 index c7270ee669..c4c56e833f 100644 --- a/RELEASE-NOTES-1.30 +++ b/RELEASE-NOTES-1.30 @@ -61,6 +61,11 @@ section). just been unwatched. * Added $wgParserTestMediaHandlers, where mock media handlers can be passed to MediaHandlerFactory for parser tests. +* Edit summaries, block reasons, and other "comments" are now stored in a + separate database table. Use the CommentFormatter class to access them. +** This is currently gated by $wgCommentTableSchemaMigrationStage. Most wikis + can set this to MIGRATION_NEW and run maintenance/migrateComments.php as + soon as any necessary extensions are updated. === External library changes in 1.30 === diff --git a/autoload.php b/autoload.php index 806238726f..3a2ae10007 100644 --- a/autoload.php +++ b/autoload.php @@ -276,6 +276,8 @@ $wgAutoloadLocalClasses = [ 'CollationFa' => __DIR__ . '/includes/collation/CollationFa.php', 'CommandLineInc' => __DIR__ . '/maintenance/commandLine.inc', 'CommandLineInstaller' => __DIR__ . '/maintenance/install.php', + 'CommentStore' => __DIR__ . '/includes/CommentStore.php', + 'CommentStoreComment' => __DIR__ . '/includes/CommentStoreComment.php', 'CompareParserCache' => __DIR__ . '/maintenance/compareParserCache.php', 'CompareParsers' => __DIR__ . '/maintenance/compareParsers.php', 'ComposerHookHandler' => __DIR__ . '/includes/composer/ComposerHookHandler.php', @@ -982,6 +984,7 @@ $wgAutoloadLocalClasses = [ 'MessageContent' => __DIR__ . '/includes/content/MessageContent.php', 'MessageLocalizer' => __DIR__ . '/languages/MessageLocalizer.php', 'MessageSpecifier' => __DIR__ . '/includes/libs/MessageSpecifier.php', + 'MigrateComments' => __DIR__ . '/maintenance/migrateComments.php', 'MigrateFileRepoLayout' => __DIR__ . '/maintenance/migrateFileRepoLayout.php', 'MigrateUserGroup' => __DIR__ . '/maintenance/migrateUserGroup.php', 'MimeAnalyzer' => __DIR__ . '/includes/libs/mime/MimeAnalyzer.php', diff --git a/includes/Block.php b/includes/Block.php index 05e97b98d9..0b17e93f3f 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -199,6 +199,8 @@ class Block { /** * Return the list of ipblocks fields that should be selected to create * a new block. + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ public static function selectFields() { @@ -207,7 +209,6 @@ class Block { 'ipb_address', 'ipb_by', 'ipb_by_text', - 'ipb_reason', 'ipb_timestamp', 'ipb_auto', 'ipb_anon_only', @@ -218,7 +219,7 @@ class Block { 'ipb_block_email', 'ipb_allow_usertalk', 'ipb_parent_block_id', - ]; + ] + CommentStore::newKey( 'ipb_reason' )->getFields(); } /** @@ -411,7 +412,6 @@ class Block { $this->setBlocker( $row->ipb_by_text ); } - $this->mReason = $row->ipb_reason; $this->mTimestamp = wfTimestamp( TS_MW, $row->ipb_timestamp ); $this->mAuto = $row->ipb_auto; $this->mHideName = $row->ipb_deleted; @@ -419,7 +419,11 @@ class Block { $this->mParentBlockId = $row->ipb_parent_block_id; // I wish I didn't have to do this - $this->mExpiry = wfGetDB( DB_REPLICA )->decodeExpiry( $row->ipb_expiry ); + $db = wfGetDB( DB_REPLICA ); + $this->mExpiry = $db->decodeExpiry( $row->ipb_expiry ); + $this->mReason = CommentStore::newKey( 'ipb_reason' ) + // Legacy because $row probably came from self::selectFields() + ->getCommentLegacy( $db, $row )->text; $this->isHardblock( !$row->ipb_anon_only ); $this->isAutoblocking( $row->ipb_enable_autoblock ); @@ -488,7 +492,7 @@ class Block { self::purgeExpired(); } - $row = $this->getDatabaseArray(); + $row = $this->getDatabaseArray( $dbw ); $row['ipb_id'] = $dbw->nextSequenceValue( "ipblocks_ipb_id_seq" ); $dbw->insert( 'ipblocks', $row, __METHOD__, [ 'IGNORE' ] ); @@ -558,7 +562,7 @@ class Block { // update corresponding autoblock(s) (T50813) $dbw->update( 'ipblocks', - $this->getAutoblockUpdateArray(), + $this->getAutoblockUpdateArray( $dbw ), [ 'ipb_parent_block_id' => $this->getId() ], __METHOD__ ); @@ -583,14 +587,11 @@ class Block { /** * Get an array suitable for passing to $dbw->insert() or $dbw->update() - * @param IDatabase $db + * @param IDatabase $dbw * @return array */ - protected function getDatabaseArray( $db = null ) { - if ( !$db ) { - $db = wfGetDB( DB_REPLICA ); - } - $expiry = $db->encodeExpiry( $this->mExpiry ); + protected function getDatabaseArray( IDatabase $dbw ) { + $expiry = $dbw->encodeExpiry( $this->mExpiry ); if ( $this->forcedTargetID ) { $uid = $this->forcedTargetID; @@ -603,8 +604,7 @@ class Block { 'ipb_user' => $uid, 'ipb_by' => $this->getBy(), 'ipb_by_text' => $this->getByName(), - 'ipb_reason' => $this->mReason, - 'ipb_timestamp' => $db->timestamp( $this->mTimestamp ), + 'ipb_timestamp' => $dbw->timestamp( $this->mTimestamp ), 'ipb_auto' => $this->mAuto, 'ipb_anon_only' => !$this->isHardblock(), 'ipb_create_account' => $this->prevents( 'createaccount' ), @@ -616,23 +616,23 @@ class Block { 'ipb_block_email' => $this->prevents( 'sendemail' ), 'ipb_allow_usertalk' => !$this->prevents( 'editownusertalk' ), 'ipb_parent_block_id' => $this->mParentBlockId - ]; + ] + CommentStore::newKey( 'ipb_reason' )->insert( $dbw, $this->mReason ); return $a; } /** + * @param IDatabase $dbw * @return array */ - protected function getAutoblockUpdateArray() { + protected function getAutoblockUpdateArray( IDatabase $dbw ) { return [ 'ipb_by' => $this->getBy(), 'ipb_by_text' => $this->getByName(), - 'ipb_reason' => $this->mReason, 'ipb_create_account' => $this->prevents( 'createaccount' ), 'ipb_deleted' => (int)$this->mHideName, // typecast required for SQLite 'ipb_allow_usertalk' => !$this->prevents( 'editownusertalk' ), - ]; + ] + CommentStore::newKey( 'ipb_reason' )->insert( $dbw, $this->mReason ); } /** diff --git a/includes/CommentStore.php b/includes/CommentStore.php new file mode 100644 index 0000000000..0c86c1e871 --- /dev/null +++ b/includes/CommentStore.php @@ -0,0 +1,567 @@ + [ + 'table' => 'revision_comment_temp', + 'pk' => 'revcomment_rev', + 'field' => 'revcomment_comment_id', + 'joinPK' => 'rev_id', + ], + 'img_description' => [ + 'table' => 'image_comment_temp', + 'pk' => 'imgcomment_name', + 'field' => 'imgcomment_description_id', + 'joinPK' => 'img_name', + ], + ]; + + /** + * 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 = []; + + /** @var string */ + protected $key; + + /** @var int One of the MIGRATION_* constants */ + protected $stage; + + /** @var array|null Cache for `self::getJoin()` */ + protected $joinCache = null; + + /** + * @param string $key A key such as "rev_comment" identifying the comment + * field being fetched. + */ + public function __construct( $key ) { + global $wgCommentTableSchemaMigrationStage; + + $this->key = $key; + $this->stage = $wgCommentTableSchemaMigrationStage; + } + + /** + * Static constructor for easier chaining + * @param string $key A key such as "rev_comment" identifying the comment + * field being fetched. + * @return CommentStore + */ + public static function newKey( $key ) { + return new CommentStore( $key ); + } + + /** + * Get SELECT fields for the comment key + * + * Each resulting row should be passed to `self::getCommentLegacy()` to get the + * actual comment. + * + * @note Use of this method may require a subsequent database query to + * actually fetch the comment. If possible, use `self::getJoin()` instead. + * @return string[] to include in the `$vars` to `IDatabase->select()`. All + * fields are aliased, so `+` is safe to use. + */ + public function getFields() { + $fields = []; + if ( $this->stage === MIGRATION_OLD ) { + $fields["{$this->key}_text"] = $this->key; + $fields["{$this->key}_data"] = 'NULL'; + $fields["{$this->key}_cid"] = 'NULL'; + } else { + if ( $this->stage < MIGRATION_NEW ) { + $fields["{$this->key}_old"] = $this->key; + } + if ( isset( self::$tempTables[$this->key] ) ) { + $fields["{$this->key}_pk"] = self::$tempTables[$this->key]['joinPK']; + } else { + $fields["{$this->key}_id"] = "{$this->key}_id"; + } + } + return $fields; + } + + /** + * Get SELECT fields and joins for the comment key + * + * Each resulting row should be passed to `self::getComment()` to get the + * actual comment. + * + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + * All tables, fields, and joins are aliased, so `+` is safe to use. + */ + public function getJoin() { + if ( $this->joinCache === null ) { + $tables = []; + $fields = []; + $joins = []; + + if ( $this->stage === MIGRATION_OLD ) { + $fields["{$this->key}_text"] = $this->key; + $fields["{$this->key}_data"] = 'NULL'; + $fields["{$this->key}_cid"] = 'NULL'; + } else { + $join = $this->stage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN'; + + if ( isset( self::$tempTables[$this->key] ) ) { + $t = self::$tempTables[$this->key]; + $alias = "temp_$this->key"; + $tables[$alias] = $t['table']; + $joins[$alias] = [ $join, "{$alias}.{$t['pk']} = {$t['joinPK']}" ]; + $joinField = "{$alias}.{$t['field']}"; + } else { + $joinField = "{$this->key}_id"; + } + + $alias = "comment_$this->key"; + $tables[$alias] = 'comment'; + $joins[$alias] = [ $join, "{$alias}.comment_id = {$joinField}" ]; + + if ( $this->stage === MIGRATION_NEW ) { + $fields["{$this->key}_text"] = "{$alias}.comment_text"; + } else { + $fields["{$this->key}_text"] = "COALESCE( {$alias}.comment_text, $this->key )"; + } + $fields["{$this->key}_data"] = "{$alias}.comment_data"; + $fields["{$this->key}_cid"] = "{$alias}.comment_id"; + } + + $this->joinCache = [ + 'tables' => $tables, + 'fields' => $fields, + 'joins' => $joins, + ]; + } + + return $this->joinCache; + } + + /** + * Extract the comment from a row + * + * Shared implementation for getComment() and getCommentLegacy() + * + * @param IDatabase|null $db Database handle for getCommentLegacy(), or null for getComment() + * @param object|array $row + * @param bool $fallback + * @return CommentStoreComment + */ + private function getCommentInternal( IDatabase $db = null, $row, $fallback = false ) { + $key = $this->key; + $row = (array)$row; + if ( array_key_exists( "{$key}_text", $row ) && array_key_exists( "{$key}_data", $row ) ) { + $cid = isset( $row["{$key}_cid"] ) ? $row["{$key}_cid"] : null; + $text = $row["{$key}_text"]; + $data = $row["{$key}_data"]; + } elseif ( $this->stage === MIGRATION_OLD ) { + $cid = null; + if ( $fallback && isset( $row[$key] ) ) { + wfLogWarning( "Using deprecated fallback handling for comment $key" ); + $text = $row[$key]; + } else { + wfLogWarning( "Missing {$key}_text and {$key}_data fields in row with MIGRATION_OLD" ); + $text = ''; + } + $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']}" ] ] ] + ); + } 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__ + ); + } 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" ); + } + } + + if ( $row2 ) { + $cid = $row2->comment_id; + $text = $row2->comment_text; + $data = $row2->comment_data; + } elseif ( $this->stage < MIGRATION_NEW && array_key_exists( "{$key}_old", $row ) ) { + $cid = null; + $text = $row["{$key}_old"]; + $data = null; + } else { + // @codeCoverageIgnoreStart + wfLogWarning( "Missing comment row for $key, id=$id" ); + $cid = null; + $text = ''; + $data = null; + // @codeCoverageIgnoreEnd + } + } + + $msg = null; + if ( $data !== null ) { + $data = FormatJson::decode( $data ); + if ( !is_object( $data ) ) { + // @codeCoverageIgnoreStart + wfLogWarning( "Invalid JSON object in comment: $data" ); + $data = null; + // @codeCoverageIgnoreEnd + } else { + $data = (array)$data; + if ( isset( $data['_message'] ) ) { + $msg = self::decodeMessage( $data['_message'] ) + ->setInterfaceMessageFlag( true ); + } + if ( !empty( $data['_null'] ) ) { + $data = null; + } else { + foreach ( $data as $k => $v ) { + if ( substr( $k, 0, 1 ) === '_' ) { + unset( $data[$k] ); + } + } + } + } + } + + return new CommentStoreComment( $cid, $text, $msg, $data ); + } + + /** + * Extract the comment from a row + * + * Use `self::getJoin()` to ensure the row contains the needed data. + * + * If you need to fake a comment in a row for some reason, set fields + * `{$key}_text` (string) and `{$key}_data` (JSON string or null). + * + * @param object|array $row Result row. + * @param bool $fallback If true, fall back as well as possible instead of throwing an exception. + * @return CommentStoreComment + */ + public function getComment( $row, $fallback = false ) { + return $this->getCommentInternal( null, $row, $fallback ); + } + + /** + * Extract the comment from a row, with legacy lookups. + * + * If `$row` might have been generated using `self::getFields()` rather + * than `self::getJoin()`, use this. Prefer `self::getComment()` if you + * know callers used `self::getJoin()` for the row fetch. + * + * If you need to fake a comment in a row for some reason, set fields + * `{$key}_text` (string) and `{$key}_data` (JSON string or null). + * + * @param IDatabase $db Database handle to use for lookup + * @param object|array $row Result row. + * @param bool $fallback If true, fall back as well as possible instead of throwing an exception. + * @return CommentStoreComment + */ + public function getCommentLegacy( IDatabase $db, $row, $fallback = false ) { + return $this->getCommentInternal( $db, $row, $fallback ); + } + + /** + * Create a new CommentStoreComment, inserting it into the database if necessary + * + * If a comment is going to be passed to `self::insert()` or the like + * multiple times, it will be more efficient to pass a CommentStoreComment + * once rather than making `self::insert()` do it every time through. + * + * @note When passing a CommentStoreComment, this may set `$comment->id` if + * it's not already set. If `$comment->id` is already set, it will not be + * verified that the specified comment actually exists or that it + * corresponds to the comment text, message, and/or data in the + * CommentStoreComment. + * @param IDatabase $dbw Database handle to insert on. Unused if `$comment` + * is a CommentStoreComment and `$comment->id` is set. + * @param string|Message|CommentStoreComment $comment Comment text or Message object, or + * a CommentStoreComment. + * @param array|null $data Structured data to store. Keys beginning with '_' are reserved. + * Ignored if $comment is a CommentStoreComment. + * @return CommentStoreComment + */ + public function createComment( IDatabase $dbw, $comment, array $data = null ) { + global $wgContLang; + + if ( !$comment instanceof CommentStoreComment ) { + if ( $data !== null ) { + foreach ( $data as $k => $v ) { + if ( substr( $k, 0, 1 ) === '_' ) { + throw new InvalidArgumentException( 'Keys in $data beginning with "_" are reserved' ); + } + } + } + if ( $comment instanceof Message ) { + $message = clone $comment; + $text = $message->inLanguage( $wgContLang ) // Avoid $wgForceUIMsgAsContentMsg + ->setInterfaceMessageFlag( true ) + ->text(); + $comment = new CommentStoreComment( null, $text, $message, $data ); + } else { + $comment = new CommentStoreComment( null, $comment, null, $data ); + } + } + + if ( $this->stage > MIGRATION_OLD && !$comment->id ) { + $dbData = $comment->data; + if ( !$comment->message instanceof RawMessage ) { + if ( $dbData === null ) { + $dbData = [ '_null' => true ]; + } + $dbData['_message'] = self::encodeMessage( $comment->message ); + } + if ( $dbData !== null ) { + $dbData = FormatJson::encode( (object)$dbData, false, FormatJson::ALL_OK ); + } + + $hash = self::hash( $comment->text, $dbData ); + $comment->id = $dbw->selectField( + 'comment', + 'comment_id', + [ + 'comment_hash' => $hash, + 'comment_text' => $comment->text, + 'comment_data' => $dbData, + ], + __METHOD__ + ); + if ( !$comment->id ) { + $comment->id = $dbw->nextSequenceValue( 'comment_comment_id_seq' ); + $dbw->insert( + 'comment', + [ + 'comment_id' => $comment->id, + 'comment_hash' => $hash, + 'comment_text' => $comment->text, + 'comment_data' => $dbData, + ], + __METHOD__ + ); + $comment->id = $dbw->insertId(); + } + } + + return $comment; + } + + /** + * Implementation for `self::insert()` and `self::insertWithTempTable()` + * @param IDatabase $dbw + * @param string|Message|CommentStoreComment $comment + * @param array|null $data + * @return array [ array $fields, callable $callback ] + */ + private function insertInternal( IDatabase $dbw, $comment, $data ) { + $fields = []; + $callback = null; + + $comment = $this->createComment( $dbw, $comment, $data ); + + if ( $this->stage <= MIGRATION_WRITE_BOTH ) { + $fields[$this->key] = $comment->text; + } + + if ( $this->stage >= MIGRATION_WRITE_BOTH ) { + if ( isset( self::$tempTables[$this->key] ) ) { + $t = self::$tempTables[$this->key]; + $func = __METHOD__; + $commentId = $comment->id; + $callback = function ( $id ) use ( $dbw, $commentId, $t, $func ) { + $dbw->insert( + $t['table'], + [ + $t['pk'] => $id, + $t['field'] => $commentId, + ], + $func + ); + }; + } else { + $fields["{$this->key}_id"] = $comment->id; + } + } + + return [ $fields, $callback ]; + } + + /** + * Prepare for the insertion of a row with a comment + * + * @note It's recommended to include both the call to this method and the + * row insert in the same transaction. + * @param IDatabase $dbw Database handle to insert on + * @param string|Message|CommentStoreComment $comment As for `self::createComment()` + * @param array|null $data As for `self::createComment()` + * @return array Fields for the insert or update + */ + public function insert( IDatabase $dbw, $comment, $data = null ) { + if ( isset( self::$tempTables[$this->key] ) ) { + throw new InvalidArgumentException( "Must use insertWithTempTable() for $this->key" ); + } + + list( $fields ) = $this->insertInternal( $dbw, $comment, $data ); + return $fields; + } + + /** + * Prepare for the insertion of a row with a comment and temporary table + * + * This is currently needed for "rev_comment" and "img_description". In the + * future that requirement will be removed. + * + * @note It's recommended to include both the call to this method and the + * row insert in the same transaction. + * @param IDatabase $dbw Database handle to insert on + * @param string|Message|CommentStoreComment $comment As for `self::createComment()` + * @param array|null $data As for `self::createComment()` + * @return array Two values: + * - array Fields for the insert or update + * - callable Function to call when the primary key of the row being + * inserted/updated is known. Pass it that primary key. + */ + public function insertWithTempTable( IDatabase $dbw, $comment, $data = null ) { + if ( isset( self::$formerTempTables[$this->key] ) ) { + wfDeprecated( __METHOD__ . " for $this->key", self::$formerTempTables[$this->key] ); + } elseif ( !isset( self::$tempTables[$this->key] ) ) { + throw new InvalidArgumentException( "Must use insert() for $this->key" ); + } + + list( $fields, $callback ) = $this->insertInternal( $dbw, $comment, $data ); + if ( !$callback ) { + $callback = function () { + // Do nothing. + }; + } + return [ $fields, $callback ]; + } + + /** + * Encode a Message as a PHP data structure + * @param Message $msg + * @return array + */ + protected static function encodeMessage( Message $msg ) { + $key = count( $msg->getKeysToTry() ) > 1 ? $msg->getKeysToTry() : $msg->getKey(); + $params = $msg->getParams(); + foreach ( $params as &$param ) { + if ( $param instanceof Message ) { + $param = [ + 'message' => self::encodeMessage( $param ) + ]; + } + } + array_unshift( $params, $key ); + return $params; + } + + /** + * Decode a message that was encoded by self::encodeMessage() + * @param array $data + * @return Message + */ + protected static function decodeMessage( $data ) { + $key = array_shift( $data ); + foreach ( $data as &$param ) { + if ( is_object( $param ) ) { + $param = (array)$param; + } + if ( is_array( $param ) && count( $param ) === 1 && isset( $param['message'] ) ) { + $param = self::decodeMessage( $param['message'] ); + } + } + return new Message( $key, $data ); + } + + /** + * Hashing function for comment storage + * @param string $text Comment text + * @param string|null $data Comment data + * @return int 32-bit signed integer + */ + public static function hash( $text, $data ) { + $hash = crc32( $text ) ^ crc32( (string)$data ); + + // 64-bit PHP returns an unsigned CRC, change it to signed for + // insertion into the database. + if ( $hash >= 0x80000000 ) { + $hash |= -1 << 32; + } + + return $hash; + } + +} diff --git a/includes/CommentStoreComment.php b/includes/CommentStoreComment.php new file mode 100644 index 0000000000..afc1374223 --- /dev/null +++ b/includes/CommentStoreComment.php @@ -0,0 +1,57 @@ +id = $id; + $this->text = $text; + $this->message = $message ?: new RawMessage( '$1', [ $text ] ); + $this->data = $data; + } +} diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 44461b36f9..cf3e569b2a 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -8763,6 +8763,13 @@ $wgExperiencedUserMemberSince = 30; # days */ $wgInterwikiPrefixDisplayTypes = []; +/** + * Comment table schema migration stage. + * @since 1.30 + * @var int One of the MIGRATION_* constants + */ +$wgCommentTableSchemaMigrationStage = MIGRATION_OLD; + /** * For really cool vim folding this needs to be at the end: * vim: foldmarker=@{,@} foldmethod=marker diff --git a/includes/EditPage.php b/includes/EditPage.php index 72a072d148..40913bbd12 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -2701,7 +2701,7 @@ class EditPage { if ( $this->wasDeletedSinceLastEdit() && 'save' == $this->formtype ) { $username = $this->lastDelete->user_name; - $comment = $this->lastDelete->log_comment; + $comment = CommentStore::newKey( 'log_comment' )->getComment( $this->lastDelete )->text; // It is better to not parse the comment at all than to have templates expanded in the middle // TODO: can the checkLabel be moved outside of the div so that wrapWikiMsg could be used? @@ -3687,8 +3687,9 @@ class EditPage { */ protected function getLastDelete() { $dbr = wfGetDB( DB_REPLICA ); + $commentQuery = CommentStore::newKey( 'log_comment' )->getJoin(); $data = $dbr->selectRow( - [ 'logging', 'user' ], + [ 'logging', 'user' ] + $commentQuery['tables'], [ 'log_type', 'log_action', @@ -3696,11 +3697,10 @@ class EditPage { 'log_user', 'log_namespace', 'log_title', - 'log_comment', 'log_params', 'log_deleted', 'user_name' - ], [ + ] + $commentQuery['fields'], [ 'log_namespace' => $this->mTitle->getNamespace(), 'log_title' => $this->mTitle->getDBkey(), 'log_type' => 'delete', @@ -3708,7 +3708,10 @@ class EditPage { 'user_id=log_user' ], __METHOD__, - [ 'LIMIT' => 1, 'ORDER BY' => 'log_timestamp DESC' ] + [ 'LIMIT' => 1, 'ORDER BY' => 'log_timestamp DESC' ], + [ + 'user' => [ 'JOIN', 'user_id=log_user' ], + ] + $commentQuery['joins'] ); // Quick paranoid permission checks... if ( is_object( $data ) ) { @@ -3717,7 +3720,8 @@ class EditPage { } if ( $data->log_deleted & LogPage::DELETED_COMMENT ) { - $data->log_comment = $this->context->msg( 'rev-deleted-comment' )->escaped(); + $data->log_comment_text = $this->context->msg( 'rev-deleted-comment' )->escaped(); + $data->log_comment_data = null; } } diff --git a/includes/FeedUtils.php b/includes/FeedUtils.php index 96a88d3d98..0def6a0402 100644 --- a/includes/FeedUtils.php +++ b/includes/FeedUtils.php @@ -72,7 +72,8 @@ class FeedUtils { /** * Format a diff for the newsfeed * - * @param object $row Row from the recentchanges table + * @param object $row Row from the recentchanges table, including fields as + * appropriate for CommentStore * @return string */ public static function formatDiff( $row ) { @@ -88,7 +89,9 @@ class FeedUtils { $timestamp, $row->rc_deleted & Revision::DELETED_COMMENT ? wfMessage( 'rev-deleted-comment' )->escaped() - : $row->rc_comment, + : CommentStore::newKey( 'rc_comment' ) + // Legacy from RecentChange::selectFields() via ChangesListSpecialPage::doMainQuery() + ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row )->text, $actiontext ); } diff --git a/includes/Revision.php b/includes/Revision.php index e457beb035..ff4a284386 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -192,7 +192,9 @@ class Revision implements IDBAccessObject { $attribs = $overrides + [ 'page' => isset( $row->ar_page_id ) ? $row->ar_page_id : null, 'id' => isset( $row->ar_rev_id ) ? $row->ar_rev_id : null, - 'comment' => $row->ar_comment, + 'comment' => CommentStore::newKey( 'ar_comment' ) + // Legacy because $row probably came from self::selectArchiveFields() + ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row, true )->text, 'user' => $row->ar_user, 'user_text' => $row->ar_user_text, 'timestamp' => $row->ar_timestamp, @@ -443,6 +445,8 @@ class Revision implements IDBAccessObject { /** * Return the list of revision fields that should be selected to create * a new revision. + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ public static function selectFields() { @@ -453,7 +457,6 @@ class Revision implements IDBAccessObject { 'rev_page', 'rev_text_id', 'rev_timestamp', - 'rev_comment', 'rev_user_text', 'rev_user', 'rev_minor_edit', @@ -463,6 +466,8 @@ class Revision implements IDBAccessObject { 'rev_sha1', ]; + $fields += CommentStore::newKey( 'rev_comment' )->getFields(); + if ( $wgContentHandlerUseDB ) { $fields[] = 'rev_content_format'; $fields[] = 'rev_content_model'; @@ -474,6 +479,8 @@ class Revision implements IDBAccessObject { /** * Return the list of revision fields that should be selected to create * a new revision from an archive row. + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ public static function selectArchiveFields() { @@ -485,7 +492,6 @@ class Revision implements IDBAccessObject { 'ar_text', 'ar_text_id', 'ar_timestamp', - 'ar_comment', 'ar_user_text', 'ar_user', 'ar_minor_edit', @@ -495,6 +501,8 @@ class Revision implements IDBAccessObject { 'ar_sha1', ]; + $fields += CommentStore::newKey( 'ar_comment' )->getFields(); + if ( $wgContentHandlerUseDB ) { $fields[] = 'ar_content_format'; $fields[] = 'ar_content_model'; @@ -568,7 +576,9 @@ class Revision implements IDBAccessObject { $this->mId = intval( $row->rev_id ); $this->mPage = intval( $row->rev_page ); $this->mTextId = intval( $row->rev_text_id ); - $this->mComment = $row->rev_comment; + $this->mComment = CommentStore::newKey( 'rev_comment' ) + // Legacy because $row probably came from self::selectFields() + ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row, true )->text; $this->mUser = intval( $row->rev_user ); $this->mMinorEdit = intval( $row->rev_minor_edit ); $this->mTimestamp = $row->rev_timestamp; @@ -1455,7 +1465,6 @@ class Revision implements IDBAccessObject { 'rev_id' => $rev_id, 'rev_page' => $this->mPage, 'rev_text_id' => $this->mTextId, - 'rev_comment' => $this->mComment, 'rev_minor_edit' => $this->mMinorEdit ? 1 : 0, 'rev_user' => $this->mUser, 'rev_user_text' => $this->mUserText, @@ -1470,6 +1479,10 @@ class Revision implements IDBAccessObject { : $this->mSha1, ]; + list( $commentFields, $commentCallback ) = + CommentStore::newKey( 'rev_comment' )->insertWithTempTable( $dbw, $this->mComment ); + $row += $commentFields; + if ( $wgContentHandlerUseDB ) { // NOTE: Store null for the default model and format, to save space. // XXX: Makes the DB sensitive to changed defaults. @@ -1498,6 +1511,7 @@ class Revision implements IDBAccessObject { // Only if nextSequenceValue() was called $this->mId = $dbw->insertId(); } + $commentCallback( $this->mId ); // Assertion to try to catch T92046 if ( (int)$this->mId === 0 ) { diff --git a/includes/Title.php b/includes/Title.php index 729628e32a..0687a15589 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -2670,24 +2670,33 @@ class Title implements LinkTarget { if ( $this->mTitleProtection === null ) { $dbr = wfGetDB( DB_REPLICA ); + $commentStore = new CommentStore( 'pt_reason' ); + $commentQuery = $commentStore->getJoin(); $res = $dbr->select( - 'protected_titles', + [ 'protected_titles' ] + $commentQuery['tables'], [ 'user' => 'pt_user', - 'reason' => 'pt_reason', 'expiry' => 'pt_expiry', 'permission' => 'pt_create_perm' - ], + ] + $commentQuery['fields'], [ 'pt_namespace' => $this->getNamespace(), 'pt_title' => $this->getDBkey() ], - __METHOD__ + __METHOD__, + [], + $commentQuery['joins'] ); // fetchRow returns false if there are no rows. $row = $dbr->fetchRow( $res ); if ( $row ) { - $row['expiry'] = $dbr->decodeExpiry( $row['expiry'] ); + $this->mTitleProtection = [ + 'user' => $row['user'], + 'expiry' => $dbr->decodeExpiry( $row['expiry'] ), + 'permission' => $row['permission'], + 'reason' => $commentStore->getComment( $row )->text, + ]; + } else { + $this->mTitleProtection = false; } - $this->mTitleProtection = $row; } return $this->mTitleProtection; } diff --git a/includes/WatchedItemQueryService.php b/includes/WatchedItemQueryService.php index 1fafb24dbe..d0f45bec17 100644 --- a/includes/WatchedItemQueryService.php +++ b/includes/WatchedItemQueryService.php @@ -55,6 +55,10 @@ class WatchedItemQueryService { /** @var WatchedItemQueryServiceExtension[]|null */ private $extensions = null; + /** + * @var CommentStore|null */ + private $commentStore = null; + public function __construct( LoadBalancer $loadBalancer ) { $this->loadBalancer = $loadBalancer; } @@ -78,6 +82,13 @@ class WatchedItemQueryService { return $this->loadBalancer->getConnectionRef( DB_REPLICA, [ 'watchlist' ] ); } + private function getCommentStore() { + if ( !$this->commentStore ) { + $this->commentStore = new CommentStore( 'rc_comment' ); + } + return $this->commentStore; + } + /** * @param User $user * @param array $options Allowed keys: @@ -172,13 +183,9 @@ class WatchedItemQueryService { ); } - $tables = [ 'recentchanges', 'watchlist' ]; - if ( !$options['allRevisions'] ) { - $tables[] = 'page'; - } - $db = $this->getConnection(); + $tables = $this->getWatchedItemsWithRCInfoQueryTables( $options ); $fields = $this->getWatchedItemsWithRCInfoQueryFields( $options ); $conds = $this->getWatchedItemsWithRCInfoQueryConds( $db, $user, $options ); $dbOptions = $this->getWatchedItemsWithRCInfoQueryDbOptions( $options ); @@ -320,6 +327,17 @@ class WatchedItemQueryService { return array_intersect_key( $allFields, array_flip( $rcKeys ) ); } + private function getWatchedItemsWithRCInfoQueryTables( array $options ) { + $tables = [ 'recentchanges', 'watchlist' ]; + if ( !$options['allRevisions'] ) { + $tables[] = 'page'; + } + if ( in_array( self::INCLUDE_COMMENT, $options['includeFields'] ) ) { + $tables += $this->getCommentStore()->getJoin()['tables']; + } + return $tables; + } + private function getWatchedItemsWithRCInfoQueryFields( array $options ) { $fields = [ 'rc_id', @@ -355,7 +373,7 @@ class WatchedItemQueryService { $fields[] = 'rc_user'; } if ( in_array( self::INCLUDE_COMMENT, $options['includeFields'] ) ) { - $fields[] = 'rc_comment'; + $fields += $this->getCommentStore()->getJoin()['fields']; } if ( in_array( self::INCLUDE_PATROL_INFO, $options['includeFields'] ) ) { $fields = array_merge( $fields, [ 'rc_patrolled', 'rc_log_type' ] ); @@ -657,6 +675,9 @@ class WatchedItemQueryService { if ( !$options['allRevisions'] ) { $joinConds['page'] = [ 'LEFT JOIN', 'rc_cur_id=page_id' ]; } + if ( in_array( self::INCLUDE_COMMENT, $options['includeFields'] ) ) { + $joinConds += $this->getCommentStore()->getJoin()['joins']; + } return $joinConds; } diff --git a/includes/api/ApiQueryAllUsers.php b/includes/api/ApiQueryAllUsers.php index fd95e1785a..d594ad44a0 100644 --- a/includes/api/ApiQueryAllUsers.php +++ b/includes/api/ApiQueryAllUsers.php @@ -49,6 +49,7 @@ class ApiQueryAllUsers extends ApiQueryBase { $activeUserDays = $this->getConfig()->get( 'ActiveUserDays' ); $db = $this->getDB(); + $commentStore = new CommentStore( 'ipb_reason' ); $prop = $params['prop']; if ( !is_null( $prop ) ) { @@ -263,7 +264,7 @@ class ApiQueryAllUsers extends ApiQueryBase { $data['blockedby'] = $row->ipb_by_text; $data['blockedbyid'] = (int)$row->ipb_by; $data['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $row->ipb_timestamp ); - $data['blockreason'] = $row->ipb_reason; + $data['blockreason'] = $commentStore->getComment( $row )->text; $data['blockexpiry'] = $row->ipb_expiry; } if ( $row->ipb_deleted ) { diff --git a/includes/api/ApiQueryBase.php b/includes/api/ApiQueryBase.php index 44526e88b1..fe16134c7b 100644 --- a/includes/api/ApiQueryBase.php +++ b/includes/api/ApiQueryBase.php @@ -456,10 +456,13 @@ abstract class ApiQueryBase extends ApiBase { 'ipb_id', 'ipb_by', 'ipb_by_text', - 'ipb_reason', 'ipb_expiry', 'ipb_timestamp' ] ); + $commentQuery = CommentStore::newKey( 'ipb_reason' )->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); } // Don't show hidden names diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php index 076a09efdf..698c13c53d 100644 --- a/includes/api/ApiQueryBlocks.php +++ b/includes/api/ApiQueryBlocks.php @@ -37,6 +37,7 @@ class ApiQueryBlocks extends ApiQueryBase { public function execute() { $db = $this->getDB(); + $commentStore = new CommentStore( 'ipb_reason' ); $params = $this->extractRequestParams(); $this->requireMaxOneParameter( $params, 'users', 'ip' ); @@ -61,12 +62,18 @@ class ApiQueryBlocks extends ApiQueryBase { $this->addFieldsIf( 'ipb_by_text', $fld_by ); $this->addFieldsIf( 'ipb_by', $fld_byid ); $this->addFieldsIf( 'ipb_expiry', $fld_expiry ); - $this->addFieldsIf( 'ipb_reason', $fld_reason ); $this->addFieldsIf( [ 'ipb_range_start', 'ipb_range_end' ], $fld_range ); $this->addFieldsIf( [ 'ipb_anon_only', 'ipb_create_account', 'ipb_enable_autoblock', 'ipb_block_email', 'ipb_deleted', 'ipb_allow_usertalk' ], $fld_flags ); + if ( $fld_reason ) { + $commentQuery = $commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } + $this->addOption( 'LIMIT', $params['limit'] + 1 ); $this->addTimestampWhereRange( 'ipb_timestamp', @@ -205,7 +212,7 @@ class ApiQueryBlocks extends ApiQueryBase { $block['expiry'] = ApiResult::formatExpiry( $row->ipb_expiry ); } if ( $fld_reason ) { - $block['reason'] = $row->ipb_reason; + $block['reason'] = $commentStore->getComment( $row )->text; } if ( $fld_range && !$row->ipb_auto ) { $block['rangestart'] = IP::formatHex( $row->ipb_range_start ); diff --git a/includes/api/ApiQueryDeletedrevs.php b/includes/api/ApiQueryDeletedrevs.php index b68a8682c5..5dd007b49a 100644 --- a/includes/api/ApiQueryDeletedrevs.php +++ b/includes/api/ApiQueryDeletedrevs.php @@ -44,6 +44,7 @@ class ApiQueryDeletedrevs extends ApiQueryBase { $user = $this->getUser(); $db = $this->getDB(); + $commentStore = new CommentStore( 'ar_comment' ); $params = $this->extractRequestParams( false ); $prop = array_flip( $params['prop'] ); $fld_parentid = isset( $prop['parentid'] ); @@ -115,11 +116,17 @@ class ApiQueryDeletedrevs extends ApiQueryBase { $this->addFieldsIf( 'ar_rev_id', $fld_revid ); $this->addFieldsIf( 'ar_user_text', $fld_user ); $this->addFieldsIf( 'ar_user', $fld_userid ); - $this->addFieldsIf( 'ar_comment', $fld_comment || $fld_parsedcomment ); $this->addFieldsIf( 'ar_minor_edit', $fld_minor ); $this->addFieldsIf( 'ar_len', $fld_len ); $this->addFieldsIf( 'ar_sha1', $fld_sha1 ); + if ( $fld_comment || $fld_parsedcomment ) { + $commentQuery = $commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } + if ( $fld_tags ) { $this->addTables( 'tag_summary' ); $this->addJoinConds( @@ -322,12 +329,13 @@ class ApiQueryDeletedrevs extends ApiQueryBase { $anyHidden = true; } if ( Revision::userCanBitfield( $row->ar_deleted, Revision::DELETED_COMMENT, $user ) ) { + $comment = $commentStore->getComment( $row )->text; if ( $fld_comment ) { - $rev['comment'] = $row->ar_comment; + $rev['comment'] = $comment; } if ( $fld_parsedcomment ) { $title = Title::makeTitle( $row->ar_namespace, $row->ar_title ); - $rev['parsedcomment'] = Linker::formatComment( $row->ar_comment, $title ); + $rev['parsedcomment'] = Linker::formatComment( $comment, $title ); } } } diff --git a/includes/api/ApiQueryFilearchive.php b/includes/api/ApiQueryFilearchive.php index 7383cba6cb..212b61340a 100644 --- a/includes/api/ApiQueryFilearchive.php +++ b/includes/api/ApiQueryFilearchive.php @@ -43,6 +43,7 @@ class ApiQueryFilearchive extends ApiQueryBase { $user = $this->getUser(); $db = $this->getDB(); + $commentStore = new CommentStore( 'fa_description' ); $params = $this->extractRequestParams(); @@ -66,13 +67,19 @@ class ApiQueryFilearchive extends ApiQueryBase { $this->addFieldsIf( 'fa_sha1', $fld_sha1 ); $this->addFieldsIf( [ 'fa_user', 'fa_user_text' ], $fld_user ); $this->addFieldsIf( [ 'fa_height', 'fa_width', 'fa_size' ], $fld_dimensions || $fld_size ); - $this->addFieldsIf( 'fa_description', $fld_description ); $this->addFieldsIf( [ 'fa_major_mime', 'fa_minor_mime' ], $fld_mime ); $this->addFieldsIf( 'fa_media_type', $fld_mediatype ); $this->addFieldsIf( 'fa_metadata', $fld_metadata ); $this->addFieldsIf( 'fa_bits', $fld_bitdepth ); $this->addFieldsIf( 'fa_archive_name', $fld_archivename ); + if ( $fld_description ) { + $commentQuery = $commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } + if ( !is_null( $params['continue'] ) ) { $cont = explode( '|', $params['continue'] ); $this->dieContinueUsageIf( count( $cont ) != 3 ); @@ -165,10 +172,10 @@ class ApiQueryFilearchive extends ApiQueryBase { if ( $fld_description && Revision::userCanBitfield( $row->fa_deleted, File::DELETED_COMMENT, $user ) ) { - $file['description'] = $row->fa_description; + $file['description'] = $commentStore->getComment( $row )->text; if ( isset( $prop['parseddescription'] ) ) { $file['parseddescription'] = Linker::formatComment( - $row->fa_description, $title ); + $file['description'], $title ); } } if ( $fld_user && diff --git a/includes/api/ApiQueryLogEvents.php b/includes/api/ApiQueryLogEvents.php index 3e8bccc74d..3066720d1c 100644 --- a/includes/api/ApiQueryLogEvents.php +++ b/includes/api/ApiQueryLogEvents.php @@ -31,6 +31,8 @@ */ class ApiQueryLogEvents extends ApiQueryBase { + private $commentStore; + public function __construct( ApiQuery $query, $moduleName ) { parent::__construct( $query, $moduleName, 'le' ); } @@ -43,6 +45,7 @@ class ApiQueryLogEvents extends ApiQueryBase { public function execute() { $params = $this->extractRequestParams(); $db = $this->getDB(); + $this->commentStore = new CommentStore( 'log_comment' ); $this->requireMaxOneParameter( $params, 'title', 'prefix', 'namespace' ); $prop = array_flip( $params['prop'] ); @@ -91,9 +94,15 @@ class ApiQueryLogEvents extends ApiQueryBase { [ 'log_namespace', 'log_title' ], $this->fld_title || $this->fld_parsedcomment ); - $this->addFieldsIf( 'log_comment', $this->fld_comment || $this->fld_parsedcomment ); $this->addFieldsIf( 'log_params', $this->fld_details ); + if ( $this->fld_comment || $this->fld_parsedcomment ) { + $commentQuery = $this->commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } + if ( $this->fld_tags ) { $this->addTables( 'tag_summary' ); $this->addJoinConds( [ 'tag_summary' => [ 'LEFT JOIN', 'log_id=ts_log_id' ] ] ); @@ -327,18 +336,19 @@ class ApiQueryLogEvents extends ApiQueryBase { $vals['timestamp'] = wfTimestamp( TS_ISO_8601, $row->log_timestamp ); } - if ( ( $this->fld_comment || $this->fld_parsedcomment ) && isset( $row->log_comment ) ) { + if ( $this->fld_comment || $this->fld_parsedcomment ) { if ( LogEventsList::isDeleted( $row, LogPage::DELETED_COMMENT ) ) { $vals['commenthidden'] = true; $anyHidden = true; } if ( LogEventsList::userCan( $row, LogPage::DELETED_COMMENT, $user ) ) { + $comment = $this->commentStore->getComment( $row )->text; if ( $this->fld_comment ) { - $vals['comment'] = $row->log_comment; + $vals['comment'] = $comment; } if ( $this->fld_parsedcomment ) { - $vals['parsedcomment'] = Linker::formatComment( $row->log_comment, $title ); + $vals['parsedcomment'] = Linker::formatComment( $comment, $title ); } } } diff --git a/includes/api/ApiQueryProtectedTitles.php b/includes/api/ApiQueryProtectedTitles.php index 5f6510ea28..b69a29967b 100644 --- a/includes/api/ApiQueryProtectedTitles.php +++ b/includes/api/ApiQueryProtectedTitles.php @@ -55,10 +55,17 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { $prop = array_flip( $params['prop'] ); $this->addFieldsIf( 'pt_user', isset( $prop['user'] ) || isset( $prop['userid'] ) ); - $this->addFieldsIf( 'pt_reason', isset( $prop['comment'] ) || isset( $prop['parsedcomment'] ) ); $this->addFieldsIf( 'pt_expiry', isset( $prop['expiry'] ) ); $this->addFieldsIf( 'pt_create_perm', isset( $prop['level'] ) ); + if ( isset( $prop['comment'] ) || isset( $prop['parsedcomment'] ) ) { + $commentStore = new CommentStore( 'pt_reason' ); + $commentQuery = $commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } + $this->addTimestampWhereRange( 'pt_timestamp', $params['dir'], $params['start'], $params['end'] ); $this->addWhereFld( 'pt_namespace', $params['namespace'] ); $this->addWhereFld( 'pt_create_perm', $params['level'] ); @@ -127,11 +134,13 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { } if ( isset( $prop['comment'] ) ) { - $vals['comment'] = $row->pt_reason; + $vals['comment'] = $commentStore->getComment( $row )->text; } if ( isset( $prop['parsedcomment'] ) ) { - $vals['parsedcomment'] = Linker::formatComment( $row->pt_reason, $title ); + $vals['parsedcomment'] = Linker::formatComment( + $commentStore->getComment( $row )->text, $titles + ); } if ( isset( $prop['expiry'] ) ) { diff --git a/includes/api/ApiQueryRecentChanges.php b/includes/api/ApiQueryRecentChanges.php index 0dc01aabc2..9af4e3e4a2 100644 --- a/includes/api/ApiQueryRecentChanges.php +++ b/includes/api/ApiQueryRecentChanges.php @@ -36,6 +36,8 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { parent::__construct( $query, $moduleName, 'rc' ); } + private $commentStore; + private $fld_comment = false, $fld_parsedcomment = false, $fld_user = false, $fld_userid = false, $fld_flags = false, $fld_timestamp = false, $fld_title = false, $fld_ids = false, $fld_sizes = false, $fld_redirect = false, $fld_patrolled = false, $fld_loginfo = false, @@ -274,7 +276,6 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { /* Add fields to our query if they are specified as a needed parameter. */ $this->addFieldsIf( [ 'rc_this_oldid', 'rc_last_oldid' ], $this->fld_ids ); - $this->addFieldsIf( 'rc_comment', $this->fld_comment || $this->fld_parsedcomment ); $this->addFieldsIf( 'rc_user', $this->fld_user || $this->fld_userid ); $this->addFieldsIf( 'rc_user_text', $this->fld_user ); $this->addFieldsIf( [ 'rc_minor', 'rc_type', 'rc_bot' ], $this->fld_flags ); @@ -286,6 +287,14 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { ); $showRedirects = $this->fld_redirect || isset( $show['redirect'] ) || isset( $show['!redirect'] ); + + if ( $this->fld_comment || $this->fld_parsedcomment ) { + $this->commentStore = new CommentStore( 'rc_comment' ); + $commentQuery = $this->commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } } $this->addFieldsIf( [ 'rc_this_oldid' ], $resultPageSet && $params['generaterevisions'] ); @@ -500,12 +509,13 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { $anyHidden = true; } if ( Revision::userCanBitfield( $row->rc_deleted, Revision::DELETED_COMMENT, $user ) ) { - if ( $this->fld_comment && isset( $row->rc_comment ) ) { - $vals['comment'] = $row->rc_comment; + $comment = $this->commentStore->getComment( $row )->text; + if ( $this->fld_comment ) { + $vals['comment'] = $comment; } - if ( $this->fld_parsedcomment && isset( $row->rc_comment ) ) { - $vals['parsedcomment'] = Linker::formatComment( $row->rc_comment, $title ); + if ( $this->fld_parsedcomment ) { + $vals['parsedcomment'] = Linker::formatComment( $comment, $title ); } } } diff --git a/includes/api/ApiQueryUserContributions.php b/includes/api/ApiQueryUserContributions.php index 181cddbeda..bb0f335bcb 100644 --- a/includes/api/ApiQueryUserContributions.php +++ b/includes/api/ApiQueryUserContributions.php @@ -36,7 +36,7 @@ class ApiQueryContributions extends ApiQueryBase { } private $params, $prefixMode, $userprefix, $multiUserMode, $idMode, $usernames, $userids, - $parentLens; + $parentLens, $commentStore; private $fld_ids = false, $fld_title = false, $fld_timestamp = false, $fld_comment = false, $fld_parsedcomment = false, $fld_flags = false, $fld_patrolled = false, $fld_tags = false, $fld_size = false, $fld_sizediff = false; @@ -45,6 +45,8 @@ class ApiQueryContributions extends ApiQueryBase { // Parse some parameters $this->params = $this->extractRequestParams(); + $this->commentStore = new CommentStore( 'rev_comment' ); + $prop = array_flip( $this->params['prop'] ); $this->fld_ids = isset( $prop['ids'] ); $this->fld_title = isset( $prop['title'] ); @@ -341,12 +343,18 @@ class ApiQueryContributions extends ApiQueryBase { $this->addFieldsIf( 'rev_page', $this->fld_ids ); $this->addFieldsIf( 'page_latest', $this->fld_flags ); // $this->addFieldsIf( 'rev_text_id', $this->fld_ids ); // Should this field be exposed? - $this->addFieldsIf( 'rev_comment', $this->fld_comment || $this->fld_parsedcomment ); $this->addFieldsIf( 'rev_len', $this->fld_size || $this->fld_sizediff ); $this->addFieldsIf( 'rev_minor_edit', $this->fld_flags ); $this->addFieldsIf( 'rev_parent_id', $this->fld_flags || $this->fld_sizediff || $this->fld_ids ); $this->addFieldsIf( 'rc_patrolled', $this->fld_patrolled ); + if ( $this->fld_comment || $this->fld_parsedcomment ) { + $commentQuery = $this->commentStore->getJoin(); + $this->addTables( $commentQuery['tables'] ); + $this->addFields( $commentQuery['fields'] ); + $this->addJoinConds( $commentQuery['joins'] ); + } + if ( $this->fld_tags ) { $this->addTables( 'tag_summary' ); $this->addJoinConds( @@ -416,7 +424,7 @@ class ApiQueryContributions extends ApiQueryBase { $vals['top'] = $row->page_latest == $row->rev_id; } - if ( ( $this->fld_comment || $this->fld_parsedcomment ) && isset( $row->rev_comment ) ) { + if ( $this->fld_comment || $this->fld_parsedcomment ) { if ( $row->rev_deleted & Revision::DELETED_COMMENT ) { $vals['commenthidden'] = true; $anyHidden = true; @@ -428,12 +436,13 @@ class ApiQueryContributions extends ApiQueryBase { ); if ( $userCanView ) { + $comment = $this->commentStore->getComment( $row )->text; if ( $this->fld_comment ) { - $vals['comment'] = $row->rev_comment; + $vals['comment'] = $comment; } if ( $this->fld_parsedcomment ) { - $vals['parsedcomment'] = Linker::formatComment( $row->rev_comment, $title ); + $vals['parsedcomment'] = Linker::formatComment( $comment, $title ); } } } diff --git a/includes/api/ApiQueryUsers.php b/includes/api/ApiQueryUsers.php index 2a0eaddfb6..fbf1f9ebfb 100644 --- a/includes/api/ApiQueryUsers.php +++ b/includes/api/ApiQueryUsers.php @@ -99,6 +99,7 @@ class ApiQueryUsers extends ApiQueryBase { public function execute() { $db = $this->getDB(); + $commentStore = new CommentStore( 'ipb_reason' ); $params = $this->extractRequestParams(); $this->requireMaxOneParameter( $params, 'userids', 'users' ); @@ -236,7 +237,7 @@ class ApiQueryUsers extends ApiQueryBase { $data[$key]['blockedby'] = $row->ipb_by_text; $data[$key]['blockedbyid'] = (int)$row->ipb_by; $data[$key]['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $row->ipb_timestamp ); - $data[$key]['blockreason'] = $row->ipb_reason; + $data[$key]['blockreason'] = $commentStore->getComment( $row )->text; $data[$key]['blockexpiry'] = $row->ipb_expiry; } diff --git a/includes/api/ApiQueryWatchlist.php b/includes/api/ApiQueryWatchlist.php index 9883480500..2ab85240d8 100644 --- a/includes/api/ApiQueryWatchlist.php +++ b/includes/api/ApiQueryWatchlist.php @@ -34,6 +34,8 @@ use MediaWiki\MediaWikiServices; */ class ApiQueryWatchlist extends ApiQueryGeneratorBase { + private $commentStore; + public function __construct( ApiQuery $query, $moduleName ) { parent::__construct( $query, $moduleName, 'wl' ); } @@ -85,6 +87,10 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { $this->dieWithError( 'apierror-permissiondenied-patrolflag', 'patrol' ); } } + + if ( $this->fld_comment || $this->fld_parsedcomment ) { + $this->commentStore = new CommentStore( 'rc_comment' ); + } } $options = [ @@ -353,12 +359,13 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { Revision::DELETED_COMMENT, $user ) ) { - if ( $this->fld_comment && isset( $recentChangeInfo['rc_comment'] ) ) { - $vals['comment'] = $recentChangeInfo['rc_comment']; + $comment = $this->commentStore->getComment( $recentChangeInfo )->text; + if ( $this->fld_comment ) { + $vals['comment'] = $comment; } - if ( $this->fld_parsedcomment && isset( $recentChangeInfo['rc_comment'] ) ) { - $vals['parsedcomment'] = Linker::formatComment( $recentChangeInfo['rc_comment'], $title ); + if ( $this->fld_parsedcomment ) { + $vals['parsedcomment'] = Linker::formatComment( $comment, $title ); } } } diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php index f1233639fe..588f602e64 100644 --- a/includes/changes/RecentChange.php +++ b/includes/changes/RecentChange.php @@ -59,6 +59,10 @@ * temporary: not stored in the database * notificationtimestamp * numberofWatchingusers + * + * @todo Deprecate access to mAttribs (direct or via getAttributes). Right now + * we're having to include both rc_comment and rc_comment_text/rc_comment_data + * so random crap works right. */ class RecentChange { // Constants for the rc_source field. Extensions may also have @@ -199,6 +203,8 @@ class RecentChange { /** * Return the list of recentchanges fields that should be selected to create * a new recentchanges object. + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ public static function selectFields() { @@ -209,7 +215,6 @@ class RecentChange { 'rc_user_text', 'rc_namespace', 'rc_title', - 'rc_comment', 'rc_minor', 'rc_bot', 'rc_new', @@ -227,7 +232,7 @@ class RecentChange { 'rc_log_type', 'rc_log_action', 'rc_params', - ]; + ] + CommentStore::newKey( 'rc_comment' )->getFields(); } # Accessors @@ -322,8 +327,14 @@ class RecentChange { unset( $this->mAttribs['rc_cur_id'] ); } + # Convert mAttribs['rc_comment'] for CommentStore + $row = $this->mAttribs; + $comment = $row['rc_comment']; + unset( $row['rc_comment'], $row['rc_comment_text'], $row['rc_comment_data'] ); + $row += CommentStore::newKey( 'rc_comment' )->insert( $dbw, $comment ); + # Insert new row - $dbw->insert( 'recentchanges', $this->mAttribs, __METHOD__ ); + $dbw->insert( 'recentchanges', $row, __METHOD__ ); # Set the ID $this->mAttribs['rc_id'] = $dbw->insertId(); @@ -586,7 +597,9 @@ class RecentChange { 'rc_cur_id' => $title->getArticleID(), 'rc_user' => $user->getId(), 'rc_user_text' => $user->getName(), - 'rc_comment' => $comment, + 'rc_comment' => &$comment, + 'rc_comment_text' => &$comment, + 'rc_comment_data' => null, 'rc_this_oldid' => $newId, 'rc_last_oldid' => $oldId, 'rc_bot' => $bot ? 1 : 0, @@ -659,7 +672,9 @@ class RecentChange { 'rc_cur_id' => $title->getArticleID(), 'rc_user' => $user->getId(), 'rc_user_text' => $user->getName(), - 'rc_comment' => $comment, + 'rc_comment' => &$comment, + 'rc_comment_text' => &$comment, + 'rc_comment_data' => null, 'rc_this_oldid' => $newId, 'rc_last_oldid' => 0, 'rc_bot' => $bot ? 1 : 0, @@ -789,7 +804,9 @@ class RecentChange { 'rc_cur_id' => $target->getArticleID(), 'rc_user' => $user->getId(), 'rc_user_text' => $user->getName(), - 'rc_comment' => $logComment, + 'rc_comment' => &$logComment, + 'rc_comment_text' => &$logComment, + 'rc_comment_data' => null, 'rc_this_oldid' => $revId, 'rc_last_oldid' => 0, 'rc_bot' => $user->isAllowed( 'bot' ) ? (int)$wgRequest->getBool( 'bot', true ) : 0, @@ -862,7 +879,9 @@ class RecentChange { 'rc_cur_id' => $pageTitle->getArticleID(), 'rc_user' => $user ? $user->getId() : 0, 'rc_user_text' => $user ? $user->getName() : '', - 'rc_comment' => $comment, + 'rc_comment' => &$comment, + 'rc_comment_text' => &$comment, + 'rc_comment_data' => null, 'rc_this_oldid' => $newRevId, 'rc_last_oldid' => $oldRevId, 'rc_bot' => $bot ? 1 : 0, @@ -922,6 +941,13 @@ class RecentChange { $this->mAttribs['rc_ip'] = substr( $this->mAttribs['rc_ip'], 0, $n ); } } + + $comment = CommentStore::newKey( 'rc_comment' ) + // Legacy because $row probably came from self::selectFields() + ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row, true )->text; + $this->mAttribs['rc_comment'] = &$comment; + $this->mAttribs['rc_comment_text'] = &$comment; + $this->mAttribs['rc_comment_data'] = null; } /** @@ -931,6 +957,9 @@ class RecentChange { * @return mixed */ public function getAttribute( $name ) { + if ( $name === 'rc_comment' ) { + return CommentStore::newKey( 'rc_comment' )->getComment( $this->mAttribs, true )->text; + } return isset( $this->mAttribs[$name] ) ? $this->mAttribs[$name] : null; } diff --git a/includes/export/WikiExporter.php b/includes/export/WikiExporter.php index e0ebaa2411..6e2a5a4f0c 100644 --- a/includes/export/WikiExporter.php +++ b/includes/export/WikiExporter.php @@ -260,7 +260,7 @@ class WikiExporter { protected function dumpFrom( $cond = '', $orderRevs = false ) { # For logging dumps... if ( $this->history & self::LOGS ) { - $where = [ 'user_id = log_user' ]; + $where = []; # Hide private logs $hideLogs = LogEventsList::getExcludeClause( $this->db ); if ( $hideLogs ) { @@ -277,12 +277,16 @@ class WikiExporter { $prev = $this->db->bufferResults( false ); } $result = null; // Assuring $result is not undefined, if exception occurs early + + $commentQuery = CommentStore::newKey( 'log_comment' )->getJoin(); + try { - $result = $this->db->select( [ 'logging', 'user' ], - [ "{$logging}.*", 'user_name' ], // grab the user name + $result = $this->db->select( [ 'logging', 'user' ] + $commentQuery['tables'], + [ "{$logging}.*", 'user_name' ] + $commentQuery['fields'], // grab the user name $where, __METHOD__, - [ 'ORDER BY' => 'log_id', 'USE INDEX' => [ 'logging' => 'PRIMARY' ] ] + [ 'ORDER BY' => 'log_id', 'USE INDEX' => [ 'logging' => 'PRIMARY' ] ], + [ 'user' => [ 'JOIN', 'user_id = log_user' ] ] + $commentQuery['joins'] ); $this->outputLogStream( $result ); if ( $this->buffer == self::STREAM ) { @@ -395,8 +399,17 @@ class WikiExporter { Hooks::run( 'ModifyExportQuery', [ $this->db, &$tables, &$cond, &$opts, &$join ] ); + $commentQuery = CommentStore::newKey( 'rev_comment' )->getJoin(); + # Do the query! - $result = $this->db->select( $tables, '*', $cond, __METHOD__, $opts, $join ); + $result = $this->db->select( + $tables + $commentQuery['tables'], + [ '*' ] + $commentQuery['fields'], + $cond, + __METHOD__, + $opts, + $join + $commentQuery['joins'] + ); # Output dump results $this->outputPageStream( $result ); diff --git a/includes/export/XmlDumpWriter.php b/includes/export/XmlDumpWriter.php index 943408cc98..990f16dca0 100644 --- a/includes/export/XmlDumpWriter.php +++ b/includes/export/XmlDumpWriter.php @@ -218,8 +218,11 @@ class XmlDumpWriter { } if ( isset( $row->rev_deleted ) && ( $row->rev_deleted & Revision::DELETED_COMMENT ) ) { $out .= " " . Xml::element( 'comment', [ 'deleted' => 'deleted' ] ) . "\n"; - } elseif ( $row->rev_comment != '' ) { - $out .= " " . Xml::elementClean( 'comment', [], strval( $row->rev_comment ) ) . "\n"; + } else { + $comment = CommentStore::newKey( 'rev_comment' )->getComment( $row )->text; + if ( $comment != '' ) { + $out .= " " . Xml::elementClean( 'comment', [], strval( $comment ) ) . "\n"; + } } if ( isset( $row->rev_content_model ) && !is_null( $row->rev_content_model ) ) { @@ -299,8 +302,11 @@ class XmlDumpWriter { if ( $row->log_deleted & LogPage::DELETED_COMMENT ) { $out .= " " . Xml::element( 'comment', [ 'deleted' => 'deleted' ] ) . "\n"; - } elseif ( $row->log_comment != '' ) { - $out .= " " . Xml::elementClean( 'comment', null, strval( $row->log_comment ) ) . "\n"; + } else { + $comment = CommentStore::newKey( 'log_comment' )->getComment( $row )->text; + if ( $comment != '' ) { + $out .= " " . Xml::elementClean( 'comment', null, strval( $comment ) ) . "\n"; + } } $out .= " " . Xml::element( 'type', null, strval( $row->log_type ) ) . "\n"; diff --git a/includes/filerepo/file/ArchivedFile.php b/includes/filerepo/file/ArchivedFile.php index 6984d48c6d..758fb4b5a1 100644 --- a/includes/filerepo/file/ArchivedFile.php +++ b/includes/filerepo/file/ArchivedFile.php @@ -215,6 +215,8 @@ class ArchivedFile { /** * Fields in the filearchive table + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ static function selectFields() { @@ -232,14 +234,13 @@ class ArchivedFile { 'fa_media_type', 'fa_major_mime', 'fa_minor_mime', - 'fa_description', 'fa_user', 'fa_user_text', 'fa_timestamp', 'fa_deleted', 'fa_deleted_timestamp', /* Used by LocalFileRestoreBatch */ 'fa_sha1', - ]; + ] + CommentStore::newKey( 'fa_description' )->getFields(); } /** @@ -261,7 +262,9 @@ class ArchivedFile { $this->metadata = $row->fa_metadata; $this->mime = "$row->fa_major_mime/$row->fa_minor_mime"; $this->media_type = $row->fa_media_type; - $this->description = $row->fa_description; + $this->description = CommentStore::newKey( 'fa_description' ) + // Legacy because $row probably came from self::selectFields() + ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row )->text; $this->user = $row->fa_user; $this->user_text = $row->fa_user_text; $this->timestamp = $row->fa_timestamp; diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 33177d3f6f..904c932d58 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -193,6 +193,8 @@ class LocalFile extends File { /** * Fields in the image table + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ static function selectFields() { @@ -206,12 +208,11 @@ class LocalFile extends File { 'img_media_type', 'img_major_mime', 'img_minor_mime', - 'img_description', 'img_user', 'img_user_text', 'img_timestamp', 'img_sha1', - ]; + ] + CommentStore::newKey( 'img_description' )->getFields(); } /** @@ -1299,6 +1300,8 @@ class LocalFile extends File { function recordUpload2( $oldver, $comment, $pageText, $props = false, $timestamp = false, $user = null, $tags = [] ) { + global $wgCommentTableSchemaMigrationStage; + if ( is_null( $user ) ) { global $wgUser; $user = $wgUser; @@ -1334,6 +1337,9 @@ class LocalFile extends File { # Test to see if the row exists using INSERT IGNORE # 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 = new CommentStore( 'img_description' ); + list( $commentFields, $commentCallback ) = + $commentStore->insertWithTempTable( $dbw, $comment ); $dbw->insert( 'image', [ 'img_name' => $this->getName(), @@ -1345,17 +1351,16 @@ class LocalFile extends File { 'img_major_mime' => $this->major_mime, 'img_minor_mime' => $this->minor_mime, 'img_timestamp' => $timestamp, - 'img_description' => $comment, 'img_user' => $user->getId(), 'img_user_text' => $user->getName(), 'img_metadata' => $dbw->encodeBlob( $this->metadata ), 'img_sha1' => $this->sha1 - ], + ] + $commentFields, __METHOD__, 'IGNORE' ); - $reupload = ( $dbw->affectedRows() == 0 ); + if ( $reupload ) { if ( $allowTimeKludge ) { # Use LOCK IN SHARE MODE to ignore any transaction snapshotting @@ -1376,33 +1381,65 @@ class LocalFile extends File { } } + $tables = [ 'image' ]; + $fields = [ + 'oi_name' => 'img_name', + 'oi_archive_name' => $dbw->addQuotes( $oldver ), + 'oi_size' => 'img_size', + 'oi_width' => 'img_width', + 'oi_height' => 'img_height', + 'oi_bits' => 'img_bits', + 'oi_timestamp' => 'img_timestamp', + 'oi_user' => 'img_user', + 'oi_user_text' => 'img_user_text', + 'oi_metadata' => 'img_metadata', + 'oi_media_type' => 'img_media_type', + 'oi_major_mime' => 'img_major_mime', + 'oi_minor_mime' => 'img_minor_mime', + 'oi_sha1' => 'img_sha1', + ]; + $joins = []; + + if ( $wgCommentTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + $fields['oi_description'] = 'img_description'; + } + if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + $tables[] = 'image_comment_temp'; + $fields['oi_description_id'] = 'imgcomment_description_id'; + $joins['image_comment_temp'] = [ + $wgCommentTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', + [ 'imgcomment_name = img_name' ] + ]; + } + + if ( $wgCommentTableSchemaMigrationStage !== MIGRATION_OLD && + $wgCommentTableSchemaMigrationStage !== MIGRATION_NEW + ) { + // Upgrade any rows that are still old-style. Otherwise an upgrade + // might be missed if a deletion happens while the migration script + // is running. + $res = $dbw->select( + [ 'image', 'image_comment_temp' ], + [ 'img_name', 'img_description' ], + [ 'img_name' => $this->getName(), 'imgcomment_name' => null ], + __METHOD__, + [], + [ 'image_comment_temp' => [ 'LEFT JOIN', [ 'imgcomment_name = img_name' ] ] ] + ); + foreach ( $res as $row ) { + list( , $callback ) = $commentStore->insertWithTempTable( $dbw, $row->img_description ); + $callback( $row->img_name ); + } + } + # (T36993) Note: $oldver can be empty here, if the previous # version of the file was broken. Allow registration of the new # version to continue anyway, because that's better than having # an image that's not fixable by user operations. # Collision, this is an update of a file # Insert previous contents into oldimage - $dbw->insertSelect( 'oldimage', 'image', - [ - 'oi_name' => 'img_name', - 'oi_archive_name' => $dbw->addQuotes( $oldver ), - 'oi_size' => 'img_size', - 'oi_width' => 'img_width', - 'oi_height' => 'img_height', - 'oi_bits' => 'img_bits', - 'oi_timestamp' => 'img_timestamp', - 'oi_description' => 'img_description', - 'oi_user' => 'img_user', - 'oi_user_text' => 'img_user_text', - 'oi_metadata' => 'img_metadata', - 'oi_media_type' => 'img_media_type', - 'oi_major_mime' => 'img_major_mime', - 'oi_minor_mime' => 'img_minor_mime', - 'oi_sha1' => 'img_sha1' - ], - [ 'img_name' => $this->getName() ], - __METHOD__ - ); + $dbw->insertSelect( 'oldimage', $tables, $fields, + [ 'img_name' => $this->getName() ], __METHOD__, [], [], $joins ); # Update the current image row $dbw->update( 'image', @@ -1415,16 +1452,20 @@ class LocalFile extends File { 'img_major_mime' => $this->major_mime, 'img_minor_mime' => $this->minor_mime, 'img_timestamp' => $timestamp, - 'img_description' => $comment, 'img_user' => $user->getId(), 'img_user_text' => $user->getName(), 'img_metadata' => $dbw->encodeBlob( $this->metadata ), 'img_sha1' => $this->sha1 - ], + ] + $commentFields, [ 'img_name' => $this->getName() ], __METHOD__ ); + if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { + // So $commentCallback can insert the new row + $dbw->delete( 'image_comment_temp', [ 'imgcomment_name' => $this->getName() ], __METHOD__ ); + } } + $commentCallback( $this->getName() ); $descTitle = $this->getTitle(); $descId = $descTitle->getArticleID(); @@ -2255,8 +2296,16 @@ class LocalFileDeleteBatch { } protected function doDBInserts() { + global $wgCommentTableSchemaMigrationStage; + $now = time(); $dbw = $this->file->repo->getMasterDB(); + + $commentStoreImgDesc = new CommentStore( 'img_description' ); + $commentStoreOiDesc = new CommentStore( 'oi_description' ); + $commentStoreFaDesc = new CommentStore( 'fa_description' ); + $commentStoreFaReason = new CommentStore( 'fa_deleted_reason' ); + $encTimestamp = $dbw->addQuotes( $dbw->timestamp( $now ) ); $encUserId = $dbw->addQuotes( $this->user->getId() ); $encReason = $dbw->addQuotes( $this->reason ); @@ -2274,39 +2323,70 @@ class LocalFileDeleteBatch { } if ( $deleteCurrent ) { - $dbw->insertSelect( - 'filearchive', - 'image', - [ - 'fa_storage_group' => $encGroup, - 'fa_storage_key' => $dbw->conditional( - [ 'img_sha1' => '' ], - $dbw->addQuotes( '' ), - $dbw->buildConcat( [ "img_sha1", $encExt ] ) - ), - 'fa_deleted_user' => $encUserId, - 'fa_deleted_timestamp' => $encTimestamp, - 'fa_deleted_reason' => $encReason, - 'fa_deleted' => $this->suppress ? $bitfield : 0, - 'fa_name' => 'img_name', - 'fa_archive_name' => 'NULL', - 'fa_size' => 'img_size', - 'fa_width' => 'img_width', - 'fa_height' => 'img_height', - 'fa_metadata' => 'img_metadata', - 'fa_bits' => 'img_bits', - 'fa_media_type' => 'img_media_type', - 'fa_major_mime' => 'img_major_mime', - 'fa_minor_mime' => 'img_minor_mime', - 'fa_description' => 'img_description', - 'fa_user' => 'img_user', - 'fa_user_text' => 'img_user_text', - 'fa_timestamp' => 'img_timestamp', - 'fa_sha1' => 'img_sha1' - ], - [ 'img_name' => $this->file->getName() ], - __METHOD__ - ); + $tables = [ 'image' ]; + $fields = [ + 'fa_storage_group' => $encGroup, + 'fa_storage_key' => $dbw->conditional( + [ 'img_sha1' => '' ], + $dbw->addQuotes( '' ), + $dbw->buildConcat( [ "img_sha1", $encExt ] ) + ), + 'fa_deleted_user' => $encUserId, + 'fa_deleted_timestamp' => $encTimestamp, + 'fa_deleted' => $this->suppress ? $bitfield : 0, + 'fa_name' => 'img_name', + 'fa_archive_name' => 'NULL', + 'fa_size' => 'img_size', + 'fa_width' => 'img_width', + 'fa_height' => 'img_height', + 'fa_metadata' => 'img_metadata', + 'fa_bits' => 'img_bits', + 'fa_media_type' => 'img_media_type', + 'fa_major_mime' => 'img_major_mime', + 'fa_minor_mime' => 'img_minor_mime', + 'fa_user' => 'img_user', + 'fa_user_text' => 'img_user_text', + 'fa_timestamp' => 'img_timestamp', + 'fa_sha1' => 'img_sha1' + ]; + $joins = []; + + $fields += $commentStoreFaReason->insert( $dbw, $encReason ); + + if ( $wgCommentTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + $fields['fa_description'] = 'img_description'; + } + if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + $tables[] = 'image_comment_temp'; + $fields['fa_description_id'] = 'imgcomment_description_id'; + $joins['image_comment_temp'] = [ + $wgCommentTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', + [ 'imgcomment_name = img_name' ] + ]; + } + + if ( $wgCommentTableSchemaMigrationStage !== MIGRATION_OLD && + $wgCommentTableSchemaMigrationStage !== MIGRATION_NEW + ) { + // Upgrade any rows that are still old-style. Otherwise an upgrade + // might be missed if a deletion happens while the migration script + // is running. + $res = $dbw->select( + [ 'image', 'image_comment_temp' ], + [ 'img_name', 'img_description' ], + [ 'img_name' => $this->file->getName(), 'imgcomment_name' => null ], + __METHOD__, + [], + [ 'image_comment_temp' => [ 'LEFT JOIN', [ 'imgcomment_name = img_name' ] ] ] + ); + foreach ( $res as $row ) { + list( , $callback ) = $commentStoreImgDesc->insertWithTempTable( $dbw, $row->img_description ); + $callback( $row->img_name ); + } + } + + $dbw->insertSelect( 'filearchive', $tables, $fields, + [ 'img_name' => $this->file->getName() ], __METHOD__, [], [], $joins ); } if ( count( $oldRels ) ) { @@ -2321,34 +2401,38 @@ class LocalFileDeleteBatch { [ 'FOR UPDATE' ] ); $rowsInsert = []; - foreach ( $res as $row ) { - $rowsInsert[] = [ - // Deletion-specific fields - 'fa_storage_group' => 'deleted', - 'fa_storage_key' => ( $row->oi_sha1 === '' ) + if ( $res->numRows() ) { + $reason = $commentStoreFaReason->createComment( $dbw, $this->reason ); + foreach ( $res as $row ) { + // Legacy from OldLocalFile::selectFields() just above + $comment = $commentStoreOiDesc->getCommentLegacy( $dbw, $row ); + $rowsInsert[] = [ + // Deletion-specific fields + 'fa_storage_group' => 'deleted', + 'fa_storage_key' => ( $row->oi_sha1 === '' ) ? '' : "{$row->oi_sha1}{$dotExt}", - 'fa_deleted_user' => $this->user->getId(), - 'fa_deleted_timestamp' => $dbw->timestamp( $now ), - 'fa_deleted_reason' => $this->reason, - // Counterpart fields - 'fa_deleted' => $this->suppress ? $bitfield : $row->oi_deleted, - 'fa_name' => $row->oi_name, - 'fa_archive_name' => $row->oi_archive_name, - 'fa_size' => $row->oi_size, - 'fa_width' => $row->oi_width, - 'fa_height' => $row->oi_height, - 'fa_metadata' => $row->oi_metadata, - 'fa_bits' => $row->oi_bits, - 'fa_media_type' => $row->oi_media_type, - 'fa_major_mime' => $row->oi_major_mime, - 'fa_minor_mime' => $row->oi_minor_mime, - 'fa_description' => $row->oi_description, - 'fa_user' => $row->oi_user, - 'fa_user_text' => $row->oi_user_text, - 'fa_timestamp' => $row->oi_timestamp, - 'fa_sha1' => $row->oi_sha1 - ]; + 'fa_deleted_user' => $this->user->getId(), + 'fa_deleted_timestamp' => $dbw->timestamp( $now ), + // Counterpart fields + 'fa_deleted' => $this->suppress ? $bitfield : $row->oi_deleted, + 'fa_name' => $row->oi_name, + 'fa_archive_name' => $row->oi_archive_name, + 'fa_size' => $row->oi_size, + 'fa_width' => $row->oi_width, + 'fa_height' => $row->oi_height, + 'fa_metadata' => $row->oi_metadata, + 'fa_bits' => $row->oi_bits, + 'fa_media_type' => $row->oi_media_type, + 'fa_major_mime' => $row->oi_major_mime, + 'fa_minor_mime' => $row->oi_minor_mime, + 'fa_user' => $row->oi_user, + 'fa_user_text' => $row->oi_user_text, + 'fa_timestamp' => $row->oi_timestamp, + 'fa_sha1' => $row->oi_sha1 + ] + $commentStoreFaReason->insert( $dbw, $reason ) + + $commentStoreFaDesc->insert( $dbw, $comment ); + } } $dbw->insert( 'filearchive', $rowsInsert, __METHOD__ ); @@ -2356,6 +2440,8 @@ class LocalFileDeleteBatch { } function doDBDeletes() { + global $wgUpdateCompatibleMetadata; + $dbw = $this->file->repo->getMasterDB(); list( $oldRels, $deleteCurrent ) = $this->getOldRels(); @@ -2369,6 +2455,11 @@ class LocalFileDeleteBatch { if ( $deleteCurrent ) { $dbw->delete( 'image', [ 'img_name' => $this->file->getName() ], __METHOD__ ); + if ( $wgUpdateCompatibleMetadata > MIGRATION_OLD ) { + $dbw->delete( + 'image_comment_temp', [ 'imgcomment_name' => $this->file->getName() ], __METHOD__ + ); + } } } @@ -2537,6 +2628,11 @@ class LocalFileRestoreBatch { $lockOwnsTrx = $this->file->lock(); $dbw = $this->file->repo->getMasterDB(); + + $commentStoreImgDesc = new CommentStore( 'img_description' ); + $commentStoreOiDesc = new CommentStore( 'oi_description' ); + $commentStoreFaDesc = new CommentStore( 'fa_description' ); + $status = $this->file->repo->newGood(); $exists = (bool)$dbw->selectField( 'image', '1', @@ -2621,9 +2717,13 @@ class LocalFileRestoreBatch { ]; } + // Legacy from ArchivedFile::selectFields() just above + $comment = $commentStoreFaDesc->getCommentLegacy( $dbw, $row ); if ( $first && !$exists ) { // This revision will be published as the new current version $destRel = $this->file->getRel(); + list( $commentFields, $commentCallback ) = + $commentStoreImgDesc->insertWithTempTable( $dbw, $comment ); $insertCurrent = [ 'img_name' => $row->fa_name, 'img_size' => $row->fa_size, @@ -2634,12 +2734,11 @@ class LocalFileRestoreBatch { 'img_media_type' => $props['media_type'], 'img_major_mime' => $props['major_mime'], 'img_minor_mime' => $props['minor_mime'], - 'img_description' => $row->fa_description, 'img_user' => $row->fa_user, 'img_user_text' => $row->fa_user_text, 'img_timestamp' => $row->fa_timestamp, 'img_sha1' => $sha1 - ]; + ] + $commentFields; // The live (current) version cannot be hidden! if ( !$this->unsuppress && $row->fa_deleted ) { @@ -2671,7 +2770,6 @@ class LocalFileRestoreBatch { 'oi_width' => $row->fa_width, 'oi_height' => $row->fa_height, 'oi_bits' => $row->fa_bits, - 'oi_description' => $row->fa_description, 'oi_user' => $row->fa_user, 'oi_user_text' => $row->fa_user_text, 'oi_timestamp' => $row->fa_timestamp, @@ -2680,7 +2778,8 @@ class LocalFileRestoreBatch { 'oi_major_mime' => $props['major_mime'], 'oi_minor_mime' => $props['minor_mime'], 'oi_deleted' => $this->unsuppress ? 0 : $row->fa_deleted, - 'oi_sha1' => $sha1 ]; + 'oi_sha1' => $sha1 + ] + $commentStoreOiDesc->insert( $dbw, $comment ); } $deleteIds[] = $row->fa_id; @@ -2738,6 +2837,7 @@ 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/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php index dfaae731c1..b46e1e466f 100644 --- a/includes/filerepo/file/OldLocalFile.php +++ b/includes/filerepo/file/OldLocalFile.php @@ -103,6 +103,8 @@ class OldLocalFile extends LocalFile { /** * Fields in the oldimage table + * @todo Deprecate this in favor of a method that returns tables and joins + * as well, and use CommentStore::getJoin(). * @return array */ static function selectFields() { @@ -117,13 +119,12 @@ class OldLocalFile extends LocalFile { 'oi_media_type', 'oi_major_mime', 'oi_minor_mime', - 'oi_description', 'oi_user', 'oi_user_text', 'oi_timestamp', 'oi_deleted', 'oi_sha1', - ]; + ] + CommentStore::newKey( 'oi_description' )->getFields(); } /** @@ -367,6 +368,7 @@ class OldLocalFile extends LocalFile { return false; } + $commentFields = CommentStore::newKey( 'oi_description' )->insert( $dbw, $comment ); $dbw->insert( 'oldimage', [ 'oi_name' => $this->getName(), @@ -376,7 +378,6 @@ class OldLocalFile extends LocalFile { 'oi_height' => intval( $props['height'] ), 'oi_bits' => $props['bits'], 'oi_timestamp' => $dbw->timestamp( $timestamp ), - 'oi_description' => $comment, 'oi_user' => $user->getId(), 'oi_user_text' => $user->getName(), 'oi_metadata' => $props['metadata'], @@ -384,7 +385,7 @@ class OldLocalFile extends LocalFile { 'oi_major_mime' => $props['major_mime'], 'oi_minor_mime' => $props['minor_mime'], 'oi_sha1' => $props['sha1'], - ], __METHOD__ + ] + $commentFields, __METHOD__ ); return true; diff --git a/includes/import/WikiImporter.php b/includes/import/WikiImporter.php index 209970971e..90660797f1 100644 --- a/includes/import/WikiImporter.php +++ b/includes/import/WikiImporter.php @@ -813,7 +813,7 @@ class WikiImporter { $this->debug( "Enter revision handler" ); $revisionInfo = []; - $normalFields = [ 'id', 'timestamp', 'comment', 'minor', 'model', 'format', 'text' ]; + $normalFields = [ 'id', 'timestamp', 'comment', 'minor', 'model', 'format', 'text', 'sha1' ]; $skip = false; @@ -916,6 +916,9 @@ class WikiImporter { } else { $revision->setUsername( 'Unknown user' ); } + if ( isset( $revisionInfo['sha1'] ) ) { + $revision->setSha1Base36( $revisionInfo['sha1'] ); + } $revision->setNoUpdates( $this->mNoUpdates ); return $this->revisionCallback( $revision ); diff --git a/includes/import/WikiRevision.php b/includes/import/WikiRevision.php index f6becb9c92..93a92ef8b5 100644 --- a/includes/import/WikiRevision.php +++ b/includes/import/WikiRevision.php @@ -607,11 +607,12 @@ class WikiRevision { $pageId = $page->getId(); $created = false; + // Note: sha1 has been in XML dumps since 2012. If you have an + // older dump, the duplicate detection here won't work. $prior = $dbw->selectField( 'revision', '1', [ 'rev_page' => $pageId, 'rev_timestamp' => $dbw->timestamp( $this->timestamp ), - 'rev_user_text' => $userText, - 'rev_comment' => $this->getComment() ], + 'rev_sha1' => $this->sha1base36 ], __METHOD__ ); if ( $prior ) { @@ -708,7 +709,6 @@ class WikiRevision { 'log_timestamp' => $dbw->timestamp( $this->timestamp ), 'log_namespace' => $this->getTitle()->getNamespace(), 'log_title' => $this->getTitle()->getDBkey(), - 'log_comment' => $this->getComment(), # 'log_user_text' => $this->user_text, 'log_params' => $this->params ], __METHOD__ @@ -730,9 +730,8 @@ class WikiRevision { 'log_user_text' => $userText, 'log_namespace' => $this->getTitle()->getNamespace(), 'log_title' => $this->getTitle()->getDBkey(), - 'log_comment' => $this->getComment(), 'log_params' => $this->params - ]; + ] + CommentStore::newKey( 'log_comment' )->insert( $dbw, $this->getComment() ); $dbw->insert( 'logging', $data, __METHOD__ ); return true; diff --git a/includes/installer/DatabaseUpdater.php b/includes/installer/DatabaseUpdater.php index b832d450ee..645fa8aa3d 100644 --- a/includes/installer/DatabaseUpdater.php +++ b/includes/installer/DatabaseUpdater.php @@ -1190,4 +1190,25 @@ abstract class DatabaseUpdater { $wgContentHandlerUseDB = $this->holdContentHandlerUseDB; } } + + /** + * Migrate comments to the new 'comment' table + * @since 1.30 + */ + protected function migrateComments() { + global $wgCommentTableSchemaMigrationStage; + if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_NEW && + !$this->updateRowExists( 'MigrateComments' ) + ) { + $this->output( + "Migrating comments to the 'comments' table, printing progress markers. For large\n" . + "databases, you may want to hit Ctrl-C and do this manually with\n" . + "maintenance/migrateComments.php.\n" + ); + $task = $this->maintenance->runChild( 'MigrateComments', 'migrateComments.php' ); + $task->execute(); + $this->output( "done.\n" ); + } + } + } diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php index 7b51ed70c1..c5919749aa 100644 --- a/includes/installer/MysqlUpdater.php +++ b/includes/installer/MysqlUpdater.php @@ -326,6 +326,8 @@ class MysqlUpdater extends DatabaseUpdater { 'patch-user_former_groups-fix-pk.sql' ], [ 'renameIndex', 'user_properties', 'user_properties_user_property', 'PRIMARY', false, 'patch-user_properties-fix-pk.sql' ], + [ 'addTable', 'comment', 'patch-comment-table.sql' ], + [ 'migrateComments' ], ]; } diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index d8db6a2d9d..e5a5c9445c 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -455,6 +455,32 @@ class PostgresUpdater extends DatabaseUpdater { // 1.30 [ 'modifyField', 'image', 'img_media_type', 'patch-add-3d.sql' ], + [ 'setDefault', 'revision', 'rev_comment', '' ], + [ 'changeNullableField', 'revision', 'rev_comment', 'NOT NULL', true ], + [ 'setDefault', 'archive', 'ar_comment', '' ], + [ 'changeNullableField', 'archive', 'ar_comment', 'NOT NULL', true ], + [ 'addPgField', 'archive', 'ar_comment_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'ipblocks', 'ipb_reason', '' ], + [ 'addPgField', 'ipblocks', 'ipb_reason_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'image', 'img_description', '' ], + [ 'setDefault', 'oldimage', 'oi_description', '' ], + [ 'changeNullableField', 'oldimage', 'oi_description', 'NOT NULL', true ], + [ 'addPgField', 'oldimage', 'oi_description_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'filearchive', 'fa_deleted_reason', '' ], + [ 'changeNullableField', 'filearchive', 'fa_deleted_reason', 'NOT NULL', true ], + [ 'addPgField', 'filearchive', 'fa_deleted_reason_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'filearchive', 'fa_description', '' ], + [ 'addPgField', 'filearchive', 'fa_description_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'recentchanges', 'rc_comment', '' ], + [ 'changeNullableField', 'recentchanges', 'rc_comment', 'NOT NULL', true ], + [ 'addPgField', 'recentchanges', 'rc_comment_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'logging', 'log_comment', '' ], + [ 'changeNullableField', 'logging', 'log_comment', 'NOT NULL', true ], + [ 'addPgField', 'logging', 'log_comment_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'setDefault', 'protected_titles', 'pt_reason', '' ], + [ 'changeNullableField', 'protected_titles', 'pt_reason', 'NOT NULL', true ], + [ 'addPgField', 'protected_titles', 'pt_reason_id', 'INTEGER NOT NULL DEFAULT 0' ], + [ 'addTable', 'comment', 'patch-comment-table.sql' ], ]; } @@ -761,7 +787,7 @@ END; } } - protected function changeNullableField( $table, $field, $null ) { + protected function changeNullableField( $table, $field, $null, $update = false ) { $fi = $this->db->fieldInfo( $table, $field ); if ( is_null( $fi ) ) { $this->output( "...ERROR: expected column $table.$field to exist\n" ); @@ -771,6 +797,9 @@ END; # # It's NULL - does it need to be NOT NULL? if ( 'NOT NULL' === $null ) { $this->output( "Changing '$table.$field' to not allow NULLs\n" ); + if ( $update ) { + $this->db->query( "UPDATE $table SET $field = DEFAULT WHERE $field IS NULL" ); + } $this->db->query( "ALTER TABLE $table ALTER $field SET NOT NULL" ); } else { $this->output( "...column '$table.$field' is already set as NULL\n" ); diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php index 95014a4b8e..e79dcb189c 100644 --- a/includes/installer/SqliteUpdater.php +++ b/includes/installer/SqliteUpdater.php @@ -190,6 +190,8 @@ class SqliteUpdater extends DatabaseUpdater { 'patch-user_former_groups-fix-pk.sql' ], [ 'renameIndex', 'user_properties', 'user_properties_user_property', 'PRIMARY', false, 'patch-user_properties-fix-pk.sql' ], + [ 'addTable', 'comment', 'patch-comment-table.sql' ], + [ 'migrateComments' ], ]; } diff --git a/includes/logging/LogEntry.php b/includes/logging/LogEntry.php index fa94fe5b23..6587304ffb 100644 --- a/includes/logging/LogEntry.php +++ b/includes/logging/LogEntry.php @@ -170,19 +170,21 @@ class DatabaseLogEntry extends LogEntryBase { * @return array */ public static function getSelectQueryData() { - $tables = [ 'logging', 'user' ]; + $commentQuery = CommentStore::newKey( 'log_comment' )->getJoin(); + + $tables = [ 'logging', 'user' ] + $commentQuery['tables']; $fields = [ 'log_id', 'log_type', 'log_action', 'log_timestamp', 'log_user', 'log_user_text', 'log_namespace', 'log_title', // unused log_page - 'log_comment', 'log_params', 'log_deleted', + 'log_params', 'log_deleted', 'user_id', 'user_name', 'user_editcount', - ]; + ] + $commentQuery['fields']; $joins = [ // IPs don't have an entry in user table 'user' => [ 'LEFT JOIN', 'log_user=user_id' ], - ]; + ] + $commentQuery['joins']; return [ 'tables' => $tables, @@ -322,7 +324,7 @@ class DatabaseLogEntry extends LogEntryBase { } public function getComment() { - return $this->row->log_comment; + return CommentStore::newKey( 'log_comment' )->getComment( $this->row )->text; } public function getDeleted() { @@ -380,7 +382,9 @@ class RCDatabaseLogEntry extends DatabaseLogEntry { } public function getComment() { - return $this->row->rc_comment; + return CommentStore::newKey( 'rc_comment' ) + // Legacy because the row probably used RecentChange::selectFields() + ->getCommentLegacy( wfGetDB( DB_REPLICA ), $this->row )->text; } public function getDeleted() { @@ -624,12 +628,12 @@ class ManualLogEntry extends LogEntryBase { 'log_namespace' => $this->getTarget()->getNamespace(), 'log_title' => $this->getTarget()->getDBkey(), 'log_page' => $this->getTarget()->getArticleID(), - 'log_comment' => $comment, 'log_params' => LogEntryBase::makeParamBlob( $params ), ]; if ( isset( $this->deleted ) ) { $data['log_deleted'] = $this->deleted; } + $data += CommentStore::newKey( 'log_comment' )->insert( $dbw, $comment ); $dbw->insert( 'logging', $data, __METHOD__ ); $this->id = $dbw->insertId(); diff --git a/includes/logging/LogPage.php b/includes/logging/LogPage.php index a085e3e114..257f76d0bb 100644 --- a/includes/logging/LogPage.php +++ b/includes/logging/LogPage.php @@ -104,9 +104,9 @@ class LogPage { 'log_namespace' => $this->target->getNamespace(), 'log_title' => $this->target->getDBkey(), 'log_page' => $this->target->getArticleID(), - 'log_comment' => $this->comment, 'log_params' => $this->params ]; + $data += CommentStore::newKey( 'log_comment' )->insert( $dbw, $this->comment ); $dbw->insert( 'logging', $data, __METHOD__ ); $newId = $dbw->insertId(); diff --git a/includes/page/PageArchive.php b/includes/page/PageArchive.php index 11e1a30db8..f6580e9389 100644 --- a/includes/page/PageArchive.php +++ b/includes/page/PageArchive.php @@ -171,20 +171,21 @@ class PageArchive { /** * List the revisions of the given page. Returns result wrapper with - * (ar_minor_edit, ar_timestamp, ar_user, ar_user_text, ar_comment) fields. + * various archive table fields. * * @return ResultWrapper */ public function listRevisions() { $dbr = wfGetDB( DB_REPLICA ); + $commentQuery = CommentStore::newKey( 'ar_comment' )->getJoin(); - $tables = [ 'archive' ]; + $tables = [ 'archive' ] + $commentQuery['tables']; $fields = [ 'ar_minor_edit', 'ar_timestamp', 'ar_user', 'ar_user_text', - 'ar_comment', 'ar_len', 'ar_deleted', 'ar_rev_id', 'ar_sha1', + 'ar_len', 'ar_deleted', 'ar_rev_id', 'ar_sha1', 'ar_page_id' - ]; + ] + $commentQuery['fields']; if ( $this->config->get( 'ContentHandlerUseDB' ) ) { $fields[] = 'ar_content_format'; @@ -196,7 +197,7 @@ class PageArchive { $options = [ 'ORDER BY' => 'ar_timestamp DESC' ]; - $join_conds = []; + $join_conds = [] + $commentQuery['joins']; ChangeTags::modifyDisplayQuery( $tables, @@ -248,11 +249,13 @@ class PageArchive { */ public function getRevision( $timestamp ) { $dbr = wfGetDB( DB_REPLICA ); + $commentQuery = CommentStore::newKey( 'ar_comment' )->getJoin(); + + $tables = [ 'archive' ] + $commentQuery['tables']; $fields = [ 'ar_rev_id', 'ar_text', - 'ar_comment', 'ar_user', 'ar_user_text', 'ar_timestamp', @@ -262,19 +265,27 @@ class PageArchive { 'ar_deleted', 'ar_len', 'ar_sha1', - ]; + ] + $commentQuery['fields']; if ( $this->config->get( 'ContentHandlerUseDB' ) ) { $fields[] = 'ar_content_format'; $fields[] = 'ar_content_model'; } - $row = $dbr->selectRow( 'archive', + $join_conds = [] + $commentQuery['joins']; + + $row = $dbr->selectRow( + $tables, $fields, - [ 'ar_namespace' => $this->title->getNamespace(), + [ + 'ar_namespace' => $this->title->getNamespace(), 'ar_title' => $this->title->getDBkey(), - 'ar_timestamp' => $dbr->timestamp( $timestamp ) ], - __METHOD__ ); + 'ar_timestamp' => $dbr->timestamp( $timestamp ) + ], + __METHOD__, + [], + $join_conds + ); if ( $row ) { return Revision::newFromArchiveRow( $row, [ 'title' => $this->title ] ); @@ -552,12 +563,15 @@ class PageArchive { $oldWhere['ar_timestamp'] = array_map( [ &$dbw, 'timestamp' ], $timestamps ); } + $commentQuery = CommentStore::newKey( 'ar_comment' )->getJoin(); + + $tables = [ 'archive', 'revision' ] + $commentQuery['tables']; + $fields = [ 'ar_id', 'ar_rev_id', 'rev_id', 'ar_text', - 'ar_comment', 'ar_user', 'ar_user_text', 'ar_timestamp', @@ -568,24 +582,28 @@ class PageArchive { 'ar_page_id', 'ar_len', 'ar_sha1' - ]; + ] + $commentQuery['fields']; if ( $this->config->get( 'ContentHandlerUseDB' ) ) { $fields[] = 'ar_content_format'; $fields[] = 'ar_content_model'; } + $join_conds = [ + 'revision' => [ 'LEFT JOIN', 'ar_rev_id=rev_id' ], + ] + $commentQuery['joins']; + /** * Select each archived revision... */ $result = $dbw->select( - [ 'archive', 'revision' ], + $tables, $fields, $oldWhere, __METHOD__, /* options */ [ 'ORDER BY' => 'ar_timestamp' ], - [ 'revision' => [ 'LEFT JOIN', 'ar_rev_id=rev_id' ] ] + $join_conds ); $rev_count = $result->numRows(); diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index edccc66750..790845e35e 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -2484,6 +2484,7 @@ class WikiPage implements Page, IDBAccessObject { $cascade = false; if ( $limit['create'] != '' ) { + $commentFields = CommentStore::newKey( 'pt_reason' )->insert( $dbw, $reason ); $dbw->replace( 'protected_titles', [ [ 'pt_namespace', 'pt_title' ] ], [ @@ -2493,8 +2494,7 @@ class WikiPage implements Page, IDBAccessObject { 'pt_timestamp' => $dbw->timestamp(), 'pt_expiry' => $dbw->encodeExpiry( $expiry['create'] ), 'pt_user' => $user->getId(), - 'pt_reason' => $reason, - ], __METHOD__ + ] + $commentFields, __METHOD__ ); $logParamsDetails[] = [ 'type' => 'create', @@ -2746,7 +2746,7 @@ class WikiPage implements Page, IDBAccessObject { $reason, $suppress = false, $u1 = null, $u2 = null, &$error = '', User $user = null, $tags = [], $logsubtype = 'delete' ) { - global $wgUser, $wgContentHandlerUseDB; + global $wgUser, $wgContentHandlerUseDB, $wgCommentTableSchemaMigrationStage; wfDebug( __METHOD__ . "\n" ); @@ -2810,6 +2810,9 @@ class WikiPage implements Page, IDBAccessObject { $content = null; } + $revCommentStore = new CommentStore( 'rev_comment' ); + $arCommentStore = new CommentStore( 'ar_comment' ); + $fields = Revision::selectFields(); $bitfield = false; @@ -2827,20 +2830,23 @@ class WikiPage implements Page, IDBAccessObject { // the rev_deleted field, which is reserved for this purpose. // Get all of the page revisions + $commentQuery = $revCommentStore->getJoin(); $res = $dbw->select( - 'revision', - $fields, + [ 'revision' ] + $commentQuery['tables'], + $fields + $commentQuery['fields'], [ 'rev_page' => $id ], __METHOD__, - 'FOR UPDATE' + 'FOR UPDATE', + $commentQuery['joins'] ); // Build their equivalent archive rows $rowsInsert = []; + $revids = []; foreach ( $res as $row ) { + $comment = $revCommentStore->getComment( $row ); $rowInsert = [ 'ar_namespace' => $namespace, 'ar_title' => $dbKey, - 'ar_comment' => $row->rev_comment, 'ar_user' => $row->rev_user, 'ar_user_text' => $row->rev_user_text, 'ar_timestamp' => $row->rev_timestamp, @@ -2854,12 +2860,13 @@ class WikiPage implements Page, IDBAccessObject { 'ar_page_id' => $id, 'ar_deleted' => $suppress ? $bitfield : $row->rev_deleted, 'ar_sha1' => $row->rev_sha1, - ]; + ] + $arCommentStore->insert( $dbw, $comment ); if ( $wgContentHandlerUseDB ) { $rowInsert['ar_content_model'] = $row->rev_content_model; $rowInsert['ar_content_format'] = $row->rev_content_format; } $rowsInsert[] = $rowInsert; + $revids[] = $row->rev_id; } // Copy them into the archive table $dbw->insert( 'archive', $rowsInsert, __METHOD__ ); @@ -2874,6 +2881,9 @@ class WikiPage implements Page, IDBAccessObject { // Now that it's safely backed up, delete it $dbw->delete( 'page', [ 'page_id' => $id ], __METHOD__ ); $dbw->delete( 'revision', [ 'rev_page' => $id ], __METHOD__ ); + if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { + $dbw->delete( 'revision_comment_temp', [ 'revcomment_rev' => $revids ], __METHOD__ ); + } // Log the deletion, if the page was suppressed, put it in the suppression log instead $logtype = $suppress ? 'suppress' : 'delete'; diff --git a/includes/rcfeed/IRCColourfulRCFeedFormatter.php b/includes/rcfeed/IRCColourfulRCFeedFormatter.php index ddea69511a..10ba83fc19 100644 --- a/includes/rcfeed/IRCColourfulRCFeedFormatter.php +++ b/includes/rcfeed/IRCColourfulRCFeedFormatter.php @@ -89,7 +89,9 @@ class IRCColourfulRCFeedFormatter implements RCFeedFormatter { ) ); $flag = $attribs['rc_log_action']; } else { - $comment = self::cleanupForIRC( $attribs['rc_comment'] ); + $comment = self::cleanupForIRC( + CommentStore::newKey( 'rc_comment' )->getComment( $attribs )->text + ); $flag = ''; if ( !$attribs['rc_patrolled'] && ( $wgUseRCPatrol || $attribs['rc_type'] == RC_NEW && $wgUseNPPatrol ) diff --git a/includes/revisiondelete/RevDelLogItem.php b/includes/revisiondelete/RevDelLogItem.php index 047d6cf10e..998c695f17 100644 --- a/includes/revisiondelete/RevDelLogItem.php +++ b/includes/revisiondelete/RevDelLogItem.php @@ -102,8 +102,9 @@ class RevDelLogItem extends RevDelItem { // User links and action text $action = $formatter->getActionText(); // Comment + $comment = CommentStore::newKey( 'log_comment' )->getComment( $this->row )->text; $comment = $this->list->getLanguage()->getDirMark() - . Linker::commentBlock( $this->row->log_comment ); + . Linker::commentBlock( $comment ); if ( LogEventsList::isDeleted( $this->row, LogPage::DELETED_COMMENT ) ) { $comment = '' . $comment . ''; @@ -135,7 +136,7 @@ class RevDelLogItem extends RevDelItem { } if ( LogEventsList::userCan( $this->row, LogPage::DELETED_COMMENT, $user ) ) { $ret += [ - 'comment' => $this->row->log_comment, + 'comment' => CommentStore::newKey( 'log_comment' )->getComment( $this->row )->text, ]; } diff --git a/includes/revisiondelete/RevDelLogList.php b/includes/revisiondelete/RevDelLogList.php index 1932778115..ceb97e4d65 100644 --- a/includes/revisiondelete/RevDelLogList.php +++ b/includes/revisiondelete/RevDelLogList.php @@ -63,7 +63,11 @@ class RevDelLogList extends RevDelList { public function doQuery( $db ) { $ids = array_map( 'intval', $this->ids ); - return $db->select( 'logging', [ + $commentQuery = CommentStore::getKey( 'log_comment' )->getJoin(); + + return $db->select( + [ 'logging' ] + $commentQuery['tables'], + [ 'log_id', 'log_type', 'log_action', @@ -73,13 +77,13 @@ class RevDelLogList extends RevDelList { 'log_namespace', 'log_title', 'log_page', - 'log_comment', 'log_params', 'log_deleted' - ], + ] + $commentQuery['fields'], [ 'log_id' => $ids ], __METHOD__, - [ 'ORDER BY' => 'log_id DESC' ] + [ 'ORDER BY' => 'log_id DESC' ], + $commentQuery['joins'] ); } diff --git a/includes/specials/SpecialNewpages.php b/includes/specials/SpecialNewpages.php index ede4898321..61590d7567 100644 --- a/includes/specials/SpecialNewpages.php +++ b/includes/specials/SpecialNewpages.php @@ -299,7 +299,7 @@ class SpecialNewpages extends IncludableSpecialPage { */ protected function revisionFromRcResult( stdClass $result ) { return new Revision( [ - 'comment' => $result->rc_comment, + 'comment' => CommentStore::newKey( 'rc_comment' )->getComment( $result )->text, 'deleted' => $result->rc_deleted, 'user_text' => $result->rc_user_text, 'user' => $result->rc_user, diff --git a/includes/specials/pagers/BlockListPager.php b/includes/specials/pagers/BlockListPager.php index 51e446d593..2206be8f01 100644 --- a/includes/specials/pagers/BlockListPager.php +++ b/includes/specials/pagers/BlockListPager.php @@ -173,6 +173,7 @@ class BlockListPager extends TablePager { break; case 'ipb_reason': + $value = CommentStore::newKey( 'ipb_reason' )->getComment( $row )->text; $formatted = Linker::formatComment( $value ); break; @@ -208,8 +209,10 @@ class BlockListPager extends TablePager { } function getQueryInfo() { + $commentQuery = CommentStore::newKey( 'ipb_reason' )->getJoin(); + $info = [ - 'tables' => [ 'ipblocks', 'user' ], + 'tables' => [ 'ipblocks', 'user' ] + $commentQuery['tables'], 'fields' => [ 'ipb_id', 'ipb_address', @@ -217,7 +220,6 @@ class BlockListPager extends TablePager { 'ipb_by', 'ipb_by_text', 'by_user_name' => 'user_name', - 'ipb_reason', 'ipb_timestamp', 'ipb_auto', 'ipb_anon_only', @@ -229,9 +231,9 @@ class BlockListPager extends TablePager { 'ipb_deleted', 'ipb_block_email', 'ipb_allow_usertalk', - ], + ] + $commentQuery['fields'], 'conds' => $this->conds, - 'join_conds' => [ 'user' => [ 'LEFT JOIN', 'user_id = ipb_by' ] ] + 'join_conds' => [ 'user' => [ 'LEFT JOIN', 'user_id = ipb_by' ] ] + $commentQuery['joins'] ]; # Filter out any expired blocks diff --git a/includes/specials/pagers/DeletedContribsPager.php b/includes/specials/pagers/DeletedContribsPager.php index 43d7ad40c7..38a332e6a1 100644 --- a/includes/specials/pagers/DeletedContribsPager.php +++ b/includes/specials/pagers/DeletedContribsPager.php @@ -69,14 +69,17 @@ class DeletedContribsPager extends IndexPager { ' != ' . Revision::SUPPRESSED_USER; } + $commentQuery = CommentStore::newKey( 'ar_comment' )->getJoin(); + return [ - 'tables' => [ 'archive' ], + 'tables' => [ 'archive' ] + $commentQuery['tables'], 'fields' => [ - 'ar_rev_id', 'ar_namespace', 'ar_title', 'ar_timestamp', 'ar_comment', + 'ar_rev_id', 'ar_namespace', 'ar_title', 'ar_timestamp', 'ar_minor_edit', 'ar_user', 'ar_user_text', 'ar_deleted' - ], + ] + $commentQuery['fields'], 'conds' => $conds, - 'options' => [ 'USE INDEX' => $index ] + 'options' => [ 'USE INDEX' => [ 'archive' => $index ] ], + 'join_conds' => $commentQuery['joins'], ]; } @@ -253,7 +256,7 @@ class DeletedContribsPager extends IndexPager { $rev = new Revision( [ 'title' => $page, 'id' => $row->ar_rev_id, - 'comment' => $row->ar_comment, + 'comment' => CommentStore::newKey( 'ar_comment' )->getComment( $row )->text, 'user' => $row->ar_user, 'user_text' => $row->ar_user_text, 'timestamp' => $row->ar_timestamp, diff --git a/includes/specials/pagers/ImageListPager.php b/includes/specials/pagers/ImageListPager.php index 47b059b765..813d1d44c0 100644 --- a/includes/specials/pagers/ImageListPager.php +++ b/includes/specials/pagers/ImageListPager.php @@ -244,7 +244,9 @@ class ImageListPager extends TablePager { $prefix = $table === 'oldimage' ? 'oi' : 'img'; $tables = [ $table ]; - $fields = array_keys( $this->getFieldNames() ); + $fields = $this->getFieldNames(); + unset( $fields['img_description'] ); + $fields = array_keys( $fields ); if ( $table === 'oldimage' ) { foreach ( $fields as $id => &$field ) { @@ -264,6 +266,13 @@ class ImageListPager extends TablePager { $options = $join_conds = []; + # Description field + $commentQuery = CommentStore::newKey( $prefix . '_description' )->getJoin(); + $tables += $commentQuery['tables']; + $fields += $commentQuery['fields']; + $join_conds += $commentQuery['joins']; + $fields['description_field'] = "'{$prefix}_description'"; + # Depends on $wgMiserMode # Will also not happen if mShowAll is true. if ( isset( $this->mFieldNames['count'] ) ) { @@ -497,6 +506,8 @@ class ImageListPager extends TablePager { case 'img_size': return htmlspecialchars( $this->getLanguage()->formatSize( $value ) ); case 'img_description': + $field = $this->mCurrentRow->description_field; + $value = CommentStore::newKey( $field )->getComment( $this->mCurrentRow )->text; return Linker::formatComment( $value ); case 'count': return $this->getLanguage()->formatNum( intval( $value ) + 1 ); diff --git a/includes/specials/pagers/NewPagesPager.php b/includes/specials/pagers/NewPagesPager.php index dafd244ee5..53362d9cbc 100644 --- a/includes/specials/pagers/NewPagesPager.php +++ b/includes/specials/pagers/NewPagesPager.php @@ -90,15 +90,17 @@ class NewPagesPager extends ReverseChronologicalPager { $conds['page_is_redirect'] = 0; } + $commentQuery = CommentStore::newKey( 'rc_comment' )->getJoin(); + // Allow changes to the New Pages query - $tables = [ 'recentchanges', 'page' ]; + $tables = [ 'recentchanges', 'page' ] + $commentQuery['tables']; $fields = [ 'rc_namespace', 'rc_title', 'rc_cur_id', 'rc_user', 'rc_user_text', - 'rc_comment', 'rc_timestamp', 'rc_patrolled', 'rc_id', 'rc_deleted', + 'rc_timestamp', 'rc_patrolled', 'rc_id', 'rc_deleted', 'length' => 'page_len', 'rev_id' => 'page_latest', 'rc_this_oldid', 'page_namespace', 'page_title' - ]; - $join_conds = [ 'page' => [ 'INNER JOIN', 'page_id=rc_cur_id' ] ]; + ] + $commentQuery['fields']; + $join_conds = [ 'page' => [ 'INNER JOIN', 'page_id=rc_cur_id' ] ] + $commentQuery['joins']; // Avoid PHP 7.1 warning from passing $this by reference $pager = $this; diff --git a/includes/specials/pagers/ProtectedPagesPager.php b/includes/specials/pagers/ProtectedPagesPager.php index 823b5da403..20b44d2150 100644 --- a/includes/specials/pagers/ProtectedPagesPager.php +++ b/includes/specials/pagers/ProtectedPagesPager.php @@ -236,6 +236,7 @@ class ProtectedPagesPager extends TablePager { LogPage::DELETED_COMMENT, $this->getUser() ) ) { + $value = CommentStore::newKey( 'log_comment' )->getComment( $row )->text; $formatted = Linker::formatComment( $value !== null ? $value : '' ); } else { $formatted = $this->msg( 'rev-deleted-comment' )->escaped(); @@ -284,8 +285,10 @@ class ProtectedPagesPager extends TablePager { $conds[] = 'page_namespace=' . $this->mDb->addQuotes( $this->namespace ); } + $commentQuery = CommentStore::newKey( 'log_comment' )->getJoin(); + return [ - 'tables' => [ 'page', 'page_restrictions', 'log_search', 'logging' ], + 'tables' => [ 'page', 'page_restrictions', 'log_search', 'logging' ] + $commentQuery['tables'], 'fields' => [ 'pr_id', 'page_namespace', @@ -297,9 +300,8 @@ class ProtectedPagesPager extends TablePager { 'pr_cascade', 'log_timestamp', 'log_user', - 'log_comment', 'log_deleted', - ], + ] + $commentQuery['fields'], 'conds' => $conds, 'join_conds' => [ 'log_search' => [ @@ -312,7 +314,7 @@ class ProtectedPagesPager extends TablePager { 'ls_log_id = log_id' ] ] - ] + ] + $commentQuery['joins'] ]; } diff --git a/maintenance/archives/patch-comment-table.sql b/maintenance/archives/patch-comment-table.sql new file mode 100644 index 0000000000..60c58d6e68 --- /dev/null +++ b/maintenance/archives/patch-comment-table.sql @@ -0,0 +1,59 @@ +-- +-- patch-comment-table.sql +-- +-- T166732. Add a `comment` table and various columns (and temporary tables) to reference it. + +CREATE TABLE /*_*/comment ( + comment_id bigint unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + comment_hash INT NOT NULL, + comment_text BLOB NOT NULL, + comment_data BLOB +) /*$wgDBTableOptions*/; +CREATE INDEX /*i*/comment_hash ON comment (comment_hash); + +CREATE TABLE /*_*/revision_comment_temp ( + revcomment_rev int unsigned NOT NULL, + revcomment_comment_id bigint unsigned NOT NULL, + PRIMARY KEY (revcomment_rev, revcomment_comment_id) +) /*$wgDBTableOptions*/; +CREATE UNIQUE INDEX /*i*/revcomment_rev ON /*_*/revision_comment_temp (revcomment_rev); + +CREATE TABLE /*_*/image_comment_temp ( + imgcomment_name varchar(255) binary NOT NULL, + imgcomment_description_id bigint unsigned NOT NULL, + PRIMARY KEY (imgcomment_name, imgcomment_description_id) +) /*$wgDBTableOptions*/; +CREATE UNIQUE INDEX /*i*/imgcomment_name ON /*_*/image_comment_temp (imgcomment_name); + +ALTER TABLE /*_*/revision + ALTER COLUMN rev_comment SET DEFAULT ''; + +ALTER TABLE /*_*/archive + ALTER COLUMN ar_comment SET DEFAULT '', + ADD COLUMN ar_comment_id bigint unsigned NOT NULL DEFAULT 0 AFTER ar_comment; + +ALTER TABLE /*_*/ipblocks + ALTER COLUMN ipb_reason SET DEFAULT '', + ADD COLUMN ipb_reason_id bigint unsigned NOT NULL DEFAULT 0 AFTER ipb_reason; + +ALTER TABLE /*_*/image + ALTER COLUMN img_description SET DEFAULT ''; + +ALTER TABLE /*_*/oldimage + ALTER COLUMN oi_description SET DEFAULT '', + ADD COLUMN oi_description_id bigint unsigned NOT NULL DEFAULT 0 AFTER oi_description; + +ALTER TABLE /*_*/filearchive + ADD COLUMN fa_deleted_reason_id bigint unsigned NOT NULL DEFAULT 0 AFTER fa_deleted_reason, + ALTER COLUMN fa_description SET DEFAULT '', + ADD COLUMN fa_description_id bigint unsigned NOT NULL DEFAULT 0 AFTER fa_description; + +ALTER TABLE /*_*/recentchanges + ADD COLUMN rc_comment_id bigint unsigned NOT NULL DEFAULT 0 AFTER rc_comment; + +ALTER TABLE /*_*/logging + ADD COLUMN log_comment_id bigint unsigned NOT NULL DEFAULT 0 AFTER log_comment; + +ALTER TABLE /*_*/protected_titles + ALTER COLUMN pt_reason SET DEFAULT '', + ADD COLUMN pt_reason_id bigint unsigned NOT NULL DEFAULT 0 AFTER pt_reason; diff --git a/maintenance/migrateComments.php b/maintenance/migrateComments.php new file mode 100644 index 0000000000..431380647a --- /dev/null +++ b/maintenance/migrateComments.php @@ -0,0 +1,291 @@ +addDescription( 'Migrates comments from pre-1.30 columns to the \'comment\' table' ); + $this->setBatchSize( 100 ); + } + + protected function getUpdateKey() { + return __CLASS__; + } + + protected function updateSkippedMessage() { + return 'comments already migrated.'; + } + + protected function doDBUpdates() { + global $wgCommentTableSchemaMigrationStage; + + if ( $wgCommentTableSchemaMigrationStage < MIGRATION_WRITE_NEW ) { + $this->output( + "...cannot update while \$wgCommentTableSchemaMigrationStage < MIGRATION_WRITE_NEW\n" + ); + return false; + } + + $this->migrateToTemp( + 'revision', 'rev_id', 'rev_comment', 'revcomment_rev', 'revcomment_comment_id' + ); + $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( 'oldimage', [ 'oi_name', 'oi_timestamp' ], 'oi_description' ); + $this->migrate( 'filearchive', 'fa_id', 'fa_deleted_reason' ); + $this->migrate( 'filearchive', 'fa_id', 'fa_description' ); + $this->migrate( 'recentchanges', 'rc_id', 'rc_comment' ); + $this->migrate( 'logging', 'log_id', 'log_comment' ); + $this->migrate( 'protected_titles', [ 'pt_namespace', 'pt_title' ], 'pt_reason' ); + return true; + } + + /** + * Fetch comment IDs for a set of comments + * @param IDatabase $dbw + * @param array &$comments Keys are comment names, values will be set to IDs. + * @return int Count of added comments + */ + private function loadCommentIDs( IDatabase $dbw, array &$comments ) { + $count = 0; + $needComments = $comments; + + while ( true ) { + $where = []; + foreach ( $needComments as $need => $dummy ) { + $where[] = $dbw->makeList( + [ + 'comment_hash' => CommentStore::hash( $need, null ), + 'comment_text' => $need, + ], + LIST_AND + ); + } + + $res = $dbw->select( + 'comment', + [ 'comment_id', 'comment_text' ], + [ + $dbw->makeList( $where, LIST_OR ), + 'comment_data' => null, + ], + __METHOD__ + ); + foreach ( $res as $row ) { + $comments[$row->comment_text] = $row->comment_id; + unset( $needComments[$row->comment_text] ); + } + + if ( !$needComments ) { + break; + } + + $dbw->insert( + 'comment', + array_map( function ( $v ) { + return [ + 'comment_hash' => CommentStore::hash( $v, null ), + 'comment_text' => $v, + ]; + }, array_keys( $needComments ) ), + __METHOD__ + ); + $count += $dbw->affectedRows(); + } + return $count; + } + + /** + * Migrate comments in a table. + * + * Assumes any row with the ID field non-zero have already been migrated. + * Assumes the new field name is the same as the old with '_id' appended. + * Blanks the old fields while migrating. + * + * @param string $table Table to migrate + * @param string|string[] $primaryKey Primary key of the table. + * @param string $oldField Old comment field name + */ + protected function migrate( $table, $primaryKey, $oldField ) { + $newField = $oldField . '_id'; + $primaryKey = (array)$primaryKey; + $pkFilter = array_flip( $primaryKey ); + $this->output( "Beginning migration of $table.$oldField to $table.$newField\n" ); + + $dbw = $this->getDB( DB_MASTER ); + $next = '1=1'; + $countUpdated = 0; + $countComments = 0; + while ( true ) { + // Fetch the rows needing update + $res = $dbw->select( + $table, + array_merge( $primaryKey, [ $oldField ] ), + [ + $newField => 0, + $next, + ], + __METHOD__, + [ + 'ORDER BY' => $primaryKey, + 'LIMIT' => $this->mBatchSize, + ] + ); + if ( !$res->numRows() ) { + break; + } + + // Collect the distinct comments from those rows + $comments = []; + foreach ( $res as $row ) { + $comments[$row->$oldField] = 0; + } + $countComments += $this->loadCommentIDs( $dbw, $comments ); + + // Update the existing rows + foreach ( $res as $row ) { + $dbw->update( + $table, + [ + $newField => $comments[$row->$oldField], + $oldField => '', + ], + array_intersect_key( (array)$row, $pkFilter ) + [ + $newField => 0 + ], + __METHOD__ + ); + $countUpdated += $dbw->affectedRows(); + } + + // Calculate the "next" condition + $next = ''; + $prompt = []; + for ( $i = count( $primaryKey ) - 1; $i >= 0; $i-- ) { + $field = $primaryKey[$i]; + $prompt[] = $row->$field; + $value = $dbw->addQuotes( $row->$field ); + if ( $next === '' ) { + $next = "$field > $value"; + } else { + $next = "$field > $value OR $field = $value AND ($next)"; + } + } + $prompt = join( ' ', array_reverse( $prompt ) ); + $this->output( "... $prompt\n" ); + } + + $this->output( + "Completed migration, updated $countUpdated row(s) with $countComments new comment(s)\n" + ); + } + + /** + * Migrate comments in a table to a temporary table. + * + * Assumes any row with the ID field non-zero have already been migrated. + * Assumes the new table is named "{$table}_comment_temp", and it has two + * columns, in order, being the primary key of the original table and the + * comment ID field. + * Blanks the old fields while migrating. + * + * @param string $oldTable Table to migrate + * @param string $primaryKey Primary key of the table. + * @param string $oldField Old comment field name + * @param string $newPrimaryKey Primary key of the new table. + * @param string $newField New comment field name + */ + protected function migrateToTemp( $table, $primaryKey, $oldField, $newPrimaryKey, $newField ) { + $newTable = $table . '_comment_temp'; + $this->output( "Beginning migration of $table.$oldField to $newTable.$newField\n" ); + + $dbw = $this->getDB( DB_MASTER ); + $next = []; + $countUpdated = 0; + $countComments = 0; + while ( true ) { + // Fetch the rows needing update + $res = $dbw->select( + [ $table, $newTable ], + [ $primaryKey, $oldField ], + [ $newPrimaryKey => null ] + $next, + __METHOD__, + [ + 'ORDER BY' => $primaryKey, + 'LIMIT' => $this->mBatchSize, + ], + [ $newTable => [ 'LEFT JOIN', "{$primaryKey}={$newPrimaryKey}" ] ] + ); + if ( !$res->numRows() ) { + break; + } + + // Collect the distinct comments from those rows + $comments = []; + foreach ( $res as $row ) { + $comments[$row->$oldField] = 0; + } + $countComments += $this->loadCommentIDs( $dbw, $comments ); + + // Update rows + $inserts = []; + $updates = []; + foreach ( $res as $row ) { + $inserts[] = [ + $newPrimaryKey => $row->$primaryKey, + $newField => $comments[$row->$oldField] + ]; + $updates[] = $row->$primaryKey; + } + $this->beginTransaction( $dbw, __METHOD__ ); + $dbw->insert( $newTable, $inserts, __METHOD__ ); + $dbw->update( $table, [ $oldField => '' ], [ $primaryKey => $updates ], __METHOD__ ); + $countUpdated += $dbw->affectedRows(); + $this->commitTransaction( $dbw, __METHOD__ ); + + // Calculate the "next" condition + $next = [ $primaryKey . ' > ' . $dbw->addQuotes( $row->$primaryKey ) ]; + $this->output( "... {$row->$primaryKey}\n" ); + } + + $this->output( + "Completed migration, updated $countUpdated row(s) with $countComments new comment(s)\n" + ); + } +} + +$maintClass = "MigrateComments"; +require_once RUN_MAINTENANCE_IF_MAIN; diff --git a/maintenance/orphans.php b/maintenance/orphans.php index e36c5b67dd..644fb958df 100644 --- a/maintenance/orphans.php +++ b/maintenance/orphans.php @@ -75,20 +75,24 @@ class Orphans extends Maintenance { */ private function checkOrphans( $fix ) { $dbw = $this->getDB( DB_MASTER ); - $page = $dbw->tableName( 'page' ); - $revision = $dbw->tableName( 'revision' ); + $commentStore = new CommentStore( 'rev_comment' ); if ( $fix ) { $this->lockTables( $dbw ); } + $commentQuery = $commentStore->getJoin(); + $this->output( "Checking for orphan revision table entries... " . "(this may take a while on a large wiki)\n" ); - $result = $dbw->query( " - SELECT * - FROM $revision LEFT OUTER JOIN $page ON rev_page=page_id - WHERE page_id IS NULL - " ); + $result = $dbw->select( + [ 'revision', 'page' ] + $commentQuery['tables'], + [ 'rev_id', 'rev_page', 'rev_timestamp', 'rev_user_text' ] + $commentQuery['fields'], + [ 'page_id' => null ], + __METHOD__, + [], + [ 'page' => [ 'LEFT JOIN', [ 'rev_page=page_id' ] ] ] + $commentQuery['joins'] + ); $orphans = $result->numRows(); if ( $orphans > 0 ) { global $wgContLang; @@ -100,9 +104,10 @@ class Orphans extends Maintenance { ) ); foreach ( $result as $row ) { - $comment = ( $row->rev_comment == '' ) - ? '' - : '(' . $wgContLang->truncate( $row->rev_comment, 40 ) . ')'; + $comment = $commentStore->getComment( $row )->text; + if ( $comment !== '' ) { + $comment = '(' . $wgContLang->truncate( $comment, 40 ) . ')'; + } $this->output( sprintf( "%10d %10d %14s %20s %s\n", $row->rev_id, $row->rev_page, diff --git a/maintenance/postgres/archives/patch-comment-table.sql b/maintenance/postgres/archives/patch-comment-table.sql new file mode 100644 index 0000000000..a84986fb1c --- /dev/null +++ b/maintenance/postgres/archives/patch-comment-table.sql @@ -0,0 +1,27 @@ +-- +-- patch-comment-table.sql +-- +-- T166732. Add a `comment` table, and temporary tables to reference it. + +CREATE SEQUENCE comment_comment_id_seq; +CREATE TABLE comment ( + comment_id INTEGER NOT NULL PRIMARY KEY DEFAULT nextval('comment_comment_id_seq'), + comment_hash INTEGER NOT NULL, + comment_text TEXT NOT NULL, + comment_data TEXT +); +CREATE INDEX comment_hash ON comment (comment_hash); + +CREATE TABLE revision_comment_temp ( + revcomment_rev INTEGER NOT NULL, + revcomment_comment_id INTEGER NOT NULL, + PRIMARY KEY (revcomment_rev, revcomment_comment_id) +); +CREATE UNIQUE INDEX revcomment_rev ON revision_comment_temp (revcomment_rev); + +CREATE TABLE image_comment_temp ( + imgcomment_name TEXT NOT NULL, + imgcomment_comment_id INTEGER NOT NULL, + PRIMARY KEY (imgcomment_name, imgcomment_comment_id) +); +CREATE UNIQUE INDEX imgcomment_name ON image_comment_temp (imgcomment_rev); diff --git a/maintenance/postgres/tables.sql b/maintenance/postgres/tables.sql index 03fd03a2ff..3516a3be2b 100644 --- a/maintenance/postgres/tables.sql +++ b/maintenance/postgres/tables.sql @@ -12,6 +12,7 @@ SET client_min_messages = 'ERROR'; DROP SEQUENCE IF EXISTS user_user_id_seq CASCADE; DROP SEQUENCE IF EXISTS page_page_id_seq CASCADE; DROP SEQUENCE IF EXISTS revision_rev_id_seq CASCADE; +DROP SEQUENCE IF EXISTS comment_comment_id_seq CASCADE; DROP SEQUENCE IF EXISTS text_old_id_seq CASCADE; DROP SEQUENCE IF EXISTS page_restrictions_pr_id_seq CASCADE; DROP SEQUENCE IF EXISTS ipblocks_ipb_id_seq CASCADE; @@ -132,7 +133,7 @@ CREATE TABLE revision ( rev_id INTEGER NOT NULL UNIQUE DEFAULT nextval('revision_rev_id_seq'), rev_page INTEGER NULL REFERENCES page (page_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, rev_text_id INTEGER NULL, -- FK - rev_comment TEXT, + rev_comment TEXT NOT NULL DEFAULT '', rev_user INTEGER NOT NULL REFERENCES mwuser(user_id) ON DELETE RESTRICT DEFERRABLE INITIALLY DEFERRED, rev_user_text TEXT NOT NULL, rev_timestamp TIMESTAMPTZ NOT NULL, @@ -150,6 +151,12 @@ CREATE INDEX rev_timestamp_idx ON revision (rev_timestamp); CREATE INDEX rev_user_idx ON revision (rev_user); CREATE INDEX rev_user_text_idx ON revision (rev_user_text); +CREATE TABLE revision_comment_temp ( + revcomment_rev INTEGER NOT NULL, + revcomment_comment_id INTEGER NOT NULL, + PRIMARY KEY (revcomment_rev, revcomment_comment_id) +); +CREATE UNIQUE INDEX revcomment_rev ON revision_comment_temp (revcomment_rev); CREATE SEQUENCE text_old_id_seq; CREATE TABLE pagecontent ( -- replaces reserved word 'text' @@ -159,6 +166,16 @@ CREATE TABLE pagecontent ( -- replaces reserved word 'text' ); +CREATE SEQUENCE comment_comment_id_seq; +CREATE TABLE comment ( + comment_id INTEGER NOT NULL PRIMARY KEY DEFAULT nextval('comment_comment_id_seq'), + comment_hash INTEGER NOT NULL, + comment_text TEXT NOT NULL, + comment_data TEXT +); +CREATE INDEX comment_hash ON comment (comment_hash); + + CREATE SEQUENCE page_restrictions_pr_id_seq; CREATE TABLE page_restrictions ( pr_id INTEGER NOT NULL UNIQUE DEFAULT nextval('page_restrictions_pr_id_seq'), @@ -191,7 +208,8 @@ CREATE TABLE archive ( ar_page_id INTEGER NULL, ar_parent_id INTEGER NULL, ar_sha1 TEXT NOT NULL DEFAULT '', - ar_comment TEXT, + ar_comment TEXT NOT NULL DEFAULT '', + ar_comment_id INTEGER NOT NULL DEFAULT 0, ar_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, ar_user_text TEXT NOT NULL, ar_timestamp TIMESTAMPTZ NOT NULL, @@ -296,7 +314,8 @@ CREATE TABLE ipblocks ( ipb_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, ipb_by INTEGER NOT NULL REFERENCES mwuser(user_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, ipb_by_text TEXT NOT NULL DEFAULT '', - ipb_reason TEXT NOT NULL, + ipb_reason TEXT NOT NULL DEFAULT '', + ipb_reason_id INTEGER NOT NULL DEFAULT 0, ipb_timestamp TIMESTAMPTZ NOT NULL, ipb_auto SMALLINT NOT NULL DEFAULT 0, ipb_anon_only SMALLINT NOT NULL DEFAULT 0, @@ -327,7 +346,7 @@ CREATE TABLE image ( img_media_type TEXT, img_major_mime TEXT DEFAULT 'unknown', img_minor_mime TEXT DEFAULT 'unknown', - img_description TEXT NOT NULL, + img_description TEXT NOT NULL DEFAULT '', img_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, img_user_text TEXT NOT NULL, img_timestamp TIMESTAMPTZ, @@ -337,6 +356,13 @@ CREATE INDEX img_size_idx ON image (img_size); CREATE INDEX img_timestamp_idx ON image (img_timestamp); CREATE INDEX img_sha1 ON image (img_sha1); +CREATE TABLE image_comment_temp ( + imgcomment_name TEXT NOT NULL, + imgcomment_comment_id INTEGER NOT NULL, + PRIMARY KEY (imgcomment_name, imgcomment_comment_id) +); +CREATE UNIQUE INDEX imgcomment_name ON image_comment_temp (imgcomment_rev); + CREATE TABLE oldimage ( oi_name TEXT NOT NULL, oi_archive_name TEXT NOT NULL, @@ -344,7 +370,8 @@ CREATE TABLE oldimage ( oi_width INTEGER NOT NULL, oi_height INTEGER NOT NULL, oi_bits SMALLINT NULL, - oi_description TEXT, + oi_description TEXT NOT NULL DEFAULT '', + oi_description_id INTEGER NOT NULL DEFAULT 0, oi_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, oi_user_text TEXT NOT NULL, oi_timestamp TIMESTAMPTZ NULL, @@ -370,7 +397,8 @@ CREATE TABLE filearchive ( fa_storage_key TEXT, fa_deleted_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, fa_deleted_timestamp TIMESTAMPTZ NOT NULL, - fa_deleted_reason TEXT, + fa_deleted_reason TEXT NOT NULL DEFAULT '', + fa_deleted_reason_id INTEGER NOT NULL DEFAULT 0, fa_size INTEGER NOT NULL, fa_width INTEGER NOT NULL, fa_height INTEGER NOT NULL, @@ -379,7 +407,8 @@ CREATE TABLE filearchive ( fa_media_type TEXT, fa_major_mime TEXT DEFAULT 'unknown', fa_minor_mime TEXT DEFAULT 'unknown', - fa_description TEXT NOT NULL, + fa_description TEXT NOT NULL DEFAULT '', + fa_description_id INTEGER NOT NULL DEFAULT 0, fa_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, fa_user_text TEXT NOT NULL, fa_timestamp TIMESTAMPTZ, @@ -429,7 +458,8 @@ CREATE TABLE recentchanges ( rc_user_text TEXT NOT NULL, rc_namespace SMALLINT NOT NULL, rc_title TEXT NOT NULL, - rc_comment TEXT, + rc_comment TEXT NOT NULL DEFAULT '', + rc_comment_id INTEGER NOT NULL DEFAULT 0, rc_minor SMALLINT NOT NULL DEFAULT 0, rc_bot SMALLINT NOT NULL DEFAULT 0, rc_new SMALLINT NOT NULL DEFAULT 0, @@ -528,7 +558,8 @@ CREATE TABLE logging ( log_user INTEGER REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, log_namespace SMALLINT NOT NULL, log_title TEXT NOT NULL, - log_comment TEXT, + log_comment TEXT NOT NULL DEFAULT '', + log_comment_id INTEGER NOT NULL DEFAULT 0, log_params TEXT, log_deleted SMALLINT NOT NULL DEFAULT 0, log_user_text TEXT NOT NULL DEFAULT '', @@ -635,7 +666,8 @@ CREATE TABLE protected_titles ( pt_namespace SMALLINT NOT NULL, pt_title TEXT NOT NULL, pt_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, - pt_reason TEXT NULL, + pt_reason TEXT NOT NULL DEFAULT '', + pt_reason_id INTEGER NOT NULL DEFAULT 0, pt_timestamp TIMESTAMPTZ NOT NULL, pt_expiry TIMESTAMPTZ NULL, pt_create_perm TEXT NOT NULL DEFAULT '' diff --git a/maintenance/rebuildrecentchanges.php b/maintenance/rebuildrecentchanges.php index b93d1127d6..a2cf3c5bca 100644 --- a/maintenance/rebuildrecentchanges.php +++ b/maintenance/rebuildrecentchanges.php @@ -80,6 +80,8 @@ class RebuildRecentchanges extends Maintenance { */ private function rebuildRecentChangesTablePass1() { $dbw = $this->getDB( DB_MASTER ); + $revCommentStore = new CommentStore( 'rev_comment' ); + $rcCommentStore = new CommentStore( 'rc_comment' ); if ( $this->hasOption( 'from' ) && $this->hasOption( 'to' ) ) { $this->cutoffFrom = wfTimestamp( TS_UNIX, $this->getOption( 'from' ) ); @@ -113,13 +115,14 @@ class RebuildRecentchanges extends Maintenance { } $this->output( "Loading from page and revision tables...\n" ); + + $commentQuery = $revCommentStore->getJoin(); $res = $dbw->select( - [ 'page', 'revision' ], + [ 'revision', 'page' ] + $commentQuery['tables'], [ 'rev_timestamp', 'rev_user', 'rev_user_text', - 'rev_comment', 'rev_minor_edit', 'rev_id', 'rev_deleted', @@ -127,19 +130,22 @@ class RebuildRecentchanges extends Maintenance { 'page_title', 'page_is_new', 'page_id' - ], + ] + $commentQuery['fields'], [ 'rev_timestamp > ' . $dbw->addQuotes( $dbw->timestamp( $this->cutoffFrom ) ), - 'rev_timestamp < ' . $dbw->addQuotes( $dbw->timestamp( $this->cutoffTo ) ), - 'rev_page=page_id' + 'rev_timestamp < ' . $dbw->addQuotes( $dbw->timestamp( $this->cutoffTo ) ) ], __METHOD__, - [ 'ORDER BY' => 'rev_timestamp DESC' ] + [ 'ORDER BY' => 'rev_timestamp DESC' ], + [ + 'page' => [ 'JOIN', 'rev_page=page_id' ], + ] + $commentQuery['joins'] ); $this->output( "Inserting from page and revision tables...\n" ); $inserted = 0; foreach ( $res as $row ) { + $comment = $revCommentStore->getComment( $row ); $dbw->insert( 'recentchanges', [ @@ -148,7 +154,6 @@ class RebuildRecentchanges extends Maintenance { 'rc_user_text' => $row->rev_user_text, 'rc_namespace' => $row->page_namespace, 'rc_title' => $row->page_title, - 'rc_comment' => $row->rev_comment, 'rc_minor' => $row->rev_minor_edit, 'rc_bot' => 0, 'rc_new' => $row->page_is_new, @@ -156,10 +161,9 @@ class RebuildRecentchanges extends Maintenance { 'rc_this_oldid' => $row->rev_id, 'rc_last_oldid' => 0, // is this ok? 'rc_type' => $row->page_is_new ? RC_NEW : RC_EDIT, - 'rc_source' => $row->page_is_new ? RecentChange::SRC_NEW : RecentChange::SRC_EDIT - , + 'rc_source' => $row->page_is_new ? RecentChange::SRC_NEW : RecentChange::SRC_EDIT, 'rc_deleted' => $row->rev_deleted - ], + ] + $rcCommentStore->insert( $dbw, $comment ), __METHOD__ ); if ( ( ++$inserted % $this->mBatchSize ) == 0 ) { @@ -266,25 +270,27 @@ class RebuildRecentchanges extends Maintenance { global $wgLogTypes, $wgLogRestrictions; $dbw = $this->getDB( DB_MASTER ); + $logCommentStore = new CommentStore( 'log_comment' ); + $rcCommentStore = new CommentStore( 'rc_comment' ); $this->output( "Loading from user, page, and logging tables...\n" ); + $commentQuery = $logCommentStore->getJoin(); $res = $dbw->select( - [ 'user', 'logging', 'page' ], + [ 'user', 'logging', 'page' ] + $commentQuery['tables'], [ 'log_timestamp', 'log_user', 'user_name', 'log_namespace', 'log_title', - 'log_comment', 'page_id', 'log_type', 'log_action', 'log_id', 'log_params', 'log_deleted' - ], + ] + $commentQuery['fields'], [ 'log_timestamp > ' . $dbw->addQuotes( $dbw->timestamp( $this->cutoffFrom ) ), 'log_timestamp < ' . $dbw->addQuotes( $dbw->timestamp( $this->cutoffTo ) ), @@ -298,13 +304,14 @@ class RebuildRecentchanges extends Maintenance { [ 'page' => [ 'LEFT JOIN', [ 'log_namespace=page_namespace', 'log_title=page_title' ] ] - ] + ] + $commentQuery['joins'] ); $field = $dbw->fieldInfo( 'recentchanges', 'rc_cur_id' ); $inserted = 0; foreach ( $res as $row ) { + $comment = $logCommentStore->getComment( $row ); $dbw->insert( 'recentchanges', [ @@ -313,7 +320,6 @@ class RebuildRecentchanges extends Maintenance { 'rc_user_text' => $row->user_name, 'rc_namespace' => $row->log_namespace, 'rc_title' => $row->log_title, - 'rc_comment' => $row->log_comment, 'rc_minor' => 0, 'rc_bot' => 0, 'rc_patrolled' => 1, @@ -330,7 +336,7 @@ class RebuildRecentchanges extends Maintenance { 'rc_logid' => $row->log_id, 'rc_params' => $row->log_params, 'rc_deleted' => $row->log_deleted - ], + ] + $rcCommentStore->insert( $dbw, $comment ), __METHOD__ ); diff --git a/maintenance/sqlite/archives/patch-comment-table.sql b/maintenance/sqlite/archives/patch-comment-table.sql new file mode 100644 index 0000000000..2017ecaccd --- /dev/null +++ b/maintenance/sqlite/archives/patch-comment-table.sql @@ -0,0 +1,332 @@ +-- +-- patch-comment-table.sql +-- +-- T166732. Add a `comment` table and various columns (and temporary tables) to reference it. +-- Sigh, sqlite, such trouble just to change the default value of a column. + +CREATE TABLE /*_*/comment ( + comment_id bigint unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + comment_hash INT NOT NULL, + comment_text BLOB NOT NULL, + comment_data BLOB +) /*$wgDBTableOptions*/; +CREATE INDEX /*i*/comment_hash ON comment (comment_hash); + +CREATE TABLE /*_*/revision_comment_temp ( + revcomment_rev int unsigned NOT NULL, + revcomment_comment_id bigint unsigned NOT NULL, + PRIMARY KEY (revcomment_rev, revcomment_comment_id) +) /*$wgDBTableOptions*/; +CREATE UNIQUE INDEX /*i*/revcomment_rev ON /*_*/revision_comment_temp (revcomment_rev); + +CREATE TABLE /*_*/image_comment_temp ( + imgcomment_name varchar(255) binary NOT NULL, + imgcomment_description_id bigint unsigned NOT NULL, + PRIMARY KEY (imgcomment_name, imgcomment_description_id) +) /*$wgDBTableOptions*/; +CREATE UNIQUE INDEX /*i*/imgcomment_name ON /*_*/image_comment_temp (imgcomment_name); + +ALTER TABLE /*_*/recentchanges + ADD COLUMN rc_comment_id bigint unsigned NOT NULL DEFAULT 0; + +ALTER TABLE /*_*/logging + ADD COLUMN log_comment_id bigint unsigned NOT NULL DEFAULT 0; + +BEGIN; + +DROP TABLE IF EXISTS /*_*/revision_tmp; +CREATE TABLE /*_*/revision_tmp ( + rev_id int unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + rev_page int unsigned NOT NULL, + rev_text_id int unsigned NOT NULL, + rev_comment varbinary(767) NOT NULL default '', + rev_user int unsigned NOT NULL default 0, + rev_user_text varchar(255) binary NOT NULL default '', + rev_timestamp binary(14) NOT NULL default '', + rev_minor_edit tinyint unsigned NOT NULL default 0, + rev_deleted tinyint unsigned NOT NULL default 0, + rev_len int unsigned, + rev_parent_id int unsigned default NULL, + rev_sha1 varbinary(32) NOT NULL default '', + rev_content_model varbinary(32) DEFAULT NULL, + rev_content_format varbinary(64) DEFAULT NULL +) /*$wgDBTableOptions*/ MAX_ROWS=10000000 AVG_ROW_LENGTH=1024; + +INSERT OR IGNORE INTO /*_*/revision_tmp ( + rev_id, rev_page, rev_text_id, rev_comment, rev_user, rev_user_text, + rev_timestamp, rev_minor_edit, rev_deleted, rev_len, rev_parent_id, + rev_sha1, rev_content_model, rev_content_format) + SELECT + rev_id, rev_page, rev_text_id, rev_comment, rev_user, rev_user_text, + rev_timestamp, rev_minor_edit, rev_deleted, rev_len, rev_parent_id, + rev_sha1, rev_content_model, rev_content_format + FROM /*_*/revision; + +DROP TABLE /*_*/revision; +ALTER TABLE /*_*/revision_tmp RENAME TO /*_*/revision; +CREATE INDEX /*i*/rev_page_id ON /*_*/revision (rev_page, rev_id); +CREATE INDEX /*i*/rev_timestamp ON /*_*/revision (rev_timestamp); +CREATE INDEX /*i*/page_timestamp ON /*_*/revision (rev_page,rev_timestamp); +CREATE INDEX /*i*/user_timestamp ON /*_*/revision (rev_user,rev_timestamp); +CREATE INDEX /*i*/usertext_timestamp ON /*_*/revision (rev_user_text,rev_timestamp); +CREATE INDEX /*i*/page_user_timestamp ON /*_*/revision (rev_page,rev_user,rev_timestamp); + +COMMIT; + +BEGIN; + +DROP TABLE IF EXISTS /*_*/archive_tmp; +CREATE TABLE /*_*/archive_tmp ( + ar_id int unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + ar_namespace int NOT NULL default 0, + ar_title varchar(255) binary NOT NULL default '', + ar_text mediumblob NOT NULL, + ar_comment varbinary(767) NOT NULL default '', + ar_comment_id bigint unsigned NOT NULL DEFAULT 0, + ar_user int unsigned NOT NULL default 0, + ar_user_text varchar(255) binary NOT NULL, + ar_timestamp binary(14) NOT NULL default '', + ar_minor_edit tinyint NOT NULL default 0, + ar_flags tinyblob NOT NULL, + ar_rev_id int unsigned, + ar_text_id int unsigned, + ar_deleted tinyint unsigned NOT NULL default 0, + ar_len int unsigned, + ar_page_id int unsigned, + ar_parent_id int unsigned default NULL, + ar_sha1 varbinary(32) NOT NULL default '', + ar_content_model varbinary(32) DEFAULT NULL, + ar_content_format varbinary(64) DEFAULT NULL +) /*$wgDBTableOptions*/; + +INSERT OR IGNORE INTO /*_*/archive_tmp ( + ar_id, ar_namespace, ar_title, ar_text, ar_comment, ar_user, ar_user_text, + ar_timestamp, ar_minor_edit, ar_flags, ar_rev_id, ar_text_id, ar_deleted, + ar_len, ar_page_id, ar_parent_id, ar_sha1, ar_content_model, + ar_content_format) + SELECT + ar_id, ar_namespace, ar_title, ar_text, ar_comment, ar_user, ar_user_text, + ar_timestamp, ar_minor_edit, ar_flags, ar_rev_id, ar_text_id, ar_deleted, + ar_len, ar_page_id, ar_parent_id, ar_sha1, ar_content_model, + ar_content_format + FROM /*_*/archive; + +DROP TABLE /*_*/archive; +ALTER TABLE /*_*/archive_tmp RENAME TO /*_*/archive; +CREATE INDEX /*i*/name_title_timestamp ON /*_*/archive (ar_namespace,ar_title,ar_timestamp); +CREATE INDEX /*i*/ar_usertext_timestamp ON /*_*/archive (ar_user_text,ar_timestamp); +CREATE INDEX /*i*/ar_revid ON /*_*/archive (ar_rev_id); + +COMMIT; + +BEGIN; + +DROP TABLE IF EXISTS ipblocks_tmp; +CREATE TABLE /*_*/ipblocks_tmp ( + ipb_id int NOT NULL PRIMARY KEY AUTO_INCREMENT, + ipb_address tinyblob NOT NULL, + ipb_user int unsigned NOT NULL default 0, + ipb_by int unsigned NOT NULL default 0, + ipb_by_text varchar(255) binary NOT NULL default '', + ipb_reason varbinary(767) NOT NULL default '', + ipb_reason_id bigint unsigned NOT NULL DEFAULT 0, + ipb_timestamp binary(14) NOT NULL default '', + ipb_auto bool NOT NULL default 0, + ipb_anon_only bool NOT NULL default 0, + ipb_create_account bool NOT NULL default 1, + ipb_enable_autoblock bool NOT NULL default '1', + ipb_expiry varbinary(14) NOT NULL default '', + ipb_range_start tinyblob NOT NULL, + ipb_range_end tinyblob NOT NULL, + ipb_deleted bool NOT NULL default 0, + ipb_block_email bool NOT NULL default 0, + ipb_allow_usertalk bool NOT NULL default 0, + ipb_parent_block_id int default NULL +) /*$wgDBTableOptions*/; + +INSERT OR IGNORE INTO /*_*/ipblocks_tmp ( + ipb_id, ipb_address, ipb_user, ipb_by, ipb_by_text, ipb_reason, + ipb_timestamp, ipb_auto, ipb_anon_only, ipb_create_account, + ipb_enable_autoblock, ipb_expiry, ipb_range_start, ipb_range_end, + ipb_deleted, ipb_block_email, ipb_allow_usertalk, ipb_parent_block_id) + SELECT + ipb_id, ipb_address, ipb_user, ipb_by, ipb_by_text, ipb_reason, + ipb_timestamp, ipb_auto, ipb_anon_only, ipb_create_account, + ipb_enable_autoblock, ipb_expiry, ipb_range_start, ipb_range_end, + ipb_deleted, ipb_block_email, ipb_allow_usertalk, ipb_parent_block_id + FROM /*_*/ipblocks; + +DROP TABLE /*_*/ipblocks; +ALTER TABLE /*_*/ipblocks_tmp RENAME TO /*_*/ipblocks; +CREATE UNIQUE INDEX /*i*/ipb_address ON /*_*/ipblocks (ipb_address(255), ipb_user, ipb_auto, ipb_anon_only); +CREATE INDEX /*i*/ipb_user ON /*_*/ipblocks (ipb_user); +CREATE INDEX /*i*/ipb_range ON /*_*/ipblocks (ipb_range_start(8), ipb_range_end(8)); +CREATE INDEX /*i*/ipb_timestamp ON /*_*/ipblocks (ipb_timestamp); +CREATE INDEX /*i*/ipb_expiry ON /*_*/ipblocks (ipb_expiry); +CREATE INDEX /*i*/ipb_parent_block_id ON /*_*/ipblocks (ipb_parent_block_id); + +COMMIT; + +BEGIN; + +DROP TABLE IF EXISTS /*_*/image_tmp; +CREATE TABLE /*_*/image_tmp ( + img_name varchar(255) binary NOT NULL default '' PRIMARY KEY, + img_size int unsigned NOT NULL default 0, + img_width int NOT NULL default 0, + img_height int NOT NULL default 0, + img_metadata mediumblob NOT NULL, + img_bits int NOT NULL default 0, + img_media_type ENUM("UNKNOWN", "BITMAP", "DRAWING", "AUDIO", "VIDEO", "MULTIMEDIA", "OFFICE", "TEXT", "EXECUTABLE", "ARCHIVE") default NULL, + img_major_mime ENUM("unknown", "application", "audio", "image", "text", "video", "message", "model", "multipart", "chemical") NOT NULL default "unknown", + img_minor_mime varbinary(100) NOT NULL default "unknown", + img_description varbinary(767) NOT NULL default '', + img_user int unsigned NOT NULL default 0, + img_user_text varchar(255) binary NOT NULL, + img_timestamp varbinary(14) NOT NULL default '', + img_sha1 varbinary(32) NOT NULL default '' +) /*$wgDBTableOptions*/; + +INSERT OR IGNORE INTO /*_*/image_tmp ( + img_name, img_size, img_width, img_height, img_metadata, img_bits, + img_media_type, img_major_mime, img_minor_mime, img_description, img_user, + img_user_text, img_timestamp, img_sha1) + SELECT + img_name, img_size, img_width, img_height, img_metadata, img_bits, + img_media_type, img_major_mime, img_minor_mime, img_description, img_user, + img_user_text, img_timestamp, img_sha1 + FROM /*_*/image; + +DROP TABLE /*_*/image; +ALTER TABLE /*_*/image_tmp RENAME TO /*_*/image; +CREATE INDEX /*i*/img_user_timestamp ON /*_*/image (img_user,img_timestamp); +CREATE INDEX /*i*/img_usertext_timestamp ON /*_*/image (img_user_text,img_timestamp); +CREATE INDEX /*i*/img_size ON /*_*/image (img_size); +CREATE INDEX /*i*/img_timestamp ON /*_*/image (img_timestamp); +CREATE INDEX /*i*/img_sha1 ON /*_*/image (img_sha1(10)); +CREATE INDEX /*i*/img_media_mime ON /*_*/image (img_media_type,img_major_mime,img_minor_mime); + +COMMIT; + +BEGIN; + +DROP TABLE IF EXISTS /*_*/oldimage_tmp; +CREATE TABLE /*_*/oldimage_tmp ( + oi_name varchar(255) binary NOT NULL default '', + oi_archive_name varchar(255) binary NOT NULL default '', + oi_size int unsigned NOT NULL default 0, + oi_width int NOT NULL default 0, + oi_height int NOT NULL default 0, + oi_bits int NOT NULL default 0, + oi_description varbinary(767) NOT NULL default '', + oi_description_id bigint unsigned NOT NULL DEFAULT 0, + oi_user int unsigned NOT NULL default 0, + oi_user_text varchar(255) binary NOT NULL, + oi_timestamp binary(14) NOT NULL default '', + oi_metadata mediumblob NOT NULL, + oi_media_type ENUM("UNKNOWN", "BITMAP", "DRAWING", "AUDIO", "VIDEO", "MULTIMEDIA", "OFFICE", "TEXT", "EXECUTABLE", "ARCHIVE") default NULL, + oi_major_mime ENUM("unknown", "application", "audio", "image", "text", "video", "message", "model", "multipart", "chemical") NOT NULL default "unknown", + oi_minor_mime varbinary(100) NOT NULL default "unknown", + oi_deleted tinyint unsigned NOT NULL default 0, + oi_sha1 varbinary(32) NOT NULL default '' +) /*$wgDBTableOptions*/; + +INSERT OR IGNORE INTO /*_*/oldimage_tmp ( + oi_name, oi_archive_name, oi_size, oi_width, oi_height, oi_bits, + oi_description, oi_user, oi_user_text, oi_timestamp, oi_metadata, + oi_media_type, oi_major_mime, oi_minor_mime, oi_deleted, oi_sha1) + SELECT + oi_name, oi_archive_name, oi_size, oi_width, oi_height, oi_bits, + oi_description, oi_user, oi_user_text, oi_timestamp, oi_metadata, + oi_media_type, oi_major_mime, oi_minor_mime, oi_deleted, oi_sha1 + FROM /*_*/oldimage; + +DROP TABLE /*_*/oldimage; +ALTER TABLE /*_*/oldimage_tmp RENAME TO /*_*/oldimage; +CREATE INDEX /*i*/oi_usertext_timestamp ON /*_*/oldimage (oi_user_text,oi_timestamp); +CREATE INDEX /*i*/oi_name_timestamp ON /*_*/oldimage (oi_name,oi_timestamp); +CREATE INDEX /*i*/oi_name_archive_name ON /*_*/oldimage (oi_name,oi_archive_name(14)); +CREATE INDEX /*i*/oi_sha1 ON /*_*/oldimage (oi_sha1(10)); + +COMMIT; + +BEGIN; + +DROP TABLE IF EXISTS /*_*/filearchive_tmp; +CREATE TABLE /*_*/filearchive_tmp ( + fa_id int NOT NULL PRIMARY KEY AUTO_INCREMENT, + fa_name varchar(255) binary NOT NULL default '', + fa_archive_name varchar(255) binary default '', + fa_storage_group varbinary(16), + fa_storage_key varbinary(64) default '', + fa_deleted_user int, + fa_deleted_timestamp binary(14) default '', + fa_deleted_reason varbinary(767) default '', + fa_deleted_reason_id bigint unsigned NOT NULL DEFAULT 0, + fa_size int unsigned default 0, + fa_width int default 0, + fa_height int default 0, + fa_metadata mediumblob, + fa_bits int default 0, + fa_media_type ENUM("UNKNOWN", "BITMAP", "DRAWING", "AUDIO", "VIDEO", "MULTIMEDIA", "OFFICE", "TEXT", "EXECUTABLE", "ARCHIVE") default NULL, + fa_major_mime ENUM("unknown", "application", "audio", "image", "text", "video", "message", "model", "multipart", "chemical") default "unknown", + fa_minor_mime varbinary(100) default "unknown", + fa_description varbinary(767) default '', + fa_description_id bigint unsigned NOT NULL DEFAULT 0, + fa_user int unsigned default 0, + fa_user_text varchar(255) binary, + fa_timestamp binary(14) default '', + fa_deleted tinyint unsigned NOT NULL default 0, + fa_sha1 varbinary(32) NOT NULL default '' +) /*$wgDBTableOptions*/; + +INSERT OR IGNORE INTO /*_*/filearchive_tmp ( + fa_id, fa_name, fa_archive_name, fa_storage_group, fa_storage_key, + fa_deleted_user, fa_deleted_timestamp, fa_deleted_reason, fa_size, + fa_width, fa_height, fa_metadata, fa_bits, fa_media_type, fa_major_mime, + fa_minor_mime, fa_description, fa_user, fa_user_text, fa_timestamp, + fa_deleted, fa_sha1) + SELECT + fa_id, fa_name, fa_archive_name, fa_storage_group, fa_storage_key, + fa_deleted_user, fa_deleted_timestamp, fa_deleted_reason, fa_size, + fa_width, fa_height, fa_metadata, fa_bits, fa_media_type, fa_major_mime, + fa_minor_mime, fa_description, fa_user, fa_user_text, fa_timestamp, + fa_deleted, fa_sha1 + FROM /*_*/filearchive; + +DROP TABLE /*_*/filearchive; +ALTER TABLE /*_*/filearchive_tmp RENAME TO /*_*/filearchive; +CREATE INDEX /*i*/fa_name ON /*_*/filearchive (fa_name, fa_timestamp); +CREATE INDEX /*i*/fa_storage_group ON /*_*/filearchive (fa_storage_group, fa_storage_key); +CREATE INDEX /*i*/fa_deleted_timestamp ON /*_*/filearchive (fa_deleted_timestamp); +CREATE INDEX /*i*/fa_user_timestamp ON /*_*/filearchive (fa_user_text,fa_timestamp); +CREATE INDEX /*i*/fa_sha1 ON /*_*/filearchive (fa_sha1(10)); + +COMMIT; + +BEGIN; + +DROP TABLE IF EXISTS /*_*/protected_titles_tmp; +CREATE TABLE /*_*/protected_titles_tmp ( + pt_namespace int NOT NULL, + pt_title varchar(255) binary NOT NULL, + pt_user int unsigned NOT NULL, + pt_reason varbinary(767) default '', + pt_reason_id bigint unsigned NOT NULL DEFAULT 0, + pt_timestamp binary(14) NOT NULL, + pt_expiry varbinary(14) NOT NULL default '', + pt_create_perm varbinary(60) NOT NULL +) /*$wgDBTableOptions*/; + +INSERT OR IGNORE INTO /*_*/protected_titles_tmp ( + pt_namespace, pt_title, pt_user, pt_reason, pt_timestamp, pt_expiry, pt_create_perm) + SELECT + pt_namespace, pt_title, pt_user, pt_reason, pt_timestamp, pt_expiry, pt_create_perm + FROM /*_*/protected_titles; + +DROP TABLE /*_*/protected_titles; +ALTER TABLE /*_*/protected_titles_tmp RENAME TO /*_*/protected_titles; +CREATE UNIQUE INDEX /*i*/pt_namespace_title ON /*_*/protected_titles (pt_namespace,pt_title); +CREATE INDEX /*i*/pt_timestamp ON /*_*/protected_titles (pt_timestamp); + +COMMIT; diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 9a18796177..3836665d90 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -345,10 +345,9 @@ CREATE TABLE /*_*/revision ( -- or a rollback to a previous version. rev_text_id int unsigned NOT NULL, - -- Text comment summarizing the change. - -- This text is shown in the history and other changes lists, - -- rendered in a subset of wiki markup by Linker::formatComment() - rev_comment varbinary(767) NOT NULL, + -- Text comment summarizing the change. Deprecated in favor of + -- revision_comment_temp.revcomment_comment_id. + rev_comment varbinary(767) NOT NULL default '', -- Key to user.user_id of the user who made this edit. -- Stores 0 for anonymous edits and for some mass imports. @@ -409,6 +408,23 @@ CREATE INDEX /*i*/usertext_timestamp ON /*_*/revision (rev_user_text,rev_timesta -- and is a logged-in user. CREATE INDEX /*i*/page_user_timestamp ON /*_*/revision (rev_page,rev_user,rev_timestamp); +-- +-- Temporary table to avoid blocking on an alter of revision. +-- +-- On large wikis like the English Wikipedia, altering the revision table is a +-- months-long process. This table is being created to avoid such an alter, and +-- will be merged back into revision in the future. +-- +CREATE TABLE /*_*/revision_comment_temp ( + -- Key to rev_id + revcomment_rev int unsigned NOT NULL, + -- Key to comment_id + revcomment_comment_id bigint unsigned NOT NULL, + PRIMARY KEY (revcomment_rev, revcomment_comment_id) +) /*$wgDBTableOptions*/; +-- Ensure uniqueness +CREATE UNIQUE INDEX /*i*/revcomment_rev ON /*_*/revision_comment_temp (revcomment_rev); + -- -- Every time an edit by a logged out user is saved, -- a row is created in ip_changes. This stores @@ -474,6 +490,40 @@ CREATE TABLE /*_*/text ( -- In case tables are created as MyISAM, use row hints for MySQL <5.0 to avoid 4GB limit +-- +-- Edits, blocks, and other actions typically have a textual comment describing +-- the action. They are stored here to reduce the size of the main tables, and +-- to allow for deduplication. +-- +-- Deduplication is currently best-effort to avoid locking on inserts that +-- would be required for strict deduplication. There MAY be multiple rows with +-- the same comment_text and comment_data. +-- +CREATE TABLE /*_*/comment ( + -- Unique ID to identify each comment + comment_id bigint unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + + -- Hash of comment_text and comment_data, for deduplication + comment_hash INT NOT NULL, + + -- Text comment summarizing the change. + -- This text is shown in the history and other changes lists, + -- rendered in a subset of wiki markup by Linker::formatComment() + -- Size limits are enforced at the application level, and should + -- take care to crop UTF-8 strings appropriately. + comment_text BLOB NOT NULL, + + -- JSON data, intended for localizing auto-generated comments. + -- This holds structured data that is intended to be used to provide + -- localized versions of automatically-generated comments. When not empty, + -- comment_text should be the generated comment localized using the wiki's + -- content language. + comment_data BLOB +) /*$wgDBTableOptions*/; +-- Index used for deduplication. +CREATE INDEX /*i*/comment_hash ON comment (comment_hash); + + -- -- Holding area for deleted articles, which may be viewed -- or restored by admins through the Special:Undelete interface. @@ -495,7 +545,8 @@ CREATE TABLE /*_*/archive ( ar_text mediumblob NOT NULL, -- Basic revision stuff... - ar_comment varbinary(767) NOT NULL, + ar_comment varbinary(767) NOT NULL default '', -- Deprecated in favor of ar_comment_id + ar_comment_id bigint unsigned NOT NULL DEFAULT 0, -- ("DEFAULT 0" is temporary, signaling that ar_comment should be used) ar_user int unsigned NOT NULL default 0, ar_user_text varchar(255) binary NOT NULL, ar_timestamp binary(14) NOT NULL default '', @@ -861,8 +912,12 @@ CREATE TABLE /*_*/ipblocks ( -- User name of blocker ipb_by_text varchar(255) binary NOT NULL default '', - -- Text comment made by blocker. - ipb_reason varbinary(767) NOT NULL, + -- Text comment made by blocker. Deprecated in favor of ipb_reason_id + ipb_reason varbinary(767) NOT NULL default '', + + -- Key to comment_id. Text comment made by blocker. + -- ("DEFAULT 0" is temporary, signaling that ipb_reason should be used) + ipb_reason_id bigint unsigned NOT NULL DEFAULT 0, -- Creation (or refresh) date in standard YMDHMS form. -- IP blocks expire automatically. @@ -969,7 +1024,8 @@ CREATE TABLE /*_*/image ( -- Description field as entered by the uploader. -- This is displayed in image upload history and logs. - img_description varbinary(767) NOT NULL, + -- Deprecated in favor of image_comment_temp.imgcomment_description_id. + img_description varbinary(767) NOT NULL default '', -- user_id and user_name of uploader. img_user int unsigned NOT NULL default 0, @@ -994,6 +1050,23 @@ CREATE INDEX /*i*/img_sha1 ON /*_*/image (img_sha1(10)); -- Used to get media of one type CREATE INDEX /*i*/img_media_mime ON /*_*/image (img_media_type,img_major_mime,img_minor_mime); +-- +-- Temporary table to avoid blocking on an alter of image. +-- +-- On large wikis like Wikimedia Commons, altering the image table is a +-- months-long process. This table is being created to avoid such an alter, and +-- will be merged back into image in the future. +-- +CREATE TABLE /*_*/image_comment_temp ( + -- Key to img_name (ugh) + imgcomment_name varchar(255) binary NOT NULL, + -- Key to comment_id + imgcomment_description_id bigint unsigned NOT NULL, + PRIMARY KEY (imgcomment_name, imgcomment_description_id) +) /*$wgDBTableOptions*/; +-- Ensure uniqueness +CREATE UNIQUE INDEX /*i*/imgcomment_name ON /*_*/image_comment_temp (imgcomment_name); + -- -- Previous revisions of uploaded files. @@ -1013,7 +1086,8 @@ CREATE TABLE /*_*/oldimage ( oi_width int NOT NULL default 0, oi_height int NOT NULL default 0, oi_bits int NOT NULL default 0, - oi_description varbinary(767) NOT NULL, + oi_description varbinary(767) NOT NULL default '', -- Deprecated. + oi_description_id bigint unsigned NOT NULL DEFAULT 0, -- ("DEFAULT 0" is temporary, signaling that oi_description should be used) oi_user int unsigned NOT NULL default 0, oi_user_text varchar(255) binary NOT NULL, oi_timestamp binary(14) NOT NULL default '', @@ -1061,7 +1135,8 @@ CREATE TABLE /*_*/filearchive ( -- Deletion information, if this file is deleted. fa_deleted_user int, fa_deleted_timestamp binary(14) default '', - fa_deleted_reason varbinary(767) default '', + fa_deleted_reason varbinary(767) default '', -- Deprecated + fa_deleted_reason_id bigint unsigned NOT NULL DEFAULT 0, -- ("DEFAULT 0" is temporary, signaling that fa_deleted_reason should be used) -- Duped fields from image fa_size int unsigned default 0, @@ -1072,7 +1147,8 @@ CREATE TABLE /*_*/filearchive ( fa_media_type ENUM("UNKNOWN", "BITMAP", "DRAWING", "AUDIO", "VIDEO", "MULTIMEDIA", "OFFICE", "TEXT", "EXECUTABLE", "ARCHIVE", "3D") default NULL, fa_major_mime ENUM("unknown", "application", "audio", "image", "text", "video", "message", "model", "multipart", "chemical") default "unknown", fa_minor_mime varbinary(100) default "unknown", - fa_description varbinary(767), + fa_description varbinary(767) default '', -- Deprecated + fa_description_id bigint unsigned NOT NULL DEFAULT 0, -- ("DEFAULT 0" is temporary, signaling that fa_description should be used) fa_user int unsigned default 0, fa_user_text varchar(255) binary, fa_timestamp binary(14) default '', @@ -1170,7 +1246,8 @@ CREATE TABLE /*_*/recentchanges ( rc_title varchar(255) binary NOT NULL default '', -- as in revision... - rc_comment varbinary(767) NOT NULL default '', + rc_comment varbinary(767) NOT NULL default '', -- Deprecated. + rc_comment_id bigint unsigned NOT NULL DEFAULT 0, -- ("DEFAULT 0" is temporary, signaling that rc_comment should be used) rc_minor tinyint unsigned NOT NULL default 0, -- Edits by user accounts with the 'bot' rights key are @@ -1399,8 +1476,13 @@ CREATE TABLE /*_*/logging ( log_page int unsigned NULL, -- Freeform text. Interpreted as edit history comments. + -- Deprecated in favor of log_comment_id. log_comment varbinary(767) NOT NULL default '', + -- Key to comment_id. Comment summarizing the change. + -- ("DEFAULT 0" is temporary, signaling that log_comment should be used) + log_comment_id bigint unsigned NOT NULL DEFAULT 0, + -- miscellaneous parameters: -- LF separated list (old system) or serialized PHP array (new system) log_params blob NOT NULL, @@ -1574,7 +1656,8 @@ CREATE TABLE /*_*/protected_titles ( pt_namespace int NOT NULL, pt_title varchar(255) binary NOT NULL, pt_user int unsigned NOT NULL, - pt_reason varbinary(767), + pt_reason varbinary(767) default '', -- Deprecated. + pt_reason_id bigint unsigned NOT NULL DEFAULT 0, -- ("DEFAULT 0" is temporary, signaling that pt_reason should be used) pt_timestamp binary(14) NOT NULL, pt_expiry varbinary(14) NOT NULL default '', pt_create_perm varbinary(60) NOT NULL diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index ed1f1ae833..c844e13fd3 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -1303,13 +1303,17 @@ abstract class MediaWikiTestCase extends PHPUnit_Framework_TestCase { private function resetDB( $db, $tablesUsed ) { if ( $db ) { $userTables = [ 'user', 'user_groups', 'user_properties' ]; - $coreDBDataTables = array_merge( $userTables, [ 'page', 'revision' ] ); + $pageTables = [ 'page', 'revision', 'revision_comment_temp', 'comment' ]; + $coreDBDataTables = array_merge( $userTables, $pageTables ); - // If any of the user tables were marked as used, we should clear all of them. + // If any of the user or page tables were marked as used, we should clear all of them. if ( array_intersect( $tablesUsed, $userTables ) ) { $tablesUsed = array_unique( array_merge( $tablesUsed, $userTables ) ); TestUserRegistry::clear(); } + if ( array_intersect( $tablesUsed, $pageTables ) ) { + $tablesUsed = array_unique( array_merge( $tablesUsed, $pageTables ) ); + } $truncate = in_array( $db->getType(), [ 'oracle', 'mysql' ] ); foreach ( $tablesUsed as $tbl ) { diff --git a/tests/phpunit/includes/CommentStoreTest.php b/tests/phpunit/includes/CommentStoreTest.php new file mode 100644 index 0000000000..6dd092524e --- /dev/null +++ b/tests/phpunit/includes/CommentStoreTest.php @@ -0,0 +1,614 @@ +stage = $stage; + return $store; + } + + /** + * @dataProvider provideGetFields + * @param int $stage + * @param string $key + * @param array $expect + */ + public function testGetFields( $stage, $key, $expect ) { + $store = $this->makeStore( $stage, $key ); + $result = $store->getFields(); + $this->assertEquals( $expect, $result ); + } + + public static function provideGetFields() { + return [ + 'Simple table, old' => [ + MIGRATION_OLD, 'ipb_reason', + [ 'ipb_reason_text' => 'ipb_reason', 'ipb_reason_data' => 'NULL', 'ipb_reason_cid' => 'NULL' ], + ], + 'Simple table, write-both' => [ + MIGRATION_WRITE_BOTH, 'ipb_reason', + [ 'ipb_reason_old' => 'ipb_reason', 'ipb_reason_id' => 'ipb_reason_id' ], + ], + 'Simple table, write-new' => [ + MIGRATION_WRITE_NEW, 'ipb_reason', + [ 'ipb_reason_old' => 'ipb_reason', 'ipb_reason_id' => 'ipb_reason_id' ], + ], + 'Simple table, new' => [ + MIGRATION_NEW, 'ipb_reason', + [ 'ipb_reason_id' => 'ipb_reason_id' ], + ], + + 'Revision, old' => [ + MIGRATION_OLD, 'rev_comment', + [ + 'rev_comment_text' => 'rev_comment', + 'rev_comment_data' => 'NULL', + 'rev_comment_cid' => 'NULL', + ], + ], + 'Revision, write-both' => [ + MIGRATION_WRITE_BOTH, 'rev_comment', + [ 'rev_comment_old' => 'rev_comment', 'rev_comment_pk' => 'rev_id' ], + ], + 'Revision, write-new' => [ + MIGRATION_WRITE_NEW, 'rev_comment', + [ 'rev_comment_old' => 'rev_comment', 'rev_comment_pk' => 'rev_id' ], + ], + 'Revision, new' => [ + MIGRATION_NEW, 'rev_comment', + [ 'rev_comment_pk' => 'rev_id' ], + ], + + 'Image, old' => [ + MIGRATION_OLD, 'img_description', + [ + 'img_description_text' => 'img_description', + 'img_description_data' => 'NULL', + 'img_description_cid' => 'NULL', + ], + ], + 'Image, write-both' => [ + MIGRATION_WRITE_BOTH, 'img_description', + [ 'img_description_old' => 'img_description', 'img_description_pk' => 'img_name' ], + ], + 'Image, write-new' => [ + MIGRATION_WRITE_NEW, 'img_description', + [ 'img_description_old' => 'img_description', 'img_description_pk' => 'img_name' ], + ], + 'Image, new' => [ + MIGRATION_NEW, 'img_description', + [ 'img_description_pk' => 'img_name' ], + ], + ]; + } + + /** + * @dataProvider provideGetJoin + * @param int $stage + * @param string $key + * @param array $expect + */ + public function testGetJoin( $stage, $key, $expect ) { + $store = $this->makeStore( $stage, $key ); + $result = $store->getJoin(); + $this->assertEquals( $expect, $result ); + } + + public static function provideGetJoin() { + return [ + 'Simple table, old' => [ + MIGRATION_OLD, 'ipb_reason', [ + 'tables' => [], + 'fields' => [ + 'ipb_reason_text' => 'ipb_reason', + 'ipb_reason_data' => 'NULL', + 'ipb_reason_cid' => 'NULL', + ], + 'joins' => [], + ], + ], + 'Simple table, write-both' => [ + MIGRATION_WRITE_BOTH, 'ipb_reason', [ + 'tables' => [ 'comment_ipb_reason' => 'comment' ], + 'fields' => [ + 'ipb_reason_text' => 'COALESCE( comment_ipb_reason.comment_text, ipb_reason )', + 'ipb_reason_data' => 'comment_ipb_reason.comment_data', + 'ipb_reason_cid' => 'comment_ipb_reason.comment_id', + ], + 'joins' => [ + 'comment_ipb_reason' => [ 'LEFT JOIN', 'comment_ipb_reason.comment_id = ipb_reason_id' ], + ], + ], + ], + 'Simple table, write-new' => [ + MIGRATION_WRITE_NEW, 'ipb_reason', [ + 'tables' => [ 'comment_ipb_reason' => 'comment' ], + 'fields' => [ + 'ipb_reason_text' => 'COALESCE( comment_ipb_reason.comment_text, ipb_reason )', + 'ipb_reason_data' => 'comment_ipb_reason.comment_data', + 'ipb_reason_cid' => 'comment_ipb_reason.comment_id', + ], + 'joins' => [ + 'comment_ipb_reason' => [ 'LEFT JOIN', 'comment_ipb_reason.comment_id = ipb_reason_id' ], + ], + ], + ], + 'Simple table, new' => [ + MIGRATION_NEW, 'ipb_reason', [ + 'tables' => [ 'comment_ipb_reason' => 'comment' ], + 'fields' => [ + 'ipb_reason_text' => 'comment_ipb_reason.comment_text', + 'ipb_reason_data' => 'comment_ipb_reason.comment_data', + 'ipb_reason_cid' => 'comment_ipb_reason.comment_id', + ], + 'joins' => [ + 'comment_ipb_reason' => [ 'JOIN', 'comment_ipb_reason.comment_id = ipb_reason_id' ], + ], + ], + ], + + 'Revision, old' => [ + MIGRATION_OLD, 'rev_comment', [ + 'tables' => [], + 'fields' => [ + 'rev_comment_text' => 'rev_comment', + 'rev_comment_data' => 'NULL', + 'rev_comment_cid' => 'NULL', + ], + 'joins' => [], + ], + ], + 'Revision, write-both' => [ + MIGRATION_WRITE_BOTH, 'rev_comment', [ + 'tables' => [ + 'temp_rev_comment' => 'revision_comment_temp', + 'comment_rev_comment' => 'comment', + ], + 'fields' => [ + 'rev_comment_text' => 'COALESCE( comment_rev_comment.comment_text, rev_comment )', + 'rev_comment_data' => 'comment_rev_comment.comment_data', + 'rev_comment_cid' => 'comment_rev_comment.comment_id', + ], + 'joins' => [ + 'temp_rev_comment' => [ 'LEFT JOIN', 'temp_rev_comment.revcomment_rev = rev_id' ], + 'comment_rev_comment' => [ 'LEFT JOIN', + 'comment_rev_comment.comment_id = temp_rev_comment.revcomment_comment_id' ], + ], + ], + ], + 'Revision, write-new' => [ + MIGRATION_WRITE_NEW, 'rev_comment', [ + 'tables' => [ + 'temp_rev_comment' => 'revision_comment_temp', + 'comment_rev_comment' => 'comment', + ], + 'fields' => [ + 'rev_comment_text' => 'COALESCE( comment_rev_comment.comment_text, rev_comment )', + 'rev_comment_data' => 'comment_rev_comment.comment_data', + 'rev_comment_cid' => 'comment_rev_comment.comment_id', + ], + 'joins' => [ + 'temp_rev_comment' => [ 'LEFT JOIN', 'temp_rev_comment.revcomment_rev = rev_id' ], + 'comment_rev_comment' => [ 'LEFT JOIN', + 'comment_rev_comment.comment_id = temp_rev_comment.revcomment_comment_id' ], + ], + ], + ], + 'Revision, new' => [ + MIGRATION_NEW, 'rev_comment', [ + 'tables' => [ + 'temp_rev_comment' => 'revision_comment_temp', + 'comment_rev_comment' => 'comment', + ], + 'fields' => [ + 'rev_comment_text' => 'comment_rev_comment.comment_text', + 'rev_comment_data' => 'comment_rev_comment.comment_data', + 'rev_comment_cid' => 'comment_rev_comment.comment_id', + ], + 'joins' => [ + 'temp_rev_comment' => [ 'JOIN', 'temp_rev_comment.revcomment_rev = rev_id' ], + 'comment_rev_comment' => [ 'JOIN', + 'comment_rev_comment.comment_id = temp_rev_comment.revcomment_comment_id' ], + ], + ], + ], + + 'Image, old' => [ + MIGRATION_OLD, 'img_description', [ + 'tables' => [], + 'fields' => [ + 'img_description_text' => 'img_description', + 'img_description_data' => 'NULL', + 'img_description_cid' => 'NULL', + ], + 'joins' => [], + ], + ], + 'Image, write-both' => [ + MIGRATION_WRITE_BOTH, 'img_description', [ + 'tables' => [ + 'temp_img_description' => 'image_comment_temp', + 'comment_img_description' => 'comment', + ], + 'fields' => [ + 'img_description_text' => 'COALESCE( comment_img_description.comment_text, img_description )', + 'img_description_data' => 'comment_img_description.comment_data', + 'img_description_cid' => 'comment_img_description.comment_id', + ], + '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' ], + ], + ], + ], + 'Image, write-new' => [ + MIGRATION_WRITE_NEW, 'img_description', [ + 'tables' => [ + 'temp_img_description' => 'image_comment_temp', + 'comment_img_description' => 'comment', + ], + 'fields' => [ + 'img_description_text' => 'COALESCE( comment_img_description.comment_text, img_description )', + 'img_description_data' => 'comment_img_description.comment_data', + 'img_description_cid' => 'comment_img_description.comment_id', + ], + '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' ], + ], + ], + ], + 'Image, new' => [ + MIGRATION_NEW, 'img_description', [ + 'tables' => [ + 'temp_img_description' => 'image_comment_temp', + 'comment_img_description' => 'comment', + ], + 'fields' => [ + 'img_description_text' => 'comment_img_description.comment_text', + 'img_description_data' => 'comment_img_description.comment_data', + 'img_description_cid' => 'comment_img_description.comment_id', + ], + 'joins' => [ + 'temp_img_description' => [ 'JOIN', 'temp_img_description.imgcomment_name = img_name' ], + 'comment_img_description' => [ 'JOIN', + 'comment_img_description.comment_id = temp_img_description.imgcomment_description_id' ], + ], + ], + ], + ]; + } + + private function assertComment( $expect, $actual, $from ) { + $this->assertSame( $expect['text'], $actual->text, "text $from" ); + $this->assertInstanceOf( get_class( $expect['message'] ), $actual->message, + "message class $from" ); + $this->assertSame( $expect['message']->getKeysToTry(), $actual->message->getKeysToTry(), + "message keys $from" ); + $this->assertEquals( $expect['message']->text(), $actual->message->text(), + "message rendering $from" ); + $this->assertEquals( $expect['data'], $actual->data, "data $from" ); + } + + /** + * @dataProvider provideInsertRoundTrip + * @param string $table + * @param string $key + * @param string $pk + * @param string $extraFields + * @param string|Message $comment + * @param array|null $data + * @param array $expect + */ + public function testInsertRoundTrip( $table, $key, $pk, $extraFields, $comment, $data, $expect ) { + $expectOld = [ + 'text' => $expect['text'], + 'message' => new RawMessage( '$1', [ $expect['text'] ] ), + 'data' => null, + ]; + + $stages = [ + MIGRATION_OLD => [ MIGRATION_OLD, MIGRATION_WRITE_NEW ], + MIGRATION_WRITE_BOTH => [ MIGRATION_OLD, MIGRATION_NEW ], + MIGRATION_WRITE_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ], + MIGRATION_NEW => [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ], + ]; + + foreach ( $stages as $writeStage => $readRange ) { + if ( $key === 'ipb_reason' ) { + $extraFields['ipb_address'] = __CLASS__ . "#$writeStage"; + } + + $wstore = $this->makeStore( $writeStage, $key ); + $usesTemp = $key === 'rev_comment'; + + if ( $usesTemp ) { + list( $fields, $callback ) = $wstore->insertWithTempTable( $this->db, $comment, $data ); + } else { + $fields = $wstore->insert( $this->db, $comment, $data ); + } + + if ( $writeStage <= MIGRATION_WRITE_BOTH ) { + $this->assertSame( $expect['text'], $fields[$key], "old field, stage=$writeStage" ); + } else { + $this->assertArrayNotHasKey( $key, $fields, "old field, stage=$writeStage" ); + } + if ( $writeStage >= MIGRATION_WRITE_BOTH && !$usesTemp ) { + $this->assertArrayHasKey( "{$key}_id", $fields, "new field, stage=$writeStage" ); + } else { + $this->assertArrayNotHasKey( "{$key}_id", $fields, "new field, stage=$writeStage" ); + } + + $this->db->insert( $table, $extraFields + $fields, __METHOD__ ); + $id = $this->db->insertId(); + if ( $usesTemp ) { + $callback( $id ); + } + + for ( $readStage = $readRange[0]; $readStage <= $readRange[1]; $readStage++ ) { + $rstore = $this->makeStore( $readStage, $key ); + + $fieldRow = $this->db->selectRow( + $table, + $rstore->getFields(), + [ $pk => $id ], + __METHOD__ + ); + + $queryInfo = $rstore->getJoin(); + $joinRow = $this->db->selectRow( + [ $table ] + $queryInfo['tables'], + $queryInfo['fields'], + [ $pk => $id ], + __METHOD__, + [], + $queryInfo['joins'] + ); + + $this->assertComment( + $writeStage === MIGRATION_OLD || $readStage === MIGRATION_OLD ? $expectOld : $expect, + $rstore->getCommentLegacy( $this->db, $fieldRow ), + "w=$writeStage, r=$readStage, from getFields()" + ); + $this->assertComment( + $writeStage === MIGRATION_OLD || $readStage === MIGRATION_OLD ? $expectOld : $expect, + $rstore->getComment( $joinRow ), + "w=$writeStage, r=$readStage, from getJoin()" + ); + } + } + } + + public static function provideInsertRoundTrip() { + $msgComment = new Message( 'parentheses', [ 'message comment' ] ); + $textCommentMsg = new RawMessage( '$1', [ 'text comment' ] ); + $nestedMsgComment = new Message( [ 'parentheses', 'rawmessage' ], [ new Message( 'mainpage' ) ] ); + $ipbfields = [ + 'ipb_range_start' => '', + 'ipb_range_end' => '', + ]; + $revfields = [ + 'rev_page' => 42, + 'rev_text_id' => 42, + 'rev_len' => 0, + ]; + $comStoreComment = new CommentStoreComment( + null, 'comment store comment', null, [ 'foo' => 'bar' ] + ); + + return [ + 'Simple table, text comment' => [ + 'ipblocks', 'ipb_reason', 'ipb_id', $ipbfields, 'text comment', null, [ + 'text' => 'text comment', + 'message' => $textCommentMsg, + 'data' => null, + ] + ], + 'Simple table, text comment with data' => [ + 'ipblocks', 'ipb_reason', 'ipb_id', $ipbfields, 'text comment', [ 'message' => 42 ], [ + 'text' => 'text comment', + 'message' => $textCommentMsg, + 'data' => [ 'message' => 42 ], + ] + ], + 'Simple table, message comment' => [ + 'ipblocks', 'ipb_reason', 'ipb_id', $ipbfields, $msgComment, null, [ + 'text' => '(message comment)', + 'message' => $msgComment, + 'data' => null, + ] + ], + 'Simple table, message comment with data' => [ + 'ipblocks', 'ipb_reason', 'ipb_id', $ipbfields, $msgComment, [ 'message' => 42 ], [ + 'text' => '(message comment)', + 'message' => $msgComment, + 'data' => [ 'message' => 42 ], + ] + ], + 'Simple table, nested message comment' => [ + 'ipblocks', 'ipb_reason', 'ipb_id', $ipbfields, $nestedMsgComment, null, [ + 'text' => '(Main Page)', + 'message' => $nestedMsgComment, + 'data' => null, + ] + ], + 'Simple table, CommentStoreComment' => [ + 'ipblocks', 'ipb_reason', 'ipb_id', $ipbfields, clone $comStoreComment, [ 'baz' => 'baz' ], [ + 'text' => 'comment store comment', + 'message' => $comStoreComment->message, + 'data' => [ 'foo' => 'bar' ], + ] + ], + + 'Revision, text comment' => [ + 'revision', 'rev_comment', 'rev_id', $revfields, 'text comment', null, [ + 'text' => 'text comment', + 'message' => $textCommentMsg, + 'data' => null, + ] + ], + 'Revision, text comment with data' => [ + 'revision', 'rev_comment', 'rev_id', $revfields, 'text comment', [ 'message' => 42 ], [ + 'text' => 'text comment', + 'message' => $textCommentMsg, + 'data' => [ 'message' => 42 ], + ] + ], + 'Revision, message comment' => [ + 'revision', 'rev_comment', 'rev_id', $revfields, $msgComment, null, [ + 'text' => '(message comment)', + 'message' => $msgComment, + 'data' => null, + ] + ], + 'Revision, message comment with data' => [ + 'revision', 'rev_comment', 'rev_id', $revfields, $msgComment, [ 'message' => 42 ], [ + 'text' => '(message comment)', + 'message' => $msgComment, + 'data' => [ 'message' => 42 ], + ] + ], + 'Revision, nested message comment' => [ + 'revision', 'rev_comment', 'rev_id', $revfields, $nestedMsgComment, null, [ + 'text' => '(Main Page)', + 'message' => $nestedMsgComment, + 'data' => null, + ] + ], + 'Revision, CommentStoreComment' => [ + 'revision', 'rev_comment', 'rev_id', $revfields, clone $comStoreComment, [ 'baz' => 'baz' ], [ + 'text' => 'comment store comment', + 'message' => $comStoreComment->message, + 'data' => [ 'foo' => 'bar' ], + ] + ], + ]; + } + + public function testGetCommentErrors() { + MediaWiki\suppressWarnings(); + $reset = new ScopedCallback( 'MediaWiki\restoreWarnings' ); + + $store = $this->makeStore( MIGRATION_OLD, 'dummy' ); + $res = $store->getComment( [ 'dummy' => 'comment' ] ); + $this->assertSame( '', $res->text ); + $res = $store->getComment( [ 'dummy' => 'comment' ], true ); + $this->assertSame( 'comment', $res->text ); + + $store = $this->makeStore( MIGRATION_NEW, 'dummy' ); + try { + $store->getComment( [ 'dummy' => 'comment' ] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( InvalidArgumentException $ex ) { + $this->assertSame( '$row does not contain fields needed for comment dummy', $ex->getMessage() ); + } + $res = $store->getComment( [ 'dummy' => 'comment' ], true ); + $this->assertSame( 'comment', $res->text ); + try { + $store->getComment( [ 'dummy_id' => 1 ] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( InvalidArgumentException $ex ) { + $this->assertSame( + '$row does not contain fields needed for comment dummy and getComment(), ' + . 'but does have fields for getCommentLegacy()', + $ex->getMessage() + ); + } + + $store = $this->makeStore( MIGRATION_NEW, 'rev_comment' ); + try { + $store->getComment( [ 'rev_comment' => 'comment' ] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( InvalidArgumentException $ex ) { + $this->assertSame( + '$row does not contain fields needed for comment rev_comment', $ex->getMessage() + ); + } + $res = $store->getComment( [ 'rev_comment' => 'comment' ], true ); + $this->assertSame( 'comment', $res->text ); + try { + $store->getComment( [ 'rev_comment_pk' => 1 ] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( InvalidArgumentException $ex ) { + $this->assertSame( + '$row does not contain fields needed for comment rev_comment and getComment(), ' + . 'but does have fields for getCommentLegacy()', + $ex->getMessage() + ); + } + } + + 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 ], + ]; + } + + /** + * @dataProvider provideStages + * @param int $stage + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Must use insertWithTempTable() for rev_comment + */ + public function testInsertWrong( $stage ) { + $store = $this->makeStore( $stage, 'rev_comment' ); + $store->insert( $this->db, 'foo' ); + } + + /** + * @dataProvider provideStages + * @param int $stage + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Must use insert() for ipb_reason + */ + public function testInsertWithTempTableWrong( $stage ) { + $store = $this->makeStore( $stage, 'ipb_reason' ); + $store->insertWithTempTable( $this->db, 'foo' ); + } + + /** + * @dataProvider provideStages + * @param int $stage + */ + public function testInsertWithTempTableDeprecated( $stage ) { + $wrap = TestingAccessWrapper::newFromClass( CommentStore::class ); + $wrap->formerTempTables += [ 'ipb_reason' => '1.30' ]; + + $this->hideDeprecated( 'CommentStore::insertWithTempTable for ipb_reason' ); + $store = $this->makeStore( $stage, 'ipb_reason' ); + list( $fields, $callback ) = $store->insertWithTempTable( $this->db, 'foo' ); + $this->assertTrue( is_callable( $callback ) ); + } + + public function testConstructor() { + $this->assertInstanceOf( CommentStore::class, CommentStore::newKey( 'dummy' ) ); + } + +} diff --git a/tests/phpunit/includes/RevisionStorageTest.php b/tests/phpunit/includes/RevisionStorageTest.php index 3ba82a6f59..b207e06915 100644 --- a/tests/phpunit/includes/RevisionStorageTest.php +++ b/tests/phpunit/includes/RevisionStorageTest.php @@ -146,7 +146,7 @@ class RevisionStorageTest extends MediaWikiTestCase { $orig = $this->makeRevision(); $dbr = wfGetDB( DB_REPLICA ); - $res = $dbr->select( 'revision', '*', [ 'rev_id' => $orig->getId() ] ); + $res = $dbr->select( 'revision', Revision::selectFields(), [ 'rev_id' => $orig->getId() ] ); $this->assertTrue( is_object( $res ), 'query failed' ); $row = $res->fetchObject(); @@ -164,7 +164,7 @@ class RevisionStorageTest extends MediaWikiTestCase { $orig = $this->makeRevision(); $dbr = wfGetDB( DB_REPLICA ); - $res = $dbr->select( 'revision', '*', [ 'rev_id' => $orig->getId() ] ); + $res = $dbr->select( 'revision', Revision::selectFields(), [ 'rev_id' => $orig->getId() ] ); $this->assertTrue( is_object( $res ), 'query failed' ); $row = $res->fetchObject(); @@ -188,7 +188,9 @@ class RevisionStorageTest extends MediaWikiTestCase { $page->doDeleteArticle( 'test Revision::newFromArchiveRow' ); $dbr = wfGetDB( DB_REPLICA ); - $res = $dbr->select( 'archive', '*', [ 'ar_rev_id' => $orig->getId() ] ); + $res = $dbr->select( + 'archive', Revision::selectArchiveFields(), [ 'ar_rev_id' => $orig->getId() ] + ); $this->assertTrue( is_object( $res ), 'query failed' ); $row = $res->fetchObject(); diff --git a/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php b/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php index 474487520a..62ba5f68fe 100644 --- a/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php +++ b/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php @@ -1,5 +1,6 @@ [ WatchedItemQueryService::INCLUDE_FLAGS ] ], null, + [], [ 'rc_type', 'rc_minor', 'rc_bot' ], [], [], + [], ], [ [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_USER ] ], null, + [], [ 'rc_user_text' ], [], [], + [], ], [ [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_USER_ID ] ], null, + [], [ 'rc_user' ], [], [], + [], + ], + [ + [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_COMMENT ] ], + null, + [], + [ + 'rc_comment_text' => 'rc_comment', + 'rc_comment_data' => 'NULL', + 'rc_comment_cid' => 'NULL', + ], + [], + [], + [], + [ 'wgCommentTableSchemaMigrationStage' => MIGRATION_OLD ], + ], + [ + [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_COMMENT ] ], + null, + [ 'comment_rc_comment' => 'comment' ], + [ + 'rc_comment_text' => 'COALESCE( comment_rc_comment.comment_text, rc_comment )', + 'rc_comment_data' => 'comment_rc_comment.comment_data', + 'rc_comment_cid' => 'comment_rc_comment.comment_id', + ], + [], + [], + [ 'comment_rc_comment' => [ 'LEFT JOIN', 'comment_rc_comment.comment_id = rc_comment_id' ] ], + [ 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_BOTH ], + ], + [ + [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_COMMENT ] ], + null, + [ 'comment_rc_comment' => 'comment' ], + [ + 'rc_comment_text' => 'COALESCE( comment_rc_comment.comment_text, rc_comment )', + 'rc_comment_data' => 'comment_rc_comment.comment_data', + 'rc_comment_cid' => 'comment_rc_comment.comment_id', + ], + [], + [], + [ 'comment_rc_comment' => [ 'LEFT JOIN', 'comment_rc_comment.comment_id = rc_comment_id' ] ], + [ 'wgCommentTableSchemaMigrationStage' => MIGRATION_WRITE_NEW ], ], [ [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_COMMENT ] ], null, - [ 'rc_comment' ], + [ 'comment_rc_comment' => 'comment' ], + [ + 'rc_comment_text' => 'comment_rc_comment.comment_text', + 'rc_comment_data' => 'comment_rc_comment.comment_data', + 'rc_comment_cid' => 'comment_rc_comment.comment_id', + ], [], [], + [ 'comment_rc_comment' => [ 'JOIN', 'comment_rc_comment.comment_id = rc_comment_id' ] ], + [ 'wgCommentTableSchemaMigrationStage' => MIGRATION_NEW ], ], [ [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_PATROL_INFO ] ], null, + [], [ 'rc_patrolled', 'rc_log_type' ], [], [], + [], ], [ [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_SIZES ] ], null, + [], [ 'rc_old_len', 'rc_new_len' ], [], [], + [], ], [ [ 'includeFields' => [ WatchedItemQueryService::INCLUDE_LOG_INFO ] ], null, + [], [ 'rc_logid', 'rc_log_type', 'rc_log_action', 'rc_params' ], [], [], + [], ], [ [ 'namespaceIds' => [ 0, 1 ] ], null, [], + [], [ 'wl_namespace' => [ 0, 1 ] ], [], + [], ], [ [ 'namespaceIds' => [ 0, "1; DROP TABLE watchlist;\n--" ] ], null, [], + [], [ 'wl_namespace' => [ 0, 1 ] ], [], + [], ], [ [ 'rcTypes' => [ RC_EDIT, RC_NEW ] ], null, [], + [], [ 'rc_type' => [ RC_EDIT, RC_NEW ] ], [], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_OLDER ], null, [], [], - [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ] + [], + [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_NEWER ], null, [], [], - [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ] + [], + [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_OLDER, 'start' => '20151212010101' ], null, [], + [], [ "rc_timestamp <= '20151212010101'" ], - [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ] + [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_OLDER, 'end' => '20151212010101' ], null, [], + [], [ "rc_timestamp >= '20151212010101'" ], - [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ] + [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ], + [], ], [ [ @@ -550,22 +626,28 @@ class WatchedItemQueryServiceUnitTest extends PHPUnit_Framework_TestCase { ], null, [], + [], [ "rc_timestamp <= '20151212020101'", "rc_timestamp >= '20151212010101'" ], - [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ] + [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_NEWER, 'start' => '20151212010101' ], null, [], + [], [ "rc_timestamp >= '20151212010101'" ], - [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ] + [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_NEWER, 'end' => '20151212010101' ], null, [], + [], [ "rc_timestamp <= '20151212010101'" ], - [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ] + [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ], + [], ], [ [ @@ -575,133 +657,169 @@ class WatchedItemQueryServiceUnitTest extends PHPUnit_Framework_TestCase { ], null, [], + [], [ "rc_timestamp >= '20151212010101'", "rc_timestamp <= '20151212020101'" ], - [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ] + [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ], + [], ], [ [ 'limit' => 10 ], null, [], [], + [], [ 'LIMIT' => 11 ], + [], ], [ [ 'limit' => "10; DROP TABLE watchlist;\n--" ], null, [], [], + [], [ 'LIMIT' => 11 ], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_MINOR ] ], null, [], + [], [ 'rc_minor != 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_NOT_MINOR ] ], null, [], + [], [ 'rc_minor = 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_BOT ] ], null, [], + [], [ 'rc_bot != 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_NOT_BOT ] ], null, [], + [], [ 'rc_bot = 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_ANON ] ], null, [], + [], [ 'rc_user = 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_NOT_ANON ] ], null, [], + [], [ 'rc_user != 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_PATROLLED ] ], null, [], + [], [ 'rc_patrolled != 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_NOT_PATROLLED ] ], null, [], + [], [ 'rc_patrolled = 0' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_UNREAD ] ], null, [], + [], [ 'rc_timestamp >= wl_notificationtimestamp' ], [], + [], ], [ [ 'filters' => [ WatchedItemQueryService::FILTER_NOT_UNREAD ] ], null, [], + [], [ 'wl_notificationtimestamp IS NULL OR rc_timestamp < wl_notificationtimestamp' ], [], + [], ], [ [ 'onlyByUser' => 'SomeOtherUser' ], null, [], + [], [ 'rc_user_text' => 'SomeOtherUser' ], [], + [], ], [ [ 'notByUser' => 'SomeOtherUser' ], null, [], + [], [ "rc_user_text != 'SomeOtherUser'" ], [], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_OLDER ], [ '20151212010101', 123 ], [], + [], [ "(rc_timestamp < '20151212010101') OR ((rc_timestamp = '20151212010101') AND (rc_id <= 123))" ], [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_NEWER ], [ '20151212010101', 123 ], [], + [], [ "(rc_timestamp > '20151212010101') OR ((rc_timestamp = '20151212010101') AND (rc_id >= 123))" ], [ 'ORDER BY' => [ 'rc_timestamp', 'rc_id' ] ], + [], ], [ [ 'dir' => WatchedItemQueryService::DIR_OLDER ], [ '20151212010101', "123; DROP TABLE watchlist;\n--" ], [], + [], [ "(rc_timestamp < '20151212010101') OR ((rc_timestamp = '20151212010101') AND (rc_id <= 123))" ], [ 'ORDER BY' => [ 'rc_timestamp DESC', 'rc_id DESC' ] ], + [], ], ]; } @@ -712,10 +830,28 @@ class WatchedItemQueryServiceUnitTest extends PHPUnit_Framework_TestCase { public function testGetWatchedItemsWithRecentChangeInfo_optionsAndEmptyResult( array $options, $startFrom, + array $expectedExtraTables, array $expectedExtraFields, array $expectedExtraConds, - array $expectedDbOptions + array $expectedDbOptions, + array $expectedExtraJoinConds, + array $globals = [] ) { + // Sigh. This test class doesn't extend MediaWikiTestCase, so we have to reinvent setMwGlobals(). + if ( $globals ) { + $resetGlobals = []; + foreach ( $globals as $k => $v ) { + $resetGlobals[$k] = $GLOBALS[$k]; + $GLOBALS[$k] = $v; + } + $reset = new ScopedCallback( function () use ( $resetGlobals ) { + foreach ( $resetGlobals as $k => $v ) { + $GLOBALS[$k] = $v; + } + } ); + } + + $expectedTables = array_merge( [ 'recentchanges', 'watchlist', 'page' ], $expectedExtraTables ); $expectedFields = array_merge( [ 'rc_id', @@ -736,29 +872,33 @@ class WatchedItemQueryServiceUnitTest extends PHPUnit_Framework_TestCase { [ 'wl_user' => 1, '(rc_this_oldid=page_latest) OR (rc_type=3)', ], $expectedExtraConds ); + $expectedJoinConds = array_merge( + [ + 'watchlist' => [ + 'INNER JOIN', + [ + 'wl_namespace=rc_namespace', + 'wl_title=rc_title' + ] + ], + 'page' => [ + 'LEFT JOIN', + 'rc_cur_id=page_id', + ], + ], + $expectedExtraJoinConds + ); $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) ->method( 'select' ) ->with( - [ 'recentchanges', 'watchlist', 'page' ], + $expectedTables, $expectedFields, $expectedConds, $this->isType( 'string' ), $expectedDbOptions, - [ - 'watchlist' => [ - 'INNER JOIN', - [ - 'wl_namespace=rc_namespace', - 'wl_title=rc_title' - ] - ], - 'page' => [ - 'LEFT JOIN', - 'rc_cur_id=page_id', - ], - ] + $expectedJoinConds ) ->will( $this->returnValue( [] ) ); diff --git a/tests/phpunit/includes/api/ApiQueryWatchlistIntegrationTest.php b/tests/phpunit/includes/api/ApiQueryWatchlistIntegrationTest.php index 4f4453fd1b..fdbededee4 100644 --- a/tests/phpunit/includes/api/ApiQueryWatchlistIntegrationTest.php +++ b/tests/phpunit/includes/api/ApiQueryWatchlistIntegrationTest.php @@ -1074,6 +1074,8 @@ class ApiQueryWatchlistIntegrationTest extends ApiTestCase { 'rc_user' => 0, 'rc_user_text' => 'External User', 'rc_comment' => '', + 'rc_comment_text' => '', + 'rc_comment_data' => null, 'rc_this_oldid' => $title->getLatestRevID(), 'rc_last_oldid' => $title->getLatestRevID(), 'rc_bot' => 0, diff --git a/tests/phpunit/includes/changes/RecentChangeTest.php b/tests/phpunit/includes/changes/RecentChangeTest.php index 68f907958f..d638d0ff5a 100644 --- a/tests/phpunit/includes/changes/RecentChangeTest.php +++ b/tests/phpunit/includes/changes/RecentChangeTest.php @@ -31,6 +31,8 @@ class RecentChangeTest extends MediaWikiTestCase { $row->rc_foo = 'AAA'; $row->rc_timestamp = '20150921134808'; $row->rc_deleted = 'bar'; + $row->rc_comment_text = 'comment'; + $row->rc_comment_data = null; $rc = RecentChange::newFromRow( $row ); @@ -38,6 +40,29 @@ class RecentChangeTest extends MediaWikiTestCase { 'rc_foo' => 'AAA', 'rc_timestamp' => '20150921134808', 'rc_deleted' => 'bar', + 'rc_comment' => 'comment', + 'rc_comment_text' => 'comment', + 'rc_comment_data' => null, + ]; + $this->assertEquals( $expected, $rc->getAttributes() ); + + $row = new stdClass(); + $row->rc_foo = 'AAA'; + $row->rc_timestamp = '20150921134808'; + $row->rc_deleted = 'bar'; + $row->rc_comment = 'comment'; + + MediaWiki\suppressWarnings(); + $rc = RecentChange::newFromRow( $row ); + MediaWiki\restoreWarnings(); + + $expected = [ + 'rc_foo' => 'AAA', + 'rc_timestamp' => '20150921134808', + 'rc_deleted' => 'bar', + 'rc_comment' => 'comment', + 'rc_comment_text' => 'comment', + 'rc_comment_data' => null, ]; $this->assertEquals( $expected, $rc->getAttributes() ); } diff --git a/tests/phpunit/includes/changes/TestRecentChangesHelper.php b/tests/phpunit/includes/changes/TestRecentChangesHelper.php index 4da09d8535..2c309487f0 100644 --- a/tests/phpunit/includes/changes/TestRecentChangesHelper.php +++ b/tests/phpunit/includes/changes/TestRecentChangesHelper.php @@ -119,6 +119,8 @@ class TestRecentChangesHelper { 'rc_last_oldid' => $lastid, 'rc_cur_id' => $curid, 'rc_comment' => '[[:Testpage]] added to category', + 'rc_comment_text' => '[[:Testpage]] added to category', + 'rc_comment_data' => null, 'rc_old_len' => 0, 'rc_new_len' => 0, ] @@ -139,6 +141,8 @@ class TestRecentChangesHelper { 'rc_old_len' => 212, 'rc_new_len' => 188, 'rc_comment' => '', + 'rc_comment_text' => '', + 'rc_comment_data' => null, 'rc_minor' => 0, 'rc_bot' => 0, 'rc_type' => 0, diff --git a/tests/phpunit/includes/deferred/LinksUpdateTest.php b/tests/phpunit/includes/deferred/LinksUpdateTest.php index 639c323cf2..4c0a5fa9c3 100644 --- a/tests/phpunit/includes/deferred/LinksUpdateTest.php +++ b/tests/phpunit/includes/deferred/LinksUpdateTest.php @@ -378,16 +378,33 @@ class LinksUpdateTest extends MediaWikiLangTestCase { protected function assertRecentChangeByCategorization( Title $pageTitle, ParserOutput $parserOutput, Title $categoryTitle, $expectedRows ) { - $this->assertSelect( - 'recentchanges', - 'rc_title, rc_comment', - [ - 'rc_type' => RC_CATEGORIZE, - 'rc_namespace' => NS_CATEGORY, - 'rc_title' => $categoryTitle->getDBkey() - ], - $expectedRows - ); + global $wgCommentTableSchemaMigrationStage; + + if ( $wgCommentTableSchemaMigrationStage <= MIGRATION_WRITE_BOTH ) { + $this->assertSelect( + 'recentchanges', + 'rc_title, rc_comment', + [ + 'rc_type' => RC_CATEGORIZE, + 'rc_namespace' => NS_CATEGORY, + 'rc_title' => $categoryTitle->getDBkey() + ], + $expectedRows + ); + } + if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { + $this->assertSelect( + [ 'recentchanges', 'comment' ], + 'rc_title, comment_text', + [ + 'rc_type' => RC_CATEGORIZE, + 'rc_namespace' => NS_CATEGORY, + 'rc_title' => $categoryTitle->getDBkey(), + 'comment_id = rc_comment_id', + ], + $expectedRows + ); + } } private function runAllRelatedJobs() { diff --git a/tests/phpunit/includes/logging/LogFormatterTestCase.php b/tests/phpunit/includes/logging/LogFormatterTestCase.php index c289839b49..2dc9a2cf4f 100644 --- a/tests/phpunit/includes/logging/LogFormatterTestCase.php +++ b/tests/phpunit/includes/logging/LogFormatterTestCase.php @@ -39,7 +39,8 @@ abstract class LogFormatterTestCase extends MediaWikiLangTestCase { 'log_namespace' => isset( $data['namespace'] ) ? $data['namespace'] : NS_MAIN, 'log_title' => isset( $data['title'] ) ? $data['title'] : 'Main_Page', 'log_page' => isset( $data['page'] ) ? $data['page'] : 0, - 'log_comment' => isset( $data['comment'] ) ? $data['comment'] : '', + 'log_comment_text' => isset( $data['comment'] ) ? $data['comment'] : '', + 'log_comment_data' => null, 'log_params' => $legacy ? LogPage::makeParamBlob( $data['params'] ) : LogEntryBase::makeParamBlob( $data['params'] ), diff --git a/tests/phpunit/includes/page/WikiPageTest.php b/tests/phpunit/includes/page/WikiPageTest.php index a9f74b6472..d0fefdea03 100644 --- a/tests/phpunit/includes/page/WikiPageTest.php +++ b/tests/phpunit/includes/page/WikiPageTest.php @@ -17,6 +17,7 @@ class WikiPageTest extends MediaWikiLangTestCase { $this->tablesUsed, [ 'page', 'revision', + 'archive', 'text', 'recentchanges', @@ -1123,4 +1124,84 @@ more stuff $page = WikiPage::factory( $title ); $this->assertEquals( 'WikiPage', get_class( $page ) ); } + + /** + * @dataProvider provideCommentMigrationOnDeletion + * @param int $wstage + * @param int $rstage + */ + public function testCommentMigrationOnDeletion( $wstage, $rstage ) { + $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', $wstage ); + $dbr = wfGetDB( DB_REPLICA ); + + $page = $this->createPage( + "WikiPageTest_testCommentMigrationOnDeletion", + "foo", + CONTENT_MODEL_WIKITEXT + ); + $revid = $page->getLatest(); + if ( $wstage > MIGRATION_OLD ) { + $comment_id = $dbr->selectField( + 'revision_comment_temp', + 'revcomment_comment_id', + [ 'revcomment_rev' => $revid ], + __METHOD__ + ); + } + + $this->setMwGlobals( 'wgCommentTableSchemaMigrationStage', $rstage ); + + $page->doDeleteArticle( "testing deletion" ); + + if ( $rstage > MIGRATION_OLD ) { + // Didn't leave behind any 'revision_comment_temp' rows + $n = $dbr->selectField( + 'revision_comment_temp', 'COUNT(*)', [ 'revcomment_rev' => $revid ], __METHOD__ + ); + $this->assertEquals( 0, $n, 'no entry in revision_comment_temp after deletion' ); + + // Copied or upgraded the comment_id, as applicable + $ar_comment_id = $dbr->selectField( + 'archive', + 'ar_comment_id', + [ 'ar_rev_id' => $revid ], + __METHOD__ + ); + if ( $wstage > MIGRATION_OLD ) { + $this->assertSame( $comment_id, $ar_comment_id ); + } else { + $this->assertNotEquals( 0, $ar_comment_id ); + } + } + + // Copied rev_comment, if applicable + if ( $rstage <= MIGRATION_WRITE_BOTH && $wstage <= MIGRATION_WRITE_BOTH ) { + $ar_comment = $dbr->selectField( + 'archive', + 'ar_comment', + [ 'ar_rev_id' => $revid ], + __METHOD__ + ); + $this->assertSame( 'testing', $ar_comment ); + } + } + + public static function provideCommentMigrationOnDeletion() { + return [ + [ MIGRATION_OLD, MIGRATION_OLD ], + [ MIGRATION_OLD, MIGRATION_WRITE_BOTH ], + [ MIGRATION_OLD, MIGRATION_WRITE_NEW ], + [ MIGRATION_WRITE_BOTH, MIGRATION_OLD ], + [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_BOTH ], + [ MIGRATION_WRITE_BOTH, MIGRATION_WRITE_NEW ], + [ MIGRATION_WRITE_BOTH, MIGRATION_NEW ], + [ MIGRATION_WRITE_NEW, MIGRATION_WRITE_BOTH ], + [ MIGRATION_WRITE_NEW, MIGRATION_WRITE_NEW ], + [ MIGRATION_WRITE_NEW, MIGRATION_NEW ], + [ MIGRATION_NEW, MIGRATION_WRITE_BOTH ], + [ MIGRATION_NEW, MIGRATION_WRITE_NEW ], + [ MIGRATION_NEW, MIGRATION_NEW ], + ]; + } + } -- 2.20.1