Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit dcf497d

Browse files
authored
sliding-sync: spider all rooms on the user's account for search (#9514)
* sliding-sync: spider all rooms on the user's account for search On startup, slowly accumulate room metadata for all rooms on the user's account. This is so we can populate the local search cache with enough data for it to function, obviating the need to have separate code paths for sliding sync searches. This will allow spotlight search to work with slow/no network connectivity, though clicking on the room will still require a round trip. This is an explicit request from @ara4n to improve the snapiness of room searches, despite the unbounded bandwidth costs requesting all N rooms on the user's account. * Comments and tweak defaults * Review comments; remove SS search code * bugfix: use setListRanges once the list has been set up If we don't, then we send needless extra data and can cause bugs because setList will wipe the index->room_id map, which trips up SlidingRoomListStore.
1 parent 253129e commit dcf497d

File tree

4 files changed

+198
-45
lines changed

4 files changed

+198
-45
lines changed

src/MatrixClientPeg.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ class MatrixClientPegClass implements IMatrixClientPeg {
249249
this.matrixClient,
250250
proxyUrl || this.matrixClient.baseUrl,
251251
);
252+
SlidingSyncManager.instance.startSpidering(100, 50); // 100 rooms at a time, 50ms apart
252253
}
253254

254255
// Connect the matrix client to the dispatcher and setting handlers

src/SlidingSyncManager.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import {
5252
SlidingSync,
5353
} from 'matrix-js-sdk/src/sliding-sync';
5454
import { logger } from "matrix-js-sdk/src/logger";
55-
import { IDeferred, defer } from 'matrix-js-sdk/src/utils';
55+
import { IDeferred, defer, sleep } from 'matrix-js-sdk/src/utils';
5656

5757
// how long to long poll for
5858
const SLIDING_SYNC_TIMEOUT_MS = 20 * 1000;
@@ -109,6 +109,9 @@ export class SlidingSyncManager {
109109
public configure(client: MatrixClient, proxyUrl: string): SlidingSync {
110110
this.client = client;
111111
this.listIdToIndex = {};
112+
DEFAULT_ROOM_SUBSCRIPTION_INFO.include_old_rooms.required_state.push(
113+
[EventType.RoomMember, client.getUserId()],
114+
);
112115
this.slidingSync = new SlidingSync(
113116
proxyUrl, [], DEFAULT_ROOM_SUBSCRIPTION_INFO, client, SLIDING_SYNC_TIMEOUT_MS,
114117
);
@@ -262,4 +265,60 @@ export class SlidingSyncManager {
262265
}
263266
return roomId;
264267
}
268+
269+
/**
270+
* Retrieve all rooms on the user's account. Used for pre-populating the local search cache.
271+
* Retrieval is gradual over time.
272+
* @param batchSize The number of rooms to return in each request.
273+
* @param gapBetweenRequestsMs The number of milliseconds to wait between requests.
274+
*/
275+
public async startSpidering(batchSize: number, gapBetweenRequestsMs: number) {
276+
await sleep(gapBetweenRequestsMs); // wait a bit as this is called on first render so let's let things load
277+
const listIndex = this.getOrAllocateListIndex(SlidingSyncManager.ListSearch);
278+
let startIndex = batchSize;
279+
let hasMore = true;
280+
let firstTime = true;
281+
while (hasMore) {
282+
const endIndex = startIndex + batchSize-1;
283+
try {
284+
const ranges = [[0, batchSize-1], [startIndex, endIndex]];
285+
if (firstTime) {
286+
await this.slidingSync.setList(listIndex, {
287+
// e.g [0,19] [20,39] then [0,19] [40,59]. We keep [0,20] constantly to ensure
288+
// any changes to the list whilst spidering are caught.
289+
ranges: ranges,
290+
sort: [
291+
"by_recency", // this list isn't shown on the UI so just sorting by timestamp is enough
292+
],
293+
timeline_limit: 0, // we only care about the room details, not messages in the room
294+
required_state: [
295+
[EventType.RoomJoinRules, ""], // the public icon on the room list
296+
[EventType.RoomAvatar, ""], // any room avatar
297+
[EventType.RoomTombstone, ""], // lets JS SDK hide rooms which are dead
298+
[EventType.RoomEncryption, ""], // lets rooms be configured for E2EE correctly
299+
[EventType.RoomCreate, ""], // for isSpaceRoom checks
300+
[EventType.RoomMember, this.client.getUserId()!], // lets the client calculate that we are in fact in the room
301+
],
302+
// we don't include_old_rooms here in an effort to reduce the impact of spidering all rooms
303+
// on the user's account. This means some data in the search dialog results may be inaccurate
304+
// e.g membership of space, but this will be corrected when the user clicks on the room
305+
// as the direct room subscription does include old room iterations.
306+
filters: { // we get spaces via a different list, so filter them out
307+
not_room_types: ["m.space"],
308+
},
309+
});
310+
} else {
311+
await this.slidingSync.setListRanges(listIndex, ranges);
312+
}
313+
// gradually request more over time
314+
await sleep(gapBetweenRequestsMs);
315+
} catch (err) {
316+
// do nothing, as we reject only when we get interrupted but that's fine as the next
317+
// request will include our data
318+
}
319+
hasMore = (endIndex+1) < this.slidingSync.getListData(listIndex)?.joinedCount;
320+
startIndex += batchSize;
321+
firstTime = false;
322+
}
323+
}
265324
}

