Fix pagination token tracking for mixed room timelines (#2855)

matrix-org · Nov 5, 2022 · 52932f5 · 52932f5
1 parent 4f00566
commit 52932f5
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 11 deletions.
diff --git a/spec/unit/event-timeline.spec.ts b/spec/unit/event-timeline.spec.ts
@@ -1,15 +1,13 @@
 import { mocked } from 'jest-mock';
 
 import * as utils from "../test-utils/test-utils";
-import { EventTimeline } from "../../src/models/event-timeline";
+import { Direction, EventTimeline } from "../../src/models/event-timeline";
 import { RoomState } from "../../src/models/room-state";
 import { MatrixClient } from "../../src/matrix";
 import { Room } from "../../src/models/room";
 import { RoomMember } from "../../src/models/room-member";
 import { EventTimelineSet } from "../../src/models/event-timeline-set";
 
-jest.mock("../../src/models/room-state");
-
 describe("EventTimeline", function() {
     const roomId = "!foo:bar";
     const userA = "@alice:bar";
@@ -23,7 +21,14 @@ describe("EventTimeline", function() {
         const timelineSet = new EventTimelineSet(room);
         jest.spyOn(room, 'getUnfilteredTimelineSet').mockReturnValue(timelineSet);
 
-        return new EventTimeline(timelineSet);
+        const timeline = new EventTimeline(timelineSet);
+        // We manually stub the methods we'll be mocking out later instead of mocking the whole module
+        // otherwise the default member property values (e.g. paginationToken) will be incorrect
+        timeline.getState(Direction.Backward)!.setStateEvents = jest.fn();
+        timeline.getState(Direction.Backward)!.getSentinelMember = jest.fn();
+        timeline.getState(Direction.Forward)!.setStateEvents = jest.fn();
+        timeline.getState(Direction.Forward)!.getSentinelMember = jest.fn();
+        return timeline;
     };
 
     beforeEach(function() {
@@ -98,7 +103,17 @@ describe("EventTimeline", function() {
             expect(timeline.getPaginationToken(EventTimeline.FORWARDS)).toBe(null);
         });
 
-        it("setPaginationToken should set  token", function() {
+        it("setPaginationToken should set token", function() {
+            timeline.setPaginationToken("back", EventTimeline.BACKWARDS);
+            timeline.setPaginationToken("fwd", EventTimeline.FORWARDS);
+            expect(timeline.getPaginationToken(EventTimeline.BACKWARDS)).toEqual("back");
+            expect(timeline.getPaginationToken(EventTimeline.FORWARDS)).toEqual("fwd");
+        });
+
+        it("should be able to store pagination tokens for mixed room timelines", () => {
+            const timelineSet = new EventTimelineSet(undefined);
+            const timeline = new EventTimeline(timelineSet);
+
             timeline.setPaginationToken("back", EventTimeline.BACKWARDS);
             timeline.setPaginationToken("fwd", EventTimeline.FORWARDS);
             expect(timeline.getPaginationToken(EventTimeline.BACKWARDS)).toEqual("back");

diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts
@@ -95,8 +95,14 @@ export class EventTimeline {
     private readonly name: string;
     private events: MatrixEvent[] = [];
     private baseIndex = 0;
+
     private startState?: RoomState;
     private endState?: RoomState;
+    // If we have a roomId then we delegate pagination token storage to the room state objects `startState` and
+    // `endState`, but for things like the notification timeline which mix multiple rooms we store the tokens ourselves.
+    private startToken: string | null = null;
+    private endToken: string | null = null;
+
     private prevTimeline: EventTimeline | null = null;
     private nextTimeline: EventTimeline | null = null;
     public paginationRequests: Record<Direction, Promise<boolean> | null> = {
@@ -128,9 +134,7 @@ export class EventTimeline {
         this.roomId = eventTimelineSet.room?.roomId ?? null;
         if (this.roomId) {
             this.startState = new RoomState(this.roomId);
-            this.startState.paginationToken = null;
             this.endState = new RoomState(this.roomId);
-            this.endState.paginationToken = null;
         }
 
         // this is used by client.js
@@ -278,7 +282,13 @@ export class EventTimeline {
      * @return {?string} pagination token
      */
     public getPaginationToken(direction: Direction): string | null {
-        return this.getState(direction)?.paginationToken ?? null;
+        if (this.roomId) {
+            return this.getState(direction)!.paginationToken;
+        } else if (direction === Direction.Backward) {
+            return this.startToken;
+        } else {
+            return this.endToken;
+        }
     }
 
     /**
@@ -291,9 +301,12 @@ export class EventTimeline {
      *   pagination token for going forwards in time.
      */
     public setPaginationToken(token: string | null, direction: Direction): void {
-        const state = this.getState(direction);
-        if (state) {
-            state.paginationToken = token;
+        if (this.roomId) {
+            this.getState(direction)!.paginationToken = token;
+        } else if (direction === Direction.Backward) {
+            this.startToken = token;
+        } else {
+            this.endToken = token;
         }
     }