From add23e4d5d0a01f6f4e20fa31f5b87b0e457da10 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 3 Feb 2023 14:09:31 +0000 Subject: [PATCH] Pass the dynamic predecessor feature flag when listing rooms (#10068) --- src/stores/room-list/RoomListStore.ts | 21 +++- test/stores/room-list/RoomListStore-test.ts | 111 +++++++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index 09d9cca2e4..0fd511e8a8 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -56,6 +56,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements public static TEST_MODE = false; private initialListsGenerated = false; + private msc3946ProcessDynamicPredecessor: boolean; + private msc3946SettingWatcherRef: string; private algorithm = new Algorithm(); private prefilterConditions: IFilterCondition[] = []; private updateFn = new MarkedExecution(() => { @@ -69,6 +71,20 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements super(dis); this.setMaxListeners(20); // RoomList + LeftPanel + 8xRoomSubList + spares this.algorithm.start(); + + this.msc3946ProcessDynamicPredecessor = SettingsStore.getValue("feature_dynamic_room_predecessors"); + this.msc3946SettingWatcherRef = SettingsStore.watchSetting( + "feature_dynamic_room_predecessors", + null, + (_settingName, _roomId, _level, _newValAtLevel, newVal) => { + this.msc3946ProcessDynamicPredecessor = newVal; + this.regenerateAllLists({ trigger: true }); + }, + ); + } + + public componentWillUnmount(): void { + SettingsStore.unwatchSetting(this.msc3946SettingWatcherRef); } private setupWatchers(): void { @@ -286,7 +302,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements // 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 = membershipPayload.room.currentState; - const predecessor = roomState.findPredecessor(SettingsStore.getValue("feature_dynamic_room_predecessors")); + const predecessor = roomState.findPredecessor(this.msc3946ProcessDynamicPredecessor); if (predecessor) { const prevRoom = this.matrixClient.getRoom(predecessor.roomId); if (prevRoom) { @@ -496,7 +512,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements private getPlausibleRooms(): Room[] { if (!this.matrixClient) return []; - let rooms = this.matrixClient.getVisibleRooms().filter((r) => VisibilityProvider.instance.isRoomVisible(r)); + let rooms = this.matrixClient.getVisibleRooms(this.msc3946ProcessDynamicPredecessor); + rooms = rooms.filter((r) => VisibilityProvider.instance.isRoomVisible(r)); if (this.prefilterConditions.length > 0) { rooms = rooms.filter((r) => { diff --git a/test/stores/room-list/RoomListStore-test.ts b/test/stores/room-list/RoomListStore-test.ts index 8ce3dff22a..e0ee766f91 100644 --- a/test/stores/room-list/RoomListStore-test.ts +++ b/test/stores/room-list/RoomListStore-test.ts @@ -14,13 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventType, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; +import { EventType, MatrixEvent, PendingEventOrdering, Room } from "matrix-js-sdk/src/matrix"; import { MatrixDispatcher } from "../../../src/dispatcher/dispatcher"; -import SettingsStore from "../../../src/settings/SettingsStore"; +import { SettingLevel } from "../../../src/settings/SettingLevel"; +import SettingsStore, { CallbackFn } from "../../../src/settings/SettingsStore"; import { ListAlgorithm, SortAlgorithm } from "../../../src/stores/room-list/algorithms/models"; import { OrderedDefaultTagIDs, RoomUpdateCause } from "../../../src/stores/room-list/models"; import RoomListStore, { RoomListStoreClass } from "../../../src/stores/room-list/RoomListStore"; +import DMRoomMap from "../../../src/utils/DMRoomMap"; import { stubClient, upsertRoomStateEvents } from "../../test-utils"; describe("RoomListStore", () => { @@ -62,7 +64,9 @@ describe("RoomListStore", () => { upsertRoomStateEvents(roomWithPredecessorEvent, [predecessor]); const roomWithCreatePredecessor = new Room(newRoomId, client, userId, {}); upsertRoomStateEvents(roomWithCreatePredecessor, [createWithPredecessor]); - const roomNoPredecessor = new Room(roomNoPredecessorId, client, userId, {}); + const roomNoPredecessor = new Room(roomNoPredecessorId, client, userId, { + pendingEventOrdering: PendingEventOrdering.Detached, + }); upsertRoomStateEvents(roomNoPredecessor, [createNoPredecessor]); const oldRoom = new Room(oldRoomId, client, userId, {}); client.getRoom = jest.fn().mockImplementation((roomId) => { @@ -138,6 +142,93 @@ describe("RoomListStore", () => { expect(handleRoomUpdate).toHaveBeenCalledTimes(1); }); + it("Lists all rooms that the client says are visible", () => { + // Given 3 rooms that are visible according to the client + const room1 = new Room("!r1:e.com", client, userId, { pendingEventOrdering: PendingEventOrdering.Detached }); + const room2 = new Room("!r2:e.com", client, userId, { pendingEventOrdering: PendingEventOrdering.Detached }); + const room3 = new Room("!r3:e.com", client, userId, { pendingEventOrdering: PendingEventOrdering.Detached }); + room1.updateMyMembership("join"); + room2.updateMyMembership("join"); + room3.updateMyMembership("join"); + DMRoomMap.makeShared(); + const { store } = createStore(); + client.getVisibleRooms = jest.fn().mockReturnValue([room1, room2, room3]); + + // When we make the list of rooms + store.regenerateAllLists({ trigger: false }); + + // Then the list contains all 3 + expect(store.orderedLists).toMatchObject({ + "im.vector.fake.recent": [room1, room2, room3], + }); + + // We asked not to use MSC3946 when we asked the client for the visible rooms + expect(client.getVisibleRooms).toHaveBeenCalledWith(false); + expect(client.getVisibleRooms).toHaveBeenCalledTimes(1); + }); + + it("Watches the feature flag setting", () => { + jest.spyOn(SettingsStore, "watchSetting").mockReturnValue("dyn_pred_ref"); + jest.spyOn(SettingsStore, "unwatchSetting"); + + // When we create a store + const { store } = createStore(); + + // Then we watch the feature flag + expect(SettingsStore.watchSetting).toHaveBeenCalledWith( + "feature_dynamic_room_predecessors", + null, + expect.any(Function), + ); + + // And when we unmount it + store.componentWillUnmount(); + + // Then we unwatch it. + expect(SettingsStore.unwatchSetting).toHaveBeenCalledWith("dyn_pred_ref"); + }); + + it("Regenerates all lists when the feature flag is set", () => { + // Given a store allowing us to spy on any use of SettingsStore + let featureFlagValue = false; + jest.spyOn(SettingsStore, "getValue").mockImplementation(() => featureFlagValue); + + let watchCallback: CallbackFn | undefined; + jest.spyOn(SettingsStore, "watchSetting").mockImplementation( + (_settingName: string, _roomId: string | null, callbackFn: CallbackFn) => { + watchCallback = callbackFn; + return "dyn_pred_ref"; + }, + ); + jest.spyOn(SettingsStore, "unwatchSetting"); + + const { store } = createStore(); + client.getVisibleRooms = jest.fn().mockReturnValue([]); + // Sanity: no calculation has happened yet + expect(client.getVisibleRooms).toHaveBeenCalledTimes(0); + + // When we calculate for the first time + store.regenerateAllLists({ trigger: false }); + + // Then we use the current feature flag value (false) + expect(client.getVisibleRooms).toHaveBeenCalledWith(false); + expect(client.getVisibleRooms).toHaveBeenCalledTimes(1); + + // But when we update the feature flag + featureFlagValue = true; + watchCallback( + "feature_dynamic_room_predecessors", + "", + SettingLevel.DEFAULT, + featureFlagValue, + featureFlagValue, + ); + + // Then we recalculate and passed the updated value (true) + expect(client.getVisibleRooms).toHaveBeenCalledWith(true); + expect(client.getVisibleRooms).toHaveBeenCalledTimes(2); + }); + describe("When feature_dynamic_room_predecessors = true", () => { beforeEach(() => { jest.spyOn(SettingsStore, "getValue").mockImplementation( @@ -168,5 +259,19 @@ describe("RoomListStore", () => { // And the new room is added expect(handleRoomUpdate).toHaveBeenCalledWith(roomWithPredecessorEvent, RoomUpdateCause.NewRoom); }); + + it("Passes the feature flag on to the client when asking for visible rooms", () => { + // Given a store that we can ask for a room list + DMRoomMap.makeShared(); + const { store } = createStore(); + client.getVisibleRooms = jest.fn().mockReturnValue([]); + + // When we make the list of rooms + store.regenerateAllLists({ trigger: false }); + + // We asked to use MSC3946 when we asked the client for the visible rooms + expect(client.getVisibleRooms).toHaveBeenCalledWith(true); + expect(client.getVisibleRooms).toHaveBeenCalledTimes(1); + }); }); });