From e4a7b6cff871dafc165cc041543b0a516b5259a5 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 15:05:22 +0530 Subject: [PATCH 01/12] Update the store on action --- src/stores/room-list-v3/RoomListStoreV3.ts | 94 +++++++++++++- .../room-list-v3/RoomListStoreV3-test.ts | 115 +++++++++++++++++- 2 files changed, 202 insertions(+), 7 deletions(-) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index 2904680ea8a..566ee7361f5 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -5,7 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import type { EmptyObject, Room } from "matrix-js-sdk/src/matrix"; +import { logger } from "matrix-js-sdk/src/logger"; +import { EventType } from "matrix-js-sdk/src/matrix"; + +import type { EmptyObject, Room, RoomState } from "matrix-js-sdk/src/matrix"; import type { MatrixDispatcher } from "../../dispatcher/dispatcher"; import type { ActionPayload } from "../../dispatcher/payloads"; import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; @@ -16,6 +19,8 @@ import { LISTS_UPDATE_EVENT } from "../room-list/RoomListStore"; import { RoomSkipList } from "./skip-list/RoomSkipList"; import { RecencySorter } from "./skip-list/sorters/RecencySorter"; import { AlphabeticSorter } from "./skip-list/sorters/AlphabeticSorter"; +import { readReceiptChangeIsFor } from "../../utils/read-receipts"; +import { EffectiveMembership, getEffectiveMembership, getEffectiveMembershipTag } from "../../utils/membership"; /** * This store allows for fast retrieval of the room list in a sorted and filtered manner. @@ -78,7 +83,92 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { } protected async onAction(payload: ActionPayload): Promise { - return; + if (!this.matrixClient || !this.roomSkipList?.initialized) return; + + switch (payload.action) { + case "MatrixActions.Room.receipt": { + if (readReceiptChangeIsFor(payload.event, this.matrixClient)) { + const room = payload.room; + if (!room) { + logger.warn(`Own read receipt was in unknown room ${room.roomId}`); + return; + } + this.addRoomAndEmit(room); + } + break; + } + case "MatrixActions.Room.tags": { + const room = payload.room; + this.addRoomAndEmit(room); + break; + } + case "MatrixActions.Event.decrypted": { + const roomId = payload.event.getRoomId(); + if (!roomId) return; + const room = this.matrixClient.getRoom(roomId); + if (!room) { + logger.warn(`Event ${payload.event.getId()} was decrypted in an unknown room ${roomId}`); + return; + } + this.addRoomAndEmit(room); + break; + } + case "MatrixActions.accountData": { + if (payload.event_type !== EventType.Direct) return; + const dmMap = payload.event.getContent(); + for (const userId of Object.keys(dmMap)) { + const roomIds = dmMap[userId]; + for (const roomId of roomIds) { + const room = this.matrixClient.getRoom(roomId); + if (!room) { + logger.warn(`${roomId} was found in DMs but the room is not in the store`); + continue; + } + this.addRoomAndEmit(room); + } + } + break; + } + case "MatrixActions.Room.timeline": { + // Ignore non-live events (backfill) and notification timeline set events (without a room) + if (!payload.isLiveEvent || !payload.isLiveUnfilteredRoomTimelineEvent || !payload.room) return; + + const roomId = payload.event.getRoomId(); + const tryAdd = (): boolean => { + const room = this.matrixClient?.getRoom(roomId); + if (room) this.addRoomAndEmit(room); + return !!room; + }; + if (!tryAdd()) setTimeout(tryAdd, 100); + break; + } + case "MatrixActions.Room.myMembership": { + const oldMembership = getEffectiveMembership(payload.oldMembership); + const newMembership = getEffectiveMembershipTag(payload.room, payload.membership); + if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) { + // If we're joining an upgraded room, we'll want to make sure we don't proliferate + // the dead room in the list. + const roomState: RoomState = payload.room.currentState; + const predecessor = roomState.findPredecessor(this.msc3946ProcessDynamicPredecessor); + if (predecessor) { + const prevRoom = this.matrixClient?.getRoom(predecessor.roomId); + if (prevRoom) { + this.roomSkipList.removeRoom(prevRoom); + } else { + logger.warn(`Unable to find predecessor room with id ${predecessor.roomId}`); + } + } + } + this.addRoomAndEmit(payload.room); + break; + } + } + } + + private addRoomAndEmit(room: Room): void { + if (!this.roomSkipList) throw new Error("roomSkipList hasn't been created yet!"); + this.roomSkipList.addRoom(room); + this.emit(LISTS_UPDATE_EVENT); } } diff --git a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts index cd37b04e346..25401bb56a4 100644 --- a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts +++ b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts @@ -5,13 +5,16 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import type { MatrixDispatcher } from "../../../../src/dispatcher/dispatcher"; +import { EventType, KnownMembership, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; + import { RoomListStoreV3Class } from "../../../../src/stores/room-list-v3/RoomListStoreV3"; import { AsyncStoreWithClient } from "../../../../src/stores/AsyncStoreWithClient"; import { RecencySorter } from "../../../../src/stores/room-list-v3/skip-list/sorters/RecencySorter"; -import { stubClient } from "../../../test-utils"; +import { mkEvent, mkMessage, stubClient, upsertRoomStateEvents } from "../../../test-utils"; import { getMockedRooms } from "./skip-list/getMockedRooms"; import { AlphabeticSorter } from "../../../../src/stores/room-list-v3/skip-list/sorters/AlphabeticSorter"; +import dispatcher from "../../../../src/dispatcher/dispatcher"; +import { LISTS_UPDATE_EVENT } from "../../../../src/stores/room-list/RoomListStore"; describe("RoomListStoreV3", () => { async function getRoomListStore() { @@ -19,10 +22,9 @@ describe("RoomListStoreV3", () => { const rooms = getMockedRooms(client); client.getVisibleRooms = jest.fn().mockReturnValue(rooms); jest.spyOn(AsyncStoreWithClient.prototype, "matrixClient", "get").mockReturnValue(client); - const fakeDispatcher = { register: jest.fn() } as unknown as MatrixDispatcher; - const store = new RoomListStoreV3Class(fakeDispatcher); + const store = new RoomListStoreV3Class(dispatcher); store.start(); - return { client, rooms, store }; + return { client, rooms, store, dispatcher }; } it("Provides an unsorted list of rooms", async () => { @@ -50,4 +52,107 @@ describe("RoomListStoreV3", () => { sortedRooms = new RecencySorter(client.getSafeUserId()).sort(rooms); expect(store.getSortedRooms()).toEqual(sortedRooms); }); + + describe("Updates", () => { + it("Room is re-inserted on timeline event", async () => { + const { store, rooms, dispatcher } = await getRoomListStore(); + + // Let's pretend like a new timeline event came on the room in 37th index. + const room = rooms[37]; + const event = mkMessage({ room: room.roomId, user: `@foo${3}:matrix.org`, ts: 1000, event: true }); + room.timeline.push(event); + + const payload = { + action: "MatrixActions.Room.timeline", + event, + isLiveEvent: true, + isLiveUnfilteredRoomTimelineEvent: true, + room, + }; + + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch(payload, true); + + expect(fn).toHaveBeenCalled(); + expect(store.getSortedRooms()[0].roomId).toEqual(room.roomId); + }); + + it("Predecessor room is removed on room upgrade", async () => { + const { store, rooms, client, dispatcher } = await getRoomListStore(); + // Let's say that !foo32:matrix.org is being upgraded + const oldRoom = rooms[32]; + // Create a new room with a predecessor event that points to oldRoom + const newRoom = new Room("!foonew:matrix.org", client, client.getSafeUserId(), {}); + const createWithPredecessor = new MatrixEvent({ + type: EventType.RoomCreate, + sender: "@foo:foo.org", + room_id: newRoom.roomId, + content: { + predecessor: { room_id: oldRoom.roomId, event_id: "tombstone_event_id" }, + }, + event_id: "$create", + state_key: "", + }); + upsertRoomStateEvents(newRoom, [createWithPredecessor]); + + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch( + { + action: "MatrixActions.Room.myMembership", + oldMembership: KnownMembership.Invite, + membership: KnownMembership.Join, + room: newRoom, + }, + true, + ); + + expect(fn).toHaveBeenCalled(); + const roomIds = store.getSortedRooms().map((r) => r.roomId); + expect(roomIds).not.toContain(oldRoom.roomId); + expect(roomIds).toContain(newRoom.roomId); + }); + + it("Rooms are inserted on m.direct event", async () => { + const { store, dispatcher } = await getRoomListStore(); + + // Let's create a m.direct event that we can dispatch + const content = { + "@bar1:matrix.org": ["!newroom1:matrix.org", "!newroom2:matrix.org"], + "@bar2:matrix.org": ["!newroom3:matrix.org", "!newroom4:matrix.org"], + "@bar3:matrix.org": ["!newroom5:matrix.org"], + }; + const event = mkEvent({ + event: true, + content, + user: "@foo:matrix.org", + type: EventType.Direct, + }); + + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch( + { + action: "MatrixActions.accountData", + event_type: EventType.Direct, + event, + }, + true, + ); + + // Each of these rooms should now appear in the store + // We don't need to mock the rooms themselves since our mocked + // client will create the rooms on getRoom() call. + expect(fn).toHaveBeenCalledTimes(5); + const roomIds = store.getSortedRooms().map((r) => r.roomId); + [ + "!newroom1:matrix.org", + "!newroom2:matrix.org", + "!newroom3:matrix.org", + "!newroom4:matrix.org", + "!newroom5:matrix.org", + ].forEach((id) => expect(roomIds).toContain(id)); + }); + }); }); From fbab49dbb93b1e761d888d74af12a89dd5c07ff9 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 15:58:30 +0530 Subject: [PATCH 02/12] Add more tests --- .../room-list-v3/RoomListStoreV3-test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts index 25401bb56a4..cc98a48eb0d 100644 --- a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts +++ b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts @@ -154,5 +154,35 @@ describe("RoomListStoreV3", () => { "!newroom5:matrix.org", ].forEach((id) => expect(roomIds).toContain(id)); }); + + it("Room is re-inserted on tag change", async () => { + const { store, rooms, dispatcher } = await getRoomListStore(); + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch( + { + action: "MatrixActions.Room.tags", + room: rooms[10], + }, + true, + ); + expect(fn).toHaveBeenCalled(); + }); + + it("Room is re-inserted on decryption", async () => { + const { store, rooms, client, dispatcher } = await getRoomListStore(); + jest.spyOn(client, "getRoom").mockImplementation(() => rooms[10]); + + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch( + { + action: "MatrixActions.Event.decrypted", + event: { getRoomId: () => rooms[10].roomId }, + }, + true, + ); + expect(fn).toHaveBeenCalled(); + }); }); }); From 72576601b82362eceec09cddf2ba1b4832cd7f09 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 16:08:40 +0530 Subject: [PATCH 03/12] Add newlines between case blocks --- src/stores/room-list-v3/RoomListStoreV3.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index 566ee7361f5..171c7c805e4 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -97,11 +97,13 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { } break; } + case "MatrixActions.Room.tags": { const room = payload.room; this.addRoomAndEmit(room); break; } + case "MatrixActions.Event.decrypted": { const roomId = payload.event.getRoomId(); if (!roomId) return; @@ -113,6 +115,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { this.addRoomAndEmit(room); break; } + case "MatrixActions.accountData": { if (payload.event_type !== EventType.Direct) return; const dmMap = payload.event.getContent(); @@ -129,6 +132,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { } break; } + case "MatrixActions.Room.timeline": { // Ignore non-live events (backfill) and notification timeline set events (without a room) if (!payload.isLiveEvent || !payload.isLiveUnfilteredRoomTimelineEvent || !payload.room) return; @@ -142,6 +146,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { if (!tryAdd()) setTimeout(tryAdd, 100); break; } + case "MatrixActions.Room.myMembership": { const oldMembership = getEffectiveMembership(payload.oldMembership); const newMembership = getEffectiveMembershipTag(payload.room, payload.membership); From 5f6aa6c397ae2fa803efca45e31eeaf42620aa13 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 17:18:17 +0530 Subject: [PATCH 04/12] Make code more readable - Make if/else more consistent - Add comment on findAndAddRoom() --- src/stores/room-list-v3/RoomListStoreV3.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index 171c7c805e4..de3502b7d8a 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -138,12 +138,18 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { if (!payload.isLiveEvent || !payload.isLiveUnfilteredRoomTimelineEvent || !payload.room) return; const roomId = payload.event.getRoomId(); - const tryAdd = (): boolean => { + const findAndAddRoom = (): boolean => { const room = this.matrixClient?.getRoom(roomId); if (room) this.addRoomAndEmit(room); return !!room; }; - if (!tryAdd()) setTimeout(tryAdd, 100); + // We'll try finding the room associated with this event. + // If we can't find the room, we'll try again after 100ms. + if (!findAndAddRoom()) { + logger.warn(`Live timeline event ${payload.event.getId()} received without associated room`); + logger.warn(`Queuing failed room update for retry as a result.`); + setTimeout(findAndAddRoom, 100); + } break; } @@ -157,11 +163,9 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { const predecessor = roomState.findPredecessor(this.msc3946ProcessDynamicPredecessor); if (predecessor) { const prevRoom = this.matrixClient?.getRoom(predecessor.roomId); - if (prevRoom) { - this.roomSkipList.removeRoom(prevRoom); - } else { - logger.warn(`Unable to find predecessor room with id ${predecessor.roomId}`); - } + if (prevRoom) this.roomSkipList.removeRoom(prevRoom); + else logger.warn(`Unable to find predecessor room with id ${predecessor.roomId}`); + } } this.addRoomAndEmit(payload.room); From c9a715e781a054ffdbac7be18caaf0d1bfb355ba Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 17:20:27 +0530 Subject: [PATCH 05/12] Add more tests --- .../room-list-v3/RoomListStoreV3-test.ts | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts index cc98a48eb0d..4908609b5ef 100644 --- a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts +++ b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts @@ -184,5 +184,59 @@ describe("RoomListStoreV3", () => { ); expect(fn).toHaveBeenCalled(); }); + + describe("Update from read receipt", () => { + function getReadReceiptEvent(userId: string) { + const content = { + some_id: { + "m.read": { + [userId]: { + ts: 5000, + }, + }, + }, + }; + const event = mkEvent({ + event: true, + content, + user: "@foo:matrix.org", + type: EventType.Receipt, + }); + return event; + } + + it("Room is re-inserted on read receipt from our user", async () => { + const { store, rooms, client, dispatcher } = await getRoomListStore(); + const event = getReadReceiptEvent(client.getSafeUserId()); + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch( + { + action: "MatrixActions.Room.receipt", + room: rooms[10], + event, + }, + true, + ); + expect(fn).toHaveBeenCalled(); + }); + + it("Read receipt from other users do not cause room to be re-inserted", async () => { + const { store, rooms, dispatcher } = await getRoomListStore(); + const event = getReadReceiptEvent("@foobar:matrix.org"); + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + dispatcher.dispatch( + { + action: "MatrixActions.Room.receipt", + room: rooms[10], + event, + }, + true, + ); + expect(fn).not.toHaveBeenCalled(); + }); + }); + }); }); From 39087a0a2f1590a7dc84e84e552989b57dfb10e4 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 18:16:43 +0530 Subject: [PATCH 06/12] Remove redundant code On a timeline action, we return early if payload.room is falsy. So then why do we need to retry fetching the room? I think this can be removed but will ask others if there's some conext I'm missing. --- src/stores/room-list-v3/RoomListStoreV3.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index de3502b7d8a..5aad4712ef3 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -138,18 +138,9 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { if (!payload.isLiveEvent || !payload.isLiveUnfilteredRoomTimelineEvent || !payload.room) return; const roomId = payload.event.getRoomId(); - const findAndAddRoom = (): boolean => { - const room = this.matrixClient?.getRoom(roomId); - if (room) this.addRoomAndEmit(room); - return !!room; - }; - // We'll try finding the room associated with this event. - // If we can't find the room, we'll try again after 100ms. - if (!findAndAddRoom()) { - logger.warn(`Live timeline event ${payload.event.getId()} received without associated room`); - logger.warn(`Queuing failed room update for retry as a result.`); - setTimeout(findAndAddRoom, 100); - } + const room = this.matrixClient?.getRoom(roomId); + if (room) this.addRoomAndEmit(room); + else logger.warn(`Live timeline event ${payload.event.getId()} received without associated room`); break; } @@ -165,7 +156,6 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { const prevRoom = this.matrixClient?.getRoom(predecessor.roomId); if (prevRoom) this.roomSkipList.removeRoom(prevRoom); else logger.warn(`Unable to find predecessor room with id ${predecessor.roomId}`); - } } this.addRoomAndEmit(payload.room); From ec05fff3a14432d3df3e2b067efff08b9a9d7513 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 18:18:24 +0530 Subject: [PATCH 07/12] Fix test --- test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts index 4908609b5ef..2fcb9c67cad 100644 --- a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts +++ b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts @@ -237,6 +237,5 @@ describe("RoomListStoreV3", () => { expect(fn).not.toHaveBeenCalled(); }); }); - }); }); From 86bcaa9779ab1843d630bcd10afca28cf0eda593 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 18:51:53 +0530 Subject: [PATCH 08/12] Remove more redundant code --- src/stores/room-list-v3/RoomListStoreV3.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index 5aad4712ef3..417f4df413b 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -136,11 +136,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { case "MatrixActions.Room.timeline": { // Ignore non-live events (backfill) and notification timeline set events (without a room) if (!payload.isLiveEvent || !payload.isLiveUnfilteredRoomTimelineEvent || !payload.room) return; - - const roomId = payload.event.getRoomId(); - const room = this.matrixClient?.getRoom(roomId); - if (room) this.addRoomAndEmit(room); - else logger.warn(`Live timeline event ${payload.event.getId()} received without associated room`); + this.addRoomAndEmit(payload.room); break; } From 2f7f16e10c7bb580a535f2f0abbc4242a5507eee Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Sun, 2 Mar 2025 18:52:06 +0530 Subject: [PATCH 09/12] Add more tests --- .../room-list-v3/RoomListStoreV3-test.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts index 2fcb9c67cad..dae9a3d58e5 100644 --- a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts +++ b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts @@ -6,6 +6,7 @@ Please see LICENSE files in the repository root for full details. */ import { EventType, KnownMembership, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; +import { logger } from "matrix-js-sdk/src/logger"; import { RoomListStoreV3Class } from "../../../../src/stores/room-list-v3/RoomListStoreV3"; import { AsyncStoreWithClient } from "../../../../src/stores/AsyncStoreWithClient"; @@ -13,8 +14,8 @@ import { RecencySorter } from "../../../../src/stores/room-list-v3/skip-list/sor import { mkEvent, mkMessage, stubClient, upsertRoomStateEvents } from "../../../test-utils"; import { getMockedRooms } from "./skip-list/getMockedRooms"; import { AlphabeticSorter } from "../../../../src/stores/room-list-v3/skip-list/sorters/AlphabeticSorter"; -import dispatcher from "../../../../src/dispatcher/dispatcher"; import { LISTS_UPDATE_EVENT } from "../../../../src/stores/room-list/RoomListStore"; +import dispatcher from "../../../../src/dispatcher/dispatcher"; describe("RoomListStoreV3", () => { async function getRoomListStore() { @@ -185,6 +186,30 @@ describe("RoomListStoreV3", () => { expect(fn).toHaveBeenCalled(); }); + it("Logs a warning if room couldn't be found from room-id on decryption action", async () => { + const { store, client, dispatcher } = await getRoomListStore(); + jest.spyOn(client, "getRoom").mockImplementation(() => null); + const warnSpy = jest.spyOn(logger, "warn"); + + const fn = jest.fn(); + store.on(LISTS_UPDATE_EVENT, fn); + + // Dispatch a decrypted action but the room does not exist. + dispatcher.dispatch( + { + action: "MatrixActions.Event.decrypted", + event: { + getRoomId: () => "!doesnotexist:matrix.org", + getId: () => "some-id", + }, + }, + true, + ); + + expect(warnSpy).toHaveBeenCalled(); + expect(fn).not.toHaveBeenCalled(); + }); + describe("Update from read receipt", () => { function getReadReceiptEvent(userId: string) { const content = { From 946581127e0227604fb917e59138ce0fe4b889d6 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Mon, 3 Mar 2025 16:56:33 +0530 Subject: [PATCH 10/12] Explain intention in comment --- src/stores/room-list-v3/RoomListStoreV3.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index 417f4df413b..dbf35388fa4 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -85,6 +85,12 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { protected async onAction(payload: ActionPayload): Promise { if (!this.matrixClient || !this.roomSkipList?.initialized) return; + /** + * For the kind of updates that we care about (represented by the cases below), + * we try to find the associated room and simply re-insert it into the + * skiplist. If the position of said room in the sorted list changed, re-inserting + * would put it in the correct place. + */ switch (payload.action) { case "MatrixActions.Room.receipt": { if (readReceiptChangeIsFor(payload.event, this.matrixClient)) { From bab8d937bc319794e39afa857c5d7991c65af816 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Mon, 3 Mar 2025 20:25:42 +0530 Subject: [PATCH 11/12] Emit only once even when adding multiple rooms --- src/stores/room-list-v3/RoomListStoreV3.ts | 5 ++++- test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index dbf35388fa4..0d54a7ce3dc 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -125,6 +125,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { case "MatrixActions.accountData": { if (payload.event_type !== EventType.Direct) return; const dmMap = payload.event.getContent(); + let needsEmit = false; for (const userId of Object.keys(dmMap)) { const roomIds = dmMap[userId]; for (const roomId of roomIds) { @@ -133,9 +134,11 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { logger.warn(`${roomId} was found in DMs but the room is not in the store`); continue; } - this.addRoomAndEmit(room); + this.roomSkipList.addRoom(room); + needsEmit = true; } } + if (needsEmit) this.emit(LISTS_UPDATE_EVENT); break; } diff --git a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts index dae9a3d58e5..ad3ccdcfd93 100644 --- a/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts +++ b/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts @@ -142,10 +142,12 @@ describe("RoomListStoreV3", () => { true, ); + // Ensure only one emit occurs + expect(fn).toHaveBeenCalledTimes(1); + // Each of these rooms should now appear in the store // We don't need to mock the rooms themselves since our mocked // client will create the rooms on getRoom() call. - expect(fn).toHaveBeenCalledTimes(5); const roomIds = store.getSortedRooms().map((r) => r.roomId); [ "!newroom1:matrix.org", From 3e32e2d632e24fb61a15348bc03a1875fb83c6c0 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Mon, 3 Mar 2025 20:28:00 +0530 Subject: [PATCH 12/12] Add missing tsdoc --- src/stores/room-list-v3/RoomListStoreV3.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/stores/room-list-v3/RoomListStoreV3.ts b/src/stores/room-list-v3/RoomListStoreV3.ts index 0d54a7ce3dc..0e704097610 100644 --- a/src/stores/room-list-v3/RoomListStoreV3.ts +++ b/src/stores/room-list-v3/RoomListStoreV3.ts @@ -169,6 +169,10 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient { } } + /** + * Add a room to the skiplist and emit an update. + * @param room The room to add to the skiplist + */ private addRoomAndEmit(room: Room): void { if (!this.roomSkipList) throw new Error("roomSkipList hasn't been created yet!"); this.roomSkipList.addRoom(room);