From 6ee8645cdb747046dcf19be2a441c9b97a0a6937 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:49 -0400 Subject: [PATCH 01/11] Revert "Fix read old support broken in 022e4ffc915af7ab39a4cee534595b9d4b9d6018." This reverts commit 532383000e6c27b548495b11996f69cbd47fda75. --- src/CheckUser/Pagers/CheckUserGetEditsPager.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/CheckUser/Pagers/CheckUserGetEditsPager.php b/src/CheckUser/Pagers/CheckUserGetEditsPager.php index efbd32f4c..c8c5ba1ec 100644 --- a/src/CheckUser/Pagers/CheckUserGetEditsPager.php +++ b/src/CheckUser/Pagers/CheckUserGetEditsPager.php @@ -491,7 +491,9 @@ protected function getQueryInfoForCuChanges(): array { 'user' => 'actor_user', 'user_text' => 'actor_name', // Needed for rows with cuc_type as RC_LOG. - ] + $commentQuery['fields'], + 'comment_text', + 'comment_data', + ], 'tables' => [ 'cu_changes', 'actor_cuc_user' => 'actor' ] + $commentQuery['tables'], 'conds' => [], 'join_conds' => [ @@ -531,12 +533,14 @@ protected function getQueryInfoForCuLogEvent(): array { 'agent' => 'cule_agent', 'user' => 'actor_user', 'user_text' => 'actor_name', + 'comment_text', + 'comment_data', 'log_type' => 'log_type', 'log_action' => 'log_action', 'log_params' => 'log_params', 'log_deleted' => 'log_deleted', 'log_id' => 'cule_log_id', - ] + $commentQuery['fields'], + ], 'tables' => [ 'cu_log_event', 'logging_cule_log_id' => 'logging', 'actor_log_actor' => 'actor' ] + $commentQuery['tables'], @@ -586,12 +590,14 @@ protected function getQueryInfoForCuPrivateEvent(): array { 'agent' => 'cupe_agent', 'user' => 'actor_user', 'user_text' => 'actor_name', + 'comment_text', + 'comment_data', 'log_type' => 'cupe_log_type', 'log_action' => 'cupe_log_action', 'log_params' => 'cupe_params', // cu_private_event log events cannot be deleted or suppressed. 'log_deleted' => 0, - ] + $commentQuery['fields'], + ], 'tables' => [ 'cu_private_event', 'actor_cupe_actor' => 'actor' ] + $commentQuery['tables'], 'conds' => [], 'join_conds' => [ From a954517f5b09898cc4aa13fd626a28be2bd8595b Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:50 -0400 Subject: [PATCH 02/11] Revert "Disable update.php migrations for PopulateCulComment, PopulateCucComment, and MoveLogEntriesFromCuChanges. The three migrations are set to write both, read old by default." This reverts commit bdf1b32106f547402449ddb49d58ec31bf7de404. --- extension.json | 7 +++---- src/HookHandler/SchemaChangesHandler.php | 6 ------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/extension.json b/extension.json index 302a9b7b4..a70e2292c 100644 --- a/extension.json +++ b/extension.json @@ -88,12 +88,11 @@ "description": "Number of seconds for which the temporary account API response is fresh" }, "CheckUserCommentMigrationStage": { - "value": 259, - "description": "A flag used to represent the stage of the migration to the comment table. Default is to write to new and old, and read from old." + "value": 769 }, "CheckUserLogReasonMigrationStage": { - "value": 259, - "description": "A flag used to represent the stage of the migration to the comment table. Default is to write to new and old, and read from old." + "value": 769, + "description": "A flag used to represent the stage of the migration to the comment table. Default is to write to new and old, and read from new." }, "CheckUserEventTablesMigrationStage": { "value": 259, diff --git a/src/HookHandler/SchemaChangesHandler.php b/src/HookHandler/SchemaChangesHandler.php index af5951017..31c20eeff 100644 --- a/src/HookHandler/SchemaChangesHandler.php +++ b/src/HookHandler/SchemaChangesHandler.php @@ -186,13 +186,11 @@ public function onLoadExtensionSchemaUpdates( $updater ) { PopulateCulActor::class, 'extensions/CheckUser/maintenance/populateCulActor.php' ] ); - /* WGL - Disable update.php migrations. $updater->addExtensionUpdate( [ 'runMaintenance', PopulateCulComment::class, 'extensions/CheckUser/maintenance/populateCulComment.php' ] ); - */ if ( $dbType === 'postgres' ) { # For wikis which ran update.php after pulling the master branch of CheckUser between # 4 June 2022 and 6 June 2022, the cul_reason_id and cul_reason_plaintext_id columns @@ -212,13 +210,11 @@ public function onLoadExtensionSchemaUpdates( $updater ) { PopulateCucActor::class, 'extensions/CheckUser/maintenance/populateCucActor.php' ] ); - /* WGL - Disable update.php migrations. $updater->addExtensionUpdate( [ 'runMaintenance', PopulateCucComment::class, 'extensions/CheckUser/maintenance/populateCucComment.php' ] ); - */ // 1.40 $updater->addExtensionTable( @@ -268,9 +264,7 @@ public function onLoadExtensionSchemaUpdates( $updater ) { // 1.41 $updater->addExtensionTable( 'cu_useragent_clienthints', "$base/$dbType/cu_useragent_clienthints.sql" ); $updater->addExtensionTable( 'cu_useragent_clienthints_map', "$base/$dbType/cu_useragent_clienthints_map.sql" ); - /* WGL - Disable update.php migrations. $updater->addPostDatabaseUpdateMaintenance( MoveLogEntriesFromCuChanges::class ); - */ if ( !$isCUInstalled ) { // First time so populate cu_changes with recentchanges data. From e52ae02c361ada9220dd9beb2d00fe2788e37e4e Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:51 -0400 Subject: [PATCH 03/11] Reapply "Remove cul_reason comment table migration code" This reverts commit 13a2dc4aa27e1e4b9121f0632a966c60bb0b8068. --- extension.json | 4 - maintenance/importCheckUserLogs.php | 11 +-- src/Api/ApiQueryCheckUserLog.php | 31 ++----- src/CheckUser/Pagers/CheckUserLogPager.php | 29 ++---- src/ServiceWiring.php | 5 +- src/Services/CheckUserLogService.php | 27 +----- .../Api/ApiQueryCheckUserLogTest.php | 11 +-- .../Services/CheckUserLogServiceTest.php | 91 ++++++------------- 8 files changed, 58 insertions(+), 151 deletions(-) diff --git a/extension.json b/extension.json index a70e2292c..aac180a50 100644 --- a/extension.json +++ b/extension.json @@ -90,10 +90,6 @@ "CheckUserCommentMigrationStage": { "value": 769 }, - "CheckUserLogReasonMigrationStage": { - "value": 769, - "description": "A flag used to represent the stage of the migration to the comment table. Default is to write to new and old, and read from new." - }, "CheckUserEventTablesMigrationStage": { "value": 259, "description": "A flag used as the migration stage to the new cu_private_event and cu_log_event tables. Currently set to SCHEMA_COMPAT_OLD | SCHEMA_COMPAT_WRITE_NEW which evaluates to the integer 259." diff --git a/maintenance/importCheckUserLogs.php b/maintenance/importCheckUserLogs.php index 8e1f6c3e9..59087ed4e 100644 --- a/maintenance/importCheckUserLogs.php +++ b/maintenance/importCheckUserLogs.php @@ -108,7 +108,6 @@ protected function importLog( $file ) { $checkUserLogCommentStore = $services->get( 'CheckUserLogCommentStore' ); /** @var CheckUserLogService $checkUserLogService */ $checkUserLogService = $services->get( 'CheckUserLogService' ); - $culReasonMigrationStage = $services->getMainConfig()->get( 'CheckUserLogReasonMigrationStage' ); while ( !feof( $file ) ) { $line = fgets( $file ); @@ -148,12 +147,10 @@ protected function importLog( $file ) { 'cul_range_end' => $end ]; $fields += $checkUserLogCommentStore->insert( $dbw, 'cul_reason', $data['reason'] ); - if ( $culReasonMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { - $fields += $checkUserLogCommentStore->insert( - $dbw, 'cul_reason_plaintext', - $checkUserLogService->getPlaintextReason( $data['reason'] ) - ); - } + $fields += $checkUserLogCommentStore->insert( + $dbw, 'cul_reason_plaintext', + $checkUserLogService->getPlaintextReason( $data['reason'] ) + ); $dbw->newInsertQueryBuilder() ->table( 'cu_log' ) ->row( $fields ) diff --git a/src/Api/ApiQueryCheckUserLog.php b/src/Api/ApiQueryCheckUserLog.php index b673cf0ed..fcc516219 100644 --- a/src/Api/ApiQueryCheckUserLog.php +++ b/src/Api/ApiQueryCheckUserLog.php @@ -12,7 +12,6 @@ use Wikimedia\IPUtils; use Wikimedia\ParamValidator\ParamValidator; use Wikimedia\ParamValidator\TypeDef\IntegerDef; -use Wikimedia\Rdbms\Platform\ISQLPlatform; /** * CheckUser API Query Module @@ -64,14 +63,10 @@ public function execute() { $fields += $reasonCommentQuery['fields']; if ( isset( $params['reason'] ) ) { - if ( $this->getConfig()->get( 'CheckUserLogReasonMigrationStage' ) & SCHEMA_COMPAT_READ_NEW ) { - $plaintextReasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason_plaintext' ); - $this->addTables( $plaintextReasonCommentQuery['tables'] ); - $this->addJoinConds( $plaintextReasonCommentQuery['joins'] ); - $fields += $plaintextReasonCommentQuery['fields']; - } else { - $this->addFields( 'cul_reason' ); - } + $plaintextReasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason_plaintext' ); + $this->addTables( $plaintextReasonCommentQuery['tables'] ); + $this->addJoinConds( $plaintextReasonCommentQuery['joins'] ); + $fields += $plaintextReasonCommentQuery['fields']; } $this->addFields( $fields ); @@ -102,20 +97,10 @@ public function execute() { if ( isset( $params['reason'] ) ) { $plaintextReason = $this->checkUserLogService->getPlaintextReason( $params['reason'] ); - if ( $this->getConfig()->get( 'CheckUserLogReasonMigrationStage' ) & SCHEMA_COMPAT_READ_NEW ) { - $this->addWhereFld( - 'comment_cul_reason_plaintext.comment_text', - $plaintextReason - ); - } else { - $this->addWhere( $this->getDB()->makeList( - [ - 'cul_reason = ' . $this->getDB()->addQuotes( $params['reason'] ), - 'cul_reason = ' . $this->getDB()->addQuotes( $plaintextReason ) - ], - ISQLPlatform::LIST_OR - ) ); - } + $this->addWhereFld( + 'comment_cul_reason_plaintext.comment_text', + $plaintextReason + ); } if ( $continue !== null ) { diff --git a/src/CheckUser/Pagers/CheckUserLogPager.php b/src/CheckUser/Pagers/CheckUserLogPager.php index fa1722dc7..987752df0 100644 --- a/src/CheckUser/Pagers/CheckUserLogPager.php +++ b/src/CheckUser/Pagers/CheckUserLogPager.php @@ -18,7 +18,6 @@ use SpecialPage; use Wikimedia\IPUtils; use Wikimedia\Rdbms\IResultWrapper; -use Wikimedia\Rdbms\Platform\ISQLPlatform; class CheckUserLogPager extends RangeChronologicalPager { @@ -313,7 +312,7 @@ public function getIndexField() { */ public function selectFields(): array { return [ - 'cul_id', 'cul_timestamp', 'cul_reason', 'cul_type', 'cul_target_id', + 'cul_id', 'cul_timestamp', 'cul_type', 'cul_target_id', 'cul_target_text', 'actor_name', 'actor_user', 'actor_id' ]; } @@ -401,26 +400,16 @@ public function getQueryInfoForReasonSearch( string $reason ): array { $queryInfo = [ 'tables' => [], 'fields' => [], 'join_conds' => [] ]; $plaintextReason = $this->checkUserLogService->getPlaintextReason( $reason ); - if ( $this->getConfig()->get( 'CheckUserLogReasonMigrationStage' ) & SCHEMA_COMPAT_READ_NEW ) { - if ( $plaintextReason == '' ) { - return $queryInfo; - } + if ( $plaintextReason == '' ) { + return $queryInfo; + } - $plaintextReasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason_plaintext' ); - $queryInfo['tables'] += $plaintextReasonCommentQuery['tables']; - $queryInfo['fields'] += $plaintextReasonCommentQuery['fields']; - $queryInfo['join_conds'] += $plaintextReasonCommentQuery['joins']; + $plaintextReasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason_plaintext' ); + $queryInfo['tables'] += $plaintextReasonCommentQuery['tables']; + $queryInfo['fields'] += $plaintextReasonCommentQuery['fields']; + $queryInfo['join_conds'] += $plaintextReasonCommentQuery['joins']; - $queryInfo['conds'] = [ 'comment_cul_reason_plaintext.comment_text' => $plaintextReason ]; - } else { - $queryInfo['conds'] = [ $this->mDb->makeList( - [ - 'cul_reason = ' . $this->mDb->addQuotes( $reason ), - 'cul_reason = ' . $this->mDb->addQuotes( $plaintextReason ) - ], - ISQLPlatform::LIST_OR - ) ]; - } + $queryInfo['conds'] = [ 'comment_cul_reason_plaintext.comment_text' => $plaintextReason ]; return $queryInfo; } diff --git a/src/ServiceWiring.php b/src/ServiceWiring.php index 77a9998c5..91f0e63f0 100644 --- a/src/ServiceWiring.php +++ b/src/ServiceWiring.php @@ -40,8 +40,7 @@ $services->getCommentStore(), $services->getCommentFormatter(), LoggerFactory::getInstance( 'CheckUser' ), - $services->getActorStore(), - $services->getMainConfig()->get( 'CheckUserLogReasonMigrationStage' ) + $services->getActorStore() ); }, 'CheckUserCommentStore' => static function ( @@ -57,7 +56,7 @@ ): CheckUserLogCommentStore { return new CheckUserLogCommentStore( $services->getContentLanguage(), - $services->getMainConfig()->get( 'CheckUserLogReasonMigrationStage' ) + SCHEMA_COMPAT_NEW ); }, 'CheckUserPreliminaryCheckService' => static function ( diff --git a/src/Services/CheckUserLogService.php b/src/Services/CheckUserLogService.php index 7ead26f6b..14ef6b3e6 100644 --- a/src/Services/CheckUserLogService.php +++ b/src/Services/CheckUserLogService.php @@ -23,31 +23,25 @@ class CheckUserLogService { private LoggerInterface $logger; private ActorStore $actorStore; - /** @var int */ - private $culReasonMigrationStage; - /** * @param IConnectionProvider $dbProvider * @param CommentStore $commentStore * @param CommentFormatter $commentFormatter * @param LoggerInterface $logger * @param ActorStore $actorStore - * @param int $culReasonMigrationStage */ public function __construct( IConnectionProvider $dbProvider, CommentStore $commentStore, CommentFormatter $commentFormatter, LoggerInterface $logger, - ActorStore $actorStore, - int $culReasonMigrationStage + ActorStore $actorStore ) { $this->dbProvider = $dbProvider; $this->commentStore = $commentStore; $this->commentFormatter = $commentFormatter; $this->logger = $logger; $this->actorStore = $actorStore; - $this->culReasonMigrationStage = $culReasonMigrationStage; } /** @@ -90,30 +84,19 @@ public function addLogEntry( 'cul_range_end' => $rangeEnd ]; - if ( $this->culReasonMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { - $plaintextReason = $this->getPlaintextReason( $reason ); - } else { - $plaintextReason = ''; - } - - if ( $this->culReasonMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { - $data['cul_reason'] = $reason; - } + $plaintextReason = $this->getPlaintextReason( $reason ); $fname = __METHOD__; $commentStore = $this->commentStore; $logger = $this->logger; - $writeNew = $this->culReasonMigrationStage & SCHEMA_COMPAT_WRITE_NEW; DeferredUpdates::addCallableUpdate( static function () use ( - $data, $timestamp, $reason, $plaintextReason, $fname, $dbw, $commentStore, $logger, $writeNew + $data, $timestamp, $reason, $plaintextReason, $fname, $dbw, $commentStore, $logger ) { try { - if ( $writeNew ) { - $data += $commentStore->insert( $dbw, 'cul_reason', $reason ); - $data += $commentStore->insert( $dbw, 'cul_reason_plaintext', $plaintextReason ); - } + $data += $commentStore->insert( $dbw, 'cul_reason', $reason ); + $data += $commentStore->insert( $dbw, 'cul_reason_plaintext', $plaintextReason ); $dbw->newInsertQueryBuilder() ->insertInto( 'cu_log' ) ->row( diff --git a/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php b/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php index 775d95c93..0d3456851 100644 --- a/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php +++ b/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php @@ -145,10 +145,8 @@ public function testGetHelpUrls() { /** @dataProvider provideExampleLogEntryDataForReasonFilterTest */ public function testReasonFilter( - $logType, $targetType, $target, $reason, $targetID, $timestamp, - $reasonToSearchFor, $shouldSeeEntry, $logReasonMigrationStage + $logType, $targetType, $target, $reason, $targetID, $timestamp, $reasonToSearchFor, $shouldSeeEntry ) { - $this->setMwGlobals( 'wgCheckUserLogReasonMigrationStage', $logReasonMigrationStage ); /** @var CheckUserLogService $checkUserLogService */ $checkUserLogService = $this->getServiceContainer()->get( 'CheckUserLogService' ); $checkUserLogService->addLogEntry( @@ -166,12 +164,7 @@ public function testReasonFilter( $result = $this->doCheckUserLogApiRequest( [ 'culreason' => $checkUserLogService->getPlaintextReason( $reasonToSearchFor ) ] )[0]['query']['checkuserlog']['entries']; - if ( - $shouldSeeEntry && - # When SCHEMA_OLD is set, there is no way to search by a plaintext reason. - ( $checkUserLogService->getPlaintextReason( $reasonToSearchFor ) === $reasonToSearchFor || - $logReasonMigrationStage !== SCHEMA_COMPAT_OLD ) - ) { + if ( $shouldSeeEntry ) { $this->assertCount( 1, $result, 'A search for the plaintext version of the reason should show one entry.' ); diff --git a/tests/phpunit/integration/Services/CheckUserLogServiceTest.php b/tests/phpunit/integration/Services/CheckUserLogServiceTest.php index 6f9248c83..80649ab2e 100644 --- a/tests/phpunit/integration/Services/CheckUserLogServiceTest.php +++ b/tests/phpunit/integration/Services/CheckUserLogServiceTest.php @@ -154,72 +154,37 @@ public function testAddLogEntryPerformer() { ); } - /** - * Tests that nothing is written to - * the cul_reason_id or cul_reason_plaintext_id - * if the migration stage doesn't include write new. - * - * @covers ::addLogEntry - */ - public function testAddLogEntryReasonIdNoWriteNew() { - $object = $this->setUpObject(); - // 3 means READ_OLD and WRITE_OLD - $object->culReasonMigrationStage = 3; - $testUser = $this->getTestUser( 'checkuser' )->getUser(); - $object->addLogEntry( $testUser, 'ipusers', 'ip', '127.0.0.1', 'Test', 0 ); - \DeferredUpdates::doUpdates(); - // Be sure that the fields exist before testing. - if ( - $this->db->fieldExists( 'cu_log', 'cul_reason_id' ) && - $this->db->fieldExists( 'cu_log', 'cul_reason_plaintext_id' ) - ) { - $this->assertSelect( - 'cu_log', - [ 'cul_reason_plaintext_id', 'cul_reason_id' ], - [], - [ [ 0, 0 ] ] - ); - } else { - $this->expectNotToPerformAssertions(); - } - } - /** @dataProvider provideAddLogEntryReasonId */ public function testAddLogEntryReasonId( $reason, $expectedPlaintextReason ) { $object = $this->setUpObject(); - // Only attempt the test if culReasonMigrationStage says we can write to the new. - if ( $object->culReasonMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { - $testUser = $this->getTestUser( 'checkuser' )->getUser(); - $object->addLogEntry( $testUser, 'ipusers', 'ip', '127.0.0.1', $reason, 0 ); - DeferredUpdates::doUpdates(); - $commentQuery = $this->getServiceContainer()->getCommentStore()->getJoin( 'cul_reason' ); - $commentQuery['tables'][] = 'cu_log'; - $row = $this->db->newSelectQueryBuilder() - ->fields( $commentQuery['fields'] ) - ->tables( $commentQuery['tables'] ) - ->joinConds( $commentQuery['joins'] ) - ->fetchRow(); - $this->assertSame( - $reason, - $row->cul_reason_text, - 'The reason saved was not correctly saved.' - ); - - $commentQuery = $this->getServiceContainer()->getCommentStore()->getJoin( 'cul_reason_plaintext' ); - $commentQuery['tables'][] = 'cu_log'; - $row = $this->db->newSelectQueryBuilder() - ->fields( $commentQuery['fields'] ) - ->tables( $commentQuery['tables'] ) - ->joinConds( $commentQuery['joins'] ) - ->fetchRow(); - $this->assertSame( - $expectedPlaintextReason, - $row->cul_reason_plaintext_text, - 'The plaintext reason saved was not correctly saved.' - ); - } else { - $this->expectNotToPerformAssertions(); - } + $testUser = $this->getTestUser( 'checkuser' )->getUser(); + $object->addLogEntry( $testUser, 'ipusers', 'ip', '127.0.0.1', $reason, 0 ); + DeferredUpdates::doUpdates(); + $commentQuery = $this->getServiceContainer()->getCommentStore()->getJoin( 'cul_reason' ); + $commentQuery['tables'][] = 'cu_log'; + $row = $this->db->newSelectQueryBuilder() + ->fields( $commentQuery['fields'] ) + ->tables( $commentQuery['tables'] ) + ->joinConds( $commentQuery['joins'] ) + ->fetchRow(); + $this->assertSame( + $reason, + $row->cul_reason_text, + 'The reason saved was not correctly saved.' + ); + + $commentQuery = $this->getServiceContainer()->getCommentStore()->getJoin( 'cul_reason_plaintext' ); + $commentQuery['tables'][] = 'cu_log'; + $row = $this->db->newSelectQueryBuilder() + ->fields( $commentQuery['fields'] ) + ->tables( $commentQuery['tables'] ) + ->joinConds( $commentQuery['joins'] ) + ->fetchRow(); + $this->assertSame( + $expectedPlaintextReason, + $row->cul_reason_plaintext_text, + 'The plaintext reason saved was not correctly saved.' + ); } /** @dataProvider provideAddLogEntryReasonId */ From 6fafbd86b3beac57462c881c6f77d6205a8b54df Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:51 -0400 Subject: [PATCH 04/11] Reapply "Remove cul_reason column from cu_log" This reverts commit 28adb283ae608a15ab52769b3f1bb64debc3bb96. --- maintenance/populateCulComment.php | 21 ++- ...json => patch-cu_log-drop-cul_reason.json} | 16 +-- .../patch-cu_log-drop-cul_reason.sql} | 6 +- schema/mysql/tables-generated.sql | 1 - .../patch-cu_log-drop-cul_reason.sql} | 6 +- schema/postgres/tables-generated.sql | 1 - .../patch-cu_log-change-reason_default.sql | 18 --- .../sqlite/patch-cu_log-drop-cul_reason.sql | 18 +++ schema/sqlite/tables-generated.sql | 1 - schema/tables.json | 14 +- src/HookHandler/SchemaChangesHandler.php | 4 +- .../maintenance/PopulateCulCommentTest.php | 127 ++++++++++++++++++ 12 files changed, 179 insertions(+), 54 deletions(-) rename schema/abstractSchemaChanges/{patch-cu_log-change-reason_default.json => patch-cu_log-drop-cul_reason.json} (97%) rename schema/{postgres/patch-cu_log-change-reason_default.sql => mysql/patch-cu_log-drop-cul_reason.sql} (73%) rename schema/{mysql/patch-cu_log-change-reason_default.sql => postgres/patch-cu_log-drop-cul_reason.sql} (66%) delete mode 100644 schema/sqlite/patch-cu_log-change-reason_default.sql create mode 100644 schema/sqlite/patch-cu_log-drop-cul_reason.sql create mode 100644 tests/phpunit/integration/maintenance/PopulateCulCommentTest.php diff --git a/maintenance/populateCulComment.php b/maintenance/populateCulComment.php index 52bee68ff..ed868dc4b 100644 --- a/maintenance/populateCulComment.php +++ b/maintenance/populateCulComment.php @@ -23,6 +23,8 @@ use LoggedUpdateMaintenance; use MediaWiki\CheckUser\Services\CheckUserLogService; use MediaWiki\MediaWikiServices; +use Psr\Log\NullLogger; +use Wikimedia\Services\NoSuchServiceException; $IP = getenv( 'MW_INSTALL_PATH' ); if ( $IP === false ) { @@ -63,8 +65,23 @@ protected function getUpdateKey() { protected function doDBUpdates() { $services = MediaWikiServices::getInstance(); $commentStore = $services->getCommentStore(); - /** @var CheckUserLogService $checkUserLogService */ - $checkUserLogService = $services->get( 'CheckUserLogService' ); + try { + /** @var CheckUserLogService $checkUserLogService */ + $checkUserLogService = $services->get( 'CheckUserLogService' ); + } catch ( NoSuchServiceException $ex ) { + # CheckUser ServiceWiring files may not loaded until + # postDatabaseUpdateMaintenance is run. + # If this is the case, manually get the service. + $checkUserLogService = new CheckUserLogService( + $services->getDBLoadBalancerFactory(), + $services->getCommentStore(), + $services->getCommentFormatter(), + // No need to log as this maintenance script does not use any methods + // that use the logger. + new NullLogger(), + $services->getActorStore() + ); + } $mainLb = $services->getDBLoadBalancerFactory()->getMainLB(); $dbr = $mainLb->getConnection( DB_REPLICA, 'vslow' ); $dbw = $mainLb->getMaintenanceConnectionRef( DB_PRIMARY ); diff --git a/schema/abstractSchemaChanges/patch-cu_log-change-reason_default.json b/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason.json similarity index 97% rename from schema/abstractSchemaChanges/patch-cu_log-change-reason_default.json rename to schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason.json index aaf70d8d6..888e633f9 100644 --- a/schema/abstractSchemaChanges/patch-cu_log-change-reason_default.json +++ b/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason.json @@ -28,7 +28,11 @@ "name": "cul_reason", "comment": "Reason given", "type": "binary", - "options": { "notnull": true, "length": 255 } + "options": { + "notnull": true, + "length": 255, + "default": "" + } }, { "name": "cul_reason_id", @@ -141,16 +145,6 @@ "default": 0 } }, - { - "name": "cul_reason", - "comment": "Reason given", - "type": "binary", - "options": { - "notnull": true, - "length": 255, - "default": "" - } - }, { "name": "cul_reason_id", "comment": "Reason for the check stored as a comment_id. Default of 0 is used to indicate using cul_reason.", diff --git a/schema/postgres/patch-cu_log-change-reason_default.sql b/schema/mysql/patch-cu_log-drop-cul_reason.sql similarity index 73% rename from schema/postgres/patch-cu_log-change-reason_default.sql rename to schema/mysql/patch-cu_log-drop-cul_reason.sql index 2f444933d..239b69344 100644 --- a/schema/postgres/patch-cu_log-change-reason_default.sql +++ b/schema/mysql/patch-cu_log-drop-cul_reason.sql @@ -1,6 +1,6 @@ -- This file is automatically generated using maintenance/generateSchemaChangeSql.php. --- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-change-reason_default.json +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason.json -- Do not modify this file directly. -- See https://www.mediawiki.org/wiki/Manual:Schema_changes -ALTER TABLE cu_log ALTER cul_reason -SET DEFAULT ''; \ No newline at end of file +ALTER TABLE /*_*/cu_log +DROP cul_reason; diff --git a/schema/mysql/tables-generated.sql b/schema/mysql/tables-generated.sql index d47f5c534..ec5455746 100644 --- a/schema/mysql/tables-generated.sql +++ b/schema/mysql/tables-generated.sql @@ -103,7 +103,6 @@ CREATE TABLE /*_*/cu_log ( cul_id INT UNSIGNED AUTO_INCREMENT NOT NULL, cul_timestamp BINARY(14) NOT NULL, cul_actor BIGINT UNSIGNED NOT NULL, - cul_reason VARBINARY(255) DEFAULT '' NOT NULL, cul_reason_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_reason_plaintext_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_type VARBINARY(30) NOT NULL, diff --git a/schema/mysql/patch-cu_log-change-reason_default.sql b/schema/postgres/patch-cu_log-drop-cul_reason.sql similarity index 66% rename from schema/mysql/patch-cu_log-change-reason_default.sql rename to schema/postgres/patch-cu_log-drop-cul_reason.sql index 23c82dd96..fa3fc6752 100644 --- a/schema/mysql/patch-cu_log-change-reason_default.sql +++ b/schema/postgres/patch-cu_log-drop-cul_reason.sql @@ -1,6 +1,6 @@ -- This file is automatically generated using maintenance/generateSchemaChangeSql.php. --- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-change-reason_default.json +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason.json -- Do not modify this file directly. -- See https://www.mediawiki.org/wiki/Manual:Schema_changes -ALTER TABLE /*_*/cu_log -CHANGE cul_reason cul_reason VARBINARY(255) DEFAULT '' NOT NULL; \ No newline at end of file +ALTER TABLE cu_log +DROP cul_reason; diff --git a/schema/postgres/tables-generated.sql b/schema/postgres/tables-generated.sql index 8f6195152..e76aeb632 100644 --- a/schema/postgres/tables-generated.sql +++ b/schema/postgres/tables-generated.sql @@ -117,7 +117,6 @@ CREATE TABLE cu_log ( cul_id SERIAL NOT NULL, cul_timestamp TIMESTAMPTZ NOT NULL, cul_actor BIGINT NOT NULL, - cul_reason TEXT DEFAULT '' NOT NULL, cul_reason_id BIGINT DEFAULT 0 NOT NULL, cul_reason_plaintext_id BIGINT DEFAULT 0 NOT NULL, cul_type TEXT NOT NULL, diff --git a/schema/sqlite/patch-cu_log-change-reason_default.sql b/schema/sqlite/patch-cu_log-change-reason_default.sql deleted file mode 100644 index 17e18b2ec..000000000 --- a/schema/sqlite/patch-cu_log-change-reason_default.sql +++ /dev/null @@ -1,18 +0,0 @@ --- This file is automatically generated using maintenance/generateSchemaChangeSql.php. --- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-change-reason_default.json --- Do not modify this file directly. --- See https://www.mediawiki.org/wiki/Manual:Schema_changes -CREATE TEMPORARY TABLE /*_*/__temp__cu_log AS -SELECT cul_id, cul_timestamp, cul_actor, cul_reason, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end -FROM /*_*/cu_log; -DROP TABLE /*_*/cu_log; -CREATE TABLE /*_*/cu_log ( cul_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cul_timestamp BLOB NOT NULL, cul_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_reason BLOB DEFAULT '' NOT NULL, cul_reason_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_reason_plaintext_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_type BLOB NOT NULL, cul_target_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cul_target_text BLOB NOT NULL, cul_target_hex BLOB DEFAULT '' NOT NULL, cul_range_start BLOB DEFAULT '' NOT NULL, cul_range_end BLOB DEFAULT '' NOT NULL ); -INSERT INTO /*_*/cu_log ( cul_id, cul_timestamp, cul_actor, cul_reason, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end ) -SELECT cul_id, cul_timestamp, cul_actor, cul_reason, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end -FROM /*_*/__temp__cu_log; -DROP TABLE /*_*/__temp__cu_log; -CREATE INDEX cul_actor_time ON /*_*/cu_log (cul_actor, cul_timestamp); -CREATE INDEX cul_type_target ON /*_*/cu_log ( cul_type, cul_target_id, cul_timestamp ); -CREATE INDEX cul_target_hex ON /*_*/cu_log (cul_target_hex, cul_timestamp); -CREATE INDEX cul_range_start ON /*_*/cu_log (cul_range_start, cul_timestamp); -CREATE INDEX cul_timestamp ON /*_*/cu_log (cul_timestamp); \ No newline at end of file diff --git a/schema/sqlite/patch-cu_log-drop-cul_reason.sql b/schema/sqlite/patch-cu_log-drop-cul_reason.sql new file mode 100644 index 000000000..df4214232 --- /dev/null +++ b/schema/sqlite/patch-cu_log-drop-cul_reason.sql @@ -0,0 +1,18 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +CREATE TEMPORARY TABLE /*_*/__temp__cu_log AS +SELECT cul_id, cul_timestamp, cul_actor, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end +FROM /*_*/cu_log; +DROP TABLE /*_*/cu_log; +CREATE TABLE /*_*/cu_log ( cul_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cul_timestamp BLOB NOT NULL, cul_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_reason_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_reason_plaintext_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_type BLOB NOT NULL, cul_target_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cul_target_text BLOB NOT NULL, cul_target_hex BLOB DEFAULT '' NOT NULL, cul_range_start BLOB DEFAULT '' NOT NULL, cul_range_end BLOB DEFAULT '' NOT NULL ); +INSERT INTO /*_*/cu_log ( cul_id, cul_timestamp, cul_actor, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end ) +SELECT cul_id, cul_timestamp, cul_actor, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end +FROM /*_*/__temp__cu_log; +DROP TABLE /*_*/__temp__cu_log; +CREATE INDEX cul_actor_time ON /*_*/cu_log (cul_actor, cul_timestamp); +CREATE INDEX cul_type_target ON /*_*/cu_log ( cul_type, cul_target_id, cul_timestamp ); +CREATE INDEX cul_target_hex ON /*_*/cu_log (cul_target_hex, cul_timestamp); +CREATE INDEX cul_range_start ON /*_*/cu_log (cul_range_start, cul_timestamp); +CREATE INDEX cul_timestamp ON /*_*/cu_log (cul_timestamp); diff --git a/schema/sqlite/tables-generated.sql b/schema/sqlite/tables-generated.sql index 78c1bf8fd..ed4ea16a9 100644 --- a/schema/sqlite/tables-generated.sql +++ b/schema/sqlite/tables-generated.sql @@ -112,7 +112,6 @@ CREATE INDEX uachm_reference_id ON /*_*/cu_useragent_clienthints_map (uachm_refe CREATE TABLE /*_*/cu_log ( cul_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cul_timestamp BLOB NOT NULL, cul_actor BIGINT UNSIGNED NOT NULL, - cul_reason BLOB DEFAULT '' NOT NULL, cul_reason_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_reason_plaintext_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cul_type BLOB NOT NULL, cul_target_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, diff --git a/schema/tables.json b/schema/tables.json index 4b7c908c7..8c03d3d5e 100644 --- a/schema/tables.json +++ b/schema/tables.json @@ -429,19 +429,9 @@ "notnull": true } }, - { - "name": "cul_reason", - "comment": "Reason given", - "type": "binary", - "options": { - "notnull": true, - "length": 255, - "default": "" - } - }, { "name": "cul_reason_id", - "comment": "Reason for the check stored as a comment_id. Default of 0 is used to indicate using cul_reason.", + "comment": "Reason for the check stored as a comment_id.", "type": "bigint", "options": { "unsigned": true, @@ -451,7 +441,7 @@ }, { "name": "cul_reason_plaintext_id", - "comment": "Reason for the check with wikitext removed stored as a comment_id. Default of 0 is used to indicate using cul_reason.", + "comment": "Reason for the check with wikitext removed stored as a comment_id.", "type": "bigint", "options": { "unsigned": true, diff --git a/src/HookHandler/SchemaChangesHandler.php b/src/HookHandler/SchemaChangesHandler.php index 31c20eeff..3b4a7791c 100644 --- a/src/HookHandler/SchemaChangesHandler.php +++ b/src/HookHandler/SchemaChangesHandler.php @@ -245,10 +245,10 @@ public function onLoadExtensionSchemaUpdates( $updater ) { "$base/$dbType/patch-cu_log-drop-actor_default.sql" ); } - $updater->modifyExtensionField( + $updater->dropExtensionField( 'cu_log', 'cul_reason', - "$base/$dbType/patch-cu_log-change-reason_default.sql" + "$base/$dbType/patch-cu_log-drop-cul_reason.sql" ); $updater->dropExtensionField( 'cu_changes', diff --git a/tests/phpunit/integration/maintenance/PopulateCulCommentTest.php b/tests/phpunit/integration/maintenance/PopulateCulCommentTest.php new file mode 100644 index 000000000..f0a77752c --- /dev/null +++ b/tests/phpunit/integration/maintenance/PopulateCulCommentTest.php @@ -0,0 +1,127 @@ +tablesUsed = [ + 'cu_log', + 'comment' + ]; + } + + /** @inheritDoc */ + protected function getMaintenanceClass() { + return PopulateCulComment::class; + } + + /** @dataProvider provideAddLogEntryReasonId */ + public function testDoDBUpdatesSingleRow( $reason, $plaintextReason ) { + if ( $this->db->getType() === 'postgres' ) { + // The test is unable to add the column to the database + // as the maintenance script even after adding the column + // is unable to see it exists. + $this->markTestSkipped( 'This test does not work on postgres' ); + } + $testTarget = $this->getTestUser()->getUserIdentity(); + // Create a test cu_log entry with a cul_reason value. + $this->db->newInsertQueryBuilder() + ->insertInto( 'cu_log' ) + ->row( [ + 'cul_timestamp' => $this->db->timestamp( ConvertibleTimestamp::time() ), + 'cul_actor' => $this->getTestSysop()->getUser()->getActorId(), + 'cul_type' => 'user', + 'cul_target_id' => $testTarget->getId(), + 'cul_target_text' => $testTarget->getName(), + 'cul_reason' => $reason, + 'cul_reason_id' => 0, + 'cul_reason_plaintext_id' => 0 + ] ) + ->execute(); + // Run the maintenance script + $this->createMaintenance()->doDBUpdates(); + // Check that cul_reason is correct + $this->assertSelect( + 'cu_log', + [ 'cul_reason' ], + [], + [ [ $reason ] ] + ); + // Get the ID to the comment table stored in cu_log + $row = $this->db->newSelectQueryBuilder() + ->fields( [ 'cul_reason_id', 'cul_reason_plaintext_id' ] ) + ->table( 'cu_log' ) + ->fetchRow(); + // Check that the comment IDs are for rows that have the correct + // expected reason. + $this->assertSame( + $reason, + $this->db->newSelectQueryBuilder() + ->field( 'comment_text' ) + ->table( 'comment' ) + ->where( [ 'comment_id' => $row->cul_reason_id ] ) + ->fetchField(), + 'The cul_reason_id is for the wrong comment.' + ); + $this->assertSame( + $plaintextReason, + $this->db->newSelectQueryBuilder() + ->field( 'comment_text' ) + ->table( 'comment' ) + ->where( [ 'comment_id' => $row->cul_reason_plaintext_id ] ) + ->fetchField(), + 'The cul_reason_plaintext_id is for the wrong comment.' + ); + } + + public static function provideAddLogEntryReasonId() { + return [ + [ 'Testing 1234', 'Testing 1234' ], + [ 'Testing 1234 [[test]]', 'Testing 1234 test' ], + [ 'Testing 1234 [[:mw:Testing|test]]', 'Testing 1234 test' ], + [ 'Testing 1234 [test]', 'Testing 1234 [test]' ], + [ 'Testing 1234 [https://example.com]', 'Testing 1234 [https://example.com]' ], + [ 'Testing 1234 [[test]', 'Testing 1234 [[test]' ], + [ 'Testing 1234 [test]]', 'Testing 1234 [test]]' ], + [ 'Testing 1234 ', 'Testing 1234 ' ], + [ 'Testing 1234 {{test}}', 'Testing 1234 {{test}}' ], + [ 'Testing 12345 [[{{test}}]]', 'Testing 12345 [[{{test}}]]' ], + ]; + } + + public function addDBDataOnce() { + // Create cul_reason on the test DB. + // This is broken for postgres so no cul_reason + // is added for that DB type. + if ( $this->db->getType() === 'sqlite' ) { + $this->db->query( + "ALTER TABLE " . + $this->db->tableName( 'cu_log' ) . + " ADD cul_reason BLOB DEFAULT '' NOT NULL;", + __METHOD__ + ); + } elseif ( $this->db->getType() !== 'postgres' ) { + $this->db->query( + "ALTER TABLE " . + $this->db->tableName( 'cu_log' ) . + " ADD cul_reason VARBINARY(255) DEFAULT '' NOT NULL;", + __METHOD__ + ); + } + } +} From 49a94457c9950b7e87599f914932f0911c7463d3 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:51 -0400 Subject: [PATCH 05/11] Reapply "Remove default from cu_log comment table ID fields" This reverts commit af7c5baee07c02cc7d6dfe1bae81bae886473c6f. --- ...tch-cu_log-drop-cul_reason_id_default.json | 220 ++++++++++++++++++ ...atch-cu_log-drop-cul_reason_id_default.sql | 7 + schema/mysql/tables-generated.sql | 4 +- ...atch-cu_log-drop-cul_reason_id_default.sql | 8 + schema/postgres/tables-generated.sql | 4 +- ...atch-cu_log-drop-cul_reason_id_default.sql | 18 ++ schema/sqlite/tables-generated.sql | 4 +- schema/tables.json | 6 +- src/HookHandler/SchemaChangesHandler.php | 5 + 9 files changed, 266 insertions(+), 10 deletions(-) create mode 100644 schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json create mode 100644 schema/mysql/patch-cu_log-drop-cul_reason_id_default.sql create mode 100644 schema/postgres/patch-cu_log-drop-cul_reason_id_default.sql create mode 100644 schema/sqlite/patch-cu_log-drop-cul_reason_id_default.sql diff --git a/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json b/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json new file mode 100644 index 000000000..02d0a7506 --- /dev/null +++ b/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json @@ -0,0 +1,220 @@ +{ + "before": { + "name": "cu_log", + "columns": [ + { + "name": "cul_id", + "comment": "Unique identifier", + "type": "integer", + "options": { "autoincrement": true, "notnull": true, "unsigned": true } + }, + { + "name": "cul_timestamp", + "comment": "Timestamp of CheckUser action", + "type": "mwtimestamp", + "options": { "notnull": true } + }, + { + "name": "cul_actor", + "comment": "User who performed the action", + "type": "bigint", + "options": { + "unsigned": true, + "notnull": true + } + }, + { + "name": "cul_reason_id", + "comment": "Reason for the check stored as a comment_id.", + "type": "bigint", + "options": { + "unsigned": true, + "notnull": true, + "default": 0 + } + }, + { + "name": "cul_reason_plaintext_id", + "comment": "Reason for the check with wikitext removed stored as a comment_id.", + "type": "bigint", + "options": { + "unsigned": true, + "notnull": true, + "default": 0 + } + }, + { + "name": "cul_type", + "comment": "String indicating the type of query, may be: 'useredits', 'userips', 'ipedits', 'ipusers', 'ipedits-xff', 'ipusers-xff' or 'investigate' if the check was performed from Special:Investigate", + "type": "binary", + "options": { "notnull": true, "length": 30 } + }, + { + "name": "cul_target_id", + "comment": " Integer target, interpretation depends on cul_type For username targets, this is the user_id", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cul_target_text", + "comment": "Text target, interpretation depends on cul_type", + "type": "blob", + "options": { "notnull": true, "length": 65530 } + }, + { + "name": "cul_target_hex", + "comment": "If the target was an IP address, this contains the hexadecimal form of the IP", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cul_range_start", + "comment": "If the target was an IP range, this field contain the start, in hex form", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cul_range_end", + "comment": "If the target was an IP range, this field contain the end, in hex form", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + } + ], + "indexes": [ + { + "name": "cul_actor_time", + "columns": [ "cul_actor", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_type_target", + "columns": [ "cul_type", "cul_target_id", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_target_hex", + "columns": [ "cul_target_hex", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_range_start", + "columns": [ "cul_range_start", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_timestamp", + "columns": [ "cul_timestamp" ], + "unique": false + } + ], + "pk": [ "cul_id" ] + }, + "after": { + "name": "cu_log", + "columns": [ + { + "name": "cul_id", + "comment": "Unique identifier", + "type": "integer", + "options": { "autoincrement": true, "notnull": true, "unsigned": true } + }, + { + "name": "cul_timestamp", + "comment": "Timestamp of CheckUser action", + "type": "mwtimestamp", + "options": { "notnull": true } + }, + { + "name": "cul_actor", + "comment": "User who performed the action", + "type": "bigint", + "options": { + "unsigned": true, + "notnull": true + } + }, + { + "name": "cul_reason_id", + "comment": "Reason for the check stored as a comment_id.", + "type": "bigint", + "options": { + "unsigned": true, + "notnull": true + } + }, + { + "name": "cul_reason_plaintext_id", + "comment": "Reason for the check with wikitext removed stored as a comment_id.", + "type": "bigint", + "options": { + "unsigned": true, + "notnull": true + } + }, + { + "name": "cul_type", + "comment": "String indicating the type of query, may be: 'useredits', 'userips', 'ipedits', 'ipusers', 'ipedits-xff', 'ipusers-xff' or 'investigate' if the check was performed from Special:Investigate", + "type": "binary", + "options": { "notnull": true, "length": 30 } + }, + { + "name": "cul_target_id", + "comment": " Integer target, interpretation depends on cul_type For username targets, this is the user_id", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cul_target_text", + "comment": "Text target, interpretation depends on cul_type", + "type": "blob", + "options": { "notnull": true, "length": 65530 } + }, + { + "name": "cul_target_hex", + "comment": "If the target was an IP address, this contains the hexadecimal form of the IP", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cul_range_start", + "comment": "If the target was an IP range, this field contain the start, in hex form", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cul_range_end", + "comment": "If the target was an IP range, this field contain the end, in hex form", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + } + ], + "indexes": [ + { + "name": "cul_actor_time", + "columns": [ "cul_actor", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_type_target", + "columns": [ "cul_type", "cul_target_id", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_target_hex", + "columns": [ "cul_target_hex", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_range_start", + "columns": [ "cul_range_start", "cul_timestamp" ], + "unique": false + }, + { + "name": "cul_timestamp", + "columns": [ "cul_timestamp" ], + "unique": false + } + ], + "pk": [ "cul_id" ] + } +} diff --git a/schema/mysql/patch-cu_log-drop-cul_reason_id_default.sql b/schema/mysql/patch-cu_log-drop-cul_reason_id_default.sql new file mode 100644 index 000000000..91646909b --- /dev/null +++ b/schema/mysql/patch-cu_log-drop-cul_reason_id_default.sql @@ -0,0 +1,7 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +ALTER TABLE /*_*/cu_log +CHANGE cul_reason_id cul_reason_id BIGINT UNSIGNED NOT NULL, +CHANGE cul_reason_plaintext_id cul_reason_plaintext_id BIGINT UNSIGNED NOT NULL; \ No newline at end of file diff --git a/schema/mysql/tables-generated.sql b/schema/mysql/tables-generated.sql index ec5455746..7dcdb228a 100644 --- a/schema/mysql/tables-generated.sql +++ b/schema/mysql/tables-generated.sql @@ -103,8 +103,8 @@ CREATE TABLE /*_*/cu_log ( cul_id INT UNSIGNED AUTO_INCREMENT NOT NULL, cul_timestamp BINARY(14) NOT NULL, cul_actor BIGINT UNSIGNED NOT NULL, - cul_reason_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, - cul_reason_plaintext_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, + cul_reason_id BIGINT UNSIGNED NOT NULL, + cul_reason_plaintext_id BIGINT UNSIGNED NOT NULL, cul_type VARBINARY(30) NOT NULL, cul_target_id INT UNSIGNED DEFAULT 0 NOT NULL, cul_target_text BLOB NOT NULL, diff --git a/schema/postgres/patch-cu_log-drop-cul_reason_id_default.sql b/schema/postgres/patch-cu_log-drop-cul_reason_id_default.sql new file mode 100644 index 000000000..94ff73adc --- /dev/null +++ b/schema/postgres/patch-cu_log-drop-cul_reason_id_default.sql @@ -0,0 +1,8 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +ALTER TABLE cu_log ALTER cul_reason_id +DROP DEFAULT; +ALTER TABLE cu_log ALTER cul_reason_plaintext_id +DROP DEFAULT; \ No newline at end of file diff --git a/schema/postgres/tables-generated.sql b/schema/postgres/tables-generated.sql index e76aeb632..5a81b26f7 100644 --- a/schema/postgres/tables-generated.sql +++ b/schema/postgres/tables-generated.sql @@ -117,8 +117,8 @@ CREATE TABLE cu_log ( cul_id SERIAL NOT NULL, cul_timestamp TIMESTAMPTZ NOT NULL, cul_actor BIGINT NOT NULL, - cul_reason_id BIGINT DEFAULT 0 NOT NULL, - cul_reason_plaintext_id BIGINT DEFAULT 0 NOT NULL, + cul_reason_id BIGINT NOT NULL, + cul_reason_plaintext_id BIGINT NOT NULL, cul_type TEXT NOT NULL, cul_target_id INT DEFAULT 0 NOT NULL, cul_target_text TEXT NOT NULL, diff --git a/schema/sqlite/patch-cu_log-drop-cul_reason_id_default.sql b/schema/sqlite/patch-cu_log-drop-cul_reason_id_default.sql new file mode 100644 index 000000000..b537ba5f0 --- /dev/null +++ b/schema/sqlite/patch-cu_log-drop-cul_reason_id_default.sql @@ -0,0 +1,18 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_log-drop-cul_reason_id_default.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +CREATE TEMPORARY TABLE /*_*/__temp__cu_log AS +SELECT cul_id, cul_timestamp, cul_actor, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end +FROM /*_*/cu_log; +DROP TABLE /*_*/cu_log; +CREATE TABLE /*_*/cu_log ( cul_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cul_timestamp BLOB NOT NULL, cul_actor BIGINT UNSIGNED NOT NULL, cul_reason_id BIGINT UNSIGNED NOT NULL, cul_reason_plaintext_id BIGINT UNSIGNED NOT NULL, cul_type BLOB NOT NULL, cul_target_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cul_target_text BLOB NOT NULL, cul_target_hex BLOB DEFAULT '' NOT NULL, cul_range_start BLOB DEFAULT '' NOT NULL, cul_range_end BLOB DEFAULT '' NOT NULL ); +INSERT INTO /*_*/cu_log ( cul_id, cul_timestamp, cul_actor, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end ) +SELECT cul_id, cul_timestamp, cul_actor, cul_reason_id, cul_reason_plaintext_id, cul_type, cul_target_id, cul_target_text, cul_target_hex, cul_range_start, cul_range_end +FROM /*_*/__temp__cu_log; +DROP TABLE /*_*/__temp__cu_log; +CREATE INDEX cul_actor_time ON /*_*/cu_log (cul_actor, cul_timestamp); +CREATE INDEX cul_type_target ON /*_*/cu_log ( cul_type, cul_target_id, cul_timestamp ); +CREATE INDEX cul_target_hex ON /*_*/cu_log (cul_target_hex, cul_timestamp); +CREATE INDEX cul_range_start ON /*_*/cu_log (cul_range_start, cul_timestamp); +CREATE INDEX cul_timestamp ON /*_*/cu_log (cul_timestamp); \ No newline at end of file diff --git a/schema/sqlite/tables-generated.sql b/schema/sqlite/tables-generated.sql index ed4ea16a9..534d54030 100644 --- a/schema/sqlite/tables-generated.sql +++ b/schema/sqlite/tables-generated.sql @@ -112,8 +112,8 @@ CREATE INDEX uachm_reference_id ON /*_*/cu_useragent_clienthints_map (uachm_refe CREATE TABLE /*_*/cu_log ( cul_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cul_timestamp BLOB NOT NULL, cul_actor BIGINT UNSIGNED NOT NULL, - cul_reason_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, - cul_reason_plaintext_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, + cul_reason_id BIGINT UNSIGNED NOT NULL, + cul_reason_plaintext_id BIGINT UNSIGNED NOT NULL, cul_type BLOB NOT NULL, cul_target_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cul_target_text BLOB NOT NULL, cul_target_hex BLOB DEFAULT '' NOT NULL, cul_range_start BLOB DEFAULT '' NOT NULL, diff --git a/schema/tables.json b/schema/tables.json index 8c03d3d5e..dc992c57c 100644 --- a/schema/tables.json +++ b/schema/tables.json @@ -435,8 +435,7 @@ "type": "bigint", "options": { "unsigned": true, - "notnull": true, - "default": 0 + "notnull": true } }, { @@ -445,8 +444,7 @@ "type": "bigint", "options": { "unsigned": true, - "notnull": true, - "default": 0 + "notnull": true } }, { diff --git a/src/HookHandler/SchemaChangesHandler.php b/src/HookHandler/SchemaChangesHandler.php index 3b4a7791c..d52459769 100644 --- a/src/HookHandler/SchemaChangesHandler.php +++ b/src/HookHandler/SchemaChangesHandler.php @@ -250,6 +250,11 @@ public function onLoadExtensionSchemaUpdates( $updater ) { 'cul_reason', "$base/$dbType/patch-cu_log-drop-cul_reason.sql" ); + $updater->modifyExtensionField( + 'cu_log', + 'cul_reason_id', + "$base/$dbType/patch-cu_log-drop-cul_reason_id_default.sql" + ); $updater->dropExtensionField( 'cu_changes', 'cuc_user', From b16b6acdca33594cfe9702c8373118922f7caa59 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:51 -0400 Subject: [PATCH 06/11] Reapply "Drop cuc_comment_id migration variable" This reverts commit 2e176e95883caf7ae8e2ca0e83d14aeed77399f6. --- extension.json | 3 --- maintenance/moveLogEntriesFromCuChanges.php | 15 ++------------- maintenance/populateCheckUserTable.php | 14 +------------- src/Hooks.php | 11 ++--------- src/ServiceWiring.php | 2 +- .../Rest/Handler/TemporaryAccountHandlerTest.php | 2 +- .../TemporaryAccountRevisionHandlerTest.php | 2 +- .../Pagers/CheckUserGetEditsPagerTest.php | 1 - tests/phpunit/integration/HooksTest.php | 5 ++--- .../Investigate/Pagers/ComparePagerTest.php | 2 +- .../Investigate/Services/CompareServiceTest.php | 2 +- 11 files changed, 12 insertions(+), 47 deletions(-) diff --git a/extension.json b/extension.json index aac180a50..5f304a710 100644 --- a/extension.json +++ b/extension.json @@ -87,9 +87,6 @@ "value": 86400, "description": "Number of seconds for which the temporary account API response is fresh" }, - "CheckUserCommentMigrationStage": { - "value": 769 - }, "CheckUserEventTablesMigrationStage": { "value": 259, "description": "A flag used as the migration stage to the new cu_private_event and cu_log_event tables. Currently set to SCHEMA_COMPAT_OLD | SCHEMA_COMPAT_WRITE_NEW which evaluates to the integer 259." diff --git a/maintenance/moveLogEntriesFromCuChanges.php b/maintenance/moveLogEntriesFromCuChanges.php index 6b68f9af8..8215878d6 100644 --- a/maintenance/moveLogEntriesFromCuChanges.php +++ b/maintenance/moveLogEntriesFromCuChanges.php @@ -95,9 +95,6 @@ private function moveLogEntriesFromCuChanges() { "Moving log entries from cu_changes to cu_private_event with cuc_id from $start to $end.\n" ); - $commentMigrationStage = $services->getMainConfig()->get( 'CheckUserCommentMigrationStage' ); - $commentStore = $services->getCommentStore(); - while ( $blockStart <= $end ) { $this->output( "...checking and moving log entries with cuc_id from $blockStart to $blockEnd\n" ); $res = $dbw->newSelectQueryBuilder() @@ -107,7 +104,6 @@ private function moveLogEntriesFromCuChanges() { 'cuc_title', 'cuc_actor', 'cuc_actiontext', - 'cuc_comment', 'cuc_comment_id', 'cuc_page_id', 'cuc_timestamp', @@ -129,7 +125,7 @@ private function moveLogEntriesFromCuChanges() { $batch = []; $setOnlyForReadOldBatch = []; foreach ( $res as $row ) { - $entry = [ + $batch[] = [ 'cupe_timestamp' => $row->cuc_timestamp, 'cupe_namespace' => $row->cuc_namespace, 'cupe_title' => $row->cuc_title, @@ -138,6 +134,7 @@ private function moveLogEntriesFromCuChanges() { 'cupe_log_action' => 'migrated-cu_changes-log-event', 'cupe_log_type' => 'checkuser-private-event', 'cupe_params' => LogEntryBase::makeParamBlob( [ '4::actiontext' => $row->cuc_actiontext ] ), + 'cupe_comment_id' => $row->cuc_comment_id, 'cupe_ip' => $row->cuc_ip, 'cupe_ip_hex' => $row->cuc_ip_hex, 'cupe_xff' => $row->cuc_xff, @@ -145,14 +142,6 @@ private function moveLogEntriesFromCuChanges() { 'cupe_agent' => $row->cuc_agent, 'cupe_private' => $row->cuc_private ]; - - if ( $commentMigrationStage & SCHEMA_COMPAT_READ_NEW ) { - $entry['cupe_comment_id'] = $row->cuc_comment_id; - } else { - $entry['cupe_comment_id'] = $commentStore->createComment( $dbw, $row->cuc_comment )->id; - } - - $batch[] = $entry; $setOnlyForReadOldBatch[] = $row->cuc_id; } if ( count( $batch ) ) { diff --git a/maintenance/populateCheckUserTable.php b/maintenance/populateCheckUserTable.php index 2a598206c..52b6e2cc3 100644 --- a/maintenance/populateCheckUserTable.php +++ b/maintenance/populateCheckUserTable.php @@ -4,7 +4,6 @@ use DatabaseLogEntry; use LoggedUpdateMaintenance; -use MediaWiki\CheckUser\Hooks; use MediaWiki\MediaWikiServices; use RecentChange; use Wikimedia\IPUtils; @@ -90,12 +89,8 @@ protected function doDBUpdates() { $services = MediaWikiServices::getInstance(); $lbFactory = $services->getDBLoadBalancerFactory(); - - $commentMigrationStage = $services->getMainConfig()->get( 'CheckUserCommentMigrationStage' ); - $commentStore = $services->getCommentStore(); $rcQuery = RecentChange::getQueryInfo(); - $contLang = $services->getContentLanguage(); while ( $blockStart <= $end ) { $this->output( "...migrating rc_id from $blockStart to $blockEnd\n" ); @@ -155,6 +150,7 @@ protected function doDBUpdates() { 'cuc_namespace' => $row->rc_namespace, 'cuc_title' => $row->rc_title, 'cuc_actor' => $row->rc_actor, + 'cuc_comment_id' => $comment->id, 'cuc_minor' => $row->rc_minor, 'cuc_page_id' => $row->rc_cur_id, 'cuc_this_oldid' => $row->rc_this_oldid, @@ -164,14 +160,6 @@ protected function doDBUpdates() { 'cuc_ip_hex' => IPUtils::toHex( $row->rc_ip ), 'cuc_only_for_read_old' => 0, ]; - if ( $commentMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { - $cuChangesRow['cuc_comment'] = $contLang->truncateForDatabase( - $comment->text, Hooks::TEXT_FIELD_LENGTH - ); - } - if ( $commentMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { - $cuChangesRow['cuc_comment_id'] = $comment->id; - } if ( $row->rc_type == RC_LOG && ( $eventTablesMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) diff --git a/src/Hooks.php b/src/Hooks.php index 78aaa071c..fea694e12 100644 --- a/src/Hooks.php +++ b/src/Hooks.php @@ -47,7 +47,7 @@ class Hooks implements /** * The maximum number of bytes that fit in CheckUser's text fields - * (cuc_agent,cuc_actiontext,cuc_comment,cuc_xff) + * (cuc_agent,cuc_actiontext,cuc_xff) */ public const TEXT_FIELD_LENGTH = 255; @@ -445,8 +445,6 @@ private static function insertIntoCuChangesTable( ); $row['cuc_xff'] = $contLang->truncateForDatabase( $row['cuc_xff'], self::TEXT_FIELD_LENGTH ); - $commentMigrationStage = $services->getMainConfig()->get( 'CheckUserCommentMigrationStage' ); - if ( !isset( $row['cuc_actor'] ) ) { $row['cuc_actor'] = $services->getActorStore()->acquireActorId( $user, @@ -461,12 +459,7 @@ private static function insertIntoCuChangesTable( $row['cuc_comment'] ); } - - if ( $commentMigrationStage & SCHEMA_COMPAT_WRITE_OLD ) { - $row['cuc_comment'] = $contLang->truncateForDatabase( $row['cuc_comment'], self::TEXT_FIELD_LENGTH ); - } else { - unset( $row['cuc_comment'] ); - } + unset( $row['cuc_comment'] ); $dbw->newInsertQueryBuilder() ->insertInto( 'cu_changes' ) diff --git a/src/ServiceWiring.php b/src/ServiceWiring.php index 91f0e63f0..9856b966a 100644 --- a/src/ServiceWiring.php +++ b/src/ServiceWiring.php @@ -48,7 +48,7 @@ ): CheckUserCommentStore { return new CheckUserCommentStore( $services->getContentLanguage(), - $services->getMainConfig()->get( 'CheckUserCommentMigrationStage' ) + SCHEMA_COMPAT_NEW ); }, 'CheckUserLogCommentStore' => static function ( diff --git a/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountHandlerTest.php b/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountHandlerTest.php index d3220e51a..17a80c4bb 100644 --- a/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountHandlerTest.php +++ b/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountHandlerTest.php @@ -410,7 +410,7 @@ public function addDBData() { 'cuc_xff' => 0, 'cuc_xff_hex' => null, 'cuc_actiontext' => '', - 'cuc_comment' => '', + 'cuc_comment_id' => 0, 'cuc_last_oldid' => 0, ]; diff --git a/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountRevisionHandlerTest.php b/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountRevisionHandlerTest.php index 30ce5e322..09730563d 100644 --- a/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountRevisionHandlerTest.php +++ b/tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountRevisionHandlerTest.php @@ -194,7 +194,7 @@ public function addDBData() { 'cuc_xff' => 0, 'cuc_xff_hex' => null, 'cuc_actiontext' => '', - 'cuc_comment' => '', + 'cuc_comment_id' => 0, 'cuc_last_oldid' => 0, ]; diff --git a/tests/phpunit/integration/CheckUser/Pagers/CheckUserGetEditsPagerTest.php b/tests/phpunit/integration/CheckUser/Pagers/CheckUserGetEditsPagerTest.php index 71e0f2308..c2cfefc19 100644 --- a/tests/phpunit/integration/CheckUser/Pagers/CheckUserGetEditsPagerTest.php +++ b/tests/phpunit/integration/CheckUser/Pagers/CheckUserGetEditsPagerTest.php @@ -339,7 +339,6 @@ public function getDefaultRowFieldValues(): array { 'ip' => '127.0.0.1', 'xff' => '', 'agent' => '', - 'comment' => '', 'comment_id' => 0, 'comment_text' => '', 'comment_data' => null, diff --git a/tests/phpunit/integration/HooksTest.php b/tests/phpunit/integration/HooksTest.php index 94b2c6afe..d95973065 100644 --- a/tests/phpunit/integration/HooksTest.php +++ b/tests/phpunit/integration/HooksTest.php @@ -111,8 +111,8 @@ public static function provideInsertIntoCuChangesTable() { 'Other defaults' => [ [], [ 'cuc_page_id', 'cuc_namespace', 'cuc_minor', 'cuc_title', 'cuc_actiontext', - 'cuc_comment', 'cuc_this_oldid', 'cuc_last_oldid', 'cuc_type', 'cuc_agent' ], - [ 0, NS_MAIN, 0, '', '', '', 0, 0, RC_LOG, '' ] + 'cuc_this_oldid', 'cuc_last_oldid', 'cuc_type', 'cuc_agent' ], + [ 0, NS_MAIN, 0, '', '', 0, 0, RC_LOG, '' ] ] ]; } @@ -128,7 +128,6 @@ public function testTruncationInsertIntoCuChangesTable( string $field ) { public static function provideTestTruncationInsertIntoCuChangesTable() { return [ - 'Comment text column' => [ 'cuc_comment' ], 'Action text column' => [ 'cuc_actiontext' ], 'XFF column' => [ 'cuc_xff' ] ]; diff --git a/tests/phpunit/integration/Investigate/Pagers/ComparePagerTest.php b/tests/phpunit/integration/Investigate/Pagers/ComparePagerTest.php index b16cfe8c9..2f86a031d 100644 --- a/tests/phpunit/integration/Investigate/Pagers/ComparePagerTest.php +++ b/tests/phpunit/integration/Investigate/Pagers/ComparePagerTest.php @@ -192,7 +192,7 @@ public function addDBData() { 'cuc_xff' => 0, 'cuc_xff_hex' => null, 'cuc_actiontext' => '', - 'cuc_comment' => '', + 'cuc_comment_id' => 0, 'cuc_this_oldid' => 0, 'cuc_last_oldid' => 0, ]; diff --git a/tests/phpunit/integration/Investigate/Services/CompareServiceTest.php b/tests/phpunit/integration/Investigate/Services/CompareServiceTest.php index a2592296f..1373a18c2 100644 --- a/tests/phpunit/integration/Investigate/Services/CompareServiceTest.php +++ b/tests/phpunit/integration/Investigate/Services/CompareServiceTest.php @@ -419,7 +419,7 @@ public function addDBData() { 'cuc_xff' => 0, 'cuc_xff_hex' => null, 'cuc_actiontext' => '', - 'cuc_comment' => '', + 'cuc_comment_id' => 0, 'cuc_this_oldid' => 0, 'cuc_last_oldid' => 0, ]; From fee5f7a0d32bca95a3c90e695124a567e272e756 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:52 -0400 Subject: [PATCH 07/11] Reapply "Remove CheckUserLogCommentStore as cu_log comment migration is done" This reverts commit 189773e5b8f0124a46860874333ea6dd421e2f80. --- extension.json | 4 +-- maintenance/importCheckUserLogs.php | 8 ++--- src/Api/ApiQueryCheckUserLog.php | 16 ++++----- src/CheckUser/Pagers/CheckUserLogPager.php | 16 ++++----- src/CheckUser/SpecialCheckUserLog.php | 12 +++---- src/CheckUserLogCommentStore.php | 42 ---------------------- src/ServiceWiring.php | 9 ----- 7 files changed, 27 insertions(+), 80 deletions(-) delete mode 100644 src/CheckUserLogCommentStore.php diff --git a/extension.json b/extension.json index 5f304a710..ebac7d8d5 100644 --- a/extension.json +++ b/extension.json @@ -178,7 +178,7 @@ "checkuserlog": { "class": "MediaWiki\\CheckUser\\Api\\ApiQueryCheckUserLog", "services": [ - "CheckUserLogCommentStore", + "CommentStore", "CheckUserLogService", "UserFactory" ] @@ -524,7 +524,7 @@ "services": [ "LinkBatchFactory", "PermissionManager", - "CheckUserLogCommentStore", + "CommentStore", "CommentFormatter", "CheckUserLogService", "UserFactory", diff --git a/maintenance/importCheckUserLogs.php b/maintenance/importCheckUserLogs.php index 59087ed4e..bdac6c65e 100644 --- a/maintenance/importCheckUserLogs.php +++ b/maintenance/importCheckUserLogs.php @@ -1,6 +1,5 @@ getUserFactory(); - /** @var CheckUserLogCommentStore $checkUserLogCommentStore */ - $checkUserLogCommentStore = $services->get( 'CheckUserLogCommentStore' ); + $commentStore = $services->getCommentStore(); /** @var CheckUserLogService $checkUserLogService */ $checkUserLogService = $services->get( 'CheckUserLogService' ); @@ -146,8 +144,8 @@ protected function importLog( $file ) { 'cul_range_start' => $start, 'cul_range_end' => $end ]; - $fields += $checkUserLogCommentStore->insert( $dbw, 'cul_reason', $data['reason'] ); - $fields += $checkUserLogCommentStore->insert( + $fields += $commentStore->insert( $dbw, 'cul_reason', $data['reason'] ); + $fields += $commentStore->insert( $dbw, 'cul_reason_plaintext', $checkUserLogService->getPlaintextReason( $data['reason'] ) ); diff --git a/src/Api/ApiQueryCheckUserLog.php b/src/Api/ApiQueryCheckUserLog.php index fcc516219..b6fbfcc29 100644 --- a/src/Api/ApiQueryCheckUserLog.php +++ b/src/Api/ApiQueryCheckUserLog.php @@ -7,7 +7,7 @@ use ApiQueryBase; use MediaWiki\CheckUser\CheckUser\Pagers\CheckUserLogPager; use MediaWiki\CheckUser\Services\CheckUserLogService; -use MediaWiki\CheckUser\CheckUserLogCommentStore; +use MediaWiki\CommentStore\CommentStore; use MediaWiki\User\UserFactory; use Wikimedia\IPUtils; use Wikimedia\ParamValidator\ParamValidator; @@ -17,25 +17,25 @@ * CheckUser API Query Module */ class ApiQueryCheckUserLog extends ApiQueryBase { - private CheckUserLogCommentStore $checkUserLogCommentStore; + private CommentStore $commentStore; private CheckUserLogService $checkUserLogService; private UserFactory $userFactory; /** * @param ApiQuery $query * @param string $moduleName - * @param CheckUserLogCommentStore $checkUserLogCommentStore + * @param CommentStore $commentStore * @param CheckUserLogService $checkUserLogService * @param UserFactory $userFactory */ public function __construct( $query, $moduleName, - CheckUserLogCommentStore $checkUserLogCommentStore, + CommentStore $commentStore, CheckUserLogService $checkUserLogService, UserFactory $userFactory ) { parent::__construct( $query, $moduleName, 'cul' ); - $this->checkUserLogCommentStore = $checkUserLogCommentStore; + $this->commentStore = $commentStore; $this->checkUserLogService = $checkUserLogService; $this->userFactory = $userFactory; } @@ -57,13 +57,13 @@ public function execute() { ]; $this->addJoinConds( [ 'actor' => [ 'JOIN', 'actor_id=cul_actor' ] ] ); - $reasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason' ); + $reasonCommentQuery = $this->commentStore->getJoin( 'cul_reason' ); $this->addTables( $reasonCommentQuery['tables'] ); $this->addJoinConds( $reasonCommentQuery['joins'] ); $fields += $reasonCommentQuery['fields']; if ( isset( $params['reason'] ) ) { - $plaintextReasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason_plaintext' ); + $plaintextReasonCommentQuery = $this->commentStore->getJoin( 'cul_reason_plaintext' ); $this->addTables( $plaintextReasonCommentQuery['tables'] ); $this->addJoinConds( $plaintextReasonCommentQuery['joins'] ); $fields += $plaintextReasonCommentQuery['fields']; @@ -125,7 +125,7 @@ public function execute() { 'timestamp' => wfTimestamp( TS_ISO_8601, $row->cul_timestamp ), 'checkuser' => $row->actor_name, 'type' => $row->cul_type, - 'reason' => $this->checkUserLogCommentStore->getComment( 'cul_reason', $row )->text, + 'reason' => $this->commentStore->getComment( 'cul_reason', $row )->text, 'target' => $row->cul_target_text, ]; diff --git a/src/CheckUser/Pagers/CheckUserLogPager.php b/src/CheckUser/Pagers/CheckUserLogPager.php index 987752df0..cebd61890 100644 --- a/src/CheckUser/Pagers/CheckUserLogPager.php +++ b/src/CheckUser/Pagers/CheckUserLogPager.php @@ -7,9 +7,9 @@ use Linker; use MediaWiki\Cache\LinkBatchFactory; use MediaWiki\CheckUser\CheckUser\SpecialCheckUserLog; -use MediaWiki\CheckUser\CheckUserLogCommentStore; use MediaWiki\CheckUser\Services\CheckUserLogService; use MediaWiki\CommentFormatter\CommentFormatter; +use MediaWiki\CommentStore\CommentStore; use MediaWiki\MediaWikiServices; use MediaWiki\User\ActorStore; use MediaWiki\User\UserFactory; @@ -25,9 +25,9 @@ class CheckUserLogPager extends RangeChronologicalPager { private array $opts; private LinkBatchFactory $linkBatchFactory; - private CheckUserLogCommentStore $checkUserLogCommentStore; private CommentFormatter $commentFormatter; private CheckUserLogService $checkUserLogService; + private CommentStore $commentStore; private UserFactory $userFactory; private ActorStore $actorStore; @@ -38,7 +38,7 @@ class CheckUserLogPager extends RangeChronologicalPager { * Start and end should be timestamps. Year and month are converted to end but ignored if end is * provided. * @param LinkBatchFactory $linkBatchFactory - * @param CheckUserLogCommentStore $checkUserLogCommentStore + * @param CommentStore $commentStore * @param CommentFormatter $commentFormatter * @param CheckUserLogService $checkUserLogService * @param UserFactory $userFactory @@ -48,7 +48,7 @@ public function __construct( IContextSource $context, array $opts, LinkBatchFactory $linkBatchFactory, - CheckUserLogCommentStore $checkUserLogCommentStore, + CommentStore $commentStore, CommentFormatter $commentFormatter, CheckUserLogService $checkUserLogService, UserFactory $userFactory, @@ -56,7 +56,7 @@ public function __construct( ) { parent::__construct( $context ); $this->linkBatchFactory = $linkBatchFactory; - $this->checkUserLogCommentStore = $checkUserLogCommentStore; + $this->commentStore = $commentStore; $this->commentFormatter = $commentFormatter; $this->checkUserLogService = $checkUserLogService; $this->userFactory = $userFactory; @@ -215,7 +215,7 @@ public function formatRow( $row ) { ) )->parse(); $rowContent .= $this->commentFormatter->formatBlock( - CheckUserLogCommentStore::getStore()->getComment( 'cul_reason', $row )->text + $this->commentStore->getComment( 'cul_reason', $row )->text ); $attribs = [ @@ -263,7 +263,7 @@ public function getQueryInfo() { 'options' => [], ]; - $reasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason' ); + $reasonCommentQuery = $this->commentStore->getJoin( 'cul_reason' ); $queryInfo['tables'] += $reasonCommentQuery['tables']; $queryInfo['fields'] += $reasonCommentQuery['fields']; $queryInfo['join_conds'] += $reasonCommentQuery['joins']; @@ -404,7 +404,7 @@ public function getQueryInfoForReasonSearch( string $reason ): array { return $queryInfo; } - $plaintextReasonCommentQuery = $this->checkUserLogCommentStore->getJoin( 'cul_reason_plaintext' ); + $plaintextReasonCommentQuery = $this->commentStore->getJoin( 'cul_reason_plaintext' ); $queryInfo['tables'] += $plaintextReasonCommentQuery['tables']; $queryInfo['fields'] += $plaintextReasonCommentQuery['fields']; $queryInfo['join_conds'] += $plaintextReasonCommentQuery['joins']; diff --git a/src/CheckUser/SpecialCheckUserLog.php b/src/CheckUser/SpecialCheckUserLog.php index 683eb06a3..cb277d4a1 100644 --- a/src/CheckUser/SpecialCheckUserLog.php +++ b/src/CheckUser/SpecialCheckUserLog.php @@ -7,9 +7,9 @@ use HTMLForm; use MediaWiki\Cache\LinkBatchFactory; use MediaWiki\CheckUser\CheckUser\Pagers\CheckUserLogPager; -use MediaWiki\CheckUser\CheckUserLogCommentStore; use MediaWiki\CheckUser\Services\CheckUserLogService; use MediaWiki\CommentFormatter\CommentFormatter; +use MediaWiki\CommentStore\CommentStore; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Title\Title; use MediaWiki\User\ActorStore; @@ -31,7 +31,7 @@ class SpecialCheckUserLog extends SpecialPage { private LinkBatchFactory $linkBatchFactory; private PermissionManager $permissionManager; - private CheckUserLogCommentStore $checkUserLogCommentStore; + private CommentStore $commentStore; private CommentFormatter $commentFormatter; private CheckUserLogService $checkUserLogService; private UserFactory $userFactory; @@ -40,7 +40,7 @@ class SpecialCheckUserLog extends SpecialPage { /** * @param LinkBatchFactory $linkBatchFactory * @param PermissionManager $permissionManager - * @param CheckUserLogCommentStore $checkUserLogCommentStore + * @param CommentStore $commentStore * @param CommentFormatter $commentFormatter * @param CheckUserLogService $checkUserLogService * @param UserFactory $userFactory @@ -50,7 +50,7 @@ class SpecialCheckUserLog extends SpecialPage { public function __construct( LinkBatchFactory $linkBatchFactory, PermissionManager $permissionManager, - CheckUserLogCommentStore $checkUserLogCommentStore, + CommentStore $commentStore, CommentFormatter $commentFormatter, CheckUserLogService $checkUserLogService, UserFactory $userFactory, @@ -60,7 +60,7 @@ public function __construct( parent::__construct( 'CheckUserLog', 'checkuser-log' ); $this->linkBatchFactory = $linkBatchFactory; $this->permissionManager = $permissionManager; - $this->checkUserLogCommentStore = $checkUserLogCommentStore; + $this->commentStore = $commentStore; $this->commentFormatter = $commentFormatter; $this->checkUserLogService = $checkUserLogService; $this->userFactory = $userFactory; @@ -145,7 +145,7 @@ public function execute( $par ) { $this->getContext(), $this->opts, $this->linkBatchFactory, - $this->checkUserLogCommentStore, + $this->commentStore, $this->commentFormatter, $this->checkUserLogService, $this->userFactory, diff --git a/src/CheckUserLogCommentStore.php b/src/CheckUserLogCommentStore.php deleted file mode 100644 index 9ce3d0d3c..000000000 --- a/src/CheckUserLogCommentStore.php +++ /dev/null @@ -1,42 +0,0 @@ -get( 'CheckUserLogCommentStore' ); - } - - /** - * @param Language $lang - * @param int $stage - */ - public function __construct( Language $lang, int $stage ) { - parent::__construct( [], $lang, $stage ); - } -} diff --git a/src/ServiceWiring.php b/src/ServiceWiring.php index 9856b966a..15e7e5c9f 100644 --- a/src/ServiceWiring.php +++ b/src/ServiceWiring.php @@ -1,7 +1,6 @@ static function ( - MediaWikiServices $services - ): CheckUserLogCommentStore { - return new CheckUserLogCommentStore( - $services->getContentLanguage(), - SCHEMA_COMPAT_NEW - ); - }, 'CheckUserPreliminaryCheckService' => static function ( MediaWikiServices $services ): PreliminaryCheckService { From 2e15cb2a3bf284d41d862ac280ca5460ea652bb4 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:52 -0400 Subject: [PATCH 08/11] Reapply "Remove CheckUserCommentStore as cu_changes comment migration is done" This reverts commit 50001664e3e3b89444b15a5ae633bd7f1a46fa0a. --- extension.json | 4 +- src/Api/ApiQueryCheckUser.php | 13 +++--- .../Pagers/CheckUserGetEditsPager.php | 10 +++-- src/CheckUser/SpecialCheckUser.php | 6 +++ src/CheckUserCommentStore.php | 42 ------------------- src/Hooks.php | 2 +- src/Investigate/Services/TimelineService.php | 26 +++++++++++- src/ServiceWiring.php | 12 +----- .../Api/ApiQueryCheckUserLogTest.php | 2 +- .../integration/Api/ApiQueryCheckUserTest.php | 2 +- .../Services/TimelineServiceTest.php | 3 +- 11 files changed, 55 insertions(+), 67 deletions(-) delete mode 100644 src/CheckUserCommentStore.php diff --git a/extension.json b/extension.json index ebac7d8d5..4129de325 100644 --- a/extension.json +++ b/extension.json @@ -172,7 +172,8 @@ "UserIdentityLookup", "RevisionLookup", "ArchivedRevisionLookup", - "CheckUserLogService" + "CheckUserLogService", + "CommentStore" ] }, "checkuserlog": { @@ -515,6 +516,7 @@ "UserNameUtils", "CheckUserHookRunner", "CheckUserUtilityService", + "CommentStore", "UserAgentClientHintsLookup", "UserAgentClientHintsFormatter" ] diff --git a/src/Api/ApiQueryCheckUser.php b/src/Api/ApiQueryCheckUser.php index 1e1fd85a5..86f4c627e 100644 --- a/src/Api/ApiQueryCheckUser.php +++ b/src/Api/ApiQueryCheckUser.php @@ -8,8 +8,8 @@ use ApiResult; use Exception; use MediaWiki\CheckUser\CheckUser\Pagers\AbstractCheckUserPager; -use MediaWiki\CheckUser\CheckUserCommentStore; use MediaWiki\CheckUser\Services\CheckUserLogService; +use MediaWiki\CommentStore\CommentStore; use MediaWiki\Revision\ArchivedRevisionLookup; use MediaWiki\Revision\RevisionLookup; use MediaWiki\Revision\RevisionRecord; @@ -28,6 +28,7 @@ class ApiQueryCheckUser extends ApiQueryBase { private RevisionLookup $revisionLookup; private ArchivedRevisionLookup $archivedRevisionLookup; private CheckUserLogService $checkUserLogService; + private CommentStore $commentStore; /** * @param ApiQuery $query @@ -36,6 +37,7 @@ class ApiQueryCheckUser extends ApiQueryBase { * @param RevisionLookup $revisionLookup * @param ArchivedRevisionLookup $archivedRevisionLookup * @param CheckUserLogService $checkUserLogService + * @param CommentStore $commentStore */ public function __construct( $query, @@ -43,13 +45,15 @@ public function __construct( UserIdentityLookup $userIdentityLookup, RevisionLookup $revisionLookup, ArchivedRevisionLookup $archivedRevisionLookup, - CheckUserLogService $checkUserLogService + CheckUserLogService $checkUserLogService, + CommentStore $commentStore ) { parent::__construct( $query, $moduleName, 'cu' ); $this->userIdentityLookup = $userIdentityLookup; $this->revisionLookup = $revisionLookup; $this->archivedRevisionLookup = $archivedRevisionLookup; $this->checkUserLogService = $checkUserLogService; + $this->commentStore = $commentStore; } public function execute() { @@ -80,8 +84,7 @@ public function execute() { $targetTitle = Title::makeTitleSafe( NS_USER, $target ); $target = $targetTitle ? $targetTitle->getText() : ''; - $commentStore = CheckUserCommentStore::getStore(); - $commentQuery = $commentStore->getJoin( 'cuc_comment' ); + $commentQuery = $this->commentStore->getJoin( 'cuc_comment' ); $this->addTables( [ 'cu_changes', 'actor_cuc_user' => 'actor' ] ); $this->addOption( 'LIMIT', $limit + 1 ); @@ -185,7 +188,7 @@ public function execute() { 'agent' => $row->cuc_agent, ]; - $comment = $commentStore->getComment( 'cuc_comment', $row )->text; + $comment = $this->commentStore->getComment( 'cuc_comment', $row )->text; if ( $row->cuc_actiontext ) { $edit['summary'] = $row->cuc_actiontext; diff --git a/src/CheckUser/Pagers/CheckUserGetEditsPager.php b/src/CheckUser/Pagers/CheckUserGetEditsPager.php index c8c5ba1ec..ddba32eea 100644 --- a/src/CheckUser/Pagers/CheckUserGetEditsPager.php +++ b/src/CheckUser/Pagers/CheckUserGetEditsPager.php @@ -14,7 +14,6 @@ use ManualLogEntry; use MediaWiki\Cache\LinkBatchFactory; use MediaWiki\CheckUser\CheckUser\SpecialCheckUser; -use MediaWiki\CheckUser\CheckUserCommentStore; use MediaWiki\CheckUser\ClientHints\ClientHintsBatchFormatterResults; use MediaWiki\CheckUser\ClientHints\ClientHintsReferenceIds; use MediaWiki\CheckUser\Hook\HookRunner; @@ -25,6 +24,7 @@ use MediaWiki\CheckUser\Services\UserAgentClientHintsLookup; use MediaWiki\CheckUser\Services\UserAgentClientHintsManager; use MediaWiki\CommentFormatter\CommentFormatter; +use MediaWiki\CommentStore\CommentStore; use MediaWiki\Html\FormOptions; use MediaWiki\Linker\LinkRenderer; use MediaWiki\Logger\LoggerFactory; @@ -78,6 +78,7 @@ class CheckUserGetEditsPager extends AbstractCheckUserPager { private UserEditTracker $userEditTracker; private HookRunner $hookRunner; private CheckUserUtilityService $checkUserUtilityService; + private CommentStore $commentStore; private UserAgentClientHintsLookup $clientHintsLookup; private UserAgentClientHintsFormatter $clientHintsFormatter; @@ -102,6 +103,7 @@ class CheckUserGetEditsPager extends AbstractCheckUserPager { * @param UserEditTracker $userEditTracker * @param HookRunner $hookRunner * @param CheckUserUtilityService $checkUserUtilityService + * @param CommentStore $commentStore * @param UserAgentClientHintsLookup $clientHintsLookup * @param UserAgentClientHintsFormatter $clientHintsFormatter * @param IContextSource|null $context @@ -129,6 +131,7 @@ public function __construct( UserEditTracker $userEditTracker, HookRunner $hookRunner, CheckUserUtilityService $checkUserUtilityService, + CommentStore $commentStore, UserAgentClientHintsLookup $clientHintsLookup, UserAgentClientHintsFormatter $clientHintsFormatter, IContextSource $context = null, @@ -149,6 +152,7 @@ public function __construct( $this->userEditTracker = $userEditTracker; $this->hookRunner = $hookRunner; $this->checkUserUtilityService = $checkUserUtilityService; + $this->commentStore = $commentStore; $this->clientHintsLookup = $clientHintsLookup; $this->clientHintsFormatter = $clientHintsFormatter; $this->preCacheMessages(); @@ -206,7 +210,7 @@ public function formatRow( $row ): string { if ( $row->type == RC_EDIT || $row->type == RC_NEW ) { $templateParams['comment'] = $this->formattedRevisionComments[$row->this_oldid]; } else { - $comment = CheckUserCommentStore::getStore()->getComment( 'cuc_comment', $row ); + $comment = $this->commentStore->getComment( 'comment', $row ); $templateParams['comment'] = $this->commentFormatter->formatBlock( $comment->text ); } // IP @@ -473,7 +477,7 @@ public function getQueryInfo( ?string $table = null ): array { /** @inheritDoc */ protected function getQueryInfoForCuChanges(): array { - $commentQuery = CheckUserCommentStore::getStore()->getJoin( 'cuc_comment' ); + $commentQuery = $this->commentStore->getJoin( 'cuc_comment' ); $queryInfo = [ 'fields' => [ 'namespace' => 'cuc_namespace', diff --git a/src/CheckUser/SpecialCheckUser.php b/src/CheckUser/SpecialCheckUser.php index fa8a9e1b3..7627a4f75 100644 --- a/src/CheckUser/SpecialCheckUser.php +++ b/src/CheckUser/SpecialCheckUser.php @@ -21,6 +21,7 @@ use MediaWiki\CheckUser\Services\UserAgentClientHintsFormatter; use MediaWiki\CheckUser\Services\UserAgentClientHintsLookup; use MediaWiki\CommentFormatter\CommentFormatter; +use MediaWiki\CommentStore\CommentStore; use MediaWiki\Html\FormOptions; use MediaWiki\Page\WikiPageFactory; use MediaWiki\Permissions\PermissionManager; @@ -85,6 +86,7 @@ class SpecialCheckUser extends SpecialPage { private UserNameUtils $userNameUtils; private HookRunner $hookRunner; private CheckUserUtilityService $checkUserUtilityService; + private CommentStore $commentStore; private UserAgentClientHintsLookup $clientHintsLookup; private UserAgentClientHintsFormatter $clientHintsFormatter; @@ -110,6 +112,7 @@ class SpecialCheckUser extends SpecialPage { * @param UserNameUtils $userNameUtils * @param HookRunner $hookRunner * @param CheckUserUtilityService $checkUserUtilityService + * @param CommentStore $commentStore * @param UserAgentClientHintsLookup $clientHintsLookup * @param UserAgentClientHintsFormatter $clientHintsFormatter */ @@ -135,6 +138,7 @@ public function __construct( UserNameUtils $userNameUtils, HookRunner $hookRunner, CheckUserUtilityService $checkUserUtilityService, + CommentStore $commentStore, UserAgentClientHintsLookup $clientHintsLookup, UserAgentClientHintsFormatter $clientHintsFormatter ) { @@ -161,6 +165,7 @@ public function __construct( $this->userNameUtils = $userNameUtils; $this->hookRunner = $hookRunner; $this->checkUserUtilityService = $checkUserUtilityService; + $this->commentStore = $commentStore; $this->clientHintsLookup = $clientHintsLookup; $this->clientHintsFormatter = $clientHintsFormatter; } @@ -784,6 +789,7 @@ public function getPager( string $checkType, UserIdentity $userIdentity, string $this->userEditTracker, $this->hookRunner, $this->checkUserUtilityService, + $this->commentStore, $this->clientHintsLookup, $this->clientHintsFormatter ); diff --git a/src/CheckUserCommentStore.php b/src/CheckUserCommentStore.php deleted file mode 100644 index bd7adee82..000000000 --- a/src/CheckUserCommentStore.php +++ /dev/null @@ -1,42 +0,0 @@ -get( 'CheckUserCommentStore' ); - } - - /** - * @param Language $lang - * @param int $stage - */ - public function __construct( Language $lang, int $stage ) { - parent::__construct( [], $lang, $stage ); - } -} diff --git a/src/Hooks.php b/src/Hooks.php index fea694e12..81838909c 100644 --- a/src/Hooks.php +++ b/src/Hooks.php @@ -453,7 +453,7 @@ private static function insertIntoCuChangesTable( } if ( !isset( $row['cuc_comment_id'] ) ) { - $row += CheckUserCommentStore::getStore()->insert( + $row += $services->getCommentStore()->insert( $dbw, 'cuc_comment', $row['cuc_comment'] diff --git a/src/Investigate/Services/TimelineService.php b/src/Investigate/Services/TimelineService.php index 0bb137b3a..6c7f0bfb4 100644 --- a/src/Investigate/Services/TimelineService.php +++ b/src/Investigate/Services/TimelineService.php @@ -2,9 +2,31 @@ namespace MediaWiki\CheckUser\Investigate\Services; -use MediaWiki\CheckUser\CheckUserCommentStore; +use MediaWiki\CommentStore\CommentStore; +use MediaWiki\User\UserIdentityLookup; +use Wikimedia\Rdbms\Database\DbQuoter; +use Wikimedia\Rdbms\Platform\ISQLPlatform; class TimelineService extends ChangeService { + private CommentStore $commentStore; + + /** + * @param DbQuoter $dbQuoter + * @param ISQLPlatform $sqlPlatform + * @param UserIdentityLookup $userIdentityLookup + * @param CommentStore $commentStore + */ + public function __construct( + DbQuoter $dbQuoter, + ISQLPlatform $sqlPlatform, + UserIdentityLookup $userIdentityLookup, + CommentStore $commentStore + ) { + parent::__construct( $dbQuoter, $sqlPlatform, $userIdentityLookup ); + + $this->commentStore = $commentStore; + } + /** * Get timeline query info * @@ -14,7 +36,7 @@ class TimelineService extends ChangeService { * @return array */ public function getQueryInfo( array $targets, array $excludeTargets, string $start ): array { - $commentQuery = CheckUserCommentStore::getStore()->getJoin( 'cuc_comment' ); + $commentQuery = $this->commentStore->getJoin( 'cuc_comment' ); return [ 'tables' => [ 'cu_changes', 'cuc_user_actor' => 'actor' ] + $commentQuery['tables'], diff --git a/src/ServiceWiring.php b/src/ServiceWiring.php index 15e7e5c9f..bcceebddf 100644 --- a/src/ServiceWiring.php +++ b/src/ServiceWiring.php @@ -1,6 +1,5 @@ getActorStore() ); }, - 'CheckUserCommentStore' => static function ( - MediaWikiServices $services - ): CheckUserCommentStore { - return new CheckUserCommentStore( - $services->getContentLanguage(), - SCHEMA_COMPAT_NEW - ); - }, 'CheckUserPreliminaryCheckService' => static function ( MediaWikiServices $services ): PreliminaryCheckService { @@ -74,7 +65,8 @@ return new TimelineService( $services->getDBLoadBalancerFactory()->getReplicaDatabase(), $services->getDBLoadBalancerFactory()->getReplicaDatabase(), - $services->getUserIdentityLookup() + $services->getUserIdentityLookup(), + $services->getCommentStore() ); }, 'CheckUserTokenManager' => static function ( MediaWikiServices $services ): TokenManager { diff --git a/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php b/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php index 0d3456851..18d3c2562 100644 --- a/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php +++ b/tests/phpunit/integration/Api/ApiQueryCheckUserLogTest.php @@ -70,7 +70,7 @@ public function setUpObject( string $action = '', string $moduleName = '' ) { return TestingAccessWrapper::newFromObject( new ApiQueryCheckUser( $query, $moduleName, $services->getUserIdentityLookup(), $services->getRevisionLookup(), $services->getArchivedRevisionLookup(), - $services->get( 'CheckUserLogService' ) + $services->get( 'CheckUserLogService' ), $services->getCommentStore() ) ); } diff --git a/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php b/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php index def363750..fdaacfcd6 100644 --- a/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php +++ b/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php @@ -89,7 +89,7 @@ public function setUpObject( string $action = '', string $moduleName = '' ) { return TestingAccessWrapper::newFromObject( new ApiQueryCheckUser( $query, $moduleName, $services->getUserIdentityLookup(), $services->getRevisionLookup(), $services->getArchivedRevisionLookup(), - $services->get( 'CheckUserLogService' ) + $services->get( 'CheckUserLogService' ), $services->getCommentStore() ) ); } diff --git a/tests/phpunit/integration/Investigate/Services/TimelineServiceTest.php b/tests/phpunit/integration/Investigate/Services/TimelineServiceTest.php index c388c52b0..db927cb12 100644 --- a/tests/phpunit/integration/Investigate/Services/TimelineServiceTest.php +++ b/tests/phpunit/integration/Investigate/Services/TimelineServiceTest.php @@ -35,7 +35,8 @@ public function testGetQueryInfo( $targets, $start, $expected ) { $timelineService = new TimelineService( new AddQuoterMock(), new SQLPlatform( new AddQuoterMock() ), - $userIdentityLookup + $userIdentityLookup, + $this->getServiceContainer()->getCommentStore() ); $q = $timelineService->getQueryInfo( $targets, [], $start ); From deee238ebccb0e8071ad7ee4daae1983fc632554 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:52 -0400 Subject: [PATCH 09/11] Reapply "Remove cuc_comment from cu_changes" This reverts commit 26c9695a023491ca2c803c78280690d9e1fd777d. --- .../patch-cu_changes-drop-cuc_comment.json | 281 ++++++++++++++++++ .../patch-cu_changes-drop-cuc_comment.sql | 6 + schema/mysql/tables-generated.sql | 1 - .../patch-cu_changes-drop-cuc_comment.sql | 6 + schema/postgres/tables-generated.sql | 1 - .../patch-cu_changes-drop-cuc_comment.sql | 17 ++ schema/sqlite/tables-generated.sql | 1 - schema/tables.json | 5 - src/HookHandler/SchemaChangesHandler.php | 5 + 9 files changed, 315 insertions(+), 8 deletions(-) create mode 100644 schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json create mode 100644 schema/mysql/patch-cu_changes-drop-cuc_comment.sql create mode 100644 schema/postgres/patch-cu_changes-drop-cuc_comment.sql create mode 100644 schema/sqlite/patch-cu_changes-drop-cuc_comment.sql diff --git a/schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json b/schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json new file mode 100644 index 000000000..532d6ac6a --- /dev/null +++ b/schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json @@ -0,0 +1,281 @@ +{ + "before": { + "name": "cu_changes", + "columns": [ + { + "name": "cuc_id", + "comment": "Primary key", + "type": "integer", + "options": { "autoincrement": true, "notnull": true, "unsigned": true } + }, + { + "name": "cuc_namespace", + "comment": "When pages are renamed, their RC entries do _not_ change.", + "type": "integer", + "options": { "notnull": true, "default": 0 } + }, + { + "name": "cuc_title", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_actor", + "type": "bigint", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_actiontext", + "comment": "Edit summary", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_comment", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_comment_id", + "type": "bigint", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_minor", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + }, + { + "name": "cuc_page_id", + "comment": "Key to page_id (was cur_id prior to 1.5). This will keep links working after moves while retaining the at-the-time name in the changes list.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_this_oldid", + "comment": "rev_id of the given revision", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_last_oldid", + "comment": "rev_id of the prior revision, for generating diff links.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_type", + "comment": "RecentChange type identifiers: RC_EDIT, RC_NEW or RC_LOG", + "type": "mwtinyint", + "options": { "notnull": true, "unsigned": true, "length": 3, "default": 0 } + }, + { + "name": "cuc_timestamp", + "comment": "Event timestamp", + "type": "mwtimestamp", + "options": { "notnull": true } + }, + { + "name": "cuc_ip", + "comment": "IP address, visible", + "type": "string", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_ip_hex", + "comment": "IP address as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_xff", + "comment": "XFF header, visible, all data", + "type": "binary", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_xff_hex", + "comment": "XFF header, last IP, as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_agent", + "comment": "User agent", + "type": "binary", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_private", + "comment": "Private Data", + "type": "blob", + "options": { "notnull": false, "length": 16777215 } + }, + { + "name": "cuc_only_for_read_old", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + } + ], + "indexes": [ + { + "name": "cuc_ip_hex_time", + "columns": [ "cuc_ip_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_xff_hex_time", + "columns": [ "cuc_xff_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_timestamp", + "columns": [ "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_actor_ip_time", + "columns": [ "cuc_actor", "cuc_ip", "cuc_timestamp" ], + "unique": false + } + ], + "pk": [ "cuc_id" ] + }, + "after": { + "name": "cu_changes", + "columns": [ + { + "name": "cuc_id", + "comment": "Primary key", + "type": "integer", + "options": { "autoincrement": true, "notnull": true, "unsigned": true } + }, + { + "name": "cuc_namespace", + "comment": "When pages are renamed, their RC entries do _not_ change.", + "type": "integer", + "options": { "notnull": true, "default": 0 } + }, + { + "name": "cuc_title", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_actor", + "type": "bigint", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_actiontext", + "comment": "Edit summary", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_comment_id", + "type": "bigint", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_minor", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + }, + { + "name": "cuc_page_id", + "comment": "Key to page_id (was cur_id prior to 1.5). This will keep links working after moves while retaining the at-the-time name in the changes list.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_this_oldid", + "comment": "rev_id of the given revision", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_last_oldid", + "comment": "rev_id of the prior revision, for generating diff links.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_type", + "comment": "RecentChange type identifiers: RC_EDIT, RC_NEW or RC_LOG", + "type": "mwtinyint", + "options": { "notnull": true, "unsigned": true, "length": 3, "default": 0 } + }, + { + "name": "cuc_timestamp", + "comment": "Event timestamp", + "type": "mwtimestamp", + "options": { "notnull": true } + }, + { + "name": "cuc_ip", + "comment": "IP address, visible", + "type": "string", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_ip_hex", + "comment": "IP address as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_xff", + "comment": "XFF header, visible, all data", + "type": "binary", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_xff_hex", + "comment": "XFF header, last IP, as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_agent", + "comment": "User agent", + "type": "binary", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_private", + "comment": "Private Data", + "type": "blob", + "options": { "notnull": false, "length": 16777215 } + }, + { + "name": "cuc_only_for_read_old", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + } + ], + "indexes": [ + { + "name": "cuc_ip_hex_time", + "columns": [ "cuc_ip_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_xff_hex_time", + "columns": [ "cuc_xff_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_timestamp", + "columns": [ "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_actor_ip_time", + "columns": [ "cuc_actor", "cuc_ip", "cuc_timestamp" ], + "unique": false + } + ], + "pk": [ "cuc_id" ] + } +} diff --git a/schema/mysql/patch-cu_changes-drop-cuc_comment.sql b/schema/mysql/patch-cu_changes-drop-cuc_comment.sql new file mode 100644 index 000000000..52637083d --- /dev/null +++ b/schema/mysql/patch-cu_changes-drop-cuc_comment.sql @@ -0,0 +1,6 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +ALTER TABLE /*_*/cu_changes +DROP cuc_comment; \ No newline at end of file diff --git a/schema/mysql/tables-generated.sql b/schema/mysql/tables-generated.sql index 7dcdb228a..5cd38165a 100644 --- a/schema/mysql/tables-generated.sql +++ b/schema/mysql/tables-generated.sql @@ -8,7 +8,6 @@ CREATE TABLE /*_*/cu_changes ( cuc_title VARBINARY(255) DEFAULT '' NOT NULL, cuc_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, cuc_actiontext VARBINARY(255) DEFAULT '' NOT NULL, - cuc_comment VARBINARY(255) DEFAULT '' NOT NULL, cuc_comment_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cuc_minor TINYINT(1) DEFAULT 0 NOT NULL, cuc_page_id INT UNSIGNED DEFAULT 0 NOT NULL, diff --git a/schema/postgres/patch-cu_changes-drop-cuc_comment.sql b/schema/postgres/patch-cu_changes-drop-cuc_comment.sql new file mode 100644 index 000000000..a81c6dd46 --- /dev/null +++ b/schema/postgres/patch-cu_changes-drop-cuc_comment.sql @@ -0,0 +1,6 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +ALTER TABLE cu_changes +DROP cuc_comment; \ No newline at end of file diff --git a/schema/postgres/tables-generated.sql b/schema/postgres/tables-generated.sql index 5a81b26f7..96c7c69ef 100644 --- a/schema/postgres/tables-generated.sql +++ b/schema/postgres/tables-generated.sql @@ -8,7 +8,6 @@ CREATE TABLE cu_changes ( cuc_title TEXT DEFAULT '' NOT NULL, cuc_actor BIGINT DEFAULT 0 NOT NULL, cuc_actiontext TEXT DEFAULT '' NOT NULL, - cuc_comment TEXT DEFAULT '' NOT NULL, cuc_comment_id BIGINT DEFAULT 0 NOT NULL, cuc_minor SMALLINT DEFAULT 0 NOT NULL, cuc_page_id INT DEFAULT 0 NOT NULL, diff --git a/schema/sqlite/patch-cu_changes-drop-cuc_comment.sql b/schema/sqlite/patch-cu_changes-drop-cuc_comment.sql new file mode 100644 index 000000000..16e695a79 --- /dev/null +++ b/schema/sqlite/patch-cu_changes-drop-cuc_comment.sql @@ -0,0 +1,17 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_changes-drop-cuc_comment.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +CREATE TEMPORARY TABLE /*_*/__temp__cu_changes AS +SELECT cuc_id, cuc_namespace, cuc_title, cuc_actor, cuc_actiontext, cuc_comment_id, cuc_minor, cuc_page_id, cuc_this_oldid, cuc_last_oldid, cuc_type, cuc_timestamp, cuc_ip, cuc_ip_hex, cuc_xff, cuc_xff_hex, cuc_agent, cuc_private, cuc_only_for_read_old +FROM /*_*/cu_changes; +DROP TABLE /*_*/cu_changes; +CREATE TABLE /*_*/cu_changes ( cuc_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cuc_namespace INTEGER DEFAULT 0 NOT NULL, cuc_title BLOB DEFAULT '' NOT NULL, cuc_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, cuc_actiontext BLOB DEFAULT '' NOT NULL, cuc_comment_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cuc_minor SMALLINT DEFAULT 0 NOT NULL, cuc_page_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_this_oldid INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_last_oldid INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_type SMALLINT UNSIGNED DEFAULT 0 NOT NULL, cuc_timestamp BLOB NOT NULL, cuc_ip VARCHAR(255) DEFAULT '', cuc_ip_hex VARCHAR(255) DEFAULT NULL, cuc_xff BLOB DEFAULT '', cuc_xff_hex VARCHAR(255) DEFAULT NULL, cuc_agent BLOB DEFAULT NULL, cuc_private BLOB DEFAULT NULL, cuc_only_for_read_old SMALLINT DEFAULT 0 NOT NULL ); +INSERT INTO /*_*/cu_changes ( cuc_id, cuc_namespace, cuc_title, cuc_actor, cuc_actiontext, cuc_comment_id, cuc_minor, cuc_page_id, cuc_this_oldid, cuc_last_oldid, cuc_type, cuc_timestamp, cuc_ip, cuc_ip_hex, cuc_xff, cuc_xff_hex, cuc_agent, cuc_private, cuc_only_for_read_old ) +SELECT cuc_id, cuc_namespace, cuc_title, cuc_actor, cuc_actiontext, cuc_comment_id, cuc_minor, cuc_page_id, cuc_this_oldid, cuc_last_oldid, cuc_type, cuc_timestamp, cuc_ip, cuc_ip_hex, cuc_xff, cuc_xff_hex, cuc_agent, cuc_private, cuc_only_for_read_old +FROM /*_*/__temp__cu_changes; +DROP TABLE /*_*/__temp__cu_changes; +CREATE INDEX cuc_ip_hex_time ON /*_*/cu_changes (cuc_ip_hex, cuc_timestamp); +CREATE INDEX cuc_xff_hex_time ON /*_*/cu_changes (cuc_xff_hex, cuc_timestamp); +CREATE INDEX cuc_timestamp ON /*_*/cu_changes (cuc_timestamp); +CREATE INDEX cuc_actor_ip_time ON /*_*/cu_changes (cuc_actor, cuc_ip, cuc_timestamp); \ No newline at end of file diff --git a/schema/sqlite/tables-generated.sql b/schema/sqlite/tables-generated.sql index 534d54030..5f6329568 100644 --- a/schema/sqlite/tables-generated.sql +++ b/schema/sqlite/tables-generated.sql @@ -8,7 +8,6 @@ CREATE TABLE /*_*/cu_changes ( cuc_title BLOB DEFAULT '' NOT NULL, cuc_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, cuc_actiontext BLOB DEFAULT '' NOT NULL, - cuc_comment BLOB DEFAULT '' NOT NULL, cuc_comment_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, cuc_minor SMALLINT DEFAULT 0 NOT NULL, cuc_page_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, diff --git a/schema/tables.json b/schema/tables.json index dc992c57c..5d28e425b 100644 --- a/schema/tables.json +++ b/schema/tables.json @@ -30,11 +30,6 @@ "type": "binary", "options": { "notnull": true, "length": 255, "default": "" } }, - { - "name": "cuc_comment", - "type": "binary", - "options": { "notnull": true, "length": 255, "default": "" } - }, { "name": "cuc_comment_id", "type": "bigint", diff --git a/src/HookHandler/SchemaChangesHandler.php b/src/HookHandler/SchemaChangesHandler.php index d52459769..e8cf760d0 100644 --- a/src/HookHandler/SchemaChangesHandler.php +++ b/src/HookHandler/SchemaChangesHandler.php @@ -265,6 +265,11 @@ public function onLoadExtensionSchemaUpdates( $updater ) { 'cuc_only_for_read_old', "$base/$dbType/patch-cu_changes-add-cuc_only_for_read_old.sql" ); + $updater->dropExtensionField( + 'cu_changes', + 'cuc_comment', + "$base/$dbType/patch-cu_changes-drop-cuc_comment.sql" + ); // 1.41 $updater->addExtensionTable( 'cu_useragent_clienthints', "$base/$dbType/cu_useragent_clienthints.sql" ); From 4eb323b4993dffde6ee96bbdde41cac4f7a106d2 Mon Sep 17 00:00:00 2001 From: TehKittyCat Date: Mon, 25 Mar 2024 22:06:52 -0400 Subject: [PATCH 10/11] Reapply "Drop default from cuc_actor and cuc_comment_id" This reverts commit b45ebedb215df7f2a73d464aea1f74bfd1251540. --- .../patch-cu_changes-drop-defaults.json | 276 ++++++++++++++++++ .../mysql/patch-cu_changes-drop-defaults.sql | 7 + schema/mysql/tables-generated.sql | 4 +- .../patch-cu_changes-drop-defaults.sql | 8 + schema/postgres/tables-generated.sql | 4 +- .../sqlite/patch-cu_changes-drop-defaults.sql | 17 ++ schema/sqlite/tables-generated.sql | 4 +- schema/tables.json | 4 +- src/HookHandler/SchemaChangesHandler.php | 5 + 9 files changed, 321 insertions(+), 8 deletions(-) create mode 100644 schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json create mode 100644 schema/mysql/patch-cu_changes-drop-defaults.sql create mode 100644 schema/postgres/patch-cu_changes-drop-defaults.sql create mode 100644 schema/sqlite/patch-cu_changes-drop-defaults.sql diff --git a/schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json b/schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json new file mode 100644 index 000000000..04b3f5337 --- /dev/null +++ b/schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json @@ -0,0 +1,276 @@ +{ + "before": { + "name": "cu_changes", + "columns": [ + { + "name": "cuc_id", + "comment": "Primary key", + "type": "integer", + "options": { "autoincrement": true, "notnull": true, "unsigned": true } + }, + { + "name": "cuc_namespace", + "comment": "When pages are renamed, their RC entries do _not_ change.", + "type": "integer", + "options": { "notnull": true, "default": 0 } + }, + { + "name": "cuc_title", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_actor", + "type": "bigint", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_actiontext", + "comment": "Edit summary", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_comment_id", + "type": "bigint", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_minor", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + }, + { + "name": "cuc_page_id", + "comment": "Key to page_id (was cur_id prior to 1.5). This will keep links working after moves while retaining the at-the-time name in the changes list.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_this_oldid", + "comment": "rev_id of the given revision", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_last_oldid", + "comment": "rev_id of the prior revision, for generating diff links.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_type", + "comment": "RecentChange type identifiers: RC_EDIT, RC_NEW or RC_LOG", + "type": "mwtinyint", + "options": { "notnull": true, "unsigned": true, "length": 3, "default": 0 } + }, + { + "name": "cuc_timestamp", + "comment": "Event timestamp", + "type": "mwtimestamp", + "options": { "notnull": true } + }, + { + "name": "cuc_ip", + "comment": "IP address, visible", + "type": "string", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_ip_hex", + "comment": "IP address as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_xff", + "comment": "XFF header, visible, all data", + "type": "binary", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_xff_hex", + "comment": "XFF header, last IP, as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_agent", + "comment": "User agent", + "type": "binary", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_private", + "comment": "Private Data", + "type": "blob", + "options": { "notnull": false, "length": 16777215 } + }, + { + "name": "cuc_only_for_read_old", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + } + ], + "indexes": [ + { + "name": "cuc_ip_hex_time", + "columns": [ "cuc_ip_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_xff_hex_time", + "columns": [ "cuc_xff_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_timestamp", + "columns": [ "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_actor_ip_time", + "columns": [ "cuc_actor", "cuc_ip", "cuc_timestamp" ], + "unique": false + } + ], + "pk": [ "cuc_id" ] + }, + "after": { + "name": "cu_changes", + "columns": [ + { + "name": "cuc_id", + "comment": "Primary key", + "type": "integer", + "options": { "autoincrement": true, "notnull": true, "unsigned": true } + }, + { + "name": "cuc_namespace", + "comment": "When pages are renamed, their RC entries do _not_ change.", + "type": "integer", + "options": { "notnull": true, "default": 0 } + }, + { + "name": "cuc_title", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_actor", + "type": "bigint", + "options": { "notnull": true, "unsigned": true } + }, + { + "name": "cuc_actiontext", + "comment": "Edit summary", + "type": "binary", + "options": { "notnull": true, "length": 255, "default": "" } + }, + { + "name": "cuc_comment_id", + "type": "bigint", + "options": { "notnull": true, "unsigned": true } + }, + { + "name": "cuc_minor", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + }, + { + "name": "cuc_page_id", + "comment": "Key to page_id (was cur_id prior to 1.5). This will keep links working after moves while retaining the at-the-time name in the changes list.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_this_oldid", + "comment": "rev_id of the given revision", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_last_oldid", + "comment": "rev_id of the prior revision, for generating diff links.", + "type": "integer", + "options": { "notnull": true, "unsigned": true, "default": 0 } + }, + { + "name": "cuc_type", + "comment": "RecentChange type identifiers: RC_EDIT, RC_NEW or RC_LOG", + "type": "mwtinyint", + "options": { "notnull": true, "unsigned": true, "length": 3, "default": 0 } + }, + { + "name": "cuc_timestamp", + "comment": "Event timestamp", + "type": "mwtimestamp", + "options": { "notnull": true } + }, + { + "name": "cuc_ip", + "comment": "IP address, visible", + "type": "string", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_ip_hex", + "comment": "IP address as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_xff", + "comment": "XFF header, visible, all data", + "type": "binary", + "options": { "notnull": false, "length": 255, "default": "" } + }, + { + "name": "cuc_xff_hex", + "comment": "XFF header, last IP, as hexidecimal", + "type": "string", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_agent", + "comment": "User agent", + "type": "binary", + "options": { "notnull": false, "length": 255 } + }, + { + "name": "cuc_private", + "comment": "Private Data", + "type": "blob", + "options": { "notnull": false, "length": 16777215 } + }, + { + "name": "cuc_only_for_read_old", + "type": "mwtinyint", + "options": { "notnull": true, "length": 1, "default": 0 } + } + ], + "indexes": [ + { + "name": "cuc_ip_hex_time", + "columns": [ "cuc_ip_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_xff_hex_time", + "columns": [ "cuc_xff_hex", "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_timestamp", + "columns": [ "cuc_timestamp" ], + "unique": false + }, + { + "name": "cuc_actor_ip_time", + "columns": [ "cuc_actor", "cuc_ip", "cuc_timestamp" ], + "unique": false + } + ], + "pk": [ "cuc_id" ] + } +} diff --git a/schema/mysql/patch-cu_changes-drop-defaults.sql b/schema/mysql/patch-cu_changes-drop-defaults.sql new file mode 100644 index 000000000..ce0751965 --- /dev/null +++ b/schema/mysql/patch-cu_changes-drop-defaults.sql @@ -0,0 +1,7 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +ALTER TABLE /*_*/cu_changes +CHANGE cuc_actor cuc_actor BIGINT UNSIGNED NOT NULL, +CHANGE cuc_comment_id cuc_comment_id BIGINT UNSIGNED NOT NULL; \ No newline at end of file diff --git a/schema/mysql/tables-generated.sql b/schema/mysql/tables-generated.sql index 5cd38165a..824b9d08e 100644 --- a/schema/mysql/tables-generated.sql +++ b/schema/mysql/tables-generated.sql @@ -6,9 +6,9 @@ CREATE TABLE /*_*/cu_changes ( cuc_id INT UNSIGNED AUTO_INCREMENT NOT NULL, cuc_namespace INT DEFAULT 0 NOT NULL, cuc_title VARBINARY(255) DEFAULT '' NOT NULL, - cuc_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, + cuc_actor BIGINT UNSIGNED NOT NULL, cuc_actiontext VARBINARY(255) DEFAULT '' NOT NULL, - cuc_comment_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, + cuc_comment_id BIGINT UNSIGNED NOT NULL, cuc_minor TINYINT(1) DEFAULT 0 NOT NULL, cuc_page_id INT UNSIGNED DEFAULT 0 NOT NULL, cuc_this_oldid INT UNSIGNED DEFAULT 0 NOT NULL, diff --git a/schema/postgres/patch-cu_changes-drop-defaults.sql b/schema/postgres/patch-cu_changes-drop-defaults.sql new file mode 100644 index 000000000..4872072a8 --- /dev/null +++ b/schema/postgres/patch-cu_changes-drop-defaults.sql @@ -0,0 +1,8 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +ALTER TABLE cu_changes ALTER cuc_actor +DROP DEFAULT; +ALTER TABLE cu_changes ALTER cuc_comment_id +DROP DEFAULT; \ No newline at end of file diff --git a/schema/postgres/tables-generated.sql b/schema/postgres/tables-generated.sql index 96c7c69ef..140839d9b 100644 --- a/schema/postgres/tables-generated.sql +++ b/schema/postgres/tables-generated.sql @@ -6,9 +6,9 @@ CREATE TABLE cu_changes ( cuc_id SERIAL NOT NULL, cuc_namespace INT DEFAULT 0 NOT NULL, cuc_title TEXT DEFAULT '' NOT NULL, - cuc_actor BIGINT DEFAULT 0 NOT NULL, + cuc_actor BIGINT NOT NULL, cuc_actiontext TEXT DEFAULT '' NOT NULL, - cuc_comment_id BIGINT DEFAULT 0 NOT NULL, + cuc_comment_id BIGINT NOT NULL, cuc_minor SMALLINT DEFAULT 0 NOT NULL, cuc_page_id INT DEFAULT 0 NOT NULL, cuc_this_oldid INT DEFAULT 0 NOT NULL, diff --git a/schema/sqlite/patch-cu_changes-drop-defaults.sql b/schema/sqlite/patch-cu_changes-drop-defaults.sql new file mode 100644 index 000000000..13d6460c9 --- /dev/null +++ b/schema/sqlite/patch-cu_changes-drop-defaults.sql @@ -0,0 +1,17 @@ +-- This file is automatically generated using maintenance/generateSchemaChangeSql.php. +-- Source: extensions/CheckUser/schema/abstractSchemaChanges/patch-cu_changes-drop-defaults.json +-- Do not modify this file directly. +-- See https://www.mediawiki.org/wiki/Manual:Schema_changes +CREATE TEMPORARY TABLE /*_*/__temp__cu_changes AS +SELECT cuc_id, cuc_namespace, cuc_title, cuc_actor, cuc_actiontext, cuc_comment_id, cuc_minor, cuc_page_id, cuc_this_oldid, cuc_last_oldid, cuc_type, cuc_timestamp, cuc_ip, cuc_ip_hex, cuc_xff, cuc_xff_hex, cuc_agent, cuc_private, cuc_only_for_read_old +FROM /*_*/cu_changes; +DROP TABLE /*_*/cu_changes; +CREATE TABLE /*_*/cu_changes ( cuc_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cuc_namespace INTEGER DEFAULT 0 NOT NULL, cuc_title BLOB DEFAULT '' NOT NULL, cuc_actor BIGINT UNSIGNED NOT NULL, cuc_actiontext BLOB DEFAULT '' NOT NULL, cuc_comment_id BIGINT UNSIGNED NOT NULL, cuc_minor SMALLINT DEFAULT 0 NOT NULL, cuc_page_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_this_oldid INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_last_oldid INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_type SMALLINT UNSIGNED DEFAULT 0 NOT NULL, cuc_timestamp BLOB NOT NULL, cuc_ip VARCHAR(255) DEFAULT '', cuc_ip_hex VARCHAR(255) DEFAULT NULL, cuc_xff BLOB DEFAULT '', cuc_xff_hex VARCHAR(255) DEFAULT NULL, cuc_agent BLOB DEFAULT NULL, cuc_private BLOB DEFAULT NULL, cuc_only_for_read_old SMALLINT DEFAULT 0 NOT NULL ); +INSERT INTO /*_*/cu_changes ( cuc_id, cuc_namespace, cuc_title, cuc_actor, cuc_actiontext, cuc_comment_id, cuc_minor, cuc_page_id, cuc_this_oldid, cuc_last_oldid, cuc_type, cuc_timestamp, cuc_ip, cuc_ip_hex, cuc_xff, cuc_xff_hex, cuc_agent, cuc_private, cuc_only_for_read_old ) +SELECT cuc_id, cuc_namespace, cuc_title, cuc_actor, cuc_actiontext, cuc_comment_id, cuc_minor, cuc_page_id, cuc_this_oldid, cuc_last_oldid, cuc_type, cuc_timestamp, cuc_ip, cuc_ip_hex, cuc_xff, cuc_xff_hex, cuc_agent, cuc_private, cuc_only_for_read_old +FROM /*_*/__temp__cu_changes; +DROP TABLE /*_*/__temp__cu_changes; +CREATE INDEX cuc_ip_hex_time ON /*_*/cu_changes (cuc_ip_hex, cuc_timestamp); +CREATE INDEX cuc_xff_hex_time ON /*_*/cu_changes (cuc_xff_hex, cuc_timestamp); +CREATE INDEX cuc_timestamp ON /*_*/cu_changes (cuc_timestamp); +CREATE INDEX cuc_actor_ip_time ON /*_*/cu_changes (cuc_actor, cuc_ip, cuc_timestamp); \ No newline at end of file diff --git a/schema/sqlite/tables-generated.sql b/schema/sqlite/tables-generated.sql index 5f6329568..62deb9bf0 100644 --- a/schema/sqlite/tables-generated.sql +++ b/schema/sqlite/tables-generated.sql @@ -6,9 +6,9 @@ CREATE TABLE /*_*/cu_changes ( cuc_id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, cuc_namespace INTEGER DEFAULT 0 NOT NULL, cuc_title BLOB DEFAULT '' NOT NULL, - cuc_actor BIGINT UNSIGNED DEFAULT 0 NOT NULL, + cuc_actor BIGINT UNSIGNED NOT NULL, cuc_actiontext BLOB DEFAULT '' NOT NULL, - cuc_comment_id BIGINT UNSIGNED DEFAULT 0 NOT NULL, + cuc_comment_id BIGINT UNSIGNED NOT NULL, cuc_minor SMALLINT DEFAULT 0 NOT NULL, cuc_page_id INTEGER UNSIGNED DEFAULT 0 NOT NULL, cuc_this_oldid INTEGER UNSIGNED DEFAULT 0 NOT NULL, diff --git a/schema/tables.json b/schema/tables.json index 5d28e425b..7e86d1ad0 100644 --- a/schema/tables.json +++ b/schema/tables.json @@ -22,7 +22,7 @@ { "name": "cuc_actor", "type": "bigint", - "options": { "notnull": true, "unsigned": true, "default": 0 } + "options": { "notnull": true, "unsigned": true } }, { "name": "cuc_actiontext", @@ -33,7 +33,7 @@ { "name": "cuc_comment_id", "type": "bigint", - "options": { "notnull": true, "unsigned": true, "default": 0 } + "options": { "notnull": true, "unsigned": true } }, { "name": "cuc_minor", diff --git a/src/HookHandler/SchemaChangesHandler.php b/src/HookHandler/SchemaChangesHandler.php index e8cf760d0..8c0792a1f 100644 --- a/src/HookHandler/SchemaChangesHandler.php +++ b/src/HookHandler/SchemaChangesHandler.php @@ -270,6 +270,11 @@ public function onLoadExtensionSchemaUpdates( $updater ) { 'cuc_comment', "$base/$dbType/patch-cu_changes-drop-cuc_comment.sql" ); + $updater->modifyExtensionField( + 'cu_changes', + 'cuc_actor', + "$base/$dbType/patch-cu_changes-drop-defaults.sql" + ); // 1.41 $updater->addExtensionTable( 'cu_useragent_clienthints', "$base/$dbType/cu_useragent_clienthints.sql" ); From 940fc2e23807ce5d244d52125e5265a3b3578b24 Mon Sep 17 00:00:00 2001 From: Dreamy Jazz Date: Wed, 13 Mar 2024 20:22:10 +0000 Subject: [PATCH 11/11] Use cule_actor instead of log_actor for join to the logging table Why: * When selecting results for Special:CheckUser 'Get actions', the query to the cu_log_event table joins to both the logging and actor tables. * However, the join to the actor table is done using the actor ID column from the logging table instead of the one in the cu_log_event table. * Using the actor ID column from the cu_log_event table fixes the query being slow on WMF production. As such, this needs to be done to improve the slow query time. What: * Update the cu_log_event query to use the cule_actor over log_actor when joining to the actor table. Bug: T354643 Change-Id: I3943f58dfc77a8f61654d82be38482141a67b066 --- src/CheckUser/Pagers/CheckUserGetEditsPager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CheckUser/Pagers/CheckUserGetEditsPager.php b/src/CheckUser/Pagers/CheckUserGetEditsPager.php index ddba32eea..842379b09 100644 --- a/src/CheckUser/Pagers/CheckUserGetEditsPager.php +++ b/src/CheckUser/Pagers/CheckUserGetEditsPager.php @@ -551,7 +551,7 @@ protected function getQueryInfoForCuLogEvent(): array { 'conds' => [], 'join_conds' => [ 'logging_cule_log_id' => [ 'JOIN', 'logging_cule_log_id.log_id=cule_log_id' ], - 'actor_log_actor' => [ 'JOIN', 'actor_log_actor.actor_id=log_actor' ], + 'actor_log_actor' => [ 'JOIN', 'actor_log_actor.actor_id=cule_actor' ], ] + $commentQuery['joins'], 'options' => [], ];