src/components/views/dialogs/spotlight/SpotlightDialog.tsx

Lines changed: 23 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ import { RoomResultContextMenus } from "./RoomResultContextMenus";
9191
import { RoomContextDetails } from "../../rooms/RoomContextDetails";
9292
import { TooltipOption } from "./TooltipOption";
9393
import { isLocalRoom } from "../../../../utils/localRoom/isLocalRoom";
94-
import { useSlidingSyncRoomSearch } from "../../../../hooks/useSlidingSyncRoomSearch";
9594
import { shouldShowFeedback } from "../../../../utils/Feedback";
9695
import RoomAvatar from "../../avatars/RoomAvatar";
9796

@@ -342,43 +341,26 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
342341
searchProfileInfo,
343342
searchParams,
344343
);
345-
const isSlidingSyncEnabled = SettingsStore.getValue("feature_sliding_sync");
346-
let {
347-
loading: slidingSyncRoomSearchLoading,
348-
rooms: slidingSyncRooms,
349-
search: searchRoomsServerside,
350-
} = useSlidingSyncRoomSearch();
351-
useDebouncedCallback(isSlidingSyncEnabled, searchRoomsServerside, searchParams);
352-
if (!isSlidingSyncEnabled) {
353-
slidingSyncRoomSearchLoading = false;
354-
}
355344

