From 70afd8c82f413d477c822ef0b6763a548e2c5938 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 29 Apr 2022 15:15:12 +0100 Subject: [PATCH 1/5] Update room thread access pattern --- spec/unit/matrix-client.spec.ts | 4 +--- src/client.ts | 4 ++-- src/models/room.ts | 6 ++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 680e14ed5eb..7b6622af4fd 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -811,9 +811,7 @@ describe("MatrixClient", function() { } }, }, - threads: { - get: jest.fn(), - }, + getThread: jest.fn(), addPendingEvent: jest.fn(), updatePendingEvent: jest.fn(), reEmitter: { diff --git a/src/client.ts b/src/client.ts index ed747c600bc..91b35f29d7d 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3739,7 +3739,7 @@ export class MatrixClient extends TypedEventEmitter { @@ -3788,7 +3788,7 @@ export class MatrixClient extends TypedEventEmitter /** * @experimental */ - public threads = new Map(); + private threads = new Map(); public lastThread: Thread; /** @@ -1208,9 +1208,7 @@ export class Room extends TypedEventEmitter * @experimental */ public getThread(eventId: string): Thread { - return this.getThreads().find(thread => { - return thread.id === eventId; - }); + return this.threads.get(eventId); } /** From 30df7e41901ba42e1f5f9b981cc690a878e4961f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 29 Apr 2022 15:20:49 +0100 Subject: [PATCH 2/5] Switch so synchronous yet lazy instantiation of threads --- spec/unit/room.spec.ts | 37 +++++++---- src/client.ts | 42 ++++++------ src/models/room.ts | 148 ++++++++++++++--------------------------- src/models/thread.ts | 103 +++++++++++++++++----------- 4 files changed, 157 insertions(+), 173 deletions(-) diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 00a3cd0438c..27201a8910c 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -1914,7 +1914,7 @@ describe("Room", function() { }, }); - room.createThread(undefined, [eventWithoutARootEvent]); + room.createThread("$000", undefined, [eventWithoutARootEvent]); const rootEvent = new MatrixEvent({ event_id: "$666", @@ -1932,7 +1932,7 @@ describe("Room", function() { }, }); - expect(() => room.createThread(rootEvent, [])).not.toThrow(); + expect(() => room.createThread(rootEvent.getId(), rootEvent, [])).not.toThrow(); }); it("Edits update the lastReply event", async () => { @@ -1959,14 +1959,16 @@ describe("Room", function() { }, }); + let prom = emitPromise(room, ThreadEvent.New); room.addLiveEvents([randomMessage, threadRoot, threadResponse]); - const thread = await emitPromise(room, ThreadEvent.New); + const thread = await prom; expect(thread.replyToEvent).toBe(threadResponse); expect(thread.replyToEvent.getContent().body).toBe(threadResponse.getContent().body); + prom = emitPromise(thread, ThreadEvent.Update); room.addLiveEvents([threadResponseEdit]); - await emitPromise(thread, ThreadEvent.Update); + await prom; expect(thread.replyToEvent.getContent().body).toBe(threadResponseEdit.getContent()["m.new_content"].body); }); @@ -1993,15 +1995,17 @@ describe("Room", function() { }, }); + let prom = emitPromise(room, ThreadEvent.New); room.addLiveEvents([threadRoot, threadResponse1, threadResponse2]); - const thread = await emitPromise(room, ThreadEvent.New); + const thread = await prom; expect(thread).toHaveLength(2); expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); + prom = emitPromise(thread, ThreadEvent.Update); const threadResponse1Redaction = mkRedaction(threadResponse1); room.addLiveEvents([threadResponse1Redaction]); - await emitPromise(thread, ThreadEvent.Update); + await prom; expect(thread).toHaveLength(1); expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); }); @@ -2030,15 +2034,17 @@ describe("Room", function() { }, }); + let prom = emitPromise(room, ThreadEvent.New); room.addLiveEvents([threadRoot, threadResponse1, threadResponse2, threadResponse2Reaction]); - const thread = await emitPromise(room, ThreadEvent.New); + const thread = await prom; expect(thread).toHaveLength(2); expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); + prom = emitPromise(thread, ThreadEvent.Update); const threadResponse2ReactionRedaction = mkRedaction(threadResponse2Reaction); room.addLiveEvents([threadResponse2ReactionRedaction]); - await emitPromise(thread, ThreadEvent.Update); + await prom; expect(thread).toHaveLength(2); expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); }); @@ -2067,15 +2073,17 @@ describe("Room", function() { }, }); + let prom = emitPromise(room, ThreadEvent.New); room.addLiveEvents([threadRoot, threadResponse1, threadResponse2, threadResponse2Reaction]); - const thread = await emitPromise(room, ThreadEvent.New); + const thread = await prom; expect(thread).toHaveLength(2); expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); + prom = emitPromise(room, ThreadEvent.Update); const threadRootRedaction = mkRedaction(threadRoot); room.addLiveEvents([threadRootRedaction]); - await emitPromise(thread, ThreadEvent.Update); + await prom; expect(thread).toHaveLength(2); }); @@ -2102,21 +2110,24 @@ describe("Room", function() { }, }); + let prom = emitPromise(room, ThreadEvent.New); room.addLiveEvents([threadRoot, threadResponse1, threadResponse2]); - const thread = await emitPromise(room, ThreadEvent.New); + const thread = await prom; expect(thread).toHaveLength(2); expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); + prom = emitPromise(room, ThreadEvent.Update); const threadResponse2Redaction = mkRedaction(threadResponse2); room.addLiveEvents([threadResponse2Redaction]); - await emitPromise(thread, ThreadEvent.Update); + await prom; expect(thread).toHaveLength(1); expect(thread.replyToEvent.getId()).toBe(threadResponse1.getId()); + prom = emitPromise(room, ThreadEvent.Update); const threadResponse1Redaction = mkRedaction(threadResponse1); room.addLiveEvents([threadResponse1Redaction]); - await emitPromise(thread, ThreadEvent.Update); + await prom; expect(thread).toHaveLength(0); expect(thread.replyToEvent.getId()).toBe(threadRoot.getId()); }); diff --git a/src/client.ts b/src/client.ts index 91b35f29d7d..7ee15d837d9 100644 --- a/src/client.ts +++ b/src/client.ts @@ -48,7 +48,9 @@ import { IRoomEncryption, RoomList } from './crypto/RoomList'; import { logger } from './logger'; import { SERVICE_TYPES } from './service-types'; import { - FileType, HttpApiEvent, HttpApiEventHandlerMap, + FileType, + HttpApiEvent, + HttpApiEventHandlerMap, IHttpOpts, IUpload, MatrixError, @@ -5183,7 +5185,7 @@ export class MatrixClient extends TypedEventEmitter { + }).then((res: IMessagesResponse) => { const matrixEvents = res.chunk.map(this.getEventMapper()); if (res.state) { const stateEvents = res.state.map(this.getEventMapper()); @@ -5194,7 +5196,7 @@ export class MatrixClient extends TypedEventEmitter { + ).then((res) => { if (res.state) { const roomState = eventTimeline.getState(dir); const stateEvents = res.state.map(this.getEventMapper()); @@ -5504,7 +5508,7 @@ export class MatrixClient extends TypedEventEmitter { const queryString = utils.encodeParams(opts as Record); @@ -8914,12 +8918,8 @@ export class MatrixClient extends TypedEventEmitter { - await room.processThreadedEvents(threadedEvents, toStartOfTimeline); + public processThreadEvents(room: Room, threadedEvents: MatrixEvent[], toStartOfTimeline: boolean): void { + room.processThreadedEvents(threadedEvents, toStartOfTimeline); } public processBeaconEvents( diff --git a/src/models/room.ts b/src/models/room.ts index 71e11973fbd..44347b55f0f 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 { defer, normalize } from "../utils"; +import { normalize } from "../utils"; import { IEvent, IThreadBundledRelationship, MatrixEvent, MatrixEventEvent, MatrixEventHandlerMap } from "./event"; import { EventStatus } from "./event-status"; import { RoomMember } from "./room-member"; @@ -214,8 +214,6 @@ 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 /** @@ -1522,7 +1520,7 @@ export class Room extends TypedEventEmitter } if (!this.getThread(rootEvent.getId())) { - this.createThread(rootEvent, [], true); + this.createThread(rootEvent.getId(), rootEvent, [], true); } } @@ -1618,58 +1616,14 @@ export class Room extends TypedEventEmitter return threadId ? this.getThread(threadId) : null; } - public async createThreadFetchRoot( - threadId: string, - events?: MatrixEvent[], - toStartOfTimeline?: boolean, - ): 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); - 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 - thread = this.createThread(rootEvent, events, toStartOfTimeline); - if (thread) { - rootEvent?.setThread(thread); - } - deferred.resolve(thread); - } - } - - return thread; - } - - private async addThreadedEvents(events: MatrixEvent[], threadId: string, toStartOfTimeline = false): Promise { + private addThreadedEvents(threadId: string, events: MatrixEvent[], toStartOfTimeline = false): void { let thread = this.getThread(threadId); - if (this.threadPromises.has(threadId)) { - thread = await this.threadPromises.get(threadId); - } - - events = events.filter(e => e.getId() !== threadId); // filter out any root events if (thread) { - for (const event of events) { - await thread.addEvent(event, toStartOfTimeline); - } + thread.addEvents(events, toStartOfTimeline); } else { - thread = await this.createThreadFetchRoot(threadId, events, toStartOfTimeline); - } - - if (thread) { + const rootEvent = events.find(e => e.getId() === threadId); + thread = this.createThread(threadId, rootEvent, events, toStartOfTimeline); this.emit(ThreadEvent.Update, thread); } } @@ -1678,30 +1632,29 @@ export class Room extends TypedEventEmitter * Adds events to a thread's timeline. Will fire "Thread.update" * @experimental */ - public async processThreadedEvents(events: MatrixEvent[], toStartOfTimeline: boolean): Promise { + public processThreadedEvents(events: MatrixEvent[], toStartOfTimeline: boolean): void { events.forEach(this.applyRedaction); const eventsByThread: { [threadId: string]: MatrixEvent[] } = {}; for (const event of events) { const { threadId, shouldLiveInThread } = this.eventShouldLiveIn(event); - if (shouldLiveInThread) { - if (!eventsByThread[threadId]) { - eventsByThread[threadId] = []; - } - eventsByThread[threadId].push(event); + if (shouldLiveInThread && !eventsByThread[threadId]) { + eventsByThread[threadId] = []; } + eventsByThread[threadId]?.push(event); } - return Promise.all(Object.entries(eventsByThread).map(([threadId, events]) => ( - this.addThreadedEvents(events, threadId, toStartOfTimeline) - ))); + Object.entries(eventsByThread).map(([threadId, events]) => ( + this.addThreadedEvents(threadId, events, toStartOfTimeline) + )); } public createThread( + threadId: string, rootEvent: MatrixEvent | undefined, events: MatrixEvent[] = [], toStartOfTimeline: boolean, - ): Thread | undefined { + ): Thread { if (rootEvent) { const tl = this.getTimelineForEvent(rootEvent.getId()); const relatedEvents = tl?.getTimelineSet().getAllRelationsEventForEvent(rootEvent.getId()); @@ -1710,45 +1663,44 @@ export class Room extends TypedEventEmitter } } - const thread = new Thread(rootEvent, { + const thread = new Thread(threadId, rootEvent, { initialEvents: events, room: this, client: this.client, }); + // If we managed to create a thread and figure out its `id` then we can use it - if (thread.id) { - this.threads.set(thread.id, thread); - this.reEmitter.reEmit(thread, [ - ThreadEvent.Update, - ThreadEvent.NewReply, - RoomEvent.Timeline, - RoomEvent.TimelineReset, - ]); - - if (!this.lastThread || this.lastThread.rootEvent?.localTimestamp < rootEvent?.localTimestamp) { - this.lastThread = thread; - } + this.threads.set(thread.id, thread); + this.reEmitter.reEmit(thread, [ + ThreadEvent.Update, + ThreadEvent.NewReply, + RoomEvent.Timeline, + RoomEvent.TimelineReset, + ]); - this.emit(ThreadEvent.New, thread, toStartOfTimeline); - - if (this.threadsReady) { - this.threadsTimelineSets.forEach(timelineSet => { - if (thread.rootEvent) { - if (Thread.hasServerSideSupport) { - timelineSet.addLiveEvent(thread.rootEvent); - } else { - timelineSet.addEventToTimeline( - thread.rootEvent, - timelineSet.getLiveTimeline(), - toStartOfTimeline, - ); - } - } - }); - } + if (!this.lastThread || this.lastThread.rootEvent?.localTimestamp < rootEvent?.localTimestamp) { + this.lastThread = thread; + } + + this.emit(ThreadEvent.New, thread, toStartOfTimeline); - return thread; + if (this.threadsReady) { + this.threadsTimelineSets.forEach(timelineSet => { + if (thread.rootEvent) { + if (Thread.hasServerSideSupport) { + timelineSet.addLiveEvent(thread.rootEvent); + } else { + timelineSet.addEventToTimeline( + thread.rootEvent, + timelineSet.getLiveTimeline(), + toStartOfTimeline, + ); + } + } + }); } + + return thread; } private applyRedaction = (event: MatrixEvent): void => { @@ -2202,12 +2154,10 @@ export class Room extends TypedEventEmitter threadId, } = threadInfos[i]; - if (shouldLiveInThread) { - if (!eventsByThread[threadId]) { - eventsByThread[threadId] = []; - } - eventsByThread[threadId].push(events[i]); + if (shouldLiveInThread && !eventsByThread[threadId]) { + eventsByThread[threadId] = []; } + eventsByThread[threadId]?.push(events[i]); if (shouldLiveInRoom) { this.addLiveEvent(events[i], duplicateStrategy, fromCache); @@ -2215,7 +2165,7 @@ export class Room extends TypedEventEmitter } Object.entries(eventsByThread).forEach(([threadId, threadEvents]) => { - this.addThreadedEvents(threadEvents, threadId, false); + this.addThreadedEvents(threadId, threadEvents, false); }); } diff --git a/src/models/thread.ts b/src/models/thread.ts index 132784effdc..57c17a13b07 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -70,12 +70,11 @@ export class Thread extends TypedEventEmitter { public readonly room: Room; public readonly client: MatrixClient; - public initialEventsFetched = false; - - public readonly id: string; + public initialEventsFetched = !Thread.hasServerSideSupport; constructor( - public readonly rootEvent: MatrixEvent | undefined, + public readonly id: string, + public rootEvent: MatrixEvent | undefined, opts: IThreadOpts, ) { super(); @@ -99,12 +98,34 @@ export class Thread extends TypedEventEmitter { 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); + if (opts.initialEvents) { + this.addEvents(opts.initialEvents, false); + } + // even if this thread is thought to be originating from this client, we initialise it as we may be in a + // gappy sync and a thread around this event may already exist. + this.initialiseThread(); + + this.rootEvent?.setThread(this); + } + + private async fetchRootEvent(): Promise { + this.rootEvent = this.room.findEventById(this.id); + // 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, this.id); + const mapper = this.client.getEventMapper(); + this.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); + } + + // 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 + this.rootEvent?.setThread(this); + // TODO test case without rootEvent visible - opts?.initialEvents?.forEach(event => this.addEvent(event, false)); + this.emit(ThreadEvent.Update, this); } public static setServerSideSupport(hasServerSideSupport: boolean, useStable: boolean): void { @@ -180,6 +201,11 @@ export class Thread extends TypedEventEmitter { } } + public addEvents(events: MatrixEvent[], toStartOfTimeline: boolean): void { + events.forEach(ev => this.addEvent(ev, toStartOfTimeline, false)); + this.emit(ThreadEvent.Update, this); + } + /** * Add an event to the thread and updates * the tail/root references if needed @@ -188,23 +214,23 @@ export class Thread extends TypedEventEmitter { * @param {boolean} toStartOfTimeline whether the event is being added * to the start (and not the end) of the timeline. */ - public async addEvent(event: MatrixEvent, toStartOfTimeline: boolean): Promise { + public addEvent(event: MatrixEvent, toStartOfTimeline: boolean, emit = true): void { + event.setThread(this); + // Add all incoming events to the thread's timeline set when there's no server support if (!Thread.hasServerSideSupport) { // all the relevant membership info to hydrate events with a sender // is held in the main room timeline // We want to fetch the room state from there and pass it down to this thread // timeline set to let it reconcile an event with its relevant RoomMember - - event.setThread(this); this.addEventToTimeline(event, toStartOfTimeline); - await this.client.decryptEventIfNeeded(event, {}); + this.client.decryptEventIfNeeded(event, {}); } else if (!toStartOfTimeline && this.initialEventsFetched && - event.localTimestamp > this.lastReply().localTimestamp + event.localTimestamp > this.lastReply()?.localTimestamp ) { - await this.fetchEditsWhereNeeded(event); + this.fetchEditsWhereNeeded(event); this.addEventToTimeline(event, false); } @@ -218,12 +244,21 @@ export class Thread extends TypedEventEmitter { this.replyCount++; } - this.emit(ThreadEvent.Update, this); + if (emit) { + this.emit(ThreadEvent.Update, this); + } } - private initialiseThread(rootEvent: MatrixEvent | undefined): void { - const bundledRelationship = rootEvent - ?.getServerAggregatedRelation(THREAD_RELATION_TYPE.name); + private getRootEventBundledRelationship(rootEvent = this.rootEvent): IThreadBundledRelationship { + return rootEvent?.getServerAggregatedRelation(THREAD_RELATION_TYPE.name); + } + + private async initialiseThread(): Promise { + let bundledRelationship = this.getRootEventBundledRelationship(); + if (Thread.hasServerSideSupport && !bundledRelationship) { + await this.fetchRootEvent(); + bundledRelationship = this.getRootEventBundledRelationship(); + } if (Thread.hasServerSideSupport && bundledRelationship) { this.replyCount = bundledRelationship.count; @@ -236,6 +271,8 @@ export class Thread extends TypedEventEmitter { this.fetchEditsWhereNeeded(event); } + + this.emit(ThreadEvent.Update, this); } // XXX: Workaround for https://github.com/matrix-org/matrix-spec-proposals/pull/2676/files#r827240084 @@ -253,24 +290,10 @@ export class Thread extends TypedEventEmitter { })); } - public async fetchInitialEvents(): Promise<{ - originalEvent: MatrixEvent; - events: MatrixEvent[]; - nextBatch?: string; - prevBatch?: string; - } | null> { - if (!Thread.hasServerSideSupport) { - this.initialEventsFetched = true; - return null; - } - - try { - const response = await this.fetchEvents(); - this.initialEventsFetched = true; - return response; - } catch (e) { - return null; - } + public async fetchInitialEvents(): Promise { + if (this.initialEventsFetched) return; + await this.fetchEvents(); + this.initialEventsFetched = true; } private setEventMetadata(event: MatrixEvent): void { @@ -319,7 +342,7 @@ export class Thread extends TypedEventEmitter { * A getter for the last event added to the thread */ public get replyToEvent(): MatrixEvent { - return this.lastEvent; + return this.lastEvent ?? this.lastReply(); } public get events(): MatrixEvent[] { @@ -338,7 +361,7 @@ export class Thread extends TypedEventEmitter { return this.timelineSet.getLiveTimeline(); } - public async fetchEvents(opts: IRelationsRequestOpts = { limit: 20 }): Promise<{ + public async fetchEvents(opts: IRelationsRequestOpts = { limit: 20, direction: Direction.Backward }): Promise<{ originalEvent: MatrixEvent; events: MatrixEvent[]; nextBatch?: string; @@ -370,7 +393,7 @@ export class Thread extends TypedEventEmitter { return this.client.decryptEventIfNeeded(event); })); - const prependEvents = !opts.direction || opts.direction === Direction.Backward; + const prependEvents = (opts.direction ?? Direction.Backward) === Direction.Backward; this.timelineSet.addEventsToTimeline( events, From 7a4ad6a1a7fc03e22146ac8b8befc1839d5ee1f9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 3 May 2022 12:04:23 +0100 Subject: [PATCH 3/5] Fix relations handling in server-supported threads --- src/models/event-timeline-set.ts | 3 +-- src/models/event.ts | 3 +-- src/models/room.ts | 5 ++--- src/models/thread.ts | 14 ++++++++++---- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 13ea8c458f2..d7396278132 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -852,14 +852,13 @@ export class EventTimelineSet extends TypedEventEmitter if (thread) { thread.addEvents(events, toStartOfTimeline); } else { - const rootEvent = events.find(e => e.getId() === threadId); + const rootEvent = this.findEventById(threadId) ?? events.find(e => e.getId() === threadId); thread = this.createThread(threadId, rootEvent, events, toStartOfTimeline); this.emit(ThreadEvent.Update, thread); } @@ -2141,7 +2141,6 @@ 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++) { @@ -2152,7 +2151,7 @@ export class Room extends TypedEventEmitter shouldLiveInRoom, shouldLiveInThread, threadId, - } = threadInfos[i]; + } = this.eventShouldLiveIn(events[i], events, threadRoots); if (shouldLiveInThread && !eventsByThread[threadId]) { eventsByThread[threadId] = []; diff --git a/src/models/thread.ts b/src/models/thread.ts index 57c17a13b07..f4beb633337 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -217,6 +217,16 @@ export class Thread extends TypedEventEmitter { public addEvent(event: MatrixEvent, toStartOfTimeline: boolean, emit = true): void { event.setThread(this); + if (!this._currentUserParticipated && event.getSender() === this.client.getUserId()) { + this._currentUserParticipated = true; + } + + // Add all annotations and replace relations to the timeline so that the relations are processed accordingly + if ([RelationType.Annotation, RelationType.Replace].includes(event.getRelation()?.rel_type as RelationType)) { + this.addEventToTimeline(event, toStartOfTimeline); + return; + } + // Add all incoming events to the thread's timeline set when there's no server support if (!Thread.hasServerSideSupport) { // all the relevant membership info to hydrate events with a sender @@ -234,10 +244,6 @@ export class Thread extends TypedEventEmitter { this.addEventToTimeline(event, false); } - if (!this._currentUserParticipated && event.getSender() === this.client.getUserId()) { - this._currentUserParticipated = true; - } - // 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 && event.isRelation(THREAD_RELATION_TYPE.name)) { From 0b1c8ad1da282527d3d26cc40005e957a88b12f1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 3 May 2022 12:04:55 +0100 Subject: [PATCH 4/5] Add comment --- src/models/thread.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/models/thread.ts b/src/models/thread.ts index f4beb633337..7796efc73d6 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -213,6 +213,7 @@ export class Thread extends TypedEventEmitter { * @param event The event to add * @param {boolean} toStartOfTimeline whether the event is being added * to the start (and not the end) of the timeline. + * @param {boolean} emit whether to emit the Update event if the thread was updated or not. */ public addEvent(event: MatrixEvent, toStartOfTimeline: boolean, emit = true): void { event.setThread(this); From 4e318374886dc4196c3a2abce736e15793c71df7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 3 May 2022 12:55:58 +0100 Subject: [PATCH 5/5] Add tests --- spec/unit/room.spec.ts | 42 +++++++++++++++++++++++++++++++++++++++++- src/models/thread.ts | 3 +-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 27201a8910c..abf839a15a0 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -36,7 +36,7 @@ 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"; +import { Thread, ThreadEvent } from "../../src/models/thread"; describe("Room", function() { const roomId = "!foo:bar"; @@ -2245,5 +2245,45 @@ describe("Room", function() { expect(room.eventShouldLiveIn(reply2, events, roots).shouldLiveInRoom).toBeTruthy(); expect(room.eventShouldLiveIn(reply2, events, roots).shouldLiveInThread).toBeFalsy(); }); + + it("should aggregate relations in thread event timeline set", () => { + Thread.setServerSideSupport(true, true); + const threadRoot = mkMessage(); + const rootReaction = mkReaction(threadRoot); + const threadResponse = mkThreadResponse(threadRoot); + const threadReaction = mkReaction(threadResponse); + + const events = [ + threadRoot, + rootReaction, + threadResponse, + threadReaction, + ]; + + room.addLiveEvents(events); + + const thread = threadRoot.getThread(); + expect(thread.rootEvent).toBe(threadRoot); + + const rootRelations = thread.timelineSet.getRelationsForEvent( + threadRoot.getId(), + RelationType.Annotation, + EventType.Reaction, + ).getSortedAnnotationsByKey(); + expect(rootRelations).toHaveLength(1); + expect(rootRelations[0][0]).toEqual(rootReaction.getRelation().key); + expect(rootRelations[0][1].size).toEqual(1); + expect(rootRelations[0][1].has(rootReaction)).toBeTruthy(); + + const responseRelations = thread.timelineSet.getRelationsForEvent( + threadResponse.getId(), + RelationType.Annotation, + EventType.Reaction, + ).getSortedAnnotationsByKey(); + expect(responseRelations).toHaveLength(1); + expect(responseRelations[0][0]).toEqual(threadReaction.getRelation().key); + expect(responseRelations[0][1].size).toEqual(1); + expect(responseRelations[0][1].has(threadReaction)).toBeTruthy(); + }); }); }); diff --git a/src/models/thread.ts b/src/models/thread.ts index 7796efc73d6..eb2f5c40e33 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -123,7 +123,6 @@ export class Thread extends TypedEventEmitter { // 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 this.rootEvent?.setThread(this); - // TODO test case without rootEvent visible this.emit(ThreadEvent.Update, this); } @@ -247,7 +246,7 @@ export class Thread extends TypedEventEmitter { // 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 && event.isRelation(THREAD_RELATION_TYPE.name)) { + if ((!Thread.hasServerSideSupport || !this.rootEvent) && event.isRelation(THREAD_RELATION_TYPE.name)) { this.replyCount++; }