From 96ef4cfd2d8653966a120987ec0268382d868026 Mon Sep 17 00:00:00 2001 From: Umherirrender Date: Sat, 27 Apr 2024 23:37:18 +0200 Subject: [PATCH] Migrate to IReadableDatabase::newSelectQueryBuilder Also use expression builder to avoid raw sql Bug: T312333 Change-Id: I6ce22de6637fccca8cf86a405bc023f268ff693b --- .../Api/ApiEchoUnreadNotificationPages.php | 31 ++++--- includes/Cache/RevisionLocalCache.php | 13 +-- includes/EmailBatch.php | 86 ++++++++--------- includes/Gateway/UserNotificationGateway.php | 42 ++++----- includes/Mapper/EventMapper.php | 93 ++++++++++--------- includes/Mapper/NotificationMapper.php | 90 ++++++++---------- includes/Push/SubscriptionManager.php | 19 ++-- includes/UnreadWikis.php | 14 +-- includes/UserLocator.php | 16 ++-- includes/UserMergeHooks.php | 58 ++++++------ maintenance/removeInvalidNotification.php | 16 ++-- tests/phpunit/Mapper/EventMapperTest.php | 5 + .../phpunit/Mapper/NotificationMapperTest.php | 9 ++ tests/phpunit/Model/NotificationTest.php | 8 +- tests/phpunit/TalkPageFunctionalTest.php | 10 +- .../Gateway/UserNotificationGatewayTest.php | 5 + 16 files changed, 253 insertions(+), 262 deletions(-) diff --git a/includes/Api/ApiEchoUnreadNotificationPages.php b/includes/Api/ApiEchoUnreadNotificationPages.php index 578c201b9..013fe2c3f 100644 --- a/includes/Api/ApiEchoUnreadNotificationPages.php +++ b/includes/Api/ApiEchoUnreadNotificationPages.php @@ -16,6 +16,7 @@ use MediaWiki\Title\TitleFactory; use MediaWiki\WikiMap\WikiMap; use Wikimedia\ParamValidator\ParamValidator; use Wikimedia\ParamValidator\TypeDef\IntegerDef; +use Wikimedia\Rdbms\SelectQueryBuilder; class ApiEchoUnreadNotificationPages extends ApiQueryBase { use ApiCrossWiki; @@ -89,24 +90,26 @@ class ApiEchoUnreadNotificationPages extends ApiQueryBase { $enabledTypes = $attributeManager->getUserEnabledEvents( $this->getUser(), 'web' ); $dbr = DbFactory::newFromDefault()->getEchoDb( DB_REPLICA ); - // If $groupPages is true, we need to fetch all pages and apply the ORDER BY and LIMIT ourselves - // after grouping. - $extraOptions = $groupPages ? [] : [ 'ORDER BY' => 'count DESC', 'LIMIT' => $limit ]; - $rows = $dbr->select( - [ 'echo_event', 'echo_notification' ], - [ 'event_page_id', 'count' => 'COUNT(*)' ], - [ + $queryBuilder = $dbr->newSelectQueryBuilder() + ->select( [ 'event_page_id', 'count' => 'COUNT(*)' ] ) + ->from( 'echo_event' ) + ->join( 'echo_notification', null, 'notification_event = event_id' ) + ->where( [ 'notification_user' => $this->getUser()->getId(), 'notification_read_timestamp' => null, 'event_deleted' => 0, 'event_type' => $enabledTypes, - ], - __METHOD__, - [ - 'GROUP BY' => 'event_page_id', - ] + $extraOptions, - [ 'echo_notification' => [ 'INNER JOIN', 'notification_event = event_id' ] ] - ); + ] ) + ->groupBy( 'event_page_id' ) + ->caller( __METHOD__ ); + // If $groupPages is true, we need to fetch all pages and apply the ORDER BY and LIMIT ourselves + // after grouping. + if ( !$groupPages ) { + $queryBuilder + ->orderBy( 'count', SelectQueryBuilder::SORT_DESC ) + ->limit( $limit ); + } + $rows = $queryBuilder->fetchResultSet(); $nullCount = 0; $pageCounts = []; diff --git a/includes/Cache/RevisionLocalCache.php b/includes/Cache/RevisionLocalCache.php index eae77eae1..783863f8f 100644 --- a/includes/Cache/RevisionLocalCache.php +++ b/includes/Cache/RevisionLocalCache.php @@ -32,14 +32,11 @@ class RevisionLocalCache extends LocalCache { protected function resolve( array $lookups ) { $dbr = $this->dbProvider->getReplicaDatabase(); $revQuery = $this->revisionStore->getQueryInfo( [ 'page', 'user' ] ); - $res = $dbr->select( - $revQuery['tables'], - $revQuery['fields'], - [ 'rev_id' => $lookups ], - __METHOD__, - [], - $revQuery['joins'] - ); + $res = $dbr->newSelectQueryBuilder() + ->queryInfo( $revQuery ) + ->where( [ 'rev_id' => $lookups ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); foreach ( $res as $row ) { yield $row->rev_id => $this->revisionStore->newRevisionFromRow( $row ); } diff --git a/includes/EmailBatch.php b/includes/EmailBatch.php index 398b58b99..a7110aedc 100644 --- a/includes/EmailBatch.php +++ b/includes/EmailBatch.php @@ -181,12 +181,12 @@ class EmailBatch { */ protected function setLastEvent() { $dbr = DbFactory::newFromDefault()->getEchoDb( DB_REPLICA ); - $res = $dbr->selectField( - [ 'echo_email_batch' ], - 'MAX( eeb_event_id )', - [ 'eeb_user_id' => $this->mUser->getId() ], - __METHOD__ - ); + $res = $dbr->newSelectQueryBuilder() + ->select( 'MAX( eeb_event_id )' ) + ->from( 'echo_email_batch' ) + ->where( [ 'eeb_user_id' => $this->mUser->getId() ] ) + ->caller( __METHOD__ ) + ->fetchField(); if ( $res ) { $this->lastEvent = $res; @@ -227,49 +227,38 @@ class EmailBatch { // performance in this case if ( $validEvents ) { $dbr = DbFactory::newFromDefault()->getEchoDb( DB_REPLICA ); + $queryBuilder = $dbr->newSelectQueryBuilder() + ->select( array_merge( Event::selectFields(), [ + 'eeb_id', + 'eeb_user_id', + 'eeb_event_priority', + 'eeb_event_id', + 'eeb_event_hash', + ] ) ) + ->from( 'echo_email_batch' ) + ->join( 'echo_event', null, 'event_id = eeb_event_id' ) + ->where( [ + 'eeb_user_id' => $this->mUser->getId(), + 'event_type' => $validEvents + ] ) + ->orderBy( 'eeb_event_priority' ) + ->limit( self::$displaySize + 1 ) + ->caller( __METHOD__ ); - $conds = [ - 'eeb_user_id' => $this->mUser->getId(), - 'event_id = eeb_event_id', - 'event_type' => $validEvents - ]; - - $tables = [ 'echo_email_batch', 'echo_event' ]; if ( $this->userOptionsManager->getOption( $this->mUser, 'echo-dont-email-read-notifications' ) ) { - $conds = array_merge( - $conds, - [ - 'notification_event = event_id', - 'notification_read_timestamp IS NULL', - ] - ); - $tables[] = 'echo_notification'; + $queryBuilder + ->join( 'echo_notification', null, 'notification_event = event_id' ) + ->andWhere( [ 'notification_read_timestamp' => null ] ); } // See setLastEvent() for more detail for this variable if ( $this->lastEvent ) { - $conds[] = 'eeb_event_id <= ' . (int)$this->lastEvent; + $queryBuilder->andWhere( $dbr->expr( 'eeb_event_id', '<=', (int)$this->lastEvent ) ); } - $fields = array_merge( Event::selectFields(), [ - 'eeb_id', - 'eeb_user_id', - 'eeb_event_priority', - 'eeb_event_id', - 'eeb_event_hash', - ] ); - $res = $dbr->select( - $tables, - $fields, - $conds, - __METHOD__, - [ - 'ORDER BY' => 'eeb_event_priority', - 'LIMIT' => self::$displaySize + 1, - ] - ); + $res = $queryBuilder->fetchResultSet(); foreach ( $res as $row ) { // records in the queue inserted before email bundling code @@ -410,18 +399,17 @@ class EmailBatch { * @param int $startUserId * @param int $batchSize * - * @return IResultWrapper|bool + * @return IResultWrapper */ public static function getUsersToNotify( $startUserId, $batchSize ) { $dbr = DbFactory::newFromDefault()->getEchoDb( DB_REPLICA ); - $res = $dbr->select( - [ 'echo_email_batch' ], - [ 'eeb_user_id' ], - [ 'eeb_user_id > ' . (int)$startUserId ], - __METHOD__, - [ 'ORDER BY' => 'eeb_user_id', 'LIMIT' => $batchSize ] - ); - - return $res; + return $dbr->newSelectQueryBuilder() + ->select( 'eeb_user_id' ) + ->from( 'echo_email_batch' ) + ->where( $dbr->expr( 'eeb_user_id', '>', (int)$startUserId ) ) + ->orderBy( 'eeb_user_id' ) + ->limit( $batchSize ) + ->caller( __METHOD__ ) + ->fetchResultSet(); } } diff --git a/includes/Gateway/UserNotificationGateway.php b/includes/Gateway/UserNotificationGateway.php index 7ab19b1ca..53fdccd26 100644 --- a/includes/Gateway/UserNotificationGateway.php +++ b/includes/Gateway/UserNotificationGateway.php @@ -175,24 +175,19 @@ class UserNotificationGateway { } $db = $this->getDB( $dbSource ); - return $db->selectRowCount( - [ - self::$notificationTable, - self::$eventTable - ], - '1', - [ + return $db->newSelectQueryBuilder() + ->select( '1' ) + ->from( self::$notificationTable ) + ->leftJoin( self::$eventTable, null, 'notification_event=event_id' ) + ->where( [ 'notification_user' => $this->user->getId(), 'notification_read_timestamp' => null, 'event_deleted' => 0, 'event_type' => $eventTypesToLoad, - ], - __METHOD__, - [ 'LIMIT' => $cap ], - [ - 'echo_event' => [ 'LEFT JOIN', 'notification_event=event_id' ], - ] - ); + ] ) + ->limit( $cap ) + ->caller( __METHOD__ ) + ->fetchRowCount(); } /** @@ -204,21 +199,18 @@ class UserNotificationGateway { */ public function getUnreadNotifications( $type ) { $dbr = $this->getDB( DB_REPLICA ); - $res = $dbr->select( - [ - self::$notificationTable, - self::$eventTable - ], - [ 'notification_event' ], - [ + $res = $dbr->newSelectQueryBuilder() + ->select( 'notification_event' ) + ->from( self::$notificationTable ) + ->join( self::$eventTable, null, 'notification_event = event_id' ) + ->where( [ 'notification_user' => $this->user->getId(), 'notification_read_timestamp' => null, 'event_deleted' => 0, 'event_type' => $type, - 'notification_event = event_id' - ], - __METHOD__ - ); + ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); $eventIds = []; foreach ( $res as $row ) { diff --git a/includes/Mapper/EventMapper.php b/includes/Mapper/EventMapper.php index 41a04a143..f431b8242 100644 --- a/includes/Mapper/EventMapper.php +++ b/includes/Mapper/EventMapper.php @@ -49,7 +49,12 @@ class EventMapper extends AbstractMapper { public function fetchById( $id, $fromPrimary = false ) { $db = $fromPrimary ? $this->dbFactory->getEchoDb( DB_PRIMARY ) : $this->dbFactory->getEchoDb( DB_REPLICA ); - $row = $db->selectRow( 'echo_event', Event::selectFields(), [ 'event_id' => $id ], __METHOD__ ); + $row = $db->newSelectQueryBuilder() + ->select( Event::selectFields() ) + ->from( 'echo_event' ) + ->where( [ 'event_id' => $id ] ) + ->caller( __METHOD__ ) + ->fetchRow(); // If the row was not found, fall back on the primary database if it makes sense to do so if ( !$row && !$fromPrimary && $this->dbFactory->canRetryPrimary() ) { @@ -95,12 +100,12 @@ class EventMapper extends AbstractMapper { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); // From echo_event - $res = $dbr->select( - [ 'echo_event' ], - Event::selectFields(), - [ 'event_page_id' => $pageId ], - __METHOD__ - ); + $res = $dbr->newSelectQueryBuilder() + ->select( Event::selectFields() ) + ->from( 'echo_event' ) + ->where( [ 'event_page_id' => $pageId ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); foreach ( $res as $row ) { $event = Event::newFromRow( $row ); $events[] = $event; @@ -112,16 +117,16 @@ class EventMapper extends AbstractMapper { if ( $seenEventIds ) { // Some events have both a title and target page(s). // Skip the events that were already found in the echo_event table (the query above). - $conds[] = 'event_id NOT IN ( ' . $dbr->makeList( $seenEventIds ) . ' )'; + $conds[] = $dbr->expr( 'event_id', '!=', $seenEventIds ); } - $res = $dbr->select( - [ 'echo_event', 'echo_target_page' ], - Event::selectFields(), - $conds, - __METHOD__, - [ 'DISTINCT' ], - [ 'echo_target_page' => [ 'INNER JOIN', 'event_id=etp_event' ] ] - ); + $res = $dbr->newSelectQueryBuilder() + ->select( Event::selectFields() ) + ->distinct() + ->from( 'echo_event' ) + ->join( 'echo_target_page', null, 'event_id=etp_event' ) + ->where( $conds ) + ->caller( __METHOD__ ) + ->fetchResultSet(); foreach ( $res as $row ) { $events[] = Event::newFromRow( $row ); } @@ -157,22 +162,19 @@ class EventMapper extends AbstractMapper { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); $fields = array_merge( Event::selectFields(), [ 'notification_timestamp' ] ); - $res = $dbr->select( - [ 'echo_event', 'echo_notification', 'echo_target_page' ], - $fields, - [ + $res = $dbr->newSelectQueryBuilder() + ->select( $fields ) + ->from( 'echo_event' ) + ->join( 'echo_notification', null, 'notification_event=event_id' ) + ->join( 'echo_target_page', null, 'etp_event=event_id' ) + ->where( [ 'event_deleted' => 0, 'notification_user' => $user->getId(), 'notification_read_timestamp' => null, 'etp_page' => $pageId, - ], - __METHOD__, - [], - [ - 'echo_target_page' => [ 'INNER JOIN', 'etp_event=event_id' ], - 'echo_notification' => [ 'INNER JOIN', [ 'notification_event=event_id' ] ], - ] - ); + ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); $data = []; foreach ( $res as $row ) { @@ -206,31 +208,30 @@ class EventMapper extends AbstractMapper { $emailJoinConds = []; if ( $ignoreUserId !== null ) { if ( $ignoreUserTable === null || $ignoreUserTable === 'echo_notification' ) { - $notifJoinConds[] = 'notification_user != ' . $dbr->addQuotes( $ignoreUserId ); + $notifJoinConds[] = $dbr->expr( 'notification_user', '!=', $ignoreUserId ); } if ( $ignoreUserTable === null || $ignoreUserTable === 'echo_email_batch' ) { - $emailJoinConds[] = 'eeb_user_id != ' . $dbr->addQuotes( $ignoreUserId ); + $emailJoinConds[] = $dbr->expr( 'eeb_user_id', '!=', $ignoreUserId ); } } - $orphanedEventIds = $dbr->selectFieldValues( - [ 'echo_event', 'echo_notification', 'echo_email_batch' ], - 'event_id', - [ + $orphanedEventIds = $dbr->newSelectQueryBuilder() + ->select( 'event_id' ) + ->from( 'echo_event' ) + ->leftJoin( 'echo_notification', null, array_merge( + [ 'notification_event=event_id' ], + $notifJoinConds + ) ) + ->leftJoin( 'echo_email_batch', null, array_merge( + [ 'eeb_event_id=event_id' ], + $emailJoinConds + ) ) + ->where( [ 'event_id' => $eventIds, 'notification_timestamp' => null, 'eeb_user_id' => null - ], - __METHOD__, - [], - [ - 'echo_notification' => [ 'LEFT JOIN', array_merge( [ - 'notification_event=event_id' - ], $notifJoinConds ) ], - 'echo_email_batch' => [ 'LEFT JOIN', array_merge( [ - 'eeb_event_id=event_id' - ], $emailJoinConds ) ] - ] - ); + ] ) + ->caller( __METHOD__ ) + ->fetchFieldValues(); if ( $orphanedEventIds ) { $dbw->newDeleteQueryBuilder() ->deleteFrom( 'echo_event' ) diff --git a/includes/Mapper/NotificationMapper.php b/includes/Mapper/NotificationMapper.php index f57079736..75732853b 100644 --- a/includes/Mapper/NotificationMapper.php +++ b/includes/Mapper/NotificationMapper.php @@ -13,6 +13,7 @@ use MediaWiki\Title\Title; use MediaWiki\User\UserIdentity; use MWExceptionHandler; use Wikimedia\Rdbms\IDatabase; +use Wikimedia\Rdbms\SelectQueryBuilder; /** * Database mapper for Notification model @@ -175,7 +176,7 @@ class NotificationMapper extends AbstractMapper { $conds = []; if ( $excludeEventIds ) { - $conds[] = 'event_id NOT IN ( ' . $dbr->makeList( $excludeEventIds ) . ' ) '; + $conds[] = $dbr->expr( 'event_id', '!=', $excludeEventIds ); } if ( $titles ) { $conds['event_page_id'] = $this->getIdsForTitles( $titles ); @@ -251,19 +252,15 @@ class NotificationMapper extends AbstractMapper { ] ); } - $res = $dbr->select( - [ 'echo_notification', 'echo_event' ], - Notification::selectFields(), - $conds, - __METHOD__, - [ - 'ORDER BY' => 'notification_timestamp DESC, notification_event DESC', - 'LIMIT' => $limit, - ], - [ - 'echo_event' => [ 'LEFT JOIN', 'notification_event=event_id' ], - ] - ); + $res = $dbr->newSelectQueryBuilder() + ->select( Notification::selectFields() ) + ->from( 'echo_notification' ) + ->leftJoin( 'echo_event', null, 'notification_event=event_id' ) + ->where( $conds ) + ->orderBy( [ 'notification_timestamp', 'notification_event' ], SelectQueryBuilder::SORT_DESC ) + ->limit( $limit ) + ->caller( __METHOD__ ) + ->fetchResultSet(); /** @var Notification[] $allNotifications */ $allNotifications = []; @@ -298,19 +295,16 @@ class NotificationMapper extends AbstractMapper { public function fetchByUserEvents( UserIdentity $userIdentity, array $eventIds ) { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); - $result = $dbr->select( - [ 'echo_notification', 'echo_event' ], - Notification::selectFields(), - [ + $result = $dbr->newSelectQueryBuilder() + ->select( Notification::selectFields() ) + ->from( 'echo_notification' ) + ->join( 'echo_event', null, 'notification_event=event_id' ) + ->where( [ 'notification_user' => $userIdentity->getId(), 'notification_event' => $eventIds - ], - __METHOD__, - [], - [ - 'echo_event' => [ 'INNER JOIN', 'notification_event=event_id' ], - ] - ); + ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); $notifications = []; foreach ( $result as $row ) { @@ -328,23 +322,18 @@ class NotificationMapper extends AbstractMapper { */ public function fetchByUserOffset( UserIdentity $userIdentity, $offset ) { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); - $row = $dbr->selectRow( - [ 'echo_notification', 'echo_event' ], - Notification::selectFields(), - [ + $row = $dbr->newSelectQueryBuilder() + ->select( Notification::selectFields() ) + ->from( 'echo_notification' ) + ->leftJoin( 'echo_event', null, 'notification_event=event_id' ) + ->where( [ 'notification_user' => $userIdentity->getId(), 'event_deleted' => 0, - ], - __METHOD__, - [ - 'ORDER BY' => 'notification_timestamp DESC, notification_event DESC', - 'OFFSET' => $offset, - 'LIMIT' => 1 - ], - [ - 'echo_event' => [ 'LEFT JOIN', 'notification_event=event_id' ], - ] - ); + ] ) + ->orderBy( [ 'notification_timestamp', 'notification_event' ], SelectQueryBuilder::SORT_DESC ) + ->offset( $offset ) + ->caller( __METHOD__ ) + ->fetchRow(); if ( $row ) { return Notification::newFromRow( $row ); @@ -414,20 +403,15 @@ class NotificationMapper extends AbstractMapper { public function fetchUsersWithNotificationsForEvents( array $eventIds ) { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); - $res = $dbr->select( - [ 'echo_notification' ], - [ 'userId' => 'DISTINCT notification_user' ], - [ + return $dbr->newSelectQueryBuilder() + ->select( 'notification_user' ) + ->distinct() + ->from( 'echo_notification' ) + ->where( [ 'notification_event' => $eventIds - ], - __METHOD__ - ); - - $userIds = []; - foreach ( $res as $row ) { - $userIds[] = $row->userId; - } - return $userIds; + ] ) + ->caller( __METHOD__ ) + ->fetchFieldValues(); } } diff --git a/includes/Push/SubscriptionManager.php b/includes/Push/SubscriptionManager.php index ac16f5a0d..9dcf7ed43 100644 --- a/includes/Push/SubscriptionManager.php +++ b/includes/Push/SubscriptionManager.php @@ -92,17 +92,14 @@ class SubscriptionManager extends AbstractMapper { * @return array array of Subscription objects */ public function getSubscriptionsForUser( int $centralId ) { - $res = $this->dbr->select( - [ 'echo_push_subscription', 'echo_push_provider', 'echo_push_topic' ], - '*', - [ 'eps_user' => $centralId ], - __METHOD__, - [], - [ - 'echo_push_provider' => [ 'INNER JOIN', [ 'eps_provider = epp_id' ] ], - 'echo_push_topic' => [ 'LEFT JOIN', [ 'eps_topic = ept_id' ] ], - ] - ); + $res = $this->dbr->newSelectQueryBuilder() + ->select( '*' ) + ->from( 'echo_push_subscription' ) + ->join( 'echo_push_provider', null, 'eps_provider = epp_id' ) + ->leftJoin( 'echo_push_topic', null, 'eps_topic = ept_id' ) + ->where( [ 'eps_user' => $centralId ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); $result = []; foreach ( $res as $row ) { $result[] = Subscription::newFromRow( $row ); diff --git a/includes/UnreadWikis.php b/includes/UnreadWikis.php index 28f654a66..4a3a8d3b0 100644 --- a/includes/UnreadWikis.php +++ b/includes/UnreadWikis.php @@ -69,16 +69,16 @@ class UnreadWikis { return []; } - $rows = $dbr->select( - 'echo_unread_wikis', - [ + $rows = $dbr->newSelectQueryBuilder() + ->select( [ 'euw_wiki', 'euw_alerts', 'euw_alerts_ts', 'euw_messages', 'euw_messages_ts', - ], - [ 'euw_user' => $this->id ], - __METHOD__ - ); + ] ) + ->from( 'echo_unread_wikis' ) + ->where( [ 'euw_user' => $this->id ] ) + ->caller( __METHOD__ ) + ->fetchResultSet(); $wikis = []; foreach ( $rows as $row ) { diff --git a/includes/UserLocator.php b/includes/UserLocator.php index 21c4c40e4..be75413f6 100644 --- a/includes/UserLocator.php +++ b/includes/UserLocator.php @@ -141,14 +141,14 @@ class UserLocator { $services = MediaWikiServices::getInstance(); $dbr = $services->getConnectionProvider()->getReplicaDatabase(); $revQuery = $services->getRevisionStore()->getQueryInfo(); - $res = $dbr->selectRow( - $revQuery['tables'], - [ 'rev_user' => $revQuery['fields']['rev_user'] ], - [ 'rev_page' => $articleId ], - __METHOD__, - [ 'LIMIT' => 1, 'ORDER BY' => 'rev_timestamp, rev_id' ], - $revQuery['joins'] - ); + $res = $dbr->newSelectQueryBuilder() + ->select( [ 'rev_user' => $revQuery['fields']['rev_user'] ] ) + ->tables( $revQuery['tables'] ) + ->where( [ 'rev_page' => $articleId ] ) + ->orderBy( [ 'rev_timestamp', 'rev_id' ] ) + ->joinConds( $revQuery['joins'] ) + ->caller( __METHOD__ ) + ->fetchRow(); if ( !$res || !$res->rev_user ) { return null; } diff --git a/includes/UserMergeHooks.php b/includes/UserMergeHooks.php index 6538b55b9..327d51a6b 100644 --- a/includes/UserMergeHooks.php +++ b/includes/UserMergeHooks.php @@ -35,48 +35,46 @@ class UserMergeHooks implements // Select notifications that are now sent to the same user $dbw = DbFactory::newFromDefault()->getEchoDb( DB_PRIMARY ); $attributeManager = Services::getInstance()->getAttributeManager(); - $selfIds = $dbw->selectFieldValues( - [ 'echo_notification', 'echo_event' ], - 'event_id', - [ + $selfIds = $dbw->newSelectQueryBuilder() + ->select( 'event_id' ) + ->from( 'echo_notification' ) + ->join( 'echo_event', null, 'notification_event = event_id' ) + ->where( [ 'notification_user' => $newUser->getId(), - 'notification_event = event_id', 'notification_user = event_agent_id', - 'event_type NOT IN (' . $dbw->makeList( $attributeManager->getNotifyAgentEvents() ) . ')' - ], - $method - ) ?: []; + $dbw->expr( 'event_type', '!=', $attributeManager->getNotifyAgentEvents() ), + ] ) + ->caller( $method ) + ->fetchFieldValues(); // Select newer welcome notification(s) - $welcomeIds = $dbw->selectFieldValues( - [ 'echo_notification', 'echo_event' ], - 'event_id', - [ + $welcomeIds = $dbw->newSelectQueryBuilder() + ->select( 'event_id' ) + ->from( 'echo_event' ) + ->join( 'echo_notification', null, 'notification_event = event_id' ) + ->where( [ 'notification_user' => $newUser->getId(), - 'notification_event = event_id', 'event_type' => 'welcome', - ], - $method, - [ - 'ORDER BY' => 'notification_timestamp ASC', - 'OFFSET' => 1, - ] - ) ?: []; + ] ) + ->orderBy( 'notification_timestamp' ) + ->offset( 1 ) + ->caller( $method ) + ->fetchFieldValues(); // Select newer milestone notifications (per milestone level) $counts = []; $thankYouIds = []; - $thankYouRows = $dbw->select( - [ 'echo_notification', 'echo_event' ], - Event::selectFields(), - [ + $thankYouRows = $dbw->newSelectQueryBuilder() + ->select( Event::selectFields() ) + ->from( 'echo_event' ) + ->join( 'echo_notification', null, 'notification_event = event_id' ) + ->where( [ 'notification_user' => $newUser->getId(), - 'notification_event = event_id', 'event_type' => 'thank-you-edit', - ], - $method, - [ 'ORDER BY' => 'notification_timestamp ASC' ] - ); + ] ) + ->orderBy( 'notification_timestamp' ) + ->caller( $method ) + ->fetchResultSet(); foreach ( $thankYouRows as $row ) { $event = Event::newFromRow( $row ); $editCount = $event ? $event->getExtraParam( 'editCount' ) : null; diff --git a/maintenance/removeInvalidNotification.php b/maintenance/removeInvalidNotification.php index edf9c0997..b9b67f9d3 100644 --- a/maintenance/removeInvalidNotification.php +++ b/maintenance/removeInvalidNotification.php @@ -44,15 +44,15 @@ class RemoveInvalidNotification extends Maintenance { $count = $batchSize; while ( $count == $batchSize ) { - $res = $dbr->select( - [ 'echo_event' ], - [ 'event_id' ], - [ + $res = $dbr->newSelectQueryBuilder() + ->select( 'event_id' ) + ->from( 'echo_event' ) + ->where( [ 'event_type' => $this->invalidEventType, - ], - __METHOD__, - [ 'LIMIT' => $batchSize ] - ); + ] ) + ->limit( $batchSize ) + ->caller( __METHOD__ ) + ->fetchResultSet(); $event = []; $count = 0; diff --git a/tests/phpunit/Mapper/EventMapperTest.php b/tests/phpunit/Mapper/EventMapperTest.php index c46bf6d20..84c4eca1e 100644 --- a/tests/phpunit/Mapper/EventMapperTest.php +++ b/tests/phpunit/Mapper/EventMapperTest.php @@ -6,6 +6,7 @@ use MediaWiki\Extension\Notifications\Model\Event; use Wikimedia\Rdbms\FakeResultWrapper; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\InsertQueryBuilder; +use Wikimedia\Rdbms\SelectQueryBuilder; /** * @group Database @@ -120,6 +121,10 @@ class EventMapperTest extends MediaWikiIntegrationTestCase { ->willReturnCallback( static function () use ( $db ) { return new InsertQueryBuilder( $db ); } ); + $db->method( 'newSelectQueryBuilder' ) + ->willReturnCallback( static function () use ( $db ) { + return new SelectQueryBuilder( $db ); + } ); return $db; } diff --git a/tests/phpunit/Mapper/NotificationMapperTest.php b/tests/phpunit/Mapper/NotificationMapperTest.php index 1ea3d8158..348d0d8db 100644 --- a/tests/phpunit/Mapper/NotificationMapperTest.php +++ b/tests/phpunit/Mapper/NotificationMapperTest.php @@ -7,6 +7,7 @@ use MediaWiki\User\User; use Wikimedia\Rdbms\DeleteQueryBuilder; use Wikimedia\Rdbms\FakeResultWrapper; use Wikimedia\Rdbms\IDatabase; +use Wikimedia\Rdbms\SelectQueryBuilder; /** * @covers \MediaWiki\Extension\Notifications\Mapper\NotificationMapper @@ -175,6 +176,10 @@ class NotificationMapperTest extends MediaWikiIntegrationTestCase { ->willReturnCallback( static function () use ( $mockDb ) { return new DeleteQueryBuilder( $mockDb ); } ); + $mockDb->method( 'newSelectQueryBuilder' ) + ->willReturnCallback( static function () use ( $mockDb ) { + return new SelectQueryBuilder( $mockDb ); + } ); $notifMapper = new NotificationMapper( $this->mockDbFactory( $mockDb ) ); $this->assertTrue( $notifMapper->deleteByUserEventOffset( User::newFromId( 1 ), 500 ) ); @@ -246,6 +251,10 @@ class NotificationMapperTest extends MediaWikiIntegrationTestCase { ->willReturn( $dbResult['selectRow'] ); $db->method( 'onTransactionCommitOrIdle' ) ->will( new EchoExecuteFirstArgumentStub ); + $db->method( 'newSelectQueryBuilder' ) + ->willReturnCallback( static function () use ( $db ) { + return new SelectQueryBuilder( $db ); + } ); return $db; } diff --git a/tests/phpunit/Model/NotificationTest.php b/tests/phpunit/Model/NotificationTest.php index e1aeb5084..a5a8c9d1e 100644 --- a/tests/phpunit/Model/NotificationTest.php +++ b/tests/phpunit/Model/NotificationTest.php @@ -6,6 +6,7 @@ use MediaWiki\Extension\Notifications\Model\TargetPage; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\ILoadBalancer; use Wikimedia\Rdbms\LBFactory; +use Wikimedia\Rdbms\SelectQueryBuilder; /** * @covers \MediaWiki\Extension\Notifications\Model\Notification @@ -45,8 +46,13 @@ class NotificationTest extends MediaWikiIntegrationTestCase { } public function testNewFromRowWithException() { + $queryBuilder = $this->createMock( SelectQueryBuilder::class ); + $queryBuilder->method( $this->logicalOr( 'select', 'from', 'where', 'caller' ) )->willReturnSelf(); + $queryBuilder->method( 'fetchRow' )->willReturn( false ); + $db = $this->createMock( IDatabase::class ); + $db->method( 'newSelectQueryBuilder' )->willReturn( $queryBuilder ); $lb = $this->createMock( ILoadBalancer::class ); - $lb->method( 'getConnection' )->willReturn( $this->createMock( IDatabase::class ) ); + $lb->method( 'getConnection' )->willReturn( $db ); $lbFactory = $this->createMock( LBFactory::class ); $lbFactory->method( 'getExternalLB' )->willReturn( $lb ); $this->setService( 'DBLoadBalancer', $lb ); diff --git a/tests/phpunit/TalkPageFunctionalTest.php b/tests/phpunit/TalkPageFunctionalTest.php index 07645e561..fd4d56564 100644 --- a/tests/phpunit/TalkPageFunctionalTest.php +++ b/tests/phpunit/TalkPageFunctionalTest.php @@ -87,9 +87,15 @@ class TalkPageFunctionalTest extends ApiTestCase { * @return \stdClass[] All talk page edit events in db sorted from oldest to newest */ protected function fetchAllEvents() { - $res = $this->db->select( 'echo_event', Event::selectFields(), [ + $res = $this->db->newSelectQueryBuilder() + ->select( Event::selectFields() ) + ->from( 'echo_event' ) + ->where( [ 'event_type' => 'edit-user-talk', - ], __METHOD__, [ 'ORDER BY' => 'event_id ASC' ] ); + ] ) + ->orderBy( 'event_id' ) + ->caller( __METHOD__ ) + ->fetchResultSet(); return iterator_to_array( $res ); } diff --git a/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php b/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php index 8905341fa..ea7a675b4 100644 --- a/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php +++ b/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php @@ -6,6 +6,7 @@ use MediaWiki\Extension\Notifications\Gateway\UserNotificationGateway; use MediaWiki\User\User; use Wikimedia\Rdbms\FakeResultWrapper; use Wikimedia\Rdbms\IDatabase; +use Wikimedia\Rdbms\SelectQueryBuilder; use Wikimedia\Rdbms\UpdateQueryBuilder; /** @@ -164,6 +165,10 @@ class UserNotificationGatewayTest extends MediaWikiUnitTestCase { ->willReturnCallback( static function () use ( $db ) { return new UpdateQueryBuilder( $db ); } ); + $db->method( 'newSelectQueryBuilder' ) + ->willReturnCallback( static function () use ( $db ) { + return new SelectQueryBuilder( $db ); + } ); return $db; }