diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 36abf86975..d5f2ed0053 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -18,7 +18,7 @@ import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import DMRoomMap from "../../../utils/DMRoomMap"; import { EventEmitter } from "events"; -import { arrayHasDiff, ArrayUtil } from "../../../utils/arrays"; +import { arrayDiff, arrayHasDiff, ArrayUtil } from "../../../utils/arrays"; import { getEnumValues } from "../../../utils/enums"; import { DefaultTagID, RoomUpdateCause, TagID } from "../models"; import { @@ -57,6 +57,7 @@ export class Algorithm extends EventEmitter { private _cachedStickyRooms: ITagMap = {}; // a clone of the _cachedRooms, with the sticky room private filteredRooms: ITagMap = {}; private _stickyRoom: IStickyRoom = null; + private _lastStickyRoom: IStickyRoom = null; // only not-null when changing the sticky room private sortAlgorithms: ITagSortingMap; private listAlgorithms: IListOrderingMap; private algorithms: IOrderingAlgorithmMap; @@ -162,9 +163,21 @@ export class Algorithm extends EventEmitter { } private async updateStickyRoom(val: Room) { + try { + return await this.doUpdateStickyRoom(val); + } finally { + this._lastStickyRoom = null; // clear to indicate we're done changing + } + } + + private async doUpdateStickyRoom(val: Room) { // Note throughout: We need async so we can wait for handleRoomUpdate() to do its thing, // otherwise we risk duplicating rooms. + // Set the last sticky room to indicate that we're in a change. The code throughout the + // class can safely handle a null room, so this should be safe to do as a backup. + this._lastStickyRoom = this._stickyRoom || {}; + // It's possible to have no selected room. In that case, clear the sticky room if (!val) { if (this._stickyRoom) { @@ -179,7 +192,7 @@ export class Algorithm extends EventEmitter { } // When we do have a room though, we expect to be able to find it - const tag = this.roomIdsToTags[val.roomId][0]; + let tag = this.roomIdsToTags[val.roomId][0]; if (!tag) throw new Error(`${val.roomId} does not belong to a tag and cannot be sticky`); // We specifically do NOT use the ordered rooms set as it contains the sticky room, which @@ -196,19 +209,41 @@ export class Algorithm extends EventEmitter { // the same thing it no-ops. After we're done calling the algorithm, we'll issue // a new update for ourselves. const lastStickyRoom = this._stickyRoom; - this._stickyRoom = null; + this._stickyRoom = null; // clear before we update the algorithm this.recalculateStickyRoom(); // When we do have the room, re-add the old room (if needed) to the algorithm // and remove the sticky room from the algorithm. This is so the underlying // algorithm doesn't try and confuse itself with the sticky room concept. - if (lastStickyRoom) { + // We don't add the new room if the sticky room isn't changing because that's + // an easy way to cause duplication. We have to do room ID checks instead of + // referential checks as the references can differ through the lifecycle. + if (lastStickyRoom && lastStickyRoom.room && lastStickyRoom.room.roomId !== val.roomId) { // Lie to the algorithm and re-add the room to the algorithm await this.handleRoomUpdate(lastStickyRoom.room, RoomUpdateCause.NewRoom); } // Lie to the algorithm and remove the room from it's field of view await this.handleRoomUpdate(val, RoomUpdateCause.RoomRemoved); + // Check for tag & position changes while we're here. We also check the room to ensure + // it is still the same room. + if (this._stickyRoom) { + if (this._stickyRoom.room !== val) { + // Check the room IDs just in case + if (this._stickyRoom.room.roomId === val.roomId) { + console.warn("Sticky room changed references"); + } else { + throw new Error("Sticky room changed while the sticky room was changing"); + } + } + + console.warn(`Sticky room changed tag & position from ${tag} / ${position} ` + + `to ${this._stickyRoom.tag} / ${this._stickyRoom.position}`); + + tag = this._stickyRoom.tag; + position = this._stickyRoom.position; + } + // Now that we're done lying to the algorithm, we need to update our position // marker only if the user is moving further down the same list. If they're switching // lists, or moving upwards, the position marker will splice in just fine but if @@ -560,7 +595,7 @@ export class Algorithm extends EventEmitter { /** * Updates the roomsToTags map */ - protected updateTagsFromCache() { + private updateTagsFromCache() { const newMap = {}; const tags = Object.keys(this.cachedRooms); @@ -607,21 +642,94 @@ export class Algorithm extends EventEmitter { * processing. */ public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Handle room update for ${room.roomId} called with cause ${cause}`); 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 + const isSticky = this._stickyRoom && this._stickyRoom.room && this._stickyRoom.room.roomId === room.roomId; if (cause === RoomUpdateCause.NewRoom) { + const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; const roomTags = this.roomIdsToTags[room.roomId]; - if (roomTags && roomTags.length > 0) { + const hasTags = roomTags && roomTags.length > 0; + + // Don't change the cause if the last sticky room is being re-added. If we fail to + // pass the cause through as NewRoom, we'll fail to lie to the algorithm and thus + // lose the room. + if (hasTags && !isForLastSticky) { console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); cause = RoomUpdateCause.PossibleTagChange; } + + // If we have tags for a room and don't have the room referenced, the room reference + // probably changed. We need to swap out the problematic reference. + if (hasTags && !this.rooms.includes(room) && !isSticky) { + console.warn(`${room.roomId} is missing from room array but is known - trying to find duplicate`); + this.rooms = this.rooms.map(r => r.roomId === room.roomId ? room : r); + + // Sanity check + if (!this.rooms.includes(room)) { + throw new Error(`Failed to replace ${room.roomId} with an updated reference`); + } + } + + // Like above, update the reference to the sticky room if we need to + if (hasTags && isSticky) { + // Go directly in and set the sticky room's new reference, being careful not + // to trigger a sticky room update ourselves. + this._stickyRoom.room = room; + } } if (cause === RoomUpdateCause.PossibleTagChange) { - // TODO: Be smarter and splice rather than regen the planet. https://github.com/vector-im/riot-web/issues/14035 - // TODO: No-op if no change. https://github.com/vector-im/riot-web/issues/14035 - await this.setKnownRooms(this.rooms); - return true; + let didTagChange = false; + const oldTags = this.roomIdsToTags[room.roomId] || []; + const newTags = this.getTagsForRoom(room); + const diff = arrayDiff(oldTags, newTags); + if (diff.removed.length > 0 || diff.added.length > 0) { + for (const rmTag of diff.removed) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Removing ${room.roomId} from ${rmTag}`); + const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; + if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); + await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); + } + for (const addTag of diff.added) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Adding ${room.roomId} to ${addTag}`); + const algorithm: OrderingAlgorithm = this.algorithms[addTag]; + if (!algorithm) throw new Error(`No algorithm for ${addTag}`); + await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); + } + + // Update the tag map so we don't regen it in a moment + this.roomIdsToTags[room.roomId] = newTags; + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Changing update cause for ${room.roomId} to Timeline to sort rooms`); + cause = RoomUpdateCause.Timeline; + didTagChange = true; + } else { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.warn(`Received no-op update for ${room.roomId} - changing to Timeline update`); + cause = RoomUpdateCause.Timeline; + } + + if (didTagChange && isSticky) { + // Manually update the tag for the sticky room without triggering a sticky room + // update. The update will be handled implicitly by the sticky room handling and + // requires no changes on our part, if we're in the middle of a sticky room change. + if (this._lastStickyRoom) { + this._stickyRoom = { + room, + tag: this.roomIdsToTags[room.roomId][0], + position: 0, // right at the top as it changed tags + }; + } else { + // We have to clear the lock as the sticky room change will trigger updates. + await this.setStickyRoomAsync(room); + } + } } // If the update is for a room change which might be the sticky room, prevent it. We @@ -635,8 +743,9 @@ export class Algorithm extends EventEmitter { } } - if (cause === RoomUpdateCause.NewRoom && !this.roomIdsToTags[room.roomId]) { - console.log(`[RoomListDebug] Updating tags for new room ${room.roomId} (${room.name})`); + if (!this.roomIdsToTags[room.roomId]) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Updating tags for room ${room.roomId} (${room.name})`); // Get the tags for the room and populate the cache const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); @@ -646,9 +755,15 @@ export class Algorithm extends EventEmitter { if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); this.roomIdsToTags[room.roomId] = roomTags; + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Updated tags for ${room.roomId}:`, roomTags); } - let tags = this.roomIdsToTags[room.roomId]; + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Reached algorithmic handling for ${room.roomId} and cause ${cause}`); + + const tags = this.roomIdsToTags[room.roomId]; if (!tags) { console.warn(`No tags known for "${room.name}" (${room.roomId})`); return false; @@ -668,6 +783,8 @@ export class Algorithm extends EventEmitter { changed = true; } - return true; + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Finished handling ${room.roomId} with cause ${cause} (changed=${changed})`); + return changed; } }