matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-07-31 15:24:23 +03:00
Code Activity

Fix edge cases around 2nd order relations and threads (#3437)

Browse Source 
* Fix tests oversimplifying threads fixtures

* Check for unsigned thread_id in MatrixEvent::threadRootId

* Fix threads order being racy

* Make Sonar happier

* Iterate
This commit is contained in:
Michael Telatynski
2023-06-05 16:18:56 +01:00
committed by GitHub
parent 3351c4f57a
commit 0329824cab
4 changed files with 45 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
spec/integ/matrix-client-event-timeline.spec.ts
View File

@ -1274,6 +1274,7 @@ describe("MatrixClient event timelines", function () {
THREAD_ROOT.event_id,
THREAD_REPLY.event_id,
THREAD_REPLY2.getId(),
THREAD_ROOT_REACTION.getId(),
THREAD_REPLY3.getId(),
]);
});
@ -1322,7 +1323,7 @@ describe("MatrixClient event timelines", function () {
request.respond(200, function () {
return {
original_event: root,
chunk: [replies],
chunk: replies,
// no next batch as this is the oldest end of the timeline
};
});
@ -1479,7 +1480,7 @@ describe("MatrixClient event timelines", function () {
user: userId,
type: "m.room.message",
content: {
"body": "thread reply",
"body": "thread2 reply",
"msgtype": "m.text",
"m.relates_to": {
// We can't use the const here because we change server support mode for test
@ -1499,7 +1500,7 @@ describe("MatrixClient event timelines", function () {
user: userId,
type: "m.room.message",
content: {
"body": "thread reply",
"body": "thread reply2",
"msgtype": "m.text",
"m.relates_to": {
// We can't use the const here because we change server support mode for test
@ -1567,7 +1568,7 @@ describe("MatrixClient event timelines", function () {
// Test adding a second event to the first thread
const thread = room.getThread(THREAD_ROOT.event_id!)!;
thread.initialEventsFetched = true;
const prom = emitPromise(room, ThreadEvent.NewReply);
const prom = emitPromise(room, ThreadEvent.Update);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);

27
spec/unit/event-timeline-set.spec.ts
View File

@ -142,13 +142,6 @@ describe("EventTimelineSet", () => {
});
describe("addEventToTimeline", () => {
let thread: Thread;
beforeEach(() => {
(client.supportsThreads as jest.Mock).mockReturnValue(true);
thread = new Thread("!thread_id:server", messageEvent, { room, client });
});
it("Adds event to timeline", () => {
const liveTimeline = eventTimelineSet.getLiveTimeline();
expect(liveTimeline.getEvents().length).toStrictEqual(0);
@ -167,6 +160,15 @@ describe("EventTimelineSet", () => {
eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline, true, false);
}).not.toThrow();
});
});
describe("addEventToTimeline (thread timeline)", () => {
let thread: Thread;
beforeEach(() => {
(client.supportsThreads as jest.Mock).mockReturnValue(true);
thread = new Thread("!thread_id:server", messageEvent, { room, client });
});
it("should not add an event to a timeline that does not belong to the timelineSet", () => {
const eventTimelineSet2 = new EventTimelineSet(room);
@ -197,7 +199,14 @@ describe("EventTimelineSet", () => {
const liveTimeline = eventTimelineSetForThread.getLiveTimeline();
expect(liveTimeline.getEvents().length).toStrictEqual(0);
eventTimelineSetForThread.addEventToTimeline(messageEvent, liveTimeline, {
const normalMessage = utils.mkMessage({
room: roomId,
user: userA,
msg: "Hello!",
event: true,
});
eventTimelineSetForThread.addEventToTimeline(normalMessage, liveTimeline, {
toStartOfTimeline: true,
});
expect(liveTimeline.getEvents().length).toStrictEqual(0);
@ -336,7 +345,9 @@ describe("EventTimelineSet", () => {
});
it("should return true if the timeline set is not for a thread and the event is a thread root", () => {
const thread = new Thread(messageEvent.getId()!, messageEvent, { room, client });
const eventTimelineSet = new EventTimelineSet(room, {}, client);
messageEvent.setThread(thread);
expect(eventTimelineSet.canContain(messageEvent)).toBeTruthy();
});

15
src/models/event.ts
View File

@ -579,9 +579,18 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
const relatesTo = this.getWireContent()?.["m.relates_to"];
if (relatesTo?.rel_type === THREAD_RELATION_TYPE.name) {
return relatesTo.event_id;
} else {
return this.getThread()?.id || this.threadId;
}
if (this.thread) {
return this.thread.id;
}
if (this.threadId !== undefined) {
return this.threadId;
}
const unsigned = this.getUnsigned();
if (typeof unsigned[UNSIGNED_THREAD_ID_FIELD.name] === "string") {
return unsigned[UNSIGNED_THREAD_ID_FIELD.name];
}
return undefined;
}
/**
@ -593,7 +602,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
// Bundled relationships only returned when the sync response is limited
// hence us having to check both bundled relation and inspect the thread
// model
return !!threadDetails || this.getThread()?.id === this.getId();
return !!threadDetails || this.threadRootId === this.getId();
}
public get replyEventId(): string | undefined {

29
src/models/room.ts
View File

@ -1957,7 +1957,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
}
}
this.on(ThreadEvent.NewReply, this.onThreadNewReply);
this.on(ThreadEvent.Update, this.onThreadUpdate);
this.on(ThreadEvent.Delete, this.onThreadDelete);
this.threadsReady = true;
}
@ -2055,7 +2055,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
}
}
private onThreadNewReply(thread: Thread): void {
private onThreadUpdate(thread: Thread): void {
this.updateThreadRootEvents(thread, false, true);
}
@ -2113,12 +2113,13 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
};
}
// A thread relation is always only shown in a thread
if (event.isRelation(THREAD_RELATION_TYPE.name)) {
// A thread relation (1st and 2nd order) is always only shown in a thread
const threadRootId = event.threadRootId;
if (threadRootId != undefined) {
return {
shouldLiveInRoom: false,
shouldLiveInThread: true,
threadId: event.threadRootId,
threadId: threadRootId,
};
}
@ -2149,15 +2150,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
};
}
const unsigned = event.getUnsigned();
if (typeof unsigned[UNSIGNED_THREAD_ID_FIELD.name] === "string") {
return {
shouldLiveInRoom: false,
shouldLiveInThread: true,
threadId: unsigned[UNSIGNED_THREAD_ID_FIELD.name],
};
}
// We've exhausted all scenarios,
// we cannot assume that it lives in the main timeline as this may be a relation for an unknown thread
// adding the event in the wrong timeline causes stuck notifications and can break ability to send read receipts
@ -2890,12 +2882,9 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
private findThreadRoots(events: MatrixEvent[]): Set<string> {
const threadRoots = new Set<string>();
for (const event of events) {
if (event.isRelation(THREAD_RELATION_TYPE.name)) {
threadRoots.add(event.relationEventId ?? "");
}
const unsigned = event.getUnsigned();
if (typeof unsigned[UNSIGNED_THREAD_ID_FIELD.name] === "string") {
threadRoots.add(unsigned[UNSIGNED_THREAD_ID_FIELD.name]!);
const threadRootId = event.threadRootId;
if (threadRootId != undefined) {
threadRoots.add(threadRootId);
}
}
return threadRoots;