Skip to content

Commit

Permalink
Merge pull request #11793 from nextcloud/bugfix/noid/have-more-data
Browse files Browse the repository at this point in the history
fix(federation): Send display name and last read message with federation
  • Loading branch information
nickvergessen authored Mar 14, 2024
2 parents 2e947ea + 30215a7 commit ad344d3
Show file tree
Hide file tree
Showing 21 changed files with 224 additions and 27 deletions.
2 changes: 1 addition & 1 deletion appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 3 additions & 1 deletion lib/Federation/BackendNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ public function sendShareAccepted(
int $remoteAttendeeId,
#[SensitiveParameter]
string $accessToken,
string $displayName,
): bool {
$remote = $this->prepareRemoteUrl($remoteServerUrl);

Expand All @@ -191,6 +192,7 @@ public function sendShareAccepted(
'remoteServerUrl' => $this->getServerRemoteUrl(),
'sharedSecret' => $accessToken,
'message' => 'Recipient accepted the share',
'displayName' => $displayName,
]
);

Expand Down Expand Up @@ -290,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,
Expand Down
9 changes: 8 additions & 1 deletion lib/Federation/CloudFederationProviderTalk.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ public function notificationReceived($notificationType, $providerId, array $noti
private function shareAccepted(int $id, array $notification): array {
$attendee = $this->getLocalAttendeeAndValidate($id, $notification['sharedSecret']);

if (!empty($notification['displayName'])) {
$attendee->setDisplayName($notification['displayName']);
$attendee->setState(Invitation::STATE_ACCEPTED);
$this->attendeeMapper->update($attendee);
}

$this->session->set('talk-overwrite-actor-type', $attendee->getActorType());
$this->session->set('talk-overwrite-actor-id', $attendee->getActorId());
$this->session->set('talk-overwrite-actor-displayname', $attendee->getDisplayName());
Expand Down Expand Up @@ -320,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
Expand Down Expand Up @@ -411,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);
Expand Down
3 changes: 2 additions & 1 deletion lib/Federation/FederationManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public function acceptRemoteRoomShare(IUser $user, int $shareId): Participant {
// Add user to the room
$room = $this->manager->getRoomById($invitation->getLocalRoomId());
if (
!$this->backendNotifier->sendShareAccepted($invitation->getRemoteServerUrl(), $invitation->getRemoteAttendeeId(), $invitation->getAccessToken())
!$this->backendNotifier->sendShareAccepted($invitation->getRemoteServerUrl(), $invitation->getRemoteAttendeeId(), $invitation->getAccessToken(), $user->getDisplayName())
) {
throw new CannotReachRemoteException();
}
Expand All @@ -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);
Expand Down
9 changes: 8 additions & 1 deletion lib/Federation/Proxy/TalkV1/Controller/ChatController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -384,6 +389,7 @@ public function setReadMarker(Room $room, Participant $participant, string $resp
$data['unreadMessages'],
$data['unreadMention'],
$data['unreadMentionDirect'],
$data['lastReadMessage'],
);

$headers = $lastCommonRead = [];
Expand Down Expand Up @@ -424,6 +430,7 @@ public function markUnread(Room $room, Participant $participant, string $respons
$data['unreadMessages'],
$data['unreadMention'],
$data['unreadMentionDirect'],
$data['lastReadMessage'],
);

$headers = $lastCommonRead = [];
Expand Down
3 changes: 2 additions & 1 deletion lib/Federation/Proxy/TalkV1/Notifier/MessageSentListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions lib/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public function createRoomObjectFromData(array $data): Room {
'breakout_room_status' => 0,
'call_recording' => 0,
'recording_consent' => 0,
'has_federation' => 0,
], $data));
}

Expand Down Expand Up @@ -202,6 +203,7 @@ public function createRoomObject(array $row): Room {
(int) $row['breakout_room_status'],
(int) $row['call_recording'],
(int) $row['recording_consent'],
(int) $row['has_federation'],
);
}

Expand Down
102 changes: 102 additions & 0 deletions lib/Migration/Version19000Date20240313134926.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?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\Migration;

