From ddeb53948e21ef7990ae98b5cf7be261e35358b9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Mar 2024 13:52:13 +0100 Subject: [PATCH] feat(federation): Expose a header with the pending invitations on room list Signed-off-by: Joas Schilling --- docs/conversation.md | 9 +++-- .../ResetAssignedSignalingServer.php | 3 +- lib/CachePrefix.php | 37 +++++++++++++++++++ lib/Chat/ChatManager.php | 5 ++- lib/Controller/RoomController.php | 31 ++++++++++++++-- .../CloudFederationProviderTalk.php | 7 +++- lib/Federation/FederationManager.php | 4 ++ .../TalkV1/Controller/ChatController.php | 3 +- lib/Model/InvitationMapper.php | 21 +++++++++++ lib/Service/ParticipantService.php | 5 ++- lib/Signaling/Manager.php | 3 +- openapi-full.json | 5 +++ openapi.json | 5 +++ src/types/openapi/openapi-full.ts | 1 + src/types/openapi/openapi.ts | 1 + .../features/bootstrap/FeatureContext.php | 14 +++++++ .../features/federation/invite.feature | 7 ++++ 17 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 lib/CachePrefix.php diff --git a/docs/conversation.md b/docs/conversation.md index 65a38c21bf4a..9b9fe58b5b79 100644 --- a/docs/conversation.md +++ b/docs/conversation.md @@ -40,10 +40,11 @@ - Header: -| field | type | Description | -|------------------------------------|--------|----------------------------------------------------------------------------------------------------------------------------------------------------------| -| `X-Nextcloud-Talk-Hash` | string | Sha1 value over some config. When you receive a different value on subsequent requests, the capabilities and the signaling settings should be refreshed. | -| `X-Nextcloud-Talk-Modified-Before` | string | Timestamp from before the database request that can be used as `modifiedSince` parameter in the next request | +| field | type | Description | +|---------------------------------------|--------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `X-Nextcloud-Talk-Hash` | string | Sha1 value over some config. When you receive a different value on subsequent requests, the capabilities and the signaling settings should be refreshed. | +| `X-Nextcloud-Talk-Modified-Before` | string | Timestamp from before the database request that can be used as `modifiedSince` parameter in the next request | +| `X-Nextcloud-Talk-Federation-Invites` | string | *Optional:* Number of pending invites to federated conversations the user has. (Only available when the user can do federation and has at least one invite pending) | - Data: Array of conversations, each conversation has at least: diff --git a/lib/BackgroundJob/ResetAssignedSignalingServer.php b/lib/BackgroundJob/ResetAssignedSignalingServer.php index 49df8df85928..9d819f62f546 100644 --- a/lib/BackgroundJob/ResetAssignedSignalingServer.php +++ b/lib/BackgroundJob/ResetAssignedSignalingServer.php @@ -23,6 +23,7 @@ namespace OCA\Talk\BackgroundJob; +use OCA\Talk\CachePrefix; use OCA\Talk\Manager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJob; @@ -49,7 +50,7 @@ public function __construct( $this->setInterval(60 * 5); $this->setTimeSensitivity(IJob::TIME_SENSITIVE); - $this->cache = $cacheFactory->createDistributed('hpb_servers'); + $this->cache = $cacheFactory->createDistributed(CachePrefix::SIGNALING_ASSIGNED_SERVER); } protected function run($argument): void { diff --git a/lib/CachePrefix.php b/lib/CachePrefix.php new file mode 100644 index 000000000000..a5c165bfa9cf --- /dev/null +++ b/lib/CachePrefix.php @@ -0,0 +1,37 @@ + + */ + +declare(strict_types=1); +/* + * @copyright Copyright (c) 2024 Joas Schilling + * + * @author Joas Schilling + * + * @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 . + * + */ + +namespace OCA\Talk; + +class CachePrefix { + public const FEDERATED_INVITES_COUNT = 'talk/fed/invites'; + public const FEDERATED_PCM = 'talk/pcm/'; + public const CHAT_LAST_MESSAGE_ID = 'talk/lastmsgid'; + public const CHAT_UNREAD_COUNT = 'talk/unreadcount'; + public const SIGNALING_ASSIGNED_SERVER = 'hpb_servers'; +} diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 8ee9a12ddd40..bb1494c4dbed 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -27,6 +27,7 @@ use DateInterval; use OC\Memcache\ArrayCache; use OC\Memcache\NullCache; +use OCA\Talk\CachePrefix; use OCA\Talk\Events\BeforeChatMessageSentEvent; use OCA\Talk\Events\BeforeSystemMessageSentEvent; use OCA\Talk\Events\ChatMessageSentEvent; @@ -110,8 +111,8 @@ public function __construct( protected IRequest $request, protected LoggerInterface $logger, ) { - $this->cache = $cacheFactory->createDistributed('talk/lastmsgid'); - $this->unreadCountCache = $cacheFactory->createDistributed('talk/unreadcount'); + $this->cache = $cacheFactory->createDistributed(CachePrefix::CHAT_LAST_MESSAGE_ID); + $this->unreadCountCache = $cacheFactory->createDistributed(CachePrefix::CHAT_UNREAD_COUNT); } /** diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 3f5cbaa53102..1c72015f31fc 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -28,6 +28,7 @@ namespace OCA\Talk\Controller; +use OCA\Talk\CachePrefix; use OCA\Talk\Capabilities; use OCA\Talk\Config; use OCA\Talk\Events\AAttendeeRemovedEvent; @@ -78,6 +79,8 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudIdManager; use OCP\HintException; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -98,6 +101,7 @@ */ class RoomController extends AEnvironmentAwareController { protected array $commonReadMessages = []; + protected ?ICache $fedInviteCountCache; public function __construct( string $appName, @@ -128,8 +132,10 @@ public function __construct( protected Authenticator $federationAuthenticator, protected Capabilities $capabilities, protected FederationManager $federationManager, + ICacheFactory $cacheFactory, ) { parent::__construct($appName, $request); + $this->fedInviteCountCache = $cacheFactory->createDistributed(CachePrefix::FEDERATED_INVITES_COUNT); } /** @@ -181,7 +187,7 @@ protected function getTalkHashHeader(): array { * @param bool $includeStatus Include the user status * @param int $modifiedSince Filter rooms modified after a timestamp * @psalm-param non-negative-int $modifiedSince - * @return DataResponse + * @return DataResponse * * 200: Return list of rooms */ @@ -191,6 +197,7 @@ public function getRooms(int $noStatusUpdate = 0, bool $includeStatus = false, i $event = new BeforeRoomsFetchEvent($this->userId); $this->dispatcher->dispatchTyped($event); + $user = $this->userManager->get($this->userId); if ($noStatusUpdate === 0) { $isMobileApp = $this->request->isUserAgent([ @@ -201,7 +208,7 @@ public function getRooms(int $noStatusUpdate = 0, bool $includeStatus = false, i if ($isMobileApp) { // Bump the user status again $event = new UserLiveStatusEvent( - $this->userManager->get($this->userId), + $user, IUserStatus::ONLINE, $this->timeFactory->getTime() ); @@ -255,7 +262,25 @@ public function getRooms(int $noStatusUpdate = 0, bool $includeStatus = false, i } } - return new DataResponse($return, Http::STATUS_OK, array_merge($this->getTalkHashHeader(), ['X-Nextcloud-Talk-Modified-Before' => (string) $nextModifiedSince])); + /** @var array{X-Nextcloud-Talk-Modified-Before: numeric-string, X-Nextcloud-Talk-Federation-Invites?: numeric-string} $headers */ + $headers = ['X-Nextcloud-Talk-Modified-Before' => (string) $nextModifiedSince]; + if ($this->talkConfig->isFederationEnabledForUserId($user)) { + $numInvites = $this->fedInviteCountCache?->get($user->getUID()); + if ($numInvites === null) { + $numInvites = $this->federationManager->getNumberOfPendingInvitationsForUser($user); + if ($this->fedInviteCountCache instanceof ICache) { + $this->fedInviteCountCache->set($user->getUID(), $numInvites, 15 * 60); + } + } + if ($numInvites !== 0) { + $headers['X-Nextcloud-Talk-Federation-Invites'] = (string) $numInvites; + } + } + + /** @var array{X-Nextcloud-Talk-Hash: string, X-Nextcloud-Talk-Modified-Before: numeric-string, X-Nextcloud-Talk-Federation-Invites?: numeric-string} $headers */ + $headers = array_merge($this->getTalkHashHeader(), $headers); + + return new DataResponse($return, Http::STATUS_OK, $headers); } /** diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index 5c9e2c8240ed..c131d904c69e 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -28,6 +28,7 @@ use Exception; use OCA\FederatedFileSharing\AddressHandler; use OCA\Talk\AppInfo\Application; +use OCA\Talk\CachePrefix; use OCA\Talk\Config; use OCA\Talk\Events\AAttendeeRemovedEvent; use OCA\Talk\Events\ARoomModifiedEvent; @@ -74,6 +75,7 @@ class CloudFederationProviderTalk implements ICloudFederationProvider { protected ?ICache $proxyCacheMessages; + protected ?ICache $fedInviteCountCache; public function __construct( private ICloudIdManager $cloudIdManager, @@ -97,7 +99,8 @@ public function __construct( private UserConverter $userConverter, ICacheFactory $cacheFactory, ) { - $this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed('talk/pcm/') : null; + $this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed(CachePrefix::FEDERATED_PCM) : null; + $this->fedInviteCountCache = $cacheFactory->createDistributed(CachePrefix::FEDERATED_INVITES_COUNT); } /** @@ -178,6 +181,7 @@ public function shareReceived(ICloudFederationShare $share): string { $invite = $this->federationManager->addRemoteRoom($shareWith, (int) $remoteId, $roomType, $roomName, $roomToken, $remote, $shareSecret, $sharedByFederatedId, $sharedByDisplayName, $invitedCloudId); $this->notifyAboutNewShare($shareWith, (string) $invite->getId(), $sharedByFederatedId, $sharedByDisplayName, $roomName, $roomToken, $remote); + $this->fedInviteCountCache?->remove($invite->getUserId()); return (string) $invite->getId(); } @@ -287,6 +291,7 @@ private function shareUnshared(int $remoteAttendeeId, array $notification): arra try { $participant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_USERS, $invite->getUserId()); $this->participantService->removeAttendee($room, $participant, AAttendeeRemovedEvent::REASON_REMOVED); + $this->fedInviteCountCache?->remove($invite->getUserId()); } catch (ParticipantNotFoundException) { // Never accepted the invite } diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php index a45c6c8cd0b1..6965443fce2e 100644 --- a/lib/Federation/FederationManager.php +++ b/lib/Federation/FederationManager.php @@ -257,6 +257,10 @@ public function getRemoteRoomShares(IUser $user): array { return $this->invitationMapper->getInvitationsForUser($user); } + public function getNumberOfPendingInvitationsForUser(IUser $user): int { + return $this->invitationMapper->countInvitationsForUser($user, Invitation::STATE_PENDING); + } + public function getNumberOfInvitations(Room $room): int { return $this->invitationMapper->countInvitationsForLocalRoom($room); } diff --git a/lib/Federation/Proxy/TalkV1/Controller/ChatController.php b/lib/Federation/Proxy/TalkV1/Controller/ChatController.php index e926e59072aa..44a20ccf5a23 100644 --- a/lib/Federation/Proxy/TalkV1/Controller/ChatController.php +++ b/lib/Federation/Proxy/TalkV1/Controller/ChatController.php @@ -26,6 +26,7 @@ namespace OCA\Talk\Federation\Proxy\TalkV1\Controller; +use OCA\Talk\CachePrefix; use OCA\Talk\Chat\Notifier; use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Federation\Proxy\TalkV1\ProxyRequest; @@ -57,7 +58,7 @@ public function __construct( protected Notifier $notifier, ICacheFactory $cacheFactory, ) { - $this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed('talk/pcm/') : null; + $this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed(CachePrefix::FEDERATED_PCM) : null; } /** diff --git a/lib/Model/InvitationMapper.php b/lib/Model/InvitationMapper.php index 1c676aa4564b..e54b9bd4aa2c 100644 --- a/lib/Model/InvitationMapper.php +++ b/lib/Model/InvitationMapper.php @@ -95,6 +95,27 @@ public function getInvitationsForUser(IUser $user): array { return $this->findEntities($qb); } + /** + * @psalm-param Invitation::STATE_*|null $state + */ + public function countInvitationsForUser(IUser $user, ?int $state = null): int { + $qb = $this->db->getQueryBuilder(); + + $qb->select($qb->func()->count('*')) + ->from($this->getTableName()) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($user->getUID()))); + + if ($state !== null) { + $qb->andWhere($qb->expr()->eq('state', $qb->createNamedParameter($state))); + } + + $result = $qb->executeQuery(); + $count = (int) $result->fetchOne(); + $result->closeCursor(); + + return $count; + } + /** * @throws DoesNotExistException */ diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php index 31c31a6644f0..ed4c6a8880e9 100644 --- a/lib/Service/ParticipantService.php +++ b/lib/Service/ParticipantService.php @@ -26,6 +26,7 @@ use OCA\Circles\CirclesManager; use OCA\Circles\Model\Circle; use OCA\Circles\Model\Member; +use OCA\Talk\CachePrefix; use OCA\Talk\Config; use OCA\Talk\Events\AAttendeeRemovedEvent; use OCA\Talk\Events\AParticipantModifiedEvent; @@ -562,9 +563,9 @@ protected function updateRoomLastMessage(Room $room, IComment $message): void { $roomService = Server::get(RoomService::class); $roomService->setLastMessage($room, $message); - $lastMessageCache = $this->cacheFactory->createDistributed('talk/lastmsgid'); + $lastMessageCache = $this->cacheFactory->createDistributed(CachePrefix::CHAT_LAST_MESSAGE_ID); $lastMessageCache->remove($room->getToken()); - $unreadCountCache = $this->cacheFactory->createDistributed('talk/unreadcount'); + $unreadCountCache = $this->cacheFactory->createDistributed(CachePrefix::CHAT_UNREAD_COUNT); $unreadCountCache->clear($room->getId() . '-'); $event = new SystemMessagesMultipleSentEvent($room, $message); diff --git a/lib/Signaling/Manager.php b/lib/Signaling/Manager.php index 472543c25916..8736c26d77d3 100644 --- a/lib/Signaling/Manager.php +++ b/lib/Signaling/Manager.php @@ -23,6 +23,7 @@ namespace OCA\Talk\Signaling; +use OCA\Talk\CachePrefix; use OCA\Talk\Config; use OCA\Talk\Room; use OCA\Talk\Service\RoomService; @@ -42,7 +43,7 @@ public function __construct( protected RoomService $roomService, ICacheFactory $cacheFactory, ) { - $this->cache = $cacheFactory->createDistributed('hpb_servers'); + $this->cache = $cacheFactory->createDistributed(CachePrefix::SIGNALING_ASSIGNED_SERVER); } public function isCompatibleSignalingServer(IResponse $response): bool { diff --git a/openapi-full.json b/openapi-full.json index e519deb7d78f..a46ecd253766 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -9560,6 +9560,11 @@ "schema": { "type": "string" } + }, + "X-Nextcloud-Talk-Federation-Invites": { + "schema": { + "type": "string" + } } }, "content": { diff --git a/openapi.json b/openapi.json index f6e203b2d29f..a352fd839b49 100644 --- a/openapi.json +++ b/openapi.json @@ -9447,6 +9447,11 @@ "schema": { "type": "string" } + }, + "X-Nextcloud-Talk-Federation-Invites": { + "schema": { + "type": "string" + } } }, "content": { diff --git a/src/types/openapi/openapi-full.ts b/src/types/openapi/openapi-full.ts index d192baa933b7..516416269060 100644 --- a/src/types/openapi/openapi-full.ts +++ b/src/types/openapi/openapi-full.ts @@ -3640,6 +3640,7 @@ export type operations = { headers: { "X-Nextcloud-Talk-Hash"?: string; "X-Nextcloud-Talk-Modified-Before"?: string; + "X-Nextcloud-Talk-Federation-Invites"?: string; }; content: { "application/json": { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 8e09440d1b48..4a6845fe3b72 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -3463,6 +3463,7 @@ export type operations = { headers: { "X-Nextcloud-Talk-Hash"?: string; "X-Nextcloud-Talk-Modified-Before"?: string; + "X-Nextcloud-Talk-Federation-Invites"?: string; }; content: { "application/json": { diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index cc8ac557e009..d8a18dd29c04 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3026,6 +3026,20 @@ public function hasChatLastCommonReadHeader($setOrLower, $message) { } } + /** + * @Then /^last response has federation invites header set to "([^"]*)"$/ + * + * @param string $count + */ + public function hasFederationInvitesHeader(string $count): void { + if ($count === 'NULL') { + Assert::assertFalse($this->response->hasHeader('X-Nextcloud-Talk-Federation-Invites'), "Should not contain 'X-Nextcloud-Talk-Federation-Invites' header\n" . json_encode($this->response->getHeaders(), JSON_PRETTY_PRINT)); + } else { + Assert::assertTrue($this->response->hasHeader('X-Nextcloud-Talk-Federation-Invites'), "Should contain 'X-Nextcloud-Talk-Federation-Invites' header\n" . json_encode($this->response->getHeaders(), JSON_PRETTY_PRINT)); + Assert::assertEquals($count, $this->response->getHeader('X-Nextcloud-Talk-Federation-Invites')[0]); + } + } + /** * @Then /^user "([^"]*)" creates (\d+) (automatic|manual|free) breakout rooms for "([^"]*)" with (\d+) \((v1)\)$/ * diff --git a/tests/integration/features/federation/invite.feature b/tests/integration/features/federation/invite.feature index 9388736433e6..43308d2cf4eb 100644 --- a/tests/integration/features/federation/invite.feature +++ b/tests/integration/features/federation/invite.feature @@ -41,6 +41,9 @@ Feature: federation/invite | room | users | participant1 | federated_user_added | You invited {federated_user} | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"},"federated_user":{"type":"user","id":"participant2","name":"participant2@localhost:8180","server":"http:\/\/localhost:8180"}} | | room | users | participant1 | conversation_created | You created the conversation | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4) + Then user "participant2" is participant of the following rooms (v4) + | id | name | type | + Then last response has federation invites header set to "1" When user "participant1" sees the following attendees in room "room" with 200 (v4) | actorType | actorId | participantType | | users | participant1 | 1 | @@ -59,6 +62,10 @@ Feature: federation/invite And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) | id | name | type | remoteServer | remoteToken | | room | room | 3 | LOCAL | room | + Then user "participant2" is participant of the following rooms (v4) + | id | name | type | remoteServer | remoteToken | + | room | room | 3 | LOCAL | room | + Then last response has federation invites header set to "NULL" And user "participant2" accepts invite to room "room" of server "LOCAL" with 400 (v1) | error | state | And user "participant2" declines invite to room "room" of server "LOCAL" with 400 (v1)