Skip to content

feat(signaling): Don't send all room userids in signaling server requests #16759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fancycode wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −73
Open

feat(signaling): Don't send all room userids in signaling server requests #16759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 14 additions & 32 deletions lib/Signaling/BackendNotifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,14 @@ public function roomInvited(Room $room, array $attendees): void {
$userIds[] = $attendee->getActorId();
}
}
if (empty($userIds)) {
return;
}
$start = microtime(true);
$this->backendRequest($room, [
'type' => 'invite',
'invite' => [
'userids' => $userIds,
// TODO(fancycode): We should try to get rid of 'alluserids' and
// find a better way to notify existing users to update the room.
'alluserids' => $this->participantService->getParticipantUserIdsAndFederatedUserCloudIds($room),
'properties' => $room->getPropertiesForSignaling('', false),
],
]);
Expand All @@ -180,22 +180,20 @@ public function roomInvited(Room $room, array $attendees): void {
* @throws \Exception
*/
public function roomsDisinvited(Room $room, array $attendees): void {
$allUserIds = $this->participantService->getParticipantUserIdsAndFederatedUserCloudIds($room);
sort($allUserIds);
$userIds = [];
foreach ($attendees as $attendee) {
if ($attendee->getActorType() === Attendee::ACTOR_USERS) {
$userIds[] = $attendee->getActorId();
}
}
if (empty($userIds)) {
return;
}
$start = microtime(true);
$this->backendRequest($room, [
'type' => 'disinvite',
'disinvite' => [
'userids' => $userIds,
// TODO(fancycode): We should try to get rid of 'alluserids' and
// find a better way to notify existing users to update the room.
'alluserids' => $allUserIds,
'properties' => $room->getPropertiesForSignaling('', false),
],
]);
Expand All @@ -216,16 +214,11 @@ public function roomsDisinvited(Room $room, array $attendees): void {
* @throws \Exception
*/
public function roomSessionsRemoved(Room $room, array $sessionIds): void {
$allUserIds = $this->participantService->getParticipantUserIdsAndFederatedUserCloudIds($room);
sort($allUserIds);
$start = microtime(true);
$this->backendRequest($room, [
'type' => 'disinvite',
'disinvite' => [
'sessionids' => $sessionIds,
// TODO(fancycode): We should try to get rid of 'alluserids' and
// find a better way to notify existing users to update the room.
'alluserids' => $allUserIds,
'properties' => $room->getPropertiesForSignaling('', false),
],
]);
Expand All @@ -249,10 +242,6 @@ public function roomModified(Room $room): void {
$this->backendRequest($room, [
'type' => 'update',
'update' => [
// Message not sent for federated users, as they will receive
// the message from their federated Nextcloud server once the
// property change is propagated.
'userids' => $this->participantService->getParticipantUserIds($room),
'properties' => $room->getPropertiesForSignaling(''),
],
]);
Expand All @@ -271,13 +260,11 @@ public function roomModified(Room $room): void {
* @param string[] $userIds
* @throws \Exception
*/
public function roomDeleted(Room $room, array $userIds): void {
public function roomDeleted(Room $room): void {
$start = microtime(true);
$this->backendRequest($room, [
'type' => 'delete',
'delete' => [
'userids' => $userIds,
],
'delete' => (object)[],
Copy link
Member

@nickvergessen nickvergessen Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP way is usually:

Suggested change
'delete' => (object)[],
'delete' => new \stdClass(),

]);
$duration = microtime(true) - $start;
$this->logger->debug('Room deleted: {token} ({duration})', [
Expand Down Expand Up @@ -323,7 +310,6 @@ public function switchToRoom(Room $room, string $switchToRoomToken, array $sessi
*/
public function participantsModified(Room $room, array $sessionIds): void {
$changed = [];
$users = [];
$participants = $this->participantService->getSessionsAndParticipantsForRoom($room);
foreach ($participants as $participant) {
$attendee = $participant->getAttendee();
Expand Down Expand Up @@ -354,7 +340,6 @@ public function participantsModified(Room $room, array $sessionIds): void {
$data['lastPing'] = $session->getLastPing();
$data['sessionId'] = $session->getSessionId();
$data['participantPermissions'] = $participant->getPermissions();
$users[] = $data;

if (\in_array($session->getSessionId(), $sessionIds, true)) {
$data['permissions'] = [];
Expand All @@ -372,17 +357,17 @@ public function participantsModified(Room $room, array $sessionIds): void {
}
$changed[] = $data;
}
} else {
$users[] = $data;
}
}

if (empty($changed)) {
return;
}
$start = microtime(true);
$this->backendRequest($room, [
'type' => 'participants',
'participants' => [
'changed' => $changed,
'users' => $users
],
]);
$duration = microtime(true) - $start;
Expand Down Expand Up @@ -411,7 +396,6 @@ public function roomInCallChanged(Room $room, int $flags, array $sessionIds, boo
];
} else {
$changed = [];
$users = [];

$participants = $this->participantService->getParticipantsForAllSessions($room);
foreach ($participants as $participant) {
Expand Down Expand Up @@ -442,19 +426,17 @@ public function roomInCallChanged(Room $room, int $flags, array $sessionIds, boo
$data['userId'] = $attendee->getActorId();
}

if ($session->getInCall() !== Participant::FLAG_DISCONNECTED) {
$users[] = $data;
}

if (\in_array($session->getSessionId(), $sessionIds, true)) {
$changed[] = $data;
}
}

if (empty($changed)) {
return;
}
$data = [
'incall' => $flags,
'changed' => $changed,
'users' => $users,
];
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Signaling/Listener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ protected function notifyCallEndedForEveryone(CallEndedForEveryoneEvent $event):

protected function notifyBeforeRoomDeleted(BeforeRoomDeletedEvent $event): void {
$room = $event->getRoom();
$this->externalSignaling->roomDeleted($room, $this->participantService->getParticipantUserIds($room));
$this->externalSignaling->roomDeleted($room);
}

protected function notifyGuestsCleanedUp(GuestsCleanedUpEvent $event): void {
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/features/callapi/recording.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Feature: callapi/recording
Then signaling server received the following requests
| token | data |
| room1 | {"type":"message","message":{"data":{"type":"recording","recording":{"status":3}}}} |
| room1 | {"type":"update","update":{"userids":["participant1"],"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":{"date":"ACTIVE_SINCE()","timezone_type":3,"timezone":"UTC"},"sip-enabled":0,"description":""}}} |
| room1 | {"type":"update","update":{"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":{"date":"ACTIVE_SINCE()","timezone_type":3,"timezone":"UTC"},"sip-enabled":0,"description":""}}} |
And user "participant1" is participant of the following unordered rooms (v4)
| type | name | callRecording |
| 2 | room1 | 3 |
Expand Down Expand Up @@ -48,7 +48,7 @@ Feature: callapi/recording
| token | data |
| room1 | {"type":"message","message":{"data":{"type":"chat","chat":{"refresh":true,"comment":[]}}}} |
| room1 | {"type":"message","message":{"data":{"type":"recording","recording":{"status":0}}}} |
| room1 | {"type":"update","update":{"userids":["participant1"],"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":{"date":"ACTIVE_SINCE()","timezone_type":3,"timezone":"UTC"},"sip-enabled":0,"description":""}}} |
| room1 | {"type":"update","update":{"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":{"date":"ACTIVE_SINCE()","timezone_type":3,"timezone":"UTC"},"sip-enabled":0,"description":""}}} |
Then user "participant1" sees the following system messages in room "room1" with 200 (v1)
| room | actorType | actorId | actorDisplayName | systemMessage |
| room1 | users | participant1 | participant1-displayname | recording_stopped |
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/features/callapi/update-call-flags.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ Feature: callapi/update-call-flags
Then signaling server received the following requests
| token | data |
| public room | {"type":"message","message":{"data":{"type":"chat","chat":{"refresh":true,"comment":[]}}}} |
| public room | {"type":"incall","incall":{"incall":7,"changed":[{"inCall":7,"lastPing":LAST_PING(),"sessionId":"SESSION(owner)","nextcloudSessionId":"SESSION(owner)","participantType":1,"participantPermissions":254,"actorType":"users","actorId":"owner","userId":"owner"}],"users":[{"inCall":7,"lastPing":LAST_PING(),"sessionId":"SESSION(owner)","nextcloudSessionId":"SESSION(owner)","participantType":1,"participantPermissions":254,"actorType":"users","actorId":"owner","userId":"owner"}]}} |
| public room | {"type":"incall","incall":{"incall":7,"changed":[{"inCall":7,"lastPing":LAST_PING(),"sessionId":"SESSION(owner)","nextcloudSessionId":"SESSION(owner)","participantType":1,"participantPermissions":254,"actorType":"users","actorId":"owner","userId":"owner"}]}} |
And reset signaling server requests
When user "owner" updates call flags in room "public room" to "1" with 200 (v4)
Then signaling server received the following requests
| token | data |
| public room | {"type":"incall","incall":{"incall":1,"changed":[{"inCall":1,"lastPing":LAST_PING(),"sessionId":"SESSION(owner)","nextcloudSessionId":"SESSION(owner)","participantType":1,"participantPermissions":254,"actorType":"users","actorId":"owner","userId":"owner"}],"users":[{"inCall":1,"lastPing":LAST_PING(),"sessionId":"SESSION(owner)","nextcloudSessionId":"SESSION(owner)","participantType":1,"participantPermissions":254,"actorType":"users","actorId":"owner","userId":"owner"}]}} |
| public room | {"type":"incall","incall":{"incall":1,"changed":[{"inCall":1,"lastPing":LAST_PING(),"sessionId":"SESSION(owner)","nextcloudSessionId":"SESSION(owner)","participantType":1,"participantPermissions":254,"actorType":"users","actorId":"owner","userId":"owner"}]}} |
And user "moderator" joins call "public room" with 200 (v4)
And user "invited user" joins call "public room" with 200 (v4)
And user "not invited but joined user" joins call "public room" with 200 (v4)
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/features/chat-1/group-read-only.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Feature: chat/group-read-only
Then signaling server received the following requests
| token | data |
| group room | {"type":"message","message":{"data":{"type":"chat","chat":{"refresh":true}}}} |
| group room | {"type":"update","update":{"userids":["participant1","participant2"],"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":1,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
| group room | {"type":"update","update":{"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":1,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
When user "participant1" sends message "Message 2" to room "group room" with 403
Then user "participant1" sees the following messages in room "group room" with 200
| room | actorType | actorId | actorDisplayName | message | messageParameters |
Expand All @@ -30,7 +30,7 @@ Feature: chat/group-read-only
Then signaling server received the following requests
| token | data |
| group room | {"type":"message","message":{"data":{"type":"chat","chat":{"refresh":true}}}} |
| group room | {"type":"update","update":{"userids":["participant1","participant2"],"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
| group room | {"type":"update","update":{"properties":{"name":"Private conversation","type":2,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
When user "participant1" sends message "Message 3" to room "group room" with 201
Then user "participant1" sees the following messages in room "group room" with 200
| room | actorType | actorId | actorDisplayName | message | messageParameters |
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/features/command/user-remove.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ Feature: command/user-remove
And invoking occ with "talk:user:remove --user participant2"
Then signaling server received the following requests
| token | data |
| room | {"type":"disinvite","disinvite":{"userids":["participant2"],"alluserids":["participant1"],"properties":{"name":"Private conversation","type":1,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"participant-list":"refresh"}}} |
| room | {"type":"disinvite","disinvite":{"userids":["participant2"],"properties":{"name":"Private conversation","type":1,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"participant-list":"refresh"}}} |
# Type changed
| room | {"type":"update","update":{"userids":["participant1"],"properties":{"name":"Private conversation","type":5,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
| room | {"type":"update","update":{"properties":{"name":"Private conversation","type":5,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
# Name changed
| room | {"type":"update","update":{"userids":["participant1"],"properties":{"name":"Private conversation","type":5,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
| room | {"type":"update","update":{"properties":{"name":"Private conversation","type":5,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
| room | {"type":"message","message":{"data":{"type":"chat","chat":{"refresh":true}}}} |
# Read only changed
| room | {"type":"update","update":{"userids":["participant1"],"properties":{"name":"Private conversation","type":5,"lobby-state":0,"lobby-timer":null,"read-only":1,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
| room | {"type":"update","update":{"properties":{"name":"Private conversation","type":5,"lobby-state":0,"lobby-timer":null,"read-only":1,"listable":0,"active-since":null,"sip-enabled":0,"description":""}}} |
And the command output contains the text "Users successfully removed from all rooms"
Then the command was successful
And user "participant2" is participant of the following rooms (v4)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/features/conversation-1/add-participant.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Feature: conversation/add-participant
And user "participant1" adds user "participant2" to room "room" with 200 (v4)
Then signaling server received the following requests
| token | data |
| room | {"type":"invite","invite":{"userids":["participant2"],"alluserids":["participant1","participant2"],"properties":{"name":"Private conversation","type":3,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"participant-list":"refresh"}}} |
| room | {"type":"invite","invite":{"userids":["participant2"],"properties":{"name":"Private conversation","type":3,"lobby-state":0,"lobby-timer":null,"read-only":0,"listable":0,"active-since":null,"sip-enabled":0,"participant-list":"refresh"}}} |
| room | {"type":"message","message":{"data":{"type":"chat","chat":{"refresh":true}}}} |
And user "participant1" is participant of the following rooms (v4)
| id | type | participantType |
Expand Down
Loading
Loading