From 21f0825703337fa5c5018588e70df92cb6a0b773 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 18 Jan 2023 17:19:12 +0000 Subject: [PATCH 01/17] refactor: sliding sync: convert to lists-as-keys rather than indexes Sister PR to https://github.com/matrix-org/matrix-js-sdk/pull/3076 --- src/SlidingSyncManager.ts | 47 +++++--------------- src/components/views/rooms/RoomSublist.tsx | 6 +-- src/hooks/useSlidingSyncRoomSearch.ts | 7 ++- src/stores/room-list/SlidingRoomListStore.ts | 20 +++------ 4 files changed, 24 insertions(+), 56 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 75a796df19..7f47e41728 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -137,10 +137,10 @@ export class SlidingSyncManager { this.listIdToIndex = {}; // by default use the encrypted subscription as that gets everything, which is a safer // default than potentially missing member events. - this.slidingSync = new SlidingSync(proxyUrl, [], ENCRYPTED_SUBSCRIPTION, client, SLIDING_SYNC_TIMEOUT_MS); + this.slidingSync = new SlidingSync(proxyUrl, new Map(), ENCRYPTED_SUBSCRIPTION, client, SLIDING_SYNC_TIMEOUT_MS); this.slidingSync.addCustomSubscription(UNENCRYPTED_SUBSCRIPTION_NAME, UNENCRYPTED_SUBSCRIPTION); // set the space list - this.slidingSync.setList(this.getOrAllocateListIndex(SlidingSyncManager.ListSpaces), { + this.slidingSync.setList(SlidingSyncManager.ListSpaces, { ranges: [[0, 20]], sort: ["by_name"], slow_get_all_rooms: true, @@ -182,38 +182,16 @@ export class SlidingSyncManager { return null; } - /** - * Allocate or retrieve the list index for an arbitrary list ID. For example SlidingSyncManager.ListSpaces - * @param listId A string which represents the list. - * @returns The index to use when registering lists or listening for callbacks. - */ - public getOrAllocateListIndex(listId: string): number { - let index = this.listIdToIndex[listId]; - if (index === undefined) { - // assign next highest index - index = -1; - for (const id in this.listIdToIndex) { - const listIndex = this.listIdToIndex[id]; - if (listIndex > index) { - index = listIndex; - } - } - index++; - this.listIdToIndex[listId] = index; - } - return index; - } - /** * Ensure that this list is registered. - * @param listIndex The list index to register + * @param listKey The list key to register * @param updateArgs The fields to update on the list. * @returns The complete list request params */ - public async ensureListRegistered(listIndex: number, updateArgs: PartialSlidingSyncRequest): Promise { - logger.debug("ensureListRegistered:::", listIndex, updateArgs); + public async ensureListRegistered(listKey: string, updateArgs: PartialSlidingSyncRequest): Promise { + logger.debug("ensureListRegistered:::", listKey, updateArgs); await this.configureDefer.promise; - let list = this.slidingSync.getList(listIndex); + let list = this.slidingSync.getListParams(listKey); if (!list) { list = { ranges: [[0, 20]], @@ -252,14 +230,14 @@ export class SlidingSyncManager { try { // if we only have range changes then call a different function so we don't nuke the list from before if (updateArgs.ranges && Object.keys(updateArgs).length === 1) { - await this.slidingSync.setListRanges(listIndex, updateArgs.ranges); + await this.slidingSync.setListRanges(listKey, updateArgs.ranges); } else { - await this.slidingSync.setList(listIndex, list); + await this.slidingSync.setList(listKey, list); } } catch (err) { logger.debug("ensureListRegistered: update failed txn_id=", err); } - return this.slidingSync.getList(listIndex); + return this.slidingSync.getListParams(listKey); } public async setRoomVisible(roomId: string, visible: boolean): Promise { @@ -304,7 +282,6 @@ export class SlidingSyncManager { */ public async startSpidering(batchSize: number, gapBetweenRequestsMs: number): Promise { await sleep(gapBetweenRequestsMs); // wait a bit as this is called on first render so let's let things load - const listIndex = this.getOrAllocateListIndex(SlidingSyncManager.ListSearch); let startIndex = batchSize; let hasMore = true; let firstTime = true; @@ -316,7 +293,7 @@ export class SlidingSyncManager { [startIndex, endIndex], ]; if (firstTime) { - await this.slidingSync.setList(listIndex, { + await this.slidingSync.setList(SlidingSyncManager.ListSearch, { // e.g [0,19] [20,39] then [0,19] [40,59]. We keep [0,20] constantly to ensure // any changes to the list whilst spidering are caught. ranges: ranges, @@ -342,7 +319,7 @@ export class SlidingSyncManager { }, }); } else { - await this.slidingSync.setListRanges(listIndex, ranges); + await this.slidingSync.setListRanges(SlidingSyncManager.ListSearch, ranges); } // gradually request more over time await sleep(gapBetweenRequestsMs); @@ -350,7 +327,7 @@ export class SlidingSyncManager { // do nothing, as we reject only when we get interrupted but that's fine as the next // request will include our data } - hasMore = endIndex + 1 < this.slidingSync.getListData(listIndex)?.joinedCount; + hasMore = endIndex + 1 < this.slidingSync.getListData(SlidingSyncManager.ListSearch)?.joinedCount; startIndex += batchSize; firstTime = false; } diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 11efcda896..7674916d70 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -340,9 +340,8 @@ export default class RoomSublist extends React.Component { private onShowAllClick = async (): Promise => { if (this.slidingSyncMode) { - const slidingSyncIndex = SlidingSyncManager.instance.getOrAllocateListIndex(this.props.tagId); const count = RoomListStore.instance.getCount(this.props.tagId); - await SlidingSyncManager.instance.ensureListRegistered(slidingSyncIndex, { + await SlidingSyncManager.instance.ensureListRegistered(this.props.tagId, { ranges: [[0, count]], }); } @@ -566,8 +565,7 @@ export default class RoomSublist extends React.Component { let isAlphabetical = RoomListStore.instance.getTagSorting(this.props.tagId) === SortAlgorithm.Alphabetic; let isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; if (this.slidingSyncMode) { - const slidingSyncIndex = SlidingSyncManager.instance.getOrAllocateListIndex(this.props.tagId); - const slidingList = SlidingSyncManager.instance.slidingSync.getList(slidingSyncIndex); + const slidingList = SlidingSyncManager.instance.slidingSync.getListParams(this.props.tagId); isAlphabetical = slidingList.sort[0] === "by_name"; isUnreadFirst = slidingList.sort[0] === "by_notification_level"; } diff --git a/src/hooks/useSlidingSyncRoomSearch.ts b/src/hooks/useSlidingSyncRoomSearch.ts index 3713ab7618..dc1caba917 100644 --- a/src/hooks/useSlidingSyncRoomSearch.ts +++ b/src/hooks/useSlidingSyncRoomSearch.ts @@ -34,7 +34,6 @@ export const useSlidingSyncRoomSearch = (): { const [rooms, setRooms] = useState([]); const [loading, setLoading] = useState(false); - const listIndex = SlidingSyncManager.instance.getOrAllocateListIndex(SlidingSyncManager.ListSearch); const [updateQuery, updateResult] = useLatestResult<{ term: string; limit?: number }, Room[]>(setRooms); @@ -50,14 +49,14 @@ export const useSlidingSyncRoomSearch = (): { try { setLoading(true); - await SlidingSyncManager.instance.ensureListRegistered(listIndex, { + await SlidingSyncManager.instance.ensureListRegistered(SlidingSyncManager.ListSearch, { ranges: [[0, limit]], filters: { room_name_like: term, }, }); const rooms = []; - const { roomIndexToRoomId } = SlidingSyncManager.instance.slidingSync.getListData(listIndex); + const { roomIndexToRoomId } = SlidingSyncManager.instance.slidingSync.getListData(SlidingSyncManager.ListSearch); let i = 0; while (roomIndexToRoomId[i]) { const roomId = roomIndexToRoomId[i]; @@ -78,7 +77,7 @@ export const useSlidingSyncRoomSearch = (): { // TODO: delete the list? } }, - [updateQuery, updateResult, listIndex], + [updateQuery, updateResult, SlidingSyncManager.ListSearch], ); return { diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index 3807dd6559..b54dd3913d 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -89,15 +89,14 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl public async setTagSorting(tagId: TagID, sort: SortAlgorithm): Promise { logger.info("SlidingRoomListStore.setTagSorting ", tagId, sort); this.tagIdToSortAlgo[tagId] = sort; - const slidingSyncIndex = this.context.slidingSyncManager.getOrAllocateListIndex(tagId); switch (sort) { case SortAlgorithm.Alphabetic: - await this.context.slidingSyncManager.ensureListRegistered(slidingSyncIndex, { + await this.context.slidingSyncManager.ensureListRegistered(tagId, { sort: SlidingSyncSortToFilter[SortAlgorithm.Alphabetic], }); break; case SortAlgorithm.Recent: - await this.context.slidingSyncManager.ensureListRegistered(slidingSyncIndex, { + await this.context.slidingSyncManager.ensureListRegistered(tagId, { sort: SlidingSyncSortToFilter[SortAlgorithm.Recent], }); break; @@ -164,8 +163,7 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl // check all lists for each tag we know about and see if the room is there const tags: TagID[] = []; for (const tagId in this.tagIdToSortAlgo) { - const index = this.context.slidingSyncManager.getOrAllocateListIndex(tagId); - const listData = this.context.slidingSyncManager.slidingSync.getListData(index); + const listData = this.context.slidingSyncManager.slidingSync.getListData(tagId); if (!listData) { continue; } @@ -259,11 +257,10 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl } private onSlidingSyncListUpdate( - listIndex: number, + tagId: string, joinCount: number, roomIndexToRoomId: Record, ): void { - const tagId = this.context.slidingSyncManager.listIdForIndex(listIndex); this.counts[tagId] = joinCount; this.refreshOrderedLists(tagId, roomIndexToRoomId); // let the UI update @@ -295,8 +292,7 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl if (room) { // resort it based on the slidingSync view of the list. This may cause this old sticky // room to cease to exist. - const index = this.context.slidingSyncManager.getOrAllocateListIndex(tagId); - const listData = this.context.slidingSyncManager.slidingSync.getListData(index); + const listData = this.context.slidingSyncManager.slidingSync.getListData(tagId); if (!listData) { continue; } @@ -334,9 +330,8 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl const sort = SortAlgorithm.Recent; // default to recency sort, TODO: read from config this.tagIdToSortAlgo[tagId] = sort; this.emit(LISTS_LOADING_EVENT, tagId, true); - const index = this.context.slidingSyncManager.getOrAllocateListIndex(tagId); this.context.slidingSyncManager - .ensureListRegistered(index, { + .ensureListRegistered(tagId, { filters: filter, sort: SlidingSyncSortToFilter[sort], }) @@ -367,9 +362,8 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl ); this.emit(LISTS_LOADING_EVENT, tagId, true); - const index = this.context.slidingSyncManager.getOrAllocateListIndex(tagId); this.context.slidingSyncManager - .ensureListRegistered(index, { + .ensureListRegistered(tagId, { filters: filters, }) .then(() => { From c34df2bf968c7896dba261736016f063717d4f1e Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 18 Jan 2023 17:24:31 +0000 Subject: [PATCH 02/17] Remove const from hook --- src/hooks/useSlidingSyncRoomSearch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useSlidingSyncRoomSearch.ts b/src/hooks/useSlidingSyncRoomSearch.ts index dc1caba917..c029d8c21b 100644 --- a/src/hooks/useSlidingSyncRoomSearch.ts +++ b/src/hooks/useSlidingSyncRoomSearch.ts @@ -77,7 +77,7 @@ export const useSlidingSyncRoomSearch = (): { // TODO: delete the list? } }, - [updateQuery, updateResult, SlidingSyncManager.ListSearch], + [updateQuery, updateResult], ); return { From 186938d32aea127e60ae1c7279836998849b8bfe Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Jan 2023 11:02:43 +0000 Subject: [PATCH 03/17] prettier --- src/SlidingSyncManager.ts | 8 +++++++- src/hooks/useSlidingSyncRoomSearch.ts | 4 +++- src/stores/room-list/SlidingRoomListStore.ts | 6 +----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 7f47e41728..0f67962681 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -137,7 +137,13 @@ export class SlidingSyncManager { this.listIdToIndex = {}; // by default use the encrypted subscription as that gets everything, which is a safer // default than potentially missing member events. - this.slidingSync = new SlidingSync(proxyUrl, new Map(), ENCRYPTED_SUBSCRIPTION, client, SLIDING_SYNC_TIMEOUT_MS); + this.slidingSync = new SlidingSync( + proxyUrl, + new Map(), + ENCRYPTED_SUBSCRIPTION, + client, + SLIDING_SYNC_TIMEOUT_MS, + ); this.slidingSync.addCustomSubscription(UNENCRYPTED_SUBSCRIPTION_NAME, UNENCRYPTED_SUBSCRIPTION); // set the space list this.slidingSync.setList(SlidingSyncManager.ListSpaces, { diff --git a/src/hooks/useSlidingSyncRoomSearch.ts b/src/hooks/useSlidingSyncRoomSearch.ts index c029d8c21b..00f26c198b 100644 --- a/src/hooks/useSlidingSyncRoomSearch.ts +++ b/src/hooks/useSlidingSyncRoomSearch.ts @@ -56,7 +56,9 @@ export const useSlidingSyncRoomSearch = (): { }, }); const rooms = []; - const { roomIndexToRoomId } = SlidingSyncManager.instance.slidingSync.getListData(SlidingSyncManager.ListSearch); + const { roomIndexToRoomId } = SlidingSyncManager.instance.slidingSync.getListData( + SlidingSyncManager.ListSearch, + ); let i = 0; while (roomIndexToRoomId[i]) { const roomId = roomIndexToRoomId[i]; diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index b54dd3913d..eeacab35f6 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -256,11 +256,7 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl this.tagMap = tagMap; } - private onSlidingSyncListUpdate( - tagId: string, - joinCount: number, - roomIndexToRoomId: Record, - ): void { + private onSlidingSyncListUpdate(tagId: string, joinCount: number, roomIndexToRoomId: Record): void { this.counts[tagId] = joinCount; this.refreshOrderedLists(tagId, roomIndexToRoomId); // let the UI update From 7c2dd7224f108e24a499e8f02023df4da6affad8 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Jan 2023 11:15:08 +0000 Subject: [PATCH 04/17] unbreak jest tests --- src/SlidingSyncManager.ts | 12 ---- test/SlidingSyncManager-test.ts | 14 ++--- .../room-list/SlidingRoomListStore-test.ts | 57 +++++-------------- 3 files changed, 22 insertions(+), 61 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 0f67962681..1489f636d4 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -119,12 +119,10 @@ export class SlidingSyncManager { public slidingSync: SlidingSync; private client: MatrixClient; - private listIdToIndex: Record; private configureDefer: IDeferred; public constructor() { - this.listIdToIndex = {}; this.configureDefer = defer(); } @@ -134,7 +132,6 @@ export class SlidingSyncManager { public configure(client: MatrixClient, proxyUrl: string): SlidingSync { this.client = client; - this.listIdToIndex = {}; // by default use the encrypted subscription as that gets everything, which is a safer // default than potentially missing member events. this.slidingSync = new SlidingSync( @@ -179,15 +176,6 @@ export class SlidingSyncManager { return this.slidingSync; } - public listIdForIndex(index: number): string | null { - for (const listId in this.listIdToIndex) { - if (this.listIdToIndex[listId] === index) { - return listId; - } - } - return null; - } - /** * Ensure that this list is registered. * @param listKey The list key to register diff --git a/test/SlidingSyncManager-test.ts b/test/SlidingSyncManager-test.ts index 58784e554d..2e47e25284 100644 --- a/test/SlidingSyncManager-test.ts +++ b/test/SlidingSyncManager-test.ts @@ -84,7 +84,7 @@ describe("SlidingSyncManager", () => { const batchSize = 10; mocked(slidingSync.setList).mockResolvedValue("yep"); mocked(slidingSync.setListRanges).mockResolvedValue("yep"); - mocked(slidingSync.getListData).mockImplementation((i) => { + mocked(slidingSync.getListData).mockImplementation((key) => { return { joinedCount: 64, roomIndexToRoomId: {}, @@ -106,7 +106,7 @@ describe("SlidingSyncManager", () => { wantWindows.forEach((range, i) => { if (i === 0) { expect(slidingSync.setList).toBeCalledWith( - manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch), + SlidingSyncManager.ListSearch, expect.objectContaining({ ranges: [[0, batchSize - 1], range], }), @@ -114,7 +114,7 @@ describe("SlidingSyncManager", () => { return; } expect(slidingSync.setListRanges).toBeCalledWith( - manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch), + SlidingSyncManager.ListSearch, [[0, batchSize - 1], range], ); }); @@ -123,7 +123,7 @@ describe("SlidingSyncManager", () => { const gapMs = 1; const batchSize = 10; mocked(slidingSync.setList).mockResolvedValue("yep"); - mocked(slidingSync.getListData).mockImplementation((i) => { + mocked(slidingSync.getListData).mockImplementation((key) => { return { joinedCount: 0, roomIndexToRoomId: {}, @@ -133,7 +133,7 @@ describe("SlidingSyncManager", () => { expect(slidingSync.getListData).toBeCalledTimes(1); expect(slidingSync.setList).toBeCalledTimes(1); expect(slidingSync.setList).toBeCalledWith( - manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch), + SlidingSyncManager.ListSearch, expect.objectContaining({ ranges: [ [0, batchSize - 1], @@ -146,7 +146,7 @@ describe("SlidingSyncManager", () => { const gapMs = 1; const batchSize = 10; mocked(slidingSync.setList).mockRejectedValue("narp"); - mocked(slidingSync.getListData).mockImplementation((i) => { + mocked(slidingSync.getListData).mockImplementation((key) => { return { joinedCount: 0, roomIndexToRoomId: {}, @@ -156,7 +156,7 @@ describe("SlidingSyncManager", () => { expect(slidingSync.getListData).toBeCalledTimes(1); expect(slidingSync.setList).toBeCalledTimes(1); expect(slidingSync.setList).toBeCalledWith( - manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch), + SlidingSyncManager.ListSearch, expect.objectContaining({ ranges: [ [0, batchSize - 1], diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 0c882ce35b..819e8dd9a8 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -42,7 +42,6 @@ describe("SlidingRoomListStore", () => { let context: TestSdkContext; let dis: MatrixDispatcher; let activeSpace: string; - let tagIdToIndex = {}; beforeEach(async () => { context = new TestSdkContext(); @@ -64,27 +63,6 @@ describe("SlidingRoomListStore", () => { getRoomId: jest.fn(), }) as unknown as RoomViewStore, ); - - // mock implementations to allow the store to map tag IDs to sliding sync list indexes and vice versa - let index = 0; - tagIdToIndex = {}; - mocked(context._SlidingSyncManager.getOrAllocateListIndex).mockImplementation((listId: string): number => { - if (tagIdToIndex[listId] != null) { - return tagIdToIndex[listId]; - } - tagIdToIndex[listId] = index; - index++; - return index; - }); - mocked(context.slidingSyncManager.listIdForIndex).mockImplementation((i) => { - for (const tagId in tagIdToIndex) { - const j = tagIdToIndex[tagId]; - if (i === j) { - return tagId; - } - } - return null; - }); mocked(context._SlidingSyncManager.ensureListRegistered).mockResolvedValue({ ranges: [[0, 10]], }); @@ -108,7 +86,7 @@ describe("SlidingRoomListStore", () => { await p; expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith( - tagIdToIndex[DefaultTagID.Untagged], + DefaultTagID.Untagged, { filters: expect.objectContaining({ spaces: [spaceRoomId], @@ -127,7 +105,7 @@ describe("SlidingRoomListStore", () => { await store.start(); // call onReady await p; expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith( - tagIdToIndex[DefaultTagID.Untagged], + DefaultTagID.Untagged, expect.objectContaining({ filters: expect.objectContaining({ spaces: [spaceRoomId], @@ -161,7 +139,7 @@ describe("SlidingRoomListStore", () => { await p; expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith( - tagIdToIndex[DefaultTagID.Untagged], + DefaultTagID.Untagged, { filters: expect.objectContaining({ spaces: [spaceRoomId, subSpace1, subSpace2], @@ -172,16 +150,15 @@ describe("SlidingRoomListStore", () => { }); it("setTagSorting alters the 'sort' option in the list", async () => { - mocked(context._SlidingSyncManager.getOrAllocateListIndex).mockReturnValue(0); const tagId: TagID = "foo"; await store.setTagSorting(tagId, SortAlgorithm.Alphabetic); - expect(context._SlidingSyncManager.ensureListRegistered).toBeCalledWith(0, { + expect(context._SlidingSyncManager.ensureListRegistered).toBeCalledWith(tagId, { sort: SlidingSyncSortToFilter[SortAlgorithm.Alphabetic], }); expect(store.getTagSorting(tagId)).toEqual(SortAlgorithm.Alphabetic); await store.setTagSorting(tagId, SortAlgorithm.Recent); - expect(context._SlidingSyncManager.ensureListRegistered).toBeCalledWith(0, { + expect(context._SlidingSyncManager.ensureListRegistered).toBeCalledWith(tagId, { sort: SlidingSyncSortToFilter[SortAlgorithm.Recent], }); expect(store.getTagSorting(tagId)).toEqual(SortAlgorithm.Recent); @@ -189,27 +166,25 @@ describe("SlidingRoomListStore", () => { it("getTagsForRoom gets the tags for the room", async () => { await store.start(); - const untaggedIndex = context._SlidingSyncManager.getOrAllocateListIndex(DefaultTagID.Untagged); - const favIndex = context._SlidingSyncManager.getOrAllocateListIndex(DefaultTagID.Favourite); const roomA = "!a:localhost"; const roomB = "!b:localhost"; - const indexToListData = { - [untaggedIndex]: { + const keyToListData = { + [DefaultTagID.Untagged]: { joinedCount: 10, roomIndexToRoomId: { 0: roomA, 1: roomB, }, }, - [favIndex]: { + [DefaultTagID.Favourite]: { joinedCount: 2, roomIndexToRoomId: { 0: roomB, }, }, }; - mocked(context._SlidingSyncManager.slidingSync.getListData).mockImplementation((i: number) => { - return indexToListData[i] || null; + mocked(context._SlidingSyncManager.slidingSync.getListData).mockImplementation((key: string) => { + return keyToListData[key] || null; }); expect(store.getTagsForRoom(new Room(roomA, context.client, context.client.getUserId()))).toEqual([ @@ -227,7 +202,6 @@ describe("SlidingRoomListStore", () => { const roomB = "!b:localhost"; const roomC = "!c:localhost"; const tagId = DefaultTagID.Favourite; - const listIndex = context.slidingSyncManager.getOrAllocateListIndex(tagId); const joinCount = 10; const roomIndexToRoomId = { // mixed to ensure we sort @@ -252,7 +226,7 @@ describe("SlidingRoomListStore", () => { return null; }); const p = untilEmission(store, LISTS_UPDATE_EVENT); - context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, listIndex, joinCount, roomIndexToRoomId); + context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); await p; expect(store.getCount(tagId)).toEqual(joinCount); expect(store.orderedLists[tagId]).toEqual(rooms); @@ -265,7 +239,6 @@ describe("SlidingRoomListStore", () => { const roomIdB = "!b:localhost"; const roomIdC = "!c:localhost"; const tagId = DefaultTagID.Favourite; - const listIndex = context.slidingSyncManager.getOrAllocateListIndex(tagId); const joinCount = 10; const roomIndexToRoomId = { // mixed to ensure we sort @@ -287,8 +260,8 @@ describe("SlidingRoomListStore", () => { } return null; }); - mocked(context._SlidingSyncManager.slidingSync.getListData).mockImplementation((i: number) => { - if (i !== listIndex) { + mocked(context._SlidingSyncManager.slidingSync.getListData).mockImplementation((key: string) => { + if (key !== tagId) { return null; } return { @@ -297,7 +270,7 @@ describe("SlidingRoomListStore", () => { }; }); let p = untilEmission(store, LISTS_UPDATE_EVENT); - context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, listIndex, joinCount, roomIndexToRoomId); + context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); await p; expect(store.orderedLists[tagId]).toEqual([roomA, roomB, roomC]); @@ -310,7 +283,7 @@ describe("SlidingRoomListStore", () => { roomIndexToRoomId[1] = roomIdA; roomIndexToRoomId[2] = roomIdB; p = untilEmission(store, LISTS_UPDATE_EVENT); - context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, listIndex, joinCount, roomIndexToRoomId); + context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); await p; // check that B didn't move and that A was put below B From 4db1928bcdc304cfe593eab97575cc55f78da47c Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Jan 2023 12:49:20 +0000 Subject: [PATCH 05/17] Prettier and strict --- test/SlidingSyncManager-test.ts | 8 +++--- .../room-list/SlidingRoomListStore-test.ts | 28 ++++++++----------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/test/SlidingSyncManager-test.ts b/test/SlidingSyncManager-test.ts index 2e47e25284..132ad769ea 100644 --- a/test/SlidingSyncManager-test.ts +++ b/test/SlidingSyncManager-test.ts @@ -113,10 +113,10 @@ describe("SlidingSyncManager", () => { ); return; } - expect(slidingSync.setListRanges).toBeCalledWith( - SlidingSyncManager.ListSearch, - [[0, batchSize - 1], range], - ); + expect(slidingSync.setListRanges).toBeCalledWith(SlidingSyncManager.ListSearch, [ + [0, batchSize - 1], + range, + ]); }); }); it("handles accounts with zero rooms", async () => { diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 819e8dd9a8..2e83a53a8a 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -85,14 +85,11 @@ describe("SlidingRoomListStore", () => { context._SpaceStore.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); await p; - expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith( - DefaultTagID.Untagged, - { - filters: expect.objectContaining({ - spaces: [spaceRoomId], - }), - }, - ); + expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { + filters: expect.objectContaining({ + spaces: [spaceRoomId], + }), + }); }); it("alters 'filters.spaces' on the DefaultTagID.Untagged list if it loads with an active space", async () => { @@ -138,14 +135,11 @@ describe("SlidingRoomListStore", () => { context._SpaceStore.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); await p; - expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith( - DefaultTagID.Untagged, - { - filters: expect.objectContaining({ - spaces: [spaceRoomId, subSpace1, subSpace2], - }), - }, - ); + expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { + filters: expect.objectContaining({ + spaces: [spaceRoomId, subSpace1, subSpace2], + }), + }); }); }); @@ -168,7 +162,7 @@ describe("SlidingRoomListStore", () => { await store.start(); const roomA = "!a:localhost"; const roomB = "!b:localhost"; - const keyToListData = { + const keyToListData: Record }> = { [DefaultTagID.Untagged]: { joinedCount: 10, roomIndexToRoomId: { From eed7340663ea3ec9d66a7333e1c70197bfe4b95e Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Jan 2023 15:02:48 +0000 Subject: [PATCH 06/17] Add additional tests --- test/SlidingSyncManager-test.ts | 63 +++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/SlidingSyncManager-test.ts b/test/SlidingSyncManager-test.ts index 132ad769ea..6b5a854e84 100644 --- a/test/SlidingSyncManager-test.ts +++ b/test/SlidingSyncManager-test.ts @@ -78,6 +78,69 @@ describe("SlidingSyncManager", () => { }); }); + describe("ensureListRegistered", () => { + it("creates a new list based on the key", async () => { + const listKey = "key"; + mocked(slidingSync.getListParams).mockReturnValue(null); + mocked(slidingSync.setList).mockResolvedValue("yep"); + await manager.ensureListRegistered(listKey, { + sort: ["by_recency"], + }); + expect(slidingSync.setList).toBeCalledWith( + listKey, + expect.objectContaining({ + sort: ["by_recency"], + }), + ); + }); + + it("updates an existing list based on the key", async () => { + const listKey = "key"; + mocked(slidingSync.getListParams).mockReturnValue({ + ranges: [[0, 42]], + }); + mocked(slidingSync.setList).mockResolvedValue("yep"); + await manager.ensureListRegistered(listKey, { + sort: ["by_recency"], + }); + expect(slidingSync.setList).toBeCalledWith( + listKey, + expect.objectContaining({ + sort: ["by_recency"], + ranges: [[0, 42]], + }), + ); + }); + + it("updates ranges on an existing list based on the key if there's no other changes", async () => { + const listKey = "key"; + mocked(slidingSync.getListParams).mockReturnValue({ + ranges: [[0, 42]], + }); + mocked(slidingSync.setList).mockResolvedValue("yep"); + await manager.ensureListRegistered(listKey, { + ranges: [[0, 52]], + }); + expect(slidingSync.setList).not.toBeCalled(); + expect(slidingSync.setListRanges).toBeCalledWith(listKey, [[0, 52]]); + }); + + it("no-ops for idential changes", async () => { + const listKey = "key"; + mocked(slidingSync.getListParams).mockReturnValue({ + ranges: [[0, 42]], + sort: ["by_recency"], + }); + mocked(slidingSync.setList).mockResolvedValue("yep"); + await manager.ensureListRegistered(listKey, { + ranges: [[0, 42]], + sort: ["by_recency"], + }); + expect(slidingSync.setList).not.toBeCalled(); + expect(slidingSync.setListRanges).not.toBeCalled(); + }); + }); + describe("startSpidering", () => { it("requests in batchSizes", async () => { const gapMs = 1; From c685c8e85657bff60e0c7d64a136584c1f174d79 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 09:59:54 +0000 Subject: [PATCH 07/17] Point cypress at a newer version of the proxy --- cypress.config.ts | 4 ++-- cypress/plugins/sliding-sync/index.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cypress.config.ts b/cypress.config.ts index dfa17ab32c..f9bc521bdd 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -30,8 +30,8 @@ export default defineConfig({ specPattern: "cypress/e2e/**/*.{js,jsx,ts,tsx}", }, env: { - // Docker tag to use for `ghcr.io/matrix-org/sliding-sync-proxy` image. - SLIDING_SYNC_PROXY_TAG: "v0.6.0", + // Docker tag to use for `ghcr.io/matrix-org/sliding-sync` image. + SLIDING_SYNC_PROXY_TAG: "v0.99.0-rc1", HOMESERVER: "synapse", }, retries: { diff --git a/cypress/plugins/sliding-sync/index.ts b/cypress/plugins/sliding-sync/index.ts index 8204fb578d..ab39c7a42b 100644 --- a/cypress/plugins/sliding-sync/index.ts +++ b/cypress/plugins/sliding-sync/index.ts @@ -23,7 +23,7 @@ import { getFreePort } from "../utils/port"; import { HomeserverInstance } from "../utils/homeserver"; // A cypress plugin to add command to start & stop https://github.com/matrix-org/sliding-sync -// SLIDING_SYNC_PROXY_TAG env used as the docker tag to use for `ghcr.io/matrix-org/sliding-sync-proxy` image. +// SLIDING_SYNC_PROXY_TAG env used as the docker tag to use for `ghcr.io/matrix-org/sliding-sync` image. export interface ProxyInstance { containerId: string; @@ -72,7 +72,7 @@ async function proxyStart(dockerTag: string, homeserver: HomeserverInstance): Pr const port = await getFreePort(); console.log(new Date(), "starting proxy container...", dockerTag); const containerId = await dockerRun({ - image: "ghcr.io/matrix-org/sliding-sync-proxy:" + dockerTag, + image: "ghcr.io/matrix-org/sliding-sync:" + dockerTag, containerName: "react-sdk-cypress-sliding-sync-proxy", params: [ "--rm", From 769fd4a78647709581dab642de5878a901d85745 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 10:31:44 +0000 Subject: [PATCH 08/17] TS errors --- src/SlidingSyncManager.ts | 14 +++++-- src/components/views/rooms/RoomSublist.tsx | 4 +- src/hooks/useSlidingSyncRoomSearch.ts | 2 +- src/stores/room-list/SlidingRoomListStore.ts | 16 ++++++-- .../room-list/SlidingRoomListStore-test.ts | 40 +++++++++---------- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 1489f636d4..4f21a69260 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -231,7 +231,7 @@ export class SlidingSyncManager { } catch (err) { logger.debug("ensureListRegistered: update failed txn_id=", err); } - return this.slidingSync.getListParams(listKey); + return this.slidingSync.getListParams(listKey)!; } public async setRoomVisible(roomId: string, visible: boolean): Promise { @@ -315,13 +315,19 @@ export class SlidingSyncManager { } else { await this.slidingSync.setListRanges(SlidingSyncManager.ListSearch, ranges); } - // gradually request more over time - await sleep(gapBetweenRequestsMs); } catch (err) { // do nothing, as we reject only when we get interrupted but that's fine as the next // request will include our data + } finally { + // gradually request more over time, even on errors. + await sleep(gapBetweenRequestsMs); } - hasMore = endIndex + 1 < this.slidingSync.getListData(SlidingSyncManager.ListSearch)?.joinedCount; + const listData = this.slidingSync.getListData(SlidingSyncManager.ListSearch); + if (!listData) { + // we failed to do the first request, keep trying + continue; + } + hasMore = endIndex + 1 < listData.joinedCount; startIndex += batchSize; firstTime = false; } diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 7674916d70..1dd25cbff5 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -566,8 +566,8 @@ export default class RoomSublist extends React.Component { let isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; if (this.slidingSyncMode) { const slidingList = SlidingSyncManager.instance.slidingSync.getListParams(this.props.tagId); - isAlphabetical = slidingList.sort[0] === "by_name"; - isUnreadFirst = slidingList.sort[0] === "by_notification_level"; + isAlphabetical = (slidingList?.sort || [])[0] === "by_name"; + isUnreadFirst = (slidingList?.sort || [])[0] === "by_notification_level"; } // Invites don't get some nonsense options, so only add them if we have to. diff --git a/src/hooks/useSlidingSyncRoomSearch.ts b/src/hooks/useSlidingSyncRoomSearch.ts index 00f26c198b..7d5d1d92aa 100644 --- a/src/hooks/useSlidingSyncRoomSearch.ts +++ b/src/hooks/useSlidingSyncRoomSearch.ts @@ -58,7 +58,7 @@ export const useSlidingSyncRoomSearch = (): { const rooms = []; const { roomIndexToRoomId } = SlidingSyncManager.instance.slidingSync.getListData( SlidingSyncManager.ListSearch, - ); + )!; let i = 0; while (roomIndexToRoomId[i]) { const roomId = roomIndexToRoomId[i]; diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index eeacab35f6..884f588bd2 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -29,6 +29,7 @@ import { MetaSpace, SpaceKey, UPDATE_SELECTED_SPACE } from "../spaces"; import { LISTS_LOADING_EVENT } from "./RoomListStore"; import { UPDATE_EVENT } from "../AsyncStore"; import { SdkContextClass } from "../../contexts/SDKContext"; +import { filter } from "lodash"; interface IState { // state is tracked in underlying classes @@ -84,6 +85,7 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl public constructor(dis: MatrixDispatcher, private readonly context: SdkContextClass) { super(dis); this.setMaxListeners(20); // RoomList + LeftPanel + 8xRoomSubList + spares + this.stickyRoomId = null; } public async setTagSorting(tagId: TagID, sort: SortAlgorithm): Promise { @@ -249,9 +251,14 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl } // now set the rooms - const rooms = orderedRoomIds.map((roomId) => { - return this.matrixClient.getRoom(roomId); - }); + const rooms: Room[] = []; + orderedRoomIds.forEach((roomId) => { + const room = this.matrixClient.getRoom(roomId); + if (!room) { + return; + } + rooms.push(room); + }) tagMap[tagId] = rooms; this.tagMap = tagMap; } @@ -352,6 +359,9 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl if (roomId === activeSpace) { return; } + if (!filters.spaces) { + filters.spaces = []; + } filters.spaces.push(roomId); // add subspace }, false, diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 2e83a53a8a..5d322daafb 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -82,10 +82,10 @@ describe("SlidingRoomListStore", () => { // change the active space activeSpace = spaceRoomId; - context._SpaceStore.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); + context._SpaceStore!.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); await p; - expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { + expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { filters: expect.objectContaining({ spaces: [spaceRoomId], }), @@ -101,7 +101,7 @@ describe("SlidingRoomListStore", () => { }); await store.start(); // call onReady await p; - expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith( + expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith( DefaultTagID.Untagged, expect.objectContaining({ filters: expect.objectContaining({ @@ -121,7 +121,7 @@ describe("SlidingRoomListStore", () => { return listName === DefaultTagID.Untagged && !isLoading; }); - mocked(context._SpaceStore.traverseSpace).mockImplementation( + mocked(context._SpaceStore!.traverseSpace).mockImplementation( (spaceId: string, fn: (roomId: string) => void) => { if (spaceId === spaceRoomId) { fn(subSpace1); @@ -132,10 +132,10 @@ describe("SlidingRoomListStore", () => { // change the active space activeSpace = spaceRoomId; - context._SpaceStore.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); + context._SpaceStore!.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); await p; - expect(context._SlidingSyncManager.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { + expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { filters: expect.objectContaining({ spaces: [spaceRoomId, subSpace1, subSpace2], }), @@ -146,13 +146,13 @@ describe("SlidingRoomListStore", () => { it("setTagSorting alters the 'sort' option in the list", async () => { const tagId: TagID = "foo"; await store.setTagSorting(tagId, SortAlgorithm.Alphabetic); - expect(context._SlidingSyncManager.ensureListRegistered).toBeCalledWith(tagId, { + expect(context._SlidingSyncManager!.ensureListRegistered).toBeCalledWith(tagId, { sort: SlidingSyncSortToFilter[SortAlgorithm.Alphabetic], }); expect(store.getTagSorting(tagId)).toEqual(SortAlgorithm.Alphabetic); await store.setTagSorting(tagId, SortAlgorithm.Recent); - expect(context._SlidingSyncManager.ensureListRegistered).toBeCalledWith(tagId, { + expect(context._SlidingSyncManager!.ensureListRegistered).toBeCalledWith(tagId, { sort: SlidingSyncSortToFilter[SortAlgorithm.Recent], }); expect(store.getTagSorting(tagId)).toEqual(SortAlgorithm.Recent); @@ -177,14 +177,14 @@ describe("SlidingRoomListStore", () => { }, }, }; - mocked(context._SlidingSyncManager.slidingSync.getListData).mockImplementation((key: string) => { + mocked(context._SlidingSyncManager!.slidingSync.getListData).mockImplementation((key: string) => { return keyToListData[key] || null; }); - expect(store.getTagsForRoom(new Room(roomA, context.client, context.client.getUserId()))).toEqual([ + expect(store.getTagsForRoom(new Room(roomA, context.client!, context.client!.getUserId()))).toEqual([ DefaultTagID.Untagged, ]); - expect(store.getTagsForRoom(new Room(roomB, context.client, context.client.getUserId()))).toEqual([ + expect(store.getTagsForRoom(new Room(roomB, context.client!, context.client!.getUserId()))).toEqual([ DefaultTagID.Favourite, DefaultTagID.Untagged, ]); @@ -204,11 +204,11 @@ describe("SlidingRoomListStore", () => { 0: roomA, }; const rooms = [ - new Room(roomA, context.client, context.client.getUserId()), - new Room(roomB, context.client, context.client.getUserId()), - new Room(roomC, context.client, context.client.getUserId()), + new Room(roomA, context.client!, context.client!.getUserId()), + new Room(roomB, context.client!, context.client!.getUserId()), + new Room(roomC, context.client!, context.client!.getUserId()), ]; - mocked(context.client.getRoom).mockImplementation((roomId: string) => { + mocked(context.client!.getRoom).mockImplementation((roomId: string) => { switch (roomId) { case roomA: return rooms[0]; @@ -240,10 +240,10 @@ describe("SlidingRoomListStore", () => { 2: roomIdC, 0: roomIdA, }; - const roomA = new Room(roomIdA, context.client, context.client.getUserId()); - const roomB = new Room(roomIdB, context.client, context.client.getUserId()); - const roomC = new Room(roomIdC, context.client, context.client.getUserId()); - mocked(context.client.getRoom).mockImplementation((roomId: string) => { + const roomA = new Room(roomIdA, context.client!, context.client!.getUserId()); + const roomB = new Room(roomIdB, context.client!, context.client!.getUserId()); + const roomC = new Room(roomIdC, context.client!, context.client!.getUserId()); + mocked(context.client!.getRoom).mockImplementation((roomId: string) => { switch (roomId) { case roomIdA: return roomA; @@ -254,7 +254,7 @@ describe("SlidingRoomListStore", () => { } return null; }); - mocked(context._SlidingSyncManager.slidingSync.getListData).mockImplementation((key: string) => { + mocked(context._SlidingSyncManager!.slidingSync.getListData).mockImplementation((key: string) => { if (key !== tagId) { return null; } From 66d4715e84150bab95f716c05d420a62312df753 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 10:41:32 +0000 Subject: [PATCH 09/17] Remove accidental import --- src/stores/room-list/SlidingRoomListStore.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index 884f588bd2..868d5c1049 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -29,7 +29,6 @@ import { MetaSpace, SpaceKey, UPDATE_SELECTED_SPACE } from "../spaces"; import { LISTS_LOADING_EVENT } from "./RoomListStore"; import { UPDATE_EVENT } from "../AsyncStore"; import { SdkContextClass } from "../../contexts/SDKContext"; -import { filter } from "lodash"; interface IState { // state is tracked in underlying classes From 8e774e192408f891bd87b0eccc1f68a26f0a8917 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 10:45:55 +0000 Subject: [PATCH 10/17] Prettier --- src/stores/room-list/SlidingRoomListStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index 868d5c1049..ff2113379d 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -257,7 +257,7 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl return; } rooms.push(room); - }) + }); tagMap[tagId] = rooms; this.tagMap = tagMap; } From 6a75054e1aae3260f50ab71ca2aae58bf2378922 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 12:38:58 +0000 Subject: [PATCH 11/17] More tests --- src/SlidingSyncManager.ts | 6 +-- .../room-list/SlidingRoomListStore-test.ts | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 4f21a69260..4a6c113253 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -322,11 +322,7 @@ export class SlidingSyncManager { // gradually request more over time, even on errors. await sleep(gapBetweenRequestsMs); } - const listData = this.slidingSync.getListData(SlidingSyncManager.ListSearch); - if (!listData) { - // we failed to do the first request, keep trying - continue; - } + const listData = this.slidingSync.getListData(SlidingSyncManager.ListSearch)!; hasMore = endIndex + 1 < listData.joinedCount; startIndex += batchSize; firstTime = false; diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 5d322daafb..868a54ba78 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -290,4 +290,43 @@ describe("SlidingRoomListStore", () => { await p; expect(store.orderedLists[tagId].map((r) => r.roomId)).toEqual([roomC, roomA, roomB].map((r) => r.roomId)); }); + + it("gracefully handles unknown room IDs", async () => { + await store.start(); + const roomIdA = "!a:localhost"; + const roomIdB = "!b:localhost"; // does not exist + const roomIdC = "!c:localhost"; + const roomIndexToRoomId = { + 0: roomIdA, + 1: roomIdB, // does not exist + 2: roomIdC, + }; + const tagId = DefaultTagID.Favourite; + const joinCount = 10; + // seed the store with 2 rooms + const roomA = new Room(roomIdA, context.client!, context.client!.getUserId()); + const roomC = new Room(roomIdC, context.client!, context.client!.getUserId()); + mocked(context.client!.getRoom).mockImplementation((roomId: string) => { + switch (roomId) { + case roomIdA: + return roomA; + case roomIdC: + return roomC; + } + return null; + }); + mocked(context._SlidingSyncManager!.slidingSync.getListData).mockImplementation((key: string) => { + if (key !== tagId) { + return null; + } + return { + roomIndexToRoomId: roomIndexToRoomId, + joinedCount: joinCount, + }; + }); + let p = untilEmission(store, LISTS_UPDATE_EVENT); + context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); + await p; + expect(store.orderedLists[tagId]).toEqual([roomA, roomC]); + }); }); From 744abd935ebfd3670445556aab5070a5d36db2ba Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 12:46:51 +0000 Subject: [PATCH 12/17] Linting --- test/stores/room-list/SlidingRoomListStore-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 868a54ba78..64e7bfaa97 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -324,7 +324,7 @@ describe("SlidingRoomListStore", () => { joinedCount: joinCount, }; }); - let p = untilEmission(store, LISTS_UPDATE_EVENT); + const p = untilEmission(store, LISTS_UPDATE_EVENT); context.slidingSyncManager.slidingSync.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); await p; expect(store.orderedLists[tagId]).toEqual([roomA, roomC]); From 676c65b55c90c77b4d7d8036bdf08a340892e9fa Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 13:27:03 +0000 Subject: [PATCH 13/17] More tests --- .../room-list/SlidingRoomListStore-test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 64e7bfaa97..3014423349 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -30,7 +30,7 @@ import { RoomViewStore } from "../../../src/stores/RoomViewStore"; import { MatrixDispatcher } from "../../../src/dispatcher/dispatcher"; import { SortAlgorithm } from "../../../src/stores/room-list/algorithms/models"; import { DefaultTagID, TagID } from "../../../src/stores/room-list/models"; -import { UPDATE_SELECTED_SPACE } from "../../../src/stores/spaces"; +import { MetaSpace, UPDATE_SELECTED_SPACE } from "../../../src/stores/spaces"; import { LISTS_LOADING_EVENT } from "../../../src/stores/room-list/RoomListStore"; import { UPDATE_EVENT } from "../../../src/stores/AsyncStore"; @@ -92,6 +92,23 @@ describe("SlidingRoomListStore", () => { }); }); + it("gracefully handles subspaces in the home metaspace", async () => { + const subspace = "!sub:space"; + mocked(context._SpaceStore!.traverseSpace).mockImplementation( + (spaceId: string, fn: (roomId: string) => void) => { + fn(subspace); + }, + ); + activeSpace = MetaSpace.Home; + await store.start(); // call onReady + + expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { + filters: expect.objectContaining({ + spaces: [subspace], + }), + }); + }); + it("alters 'filters.spaces' on the DefaultTagID.Untagged list if it loads with an active space", async () => { // change the active space before we are ready const spaceRoomId = "!foo2:bar"; From 153a3a79981693343129a4d8a61a2680ff84aec3 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 15:58:06 +0000 Subject: [PATCH 14/17] Add hook test --- test/hooks/useSlidingSyncRoomSearch-test.tsx | 105 +++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 test/hooks/useSlidingSyncRoomSearch-test.tsx diff --git a/test/hooks/useSlidingSyncRoomSearch-test.tsx b/test/hooks/useSlidingSyncRoomSearch-test.tsx new file mode 100644 index 0000000000..d9d9fe0cfe --- /dev/null +++ b/test/hooks/useSlidingSyncRoomSearch-test.tsx @@ -0,0 +1,105 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// eslint-disable-next-line deprecate/import +import { mount } from "enzyme"; +import { sleep } from "matrix-js-sdk/src/utils"; +import React from "react"; +import { act } from "react-dom/test-utils"; +import { mocked } from "jest-mock"; +import { SlidingSync } from "matrix-js-sdk/src/sliding-sync"; +import { Room } from "matrix-js-sdk/src/matrix"; + +import { useSlidingSyncRoomSearch } from "../../src/hooks/useSlidingSyncRoomSearch"; +import { MockEventEmitter, stubClient } from "../test-utils"; +import { SlidingSyncManager } from "../../src/SlidingSyncManager"; + +// hooks must be inside a React component else you get: +// "Invalid hook call. Hooks can only be called inside of the body of a function component." +function RoomSearchComponent({ onClick }) { + const roomSearch = useSlidingSyncRoomSearch(); + return
onClick(roomSearch)} />; +} + +describe("useSlidingSyncRoomSearch", () => { + it("should display rooms when searching", async () => { + const client = stubClient(); + const roomA = new Room("!a:localhost", client, client.getUserId()); + const roomB = new Room("!b:localhost", client, client.getUserId()); + const slidingSync = mocked( + new MockEventEmitter({ + getListData: jest.fn(), + }) as unknown as SlidingSync, + ); + jest.spyOn(SlidingSyncManager.instance, "ensureListRegistered").mockResolvedValue({ + ranges: [[0, 9]], + }); + SlidingSyncManager.instance.slidingSync = slidingSync; + mocked(slidingSync.getListData).mockReturnValue({ + joinedCount: 2, + roomIndexToRoomId: { + 0: roomA.roomId, + 1: roomB.roomId, + }, + }); + mocked(client.getRoom).mockImplementation((roomId) => { + switch (roomId) { + case roomA.roomId: + return roomA; + case roomB.roomId: + return roomB; + default: + return null; + } + }); + + // first check that everything is empty and then do the search + let executeHook = (roomSearch) => { + expect(roomSearch.loading).toBe(false); + expect(roomSearch.rooms).toEqual([]); + roomSearch.search({ + limit: 10, + query: "foo", + }); + }; + const wrapper = mount( + { + executeHook(roomSearch); + }} + />, + ); + + // run the query + await act(async () => { + await sleep(1); + wrapper.simulate("click"); + return act(() => sleep(1)); + }); + // now we expect there to be rooms + executeHook = (roomSearch) => { + expect(roomSearch.loading).toBe(false); + expect(roomSearch.rooms).toEqual([roomA, roomB]); + }; + + // run the query + await act(async () => { + await sleep(1); + wrapper.simulate("click"); + return act(() => sleep(1)); + }); + }); +}); From 7616f71ff2db211df3ba232833e66e314a54d328 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Jan 2023 17:20:10 +0000 Subject: [PATCH 15/17] TS --- test/hooks/useSlidingSyncRoomSearch-test.tsx | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/hooks/useSlidingSyncRoomSearch-test.tsx b/test/hooks/useSlidingSyncRoomSearch-test.tsx index d9d9fe0cfe..fbac2fe06e 100644 --- a/test/hooks/useSlidingSyncRoomSearch-test.tsx +++ b/test/hooks/useSlidingSyncRoomSearch-test.tsx @@ -23,22 +23,28 @@ import { mocked } from "jest-mock"; import { SlidingSync } from "matrix-js-sdk/src/sliding-sync"; import { Room } from "matrix-js-sdk/src/matrix"; -import { useSlidingSyncRoomSearch } from "../../src/hooks/useSlidingSyncRoomSearch"; +import { SlidingSyncRoomSearchOpts, useSlidingSyncRoomSearch } from "../../src/hooks/useSlidingSyncRoomSearch"; import { MockEventEmitter, stubClient } from "../test-utils"; import { SlidingSyncManager } from "../../src/SlidingSyncManager"; +type RoomSearchHook = { + loading: boolean; + rooms: Room[]; + search(opts: SlidingSyncRoomSearchOpts): Promise; +}; + // hooks must be inside a React component else you get: // "Invalid hook call. Hooks can only be called inside of the body of a function component." -function RoomSearchComponent({ onClick }) { +function RoomSearchComponent(props: { onClick: (h: RoomSearchHook) => void }) { const roomSearch = useSlidingSyncRoomSearch(); - return
onClick(roomSearch)} />; + return
props.onClick(roomSearch)} />; } describe("useSlidingSyncRoomSearch", () => { it("should display rooms when searching", async () => { const client = stubClient(); - const roomA = new Room("!a:localhost", client, client.getUserId()); - const roomB = new Room("!b:localhost", client, client.getUserId()); + const roomA = new Room("!a:localhost", client, client.getUserId()!); + const roomB = new Room("!b:localhost", client, client.getUserId()!); const slidingSync = mocked( new MockEventEmitter({ getListData: jest.fn(), @@ -67,7 +73,7 @@ describe("useSlidingSyncRoomSearch", () => { }); // first check that everything is empty and then do the search - let executeHook = (roomSearch) => { + let executeHook = (roomSearch: RoomSearchHook) => { expect(roomSearch.loading).toBe(false); expect(roomSearch.rooms).toEqual([]); roomSearch.search({ @@ -77,7 +83,7 @@ describe("useSlidingSyncRoomSearch", () => { }; const wrapper = mount( { + onClick={(roomSearch: RoomSearchHook) => { executeHook(roomSearch); }} />, From 0fcc1f45d992950c2f61cb1fa4e35eeaa55887ce Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 23 Jan 2023 13:18:05 +0000 Subject: [PATCH 16/17] Remove flakey test which wasn't a useful test of anything --- cypress/e2e/sliding-sync/sliding-sync.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/cypress/e2e/sliding-sync/sliding-sync.ts b/cypress/e2e/sliding-sync/sliding-sync.ts index 1b2642eeb4..8689bd2088 100644 --- a/cypress/e2e/sliding-sync/sliding-sync.ts +++ b/cypress/e2e/sliding-sync/sliding-sync.ts @@ -102,21 +102,6 @@ describe("Sliding Sync", () => { }); }; - // sanity check everything works - it("should correctly render expected messages", () => { - cy.get("@roomId").then((roomId) => cy.visit("/#/room/" + roomId)); - cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.IRC); - - // Wait until configuration is finished - cy.contains( - ".mx_RoomView_body .mx_GenericEventListSummary .mx_GenericEventListSummary_summary", - "created and configured the room.", - ); - - // Click "expand" link button - cy.get(".mx_GenericEventListSummary_toggle[aria-expanded=false]").click(); - }); - it("should render the Rooms list in reverse chronological order by default and allowing sorting A-Z", () => { // create rooms and check room names are correct cy.createRoom({ name: "Apple" }).then(() => cy.contains(".mx_RoomSublist", "Apple")); From 7df07fa336e5d21b2a22e9b76fe61251ed1729e1 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 23 Jan 2023 13:21:36 +0000 Subject: [PATCH 17/17] Remove unused imports --- cypress/e2e/sliding-sync/sliding-sync.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/cypress/e2e/sliding-sync/sliding-sync.ts b/cypress/e2e/sliding-sync/sliding-sync.ts index 8689bd2088..c6d2c298fe 100644 --- a/cypress/e2e/sliding-sync/sliding-sync.ts +++ b/cypress/e2e/sliding-sync/sliding-sync.ts @@ -21,8 +21,6 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { Interception } from "cypress/types/net-stubbing"; import { HomeserverInstance } from "../../plugins/utils/homeserver"; -import { SettingLevel } from "../../../src/settings/SettingLevel"; -import { Layout } from "../../../src/settings/enums/Layout"; import { ProxyInstance } from "../../plugins/sliding-sync"; describe("Sliding Sync", () => {