fix(federation): Fix unread marker handling in federated chats

Signed-off-by: Joas Schilling <coding@schilljs.com>

appinfo/info.xml

@@ -16,7 +16,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m
 
 	]]></description>
 
-	<version>19.0.0-beta.1.1</version>
+	<version>19.0.0-beta.1.2</version>
 	<licence>agpl</licence>
 
 	<author>Daniel Calviño Sánchez</author>

lib/Federation/BackendNotifier.php

@@ -292,7 +292,7 @@ public function sendRoomModifiedUpdate(
 	 * Sent from Host server to Remote participant server
 	 *
 	 * @param array{remoteMessageId: int, actorType: string, actorId: string, actorDisplayName: string, messageType: string, systemMessage: string, expirationDatetime: string, message: string, messageParameter: string, creationDatetime: string, metaData: string} $messageData
-	 * @param array{unreadMessages: int, unreadMention: bool, unreadMentionDirect: bool} $unreadInfo
+	 * @param array{unreadMessages: int, unreadMention: bool, unreadMentionDirect: bool, lastReadMessage: int} $unreadInfo
 	 */
 	public function sendMessageUpdate(
 		string $remoteServer,

lib/Federation/CloudFederationProviderTalk.php

@@ -215,6 +215,7 @@ private function shareAccepted(int $id, array $notification): array {
 
 		if (!empty($notification['displayName'])) {
 			$attendee->setDisplayName($notification['displayName']);
+			$attendee->setState(Invitation::STATE_ACCEPTED);
 			$this->attendeeMapper->update($attendee);
 		}
 
@@ -325,7 +326,7 @@ private function roomModified(int $remoteAttendeeId, array $notification): array
 
 	/**
 	 * @param int $remoteAttendeeId
-	 * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, messageData: array{remoteMessageId: int, actorType: string, actorId: string, actorDisplayName: string, messageType: string, systemMessage: string, expirationDatetime: string, message: string, messageParameter: string, creationDatetime: string, metaData: string}, unreadInfo: array{unreadMessages: int, unreadMention: bool, unreadMentionDirect: bool}} $notification
+	 * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, messageData: array{remoteMessageId: int, actorType: string, actorId: string, actorDisplayName: string, messageType: string, systemMessage: string, expirationDatetime: string, message: string, messageParameter: string, creationDatetime: string, metaData: string}, unreadInfo: array{unreadMessages: int, unreadMention: bool, unreadMentionDirect: bool, lastReadMessage: int}} $notification
 	 * @return array
 	 * @throws ActionNotSupportedException
 	 * @throws AuthenticationFailedException
@@ -416,6 +417,7 @@ private function messagePosted(int $remoteAttendeeId, array $notification): arra
 			$notification['unreadInfo']['unreadMessages'],
 			$notification['unreadInfo']['unreadMention'],
 			$notification['unreadInfo']['unreadMentionDirect'],
+			$notification['unreadInfo']['lastReadMessage'],
 		);
 
 		$this->federationChatNotifier->handleChatMessage($room, $participant, $message, $notification);

lib/Federation/FederationManager.php

@@ -147,6 +147,7 @@ public function acceptRemoteRoomShare(IUser $user, int $shareId): Participant {
 				'accessToken' => $invitation->getAccessToken(),
 				'remoteId' => $invitation->getRemoteAttendeeId(),
 				'invitedCloudId' => $invitation->getLocalCloudId(),
+				'lastReadMessage' => $room->getLastMessageId(),
 			]
 		];
 		$attendees = $this->participantService->addUsers($room, $participant, $user);

lib/Federation/Proxy/TalkV1/Controller/ChatController.php

@@ -173,7 +173,12 @@ public function receiveMessages(
 		);
 
 		if ($lookIntoFuture && $setReadMarker) {
-			$this->participantService->updateUnreadInfoForProxyParticipant($participant, 0, false, false);
+			$this->participantService->updateUnreadInfoForProxyParticipant($participant,
+				0,
+				false,
+				false,
+				(int) ($proxy->getHeader('X-Chat-Last-Given') ?: $lastKnownMessageId),
+			);
 		}
 
 		if ($proxy->getStatusCode() === Http::STATUS_NOT_MODIFIED) {
@@ -384,6 +389,7 @@ public function setReadMarker(Room $room, Participant $participant, string $resp
 			$data['unreadMessages'],
 			$data['unreadMention'],
 			$data['unreadMentionDirect'],
+			$data['lastReadMessage'],
 		);
 
 		$headers = $lastCommonRead = [];
@@ -424,6 +430,7 @@ public function markUnread(Room $room, Participant $participant, string $respons
 			$data['unreadMessages'],
 			$data['unreadMention'],
 			$data['unreadMentionDirect'],
+			$data['lastReadMessage'],
 		);
 
 		$headers = $lastCommonRead = [];

lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php

@@ -114,9 +114,10 @@ public function handle(Event $event): void {
 			$lastMentionDirect = $attendee->getLastMentionDirect();
 
 			$unreadInfo = [
+				'lastReadMessage' => $lastReadMessage,
 				'unreadMessages' => $this->chatManager->getUnreadCount($event->getRoom(), $lastReadMessage),
 				'unreadMention' => $lastMention !== 0 && $lastReadMessage < $lastMention,
-				'unreadMentionDirect' => $lastMentionDirect !== 0 && $lastReadMessage < $lastMentionDirect
+				'unreadMentionDirect' => $lastMentionDirect !== 0 && $lastReadMessage < $lastMentionDirect,
 			];
 
 			$success = $this->backendNotifier->sendMessageUpdate(

lib/Model/Attendee.php

@@ -66,6 +66,10 @@
  * @method null|string getPhoneNumber()
  * @method void setCallId(?string $callId)
  * @method null|string getCallId()
+ * @method void setState(int $state)
+ * @method int getState()
+ * @method void setUnreadMessages(int $unreadMessages)
+ * @method int getUnreadMessages()
  */
 class Attendee extends Entity {
 	public const ACTOR_USERS = 'users';
@@ -168,6 +172,12 @@ class Attendee extends Entity {
 	/** @var null|string */
 	protected $callId;
 
+	/** @var int */
+	protected $state;
+
+	/** @var int */
+	protected $unreadMessages;
+
 	public function __construct() {
 		$this->addType('roomId', 'int');
 		$this->addType('actorType', 'string');
@@ -189,6 +199,8 @@ public function __construct() {
 		$this->addType('invitedCloudId', 'string');
 		$this->addType('phoneNumber', 'string');
 		$this->addType('callId', 'string');
+		$this->addType('state', 'int');
+		$this->addType('unreadMessages', 'int');
 	}
 
 	public function getDisplayName(): string {

lib/Model/AttendeeMapper.php

@@ -299,6 +299,8 @@ public function createAttendeeFromRow(array $row): Attendee {
 			'invited_cloud_id' => (string) $row['invited_cloud_id'],
 			'phone_number' => $row['phone_number'],
 			'call_id' => $row['call_id'],
+			'state' => (int) $row['state'],
+			'unread_messages' => (int) $row['unread_messages'],
 		]);
 	}
 }

lib/Model/SelectHelper.php

@@ -87,6 +87,8 @@ public function selectAttendeesTable(IQueryBuilder $query, string $alias = 'a'):
 			->addSelect($alias . 'invited_cloud_id')
 			->addSelect($alias . 'phone_number')
 			->addSelect($alias . 'call_id')
+			->addSelect($alias . 'state')
+			->addSelect($alias . 'unread_messages')
 			->selectAlias($alias . 'id', 'a_id');
 	}
 

lib/Notification/FederationChatNotifier.php

@@ -45,7 +45,7 @@ public function __construct(
 	}
 
 	/**
-	 * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, messageData: array{remoteMessageId: int, actorType: string, actorId: string, actorDisplayName: string, messageType: string, systemMessage: string, expirationDatetime: string, message: string, messageParameter: string, creationDatetime: string, metaData: string}, unreadInfo: array{unreadMessages: int, unreadMention: bool, unreadMentionDirect: bool}} $inboundNotification
+	 * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, messageData: array{remoteMessageId: int, actorType: string, actorId: string, actorDisplayName: string, messageType: string, systemMessage: string, expirationDatetime: string, message: string, messageParameter: string, creationDatetime: string, metaData: string}, unreadInfo: array{unreadMessages: int, unreadMention: bool, unreadMentionDirect: bool, lastReadMessage: int}} $inboundNotification
 	 */
 	public function handleChatMessage(Room $room, Participant $participant, ProxyCacheMessage $message, array $inboundNotification): void {
 		/** @var array{silent?: bool, last_edited_time?: int, last_edited_by_type?: string, last_edited_by_id?: string, replyToActorType?: string, replyToActorId?: string} $metaData */

lib/Service/ParticipantService.php

@@ -249,9 +249,10 @@ public function updateLastReadMessage(Participant $participant, int $lastReadMes
 		$this->attendeeMapper->update($attendee);
 	}
 
-	public function updateUnreadInfoForProxyParticipant(Participant $participant, int $unreadMessageCount, bool $hasMention, bool $hadDirectMention): void {
+	public function updateUnreadInfoForProxyParticipant(Participant $participant, int $unreadMessageCount, bool $hasMention, bool $hadDirectMention, int $lastReadMessageId): void {
 		$attendee = $participant->getAttendee();
-		$attendee->setLastReadMessage($unreadMessageCount);
+		$attendee->setUnreadMessages($unreadMessageCount);
+		$attendee->setLastReadMessage($lastReadMessageId);
 		$attendee->setLastMentionMessage($hasMention ? 1 : 0);
 		$attendee->setLastMentionDirect($hadDirectMention ? 1 : 0);
 		$this->attendeeMapper->update($attendee);
@@ -505,7 +506,7 @@ public function addUsers(Room $room, array $participants, ?IUser $addedBy = null
 			}
 			$attendee->setParticipantType($participant['participantType'] ?? Participant::USER);
 			$attendee->setPermissions(Attendee::PERMISSIONS_DEFAULT);
-			$attendee->setLastReadMessage($lastMessage);
+			$attendee->setLastReadMessage($participant['lastReadMessage'] ?? $lastMessage);
 			$attendee->setReadPrivacy($readPrivacy);
 			$attendees[] = $attendee;
 		}

lib/Service/RoomFormatter.php

@@ -292,12 +292,11 @@ public function formatRoomV4(
 
 		if ($attendee->getActorType() === Attendee::ACTOR_USERS) {
 			$currentUser = $this->userManager->get($attendee->getActorId());
-			if ($room->getRemoteServer() !== '') {
-				// For proxy conversations the information is the real counter,
-				// not the message ID requiring math afterward.
-				$roomData['unreadMessages'] = $attendee->getLastReadMessage();
-				$roomData['unreadMention'] = (bool) $attendee->getLastMentionMessage();
-				$roomData['unreadMentionDirect'] = (bool) $attendee->getLastMentionDirect();
+			if ($room->isFederatedConversation()) {
+				$roomData['lastReadMessage'] = $attendee->getLastReadMessage();
+				$roomData['unreadMention'] = (bool)$attendee->getLastMentionMessage();
+				$roomData['unreadMentionDirect'] = (bool)$attendee->getLastMentionDirect();
+				$roomData['unreadMessages'] = $attendee->getUnreadMessages();
 			} elseif ($currentUser instanceof IUser) {
 				$lastReadMessage = $attendee->getLastReadMessage();
 				if ($lastReadMessage === -1) {
@@ -338,6 +337,7 @@ public function formatRoomV4(
 			$lastReadMessage = $attendee->getLastReadMessage();
 			$lastMention = $attendee->getLastMentionMessage();
 			$lastMentionDirect = $attendee->getLastMentionDirect();
+			$roomData['lastReadMessage'] = $lastReadMessage;
 			$roomData['unreadMessages'] = $this->chatManager->getUnreadCount($room, $lastReadMessage);
 			$roomData['unreadMention'] = $lastMention !== 0 && $lastReadMessage < $lastMention;
 			$roomData['unreadMentionDirect'] = $lastMentionDirect !== 0 && $lastReadMessage < $lastMentionDirect;
@@ -388,6 +388,7 @@ public function formatRoomV4(
 				$lastMessage,
 			);
 		} elseif ($room->getRemoteServer() !== '') {
+			$roomData['lastCommonReadMessage'] = 0;
 			try {
 				$cachedMessage = $this->proxyCacheMessageMapper->findByRemote(
 					$room->getRemoteServer(),

tests/php/Chat/ChatManagerTest.php

@@ -29,6 +29,7 @@
 use OCA\Talk\Chat\Notifier;
 use OCA\Talk\Model\Attendee;
 use OCA\Talk\Model\AttendeeMapper;
+use OCA\Talk\Model\Invitation;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\AttachmentService;
@@ -446,6 +447,8 @@ public function testDeleteMessage(): void {
 			'phone_number' => '',
 			'call_id' => '',
 			'invited_cloud_id' => '',
+			'state' => Invitation::STATE_ACCEPTED,
+			'unread_messages' => 0,
 		]);
 		$chat = $this->createMock(Room::class);
 		$chat->expects($this->any())
@@ -505,6 +508,8 @@ public function testDeleteMessageFileShare(): void {
 			'phone_number' => '',
 			'call_id' => '',
 			'invited_cloud_id' => '',
+			'state' => Invitation::STATE_ACCEPTED,
+			'unread_messages' => 0,
 		]);
 		$chat = $this->createMock(Room::class);
 		$chat->expects($this->any())
@@ -586,6 +591,8 @@ public function testDeleteMessageFileShareNotFound(): void {
 			'phone_number' => '',
 			'call_id' => '',
 			'invited_cloud_id' => '',
+			'state' => Invitation::STATE_ACCEPTED,
+			'unread_messages' => 0,
 		]);
 		$chat = $this->createMock(Room::class);
 		$chat->expects($this->any())