diff options
author | Joas Schilling <coding@schilljs.com> | 2024-03-15 15:28:53 +0100 |
---|---|---|
committer | Joas Schilling <coding@schilljs.com> | 2024-03-15 15:31:36 +0100 |
commit | 99cafa2ceb247aa5541cc80ca3b4fcbd9d07c91d (patch) | |
tree | d0f38e83c65d2f0e624162e526ce203d5301b631 | |
parent | eb45b2eb8787207d072e5399da50f24f000df658 (diff) |
feat(reminders): Implement reminders for messages not in cache
Signed-off-by: Joas Schilling <coding@schilljs.com>
-rw-r--r-- | lib/Controller/ChatController.php | 35 | ||||
-rw-r--r-- | lib/Service/ProxyCacheMessageService.php | 128 | ||||
-rw-r--r-- | lib/Service/ReminderService.php | 7 | ||||
-rw-r--r-- | lib/Service/RoomFormatter.php | 5 | ||||
-rw-r--r-- | openapi-full.json | 36 | ||||
-rw-r--r-- | openapi.json | 36 | ||||
-rw-r--r-- | src/types/openapi/openapi-full.ts | 16 | ||||
-rw-r--r-- | src/types/openapi/openapi.ts | 16 | ||||
-rw-r--r-- | tests/integration/features/federation/chat.feature | 7 | ||||
-rw-r--r-- | tests/integration/features/federation/reminder.feature | 75 | ||||
-rw-r--r-- | tests/php/Controller/ChatControllerTest.php | 4 |
11 files changed, 323 insertions, 42 deletions
diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 0b1a6bb72..c1bd4907a 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -31,6 +31,7 @@ use OCA\Talk\Chat\AutoComplete\Sorter; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Chat\MessageParser; use OCA\Talk\Chat\ReactionManager; +use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Federation\Authenticator; use OCA\Talk\GuestManager; use OCA\Talk\MatterbridgeManager; @@ -46,7 +47,6 @@ use OCA\Talk\Model\Attachment; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Bot; use OCA\Talk\Model\Message; -use OCA\Talk\Model\ProxyCacheMessageMapper; use OCA\Talk\Model\Session; use OCA\Talk\Participant; use OCA\Talk\ResponseDefinitions; @@ -55,6 +55,7 @@ use OCA\Talk\Service\AttachmentService; use OCA\Talk\Service\AvatarService; use OCA\Talk\Service\BotService; use OCA\Talk\Service\ParticipantService; +use OCA\Talk\Service\ProxyCacheMessageService; use OCA\Talk\Service\ReminderService; use OCA\Talk\Service\RoomFormatter; use OCA\Talk\Service\SessionService; @@ -128,7 +129,7 @@ class ChatController extends AEnvironmentAwareController { protected ITrustedDomainHelper $trustedDomainHelper, private IL10N $l, protected Authenticator $federationAuthenticator, - protected ProxyCacheMessageMapper $proxyCacheMapper, + protected ProxyCacheMessageService $pcmService, ) { parent::__construct($appName, $request); } @@ -916,7 +917,7 @@ class ChatController extends AEnvironmentAwareController { * @psalm-param non-negative-int $messageId * @param int $timestamp Timestamp of the reminder * @psalm-param non-negative-int $timestamp - * @return DataResponse<Http::STATUS_CREATED, TalkChatReminder, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array<empty>, array{}> + * @return DataResponse<Http::STATUS_CREATED, TalkChatReminder, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error?: string}, array{}> * * 201: Reminder created successfully * 404: Message not found @@ -928,9 +929,9 @@ class ChatController extends AEnvironmentAwareController { #[UserRateLimit(limit: 60, period: 3600)] public function setReminder(int $messageId, int $timestamp): DataResponse { try { - $this->validateMessageExists($messageId); + $this->validateMessageExists($messageId, sync: true); } catch (DoesNotExistException) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'message'], Http::STATUS_NOT_FOUND); } $reminder = $this->reminderService->setReminder( @@ -948,7 +949,7 @@ class ChatController extends AEnvironmentAwareController { * * @param int $messageId ID of the message * @psalm-param non-negative-int $messageId - * @return DataResponse<Http::STATUS_OK, TalkChatReminder, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array<empty>, array{}> + * @return DataResponse<Http::STATUS_OK, TalkChatReminder, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error?: string}, array{}> * * 200: Reminder returned * 404: No reminder found @@ -962,7 +963,7 @@ class ChatController extends AEnvironmentAwareController { try { $this->validateMessageExists($messageId); } catch (DoesNotExistException) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'message'], Http::STATUS_NOT_FOUND); } try { @@ -973,7 +974,7 @@ class ChatController extends AEnvironmentAwareController { ); return new DataResponse($reminder->jsonSerialize(), Http::STATUS_OK); } catch (DoesNotExistException) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'reminder'], Http::STATUS_NOT_FOUND); } } @@ -982,7 +983,7 @@ class ChatController extends AEnvironmentAwareController { * * @param int $messageId ID of the message * @psalm-param non-negative-int $messageId - * @return DataResponse<Http::STATUS_OK|Http::STATUS_NOT_FOUND, array<empty>, array{}> + * @return DataResponse<Http::STATUS_OK|Http::STATUS_NOT_FOUND, array{error?: string}, array{}> * * 200: Reminder deleted successfully * 404: Message not found @@ -995,7 +996,7 @@ class ChatController extends AEnvironmentAwareController { try { $this->validateMessageExists($messageId); } catch (DoesNotExistException) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'message'], Http::STATUS_NOT_FOUND); } $this->reminderService->deleteReminder( @@ -1007,9 +1008,19 @@ class ChatController extends AEnvironmentAwareController { return new DataResponse([], Http::STATUS_OK); } - protected function validateMessageExists(int $messageId): void { + /** + * @throws DoesNotExistException + * @throws CannotReachRemoteException + */ + protected function validateMessageExists(int $messageId, bool $sync = false): void { if ($this->room->isFederatedConversation()) { - $this->proxyCacheMapper->findByRemote($this->room->getRemoteServer(), $this->room->getRemoteToken(), $messageId); + try { + $this->pcmService->findByRemote($this->room->getRemoteServer(), $this->room->getRemoteToken(), $messageId); + } catch (DoesNotExistException) { + if ($sync) { + $this->pcmService->syncRemoteMessage($this->room, $this->participant, $messageId); + } + } return; } diff --git a/lib/Service/ProxyCacheMessageService.php b/lib/Service/ProxyCacheMessageService.php new file mode 100644 index 000000000..a7ebb1c8e --- /dev/null +++ b/lib/Service/ProxyCacheMessageService.php @@ -0,0 +1,128 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @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 <http://www.gnu.org/licenses/>. + * + */ + +namespace OCA\Talk\Service; + +use OCA\Talk\Exceptions\CannotReachRemoteException; +use OCA\Talk\Model\Message; +use OCA\Talk\Model\ProxyCacheMessage; +use OCA\Talk\Model\ProxyCacheMessageMapper; +use OCA\Talk\Participant; +use OCA\Talk\ResponseDefinitions; +use OCA\Talk\Room; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Http; +use OCP\DB\Exception as DBException; +use Psr\Log\LoggerInterface; + +/** + * @psalm-import-type TalkChatMessageWithParent from ResponseDefinitions + */ +class ProxyCacheMessageService { + public function __construct( + protected ProxyCacheMessageMapper $mapper, + protected LoggerInterface $logger, + ) { + } + + /** + * @throws DoesNotExistException + */ + public function findByRemote(string $remoteServerUrl, string $remoteToken, int $remoteMessageId): ProxyCacheMessage { + return $this->mapper->findByRemote($remoteServerUrl, $remoteToken, $remoteMessageId); + } + + /** + * @throws \InvalidArgumentException + * @throws CannotReachRemoteException + */ + public function syncRemoteMessage(Room $room, Participant $participant, int $messageId): ProxyCacheMessage { + if (!$room->isFederatedConversation()) { + throw new \InvalidArgumentException('room'); + } + + /** @var \OCA\Talk\Federation\Proxy\TalkV1\Controller\ChatController $proxy */ + $proxy = \OCP\Server::get(\OCA\Talk\Federation\Proxy\TalkV1\Controller\ChatController::class); + $ocsResponse = $proxy->getMessageContext($room, $participant, $messageId, 1); + + if ($ocsResponse->getStatus() !== Http::STATUS_OK || !isset($ocsResponse->getData()[0])) { + throw new \InvalidArgumentException('message'); + } + + /** @var TalkChatMessageWithParent $messageData */ + $messageData = $ocsResponse->getData()[0]; + + $proxy = new ProxyCacheMessage(); + $proxy->setLocalToken($room->getToken()); + $proxy->setRemoteServerUrl($room->getRemoteServer()); + $proxy->setRemoteToken($room->getRemoteToken()); + $proxy->setRemoteMessageId($messageData['id']); + $proxy->setActorType($messageData['actorType']); + $proxy->setActorId($messageData['actorId']); + $proxy->setActorDisplayName($messageData['actorDisplayName']); + $proxy->setMessageType($messageData['messageType']); + $proxy->setSystemMessage($messageData['systemMessage']); + if ($messageData['expirationTimestamp']) { + $proxy->setExpirationDatetime(new \DateTime('@' . $messageData['expirationTimestamp'])); + } + $proxy->setCreationDatetime(new \DateTime('@' . $messageData['timestamp'])); + $proxy->setMessage($messageData['message']); + $proxy->setMessageParameters(json_encode($messageData['messageParameters'])); + + $metaData = []; + if (!empty($messageData['lastEditActorType']) && !empty($messageData['lastEditActorId'])) { + $metaData[Message::METADATA_LAST_EDITED_BY_TYPE] = $messageData['lastEditActorType']; + $metaData[Message::METADATA_LAST_EDITED_BY_ID] = $messageData['lastEditActorId']; + } + if (!empty($messageData['lastEditTimestamp'])) { + $metaData[Message::METADATA_LAST_EDITED_TIME] = $messageData['lastEditTimestamp']; + } + if (!empty($messageData['silent'])) { + $metaData[Message::METADATA_SILENT] = $messageData['silent']; + } + $proxy->setMetaData(json_encode($metaData)); + + try { + $this->mapper->insert($proxy); + } 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; + } + + $proxy = $this->mapper->findByRemote( + $room->getRemoteServer(), + $room->getRemoteToken(), + $messageData['id'], + ); + } + + return $proxy; + } +} diff --git a/lib/Service/ReminderService.php b/lib/Service/ReminderService.php index 4d3f66c09..13396b3b8 100644 --- a/lib/Service/ReminderService.php +++ b/lib/Service/ReminderService.php @@ -30,7 +30,6 @@ use OCA\Talk\AppInfo\Application; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Manager; use OCA\Talk\Model\ProxyCacheMessage; -use OCA\Talk\Model\ProxyCacheMessageMapper; use OCA\Talk\Model\Reminder; use OCA\Talk\Model\ReminderMapper; use OCP\AppFramework\Db\DoesNotExistException; @@ -41,7 +40,7 @@ class ReminderService { protected IManager $notificationManager, protected ReminderMapper $reminderMapper, protected ChatManager $chatManager, - protected ProxyCacheMessageMapper $proxyCacheMapper, + protected ProxyCacheMessageService $pcmService, protected Manager $manager, ) { } @@ -119,7 +118,7 @@ class ReminderService { $key = json_encode([$room->getRemoteServer(), $room->getRemoteToken(), $reminder->getMessageId()]); if (!isset($proxyMessages[$key])) { try { - $proxyMessages[$key] = $this->proxyCacheMapper->findByRemote($room->getRemoteServer(), $room->getRemoteToken(), $reminder->getMessageId()); + $proxyMessages[$key] = $this->pcmService->findByRemote($room->getRemoteServer(), $room->getRemoteToken(), $reminder->getMessageId()); } catch (DoesNotExistException) { } } @@ -142,7 +141,7 @@ class ReminderService { $key = json_encode([$room->getRemoteServer(), $room->getRemoteToken(), $reminder->getMessageId()]); $messageList = $proxyMessages; $messageParameters = [ - 'proxyId' => $reminder->getMessageId(), + 'proxyId' => $messageList[$key]?->getId(), ]; } diff --git a/lib/Service/RoomFormatter.php b/lib/Service/RoomFormatter.php index 1f6fee7b7..f92d1413a 100644 --- a/lib/Service/RoomFormatter.php +++ b/lib/Service/RoomFormatter.php @@ -31,7 +31,6 @@ use OCA\Talk\Config; use OCA\Talk\Federation\Proxy\TalkV1\UserConverter; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\BreakoutRoom; -use OCA\Talk\Model\ProxyCacheMessageMapper; use OCA\Talk\Model\Session; use OCA\Talk\Participant; use OCA\Talk\ResponseDefinitions; @@ -64,7 +63,7 @@ class RoomFormatter { protected IAppManager $appManager, protected IManager $userStatusManager, protected IUserManager $userManager, - protected ProxyCacheMessageMapper $proxyCacheMessageMapper, + protected ProxyCacheMessageService $pcmService, protected UserConverter $userConverter, protected IL10N $l10n, protected ?string $userId, @@ -390,7 +389,7 @@ class RoomFormatter { } elseif ($room->getRemoteServer() !== '') { $roomData['lastCommonReadMessage'] = 0; try { - $cachedMessage = $this->proxyCacheMessageMapper->findByRemote( + $cachedMessage = $this->pcmService->findByRemote( $room->getRemoteServer(), $room->getRemoteToken(), $room->getLastMessageId(), diff --git a/openapi-full.json b/openapi-full.json index cec04ca57..d65693ed2 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -6108,7 +6108,14 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } } } } @@ -6227,7 +6234,14 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } } } } @@ -6316,7 +6330,14 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } } } } @@ -6344,7 +6365,14 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } } } } diff --git a/openapi.json b/openapi.json index 93fb28bc7..f1cc7f89f 100644 --- a/openapi.json +++ b/openapi.json @@ -5995,7 +5995,14 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } } } } @@ -6114,7 +6121,14 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } } } |