From c7a83e65f0487964c8597699951525cf28d17766 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 24 Jun 2020 21:31:44 -0600 Subject: [PATCH] Fix proliferation when joining upgraded rooms We have to do a bit of a dance to return the sticky room to the list so we can remove it, if needed, and ensure that we generally swap the rooms out of the list. --- src/stores/room-list/RoomListStore2.ts | 43 +++++++++--- src/stores/room-list/algorithms/Algorithm.ts | 74 +++++++++++++++++--- 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 58a78f4dd8..b4b643687d 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -97,8 +97,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 +189,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); }; @@ -245,7 +250,29 @@ export class RoomListStore2 extends AsyncStore { if (membershipPayload.oldMembership !== "join" && membershipPayload.membership === "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; } @@ -253,7 +280,7 @@ export class RoomListStore2 extends AsyncStore { if (membershipPayload.oldMembership !== membershipPayload.membership) { // 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..4fbc576dfa 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 @@ -458,14 +471,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) { @@ -496,6 +502,39 @@ export class Algorithm extends EventEmitter { this.updateTagsFromCache(); } + 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; + } + /** * Updates the roomsToTags map */ @@ -566,6 +605,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})`);