Merge pull request #5825 from matrix-org/travis/spaces/room-list

Restabilize room list ordering with prefiltering on spaces/communities

Author: turt2live

docs/room-list-store.md

@@ -6,7 +6,7 @@ It's so complicated it needs its own README.
 
 Legend:
 * Orange = External event.
-* Purple = Deterministic flow. 
+* Purple = Deterministic flow.
 * Green = Algorithm definition.
 * Red = Exit condition/point.
 * Blue = Process definition.
@@ -24,8 +24,8 @@ algorithm to call, instead of having all the logic in the room list store itself
 
 
 Tag sorting is effectively the comparator supplied to the list algorithm. This gives the list algorithm
-the power to decide when and how to apply the tag sorting, if at all. For example, the importance algorithm, 
-later described in this document, heavily uses the list ordering behaviour to break the tag into categories. 
+the power to decide when and how to apply the tag sorting, if at all. For example, the importance algorithm,
+later described in this document, heavily uses the list ordering behaviour to break the tag into categories.
 Each category then gets sorted by the appropriate tag sorting algorithm.
 
 ### Tag sorting algorithm: Alphabetical
@@ -36,7 +36,7 @@ useful.
 
 ### Tag sorting algorithm: Manual
 
-Manual sorting makes use of the `order` property present on all tags for a room, per the 
+Manual sorting makes use of the `order` property present on all tags for a room, per the
 [Matrix specification](https://matrix.org/docs/spec/client_server/r0.6.0#room-tagging). Smaller values
 of `order` cause rooms to appear closer to the top of the list.
 
@@ -74,15 +74,15 @@ relative (perceived) importance to the user:
   set to 'All Messages'.
 * **Bold**: The room has unread messages waiting for the user. Essentially this is a grey room without
   a badge/notification count (or 'Mentions Only'/'Muted').
-* **Idle**: No useful (see definition of useful above) activity has occurred in the room since the user 
+* **Idle**: No useful (see definition of useful above) activity has occurred in the room since the user
   last read it.
 
 Conveniently, each tag gets ordered by those categories as presented: red rooms appear above grey, grey
 above bold, etc.
 
 Once the algorithm has determined which rooms belong in which categories, the tag sorting algorithm
 gets applied to each category in a sub-list fashion. This should result in the red rooms (for example)
-being sorted alphabetically amongst each other as well as the grey rooms sorted amongst each other, but 
+being sorted alphabetically amongst each other as well as the grey rooms sorted amongst each other, but
 collectively the tag will be sorted into categories with red being at the top.
 
 ## Sticky rooms
@@ -103,48 +103,62 @@ receive another notification which causes the room to move into the topmost posi
 above the sticky room will move underneath to allow for the new room to take the top slot, maintaining
 the sticky room's position.
 
-Though only applicable to the importance algorithm, the sticky room is not aware of category boundaries 
-and thus the user can see a shift in what kinds of rooms move around their selection. An example would 
-be the user having 4 red rooms, the user selecting the third room (leaving 2 above it), and then having 
-the rooms above it read on another device. This would result in 1 red room and 1 other kind of room 
+Though only applicable to the importance algorithm, the sticky room is not aware of category boundaries
+and thus the user can see a shift in what kinds of rooms move around their selection. An example would
+be the user having 4 red rooms, the user selecting the third room (leaving 2 above it), and then having
+the rooms above it read on another device. This would result in 1 red room and 1 other kind of room
 above the sticky room as it will try to maintain 2 rooms above the sticky room.
 
 An exception for the sticky room placement is when there's suddenly not enough rooms to maintain the placement
 exactly. This typically happens if the user selects a room and leaves enough rooms where it cannot maintain
 the N required rooms above the sticky room. In this case, the sticky room will simply decrease N as needed.
-The N value will never increase while selection remains unchanged: adding a bunch of rooms after having 
+The N value will never increase while selection remains unchanged: adding a bunch of rooms after having
 put the sticky room in a position where it's had to decrease N will not increase N.
 
 ## Responsibilities of the store
 
-The store is responsible for the ordering, upkeep, and tracking of all rooms. The room list component simply gets 
-an object containing the tags it needs to worry about and the rooms within. The room list component will 
-decide which tags need rendering (as it commonly filters out empty tags in most cases), and will deal with 
+The store is responsible for the ordering, upkeep, and tracking of all rooms. The room list component simply gets
+an object containing the tags it needs to worry about and the rooms within. The room list component will
+decide which tags need rendering (as it commonly filters out empty tags in most cases), and will deal with
 all kinds of filtering.
 
 ## Filtering
 
-Filters are provided to the store as condition classes, which are then passed along to the algorithm
-implementations. The implementations then get to decide how to actually filter the rooms, however in
-practice the base `Algorithm` class deals with the filtering in a more optimized/generic way.
+Filters are provided to the store as condition classes and have two major kinds: Prefilters and Runtime.
 
-The results of filters get cached to avoid needlessly iterating over potentially thousands of rooms,
-as the old room list store does. When a filter condition changes, it emits an update which (in this
-case) the `Algorithm` class will pick up and act accordingly. Typically, this also means filtering a 
+Prefilters flush out rooms which shouldn't appear to the algorithm implementations. Typically this is
+due to some higher order room list filtering (such as spaces or tags) deliberately exposing a subset of
+rooms to the user. The algorithm implementations will not see a room being prefiltered out.
+
+Runtime filters are used for more dynamic filtering, such as the user filtering by room name. These
+filters are passed along to the algorithm implementations where those implementations decide how and
+when to apply the filter. In practice, the base `Algorithm` class ends up doing the heavy lifting for
+optimization reasons.
+
+The results of runtime filters get cached to avoid needlessly iterating over potentially thousands of
+rooms, as the old room list store does. When a filter condition changes, it emits an update which (in this
+case) the `Algorithm` class will pick up and act accordingly. Typically, this also means filtering a
 minor subset where possible to avoid over-iterating rooms.
 
 All filter conditions are considered "stable" by the consumers, meaning that the consumer does not
 expect a change in the condition unless the condition says it has changed. This is intentional to
 maintain the caching behaviour described above.
 
+One might ask why we don't just use prefilter conditions for everything, and the answer is one of slight
+subtlety: in the cases of prefilters we are knowingly exposing the user to a workspace-style UX where
+room notifications are self-contained within that workspace. Runtime filters tend to not want to affect
+visible notification counts (as it doesn't want the room header to suddenly be confusing to the user as
+they type), and occasionally UX like "found 2/12 rooms" is desirable. If prefiltering were used instead,
+the notification counts would vary while the user was typing and "found 2/12" UX would not be possible.
+
 ## Class breakdowns
 
-The `RoomListStore` is the major coordinator of various algorithm implementations, which take care 
-of the various `ListAlgorithm` and `SortingAlgorithm` options. The `Algorithm` class is responsible 
-for figuring out which tags get which rooms, as Matrix specifies them as a reverse map: tags get 
-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 
+The `RoomListStore` is the major coordinator of various algorithm implementations, which take care
+of the various `ListAlgorithm` and `SortingAlgorithm` options. The `Algorithm` class is responsible
+for figuring out which tags get which rooms, as Matrix specifies them as a reverse map: tags get
+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 various bits throughout the room list store should also have jsdoc of some kind to help describe

src/stores/room-list/RoomListStore.ts

@@ -1,6 +1,5 @@
 /*
-Copyright 2018, 2019 New Vector Ltd
-Copyright 2020 The Matrix.org Foundation C.I.C.
+Copyright 2018-2021 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.
@@ -15,27 +14,27 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { MatrixClient } from "matrix-js-sdk/src/client";
+import {MatrixClient} from "matrix-js-sdk/src/client";
 import SettingsStore from "../../settings/SettingsStore";
-import { DefaultTagID, isCustomTag, OrderedDefaultTagIDs, RoomUpdateCause, TagID } from "./models";
-import { Room } from "matrix-js-sdk/src/models/room";
-import { IListOrderingMap, ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/models";
-import { ActionPayload } from "../../dispatcher/payloads";
+import {DefaultTagID, isCustomTag, OrderedDefaultTagIDs, RoomUpdateCause, TagID} from "./models";
+import {Room} from "matrix-js-sdk/src/models/room";
+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 { FILTER_CHANGED, IFilterCondition } from "./filters/IFilterCondition";
-import { TagWatcher } from "./TagWatcher";
+import {readReceiptChangeIsFor} from "../../utils/read-receipts";
+import {FILTER_CHANGED, FilterKind, IFilterCondition} from "./filters/IFilterCondition";
+import {TagWatcher} from "./TagWatcher";
 import RoomViewStore from "../RoomViewStore";
-import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm";
-import { EffectiveMembership, getEffectiveMembership } from "../../utils/membership";
-import { isNullOrUndefined } from "matrix-js-sdk/src/utils";
+import {Algorithm, LIST_UPDATED_EVENT} from "./algorithms/Algorithm";
+import {EffectiveMembership, getEffectiveMembership} from "../../utils/membership";
+import {isNullOrUndefined} from "matrix-js-sdk/src/utils";
 import RoomListLayoutStore from "./RoomListLayoutStore";
-import { MarkedExecution } from "../../utils/MarkedExecution";
-import { AsyncStoreWithClient } from "../AsyncStoreWithClient";
-import { NameFilterCondition } from "./filters/NameFilterCondition";
-import { RoomNotificationStateStore } from "../notifications/RoomNotificationStateStore";
-import { VisibilityProvider } from "./filters/VisibilityProvider";
-import { SpaceWatcher } from "./SpaceWatcher";
+import {MarkedExecution} from "../../utils/MarkedExecution";
+import {AsyncStoreWithClient} from "../AsyncStoreWithClient";
+import {NameFilterCondition} from "./filters/NameFilterCondition";
+import {RoomNotificationStateStore} from "../notifications/RoomNotificationStateStore";
+import {VisibilityProvider} from "./filters/VisibilityProvider";
+import {SpaceWatcher} from "./SpaceWatcher";
 
 interface IState {
     tagsEnabled?: boolean;
@@ -57,6 +56,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
     private initialListsGenerated = false;
     private algorithm = new Algorithm();
     private filterConditions: IFilterCondition[] = [];
+    private prefilterConditions: IFilterCondition[] = [];
     private tagWatcher: TagWatcher;
     private spaceWatcher: SpaceWatcher;
     private updateFn = new MarkedExecution(() => {
@@ -104,6 +104,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
     public async resetStore() {
         await this.reset();
         this.filterConditions = [];
+        this.prefilterConditions = [];
         this.initialListsGenerated = false;
         this.setupWatchers();
 
@@ -435,6 +436,39 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
         }
     }
 
+    private async recalculatePrefiltering() {
+        if (!this.algorithm) return;
+        if (!this.algorithm.hasTagSortingMap) return; // we're still loading
+
+        if (SettingsStore.getValue("advancedRoomListLogging")) {
+            // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
+            console.log("Calculating new prefiltered room list");
+        }
+
+        // Inhibit updates because we're about to lie heavily to the algorithm
+        this.algorithm.updatesInhibited = true;
+
+        // Figure out which rooms are about to be valid, and the state of affairs
+        const rooms = this.getPlausibleRooms();
+        const currentSticky = this.algorithm.stickyRoom;
+        const stickyIsStillPresent = currentSticky && rooms.includes(currentSticky);
+
+        // Reset the sticky room before resetting the known rooms so the algorithm
+        // doesn't freak out.
+        await this.algorithm.setStickyRoom(null);
+        await this.algorithm.setKnownRooms(rooms);
+
+        // Set the sticky room back, if needed, now that we have updated the store.
+        // This will use relative stickyness to the new room set.
+        if (stickyIsStillPresent) {
+            await this.algorithm.setStickyRoom(currentSticky);
+        }
+
+        // Finally, mark an update and resume updates from the algorithm
+        this.updateFn.mark();
+        this.algorithm.updatesInhibited = false;
+    }
+
     public async setTagSorting(tagId: TagID, sort: SortAlgorithm) {
         await this.setAndPersistTagSorting(tagId, sort);
         this.updateFn.trigger();
@@ -557,6 +591,34 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
         this.updateFn.trigger();
     };
 
+    private onPrefilterUpdated = async () => {
+        await this.recalculatePrefiltering();
+        this.updateFn.trigger();
+    };
+
+    private getPlausibleRooms(): Room[] {
+        if (!this.matrixClient) return [];
+
+        let rooms = [
+            ...this.matrixClient.getVisibleRooms(),
+            // also show space invites in the room list
+            ...this.matrixClient.getRooms().filter(r => r.isSpaceRoom() && r.getMyMembership() === "invite"),
+        ].filter(r => VisibilityProvider.instance.isRoomVisible(r));
+
+        if (this.prefilterConditions.length > 0) {
+            rooms = rooms.filter(r => {
+                for (const filter of this.prefilterConditions) {
+                    if (!filter.isVisible(r)) {
+                        return false;
+                    }
+                }
+                return true;
+            });
+        }
+
+        return rooms;
+    }
+
     /**
      * Regenerates the room whole room list, discarding any previous results.
      *
@@ -568,11 +630,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
     public async regenerateAllLists({trigger = true}) {
         console.warn("Regenerating all room lists");
 
-        const rooms = [
-            ...this.matrixClient.getVisibleRooms(),
-            // also show space invites in the room list
-            ...this.matrixClient.getRooms().filter(r => r.isSpaceRoom() && r.getMyMembership() === "invite"),
-        ].filter(r => VisibilityProvider.instance.isRoomVisible(r));
+        const rooms = this.getPlausibleRooms();
 
         const customTags = new Set<TagID>();
         if (this.state.tagsEnabled) {
@@ -601,32 +659,58 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
         if (trigger) this.updateFn.trigger();
     }
 
+    /**
+     * Adds a filter condition to the room list store. Filters may be applied async,
+     * and thus might not cause an update to the store immediately.
+     * @param {IFilterCondition} filter The filter condition to add.
+     */
     public addFilter(filter: IFilterCondition): void {
         if (SettingsStore.getValue("advancedRoomListLogging")) {
             // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
             console.log("Adding filter condition:", filter);
         }
-        this.filterConditions.push(filter);
-        if (this.algorithm) {
-            this.algorithm.addFilterCondition(filter);
+        let promise = Promise.resolve();
+        if (filter.kind === FilterKind.Prefilter) {
+            filter.on(FILTER_CHANGED, this.onPrefilterUpdated);
+            this.prefilterConditions.push(filter);
+            promise = this.recalculatePrefiltering();
+        } else {
+            this.filterConditions.push(filter);
+            if (this.algorithm) {
+                this.algorithm.addFilterCondition(filter);
+            }
         }
-        this.updateFn.trigger();
+        promise.then(() => this.updateFn.trigger());
     }
 
+    /**
+     * Removes a filter condition from the room list store. If the filter was
+     * not previously added to the room list store, this will no-op. The effects
+     * of removing a filter may be applied async and therefore might not cause
+     * an update right away.
+     * @param {IFilterCondition} filter The filter condition to remove.
+     */
     public removeFilter(filter: IFilterCondition): void {
         if (SettingsStore.getValue("advancedRoomListLogging")) {
             // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
             console.log("Removing filter condition:", filter);
         }
-        const idx = this.filterConditions.indexOf(filter);
+        let promise = Promise.resolve();
+        let idx = this.filterConditions.indexOf(filter);
         if (idx >= 0) {
             this.filterConditions.splice(idx, 1);
 
             if (this.algorithm) {
                 this.algorithm.removeFilterCondition(filter);
             }
         }
-        this.updateFn.trigger();
+        idx = this.prefilterConditions.indexOf(filter);
+        if (idx >= 0) {
+            filter.off(FILTER_CHANGED, this.onPrefilterUpdated);
+            this.prefilterConditions.splice(idx, 1);
+            promise = this.recalculatePrefiltering();
+        }
+        promise.then(() => this.updateFn.trigger());
     }
 
     /**