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

@@ -97,8 +97,10 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
             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) {
-            if (activeRoom !== this.algorithm.stickyRoom) {
+                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<ActionPayload> {
                 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`);
+                    console.log(`[RoomListDebug] Got tombstone event - trying to remove now-dead room`);
-                    // TODO: We could probably be smarter about this: https://github.com/vector-im/riot-web/issues/14035
+                    const newRoom = this.matrixClient.getRoom(eventPayload.event.getContent()['replacement_room']);
-                    await this.regenerateAllLists();
+                    if (newRoom) {
-                    return; // don't pass the update down - we will have already handled it in the regen
+                        // 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<ActionPayload> {
             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<ActionPayload> {
             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;
             }
         }

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 || {});
+            const tags = this.getTagsOfJoinedRoom(room);
-
-            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];
-                }
-            }
 
             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})`);