From 2feece8bade06d1318e651700f9032fb2c077849 Mon Sep 17 00:00:00 2001 From: Umherirrender Date: Thu, 18 Apr 2024 21:54:01 +0200 Subject: [PATCH] IReadableDatabase::select cannot return false Remove check for false from IDatabase::select as this is not possible A DBQueryError is thrown (documented since efda8cd3 / I056b7148) Change-Id: I465a9158aa6430e7ff8a5a83fe55c5944315aa40 --- .../Api/ApiEchoUnreadNotificationPages.php | 7 ---- includes/Gateway/UserNotificationGateway.php | 6 ++-- includes/Mapper/EventMapper.php | 16 ++++----- includes/Mapper/NotificationMapper.php | 33 ++++++------------- includes/UserMergeHooks.php | 2 +- .../phpunit/Mapper/NotificationMapperTest.php | 12 ------- .../Gateway/UserNotificationGatewayTest.php | 7 ---- 7 files changed, 19 insertions(+), 64 deletions(-) diff --git a/includes/Api/ApiEchoUnreadNotificationPages.php b/includes/Api/ApiEchoUnreadNotificationPages.php index 5f31a254a..578c201b9 100644 --- a/includes/Api/ApiEchoUnreadNotificationPages.php +++ b/includes/Api/ApiEchoUnreadNotificationPages.php @@ -108,13 +108,6 @@ class ApiEchoUnreadNotificationPages extends ApiQueryBase { [ 'echo_notification' => [ 'INNER JOIN', 'notification_event = event_id' ] ] ); - if ( $rows === false ) { - return [ - 'pages' => [], - 'totalCount' => 0, - ]; - } - $nullCount = 0; $pageCounts = []; foreach ( $rows as $row ) { diff --git a/includes/Gateway/UserNotificationGateway.php b/includes/Gateway/UserNotificationGateway.php index 49fb9ba36..7ab19b1ca 100644 --- a/includes/Gateway/UserNotificationGateway.php +++ b/includes/Gateway/UserNotificationGateway.php @@ -221,10 +221,8 @@ class UserNotificationGateway { ); $eventIds = []; - if ( $res ) { - foreach ( $res as $row ) { - $eventIds[$row->notification_event] = $row->notification_event; - } + foreach ( $res as $row ) { + $eventIds[$row->notification_event] = $row->notification_event; } return $eventIds; diff --git a/includes/Mapper/EventMapper.php b/includes/Mapper/EventMapper.php index 8953dfbe5..41a04a143 100644 --- a/includes/Mapper/EventMapper.php +++ b/includes/Mapper/EventMapper.php @@ -101,12 +101,10 @@ class EventMapper extends AbstractMapper { [ 'event_page_id' => $pageId ], __METHOD__ ); - if ( $res ) { - foreach ( $res as $row ) { - $event = Event::newFromRow( $row ); - $events[] = $event; - $seenEventIds[] = $event->getId(); - } + foreach ( $res as $row ) { + $event = Event::newFromRow( $row ); + $events[] = $event; + $seenEventIds[] = $event->getId(); } // From echo_target_page @@ -124,10 +122,8 @@ class EventMapper extends AbstractMapper { [ 'DISTINCT' ], [ 'echo_target_page' => [ 'INNER JOIN', 'event_id=etp_event' ] ] ); - if ( $res ) { - foreach ( $res as $row ) { - $events[] = Event::newFromRow( $row ); - } + foreach ( $res as $row ) { + $events[] = Event::newFromRow( $row ); } return $events; diff --git a/includes/Mapper/NotificationMapper.php b/includes/Mapper/NotificationMapper.php index 02037f4a6..f57079736 100644 --- a/includes/Mapper/NotificationMapper.php +++ b/includes/Mapper/NotificationMapper.php @@ -265,11 +265,6 @@ class NotificationMapper extends AbstractMapper { ] ); - // query failure of some sort - if ( !$res ) { - return []; - } - /** @var Notification[] $allNotifications */ $allNotifications = []; foreach ( $res as $row ) { @@ -298,7 +293,7 @@ class NotificationMapper extends AbstractMapper { * * @param UserIdentity $userIdentity * @param int[] $eventIds - * @return Notification[]|false + * @return Notification[] */ public function fetchByUserEvents( UserIdentity $userIdentity, array $eventIds ) { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); @@ -317,15 +312,11 @@ class NotificationMapper extends AbstractMapper { ] ); - if ( $result ) { - $notifications = []; - foreach ( $result as $row ) { - $notifications[] = Notification::newFromRow( $row ); - } - return $notifications; - } else { - return false; + $notifications = []; + foreach ( $result as $row ) { + $notifications[] = Notification::newFromRow( $row ); } + return $notifications; } /** @@ -418,7 +409,7 @@ class NotificationMapper extends AbstractMapper { * Fetch ids of users that have notifications for certain events * * @param int[] $eventIds - * @return int[]|false + * @return int[] */ public function fetchUsersWithNotificationsForEvents( array $eventIds ) { $dbr = $this->dbFactory->getEchoDb( DB_REPLICA ); @@ -432,15 +423,11 @@ class NotificationMapper extends AbstractMapper { __METHOD__ ); - if ( $res ) { - $userIds = []; - foreach ( $res as $row ) { - $userIds[] = $row->userId; - } - return $userIds; - } else { - return false; + $userIds = []; + foreach ( $res as $row ) { + $userIds[] = $row->userId; } + return $userIds; } } diff --git a/includes/UserMergeHooks.php b/includes/UserMergeHooks.php index 63e881624..6538b55b9 100644 --- a/includes/UserMergeHooks.php +++ b/includes/UserMergeHooks.php @@ -76,7 +76,7 @@ class UserMergeHooks implements ], $method, [ 'ORDER BY' => 'notification_timestamp ASC' ] - ) ?: []; + ); foreach ( $thankYouRows as $row ) { $event = Event::newFromRow( $row ); $editCount = $event ? $event->getExtraParam( 'editCount' ) : null; diff --git a/tests/phpunit/Mapper/NotificationMapperTest.php b/tests/phpunit/Mapper/NotificationMapperTest.php index e824f2dd1..8563e6c51 100644 --- a/tests/phpunit/Mapper/NotificationMapperTest.php +++ b/tests/phpunit/Mapper/NotificationMapperTest.php @@ -20,12 +20,6 @@ class NotificationMapperTest extends MediaWikiIntegrationTestCase { } public function fetchUnreadByUser( User $user, $limit, array $eventTypes = [] ) { - // Unsuccessful select - $notifMapper = new NotificationMapper( $this->mockDbFactory( [ 'select' => false ] ) ); - $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10, null, '' ); - $this->assertSame( [], $res ); - - // Successful select $dbResult = [ (object)[ 'event_id' => 1, @@ -55,12 +49,6 @@ class NotificationMapperTest extends MediaWikiIntegrationTestCase { } public function testFetchByUser() { - // Unsuccessful select - $notifMapper = new NotificationMapper( $this->mockDbFactory( [ 'select' => false ] ) ); - $res = $notifMapper->fetchByUser( $this->mockUser(), 10, '' ); - $this->assertSame( [], $res ); - - // Successful select $notifDbResult = [ (object)[ 'event_id' => 1, diff --git a/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php b/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php index 6e5f8fb10..ce80d191f 100644 --- a/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php +++ b/tests/phpunit/unit/Gateway/UserNotificationGatewayTest.php @@ -91,13 +91,6 @@ class UserNotificationGatewayTest extends MediaWikiUnitTestCase { } public function testGetUnreadNotifications() { - $gateway = new UserNotificationGateway( - $this->mockUser(), - $this->mockDbFactory( [ 'select' => false ] ), - $this->mockConfig() - ); - $this->assertSame( [], $gateway->getUnreadNotifications( 'user_talk' ) ); - $dbResult = [ (object)[ 'notification_event' => 1 ], (object)[ 'notification_event' => 2 ],