Skip to content

Commit

Permalink
fix(federation): Start using metaData for notifications
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
nickvergessen committed Mar 5, 2024
1 parent 43f0b4c commit e38d282
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 37 deletions.
38 changes: 25 additions & 13 deletions lib/Federation/CloudFederationProviderTalk.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@
use OCA\Talk\Model\AttendeeMapper;
use OCA\Talk\Model\Invitation;
use OCA\Talk\Model\InvitationMapper;
use OCA\Talk\Model\ProxyCacheMessages;
use OCA\Talk\Model\ProxyCacheMessagesMapper;
use OCA\Talk\Model\Message;
use OCA\Talk\Model\ProxyCacheMessage;
use OCA\Talk\Model\ProxyCacheMessageMapper;
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\ParticipantService;
Expand Down Expand Up @@ -89,7 +90,7 @@ public function __construct(
private ISession $session,
private IEventDispatcher $dispatcher,
private LoggerInterface $logger,
private ProxyCacheMessagesMapper $proxyCacheMessagesMapper,
private ProxyCacheMessageMapper $proxyCacheMessagesMapper,
ICacheFactory $cacheFactory,
) {
$this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed('talk/pcm/') : null;
Expand Down Expand Up @@ -337,7 +338,7 @@ private function messagePosted(int $remoteAttendeeId, array $notification): arra
throw new ShareNotFound();
}

$message = new ProxyCacheMessages();
$message = new ProxyCacheMessage();
$message->setLocalToken($room->getToken());
$message->setRemoteServerUrl($notification['remoteServerUrl']);
$message->setRemoteToken($notification['remoteToken']);
Expand Down Expand Up @@ -398,19 +399,30 @@ private function messagePosted(int $remoteAttendeeId, array $notification): arra
$notification['unreadInfo']['unreadMentionDirect'],
);

$metaData = json_decode($notification['messageData']['metaData'] ?? '', true, flags: JSON_THROW_ON_ERROR);

Check failure on line 402 in lib/Federation/CloudFederationProviderTalk.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidArrayOffset

