Fix getLatestTimeline not working when the latest event in the room is a threaded message

.gitignore

@@ -6,6 +6,7 @@ node_modules
 /*.log
 package-lock.json
 .lock-wscript
+.DS_Store
 build/Release
 coverage
 lib-cov

spec/integ/matrix-client-event-timeline.spec.ts

@@ -627,7 +627,7 @@ describe("MatrixClient event timelines", function() {
             expect(timeline.getEvents().find(e => e.getId() === THREAD_ROOT.event_id)).toBeTruthy();
         });
 
-        it("should return undefined when event is not in the thread that the given timelineSet is representing", () => {
+        it("should not include main timeline event when timelineSet is representing a thread", async () => {
             // @ts-ignore
             client.clientOpts.experimentalThreadSupport = true;
             Thread.setServerSideSupport(true, false);
@@ -649,13 +649,40 @@ describe("MatrixClient event timelines", function() {
                     };
                 });
 
-            return Promise.all([
-                expect(client.getEventTimeline(timelineSet, EVENTS[0].event_id)).resolves.toBeUndefined(),
-                httpBackend.flushAllExpected(),
-            ]);
+            // getEventTimeline -> thread.fetchInitialEvents
+            httpBackend.when("GET", "/rooms/!foo%3Abar/relations/" +
+                encodeURIComponent(THREAD_ROOT.event_id) + "/" +
+                encodeURIComponent(THREAD_RELATION_TYPE.name) + "?limit=20&direction=b")
+                .respond(200, function() {
+                    return {
+                        original_event: THREAD_ROOT,
+                        chunk: [THREAD_REPLY],
+                        // no next batch as this is the oldest end of the timeline
+                    };
+                });
+
+            // getEventTimeline -> thread.fetchEvents
+            httpBackend.when("GET", "/rooms/!foo%3Abar/relations/" +
+                encodeURIComponent(THREAD_ROOT.event_id) + "/" +
+                encodeURIComponent(THREAD_RELATION_TYPE.name) + "?direction=b&limit=50")
+                .respond(200, function() {
+                    return {
+                        original_event: THREAD_ROOT,
+                        chunk: [THREAD_REPLY],
+                        // no next batch as this is the oldest end of the timeline
+                    };
+                });
+
+            const timelinePromise = client.getEventTimeline(timelineSet, EVENTS[0].event_id);
+            await httpBackend.flushAllExpected();
+
+            const timeline = await timelinePromise;
+
+            // The main timeline event should not be in the timelineSet representing a thread
+            expect(timeline.getEvents().find(e => e.getId() === EVENTS[0].event_id)).toBeFalsy();
         });
 
-        it("should return undefined when event is within a thread but timelineSet is not", () => {
+        it("should not include threaded reply when timelineSet is representing the main room", async () => {
             // @ts-ignore
             client.clientOpts.experimentalThreadSupport = true;
             Thread.setServerSideSupport(true, false);
@@ -675,10 +702,13 @@ describe("MatrixClient event timelines", function() {
                     };
                 });
 
-            return Promise.all([
-                expect(client.getEventTimeline(timelineSet, THREAD_REPLY.event_id)).resolves.toBeUndefined(),
-                httpBackend.flushAllExpected(),
-            ]);
+            const timelinePromise = client.getEventTimeline(timelineSet, THREAD_REPLY.event_id);
+            await httpBackend.flushAllExpected();
+
+            const timeline = await timelinePromise;
+
+            // The threaded reply should not be in a main room timeline
+            expect(timeline.getEvents().find(e => e.getId() === THREAD_REPLY.event_id)).toBeFalsy();
         });
 
         it("should should add lazy loading filter when requested", async () => {

spec/integ/matrix-client-room-timeline.spec.js

@@ -688,7 +688,7 @@ describe("MatrixClient room timelines", function() {
                 });
 
             // Wait for the timeline to reset(when it goes blank) which means
-            // it's in the middle of the refrsh logic right before the
+            // it's in the middle of the refresh logic right before the
             // `getEventTimeline()` -> `/context`. Then simulate a racey `/sync`
             // to happen in the middle of all of this refresh timeline logic. We
             // want to make sure the sync pagination still works as expected

spec/unit/event-timeline-set.spec.ts

@@ -54,6 +54,23 @@ describe('EventTimelineSet', () => {
         });
     };
 
+    const mkThreadResponse = (root: MatrixEvent) => utils.mkEvent({
+        event: true,
+        type: EventType.RoomMessage,
+        user: userA,
+        room: roomId,
+        content: {
+            "body": "Thread response :: " + Math.random(),
+            "m.relates_to": {
+                "event_id": root.getId(),
+                "m.in_reply_to": {
+                    "event_id": root.getId(),
+                },
+                "rel_type": "m.thread",
+            },
+        },
+    }, room.client);
+
     beforeEach(() => {
         client = utils.mock(MatrixClient, 'MatrixClient');
         client.reEmitter = utils.mock(ReEmitter, 'ReEmitter');
@@ -116,6 +133,13 @@ describe('EventTimelineSet', () => {
     });
 
     describe('addEventToTimeline', () => {
+        let thread: Thread;
+
+        beforeEach(() => {
+            (client.supportsExperimentalThreads as jest.Mock).mockReturnValue(true);
+            thread = new Thread("!thread_id:server", messageEvent, { room, client });
+        });
+
         it("Adds event to timeline", () => {
             const liveTimeline = eventTimelineSet.getLiveTimeline();
             expect(liveTimeline.getEvents().length).toStrictEqual(0);
@@ -143,6 +167,41 @@ describe('EventTimelineSet', () => {
                 );
             }).not.toThrow();
         });
+
+        it("should not add an event to a timeline that does not belong to the timelineSet", () => {
+            const eventTimelineSet2 = new EventTimelineSet(room);
+            const liveTimeline2 = eventTimelineSet2.getLiveTimeline();
+            expect(liveTimeline2.getEvents().length).toStrictEqual(0);
+
+            expect(() => {
+                eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline2, {
+                    toStartOfTimeline: true,
+                });
+            }).toThrowError();
+        });
+
+        it("should not add a threaded reply to the main room timeline", () => {
+            const liveTimeline = eventTimelineSet.getLiveTimeline();
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+
+            const threadedReplyEvent = mkThreadResponse(messageEvent);
+
+            eventTimelineSet.addEventToTimeline(threadedReplyEvent, liveTimeline, {
+                toStartOfTimeline: true,
+            });
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+        });
+
+        it("should not add a normal message to the timelineSet representing a thread", () => {
+            const eventTimelineSetForThread = new EventTimelineSet(room, {}, client, thread);
+            const liveTimeline = eventTimelineSetForThread.getLiveTimeline();
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+
+            eventTimelineSetForThread.addEventToTimeline(messageEvent, liveTimeline, {
+                toStartOfTimeline: true,
+            });
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+        });
     });
 
     describe('aggregateRelations', () => {
@@ -225,23 +284,6 @@ describe('EventTimelineSet', () => {
     });
 
     describe("canContain", () => {
-        const mkThreadResponse = (root: MatrixEvent) => utils.mkEvent({
-            event: true,
-            type: EventType.RoomMessage,
-            user: userA,
-            room: roomId,
-            content: {
-                "body": "Thread response :: " + Math.random(),
-                "m.relates_to": {
-                    "event_id": root.getId(),
-                    "m.in_reply_to": {
-                        "event_id": root.getId(),
-                    },
-                    "rel_type": "m.thread",
-                },
-            },
-        }, room.client);
-
         let thread: Thread;
 
         beforeEach(() => {

src/client.ts

@@ -5288,10 +5288,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         ];
 
         if (this.supportsExperimentalThreads()) {
-            if (!timelineSet.canContain(event)) {
-                return undefined;
-            }
-
             // Where the event is a thread reply (not a root) and running in MSC-enabled mode the Thread timeline only
             // functions contiguously, so we have to jump through some hoops to get our target event in it.
             // XXX: workaround for https://github.com/vector-im/element-meta/issues/150

src/models/event-timeline-set.ts

@@ -691,6 +691,18 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
             );
         }
 
+        if (timeline.getTimelineSet() !== this) {
+            throw new Error(`EventTimelineSet.addEventToTimeline: Timeline=${timeline.toString()} does not belong " + 
+                "in timelineSet(threadId=${this.thread?.id})`);
+        }
+
+        // Make sure events don't get mixed in timelines they shouldn't be in
+        // (e.g. a threaded message should not be in the main timeline).
+        if (!this.canContain(event)) {
+            logger.warn(`EventTimelineSet.addEventToTimeline: Ignoring event=${event.getId()} that does not belong in timeline=${timeline.toString()} timelineSet(threadId=${this.thread?.id})`);
+            return;
+        }
+
         const eventId = event.getId();
         timeline.addEvent(event, {
             toStartOfTimeline,

src/models/room.ts

@@ -981,6 +981,10 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
             newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId());
         }
 
+        if (!newTimeline) {
+            throw new Error(`[refreshLiveTimeline for ${this.roomId}] No new timeline created`);
+        }
+
         // If a racing `/sync` beat us to creating a new timeline, use that
         // instead because it's the latest in the room and any new messages in
         // the scrollback will include the history.