From e98ce7802718fba194cd1da11e1a4479f724fd85 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Mon, 20 Nov 2023 18:20:04 +0100 Subject: [PATCH] Better fallback for the event `localTimestamp` calculation. (#3900) * better fallback to localTimestamp calculation Signed-off-by: Timo K * make `isExpired` impl simpler to read Signed-off-by: Timo K * update tests Signed-off-by: Timo K * refactor to use localTimestamp in the mocks Signed-off-by: Timo K * Update src/models/event.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update src/models/event.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update and clarify comments. So that the localTimestamp and localAge behavior is easier to understand. Signed-off-by: Timo K * Replace localTimestamp biding with binding the whole roomState Signed-off-by: Timo K --------- Signed-off-by: Timo K Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- spec/unit/matrixrtc/CallMembership.spec.ts | 2 +- spec/unit/matrixrtc/MatrixRTCSession.spec.ts | 75 ++++++++++---------- spec/unit/matrixrtc/mocks.ts | 29 +++----- src/matrixrtc/CallMembership.ts | 2 +- src/models/event.ts | 8 ++- 5 files changed, 54 insertions(+), 62 deletions(-) diff --git a/spec/unit/matrixrtc/CallMembership.spec.ts b/spec/unit/matrixrtc/CallMembership.spec.ts index 4bc5492e0..b72d1c798 100644 --- a/spec/unit/matrixrtc/CallMembership.spec.ts +++ b/spec/unit/matrixrtc/CallMembership.spec.ts @@ -85,7 +85,7 @@ describe("CallMembership", () => { it("considers memberships expired when local age large", () => { const fakeEvent = makeMockEvent(1000); - fakeEvent.getLocalAge = jest.fn().mockReturnValue(6000); + fakeEvent.localTimestamp = Date.now() - 6000; const membership = new CallMembership(fakeEvent, membershipTemplate); expect(membership.isExpired()).toEqual(true); }); diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts index 6156ad15e..af71a90b0 100644 --- a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts +++ b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts @@ -67,7 +67,7 @@ describe("MatrixRTCSession", () => { const expiredMembership = Object.assign({}, membershipTemplate); expiredMembership.expires = 1000; expiredMembership.device_id = "EXPIRED"; - const mockRoom = makeMockRoom([membershipTemplate, expiredMembership], () => 10000); + const mockRoom = makeMockRoom([membershipTemplate, expiredMembership], 10000); sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom); expect(sess?.memberships.length).toEqual(1); @@ -266,7 +266,7 @@ describe("MatrixRTCSession", () => { const timeElapsed = 60 * 60 * 1000 - 1000; mockRoom.getLiveTimeline().getState(EventTimeline.FORWARDS)!.getStateEvents = jest .fn() - .mockReturnValue(mockRTCEvent(eventContent.memberships, mockRoom.roomId, () => timeElapsed)); + .mockReturnValue(mockRTCEvent(eventContent.memberships, mockRoom.roomId, timeElapsed)); const eventReSentPromise = new Promise>((r) => { resolveFn = (_roomId: string, _type: string, val: Record) => { @@ -539,10 +539,8 @@ describe("MatrixRTCSession", () => { it("emits an event at the time a membership event expires", () => { jest.useFakeTimers(); try { - let eventAge = 0; - const membership = Object.assign({}, membershipTemplate); - const mockRoom = makeMockRoom([membership], () => eventAge); + const mockRoom = makeMockRoom([membership], 0); sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom); const membershipObject = sess.memberships[0]; @@ -550,7 +548,6 @@ describe("MatrixRTCSession", () => { const onMembershipsChanged = jest.fn(); sess.on(MatrixRTCSessionEvent.MembershipsChanged, onMembershipsChanged); - eventAge = 61 * 1000 * 1000; jest.advanceTimersByTime(61 * 1000 * 1000); expect(onMembershipsChanged).toHaveBeenCalledWith([membershipObject], []); @@ -561,47 +558,49 @@ describe("MatrixRTCSession", () => { }); it("prunes expired memberships on update", () => { - client.sendStateEvent = jest.fn(); + jest.useFakeTimers(); + try { + client.sendStateEvent = jest.fn(); - let eventAge = 0; - - const mockRoom = makeMockRoom( - [ + const mockMemberships = [ Object.assign({}, membershipTemplate, { device_id: "OTHERDEVICE", expires: 1000, }), - ], - () => eventAge, - ); - sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom); + ]; + const mockRoomNoExpired = makeMockRoom(mockMemberships, 0); - // sanity check - expect(sess.memberships).toHaveLength(1); - expect(sess.memberships[0].deviceId).toEqual("OTHERDEVICE"); + sess = MatrixRTCSession.roomSessionForRoom(client, mockRoomNoExpired); - eventAge = 10000; + // sanity check + expect(sess.memberships).toHaveLength(1); + expect(sess.memberships[0].deviceId).toEqual("OTHERDEVICE"); - sess.joinRoomSession([mockFocus]); + jest.advanceTimersByTime(10000); - expect(client.sendStateEvent).toHaveBeenCalledWith( - mockRoom!.roomId, - EventType.GroupCallMemberPrefix, - { - memberships: [ - { - application: "m.call", - scope: "m.room", - call_id: "", - device_id: "AAAAAAA", - expires: 3600000, - foci_active: [mockFocus], - membershipID: expect.stringMatching(".*"), - }, - ], - }, - "@alice:example.org", - ); + sess.joinRoomSession([mockFocus]); + + expect(client.sendStateEvent).toHaveBeenCalledWith( + mockRoomNoExpired!.roomId, + EventType.GroupCallMemberPrefix, + { + memberships: [ + { + application: "m.call", + scope: "m.room", + call_id: "", + device_id: "AAAAAAA", + expires: 3600000, + foci_active: [mockFocus], + membershipID: expect.stringMatching(".*"), + }, + ], + }, + "@alice:example.org", + ); + } finally { + jest.useRealTimers(); + } }); it("fills in created_ts for other memberships on update", () => { diff --git a/spec/unit/matrixrtc/mocks.ts b/spec/unit/matrixrtc/mocks.ts index f710c49ab..e342eb2dc 100644 --- a/spec/unit/matrixrtc/mocks.ts +++ b/spec/unit/matrixrtc/mocks.ts @@ -18,41 +18,29 @@ import { EventType, MatrixEvent, Room } from "../../../src"; import { CallMembershipData } from "../../../src/matrixrtc/CallMembership"; import { randomString } from "../../../src/randomstring"; -export function makeMockRoom( - memberships: CallMembershipData[], - getLocalAge: (() => number) | undefined = undefined, -): Room { +export function makeMockRoom(memberships: CallMembershipData[], localAge: number | null = null): Room { const roomId = randomString(8); + // Caching roomState here so it does not get recreated when calling `getLiveTimeline.getState()` + const roomState = makeMockRoomState(memberships, roomId, localAge); return { roomId: roomId, getLiveTimeline: jest.fn().mockReturnValue({ - getState: jest.fn().mockReturnValue(makeMockRoomState(memberships, roomId, getLocalAge)), + getState: jest.fn().mockReturnValue(roomState), }), } as unknown as Room; } -export function makeMockRoomState( - memberships: CallMembershipData[], - roomId: string, - getLocalAge: (() => number) | undefined, -) { +export function makeMockRoomState(memberships: CallMembershipData[], roomId: string, localAge: number | null = null) { + const event = mockRTCEvent(memberships, roomId, localAge); return { getStateEvents: (_: string, stateKey: string) => { - const event = mockRTCEvent(memberships, roomId, getLocalAge); - if (stateKey !== undefined) return event; return [event]; }, }; } -export function mockRTCEvent( - memberships: CallMembershipData[], - roomId: string, - getLocalAge: (() => number) | undefined, -): MatrixEvent { - const getLocalAgeFn = getLocalAge ?? (() => 10); - +export function mockRTCEvent(memberships: CallMembershipData[], roomId: string, localAge: number | null): MatrixEvent { return { getType: jest.fn().mockReturnValue(EventType.GroupCallMemberPrefix), getContent: jest.fn().mockReturnValue({ @@ -60,8 +48,7 @@ export function mockRTCEvent( }), getSender: jest.fn().mockReturnValue("@mock:user.example"), getTs: jest.fn().mockReturnValue(1000), - getLocalAge: getLocalAgeFn, - localTimestamp: Date.now(), + localTimestamp: Date.now() - (localAge ?? 10), getRoomId: jest.fn().mockReturnValue(roomId), sender: { userId: "@mock:user.example", diff --git a/src/matrixrtc/CallMembership.ts b/src/matrixrtc/CallMembership.ts index d304c9df4..d24b32add 100644 --- a/src/matrixrtc/CallMembership.ts +++ b/src/matrixrtc/CallMembership.ts @@ -91,7 +91,7 @@ export class CallMembership { } public isExpired(): boolean { - return this.getAbsoluteExpiry() < this.parentEvent.getTs() + this.parentEvent.getLocalAge(); + return this.getMsUntilExpiry() <= 0; } public getActiveFoci(): Focus[] { diff --git a/src/models/event.ts b/src/models/event.ts index 7d919d02c..370f94c29 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -404,7 +404,13 @@ export class MatrixEvent extends TypedEventEmitter