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

@@ -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<IRoomProps, IRoomState> {
     private readonly dispatcherRef: string;
-    private readonly roomStoreToken: EventSubscription;
     private settingWatchers: string[];
 
     private unmounted = false;
@@ -439,7 +437,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
         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<IRoomProps, IRoomState> {
 
         window.removeEventListener('beforeunload', this.onPageUnload);
 
-        // Remove RoomStore listener
+        RoomViewStore.instance.off(UPDATE_EVENT, this.onRoomViewStoreUpdate);
-        if (this.roomStoreToken) {
-            this.roomStoreToken.remove();
-        }
 
         RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate);
         WidgetEchoStore.removeListener(UPDATE_EVENT, this.onWidgetEchoStoreUpdate);

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<IProps, IState> {
     private layoutWatcherRef: string;
     private timelinePanel = React.createRef<TimelinePanel>();
     private card = React.createRef<HTMLDivElement>();
-    private roomStoreToken: EventSubscription;
     private readReceiptsSettingWatcher: string;
 
     constructor(props: IProps) {
@@ -92,7 +91,7 @@ export default class TimelineCard extends React.Component<IProps, IState> {
     }
 
     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<IProps, IState> {
     }
 
     public componentWillUnmount(): void {
-        // Remove RoomStore listener
+        RoomViewStore.instance.removeListener(UPDATE_EVENT, this.onRoomViewStoreUpdate);
-
-        this.roomStoreToken?.remove();
 
         if (this.readReceiptsSettingWatcher) {
             SettingsStore.unwatchSetting(this.readReceiptsSettingWatcher);

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<IProps, IState> {
     private dispatcherRef;
-    private roomStoreToken: fbEmitter.EventSubscription;
     private treeRef = createRef<HTMLDivElement>();
     private favouriteMessageWatcher: string;
 
@@ -422,7 +421,7 @@ export default class RoomList extends React.PureComponent<IProps, IState> {
 
     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<IProps, IState> {
         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 = () => {

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<IProps, IState> {
-    private roomStoreToken: EventSubscription;
     private settingsWatcherRef: string;
     private movePersistedElement = createRef<() => void>();
 
@@ -141,7 +140,7 @@ export default class PipView extends React.Component<IProps, IState> {
     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<IProps, IState> {
         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) {

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
+ * A class for storing application state for RoomView.
-*  with a subset of the js-sdk.
- *  ```
  */
-export class RoomViewStore extends Store<ActionPayload> {
+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 dis: MatrixDispatcher;
-    private roomIdActivityListeners: Record<string, Listener[]> = {};
+    private dispatchToken: string;
 
-    public constructor() {
+    public constructor(dis: MatrixDispatcher) {
-        super(dis);
+        super();
+        this.resetDispatcher(dis);
     }
 
     public addRoomListener(roomId: string, fn: Listener): void {
-        if (!this.roomIdActivityListeners[roomId]) this.roomIdActivityListeners[roomId] = [];
+        this.on(roomId, fn);
-        this.roomIdActivityListeners[roomId].push(fn);
     }
 
     public removeRoomListener(roomId: string, fn: Listener): void {
-        if (this.roomIdActivityListeners[roomId]) {
+        this.off(roomId, fn);
-            const i = this.roomIdActivityListeners[roomId].indexOf(fn);
-            if (i > -1) {
-                this.roomIdActivityListeners[roomId].splice(i, 1);
-            }
-        } else {
-            logger.warn("Unregistering unrecognised listener (roomId=" + roomId + ")");
-        }
     }
 
     private emitForRoom(roomId: string, isActive: boolean): void {
-        if (!this.roomIdActivityListeners[roomId]) return;
+        this.emit(roomId, isActive);
-
-        for (const fn of this.roomIdActivityListeners[roomId]) {
-            fn.call(null, isActive);
-        }
     }
 
     private setState(newState: Partial<typeof INITIAL_STATE>): void {
@@ -156,17 +143,17 @@ export class RoomViewStore extends Store<ActionPayload> {
 
             // 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<ActiveRoomChangedPayload>({
+            this.dis.dispatch<ActiveRoomChangedPayload>({
                 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<ActionPayload> {
                 // 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<ViewRoomPayload>({
+                        this.dis.dispatch<ViewRoomPayload>({
                             action: Action.ViewRoom,
                             room_id: payload.event.getRoomId(),
                             replyingToEvent: payload.event,
@@ -283,9 +270,9 @@ export class RoomViewStore extends Store<ActionPayload> {
                 });
             }
             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<ActionPayload> {
                 // 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<ActionPayload> {
             this.setState(newState);
 
             if (payload.auto_join) {
-                dis.dispatch<JoinRoomPayload>({
+                this.dis.dispatch<JoinRoomPayload>({
                     ...payload,
                     action: Action.JoinRoom,
                     roomId: payload.room_id,
@@ -378,7 +365,7 @@ export class RoomViewStore extends Store<ActionPayload> {
                     roomId = result.room_id;
                 } catch (err) {
                     logger.error("RVS failed to get room id for alias: ", err);
-                    dis.dispatch<ViewRoomErrorPayload>({
+                    this.dis.dispatch<ViewRoomErrorPayload>({
                         action: Action.ViewRoomError,
                         room_id: null,
                         room_alias: payload.room_alias,
@@ -389,7 +376,7 @@ export class RoomViewStore extends Store<ActionPayload> {
             }
 
             // 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<ActionPayload> {
             // 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<JoinRoomReadyPayload>({
+            this.dis.dispatch<JoinRoomReadyPayload>({
                 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<ActionPayload> {
         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<string> {
         return this.state.roomId;

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<IState> 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();

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<IState> 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);

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<ReactWrapper> => {
         if (RoomViewStore.instance.getRoomId() !== room.roomId) {
             const switchedRoom = new Promise<void>(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<ViewRoomPayload>({

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 { getMockClientWithEventEmitter, untilDispatch, untilEmission } from '../test-utils';
-import { flushPromises, getMockClientWithEventEmitter } from '../test-utils';
 import SettingsStore from '../../src/settings/SettingsStore';
 import { SlidingSyncManager } from '../../src/SlidingSyncManager';
 import { TimelineRenderingType } from '../../src/contexts/RoomContext';
-
+import { MatrixDispatcher } from '../../src/dispatcher/dispatcher';
-const dispatch = testUtils.getDispatchForStore(RoomViewStore.instance);
+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' });
+        dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
-        dispatch({ action: Action.JoinRoom });
+        dis.dispatch({ action: Action.JoinRoom });
-        await flushPromises();
+        await untilDispatch(Action.JoinRoomReady, dis);
-        expect(mockClient.joinRoom).toHaveBeenCalledWith('!randomcharacters:aser.ver', { viaServers: [] });
+        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: [] });
-
+        dis.dispatch({ action: Action.ViewRoom, room_alias: alias });
-        dispatch({ action: Action.ViewRoom, room_alias: alias });
+        await untilDispatch((p) => { // wait for the re-dispatch with the room ID
-        await flushPromises();
+            return p.action === Action.ViewRoom && p.room_id === roomId;
-        await flushPromises();
+        }, 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' });
+        dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
-        await flushPromises();
+        await untilDispatch(Action.ActiveRoomChanged, dis);
         const replyToEvent = {
-            getRoomId: () => '!randomcharacters:aser.ver',
+            getRoomId: () => roomId,
         };
-        dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room });
+        dis.dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room });
-        await flushPromises();
+        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' });
+        // set the highlighed flag to make sure there is a state change so we get an update event
-        await flushPromises();
+        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 });
+            dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId });
-            await flushPromises();
+            await untilDispatch(Action.ActiveRoomChanged, dis);
-            await flushPromises();
             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 subscribedRoomId = "!sub1:localhost";
-            const subscribedRoomId2 = "!sub3:localhost";
+            const subscribedRoomId2 = "!sub2:localhost";
-            dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId });
+            dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId }, true);
-            dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId2 });
+            dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId2 }, true);
-            // sub(1) then unsub(1) sub(2)
+            await untilDispatch(Action.ActiveRoomChanged, dis);
-            expect(setRoomVisible).toHaveBeenCalledTimes(3);
+            // sub(1) then unsub(1) sub(2), unsub(1)
-            await flushPromises();
+            const wantCalls = [
-            await flushPromises();
+                [subscribedRoomId, true],
-            // this should not churn, extra call to allow unsub(1)
+                [subscribedRoomId, false],
-            expect(setRoomVisible).toHaveBeenCalledTimes(4);
+                [subscribedRoomId2, true],
-            // flush a bit more to ensure this doesn't change
+                [subscribedRoomId, false],
-            await flushPromises();
+            ];
-            await flushPromises();
+            expect(setRoomVisible).toHaveBeenCalledTimes(wantCalls.length);
-            expect(setRoomVisible).toHaveBeenCalledTimes(4);
+            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}`);
+                }
+            });
         });
     });
 });

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.
 

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<ActionPayload> {
+/**
-    let dispatchHandle: string;
+ * Waits for a certain payload to be dispatched.
-    return new Promise<ActionPayload>(resolve => {
+ * @param waitForAction The action string to wait for or the callback which is invoked for every dispatch. If this returns true, stops waiting.
-        dispatchHandle = defaultDispatcher.register(payload => {
+ * @param timeout The max time to wait before giving up and stop waiting. If 0, no timeout.
-            if (payload.action === waitForAction) {
+ * @param dispatcher The dispatcher to listen on.
-                defaultDispatcher.unregister(dispatchHandle);
+ * @returns A promise which resolves when the callback returns true. Resolves with the payload that made it stop waiting.
-                resolve(payload);
+ * Rejects when the timeout is reached.
+ */
+export function untilDispatch(
+    waitForAction: DispatcherAction | ((payload: ActionPayload) => boolean), dispatcher=defaultDispatcher, timeout=1000,
+): Promise<ActionPayload> {
+    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<void> {
+    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');