From d8a60d6b888df2011ec9be538e85e53ff49b10fc Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 20 Apr 2021 13:34:13 +0200 Subject: Only push delete-push to devices that also got the notification Signed-off-by: Joas Schilling --- lib/App.php | 4 +-- lib/Controller/EndpointController.php | 3 +- lib/Handler.php | 11 ++++--- lib/Push.php | 62 +++++++++++++++++++++-------------- tests/Unit/PushTest.php | 4 +-- 5 files changed, 50 insertions(+), 34 deletions(-) diff --git a/lib/App.php b/lib/App.php index a7cfcc0..8b0555e 100644 --- a/lib/App.php +++ b/lib/App.php @@ -78,8 +78,8 @@ class App implements IDeferrableApp { $this->push->deferPayloads(); } foreach ($deleted as $user => $notifications) { - foreach ($notifications as $notificationId) { - $this->push->pushDeleteToDevice($user, $notificationId); + foreach ($notifications as $data) { + $this->push->pushDeleteToDevice($user, $data['id'], $data['app']); } } if (!$isAlreadyDeferring) { diff --git a/lib/Controller/EndpointController.php b/lib/Controller/EndpointController.php index 8048e9e..df56b4b 100644 --- a/lib/Controller/EndpointController.php +++ b/lib/Controller/EndpointController.php @@ -174,10 +174,11 @@ class EndpointController extends OCSController { } try { + $notification = $this->handler->getById($id, $this->getCurrentUser()); $deleted = $this->handler->deleteById($id, $this->getCurrentUser()); if ($deleted) { - $this->push->pushDeleteToDevice($this->getCurrentUser(), $id); + $this->push->pushDeleteToDevice($this->getCurrentUser(), $id, $notification->getApp()); } } catch (NotificationNotFoundException $e) { } diff --git a/lib/Handler.php b/lib/Handler.php index cd505f6..71b4606 100644 --- a/lib/Handler.php +++ b/lib/Handler.php @@ -100,14 +100,17 @@ class Handler { $deleted[$row['user']] = []; } - $deleted[$row['user']][] = (int) $row['notification_id']; + $deleted[$row['user']][] = [ + 'id' => (int) $row['notification_id'], + 'app' => $row['app'], + ]; $notifications[(int) $row['notification_id']] = $this->notificationFromRow($row); } $statement->closeCursor(); - foreach ($deleted as $user => $notificationIds) { - foreach ($notificationIds as $notificationId) { - $this->deleteById($notificationId, $user, $notifications[$notificationId]); + foreach ($deleted as $user => $entries) { + foreach ($entries as $entry) { + $this->deleteById($entry['id'], $user, $notifications[$entry['id']]); } } diff --git a/lib/Push.php b/lib/Push.php index 902f414..116b4fc 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -121,6 +121,35 @@ class Push { $this->sendNotificationsToProxies(); } + public function filterDeviceList(array $devices, string $app): array { + $isTalkNotification = \in_array($app, ['spreed', 'talk', 'admin_notification_talk'], true); + + $talkDevices = array_filter($devices, static function ($device) { + return $device['apptype'] === 'talk'; + }); + $otherDevices = array_filter($devices, static function ($device) { + return $device['apptype'] !== 'talk'; + }); + + $this->printInfo('Identified ' . count($talkDevices) . ' Talk devices and ' . count($otherDevices) . ' others.'); + + if (!$isTalkNotification) { + if (empty($otherDevices)) { + // We only send file notifications to the files app. + // If you don't have such a device, bye! + return []; + } + return $otherDevices; + } + + if (empty($talkDevices)) { + // If you don't have a talk device, + // we fall back to the files app. + return $otherDevices; + } + return $talkDevices; + } + public function pushToDevice(int $id, INotification $notification, ?OutputInterface $output = null): void { if (!$this->config->getSystemValueBool('has_internet_connection', true)) { return; @@ -170,30 +199,9 @@ class Push { $this->printInfo('Public user key size: ' . strlen($userKey->getPublic())); $isTalkNotification = \in_array($notification->getApp(), ['spreed', 'talk', 'admin_notification_talk'], true); - $talkDevices = array_filter($devices, function ($device) { - return $device['apptype'] === 'talk'; - }); - $otherDevices = array_filter($devices, function ($device) { - return $device['apptype'] !== 'talk'; - }); - - $this->printInfo('Identified ' . count($talkDevices) . ' Talk devices and ' . count($otherDevices) . ' others.'); - - if (!$isTalkNotification) { - if (empty($otherDevices)) { - // We only send file notifications to the files app. - // If you don't have such a device, bye! - return; - } - $devices = $otherDevices; - } else { - if (empty($talkDevices)) { - // If you don't have a talk device, - // we fall back to the files app. - $devices = $otherDevices; - } else { - $devices = $talkDevices; - } + $devices = $this->filterDeviceList($devices, $notification->getApp()); + if (empty($devices)) { + return; } // We don't push to devices that are older than 60 days @@ -227,7 +235,7 @@ class Push { } } - public function pushDeleteToDevice(string $userId, int $notificationId): void { + public function pushDeleteToDevice(string $userId, int $notificationId, string $app = ''): void { if (!$this->config->getSystemValueBool('has_internet_connection', true)) { return; } @@ -238,6 +246,10 @@ class Push { } $devices = $this->getDevicesForUser($userId); + if ($notificationId !== 0 && $app !== '') { + // Only filter when it's not a single delete + $devices = $this->filterDeviceList($devices, $app); + } if (empty($devices)) { return; } diff --git a/tests/Unit/PushTest.php b/tests/Unit/PushTest.php index 63497ba..7ef71b7 100644 --- a/tests/Unit/PushTest.php +++ b/tests/Unit/PushTest.php @@ -646,11 +646,11 @@ class PushTest extends TestCase { ->willReturn('valid'); if ($isTalkNotification) { - $notification->expects($this->once()) + $notification->expects($this->any()) ->method('getApp') ->willReturn('spreed'); } else { - $notification->expects($this->once()) + $notification->expects($this->any()) ->method('getApp') ->willReturn('notifications'); } -- cgit v1.2.3