From 861268d39f4dd612f1ad2e02a7f1097d1f590be4 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 27 Apr 2020 15:25:04 -0600 Subject: [PATCH] Invent an AsyncStore and use it for room lists This is to get around the problem of a slow dispatch loop. Instead of slowing the whole app down to deal with room lists, we'll just raise events to say we're ready. Based upon the EventEmitter class. --- package.json | 1 + src/components/views/rooms/RoomList2.tsx | 6 +- src/stores/AsyncStore.ts | 105 ++++++++++++++++++ src/stores/room-list/RoomListStore2.ts | 55 +++++---- .../room-list/RoomListStoreTempProxy.ts | 6 +- .../room-list/algorithms/ChaoticAlgorithm.ts | 1 - yarn.lock | 5 + 7 files changed, 144 insertions(+), 35 deletions(-) create mode 100644 src/stores/AsyncStore.ts diff --git a/package.json b/package.json index 22ff071ba7..2dfb366972 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ }, "dependencies": { "@babel/runtime": "^7.8.3", + "await-lock": "^2.0.1", "blueimp-canvas-to-blob": "^3.5.0", "browser-encrypt-attachment": "^0.3.0", "browser-request": "^0.3.3", diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 1790fa8cf6..f97f599ae3 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -21,7 +21,7 @@ import { _t } from "../../../languageHandler"; import { Layout } from '../../../resizer/distributors/roomsublist2'; import { RovingTabIndexProvider } from "../../../accessibility/RovingTabIndex"; import { ResizeNotifier } from "../../../utils/ResizeNotifier"; -import RoomListStore from "../../../stores/room-list/RoomListStore2"; +import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore2"; interface IProps { onKeyDown: (ev: React.KeyboardEvent) => void; @@ -56,8 +56,8 @@ export default class RoomList2 extends React.Component { } public componentDidMount(): void { - RoomListStore.instance.addListener(() => { - console.log(RoomListStore.instance.orderedLists); + RoomListStore.instance.on(LISTS_UPDATE_EVENT, (store) => { + console.log("new lists", store.orderedLists); }); } diff --git a/src/stores/AsyncStore.ts b/src/stores/AsyncStore.ts new file mode 100644 index 0000000000..5e19e17248 --- /dev/null +++ b/src/stores/AsyncStore.ts @@ -0,0 +1,105 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { EventEmitter } from 'events'; +import AwaitLock from 'await-lock'; +import { ActionPayload } from "../dispatcher-types"; +import { Dispatcher } from "flux"; + +/** + * The event/channel to listen for in an AsyncStore. + */ +export const UPDATE_EVENT = "update"; + +/** + * Represents a minimal store which works similar to Flux stores. Instead + * of everything needing to happen in a dispatch cycle, everything can + * happen async to that cycle. + * + * The store's core principle is Object.assign(), therefore it is recommended + * to break out your state to be as safe as possible. The state mutations are + * also locked, preventing concurrent writes. + * + * All updates to the store happen on the UPDATE_EVENT event channel with the + * one argument being the instance of the store. + * + * To update the state, use updateState() and preferably await the result to + * help prevent lock conflicts. + */ +export abstract class AsyncStore extends EventEmitter { + private storeState: T = {}; + private lock = new AwaitLock(); + private readonly dispatcherRef: string; + + /** + * Creates a new AsyncStore using the given dispatcher. + * @param {Dispatcher} dispatcher The dispatcher to rely upon. + */ + protected constructor(private dispatcher: Dispatcher) { + super(); + + this.dispatcherRef = dispatcher.register(this.onDispatch.bind(this)); + } + + /** + * The current state of the store. Cannot be mutated. + */ + protected get state(): T { + return Object.freeze(this.storeState); + } + + /** + * Stops the store's listening functions, such as the listener to the dispatcher. + */ + protected stop() { + if (this.dispatcherRef) this.dispatcher.unregister(this.dispatcherRef); + } + + /** + * Updates the state of the store. + * @param {T|*} newState The state to update in the store using Object.assign() + */ + protected async updateState(newState: T | Object) { + await this.lock.acquireAsync(); + try { + this.storeState = Object.assign({}, this.storeState, newState); + this.emit(UPDATE_EVENT, this); + } finally { + await this.lock.release(); + } + } + + /** + * Resets the store's to the provided state or an empty object. + * @param {T|*} newState The new state of the store. + * @param {boolean} quiet If true, the function will not raise an UPDATE_EVENT. + */ + protected async reset(newState: T | Object = null, quiet = false) { + await this.lock.acquireAsync(); + try { + this.storeState = (newState || {}); + if (!quiet) this.emit(UPDATE_EVENT, this); + } finally { + await this.lock.release(); + } + } + + /** + * Called when the dispatcher broadcasts a dispatch event. + * @param {ActionPayload} payload The event being dispatched. + */ + protected abstract onDispatch(payload: ActionPayload); +} diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 70d6d4a598..7fab8c7ff9 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -15,15 +15,14 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {Store} from 'flux/utils'; -import {Room} from "matrix-js-sdk/src/models/room"; -import {MatrixClient} from "matrix-js-sdk/src/client"; +import { MatrixClient } from "matrix-js-sdk/src/client"; import { ActionPayload, defaultDispatcher } from "../../dispatcher-types"; import SettingsStore from "../../settings/SettingsStore"; -import { OrderedDefaultTagIDs, DefaultTagID, TagID } from "./models"; +import { DefaultTagID, OrderedDefaultTagIDs, TagID } from "./models"; import { IAlgorithm, ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/IAlgorithm"; import TagOrderStore from "../TagOrderStore"; import { getAlgorithmInstance } from "./algorithms"; +import { AsyncStore } from "../AsyncStore"; interface IState { tagsEnabled?: boolean; @@ -32,8 +31,13 @@ interface IState { preferredAlgorithm?: ListAlgorithm; } -class _RoomListStore extends Store { - private state: IState = {}; +/** + * The event/channel which is called when the room lists have been changed. Raised + * with one argument: the instance of the store. + */ +export const LISTS_UPDATE_EVENT = "lists_update"; + +class _RoomListStore extends AsyncStore { private matrixClient: MatrixClient; private initialListsGenerated = false; private enabled = false; @@ -49,7 +53,6 @@ class _RoomListStore extends Store { super(defaultDispatcher); this.checkEnabled(); - this.reset(); for (const settingName of this.watchedSettings) SettingsStore.monitorSetting(settingName, null); } @@ -66,17 +69,11 @@ class _RoomListStore extends Store { } } - private reset(): void { - // We don't call setState() because it'll cause changes to emitted which could - // crash the app during logout/signin/etc. - this.state = {}; - } - - private readAndCacheSettingsFromStore() { + private async readAndCacheSettingsFromStore() { const tagsEnabled = SettingsStore.isFeatureEnabled("feature_custom_tags"); const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance"); const orderAlphabetically = SettingsStore.getValue("RoomList.orderAlphabetically"); - this.setState({ + await this.updateState({ tagsEnabled, preferredSort: orderAlphabetically ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent, preferredAlgorithm: orderByImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural, @@ -84,23 +81,23 @@ class _RoomListStore extends Store { this.setAlgorithmClass(); } - protected __onDispatch(payload: ActionPayload): void { + protected async onDispatch(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; } + // TODO: Remove this once the RoomListStore becomes default this.checkEnabled(); if (!this.enabled) return; this.matrixClient = payload.matrixClient; // Update any settings here, as some may have happened before we were logically ready. - this.readAndCacheSettingsFromStore(); - - // noinspection JSIgnoredPromiseFromCall - this.regenerateAllLists(); + console.log("Regenerating room lists: Startup"); + await this.readAndCacheSettingsFromStore(); + await this.regenerateAllLists(); } // TODO: Remove this once the RoomListStore becomes default @@ -109,7 +106,7 @@ class _RoomListStore extends Store { 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. - this.reset(); + this.reset(null, true); this.matrixClient = null; this.initialListsGenerated = false; // we'll want to regenerate them } @@ -120,14 +117,15 @@ class _RoomListStore extends Store { if (payload.action === 'setting_updated') { if (this.watchedSettings.includes(payload.settingName)) { - this.readAndCacheSettingsFromStore(); + console.log("Regenerating room lists: Settings changed"); + await this.readAndCacheSettingsFromStore(); - // noinspection JSIgnoredPromiseFromCall - this.regenerateAllLists(); // regenerate the lists now + await this.regenerateAllLists(); // regenerate the lists now } } else if (payload.action === 'MatrixActions.Room.receipt') { // First see if the receipt event is for our own user. If it was, trigger // a room update (we probably read the room on a different device). + // noinspection JSObjectNullOrUndefined - this.matrixClient can't be null by this point in the lifecycle const myUserId = this.matrixClient.getUserId(); for (const eventId of Object.keys(payload.event.getContent())) { const receiptUsers = Object.keys(payload.event.getContent()[eventId]['m.read'] || {}); @@ -167,11 +165,10 @@ class _RoomListStore extends Store { } } - private setState(newState: IState) { + protected async updateState(newState: IState) { if (!this.enabled) return; - this.state = Object.assign(this.state, newState); - this.__emitChange(); + await super.updateState(newState); } private setAlgorithmClass() { @@ -179,7 +176,7 @@ class _RoomListStore extends Store { } private async regenerateAllLists() { - console.log("REGEN"); + console.warn("Regenerating all room lists"); const tags: ITagSortingMap = {}; for (const tagId of OrderedDefaultTagIDs) { tags[tagId] = this.getSortAlgorithmFor(tagId); @@ -196,7 +193,7 @@ class _RoomListStore extends Store { this.initialListsGenerated = true; - // TODO: How do we asynchronously update the store's state? or do we just give in and make it all sync? + this.emit(LISTS_UPDATE_EVENT, this); } } diff --git a/src/stores/room-list/RoomListStoreTempProxy.ts b/src/stores/room-list/RoomListStoreTempProxy.ts index 7b12602541..88171c0809 100644 --- a/src/stores/room-list/RoomListStoreTempProxy.ts +++ b/src/stores/room-list/RoomListStoreTempProxy.ts @@ -19,6 +19,8 @@ import { Room } from "matrix-js-sdk/src/models/room"; import SettingsStore from "../../settings/SettingsStore"; import RoomListStore from "./RoomListStore2"; import OldRoomListStore from "../RoomListStore"; +import { ITagMap } from "./algorithms/IAlgorithm"; +import { UPDATE_EVENT } from "../AsyncStore"; /** * Temporary RoomListStore proxy. Should be replaced with RoomListStore2 when @@ -33,13 +35,13 @@ export class RoomListStoreTempProxy { public static addListener(handler: () => void) { if (RoomListStoreTempProxy.isUsingNewStore()) { - return RoomListStore.instance.addListener(handler); + return RoomListStore.instance.on(UPDATE_EVENT, handler); } else { return OldRoomListStore.addListener(handler); } } - public static getRoomLists(): {[tagId in TagID]: Room[]} { + public static getRoomLists(): ITagMap { if (RoomListStoreTempProxy.isUsingNewStore()) { return RoomListStore.instance.orderedLists; } else { diff --git a/src/stores/room-list/algorithms/ChaoticAlgorithm.ts b/src/stores/room-list/algorithms/ChaoticAlgorithm.ts index 4fe5125a15..f72adb3aa8 100644 --- a/src/stores/room-list/algorithms/ChaoticAlgorithm.ts +++ b/src/stores/room-list/algorithms/ChaoticAlgorithm.ts @@ -65,7 +65,6 @@ export class ChaoticAlgorithm implements IAlgorithm { } // TODO: Remove logging - console.log('setting known rooms - regen in progress'); console.log({alg: this.representativeAlgorithm}); // Step through each room and determine which tags it should be in. diff --git a/yarn.lock b/yarn.lock index 6375c745fc..80c7e581a1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1834,6 +1834,11 @@ autoprefixer@^9.0.0: postcss "^7.0.27" postcss-value-parser "^4.0.3" +await-lock@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/await-lock/-/await-lock-2.0.1.tgz#b3f65fdf66e08f7538260f79b46c15bcfc18cadd" + integrity sha512-ntLi9fzlMT/vWjC1wwVI11/cSRJ3nTS35qVekNc9WnaoMOP2eWH0RvIqwLQkDjX4a4YynsKEv+Ere2VONp9wxg== + aws-sign2@~0.7.0: version "0.7.0" resolved "https://registry.yarnpkg.com/aws-sign2/-/aws-sign2-0.7.0.tgz#b46e890934a9591f2d2f6f86d7e6a9f1b3fe76a8"