use Closure;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\Invitation;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Cache the invite state in the attendees and room table to allow reducing efforts
*/
class Version19000Date20240313134926 extends SimpleMigrationStep {
public function __construct(
protected IDBConnection $connection,
) {
}

/**
* @param IOutput $output
* @param Closure(): ISchemaWrapper $schemaClosure
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('talk_attendees');
$table->addColumn('state', Types::SMALLINT, [
'default' => 0,
'unsigned' => true,
]);
$table->addColumn('unread_messages', Types::BIGINT, [
'default' => 0,
'unsigned' => true,
]);

$table = $schema->getTable('talk_rooms');
$table->addColumn('has_federation', Types::SMALLINT, [
'default' => 0,
'unsigned' => true,
]);

return $schema;
}

/**
* Set the invitation state to accepted for existing federated users
* Set the "has federation" for rooms with TalkV1 users
*/
public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
$query = $this->connection->getQueryBuilder();
$query->update('talk_attendees')
->set('state', $query->createNamedParameter(Invitation::STATE_ACCEPTED))
->where($query->expr()->eq('actor_type', $query->createNamedParameter(Attendee::ACTOR_FEDERATED_USERS)));
$query->executeStatement();

$query = $this->connection->getQueryBuilder();
$subQuery = $this->connection->getQueryBuilder();
$subQuery->select('room_id')
->from('talk_attendees')
->where($subQuery->expr()->eq('actor_type', $query->createNamedParameter(Attendee::ACTOR_FEDERATED_USERS)))
->groupBy('room_id');

$query = $this->connection->getQueryBuilder();
$query->update('talk_rooms')
// Don't use const Room::HAS_FEDERATION_TALKv1 because the file might have been loaded with old content before the migration
// ->set('has_federation', $query->createNamedParameter(Room::HAS_FEDERATION_TALKv1))
->set('has_federation', $query->createNamedParameter(1))
->where($query->expr()->in('id', $query->createFunction($subQuery->getSQL())));
$query->executeStatement();
}
}
12 changes: 12 additions & 0 deletions lib/Model/Attendee.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions lib/Model/AttendeeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
]);
}
}
3 changes: 3 additions & 0 deletions lib/Model/SelectHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public function selectRoomsTable(IQueryBuilder $query, string $alias = 'r'): voi
->addSelect($alias . 'breakout_room_status')
->addSelect($alias . 'call_recording')
->addSelect($alias . 'recording_consent')
->addSelect($alias . 'has_federation')
->selectAlias($alias . 'id', 'r_id');
}

Expand Down Expand Up @@ -87,6 +88,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');
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Notification/FederationChatNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
23 changes: 23 additions & 0 deletions lib/Room.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,16 @@ class Room {

public const DESCRIPTION_MAXIMUM_LENGTH = 500;

public const HAS_FEDERATION_NONE = 0;
public const HAS_FEDERATION_TALKv1 = 1;

protected ?string $currentUser = null;
protected ?Participant $participant = null;

/**
* @psalm-param Room::TYPE_* $type
* @psalm-param RecordingService::CONSENT_REQUIRED_* $recordingConsent
* @psalm-param int-mask-of<self::HAS_FEDERATION_*> $hasFederation
*/
public function __construct(
private Manager $manager,
Expand Down Expand Up @@ -138,6 +142,7 @@ public function __construct(
private int $breakoutRoomStatus,
private int $callRecording,
private int $recordingConsent,
private int $hasFederation,
) {
}

Expand Down Expand Up @@ -410,6 +415,9 @@ public function getRemoteToken(): string {
return $this->remoteToken;
}

/**
* @deprecated
*/
public function isFederatedRemoteRoom(): bool {
return $this->remoteServer !== '';
}
Expand Down Expand Up @@ -558,4 +566,19 @@ public function getRecordingConsent(): int {
public function setRecordingConsent(int $recordingConsent): void {
$this->recordingConsent = $recordingConsent;
}

/**
* @psalm-return int-mask-of<self::HAS_FEDERATION_*>
*/
public function hasFederatedParticipants(): int {
return $this->hasFederation;
}

/**
* @param int $hasFederation
* @psalm-param int-mask-of<self::HAS_FEDERATION_*> $hasFederation (bit map)
*/
public function setFederatedParticipants(int $hasFederation): void {
$this->hasFederation = $hasFederation;
}
}
Loading

0 comments on commit ad344d3

Please sign in to comment.