Merge pull request #6391 from matrix-org/t3chguy/fix/14508.1

This commit is contained in:
Michael Telatynski 2021-07-16 19:58:38 +01:00 committed by GitHub
commit aaa9040634
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 144 additions and 172 deletions

View file

@ -408,10 +408,10 @@ export default class RoomSublist extends React.Component<IProps, IState> {
this.setState({ addRoomContextMenuPosition: null }); this.setState({ addRoomContextMenuPosition: null });
}; };
private onUnreadFirstChanged = async () => { private onUnreadFirstChanged = () => {
const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance;
const newAlgorithm = isUnreadFirst ? ListAlgorithm.Natural : ListAlgorithm.Importance; const newAlgorithm = isUnreadFirst ? ListAlgorithm.Natural : ListAlgorithm.Importance;
await RoomListStore.instance.setListOrder(this.props.tagId, newAlgorithm); RoomListStore.instance.setListOrder(this.props.tagId, newAlgorithm);
this.forceUpdate(); // because if the sublist doesn't have any changes then we will miss the list order change this.forceUpdate(); // because if the sublist doesn't have any changes then we will miss the list order change
}; };

View file

@ -132,8 +132,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
// Update any settings here, as some may have happened before we were logically ready. // Update any settings here, as some may have happened before we were logically ready.
console.log("Regenerating room lists: Startup"); console.log("Regenerating room lists: Startup");
await this.readAndCacheSettingsFromStore(); await this.readAndCacheSettingsFromStore();
await this.regenerateAllLists({ trigger: false }); this.regenerateAllLists({ trigger: false });
await this.handleRVSUpdate({ trigger: false }); // fake an RVS update to adjust sticky room, if needed this.handleRVSUpdate({ trigger: false }); // fake an RVS update to adjust sticky room, if needed
this.updateFn.mark(); // we almost certainly want to trigger an update. this.updateFn.mark(); // we almost certainly want to trigger an update.
this.updateFn.trigger(); this.updateFn.trigger();
@ -150,7 +150,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
await this.updateState({ await this.updateState({
tagsEnabled, tagsEnabled,
}); });
await this.updateAlgorithmInstances(); this.updateAlgorithmInstances();
} }
/** /**
@ -158,23 +158,23 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
* @param trigger Set to false to prevent a list update from being sent. Should only * @param trigger Set to false to prevent a list update from being sent. Should only
* be used if the calling code will manually trigger the update. * be used if the calling code will manually trigger the update.
*/ */
private async handleRVSUpdate({ trigger = true }) { private handleRVSUpdate({ trigger = true }) {
if (!this.matrixClient) return; // We assume there won't be RVS updates without a client if (!this.matrixClient) return; // We assume there won't be RVS updates without a client
const activeRoomId = RoomViewStore.getRoomId(); const activeRoomId = RoomViewStore.getRoomId();
if (!activeRoomId && this.algorithm.stickyRoom) { if (!activeRoomId && this.algorithm.stickyRoom) {
await this.algorithm.setStickyRoom(null); this.algorithm.setStickyRoom(null);
} else if (activeRoomId) { } else if (activeRoomId) {
const activeRoom = this.matrixClient.getRoom(activeRoomId); const activeRoom = this.matrixClient.getRoom(activeRoomId);
if (!activeRoom) { if (!activeRoom) {
console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`); console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`);
await this.algorithm.setStickyRoom(null); this.algorithm.setStickyRoom(null);
} else if (activeRoom !== this.algorithm.stickyRoom) { } else if (activeRoom !== this.algorithm.stickyRoom) {
if (SettingsStore.getValue("advancedRoomListLogging")) { if (SettingsStore.getValue("advancedRoomListLogging")) {
// TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
console.log(`Changing sticky room to ${activeRoomId}`); console.log(`Changing sticky room to ${activeRoomId}`);
} }
await this.algorithm.setStickyRoom(activeRoom); this.algorithm.setStickyRoom(activeRoom);
} }
} }
@ -226,7 +226,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
console.log("Regenerating room lists: Settings changed"); console.log("Regenerating room lists: Settings changed");
await this.readAndCacheSettingsFromStore(); await this.readAndCacheSettingsFromStore();
await this.regenerateAllLists({ trigger: false }); // regenerate the lists now this.regenerateAllLists({ trigger: false }); // regenerate the lists now
this.updateFn.trigger(); this.updateFn.trigger();
} }
} }
@ -368,7 +368,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
// TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`); console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`);
} }
await this.algorithm.setStickyRoom(null); this.algorithm.setStickyRoom(null);
} }
// Note: we hit the algorithm instead of our handleRoomUpdate() function to // Note: we hit the algorithm instead of our handleRoomUpdate() function to
@ -377,7 +377,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
// TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
console.log(`[RoomListDebug] Removing previous room from room list`); console.log(`[RoomListDebug] Removing previous room from room list`);
} }
await this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved); this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved);
} }
} }
@ -433,7 +433,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
return; // don't do anything on new/moved rooms which ought not to be shown return; // don't do anything on new/moved rooms which ought not to be shown
} }
const shouldUpdate = await this.algorithm.handleRoomUpdate(room, cause); const shouldUpdate = this.algorithm.handleRoomUpdate(room, cause);
if (shouldUpdate) { if (shouldUpdate) {
if (SettingsStore.getValue("advancedRoomListLogging")) { if (SettingsStore.getValue("advancedRoomListLogging")) {
// TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
@ -462,13 +462,13 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
// Reset the sticky room before resetting the known rooms so the algorithm // Reset the sticky room before resetting the known rooms so the algorithm
// doesn't freak out. // doesn't freak out.
await this.algorithm.setStickyRoom(null); this.algorithm.setStickyRoom(null);
await this.algorithm.setKnownRooms(rooms); this.algorithm.setKnownRooms(rooms);
// Set the sticky room back, if needed, now that we have updated the store. // Set the sticky room back, if needed, now that we have updated the store.
// This will use relative stickyness to the new room set. // This will use relative stickyness to the new room set.
if (stickyIsStillPresent) { if (stickyIsStillPresent) {
await this.algorithm.setStickyRoom(currentSticky); this.algorithm.setStickyRoom(currentSticky);
} }
// Finally, mark an update and resume updates from the algorithm // Finally, mark an update and resume updates from the algorithm
@ -477,12 +477,12 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
} }
public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { public async setTagSorting(tagId: TagID, sort: SortAlgorithm) {
await this.setAndPersistTagSorting(tagId, sort); this.setAndPersistTagSorting(tagId, sort);
this.updateFn.trigger(); this.updateFn.trigger();
} }
private async setAndPersistTagSorting(tagId: TagID, sort: SortAlgorithm) { private setAndPersistTagSorting(tagId: TagID, sort: SortAlgorithm) {
await this.algorithm.setTagSorting(tagId, sort); this.algorithm.setTagSorting(tagId, sort);
// TODO: Per-account? https://github.com/vector-im/element-web/issues/14114 // TODO: Per-account? https://github.com/vector-im/element-web/issues/14114
localStorage.setItem(`mx_tagSort_${tagId}`, sort); localStorage.setItem(`mx_tagSort_${tagId}`, sort);
} }
@ -520,13 +520,13 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
return tagSort; return tagSort;
} }
public async setListOrder(tagId: TagID, order: ListAlgorithm) { public setListOrder(tagId: TagID, order: ListAlgorithm) {
await this.setAndPersistListOrder(tagId, order); this.setAndPersistListOrder(tagId, order);
this.updateFn.trigger(); this.updateFn.trigger();
} }
private async setAndPersistListOrder(tagId: TagID, order: ListAlgorithm) { private setAndPersistListOrder(tagId: TagID, order: ListAlgorithm) {
await this.algorithm.setListOrdering(tagId, order); this.algorithm.setListOrdering(tagId, order);
// TODO: Per-account? https://github.com/vector-im/element-web/issues/14114 // TODO: Per-account? https://github.com/vector-im/element-web/issues/14114
localStorage.setItem(`mx_listOrder_${tagId}`, order); localStorage.setItem(`mx_listOrder_${tagId}`, order);
} }
@ -563,7 +563,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
return listOrder; return listOrder;
} }
private async updateAlgorithmInstances() { private updateAlgorithmInstances() {
// We'll require an update, so mark for one. Marking now also prevents the calls // We'll require an update, so mark for one. Marking now also prevents the calls
// to setTagSorting and setListOrder from causing triggers. // to setTagSorting and setListOrder from causing triggers.
this.updateFn.mark(); this.updateFn.mark();
@ -576,10 +576,10 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
const listOrder = this.calculateListOrder(tag); const listOrder = this.calculateListOrder(tag);
if (tagSort !== definedSort) { if (tagSort !== definedSort) {
await this.setAndPersistTagSorting(tag, tagSort); this.setAndPersistTagSorting(tag, tagSort);
} }
if (listOrder !== definedOrder) { if (listOrder !== definedOrder) {
await this.setAndPersistListOrder(tag, listOrder); this.setAndPersistListOrder(tag, listOrder);
} }
} }
} }
@ -632,7 +632,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
* @param trigger Set to false to prevent a list update from being sent. Should only * @param trigger Set to false to prevent a list update from being sent. Should only
* be used if the calling code will manually trigger the update. * be used if the calling code will manually trigger the update.
*/ */
public async regenerateAllLists({ trigger = true }) { public regenerateAllLists({ trigger = true }) {
console.warn("Regenerating all room lists"); console.warn("Regenerating all room lists");
const rooms = this.getPlausibleRooms(); const rooms = this.getPlausibleRooms();
@ -656,8 +656,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
RoomListLayoutStore.instance.ensureLayoutExists(tagId); RoomListLayoutStore.instance.ensureLayoutExists(tagId);
} }
await this.algorithm.populateTags(sorts, orders); this.algorithm.populateTags(sorts, orders);
await this.algorithm.setKnownRooms(rooms); this.algorithm.setKnownRooms(rooms);
this.initialListsGenerated = true; this.initialListsGenerated = true;

View file

@ -16,8 +16,9 @@ limitations under the License.
import { Room } from "matrix-js-sdk/src/models/room"; import { Room } from "matrix-js-sdk/src/models/room";
import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils";
import DMRoomMap from "../../../utils/DMRoomMap";
import { EventEmitter } from "events"; import { EventEmitter } from "events";
import DMRoomMap from "../../../utils/DMRoomMap";
import { arrayDiff, arrayHasDiff } from "../../../utils/arrays"; import { arrayDiff, arrayHasDiff } from "../../../utils/arrays";
import { DefaultTagID, RoomUpdateCause, TagID } from "../models"; import { DefaultTagID, RoomUpdateCause, TagID } from "../models";
import { import {
@ -122,8 +123,8 @@ export class Algorithm extends EventEmitter {
* Awaitable version of the sticky room setter. * Awaitable version of the sticky room setter.
* @param val The new room to sticky. * @param val The new room to sticky.
*/ */
public async setStickyRoom(val: Room) { public setStickyRoom(val: Room) {
await this.updateStickyRoom(val); this.updateStickyRoom(val);
} }
public getTagSorting(tagId: TagID): SortAlgorithm { public getTagSorting(tagId: TagID): SortAlgorithm {
@ -131,13 +132,13 @@ export class Algorithm extends EventEmitter {
return this.sortAlgorithms[tagId]; return this.sortAlgorithms[tagId];
} }
public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { public setTagSorting(tagId: TagID, sort: SortAlgorithm) {
if (!tagId) throw new Error("Tag ID must be defined"); if (!tagId) throw new Error("Tag ID must be defined");
if (!sort) throw new Error("Algorithm must be defined"); if (!sort) throw new Error("Algorithm must be defined");
this.sortAlgorithms[tagId] = sort; this.sortAlgorithms[tagId] = sort;
const algorithm: OrderingAlgorithm = this.algorithms[tagId]; const algorithm: OrderingAlgorithm = this.algorithms[tagId];
await algorithm.setSortAlgorithm(sort); algorithm.setSortAlgorithm(sort);
this._cachedRooms[tagId] = algorithm.orderedRooms; this._cachedRooms[tagId] = algorithm.orderedRooms;
this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list
this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed
@ -148,7 +149,7 @@ export class Algorithm extends EventEmitter {
return this.listAlgorithms[tagId]; return this.listAlgorithms[tagId];
} }
public async setListOrdering(tagId: TagID, order: ListAlgorithm) { public setListOrdering(tagId: TagID, order: ListAlgorithm) {
if (!tagId) throw new Error("Tag ID must be defined"); if (!tagId) throw new Error("Tag ID must be defined");
if (!order) throw new Error("Algorithm must be defined"); if (!order) throw new Error("Algorithm must be defined");
this.listAlgorithms[tagId] = order; this.listAlgorithms[tagId] = order;
@ -156,7 +157,7 @@ export class Algorithm extends EventEmitter {
const algorithm = getListAlgorithmInstance(order, tagId, this.sortAlgorithms[tagId]); const algorithm = getListAlgorithmInstance(order, tagId, this.sortAlgorithms[tagId]);
this.algorithms[tagId] = algorithm; this.algorithms[tagId] = algorithm;
await algorithm.setRooms(this._cachedRooms[tagId]); algorithm.setRooms(this._cachedRooms[tagId]);
this._cachedRooms[tagId] = algorithm.orderedRooms; this._cachedRooms[tagId] = algorithm.orderedRooms;
this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list
this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed
@ -183,31 +184,25 @@ export class Algorithm extends EventEmitter {
} }
} }
private async handleFilterChange() { private handleFilterChange() {
await this.recalculateFilteredRooms(); this.recalculateFilteredRooms();
// re-emit the update so the list store can fire an off-cycle update if needed // re-emit the update so the list store can fire an off-cycle update if needed
if (this.updatesInhibited) return; if (this.updatesInhibited) return;
this.emit(FILTER_CHANGED); this.emit(FILTER_CHANGED);
} }
private async updateStickyRoom(val: Room) { private updateStickyRoom(val: Room) {
try { this.doUpdateStickyRoom(val);
return await this.doUpdateStickyRoom(val);
} finally {
this._lastStickyRoom = null; // clear to indicate we're done changing this._lastStickyRoom = null; // clear to indicate we're done changing
} }
}
private async doUpdateStickyRoom(val: Room) { private doUpdateStickyRoom(val: Room) {
if (SpaceStore.spacesEnabled && val?.isSpaceRoom() && val.getMyMembership() !== "invite") { if (SpaceStore.spacesEnabled && val?.isSpaceRoom() && val.getMyMembership() !== "invite") {
// no-op sticky rooms for spaces - they're effectively virtual rooms // no-op sticky rooms for spaces - they're effectively virtual rooms
val = null; val = null;
} }
// Note throughout: We need async so we can wait for handleRoomUpdate() to do its thing,
// otherwise we risk duplicating rooms.
if (val && !VisibilityProvider.instance.isRoomVisible(val)) { if (val && !VisibilityProvider.instance.isRoomVisible(val)) {
val = null; // the room isn't visible - lie to the rest of this function val = null; // the room isn't visible - lie to the rest of this function
} }
@ -223,7 +218,7 @@ export class Algorithm extends EventEmitter {
this._stickyRoom = null; // clear before we go to update the algorithm this._stickyRoom = null; // clear before we go to update the algorithm
// Lie to the algorithm and re-add the room to the algorithm // Lie to the algorithm and re-add the room to the algorithm
await this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom); this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom);
return; return;
} }
return; return;
@ -269,10 +264,10 @@ export class Algorithm extends EventEmitter {
// referential checks as the references can differ through the lifecycle. // referential checks as the references can differ through the lifecycle.
if (lastStickyRoom && lastStickyRoom.room && lastStickyRoom.room.roomId !== val.roomId) { if (lastStickyRoom && lastStickyRoom.room && lastStickyRoom.room.roomId !== val.roomId) {
// Lie to the algorithm and re-add the room to the algorithm // Lie to the algorithm and re-add the room to the algorithm
await this.handleRoomUpdate(lastStickyRoom.room, RoomUpdateCause.NewRoom); this.handleRoomUpdate(lastStickyRoom.room, RoomUpdateCause.NewRoom);
} }
// Lie to the algorithm and remove the room from it's field of view // Lie to the algorithm and remove the room from it's field of view
await this.handleRoomUpdate(val, RoomUpdateCause.RoomRemoved); this.handleRoomUpdate(val, RoomUpdateCause.RoomRemoved);
// Check for tag & position changes while we're here. We also check the room to ensure // Check for tag & position changes while we're here. We also check the room to ensure
// it is still the same room. // it is still the same room.
@ -462,9 +457,8 @@ export class Algorithm extends EventEmitter {
* them. * them.
* @param {ITagSortingMap} tagSortingMap The tags to generate. * @param {ITagSortingMap} tagSortingMap The tags to generate.
* @param {IListOrderingMap} listOrderingMap The ordering of those tags. * @param {IListOrderingMap} listOrderingMap The ordering of those tags.
* @returns {Promise<*>} A promise which resolves when complete.
*/ */
public async populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): Promise<any> { public populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): void {
if (!tagSortingMap) throw new Error(`Sorting map cannot be null or empty`); 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 (!listOrderingMap) throw new Error(`Ordering ma cannot be null or empty`);
if (arrayHasDiff(Object.keys(tagSortingMap), Object.keys(listOrderingMap))) { if (arrayHasDiff(Object.keys(tagSortingMap), Object.keys(listOrderingMap))) {
@ -513,9 +507,8 @@ export class Algorithm extends EventEmitter {
* Seeds the Algorithm with a set of rooms. The algorithm will discard all * Seeds the Algorithm with a set of rooms. The algorithm will discard all
* previously known information and instead use these rooms instead. * previously known information and instead use these rooms instead.
* @param {Room[]} rooms The rooms to force the algorithm to use. * @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<any> { public setKnownRooms(rooms: Room[]): void {
if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); 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`); if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`);
@ -529,7 +522,7 @@ export class Algorithm extends EventEmitter {
// Before we go any further we need to clear (but remember) the sticky room to // Before we go any further we need to clear (but remember) the sticky room to
// avoid accidentally duplicating it in the list. // avoid accidentally duplicating it in the list.
const oldStickyRoom = this._stickyRoom; const oldStickyRoom = this._stickyRoom;
await this.updateStickyRoom(null); if (oldStickyRoom) this.updateStickyRoom(null);
this.rooms = rooms; this.rooms = rooms;
@ -541,7 +534,7 @@ export class Algorithm extends EventEmitter {
// If we can avoid doing work, do so. // If we can avoid doing work, do so.
if (!rooms.length) { if (!rooms.length) {
await this.generateFreshTags(newTags); // just in case it wants to do something this.generateFreshTags(newTags); // just in case it wants to do something
this.cachedRooms = newTags; this.cachedRooms = newTags;
return; return;
} }
@ -578,7 +571,7 @@ export class Algorithm extends EventEmitter {
} }
} }
await this.generateFreshTags(newTags); this.generateFreshTags(newTags);
this.cachedRooms = newTags; // this recalculates the filtered rooms for us this.cachedRooms = newTags; // this recalculates the filtered rooms for us
this.updateTagsFromCache(); this.updateTagsFromCache();
@ -587,7 +580,7 @@ export class Algorithm extends EventEmitter {
// it was. It's entirely possible that it changed lists though, so if it did then // it was. It's entirely possible that it changed lists though, so if it did then
// we also have to update the position of it. // we also have to update the position of it.
if (oldStickyRoom && oldStickyRoom.room) { if (oldStickyRoom && oldStickyRoom.room) {
await this.updateStickyRoom(oldStickyRoom.room); this.updateStickyRoom(oldStickyRoom.room);
if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan
if (this._stickyRoom.tag !== oldStickyRoom.tag) { if (this._stickyRoom.tag !== oldStickyRoom.tag) {
// We put the sticky room at the top of the list to treat it as an obvious tag change. // We put the sticky room at the top of the list to treat it as an obvious tag change.
@ -652,16 +645,15 @@ export class Algorithm extends EventEmitter {
* @param {ITagMap} updatedTagMap The tag map which needs populating. Each tag * @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 * will already have the rooms which belong to it - they just need ordering. Must
* be mutated in place. * be mutated in place.
* @returns {Promise<*>} A promise which resolves when complete.
*/ */
private async generateFreshTags(updatedTagMap: ITagMap): Promise<any> { private generateFreshTags(updatedTagMap: ITagMap): void {
if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from");
for (const tag of Object.keys(updatedTagMap)) { for (const tag of Object.keys(updatedTagMap)) {
const algorithm: OrderingAlgorithm = this.algorithms[tag]; const algorithm: OrderingAlgorithm = this.algorithms[tag];
if (!algorithm) throw new Error(`No algorithm for ${tag}`); if (!algorithm) throw new Error(`No algorithm for ${tag}`);
await algorithm.setRooms(updatedTagMap[tag]); algorithm.setRooms(updatedTagMap[tag]);
updatedTagMap[tag] = algorithm.orderedRooms; updatedTagMap[tag] = algorithm.orderedRooms;
} }
} }
@ -673,11 +665,10 @@ export class Algorithm extends EventEmitter {
* may no-op this request if no changes are required. * may no-op this request if no changes are required.
* @param {Room} room The room which might have affected sorting. * @param {Room} room The room which might have affected sorting.
* @param {RoomUpdateCause} cause The reason for the update being triggered. * @param {RoomUpdateCause} cause The reason for the update being triggered.
* @returns {Promise<boolean>} A promise which resolve to true or false * @returns {Promise<boolean>} A boolean of whether or not getOrderedRooms()
* depending on whether or not getOrderedRooms() should be called after * should be called after processing.
* processing.
*/ */
public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> { public handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean {
if (SettingsStore.getValue("advancedRoomListLogging")) { if (SettingsStore.getValue("advancedRoomListLogging")) {
// TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602
console.log(`Handle room update for ${room.roomId} called with cause ${cause}`); console.log(`Handle room update for ${room.roomId} called with cause ${cause}`);
@ -685,9 +676,9 @@ export class Algorithm extends EventEmitter {
if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from");
// Note: check the isSticky against the room ID just in case the reference is wrong // Note: check the isSticky against the room ID just in case the reference is wrong
const isSticky = this._stickyRoom && this._stickyRoom.room && this._stickyRoom.room.roomId === room.roomId; const isSticky = this._stickyRoom?.room?.roomId === room.roomId;
if (cause === RoomUpdateCause.NewRoom) { if (cause === RoomUpdateCause.NewRoom) {
const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; const isForLastSticky = this._lastStickyRoom?.room === room;
const roomTags = this.roomIdsToTags[room.roomId]; const roomTags = this.roomIdsToTags[room.roomId];
const hasTags = roomTags && roomTags.length > 0; const hasTags = roomTags && roomTags.length > 0;
@ -744,7 +735,7 @@ export class Algorithm extends EventEmitter {
} }
const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; const algorithm: OrderingAlgorithm = this.algorithms[rmTag];
if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); if (!algorithm) throw new Error(`No algorithm for ${rmTag}`);
await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved);
this._cachedRooms[rmTag] = algorithm.orderedRooms; this._cachedRooms[rmTag] = algorithm.orderedRooms;
this.recalculateFilteredRoomsForTag(rmTag); // update filter to re-sort the list this.recalculateFilteredRoomsForTag(rmTag); // update filter to re-sort the list
this.recalculateStickyRoom(rmTag); // update sticky room to make sure it moves if needed this.recalculateStickyRoom(rmTag); // update sticky room to make sure it moves if needed
@ -756,7 +747,7 @@ export class Algorithm extends EventEmitter {
} }
const algorithm: OrderingAlgorithm = this.algorithms[addTag]; const algorithm: OrderingAlgorithm = this.algorithms[addTag];
if (!algorithm) throw new Error(`No algorithm for ${addTag}`); if (!algorithm) throw new Error(`No algorithm for ${addTag}`);
await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom);
this._cachedRooms[addTag] = algorithm.orderedRooms; this._cachedRooms[addTag] = algorithm.orderedRooms;
} }
@ -789,7 +780,7 @@ export class Algorithm extends EventEmitter {
}; };
} else { } else {
// We have to clear the lock as the sticky room change will trigger updates. // We have to clear the lock as the sticky room change will trigger updates.
await this.setStickyRoom(room); this.setStickyRoom(room);
} }
} }
} }
@ -852,7 +843,7 @@ export class Algorithm extends EventEmitter {
const algorithm: OrderingAlgorithm = this.algorithms[tag]; const algorithm: OrderingAlgorithm = this.algorithms[tag];
if (!algorithm) throw new Error(`No algorithm for ${tag}`); if (!algorithm) throw new Error(`No algorithm for ${tag}`);
await algorithm.handleRoomUpdate(room, cause); algorithm.handleRoomUpdate(room, cause);
this._cachedRooms[tag] = algorithm.orderedRooms; this._cachedRooms[tag] = algorithm.orderedRooms;
// Flag that we've done something // Flag that we've done something

View file

@ -94,15 +94,15 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
return state.color; return state.color;
} }
public async setRooms(rooms: Room[]): Promise<any> { public setRooms(rooms: Room[]): void {
if (this.sortingAlgorithm === SortAlgorithm.Manual) { if (this.sortingAlgorithm === SortAlgorithm.Manual) {
this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); this.cachedOrderedRooms = sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm);
} else { } else {
// Every other sorting type affects the categories, not the whole tag. // Every other sorting type affects the categories, not the whole tag.
const categorized = this.categorizeRooms(rooms); const categorized = this.categorizeRooms(rooms);
for (const category of Object.keys(categorized)) { for (const category of Object.keys(categorized)) {
const roomsToOrder = categorized[category]; const roomsToOrder = categorized[category];
categorized[category] = await sortRoomsWithAlgorithm(roomsToOrder, this.tagId, this.sortingAlgorithm); categorized[category] = sortRoomsWithAlgorithm(roomsToOrder, this.tagId, this.sortingAlgorithm);
} }
const newlyOrganized: Room[] = []; const newlyOrganized: Room[] = [];
@ -118,12 +118,12 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
} }
} }
private async handleSplice(room: Room, cause: RoomUpdateCause): Promise<boolean> { private handleSplice(room: Room, cause: RoomUpdateCause): boolean {
if (cause === RoomUpdateCause.NewRoom) { if (cause === RoomUpdateCause.NewRoom) {
const category = this.getRoomCategory(room); const category = this.getRoomCategory(room);
this.alterCategoryPositionBy(category, 1, this.indices); this.alterCategoryPositionBy(category, 1, this.indices);
this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted)
await this.sortCategory(category); this.sortCategory(category);
} else if (cause === RoomUpdateCause.RoomRemoved) { } else if (cause === RoomUpdateCause.RoomRemoved) {
const roomIdx = this.getRoomIndex(room); const roomIdx = this.getRoomIndex(room);
if (roomIdx === -1) { if (roomIdx === -1) {
@ -141,10 +141,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
return true; return true;
} }
public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> { public handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean {
try {
await this.updateLock.acquireAsync();
if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) { if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) {
return this.handleSplice(room, cause); return this.handleSplice(room, cause);
} }
@ -181,15 +178,12 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
} }
// Sort the category now that we've dumped the room in // Sort the category now that we've dumped the room in
await this.sortCategory(category); this.sortCategory(category);
return true; // change made return true; // change made
} finally {
await this.updateLock.release();
}
} }
private async sortCategory(category: NotificationColor) { private sortCategory(category: NotificationColor) {
// This should be relatively quick because the room is usually inserted at the top of the // This should be relatively quick because the room is usually inserted at the top of the
// category, and most popular sorting algorithms will deal with trying to keep the active // 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 // room at the top/start of the category. For the few algorithms that will have to move the
@ -201,7 +195,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
const startIdx = this.indices[category]; const startIdx = this.indices[category];
const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine
const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort); const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort);
const sorted = await sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm); const sorted = sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm);
this.cachedOrderedRooms.splice(startIdx, 0, ...sorted); this.cachedOrderedRooms.splice(startIdx, 0, ...sorted);
} }

View file

@ -29,14 +29,11 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
super(tagId, initialSortingAlgorithm); super(tagId, initialSortingAlgorithm);
} }
public async setRooms(rooms: Room[]): Promise<any> { public setRooms(rooms: Room[]): void {
this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); this.cachedOrderedRooms = sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm);
} }
public async handleRoomUpdate(room, cause): Promise<boolean> { public handleRoomUpdate(room, cause): boolean {
try {
await this.updateLock.acquireAsync();
const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved; const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved;
const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt; const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt;
if (!isSplice && !isInPlace) { if (!isSplice && !isInPlace) {
@ -56,15 +53,8 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
// TODO: Optimize this to avoid useless operations: https://github.com/vector-im/element-web/issues/14457 // TODO: Optimize this to avoid useless operations: https://github.com/vector-im/element-web/issues/14457
// For example, we can skip updates to alphabetic (sometimes) and manually ordered tags // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags
this.cachedOrderedRooms = await sortRoomsWithAlgorithm( this.cachedOrderedRooms = sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm);
this.cachedOrderedRooms,
this.tagId,
this.sortingAlgorithm,
);
return true; return true;
} finally {
await this.updateLock.release();
}
} }
} }

View file

@ -17,7 +17,6 @@ limitations under the License.
import { Room } from "matrix-js-sdk/src/models/room"; import { Room } from "matrix-js-sdk/src/models/room";
import { RoomUpdateCause, TagID } from "../../models"; import { RoomUpdateCause, TagID } from "../../models";
import { SortAlgorithm } from "../models"; import { SortAlgorithm } from "../models";
import AwaitLock from "await-lock";
/** /**
* Represents a list ordering algorithm. Subclasses should populate the * Represents a list ordering algorithm. Subclasses should populate the
@ -26,7 +25,6 @@ import AwaitLock from "await-lock";
export abstract class OrderingAlgorithm { export abstract class OrderingAlgorithm {
protected cachedOrderedRooms: Room[]; protected cachedOrderedRooms: Room[];
protected sortingAlgorithm: SortAlgorithm; protected sortingAlgorithm: SortAlgorithm;
protected readonly updateLock = new AwaitLock();
protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
// noinspection JSIgnoredPromiseFromCall // noinspection JSIgnoredPromiseFromCall
@ -45,21 +43,20 @@ export abstract class OrderingAlgorithm {
* @param newAlgorithm The new algorithm. Must be defined. * @param newAlgorithm The new algorithm. Must be defined.
* @returns Resolves when complete. * @returns Resolves when complete.
*/ */
public async setSortAlgorithm(newAlgorithm: SortAlgorithm) { public setSortAlgorithm(newAlgorithm: SortAlgorithm) {
if (!newAlgorithm) throw new Error("A sorting algorithm must be defined"); if (!newAlgorithm) throw new Error("A sorting algorithm must be defined");
this.sortingAlgorithm = newAlgorithm; this.sortingAlgorithm = newAlgorithm;
// Force regeneration of the rooms // Force regeneration of the rooms
await this.setRooms(this.orderedRooms); this.setRooms(this.orderedRooms);
} }
/** /**
* Sets the rooms the algorithm should be handling, implying a reconstruction * Sets the rooms the algorithm should be handling, implying a reconstruction
* of the ordering. * of the ordering.
* @param rooms The rooms to use going forward. * @param rooms The rooms to use going forward.
* @returns Resolves when complete.
*/ */
public abstract setRooms(rooms: Room[]): Promise<any>; public abstract setRooms(rooms: Room[]): void;
/** /**
* Handle a room update. The Algorithm will only call this for causes which * Handle a room update. The Algorithm will only call this for causes which
@ -69,7 +66,7 @@ export abstract class OrderingAlgorithm {
* @param cause The cause of the update. * @param cause The cause of the update.
* @returns True if the update requires the Algorithm to update the presentation layers. * @returns True if the update requires the Algorithm to update the presentation layers.
*/ */
public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean>; public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean;
protected getRoomIndex(room: Room): number { protected getRoomIndex(room: Room): number {
let roomIdx = this.cachedOrderedRooms.indexOf(room); let roomIdx = this.cachedOrderedRooms.indexOf(room);

View file

@ -23,7 +23,7 @@ import { compare } from "../../../../utils/strings";
* Sorts rooms according to the browser's determination of alphabetic. * Sorts rooms according to the browser's determination of alphabetic.
*/ */
export class AlphabeticAlgorithm implements IAlgorithm { export class AlphabeticAlgorithm implements IAlgorithm {
public async sortRooms(rooms: Room[], tagId: TagID): Promise<Room[]> { public sortRooms(rooms: Room[], tagId: TagID): Room[] {
return rooms.sort((a, b) => { return rooms.sort((a, b) => {
return compare(a.name, b.name); return compare(a.name, b.name);
}); });

View file

@ -25,7 +25,7 @@ export interface IAlgorithm {
* Sorts the given rooms according to the sorting rules of the algorithm. * Sorts the given rooms according to the sorting rules of the algorithm.
* @param {Room[]} rooms The rooms to sort. * @param {Room[]} rooms The rooms to sort.
* @param {TagID} tagId The tag ID in which the rooms are being sorted. * @param {TagID} tagId The tag ID in which the rooms are being sorted.
* @returns {Promise<Room[]>} Resolves to the sorted rooms. * @returns {Room[]} Returns the sorted rooms.
*/ */
sortRooms(rooms: Room[], tagId: TagID): Promise<Room[]>; sortRooms(rooms: Room[], tagId: TagID): Room[];
} }

View file

@ -22,7 +22,7 @@ import { IAlgorithm } from "./IAlgorithm";
* Sorts rooms according to the tag's `order` property on the room. * Sorts rooms according to the tag's `order` property on the room.
*/ */
export class ManualAlgorithm implements IAlgorithm { export class ManualAlgorithm implements IAlgorithm {
public async sortRooms(rooms: Room[], tagId: TagID): Promise<Room[]> { public sortRooms(rooms: Room[], tagId: TagID): Room[] {
const getOrderProp = (r: Room) => r.tags[tagId].order || 0; const getOrderProp = (r: Room) => r.tags[tagId].order || 0;
return rooms.sort((a, b) => { return rooms.sort((a, b) => {
return getOrderProp(a) - getOrderProp(b); return getOrderProp(a) - getOrderProp(b);

View file

@ -97,7 +97,7 @@ export const sortRooms = (rooms: Room[]): Room[] => {
* useful to the user. * useful to the user.
*/ */
export class RecentAlgorithm implements IAlgorithm { export class RecentAlgorithm implements IAlgorithm {
public async sortRooms(rooms: Room[], tagId: TagID): Promise<Room[]> { public sortRooms(rooms: Room[], tagId: TagID): Room[] {
return sortRooms(rooms); return sortRooms(rooms);
} }
} }

View file

@ -46,8 +46,8 @@ export function getSortingAlgorithmInstance(algorithm: SortAlgorithm): IAlgorith
* @param {Room[]} rooms The rooms to sort. * @param {Room[]} rooms The rooms to sort.
* @param {TagID} tagId The tag in which the sorting is occurring. * @param {TagID} tagId The tag in which the sorting is occurring.
* @param {SortAlgorithm} algorithm The algorithm to use for sorting. * @param {SortAlgorithm} algorithm The algorithm to use for sorting.
* @returns {Promise<Room[]>} Resolves to the sorted rooms. * @returns {Room[]} Returns the sorted rooms.
*/ */
export function sortRoomsWithAlgorithm(rooms: Room[], tagId: TagID, algorithm: SortAlgorithm): Promise<Room[]> { export function sortRoomsWithAlgorithm(rooms: Room[], tagId: TagID, algorithm: SortAlgorithm): Room[] {
return getSortingAlgorithmInstance(algorithm).sortRooms(rooms, tagId); return getSortingAlgorithmInstance(algorithm).sortRooms(rooms, tagId);
} }