From e7fffee17568fba9b89bbe98bc0b9382e111ba1b Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 29 Apr 2020 16:19:10 -0600 Subject: [PATCH] Remove the need for a tag manager Instead putting the tag handling in the Algorithm class --- src/stores/room-list/README.md | 13 +- src/stores/room-list/RoomListStore2.ts | 4 +- .../room-list/RoomListStoreTempProxy.ts | 2 +- src/stores/room-list/TagManager.ts | 32 ---- src/stores/room-list/algorithms/Algorithm.ts | 178 ++++++++++++++++++ .../room-list/algorithms/ChaoticAlgorithm.ts | 80 +------- src/stores/room-list/algorithms/IAlgorithm.ts | 88 --------- .../algorithms/ImportanceAlgorithm.ts | 76 +------- src/stores/room-list/algorithms/index.ts | 8 +- 9 files changed, 212 insertions(+), 269 deletions(-) delete mode 100644 src/stores/room-list/TagManager.ts create mode 100644 src/stores/room-list/algorithms/Algorithm.ts delete mode 100644 src/stores/room-list/algorithms/IAlgorithm.ts diff --git a/src/stores/room-list/README.md b/src/stores/room-list/README.md index ed13210420..0dd6c104d8 100644 --- a/src/stores/room-list/README.md +++ b/src/stores/room-list/README.md @@ -109,9 +109,10 @@ all kinds of filtering. ## Class breakdowns -The `RoomListStore` is the major coordinator of various `IAlgorithm` implementations, which take care -of the various `ListAlgorithm` and `SortingAlgorithm` options. A `TagManager` is responsible for figuring -out which tags get which rooms, as Matrix specifies them as a reverse map: tags are defined on rooms and -are not defined as a collection of rooms (unlike how they are presented to the user). Various list-specific -utilities are also included, though they are expected to move somewhere more general when needed. For -example, the `membership` utilities could easily be moved elsewhere as needed. +The `RoomListStore` is the major coordinator of various `Algorithm` implementations, which take care +of the various `ListAlgorithm` and `SortingAlgorithm` options. The `Algorithm` superclass is also +responsible for figuring out which tags get which rooms, as Matrix specifies them as a reverse map: +tags are defined on rooms and are not defined as a collection of rooms (unlike how they are presented +to the user). Various list-specific utilities are also included, though they are expected to move +somewhere more general when needed. For example, the `membership` utilities could easily be moved +elsewhere as needed. diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 7fab8c7ff9..dc1cb49cd6 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -19,7 +19,7 @@ import { MatrixClient } from "matrix-js-sdk/src/client"; import { ActionPayload, defaultDispatcher } from "../../dispatcher-types"; import SettingsStore from "../../settings/SettingsStore"; import { DefaultTagID, OrderedDefaultTagIDs, TagID } from "./models"; -import { IAlgorithm, ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/IAlgorithm"; +import { Algorithm, ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/Algorithm"; import TagOrderStore from "../TagOrderStore"; import { getAlgorithmInstance } from "./algorithms"; import { AsyncStore } from "../AsyncStore"; @@ -41,7 +41,7 @@ class _RoomListStore extends AsyncStore { private matrixClient: MatrixClient; private initialListsGenerated = false; private enabled = false; - private algorithm: IAlgorithm; + private algorithm: Algorithm; private readonly watchedSettings = [ 'RoomList.orderAlphabetically', diff --git a/src/stores/room-list/RoomListStoreTempProxy.ts b/src/stores/room-list/RoomListStoreTempProxy.ts index 88171c0809..8ad3c5d35e 100644 --- a/src/stores/room-list/RoomListStoreTempProxy.ts +++ b/src/stores/room-list/RoomListStoreTempProxy.ts @@ -19,7 +19,7 @@ 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 { ITagMap } from "./algorithms/Algorithm"; import { UPDATE_EVENT } from "../AsyncStore"; /** diff --git a/src/stores/room-list/TagManager.ts b/src/stores/room-list/TagManager.ts deleted file mode 100644 index 368c4e2c21..0000000000 --- a/src/stores/room-list/TagManager.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* -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"; - -// TODO: Docs on what this is -export class TagManager extends EventEmitter { - constructor() { - super(); - } - - // TODO: Implementation. - // This will need to track where rooms belong in tags, and which tags they - // should be tracked within. This is algorithm independent because all the - // algorithms need to know what each tag contains. - // - // This will likely make use of the effective membership to determine the - // invite+historical sections. -} diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts new file mode 100644 index 0000000000..15fc208b21 --- /dev/null +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -0,0 +1,178 @@ +/* +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 { DefaultTagID, 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"; + +export enum SortAlgorithm { + Manual = "MANUAL", + Alphabetic = "ALPHABETIC", + Recent = "RECENT", +} + +export enum ListAlgorithm { + // Orders Red > Grey > Bold > Idle + Importance = "IMPORTANCE", + + // Orders however the SortAlgorithm decides + Natural = "NATURAL", +} + +export interface ITagSortingMap { + // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. + [tagId: TagID]: SortAlgorithm; +} + +export interface ITagMap { + // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. + [tagId: TagID]: Room[]; +} + +// TODO: Add locking support to avoid concurrent writes? +// TODO: EventEmitter support? Might not be needed. + +export abstract class Algorithm { + protected cached: ITagMap = {}; + protected sortAlgorithms: ITagSortingMap; + protected rooms: Room[] = []; + protected roomsByTag: { + // @ts-ignore - TS wants this to be a string but we know better + [tagId: TagID]: Room[]; + } = {}; + + protected constructor() { + } + + /** + * Asks the Algorithm to regenerate all lists, using the tags given + * as reference for which lists to generate and which way to generate + * them. + * @param {ITagSortingMap} tagSortingMap The tags to generate. + * @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`); + this.sortAlgorithms = tagSortingMap; + return this.setKnownRooms(this.rooms); + } + + /** + * Gets an ordered set of rooms for the all known tags. + * @returns {ITagMap} The cached list of rooms, ordered, + * for each tag. May be empty, but never null/undefined. + */ + public getOrderedRooms(): ITagMap { + return this.cached; + } + + /** + * Seeds the Algorithm with a set of rooms. The algorithm will discard all + * previously known information and instead use these rooms instead. + * @param {Room[]} rooms The rooms to force the algorithm to use. + * @returns {Promise<*>} A promise which resolves when complete. + */ + public async setKnownRooms(rooms: Room[]): Promise { + if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); + if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`); + + this.rooms = rooms; + + const newTags: ITagMap = {}; + for (const tagId in this.sortAlgorithms) { + // noinspection JSUnfilteredForInLoop + newTags[tagId] = []; + } + + // If we can avoid doing work, do so. + if (!rooms.length) { + await this.generateFreshTags(newTags); // just in case it wants to do something + this.cached = newTags; + return; + } + + // Split out the easy rooms first (leave and invite) + const memberships = splitRoomsByMembership(rooms); + for (const room of memberships[EffectiveMembership.Invite]) { + console.log(`[DEBUG] "${room.name}" (${room.roomId}) is an Invite`); + newTags[DefaultTagID.Invite].push(room); + } + for (const room of memberships[EffectiveMembership.Leave]) { + console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Historical`); + newTags[DefaultTagID.Archived].push(room); + } + + // Now process all the joined rooms. This is a bit more complicated + for (const room of memberships[EffectiveMembership.Join]) { + const tags = Object.keys(room.tags || {}); + + let inTag = false; + if (tags.length > 0) { + for (const tag of tags) { + if (!isNullOrUndefined(newTags[tag])) { + console.log(`[DEBUG] "${room.name}" (${room.roomId}) is tagged as ${tag}`); + newTags[tag].push(room); + inTag = true; + } + } + } + + if (!inTag) { + // TODO: Determine if DM and push there instead + newTags[DefaultTagID.Untagged].push(room); + console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Untagged`); + } + } + + await this.generateFreshTags(newTags); + + this.cached = newTags; + } + + /** + * Called when the Algorithm believes a complete regeneration of the existing + * lists is needed. + * @param {ITagMap} updatedTagMap The tag map which needs populating. Each tag + * will already have the rooms which belong to it - they just need ordering. Must + * be mutated in place. + * @returns {Promise<*>} A promise which resolves when complete. + */ + protected abstract generateFreshTags(updatedTagMap: ITagMap): Promise; + + /** + * Called when the Algorithm wants a whole tag to be reordered. Typically this will + * be done whenever the tag's scope changes (added/removed rooms). + * @param {TagID} tagId The tag ID which changed. + * @param {Room[]} rooms The rooms within the tag, unordered. + * @returns {Promise} Resolves to the ordered rooms in the tag. + */ + protected abstract regenerateTag(tagId: TagID, rooms: Room[]): Promise; + + /** + * Asks the Algorithm to update its knowledge of a room. For example, when + * a user tags a room, joins/creates a room, or leaves a room the Algorithm + * should be told that the room's info might have changed. The Algorithm + * may no-op this request if no changes are required. + * @param {Room} room The room which might have affected sorting. + * @returns {Promise} A promise which resolve to true or false + * depending on whether or not getOrderedRooms() should be called after + * processing. + */ + // TODO: Take a ReasonForChange to better predict the behaviour? + // TODO: Intercept here and handle tag changes automatically + public abstract handleRoomUpdate(room: Room): Promise; +} diff --git a/src/stores/room-list/algorithms/ChaoticAlgorithm.ts b/src/stores/room-list/algorithms/ChaoticAlgorithm.ts index 1b640669c0..5d4177db8b 100644 --- a/src/stores/room-list/algorithms/ChaoticAlgorithm.ts +++ b/src/stores/room-list/algorithms/ChaoticAlgorithm.ts @@ -14,89 +14,29 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { IAlgorithm, ITagMap, ITagSortingMap, ListAlgorithm } from "./IAlgorithm"; -import { Room } from "matrix-js-sdk/src/models/room"; -import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; +import { Algorithm, ITagMap } from "./Algorithm"; import { DefaultTagID } from "../models"; -import { splitRoomsByMembership } from "../membership"; /** * A demonstration/temporary algorithm to verify the API surface works. * TODO: Remove this before shipping */ -export class ChaoticAlgorithm implements IAlgorithm { +export class ChaoticAlgorithm extends Algorithm { - private cached: ITagMap = {}; - private sortAlgorithms: ITagSortingMap; - private rooms: Room[] = []; - - constructor(private representativeAlgorithm: ListAlgorithm) { + constructor() { + super(); console.log("Constructed a ChaoticAlgorithm"); } - getOrderedRooms(): ITagMap { - return this.cached; + protected async generateFreshTags(updatedTagMap: ITagMap): Promise { + return Promise.resolve(); } - async populateTags(tagSortingMap: ITagSortingMap): Promise { - if (!tagSortingMap) throw new Error(`Map cannot be null or empty`); - this.sortAlgorithms = tagSortingMap; - this.setKnownRooms(this.rooms); // regenerate the room lists + protected async regenerateTag(tagId: string | DefaultTagID, rooms: []): Promise<[]> { + return Promise.resolve(rooms); } - handleRoomUpdate(room): Promise { - return undefined; - } - - setKnownRooms(rooms: Room[]): Promise { - if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); - if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`); - - this.rooms = rooms; - - const newTags = {}; - for (const tagId in this.sortAlgorithms) { - // noinspection JSUnfilteredForInLoop - newTags[tagId] = []; - } - - // If we can avoid doing work, do so. - if (!rooms.length) { - this.cached = newTags; - return; - } - - // TODO: Remove logging - const memberships = splitRoomsByMembership(rooms); - console.log({alg: this.representativeAlgorithm, memberships}); - - // Step through each room and determine which tags it should be in. - // We don't care about ordering or sorting here - we're simply organizing things. - for (const room of rooms) { - const tags = room.tags; - let inTag = false; - for (const tagId in tags) { - // noinspection JSUnfilteredForInLoop - if (isNullOrUndefined(newTags[tagId])) { - // skip the tag if we don't know about it - continue; - } - - inTag = true; - - // noinspection JSUnfilteredForInLoop - newTags[tagId].push(room); - } - - // If the room wasn't pushed to a tag, push it to the untagged tag. - if (!inTag) { - newTags[DefaultTagID.Untagged].push(room); - } - } - - // TODO: Do sorting - - // Finally, assign the tags to our cache - this.cached = newTags; + public async handleRoomUpdate(room): Promise { + return Promise.resolve(false); } } diff --git a/src/stores/room-list/algorithms/IAlgorithm.ts b/src/stores/room-list/algorithms/IAlgorithm.ts deleted file mode 100644 index b49f0b10b2..0000000000 --- a/src/stores/room-list/algorithms/IAlgorithm.ts +++ /dev/null @@ -1,88 +0,0 @@ -/* -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 { TagID } from "../models"; -import { Room } from "matrix-js-sdk/src/models/room"; - - -export enum SortAlgorithm { - Manual = "MANUAL", - Alphabetic = "ALPHABETIC", - Recent = "RECENT", -} - -export enum ListAlgorithm { - // Orders Red > Grey > Bold > Idle - Importance = "IMPORTANCE", - - // Orders however the SortAlgorithm decides - Natural = "NATURAL", -} - -export interface ITagSortingMap { - // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. - [tagId: TagID]: SortAlgorithm; -} - -export interface ITagMap { - // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. - [tagId: TagID]: Room[]; -} - -// TODO: Convert IAlgorithm to an abstract class? -// TODO: Add locking support to avoid concurrent writes? -// TODO: EventEmitter support - -/** - * Represents an algorithm for the RoomListStore to use - */ -export interface IAlgorithm { - /** - * Asks the Algorithm to regenerate all lists, using the tags given - * as reference for which lists to generate and which way to generate - * them. - * @param {ITagSortingMap} tagSortingMap The tags to generate. - * @returns {Promise<*>} A promise which resolves when complete. - */ - populateTags(tagSortingMap: ITagSortingMap): Promise; - - /** - * Gets an ordered set of rooms for the all known tags. - * @returns {ITagMap} The cached list of rooms, ordered, - * for each tag. May be empty, but never null/undefined. - */ - getOrderedRooms(): ITagMap; - - /** - * Seeds the Algorithm with a set of rooms. The algorithm will discard all - * previously known information and instead use these rooms instead. - * @param {Room[]} rooms The rooms to force the algorithm to use. - * @returns {Promise<*>} A promise which resolves when complete. - */ - setKnownRooms(rooms: Room[]): Promise; - - /** - * Asks the Algorithm to update its knowledge of a room. For example, when - * a user tags a room, joins/creates a room, or leaves a room the Algorithm - * should be told that the room's info might have changed. The Algorithm - * may no-op this request if no changes are required. - * @param {Room} room The room which might have affected sorting. - * @returns {Promise} A promise which resolve to true or false - * depending on whether or not getOrderedRooms() should be called after - * processing. - */ - handleRoomUpdate(room: Room): Promise; // TODO: Take a ReasonForChange to better predict the behaviour? -} diff --git a/src/stores/room-list/algorithms/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/ImportanceAlgorithm.ts index 0a2184eb43..1a7a73a9d5 100644 --- a/src/stores/room-list/algorithms/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/ImportanceAlgorithm.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { IAlgorithm, ITagMap, ITagSortingMap } from "./IAlgorithm"; +import { Algorithm, ITagMap, ITagSortingMap } from "./Algorithm"; import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import { DefaultTagID, TagID } from "../models"; @@ -60,7 +60,7 @@ export enum Category { * within the same category. For more information, see the comments contained * within the class. */ -export class ImportanceAlgorithm implements IAlgorithm { +export class ImportanceAlgorithm extends Algorithm { // HOW THIS WORKS // -------------- @@ -68,7 +68,7 @@ export class ImportanceAlgorithm implements IAlgorithm { // This block of comments assumes you've read the README one level higher. // You should do that if you haven't already. // - // Tags are fed into the algorithmic functions from the TagManager changes, + // 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 @@ -84,9 +84,6 @@ export class ImportanceAlgorithm implements IAlgorithm { // updated as needed and not recalculated often. For example, when a room needs to // move within a tag, the array in `this.cached` will be spliced instead of iterated. - private cached: ITagMap = {}; - private sortAlgorithms: ITagSortingMap; - private rooms: Room[] = []; private indices: { // @ts-ignore - TS wants this to be a string but we know better than it [tag: TagID]: { @@ -118,72 +115,19 @@ export class ImportanceAlgorithm implements IAlgorithm { } = {}; constructor() { + super(); console.log("Constructed an ImportanceAlgorithm"); } - getOrderedRooms(): ITagMap { - return this.cached; + protected async generateFreshTags(updatedTagMap: ITagMap): Promise { + return Promise.resolve(); } - async populateTags(tagSortingMap: ITagSortingMap): Promise { - if (!tagSortingMap) throw new Error(`Map cannot be null or empty`); - this.sortAlgorithms = tagSortingMap; - this.setKnownRooms(this.rooms); // regenerate the room lists + protected async regenerateTag(tagId: string | DefaultTagID, rooms: []): Promise<[]> { + return Promise.resolve(rooms); } - handleRoomUpdate(room): Promise { - return undefined; - } - - setKnownRooms(rooms: Room[]): Promise { - if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); - if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`); - - this.rooms = rooms; - - const newTags = {}; - for (const tagId in this.sortAlgorithms) { - // noinspection JSUnfilteredForInLoop - newTags[tagId] = []; - } - - // If we can avoid doing work, do so. - if (!rooms.length) { - this.cached = newTags; - return; - } - - // TODO: Remove logging - const memberships = splitRoomsByMembership(rooms); - console.log({memberships}); - - // Step through each room and determine which tags it should be in. - // We don't care about ordering or sorting here - we're simply organizing things. - for (const room of rooms) { - const tags = room.tags; - let inTag = false; - for (const tagId in tags) { - // noinspection JSUnfilteredForInLoop - if (isNullOrUndefined(newTags[tagId])) { - // skip the tag if we don't know about it - continue; - } - - inTag = true; - - // noinspection JSUnfilteredForInLoop - newTags[tagId].push(room); - } - - // If the room wasn't pushed to a tag, push it to the untagged tag. - if (!inTag) { - newTags[DefaultTagID.Untagged].push(room); - } - } - - // TODO: Do sorting - - // Finally, assign the tags to our cache - this.cached = newTags; + public async handleRoomUpdate(room): Promise { + return Promise.resolve(false); } } diff --git a/src/stores/room-list/algorithms/index.ts b/src/stores/room-list/algorithms/index.ts index 918b176f48..1277b66ac9 100644 --- a/src/stores/room-list/algorithms/index.ts +++ b/src/stores/room-list/algorithms/index.ts @@ -14,11 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { IAlgorithm, ListAlgorithm } from "./IAlgorithm"; +import { Algorithm, ListAlgorithm } from "./Algorithm"; import { ChaoticAlgorithm } from "./ChaoticAlgorithm"; import { ImportanceAlgorithm } from "./ImportanceAlgorithm"; -const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: () => IAlgorithm } = { +const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: () => Algorithm } = { [ListAlgorithm.Natural]: () => new ChaoticAlgorithm(ListAlgorithm.Natural), [ListAlgorithm.Importance]: () => new ImportanceAlgorithm(), }; @@ -26,9 +26,9 @@ const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: () => IAlgorithm } = /** * Gets an instance of the defined algorithm * @param {ListAlgorithm} algorithm The algorithm to get an instance of. - * @returns {IAlgorithm} The algorithm instance. + * @returns {Algorithm} The algorithm instance. */ -export function getAlgorithmInstance(algorithm: ListAlgorithm): IAlgorithm { +export function getAlgorithmInstance(algorithm: ListAlgorithm): Algorithm { if (!ALGORITHM_FACTORIES[algorithm]) { throw new Error(`${algorithm} is not a known algorithm`); }