diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index eee8e60b86..35511a461d 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -41,6 +41,17 @@ import { getListAlgorithmInstance } from "./list-ordering"; */ export const LIST_UPDATED_EVENT = "list_updated_event"; +// These are the causes which require a room to be known in order for us to handle them. If +// a cause in this list is raised and we don't know about the room, we don't handle the update. +// +// Note: these typically happen when a new room is coming in, such as the user creating or +// joining the room. For these cases, we need to know about the room prior to handling it otherwise +// we'll make bad assumptions. +const CAUSES_REQUIRING_ROOM = [ + RoomUpdateCause.Timeline, + RoomUpdateCause.ReadReceipt, +]; + interface IStickyRoom { room: Room; position: number; @@ -666,18 +677,6 @@ export class Algorithm extends EventEmitter { } } - if (hasTags && isForLastSticky && !knownRoomRef) { - // we have a fairly good chance at losing a room right now. Under some circumstances, - // we can end up with a room which transitions references and tag changes, then gets - // lost when the sticky room changes. To counter this, we try and add the room to the - // list manually as the condition below to update the reference will fail. - // - // Other conditions *should* result in the room being sorted into the right place. - console.warn(`${room.roomId} was about to be lost - inserting at end of room list`); - this.rooms.push(room); - knownRoomRef = true; - } - // If we have tags for a room and don't have the room referenced, something went horribly // wrong - the reference should have been updated above. if (hasTags && !knownRoomRef && !isSticky) { @@ -690,6 +689,13 @@ export class Algorithm extends EventEmitter { // to trigger a sticky room update ourselves. this._stickyRoom.room = room; } + + // If after all that we're still a NewRoom update, add the room if applicable. + // We don't do this for the sticky room (because it causes duplication issues) + // or if we know about the reference (as it should be replaced). + if (cause === RoomUpdateCause.NewRoom && !isSticky && !knownRoomRef) { + this.rooms.push(room); + } } if (cause === RoomUpdateCause.PossibleTagChange) { @@ -704,6 +710,7 @@ export class Algorithm extends EventEmitter { const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); + this.cachedRooms[rmTag] = algorithm.orderedRooms; } for (const addTag of diff.added) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 @@ -711,6 +718,7 @@ export class Algorithm extends EventEmitter { const algorithm: OrderingAlgorithm = this.algorithms[addTag]; if (!algorithm) throw new Error(`No algorithm for ${addTag}`); await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); + this.cachedRooms[addTag] = algorithm.orderedRooms; } // Update the tag map so we don't regen it in a moment @@ -755,6 +763,11 @@ export class Algorithm extends EventEmitter { } if (!this.roomIdsToTags[room.roomId]) { + if (CAUSES_REQUIRING_ROOM.includes(cause)) { + console.warn(`Skipping tag update for ${room.roomId} because we don't know about the room`); + return false; + } + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Updating tags for room ${room.roomId} (${room.name})`); diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index e95f92f985..3acd9f924e 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -160,7 +160,10 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) } else if (cause === RoomUpdateCause.RoomRemoved) { const roomIdx = this.getRoomIndex(room); - if (roomIdx === -1) return false; // no change + if (roomIdx === -1) { + console.warn(`Tried to remove unknown room from ${this.tagId}: ${room.roomId}`); + return false; // no change + } const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); this.alterCategoryPositionBy(oldCategory, -1, this.indices); this.cachedOrderedRooms.splice(roomIdx, 1); // remove the room @@ -169,15 +172,6 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { } } - private getRoomIndex(room: Room): number { - 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); - } - return roomIdx; - } - public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { try { await this.updateLock.acquireAsync(); diff --git a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts index f74329cb4d..849c8a2877 100644 --- a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts @@ -50,8 +50,12 @@ export class NaturalAlgorithm extends OrderingAlgorithm { if (cause === RoomUpdateCause.NewRoom) { this.cachedOrderedRooms.push(room); } else if (cause === RoomUpdateCause.RoomRemoved) { - const idx = this.cachedOrderedRooms.indexOf(room); - if (idx >= 0) this.cachedOrderedRooms.splice(idx, 1); + const idx = this.getRoomIndex(room); + if (idx >= 0) { + this.cachedOrderedRooms.splice(idx, 1); + } else { + console.warn(`Tried to remove unknown room from ${this.tagId}: ${room.roomId}`); + } } // TODO: Optimize this to avoid useless operations: https://github.com/vector-im/riot-web/issues/14035 diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts index 4ab7650367..c47a35523c 100644 --- a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -70,4 +70,13 @@ export abstract class OrderingAlgorithm { * @returns True if the update requires the Algorithm to update the presentation layers. */ public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise; + + protected getRoomIndex(room: Room): number { + 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); + } + return roomIdx; + } }