Browse Source

feat(reminders): Implement reminders for messages not in cache

Signed-off-by: Joas Schilling <coding@schilljs.com>
pull/11814/head
Joas Schilling 2 years ago
parent
commit
99cafa2ceb
No known key found for this signature in database GPG Key ID: 74434EFE0D2E2205
  1. 35
      lib/Controller/ChatController.php
  2. 128
      lib/Service/ProxyCacheMessageService.php
  3. 7
      lib/Service/ReminderService.php
  4. 5
      lib/Service/RoomFormatter.php
  5. 36
      openapi-full.json
  6. 36
      openapi.json
  7. 16
      src/types/openapi/openapi-full.ts
  8. 16
      src/types/openapi/openapi.ts
  9. 7
      tests/integration/features/federation/chat.feature
  10. 75
      tests/integration/features/federation/reminder.feature
  11. 4
      tests/php/Controller/ChatControllerTest.php

35
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;
}

128
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;
}
}

7
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(),
];
}

5
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(),

36
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"
}
}
}
}
}
}

36
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"
}
}
}
}
}
}
@ -6203,7 +6217,14 @@
"meta": {
"$ref": "#/components/schemas/OCSMeta"
},
"data": {}
"data": {
"type": "object",
"properties": {
"error": {
"type": "string"
}
}
}
}
}
}
@ -6231,7 +6252,14 @@
"meta": {
"$ref": "#/components/schemas/OCSMeta"
},
"data": {}
"data": {
"type": "object",
"properties": {
"error": {
"type": "string"
}
}
}
}
}
}

16
src/types/openapi/openapi-full.ts

@ -2376,7 +2376,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};
@ -2419,7 +2421,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};
@ -2447,7 +2451,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};
@ -2458,7 +2464,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};

16
src/types/openapi/openapi.ts

@ -2207,7 +2207,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};
@ -2250,7 +2252,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};
@ -2278,7 +2282,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};
@ -2289,7 +2295,9 @@ export type operations = {
"application/json": {
ocs: {
meta: components["schemas"]["OCSMeta"];
data: unknown;
data: {
error?: string;
};
};
};
};

7
tests/integration/features/federation/chat.feature

@ -175,19 +175,12 @@ Feature: federation/chat
| roomType | 2 |
| roomName | room |
And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4)
And user "participant1" adds federated_user "participant3" to room "room" with 200 (v4)
And user "participant2" has the following invitations (v1)
| remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName |
| LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname |
And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1)
| id | name | type | remoteServer | remoteToken |
| room | room | 2 | LOCAL | room |
And user "participant3" has the following invitations (v1)
| remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName |
| LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname |
And user "participant3" accepts invite to room "room" of server "LOCAL" with 200 (v1)
| id | name | type | remoteServer | remoteToken |
| room | room | 2 | LOCAL | room |
Then user "participant2" is participant of the following rooms (v4)
| id | type |
| room | 2 |

75
tests/integration/features/federation/reminder.feature

@ -0,0 +1,75 @@
Feature: federation/chat
Background:
Given user "participant1" exists
Given user "participant2" exists
Given user "participant3" exists
Scenario: Get mention suggestions (translating local users to federated users)
Given the following "spreed" app config is set
| federation_enabled | yes |
Given user "participant1" creates room "room" (v4)
| roomType | 2 |
| roomName | room |
And user "participant1" sends message "Message 1" to room "room" with 201
And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4)
And user "participant2" has the following invitations (v1)
| remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName |
| LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname |
And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1)
| id | name | type | remoteServer | remoteToken |
| room | room | 2 | LOCAL | room |
Then user "participant2" is participant of the following rooms (v4)
| id | type |
| room | 2 |
And user "participant2" joins room "LOCAL::room" with 200 (v4)
And user "participant2" leaves room "LOCAL::room" with 200 (v4)
And user "participant2" sends message "Message 2" to room "LOCAL::room" with 201
When user "participant1" sets reminder for message "Message 2" in room "room" for time 2133349024 with 201 (v1)
And user "participant2" sets reminder for message "Message 1" in room "LOCAL::room" for time 1234567 with 201 (v1)
And user "participant1" has the following notifications
| app | object_type | object_id | subject |
And user "participant2" has the following notifications
| app | object_type | object_id | subject |
And force run "OCA\Talk\BackgroundJob\Reminder" background jobs
Then user "participant1" has the following notifications
| app | object_type | object_id | subject |
And user "participant2" has the following notifications
| app | object_type | object_id | subject |
| spreed | reminder | room/Message 1 | Reminder: participant1-displayname in conversation room |
# Participant1 sets timestamp to past so it should trigger now
When user "participant1" sets reminder for message "Message 2" in room "room" for time 1234567 with 201 (v1)
And force run "OCA\Talk\BackgroundJob\Reminder" background jobs
Then user "participant1" has the following notifications
| app | object_type | object_id | subject |
| spreed | reminder | room/Message 2 | Reminder: participant2-displayname in conversation room |
And user "participant2" deletes reminder for message "Message 1" in room "LOCAL::room" with 200 (v1)
And user "participant2" has the following notifications
| app | object_type | object_id | subject |
Scenario: Deleting reminder before the job is executed never triggers a notification
Given the following "spreed" app config is set
| federation_enabled | yes |
Given user "participant1" creates room "room" (v4)
| roomType | 2 |
| roomName | room |
And user "participant1" sends message "Message 1" to room "room" with 201
And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4)
And user "participant1" adds user "participant3" to room "room" with 200 (v4)
And user "participant2" has the following invitations (v1)
| remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName |
| LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname |
And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1)
| id | name | type | remoteServer | remoteToken |
| room | room | 2 | LOCAL | room |
Then user "participant2" is participant of the following rooms (v4)
| id | type |
| room | 2 |
And user "participant2" joins room "LOCAL::room" with 200 (v4)
And user "participant2" leaves room "LOCAL::room" with 200 (v4)
When user "participant2" sets reminder for message "Message 1" in room "LOCAL::room" for time 1234567 with 201 (v1)
And user "participant2" deletes reminder for message "Message 1" in room "LOCAL::room" with 200 (v1)
And user "participant2" has the following notifications
| app | object_type | object_id | subject |
And force run "OCA\Talk\BackgroundJob\Reminder" background jobs
And user "participant2" has the following notifications
| app | object_type | object_id | subject |

4
tests/php/Controller/ChatControllerTest.php

@ -39,6 +39,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;
@ -115,6 +116,7 @@ class ChatControllerTest extends TestCase {
/** @var IL10N|MockObject */
private $l;
private Authenticator|MockObject $federationAuthenticator;
private ProxyCacheMessageService|MockObject $pcmService;
/** @var Room|MockObject */
protected $room;
@ -154,6 +156,7 @@ class ChatControllerTest extends TestCase {
$this->trustedDomainHelper = $this->createMock(ITrustedDomainHelper::class);
$this->l = $this->createMock(IL10N::class);
$this->federationAuthenticator = $this->createMock(Authenticator::class);
$this->pcmService = $this->createMock(ProxyCacheMessageService::class);
$this->room = $this->createMock(Room::class);
@ -198,6 +201,7 @@ class ChatControllerTest extends TestCase {
$this->trustedDomainHelper,
$this->l,
$this->federationAuthenticator,
$this->pcmService,
);
}

Loading…
Cancel
Save