lib/Federation/CloudFederationProviderTalk.php:402:27: InvalidArrayOffset: Cannot access value on variable $notification['messageData'] using offset value of 'metaData', expecting 'remoteMessageId', 'actorType', 'actorId', 'actorDisplayName', 'messageType', 'systemMessage', 'expirationDatetime', 'message' or 'messageParameter' (see https://psalm.dev/115)

if (isset($metaData[Message::METADATA_SILENT])) {
// Silent message, skip notification handling
return [];
}

// Also notify default participants in one-to-one chats or when the admin default is "always"
$defaultLevel = $this->appConfig->getAppValueInt('default_group_notification', Participant::NOTIFY_MENTION);
if ($participant->getAttendee()->getNotificationLevel() === Participant::NOTIFY_MENTION
|| ($defaultLevel !== Participant::NOTIFY_NEVER && $participant->getAttendee()->getNotificationLevel() === Participant::NOTIFY_DEFAULT)) {
// FIXME Check if actually mentioned OR reply
$notification = $this->createNotification($room, $message, 'mention');
$notification->setUser($participant->getAttendee()->getActorId());
$this->notificationManager->notify($notification);
// FIXME Check if actually mentioned OR replyTo is "us"
// FIXME transform federated users to local users
if ($metaData[ProxyCacheMessage::METADATA_REPLYTO_TYPE] === $participant->getAttendee()->getActorType()
&& $metaData[ProxyCacheMessage::METADATA_REPLYTO_ID] === $participant->getAttendee()->getActorType()) {
$n = $this->createNotification($room, $message, 'mention');
$n->setUser($participant->getAttendee()->getActorId());
$this->notificationManager->notify($n);
}
} elseif ($participant->getAttendee()->getNotificationLevel() === Participant::NOTIFY_ALWAYS
|| ($defaultLevel === Participant::NOTIFY_ALWAYS && $participant->getAttendee()->getNotificationLevel() === Participant::NOTIFY_DEFAULT)) {
$notification = $this->createNotification($room, $message, 'chat');
$notification->setUser($participant->getAttendee()->getActorId());
$this->notificationManager->notify($notification);
$n = $this->createNotification($room, $message, 'chat');
$n->setUser($participant->getAttendee()->getActorId());
$this->notificationManager->notify($n);
}

return [];
Expand All @@ -419,7 +431,7 @@ private function messagePosted(int $remoteAttendeeId, array $notification): arra
/**
* Creates a notification for the given proxy message and mentioned users
*/
private function createNotification(Room $chat, ProxyCacheMessages $message, string $subject, array $subjectData = [], ?IComment $reaction = null): INotification {
private function createNotification(Room $chat, ProxyCacheMessage $message, string $subject, array $subjectData = [], ?IComment $reaction = null): INotification {
$subjectData['userType'] = $reaction ? $reaction->getActorType() : $message->getActorType();
$subjectData['userId'] = $reaction ? $reaction->getActorId() : $message->getActorId();

Expand All @@ -432,7 +444,7 @@ private function createNotification(Room $chat, ProxyCacheMessages $message, str
'proxyId' => $message->getId(),
// FIXME Store more info to allow querying remote?
])
->setDateTime($reaction ? $reaction->getCreationDateTime() : new \DateTime());
->setDateTime($reaction ? $reaction->getCreationDateTime() : $message->getCreationDatetime());

return $notification;
}
Expand Down
5 changes: 3 additions & 2 deletions lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use OCA\Talk\Events\SystemMessagesMultipleSentEvent;
use OCA\Talk\Federation\BackendNotifier;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\ProxyCacheMessage;
use OCA\Talk\Service\ParticipantService;
use OCP\Comments\IComment;
use OCP\EventDispatcher\Event;
Expand Down Expand Up @@ -84,8 +85,8 @@ public function handle(Event $event): void {
$metaData = $event->getComment()->getMetaData() ?? [];
$parent = $event->getParent();
if ($parent instanceof IComment) {
$metaData['replyToActorType'] = $parent->getActorType();
$metaData['replyToActorId'] = $parent->getActorId();
$metaData[ProxyCacheMessage::METADATA_REPLYTO_TYPE] = $parent->getActorType();
$metaData[ProxyCacheMessage::METADATA_REPLYTO_ID] = $parent->getActorId();
}

$messageData = [
Expand Down
10 changes: 5 additions & 5 deletions lib/Model/Message.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ class Message {
protected $lastEditTimestamp = 0;

public function __construct(
protected Room $room,
protected ?Participant $participant,
protected ?IComment $comment,
protected IL10N $l,
protected ?ProxyCacheMessages $proxy = null,
protected Room $room,
protected ?Participant $participant,
protected ?IComment $comment,
protected IL10N $l,
protected ?ProxyCacheMessage $proxy = null,
) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
declare(strict_types=1);

/**
* @copyright Copyright (c) 2022 Joas Schilling <[email protected]>
* @copyright Copyright (c) 2024 Joas Schilling <[email protected]>
*
* @author Joas Schilling <[email protected]>
*
Expand Down Expand Up @@ -54,10 +54,17 @@
* @method string|null getMessage()
* @method void setMessageParameters(?string $messageParameters)
* @method string|null getMessageParameters()
* @method void setCreationDatetime(?\DateTimeImmutable $creationDatetime)
* @method \DateTimeImmutable|null getCreationDatetime()
* @method void setMetaData(?string $metaData)
* @method string|null getMetaData()
*
* @psalm-import-type TalkRoomProxyMessage from ResponseDefinitions
*/
class ProxyCacheMessages extends Entity implements \JsonSerializable {
class ProxyCacheMessage extends Entity implements \JsonSerializable {
public const METADATA_REPLYTO_TYPE = 'replyToActorType';
public const METADATA_REPLYTO_ID = 'replyToActorId';


protected string $localToken = '';
protected string $remoteServerUrl = '';
Expand All @@ -71,6 +78,8 @@ class ProxyCacheMessages extends Entity implements \JsonSerializable {
protected ?\DateTimeImmutable $expirationDatetime = null;
protected ?string $message = null;
protected ?string $messageParameters = null;
protected ?\DateTimeImmutable $creationDatetime = null;
protected ?string $metaData = null;

public function __construct() {
$this->addType('localToken', 'string');
Expand All @@ -85,16 +94,18 @@ public function __construct() {
$this->addType('expirationDatetime', 'datetime');
$this->addType('message', 'string');
$this->addType('messageParameters', 'string');
// Reply author
// Silent
// Creation date
// Verb?!
$this->addType('creationDatetime', 'datetime');
$this->addType('metaData', 'string');
}

public function getParsedMessageParameters(): array {
return json_decode($this->getMessageParameters() ?? '[]', true);
}

public function getParsedMetaData(): array {
return json_decode($this->getMetaData() ?? '[]', true);
}

/**
* @return TalkRoomProxyMessage
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,24 @@
use OCP\IDBConnection;

/**
* @method ProxyCacheMessages mapRowToEntity(array $row)
* @method ProxyCacheMessages findEntity(IQueryBuilder $query)
* @method ProxyCacheMessages[] findEntities(IQueryBuilder $query)
* @template-extends QBMapper<ProxyCacheMessages>
* @method ProxyCacheMessage mapRowToEntity(array $row)
* @method ProxyCacheMessage findEntity(IQueryBuilder $query)
* @method ProxyCacheMessage[] findEntities(IQueryBuilder $query)
* @template-extends QBMapper<ProxyCacheMessage>
*/
class ProxyCacheMessagesMapper extends QBMapper {
class ProxyCacheMessageMapper extends QBMapper {
use TTransactional;

public function __construct(
IDBConnection $db,
) {
parent::__construct($db, 'talk_proxy_messages', ProxyCacheMessages::class);
parent::__construct($db, 'talk_proxy_messages', ProxyCacheMessage::class);
}

/**
* @throws DoesNotExistException
*/
public function findById(int $proxyId): ProxyCacheMessages {
public function findById(int $proxyId): ProxyCacheMessage {
$query = $this->db->getQueryBuilder();
$query->select('*')
->from($this->getTableName())
Expand All @@ -61,7 +61,7 @@ public function findById(int $proxyId): ProxyCacheMessages {
/**
* @throws DoesNotExistException
*/
public function findByRemote(string $remoteServerUrl, string $remoteToken, int $remoteMessageId): ProxyCacheMessages {
public function findByRemote(string $remoteServerUrl, string $remoteToken, int $remoteMessageId): ProxyCacheMessage {
$query = $this->db->getQueryBuilder();
$query->select('*')
->from($this->getTableName())
Expand Down
6 changes: 3 additions & 3 deletions tests/php/Federation/FederationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
use OCA\Talk\Model\AttendeeMapper;
use OCA\Talk\Model\Invitation;
use OCA\Talk\Model\InvitationMapper;
use OCA\Talk\Model\ProxyCacheMessagesMapper;
use OCA\Talk\Model\ProxyCacheMessageMapper;
use OCA\Talk\Room;
use OCA\Talk\Service\ParticipantService;
use OCA\Talk\Service\RoomService;
Expand Down Expand Up @@ -95,7 +95,7 @@ class FederationTest extends TestCase {
/** @var AttendeeMapper|MockObject */
protected $attendeeMapper;

protected ProxyCacheMessagesMapper|MockObject $proxyCacheMessageMapper;
protected ProxyCacheMessageMapper|MockObject $proxyCacheMessageMapper;
protected ICacheFactory|MockObject $cacheFactory;

public function setUp(): void {
Expand All @@ -112,7 +112,7 @@ public function setUp(): void {
$this->appManager = $this->createMock(IAppManager::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->url = $this->createMock(IURLGenerator::class);
$this->proxyCacheMessageMapper = $this->createMock(ProxyCacheMessagesMapper::class);
$this->proxyCacheMessageMapper = $this->createMock(ProxyCacheMessageMapper::class);
$this->cacheFactory = $this->createMock(ICacheFactory::class);

$this->backendNotifier = new BackendNotifier(
Expand Down

0 comments on commit e38d282

Please sign in to comment.