From 7b89862ed0e51ab9dc5d8987ee8844c82b4545ab Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 14 Oct 2021 16:56:48 +0100 Subject: [PATCH 1/3] Make threads use 'm.thread' relation --- src/@types/event.ts | 17 +++++++++-- src/models/event.ts | 51 ++++++++++++++++---------------- src/models/room.ts | 69 +++++++++++++------------------------------- src/models/thread.ts | 69 +++++++------------------------------------- src/sync.ts | 4 +-- 5 files changed, 73 insertions(+), 137 deletions(-) diff --git a/src/@types/event.ts b/src/@types/event.ts index f23d7e76727..581b80d4425 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -92,6 +92,12 @@ export enum EventType { export enum RelationType { Annotation = "m.annotation", Replace = "m.replace", + /** + * Note, "io.element.thread" is hardcoded + * TypeScript does not allow computed values in enums + * https://github.com/microsoft/TypeScript/issues/27976 + */ + Thread = "io.element.thread", } export enum MsgType { @@ -168,9 +174,14 @@ export const UNSTABLE_ELEMENT_FUNCTIONAL_USERS = new UnstableValue( "io.element.functional_members", "io.element.functional_members"); -export const UNSTABLE_ELEMENT_REPLY_IN_THREAD = new UnstableValue( - "m.in_thread", - "io.element.in_thread", +/** + * Note, "io.element.thread" is hardcoded in the RelationType enum + * TypeScript does not allow computed values in enums + * https://github.com/microsoft/TypeScript/issues/27976 + */ +export const UNSTABLE_ELEMENT_THREAD_RELATION = new UnstableValue( + "m.thread", + "io.element.thread", ); export interface IEncryptedFile { diff --git a/src/models/event.ts b/src/models/event.ts index 2b419fc1a92..70bc02d4b5c 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -28,7 +28,7 @@ import { EventType, MsgType, RelationType, - UNSTABLE_ELEMENT_REPLY_IN_THREAD, + UNSTABLE_ELEMENT_THREAD_RELATION, } from "../@types/event"; import { Crypto } from "../crypto"; import { deepSortedObjectEntries } from "../utils"; @@ -119,7 +119,7 @@ interface IAggregatedRelation { key?: string; } -interface IEventRelation { +export interface IEventRelation { rel_type: RelationType | string; event_id: string; key?: string; @@ -419,38 +419,39 @@ export class MatrixEvent extends EventEmitter { /** * @experimental - * Get the event ID of the replied event + * Get the event ID of the thread head */ - public get replyEventId(): string { - const relations = this.getWireContent()["m.relates_to"]; - return relations?.["m.in_reply_to"]?.["event_id"]; + public get threadRootId(): string { + const relatesTo = this.getWireContent()?.["m.relates_to"]; + if (relatesTo?.rel_type === UNSTABLE_ELEMENT_THREAD_RELATION.name) { + return relatesTo.event_id; + } } /** * @experimental - * Determines whether a reply should be rendered in a thread - * or in the main room timeline - */ - public get replyInThread(): boolean { - /** - * UNSTABLE_ELEMENT_REPLY_IN_THREAD can live either - * at the m.relates_to and m.in_reply_to level - * This will likely change once we settle on a - * way to achieve threads - * TODO: Clean this up once we have a clear way forward - */ - - const relatesTo = this.getWireContent()?.["m.relates_to"]; - const replyTo = relatesTo?.["m.in_reply_to"]; + */ + public get isThreadRelation(): boolean { + return !!this.threadRootId; + } - return relatesTo?.[UNSTABLE_ELEMENT_REPLY_IN_THREAD.name] - || (this.replyEventId && replyTo[UNSTABLE_ELEMENT_REPLY_IN_THREAD.name]) - || this.thread instanceof Thread; + /** + * @experimental + */ + public get isThreadRoot(): boolean { + const thread = this.getThread(); + return thread.id === this.getId(); } public get parentEventId(): string { - return this.replyEventId - || this.getWireContent()["m.relates_to"]?.event_id; + const relations = this.getWireContent()["m.relates_to"]; + return relations?.["m.in_reply_to"]?.["event_id"] + || relations?.event_id; + } + + public get replyEventId(): string { + const relations = this.getWireContent()["m.relates_to"]; + return relations?.["m.in_reply_to"]?.["event_id"]; } /** diff --git a/src/models/room.ts b/src/models/room.ts index 207dcd393ef..e262ea8ccf2 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -149,7 +149,7 @@ export class Room extends EventEmitter { /** * @experimental */ - public threads = new Set(); + public threads = new Map(); /** * Construct a new Room. @@ -1068,19 +1068,6 @@ export class Room extends EventEmitter { ); } - /** - * @experimental - */ - public addThread(thread: Thread): Set { - this.threads.add(thread); - if (!thread.ready) { - thread.once(ThreadEvent.Ready, this.dedupeThreads); - this.emit(ThreadEvent.Update, thread); - this.reEmitter.reEmit(thread, [ThreadEvent.Update, ThreadEvent.Ready]); - } - return this.threads; - } - /** * @experimental */ @@ -1097,26 +1084,6 @@ export class Room extends EventEmitter { return Array.from(this.threads.values()); } - /** - * Two threads starting from a different child event can end up - * with the same event root. This method ensures that the duplicates - * are removed - * @experimental - */ - private dedupeThreads = (readyThread): void => { - const deduped = Array.from(this.threads).reduce((dedupedThreads, thread) => { - if (dedupedThreads.has(thread.id)) { - dedupedThreads.get(thread.id).merge(thread); - } else { - dedupedThreads.set(thread.id, thread); - } - - return dedupedThreads; - }, new Map()); - - this.threads = new Set(deduped.values()); - }; - /** * Get a member from the current room state. * @param {string} userId The user ID of the member. @@ -1293,21 +1260,33 @@ export class Room extends EventEmitter { } } + public findThreadForEvent(event: MatrixEvent): Thread { + if (!event) { + return null; + } + if (event.isThreadRelation) { + return this.threads.get(event.threadRootId); + } else { + const parentEvent = this.findEventById(event.parentEventId); + return this.findThreadForEvent(parentEvent); + } + } + /** * Add an event to a thread's timeline. Will fire "Thread.update" * @experimental */ public addThreadedEvent(event: MatrixEvent): void { - let thread = this.findEventById(event.parentEventId)?.getThread(); + let thread = this.findThreadForEvent(event); if (thread) { thread.addEvent(event); } else { - thread = new Thread([event], this, this.client); - } - - if (!this.threads.has(thread)) { - this.addThread(thread); + const rootEvent = this.findEventById(event.threadRootId); + thread = new Thread([rootEvent, event], this, this.client); + this.reEmitter.reEmit(thread, [ThreadEvent.Update, ThreadEvent.Ready]); + this.threads.set(thread.id, thread); } + this.emit(ThreadEvent.Update, thread); } /** @@ -1409,7 +1388,7 @@ export class Room extends EventEmitter { // TODO: Enable "pending events" for threads // There's a fair few things to update to make them work with Threads // Will get back to it when the plan is to build a more polished UI ready for production - if (this.client?.supportsExperimentalThreads() && event.replyInThread) { + if (this.client?.supportsExperimentalThreads() && event.threadRootId) { return; } @@ -1585,14 +1564,6 @@ export class Room extends EventEmitter { oldEventId, oldStatus); } - public findThreadByEventId(eventId: string): Thread { - for (const thread of this.threads) { - if (thread.has(eventId)) { - return thread; - } - } - } - /** * Update the status / event id on a pending event, to reflect its transmission * progress. diff --git a/src/models/thread.ts b/src/models/thread.ts index 8aad6e4484e..fe90dd60dea 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -26,11 +26,6 @@ export enum ThreadEvent { Update = "Thread.update" } -interface ISerialisedThread { - id: string; - tails: string[]; -} - /** * @experimental */ @@ -42,7 +37,6 @@ export class Thread extends EventEmitter { /** * A reference to all the events ID at the bottom of the threads */ - public readonly tail = new Set(); public readonly timelineSet: EventTimelineSet; constructor( @@ -69,13 +63,12 @@ export class Thread extends EventEmitter { return; } - if (this.tail.has(event.replyEventId)) { - this.tail.delete(event.replyEventId); - } - this.tail.add(event.getId()); - - if (!event.replyEventId || !this.timelineSet.findEventById(event.replyEventId)) { - this.root = event.getId(); + if (!this.root) { + if (event.isThreadRelation) { + this.root = event.threadRootId; + } else { + this.root = event.getId(); + } } // all the relevant membership info to hydrate events with a sender @@ -99,31 +92,6 @@ export class Thread extends EventEmitter { this.emit(ThreadEvent.Update, this); } - /** - * Completes the reply chain with all events - * missing from the current sync data - * Will fire "Thread.ready" - */ - public async fetchReplyChain(): Promise { - if (!this.ready) { - let mxEvent = this.room.findEventById(this.rootEvent.replyEventId); - if (!mxEvent) { - mxEvent = await this.fetchEventById( - this.rootEvent.getRoomId(), - this.rootEvent.replyEventId, - ); - } - - this.addEvent(mxEvent, true); - if (mxEvent.replyEventId) { - await this.fetchReplyChain(); - } else { - await this.decryptEvents(); - this.emit(ThreadEvent.Ready, this); - } - } - } - private async decryptEvents(): Promise { await Promise.allSettled( Array.from(this.timelineSet.getLiveTimeline().getEvents()).map(event => { @@ -132,18 +100,6 @@ export class Thread extends EventEmitter { ); } - /** - * Fetches an event over the network - */ - private async fetchEventById(roomId: string, eventId: string): Promise { - const response = await this.client.http.authedRequest( - undefined, - "GET", - `/rooms/${roomId}/event/${eventId}`, - ); - return new MatrixEvent(response); - } - /** * Finds an event by ID in the current thread */ @@ -155,7 +111,7 @@ export class Thread extends EventEmitter { * Determines thread's ready status */ public get ready(): boolean { - return this.rootEvent.replyEventId === undefined; + return this.rootEvent !== undefined; } /** @@ -217,29 +173,26 @@ export class Thread extends EventEmitter { return this.timelineSet.findEventById(eventId) instanceof MatrixEvent; } - public toJson(): ISerialisedThread { - return { - id: this.id, - tails: Array.from(this.tail), - }; - } - public on(event: ThreadEvent, listener: (...args: any[]) => void): this { super.on(event, listener); return this; } + public once(event: ThreadEvent, listener: (...args: any[]) => void): this { super.once(event, listener); return this; } + public off(event: ThreadEvent, listener: (...args: any[]) => void): this { super.off(event, listener); return this; } + public addListener(event: ThreadEvent, listener: (...args: any[]) => void): this { super.addListener(event, listener); return this; } + public removeListener(event: ThreadEvent, listener: (...args: any[]) => void): this { super.removeListener(event, listener); return this; diff --git a/src/sync.ts b/src/sync.ts index 4bc08f07d53..b89b257021a 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -319,13 +319,13 @@ export class SyncApi { // An event should live in the thread timeline if // - It's a reply in thread event // - It's related to a reply in thread event - let shouldLiveInThreadTimeline = event.replyInThread; + let shouldLiveInThreadTimeline = event.isThreadRelation; if (!shouldLiveInThreadTimeline) { const parentEventId = event.parentEventId; const parentEvent = room?.findEventById(parentEventId) || events.find((mxEv: MatrixEvent) => { return mxEv.getId() === parentEventId; }); - shouldLiveInThreadTimeline = parentEvent?.replyInThread; + shouldLiveInThreadTimeline = parentEvent?.isThreadRelation; } memo[shouldLiveInThreadTimeline ? 1 : 0].push(event); return memo; From 9ceeb2d220acb0acdccd98cb57bbb960f338c19e Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 14 Oct 2021 17:09:00 +0100 Subject: [PATCH 2/3] Delete UnstableValue and use hardcoded value for m.thread --- src/@types/event.ts | 14 +++----------- src/models/event.ts | 3 +-- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/@types/event.ts b/src/@types/event.ts index 581b80d4425..b7ecb83390f 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -94,7 +94,9 @@ export enum RelationType { Replace = "m.replace", /** * Note, "io.element.thread" is hardcoded - * TypeScript does not allow computed values in enums + * Should be replaced with "m.thread" once MSC3440 lands + * Can not use `UnstableValue` as TypeScript does not + * allow computed values in enums * https://github.com/microsoft/TypeScript/issues/27976 */ Thread = "io.element.thread", @@ -174,16 +176,6 @@ export const UNSTABLE_ELEMENT_FUNCTIONAL_USERS = new UnstableValue( "io.element.functional_members", "io.element.functional_members"); -/** - * Note, "io.element.thread" is hardcoded in the RelationType enum - * TypeScript does not allow computed values in enums - * https://github.com/microsoft/TypeScript/issues/27976 - */ -export const UNSTABLE_ELEMENT_THREAD_RELATION = new UnstableValue( - "m.thread", - "io.element.thread", -); - export interface IEncryptedFile { url: string; mimetype?: string; diff --git a/src/models/event.ts b/src/models/event.ts index 70bc02d4b5c..b6e41982f5b 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -28,7 +28,6 @@ import { EventType, MsgType, RelationType, - UNSTABLE_ELEMENT_THREAD_RELATION, } from "../@types/event"; import { Crypto } from "../crypto"; import { deepSortedObjectEntries } from "../utils"; @@ -423,7 +422,7 @@ export class MatrixEvent extends EventEmitter { */ public get threadRootId(): string { const relatesTo = this.getWireContent()?.["m.relates_to"]; - if (relatesTo?.rel_type === UNSTABLE_ELEMENT_THREAD_RELATION.name) { + if (relatesTo?.rel_type === RelationType.Thread) { return relatesTo.event_id; } } From efbc46937f296a4402bf684c97c64c0f7a12480e Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 14 Oct 2021 17:37:33 +0100 Subject: [PATCH 3/3] Null-guard for event that do not have a thread --- src/models/event.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/event.ts b/src/models/event.ts index b6e41982f5b..05c97ae1556 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -439,7 +439,7 @@ export class MatrixEvent extends EventEmitter { */ public get isThreadRoot(): boolean { const thread = this.getThread(); - return thread.id === this.getId(); + return thread?.id === this.getId(); } public get parentEventId(): string {