Fix handling of threaded messages around edits & echoes (#2267)

spec/integ/matrix-client-methods.spec.js

@@ -145,12 +145,14 @@ describe("MatrixClient", function() {
     describe("joinRoom", function() {
         it("should no-op if you've already joined a room", function() {
             const roomId = "!foo:bar";
-            const room = new Room(roomId, userId);
+            const room = new Room(roomId, client, userId);
+            client.fetchRoomEvent = () => Promise.resolve({});
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userId, room: roomId, mship: "join", event: true,
                 }),
             ]);
+            httpBackend.verifyNoOutstandingRequests();
             store.storeRoom(room);
             client.joinRoom(roomId);
             httpBackend.verifyNoOutstandingRequests();
@@ -556,11 +558,14 @@ describe("MatrixClient", function() {
     });
 
     describe("partitionThreadedEvents", function() {
-        const room = new Room("!STrMRsukXHtqQdSeHa:matrix.org", client, userId);
+        let room;
+        beforeEach(() => {
+            room = new Room("!STrMRsukXHtqQdSeHa:matrix.org", client, userId);
+        });
 
         it("returns empty arrays when given an empty arrays", function() {
             const events = [];
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
             expect(timeline).toEqual([]);
             expect(threaded).toEqual([]);
         });
@@ -580,7 +585,7 @@ describe("MatrixClient", function() {
             // Vote has no threadId yet
             expect(eventPollResponseReference.threadId).toBeFalsy();
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 // The message that was sent in a thread is missing
@@ -613,7 +618,7 @@ describe("MatrixClient", function() {
                 eventReaction,
             ];
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 eventPollStartThreadRoot,
@@ -640,7 +645,7 @@ describe("MatrixClient", function() {
                 eventMessageInThread,
             ];
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 eventPollStartThreadRoot,
@@ -667,7 +672,7 @@ describe("MatrixClient", function() {
                 eventReaction,
             ];
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 eventPollStartThreadRoot,
@@ -710,7 +715,7 @@ describe("MatrixClient", function() {
                 eventMember,
                 eventCreate,
             ];
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 // The message that was sent in a thread is missing
@@ -749,7 +754,7 @@ describe("MatrixClient", function() {
                 threadedReactionRedaction,
             ];
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 threadRootEvent,
@@ -778,7 +783,7 @@ describe("MatrixClient", function() {
                 replyToReply,
             ];
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 threadRootEvent,
@@ -805,7 +810,7 @@ describe("MatrixClient", function() {
                 replyToThreadResponse,
             ];
 
-            const [timeline, threaded] = client.partitionThreadedEvents(room, events);
+            const [timeline, threaded] = room.partitionThreadedEvents(events);
 
             expect(timeline).toEqual([
                 threadRootEvent,

spec/test-utils/test-utils.ts

@@ -101,6 +101,7 @@ export function mkEvent(opts: IEventOpts): object | MatrixEvent {
         content: opts.content,
         unsigned: opts.unsigned || {},
         event_id: "$" + Math.random() + "-" + Math.random(),
+        txn_id: "~" + Math.random(),
         redacts: opts.redacts,
     };
     if (opts.skey !== undefined) {

spec/unit/matrix-client.spec.ts

@@ -981,7 +981,7 @@ describe("MatrixClient", function() {
 
             expect(rootEvent.isThreadRoot).toBe(true);
 
-            const [roomEvents, threadEvents] = client.partitionThreadedEvents(room, [rootEvent]);
+            const [roomEvents, threadEvents] = room.partitionThreadedEvents([rootEvent]);
             expect(roomEvents).toHaveLength(1);
             expect(threadEvents).toHaveLength(1);
 

spec/unit/room.spec.ts

@@ -35,6 +35,8 @@ import { Room } from "../../src/models/room";
 import { RoomState } from "../../src/models/room-state";
 import { UNSTABLE_ELEMENT_FUNCTIONAL_USERS } from "../../src/@types/event";
 import { TestClient } from "../TestClient";
+import { emitPromise } from "../test-utils/test-utils";
+import { ThreadEvent } from "../../src/models/thread";
 
 describe("Room", function() {
     const roomId = "!foo:bar";
@@ -44,8 +46,86 @@ describe("Room", function() {
     const userD = "@dorothy:bar";
     let room;
 
+    const mkMessage = () => utils.mkMessage({
+        event: true,
+        user: userA,
+        room: roomId,
+    }) as MatrixEvent;
+
+    const mkReply = (target: MatrixEvent) => utils.mkEvent({
+        event: true,
+        type: EventType.RoomMessage,
+        user: userA,
+        room: roomId,
+        content: {
+            "body": "Reply :: " + Math.random(),
+            "m.relates_to": {
+                "m.in_reply_to": {
+                    "event_id": target.getId(),
+                },
+            },
+        },
+    }) as MatrixEvent;
+
+    const mkEdit = (target: MatrixEvent, salt = Math.random()) => utils.mkEvent({
+        event: true,
+        type: EventType.RoomMessage,
+        user: userA,
+        room: roomId,
+        content: {
+            "body": "* Edit of :: " + target.getId() + " :: " + salt,
+            "m.new_content": {
+                body: "Edit of :: " + target.getId() + " :: " + salt,
+            },
+            "m.relates_to": {
+                rel_type: RelationType.Replace,
+                event_id: target.getId(),
+            },
+        },
+    }) as MatrixEvent;
+
+    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",
+            },
+        },
+    }) as MatrixEvent;
+
+    const mkReaction = (target: MatrixEvent) => utils.mkEvent({
+        event: true,
+        type: EventType.Reaction,
+        user: userA,
+        room: roomId,
+        content: {
+            "m.relates_to": {
+                "rel_type": RelationType.Annotation,
+                "event_id": target.getId(),
+                "key": Math.random().toString(),
+            },
+        },
+    }) as MatrixEvent;
+
+    const mkRedaction = (target: MatrixEvent) => utils.mkEvent({
+        event: true,
+        type: EventType.RoomRedaction,
+        user: userA,
+        room: roomId,
+        redacts: target.getId(),
+        content: {},
+    }) as MatrixEvent;
+
     beforeEach(function() {
-        room = new Room(roomId, null, userA);
+        room = new Room(roomId, new TestClient(userA, "device").client, userA);
         // mock RoomStates
         room.oldState = room.getLiveTimeline().startState = utils.mock(RoomState, "oldState");
         room.currentState = room.getLiveTimeline().endState = utils.mock(RoomState, "currentState");
@@ -157,19 +237,18 @@ describe("Room", function() {
             expect(room.timeline[0]).toEqual(events[0]);
         });
 
-        it("should emit 'Room.timeline' events",
+        it("should emit 'Room.timeline' events", function() {
-            function() {
+            let callCount = 0;
-                let callCount = 0;
+            room.on("Room.timeline", function(event, emitRoom, toStart) {
-                room.on("Room.timeline", function(event, emitRoom, toStart) {
+                callCount += 1;
-                    callCount += 1;
+                expect(room.timeline.length).toEqual(callCount);
-                    expect(room.timeline.length).toEqual(callCount);
+                expect(event).toEqual(events[callCount - 1]);
-                    expect(event).toEqual(events[callCount - 1]);
+                expect(emitRoom).toEqual(room);
-                    expect(emitRoom).toEqual(room);
+                expect(toStart).toBeFalsy();
-                    expect(toStart).toBeFalsy();
-                });
-                room.addLiveEvents(events);
-                expect(callCount).toEqual(2);
             });
+            room.addLiveEvents(events);
+            expect(callCount).toEqual(2);
+        });
 
         it("should call setStateEvents on the right RoomState with the right forwardLooking value for new events",
             function() {
@@ -338,42 +417,41 @@ describe("Room", function() {
             expect(oldEv.sender).toEqual(oldSentinel);
         });
 
-        it("should set event.target for new and old m.room.member events",
+        it("should set event.target for new and old m.room.member events", function() {
-            function() {
+            const sentinel = {
-                const sentinel = {
+                userId: userA,
-                    userId: userA,
+                membership: "join",
-                    membership: "join",
+                name: "Alice",
-                    name: "Alice",
+            };
-                };
+            const oldSentinel = {
-                const oldSentinel = {
+                userId: userA,
-                    userId: userA,
+                membership: "join",
-                    membership: "join",
+                name: "Old Alice",
-                    name: "Old Alice",
+            };
-                };
+            room.currentState.getSentinelMember.mockImplementation(function(uid) {
-                room.currentState.getSentinelMember.mockImplementation(function(uid) {
+                if (uid === userA) {
-                    if (uid === userA) {
+                    return sentinel;
-                        return sentinel;
+                }
-                    }
+                return null;
-                    return null;
-                });
-                room.oldState.getSentinelMember.mockImplementation(function(uid) {
-                    if (uid === userA) {
-                        return oldSentinel;
-                    }
-                    return null;
-                });
-
-                const newEv = utils.mkMembership({
-                    room: roomId, mship: "invite", user: userB, skey: userA, event: true,
-                }) as MatrixEvent;
-                const oldEv = utils.mkMembership({
-                    room: roomId, mship: "ban", user: userB, skey: userA, event: true,
-                }) as MatrixEvent;
-                room.addLiveEvents([newEv]);
-                expect(newEv.target).toEqual(sentinel);
-                room.addEventsToTimeline([oldEv], true, room.getLiveTimeline());
-                expect(oldEv.target).toEqual(oldSentinel);
             });
+            room.oldState.getSentinelMember.mockImplementation(function(uid) {
+                if (uid === userA) {
+                    return oldSentinel;
+                }
+                return null;
+            });
+
+            const newEv = utils.mkMembership({
+                room: roomId, mship: "invite", user: userB, skey: userA, event: true,
+            }) as MatrixEvent;
+            const oldEv = utils.mkMembership({
+                room: roomId, mship: "ban", user: userB, skey: userA, event: true,
+            }) as MatrixEvent;
+            room.addLiveEvents([newEv]);
+            expect(newEv.target).toEqual(sentinel);
+            room.addEventsToTimeline([oldEv], true, room.getLiveTimeline());
+            expect(oldEv.target).toEqual(oldSentinel);
+        });
 
         it("should call setStateEvents on the right RoomState with the right " +
         "forwardLooking value for old events", function() {
@@ -406,7 +484,7 @@ describe("Room", function() {
         let events = null;
 
         beforeEach(function() {
-            room = new Room(roomId, null, null, { timelineSupport: timelineSupport });
+            room = new Room(roomId, new TestClient(userA).client, userA, { timelineSupport: timelineSupport });
             // set events each time to avoid resusing Event objects (which
             // doesn't work because they get frozen)
             events = [
@@ -469,8 +547,7 @@ describe("Room", function() {
             expect(callCount).toEqual(1);
         });
 
-        it("should " + (timelineSupport ? "remember" : "forget") +
+        it("should " + (timelineSupport ? "remember" : "forget") + " old timelines", function() {
-                " old timelines", function() {
             room.addLiveEvents([events[0]]);
             expect(room.timeline.length).toEqual(1);
             const firstLiveTimeline = room.getLiveTimeline();
@@ -486,7 +563,7 @@ describe("Room", function() {
 
     describe("compareEventOrdering", function() {
         beforeEach(function() {
-            room = new Room(roomId, null, null, { timelineSupport: true });
+            room = new Room(roomId, new TestClient(userA).client, userA, { timelineSupport: true });
         });
 
         const events: MatrixEvent[] = [
@@ -673,7 +750,7 @@ describe("Room", function() {
 
         beforeEach(function() {
             // no mocking
-            room = new Room(roomId, null, userA);
+            room = new Room(roomId, new TestClient(userA).client, userA);
         });
 
         describe("Room.recalculate => Stripped State Events", function() {
@@ -1259,6 +1336,7 @@ describe("Room", function() {
             const client = (new TestClient(
                 "@alice:example.com", "alicedevice",
             )).client;
+            client.supportsExperimentalThreads = () => true;
             const room = new Room(roomId, client, userA, {
                 pendingEventOrdering: PendingEventOrdering.Detached,
             });
@@ -1285,7 +1363,7 @@ describe("Room", function() {
 
         it("should add pending events to the timeline if " +
                       "pendingEventOrdering == 'chronological'", function() {
-            room = new Room(roomId, null, userA, {
+            const room = new Room(roomId, new TestClient(userA).client, userA, {
                 pendingEventOrdering: PendingEventOrdering.Chronological,
             });
             const eventA = utils.mkMessage({
@@ -1504,7 +1582,7 @@ describe("Room", function() {
 
     describe("guessDMUserId", function() {
         it("should return first hero id", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.setSummary({
                 'm.heroes': [userB],
                 'm.joined_member_count': 1,
@@ -1513,7 +1591,7 @@ describe("Room", function() {
             expect(room.guessDMUserId()).toEqual(userB);
         });
         it("should return first member that isn't self", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([utils.mkMembership({
                 user: userB,
                 mship: "join",
@@ -1523,7 +1601,7 @@ describe("Room", function() {
             expect(room.guessDMUserId()).toEqual(userB);
         });
         it("should return self if only member present", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             expect(room.guessDMUserId()).toEqual(userA);
         });
     });
@@ -1542,12 +1620,12 @@ describe("Room", function() {
 
     describe("getDefaultRoomName", function() {
         it("should return 'Empty room' if a user is the only member", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             expect(room.getDefaultRoomName(userA)).toEqual("Empty room");
         });
 
         it("should return a display name if one other member is in the room", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1562,7 +1640,7 @@ describe("Room", function() {
         });
 
         it("should return a display name if one other member is banned", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1577,7 +1655,7 @@ describe("Room", function() {
         });
 
         it("should return a display name if one other member is invited", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1592,7 +1670,7 @@ describe("Room", function() {
         });
 
         it("should return 'Empty room (was User B)' if User B left the room", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1607,7 +1685,7 @@ describe("Room", function() {
         });
 
         it("should return 'User B and User C' if in a room with two other users", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1626,7 +1704,7 @@ describe("Room", function() {
         });
 
         it("should return 'User B and 2 others' if in a room with three other users", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1651,7 +1729,7 @@ describe("Room", function() {
 
     describe("io.element.functional_users", function() {
         it("should return a display name (default behaviour) if no one is marked as a functional member", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1673,7 +1751,7 @@ describe("Room", function() {
         });
 
         it("should return a display name (default behaviour) if service members is a number (invalid)", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1697,7 +1775,7 @@ describe("Room", function() {
         });
 
         it("should return a display name (default behaviour) if service members is a string (invalid)", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1719,7 +1797,7 @@ describe("Room", function() {
         });
 
         it("should return 'Empty room' if the only other member is a functional member", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1741,7 +1819,7 @@ describe("Room", function() {
         });
 
         it("should return 'User B' if User B is the only other member who isn't a functional member", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1767,7 +1845,7 @@ describe("Room", function() {
         });
 
         it("should return 'Empty room' if all other members are functional members", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1793,7 +1871,7 @@ describe("Room", function() {
         });
 
         it("should not break if an unjoined user is marked as a service user", function() {
-            const room = new Room(roomId, null, userA);
+            const room = new Room(roomId, new TestClient(userA).client, userA);
             room.addLiveEvents([
                 utils.mkMembership({
                     user: userA, mship: "join",
@@ -1858,71 +1936,51 @@ describe("Room", function() {
 
             expect(() => room.createThread(rootEvent, [])).not.toThrow();
         });
+
+        it("Edits update the lastReply event", async () => {
+            const client = (new TestClient(
+                "@alice:example.com", "alicedevice",
+            )).client;
+            client.supportsExperimentalThreads = () => true;
+            room = new Room(roomId, client, userA);
+
+            const randomMessage = mkMessage();
+            const threadRoot = mkMessage();
+            const threadResponse = mkThreadResponse(threadRoot);
+            threadResponse.localTimestamp += 1000;
+            const threadResponseEdit = mkEdit(threadResponse);
+            threadResponseEdit.localTimestamp += 2000;
+
+            client.fetchRoomEvent = (eventId: string) => Promise.resolve({
+                ...threadRoot.event,
+                unsigned: {
+                    "age": 123,
+                    "m.relations": {
+                        "m.thread": {
+                            latest_event: threadResponse.event,
+                            count: 2,
+                            current_user_participated: true,
+                        },
+                    },
+                },
+            });
+
+            room.addLiveEvents([randomMessage, threadRoot, threadResponse]);
+            const thread = await emitPromise(room, ThreadEvent.New);
+
+            expect(thread.replyToEvent).toBe(threadResponse);
+            expect(thread.replyToEvent.getContent().body).toBe(threadResponse.getContent().body);
+
+            room.addLiveEvents([threadResponseEdit]);
+            await emitPromise(thread, ThreadEvent.Update);
+            expect(thread.replyToEvent.getContent().body).toBe(threadResponseEdit.getContent()["m.new_content"].body);
+        });
     });
 
     describe("eventShouldLiveIn", () => {
-        const room = new Room(roomId, null, userA);
+        const client = new TestClient(userA).client;
-
+        client.supportsExperimentalThreads = () => true;
-        const mkMessage = () => utils.mkMessage({
+        const room = new Room(roomId, client, userA);
-            event: true,
-            user: userA,
-            room: roomId,
-        }) as MatrixEvent;
-
-        const mkReply = (target: MatrixEvent) => utils.mkEvent({
-            event: true,
-            type: EventType.RoomMessage,
-            user: userA,
-            room: roomId,
-            content: {
-                "body": "Reply :: " + Math.random(),
-                "m.relates_to": {
-                    "m.in_reply_to": {
-                        "event_id": target.getId(),
-                    },
-                },
-            },
-        }) as MatrixEvent;
-
-        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",
-                },
-            },
-        }) as MatrixEvent;
-
-        const mkReaction = (target: MatrixEvent) => utils.mkEvent({
-            event: true,
-            type: EventType.Reaction,
-            user: userA,
-            room: roomId,
-            content: {
-                "m.relates_to": {
-                    "rel_type": RelationType.Annotation,
-                    "event_id": target.getId(),
-                    "key": Math.random().toString(),
-                },
-            },
-        }) as MatrixEvent;
-
-        const mkRedaction = (target: MatrixEvent) => utils.mkEvent({
-            event: true,
-            type: EventType.RoomRedaction,
-            user: userA,
-            room: roomId,
-            redacts: target.getId(),
-            content: {},
-        }) as MatrixEvent;
 
         it("thread root and its relations&redactions should be in both", () => {
             const randomMessage = mkMessage();

src/client.ts

@@ -3771,9 +3771,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             txnId = this.makeTxnId();
         }
 
-        // we always construct a MatrixEvent when sending because the store and
+        // We always construct a MatrixEvent when sending because the store and scheduler use them.
-        // scheduler use them. We'll extract the params back out if it turns out
+        // We'll extract the params back out if it turns out the client has no scheduler or store.
-        // the client has no scheduler or store.
         const localEvent = new MatrixEvent(Object.assign(eventObject, {
             event_id: "~" + roomId + ":" + txnId,
             user_id: this.credentials.userId,
@@ -3808,9 +3807,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         localEvent.setStatus(EventStatus.SENDING);
 
         // add this event immediately to the local store as 'sending'.
-        if (room) {
+        room?.addPendingEvent(localEvent, txnId);
-            room.addPendingEvent(localEvent, txnId);
-        }
 
         // addPendingEvent can change the state to NOT_SENT if it believes
         // that there's other events that have failed. We won't bother to
@@ -5179,7 +5176,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
                     room.currentState.setUnknownStateEvents(stateEvents);
                 }
 
-                const [timelineEvents, threadedEvents] = this.partitionThreadedEvents(room, matrixEvents);
+                const [timelineEvents, threadedEvents] = room.partitionThreadedEvents(matrixEvents);
 
                 room.addEventsToTimeline(timelineEvents, true, room.getLiveTimeline());
                 await this.processThreadEvents(room, threadedEvents, true);
@@ -5281,7 +5278,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         // 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
         if (Thread.hasServerSideSupport && event.isRelation(THREAD_RELATION_TYPE.name)) {
-            const [, threadedEvents] = this.partitionThreadedEvents(timelineSet.room, events);
+            const [, threadedEvents] = timelineSet.room.partitionThreadedEvents(events);
             const thread = await timelineSet.room.createThreadFetchRoot(event.threadRootId, threadedEvents, true);
 
             let nextBatch: string;
@@ -5316,7 +5313,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             timeline.getState(EventTimeline.FORWARDS).paginationToken = res.end;
         }
 
-        const [timelineEvents, threadedEvents] = this.partitionThreadedEvents(timelineSet.room, events);
+        const [timelineEvents, threadedEvents] = timelineSet.room.partitionThreadedEvents(events);
         timelineSet.addEventsToTimeline(timelineEvents, true, timeline, res.start);
         // The target event is not in a thread but process the contextual events, so we can show any threads around it.
         await this.processThreadEvents(timelineSet.room, threadedEvents, true);
@@ -5447,7 +5444,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
                 }
 
                 const timelineSet = eventTimeline.getTimelineSet();
-                const [timelineEvents, threadedEvents] = this.partitionThreadedEvents(timelineSet.room, matrixEvents);
+                const [timelineEvents, threadedEvents] = timelineSet.room.partitionThreadedEvents(matrixEvents);
                 timelineSet.addEventsToTimeline(timelineEvents, backwards, eventTimeline, token);
                 await this.processThreadEvents(timelineSet.room, threadedEvents, backwards);
 
@@ -5484,7 +5481,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
                 const matrixEvents = res.chunk.map(this.getEventMapper());
 
                 const timelineSet = eventTimeline.getTimelineSet();
-                const [timelineEvents, threadedEvents] = this.partitionThreadedEvents(timelineSet.room, matrixEvents);
+                const [timelineEvents, threadedEvents] = timelineSet.room.partitionThreadedEvents(matrixEvents);
                 timelineSet.addEventsToTimeline(timelineEvents, backwards, eventTimeline, token);
                 await this.processThreadEvents(room, threadedEvents, backwards);
 
@@ -8852,57 +8849,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         });
     }
 
-    /**
-     * Given some events, find the IDs of all the thread roots that are
-     * referred to by them.
-     */
-    private findThreadRoots(events: MatrixEvent[]): Set<string> {
-        const threadRoots = new Set<string>();
-        for (const event of events) {
-            if (event.isThreadRelation) {
-                threadRoots.add(event.relationEventId);
-            }
-        }
-        return threadRoots;
-    }
-
-    public partitionThreadedEvents(room: Room, events: MatrixEvent[]): [
-        timelineEvents: MatrixEvent[],
-        threadedEvents: MatrixEvent[],
-    ] {
-        // Indices to the events array, for readability
-        const ROOM = 0;
-        const THREAD = 1;
-        if (this.supportsExperimentalThreads()) {
-            const threadRoots = this.findThreadRoots(events);
-            return events.reduce((memo, event: MatrixEvent) => {
-                const {
-                    shouldLiveInRoom,
-                    shouldLiveInThread,
-                    threadId,
-                } = room.eventShouldLiveIn(event, events, threadRoots);
-
-                if (shouldLiveInRoom) {
-                    memo[ROOM].push(event);
-                }
-
-                if (shouldLiveInThread) {
-                    event.setThreadId(threadId);
-                    memo[THREAD].push(event);
-                }
-
-                return memo;
-            }, [[], []]);
-        } else {
-            // When `experimentalThreadSupport` is disabled
-            // treat all events as timelineEvents
-            return [
-                events,
-                [],
-            ];
-        }
-    }
-
     /**
      * @experimental
      */
@@ -8911,9 +8857,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         threadedEvents: MatrixEvent[],
         toStartOfTimeline: boolean,
     ): Promise<void> {
-        for (const event of threadedEvents) {
+        await room.processThreadedEvents(threadedEvents, toStartOfTimeline);
-            await room.addThreadedEvent(event, toStartOfTimeline);
-        }
     }
 
     /**

src/event-mapper.ts

@@ -45,8 +45,9 @@ export function eventMapperFor(client: MatrixClient, options: MapperOpts): Event
             event.setUnsigned({ ...event.getUnsigned(), ...plainOldJsObject.unsigned });
         }
 
-        if (room?.threads.has(event.getId())) {
+        const thread = room?.findThreadForEvent(event);
-            event.setThread(room.threads.get(event.getId()));
+        if (thread) {
+            event.setThread(thread);
         }
 
         if (event.isEncrypted()) {

src/models/event-timeline-set.ts

@@ -775,7 +775,7 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
     }
 
     public getAllRelationsEventForEvent(eventId: string): MatrixEvent[] {
-        const relationsForEvent = this.relations[eventId] || {};
+        const relationsForEvent = this.relations?.[eventId] || {};
         const events = [];
         for (const relationsRecord of Object.values(relationsForEvent)) {
             for (const relations of Object.values(relationsRecord)) {
@@ -852,7 +852,7 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
         }
         let relationsWithEventType = relationsWithRelType[eventType];
 
-        let relatesToEvent;
+        let relatesToEvent: MatrixEvent;
         if (!relationsWithEventType) {
             relationsWithEventType = relationsWithRelType[eventType] = new Relations(
                 relationType,

src/models/event.ts

@@ -119,11 +119,6 @@ export interface IEventRelation {
     key?: string;
 }
 
-export interface IVisibilityEventRelation extends IEventRelation {
-    visibility: "visible" | "hidden";
-    reason?: string;
-}
-
 /**
  * When an event is a visibility change event, as per MSC3531,
  * the visibility change implied by the event.

src/models/relations.ts

@@ -331,7 +331,7 @@ export class Relations extends TypedEventEmitter<RelationsEvent, EventHandlerMap
         // the all-knowning server tells us that the event at some point had
         // this timestamp for its replacement, so any following replacement should definitely not be less
         const replaceRelation = this.targetEvent.getServerAggregatedRelation<IAggregatedRelation>(RelationType.Replace);
-        const minTs = replaceRelation && replaceRelation.origin_server_ts;
+        const minTs = replaceRelation?.origin_server_ts;
 
         const lastReplacement = this.getRelations().reduce((last, event) => {
             if (event.getSender() !== this.targetEvent.getSender()) {

src/models/room.ts

@@ -22,7 +22,7 @@ import { EventTimelineSet, DuplicateStrategy } from "./event-timeline-set";
 import { Direction, EventTimeline } from "./event-timeline";
 import { getHttpUriForMxc } from "../content-repo";
 import * as utils from "../utils";
-import { normalize } from "../utils";
+import { defer, normalize } from "../utils";
 import { IEvent, IThreadBundledRelationship, MatrixEvent } from "./event";
 import { EventStatus } from "./event-status";
 import { RoomMember } from "./room-member";
@@ -213,6 +213,8 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
     private getTypeWarning = false;
     private getVersionWarning = false;
     private membersPromise?: Promise<boolean>;
+    // Map from threadId to pending Thread instance created by createThreadFetchRoot
+    private threadPromises = new Map<string, Promise<Thread>>();
 
     // XXX: These should be read-only
     /**
@@ -1567,6 +1569,13 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
         shouldLiveInThread: boolean;
         threadId?: string;
     } {
+        if (!this.client.supportsExperimentalThreads()) {
+            return {
+                shouldLiveInRoom: true,
+                shouldLiveInThread: false,
+            };
+        }
+
         // A thread root is always shown in both timelines
         if (event.isThreadRoot || roots?.has(event.getId())) {
             return {
@@ -1581,7 +1590,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
             return {
                 shouldLiveInRoom: false,
                 shouldLiveInThread: true,
-                threadId: event.relationEventId,
+                threadId: event.threadRootId,
             };
         }
 
@@ -1630,21 +1639,23 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
         threadId: string,
         events?: MatrixEvent[],
         toStartOfTimeline?: boolean,
-    ): Promise<Thread> {
+    ): Promise<Thread | null> {
         let thread = this.getThread(threadId);
 
         if (!thread) {
+            const deferred = defer<Thread | null>();
+            this.threadPromises.set(threadId, deferred.promise);
+
             let rootEvent = this.findEventById(threadId);
             // If the rootEvent does not exist in the local stores, then fetch it from the server.
             try {
                 const eventData = await this.client.fetchRoomEvent(this.roomId, threadId);
-
+                const mapper = this.client.getEventMapper();
-                if (!rootEvent) {
+                rootEvent = mapper(eventData); // will merge with existing event object if such is known
-                    rootEvent = new MatrixEvent(eventData);
+            } catch (e) {
-                } else {
+                logger.error("Failed to fetch thread root to construct thread with", e);
-                    rootEvent.setUnsigned(eventData.unsigned);
-                }
             } finally {
+                this.threadPromises.delete(threadId);
                 // The root event might be not be visible to the person requesting it.
                 // If it wasn't fetched successfully the thread will work in "limited" mode and won't
                 // benefit from all the APIs a homeserver can provide to enhance the thread experience
@@ -1652,26 +1663,51 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
                 if (thread) {
                     rootEvent.setThread(thread);
                 }
+                deferred.resolve(thread);
             }
         }
 
         return thread;
     }
 
-    /**
+    private async addThreadedEvents(events: MatrixEvent[], threadId: string, toStartOfTimeline = false): Promise<void> {
-     * Add an event to a thread's timeline. Will fire "Thread.update"
+        let thread = this.getThread(threadId);
-     * @experimental
+        if (this.threadPromises.has(threadId)) {
-     */
+            thread = await this.threadPromises.get(threadId);
-    public async addThreadedEvent(event: MatrixEvent, toStartOfTimeline: boolean): Promise<void> {
-        this.applyRedaction(event);
-        let thread = this.findThreadForEvent(event);
-        if (thread) {
-            await thread.addEvent(event, toStartOfTimeline);
-        } else {
-            thread = await this.createThreadFetchRoot(event.threadRootId, [event], toStartOfTimeline);
         }
 
-        this.emit(ThreadEvent.Update, thread);
+        if (thread) {
+            for (const event of events) {
+                await thread.addEvent(event, toStartOfTimeline);
+            }
+        } else {
+            thread = await this.createThreadFetchRoot(threadId, events, toStartOfTimeline);
+        }
+
+        if (thread) {
+            this.emit(ThreadEvent.Update, thread);
+        }
+    }
+
+    /**
+     * Adds events to a thread's timeline. Will fire "Thread.update"
+     * @experimental
+     */
+    public async processThreadedEvents(events: MatrixEvent[], toStartOfTimeline: boolean): Promise<unknown> {
+        events.forEach(this.applyRedaction);
+
+        const eventsByThread: { [threadId: string]: MatrixEvent[] } = {};
+        for (const event of events) {
+            const { threadId } = this.eventShouldLiveIn(event);
+            if (!eventsByThread[threadId]) {
+                eventsByThread[threadId] = [];
+            }
+            eventsByThread[threadId].push(event);
+        }
+
+        return Promise.all(Object.entries(eventsByThread).map(([threadId, events]) => (
+            this.addThreadedEvents(events, threadId, toStartOfTimeline)
+        )));
     }
 
     public createThread(
@@ -1728,7 +1764,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
         }
     }
 
-    private applyRedaction(event: MatrixEvent): void {
+    private applyRedaction = (event: MatrixEvent): void => {
         if (event.isRedaction()) {
             const redactId = event.event.redacts;
 
@@ -1738,7 +1774,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
                 redactedEvent.makeRedacted(event);
 
                 // If this is in the current state, replace it with the redacted version
-                if (redactedEvent.getStateKey()) {
+                if (redactedEvent.isState()) {
                     const currentStateEvent = this.currentState.getStateEvents(
                         redactedEvent.getType(),
                         redactedEvent.getStateKey(),
@@ -1772,19 +1808,9 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
             // clients can say "so and so redacted an event" if they wish to. Also
             // this may be needed to trigger an update.
         }
-    }
+    };
 
-    /**
+    private processLiveEvent(event: MatrixEvent): Promise<void> {
-     * Add an event to the end of this room's live timelines. Will fire
-     * "Room.timeline".
-     *
-     * @param {MatrixEvent} event Event to be added
-     * @param {string?} duplicateStrategy 'ignore' or 'replace'
-     * @param {boolean} fromCache whether the sync response came from cache
-     * @fires module:client~MatrixClient#event:"Room.timeline"
-     * @private
-     */
-    private addLiveEvent(event: MatrixEvent, duplicateStrategy?: DuplicateStrategy, fromCache = false): void {
         this.applyRedaction(event);
 
         // Implement MSC3531: hiding messages.
@@ -1804,7 +1830,19 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
                 return;
             }
         }
+    }
 
+    /**
+     * Add an event to the end of this room's live timelines. Will fire
+     * "Room.timeline".
+     *
+     * @param {MatrixEvent} event Event to be added
+     * @param {string?} duplicateStrategy 'ignore' or 'replace'
+     * @param {boolean} fromCache whether the sync response came from cache
+     * @fires module:client~MatrixClient#event:"Room.timeline"
+     * @private
+     */
+    private addLiveEvent(event: MatrixEvent, duplicateStrategy: DuplicateStrategy, fromCache = false): void {
         // add to our timeline sets
         for (let i = 0; i < this.timelineSets.length; i++) {
             this.timelineSets[i].addLiveEvent(event, duplicateStrategy, fromCache);
@@ -1998,10 +2036,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
         const newEventId = remoteEvent.getId();
         const oldStatus = localEvent.status;
 
-        logger.debug(
+        logger.debug(`Got remote echo for event ${oldEventId} -> ${newEventId} old status ${oldStatus}`);
-            `Got remote echo for event ${oldEventId} -> ${newEventId} ` +
-            `old status ${oldStatus}`,
-        );
 
         // no longer pending
         delete this.txnToEvent[remoteEvent.getUnsigned().transaction_id];
@@ -2167,10 +2202,84 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
             }
         }
 
+        const threadRoots = this.findThreadRoots(events);
+        const threadInfos = events.map(e => this.eventShouldLiveIn(e, events, threadRoots));
+        const eventsByThread: { [threadId: string]: MatrixEvent[] } = {};
+
         for (let i = 0; i < events.length; i++) {
             // TODO: We should have a filter to say "only add state event types X Y Z to the timeline".
-            this.addLiveEvent(events[i], duplicateStrategy, fromCache);
+            this.processLiveEvent(events[i]);
+
+            const {
+                shouldLiveInRoom,
+                shouldLiveInThread,
+                threadId,
+            } = threadInfos[i];
+
+            if (shouldLiveInThread) {
+                if (!eventsByThread[threadId]) {
+                    eventsByThread[threadId] = [];
+                }
+                eventsByThread[threadId].push(events[i]);
+            }
+
+            if (shouldLiveInRoom) {
+                this.addLiveEvent(events[i], duplicateStrategy, fromCache);
+            }
         }
+
+        Object.entries(eventsByThread).forEach(([threadId, threadEvents]) => {
+            this.addThreadedEvents(threadEvents, threadId, false);
+        });
+    }
+
+    public partitionThreadedEvents(events: MatrixEvent[]): [
+        timelineEvents: MatrixEvent[],
+        threadedEvents: MatrixEvent[],
+    ] {
+        // Indices to the events array, for readability
+        const ROOM = 0;
+        const THREAD = 1;
+        if (this.client.supportsExperimentalThreads()) {
+            const threadRoots = this.findThreadRoots(events);
+            return events.reduce((memo, event: MatrixEvent) => {
+                const {
+                    shouldLiveInRoom,
+                    shouldLiveInThread,
+                    threadId,
+                } = this.eventShouldLiveIn(event, events, threadRoots);
+
+                if (shouldLiveInRoom) {
+                    memo[ROOM].push(event);
+                }
+
+                if (shouldLiveInThread) {
+                    event.setThreadId(threadId);
+                    memo[THREAD].push(event);
+                }
+
+                return memo;
+            }, [[], []]);
+        } else {
+            // When `experimentalThreadSupport` is disabled treat all events as timelineEvents
+            return [
+                events,
+                [],
+            ];
+        }
+    }
+
+    /**
+     * Given some events, find the IDs of all the thread roots that are referred to by them.
+     */
+    private findThreadRoots(events: MatrixEvent[]): Set<string> {
+        const threadRoots = new Set<string>();
+        for (const event of events) {
+            if (event.isThreadRelation) {
+                threadRoots.add(event.relationEventId);
+            }
+        }
+        return threadRoots;
     }
 
     /**

src/models/thread.ts

@@ -94,15 +94,15 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
             RoomEvent.TimelineReset,
         ]);
 
+        this.room.on(RoomEvent.LocalEchoUpdated, this.onEcho);
+        this.timelineSet.on(RoomEvent.Timeline, this.onEcho);
+
         // If we weren't able to find the root event, it's probably missing,
         // and we define the thread ID from one of the thread relation
         this.id = rootEvent?.getId() ?? opts?.initialEvents?.find(event => event.isThreadRelation)?.relationEventId;
         this.initialiseThread(this.rootEvent);
 
         opts?.initialEvents?.forEach(event => this.addEvent(event, false));
-
-        this.room.on(RoomEvent.LocalEchoUpdated, this.onEcho);
-        this.room.on(RoomEvent.Timeline, this.onEcho);
     }
 
     public static setServerSideSupport(hasServerSideSupport: boolean, useStable: boolean): void {
@@ -115,6 +115,26 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
     }
 
     private onEcho = (event: MatrixEvent) => {
+        // There is a risk that the `localTimestamp` approximation will not be accurate
+        // when threads are used over federation. That could result in the reply
+        // count value drifting away from the value returned by the server
+        const isThreadReply = event.isRelation(THREAD_RELATION_TYPE.name);
+        if (!this.lastEvent || (isThreadReply
+            && (event.getId() !== this.lastEvent.getId())
+            && (event.localTimestamp > this.lastEvent.localTimestamp))
+        ) {
+            this.lastEvent = event;
+            if (this.lastEvent.getId() !== this.id) {
+                // This counting only works when server side support is enabled as we started the counting
+                // from the value returned within the bundled relationship
+                if (Thread.hasServerSideSupport) {
+                    this.replyCount++;
+                }
+
+                this.emit(ThreadEvent.NewReply, this, event);
+            }
+        }
+
         if (this.timelineSet.eventIdToTimeline(event.getId())) {
             this.emit(ThreadEvent.Update, this);
         }
@@ -125,15 +145,6 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
     }
 
     private addEventToTimeline(event: MatrixEvent, toStartOfTimeline: boolean): void {
-        if (event.getUnsigned().transaction_id) {
-            const existingEvent = this.room.getEventForTxnId(event.getUnsigned().transaction_id);
-            if (existingEvent) {
-                // remote echo of an event we sent earlier
-                this.room.handleRemoteEcho(event, existingEvent);
-                return;
-            }
-        }
-
         if (!this.findEventById(event.getId())) {
             this.timelineSet.addEventToTimeline(
                 event,
@@ -177,33 +188,13 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
             this._currentUserParticipated = true;
         }
 
-        const isThreadReply = event.getRelation()?.rel_type === THREAD_RELATION_TYPE.name;
+        const isThreadReply = event.isRelation(THREAD_RELATION_TYPE.name);
         // If no thread support exists we want to count all thread relation
         // added as a reply. We can't rely on the bundled relationships count
         if (!Thread.hasServerSideSupport && isThreadReply) {
             this.replyCount++;
         }
 
-        // There is a risk that the `localTimestamp` approximation will not be accurate
-        // when threads are used over federation. That could results in the reply
-        // count value drifting away from the value returned by the server
-        if (!this.lastEvent || (isThreadReply
-            && (event.getId() !== this.lastEvent.getId())
-            && (event.localTimestamp > this.lastEvent.localTimestamp))
-        ) {
-            this.lastEvent = event;
-            if (this.lastEvent.getId() !== this.id) {
-                // This counting only works when server side support is enabled
-                // as we started the counting from the value returned in the
-                // bundled relationship
-                if (Thread.hasServerSideSupport) {
-                    this.replyCount++;
-                }
-
-                this.emit(ThreadEvent.NewReply, this, event);
-            }
-        }
-
         this.emit(ThreadEvent.Update, this);
     }
 

src/sync.ts

@@ -1635,36 +1635,9 @@ export class SyncApi {
         // if the timeline has any state events in it.
         // This also needs to be done before running push rules on the events as they need
         // to be decorated with sender etc.
-        const [mainTimelineEvents, threadedEvents] = this.client.partitionThreadedEvents(room, timelineEventList || []);
+        room.addLiveEvents(timelineEventList || [], null, fromCache);
-        room.addLiveEvents(mainTimelineEvents, null, fromCache);
-        await this.processThreadEvents(room, threadedEvents, false);
     }
 
-    /**
-     * @experimental
-     */
-    private processThreadEvents(
-        room: Room,
-        threadedEvents: MatrixEvent[],
-        toStartOfTimeline: boolean,
-    ): Promise<void> {
-        return this.client.processThreadEvents(room, threadedEvents, toStartOfTimeline);
-    }
-
-    // extractRelatedEvents(event: MatrixEvent, events: MatrixEvent[], relatedEvents: MatrixEvent[] = []): MatrixEvent[] {
-    //     relatedEvents.push(event);
-
-    //     const parentEventId = event.getAssociatedId();
-    //     const parentEventIndex = events.findIndex(event => event.getId() === parentEventId);
-
-    //     if (parentEventIndex > -1) {
-    //         const [relatedEvent] = events.splice(parentEventIndex, 1);
-    //         return this.extractRelatedEvents(relatedEvent, events, relatedEvents);
-    //     } else {
-    //         return relatedEvents;
-    //     }
-    // }
-
     /**
      * Takes a list of timelineEvents and adds and adds to notifEvents
      * as appropriate.