Skip to content

Commit

Permalink
fix(federation): Cache whether we have federated participants in a co…
Browse files Browse the repository at this point in the history
…nversation

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Mar 14, 2024
1 parent ec2ce3b commit 30215a7
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 0 deletions.
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();
}
}
1 change: 1 addition & 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
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;
}
}
11 changes: 11 additions & 0 deletions lib/Service/ParticipantService.php
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ public function addUsers(Room $room, array $participants, ?IUser $addedBy = null
$event = new BeforeAttendeesAddedEvent($room, $attendees);
$this->dispatcher->dispatchTyped($event);

$setFederationFlagAlready = $room->hasFederatedParticipants() & Room::HAS_FEDERATION_TALKv1;
foreach ($attendees as $attendee) {
try {
$this->attendeeMapper->insert($attendee);
Expand All @@ -524,6 +525,16 @@ public function addUsers(Room $room, array $participants, ?IUser $addedBy = null
$this->attendeeMapper->delete($attendee);
throw new CannotReachRemoteException();
}

if (!$setFederationFlagAlready) {
$flag = $room->hasFederatedParticipants() | Room::HAS_FEDERATION_TALKv1;

/** @var RoomService $roomService */
$roomService = Server::get(RoomService::class);
$roomService->setHasFederation($room, $flag);

$setFederationFlagAlready = true;
}
}
} catch (Exception $e) {
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
Expand Down
13 changes: 13 additions & 0 deletions lib/Service/RoomService.php
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,19 @@ public function setLastMessageInfo(Room $room, int $messageId, \DateTime $dateTi
$room->setLastActivity($dateTime);
}

/**
* @psalm-param int-mask-of<Room::HAS_FEDERATION_*> $hasFederation
*/
public function setHasFederation(Room $room, int $hasFederation): void {
$update = $this->db->getQueryBuilder();
$update->update('talk_rooms')
->set('has_federation', $update->createNamedParameter($hasFederation, IQueryBuilder::PARAM_INT))
->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
$update->executeStatement();

$room->setFederatedParticipants($hasFederation);
}

public function setLastActivity(Room $room, \DateTime $now): void {
$update = $this->db->getQueryBuilder();
$update->update('talk_rooms')
Expand Down
1 change: 1 addition & 0 deletions tests/php/Service/RoomServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ public function testVerifyPassword(): void {
BreakoutRoom::STATUS_STOPPED,
Room::RECORDING_NONE,
RecordingService::CONSENT_REQUIRED_NO,
Room::HAS_FEDERATION_NONE,
);

$verificationResult = $service->verifyPassword($room, '1234');
Expand Down

0 comments on commit 30215a7

Please sign in to comment.