From fd029e8e8021cdf475d95ed13f65058900695d82 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Jun 2020 21:24:25 -0600 Subject: [PATCH] Dumb down list algorithms in favour of smarter tags This commit is a bit involved, as it factors the tag specific handling out of `/list-ordering` (and moves the `Algorithm` class one higher as a result), leaving it in the `Algorithm`. The algorithms for list ordering now only know how to handle a single tag, and this is managed by the `Algorithm` class - which is also no longer the base class for the list ordering. The list ordering algorithms now inherit from a generic `OrderingAlgorithm` base class which handles some rudimentary things. Overall the logic hasn't changed much: the tag-specific stuff has been moved into the `Algorithm`, and the list ordering algorithms essentially just removed the iteration on tags. The `RoomListStore2` still shovels a bunch of information over to the `Algorithm`, which can lead to an awkward code flow however this commit is meant to keep the number of surfaces touched to a minimum. The RoomListStore has also gained the ability to set per-list (tag) ordering and sorting, which is required for the new room list. The assumption that it defaults from the account-level settings is not reviewed by design, yet. This decision is deferred. --- src/stores/room-list/RoomListStore2.ts | 98 +++++--- .../{list-ordering => }/Algorithm.ts | 153 ++++++++++--- .../list-ordering/ImportanceAlgorithm.ts | 210 +++++++----------- .../list-ordering/NaturalAlgorithm.ts | 43 ++-- .../list-ordering/OrderingAlgorithm.ts | 72 ++++++ .../algorithms/list-ordering/index.ts | 21 +- src/stores/room-list/algorithms/models.ts | 11 + 7 files changed, 374 insertions(+), 234 deletions(-) rename src/stores/room-list/algorithms/{list-ordering => }/Algorithm.ts (76%) create mode 100644 src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 63a06dcf81..1c4e66c4b0 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -17,25 +17,21 @@ limitations under the License. import { MatrixClient } from "matrix-js-sdk/src/client"; import SettingsStore from "../../settings/SettingsStore"; -import { DefaultTagID, OrderedDefaultTagIDs, RoomUpdateCause, TagID } from "./models"; -import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/list-ordering/Algorithm"; +import { OrderedDefaultTagIDs, RoomUpdateCause, TagID } from "./models"; import TagOrderStore from "../TagOrderStore"; import { AsyncStore } from "../AsyncStore"; import { Room } from "matrix-js-sdk/src/models/room"; -import { ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/models"; -import { getListAlgorithmInstance } from "./algorithms/list-ordering"; +import { IListOrderingMap, ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/models"; import { ActionPayload } from "../../dispatcher/payloads"; import defaultDispatcher from "../../dispatcher/dispatcher"; import { readReceiptChangeIsFor } from "../../utils/read-receipts"; import { IFilterCondition } from "./filters/IFilterCondition"; import { TagWatcher } from "./TagWatcher"; import RoomViewStore from "../RoomViewStore"; +import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm"; interface IState { tagsEnabled?: boolean; - - preferredSort?: SortAlgorithm; - preferredAlgorithm?: ListAlgorithm; } /** @@ -48,7 +44,7 @@ export class RoomListStore2 extends AsyncStore { private _matrixClient: MatrixClient; private initialListsGenerated = false; private enabled = false; - private algorithm: Algorithm; + private algorithm = new Algorithm(); private filterConditions: IFilterCondition[] = []; private tagWatcher = new TagWatcher(this); @@ -64,6 +60,7 @@ export class RoomListStore2 extends AsyncStore { this.checkEnabled(); for (const settingName of this.watchedSettings) SettingsStore.monitorSetting(settingName, null); RoomViewStore.addListener(this.onRVSUpdate); + this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); } public get orderedLists(): ITagMap { @@ -85,14 +82,10 @@ export class RoomListStore2 extends AsyncStore { private async readAndCacheSettingsFromStore() { const tagsEnabled = SettingsStore.isFeatureEnabled("feature_custom_tags"); - const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance"); - const orderAlphabetically = SettingsStore.getValue("RoomList.orderAlphabetically"); await this.updateState({ tagsEnabled, - preferredSort: orderAlphabetically ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent, - preferredAlgorithm: orderByImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural, }); - this.setAlgorithmClass(); + await this.updateAlgorithmInstances(); } private onRVSUpdate = () => { @@ -259,17 +252,57 @@ export class RoomListStore2 extends AsyncStore { } } - private getSortAlgorithmFor(tagId: TagID): SortAlgorithm { - switch (tagId) { - case DefaultTagID.Invite: - case DefaultTagID.Untagged: - case DefaultTagID.Archived: - case DefaultTagID.LowPriority: - case DefaultTagID.DM: - return this.state.preferredSort; - case DefaultTagID.Favourite: - default: - return SortAlgorithm.Manual; + public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { + await this.algorithm.setTagSorting(tagId, sort); + localStorage.setItem(`mx_tagSort_${tagId}`, sort); + } + + public getTagSorting(tagId: TagID): SortAlgorithm { + return this.algorithm.getTagSorting(tagId); + } + + // noinspection JSMethodCanBeStatic + private getStoredTagSorting(tagId: TagID): SortAlgorithm { + return localStorage.getItem(`mx_tagSort_${tagId}`); + } + + public async setListOrder(tagId: TagID, order: ListAlgorithm) { + await this.algorithm.setListOrdering(tagId, order); + localStorage.setItem(`mx_listOrder_${tagId}`, order); + } + + public getListOrder(tagId: TagID): ListAlgorithm { + return this.algorithm.getListOrdering(tagId); + } + + // noinspection JSMethodCanBeStatic + private getStoredListOrder(tagId: TagID): ListAlgorithm { + return localStorage.getItem(`mx_listOrder_${tagId}`); + } + + private async updateAlgorithmInstances() { + const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance"); + const orderAlphabetically = SettingsStore.getValue("RoomList.orderAlphabetically"); + + const defaultSort = orderAlphabetically ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent; + const defaultOrder = orderByImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural; + + for (const tag of Object.keys(this.orderedLists)) { + const definedSort = this.getTagSorting(tag); + const definedOrder = this.getListOrder(tag); + + const storedSort = this.getStoredTagSorting(tag); + const storedOrder = this.getStoredListOrder(tag); + + const tagSort = storedSort ? storedSort : (definedSort ? definedSort : defaultSort); + const listOrder = storedOrder ? storedOrder : (definedOrder ? definedOrder : defaultOrder); + + if (tagSort !== definedSort) { + await this.setTagSorting(tag, tagSort); + } + if (listOrder !== definedOrder) { + await this.setListOrder(tag, listOrder); + } } } @@ -279,15 +312,6 @@ export class RoomListStore2 extends AsyncStore { await super.updateState(newState); } - private setAlgorithmClass() { - if (this.algorithm) { - this.algorithm.off(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); - } - this.algorithm = getListAlgorithmInstance(this.state.preferredAlgorithm); - this.algorithm.setFilterConditions(this.filterConditions); - this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); - } - private onAlgorithmListUpdated = () => { console.log("Underlying algorithm has triggered a list update - refiring"); this.emit(LISTS_UPDATE_EVENT, this); @@ -296,9 +320,11 @@ export class RoomListStore2 extends AsyncStore { private async regenerateAllLists() { console.warn("Regenerating all room lists"); - const tags: ITagSortingMap = {}; + const sorts: ITagSortingMap = {}; + const orders: IListOrderingMap = {}; for (const tagId of OrderedDefaultTagIDs) { - tags[tagId] = this.getSortAlgorithmFor(tagId); + sorts[tagId] = this.getStoredTagSorting(tagId) || SortAlgorithm.Alphabetic; + orders[tagId] = this.getStoredListOrder(tagId) || ListAlgorithm.Natural; } if (this.state.tagsEnabled) { @@ -307,7 +333,7 @@ export class RoomListStore2 extends AsyncStore { console.log("rtags", roomTags); } - await this.algorithm.populateTags(tags); + await this.algorithm.populateTags(sorts, orders); await this.algorithm.setKnownRooms(this.matrixClient.getRooms()); this.initialListsGenerated = true; diff --git a/src/stores/room-list/algorithms/list-ordering/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts similarity index 76% rename from src/stores/room-list/algorithms/list-ordering/Algorithm.ts rename to src/stores/room-list/algorithms/Algorithm.ts index a8512c0bd6..438ef78e7e 100644 --- a/src/stores/room-list/algorithms/list-ordering/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -14,17 +14,25 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { DefaultTagID, RoomUpdateCause, TagID } from "../../models"; import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; -import { EffectiveMembership, splitRoomsByMembership } from "../../membership"; -import { ITagMap, ITagSortingMap } from "../models"; -import DMRoomMap from "../../../../utils/DMRoomMap"; -import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../../filters/IFilterCondition"; +import DMRoomMap from "../../../utils/DMRoomMap"; import { EventEmitter } from "events"; -import { UPDATE_EVENT } from "../../../AsyncStore"; -import { ArrayUtil } from "../../../../utils/arrays"; -import { getEnumValues } from "../../../../utils/enums"; +import { arrayHasDiff, ArrayUtil } from "../../../utils/arrays"; +import { getEnumValues } from "../../../utils/enums"; +import { DefaultTagID, RoomUpdateCause, TagID } from "../models"; +import { + IListOrderingMap, + IOrderingAlgorithmMap, + ITagMap, + ITagSortingMap, + ListAlgorithm, + SortAlgorithm +} from "./models"; +import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../filters/IFilterCondition"; +import { EffectiveMembership, splitRoomsByMembership } from "../membership"; +import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; +import { getListAlgorithmInstance } from "./list-ordering"; // TODO: Add locking support to avoid concurrent writes? @@ -44,21 +52,22 @@ interface IStickyRoom { * management (which rooms go in which tags) and ask the implementation to * deal with ordering mechanics. */ -export abstract class Algorithm extends EventEmitter { +export class Algorithm extends EventEmitter { private _cachedRooms: ITagMap = {}; private _cachedStickyRooms: ITagMap = {}; // a clone of the _cachedRooms, with the sticky room private filteredRooms: ITagMap = {}; private _stickyRoom: IStickyRoom = null; - - protected sortAlgorithms: ITagSortingMap; - protected rooms: Room[] = []; - protected roomIdsToTags: { + private sortAlgorithms: ITagSortingMap; + private listAlgorithms: IListOrderingMap; + private algorithms: IOrderingAlgorithmMap; + private rooms: Room[] = []; + private roomIdsToTags: { [roomId: string]: TagID[]; } = {}; - protected allowedByFilter: Map = new Map(); - protected allowedRoomsByFilters: Set = new Set(); + private allowedByFilter: Map = new Map(); + private allowedRoomsByFilters: Set = new Set(); - protected constructor() { + public constructor() { super(); } @@ -68,6 +77,7 @@ export abstract class Algorithm extends EventEmitter { public set stickyRoom(val: Room) { // setters can't be async, so we call a private function to do the work + // noinspection JSIgnoredPromiseFromCall this.updateStickyRoom(val); } @@ -89,14 +99,38 @@ export abstract class Algorithm extends EventEmitter { return this._cachedRooms; } - /** - * Sets the filter conditions the Algorithm should use. - * @param filterConditions The filter conditions to use. - */ - public setFilterConditions(filterConditions: IFilterCondition[]): void { - for (const filter of filterConditions) { - this.addFilterCondition(filter); - } + public getTagSorting(tagId: TagID): SortAlgorithm { + return this.sortAlgorithms[tagId]; + } + + public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { + if (!tagId) throw new Error("Tag ID must be defined"); + if (!sort) throw new Error("Algorithm must be defined"); + this.sortAlgorithms[tagId] = sort; + + const algorithm: OrderingAlgorithm = this.algorithms[tagId]; + await algorithm.setSortAlgorithm(sort); + this._cachedRooms[tagId] = algorithm.orderedRooms; + this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list + this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed + } + + public getListOrdering(tagId: TagID): ListAlgorithm { + return this.listAlgorithms[tagId]; + } + + public async setListOrdering(tagId: TagID, order: ListAlgorithm) { + if (!tagId) throw new Error("Tag ID must be defined"); + if (!order) throw new Error("Algorithm must be defined"); + this.listAlgorithms[tagId] = order; + + const algorithm = getListAlgorithmInstance(order, tagId, this.sortAlgorithms[tagId]); + this.algorithms[tagId] = algorithm; + + await algorithm.setRooms(this._cachedRooms[tagId]) + this._cachedRooms[tagId] = algorithm.orderedRooms; + this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list + this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed } public addFilterCondition(filterCondition: IFilterCondition): void { @@ -310,11 +344,21 @@ export abstract class Algorithm extends EventEmitter { * as reference for which lists to generate and which way to generate * them. * @param {ITagSortingMap} tagSortingMap The tags to generate. + * @param {IListOrderingMap} listOrderingMap The ordering of those tags. * @returns {Promise<*>} A promise which resolves when complete. */ - public async populateTags(tagSortingMap: ITagSortingMap): Promise { - if (!tagSortingMap) throw new Error(`Map cannot be null or empty`); + public async populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): Promise { + if (!tagSortingMap) throw new Error(`Sorting map cannot be null or empty`); + if (!listOrderingMap) throw new Error(`Ordering ma cannot be null or empty`); + if (arrayHasDiff(Object.keys(tagSortingMap), Object.keys(listOrderingMap))) { + throw new Error(`Both maps must contain the exact same tags`); + } this.sortAlgorithms = tagSortingMap; + this.listAlgorithms = listOrderingMap; + this.algorithms = {}; + for (const tag of Object.keys(tagSortingMap)) { + this.algorithms[tag] = getListAlgorithmInstance(this.listAlgorithms[tag], tag, this.sortAlgorithms[tag]); + } return this.setKnownRooms(this.rooms); } @@ -428,7 +472,17 @@ export abstract class Algorithm extends EventEmitter { * be mutated in place. * @returns {Promise<*>} A promise which resolves when complete. */ - protected abstract generateFreshTags(updatedTagMap: ITagMap): Promise; + private async generateFreshTags(updatedTagMap: ITagMap): Promise { + if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + + for (const tag of Object.keys(updatedTagMap)) { + const algorithm: OrderingAlgorithm = this.algorithms[tag]; + if (!algorithm) throw new Error(`No algorithm for ${tag}`); + + await algorithm.setRooms(updatedTagMap[tag]); + updatedTagMap[tag] = algorithm.orderedRooms; + } + } /** * Asks the Algorithm to update its knowledge of a room. For example, when @@ -441,5 +495,48 @@ export abstract class Algorithm extends EventEmitter { * depending on whether or not getOrderedRooms() should be called after * processing. */ - public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise; + public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { + if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + + if (cause === RoomUpdateCause.PossibleTagChange) { + // TODO: Be smarter and splice rather than regen the planet. + // TODO: No-op if no change. + await this.setKnownRooms(this.rooms); + return true; + } + + if (cause === RoomUpdateCause.NewRoom) { + // TODO: Be smarter and insert rather than regen the planet. + await this.setKnownRooms([room, ...this.rooms]); + return; + } + + if (cause === RoomUpdateCause.RoomRemoved) { + // TODO: Be smarter and splice rather than regen the planet. + await this.setKnownRooms(this.rooms.filter(r => r !== room)); + return true; + } + + let tags = this.roomIdsToTags[room.roomId]; + if (!tags) { + console.warn(`No tags known for "${room.name}" (${room.roomId})`); + return false; + } + + let changed = false; + for (const tag of tags) { + const algorithm: OrderingAlgorithm = this.algorithms[tag]; + if (!algorithm) throw new Error(`No algorithm for ${tag}`); + + await algorithm.handleRoomUpdate(room, cause); + this.cachedRooms[tag] = algorithm.orderedRooms; + + // Flag that we've done something + this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list + this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed + changed = true; + } + + return true; + }; } diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index 7cbcc504c2..2e24cb23f1 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -15,12 +15,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Algorithm } from "./Algorithm"; import { Room } from "matrix-js-sdk/src/models/room"; import { RoomUpdateCause, TagID } from "../../models"; -import { ITagMap, SortAlgorithm } from "../models"; +import { SortAlgorithm } from "../models"; import { sortRoomsWithAlgorithm } from "../tag-sorting"; import * as Unread from '../../../../Unread'; +import { OrderingAlgorithm } from "./OrderingAlgorithm"; /** * The determined category of a room. @@ -77,32 +77,16 @@ const CATEGORY_ORDER = [Category.Red, Category.Grey, Category.Bold, Category.Idl * within the same category. For more information, see the comments contained * within the class. */ -export class ImportanceAlgorithm extends Algorithm { +export class ImportanceAlgorithm extends OrderingAlgorithm { + // This tracks the category for the tag it represents by tracking the index of + // each category within the list, where zero is the top of the list. This then + // tracks when rooms change categories and splices the orderedRooms array as + // needed, preventing many ordering operations. - // HOW THIS WORKS - // -------------- - // - // This block of comments assumes you've read the README two levels higher. - // You should do that if you haven't already. - // - // Tags are fed into the algorithmic functions from the Algorithm superclass, - // which cause subsequent updates to the room list itself. Categories within - // those tags are tracked as index numbers within the array (zero = top), with - // each sticky room being tracked separately. Internally, the category index - // can be found from `this.indices[tag][category]`. - // - // The room list store is always provided with the `this.cachedRooms` results, which are - // updated as needed and not recalculated often. For example, when a room needs to - // move within a tag, the array in `this.cachedRooms` will be spliced instead of iterated. - // The `indices` help track the positions of each category to make splicing easier. + private indices: ICategoryIndex = {}; - private indices: { - // @ts-ignore - TS wants this to be a string but we know better than it - [tag: TagID]: ICategoryIndex; - } = {}; - - constructor() { - super(); + public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { + super(tagId, initialSortingAlgorithm); console.log("Constructed an ImportanceAlgorithm"); } @@ -143,122 +127,81 @@ export class ImportanceAlgorithm extends Algorithm { return Category.Idle; } - protected async generateFreshTags(updatedTagMap: ITagMap): Promise { - for (const tagId of Object.keys(updatedTagMap)) { - const unorderedRooms = updatedTagMap[tagId]; - - const sortBy = this.sortAlgorithms[tagId]; - if (!sortBy) throw new Error(`${tagId} does not have a sorting algorithm`); - - if (sortBy === SortAlgorithm.Manual) { - // Manual tags essentially ignore the importance algorithm, so don't do anything - // special about them. - updatedTagMap[tagId] = await sortRoomsWithAlgorithm(unorderedRooms, tagId, sortBy); - } else { - // Every other sorting type affects the categories, not the whole tag. - const categorized = this.categorizeRooms(unorderedRooms); - for (const category of Object.keys(categorized)) { - const roomsToOrder = categorized[category]; - categorized[category] = await sortRoomsWithAlgorithm(roomsToOrder, tagId, sortBy); - } - - const newlyOrganized: Room[] = []; - const newIndices: ICategoryIndex = {}; - - for (const category of CATEGORY_ORDER) { - newIndices[category] = newlyOrganized.length; - newlyOrganized.push(...categorized[category]); - } - - this.indices[tagId] = newIndices; - updatedTagMap[tagId] = newlyOrganized; + public async setRooms(rooms: Room[]): Promise { + if (this.sortingAlgorithm === SortAlgorithm.Manual) { + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); + } else { + // Every other sorting type affects the categories, not the whole tag. + const categorized = this.categorizeRooms(rooms); + for (const category of Object.keys(categorized)) { + const roomsToOrder = categorized[category]; + categorized[category] = await sortRoomsWithAlgorithm(roomsToOrder, this.tagId, this.sortingAlgorithm); } + + const newlyOrganized: Room[] = []; + const newIndices: ICategoryIndex = {}; + + for (const category of CATEGORY_ORDER) { + newIndices[category] = newlyOrganized.length; + newlyOrganized.push(...categorized[category]); + } + + this.indices = newIndices; + this.cachedOrderedRooms = newlyOrganized; } } public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { - if (cause === RoomUpdateCause.PossibleTagChange) { - // TODO: Be smarter and splice rather than regen the planet. - // TODO: No-op if no change. - await this.setKnownRooms(this.rooms); - return; - } - - if (cause === RoomUpdateCause.NewRoom) { - // TODO: Be smarter and insert rather than regen the planet. - await this.setKnownRooms([room, ...this.rooms]); - return; - } - - if (cause === RoomUpdateCause.RoomRemoved) { - // TODO: Be smarter and splice rather than regen the planet. - await this.setKnownRooms(this.rooms.filter(r => r !== room)); - return; - } - - let tags = this.roomIdsToTags[room.roomId]; - if (!tags) { - console.warn(`No tags known for "${room.name}" (${room.roomId})`); - return false; - } const category = this.getRoomCategory(room); - let changed = false; - for (const tag of tags) { - if (this.sortAlgorithms[tag] === SortAlgorithm.Manual) { - continue; // Nothing to do here. - } - - const taggedRooms = this.cachedRooms[tag]; - const indices = this.indices[tag]; - let roomIdx = taggedRooms.indexOf(room); - if (roomIdx === -1) { - console.warn(`Degrading performance to find missing room in "${tag}": ${room.roomId}`); - roomIdx = taggedRooms.findIndex(r => r.roomId === room.roomId); - } - if (roomIdx === -1) { - throw new Error(`Room ${room.roomId} has no index in ${tag}`); - } - - // Try to avoid doing array operations if we don't have to: only move rooms within - // the categories if we're jumping categories - const oldCategory = this.getCategoryFromIndices(roomIdx, indices); - if (oldCategory !== category) { - // Move the room and update the indices - this.moveRoomIndexes(1, oldCategory, category, indices); - taggedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) - taggedRooms.splice(indices[category], 0, room); // splice in the new room (pre-adjusted) - // Note: if moveRoomIndexes() is called after the splice then the insert operation - // will happen in the wrong place. Because we would have already adjusted the index - // for the category, we don't need to determine how the room is moving in the list. - // If we instead tried to insert before updating the indices, we'd have to determine - // whether the room was moving later (towards IDLE) or earlier (towards RED) from its - // current position, as it'll affect the category's start index after we remove the - // room from the array. - } - - // The room received an update, so take out the slice and sort it. This should be relatively - // quick because the room is inserted at the top of the category, and most popular sorting - // algorithms will deal with trying to keep the active room at the top/start of the category. - // For the few algorithms that will have to move the thing quite far (alphabetic with a Z room - // for example), the list should already be sorted well enough that it can rip through the - // array and slot the changed room in quickly. - const nextCategoryStartIdx = category === CATEGORY_ORDER[CATEGORY_ORDER.length - 1] - ? Number.MAX_SAFE_INTEGER - : indices[CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]]; - const startIdx = indices[category]; - const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine - const unsortedSlice = taggedRooms.splice(startIdx, numSort); - const sorted = await sortRoomsWithAlgorithm(unsortedSlice, tag, this.sortAlgorithms[tag]); - taggedRooms.splice(startIdx, 0, ...sorted); - - // Finally, flag that we've done something - this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list - this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed - changed = true; + if (this.sortingAlgorithm === SortAlgorithm.Manual) { + return; // Nothing to do here. } - return changed; + + let roomIdx = this.cachedOrderedRooms.indexOf(room); + if (roomIdx === -1) { // can only happen if the js-sdk's store goes sideways. + console.warn(`Degrading performance to find missing room in "${this.tagId}": ${room.roomId}`); + roomIdx = this.cachedOrderedRooms.findIndex(r => r.roomId === room.roomId); + } + if (roomIdx === -1) { + throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`); + } + + // Try to avoid doing array operations if we don't have to: only move rooms within + // the categories if we're jumping categories + const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); + if (oldCategory !== category) { + // Move the room and update the indices + this.moveRoomIndexes(1, oldCategory, category, this.indices); + this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) + this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) + // Note: if moveRoomIndexes() is called after the splice then the insert operation + // will happen in the wrong place. Because we would have already adjusted the index + // for the category, we don't need to determine how the room is moving in the list. + // If we instead tried to insert before updating the indices, we'd have to determine + // whether the room was moving later (towards IDLE) or earlier (towards RED) from its + // current position, as it'll affect the category's start index after we remove the + // room from the array. + } + + // The room received an update, so take out the slice and sort it. This should be relatively + // quick because the room is inserted at the top of the category, and most popular sorting + // algorithms will deal with trying to keep the active room at the top/start of the category. + // For the few algorithms that will have to move the thing quite far (alphabetic with a Z room + // for example), the list should already be sorted well enough that it can rip through the + // array and slot the changed room in quickly. + const nextCategoryStartIdx = category === CATEGORY_ORDER[CATEGORY_ORDER.length - 1] + ? Number.MAX_SAFE_INTEGER + : this.indices[CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]]; + const startIdx = this.indices[category]; + const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine + const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort); + const sorted = await sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm); + this.cachedOrderedRooms.splice(startIdx, 0, ...sorted); + + return true; // change made } + // noinspection JSMethodCanBeStatic private getCategoryFromIndices(index: number, indices: ICategoryIndex): Category { for (let i = 0; i < CATEGORY_ORDER.length; i++) { const category = CATEGORY_ORDER[i]; @@ -274,6 +217,7 @@ export class ImportanceAlgorithm extends Algorithm { throw new Error("Programming error: somehow you've ended up with an index that isn't in a category"); } + // noinspection JSMethodCanBeStatic private moveRoomIndexes(nRooms: number, fromCategory: Category, toCategory: Category, indices: ICategoryIndex) { // We have to update the index of the category *after* the from/toCategory variables // in order to update the indices correctly. Because the room is moving from/to those diff --git a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts index d544b1196f..2302768fbf 100644 --- a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts @@ -14,49 +14,32 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Algorithm } from "./Algorithm"; -import { ITagMap } from "../models"; +import { SortAlgorithm } from "../models"; import { sortRoomsWithAlgorithm } from "../tag-sorting"; +import { OrderingAlgorithm } from "./OrderingAlgorithm"; +import { TagID } from "../../models"; +import { Room } from "matrix-js-sdk/src/models/room"; /** * Uses the natural tag sorting algorithm order to determine tag ordering. No * additional behavioural changes are present. */ -export class NaturalAlgorithm extends Algorithm { +export class NaturalAlgorithm extends OrderingAlgorithm { - constructor() { - super(); + public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { + super(tagId, initialSortingAlgorithm); console.log("Constructed a NaturalAlgorithm"); } - protected async generateFreshTags(updatedTagMap: ITagMap): Promise { - for (const tagId of Object.keys(updatedTagMap)) { - const unorderedRooms = updatedTagMap[tagId]; - - const sortBy = this.sortAlgorithms[tagId]; - if (!sortBy) throw new Error(`${tagId} does not have a sorting algorithm`); - - updatedTagMap[tagId] = await sortRoomsWithAlgorithm(unorderedRooms, tagId, sortBy); - } + public async setRooms(rooms: Room[]): Promise { + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); } public async handleRoomUpdate(room, cause): Promise { - const tags = this.roomIdsToTags[room.roomId]; - if (!tags) { - console.warn(`No tags known for "${room.name}" (${room.roomId})`); - return false; - } - let changed = false; - for (const tag of tags) { - // TODO: Optimize this loop to avoid useless operations - // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags - this.cachedRooms[tag] = await sortRoomsWithAlgorithm(this.cachedRooms[tag], tag, this.sortAlgorithms[tag]); + // TODO: Optimize this to avoid useless operations + // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm); - // Flag that we've done something - this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list - this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed - changed = true; - } - return changed; + return true; } } diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts new file mode 100644 index 0000000000..263e8a4cd4 --- /dev/null +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -0,0 +1,72 @@ +/* +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 { Room } from "matrix-js-sdk/src/models/room"; +import { RoomUpdateCause, TagID } from "../../models"; +import { SortAlgorithm } from "../models"; + +/** + * Represents a list ordering algorithm. Subclasses should populate the + * `cachedOrderedRooms` field. + */ +export abstract class OrderingAlgorithm { + protected cachedOrderedRooms: Room[]; + protected sortingAlgorithm: SortAlgorithm; + + protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { + // noinspection JSIgnoredPromiseFromCall + this.setSortAlgorithm(initialSortingAlgorithm); // we use the setter for validation + } + + /** + * The rooms as ordered by the algorithm. + */ + public get orderedRooms(): Room[] { + return this.cachedOrderedRooms || []; + } + + /** + * Sets the sorting algorithm to use within the list. + * @param newAlgorithm The new algorithm. Must be defined. + * @returns Resolves when complete. + */ + public async setSortAlgorithm(newAlgorithm: SortAlgorithm) { + if (!newAlgorithm) throw new Error("A sorting algorithm must be defined"); + this.sortingAlgorithm = newAlgorithm; + + // Force regeneration of the rooms + await this.setRooms(this.orderedRooms); + } + + /** + * Sets the rooms the algorithm should be handling, implying a reconstruction + * of the ordering. + * @param rooms The rooms to use going forward. + * @returns Resolves when complete. + */ + public abstract setRooms(rooms: Room[]): Promise; + + /** + * Handle a room update. The Algorithm will only call this for causes which + * the list ordering algorithm can handle within the same tag. For example, + * tag changes will not be sent here. + * @param room The room where the update happened. + * @param cause The cause of the update. + * @returns True if the update requires the Algorithm to update the presentation layers. + */ + // XXX: TODO: We assume this will only ever be a position update and NOT a NewRoom or RemoveRoom change!! + public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise; +} diff --git a/src/stores/room-list/algorithms/list-ordering/index.ts b/src/stores/room-list/algorithms/list-ordering/index.ts index bcccd150cd..8156c3a250 100644 --- a/src/stores/room-list/algorithms/list-ordering/index.ts +++ b/src/stores/room-list/algorithms/list-ordering/index.ts @@ -14,25 +14,32 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Algorithm } from "./Algorithm"; import { ImportanceAlgorithm } from "./ImportanceAlgorithm"; -import { ListAlgorithm } from "../models"; +import { ListAlgorithm, SortAlgorithm } from "../models"; import { NaturalAlgorithm } from "./NaturalAlgorithm"; +import { TagID } from "../../models"; +import { OrderingAlgorithm } from "./OrderingAlgorithm"; -const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: () => Algorithm } = { - [ListAlgorithm.Natural]: () => new NaturalAlgorithm(), - [ListAlgorithm.Importance]: () => new ImportanceAlgorithm(), +interface AlgorithmFactory { + (tagId: TagID, initialSortingAlgorithm: SortAlgorithm): OrderingAlgorithm; +} + +const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: AlgorithmFactory } = { + [ListAlgorithm.Natural]: (tagId, initSort) => new NaturalAlgorithm(tagId, initSort), + [ListAlgorithm.Importance]: (tagId, initSort) => new ImportanceAlgorithm(tagId, initSort), }; /** * Gets an instance of the defined algorithm * @param {ListAlgorithm} algorithm The algorithm to get an instance of. + * @param {TagID} tagId The tag the algorithm is for. + * @param {SortAlgorithm} initSort The initial sorting algorithm for the ordering algorithm. * @returns {Algorithm} The algorithm instance. */ -export function getListAlgorithmInstance(algorithm: ListAlgorithm): Algorithm { +export function getListAlgorithmInstance(algorithm: ListAlgorithm, tagId: TagID, initSort: SortAlgorithm): OrderingAlgorithm { if (!ALGORITHM_FACTORIES[algorithm]) { throw new Error(`${algorithm} is not a known algorithm`); } - return ALGORITHM_FACTORIES[algorithm](); + return ALGORITHM_FACTORIES[algorithm](tagId, initSort); } diff --git a/src/stores/room-list/algorithms/models.ts b/src/stores/room-list/algorithms/models.ts index 284600a776..943d833b67 100644 --- a/src/stores/room-list/algorithms/models.ts +++ b/src/stores/room-list/algorithms/models.ts @@ -16,6 +16,7 @@ limitations under the License. import { TagID } from "../models"; import { Room } from "matrix-js-sdk/src/models/room"; +import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; export enum SortAlgorithm { Manual = "MANUAL", @@ -36,6 +37,16 @@ export interface ITagSortingMap { [tagId: TagID]: SortAlgorithm; } +export interface IListOrderingMap { + // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. + [tagId: TagID]: ListAlgorithm; +} + +export interface IOrderingAlgorithmMap { + // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. + [tagId: TagID]: OrderingAlgorithm; +} + export interface ITagMap { // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. [tagId: TagID]: Room[];