-
-
Notifications
You must be signed in to change notification settings - Fork 581
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.
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
Avoid unnecessary work when processing threads #3530
Changes from 1 commit
7cfedaa
77f2dd2
0003301
6cd3b62
8686559
e811ba1
9e42538
8ee38d0
fe4e5a7
d7b6007
fa4d37a
165d47e
a555e70
8fd2a16
e3d608c
bfdda8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2559,7 +2559,7 @@ | |
let prom = emitPromise(room, ThreadEvent.New); | ||
await room.addLiveEvents([randomMessage, threadRoot, threadResponse]); | ||
const thread: Thread = await prom; | ||
await emitPromise(room, ThreadEvent.Update); | ||
await Promise.all([emitPromise(room, RoomEvent.TimelineReset), emitPromise(room, ThreadEvent.Update)]); | ||
|
||
expect(thread.initialEventsFetched).toBeTruthy(); | ||
expect(thread.replyToEvent!.event).toEqual(threadResponse.event); | ||
|
@@ -2586,7 +2586,7 @@ | |
prom = emitPromise(room, ThreadEvent.Update); | ||
await room.addLiveEvents([threadResponseEdit]); | ||
await prom; | ||
expect(thread.replyToEvent!.getContent().body).toBe(threadResponseEdit.getContent()["m.new_content"].body); | ||
Check failure on line 2589 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node 18)Room › threads › Edits update the lastReply event
Check failure on line 2589 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node latest)Room › threads › Edits update the lastReply event
|
||
}); | ||
|
||
it("Redactions to thread responses decrement the length", async () => { | ||
|
@@ -2619,7 +2619,7 @@ | |
const thread = await prom; | ||
await emitPromise(room, ThreadEvent.Update); | ||
|
||
expect(thread).toHaveLength(2); | ||
Check failure on line 2622 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node 18)Room › threads › Redactions to thread responses decrement the length
Check failure on line 2622 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node latest)Room › threads › Redactions to thread responses decrement the length
|
||
expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); | ||
|
||
thread.timelineSet.addEventToTimeline(threadResponse1, thread.liveTimeline, { | ||
|
@@ -2687,7 +2687,7 @@ | |
const thread = await prom; | ||
await emitPromise(room, ThreadEvent.Update); | ||
|
||
expect(thread).toHaveLength(2); | ||
Check failure on line 2690 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node 18)Room › threads › Redactions to reactions in threads do not decrement the length
Check failure on line 2690 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node latest)Room › threads › Redactions to reactions in threads do not decrement the length
|
||
expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); | ||
|
||
const threadResponse2ReactionRedaction = mkRedaction(threadResponse2Reaction); | ||
|
@@ -2727,7 +2727,7 @@ | |
const thread = await prom; | ||
await emitPromise(room, ThreadEvent.Update); | ||
|
||
expect(thread).toHaveLength(2); | ||
Check failure on line 2730 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node 18)Room › threads › should not decrement the length when the thread root is redacted
Check failure on line 2730 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node latest)Room › threads › should not decrement the length when the thread root is redacted
|
||
expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId()); | ||
|
||
prom = emitPromise(room, ThreadEvent.Update); | ||
|
@@ -2791,7 +2791,7 @@ | |
|
||
expect(thread.initialEventsFetched).toBeTruthy(); | ||
await room.addLiveEvents([threadResponse2]); | ||
expect(thread).toHaveLength(2); | ||
Check failure on line 2794 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node 18)Room › threads › Redacting the lastEvent finds a new lastEvent
Check failure on line 2794 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node latest)Room › threads › Redacting the lastEvent finds a new lastEvent
|
||
expect(thread.replyToEvent!.getId()).toBe(threadResponse2.getId()); | ||
|
||
room.client.fetchRoomEvent = (eventId: string) => | ||
|
@@ -2969,7 +2969,7 @@ | |
expect(rootRelations![0][1].has(rootReaction)).toBeTruthy(); | ||
|
||
const responseRelations = thread.timelineSet.relations | ||
.getChildEventsForEvent(threadResponse.getId()!, RelationType.Annotation, EventType.Reaction)! | ||
Check failure on line 2972 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node 18)Room › eventShouldLiveIn › should aggregate relations in thread event timeline set
Check failure on line 2972 in spec/unit/room.spec.ts GitHub Actions / Jest [unit] (Node latest)Room › eventShouldLiveIn › should aggregate relations in thread event timeline set
|
||
.getSortedAnnotationsByKey(); | ||
expect(responseRelations).toHaveLength(1); | ||
expect(responseRelations![0][0]).toEqual(threadReaction.getRelation()!.key); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2256,6 +2256,13 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> { | |
client: this.client, | ||
pendingEventOrdering: this.opts.pendingEventOrdering, | ||
receipts: this.cachedThreadReadReceipts.get(threadId) ?? [], | ||
|
||
// This is necessary to be able to jump to events in threads: | ||
// If we jump to an event in a thread where neither the event, nor | ||
// the root, nor any thread event are loaded yet, we'll load the | ||
// event as well as the thread root, create the thread, and pass | ||
// the event through this. | ||
events, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adding
|
||
}); | ||
|
||
// All read receipts should now come down from sync, we do not need to keep | ||
|
@@ -2267,12 +2274,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> { | |
// eventtimeline sometimes looks up thread information via the room. | ||
this.threads.set(thread.id, thread); | ||
|
||
// This is necessary to be able to jump to events in threads: | ||
// If we jump to an event in a thread where neither the event, nor the root, | ||
// nor any thread event are loaded yet, we'll load the event as well as the thread root, create the thread, | ||
// and pass the event through this. | ||
thread.addEvents(events, false); | ||
|
||
this.reEmitter.reEmit(thread, [ | ||
ThreadEvent.Delete, | ||
ThreadEvent.Update, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,7 @@ interface IThreadOpts { | |
client: MatrixClient; | ||
pendingEventOrdering?: PendingEventOrdering; | ||
receipts?: CachedReceiptStructure[]; | ||
events?: MatrixEvent[]; | ||
} | ||
|
||
export enum FeatureSupport { | ||
|
@@ -139,6 +140,8 @@ export class Thread extends ReadReceipt<ThreadEmittedEvents, ThreadEventHandlerM | |
|
||
this.processReceipts(opts.receipts); | ||
|
||
this.replayEvents = opts.events ?? []; | ||
|
||
// 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.updateThreadMetadata(); | ||
|
@@ -310,6 +313,11 @@ export class Thread extends ReadReceipt<ThreadEmittedEvents, ThreadEventHandlerM | |
const lastReply = this.lastReply(); | ||
const isNewestReply = !lastReply || event.localTimestamp >= lastReply!.localTimestamp; | ||
|
||
if (isNewestReply) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. workaround for not refetching the thread root (on refetch, we'd get the latest reply event ID in the event's content) |
||
const mapper = this.client.getEventMapper(); | ||
this.lastEvent = mapper(event.getEffectiveEvent()); | ||
} | ||
|
||
// 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 | ||
|
@@ -321,6 +329,7 @@ export class Thread extends ReadReceipt<ThreadEmittedEvents, ThreadEventHandlerM | |
this.client.decryptEventIfNeeded(event, {}); | ||
} else if (!toStartOfTimeline && this.initialEventsFetched && isNewestReply) { | ||
this.addEventToTimeline(event, false); | ||
// TODO: This is another async call without awaiting | ||
this.fetchEditsWhereNeeded(event); | ||
} else if (event.isRelation(RelationType.Annotation) || event.isRelation(RelationType.Replace)) { | ||
if (!this.initialEventsFetched) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This waits for the correct signal that
Thread.constructor
has completed