Skip to content

Commit

Permalink
fix(federation): Cancel retries and remove participants when the remo…
Browse files Browse the repository at this point in the history
…te says they are gone

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Mar 13, 2024
1 parent b2a7e41 commit 80344d6
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 17 deletions.
28 changes: 22 additions & 6 deletions lib/Federation/BackendNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public function sendShareAccepted(
]
);

return $this->sendUpdateToRemote($remote, $notification, retry: false);
return $this->sendUpdateToRemote($remote, $notification, retry: false) === true;
}

/**
Expand All @@ -221,6 +221,8 @@ public function sendShareDeclined(
]
);

// 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);
}

Expand All @@ -244,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);
}

Expand All @@ -260,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();
Expand All @@ -278,7 +282,7 @@ public function sendRoomModifiedUpdate(
],
);

$this->sendUpdateToRemote($remote, $notification);
return $this->sendUpdateToRemote($remote, $notification);
}

/**
Expand All @@ -296,7 +300,7 @@ public function sendMessageUpdate(
string $localToken,
array $messageData,
array $unreadInfo,
): void {
): ?bool {
$remote = $this->prepareRemoteUrl($remoteServer);

$notification = $this->cloudFederationFactory->getCloudFederationNotification();
Expand All @@ -313,16 +317,25 @@ public function sendMessageUpdate(
],
);

$this->sendUpdateToRemote($remote, $notification);
return $this->sendUpdateToRemote($remote, $notification);
}

protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0, bool $retry = true): bool {
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 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}", [
'code' => $response->getStatusCode(),
'body' => (string) $response->getBody(),
Expand Down Expand Up @@ -375,6 +388,9 @@ protected function retrySendingFailedNotification(RetryNotification $retryNotifi

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);
Expand Down
18 changes: 9 additions & 9 deletions lib/Federation/CloudFederationProviderTalk.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}

Expand Down
1 change: 1 addition & 0 deletions lib/Federation/FederationManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
7 changes: 6 additions & 1 deletion lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -118,14 +119,18 @@ 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(),
$event->getRoom()->getToken(),
$messageData,
$unreadInfo,
);

if ($success === null) {
$this->participantService->removeAttendee($event->getRoom(), $participant, AAttendeeRemovedEvent::REASON_LEFT);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand All @@ -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);
}
}
}
}

0 comments on commit 80344d6

Please sign in to comment.