diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 58a78f4dd8..497b8e5530 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -29,6 +29,7 @@ import { IFilterCondition } from "./filters/IFilterCondition"; import { TagWatcher } from "./TagWatcher"; import RoomViewStore from "../RoomViewStore"; import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm"; +import { EffectiveMembership, getEffectiveMembership } from "./membership"; interface IState { tagsEnabled?: boolean; @@ -97,8 +98,10 @@ export class RoomListStore2 extends AsyncStore { this.algorithm.stickyRoom = null; } else if (activeRoomId) { const activeRoom = this.matrixClient.getRoom(activeRoomId); - if (!activeRoom) throw new Error(`${activeRoomId} is current in RVS but missing from client`); - if (activeRoom !== this.algorithm.stickyRoom) { + if (!activeRoom) { + console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`); + this.algorithm.stickyRoom = null; + } else if (activeRoom !== this.algorithm.stickyRoom) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`Changing sticky room to ${activeRoomId}`); this.algorithm.stickyRoom = activeRoom; @@ -187,10 +190,13 @@ export class RoomListStore2 extends AsyncStore { console.log(`[RoomListDebug] Live timeline event ${eventPayload.event.getId()} in ${updatedRoom.roomId}`); if (eventPayload.event.getType() === 'm.room.tombstone' && eventPayload.event.getStateKey() === '') { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Got tombstone event - regenerating room list`); - // TODO: We could probably be smarter about this: https://github.com/vector-im/riot-web/issues/14035 - await this.regenerateAllLists(); - return; // don't pass the update down - we will have already handled it in the regen + console.log(`[RoomListDebug] Got tombstone event - trying to remove now-dead room`); + const newRoom = this.matrixClient.getRoom(eventPayload.event.getContent()['replacement_room']); + if (newRoom) { + // If we have the new room, then the new room check will have seen the predecessor + // and did the required updates, so do nothing here. + return; + } } await this.handleRoomUpdate(updatedRoom, RoomUpdateCause.Timeline); }; @@ -242,18 +248,49 @@ export class RoomListStore2 extends AsyncStore { } } else if (payload.action === 'MatrixActions.Room.myMembership') { const membershipPayload = (payload); // TODO: Type out the dispatcher types - if (membershipPayload.oldMembership !== "join" && membershipPayload.membership === "join") { + const oldMembership = getEffectiveMembership(membershipPayload.oldMembership); + const newMembership = getEffectiveMembership(membershipPayload.membership); + if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Handling new room ${membershipPayload.room.roomId}`); - await this.algorithm.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); + + // If we're joining an upgraded room, we'll want to make sure we don't proliferate + // the dead room in the list. + const createEvent = membershipPayload.room.currentState.getStateEvents("m.room.create", ""); + if (createEvent && createEvent.getContent()['predecessor']) { + console.log(`[RoomListDebug] Room has a predecessor`); + const prevRoom = this.matrixClient.getRoom(createEvent.getContent()['predecessor']['room_id']); + if (prevRoom) { + const isSticky = this.algorithm.stickyRoom === prevRoom; + if (isSticky) { + console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`); + await this.algorithm.setStickyRoomAsync(null); + } + + // Note: we hit the algorithm instead of our handleRoomUpdate() function to + // avoid redundant updates. + console.log(`[RoomListDebug] Removing previous room from room list`); + await this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved); + } + } + + console.log(`[RoomListDebug] Adding new room to room list`); + await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); + return; + } + + if (oldMembership !== EffectiveMembership.Invite && newMembership === EffectiveMembership.Invite) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Handling invite to ${membershipPayload.room.roomId}`); + await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); return; } // If it's not a join, it's transitioning into a different list (possibly historical) - if (membershipPayload.oldMembership !== membershipPayload.membership) { + if (oldMembership !== newMembership) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Handling membership change in ${membershipPayload.room.roomId}`); - await this.algorithm.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange); + await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange); return; } } diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 37ce7e4ba7..8215d2ef57 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -30,7 +30,7 @@ import { SortAlgorithm } from "./models"; import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../filters/IFilterCondition"; -import { EffectiveMembership, splitRoomsByMembership } from "../membership"; +import { EffectiveMembership, getEffectiveMembership, splitRoomsByMembership } from "../membership"; import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; import { getListAlgorithmInstance } from "./list-ordering"; @@ -99,6 +99,14 @@ export class Algorithm extends EventEmitter { return this._cachedRooms; } + /** + * Awaitable version of the sticky room setter. + * @param val The new room to sticky. + */ + public async setStickyRoomAsync(val: Room) { + await this.updateStickyRoom(val); + } + public getTagSorting(tagId: TagID): SortAlgorithm { return this.sortAlgorithms[tagId]; } @@ -160,10 +168,13 @@ export class Algorithm extends EventEmitter { // It's possible to have no selected room. In that case, clear the sticky room if (!val) { if (this._stickyRoom) { + const stickyRoom = this._stickyRoom.room; + this._stickyRoom = null; // clear before we go to update the algorithm + // Lie to the algorithm and re-add the room to the algorithm - await this.handleRoomUpdate(this._stickyRoom.room, RoomUpdateCause.NewRoom); + await this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom); + return; } - this._stickyRoom = null; return; } @@ -289,6 +300,8 @@ export class Algorithm extends EventEmitter { } protected recalculateFilteredRoomsForTag(tagId: TagID): void { + if (!this.hasFilters) return; // don't bother doing work if there's nothing to do + console.log(`Recalculating filtered rooms for ${tagId}`); delete this.filteredRooms[tagId]; const rooms = this.cachedRooms[tagId].map(r => r); // cheap clone @@ -428,6 +441,13 @@ export class Algorithm extends EventEmitter { 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`); + console.warn("Resetting known rooms, initiating regeneration"); + + // Before we go any further we need to clear (but remember) the sticky room to + // avoid accidentally duplicating it in the list. + const oldStickyRoom = this._stickyRoom; + await this.updateStickyRoom(null); + this.rooms = rooms; const newTags: ITagMap = {}; @@ -458,14 +478,7 @@ export class Algorithm extends EventEmitter { // Now process all the joined rooms. This is a bit more complicated for (const room of memberships[EffectiveMembership.Join]) { - let tags = Object.keys(room.tags || {}); - - if (tags.length === 0) { - // Check to see if it's a DM if it isn't anything else - if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { - tags = [DefaultTagID.DM]; - } - } + const tags = this.getTagsOfJoinedRoom(room); let inTag = false; if (tags.length > 0) { @@ -494,6 +507,54 @@ export class Algorithm extends EventEmitter { this.cachedRooms = newTags; this.updateTagsFromCache(); + this.recalculateFilteredRooms(); + + // Now that we've finished generation, we need to update the sticky room to what + // 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. + if (oldStickyRoom && oldStickyRoom.room) { + await this.updateStickyRoom(oldStickyRoom.room); + if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan + 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. + this._stickyRoom.position = 0; + this.recalculateStickyRoom(this._stickyRoom.tag); + } + } + } + } + + private getTagsForRoom(room: Room): TagID[] { + // XXX: This duplicates a lot of logic from setKnownRooms above, but has a slightly + // different use case and therefore different performance curve + + const tags: TagID[] = []; + + const membership = getEffectiveMembership(room.getMyMembership()); + if (membership === EffectiveMembership.Invite) { + tags.push(DefaultTagID.Invite); + } else if (membership === EffectiveMembership.Leave) { + tags.push(DefaultTagID.Archived); + } else { + tags.push(...this.getTagsOfJoinedRoom(room)); + } + + if (!tags.length) tags.push(DefaultTagID.Untagged); + + return tags; + } + + private getTagsOfJoinedRoom(room: Room): TagID[] { + let tags = Object.keys(room.tags || {}); + + if (tags.length === 0) { + // Check to see if it's a DM if it isn't anything else + if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { + tags = [DefaultTagID.DM]; + } + } + + return tags; } /** @@ -548,6 +609,14 @@ export class Algorithm extends EventEmitter { public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + if (cause === RoomUpdateCause.NewRoom) { + const roomTags = this.roomIdsToTags[room.roomId]; + if (roomTags && roomTags.length > 0) { + console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); + cause = RoomUpdateCause.PossibleTagChange; + } + } + 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 @@ -566,6 +635,19 @@ 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})`); + + // Get the tags for the room and populate the cache + const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); + + // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), + // which means we should *always* have a tag to go off of. + if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); + + this.roomIdsToTags[room.roomId] = roomTags; + } + let tags = this.roomIdsToTags[room.roomId]; if (!tags) { console.warn(`No tags known for "${room.name}" (${room.roomId})`); diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index e2a9fc1952..e95f92f985 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -179,45 +179,51 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { } public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { - if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) { - return this.handleSplice(room, cause); + try { + await this.updateLock.acquireAsync(); + + if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) { + return this.handleSplice(room, cause); + } + + if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) { + throw new Error(`Unsupported update cause: ${cause}`); + } + + const category = this.getRoomCategory(room); + if (this.sortingAlgorithm === SortAlgorithm.Manual) { + return; // Nothing to do here. + } + + const roomIdx = this.getRoomIndex(room); + if (roomIdx === -1) { + throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`); + } + + // Try to avoid doing array operations if we don't have to: only move rooms within + // the categories if we're jumping categories + const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); + if (oldCategory !== category) { + // Move the room and update the indices + this.moveRoomIndexes(1, oldCategory, category, this.indices); + this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) + this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) + // Note: if moveRoomIndexes() is called after the splice then the insert operation + // will happen in the wrong place. Because we would have already adjusted the index + // for the category, we don't need to determine how the room is moving in the list. + // If we instead tried to insert before updating the indices, we'd have to determine + // whether the room was moving later (towards IDLE) or earlier (towards RED) from its + // current position, as it'll affect the category's start index after we remove the + // room from the array. + } + + // Sort the category now that we've dumped the room in + await this.sortCategory(category); + + return true; // change made + } finally { + await this.updateLock.release(); } - - if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) { - throw new Error(`Unsupported update cause: ${cause}`); - } - - const category = this.getRoomCategory(room); - if (this.sortingAlgorithm === SortAlgorithm.Manual) { - return; // Nothing to do here. - } - - const roomIdx = this.getRoomIndex(room); - if (roomIdx === -1) { - throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`); - } - - // Try to avoid doing array operations if we don't have to: only move rooms within - // the categories if we're jumping categories - const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); - if (oldCategory !== category) { - // Move the room and update the indices - this.moveRoomIndexes(1, oldCategory, category, this.indices); - this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) - this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) - // Note: if moveRoomIndexes() is called after the splice then the insert operation - // will happen in the wrong place. Because we would have already adjusted the index - // for the category, we don't need to determine how the room is moving in the list. - // If we instead tried to insert before updating the indices, we'd have to determine - // whether the room was moving later (towards IDLE) or earlier (towards RED) from its - // current position, as it'll affect the category's start index after we remove the - // room from the array. - } - - // Sort the category now that we've dumped the room in - await this.sortCategory(category); - - return true; // change made } private async sortCategory(category: Category) { diff --git a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts index 79fa2ed604..f74329cb4d 100644 --- a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts @@ -38,23 +38,29 @@ export class NaturalAlgorithm extends OrderingAlgorithm { } public async handleRoomUpdate(room, cause): Promise { - const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved; - const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt; - if (!isSplice && !isInPlace) { - throw new Error(`Unsupported update cause: ${cause}`); + try { + await this.updateLock.acquireAsync(); + + const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved; + const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt; + if (!isSplice && !isInPlace) { + throw new Error(`Unsupported update cause: ${cause}`); + } + + 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); + } + + // TODO: Optimize this to avoid useless operations: https://github.com/vector-im/riot-web/issues/14035 + // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm); + + return true; + } finally { + await this.updateLock.release(); } - - 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); - } - - // TODO: Optimize this to avoid useless operations: https://github.com/vector-im/riot-web/issues/14035 - // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags - this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm); - - return true; } } diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts index f581e30630..4ab7650367 100644 --- a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -17,6 +17,7 @@ limitations under the License. import { Room } from "matrix-js-sdk/src/models/room"; import { RoomUpdateCause, TagID } from "../../models"; import { SortAlgorithm } from "../models"; +import AwaitLock from "await-lock"; /** * Represents a list ordering algorithm. Subclasses should populate the @@ -25,6 +26,7 @@ import { SortAlgorithm } from "../models"; export abstract class OrderingAlgorithm { protected cachedOrderedRooms: Room[]; protected sortingAlgorithm: SortAlgorithm; + protected readonly updateLock = new AwaitLock(); protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { // noinspection JSIgnoredPromiseFromCall