Test & Refactor SpaceWatcher to allow all rooms/home change without needing reload

src/stores/room-list/SpaceWatcher.ts

@@ -18,39 +18,47 @@ import { Room } from "matrix-js-sdk/src/models/room";
 
 import { RoomListStoreClass } from "./RoomListStore";
 import { SpaceFilterCondition } from "./filters/SpaceFilterCondition";
-import SpaceStore, { UPDATE_SELECTED_SPACE } from "../SpaceStore";
+import SpaceStore, { UPDATE_HOME_BEHAVIOUR, UPDATE_SELECTED_SPACE } from "../SpaceStore";
 
 /**
  * Watches for changes in spaces to manage the filter on the provided RoomListStore
  */
 export class SpaceWatcher {
-    private filter: SpaceFilterCondition;
+    private readonly filter = new SpaceFilterCondition();
+    // we track these separately to the SpaceStore as we need to observe transitions
     private activeSpace: Room = SpaceStore.instance.activeSpace;
+    private allRoomsInHome: boolean = SpaceStore.instance.allRoomsInHome;
 
     constructor(private store: RoomListStoreClass) {
-        if (!SpaceStore.spacesTweakAllRoomsEnabled) {
+        if (!this.allRoomsInHome || this.activeSpace) {
-            this.filter = new SpaceFilterCondition();
             this.updateFilter();
             store.addFilter(this.filter);
         }
         SpaceStore.instance.on(UPDATE_SELECTED_SPACE, this.onSelectedSpaceUpdated);
+        SpaceStore.instance.on(UPDATE_HOME_BEHAVIOUR, this.onHomeBehaviourUpdated);
     }
 
-    private onSelectedSpaceUpdated = (activeSpace?: Room) => {
+    private onSelectedSpaceUpdated = (activeSpace?: Room, allRoomsInHome = this.allRoomsInHome) => {
-        this.activeSpace = activeSpace;
+        if (activeSpace === this.activeSpace && allRoomsInHome === this.allRoomsInHome) return; // nop
 
-        if (this.filter) {
+        const oldActiveSpace = this.activeSpace;
-            if (activeSpace || !SpaceStore.spacesTweakAllRoomsEnabled) {
+        const oldAllRoomsInHome = this.allRoomsInHome;
-                this.updateFilter();
+        this.activeSpace = activeSpace;
-            } else {
+        this.allRoomsInHome = allRoomsInHome;
-                this.store.removeFilter(this.filter);
+
-                this.filter = null;
+        if (activeSpace || !allRoomsInHome) {
-            }
-        } else if (activeSpace) {
-            this.filter = new SpaceFilterCondition();
             this.updateFilter();
-            this.store.addFilter(this.filter);
         }
+
+        if (oldAllRoomsInHome && !oldActiveSpace) {
+            this.store.addFilter(this.filter);
+        } else if (allRoomsInHome && !activeSpace) {
+            this.store.removeFilter(this.filter);
+        }
+    };
+
+    private onHomeBehaviourUpdated = (allRoomsInHome: boolean) => {
+        this.onSelectedSpaceUpdated(this.activeSpace, allRoomsInHome);
     };
 
     private updateFilter = () => {

test/stores/SpaceStore-test.ts

@@ -16,7 +16,6 @@ limitations under the License.
 
 import { EventEmitter } from "events";
 import { EventType } from "matrix-js-sdk/src/@types/event";
-import { MatrixEvent } from "matrix-js-sdk/src/models/event";
 import { RoomMember } from "matrix-js-sdk/src/models/room-member";
 
 import "./SpaceStore-setup"; // enable space lab
@@ -26,31 +25,14 @@ import SpaceStore, {
     UPDATE_SELECTED_SPACE,
     UPDATE_TOP_LEVEL_SPACES,
 } from "../../src/stores/SpaceStore";
-import { resetAsyncStoreWithClient, setupAsyncStoreWithClient } from "../utils/test-utils";
+import * as testUtils from "../utils/test-utils";
-import { mkEvent, mkStubRoom, stubClient } from "../test-utils";
+import { mkEvent, stubClient } from "../test-utils";
-import { EnhancedMap } from "../../src/utils/maps";
 import DMRoomMap from "../../src/utils/DMRoomMap";
 import { MatrixClientPeg } from "../../src/MatrixClientPeg";
 import defaultDispatcher from "../../src/dispatcher/dispatcher";
 
 jest.useFakeTimers();
 
-const mockStateEventImplementation = (events: MatrixEvent[]) => {
-    const stateMap = new EnhancedMap<string, Map<string, MatrixEvent>>();
-    events.forEach(event => {
-        stateMap.getOrCreate(event.getType(), new Map()).set(event.getStateKey(), event);
-    });
-
-    return (eventType: string, stateKey?: string) => {
-        if (stateKey || stateKey === "") {
-            return stateMap.get(eventType)?.get(stateKey) || null;
-        }
-        return Array.from(stateMap.get(eventType)?.values() || []);
-    };
-};
-
-const emitPromise = (e: EventEmitter, k: string | symbol) => new Promise(r => e.once(k, r));
-
 const testUserId = "@test:user";
 
 const getUserIdForRoomId = jest.fn();
@@ -87,36 +69,13 @@ describe("SpaceStore", () => {
     const client = MatrixClientPeg.get();
 
     let rooms = [];
-
+    const mkRoom = (roomId: string) => testUtils.mkRoom(client, roomId, rooms);
-    const mkRoom = (roomId: string) => {
+    const mkSpace = (spaceId: string, children: string[] = []) => testUtils.mkSpace(client, spaceId, rooms, children);
-        const room = mkStubRoom(roomId, roomId, client);
-        room.currentState.getStateEvents.mockImplementation(mockStateEventImplementation([]));
-        rooms.push(room);
-        return room;
-    };
-
-    const mkSpace = (spaceId: string, children: string[] = []) => {
-        const space = mkRoom(spaceId);
-        space.isSpaceRoom.mockReturnValue(true);
-        space.currentState.getStateEvents.mockImplementation(mockStateEventImplementation(children.map(roomId =>
-            mkEvent({
-                event: true,
-                type: EventType.SpaceChild,
-                room: spaceId,
-                user: testUserId,
-                skey: roomId,
-                content: { via: [] },
-                ts: Date.now(),
-            }),
-        )));
-        return space;
-    };
-
     const viewRoom = roomId => defaultDispatcher.dispatch({ action: "view_room", room_id: roomId }, true);
 
     const run = async () => {
         client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId));
-        await setupAsyncStoreWithClient(store, client);
+        await testUtils.setupAsyncStoreWithClient(store, client);
         jest.runAllTimers();
     };
 
@@ -125,7 +84,7 @@ describe("SpaceStore", () => {
         client.getVisibleRooms.mockReturnValue(rooms = []);
     });
     afterEach(async () => {
-        await resetAsyncStoreWithClient(store);
+        await testUtils.resetAsyncStoreWithClient(store);
     });
 
     describe("static hierarchy resolution tests", () => {
@@ -488,7 +447,7 @@ describe("SpaceStore", () => {
             await run();
             expect(store.spacePanelSpaces).toStrictEqual([]);
             const space = mkSpace(space1);
-            const prom = emitPromise(store, UPDATE_TOP_LEVEL_SPACES);
+            const prom = testUtils.emitPromise(store, UPDATE_TOP_LEVEL_SPACES);
             emitter.emit("Room", space);
             await prom;
             expect(store.spacePanelSpaces).toStrictEqual([space]);
@@ -501,7 +460,7 @@ describe("SpaceStore", () => {
 
             expect(store.spacePanelSpaces).toStrictEqual([space]);
             space.getMyMembership.mockReturnValue("leave");
-            const prom = emitPromise(store, UPDATE_TOP_LEVEL_SPACES);
+            const prom = testUtils.emitPromise(store, UPDATE_TOP_LEVEL_SPACES);
             emitter.emit("Room.myMembership", space, "leave", "join");
             await prom;
             expect(store.spacePanelSpaces).toStrictEqual([]);
@@ -513,7 +472,7 @@ describe("SpaceStore", () => {
             expect(store.invitedSpaces).toStrictEqual([]);
             const space = mkSpace(space1);
             space.getMyMembership.mockReturnValue("invite");
-            const prom = emitPromise(store, UPDATE_INVITED_SPACES);
+            const prom = testUtils.emitPromise(store, UPDATE_INVITED_SPACES);
             emitter.emit("Room", space);
             await prom;
             expect(store.spacePanelSpaces).toStrictEqual([]);
@@ -528,7 +487,7 @@ describe("SpaceStore", () => {
             expect(store.spacePanelSpaces).toStrictEqual([]);
             expect(store.invitedSpaces).toStrictEqual([space]);
             space.getMyMembership.mockReturnValue("join");
-            const prom = emitPromise(store, UPDATE_TOP_LEVEL_SPACES);
+            const prom = testUtils.emitPromise(store, UPDATE_TOP_LEVEL_SPACES);
             emitter.emit("Room.myMembership", space, "join", "invite");
             await prom;
             expect(store.spacePanelSpaces).toStrictEqual([space]);
@@ -543,7 +502,7 @@ describe("SpaceStore", () => {
             expect(store.spacePanelSpaces).toStrictEqual([]);
             expect(store.invitedSpaces).toStrictEqual([space]);
             space.getMyMembership.mockReturnValue("leave");
-            const prom = emitPromise(store, UPDATE_INVITED_SPACES);
+            const prom = testUtils.emitPromise(store, UPDATE_INVITED_SPACES);
             emitter.emit("Room.myMembership", space, "leave", "invite");
             await prom;
             expect(store.spacePanelSpaces).toStrictEqual([]);
@@ -563,7 +522,7 @@ describe("SpaceStore", () => {
 
             const invite = mkRoom(invite1);
             invite.getMyMembership.mockReturnValue("invite");
-            const prom = emitPromise(store, space1);
+            const prom = testUtils.emitPromise(store, space1);
             emitter.emit("Room", space);
             await prom;
 
@@ -704,7 +663,8 @@ describe("SpaceStore", () => {
             mkSpace(space1, [room1, room2, room3]);
             mkSpace(space2, [room1, room2]);
 
-            client.getRoom(room2).currentState.getStateEvents.mockImplementation(mockStateEventImplementation([
+            const cliRoom2 = client.getRoom(room2);
+            cliRoom2.currentState.getStateEvents.mockImplementation(testUtils.mockStateEventImplementation([
                 mkEvent({
                     event: true,
                     type: EventType.SpaceParent,

test/stores/room-list/SpaceWatcher-test.ts

@@ -0,0 +1,186 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import "../SpaceStore-setup"; // enable space lab
+import "../../skinned-sdk"; // Must be first for skinning to work
+import { SpaceWatcher } from "../../../src/stores/room-list/SpaceWatcher";
+import type { RoomListStoreClass } from "../../../src/stores/room-list/RoomListStore";
+import SettingsStore from "../../../src/settings/SettingsStore";
+import SpaceStore, { UPDATE_HOME_BEHAVIOUR } from "../../../src/stores/SpaceStore";
+import { stubClient } from "../../test-utils";
+import { SettingLevel } from "../../../src/settings/SettingLevel";
+import { setupAsyncStoreWithClient } from "../../utils/test-utils";
+import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
+import * as testUtils from "../../utils/test-utils";
+import { SpaceFilterCondition } from "../../../src/stores/room-list/filters/SpaceFilterCondition";
+
+let filter: SpaceFilterCondition = null;
+
+const mockRoomListStore = {
+    addFilter: f => filter = f,
+    removeFilter: () => filter = null,
+} as unknown as RoomListStoreClass;
+
+const space1Id = "!space1:server";
+const space2Id = "!space2:server";
+
+describe("SpaceWatcher", () => {
+    stubClient();
+    const store = SpaceStore.instance;
+    const client = MatrixClientPeg.get();
+
+    let rooms = [];
+    const mkSpace = (spaceId: string, children: string[] = []) => testUtils.mkSpace(client, spaceId, rooms, children);
+
+    const setShowAllRooms = async (value: boolean) => {
+        if (store.allRoomsInHome === value) return;
+        await SettingsStore.setValue("feature_spaces.all_rooms", null, SettingLevel.DEVICE, value);
+        await testUtils.emitPromise(store, UPDATE_HOME_BEHAVIOUR);
+    };
+
+    let space1;
+    let space2;
+
+    beforeEach(async () => {
+        filter = null;
+        store.removeAllListeners();
+        await store.setActiveSpace(null);
+        client.getVisibleRooms.mockReturnValue(rooms = []);
+
+        space1 = mkSpace(space1Id);
+        space2 = mkSpace(space2Id);
+
+        client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId));
+        await setupAsyncStoreWithClient(store, client);
+    });
+
+    it("initialises sanely with home behaviour", async () => {
+        await setShowAllRooms(false);
+        new SpaceWatcher(mockRoomListStore);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+    });
+
+    it("initialises sanely with all behaviour", async () => {
+        await setShowAllRooms(true);
+        new SpaceWatcher(mockRoomListStore);
+
+        expect(filter).toBeNull();
+    });
+
+    it("sets space=null filter for all -> home transition", async () => {
+        await setShowAllRooms(true);
+        new SpaceWatcher(mockRoomListStore);
+
+        await setShowAllRooms(false);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBeNull();
+    });
+
+    it("sets filter correctly for all -> space transition", async () => {
+        await setShowAllRooms(true);
+        new SpaceWatcher(mockRoomListStore);
+
+        await SpaceStore.instance.setActiveSpace(space1);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+    });
+
+    it("removes filter for home -> all transition", async () => {
+        await setShowAllRooms(false);
+        new SpaceWatcher(mockRoomListStore);
+
+        await setShowAllRooms(true);
+
+        expect(filter).toBeNull();
+    });
+
+    it("sets filter correctly for home -> space transition", async () => {
+        await setShowAllRooms(false);
+        new SpaceWatcher(mockRoomListStore);
+
+        await SpaceStore.instance.setActiveSpace(space1);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+    });
+
+    it("removes filter for space -> all transition", async () => {
+        await setShowAllRooms(true);
+        new SpaceWatcher(mockRoomListStore);
+
+        await SpaceStore.instance.setActiveSpace(space1);
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+        await SpaceStore.instance.setActiveSpace(null);
+
+        expect(filter).toBeNull();
+    });
+
+    it("updates filter correctly for space -> home transition", async () => {
+        await setShowAllRooms(false);
+        await SpaceStore.instance.setActiveSpace(space1);
+
+        new SpaceWatcher(mockRoomListStore);
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+        await SpaceStore.instance.setActiveSpace(null);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(null);
+    });
+
+    it("updates filter correctly for space -> space transition", async () => {
+        await setShowAllRooms(false);
+        await SpaceStore.instance.setActiveSpace(space1);
+
+        new SpaceWatcher(mockRoomListStore);
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+        await SpaceStore.instance.setActiveSpace(space2);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space2);
+    });
+
+    it("doesn't change filter when changing showAllRooms mode to true", async () => {
+        await setShowAllRooms(false);
+        await SpaceStore.instance.setActiveSpace(space1);
+
+        new SpaceWatcher(mockRoomListStore);
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+        await setShowAllRooms(true);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+    });
+
+    it("doesn't change filter when changing showAllRooms mode to false", async () => {
+        await setShowAllRooms(true);
+        await SpaceStore.instance.setActiveSpace(space1);
+
+        new SpaceWatcher(mockRoomListStore);
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+        await setShowAllRooms(false);
+
+        expect(filter).toBeInstanceOf(SpaceFilterCondition);
+        expect(filter["space"]).toBe(space1);
+    });
+});

test/utils/test-utils.ts

@@ -15,7 +15,13 @@ limitations under the License.
 */
 
 import { MatrixClient } from "matrix-js-sdk/src/client";
+import { MatrixEvent } from "matrix-js-sdk/src/models/event";
+import { EventType } from "matrix-js-sdk/src/@types/event";
+
 import { AsyncStoreWithClient } from "../../src/stores/AsyncStoreWithClient";
+import { mkEvent, mkStubRoom } from "../test-utils";
+import { EnhancedMap } from "../../src/utils/maps";
+import { EventEmitter } from "events";
 
 // These methods make some use of some private methods on the AsyncStoreWithClient to simplify getting into a consistent
 // ready state without needing to wire up a dispatcher and pretend to be a js-sdk client.
@@ -31,3 +37,48 @@ export const resetAsyncStoreWithClient = async (store: AsyncStoreWithClient<any>
     // @ts-ignore
     await store.onNotReady();
 };
+
+export const mockStateEventImplementation = (events: MatrixEvent[]) => {
+    const stateMap = new EnhancedMap<string, Map<string, MatrixEvent>>();
+    events.forEach(event => {
+        stateMap.getOrCreate(event.getType(), new Map()).set(event.getStateKey(), event);
+    });
+
+    return (eventType: string, stateKey?: string) => {
+        if (stateKey || stateKey === "") {
+            return stateMap.get(eventType)?.get(stateKey) || null;
+        }
+        return Array.from(stateMap.get(eventType)?.values() || []);
+    };
+};
+
+export const mkRoom = (client: MatrixClient, roomId: string, rooms?: ReturnType<typeof mkStubRoom>[]) => {
+    const room = mkStubRoom(roomId, roomId, client);
+    room.currentState.getStateEvents.mockImplementation(mockStateEventImplementation([]));
+    rooms?.push(room);
+    return room;
+};
+
+export const mkSpace = (
+    client: MatrixClient,
+    spaceId: string,
+    rooms?: ReturnType<typeof mkStubRoom>[],
+    children: string[] = [],
+) => {
+    const space = mkRoom(client, spaceId, rooms);
+    space.isSpaceRoom.mockReturnValue(true);
+    space.currentState.getStateEvents.mockImplementation(mockStateEventImplementation(children.map(roomId =>
+        mkEvent({
+            event: true,
+            type: EventType.SpaceChild,
+            room: spaceId,
+            user: "@user:server",
+            skey: roomId,
+            content: { via: [] },
+            ts: Date.now(),
+        }),
+    )));
+    return space;
+};
+
+export const emitPromise = (e: EventEmitter, k: string | symbol) => new Promise(r => e.once(k, r));