Room List - Update the room list store on actions from the dispatcher (#29397)

* Update the store on action

* Add more tests

* Add newlines between case blocks

* Make code more readable

- Make if/else more consistent
- Add comment on findAndAddRoom()

* Add more tests

* 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.

* Fix test

* Remove more redundant code

* Add more tests

* Explain intention in comment

* Emit only once even when adding multiple rooms

* Add missing tsdoc

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,100 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient<EmptyObject> {
     }
 
     protected async onAction(payload: ActionPayload): Promise<void> {
-        return;
+        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)) {
+                    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();
+                let needsEmit = false;
+                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.roomSkipList.addRoom(room);
+                        needsEmit = true;
+                    }
+                }
+                if (needsEmit) this.emit(LISTS_UPDATE_EVENT);
+                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;
+                this.addRoomAndEmit(payload.room);
+                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;
+            }
+        }
+    }
+
+    /**
+     * 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);
+        this.emit(LISTS_UPDATE_EVENT);
     }
 }
 

test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts

@@ -5,13 +5,17 @@ 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 { logger } from "matrix-js-sdk/src/logger";
+
 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 { LISTS_UPDATE_EVENT } from "../../../../src/stores/room-list/RoomListStore";
+import dispatcher from "../../../../src/dispatcher/dispatcher";
 
 describe("RoomListStoreV3", () => {
     async function getRoomListStore() {
@@ -19,10 +23,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(dispatcher);
-        const store = new RoomListStoreV3Class(fakeDispatcher);
         store.start();
-        return { client, rooms, store };
+        return { client, rooms, store, dispatcher };
     }
 
     it("Provides an unsorted list of rooms", async () => {
@@ -50,4 +53,216 @@ 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,
+            );
+
+            // 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.
+            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));
+        });
+
+        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();
+        });
+
+        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 = {
+                    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();
+            });
+        });
+    });
 });