Skip to content

Commit f2157f2

Browse files
toger5toger5
authored
[MatrixRTC] Minimal change to transition from "" to "ROOM" as the callId/slotId (#5166)
* Minimal change to transition from "" to "ROOM" as the callId/slotId * Also transition MembershipManager tests to `"ROOM"` * fix merge
1 parent 739b8e1 commit f2157f2

File tree

7 files changed

+48
-24
lines changed

7 files changed

+48
-24
lines changed

‎spec/unit/matrixrtc/CallMembership.spec.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,10 @@ describe("CallMembership", () => {
158158
expect(membership.eventId).toBe("$eventid");
159159
});
160160
it("returns correct slot_id", () => {
161-
expect(membership.slotId).toBe("m.call#");
162-
expect(membership.slotDescription).toStrictEqual({ id: "", application: "m.call" });
161+
// for legacy events we expect the room to be added automagically
162+
// See INFO_SLOT_ID_LEGACY_CASE comments
163+
expect(membership.slotId).toBe("m.call#ROOM");
164+
expect(membership.slotDescription).toStrictEqual({ id: "ROOM", application: "m.call" });
163165
});
164166
it("returns correct deviceId", () => {
165167
expect(membership.deviceId).toBe("AAAAAAA");

‎spec/unit/matrixrtc/MatrixRTCSession.spec.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ const mockFocus = { type: "mock" };
4949

5050
const textEncoder = new TextEncoder();
5151

52-
const callSession = { id: "", application: "m.call" };
52+
const callSession = { id: "ROOM", application: "m.call" };
5353

5454
describe("MatrixRTCSession", () => {
5555
let client: MatrixClient;
@@ -134,12 +134,12 @@ describe("MatrixRTCSession", () => {
134134
);
135135
await flushPromises();
136136
expect(sess?.memberships.length).toEqual(1);
137-
expect(sess?.memberships[0].slotDescription.id).toEqual("");
137+
expect(sess?.memberships[0].slotDescription.id).toEqual("ROOM");
138138
expect(sess?.memberships[0].scope).toEqual("m.room");
139139
expect(sess?.memberships[0].application).toEqual("m.call");
140140
expect(sess?.memberships[0].deviceId).toEqual("AAAAAAA");
141141
expect(sess?.memberships[0].isExpired()).toEqual(false);
142-
expect(sess?.slotDescription.id).toEqual("");
142+
expect(sess?.slotDescription.id).toEqual("ROOM");
143143
});
144144

145145
it("ignores memberships where application is not m.call", () => {
@@ -381,12 +381,12 @@ describe("MatrixRTCSession", () => {
381381
});
382382
await flushPromises();
383383
expect(sess?.memberships.length).toEqual(1);
384-
expect(sess?.memberships[0].slotDescription.id).toEqual("");
384+
expect(sess?.memberships[0].slotDescription.id).toEqual("ROOM");
385385
expect(sess?.memberships[0].scope).toEqual("m.room");
386386
expect(sess?.memberships[0].application).toEqual("m.call");
387387
expect(sess?.memberships[0].deviceId).toEqual("AAAAAAA");
388388
expect(sess?.memberships[0].isExpired()).toEqual(false);
389-
expect(sess?.slotDescription.id).toEqual("");
389+
expect(sess?.slotDescription.id).toEqual("ROOM");
390390
});
391391
it("combines sticky and membership events when both exist", async () => {
392392
// Create a room with identical member state and sticky state for the same user.
@@ -415,7 +415,7 @@ describe("MatrixRTCSession", () => {
415415
const memberships = sess.memberships;
416416
expect(memberships.length).toEqual(2);
417417
expect(memberships[0].sender).toEqual(stickyUserId);
418-
expect(memberships[0].slotDescription.id).toEqual("");
418+
expect(memberships[0].slotDescription.id).toEqual("ROOM");
419419
expect(memberships[0].scope).toEqual("m.room");
420420
expect(memberships[0].application).toEqual("m.call");
421421
expect(memberships[0].deviceId).toEqual("AAAAAAA");
@@ -424,7 +424,7 @@ describe("MatrixRTCSession", () => {
424424
// Then state
425425
expect(memberships[1].sender).toEqual(membershipTemplate.user_id);
426426

427-
expect(sess?.slotDescription.id).toEqual("");
427+
expect(sess?.slotDescription.id).toEqual("ROOM");
428428
});
429429
it("handles an incoming sticky event to an existing session", async () => {
430430
const mockRoom = makeMockRoom([membershipTemplate]);

‎spec/unit/matrixrtc/MembershipManager.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ function createAsyncHandle<T>(method: MockedFunction<(...args: any[]) => any>) {
7171
return { reject, resolve };
7272
}
7373

74-
const callSession = { id: "", application: "m.call" };
74+
const callSession = { id: "ROOM", application: "m.call" };
7575

7676
describe("MembershipManager", () => {
7777
let client: MockClient;
@@ -969,7 +969,7 @@ describe("MembershipManager", () => {
969969
id: "@alice:example.org:AAAAAAA_m.call",
970970
device_id: "AAAAAAA",
971971
},
972-
slot_id: "m.call#",
972+
slot_id: "m.call#ROOM",
973973
rtc_transports: [{ type: focus.type, livekit_service_url: focus.livekit_service_url }],
974974
versions: [],
975975
msc4354_sticky_key: "@alice:example.org:AAAAAAA_m.call",

‎src/matrixrtc/CallMembership.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,23 @@ export class CallMembership {
377377
return data.slot_id;
378378
case "session":
379379
default:
380-
return slotDescriptionToId({ application: this.application, id: data.call_id });
380+
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
381+
// The spec got changed to use `"ROOM"` instead of `""` empyt string for the implicit default call.
382+
// State events still are sent with `""` however. To find other events that should end up in the same call,
383+
// we use the slotId.
384+
// Since the CallMembership is the public representation of a rtc.member event, we just pretend it is a
385+
// "ROOM" slotId/call_id.
386+
// This makes all the remote members work with just this simple trick.
387+
//
388+
// We of course now need to be careful when sending legacy events (state events)
389+
// They get a slotDescription containing "ROOM" since this is what we use starting at the time this comment
390+
// is commited.
391+
//
392+
// See the Other INFO_SLOT_ID_LEGACY_CASE comments to see where we revert back to "" just before sending the event.
393+
return slotDescriptionToId({
394+
application: this.application,
395+
id: data.call_id === "" ? "ROOM" : data.call_id,
396+
});
381397
}
382398
}
383399

‎src/matrixrtc/MatrixRTCSession.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { CallMembership } from "./CallMembership.ts";
2626
import { RoomStateEvent } from "../models/room-state.ts";
2727
import { MembershipManager, StickyEventMembershipManager } from "./MembershipManager.ts";
2828
import { type CallMembershipIdentityParts, EncryptionManager, type IEncryptionManager } from "./EncryptionManager.ts";
29-
import { deepCompare, logDurationSync } from "../utils.ts";
29+
import { logDurationSync } from "../utils.ts";
3030
import type {
3131
Statistics,
3232
RTCNotificationType,
@@ -326,7 +326,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
326326
*/
327327
public static async sessionMembershipsForSlot(
328328
room: Pick<Room, "getLiveTimeline" | "roomId" | "hasMembershipState" | "_unstable_getStickyEvents">,
329-
slotDescription: SlotDescription,
329+
slotId: string,
330330
// default both true this implied we combine sticky and state events for the final call state
331331
// (prefer sticky events in case of a duplicate)
332332
options: SessionMembershipsForSlotOpts = DEFAULT_SESSION_MEMBERSHIPS_FOR_SLOT_OPTS,
@@ -337,7 +337,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
337337
const callMemberships = await computeBackendIdentityAndVerifyMemberEvents(
338338
room,
339339
callMemberEvents,
340-
slotDescription,
340+
slotId,
341341
logger,
342342
);
343343

@@ -807,7 +807,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
807807

808808
this.memberships = await MatrixRTCSession.sessionMembershipsForSlot(
809809
this.room,
810-
this.slotDescription,
810+
slotDescriptionToId(this.slotDescription),
811811
this.calculateMembershipsOpts,
812812
);
813813

@@ -857,7 +857,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
857857
async function computeBackendIdentityAndVerifyMemberEvents(
858858
room: Pick<Room, "hasMembershipState">,
859859
callMemberEvents: MatrixEvent[],
860-
slotDescription: SlotDescription,
860+
slotId: string,
861861
logger: Logger,
862862
): Promise<CallMembership[]> {
863863
const callMemberships: CallMembership[] = [];
@@ -881,7 +881,7 @@ async function computeBackendIdentityAndVerifyMemberEvents(
881881
logger,
882882
);
883883

884-
if (isValidMembership(membership, room, slotDescription, logger)) {
884+
if (isValidMembership(membership, room, slotId, logger)) {
885885
callMemberships.push(membership);
886886
}
887887
} catch (e) {
@@ -914,12 +914,12 @@ function quickFilterNonRelevantContents(content: IContent, logger: Logger): bool
914914
function isValidMembership(
915915
membership: CallMembership,
916916
room: Pick<Room, "hasMembershipState">,
917-
slotDescription: SlotDescription,
917+
slotId: string,
918918
logger: Logger,
919919
): boolean {
920-
if (!deepCompare(membership.slotDescription, slotDescription)) {
920+
if (membership.slotId !== slotId) {
921921
logger.info(
922-
`Ignoring membership of user ${membership.userId} for a different slot: ${JSON.stringify(membership.slotDescription)}`,
922+
`Ignoring membership of user ${membership.userId} for a different slot: user: ${JSON.stringify(membership.slotDescription)}, slotId: ${slotId})`,
923923
);
924924
return false;
925925
}

‎src/matrixrtc/MatrixRTCSessionManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export class MatrixRTCSessionManager extends TypedEventEmitter<MatrixRTCSessionM
5656
public constructor(
5757
rootLogger: Logger,
5858
private client: MatrixClient,
59-
private readonly slotDescription: SlotDescription = { application: "m.call", id: "" }, // Default to the Matrix Call application
59+
private readonly slotDescription: SlotDescription = { application: "m.call", id: "ROOM" }, // Default to the Matrix Call application
6060
) {
6161
super();
6262
this.logger = rootLogger.getChild("[MatrixRTCSessionManager]");

‎src/matrixrtc/MembershipManager.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,11 @@ export class MembershipManager
774774
* which is not compatible with membershipID of session type member events. They have to be `${localUserId}:${localDeviceId}`
775775
*/
776776
private makeMembershipStateKey(localUserId: string, localDeviceId: string): string {
777-
const stateKey = `${localUserId}_${localDeviceId}_${this.slotDescription.application}${this.slotDescription.id}`;
777+
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
778+
// Revert back to "" just for the state key (state keys are always legacy. we use sticky events for non legacy events)
779+
const application = this.slotDescription.application;
780+
const slotId = this.slotDescription.id === "ROOM" ? "" : this.slotDescription.id;
781+
const stateKey = `${localUserId}_${localDeviceId}_${application}${slotId}`;
778782
if (/^org\.matrix\.msc(3757|3779)\b/.exec(this.room.getVersion())) {
779783
return stateKey;
780784
} else {
@@ -800,7 +804,9 @@ export class MembershipManager
800804
};
801805
return {
802806
"application": this.slotDescription.application,
803-
"call_id": this.slotDescription.id,
807+
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
808+
// Revert back to "" just for the sending the event.
809+
"call_id": this.slotDescription.id === "ROOM" ? "" : this.slotDescription.id,
804810
"scope": "m.room",
805811
"device_id": this.deviceId,
806812
// DO NOT use this.memberId here since that is the state key (using application...)

0 commit comments

Comments
 (0)