Shorten TimelineWindow when an event is removed (#3862)

* Shorten TimelineWindow when an event is removed

Needed for the fix for https://github.com/vector-im/element-web/issues/26498

* Declare onTimelineEvent as a standard method to match surrounding code

spec/test-utils/test-utils.ts

@@ -315,6 +315,7 @@ export interface IMessageOpts {
     event?: boolean;
     relatesTo?: IEventRelation;
     ts?: number;
+    unsigned?: IUnsigned;
 }
 
 /**

spec/unit/timeline-window.spec.ts

@@ -22,12 +22,15 @@ import { Room } from "../../src/models/room";
 import { EventTimeline } from "../../src/models/event-timeline";
 import { TimelineIndex, TimelineWindow } from "../../src/timeline-window";
 import { mkMessage } from "../test-utils/test-utils";
+import { MatrixEvent } from "../../src/models/event";
 
 const ROOM_ID = "roomId";
 const USER_ID = "userId";
 const mockClient = {
     getEventTimeline: jest.fn(),
     paginateEventTimeline: jest.fn(),
+    supportsThreads: jest.fn(),
+    getUserId: jest.fn().mockReturnValue(USER_ID),
 } as unknown as MockedObject<MatrixClient>;
 
 /*
@@ -64,6 +67,23 @@ function addEventsToTimeline(timeline: EventTimeline, numEvents: number, toStart
     }
 }
 
+function createEvents(numEvents: number): Array<MatrixEvent> {
+    const ret = [];
+
+    for (let i = 0; i < numEvents; i++) {
+        ret.push(
+            mkMessage({
+                room: ROOM_ID,
+                user: USER_ID,
+                event: true,
+                unsigned: { age: 1 },
+            }),
+        );
+    }
+
+    return ret;
+}
+
 /*
  * create a pair of linked timelines
  */
@@ -412,4 +432,46 @@ describe("TimelineWindow", function () {
             expect(timelineWindow.canPaginate(EventTimeline.FORWARDS)).toBe(true);
         });
     });
+
+    function idsOf(events: Array<MatrixEvent>): Array<string> {
+        return events.map((e) => (e ? e.getId() ?? "MISSING_ID" : "MISSING_EVENT"));
+    }
+
+    describe("removing events", () => {
+        it("should shorten if removing an event within the window makes it overflow", function () {
+            // Given a room with events in two timelines
+            const room = new Room(ROOM_ID, mockClient, USER_ID, { timelineSupport: true });
+            const timelineSet = room.getUnfilteredTimelineSet();
+            const liveTimeline = room.getLiveTimeline();
+            const oldTimeline = room.addTimeline();
+            liveTimeline.setNeighbouringTimeline(oldTimeline, EventTimeline.BACKWARDS);
+            oldTimeline.setNeighbouringTimeline(liveTimeline, EventTimeline.FORWARDS);
+
+            const oldEvents = createEvents(5);
+            const liveEvents = createEvents(5);
+            const [, , e3, e4, e5] = oldEvents;
+            const [, e7, e8, e9, e10] = liveEvents;
+            room.addLiveEvents(liveEvents);
+            room.addEventsToTimeline(oldEvents, true, oldTimeline);
+
+            // And 2 windows over the timelines in this room
+            const oldWindow = new TimelineWindow(mockClient, timelineSet);
+            oldWindow.load(e5.getId(), 6);
+            expect(idsOf(oldWindow.getEvents())).toEqual(idsOf([e5, e4, e3]));
+
+            const newWindow = new TimelineWindow(mockClient, timelineSet);
+            newWindow.load(e9.getId(), 4);
+            expect(idsOf(newWindow.getEvents())).toEqual(idsOf([e7, e8, e9, e10]));
+
+            // When I remove an event
+            room.removeEvent(e8.getId()!);
+
+            // Then the affected timeline is shortened (because it would have
+            // been too long with the removed event gone)
+            expect(idsOf(newWindow.getEvents())).toEqual(idsOf([e7, e9, e10]));
+
+            // And the unaffected one is not
+            expect(idsOf(oldWindow.getEvents())).toEqual(idsOf([e5, e4, e3]));
+        });
+    });
 });

src/timeline-window.ts

@@ -21,6 +21,7 @@ import { logger } from "./logger";
 import { MatrixClient } from "./client";
 import { EventTimelineSet } from "./models/event-timeline-set";
 import { MatrixEvent } from "./models/event";
+import { Room, RoomEvent } from "./models/room";
 
 /**
  * @internal
@@ -74,6 +75,10 @@ export class TimelineWindow {
      * are received from /sync; you should arrange to call {@link TimelineWindow#paginate}
      * on {@link RoomEvent.Timeline} events.
      *
+     * <p>Note that constructing an instance of this class for a room adds a
+     * listener for RoomEvent.Timeline events which is never removed. In theory
+     * this should not cause a leak since the EventEmitter uses weak mappings.
+     *
      * @param client -   MatrixClient to be used for context/pagination
      *   requests.
      *
@@ -87,6 +92,7 @@ export class TimelineWindow {
         opts: IOpts = {},
     ) {
         this.windowLimit = opts.windowLimit || 1000;
+        timelineSet.room?.on(RoomEvent.Timeline, this.onTimelineEvent.bind(this));
     }
 
     /**
@@ -193,6 +199,23 @@ export class TimelineWindow {
         return false;
     }
 
+    private onTimelineEvent(_event?: MatrixEvent, _room?: Room, _atStart?: boolean, removed?: boolean): void {
+        if (removed) {
+            this.onEventRemoved();
+        }
+    }
+
+    /**
+     * If an event was removed, meaning this window is longer than the timeline,
+     * shorten the window.
+     */
+    private onEventRemoved(): void {
+        const events = this.getEvents();
+        if (events.length > 0 && events[events.length - 1] === undefined && this.end) {
+            this.end.index--;
+        }
+    }
+
     /**
      * Check if this window can be extended
      *