Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make threads use 'm.thread' relation #1980

Merged
merged 3 commits into from
Oct 15, 2021
Merged

Make threads use 'm.thread' relation #1980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b89862
Make threads use 'm.thread' relation
Oct 14, 2021
9ceeb2d
Delete UnstableValue and use hardcoded value for m.thread
Oct 14, 2021
efbc469
Null-guard for event that do not have a thread
Oct 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/@types/event.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ export enum EventType {
export enum RelationType {
Annotation = "m.annotation",
Replace = "m.replace",
/**
* Note, "io.element.thread" is hardcoded
* 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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a challenge later on when it does turn stable where existing threads will be "forgotten" by the client, but that should be fairly easy to overcome down the road.

}

export enum MsgType {
Expand Down Expand Up @@ -168,11 +176,6 @@ 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",
);

export interface IEncryptedFile {
url: string;
mimetype?: string;
Expand Down
50 changes: 25 additions & 25 deletions src/models/event.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
EventType,
MsgType,
RelationType,
UNSTABLE_ELEMENT_REPLY_IN_THREAD,
} from "../@types/event";
import { Crypto } from "../crypto";
import { deepSortedObjectEntries } from "../utils";
Expand Down Expand Up @@ -119,7 +118,7 @@ interface IAggregatedRelation {
key?: string;
}

interface IEventRelation {
export interface IEventRelation {
rel_type: RelationType | string;
event_id: string;
key?: string;
Expand Down Expand Up @@ -419,38 +418,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 === RelationType.Thread) {
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"];
}

/**
Expand Down
69 changes: 20 additions & 49 deletions src/models/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class Room extends EventEmitter {
/**
* @experimental
*/
public threads = new Set<Thread>();
public threads = new Map<string, Thread>();

/**
* Construct a new Room.
Expand Down Expand Up @@ -1068,19 +1068,6 @@ export class Room extends EventEmitter {
);
}

/**
* @experimental
*/
public addThread(thread: Thread): Set<Thread> {
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
*/
Expand All @@ -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<string, Thread>());

this.threads = new Set<Thread>(deduped.values());
};

/**
* Get a member from the current room state.
* @param {string} userId The user ID of the member.
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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.
Expand Down
69 changes: 11 additions & 58 deletions src/models/thread.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ export enum ThreadEvent {
Update = "Thread.update"
}

interface ISerialisedThread {
id: string;
tails: string[];
}

/**
* @experimental
*/
Expand All @@ -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<string>();
public readonly timelineSet: EventTimelineSet;

constructor(
Expand All @@ -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
Expand All @@ -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<void> {
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<void> {
await Promise.allSettled(
Array.from(this.timelineSet.getLiveTimeline().getEvents()).map(event => {
Expand All @@ -132,18 +100,6 @@ export class Thread extends EventEmitter {
);
}

/**
* Fetches an event over the network
*/
private async fetchEventById(roomId: string, eventId: string): Promise<MatrixEvent> {
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
*/
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down