356345
const possibleResults = useMemo<Result[]>(
357346
() => {
358347
const userResults: IMemberResult[] = [];
359-
let roomResults: IRoomResult[];
360-
let alreadyAddedUserIds: Set<string>;
361-
if (isSlidingSyncEnabled) {
362-
// use the rooms sliding sync returned as the server has already worked it out for us
363-
roomResults = slidingSyncRooms.map(toRoomResult);
364-
} else {
365-
roomResults = findVisibleRooms(cli).map(toRoomResult);
366-
// If we already have a DM with the user we're looking for, we will
367-
// show that DM instead of the user themselves
368-
alreadyAddedUserIds = roomResults.reduce((userIds, result) => {
369-
const userId = DMRoomMap.shared().getUserIdForRoomId(result.room.roomId);
370-
if (!userId) return userIds;
371-
if (result.room.getJoinedMemberCount() > 2) return userIds;
372-
userIds.add(userId);
373-
return userIds;
374-
}, new Set<string>());
375-
for (const user of [...findVisibleRoomMembers(cli), ...users]) {
376-
// Make sure we don't have any user more than once
377-
if (alreadyAddedUserIds.has(user.userId)) continue;
378-
alreadyAddedUserIds.add(user.userId);
379-
380-
userResults.push(toMemberResult(user));
381-
}
348+
const roomResults = findVisibleRooms(cli).map(toRoomResult);
349+
// If we already have a DM with the user we're looking for, we will
350+
// show that DM instead of the user themselves
351+
const alreadyAddedUserIds = roomResults.reduce((userIds, result) => {
352+
const userId = DMRoomMap.shared().getUserIdForRoomId(result.room.roomId);
353+
if (!userId) return userIds;
354+
if (result.room.getJoinedMemberCount() > 2) return userIds;
355+
userIds.add(userId);
356+
return userIds;
357+
}, new Set<string>());
358+
for (const user of [...findVisibleRoomMembers(cli), ...users]) {
359+
// Make sure we don't have any user more than once
360+
if (alreadyAddedUserIds.has(user.userId)) continue;
361+
alreadyAddedUserIds.add(user.userId);
362+
363+
userResults.push(toMemberResult(user));
382364
}
383365

384366
return [
@@ -402,7 +384,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
402384
...publicRooms.map(toPublicRoomResult),
403385
].filter(result => filter === null || result.filter.includes(filter));
404386
},
405-
[cli, users, profile, publicRooms, slidingSyncRooms, isSlidingSyncEnabled, filter],
387+
[cli, users, profile, publicRooms, filter],
406388
);
407389

408390
const results = useMemo<Record<Section, Result[]>>(() => {
@@ -421,13 +403,10 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
421403

422404
possibleResults.forEach(entry => {
423405
if (isRoomResult(entry)) {
424-
// sliding sync gives the correct rooms in the list so we don't need to filter
425-
if (!isSlidingSyncEnabled) {
426-
if (!entry.room.normalizedName?.includes(normalizedQuery) &&
427-
!entry.room.getCanonicalAlias()?.toLowerCase().includes(lcQuery) &&
428-
!entry.query?.some(q => q.includes(lcQuery))
429-
) return; // bail, does not match query
430-
}
406+
if (!entry.room.normalizedName?.includes(normalizedQuery) &&
407+
!entry.room.getCanonicalAlias()?.toLowerCase().includes(lcQuery) &&
408+
!entry.query?.some(q => q.includes(lcQuery))
409+
) return; // bail, does not match query
431410
} else if (isMemberResult(entry)) {
432411
if (!entry.query?.some(q => q.includes(lcQuery))) return; // bail, does not match query
433412
} else if (isPublicRoomResult(entry)) {
@@ -478,7 +457,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
478457
}
479458

480459
return results;
481-
}, [trimmedQuery, filter, cli, possibleResults, isSlidingSyncEnabled, memberComparator]);
460+
}, [trimmedQuery, filter, cli, possibleResults, memberComparator]);
482461

483462
const numResults = sum(Object.values(results).map(it => it.length));
484463
useWebSearchMetrics(numResults, query.length, true);
@@ -1236,7 +1215,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
12361215
aria-label={_t("Search")}
12371216
aria-describedby="mx_SpotlightDialog_keyboardPrompt"
12381217
/>
1239-
{ (publicRoomsLoading || peopleLoading || profileLoading || slidingSyncRoomSearchLoading) && (
1218+
{ (publicRoomsLoading || peopleLoading || profileLoading) && (
12401219
<Spinner w={24} h={24} />
12411220
) }
12421221
</div>

test/SlidingSyncManager-test.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
Copyright 2022 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import { SlidingSync } from 'matrix-js-sdk/src/sliding-sync';
18+
import { mocked } from 'jest-mock';
19+
20+
import { SlidingSyncManager } from '../src/SlidingSyncManager';
21+
import { stubClient } from './test-utils';
22+
23+
jest.mock('matrix-js-sdk/src/sliding-sync');
24+
const MockSlidingSync = <jest.Mock<SlidingSync>><unknown>SlidingSync;
25+
26+
describe('SlidingSyncManager', () => {
27+
let manager: SlidingSyncManager;
28+
let slidingSync: SlidingSync;
29+
30+
beforeEach(() => {
31+
slidingSync = new MockSlidingSync();
32+
manager = new SlidingSyncManager();
33+
manager.configure(stubClient(), "invalid");
34+
manager.slidingSync = slidingSync;
35+
});
36+
37+
describe("startSpidering", () => {
38+
it("requests in batchSizes", async () => {
39+
const gapMs = 1;
40+
const batchSize = 10;
41+
mocked(slidingSync.setList).mockResolvedValue("yep");
42+
mocked(slidingSync.setListRanges).mockResolvedValue("yep");
43+
mocked(slidingSync.getListData).mockImplementation((i) => {
44+
return {
45+
joinedCount: 64,
46+
roomIndexToRoomId: {},
47+
};
48+
});
49+
await manager.startSpidering(batchSize, gapMs);
50+
// we expect calls for 10,19 -> 20,29 -> 30,39 -> 40,49 -> 50,59 -> 60,69
51+
const wantWindows = [
52+
[10, 19], [20, 29], [30, 39], [40, 49], [50, 59], [60, 69],
53+
];
54+
expect(slidingSync.getListData).toBeCalledTimes(wantWindows.length);
55+
expect(slidingSync.setList).toBeCalledTimes(1);
56+
expect(slidingSync.setListRanges).toBeCalledTimes(wantWindows.length-1);
57+
wantWindows.forEach((range, i) => {
58+
if (i === 0) {
59+
expect(slidingSync.setList).toBeCalledWith(
60+
manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch),
61+
expect.objectContaining({
62+
ranges: [[0, batchSize-1], range],
63+
}),
64+
);
65+
return;
66+
}
67+
expect(slidingSync.setListRanges).toBeCalledWith(
68+
manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch),
69+
[[0, batchSize-1], range],
70+
);
71+
});
72+
});
73+
it("handles accounts with zero rooms", async () => {
74+
const gapMs = 1;
75+
const batchSize = 10;
76+
mocked(slidingSync.setList).mockResolvedValue("yep");
77+
mocked(slidingSync.getListData).mockImplementation((i) => {
78+
return {
79+
joinedCount: 0,
80+
roomIndexToRoomId: {},
81+
};
82+
});
83+
await manager.startSpidering(batchSize, gapMs);
84+
expect(slidingSync.getListData).toBeCalledTimes(1);
85+
expect(slidingSync.setList).toBeCalledTimes(1);
86+
expect(slidingSync.setList).toBeCalledWith(
87+
manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch),
88+
expect.objectContaining({
89+
ranges: [[0, batchSize-1], [batchSize, batchSize+batchSize-1]],
90+
}),
91+
);
92+
});
93+
it("continues even when setList rejects", async () => {
94+
const gapMs = 1;
95+
const batchSize = 10;
96+
mocked(slidingSync.setList).mockRejectedValue("narp");
97+
mocked(slidingSync.getListData).mockImplementation((i) => {
98+
return {
99+
joinedCount: 0,
100+
roomIndexToRoomId: {},
101+
};
102+
});
103+
await manager.startSpidering(batchSize, gapMs);
104+
expect(slidingSync.getListData).toBeCalledTimes(1);
105+
expect(slidingSync.setList).toBeCalledTimes(1);
106+
expect(slidingSync.setList).toBeCalledWith(
107+
manager.getOrAllocateListIndex(SlidingSyncManager.ListSearch),
108+
expect.objectContaining({
109+
ranges: [[0, batchSize-1], [batchSize, batchSize+batchSize-1]],
110+
}),
111+
);
112+
});
113+
});
114+
});

0 commit comments

Comments
 (0)