From 725336ffc6c913d5a64018da0848b04b99f851e1 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2022 17:41:54 +0100 Subject: [PATCH 1/6] sliding sync: add invited|joined_count This is critical for calculating client-side push rules correctly. Without it, the push processor may think rooms have a different number of members, resulting typically in annoying failure modes where rooms constantly make noises because the code thinks they are 1:1 rooms. --- spec/integ/sliding-sync-sdk.spec.ts | 33 +++++++++++++++++++++++++++++ src/sliding-sync-sdk.ts | 7 ++++++ src/sliding-sync.ts | 2 ++ 3 files changed, 42 insertions(+) diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 34d84bdda..299fe0027 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/spec/integ/sliding-sync-sdk.spec.ts @@ -168,6 +168,7 @@ describe("SlidingSyncSdk", () => { const roomD = "!d_with_notif_count:localhost"; const roomE = "!e_with_invite:localhost"; const roomF = "!f_calc_room_name:localhost"; + const roomG = "!g_join_invite_counts:localhost"; const data: Record = { [roomA]: { name: "A", @@ -261,6 +262,18 @@ describe("SlidingSyncSdk", () => { ], initial: true, }, + [roomG]: { + name: "G", + required_state: [], + timeline: [ + mkOwnStateEvent(EventType.RoomCreate, { creator: selfUserId }, ""), + mkOwnStateEvent(EventType.RoomMember, { membership: "join" }, selfUserId), + mkOwnStateEvent(EventType.RoomPowerLevels, { users: { [selfUserId]: 100 } }, ""), + ], + joined_count: 5, + invited_count: 2, + initial: true, + }, }; it("can be created with required_state and timeline", () => { @@ -299,6 +312,14 @@ describe("SlidingSyncSdk", () => { ).toEqual(data[roomD].notification_count); }); + it("can be created with an invited/joined_count", () => { + mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomG, data[roomG]); + const gotRoom = client.getRoom(roomG); + expect(gotRoom).toBeDefined(); + expect(gotRoom.getInvitedMemberCount()).toEqual(data[roomG].invited_count); + expect(gotRoom.getJoinedMemberCount()).toEqual(data[roomG].joined_count); + }); + it("can be created with invite_state", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomE, data[roomE]); const gotRoom = client.getRoom(roomE); @@ -374,6 +395,18 @@ describe("SlidingSyncSdk", () => { ).toEqual(1); }); + it("can update with a new joined_count", () => { + mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomG, { + name: data[roomD].name, + required_state: [], + timeline: [], + joined_count: 1, + }); + const gotRoom = client.getRoom(roomG); + expect(gotRoom).toBeDefined(); + expect(gotRoom.getJoinedMemberCount()).toEqual(1); + }); + // Regression test for a bug which caused the timeline entries to be out-of-order // when the same room appears twice with different timeline limits. E.g appears in // the list with timeline_limit:1 then appears again as a room subscription with diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index 99f33c9e5..c20c001bf 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -472,6 +472,13 @@ export class SlidingSyncSdk { } } + if (Number.isInteger(roomData.invited_count)) { + room.currentState.setInvitedMemberCount(roomData.invited_count); + } + if (Number.isInteger(roomData.joined_count)) { + room.currentState.setJoinedMemberCount(roomData.joined_count); + } + if (roomData.invite_state) { const inviteStateEvents = mapEvents(this.client, room.roomId, roomData.invite_state); this.processRoomEvents(room, inviteStateEvents); diff --git a/src/sliding-sync.ts b/src/sliding-sync.ts index 5b5ec6a65..175e2f68a 100644 --- a/src/sliding-sync.ts +++ b/src/sliding-sync.ts @@ -84,6 +84,8 @@ export interface MSC3575RoomData { timeline: (IRoomEvent | IStateEvent)[]; notification_count?: number; highlight_count?: number; + joined_count?: number; + invited_count?: number; invite_state?: IStateEvent[]; initial?: boolean; limited?: boolean; From 818b70554aa824cd6dcb519bb570d9d969d13070 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2022 17:50:48 +0100 Subject: [PATCH 2/6] Strict TS checks --- spec/integ/sliding-sync-sdk.spec.ts | 13 +++++++++++++ src/sliding-sync-sdk.ts | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 299fe0027..8244b5f9c 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/spec/integ/sliding-sync-sdk.spec.ts @@ -280,6 +280,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomA, data[roomA]); const gotRoom = client.getRoom(roomA); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.name).toEqual(data[roomA].name); expect(gotRoom.getMyMembership()).toEqual("join"); assertTimelineEvents(gotRoom.getLiveTimeline().getEvents().slice(-2), data[roomA].timeline); @@ -289,6 +290,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomB, data[roomB]); const gotRoom = client.getRoom(roomB); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.name).toEqual(data[roomB].name); expect(gotRoom.getMyMembership()).toEqual("join"); assertTimelineEvents(gotRoom.getLiveTimeline().getEvents().slice(-5), data[roomB].timeline); @@ -298,6 +300,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomC, data[roomC]); const gotRoom = client.getRoom(roomC); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect( gotRoom.getUnreadNotificationCount(NotificationCountType.Highlight), ).toEqual(data[roomC].highlight_count); @@ -307,6 +310,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomD, data[roomD]); const gotRoom = client.getRoom(roomD); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect( gotRoom.getUnreadNotificationCount(NotificationCountType.Total), ).toEqual(data[roomD].notification_count); @@ -316,6 +320,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomG, data[roomG]); const gotRoom = client.getRoom(roomG); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.getInvitedMemberCount()).toEqual(data[roomG].invited_count); expect(gotRoom.getJoinedMemberCount()).toEqual(data[roomG].joined_count); }); @@ -324,6 +329,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomE, data[roomE]); const gotRoom = client.getRoom(roomE); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.getMyMembership()).toEqual("invite"); expect(gotRoom.currentState.getJoinRule()).toEqual(JoinRule.Invite); }); @@ -332,6 +338,7 @@ describe("SlidingSyncSdk", () => { mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomF, data[roomF]); const gotRoom = client.getRoom(roomF); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect( gotRoom.name, ).toEqual(data[roomF].name); @@ -347,6 +354,7 @@ describe("SlidingSyncSdk", () => { }); const gotRoom = client.getRoom(roomA); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } const newTimeline = data[roomA].timeline; newTimeline.push(newEvent); assertTimelineEvents(gotRoom.getLiveTimeline().getEvents().slice(-3), newTimeline); @@ -364,6 +372,7 @@ describe("SlidingSyncSdk", () => { }); gotRoom = client.getRoom(roomB); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.getJoinRule()).toEqual(JoinRule.Restricted); }); @@ -376,6 +385,7 @@ describe("SlidingSyncSdk", () => { }); const gotRoom = client.getRoom(roomC); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect( gotRoom.getUnreadNotificationCount(NotificationCountType.Highlight), ).toEqual(1); @@ -390,6 +400,7 @@ describe("SlidingSyncSdk", () => { }); const gotRoom = client.getRoom(roomD); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect( gotRoom.getUnreadNotificationCount(NotificationCountType.Total), ).toEqual(1); @@ -404,6 +415,7 @@ describe("SlidingSyncSdk", () => { }); const gotRoom = client.getRoom(roomG); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.getJoinedMemberCount()).toEqual(1); }); @@ -427,6 +439,7 @@ describe("SlidingSyncSdk", () => { }); const gotRoom = client.getRoom(roomA); expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } logger.log("want:", oldTimeline.map((e) => (e.type + " : " + (e.content || {}).body))); logger.log("got:", gotRoom.getLiveTimeline().getEvents().map( diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index c20c001bf..aab0ea45f 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -473,10 +473,10 @@ export class SlidingSyncSdk { } if (Number.isInteger(roomData.invited_count)) { - room.currentState.setInvitedMemberCount(roomData.invited_count); + room.currentState.setInvitedMemberCount(roomData.invited_count!); } if (Number.isInteger(roomData.joined_count)) { - room.currentState.setJoinedMemberCount(roomData.joined_count); + room.currentState.setJoinedMemberCount(roomData.joined_count!); } if (roomData.invite_state) { From 2d9556c1bbbebc3d57f3e30eb6a865357e28a186 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2022 18:00:03 +0100 Subject: [PATCH 3/6] More strict TS type checking --- spec/integ/sliding-sync-sdk.spec.ts | 2 ++ src/sliding-sync.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 8244b5f9c..f7dc68754 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/spec/integ/sliding-sync-sdk.spec.ts @@ -362,6 +362,8 @@ describe("SlidingSyncSdk", () => { it("can update with a new required_state event", async () => { let gotRoom = client.getRoom(roomB); + expect(gotRoom).toBeDefined(); + if (gotRoom == null) { return; } expect(gotRoom.getJoinRule()).toEqual(JoinRule.Invite); // default mockSlidingSync.emit(SlidingSyncEvent.RoomData, roomB, { required_state: [ diff --git a/src/sliding-sync.ts b/src/sliding-sync.ts index 175e2f68a..5730c8f36 100644 --- a/src/sliding-sync.ts +++ b/src/sliding-sync.ts @@ -322,7 +322,7 @@ export enum SlidingSyncEvent { export type SlidingSyncEventHandlerMap = { [SlidingSyncEvent.RoomData]: (roomId: string, roomData: MSC3575RoomData) => void; - [SlidingSyncEvent.Lifecycle]: (state: SlidingSyncState, resp: MSC3575SlidingSyncResponse, err: Error) => void; + [SlidingSyncEvent.Lifecycle]: (state: SlidingSyncState, resp: MSC3575SlidingSyncResponse | null, err: Error) => void; [SlidingSyncEvent.List]: ( listIndex: number, joinedCount: number, roomIndexToRoomId: Record, ) => void; From c32a83fdac4ba6fac791b01fb94d86f68b28f824 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2022 18:02:56 +0100 Subject: [PATCH 4/6] Linting --- src/sliding-sync.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sliding-sync.ts b/src/sliding-sync.ts index 5730c8f36..712564e9d 100644 --- a/src/sliding-sync.ts +++ b/src/sliding-sync.ts @@ -322,7 +322,9 @@ export enum SlidingSyncEvent { export type SlidingSyncEventHandlerMap = { [SlidingSyncEvent.RoomData]: (roomId: string, roomData: MSC3575RoomData) => void; - [SlidingSyncEvent.Lifecycle]: (state: SlidingSyncState, resp: MSC3575SlidingSyncResponse | null, err: Error) => void; + [SlidingSyncEvent.Lifecycle]: ( + state: SlidingSyncState, resp: MSC3575SlidingSyncResponse | null, err: Error, + ) => void; [SlidingSyncEvent.List]: ( listIndex: number, joinedCount: number, roomIndexToRoomId: Record, ) => void; From b5576758e4bd2d3cf1be7a1f7de777a6eb9daa1c Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2022 18:07:53 +0100 Subject: [PATCH 5/6] Even more strict TS type checking --- src/sliding-sync-sdk.ts | 5 ++++- src/sliding-sync.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index aab0ea45f..233c0c31d 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -293,13 +293,16 @@ export class SlidingSyncSdk { this.processRoomData(this.client, room, roomData); } - private onLifecycle(state: SlidingSyncState, resp: MSC3575SlidingSyncResponse, err?: Error): void { + private onLifecycle(state: SlidingSyncState, resp?: MSC3575SlidingSyncResponse, err?: Error): void { if (err) { logger.debug("onLifecycle", state, err); } switch (state) { case SlidingSyncState.Complete: this.purgeNotifications(); + if (!resp) { + break; + } // Element won't stop showing the initial loading spinner unless we fire SyncState.Prepared if (!this.lastPos) { this.updateSyncState(SyncState.Prepared, { diff --git a/src/sliding-sync.ts b/src/sliding-sync.ts index 712564e9d..da6419c96 100644 --- a/src/sliding-sync.ts +++ b/src/sliding-sync.ts @@ -323,7 +323,7 @@ export enum SlidingSyncEvent { export type SlidingSyncEventHandlerMap = { [SlidingSyncEvent.RoomData]: (roomId: string, roomData: MSC3575RoomData) => void; [SlidingSyncEvent.Lifecycle]: ( - state: SlidingSyncState, resp: MSC3575SlidingSyncResponse | null, err: Error, + state: SlidingSyncState, resp: MSC3575SlidingSyncResponse | null, err: Error | null, ) => void; [SlidingSyncEvent.List]: ( listIndex: number, joinedCount: number, roomIndexToRoomId: Record, From ac7f505a2b67b1a6157efe07afcf602803e33f95 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2022 18:13:37 +0100 Subject: [PATCH 6/6] Use the right nulls --- src/sliding-sync-sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index 233c0c31d..21d2af63f 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -293,7 +293,7 @@ export class SlidingSyncSdk { this.processRoomData(this.client, room, roomData); } - private onLifecycle(state: SlidingSyncState, resp?: MSC3575SlidingSyncResponse, err?: Error): void { + private onLifecycle(state: SlidingSyncState, resp: MSC3575SlidingSyncResponse | null, err: Error | null): void { if (err) { logger.debug("onLifecycle", state, err); }