diff options
author | Joas Schilling <coding@schilljs.com> | 2024-03-18 10:29:45 +0100 |
---|---|---|
committer | Joas Schilling <coding@schilljs.com> | 2024-03-18 11:02:19 +0100 |
commit | 03605b04b41aede858de98d1b7e3b8625be06293 (patch) | |
tree | ccb7d89bc99acdd9f6eb8cd421e6fbba8a3f839a /lib/Federation | |
parent | a330ccce2b55875972c164fba89006642adea200 (diff) |
fix(federation): Fix updating PCM after editing or deleting a message
Signed-off-by: Joas Schilling <coding@schilljs.com>
Diffstat (limited to 'lib/Federation')
-rw-r--r-- | lib/Federation/CloudFederationProviderTalk.php | 125 | ||||
-rw-r--r-- | lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php | 19 |
2 files changed, 91 insertions, 53 deletions
diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index 2788300a0..6a0ae28a0 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -32,6 +32,7 @@ use OCA\Talk\Config; use OCA\Talk\Events\AAttendeeRemovedEvent; use OCA\Talk\Events\ARoomModifiedEvent; use OCA\Talk\Events\AttendeesAddedEvent; +use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Federation\Proxy\TalkV1\UserConverter; @@ -46,6 +47,7 @@ use OCA\Talk\Notification\FederationChatNotifier; use OCA\Talk\Participant; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; +use OCA\Talk\Service\ProxyCacheMessageService; use OCA\Talk\Service\RoomService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; @@ -90,6 +92,7 @@ class CloudFederationProviderTalk implements ICloudFederationProvider { private IEventDispatcher $dispatcher, private LoggerInterface $logger, private ProxyCacheMessageMapper $proxyCacheMessageMapper, + private ProxyCacheMessageService $pcmService, private FederationChatNotifier $federationChatNotifier, private UserConverter $userConverter, ICacheFactory $cacheFactory, @@ -345,6 +348,15 @@ class CloudFederationProviderTalk implements ICloudFederationProvider { throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); } + $removeParentMessage = null; + if ($notification['messageData']['systemMessage'] === 'message_edited' + || $notification['messageData']['systemMessage'] === 'message_deleted') { + $metaData = json_decode($notification['messageData']['metaData'], true); + if (isset($metaData['replyToMessageId'])) { + $removeParentMessage = $metaData['replyToMessageId']; + } + } + // We transform the parameters when storing in the PCM, so we only have // to do it once for each message. // Note: `messageParameters` (array during parsing) vs `messageParameter` (string during sending) @@ -357,55 +369,57 @@ class CloudFederationProviderTalk implements ICloudFederationProvider { /** @var array{remoteMessageId: int, actorType: string, actorId: string, actorDisplayName: string, messageType: string, systemMessage: string, expirationDatetime: string, message: string, messageParameter: string, creationDatetime: string, metaData: string} $converted */ $notification['messageData'] = $converted; - - $message = new ProxyCacheMessage(); - $message->setLocalToken($room->getToken()); - $message->setRemoteServerUrl($notification['remoteServerUrl']); - $message->setRemoteToken($notification['remoteToken']); - $message->setRemoteMessageId($notification['messageData']['remoteMessageId']); - $message->setActorType($notification['messageData']['actorType']); - $message->setActorId($notification['messageData']['actorId']); - $message->setActorDisplayName($notification['messageData']['actorDisplayName']); - $message->setMessageType($notification['messageData']['messageType']); - $message->setSystemMessage($notification['messageData']['systemMessage']); - if ($notification['messageData']['expirationDatetime']) { - $message->setExpirationDatetime(new \DateTime($notification['messageData']['expirationDatetime'])); - } - $message->setMessage($notification['messageData']['message']); - $message->setMessageParameters($notification['messageData']['messageParameter']); - $message->setCreationDatetime(new \DateTime($notification['messageData']['creationDatetime'])); - $message->setMetaData($notification['messageData']['metaData']); - - try { - $this->proxyCacheMessageMapper->insert($message); - - $lastMessageId = $room->getLastMessageId(); - if ($notification['messageData']['remoteMessageId'] > $lastMessageId) { - $lastMessageId = (int) $notification['messageData']['remoteMessageId']; + $message = null; + if ($removeParentMessage === null) { + $message = new ProxyCacheMessage(); + $message->setLocalToken($room->getToken()); + $message->setRemoteServerUrl($notification['remoteServerUrl']); + $message->setRemoteToken($notification['remoteToken']); + $message->setRemoteMessageId($notification['messageData']['remoteMessageId']); + $message->setActorType($notification['messageData']['actorType']); + $message->setActorId($notification['messageData']['actorId']); + $message->setActorDisplayName($notification['messageData']['actorDisplayName']); + $message->setMessageType($notification['messageData']['messageType']); + $message->setSystemMessage($notification['messageData']['systemMessage']); + if ($notification['messageData']['expirationDatetime']) { + $message->setExpirationDatetime(new \DateTime($notification['messageData']['expirationDatetime'])); } - $this->roomService->setLastMessageInfo($room, $lastMessageId, new \DateTime()); + $message->setMessage($notification['messageData']['message']); + $message->setMessageParameters($notification['messageData']['messageParameter']); + $message->setCreationDatetime(new \DateTime($notification['messageData']['creationDatetime'])); + $message->setMetaData($notification['messageData']['metaData']); - if ($this->proxyCacheMessages instanceof ICache) { - $cacheKey = sha1(json_encode([$notification['remoteServerUrl'], $notification['remoteToken']])); - $cacheData = $this->proxyCacheMessages->get($cacheKey); - if ($cacheData === null || $cacheData < $notification['messageData']['remoteMessageId']) { - $this->proxyCacheMessages->set($cacheKey, $notification['messageData']['remoteMessageId'], 300); + try { + $this->proxyCacheMessageMapper->insert($message); + + $lastMessageId = $room->getLastMessageId(); + if ($notification['messageData']['remoteMessageId'] > $lastMessageId) { + $lastMessageId = (int) $notification['messageData']['remoteMessageId']; + } + $this->roomService->setLastMessageInfo($room, $lastMessageId, new \DateTime()); + + if ($this->proxyCacheMessages instanceof ICache) { + $cacheKey = sha1(json_encode([$notification['remoteServerUrl'], $notification['remoteToken']])); + $cacheData = $this->proxyCacheMessages->get($cacheKey); + if ($cacheData === null || $cacheData < $notification['messageData']['remoteMessageId']) { + $this->proxyCacheMessages->set($cacheKey, $notification['messageData']['remoteMessageId'], 300); + } + } + } catch (DBException $e) { + // DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION happens when + // multiple users are in the same conversation. We are therefore + // informed multiple times about the same remote message. + if ($e->getReason() !== DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + $this->logger->error('Error saving proxy cache message failed: ' . $e->getMessage(), ['exception' => $e]); + throw $e; } - } - } catch (DBException $e) { - // DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION happens when - // multiple users are in the same conversation. We are therefore - // informed multiple times about the same remote message. - if ($e->getReason() !== DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - $this->logger->error('Error saving proxy cache message failed: ' . $e->getMessage(), ['exception' => $e]); - throw $e; - } - $message = $this->proxyCacheMessageMapper->findByRemote( - $notification['remoteServerUrl'], - $notification['remoteToken'], - $notification['messageData']['remoteMessageId'], - ); + $message = $this->pcmService->findByRemote( + $notification['remoteServerUrl'], + $notification['remoteToken'], + $notification['messageData']['remoteMessageId'], + ); + } } try { @@ -415,6 +429,23 @@ class CloudFederationProviderTalk implements ICloudFederationProvider { return []; } + if ($removeParentMessage !== null) { + try { + $this->pcmService->syncRemoteMessage($room, $participant, $removeParentMessage); + } catch (\InvalidArgumentException|CannotReachRemoteException) { + $oldMessage = $this->pcmService->findByRemote( + $notification['remoteServerUrl'], + $notification['remoteToken'], + $removeParentMessage, + ); + $this->pcmService->delete($oldMessage); + $this->logger->info('Failed to resync chat message #' . $removeParentMessage . ' after being notified by host ' . $notification['remoteServerUrl']); + } + + // Update the last activity so the left sidebar refreshes the data as well + $this->roomService->setLastMessageInfo($room, $room->getLastMessageId(), new \DateTime()); + } + $this->logger->debug('Setting unread info for local federated user ' . $invite->getUserId() . ' in ' . $room->getToken() . ' to ' . json_encode($notification['unreadInfo']), [ 'app' => 'spreed-federation', ]); @@ -427,7 +458,9 @@ class CloudFederationProviderTalk implements ICloudFederationProvider { $notification['unreadInfo']['lastReadMessage'], ); - $this->federationChatNotifier->handleChatMessage($room, $participant, $message, $notification); + if ($message instanceof ProxyCacheMessage) { + $this->federationChatNotifier->handleChatMessage($room, $participant, $message, $notification); + } return []; } diff --git a/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php b/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php index 6f1277c1b..5447264f2 100644 --- a/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php +++ b/lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php @@ -62,10 +62,6 @@ class MessageSentListener implements IEventListener { return; } - if ($event instanceof ASystemMessageSentEvent && $event->shouldSkipLastActivityUpdate()) { - return; - } - // FIXME once we store/cache the info skip this if the room has no federation participant // if (!$event->getRoom()->hasFederatedParticipants()) { // return; @@ -76,6 +72,14 @@ class MessageSentListener implements IEventListener { $chatMessage = $this->messageParser->createMessage($event->getRoom(), null, $event->getComment(), $l); $this->messageParser->parseMessage($chatMessage); + $systemMessage = $chatMessage->getMessageType() === ChatManager::VERB_SYSTEM ? $chatMessage->getMessageRaw() : ''; + if ($systemMessage !== 'message_edited' + && $systemMessage !== 'message_deleted' + && $event instanceof ASystemMessageSentEvent + && $event->shouldSkipLastActivityUpdate()) { + return; + } + if (!$chatMessage->getVisibility()) { return; } @@ -86,8 +90,9 @@ class MessageSentListener implements IEventListener { $metaData = $event->getComment()->getMetaData() ?? []; $parent = $event->getParent(); if ($parent instanceof IComment) { - $metaData[ProxyCacheMessage::METADATA_REPLYTO_TYPE] = $parent->getActorType(); - $metaData[ProxyCacheMessage::METADATA_REPLYTO_ID] = $parent->getActorId(); + $metaData[ProxyCacheMessage::METADATA_REPLY_TO_ACTOR_TYPE] = $parent->getActorType(); + $metaData[ProxyCacheMessage::METADATA_REPLY_TO_ACTOR_ID] = $parent->getActorId(); + $metaData[ProxyCacheMessage::METADATA_REPLY_TO_MESSAGE_ID] = (int) $parent->getId(); } $messageData = [ @@ -96,7 +101,7 @@ class MessageSentListener implements IEventListener { 'actorId' => $chatMessage->getActorId(), 'actorDisplayName' => $chatMessage->getActorDisplayName(), 'messageType' => $chatMessage->getMessageType(), - 'systemMessage' => $chatMessage->getMessageType() === ChatManager::VERB_SYSTEM ? $chatMessage->getMessageRaw() : '', + 'systemMessage' => $systemMessage, 'expirationDatetime' => $expireDate ? $expireDate->format(\DateTime::ATOM) : '', 'message' => $chatMessage->getMessage(), 'messageParameter' => json_encode($chatMessage->getMessageParameters(), JSON_THROW_ON_ERROR), |