PR review suggestions for https://github.com/matrix-org/matrix-js-sdk/pull/4583

spec/unit/matrixrtc/CallMembership.spec.ts

@@ -15,8 +15,7 @@ limitations under the License.
 */
 
 import { MatrixEvent } from "../../../src";
-import { CallMembership, DEFAULT_EXPIRE_DURATION, SessionMembershipData } from "../../../src/matrixrtc/CallMembership";
-import { membershipTemplate } from "./mocks";
+import { CallMembership, SessionMembershipData } from "../../../src/matrixrtc/CallMembership";
 
 function makeMockEvent(originTs = 0): MatrixEvent {
     return {
@@ -89,6 +88,15 @@ describe("CallMembership", () => {
     describe("expiry calculation", () => {
         let fakeEvent: MatrixEvent;
         let membership: CallMembership;
+        const membershipTemplate: SessionMembershipData = {
+            call_id: "",
+            scope: "m.room",
+            application: "m.call",
+            device_id: "AAAAAAA",
+            expires: 5000,
+            focus_active: { type: "livekit" },
+            foci_preferred: [{ type: "livekit" }],
+        };
 
         beforeEach(() => {
             // server origin timestamp for this event is 1000
@@ -102,24 +110,10 @@ describe("CallMembership", () => {
             jest.useRealTimers();
         });
 
-        it("converts expiry time into local clock", () => {
-            // our clock would have been at 2000 at the creation time (our clock at event receive time - age)
-            // (ie. the local clock is 1 second ahead of the servers' clocks)
-            fakeEvent.localTimestamp = 2000;
-
-            // for simplicity's sake, we say that the event's age is zero
-            fakeEvent.getLocalAge = jest.fn().mockReturnValue(0);
-
-            // for sanity's sake, make sure the server-relative expiry time is what we expect
-            expect(membership.getAbsoluteExpiry()).toEqual(DEFAULT_EXPIRE_DURATION + 1000);
-            // therefore the expiry time converted to our clock should be 1 second later
-            expect(membership.getLocalExpiry()).toEqual(DEFAULT_EXPIRE_DURATION + 2000);
-        });
-
         it("calculates time until expiry", () => {
             jest.setSystemTime(2000);
             // should be using absolute expiry time
-            expect(membership.getMsUntilExpiry()).toEqual(DEFAULT_EXPIRE_DURATION - 1000);
+            expect(membership.getMsUntilExpiry()).toEqual(4000);
         });
     });
 });

spec/unit/matrixrtc/MatrixRTCSession.spec.ts

@@ -20,7 +20,17 @@ import { SessionMembershipData } from "../../../src/matrixrtc/CallMembership";
 import { MatrixRTCSession, MatrixRTCSessionEvent } from "../../../src/matrixrtc/MatrixRTCSession";
 import { EncryptionKeysEventContent } from "../../../src/matrixrtc/types";
 import { randomString } from "../../../src/randomstring";
-import { makeMockRoom, makeMockRoomState, membershipTemplate } from "./mocks";
+import { makeMockRoom, makeMockRoomState } from "./mocks";
+
+const membershipTemplate: SessionMembershipData = {
+    call_id: "",
+    scope: "m.room",
+    application: "m.call",
+    device_id: "AAAAAAA",
+    expires: 60 * 60 * 1000,
+    focus_active: { type: "livekit", livekit_service_url: "https://lk.url" },
+    foci_preferred: [{ type: "livekit", livekit_service_url: "https://lk.url" }],
+};
 
 const mockFocus = { type: "mock" };
 
@@ -248,7 +258,7 @@ describe("MatrixRTCSession", () => {
             expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(1);
         }
 
-        it("uses non-legacy events if there are only non-legacy calls", async () => {
+        it("sends events", async () => {
             await testSession(sessionMembershipData);
         });
     });
@@ -398,7 +408,7 @@ describe("MatrixRTCSession", () => {
                         application: "m.call",
                         scope: "m.room",
                         call_id: "",
-                        expires: 2400000,
+                        expires: 3600000,
                         device_id: "AAAAAAA",
                         foci_preferred: [activeFocusConfig],
                         focus_active: activeFocus,
@@ -418,7 +428,7 @@ describe("MatrixRTCSession", () => {
                 jest.useRealTimers();
             }
 
-            it("sends a membership event with session payload when joining a non-legacy call", async () => {
+            it("sends a membership event with session payload when joining a call", async () => {
                 await testJoin(false);
             });
 

spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts

@@ -27,7 +27,18 @@ import {
 } from "../../../src";
 import { RoomStateEvent } from "../../../src/models/room-state";
 import { MatrixRTCSessionManagerEvents } from "../../../src/matrixrtc/MatrixRTCSessionManager";
-import { makeMockRoom, makeMockRoomState, membershipTemplate } from "./mocks";
+import { makeMockRoom, makeMockRoomState } from "./mocks";
+import { SessionMembershipData } from "../../../src/matrixrtc";
+
+const membershipTemplate: SessionMembershipData = {
+    call_id: "",
+    scope: "m.room",
+    application: "m.call",
+    device_id: "AAAAAAA",
+    expires: 60 * 60 * 1000,
+    focus_active: { type: "test" },
+    foci_preferred: [{ type: "test" }],
+};
 
 describe("MatrixRTCSessionManager", () => {
     let client: MatrixClient;

spec/unit/matrixrtc/mocks.ts

@@ -20,26 +20,6 @@ import { randomString } from "../../../src/randomstring";
 
 type MembershipData = SessionMembershipData[] | SessionMembershipData | {};
 
-export const membershipTemplate: SessionMembershipData = {
-    application: "m.call",
-    call_id: "",
-    device_id: "AAAAAAA",
-    scope: "m.room",
-    focus_active: { type: "livekit", livekit_service_url: "https://lk.url" },
-    foci_preferred: [
-        {
-            livekit_alias: "!alias:something.org",
-            livekit_service_url: "https://livekit-jwt.something.io",
-            type: "livekit",
-        },
-        {
-            livekit_alias: "!alias:something.org",
-            livekit_service_url: "https://livekit-jwt.something.dev",
-            type: "livekit",
-        },
-    ],
-};
-
 export function makeMockRoom(membershipData: MembershipData): Room {
     const roomId = randomString(8);
     // Caching roomState here so it does not get recreated when calling `getLiveTimeline.getState()`

src/matrixrtc/CallMembership.ts

@@ -19,15 +19,14 @@ import { deepCompare } from "../utils.ts";
 import { Focus } from "./focus.ts";
 import { isLivekitFocusActive } from "./LivekitFocus.ts";
 
-export const DEFAULT_EXPIRE_DURATION = 1000 * 60 * 10 * 4; // 4 hours
+export const DEFAULT_EXPIRE_DURATION = 1000 * 60 * 60; // 1 hour
 
 type CallScope = "m.room" | "m.user";
-// Represents an entry in the memberships section of an m.call.member event as it is on the wire
-
-// There are two different data interfaces. One for the Legacy types and one compliant with MSC4143
-
-// MSC4143 (MatrixRTC) session membership data
 
+/**
+ * MSC4143 (MatrixRTC) session membership data.
+ * Represents an entry in the memberships section of an m.call.member event as it is on the wire.
+ **/
 export type SessionMembershipData = {
     application: string;
     call_id: string;
@@ -40,8 +39,10 @@ export type SessionMembershipData = {
     // Application specific data
     scope?: CallScope;
 
-    // Optionally we allow to define a delta to the created_ts when it expires. This should be set to multiple hours.
-    // The only reason it exist is if delayed events fail. (for example because if a homeserver crashes)
+    /**
+     * Optionally we allow to define a delta to the created_ts when it expires. This should be set to multiple hours.
+     * The only reason it exist is if delayed events fail. (for example because if a homeserver crashes)
+     **/
     expires?: number;
 };
 
@@ -104,10 +105,6 @@ export class CallMembership {
         return this.membershipData.scope;
     }
 
-    public get expires(): number {
-        return this.membershipData.expires ?? DEFAULT_EXPIRE_DURATION;
-    }
-
     public get membershipID(): string {
         // the createdTs behaves equivalent to the membershipID.
         // we only need the field for the legacy member envents where we needed to update them
@@ -124,20 +121,8 @@ export class CallMembership {
      * @returns The absolute expiry time of the membership as a unix timestamp in milliseconds or undefined if not applicable
      */
     public getAbsoluteExpiry(): number | undefined {
-        return this.createdTs() + this.expires;
-    }
-
-    /**
-     * Gets the expiry time of the event, converted into the device's local time.
-     * @deprecated This function has been observed returning bad data and is no longer used by MatrixRTC.
-     * @returns The local expiry time of the membership as a unix timestamp in milliseconds or undefined if not applicable
-     */
-    public getLocalExpiry(): number | undefined {
-        const relativeCreationTime = this.parentEvent.getTs() - this.createdTs();
-
-        const localCreationTs = this.parentEvent.localTimestamp - relativeCreationTime;
-
-        return localCreationTs + this.expires;
+        // TODO: calculate this from the MatrixRTCSession join configuration directly
+        return this.createdTs() + (this.membershipData.expires ?? DEFAULT_EXPIRE_DURATION);
     }
 
     /**