From df7bb61a5b24aeb9ac6acb30d599a5507dda788e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 12 Sep 2024 14:42:27 +0100 Subject: [PATCH 01/13] Experimental SSS Working branch to get SSS functional on element-web. Requires https://github.com/matrix-org/matrix-js-sdk/pull/4400 --- src/SlidingSyncManager.ts | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 578a175749..2752e2ce1c 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -214,9 +214,9 @@ 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(listKey, updateArgs.ranges); + this.slidingSync!.setListRanges(listKey, updateArgs.ranges); } else { - await this.slidingSync!.setList(listKey, list); + this.slidingSync!.setList(listKey, list); } } catch (err) { logger.debug("ensureListRegistered: update failed txn_id=", err); @@ -266,21 +266,14 @@ 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 - let startIndex = batchSize; + let windowSize = batchSize; let hasMore = true; let firstTime = true; while (hasMore) { - const endIndex = startIndex + batchSize - 1; try { - const ranges = [ - [0, batchSize - 1], - [startIndex, endIndex], - ]; if (firstTime) { 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, + ranges: [[0, windowSize]], sort: [ "by_recency", // this list isn't shown on the UI so just sorting by timestamp is enough ], @@ -313,8 +306,8 @@ export class SlidingSyncManager { await sleep(gapBetweenRequestsMs); } const listData = this.slidingSync!.getListData(SlidingSyncManager.ListSearch)!; - hasMore = endIndex + 1 < listData.joinedCount; - startIndex += batchSize; + hasMore = windowSize < listData.joinedCount; + windowSize += batchSize; firstTime = false; } } @@ -375,7 +368,7 @@ export class SlidingSyncManager { public async nativeSlidingSyncSupport(client: MatrixClient): Promise { // Per https://github.com/matrix-org/matrix-spec-proposals/pull/3575/files#r1589542561 // `client` can be undefined/null in tests for some reason. - const support = await client?.doesServerSupportUnstableFeature("org.matrix.msc3575"); + const support = await client?.doesServerSupportUnstableFeature("org.matrix.simplified_msc3575"); if (support) { logger.log("nativeSlidingSyncSupport: sliding sync advertised as unstable"); } From 7fbac104902ed6f8a50e4cd5b51b4db8051fb70a Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Fri, 13 Sep 2024 13:45:40 +0100 Subject: [PATCH 02/13] Adjust tests to use new behaviour --- src/SlidingSyncManager.ts | 10 +++---- test/SlidingSyncManager-test.ts | 52 +++++++-------------------------- 2 files changed, 16 insertions(+), 46 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 2752e2ce1c..97ee976f60 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -266,14 +266,14 @@ 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 - let windowSize = batchSize; + let fetchUpTo = batchSize; let hasMore = true; let firstTime = true; while (hasMore) { try { if (firstTime) { await this.slidingSync!.setList(SlidingSyncManager.ListSearch, { - ranges: [[0, windowSize]], + ranges: [[0, fetchUpTo]], sort: [ "by_recency", // this list isn't shown on the UI so just sorting by timestamp is enough ], @@ -296,7 +296,7 @@ export class SlidingSyncManager { }, }); } else { - await this.slidingSync!.setListRanges(SlidingSyncManager.ListSearch, ranges); + await this.slidingSync!.setListRanges(SlidingSyncManager.ListSearch, [[0,fetchUpTo]]); } } catch (err) { // do nothing, as we reject only when we get interrupted but that's fine as the next @@ -306,8 +306,8 @@ export class SlidingSyncManager { await sleep(gapBetweenRequestsMs); } const listData = this.slidingSync!.getListData(SlidingSyncManager.ListSearch)!; - hasMore = windowSize < listData.joinedCount; - windowSize += batchSize; + hasMore = fetchUpTo < listData.joinedCount; + fetchUpTo += batchSize; firstTime = false; } } diff --git a/test/SlidingSyncManager-test.ts b/test/SlidingSyncManager-test.ts index 3625bf2a18..343269c947 100644 --- a/test/SlidingSyncManager-test.ts +++ b/test/SlidingSyncManager-test.ts @@ -79,7 +79,6 @@ describe("SlidingSyncManager", () => { 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"], }); @@ -114,7 +113,6 @@ describe("SlidingSyncManager", () => { mocked(slidingSync.getListParams).mockReturnValue({ ranges: [[0, 42]], }); - mocked(slidingSync.setList).mockResolvedValue("yep"); await manager.ensureListRegistered(listKey, { ranges: [[0, 52]], }); @@ -128,7 +126,6 @@ describe("SlidingSyncManager", () => { ranges: [[0, 42]], sort: ["by_recency"], }); - mocked(slidingSync.setList).mockResolvedValue("yep"); await manager.ensureListRegistered(listKey, { ranges: [[0, 42]], sort: ["by_recency"], @@ -139,11 +136,9 @@ describe("SlidingSyncManager", () => { }); describe("startSpidering", () => { - it("requests in batchSizes", async () => { + it("requests in expanding batchSizes", async () => { const gapMs = 1; const batchSize = 10; - mocked(slidingSync.setList).mockResolvedValue("yep"); - mocked(slidingSync.setListRanges).mockResolvedValue("yep"); mocked(slidingSync.getListData).mockImplementation((key) => { return { joinedCount: 64, @@ -153,12 +148,13 @@ describe("SlidingSyncManager", () => { await manager.startSpidering(batchSize, gapMs); // we expect calls for 10,19 -> 20,29 -> 30,39 -> 40,49 -> 50,59 -> 60,69 const wantWindows = [ - [10, 19], - [20, 29], - [30, 39], - [40, 49], - [50, 59], - [60, 69], + [0, 10], + [0, 20], + [0, 30], + [0, 40], + [0, 50], + [0, 60], + [0, 70], ]; expect(slidingSync.getListData).toHaveBeenCalledTimes(wantWindows.length); expect(slidingSync.setList).toHaveBeenCalledTimes(1); @@ -170,13 +166,12 @@ describe("SlidingSyncManager", () => { SlidingSyncManager.ListSearch, // eslint-disable-next-line jest/no-conditional-expect expect.objectContaining({ - ranges: [[0, batchSize - 1], range], + ranges: [range], }), ); return; } expect(slidingSync.setListRanges).toHaveBeenCalledWith(SlidingSyncManager.ListSearch, [ - [0, batchSize - 1], range, ]); }); @@ -184,7 +179,6 @@ describe("SlidingSyncManager", () => { it("handles accounts with zero rooms", async () => { const gapMs = 1; const batchSize = 10; - mocked(slidingSync.setList).mockResolvedValue("yep"); mocked(slidingSync.getListData).mockImplementation((key) => { return { joinedCount: 0, @@ -198,31 +192,7 @@ describe("SlidingSyncManager", () => { SlidingSyncManager.ListSearch, expect.objectContaining({ ranges: [ - [0, batchSize - 1], - [batchSize, batchSize + batchSize - 1], - ], - }), - ); - }); - it("continues even when setList rejects", async () => { - const gapMs = 1; - const batchSize = 10; - mocked(slidingSync.setList).mockRejectedValue("narp"); - mocked(slidingSync.getListData).mockImplementation((key) => { - return { - joinedCount: 0, - roomIndexToRoomId: {}, - }; - }); - await manager.startSpidering(batchSize, gapMs); - expect(slidingSync.getListData).toHaveBeenCalledTimes(1); - expect(slidingSync.setList).toHaveBeenCalledTimes(1); - expect(slidingSync.setList).toHaveBeenCalledWith( - SlidingSyncManager.ListSearch, - expect.objectContaining({ - ranges: [ - [0, batchSize - 1], - [batchSize, batchSize + batchSize - 1], + [0, batchSize], ], }), ); @@ -277,7 +247,7 @@ describe("SlidingSyncManager", () => { const unstableSpy = jest .spyOn(client, "doesServerSupportUnstableFeature") .mockImplementation(async (feature: string) => { - expect(feature).toBe("org.matrix.msc3575"); + expect(feature).toBe("org.matrix.simplified_msc3575"); return true; }); const proxySpy = jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("https://proxy/"); From 194362a4879b6e7ea1b6ed33e19e0f66945dfb18 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Fri, 13 Sep 2024 14:33:29 +0100 Subject: [PATCH 03/13] Remove well-known proxy URL lookup; always use native This is actually required for SSS because otherwise it would use the proxy over native support. --- src/SlidingSyncManager.ts | 51 ++--------------------- src/i18n/strings/en_EN.json | 2 +- src/settings/Settings.tsx | 5 --- test/SlidingSyncManager-test.ts | 73 +-------------------------------- 4 files changed, 6 insertions(+), 125 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 97ee976f60..bdee8e202f 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -322,44 +322,12 @@ export class SlidingSyncManager { * @returns A working Sliding Sync or undefined */ public async setup(client: MatrixClient): Promise { - const baseUrl = client.baseUrl; - const proxyUrl = SettingsStore.getValue("feature_sliding_sync_proxy_url"); - const wellKnownProxyUrl = await this.getProxyFromWellKnown(client); - - const slidingSyncEndpoint = proxyUrl || wellKnownProxyUrl || baseUrl; - - this.configure(client, slidingSyncEndpoint); - logger.info("Sliding sync activated at", slidingSyncEndpoint); + this.configure(client, client.baseUrl); + logger.info("Simplified Sliding Sync activated at", client.baseUrl); this.startSpidering(100, 50); // 100 rooms at a time, 50ms apart - return this.slidingSync; } - /** - * Get the sliding sync proxy URL from the client well known - * @param client The MatrixClient to use - * @return The proxy url - */ - public async getProxyFromWellKnown(client: MatrixClient): Promise { - let proxyUrl: string | undefined; - - try { - const clientDomain = await client.getDomain(); - if (clientDomain === null) { - throw new RangeError("Homeserver domain is null"); - } - const clientWellKnown = await AutoDiscovery.findClientConfig(clientDomain); - proxyUrl = clientWellKnown?.["org.matrix.msc3575.proxy"]?.url; - } catch (e) { - // Either client.getDomain() is null so we've shorted out, or is invalid so `AutoDiscovery.findClientConfig` has thrown - } - - if (proxyUrl != undefined) { - logger.log("getProxyFromWellKnown: client well-known declares sliding sync proxy at", proxyUrl); - } - return proxyUrl; - } - /** * Check if the server "natively" supports sliding sync (with an unstable endpoint). * @param client The MatrixClient to use @@ -370,7 +338,7 @@ export class SlidingSyncManager { // `client` can be undefined/null in tests for some reason. const support = await client?.doesServerSupportUnstableFeature("org.matrix.simplified_msc3575"); if (support) { - logger.log("nativeSlidingSyncSupport: sliding sync advertised as unstable"); + logger.log("nativeSlidingSyncSupport: org.matrix.simplified_msc3575 sliding sync advertised as unstable"); } return support; } @@ -387,17 +355,6 @@ export class SlidingSyncManager { SlidingSyncController.serverSupportsSlidingSync = true; return; } - - const proxyUrl = await this.getProxyFromWellKnown(client); - if (proxyUrl != undefined) { - const response = await fetch(new URL("/client/server.json", proxyUrl), { - method: Method.Get, - signal: timeoutSignal(10 * 1000), // 10s - }); - if (response.status === 200) { - logger.log("checkSupport: well-known sliding sync proxy is up at", proxyUrl); - SlidingSyncController.serverSupportsSlidingSync = true; - } - } + SlidingSyncController.serverSupportsSlidingSync = false; } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 3130de7a76..24de78a6cc 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1470,7 +1470,7 @@ "render_reaction_images_description": "Sometimes referred to as \"custom emojis\".", "report_to_moderators": "Report to moderators", "report_to_moderators_description": "In rooms that support moderation, the “Report” button will let you report abuse to room moderators.", - "sliding_sync": "Sliding Sync mode", + "sliding_sync": "Simplified Sliding Sync mode", "sliding_sync_description": "Under active development, cannot be disabled.", "sliding_sync_disabled_notice": "Log out and back in to disable", "sliding_sync_server_no_support": "Your server lacks support", diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 5e5c9a1535..bf9ae417e2 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -388,11 +388,6 @@ export const SETTINGS: { [setting: string]: ISetting } = { default: false, controller: new SlidingSyncController(), }, - "feature_sliding_sync_proxy_url": { - // This is not a distinct feature, it is a legacy setting for feature_sliding_sync above - supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, - default: "", - }, "feature_element_call_video_rooms": { isFeature: true, labsGroup: LabGroup.VoiceAndVideo, diff --git a/test/SlidingSyncManager-test.ts b/test/SlidingSyncManager-test.ts index 343269c947..4fdac93742 100644 --- a/test/SlidingSyncManager-test.ts +++ b/test/SlidingSyncManager-test.ts @@ -14,7 +14,6 @@ import fetchMockJest from "fetch-mock-jest"; import { SlidingSyncManager } from "../src/SlidingSyncManager"; import { stubClient } from "./test-utils"; import SlidingSyncController from "../src/settings/controllers/SlidingSyncController"; -import SettingsStore from "../src/settings/SettingsStore"; jest.mock("matrix-js-sdk/src/sliding-sync"); const MockSlidingSync = >(SlidingSync); @@ -41,7 +40,6 @@ describe("SlidingSyncManager", () => { const roomId = "!room:id"; const subs = new Set(); mocked(slidingSync.getRoomSubscriptions).mockReturnValue(subs); - mocked(slidingSync.modifyRoomSubscriptions).mockResolvedValue("yep"); await manager.setRoomVisible(roomId, true); expect(slidingSync.modifyRoomSubscriptions).toHaveBeenCalledWith(new Set([roomId])); }); @@ -67,7 +65,6 @@ describe("SlidingSyncManager", () => { }); const subs = new Set(); mocked(slidingSync.getRoomSubscriptions).mockReturnValue(subs); - mocked(slidingSync.modifyRoomSubscriptions).mockResolvedValue("yep"); await manager.setRoomVisible(roomId, true); expect(slidingSync.modifyRoomSubscriptions).toHaveBeenCalledWith(new Set([roomId])); // we aren't prescriptive about what the sub name is. @@ -95,7 +92,6 @@ describe("SlidingSyncManager", () => { mocked(slidingSync.getListParams).mockReturnValue({ ranges: [[0, 42]], }); - mocked(slidingSync.setList).mockResolvedValue("yep"); await manager.ensureListRegistered(listKey, { sort: ["by_recency"], }); @@ -201,61 +197,11 @@ describe("SlidingSyncManager", () => { describe("checkSupport", () => { beforeEach(() => { SlidingSyncController.serverSupportsSlidingSync = false; - jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("https://proxy/"); }); it("shorts out if the server has 'native' sliding sync support", async () => { jest.spyOn(manager, "nativeSlidingSyncSupport").mockResolvedValue(true); expect(SlidingSyncController.serverSupportsSlidingSync).toBeFalsy(); await manager.checkSupport(client); - expect(manager.getProxyFromWellKnown).not.toHaveBeenCalled(); // We return earlier - expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy(); - }); - it("tries to find a sliding sync proxy url from the client well-known if there's no 'native' support", async () => { - jest.spyOn(manager, "nativeSlidingSyncSupport").mockResolvedValue(false); - expect(SlidingSyncController.serverSupportsSlidingSync).toBeFalsy(); - await manager.checkSupport(client); - expect(manager.getProxyFromWellKnown).toHaveBeenCalled(); - expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy(); - }); - it("should query well-known on server_name not baseUrl", async () => { - fetchMockJest.get("https://matrix.org/.well-known/matrix/client", { - "m.homeserver": { - base_url: "https://matrix-client.matrix.org", - server: "matrix.org", - }, - "org.matrix.msc3575.proxy": { - url: "https://proxy/", - }, - }); - fetchMockJest.get("https://matrix-client.matrix.org/_matrix/client/versions", { versions: ["v1.4"] }); - - mocked(manager.getProxyFromWellKnown).mockRestore(); - jest.spyOn(manager, "nativeSlidingSyncSupport").mockResolvedValue(false); - expect(SlidingSyncController.serverSupportsSlidingSync).toBeFalsy(); - await manager.checkSupport(client); - expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy(); - expect(fetchMockJest).not.toHaveFetched("https://matrix-client.matrix.org/.well-known/matrix/client"); - }); - }); - describe("nativeSlidingSyncSupport", () => { - beforeEach(() => { - SlidingSyncController.serverSupportsSlidingSync = false; - }); - it("should make an OPTIONS request to avoid unintended side effects", async () => { - // See https://github.com/element-hq/element-web/issues/27426 - - const unstableSpy = jest - .spyOn(client, "doesServerSupportUnstableFeature") - .mockImplementation(async (feature: string) => { - expect(feature).toBe("org.matrix.simplified_msc3575"); - return true; - }); - const proxySpy = jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("https://proxy/"); - - expect(SlidingSyncController.serverSupportsSlidingSync).toBeFalsy(); - await manager.checkSupport(client); // first thing it does is call nativeSlidingSyncSupport - expect(proxySpy).not.toHaveBeenCalled(); - expect(unstableSpy).toHaveBeenCalled(); expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy(); }); }); @@ -264,28 +210,11 @@ describe("SlidingSyncManager", () => { jest.spyOn(manager, "configure"); jest.spyOn(manager, "startSpidering"); }); - it("uses the baseUrl as a proxy if no proxy is set in the client well-known and the server has no native support", async () => { + it("uses the baseUrl", async () => { await manager.setup(client); expect(manager.configure).toHaveBeenCalled(); expect(manager.configure).toHaveBeenCalledWith(client, client.baseUrl); expect(manager.startSpidering).toHaveBeenCalled(); }); - it("uses the proxy declared in the client well-known", async () => { - jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("https://proxy/"); - await manager.setup(client); - expect(manager.configure).toHaveBeenCalled(); - expect(manager.configure).toHaveBeenCalledWith(client, "https://proxy/"); - expect(manager.startSpidering).toHaveBeenCalled(); - }); - it("uses the legacy `feature_sliding_sync_proxy_url` if it was set", async () => { - jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("https://proxy/"); - jest.spyOn(SettingsStore, "getValue").mockImplementation((name: string) => { - if (name === "feature_sliding_sync_proxy_url") return "legacy-proxy"; - }); - await manager.setup(client); - expect(manager.configure).toHaveBeenCalled(); - expect(manager.configure).toHaveBeenCalledWith(client, "legacy-proxy"); - expect(manager.startSpidering).toHaveBeenCalled(); - }); }); }); From 48eec705ec324af1d51dd3503e6c2871b109839d Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Fri, 13 Sep 2024 14:54:02 +0100 Subject: [PATCH 04/13] Linting --- src/SlidingSyncManager.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index bdee8e202f..6f66316769 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -36,7 +36,7 @@ Please see LICENSE files in the repository root for full details. * list ops) */ -import { MatrixClient, EventType, AutoDiscovery, Method, timeoutSignal } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, EventType } from "matrix-js-sdk/src/matrix"; import { MSC3575Filter, MSC3575List, @@ -48,7 +48,6 @@ import { import { logger } from "matrix-js-sdk/src/logger"; import { defer, sleep } from "matrix-js-sdk/src/utils"; -import SettingsStore from "./settings/SettingsStore"; import SlidingSyncController from "./settings/controllers/SlidingSyncController"; // how long to long poll for From 8325c5934dc3befbe3f0537601f606dabfc952ae Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:34:14 +0100 Subject: [PATCH 05/13] Debug logging --- src/components/structures/TimelinePanel.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index f4c9b639ac..1d71b28c55 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -319,6 +319,7 @@ class TimelinePanel extends React.Component { } public componentDidMount(): void { + console.log("componentDidMount"); if (this.props.manageReadReceipts) { this.updateReadReceiptOnUserActivity(); } @@ -364,6 +365,7 @@ class TimelinePanel extends React.Component { } public componentWillUnmount(): void { + console.log("componentWillUnmount"); // set a boolean to say we've been unmounted, which any pending // promises can use to throw away their results. // @@ -704,6 +706,7 @@ class TimelinePanel extends React.Component { removed: boolean, data: IRoomTimelineData, ): void => { + console.log("onRoomTimeline ", ev, room, "toStart:",toStartOfTimeline, "removed:",removed,"data:",data); // ignore events for other timeline sets if ( data.timeline.getTimelineSet() !== this.props.timelineSet && @@ -711,7 +714,6 @@ class TimelinePanel extends React.Component { ) { return; } - if (!Thread.hasServerSideSupport && this.context.timelineRenderingType === TimelineRenderingType.Thread) { if (toStartOfTimeline && !this.state.canBackPaginate) { this.setState({ @@ -810,6 +812,7 @@ class TimelinePanel extends React.Component { } private onRoomTimelineReset = (room: Room | undefined, timelineSet: EventTimelineSet): void => { + console.log("onRoomTimelineReset", room, timelineSet); if (timelineSet !== this.props.timelineSet && timelineSet !== this.props.overlayTimelineSet) return; if (this.canResetTimeline()) { @@ -1425,6 +1428,7 @@ class TimelinePanel extends React.Component { }; private initTimeline(props: IProps): void { + console.log("initTimeline"); const initialEvent = props.eventId; const pixelOffset = props.eventPixelOffset; @@ -1534,6 +1538,7 @@ class TimelinePanel extends React.Component { * @param {boolean?} scrollIntoView whether to scroll the event into view. */ private loadTimeline(eventId?: string, pixelOffset?: number, offsetBase?: number, scrollIntoView = true): void { + console.log("loadTimeline"); const cli = MatrixClientPeg.safeGet(); this.timelineWindow = new TimelineWindow(cli, this.props.timelineSet, { windowLimit: this.props.timelineCap }); this.overlayTimelineWindow = this.props.overlayTimelineSet @@ -1542,7 +1547,7 @@ class TimelinePanel extends React.Component { const onLoaded = (): void => { if (this.unmounted) return; - + console.log("loadTimeline -> onLoaded"); // clear the timeline min-height when (re)loading the timeline this.messagePanel.current?.onTimelineReset(); this.reloadEvents(); @@ -1590,6 +1595,7 @@ class TimelinePanel extends React.Component { const onError = (error: MatrixError): void => { if (this.unmounted) return; + console.log("loadTimeline -> onError", error); this.setState({ timelineLoading: false }); logger.error(`Error loading timeline panel at ${this.props.timelineSet.room?.roomId}/${eventId}`, error); @@ -1644,7 +1650,7 @@ class TimelinePanel extends React.Component { onLoaded(); return; } - + console.log("calling timelineWindow.load"); const prom = this.timelineWindow.load(eventId, INITIAL_SIZE).then(async (): Promise => { if (this.overlayTimelineWindow) { // TODO: use timestampToEvent to load the overlay timeline @@ -1685,6 +1691,7 @@ class TimelinePanel extends React.Component { // get the list of events from the timeline windows and the pending event list private getEvents(): Pick { + console.log("getEvents"); const mainEvents = this.timelineWindow!.getEvents(); let overlayEvents = this.overlayTimelineWindow?.getEvents() ?? []; if (this.props.overlayTimelineSetFilter !== undefined) { From ee85f73ad1164769fc9f959f04a7b400dfded205 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Mon, 16 Sep 2024 15:10:26 +0100 Subject: [PATCH 06/13] Control the race condition when swapping between rooms --- src/SlidingSyncManager.ts | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 6f66316769..7231ca2108 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -36,7 +36,7 @@ Please see LICENSE files in the repository root for full details. * list ops) */ -import { MatrixClient, EventType } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, EventType, ClientEvent, Room } from "matrix-js-sdk/src/matrix"; import { MSC3575Filter, MSC3575List, @@ -232,29 +232,35 @@ export class SlidingSyncManager { subscriptions.delete(roomId); } const room = this.client?.getRoom(roomId); - let shouldLazyLoad = !this.client?.isRoomEncrypted(roomId); - if (!room) { - // default to safety: request all state if we can't work it out. This can happen if you - // refresh the app whilst viewing a room: we call setRoomVisible before we know anything - // about the room. - shouldLazyLoad = false; + // default to safety: request all state if we can't work it out. This can happen if you + // refresh the app whilst viewing a room: we call setRoomVisible before we know anything + // about the room. + let shouldLazyLoad = false; + if (room) { + // do not lazy load encrypted rooms as we need the entire member list. + shouldLazyLoad = !room.hasEncryptionStateEvent() } logger.log("SlidingSync setRoomVisible:", roomId, visible, "shouldLazyLoad:", shouldLazyLoad); if (shouldLazyLoad) { // lazy load this room this.slidingSync!.useCustomSubscription(roomId, UNENCRYPTED_SUBSCRIPTION_NAME); } - const p = this.slidingSync!.modifyRoomSubscriptions(subscriptions); + this.slidingSync!.modifyRoomSubscriptions(subscriptions); if (room) { return roomId; // we have data already for this room, show immediately e.g it's in a list } - try { - // wait until the next sync before returning as RoomView may need to know the current state - await p; - } catch (err) { - logger.warn("SlidingSync setRoomVisible:", roomId, visible, "failed to confirm transaction"); - } - return roomId; + // wait until we know about this room. This may take a little while. + return new Promise((resolve) => { + logger.log(`SlidingSync setRoomVisible room ${roomId} not found, waiting for ClientEvent.Room`); + const waitForRoom = (r: Room) => { + if (r.roomId === roomId) { + this.client?.off(ClientEvent.Room, waitForRoom); + logger.log(`SlidingSync room ${roomId} found, resolving setRoomVisible`); + resolve(roomId); + } + }; + this.client?.on(ClientEvent.Room, waitForRoom); + }); } /** From be768525295c024eb9c1a29904ab59007ccccc31 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Mon, 16 Sep 2024 16:31:35 +0100 Subject: [PATCH 07/13] Dont' filter by space as synapse doesn't support it --- src/stores/room-list/SlidingRoomListStore.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index 26d3291625..e87d12dcf8 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -337,7 +337,8 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl } private onSelectedSpaceUpdated = (activeSpace: SpaceKey, allRoomsInHome: boolean): void => { - logger.info("SlidingRoomListStore.onSelectedSpaceUpdated", activeSpace); + logger.info("TODO: Synapse does not implement filters.spaces yet. SlidingRoomListStore.onSelectedSpaceUpdated", activeSpace); + return; // update the untagged filter const tagId = DefaultTagID.Untagged; const filters = filterConditions[tagId]; From 0b142b496d4e23434e1a16762ec7ac80a2bed1fd Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:52:55 +0100 Subject: [PATCH 08/13] Remove SS code related to registering lists and managing ranges - Update the spidering code to spider all the relevant lists. - Add canonical alias to the required_state to allow room name calcs to work. Room sort order is busted because we don't yet look at `bump_stamp`. --- src/MatrixClientPeg.ts | 7 + src/SlidingSyncManager.ts | 288 +++++++------ src/components/structures/TimelinePanel.tsx | 10 - src/components/views/rooms/RoomSublist.tsx | 6 - src/hooks/useSlidingSyncRoomSearch.ts | 82 ---- src/stores/room-list/RoomListStore.ts | 14 +- src/stores/room-list/SlidingRoomListStore.ts | 396 ------------------ test/hooks/useSlidingSyncRoomSearch-test.tsx | 85 ---- .../room-list/SlidingRoomListStore-test.ts | 341 --------------- 9 files changed, 150 insertions(+), 1079 deletions(-) delete mode 100644 src/hooks/useSlidingSyncRoomSearch.ts delete mode 100644 src/stores/room-list/SlidingRoomListStore.ts delete mode 100644 test/hooks/useSlidingSyncRoomSearch-test.tsx delete mode 100644 test/stores/room-list/SlidingRoomListStore-test.ts diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 87ad8ec0cb..2c91ac5a93 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -42,6 +42,7 @@ import PlatformPeg from "./PlatformPeg"; import { formatList } from "./utils/FormattingUtils"; import SdkConfig from "./SdkConfig"; import { Features } from "./settings/Settings"; +import SlidingSyncController from "./settings/controllers/SlidingSyncController"; export interface IMatrixClientCreds { homeserverUrl: string; @@ -298,10 +299,16 @@ class MatrixClientPegClass implements IMatrixClientPeg { opts.clientWellKnownPollPeriod = 2 * 60 * 60; // 2 hours opts.threadSupport = true; + /* TODO: Uncomment before PR lands if (SettingsStore.getValue("feature_sliding_sync")) { opts.slidingSync = await SlidingSyncManager.instance.setup(this.matrixClient); } else { SlidingSyncManager.instance.checkSupport(this.matrixClient); + } */ + // TODO: remove before PR lands. Defaults to SSS if the server entered supports it. + await SlidingSyncManager.instance.checkSupport(this.matrixClient); + if (SlidingSyncController.serverSupportsSlidingSync) { + opts.slidingSync = await SlidingSyncManager.instance.setup(this.matrixClient); } // Connect the matrix client to the dispatcher and setting handlers diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 7231ca2108..8ae6917bd2 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -40,10 +40,13 @@ import { MatrixClient, EventType, ClientEvent, Room } from "matrix-js-sdk/src/ma import { MSC3575Filter, MSC3575List, + MSC3575SlidingSyncResponse, MSC3575_STATE_KEY_LAZY, MSC3575_STATE_KEY_ME, MSC3575_WILDCARD, SlidingSync, + SlidingSyncEvent, + SlidingSyncState, } from "matrix-js-sdk/src/sliding-sync"; import { logger } from "matrix-js-sdk/src/logger"; import { defer, sleep } from "matrix-js-sdk/src/utils"; @@ -53,20 +56,27 @@ import SlidingSyncController from "./settings/controllers/SlidingSyncController" // how long to long poll for const SLIDING_SYNC_TIMEOUT_MS = 20 * 1000; +// The state events we will get for every single room/space/old room/etc +// This list is only augmented when a direct room subscription is made. (e.g you view a room) +const REQUIRED_STATE_LIST = [ + [EventType.RoomJoinRules, ""], // the public icon on the room list + [EventType.RoomAvatar, ""], // any room avatar + [EventType.RoomCanonicalAlias, ""], // for room name calculations + [EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead + [EventType.RoomEncryption, ""], // lets rooms be configured for E2EE correctly + [EventType.RoomCreate, ""], // for isSpaceRoom checks + [EventType.SpaceChild, MSC3575_WILDCARD], // all space children + [EventType.SpaceParent, MSC3575_WILDCARD], // all space parents + [EventType.RoomMember, MSC3575_STATE_KEY_ME], // lets the client calculate that we are in fact in the room +]; + // the things to fetch when a user clicks on a room const DEFAULT_ROOM_SUBSCRIPTION_INFO = { timeline_limit: 50, // missing required_state which will change depending on the kind of room include_old_rooms: { timeline_limit: 0, - required_state: [ - // state needed to handle space navigation and tombstone chains - [EventType.RoomCreate, ""], - [EventType.RoomTombstone, ""], - [EventType.SpaceChild, MSC3575_WILDCARD], - [EventType.SpaceParent, MSC3575_WILDCARD], - [EventType.RoomMember, MSC3575_STATE_KEY_ME], - ], + required_state: REQUIRED_STATE_LIST, }, }; // lazy load room members so rooms like Matrix HQ don't take forever to load @@ -74,7 +84,6 @@ const UNENCRYPTED_SUBSCRIPTION_NAME = "unencrypted"; const UNENCRYPTED_SUBSCRIPTION = Object.assign( { required_state: [ - [MSC3575_WILDCARD, MSC3575_WILDCARD], // all events [EventType.RoomMember, MSC3575_STATE_KEY_ME], // except for m.room.members, get our own membership [EventType.RoomMember, MSC3575_STATE_KEY_LAZY], // ...and lazy load the rest. ], @@ -93,6 +102,72 @@ const ENCRYPTED_SUBSCRIPTION = Object.assign( DEFAULT_ROOM_SUBSCRIPTION_INFO, ); +// the complete set of lists made in SSS. The manager will spider all of these lists depending +// on the count for each one. +const sssLists: Record = { + spaces: { + ranges: [[0, 20]], + timeline_limit: 0, // we don't care about the most recent message for spaces + required_state: REQUIRED_STATE_LIST, + include_old_rooms: { + timeline_limit: 0, + required_state: REQUIRED_STATE_LIST, + }, + filters: { + room_types: ["m.space"], + }, + }, + invites: { + ranges: [[0, 20]], + timeline_limit: 1, // most recent message display + required_state: REQUIRED_STATE_LIST, + include_old_rooms: { + timeline_limit: 0, + required_state: REQUIRED_STATE_LIST, + }, + filters: { + is_invite: true, + }, + }, + favourites: { + ranges: [[0, 20]], + timeline_limit: 1, // most recent message display + required_state: REQUIRED_STATE_LIST, + include_old_rooms: { + timeline_limit: 0, + required_state: REQUIRED_STATE_LIST, + }, + filters: { + tags: ["m.favourite"], + }, + }, + dms: { + ranges: [[0, 20]], + timeline_limit: 1, // most recent message display + required_state: REQUIRED_STATE_LIST, + include_old_rooms: { + timeline_limit: 0, + required_state: REQUIRED_STATE_LIST, + }, + filters: { + is_dm: true, + is_invite: false, + // If a DM has a Favourite & Low Prio tag then it'll be shown in those lists instead + not_tags: ["m.favourite", "m.lowpriority"], + }, + }, + untagged: { + // SSS will dupe suppress invites/dms from here, so we don't need "not dms, not invites" + ranges: [[0, 20]], + timeline_limit: 1, // most recent message display + required_state: REQUIRED_STATE_LIST, + include_old_rooms: { + timeline_limit: 0, + required_state: REQUIRED_STATE_LIST, + }, + }, +}; + export type PartialSlidingSyncRequest = { filters?: MSC3575Filter; sort?: string[]; @@ -119,110 +194,27 @@ export class SlidingSyncManager { return SlidingSyncManager.internalInstance; } - public configure(client: MatrixClient, proxyUrl: string): SlidingSync { + private configure(client: MatrixClient, proxyUrl: string): SlidingSync { this.client = client; + // create the set of lists we will use. + const lists = new Map(); + for (let listName in sssLists) { + lists.set(listName, sssLists[listName]); + } // 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(), + lists, ENCRYPTED_SUBSCRIPTION, client, SLIDING_SYNC_TIMEOUT_MS, ); this.slidingSync.addCustomSubscription(UNENCRYPTED_SUBSCRIPTION_NAME, UNENCRYPTED_SUBSCRIPTION); - // set the space list - this.slidingSync.setList(SlidingSyncManager.ListSpaces, { - ranges: [[0, 20]], - sort: ["by_name"], - slow_get_all_rooms: true, - timeline_limit: 0, - required_state: [ - [EventType.RoomJoinRules, ""], // the public icon on the room list - [EventType.RoomAvatar, ""], // any room avatar - [EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead - [EventType.RoomEncryption, ""], // lets rooms be configured for E2EE correctly - [EventType.RoomCreate, ""], // for isSpaceRoom checks - [EventType.SpaceChild, MSC3575_WILDCARD], // all space children - [EventType.SpaceParent, MSC3575_WILDCARD], // all space parents - [EventType.RoomMember, MSC3575_STATE_KEY_ME], // lets the client calculate that we are in fact in the room - ], - include_old_rooms: { - timeline_limit: 0, - required_state: [ - [EventType.RoomCreate, ""], - [EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead - [EventType.SpaceChild, MSC3575_WILDCARD], // all space children - [EventType.SpaceParent, MSC3575_WILDCARD], // all space parents - [EventType.RoomMember, MSC3575_STATE_KEY_ME], // lets the client calculate that we are in fact in the room - ], - }, - filters: { - room_types: ["m.space"], - }, - }); this.configureDefer.resolve(); return this.slidingSync; } - /** - * Ensure that this list is registered. - * @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(listKey: string, updateArgs: PartialSlidingSyncRequest): Promise { - logger.debug("ensureListRegistered:::", listKey, updateArgs); - await this.configureDefer.promise; - let list = this.slidingSync!.getListParams(listKey); - if (!list) { - list = { - ranges: [[0, 20]], - sort: ["by_notification_level", "by_recency"], - timeline_limit: 1, // most recent message display: though this seems to only be needed for favourites? - required_state: [ - [EventType.RoomJoinRules, ""], // the public icon on the room list - [EventType.RoomAvatar, ""], // any room avatar - [EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead - [EventType.RoomEncryption, ""], // lets rooms be configured for E2EE correctly - [EventType.RoomCreate, ""], // for isSpaceRoom checks - [EventType.RoomMember, MSC3575_STATE_KEY_ME], // lets the client calculate that we are in fact in the room - ], - include_old_rooms: { - timeline_limit: 0, - required_state: [ - [EventType.RoomCreate, ""], - [EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead - [EventType.SpaceChild, MSC3575_WILDCARD], // all space children - [EventType.SpaceParent, MSC3575_WILDCARD], // all space parents - [EventType.RoomMember, MSC3575_STATE_KEY_ME], // lets the client calculate that we are in fact in the room - ], - }, - }; - list = Object.assign(list, updateArgs); - } else { - const updatedList = Object.assign({}, list, updateArgs); - // cannot use objectHasDiff as we need to do deep diff checking - if (JSON.stringify(list) === JSON.stringify(updatedList)) { - logger.debug("list matches, not sending, update => ", updateArgs); - return list; - } - list = updatedList; - } - - 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) { - this.slidingSync!.setListRanges(listKey, updateArgs.ranges); - } else { - this.slidingSync!.setList(listKey, list); - } - } catch (err) { - logger.debug("ensureListRegistered: update failed txn_id=", err); - } - return this.slidingSync!.getListParams(listKey)!; - } - public async setRoomVisible(roomId: string, visible: boolean): Promise { await this.configureDefer.promise; const subscriptions = this.slidingSync!.getRoomSubscriptions(); @@ -264,57 +256,57 @@ export class SlidingSyncManager { } /** - * Retrieve all rooms on the user's account. Used for pre-populating the local search cache. - * Retrieval is gradual over time. + * Retrieve all rooms on the user's account. Retrieval is gradual over time. + * This function MUST be called BEFORE the first sync request goes out. * @param batchSize The number of rooms to return in each request. * @param gapBetweenRequestsMs The number of milliseconds to wait between requests. */ - 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 - let fetchUpTo = batchSize; - let hasMore = true; - let firstTime = true; - while (hasMore) { - try { - if (firstTime) { - await this.slidingSync!.setList(SlidingSyncManager.ListSearch, { - ranges: [[0, fetchUpTo]], - sort: [ - "by_recency", // this list isn't shown on the UI so just sorting by timestamp is enough - ], - timeline_limit: 0, // we only care about the room details, not messages in the room - required_state: [ - [EventType.RoomJoinRules, ""], // the public icon on the room list - [EventType.RoomAvatar, ""], // any room avatar - [EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead - [EventType.RoomEncryption, ""], // lets rooms be configured for E2EE correctly - [EventType.RoomCreate, ""], // for isSpaceRoom checks - [EventType.RoomMember, MSC3575_STATE_KEY_ME], // lets the client calculate that we are in fact in the room - ], - // we don't include_old_rooms here in an effort to reduce the impact of spidering all rooms - // on the user's account. This means some data in the search dialog results may be inaccurate - // e.g membership of space, but this will be corrected when the user clicks on the room - // as the direct room subscription does include old room iterations. - filters: { - // we get spaces via a different list, so filter them out - not_room_types: ["m.space"], - }, - }); - } else { - await this.slidingSync!.setListRanges(SlidingSyncManager.ListSearch, [[0,fetchUpTo]]); - } - } 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); + private async startSpidering(slidingSync: SlidingSync, batchSize: number, gapBetweenRequestsMs: number): Promise { + // The manager has created several lists (see `sssLists` in this file), all of which will be spidered simultaneously. + // There are multiple lists to ensure that we can populate invites/favourites/DMs sections immediately, rather than + // potentially waiting minutes if they are all very old rooms (and hence are returned last by the server). In this + // way, the lists are effectively priority requests. We don't actually care which room goes into which list at this + // point, as the RoomListStore will calculate this based on the returned data. + + // copy the initial set of list names and ranges, we'll keep this map updated. + const listToUpperBound = new Map(Object.keys(sssLists).map((listName) => { + return [listName, sssLists[listName].ranges[0][1]]; + })); + console.log("startSpidering:",listToUpperBound); + + // listen for a response from the server. ANY 200 OK will do here, as we assume that it is ACKing + // the request change we have sent out. TODO: this may not be true if you concurrently subscribe to a room :/ + // but in that case, for spidering at least, it isn't the end of the world as request N+1 includes all indexes + // from request N. + const lifecycle = async (state: SlidingSyncState, _: MSC3575SlidingSyncResponse | null, err?: Error) => { + if (state !== SlidingSyncState.Complete) { + return; + } + await sleep(gapBetweenRequestsMs); // don't tightloop; even on errors + if (err) { + return; + } + + // for all lists with total counts > range => increase the range + let hasSetRanges = false; + listToUpperBound.forEach((currentUpperBound, listName) => { + const totalCount = slidingSync.getListData(listName)?.joinedCount || 0; + if (currentUpperBound < totalCount) { + // increment the upper bound + const newUpperBound = currentUpperBound + batchSize; + console.log(`startSpidering: ${listName} ${currentUpperBound} => ${newUpperBound}`); + listToUpperBound.set(listName, newUpperBound); + // make the next request. This will only send the request when this callback has finished, so if + // we set all the list ranges at once we will only send 1 new request. + slidingSync.setListRanges(listName, [[0,newUpperBound]]); + hasSetRanges = true; + } + }) + if (!hasSetRanges) { // finish spidering + slidingSync.off(SlidingSyncEvent.Lifecycle, lifecycle); } - const listData = this.slidingSync!.getListData(SlidingSyncManager.ListSearch)!; - hasMore = fetchUpTo < listData.joinedCount; - fetchUpTo += batchSize; - firstTime = false; } + slidingSync.on(SlidingSyncEvent.Lifecycle, lifecycle); } /** @@ -327,10 +319,10 @@ export class SlidingSyncManager { * @returns A working Sliding Sync or undefined */ public async setup(client: MatrixClient): Promise { - this.configure(client, client.baseUrl); + const slidingSync = this.configure(client, client.baseUrl); logger.info("Simplified Sliding Sync activated at", client.baseUrl); - this.startSpidering(100, 50); // 100 rooms at a time, 50ms apart - return this.slidingSync; + this.startSpidering(slidingSync, 100, 50); // 100 rooms at a time, 50ms apart + return slidingSync; } /** diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index 1d71b28c55..f9cfa8faa8 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -319,7 +319,6 @@ class TimelinePanel extends React.Component { } public componentDidMount(): void { - console.log("componentDidMount"); if (this.props.manageReadReceipts) { this.updateReadReceiptOnUserActivity(); } @@ -365,7 +364,6 @@ class TimelinePanel extends React.Component { } public componentWillUnmount(): void { - console.log("componentWillUnmount"); // set a boolean to say we've been unmounted, which any pending // promises can use to throw away their results. // @@ -706,7 +704,6 @@ class TimelinePanel extends React.Component { removed: boolean, data: IRoomTimelineData, ): void => { - console.log("onRoomTimeline ", ev, room, "toStart:",toStartOfTimeline, "removed:",removed,"data:",data); // ignore events for other timeline sets if ( data.timeline.getTimelineSet() !== this.props.timelineSet && @@ -812,7 +809,6 @@ class TimelinePanel extends React.Component { } private onRoomTimelineReset = (room: Room | undefined, timelineSet: EventTimelineSet): void => { - console.log("onRoomTimelineReset", room, timelineSet); if (timelineSet !== this.props.timelineSet && timelineSet !== this.props.overlayTimelineSet) return; if (this.canResetTimeline()) { @@ -1428,7 +1424,6 @@ class TimelinePanel extends React.Component { }; private initTimeline(props: IProps): void { - console.log("initTimeline"); const initialEvent = props.eventId; const pixelOffset = props.eventPixelOffset; @@ -1538,7 +1533,6 @@ class TimelinePanel extends React.Component { * @param {boolean?} scrollIntoView whether to scroll the event into view. */ private loadTimeline(eventId?: string, pixelOffset?: number, offsetBase?: number, scrollIntoView = true): void { - console.log("loadTimeline"); const cli = MatrixClientPeg.safeGet(); this.timelineWindow = new TimelineWindow(cli, this.props.timelineSet, { windowLimit: this.props.timelineCap }); this.overlayTimelineWindow = this.props.overlayTimelineSet @@ -1547,7 +1541,6 @@ class TimelinePanel extends React.Component { const onLoaded = (): void => { if (this.unmounted) return; - console.log("loadTimeline -> onLoaded"); // clear the timeline min-height when (re)loading the timeline this.messagePanel.current?.onTimelineReset(); this.reloadEvents(); @@ -1595,7 +1588,6 @@ class TimelinePanel extends React.Component { const onError = (error: MatrixError): void => { if (this.unmounted) return; - console.log("loadTimeline -> onError", error); this.setState({ timelineLoading: false }); logger.error(`Error loading timeline panel at ${this.props.timelineSet.room?.roomId}/${eventId}`, error); @@ -1650,7 +1642,6 @@ class TimelinePanel extends React.Component { onLoaded(); return; } - console.log("calling timelineWindow.load"); const prom = this.timelineWindow.load(eventId, INITIAL_SIZE).then(async (): Promise => { if (this.overlayTimelineWindow) { // TODO: use timestampToEvent to load the overlay timeline @@ -1691,7 +1682,6 @@ class TimelinePanel extends React.Component { // get the list of events from the timeline windows and the pending event list private getEvents(): Pick { - console.log("getEvents"); const mainEvents = this.timelineWindow!.getEvents(); let overlayEvents = this.overlayTimelineWindow?.getEvents() ?? []; if (this.props.overlayTimelineSetFilter !== undefined) { diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 12f6e70d31..8c928f51e3 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -329,12 +329,6 @@ export default class RoomSublist extends React.Component { }; private onShowAllClick = async (): Promise => { - if (this.slidingSyncMode) { - const count = RoomListStore.instance.getCount(this.props.tagId); - await SlidingSyncManager.instance.ensureListRegistered(this.props.tagId, { - ranges: [[0, count]], - }); - } // read number of visible tiles before we mutate it const numVisibleTiles = this.numVisibleTiles; const newHeight = this.layout.tilesToPixelsWithPadding(this.numTiles, this.padding); diff --git a/src/hooks/useSlidingSyncRoomSearch.ts b/src/hooks/useSlidingSyncRoomSearch.ts deleted file mode 100644 index bd8529bfd9..0000000000 --- a/src/hooks/useSlidingSyncRoomSearch.ts +++ /dev/null @@ -1,82 +0,0 @@ -/* -Copyright 2024 New Vector Ltd. -Copyright 2022 The Matrix.org Foundation C.I.C. - -SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only -Please see LICENSE files in the repository root for full details. -*/ - -import { useCallback, useState } from "react"; -import { Room } from "matrix-js-sdk/src/matrix"; - -import { MatrixClientPeg } from "../MatrixClientPeg"; -import { useLatestResult } from "./useLatestResult"; -import { SlidingSyncManager } from "../SlidingSyncManager"; - -export interface SlidingSyncRoomSearchOpts { - limit: number; - query: string; -} - -export const useSlidingSyncRoomSearch = (): { - loading: boolean; - rooms: Room[]; - search(opts: SlidingSyncRoomSearchOpts): Promise; -} => { - const [rooms, setRooms] = useState([]); - - const [loading, setLoading] = useState(false); - - const [updateQuery, updateResult] = useLatestResult<{ term: string; limit?: number }, Room[]>(setRooms); - - const search = useCallback( - async ({ limit = 100, query: term }: SlidingSyncRoomSearchOpts): Promise => { - const opts = { limit, term }; - updateQuery(opts); - - if (!term?.length) { - setRooms([]); - return true; - } - - try { - setLoading(true); - await SlidingSyncManager.instance.ensureListRegistered(SlidingSyncManager.ListSearch, { - ranges: [[0, limit]], - filters: { - room_name_like: term, - }, - }); - const rooms: Room[] = []; - const { roomIndexToRoomId } = SlidingSyncManager.instance.slidingSync!.getListData( - SlidingSyncManager.ListSearch, - )!; - let i = 0; - while (roomIndexToRoomId[i]) { - const roomId = roomIndexToRoomId[i]; - const room = MatrixClientPeg.safeGet().getRoom(roomId); - if (room) { - rooms.push(room); - } - i++; - } - updateResult(opts, rooms); - return true; - } catch (e) { - console.error("Could not fetch sliding sync rooms for params", { limit, term }, e); - updateResult(opts, []); - return false; - } finally { - setLoading(false); - // TODO: delete the list? - } - }, - [updateQuery, updateResult], - ); - - return { - loading, - rooms, - search, - } as const; -}; diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index 53377e0a01..1b123a316d 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -27,7 +27,6 @@ import { VisibilityProvider } from "./filters/VisibilityProvider"; import { SpaceWatcher } from "./SpaceWatcher"; import { IRoomTimelineActionPayload } from "../../actions/MatrixActionCreators"; import { RoomListStore as Interface, RoomListStoreEvent } from "./Interface"; -import { SlidingRoomListStoreClass } from "./SlidingRoomListStore"; import { UPDATE_EVENT } from "../AsyncStore"; import { SdkContextClass } from "../../contexts/SDKContext"; import { getChangedOverrideRoomMutePushRules } from "./utils/roomMute"; @@ -640,16 +639,9 @@ export default class RoomListStore { public static get instance(): Interface { if (!RoomListStore.internalInstance) { - if (SettingsStore.getValue("feature_sliding_sync")) { - logger.info("using SlidingRoomListStoreClass"); - const instance = new SlidingRoomListStoreClass(defaultDispatcher, SdkContextClass.instance); - instance.start(); - RoomListStore.internalInstance = instance; - } else { - const instance = new RoomListStoreClass(defaultDispatcher); - instance.start(); - RoomListStore.internalInstance = instance; - } + const instance = new RoomListStoreClass(defaultDispatcher); + instance.start(); + RoomListStore.internalInstance = instance; } return this.internalInstance; diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts deleted file mode 100644 index e87d12dcf8..0000000000 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ /dev/null @@ -1,396 +0,0 @@ -/* -Copyright 2024 New Vector Ltd. -Copyright 2022 The Matrix.org Foundation C.I.C. - -SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only -Please see LICENSE files in the repository root for full details. -*/ - -import { Room } from "matrix-js-sdk/src/matrix"; -import { logger } from "matrix-js-sdk/src/logger"; -import { MSC3575Filter, SlidingSyncEvent } from "matrix-js-sdk/src/sliding-sync"; -import { Optional } from "matrix-events-sdk"; - -import { RoomUpdateCause, TagID, OrderedDefaultTagIDs, DefaultTagID } from "./models"; -import { ITagMap, ListAlgorithm, SortAlgorithm } from "./algorithms/models"; -import { ActionPayload } from "../../dispatcher/payloads"; -import { MatrixDispatcher } from "../../dispatcher/dispatcher"; -import { IFilterCondition } from "./filters/IFilterCondition"; -import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; -import { RoomListStore as Interface, RoomListStoreEvent } from "./Interface"; -import { MetaSpace, SpaceKey, UPDATE_SELECTED_SPACE } from "../spaces"; -import { LISTS_LOADING_EVENT } from "./RoomListStore"; -import { UPDATE_EVENT } from "../AsyncStore"; -import { SdkContextClass } from "../../contexts/SDKContext"; - -interface IState { - // state is tracked in underlying classes -} - -export const SlidingSyncSortToFilter: Record = { - [SortAlgorithm.Alphabetic]: ["by_name", "by_recency"], - [SortAlgorithm.Recent]: ["by_notification_level", "by_recency"], - [SortAlgorithm.Manual]: ["by_recency"], -}; - -const filterConditions: Record = { - [DefaultTagID.Invite]: { - is_invite: true, - }, - [DefaultTagID.Favourite]: { - tags: ["m.favourite"], - }, - [DefaultTagID.DM]: { - is_dm: true, - is_invite: false, - // If a DM has a Favourite & Low Prio tag then it'll be shown in those lists instead - not_tags: ["m.favourite", "m.lowpriority"], - }, - [DefaultTagID.Untagged]: { - is_dm: false, - is_invite: false, - not_room_types: ["m.space"], - not_tags: ["m.favourite", "m.lowpriority"], - // spaces filter added dynamically - }, - [DefaultTagID.LowPriority]: { - tags: ["m.lowpriority"], - // If a room has both Favourite & Low Prio tags then it'll be shown under Favourites - not_tags: ["m.favourite"], - }, - // TODO https://github.com/vector-im/element-web/issues/23207 - // DefaultTagID.ServerNotice, - // DefaultTagID.Suggested, - // DefaultTagID.Archived, -}; - -export const LISTS_UPDATE_EVENT = RoomListStoreEvent.ListsUpdate; - -export class SlidingRoomListStoreClass extends AsyncStoreWithClient implements Interface { - private tagIdToSortAlgo: Record = {}; - private tagMap: ITagMap = {}; - private counts: Record = {}; - private stickyRoomId: Optional; - - public constructor( - dis: MatrixDispatcher, - private readonly context: SdkContextClass, - ) { - super(dis); - this.setMaxListeners(20); // RoomList + LeftPanel + 8xRoomSubList + spares - } - - public async setTagSorting(tagId: TagID, sort: SortAlgorithm): Promise { - logger.info("SlidingRoomListStore.setTagSorting ", tagId, sort); - this.tagIdToSortAlgo[tagId] = sort; - switch (sort) { - case SortAlgorithm.Alphabetic: - await this.context.slidingSyncManager.ensureListRegistered(tagId, { - sort: SlidingSyncSortToFilter[SortAlgorithm.Alphabetic], - }); - break; - case SortAlgorithm.Recent: - await this.context.slidingSyncManager.ensureListRegistered(tagId, { - sort: SlidingSyncSortToFilter[SortAlgorithm.Recent], - }); - break; - case SortAlgorithm.Manual: - logger.error("cannot enable manual sort in sliding sync mode"); - break; - default: - logger.error("unknown sort mode: ", sort); - } - } - - public getTagSorting(tagId: TagID): SortAlgorithm { - let algo = this.tagIdToSortAlgo[tagId]; - if (!algo) { - logger.warn("SlidingRoomListStore.getTagSorting: no sort algorithm for tag ", tagId); - algo = SortAlgorithm.Recent; // why not, we have to do something.. - } - return algo; - } - - public getCount(tagId: TagID): number { - return this.counts[tagId] || 0; - } - - public setListOrder(tagId: TagID, order: ListAlgorithm): void { - // TODO: https://github.com/vector-im/element-web/issues/23207 - } - - public getListOrder(tagId: TagID): ListAlgorithm { - // TODO: handle unread msgs first? https://github.com/vector-im/element-web/issues/23207 - return ListAlgorithm.Natural; - } - - /** - * Adds a filter condition to the room list store. Filters may be applied async, - * and thus might not cause an update to the store immediately. - * @param {IFilterCondition} filter The filter condition to add. - */ - public async addFilter(filter: IFilterCondition): Promise { - // Do nothing, the filters are only used by SpaceWatcher to see if a room should appear - // in the room list. We do not support arbitrary code for filters in sliding sync. - } - - /** - * Removes a filter condition from the room list store. If the filter was - * not previously added to the room list store, this will no-op. The effects - * of removing a filter may be applied async and therefore might not cause - * an update right away. - * @param {IFilterCondition} filter The filter condition to remove. - */ - public removeFilter(filter: IFilterCondition): void { - // Do nothing, the filters are only used by SpaceWatcher to see if a room should appear - // in the room list. We do not support arbitrary code for filters in sliding sync. - } - - /** - * Gets the tags for a room identified by the store. The returned set - * should never be empty, and will contain DefaultTagID.Untagged if - * the store is not aware of any tags. - * @param room The room to get the tags for. - * @returns The tags for the room. - */ - public getTagsForRoom(room: Room): TagID[] { - // 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 listData = this.context.slidingSyncManager.slidingSync?.getListData(tagId); - if (!listData) { - continue; - } - for (const roomIndex in listData.roomIndexToRoomId) { - const roomId = listData.roomIndexToRoomId[roomIndex]; - if (roomId === room.roomId) { - tags.push(tagId); - break; - } - } - } - return tags; - } - - /** - * Manually update a room with a given cause. This should only be used if the - * room list store would otherwise be incapable of doing the update itself. Note - * that this may race with the room list's regular operation. - * @param {Room} room The room to update. - * @param {RoomUpdateCause} cause The cause to update for. - */ - public async manualRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { - // TODO: this is only used when you forget a room, not that important for now. - } - - public get orderedLists(): ITagMap { - return this.tagMap; - } - - private refreshOrderedLists(tagId: string, roomIndexToRoomId: Record): void { - const tagMap = this.tagMap; - - // this room will not move due to it being viewed: it is sticky. This can be null to indicate - // no sticky room if you aren't viewing a room. - this.stickyRoomId = this.context.roomViewStore.getRoomId(); - let stickyRoomNewIndex = -1; - const stickyRoomOldIndex = (tagMap[tagId] || []).findIndex((room): boolean => { - return room.roomId === this.stickyRoomId; - }); - - // order from low to high - const orderedRoomIndexes = Object.keys(roomIndexToRoomId) - .map((numStr) => { - return Number(numStr); - }) - .sort((a, b) => { - return a - b; - }); - const seenRoomIds = new Set(); - const orderedRoomIds = orderedRoomIndexes.map((i) => { - const rid = roomIndexToRoomId[i]; - if (seenRoomIds.has(rid)) { - logger.error("room " + rid + " already has an index position: duplicate room!"); - } - seenRoomIds.add(rid); - if (!rid) { - throw new Error("index " + i + " has no room ID: Map => " + JSON.stringify(roomIndexToRoomId)); - } - if (rid === this.stickyRoomId) { - stickyRoomNewIndex = i; - } - return rid; - }); - logger.debug( - `SlidingRoomListStore.refreshOrderedLists ${tagId} sticky: ${this.stickyRoomId}`, - `${stickyRoomOldIndex} -> ${stickyRoomNewIndex}`, - "rooms:", - orderedRoomIds.length < 30 ? orderedRoomIds : orderedRoomIds.length, - ); - - if (this.stickyRoomId && stickyRoomOldIndex >= 0 && stickyRoomNewIndex >= 0) { - // this update will move this sticky room from old to new, which we do not want. - // Instead, keep the sticky room ID index position as it is, swap it with - // whatever was in its place. - // Some scenarios with sticky room S and bump room B (other letters unimportant): - // A, S, C, B S, A, B - // B, A, S, C <---- without sticky rooms ---> B, S, A - // B, S, A, C <- with sticky rooms applied -> S, B, A - // In other words, we need to swap positions to keep it locked in place. - const inWayRoomId = orderedRoomIds[stickyRoomOldIndex]; - orderedRoomIds[stickyRoomOldIndex] = this.stickyRoomId; - orderedRoomIds[stickyRoomNewIndex] = inWayRoomId; - } - - // now set the rooms - const rooms: Room[] = []; - orderedRoomIds.forEach((roomId) => { - const room = this.matrixClient?.getRoom(roomId); - if (!room) { - return; - } - rooms.push(room); - }); - tagMap[tagId] = rooms; - this.tagMap = tagMap; - } - - private onSlidingSyncListUpdate(tagId: string, joinCount: number, roomIndexToRoomId: Record): void { - this.counts[tagId] = joinCount; - this.refreshOrderedLists(tagId, roomIndexToRoomId); - // let the UI update - this.emit(LISTS_UPDATE_EVENT); - } - - private onRoomViewStoreUpdated(): void { - // we only care about this to know when the user has clicked on a room to set the stickiness value - if (this.context.roomViewStore.getRoomId() === this.stickyRoomId) { - return; - } - - let hasUpdatedAnyList = false; - - // every list with the OLD sticky room ID needs to be resorted because it now needs to take - // its proper place as it is no longer sticky. The newly sticky room can remain the same though, - // as we only actually care about its sticky status when we get list updates. - const oldStickyRoom = this.stickyRoomId; - // it's not safe to check the data in slidingSync as it is tracking the server's view of the - // room list. There's an edge case whereby the sticky room has gone outside the window and so - // would not be present in the roomIndexToRoomId map anymore, and hence clicking away from it - // will make it disappear eventually. We need to check orderedLists as that is the actual - // sorted renderable list of rooms which sticky rooms apply to. - for (const tagId in this.orderedLists) { - const list = this.orderedLists[tagId]; - const room = list.find((room) => { - return room.roomId === oldStickyRoom; - }); - if (room) { - // resort it based on the slidingSync view of the list. This may cause this old sticky - // room to cease to exist. - const listData = this.context.slidingSyncManager.slidingSync?.getListData(tagId); - if (!listData) { - continue; - } - this.refreshOrderedLists(tagId, listData.roomIndexToRoomId); - hasUpdatedAnyList = true; - } - } - // in the event we didn't call refreshOrderedLists, it helps to still remember the sticky room ID. - this.stickyRoomId = this.context.roomViewStore.getRoomId(); - - if (hasUpdatedAnyList) { - this.emit(LISTS_UPDATE_EVENT); - } - } - - protected async onReady(): Promise { - logger.info("SlidingRoomListStore.onReady"); - // permanent listeners: never get destroyed. Could be an issue if we want to test this in isolation. - this.context.slidingSyncManager.slidingSync!.on(SlidingSyncEvent.List, this.onSlidingSyncListUpdate.bind(this)); - this.context.roomViewStore.addListener(UPDATE_EVENT, this.onRoomViewStoreUpdated.bind(this)); - this.context.spaceStore.on(UPDATE_SELECTED_SPACE, this.onSelectedSpaceUpdated.bind(this)); - if (this.context.spaceStore.activeSpace) { - this.onSelectedSpaceUpdated(this.context.spaceStore.activeSpace, false); - } - - // sliding sync has an initial response for spaces. Now request all the lists. - // We do the spaces list _first_ to avoid potential flickering on DefaultTagID.Untagged list - // which would be caused by initially having no `spaces` filter set, and then suddenly setting one. - OrderedDefaultTagIDs.forEach((tagId) => { - const filter = filterConditions[tagId]; - if (!filter) { - logger.info("SlidingRoomListStore.onReady unsupported list ", tagId); - return; // we do not support this list yet. - } - const sort = SortAlgorithm.Recent; // default to recency sort, TODO: read from config - this.tagIdToSortAlgo[tagId] = sort; - this.emit(LISTS_LOADING_EVENT, tagId, true); - this.context.slidingSyncManager - .ensureListRegistered(tagId, { - filters: filter, - sort: SlidingSyncSortToFilter[sort], - }) - .then(() => { - this.emit(LISTS_LOADING_EVENT, tagId, false); - }); - }); - } - - private onSelectedSpaceUpdated = (activeSpace: SpaceKey, allRoomsInHome: boolean): void => { - logger.info("TODO: Synapse does not implement filters.spaces yet. SlidingRoomListStore.onSelectedSpaceUpdated", activeSpace); - return; - // update the untagged filter - const tagId = DefaultTagID.Untagged; - const filters = filterConditions[tagId]; - const oldSpace = filters.spaces?.[0]; - filters.spaces = activeSpace && activeSpace != MetaSpace.Home ? [activeSpace] : undefined; - if (oldSpace !== activeSpace) { - // include subspaces in this list - this.context.spaceStore.traverseSpace( - activeSpace, - (roomId: string) => { - if (roomId === activeSpace) { - return; - } - if (!filters.spaces) { - filters.spaces = []; - } - filters.spaces.push(roomId); // add subspace - }, - false, - ); - - this.emit(LISTS_LOADING_EVENT, tagId, true); - this.context.slidingSyncManager - .ensureListRegistered(tagId, { - filters: filters, - }) - .then(() => { - this.emit(LISTS_LOADING_EVENT, tagId, false); - }); - } - }; - - // Intended for test usage - public async resetStore(): Promise { - // Test function - } - - /** - * Regenerates the room whole room list, discarding any previous results. - * - * Note: This is only exposed externally for the tests. Do not call this from within - * the app. - * @param trigger Set to false to prevent a list update from being sent. Should only - * be used if the calling code will manually trigger the update. - */ - public regenerateAllLists({ trigger = true }): void { - // Test function - } - - protected async onNotReady(): Promise { - await this.resetStore(); - } - - protected async onAction(payload: ActionPayload): Promise {} -} diff --git a/test/hooks/useSlidingSyncRoomSearch-test.tsx b/test/hooks/useSlidingSyncRoomSearch-test.tsx deleted file mode 100644 index 003fa2cafe..0000000000 --- a/test/hooks/useSlidingSyncRoomSearch-test.tsx +++ /dev/null @@ -1,85 +0,0 @@ -/* -Copyright 2024 New Vector Ltd. -Copyright 2023 The Matrix.org Foundation C.I.C. - -SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only -Please see LICENSE files in the repository root for full details. -*/ - -import { waitFor } from "@testing-library/react"; -import { renderHook, act } from "@testing-library/react-hooks/dom"; -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"; - -describe("useSlidingSyncRoomSearch", () => { - afterAll(() => { - jest.restoreAllMocks(); - }); - - 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 - const { result } = renderHook(() => useSlidingSyncRoomSearch()); - const query = { - limit: 10, - query: "foo", - }; - expect(result.current.loading).toBe(false); - expect(result.current.rooms).toEqual([]); - - // run the query - act(() => { - result.current.search(query); - }); - - // wait for loading to finish - await waitFor(() => { - expect(result.current.loading).toBe(false); - }); - - // now we expect there to be rooms - expect(result.current.rooms).toEqual([roomA, roomB]); - - // run the query again - act(() => { - result.current.search(query); - }); - await waitFor(() => { - expect(result.current.loading).toBe(false); - }); - }); -}); diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts deleted file mode 100644 index f667ef7dca..0000000000 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ /dev/null @@ -1,341 +0,0 @@ -/* -Copyright 2024 New Vector Ltd. -Copyright 2022 The Matrix.org Foundation C.I.C. - -SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only -Please see LICENSE files in the repository root for full details. -*/ -import { mocked } from "jest-mock"; -import { SlidingSync, SlidingSyncEvent } from "matrix-js-sdk/src/sliding-sync"; -import { Room } from "matrix-js-sdk/src/matrix"; - -import { - LISTS_UPDATE_EVENT, - SlidingRoomListStoreClass, - SlidingSyncSortToFilter, -} from "../../../src/stores/room-list/SlidingRoomListStore"; -import { SpaceStoreClass } from "../../../src/stores/spaces/SpaceStore"; -import { MockEventEmitter, stubClient, untilEmission } from "../../test-utils"; -import { TestSdkContext } from "../../TestSdkContext"; -import { SlidingSyncManager } from "../../../src/SlidingSyncManager"; -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 { 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"; - -jest.mock("../../../src/SlidingSyncManager"); -const MockSlidingSyncManager = >(SlidingSyncManager); - -describe("SlidingRoomListStore", () => { - let store: SlidingRoomListStoreClass; - let context: TestSdkContext; - let dis: MatrixDispatcher; - let activeSpace: string; - - beforeEach(async () => { - context = new TestSdkContext(); - context.client = stubClient(); - context._SpaceStore = new MockEventEmitter({ - traverseSpace: jest.fn(), - get activeSpace() { - return activeSpace; - }, - }) as SpaceStoreClass; - context._SlidingSyncManager = new MockSlidingSyncManager(); - context._SlidingSyncManager.slidingSync = mocked( - new MockEventEmitter({ - getListData: jest.fn(), - }) as unknown as SlidingSync, - ); - context._RoomViewStore = mocked( - new MockEventEmitter({ - getRoomId: jest.fn(), - }) as unknown as RoomViewStore, - ); - mocked(context._SlidingSyncManager.ensureListRegistered).mockResolvedValue({ - ranges: [[0, 10]], - }); - - dis = new MatrixDispatcher(); - store = new SlidingRoomListStoreClass(dis, context); - }); - - describe("spaces", () => { - it("alters 'filters.spaces' on the DefaultTagID.Untagged list when the selected space changes", async () => { - await store.start(); // call onReady - const spaceRoomId = "!foo:bar"; - - const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => { - return listName === DefaultTagID.Untagged && !isLoading; - }); - - // change the active space - activeSpace = spaceRoomId; - context._SpaceStore!.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); - await p; - - expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { - filters: expect.objectContaining({ - spaces: [spaceRoomId], - }), - }); - }); - - 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"; - activeSpace = spaceRoomId; - const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => { - return listName === DefaultTagID.Untagged && !isLoading; - }); - await store.start(); // call onReady - await p; - expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith( - DefaultTagID.Untagged, - expect.objectContaining({ - filters: expect.objectContaining({ - spaces: [spaceRoomId], - }), - }), - ); - }); - - it("includes subspaces in 'filters.spaces' when the selected space has subspaces", async () => { - await store.start(); // call onReady - const spaceRoomId = "!foo:bar"; - const subSpace1 = "!ss1:bar"; - const subSpace2 = "!ss2:bar"; - - const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => { - return listName === DefaultTagID.Untagged && !isLoading; - }); - - mocked(context._SpaceStore!.traverseSpace).mockImplementation( - (spaceId: string, fn: (roomId: string) => void) => { - if (spaceId === spaceRoomId) { - fn(subSpace1); - fn(subSpace2); - } - }, - ); - - // change the active space - activeSpace = spaceRoomId; - context._SpaceStore!.emit(UPDATE_SELECTED_SPACE, spaceRoomId, false); - await p; - - expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { - filters: expect.objectContaining({ - spaces: [spaceRoomId, subSpace1, subSpace2], - }), - }); - }); - }); - - it("setTagSorting alters the 'sort' option in the list", async () => { - const tagId: TagID = "foo"; - await store.setTagSorting(tagId, SortAlgorithm.Alphabetic); - expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(tagId, { - sort: SlidingSyncSortToFilter[SortAlgorithm.Alphabetic], - }); - expect(store.getTagSorting(tagId)).toEqual(SortAlgorithm.Alphabetic); - - await store.setTagSorting(tagId, SortAlgorithm.Recent); - expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(tagId, { - sort: SlidingSyncSortToFilter[SortAlgorithm.Recent], - }); - expect(store.getTagSorting(tagId)).toEqual(SortAlgorithm.Recent); - }); - - it("getTagsForRoom gets the tags for the room", async () => { - await store.start(); - const roomA = "!a:localhost"; - const roomB = "!b:localhost"; - const keyToListData: Record }> = { - [DefaultTagID.Untagged]: { - joinedCount: 10, - roomIndexToRoomId: { - 0: roomA, - 1: roomB, - }, - }, - [DefaultTagID.Favourite]: { - joinedCount: 2, - roomIndexToRoomId: { - 0: roomB, - }, - }, - }; - mocked(context._SlidingSyncManager!.slidingSync!.getListData).mockImplementation((key: string) => { - return keyToListData[key] || null; - }); - - 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([ - DefaultTagID.Favourite, - DefaultTagID.Untagged, - ]); - }); - - it("emits LISTS_UPDATE_EVENT when slidingSync lists update", async () => { - await store.start(); - const roomA = "!a:localhost"; - const roomB = "!b:localhost"; - const roomC = "!c:localhost"; - const tagId = DefaultTagID.Favourite; - const joinCount = 10; - const roomIndexToRoomId = { - // mixed to ensure we sort - 1: roomB, - 2: roomC, - 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()!), - ]; - mocked(context.client!.getRoom).mockImplementation((roomId: string) => { - switch (roomId) { - case roomA: - return rooms[0]; - case roomB: - return rooms[1]; - case roomC: - return rooms[2]; - } - return null; - }); - const p = untilEmission(store, LISTS_UPDATE_EVENT); - context.slidingSyncManager.slidingSync!.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); - await p; - expect(store.getCount(tagId)).toEqual(joinCount); - expect(store.orderedLists[tagId]).toEqual(rooms); - }); - - it("sets the sticky room on the basis of the viewed room in RoomViewStore", async () => { - await store.start(); - // seed the store with 3 rooms - const roomIdA = "!a:localhost"; - const roomIdB = "!b:localhost"; - const roomIdC = "!c:localhost"; - const tagId = DefaultTagID.Favourite; - const joinCount = 10; - const roomIndexToRoomId = { - // mixed to ensure we sort - 1: roomIdB, - 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) => { - switch (roomId) { - case roomIdA: - return roomA; - case roomIdB: - return roomB; - 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, roomB, roomC]); - - // make roomB sticky and inform the store - mocked(context.roomViewStore.getRoomId).mockReturnValue(roomIdB); - context.roomViewStore.emit(UPDATE_EVENT); - - // bump room C to the top, room B should not move from i=1 despite the list update saying to - roomIndexToRoomId[0] = roomIdC; - roomIndexToRoomId[1] = roomIdA; - roomIndexToRoomId[2] = roomIdB; - p = untilEmission(store, LISTS_UPDATE_EVENT); - context.slidingSyncManager.slidingSync!.emit(SlidingSyncEvent.List, tagId, joinCount, roomIndexToRoomId); - await p; - - // check that B didn't move and that A was put below B - expect(store.orderedLists[tagId]).toEqual([roomC, roomB, roomA]); - - // make room C sticky: rooms should move as a result, without needing an additional list update - mocked(context.roomViewStore.getRoomId).mockReturnValue(roomIdC); - p = untilEmission(store, LISTS_UPDATE_EVENT); - context.roomViewStore.emit(UPDATE_EVENT); - 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, - }; - }); - 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 7621fe268266bccb9043797c7b4429a5f1bc5fe4 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Tue, 17 Sep 2024 12:11:25 +0100 Subject: [PATCH 09/13] User bumpStamp if it is present --- .../room-list/algorithms/tag-sorting/RecentAlgorithm.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts index d30703d288..82fe885a5b 100644 --- a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts @@ -69,6 +69,12 @@ const getLastTs = (r: Room, userId: string): number => { if (!r?.timeline) { return Number.MAX_SAFE_INTEGER; } + // MSC4186: Simplified Sliding Sync sets this. + // If it's present, sort by it. + const bumpStamp = r.getBumpStamp(); + if (bumpStamp) { + return bumpStamp; + } // If the room hasn't been joined yet, it probably won't have a timeline to // parse. We'll still fall back to the timeline if this fails, but chances From 7662b990a3cf9473e34a78e9d57ee109a58e27c0 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:02:59 +0100 Subject: [PATCH 10/13] Drop initial room load from 20 per list to 10 --- src/SlidingSyncManager.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index 8ae6917bd2..abc6157117 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -106,7 +106,7 @@ const ENCRYPTED_SUBSCRIPTION = Object.assign( // on the count for each one. const sssLists: Record = { spaces: { - ranges: [[0, 20]], + ranges: [[0, 10]], timeline_limit: 0, // we don't care about the most recent message for spaces required_state: REQUIRED_STATE_LIST, include_old_rooms: { @@ -118,7 +118,7 @@ const sssLists: Record = { }, }, invites: { - ranges: [[0, 20]], + ranges: [[0, 10]], timeline_limit: 1, // most recent message display required_state: REQUIRED_STATE_LIST, include_old_rooms: { @@ -130,7 +130,7 @@ const sssLists: Record = { }, }, favourites: { - ranges: [[0, 20]], + ranges: [[0, 10]], timeline_limit: 1, // most recent message display required_state: REQUIRED_STATE_LIST, include_old_rooms: { @@ -142,7 +142,7 @@ const sssLists: Record = { }, }, dms: { - ranges: [[0, 20]], + ranges: [[0, 10]], timeline_limit: 1, // most recent message display required_state: REQUIRED_STATE_LIST, include_old_rooms: { @@ -158,7 +158,7 @@ const sssLists: Record = { }, untagged: { // SSS will dupe suppress invites/dms from here, so we don't need "not dms, not invites" - ranges: [[0, 20]], + ranges: [[0, 10]], timeline_limit: 1, // most recent message display required_state: REQUIRED_STATE_LIST, include_old_rooms: { From 52c2436f4537f6d1d59ad8782253b5aa21e780bf Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:00:01 +0100 Subject: [PATCH 11/13] Half the batch size to trickle more quickly --- src/SlidingSyncManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index abc6157117..f7f52cb5a1 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -321,7 +321,7 @@ export class SlidingSyncManager { public async setup(client: MatrixClient): Promise { const slidingSync = this.configure(client, client.baseUrl); logger.info("Simplified Sliding Sync activated at", client.baseUrl); - this.startSpidering(slidingSync, 100, 50); // 100 rooms at a time, 50ms apart + this.startSpidering(slidingSync, 50, 50); // 50 rooms at a time, 50ms apart return slidingSync; } From 628d2bdadb6ef01fcdd257fdb45eb80739d5955f Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 21 Nov 2024 16:07:21 +0000 Subject: [PATCH 12/13] Prettier --- src/SlidingSyncManager.ts | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts index edc5138fd8..0bb7c372b8 100644 --- a/src/SlidingSyncManager.ts +++ b/src/SlidingSyncManager.ts @@ -199,13 +199,7 @@ export class SlidingSyncManager { } // 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, - lists, - ENCRYPTED_SUBSCRIPTION, - client, - SLIDING_SYNC_TIMEOUT_MS, - ); + this.slidingSync = new SlidingSync(proxyUrl, lists, ENCRYPTED_SUBSCRIPTION, client, SLIDING_SYNC_TIMEOUT_MS); this.slidingSync.addCustomSubscription(UNENCRYPTED_SUBSCRIPTION_NAME, UNENCRYPTED_SUBSCRIPTION); this.configureDefer.resolve(); return this.slidingSync; @@ -315,7 +309,11 @@ export class SlidingSyncManager { * @param batchSize The number of rooms to return in each request. * @param gapBetweenRequestsMs The number of milliseconds to wait between requests. */ - private async startSpidering(slidingSync: SlidingSync, batchSize: number, gapBetweenRequestsMs: number): Promise { + private async startSpidering( + slidingSync: SlidingSync, + batchSize: number, + gapBetweenRequestsMs: number, + ): Promise { // The manager has created several lists (see `sssLists` in this file), all of which will be spidered simultaneously. // There are multiple lists to ensure that we can populate invites/favourites/DMs sections immediately, rather than // potentially waiting minutes if they are all very old rooms (and hence are returned last by the server). In this @@ -323,16 +321,22 @@ export class SlidingSyncManager { // point, as the RoomListStore will calculate this based on the returned data. // copy the initial set of list names and ranges, we'll keep this map updated. - const listToUpperBound = new Map(Object.keys(sssLists).map((listName) => { - return [listName, sssLists[listName].ranges[0][1]]; - })); - console.log("startSpidering:",listToUpperBound); + const listToUpperBound = new Map( + Object.keys(sssLists).map((listName) => { + return [listName, sssLists[listName].ranges[0][1]]; + }), + ); + console.log("startSpidering:", listToUpperBound); // listen for a response from the server. ANY 200 OK will do here, as we assume that it is ACKing // the request change we have sent out. TODO: this may not be true if you concurrently subscribe to a room :/ // but in that case, for spidering at least, it isn't the end of the world as request N+1 includes all indexes // from request N. - const lifecycle = async (state: SlidingSyncState, _: MSC3575SlidingSyncResponse | null, err?: Error): Promise => { + const lifecycle = async ( + state: SlidingSyncState, + _: MSC3575SlidingSyncResponse | null, + err?: Error, + ): Promise => { if (state !== SlidingSyncState.Complete) { return; } @@ -341,7 +345,7 @@ export class SlidingSyncManager { return; } - // for all lists with total counts > range => increase the range + // for all lists with total counts > range => increase the range let hasSetRanges = false; listToUpperBound.forEach((currentUpperBound, listName) => { const totalCount = slidingSync.getListData(listName)?.joinedCount || 0; @@ -352,11 +356,12 @@ export class SlidingSyncManager { listToUpperBound.set(listName, newUpperBound); // make the next request. This will only send the request when this callback has finished, so if // we set all the list ranges at once we will only send 1 new request. - slidingSync.setListRanges(listName, [[0,newUpperBound]]); + slidingSync.setListRanges(listName, [[0, newUpperBound]]); hasSetRanges = true; } }); - if (!hasSetRanges) { // finish spidering + if (!hasSetRanges) { + // finish spidering slidingSync.off(SlidingSyncEvent.Lifecycle, lifecycle); } }; From 473d86120bff0ea862d22af12a8a244c6e00b1d8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 21 Nov 2024 16:12:15 +0000 Subject: [PATCH 13/13] prettier on tests too --- test/unit-tests/SlidingSyncManager-test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/unit-tests/SlidingSyncManager-test.ts b/test/unit-tests/SlidingSyncManager-test.ts index 3a515fa312..80ac19e766 100644 --- a/test/unit-tests/SlidingSyncManager-test.ts +++ b/test/unit-tests/SlidingSyncManager-test.ts @@ -167,9 +167,7 @@ describe("SlidingSyncManager", () => { ); return; } - expect(slidingSync.setListRanges).toHaveBeenCalledWith(SlidingSyncManager.ListSearch, [ - range, - ]); + expect(slidingSync.setListRanges).toHaveBeenCalledWith(SlidingSyncManager.ListSearch, [range]); }); }); it("handles accounts with zero rooms", async () => { @@ -187,9 +185,7 @@ describe("SlidingSyncManager", () => { expect(slidingSync.setList).toHaveBeenCalledWith( SlidingSyncManager.ListSearch, expect.objectContaining({ - ranges: [ - [0, batchSize], - ], + ranges: [[0, batchSize]], }), ); });