From dde4285cdf5f7a2e49bd90a39b690c8c38ac8fab Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Apr 2022 13:46:50 +0100 Subject: [PATCH] Fix handling of threaded messages around edits & echoes (#2267) --- spec/integ/matrix-client-methods.spec.js | 27 +- spec/test-utils/test-utils.ts | 1 + spec/unit/matrix-client.spec.ts | 2 +- spec/unit/room.spec.ts | 324 +++++++++++++---------- src/client.ts | 74 +----- src/event-mapper.ts | 5 +- src/models/event-timeline-set.ts | 4 +- src/models/event.ts | 5 - src/models/relations.ts | 2 +- src/models/room.ts | 189 ++++++++++--- src/models/thread.ts | 57 ++-- src/sync.ts | 29 +- 12 files changed, 398 insertions(+), 321 deletions(-) diff --git a/spec/integ/matrix-client-methods.spec.js b/spec/integ/matrix-client-methods.spec.js index 7b6a7be0a..b56744cba 100644 --- a/spec/integ/matrix-client-methods.spec.js +++ b/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, diff --git a/spec/test-utils/test-utils.ts b/spec/test-utils/test-utils.ts index 24f7b9966..ab6b7d710 100644 --- a/spec/test-utils/test-utils.ts +++ b/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) { diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index c49bdccc6..b363daa23 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/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); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index faa73ba29..8cf41b5f5 100644 --- a/spec/unit/room.spec.ts +++ b/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", - function() { - let callCount = 0; - room.on("Room.timeline", function(event, emitRoom, toStart) { - callCount += 1; - expect(room.timeline.length).toEqual(callCount); - expect(event).toEqual(events[callCount - 1]); - expect(emitRoom).toEqual(room); - expect(toStart).toBeFalsy(); - }); - room.addLiveEvents(events); - expect(callCount).toEqual(2); + it("should emit 'Room.timeline' events", function() { + let callCount = 0; + room.on("Room.timeline", function(event, emitRoom, toStart) { + callCount += 1; + expect(room.timeline.length).toEqual(callCount); + expect(event).toEqual(events[callCount - 1]); + expect(emitRoom).toEqual(room); + expect(toStart).toBeFalsy(); }); + 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", - function() { - const sentinel = { - userId: userA, - membership: "join", - name: "Alice", - }; - const oldSentinel = { - userId: userA, - membership: "join", - name: "Old Alice", - }; - room.currentState.getSentinelMember.mockImplementation(function(uid) { - if (uid === userA) { - return sentinel; - } - 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); + it("should set event.target for new and old m.room.member events", function() { + const sentinel = { + userId: userA, + membership: "join", + name: "Alice", + }; + const oldSentinel = { + userId: userA, + membership: "join", + name: "Old Alice", + }; + room.currentState.getSentinelMember.mockImplementation(function(uid) { + if (uid === userA) { + return sentinel; + } + 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); + }); 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") + - " old timelines", function() { + it("should " + (timelineSupport ? "remember" : "forget") + " 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 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 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; + const client = new TestClient(userA).client; + client.supportsExperimentalThreads = () => true; + const room = new Room(roomId, client, userA); it("thread root and its relations&redactions should be in both", () => { const randomMessage = mkMessage(); diff --git a/src/client.ts b/src/client.ts index 11ec7fa4d..982feb2bf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3771,9 +3771,8 @@ export class MatrixClient extends TypedEventEmitter { - const threadRoots = new Set(); - 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 { - for (const event of threadedEvents) { - await room.addThreadedEvent(event, toStartOfTimeline); - } + await room.processThreadedEvents(threadedEvents, toStartOfTimeline); } /** diff --git a/src/event-mapper.ts b/src/event-mapper.ts index 7b4f106f8..5349a565c 100644 --- a/src/event-mapper.ts +++ b/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())) { - event.setThread(room.threads.get(event.getId())); + const thread = room?.findThreadForEvent(event); + if (thread) { + event.setThread(thread); } if (event.isEncrypted()) { diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 1fda0d977..13ea8c458 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -775,7 +775,7 @@ export class EventTimelineSet extends TypedEventEmitter(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()) { diff --git a/src/models/room.ts b/src/models/room.ts index 4a8335989..35945e71f 100644 --- a/src/models/room.ts +++ b/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 private getTypeWarning = false; private getVersionWarning = false; private membersPromise?: Promise; + // Map from threadId to pending Thread instance created by createThreadFetchRoot + private threadPromises = new Map>(); // XXX: These should be read-only /** @@ -1567,6 +1569,13 @@ export class Room extends TypedEventEmitter 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 return { shouldLiveInRoom: false, shouldLiveInThread: true, - threadId: event.relationEventId, + threadId: event.threadRootId, }; } @@ -1630,21 +1639,23 @@ export class Room extends TypedEventEmitter threadId: string, events?: MatrixEvent[], toStartOfTimeline?: boolean, - ): Promise { + ): Promise { let thread = this.getThread(threadId); if (!thread) { + const deferred = defer(); + 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); - - if (!rootEvent) { - rootEvent = new MatrixEvent(eventData); - } else { - rootEvent.setUnsigned(eventData.unsigned); - } + const mapper = this.client.getEventMapper(); + rootEvent = mapper(eventData); // will merge with existing event object if such is known + } catch (e) { + logger.error("Failed to fetch thread root to construct thread with", e); } 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 if (thread) { rootEvent.setThread(thread); } + deferred.resolve(thread); } } return thread; } - /** - * Add an event to a thread's timeline. Will fire "Thread.update" - * @experimental - */ - public async addThreadedEvent(event: MatrixEvent, toStartOfTimeline: boolean): Promise { - this.applyRedaction(event); - let thread = this.findThreadForEvent(event); - if (thread) { - await thread.addEvent(event, toStartOfTimeline); - } else { - thread = await this.createThreadFetchRoot(event.threadRootId, [event], toStartOfTimeline); + private async addThreadedEvents(events: MatrixEvent[], threadId: string, toStartOfTimeline = false): Promise { + let thread = this.getThread(threadId); + if (this.threadPromises.has(threadId)) { + thread = await this.threadPromises.get(threadId); } - 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 { + 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 } } - 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 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 // clients can say "so and so redacted an event" if they wish to. Also // this may be needed to trigger an update. } - } + }; - /** - * 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 { + private processLiveEvent(event: MatrixEvent): Promise { this.applyRedaction(event); // Implement MSC3531: hiding messages. @@ -1804,7 +1830,19 @@ export class Room extends TypedEventEmitter 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 const newEventId = remoteEvent.getId(); const oldStatus = localEvent.status; - logger.debug( - `Got remote echo for event ${oldEventId} -> ${newEventId} ` + - `old status ${oldStatus}`, - ); + logger.debug(`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 } } + 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 { + const threadRoots = new Set(); + for (const event of events) { + if (event.isThreadRelation) { + threadRoots.add(event.relationEventId); + } + } + return threadRoots; } /** diff --git a/src/models/thread.ts b/src/models/thread.ts index 7879cc89f..3b13c1be8 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -94,15 +94,15 @@ export class Thread extends TypedEventEmitter { 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 { } 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 { } 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 { 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); } diff --git a/src/sync.ts b/src/sync.ts index 337a2593d..c6e36e0f9 100644 --- a/src/sync.ts +++ b/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(mainTimelineEvents, null, fromCache); - await this.processThreadEvents(room, threadedEvents, false); + room.addLiveEvents(timelineEventList || [], null, fromCache); } - /** - * @experimental - */ - private processThreadEvents( - room: Room, - threadedEvents: MatrixEvent[], - toStartOfTimeline: boolean, - ): Promise { - 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.