Make membership manager only be responsible for sessions starts

toger5 · toger5 · commit 0dbc652d2c9f · 2025-10-11T10:20:43.000+02:00
Signed-off-by: Timo K &lt;toger5@hotmail.de&gt;
diff --git a/spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts b/spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts
@@ -19,6 +19,7 @@ import { RoomStateEvent } from "../../../src/models/room-state";
 import { MatrixRTCSessionManager, MatrixRTCSessionManagerEvents } from "../../../src/matrixrtc/MatrixRTCSessionManager";
 import { makeMockRoom, type MembershipData, membershipTemplate, mockRoomState, mockRTCEvent } from "./mocks";
 import { logger } from "../../../src/logger";
+import { RoomStickyEventsEvent } from "../../../src/models/room-sticky-events";
 
 describe.each([{ eventKind: "sticky" }, { eventKind: "memberState" }])(
     "MatrixRTCSessionManager ($eventKind)",
@@ -30,10 +31,15 @@ describe.each([{ eventKind: "sticky" }, { eventKind: "memberState" }])(
                 mockRoomState(room, [{ user_id: membershipTemplate.user_id }]);
                 const roomState = room.getLiveTimeline().getState(EventTimeline.FORWARDS)!;
                 const membEvent = roomState.getStateEvents("org.matrix.msc3401.call.member")[0];
-                client.emit(RoomStateEvent.Events, membEvent, roomState, null);
+                roomState.emit(RoomStateEvent.Events, membEvent, roomState, null);
             } else {
-                membershipData.splice(0, 1, { user_id: membershipTemplate.user_id });
-                client.emit(ClientEvent.Event, mockRTCEvent(membershipData[0], room.roomId, 10000));
+                const previousData = membershipData.splice(0, 1, {
+                    user_id: membershipTemplate.user_id,
+                    msc4354_sticky_key: membershipTemplate.msc4354_sticky_key,
+                })[0];
+                const current = mockRTCEvent(membershipData[0], room.roomId, 10000);
+                const previous = mockRTCEvent(previousData, room.roomId, 10000);
+                room.emit(RoomStickyEventsEvent.Update, [], [{ current, previous }], []);
             }
         }
 
diff --git a/spec/unit/matrixrtc/mocks.ts b/spec/unit/matrixrtc/mocks.ts
@@ -14,12 +14,20 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { EventEmitter } from "stream";
 import { type Mocked } from "jest-mock";
 
-import { EventType, type Room, RoomEvent, type MatrixClient, type MatrixEvent } from "../../../src";
+import {
+    EventType,
+    type Room,
+    RoomEvent,
+    type MatrixClient,
+    type MatrixEvent,
+    Direction,
+    TypedEventEmitter,
+} from "../../../src";
 import { CallMembership, type SessionMembershipData } from "../../../src/matrixrtc/CallMembership";
 import { secureRandomString } from "../../../src/randomstring";
+import { type StickyMatrixEvent } from "../../../src/models/room-sticky-events";
 
 export type MembershipData = (SessionMembershipData | {}) & { user_id: string };
 
