From 06c4ba32cd3306c77c085672d5f13174bd573038 Mon Sep 17 00:00:00 2001 From: kegsay Date: Tue, 20 Sep 2022 16:32:39 +0100 Subject: [PATCH] Store refactor: make it easier to test stores (#9290) * refactor: convert RoomViewStore from flux Store to standard EventEmitter Parts of a series of experimental changes to improve the design of stores. * Use a gen5 store for RoomViewStore for now due to lock handling * Revert "Use a gen5 store for RoomViewStore for now due to lock handling" This reverts commit 1076af071d997d87b8ae0b0dcddfd1ae428665af. * Add untilEmission and tweak untilDispatch; use it in RoomViewStore * Add more RVS tests; remove custom room ID listener code and use EventEmitter * Better comments * Null guard `dis` as tests mock out `defaultDispatcher` * Additional tests --- src/components/structures/RoomView.tsx | 9 +- .../views/right_panel/TimelineCard.tsx | 9 +- src/components/views/rooms/RoomList.tsx | 7 +- src/components/views/voip/PipView.tsx | 7 +- src/stores/RoomViewStore.tsx | 85 +++++---- src/stores/room-list/RoomListStore.ts | 3 +- src/stores/room-list/SlidingRoomListStore.ts | 3 +- test/components/structures/RoomView-test.tsx | 8 +- test/stores/RoomViewStore-test.tsx | 174 +++++++++++++----- test/test-utils/test-utils.ts | 13 -- test/test-utils/utilities.ts | 100 +++++++++- 11 files changed, 289 insertions(+), 129 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index b94c8d4ea4..fed78d7617 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -24,7 +24,6 @@ import React, { createRef, ReactElement, ReactNode, RefObject, useContext } from import classNames from 'classnames'; import { IRecommendedVersion, NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room"; import { IThreadBundledRelationship, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event"; -import { EventSubscription } from "fbemitter"; import { ISearchResults } from 'matrix-js-sdk/src/@types/search'; import { logger } from "matrix-js-sdk/src/logger"; import { EventTimeline } from 'matrix-js-sdk/src/models/event-timeline'; @@ -366,7 +365,6 @@ function LocalRoomCreateLoader(props: ILocalRoomCreateLoaderProps): ReactElement export class RoomView extends React.Component { private readonly dispatcherRef: string; - private readonly roomStoreToken: EventSubscription; private settingWatchers: string[]; private unmounted = false; @@ -439,7 +437,7 @@ export class RoomView extends React.Component { context.on(CryptoEvent.KeysChanged, this.onCrossSigningKeysChanged); context.on(MatrixEventEvent.Decrypted, this.onEventDecrypted); // Start listening for RoomViewStore updates - this.roomStoreToken = RoomViewStore.instance.addListener(this.onRoomViewStoreUpdate); + RoomViewStore.instance.on(UPDATE_EVENT, this.onRoomViewStoreUpdate); RightPanelStore.instance.on(UPDATE_EVENT, this.onRightPanelStoreUpdate); @@ -883,10 +881,7 @@ export class RoomView extends React.Component { window.removeEventListener('beforeunload', this.onPageUnload); - // Remove RoomStore listener - if (this.roomStoreToken) { - this.roomStoreToken.remove(); - } + RoomViewStore.instance.off(UPDATE_EVENT, this.onRoomViewStoreUpdate); RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate); WidgetEchoStore.removeListener(UPDATE_EVENT, this.onWidgetEchoStoreUpdate); diff --git a/src/components/views/right_panel/TimelineCard.tsx b/src/components/views/right_panel/TimelineCard.tsx index 941a74163c..f1eea5ad49 100644 --- a/src/components/views/right_panel/TimelineCard.tsx +++ b/src/components/views/right_panel/TimelineCard.tsx @@ -15,7 +15,6 @@ limitations under the License. */ import React from 'react'; -import { EventSubscription } from "fbemitter"; import { IEventRelation, MatrixEvent } from 'matrix-js-sdk/src/models/event'; import { EventTimelineSet } from 'matrix-js-sdk/src/models/event-timeline-set'; import { NotificationCountType, Room } from 'matrix-js-sdk/src/models/room'; @@ -42,6 +41,7 @@ import JumpToBottomButton from '../rooms/JumpToBottomButton'; import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload"; import Measured from '../elements/Measured'; import Heading from '../typography/Heading'; +import { UPDATE_EVENT } from '../../../stores/AsyncStore'; interface IProps { room: Room; @@ -77,7 +77,6 @@ export default class TimelineCard extends React.Component { private layoutWatcherRef: string; private timelinePanel = React.createRef(); private card = React.createRef(); - private roomStoreToken: EventSubscription; private readReceiptsSettingWatcher: string; constructor(props: IProps) { @@ -92,7 +91,7 @@ export default class TimelineCard extends React.Component { } public componentDidMount(): void { - this.roomStoreToken = RoomViewStore.instance.addListener(this.onRoomViewStoreUpdate); + RoomViewStore.instance.addListener(UPDATE_EVENT, this.onRoomViewStoreUpdate); this.dispatcherRef = dis.register(this.onAction); this.readReceiptsSettingWatcher = SettingsStore.watchSetting("showReadReceipts", null, (...[,,, value]) => this.setState({ showReadReceipts: value as boolean }), @@ -103,9 +102,7 @@ export default class TimelineCard extends React.Component { } public componentWillUnmount(): void { - // Remove RoomStore listener - - this.roomStoreToken?.remove(); + RoomViewStore.instance.removeListener(UPDATE_EVENT, this.onRoomViewStoreUpdate); if (this.readReceiptsSettingWatcher) { SettingsStore.unwatchSetting(this.readReceiptsSettingWatcher); diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 0d6756a7e1..13b1011088 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import * as fbEmitter from "fbemitter"; import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import { Room } from "matrix-js-sdk/src/models/room"; import React, { ComponentType, createRef, ReactComponentElement, RefObject } from "react"; @@ -37,6 +36,7 @@ import { UIComponent } from "../../../settings/UIFeature"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { ITagMap } from "../../../stores/room-list/algorithms/models"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; +import { UPDATE_EVENT } from "../../../stores/AsyncStore"; import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore"; import { RoomViewStore } from "../../../stores/RoomViewStore"; import { @@ -403,7 +403,6 @@ const TAG_AESTHETICS: ITagAestheticsMap = { export default class RoomList extends React.PureComponent { private dispatcherRef; - private roomStoreToken: fbEmitter.EventSubscription; private treeRef = createRef(); private favouriteMessageWatcher: string; @@ -422,7 +421,7 @@ export default class RoomList extends React.PureComponent { public componentDidMount(): void { this.dispatcherRef = defaultDispatcher.register(this.onAction); - this.roomStoreToken = RoomViewStore.instance.addListener(this.onRoomViewStoreUpdate); + RoomViewStore.instance.on(UPDATE_EVENT, this.onRoomViewStoreUpdate); SpaceStore.instance.on(UPDATE_SUGGESTED_ROOMS, this.updateSuggestedRooms); RoomListStore.instance.on(LISTS_UPDATE_EVENT, this.updateLists); this.favouriteMessageWatcher = @@ -437,7 +436,7 @@ export default class RoomList extends React.PureComponent { RoomListStore.instance.off(LISTS_UPDATE_EVENT, this.updateLists); SettingsStore.unwatchSetting(this.favouriteMessageWatcher); defaultDispatcher.unregister(this.dispatcherRef); - if (this.roomStoreToken) this.roomStoreToken.remove(); + RoomViewStore.instance.off(UPDATE_EVENT, this.onRoomViewStoreUpdate); } private onRoomViewStoreUpdate = () => { diff --git a/src/components/views/voip/PipView.tsx b/src/components/views/voip/PipView.tsx index be411bb155..691f422e5b 100644 --- a/src/components/views/voip/PipView.tsx +++ b/src/components/views/voip/PipView.tsx @@ -16,7 +16,6 @@ limitations under the License. import React, { createRef } from 'react'; import { CallEvent, CallState, MatrixCall } from 'matrix-js-sdk/src/webrtc/call'; -import { EventSubscription } from 'fbemitter'; import { logger } from "matrix-js-sdk/src/logger"; import classNames from 'classnames'; import { Room } from "matrix-js-sdk/src/models/room"; @@ -35,6 +34,7 @@ import LegacyCallViewHeader from './LegacyCallView/LegacyCallViewHeader'; import ActiveWidgetStore, { ActiveWidgetStoreEvent } from '../../../stores/ActiveWidgetStore'; import WidgetStore, { IApp } from "../../../stores/WidgetStore"; import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload"; +import { UPDATE_EVENT } from '../../../stores/AsyncStore'; const SHOW_CALL_IN_STATES = [ CallState.Connected, @@ -116,7 +116,6 @@ function getPrimarySecondaryCallsForPip(roomId: string): [MatrixCall, MatrixCall */ export default class PipView extends React.Component { - private roomStoreToken: EventSubscription; private settingsWatcherRef: string; private movePersistedElement = createRef<() => void>(); @@ -141,7 +140,7 @@ export default class PipView extends React.Component { public componentDidMount() { LegacyCallHandler.instance.addListener(LegacyCallHandlerEvent.CallChangeRoom, this.updateCalls); LegacyCallHandler.instance.addListener(LegacyCallHandlerEvent.CallState, this.updateCalls); - this.roomStoreToken = RoomViewStore.instance.addListener(this.onRoomViewStoreUpdate); + RoomViewStore.instance.addListener(UPDATE_EVENT, this.onRoomViewStoreUpdate); MatrixClientPeg.get().on(CallEvent.RemoteHoldUnhold, this.onCallRemoteHold); const room = MatrixClientPeg.get()?.getRoom(this.state.viewedRoomId); if (room) { @@ -157,7 +156,7 @@ export default class PipView extends React.Component { LegacyCallHandler.instance.removeListener(LegacyCallHandlerEvent.CallChangeRoom, this.updateCalls); LegacyCallHandler.instance.removeListener(LegacyCallHandlerEvent.CallState, this.updateCalls); MatrixClientPeg.get().removeListener(CallEvent.RemoteHoldUnhold, this.onCallRemoteHold); - this.roomStoreToken?.remove(); + RoomViewStore.instance.removeListener(UPDATE_EVENT, this.onRoomViewStoreUpdate); SettingsStore.unwatchSetting(this.settingsWatcherRef); const room = MatrixClientPeg.get().getRoom(this.state.viewedRoomId); if (room) { diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index 593dd7115f..3c127275a2 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -17,7 +17,6 @@ limitations under the License. */ import React, { ReactNode } from "react"; -import { Store } from 'flux/utils'; import { MatrixError } from "matrix-js-sdk/src/http-api"; import { logger } from "matrix-js-sdk/src/logger"; import { ViewRoom as ViewRoomEvent } from "@matrix-org/analytics-events/types/typescript/ViewRoom"; @@ -26,13 +25,13 @@ import { JoinRule } from "matrix-js-sdk/src/@types/partials"; import { Room } from "matrix-js-sdk/src/models/room"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { Optional } from "matrix-events-sdk"; +import EventEmitter from "events"; -import dis from '../dispatcher/dispatcher'; +import { defaultDispatcher, MatrixDispatcher } from '../dispatcher/dispatcher'; import { MatrixClientPeg } from '../MatrixClientPeg'; import Modal from '../Modal'; import { _t } from '../languageHandler'; import { getCachedRoomIDForAlias, storeRoomAliasInCache } from '../RoomAliasCache'; -import { ActionPayload } from "../dispatcher/payloads"; import { Action } from "../dispatcher/actions"; import { retry } from "../utils/promise"; import { TimelineRenderingType } from "../contexts/RoomContext"; @@ -50,6 +49,7 @@ import { ActiveRoomChangedPayload } from "../dispatcher/payloads/ActiveRoomChang import SettingsStore from "../settings/SettingsStore"; import { SlidingSyncManager } from "../SlidingSyncManager"; import { awaitRoomDownSync } from "../utils/RoomUpgrade"; +import { UPDATE_EVENT } from "./AsyncStore"; const NUM_JOIN_RETRY = 5; @@ -90,47 +90,34 @@ const INITIAL_STATE = { type Listener = (isActive: boolean) => void; /** - * A class for storing application state for RoomView. This is the RoomView's interface -* with a subset of the js-sdk. - * ``` + * A class for storing application state for RoomView. */ -export class RoomViewStore extends Store { +export class RoomViewStore extends EventEmitter { // Important: This cannot be a dynamic getter (lazily-constructed instance) because // otherwise we'll miss view_room dispatches during startup, breaking relaunches of // the app. We need to eagerly create the instance. - public static readonly instance = new RoomViewStore(); + public static readonly instance = new RoomViewStore(defaultDispatcher); private state = INITIAL_STATE; // initialize state - // Keep these out of state to avoid causing excessive/recursive updates - private roomIdActivityListeners: Record = {}; + private dis: MatrixDispatcher; + private dispatchToken: string; - public constructor() { - super(dis); + public constructor(dis: MatrixDispatcher) { + super(); + this.resetDispatcher(dis); } public addRoomListener(roomId: string, fn: Listener): void { - if (!this.roomIdActivityListeners[roomId]) this.roomIdActivityListeners[roomId] = []; - this.roomIdActivityListeners[roomId].push(fn); + this.on(roomId, fn); } public removeRoomListener(roomId: string, fn: Listener): void { - if (this.roomIdActivityListeners[roomId]) { - const i = this.roomIdActivityListeners[roomId].indexOf(fn); - if (i > -1) { - this.roomIdActivityListeners[roomId].splice(i, 1); - } - } else { - logger.warn("Unregistering unrecognised listener (roomId=" + roomId + ")"); - } + this.off(roomId, fn); } private emitForRoom(roomId: string, isActive: boolean): void { - if (!this.roomIdActivityListeners[roomId]) return; - - for (const fn of this.roomIdActivityListeners[roomId]) { - fn.call(null, isActive); - } + this.emit(roomId, isActive); } private setState(newState: Partial): void { @@ -156,17 +143,17 @@ export class RoomViewStore extends Store { // Fired so we can reduce dependency on event emitters to this store, which is relatively // central to the application and can easily cause import cycles. - dis.dispatch({ + this.dis.dispatch({ action: Action.ActiveRoomChanged, oldRoomId: lastRoomId, newRoomId: this.state.roomId, }); } - this.__emitChange(); + this.emit(UPDATE_EVENT); } - protected __onDispatch(payload): void { // eslint-disable-line @typescript-eslint/naming-convention + private onDispatch(payload): void { // eslint-disable-line @typescript-eslint/naming-convention switch (payload.action) { // view_room: // - room_alias: '#somealias:matrix.org' @@ -243,7 +230,7 @@ export class RoomViewStore extends Store { // both room and search timeline rendering types, search will get auto-closed by RoomView at this time. if ([TimelineRenderingType.Room, TimelineRenderingType.Search].includes(payload.context)) { if (payload.event && payload.event.getRoomId() !== this.state.roomId) { - dis.dispatch({ + this.dis.dispatch({ action: Action.ViewRoom, room_id: payload.event.getRoomId(), replyingToEvent: payload.event, @@ -283,9 +270,9 @@ export class RoomViewStore extends Store { }); } if (SettingsStore.getValue("feature_sliding_sync") && this.state.roomId !== payload.room_id) { - if (this.state.roomId) { + if (this.state.subscribingRoomId && this.state.subscribingRoomId !== payload.room_id) { // unsubscribe from this room, but don't await it as we don't care when this gets done. - SlidingSyncManager.instance.setRoomVisible(this.state.roomId, false); + SlidingSyncManager.instance.setRoomVisible(this.state.subscribingRoomId, false); } this.setState({ subscribingRoomId: payload.room_id, @@ -306,11 +293,11 @@ export class RoomViewStore extends Store { // Whilst we were subscribing another room was viewed, so stop what we're doing and // unsubscribe if (this.state.subscribingRoomId !== payload.room_id) { - SlidingSyncManager.instance.setRoomVisible(this.state.roomId, false); + SlidingSyncManager.instance.setRoomVisible(payload.room_id, false); return; } // Re-fire the payload: we won't re-process it because the prev room ID == payload room ID now - dis.dispatch({ + this.dis.dispatch({ ...payload, }); return; @@ -346,7 +333,7 @@ export class RoomViewStore extends Store { this.setState(newState); if (payload.auto_join) { - dis.dispatch({ + this.dis.dispatch({ ...payload, action: Action.JoinRoom, roomId: payload.room_id, @@ -378,7 +365,7 @@ export class RoomViewStore extends Store { roomId = result.room_id; } catch (err) { logger.error("RVS failed to get room id for alias: ", err); - dis.dispatch({ + this.dis.dispatch({ action: Action.ViewRoomError, room_id: null, room_alias: payload.room_alias, @@ -389,7 +376,7 @@ export class RoomViewStore extends Store { } // Re-fire the payload with the newly found room_id - dis.dispatch({ + this.dis.dispatch({ ...payload, room_id: roomId, }); @@ -427,13 +414,13 @@ export class RoomViewStore extends Store { // We do *not* clear the 'joining' flag because the Room object and/or our 'joined' member event may not // have come down the sync stream yet, and that's the point at which we'd consider the user joined to the // room. - dis.dispatch({ + this.dis.dispatch({ action: Action.JoinRoomReady, roomId, metricsTrigger: payload.metricsTrigger, }); } catch (err) { - dis.dispatch({ + this.dis.dispatch({ action: Action.JoinRoomError, roomId, err, @@ -493,6 +480,24 @@ export class RoomViewStore extends Store { this.state = Object.assign({}, INITIAL_STATE); } + /** + * Reset which dispatcher should be used to listen for actions. The old dispatcher will be + * unregistered. + * @param dis The new dispatcher to use. + */ + public resetDispatcher(dis: MatrixDispatcher) { + if (this.dispatchToken) { + this.dis.unregister(this.dispatchToken); + } + this.dis = dis; + if (dis) { + // Some tests mock the dispatcher file resulting in an empty defaultDispatcher + // so rather than dying here, just ignore it. When we no longer mock files like this, + // we should remove the null check. + this.dispatchToken = this.dis.register(this.onDispatch.bind(this)); + } + } + // The room ID of the room currently being viewed public getRoomId(): Optional { return this.state.roomId; diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index c74a58494a..83c79a16a9 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -39,6 +39,7 @@ import { SpaceWatcher } from "./SpaceWatcher"; import { IRoomTimelineActionPayload } from "../../actions/MatrixActionCreators"; import { RoomListStore as Interface, RoomListStoreEvent } from "./Interface"; import { SlidingRoomListStoreClass } from "./SlidingRoomListStore"; +import { UPDATE_EVENT } from "../AsyncStore"; interface IState { // state is tracked in underlying classes @@ -104,7 +105,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements this.readyStore.useUnitTestClient(forcedClient); } - RoomViewStore.instance.addListener(() => this.handleRVSUpdate({})); + RoomViewStore.instance.addListener(UPDATE_EVENT, () => this.handleRVSUpdate({})); this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); this.algorithm.on(FILTER_CHANGED, this.onAlgorithmFilterUpdated); this.setupWatchers(); diff --git a/src/stores/room-list/SlidingRoomListStore.ts b/src/stores/room-list/SlidingRoomListStore.ts index b8ba48659f..3d532fe0c9 100644 --- a/src/stores/room-list/SlidingRoomListStore.ts +++ b/src/stores/room-list/SlidingRoomListStore.ts @@ -30,6 +30,7 @@ import SpaceStore from "../spaces/SpaceStore"; import { MetaSpace, SpaceKey, UPDATE_SELECTED_SPACE } from "../spaces"; import { LISTS_LOADING_EVENT } from "./RoomListStore"; import { RoomViewStore } from "../RoomViewStore"; +import { UPDATE_EVENT } from "../AsyncStore"; interface IState { // state is tracked in underlying classes @@ -313,7 +314,7 @@ export class SlidingRoomListStoreClass extends AsyncStoreWithClient impl logger.info("SlidingRoomListStore.onReady"); // permanent listeners: never get destroyed. Could be an issue if we want to test this in isolation. SlidingSyncManager.instance.slidingSync.on(SlidingSyncEvent.List, this.onSlidingSyncListUpdate.bind(this)); - RoomViewStore.instance.addListener(this.onRoomViewStoreUpdated.bind(this)); + RoomViewStore.instance.addListener(UPDATE_EVENT, this.onRoomViewStoreUpdated.bind(this)); SpaceStore.instance.on(UPDATE_SELECTED_SPACE, this.onSelectedSpaceUpdated.bind(this)); if (SpaceStore.instance.activeSpace) { this.onSelectedSpaceUpdated(SpaceStore.instance.activeSpace, false); diff --git a/test/components/structures/RoomView-test.tsx b/test/components/structures/RoomView-test.tsx index a9e9fe87ff..dd45c7df09 100644 --- a/test/components/structures/RoomView-test.tsx +++ b/test/components/structures/RoomView-test.tsx @@ -42,6 +42,7 @@ import { RightPanelPhases } from "../../../src/stores/right-panel/RightPanelStor import { LocalRoom, LocalRoomState } from "../../../src/models/LocalRoom"; import { DirectoryMember } from "../../../src/utils/direct-messages"; import { createDmLocalRoom } from "../../../src/utils/dm/createDmLocalRoom"; +import { UPDATE_EVENT } from "../../../src/stores/AsyncStore"; const RoomView = wrapInMatrixClientContext(_RoomView); @@ -74,12 +75,13 @@ describe("RoomView", () => { const mountRoomView = async (): Promise => { if (RoomViewStore.instance.getRoomId() !== room.roomId) { const switchedRoom = new Promise(resolve => { - const subscription = RoomViewStore.instance.addListener(() => { + const subFn = () => { if (RoomViewStore.instance.getRoomId()) { - subscription.remove(); + RoomViewStore.instance.off(UPDATE_EVENT, subFn); resolve(); } - }); + }; + RoomViewStore.instance.on(UPDATE_EVENT, subFn); }); defaultDispatcher.dispatch({ diff --git a/test/stores/RoomViewStore-test.tsx b/test/stores/RoomViewStore-test.tsx index 6d2bbb33f8..3ea402438d 100644 --- a/test/stores/RoomViewStore-test.tsx +++ b/test/stores/RoomViewStore-test.tsx @@ -18,13 +18,13 @@ import { Room } from 'matrix-js-sdk/src/matrix'; import { RoomViewStore } from '../../src/stores/RoomViewStore'; import { Action } from '../../src/dispatcher/actions'; -import * as testUtils from '../test-utils'; -import { flushPromises, getMockClientWithEventEmitter } from '../test-utils'; +import { getMockClientWithEventEmitter, untilDispatch, untilEmission } from '../test-utils'; import SettingsStore from '../../src/settings/SettingsStore'; import { SlidingSyncManager } from '../../src/SlidingSyncManager'; import { TimelineRenderingType } from '../../src/contexts/RoomContext'; - -const dispatch = testUtils.getDispatchForStore(RoomViewStore.instance); +import { MatrixDispatcher } from '../../src/dispatcher/dispatcher'; +import { UPDATE_EVENT } from '../../src/stores/AsyncStore'; +import { ActiveRoomChangedPayload } from '../../src/dispatcher/payloads/ActiveRoomChangedPayload'; jest.mock('../../src/utils/DMRoomMap', () => { const mock = { @@ -40,13 +40,18 @@ jest.mock('../../src/utils/DMRoomMap', () => { describe('RoomViewStore', function() { const userId = '@alice:server'; + const roomId = "!randomcharacters:aser.ver"; + // we need to change the alias to ensure cache misses as the cache exists + // through all tests. + let alias = "#somealias2:aser.ver"; const mockClient = getMockClientWithEventEmitter({ joinRoom: jest.fn(), getRoom: jest.fn(), getRoomIdForAlias: jest.fn(), isGuest: jest.fn(), }); - const room = new Room('!room:server', mockClient, userId); + const room = new Room(roomId, mockClient, userId); + let dis: MatrixDispatcher; beforeEach(function() { jest.clearAllMocks(); @@ -56,54 +61,131 @@ describe('RoomViewStore', function() { mockClient.isGuest.mockReturnValue(false); // Reset the state of the store + dis = new MatrixDispatcher(); RoomViewStore.instance.reset(); + RoomViewStore.instance.resetDispatcher(dis); }); it('can be used to view a room by ID and join', async () => { - dispatch({ action: Action.ViewRoom, room_id: '!randomcharacters:aser.ver' }); - dispatch({ action: Action.JoinRoom }); - await flushPromises(); - expect(mockClient.joinRoom).toHaveBeenCalledWith('!randomcharacters:aser.ver', { viaServers: [] }); + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + dis.dispatch({ action: Action.JoinRoom }); + await untilDispatch(Action.JoinRoomReady, dis); + expect(mockClient.joinRoom).toHaveBeenCalledWith(roomId, { viaServers: [] }); expect(RoomViewStore.instance.isJoining()).toBe(true); }); + it('can auto-join a room', async () => { + dis.dispatch({ action: Action.ViewRoom, room_id: roomId, auto_join: true }); + await untilDispatch(Action.JoinRoomReady, dis); + expect(mockClient.joinRoom).toHaveBeenCalledWith(roomId, { viaServers: [] }); + expect(RoomViewStore.instance.isJoining()).toBe(true); + }); + + it('emits ActiveRoomChanged when the viewed room changes', async () => { + const roomId2 = "!roomid:2"; + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + let payload = await untilDispatch(Action.ActiveRoomChanged, dis) as ActiveRoomChangedPayload; + expect(payload.newRoomId).toEqual(roomId); + expect(payload.oldRoomId).toEqual(null); + + dis.dispatch({ action: Action.ViewRoom, room_id: roomId2 }); + payload = await untilDispatch(Action.ActiveRoomChanged, dis) as ActiveRoomChangedPayload; + expect(payload.newRoomId).toEqual(roomId2); + expect(payload.oldRoomId).toEqual(roomId); + }); + + it('invokes room activity listeners when the viewed room changes', async () => { + const roomId2 = "!roomid:2"; + const callback = jest.fn(); + RoomViewStore.instance.addRoomListener(roomId, callback); + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ActiveRoomChanged, dis) as ActiveRoomChangedPayload; + expect(callback).toHaveBeenCalledWith(true); + expect(callback).not.toHaveBeenCalledWith(false); + + dis.dispatch({ action: Action.ViewRoom, room_id: roomId2 }); + await untilDispatch(Action.ActiveRoomChanged, dis) as ActiveRoomChangedPayload; + expect(callback).toHaveBeenCalledWith(false); + }); + it('can be used to view a room by alias and join', async () => { - const roomId = "!randomcharacters:aser.ver"; - const alias = "#somealias2:aser.ver"; - mockClient.getRoomIdForAlias.mockResolvedValue({ room_id: roomId, servers: [] }); - - dispatch({ action: Action.ViewRoom, room_alias: alias }); - await flushPromises(); - await flushPromises(); + dis.dispatch({ action: Action.ViewRoom, room_alias: alias }); + await untilDispatch((p) => { // wait for the re-dispatch with the room ID + return p.action === Action.ViewRoom && p.room_id === roomId; + }, dis); // roomId is set to id of the room alias expect(RoomViewStore.instance.getRoomId()).toBe(roomId); // join the room - dispatch({ action: Action.JoinRoom }); + dis.dispatch({ action: Action.JoinRoom }, true); + + await untilDispatch(Action.JoinRoomReady, dis); expect(RoomViewStore.instance.isJoining()).toBeTruthy(); - await flushPromises(); - expect(mockClient.joinRoom).toHaveBeenCalledWith(alias, { viaServers: [] }); }); + it('emits ViewRoomError if the alias lookup fails', async () => { + alias = "#something-different:to-ensure-cache-miss"; + mockClient.getRoomIdForAlias.mockRejectedValue(new Error("network error or something")); + dis.dispatch({ action: Action.ViewRoom, room_alias: alias }); + const payload = await untilDispatch(Action.ViewRoomError, dis); + expect(payload.room_id).toBeNull(); + expect(payload.room_alias).toEqual(alias); + expect(RoomViewStore.instance.getRoomAlias()).toEqual(alias); + }); + + it('emits JoinRoomError if joining the room fails', async () => { + const joinErr = new Error("network error or something"); + mockClient.joinRoom.mockRejectedValue(joinErr); + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + dis.dispatch({ action: Action.JoinRoom }); + await untilDispatch(Action.JoinRoomError, dis); + expect(RoomViewStore.instance.isJoining()).toBe(false); + expect(RoomViewStore.instance.getJoinError()).toEqual(joinErr); + }); + it('remembers the event being replied to when swapping rooms', async () => { - dispatch({ action: Action.ViewRoom, room_id: '!randomcharacters:aser.ver' }); - await flushPromises(); + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ActiveRoomChanged, dis); const replyToEvent = { - getRoomId: () => '!randomcharacters:aser.ver', + getRoomId: () => roomId, }; - dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room }); - await flushPromises(); + dis.dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room }); + await untilEmission(RoomViewStore.instance, UPDATE_EVENT); expect(RoomViewStore.instance.getQuotingEvent()).toEqual(replyToEvent); // view the same room, should remember the event. - dispatch({ action: Action.ViewRoom, room_id: '!randomcharacters:aser.ver' }); - await flushPromises(); + // set the highlighed flag to make sure there is a state change so we get an update event + dis.dispatch({ action: Action.ViewRoom, room_id: roomId, highlighted: true }); + await untilEmission(RoomViewStore.instance, UPDATE_EVENT); expect(RoomViewStore.instance.getQuotingEvent()).toEqual(replyToEvent); }); + it('swaps to the replied event room if it is not the current room', async () => { + const roomId2 = "!room2:bar"; + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ActiveRoomChanged, dis); + const replyToEvent = { + getRoomId: () => roomId2, + }; + dis.dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room }); + await untilDispatch(Action.ViewRoom, dis); + expect(RoomViewStore.instance.getQuotingEvent()).toEqual(replyToEvent); + expect(RoomViewStore.instance.getRoomId()).toEqual(roomId2); + }); + + it('removes the roomId on ViewHomePage', async () => { + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ActiveRoomChanged, dis); + expect(RoomViewStore.instance.getRoomId()).toEqual(roomId); + + dis.dispatch({ action: Action.ViewHomePage }); + await untilEmission(RoomViewStore.instance, UPDATE_EVENT); + expect(RoomViewStore.instance.getRoomId()).toBeNull(); + }); + describe('Sliding Sync', function() { beforeEach(() => { jest.spyOn(SettingsStore, 'getValue').mockImplementation((settingName, roomId, value) => { @@ -117,9 +199,8 @@ describe('RoomViewStore', function() { Promise.resolve(""), ); const subscribedRoomId = "!sub1:localhost"; - dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId }); - await flushPromises(); - await flushPromises(); + dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId }); + await untilDispatch(Action.ActiveRoomChanged, dis); expect(RoomViewStore.instance.getRoomId()).toBe(subscribedRoomId); expect(setRoomVisible).toHaveBeenCalledWith(subscribedRoomId, true); }); @@ -129,20 +210,27 @@ describe('RoomViewStore', function() { const setRoomVisible = jest.spyOn(SlidingSyncManager.instance, "setRoomVisible").mockReturnValue( Promise.resolve(""), ); - const subscribedRoomId = "!sub2:localhost"; - const subscribedRoomId2 = "!sub3:localhost"; - dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId }); - dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId2 }); - // sub(1) then unsub(1) sub(2) - expect(setRoomVisible).toHaveBeenCalledTimes(3); - await flushPromises(); - await flushPromises(); - // this should not churn, extra call to allow unsub(1) - expect(setRoomVisible).toHaveBeenCalledTimes(4); - // flush a bit more to ensure this doesn't change - await flushPromises(); - await flushPromises(); - expect(setRoomVisible).toHaveBeenCalledTimes(4); + const subscribedRoomId = "!sub1:localhost"; + const subscribedRoomId2 = "!sub2:localhost"; + dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId }, true); + dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId2 }, true); + await untilDispatch(Action.ActiveRoomChanged, dis); + // sub(1) then unsub(1) sub(2), unsub(1) + const wantCalls = [ + [subscribedRoomId, true], + [subscribedRoomId, false], + [subscribedRoomId2, true], + [subscribedRoomId, false], + ]; + expect(setRoomVisible).toHaveBeenCalledTimes(wantCalls.length); + wantCalls.forEach((v, i) => { + try { + expect(setRoomVisible.mock.calls[i][0]).toEqual(v[0]); + expect(setRoomVisible.mock.calls[i][1]).toEqual(v[1]); + } catch (err) { + throw new Error(`i=${i} got ${setRoomVisible.mock.calls[i]} want ${v}`); + } + }); }); }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index ed60040346..029c88df8f 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -35,7 +35,6 @@ import { normalize } from "matrix-js-sdk/src/utils"; import { ReEmitter } from "matrix-js-sdk/src/ReEmitter"; import { MatrixClientPeg as peg } from '../../src/MatrixClientPeg'; -import dis from '../../src/dispatcher/dispatcher'; import { makeType } from "../../src/utils/TypeUtils"; import { ValidatedServerConfig } from "../../src/utils/ValidatedServerConfig"; import { EnhancedMap } from "../../src/utils/maps"; @@ -456,18 +455,6 @@ export function mkServerConfig(hsUrl, isUrl) { }); } -export function getDispatchForStore(store) { - // Mock the dispatcher by gut-wrenching. Stores can only __emitChange whilst a - // dispatcher `_isDispatching` is true. - return (payload) => { - // these are private properties in flux dispatcher - // fool ts - (dis as any)._isDispatching = true; - (dis as any)._callbacks[store._dispatchToken](payload); - (dis as any)._isDispatching = false; - }; -} - // 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. diff --git a/test/test-utils/utilities.ts b/test/test-utils/utilities.ts index 4da1389477..76859da263 100644 --- a/test/test-utils/utilities.ts +++ b/test/test-utils/utilities.ts @@ -24,18 +24,104 @@ import { DispatcherAction } from "../../src/dispatcher/actions"; export const emitPromise = (e: EventEmitter, k: string | symbol) => new Promise(r => e.once(k, r)); -export function untilDispatch(waitForAction: DispatcherAction): Promise { - let dispatchHandle: string; - return new Promise(resolve => { - dispatchHandle = defaultDispatcher.register(payload => { - if (payload.action === waitForAction) { - defaultDispatcher.unregister(dispatchHandle); - resolve(payload); +/** + * Waits for a certain payload to be dispatched. + * @param waitForAction The action string to wait for or the callback which is invoked for every dispatch. If this returns true, stops waiting. + * @param timeout The max time to wait before giving up and stop waiting. If 0, no timeout. + * @param dispatcher The dispatcher to listen on. + * @returns A promise which resolves when the callback returns true. Resolves with the payload that made it stop waiting. + * Rejects when the timeout is reached. + */ +export function untilDispatch( + waitForAction: DispatcherAction | ((payload: ActionPayload) => boolean), dispatcher=defaultDispatcher, timeout=1000, +): Promise { + const callerLine = new Error().stack.toString().split("\n")[2]; + if (typeof waitForAction === "string") { + const action = waitForAction; + waitForAction = (payload) => { + return payload.action === action; + }; + } + const callback = waitForAction as ((payload: ActionPayload) => boolean); + return new Promise((resolve, reject) => { + let fulfilled = false; + let timeoutId; + // set a timeout handler if needed + if (timeout > 0) { + timeoutId = setTimeout(() => { + if (!fulfilled) { + reject(new Error(`untilDispatch: timed out at ${callerLine}`)); + fulfilled = true; + } + }, timeout); + } + // listen for dispatches + const token = dispatcher.register((p: ActionPayload) => { + const finishWaiting = callback(p); + if (finishWaiting || fulfilled) { // wait until we're told or we timeout + // if we haven't timed out, resolve now with the payload. + if (!fulfilled) { + resolve(p); + fulfilled = true; + } + // cleanup + dispatcher.unregister(token); + if (timeoutId) { + clearTimeout(timeoutId); + } } }); }); } +/** + * Waits for a certain event to be emitted. + * @param emitter The EventEmitter to listen on. + * @param eventName The event string to wait for. + * @param check Optional function which is invoked when the event fires. If this returns true, stops waiting. + * @param timeout The max time to wait before giving up and stop waiting. If 0, no timeout. + * @returns A promise which resolves when the callback returns true or when the event is emitted if + * no callback is provided. Rejects when the timeout is reached. + */ +export function untilEmission( + emitter: EventEmitter, eventName: string, check: ((...args: any[]) => boolean)=undefined, timeout=1000, +): Promise { + const callerLine = new Error().stack.toString().split("\n")[2]; + return new Promise((resolve, reject) => { + let fulfilled = false; + let timeoutId; + // set a timeout handler if needed + if (timeout > 0) { + timeoutId = setTimeout(() => { + if (!fulfilled) { + reject(new Error(`untilEmission: timed out at ${callerLine}`)); + fulfilled = true; + } + }, timeout); + } + const callback = (...args: any[]) => { + // if they supplied a check function, call it now. Bail if it returns false. + if (check) { + if (!check(...args)) { + return; + } + } + // we didn't time out, resolve. Otherwise, we already rejected so don't resolve now. + if (!fulfilled) { + resolve(); + fulfilled = true; + } + // cleanup + emitter.off(eventName, callback); + if (timeoutId) { + clearTimeout(timeoutId); + } + }; + // listen for emissions + emitter.on(eventName, callback); + }); +} + export const findByAttr = (attr: string) => (component: ReactWrapper, value: string) => component.find(`[${attr}="${value}"]`); export const findByTestId = findByAttr('data-test-id');