From a8829f09d0ed23dc33148009322cb4019148e35d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 13 Jul 2020 13:15:44 -0600 Subject: [PATCH 1/3] Ensure RoomListStore2 gets reset when the client becomes invalidated Fixes https://github.com/vector-im/riot-web/issues/14384 We also make use of the new AsyncStore type to handle this more safely. --- src/stores/room-list/RoomListStore2.ts | 48 +++++++------------- test/components/views/rooms/RoomList-test.js | 2 +- 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index f3a77e765b..1be2bae6be 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -33,6 +33,7 @@ import { EffectiveMembership, getEffectiveMembership } from "./membership"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import RoomListLayoutStore from "./RoomListLayoutStore"; import { MarkedExecution } from "../../utils/MarkedExecution"; +import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; interface IState { tagsEnabled?: boolean; @@ -44,14 +45,13 @@ interface IState { */ export const LISTS_UPDATE_EVENT = "lists_update"; -export class RoomListStore2 extends AsyncStore { +export class RoomListStore2 extends AsyncStoreWithClient { /** * Set to true if you're running tests on the store. Should not be touched in * any other environment. */ public static TEST_MODE = false; - private _matrixClient: MatrixClient; private initialListsGenerated = false; private enabled = false; private algorithm = new Algorithm(); @@ -78,33 +78,30 @@ export class RoomListStore2 extends AsyncStore { return this.algorithm.getOrderedRooms(); } - public get matrixClient(): MatrixClient { - return this._matrixClient; - } - // Intended for test usage public async resetStore() { await this.reset(); this.tagWatcher = new TagWatcher(this); this.filterConditions = []; this.initialListsGenerated = false; - this._matrixClient = null; this.algorithm.off(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); this.algorithm.off(FILTER_CHANGED, this.onAlgorithmListUpdated); this.algorithm = new Algorithm(); this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); this.algorithm.on(FILTER_CHANGED, this.onAlgorithmListUpdated); + + // Reset state without causing updates as the client will have been destroyed + // and downstream code will throw NPE errors. + await this.reset(null, true); } // Public for test usage. Do not call this. - public async makeReady(client: MatrixClient) { + public async makeReady() { // TODO: Remove with https://github.com/vector-im/riot-web/issues/14367 this.checkEnabled(); if (!this.enabled) return; - this._matrixClient = client; - // Update any settings here, as some may have happened before we were logically ready. // Update any settings here, as some may have happened before we were logically ready. console.log("Regenerating room lists: Startup"); @@ -161,7 +158,15 @@ export class RoomListStore2 extends AsyncStore { if (trigger) this.updateFn.trigger(); } - protected async onDispatch(payload: ActionPayload) { + protected async onReady(): Promise { + await this.makeReady(); + } + + protected async onNotReady(): Promise { + await this.resetStore(); + } + + protected async onAction(payload: ActionPayload) { // When we're running tests we can't reliably use setImmediate out of timing concerns. // As such, we use a more synchronous model. if (RoomListStore2.TEST_MODE) { @@ -175,29 +180,10 @@ export class RoomListStore2 extends AsyncStore { } protected async onDispatchAsync(payload: ActionPayload) { - if (payload.action === 'MatrixActions.sync') { - // Filter out anything that isn't the first PREPARED sync. - if (!(payload.prevState === 'PREPARED' && payload.state !== 'PREPARED')) { - return; - } - - await this.makeReady(payload.matrixClient); - - return; // no point in running the next conditions - they won't match - } - // TODO: Remove this once the RoomListStore becomes default if (!this.enabled) return; - if (payload.action === 'on_client_not_viable' || payload.action === 'on_logged_out') { - // Reset state without causing updates as the client will have been destroyed - // and downstream code will throw NPE errors. - await this.reset(null, true); - this._matrixClient = null; - this.initialListsGenerated = false; // we'll want to regenerate them - } - - // Everything below here requires a MatrixClient or some sort of logical readiness. + // Everything here requires a MatrixClient or some sort of logical readiness. const logicallyReady = this.matrixClient && this.initialListsGenerated; if (!logicallyReady) return; diff --git a/test/components/views/rooms/RoomList-test.js b/test/components/views/rooms/RoomList-test.js index e84f943708..313bd4b77e 100644 --- a/test/components/views/rooms/RoomList-test.js +++ b/test/components/views/rooms/RoomList-test.js @@ -109,7 +109,7 @@ describe('RoomList', () => { client.getRoom = (roomId) => roomMap[roomId]; // Now that everything has been set up, prepare and update the store - await RoomListStore.instance.makeReady(client); + await RoomListStore.instance.makeReady(); done(); }); From eb78b1b328a01fc7a6e2013c10a31e0feba80e6c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 13 Jul 2020 13:18:01 -0600 Subject: [PATCH 2/3] Export the matrix client from the store --- src/stores/room-list/RoomListStore2.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 1be2bae6be..a3311ce90d 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -78,6 +78,10 @@ export class RoomListStore2 extends AsyncStoreWithClient { return this.algorithm.getOrderedRooms(); } + public get matrixClient(): MatrixClient { + return super.matrixClient; + } + // Intended for test usage public async resetStore() { await this.reset(); From 19500cf96abd23f761c5e6426da96af7329149ff Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 13 Jul 2020 13:24:02 -0600 Subject: [PATCH 3/3] Allow the tests to force a MatrixClient --- src/stores/room-list/RoomListStore2.ts | 6 +++++- test/components/views/rooms/RoomList-test.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index a3311ce90d..d67c728bf0 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -101,7 +101,11 @@ export class RoomListStore2 extends AsyncStoreWithClient { } // Public for test usage. Do not call this. - public async makeReady() { + public async makeReady(forcedClient?: MatrixClient) { + if (forcedClient) { + super.matrixClient = forcedClient; + } + // TODO: Remove with https://github.com/vector-im/riot-web/issues/14367 this.checkEnabled(); if (!this.enabled) return; diff --git a/test/components/views/rooms/RoomList-test.js b/test/components/views/rooms/RoomList-test.js index 313bd4b77e..e84f943708 100644 --- a/test/components/views/rooms/RoomList-test.js +++ b/test/components/views/rooms/RoomList-test.js @@ -109,7 +109,7 @@ describe('RoomList', () => { client.getRoom = (roomId) => roomMap[roomId]; // Now that everything has been set up, prepare and update the store - await RoomListStore.instance.makeReady(); + await RoomListStore.instance.makeReady(client); done(); });