@@ -81,7 +89,7 @@ export function makeMockRoom(
     // Caching roomState here so it does not get recreated when calling `getLiveTimeline.getState()`
     const roomState = makeMockRoomState(useStickyEvents ? [] : membershipData, roomId);
     const ts = Date.now();
-    const room = Object.assign(new EventEmitter(), {
+    const room = Object.assign(new TypedEventEmitter(), {
         roomId: roomId,
         hasMembershipState: jest.fn().mockReturnValue(true),
         getLiveTimeline: jest.fn().mockReturnValue({
@@ -102,14 +110,13 @@ export function makeMockRoom(
 
 function makeMockRoomState(membershipData: MembershipData[], roomId: string) {
     const events = membershipData.map((m) => mockRTCEvent(m, roomId));
+
     const keysAndEvents = events.map((e) => {
         const data = e.getContent() as SessionMembershipData;
         return [`_${e.sender?.userId}_${data.device_id}`];
     });
 
-    return {
-        on: jest.fn(),
-        off: jest.fn(),
+    return Object.assign(new TypedEventEmitter(), {
         getStateEvents: (_: string, stateKey: string) => {
             if (stateKey !== undefined) return keysAndEvents.find(([k]) => k === stateKey)?.[1];
             return events;
@@ -128,11 +135,17 @@ function makeMockRoomState(membershipData: MembershipData[], roomId: string) {
                           },
                       ],
                   ]),
-    };
+    });
 }
 
 export function mockRoomState(room: Room, membershipData: MembershipData[]): void {
-    room.getLiveTimeline().getState = jest.fn().mockReturnValue(makeMockRoomState(membershipData, room.roomId));
+    const prevState = room.getLiveTimeline().getState(Direction.Forward)!;
+    const newState = makeMockRoomState(membershipData, room.roomId);
+    room.getLiveTimeline().getState = jest
+        .fn()
+        .mockReturnValue(
+            Object.assign(prevState, { events: newState.events, getStateEvents: newState.getStateEvents }),
+        );
 }
 
 export function makeMockEvent(
@@ -160,7 +173,7 @@ export function mockRTCEvent(
     roomId: string,
     stickyDuration?: number,
     timestamp?: number,
-): MatrixEvent {
+): StickyMatrixEvent {
     return {
         ...makeMockEvent(
             EventType.GroupCallMemberPrefix,
@@ -171,7 +184,7 @@ export function mockRTCEvent(
             !stickyDuration && "device_id" in membershipData ? `_${sender}_${membershipData.device_id}` : "",
         ),
         unstableStickyExpiresAt: stickyDuration,
-    } as unknown as MatrixEvent;
+    } as unknown as StickyMatrixEvent;
 }
 
 export function mockCallMembership(membershipData: MembershipData, roomId: string): CallMembership {
diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts
@@ -75,6 +75,7 @@ export type MatrixRTCSessionEventHandlerMap = {
     [MatrixRTCSessionEvent.MembershipsChanged]: (
         oldMemberships: CallMembership[],
         newMemberships: CallMembership[],
+        session: MatrixRTCSession,
     ) => void;
     [MatrixRTCSessionEvent.JoinStateChanged]: (isJoined: boolean) => void;
     [MatrixRTCSessionEvent.EncryptionKeyChanged]: (
@@ -483,11 +484,14 @@ export class MatrixRTCSession extends TypedEventEmitter<
      * this class.
      * Outside of tests this most likely will be a full room, however.
      * @deprecated Relying on a full Room object being available here is an anti-pattern. You should be tracking
-     * the room object in your own code and passing it in when needed.
+     * the room object in your own code and passing it in when needed. use roomId instead.
      */
     public get room(): Room {
         return this.roomSubset as Room;
     }
+    public get roomId(): string {
+        return this.roomSubset.roomId;
+    }
 
     /**
      * This constructs a room session. When using MatrixRTC inside the js-sdk this is expected
@@ -532,8 +536,8 @@ export class MatrixRTCSession extends TypedEventEmitter<
         super();
         this.logger = rootLogger.getChild(`[MatrixRTCSession ${roomSubset.roomId}]`);
         const roomState = this.roomSubset.getLiveTimeline().getState(EventTimeline.FORWARDS);
-        // TODO: double check if this is actually needed. Should be covered by refreshRoom in MatrixRTCSessionManager
         roomState?.on(RoomStateEvent.Members, this.onRoomMemberUpdate);
+        roomState?.on(RoomStateEvent.Events, this.onRoomStateUpdate);
         this.roomSubset.on(RoomStickyEventsEvent.Update, this.onStickyEventUpdate);
 
         this.setExpiryTimer();
@@ -557,6 +561,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
         }
         const roomState = this.roomSubset.getLiveTimeline().getState(EventTimeline.FORWARDS);
         roomState?.off(RoomStateEvent.Members, this.onRoomMemberUpdate);
+        roomState?.off(RoomStateEvent.Events, this.onRoomStateUpdate);
         this.roomSubset.off(RoomStickyEventsEvent.Update, this.onStickyEventUpdate);
     }
 
@@ -857,6 +862,13 @@ export class MatrixRTCSession extends TypedEventEmitter<
             this.recalculateSessionMembers();
         }
     };
+    /**
+     * Call this when a sticky event update has occured.
+     */
+    private readonly onRoomStateUpdate = (event: MatrixEvent): void => {
+        if (event.getType() !== EventType.GroupCallMemberPrefix) return;
+        this.recalculateSessionMembers();
+    };
 
     /**
      * Call this when something changed that may impacts the current MatrixRTC members in this session.
@@ -885,7 +897,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
                 `Memberships for call in room ${this.roomSubset.roomId} have changed: emitting (${this.memberships.length} members)`,
             );
             logDurationSync(this.logger, "emit MatrixRTCSessionEvent.MembershipsChanged", () => {
-                this.emit(MatrixRTCSessionEvent.MembershipsChanged, oldMemberships, this.memberships);
+                this.emit(MatrixRTCSessionEvent.MembershipsChanged, oldMemberships, this.memberships, this);
             });
 
             void this.membershipManager?.onRTCSessionMemberUpdate(this.memberships);
diff --git a/src/matrixrtc/MatrixRTCSessionManager.ts b/src/matrixrtc/MatrixRTCSessionManager.ts
@@ -20,8 +20,9 @@ import { TypedEventEmitter } from "../models/typed-event-emitter.ts";
 import { type Room } from "../models/room.ts";
 import { RoomStateEvent } from "../models/room-state.ts";
 import { type MatrixEvent } from "../models/event.ts";
-import { MatrixRTCSession, type SlotDescription } from "./MatrixRTCSession.ts";
+import { MatrixRTCSession, MatrixRTCSessionEvent, type SlotDescription } from "./MatrixRTCSession.ts";
 import { EventType } from "../@types/event.ts";
+import { type CallMembership } from "./CallMembership.ts";
 
 export enum MatrixRTCSessionManagerEvents {
     // A member has joined the MatrixRTC session, creating an active session in a room where there wasn't previously
@@ -66,10 +67,7 @@ export class MatrixRTCSessionManager extends TypedEventEmitter<MatrixRTCSessionM
         // We shouldn't need to null-check here, but matrix-client.spec.ts mocks getRooms
         // returning nothing, and breaks tests if you change it to return an empty array :'(
         for (const room of this.client.getRooms() ?? []) {
-            const session = MatrixRTCSession.sessionForRoom(this.client, room, this.slotDescription);
-            if (session.memberships.length > 0) {
-                this.roomSessions.set(room.roomId, session);
-            }
+            this.createSessionIfNeeded(room);
         }
 
         this.client.on(ClientEvent.Room, this.onRoom);
@@ -79,10 +77,10 @@ export class MatrixRTCSessionManager extends TypedEventEmitter<MatrixRTCSessionM
 
     public stop(): void {
         for (const sess of this.roomSessions.values()) {
+            sess.off(MatrixRTCSessionEvent.MembershipsChanged, this.onRtcMembershipChange);
             void sess.stop();
         }
         this.roomSessions.clear();
-
         this.client.off(ClientEvent.Room, this.onRoom);
         this.client.off(ClientEvent.Event, this.onEvent);
         this.client.off(RoomStateEvent.Events, this.onRoomState);
@@ -98,67 +96,59 @@ export class MatrixRTCSessionManager extends TypedEventEmitter<MatrixRTCSessionM
 
     /**
      * Gets the main MatrixRTC session for a room, returning an empty session
-     * if no members are currently participating
+     * if no members are currently participating.
+     * Getting the session will create it if needed (and setup listeners and emit a SessionStarted event if it has members)
      */
     public getRoomSession(room: Room): MatrixRTCSession {
-        if (!this.roomSessions.has(room.roomId)) {
-            this.roomSessions.set(
-                room.roomId,
-                MatrixRTCSession.sessionForRoom(this.client, room, this.slotDescription),
-            );
-        }
-
+        this.createSessionIfNeeded(room);
         return this.roomSessions.get(room.roomId)!;
     }
 
-    private onRoom = (room: Room): void => {
-        this.refreshRoom(room);
+    private createRoomSession(room: Room): MatrixRTCSession {
+        const sess = MatrixRTCSession.sessionForSlot(this.client, room, this.slotDescription);
+        this.roomSessions.set(room.roomId, sess);
+
+        sess.on(MatrixRTCSessionEvent.MembershipsChanged, this.onRtcMembershipChange);
+        this.logger.trace(`Session started for ${room.roomId} (${sess.memberships.length} members)`);
+        if (sess.memberships.length > 0) this.emit(MatrixRTCSessionManagerEvents.SessionStarted, room.roomId, sess);
+        return sess;
+    }
+
+    private readonly onRtcMembershipChange = (
+        oldM: CallMembership[],
+        newM: CallMembership[],
+        session: MatrixRTCSession,
+    ): void => {
+        if (oldM.length > 0 && newM.length === 0) {
+            this.logger.trace(`Session ended for ${session.roomId}`);
+            this.emit(MatrixRTCSessionManagerEvents.SessionEnded, session.roomId, session);
+        } else if (oldM.length === 0 && newM.length > 0) {
+            this.logger.trace(`Session started for ${session.roomId}`);
+            this.emit(MatrixRTCSessionManagerEvents.SessionStarted, session.roomId, session);
+        }
     };
 
+    // Possible cases in which we need to create a session if one doesn't already exist:
+    private createSessionIfNeeded(room: Room): void {
+        if (!this.roomSessions.has(room.roomId)) this.createRoomSession(room);
+    }
+    private onRoom = (room: Room): void => {
+        this.createSessionIfNeeded(room);
+    };
     private readonly onEvent = (event: MatrixEvent): void => {
-        if (!event.unstableStickyExpiresAt) return; // Not sticky, not interested.
-
         if (event.getType() !== EventType.GroupCallMemberPrefix) return;
+        if (!event.unstableStickyExpiresAt) return; // Not sticky, not interested.
 
         const room = this.client.getRoom(event.getRoomId());
         if (!room) return;
 
-        this.refreshRoom(room);
+        this.createSessionIfNeeded(room);
     };
-
     private readonly onRoomState = (event: MatrixEvent): void => {
-        if (event.getType() !== EventType.GroupCallMemberPrefix) {
-            return;
-        }
-        const room = this.client.getRoom(event.getRoomId());
-        if (!room) {
-            this.logger.error(`Got room state event for unknown room ${event.getRoomId()}!`);
-            return;
-        }
+        if (event.getType() !== EventType.GroupCallMemberPrefix) return;
 
-        this.refreshRoom(room);
+        const room = this.client.getRoom(event.getRoomId());
+        if (!room) return;
+        this.createSessionIfNeeded(room);
     };
-
-    private refreshRoom(room: Room): void {
-        const isNewSession = !this.roomSessions.has(room.roomId);
-        const session = this.getRoomSession(room);
-
-        const wasActiveAndKnown = session.memberships.length > 0 && !isNewSession;
-        // This needs to be here and the event listener cannot be setup in the MatrixRTCSession,
-        // because we need the update to happen between:
-        // wasActiveAndKnown = session.memberships.length > 0 and
-        // nowActive = session.memberships.length
-        // Alternatively we would need to setup some event emission when the RTC session ended.
-        session.onRTCSessionMemberUpdate();
-
-        const nowActive = session.memberships.length > 0;
-
-        if (wasActiveAndKnown && !nowActive) {
-            this.logger.trace(`Session ended for ${room.roomId} (${session.memberships.length} members)`);
-            this.emit(MatrixRTCSessionManagerEvents.SessionEnded, room.roomId, this.roomSessions.get(room.roomId)!);
-        } else if (!wasActiveAndKnown && nowActive) {
-            this.logger.trace(`Session started for ${room.roomId} (${session.memberships.length} members)`);
-            this.emit(MatrixRTCSessionManagerEvents.SessionStarted, room.roomId, this.roomSessions.get(room.roomId)!);
-        }
-    }
 }
