diff --git a/appinfo/info.xml b/appinfo/info.xml index e141bb01559..d0d84c68973 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -16,7 +16,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m ]]> - 19.0.0-beta.1 + 19.0.0-beta.1.1 agpl Daniel Calviño Sánchez @@ -65,6 +65,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m OCA\Talk\BackgroundJob\Reminder OCA\Talk\BackgroundJob\RemoveEmptyRooms OCA\Talk\BackgroundJob\ResetAssignedSignalingServer + OCA\Talk\BackgroundJob\RetryNotificationsJob diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 88b40cbb198..c627779fecf 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -84,6 +84,7 @@ use OCA\Talk\Events\SystemMessagesMultipleSentEvent; use OCA\Talk\Events\UserJoinedRoomEvent; use OCA\Talk\Federation\CloudFederationProviderTalk; +use OCA\Talk\Federation\Proxy\TalkV1\Notifier\CancelRetryOCMListener as TalkV1CancelRetryOCMListener; use OCA\Talk\Federation\Proxy\TalkV1\Notifier\MessageSentListener as TalkV1MessageSentListener; use OCA\Talk\Federation\Proxy\TalkV1\Notifier\RoomModifiedListener as TalkV1RoomModifiedListener; use OCA\Talk\Files\Listener as FilesListener; @@ -284,6 +285,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(ChatMessageSentEvent::class, TalkV1MessageSentListener::class); $context->registerEventListener(SystemMessageSentEvent::class, TalkV1MessageSentListener::class); $context->registerEventListener(SystemMessagesMultipleSentEvent::class, TalkV1MessageSentListener::class); + $context->registerEventListener(AttendeeRemovedEvent::class, TalkV1CancelRetryOCMListener::class); // Signaling listeners (External) $context->registerEventListener(AttendeesAddedEvent::class, SignalingListener::class); diff --git a/lib/BackgroundJob/RetryJob.php b/lib/BackgroundJob/RetryJob.php deleted file mode 100644 index 591f88611d2..00000000000 --- a/lib/BackgroundJob/RetryJob.php +++ /dev/null @@ -1,100 +0,0 @@ - - * - * @author Bjoern Schiessle - * @author Björn Schießle - * @author Joas Schilling - * @author Lukas Reschke - * @author Morris Jobke - * @author Roeland Jago Douma - * @author Gary Kim - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ -namespace OCA\Talk\BackgroundJob; - -use OCA\Talk\Federation\BackendNotifier; -use OCP\AppFramework\Utility\ITimeFactory; -use OCP\BackgroundJob\IJobList; -use OCP\BackgroundJob\Job; -use OCP\ILogger; - -/** - * Class RetryJob - * - * Background job to re-send update of federated re-shares to the remote server in - * case the server was not available on the first try - * - * @package OCA\Talk\BackgroundJob - */ -class RetryJob extends Job { - - /** @var int max number of attempts to send the request */ - private int $maxTry = 20; - - - public function __construct( - private BackendNotifier $backendNotifier, - ITimeFactory $timeFactory, - ) { - parent::__construct($timeFactory); - } - - /** - * run the job, then remove it from the jobList - * - * @param IJobList $jobList - * @param ILogger|null $logger - */ - public function execute(IJobList $jobList, ?ILogger $logger = null): void { - if (((int)$this->argument['try']) > $this->maxTry) { - $jobList->remove($this, $this->argument); - return; - } - if ($this->shouldRun($this->argument)) { - parent::execute($jobList, $logger); - $jobList->remove($this, $this->argument); - } - } - - protected function run($argument): void { - $remote = $argument['remote']; - $data = json_decode($argument['data'], true, flags: JSON_THROW_ON_ERROR); - $try = (int)$argument['try'] + 1; - - $this->backendNotifier->sendUpdateDataToRemote($remote, $data, $try); - } - - /** - * test if it is time for the next run - * - * @param array $argument - * @return bool - */ - protected function shouldRun(array $argument): bool { - $lastRun = (int)$argument['lastRun']; - $try = (int)$argument['try']; - return (($this->time->getTime() - $lastRun) > $this->nextRunBreak($try)); - } - - protected function nextRunBreak(int $try): int { - return min(($try + 1) * 300, 3600); - } -} diff --git a/lib/BackgroundJob/RetryNotificationsJob.php b/lib/BackgroundJob/RetryNotificationsJob.php new file mode 100644 index 00000000000..de0454a9a27 --- /dev/null +++ b/lib/BackgroundJob/RetryNotificationsJob.php @@ -0,0 +1,49 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Talk\BackgroundJob; + +use OCA\Talk\Federation\BackendNotifier; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; + +/** + * Retry to send OCM notifications + */ +class RetryNotificationsJob extends TimedJob { + public function __construct( + private BackendNotifier $backendNotifier, + ITimeFactory $timeFactory, + ) { + parent::__construct($timeFactory); + + // Every time the jobs run + $this->setInterval(1); + } + + protected function run($argument): void { + $this->backendNotifier->retrySendingFailedNotifications($this->time->getDateTime()); + } +} diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index e725eb66315..e9ba26e551f 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -39,7 +39,7 @@ use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Exceptions\UnauthorizedException; use OCA\Talk\Federation\Authenticator; -use OCA\Talk\Federation\BackendNotifier; +use OCA\Talk\Federation\FederationManager; use OCA\Talk\GuestManager; use OCA\Talk\Manager; use OCA\Talk\MatterbridgeManager; @@ -126,7 +126,7 @@ public function __construct( protected LoggerInterface $logger, protected Authenticator $federationAuthenticator, protected Capabilities $capabilities, - protected BackendNotifier $federationBackendNotifier, + protected FederationManager $federationManager, ) { parent::__construct($appName, $request); } @@ -1263,11 +1263,7 @@ public function removeSelfFromRoom(): DataResponse { */ protected function removeSelfFromRoomLogic(Room $room, Participant $participant): DataResponse { if ($room->getRemoteServer() !== '') { - $this->federationBackendNotifier->sendShareDeclined( - $room->getRemoteServer(), - (int) $participant->getAttendee()->getRemoteId(), - $participant->getAttendee()->getAccessToken(), - ); + $this->federationManager->rejectByRemoveSelf($room, $this->userId); } if ($room->getType() !== Room::TYPE_ONE_TO_ONE && $room->getType() !== Room::TYPE_ONE_TO_ONE_FORMER) { diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php index ccb422b23bd..15cfb0ac34e 100644 --- a/lib/Federation/BackendNotifier.php +++ b/lib/Federation/BackendNotifier.php @@ -27,15 +27,16 @@ use OCA\FederatedFileSharing\AddressHandler; use OCA\Federation\TrustedServers; -use OCA\Talk\BackgroundJob\RetryJob; use OCA\Talk\Config; use OCA\Talk\Exceptions\RoomHasNoModeratorException; use OCA\Talk\Model\Attendee; +use OCA\Talk\Model\RetryNotification; +use OCA\Talk\Model\RetryNotificationMapper; use OCA\Talk\Room; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; -use OCP\BackgroundJob\IJobList; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\Exception; use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationNotification; @@ -56,12 +57,13 @@ public function __construct( private AddressHandler $addressHandler, private LoggerInterface $logger, private ICloudFederationProviderManager $federationProviderManager, - private IJobList $jobList, private IUserManager $userManager, private IURLGenerator $url, private IAppManager $appManager, private Config $talkConfig, private IAppConfig $appConfig, + private RetryNotificationMapper $retryNotificationMapper, + private ITimeFactory $timeFactory, ) { } @@ -192,22 +194,7 @@ public function sendShareAccepted( ] ); - try { - $response = $this->federationProviderManager->sendCloudNotification($remote, $notification); - if ($response->getStatusCode() === Http::STATUS_CREATED) { - return true; - } - - $this->logger->warning("Failed to send share accepted notification for share from $remote, received status code {code}\n{body}", [ - 'code' => $response->getStatusCode(), - 'body' => (string) $response->getBody(), - ]); - - return false; - } catch (OCMProviderException $e) { - $this->logger->error("Failed to send share accepted notification for share from $remote, received OCMProviderException", ['exception' => $e]); - return false; - } + return $this->sendUpdateToRemote($remote, $notification, retry: false) === true; } /** @@ -219,7 +206,7 @@ public function sendShareDeclined( int $remoteAttendeeId, #[SensitiveParameter] string $accessToken, - ): bool { + ): void { $remote = $this->prepareRemoteUrl($remoteServerUrl); $notification = $this->cloudFederationFactory->getCloudFederationNotification(); @@ -234,22 +221,9 @@ public function sendShareDeclined( ] ); - try { - $response = $this->federationProviderManager->sendCloudNotification($remote, $notification); - if ($response->getStatusCode() === Http::STATUS_CREATED) { - return true; - } - - $this->logger->warning("Failed to send share declined notification for share from $remote, received status code {code}\n{body}", [ - 'code' => $response->getStatusCode(), - 'body' => (string) $response->getBody(), - ]); - - return false; - } catch (OCMProviderException $e) { - $this->logger->error("Failed to send share declined notification for share from $remote, received OCMProviderException", ['exception' => $e]); - return false; - } + // We don't handle the return here as all local data is already deleted. + // If the retry ever aborts due to "unknown" we are fine with it. + $this->sendUpdateToRemote($remote, $notification); } public function sendRemoteUnShare( @@ -272,6 +246,8 @@ public function sendRemoteUnShare( ] ); + // We don't handle the return here as when the retry ever + // aborts due to "unknown" we are fine with it. $this->sendUpdateToRemote($remote, $notification); } @@ -288,7 +264,7 @@ public function sendRoomModifiedUpdate( string $changedProperty, string|int|bool|null $newValue, string|int|bool|null $oldValue, - ): void { + ): ?bool { $remote = $this->prepareRemoteUrl($remoteServer); $notification = $this->cloudFederationFactory->getCloudFederationNotification(); @@ -306,7 +282,7 @@ public function sendRoomModifiedUpdate( ], ); - $this->sendUpdateToRemote($remote, $notification); + return $this->sendUpdateToRemote($remote, $notification); } /** @@ -324,7 +300,7 @@ public function sendMessageUpdate( string $localToken, array $messageData, array $unreadInfo, - ): void { + ): ?bool { $remote = $this->prepareRemoteUrl($remoteServer); $notification = $this->cloudFederationFactory->getCloudFederationNotification(); @@ -341,32 +317,23 @@ public function sendMessageUpdate( ], ); - $this->sendUpdateToRemote($remote, $notification); - } - - /** - * @param string $remote - * @param array{notificationType: string, resourceType: string, providerId: string, notification: array} $data - * @param int $try - * @return void - * @internal Used to send retries in background jobs - */ - public function sendUpdateDataToRemote(string $remote, array $data, int $try): void { - $notification = $this->cloudFederationFactory->getCloudFederationNotification(); - $notification->setMessage( - $data['notificationType'], - $data['resourceType'], - $data['providerId'], - $data['notification'] - ); - $this->sendUpdateToRemote($remote, $notification, $try); + return $this->sendUpdateToRemote($remote, $notification); } - protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0): void { + protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0, bool $retry = true): ?bool { try { $response = $this->federationProviderManager->sendCloudNotification($remote, $notification); if ($response->getStatusCode() === Http::STATUS_CREATED) { - return; + return true; + } + + if ($response->getStatusCode() === Http::STATUS_BAD_REQUEST) { + $ocmBody = json_decode((string) $response->getBody(), true) ?? []; + if (isset($ocmBody['message']) && $ocmBody['message'] === FederationManager::OCM_RESOURCE_NOT_FOUND) { + // Remote exists but tells us the OCM notification can not be received (invalid invite data) + // So we stop retrying + return null; + } } $this->logger->warning("Failed to send notification for share from $remote, received status code {code}\n{body}", [ @@ -377,14 +344,87 @@ protected function sendUpdateToRemote(string $remote, ICloudFederationNotificati $this->logger->error("Failed to send notification for share from $remote, received OCMProviderException", ['exception' => $e]); } - $this->jobList->add( - RetryJob::class, - [ - 'remote' => $remote, - 'data' => json_encode($notification->getMessage(), JSON_THROW_ON_ERROR), - 'try' => $try, - ] + if ($retry && $try === 0) { + $now = $this->timeFactory->getTime(); + $now += $this->getRetryDelay(1); + + // Talk data + $retryNotification = new RetryNotification(); + $retryNotification->setRemoteServer($remote); + $retryNotification->setNumAttempts(1); + $retryNotification->setNextRetry($this->timeFactory->getDateTime('@' . $now)); + + // OCM notification data + $data = $notification->getMessage(); + $retryNotification->setNotificationType($data['notificationType']); + $retryNotification->setResourceType($data['resourceType']); + $retryNotification->setProviderId($data['providerId']); + $retryNotification->setNotification(json_encode($data['notification'])); + + $this->retryNotificationMapper->insert($retryNotification); + } + + return false; + } + + public function retrySendingFailedNotifications(\DateTimeInterface $dueDateTime): void { + $retryNotifications = $this->retryNotificationMapper->getAllDue($dueDateTime); + + foreach ($retryNotifications as $retryNotification) { + $this->retrySendingFailedNotification($retryNotification); + } + } + + protected function retrySendingFailedNotification(RetryNotification $retryNotification): void { + $notification = $this->cloudFederationFactory->getCloudFederationNotification(); + $notification->setMessage( + $retryNotification->getNotificationType(), + $retryNotification->getResourceType(), + $retryNotification->getProviderId(), + json_decode($retryNotification->getNotification(), true, flags: JSON_THROW_ON_ERROR), ); + + $success = $this->sendUpdateToRemote($retryNotification->getRemoteServer(), $notification, $retryNotification->getNumAttempts()); + + if ($success) { + $this->retryNotificationMapper->delete($retryNotification); + } elseif ($success === null) { + $this->logger->error('Server signaled the OCM notification is not accepted at ' . $retryNotification->getRemoteServer() . ', giving up!'); + $this->retryNotificationMapper->delete($retryNotification); + } elseif ($retryNotification->getNumAttempts() === RetryNotification::MAX_NUM_ATTEMPTS) { + $this->logger->error('Failed to send notification to ' . $retryNotification->getRemoteServer() . ' ' . RetryNotification::MAX_NUM_ATTEMPTS . ' times, giving up!'); + $this->retryNotificationMapper->delete($retryNotification); + } else { + $retryNotification->setNumAttempts($retryNotification->getNumAttempts() + 1); + + $now = $this->timeFactory->getTime(); + $now += $this->getRetryDelay($retryNotification->getNumAttempts()); + + $retryNotification->setNextRetry($this->timeFactory->getDateTime('@' . $now)); + $this->retryNotificationMapper->update($retryNotification); + } + } + + /** + * First 5 attempts are retried on the next cron run. + * Attempts 6-10 we back off to cover slightly longer maintenance/downtimes (5 minutes * per attempt) + * And the last tries 11-20 are retried with ~8 hours delay + * + * This means the last retry is after ~84 hours so a downtime from Friday to Monday would be covered + */ + protected function getRetryDelay(int $attempt): int { + if ($attempt < 5) { + // Retry after "attempt" minutes + return 5 * 60; + } + + if ($attempt > 10) { + // Retry after 8 hours + return 8 * 3600; + } + + // Retry after "attempt" * 5 minutes + return $attempt * 5 * 60; } protected function prepareRemoteUrl(string $remote): string { diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index abe0dccd632..4615585afd7 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -260,12 +260,12 @@ private function shareUnshared(int $remoteAttendeeId, array $notification): arra try { $room = $this->manager->getRoomById($invite->getLocalRoomId()); } catch (RoomNotFoundException) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } // Sanity check to make sure the room is a remote room if (!$room->isFederatedRemoteRoom()) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } $this->invitationMapper->delete($invite); @@ -293,12 +293,12 @@ private function roomModified(int $remoteAttendeeId, array $notification): array try { $room = $this->manager->getRoomById($invite->getLocalRoomId()); } catch (RoomNotFoundException) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } // Sanity check to make sure the room is a remote room if (!$room->isFederatedRemoteRoom()) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } if ($notification['changedProperty'] === ARoomModifiedEvent::PROPERTY_AVATAR) { @@ -331,12 +331,12 @@ private function messagePosted(int $remoteAttendeeId, array $notification): arra try { $room = $this->manager->getRoomById($invite->getLocalRoomId()); } catch (RoomNotFoundException) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } // Sanity check to make sure the room is a remote room if (!$room->isFederatedRemoteRoom()) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } $message = new ProxyCacheMessage(); @@ -435,10 +435,10 @@ private function getLocalAttendeeAndValidate( try { $attendee = $this->attendeeMapper->getById($attendeeId); } catch (Exception) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } if ($attendee->getActorType() !== Attendee::ACTOR_FEDERATED_USERS) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } if ($attendee->getAccessToken() !== $sharedSecret) { throw new AuthenticationFailedException(); @@ -468,7 +468,7 @@ private function getByRemoteAttendeeAndValidate( try { return $this->invitationMapper->getByRemoteAndAccessToken($remoteServerUrl, $remoteAttendeeId, $sharedSecret); } catch (DoesNotExistException) { - throw new ShareNotFound(); + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } } diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php index 6dfaf27ffcb..b5bd2ea25d9 100644 --- a/lib/Federation/FederationManager.php +++ b/lib/Federation/FederationManager.php @@ -51,6 +51,7 @@ * FederationManager handles incoming federated rooms */ class FederationManager { + public const OCM_RESOURCE_NOT_FOUND = 'RESOURCE_NOT_FOUND'; public const TALK_ROOM_RESOURCE = 'talk-room'; public const TALK_PROTOCOL_NAME = 'nctalk'; public const NOTIFICATION_SHARE_ACCEPTED = 'SHARE_ACCEPTED'; @@ -178,16 +179,38 @@ public function rejectRemoteRoomShare(IUser $user, int $shareId): void { throw new \InvalidArgumentException('invitation'); } + if ($invitation->getUserId() !== $user->getUID()) { + throw new UnauthorizedException('user'); + } + if ($invitation->getState() !== Invitation::STATE_PENDING) { throw new \InvalidArgumentException('state'); } - if ($invitation->getUserId() !== $user->getUID()) { - throw new UnauthorizedException('user'); + $this->rejectInvitation($invitation, $user->getUID()); + } + + /** + * @throws \InvalidArgumentException + * @throws UnauthorizedException + */ + public function rejectByRemoveSelf(Room $room, string $userId): void { + try { + $invitation = $this->invitationMapper->getInvitationForUserByLocalRoom($room, $userId); + } catch (DoesNotExistException $e) { + throw new \InvalidArgumentException('invitation'); } + $this->rejectInvitation($invitation, $userId); + } + + /** + * @throws \InvalidArgumentException + * @throws UnauthorizedException + */ + protected function rejectInvitation(Invitation $invitation, string $userId): void { $this->invitationMapper->delete($invitation); - $this->markNotificationProcessed($user->getUID(), $shareId); + $this->markNotificationProcessed($userId, $invitation->getId()); $this->backendNotifier->sendShareDeclined($invitation->getRemoteServerUrl(), $invitation->getRemoteAttendeeId(), $invitation->getAccessToken()); } diff --git a/lib/Federation/Proxy/TalkV1/Notifier/CancelRetryOCMListener.php b/lib/Federation/Proxy/TalkV1/Notifier/CancelRetryOCMListener.php new file mode 100644 index 00000000000..a924ecbf124 --- /dev/null +++ b/lib/Federation/Proxy/TalkV1/Notifier/CancelRetryOCMListener.php @@ -0,0 +1,55 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Talk\Federation\Proxy\TalkV1\Notifier; + +use OCA\Talk\Events\AttendeeRemovedEvent; +use OCA\Talk\Model\Attendee; +use OCA\Talk\Model\RetryNotificationMapper; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; + +/** + * @template-implements IEventListener + */ +class CancelRetryOCMListener implements IEventListener { + public function __construct( + protected RetryNotificationMapper $retryNotificationMapper, + ) { + } + + public function handle(Event $event): void { + if (!$event instanceof AttendeeRemovedEvent) { + return; + } + + $attendee = $event->getAttendee(); + if ($attendee->getActorType() !== Attendee::ACTOR_FEDERATED_USERS) { + return; + } + + $this->retryNotificationMapper->deleteByProviderId( + (string) $event->getAttendee()->getId() + ); + } +} diff --git a/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php b/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php index fb795b80425..7c12c9b32ed 100644 --- a/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php +++ b/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php @@ -26,6 +26,7 @@ use OCA\Talk\Chat\ChatManager; use OCA\Talk\Chat\MessageParser; +use OCA\Talk\Events\AAttendeeRemovedEvent; use OCA\Talk\Events\ASystemMessageSentEvent; use OCA\Talk\Events\ChatMessageSentEvent; use OCA\Talk\Events\SystemMessageSentEvent; @@ -118,7 +119,7 @@ public function handle(Event $event): void { 'unreadMentionDirect' => $lastMentionDirect !== 0 && $lastReadMessage < $lastMentionDirect ]; - $this->backendNotifier->sendMessageUpdate( + $success = $this->backendNotifier->sendMessageUpdate( $cloudId->getRemote(), $participant->getAttendee()->getId(), $participant->getAttendee()->getAccessToken(), @@ -126,6 +127,10 @@ public function handle(Event $event): void { $messageData, $unreadInfo, ); + + if ($success === null) { + $this->participantService->removeAttendee($event->getRoom(), $participant, AAttendeeRemovedEvent::REASON_LEFT); + } } } } diff --git a/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php b/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php index 2047cca5e57..ec7a3012fe2 100644 --- a/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php +++ b/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php @@ -23,6 +23,7 @@ namespace OCA\Talk\Federation\Proxy\TalkV1\Notifier; +use OCA\Talk\Events\AAttendeeRemovedEvent; use OCA\Talk\Events\ARoomModifiedEvent; use OCA\Talk\Events\RoomModifiedEvent; use OCA\Talk\Federation\BackendNotifier; @@ -62,7 +63,7 @@ public function handle(Event $event): void { foreach ($participants as $participant) { $cloudId = $this->cloudIdManager->resolveCloudId($participant->getAttendee()->getActorId()); - $this->backendNotifier->sendRoomModifiedUpdate( + $success = $this->backendNotifier->sendRoomModifiedUpdate( $cloudId->getRemote(), $participant->getAttendee()->getId(), $participant->getAttendee()->getAccessToken(), @@ -71,6 +72,10 @@ public function handle(Event $event): void { $event->getNewValue(), $event->getOldValue(), ); + + if ($success === null) { + $this->participantService->removeAttendee($event->getRoom(), $participant, AAttendeeRemovedEvent::REASON_LEFT); + } } } } diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php index 31b961d24e7..d908d912246 100644 --- a/lib/Middleware/InjectionMiddleware.php +++ b/lib/Middleware/InjectionMiddleware.php @@ -264,7 +264,7 @@ protected function getRoomByInvite(AEnvironmentAwareController $controller): voi $participant = $controller->getParticipant(); if (!$participant instanceof Participant) { try { - $invitation = $this->invitationMapper->getInvitationsForUserByLocalRoom($room, $this->userId); + $invitation = $this->invitationMapper->getInvitationForUserByLocalRoom($room, $this->userId); $controller->setRoom($room); $controller->setInvitation($invitation); } catch (DoesNotExistException $e) { diff --git a/lib/Migration/Version19000Date20240312105627.php b/lib/Migration/Version19000Date20240312105627.php new file mode 100644 index 00000000000..c8e3b45e548 --- /dev/null +++ b/lib/Migration/Version19000Date20240312105627.php @@ -0,0 +1,106 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Talk\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Add table to queue federation notifications to retry + */ +class Version19000Date20240312105627 extends SimpleMigrationStep { + public function __construct( + protected IDBConnection $connection, + ) { + } + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->createTable('talk_retry_ocm'); + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + ]); + $table->addColumn('remote_server', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + $table->addColumn('num_attempts', Types::INTEGER, [ + 'default' => 0, + 'unsigned' => true, + ]); + $table->addColumn('next_retry', Types::DATETIME, [ + 'notnull' => false, + ]); + $table->addColumn('notification_type', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('resource_type', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('provider_id', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('notification', Types::TEXT, [ + 'notnull' => true, + ]); + + + $table->setPrimaryKey(['id']); + $table->addIndex(['next_retry'], 'talk_retry_ocm_next'); + + return $schema; + } + + /** + * Remove legacy RetryJobs + */ + public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) { + /** @psalm-suppress UndefinedClass */ + $formerClassName = \OCA\Talk\BackgroundJob\RetryJob::class; + + $query = $this->connection->getQueryBuilder(); + $query->delete('jobs') + ->where($query->expr()->eq('class', $query->createNamedParameter($formerClassName))); + $query->executeStatement(); + } +} diff --git a/lib/Model/InvitationMapper.php b/lib/Model/InvitationMapper.php index ceb4593f740..5568f2fabe9 100644 --- a/lib/Model/InvitationMapper.php +++ b/lib/Model/InvitationMapper.php @@ -98,7 +98,7 @@ public function getInvitationsForUser(IUser $user): array { /** * @throws DoesNotExistException */ - public function getInvitationsForUserByLocalRoom(Room $room, string $userId): Invitation { + public function getInvitationForUserByLocalRoom(Room $room, string $userId): Invitation { $query = $this->db->getQueryBuilder(); $query->select('*') diff --git a/lib/Model/RetryNotification.php b/lib/Model/RetryNotification.php new file mode 100644 index 00000000000..d460f934c09 --- /dev/null +++ b/lib/Model/RetryNotification.php @@ -0,0 +1,66 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Talk\Model; + +use OCP\AppFramework\Db\Entity; + +/** + * @method void setRemoteServer(string $remoteServer) + * @method string getRemoteServer() + * @method void setNumAttempts(int $numAttempts) + * @method int getNumAttempts() + * @method void setNextRetry(\DateTime $nextRetry) + * @method \DateTime getNextRetry() + * @method void setNotificationType(string $notificationType) + * @method string getNotificationType() + * @method void setResourceType(string $resourceType) + * @method string getResourceType() + * @method void setProviderId(string $providerId) + * @method string getProviderId() + * @method void setNotification(string $notification) + * @method string getNotification() + */ +class RetryNotification extends Entity { + public const MAX_NUM_ATTEMPTS = 20; + + protected string $remoteServer = ''; + protected int $numAttempts = 0; + protected ?\DateTime $nextRetry = null; + protected string $notificationType = ''; + protected string $resourceType = ''; + protected string $providerId = ''; + protected string $notification = ''; + + public function __construct() { + $this->addType('remoteServer', 'string'); + $this->addType('numAttempts', 'int'); + $this->addType('nextRetry', 'datetime'); + $this->addType('notificationType', 'string'); + $this->addType('resourceType', 'string'); + $this->addType('providerId', 'string'); + $this->addType('notification', 'string'); + } +} diff --git a/lib/Model/RetryNotificationMapper.php b/lib/Model/RetryNotificationMapper.php new file mode 100644 index 00000000000..0395dd80e37 --- /dev/null +++ b/lib/Model/RetryNotificationMapper.php @@ -0,0 +1,69 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Talk\Model; + +use OCP\AppFramework\Db\QBMapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * @method RetryNotification mapRowToEntity(array $row) + * @method RetryNotification findEntity(IQueryBuilder $query) + * @method RetryNotification[] findEntities(IQueryBuilder $query) + * @template-extends QBMapper + */ +class RetryNotificationMapper extends QBMapper { + public function __construct( + IDBConnection $db, + ) { + parent::__construct($db, 'talk_retry_ocm', RetryNotification::class); + } + + /** + * @return RetryNotification[] + */ + public function getAllDue(\DateTimeInterface $dueDateTime, ?int $limit = 500): array { + $query = $this->db->getQueryBuilder(); + $query->select('*') + ->from($this->getTableName()) + ->where($query->expr()->lte('next_retry', $query->createNamedParameter($dueDateTime, IQueryBuilder::PARAM_DATE), IQueryBuilder::PARAM_DATE)); + + if ($limit !== null) { + $query->setMaxResults($limit) + ->orderBy('next_retry', 'ASC') + ->addOrderBy('id', 'ASC'); + } + + return $this->findEntities($query); + } + + public function deleteByProviderId($providerId): void { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('provider_id', $query->createNamedParameter($providerId))); + $query->executeStatement(); + } +} diff --git a/tests/integration/spreedcheats/lib/Controller/ApiController.php b/tests/integration/spreedcheats/lib/Controller/ApiController.php index 72f1c7ceefb..b1d40cbadc2 100644 --- a/tests/integration/spreedcheats/lib/Controller/ApiController.php +++ b/tests/integration/spreedcheats/lib/Controller/ApiController.php @@ -27,6 +27,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IRequest; use OCP\Share\IShare; @@ -85,16 +86,21 @@ public function resetSpreed(): DataResponse { $delete = $this->db->getQueryBuilder(); $delete->delete('talk_poll_votes')->executeStatement(); + $delete = $this->db->getQueryBuilder(); + $delete->delete('talk_proxy_messages')->executeStatement(); + $delete = $this->db->getQueryBuilder(); $delete->delete('talk_reminders')->executeStatement(); + $delete = $this->db->getQueryBuilder(); + $delete->delete('talk_retry_ocm')->executeStatement(); + $delete = $this->db->getQueryBuilder(); $delete->delete('talk_rooms')->executeStatement(); $delete = $this->db->getQueryBuilder(); $delete->delete('talk_sessions')->executeStatement(); - $delete = $this->db->getQueryBuilder(); $delete->delete('share') ->where($delete->expr()->orX( @@ -103,6 +109,13 @@ public function resetSpreed(): DataResponse { )) ->executeStatement(); + + $delete = $this->db->getQueryBuilder(); + $delete->delete('preferences') + ->where($delete->expr()->in('configkey', $delete->createNamedParameter(['changelog', 'note_to_self'], IQueryBuilder::PARAM_STR_ARRAY))) + ->andWhere($delete->expr()->eq('appid', $delete->createNamedParameter('spreed'))) + ->executeStatement(); + try { $delete = $this->db->getQueryBuilder(); $delete->delete('notifications') diff --git a/tests/php/Federation/FederationTest.php b/tests/php/Federation/FederationTest.php index 2d94caca280..36b9143ecba 100644 --- a/tests/php/Federation/FederationTest.php +++ b/tests/php/Federation/FederationTest.php @@ -35,6 +35,7 @@ use OCA\Talk\Model\Invitation; use OCA\Talk\Model\InvitationMapper; use OCA\Talk\Model\ProxyCacheMessageMapper; +use OCA\Talk\Model\RetryNotificationMapper; use OCA\Talk\Notification\FederationChatNotifier; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; @@ -42,7 +43,7 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; -use OCP\BackgroundJob\IJobList; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationNotification; @@ -101,6 +102,8 @@ class FederationTest extends TestCase { protected FederationChatNotifier|MockObject $federationChatNotifier; protected UserConverter|MockObject $userConverter; protected ICacheFactory|MockObject $cacheFactory; + protected RetryNotificationMapper|MockObject $retryNotificationMapper; + protected ITimeFactory|MockObject $timeFactory; public function setUp(): void { parent::setUp(); @@ -118,18 +121,21 @@ public function setUp(): void { $this->url = $this->createMock(IURLGenerator::class); $this->proxyCacheMessageMapper = $this->createMock(ProxyCacheMessageMapper::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->retryNotificationMapper = $this->createMock(RetryNotificationMapper::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); $this->backendNotifier = new BackendNotifier( $this->cloudFederationFactory, $this->addressHandler, $this->logger, $this->cloudFederationProviderManager, - $this->createMock(IJobList::class), $this->userManager, $this->url, $this->appManager, $this->config, $this->appConfig, + $this->retryNotificationMapper, + $this->timeFactory, ); $this->federationManager = $this->createMock(FederationManager::class); @@ -424,7 +430,7 @@ public function testSendAcceptNotification() { $success = $this->backendNotifier->sendShareAccepted($remote, $id, $token); - $this->assertEquals(true, $success); + $this->assertTrue($success); } public function testSendRejectNotification() { @@ -467,8 +473,6 @@ public function testSendRejectNotification() { ->with('/') ->willReturn('https://example.tld/index.php/'); - $success = $this->backendNotifier->sendShareDeclined($remote, $id, $token); - - $this->assertEquals(true, $success); + $this->backendNotifier->sendShareDeclined($remote, $id, $token); } }