Skip to content

Commit

Permalink
Attempt a potential workaround for stuck notifs (#3384)
Browse files Browse the repository at this point in the history
* Attempt a potential workaround for stuck notifs

* Remove TODOs

* Fix backwards logic about server support for MSC3981 in fetchEditsWhereNeeded

* Check for lack of MSC3981 server support before calling insertEventIntoTimeline

* If no parent event is found, insert purely based on timestamp

* Mark temporary methods as internal

(cherry picked from commit cd26ba6)
  • Loading branch information
andybalaam authored and github-actions[bot] committed May 19, 2023
1 parent 2ec1fa6 commit d37a76f
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 2 deletions.
88 changes: 88 additions & 0 deletions src/models/event-timeline-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,94 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
this.emit(RoomEvent.Timeline, event, this.room, Boolean(toStartOfTimeline), false, data);
}

/**
* Insert event to the given timeline, and emit Room.timeline. Assumes
* we have already checked we don't know about this event.
*
* TEMPORARY: until we have recursive relations, we need this function
* to exist to allow us to insert events in timeline order, which is our
* best guess for Sync Order.
* This is a copy of addEventToTimeline above, modified to insert the event
* after the event it relates to, and before any event with a later
* timestamp. This is our best guess at Sync Order.
*
* Will fire "Room.timeline" for each event added.
*
* @internal
*
* @param options - addEventToTimeline options
*
* @remarks
* Fires {@link RoomEvent.Timeline}
*/
public insertEventIntoTimeline(event: MatrixEvent, timeline: EventTimeline, roomState: RoomState): void {
if (timeline.getTimelineSet() !== this) {
throw new Error(`EventTimelineSet.addEventToTimeline: Timeline=${timeline.toString()} does not belong " +
"in timelineSet(threadId=${this.thread?.id})`);
}

// Make sure events don't get mixed in timelines they shouldn't be in (e.g. a
// threaded message should not be in the main timeline).
//
// We can only run this check for timelines with a `room` because `canContain`
// requires it
if (this.room && !this.canContain(event)) {
let eventDebugString = `event=${event.getId()}`;
if (event.threadRootId) {
eventDebugString += `(belongs to thread=${event.threadRootId})`;
}
logger.warn(
`EventTimelineSet.addEventToTimeline: Ignoring ${eventDebugString} that does not belong ` +
`in timeline=${timeline.toString()} timelineSet(threadId=${this.thread?.id})`,
);
return;
}

// Find the event that this event is related to - the "parent"
const parentEventId = event.relationEventId;
if (!parentEventId) {
// Not related to anything - we just append
this.addEventToTimeline(event, timeline, {
toStartOfTimeline: false,
fromCache: false,
timelineWasEmpty: false,
roomState,
});
return;
}

const parentEvent = this.findEventById(parentEventId);

const timelineEvents = timeline.getEvents();

// Start searching from the parent event, or if it's not loaded, start
// at the beginning and insert purely using timestamp order.
const parentIndex = parentEvent !== undefined ? timelineEvents.indexOf(parentEvent) : 0;
let insertIndex = parentIndex;
for (; insertIndex < timelineEvents.length; insertIndex++) {
const nextEvent = timelineEvents[insertIndex];
if (nextEvent.getTs() > event.getTs()) {
// We found an event later than ours, so insert before that.
break;
}
}
// If we got to the end of the loop, insertIndex points at the end of
// the list.

const eventId = event.getId()!;
timeline.insertEvent(event, insertIndex, roomState);
this._eventIdToTimeline.set(eventId, timeline);

this.relations.aggregateParentEvent(event);
this.relations.aggregateChildEvent(event, this);

const data: IRoomTimelineData = {
timeline: timeline,
liveEvent: timeline == this.liveTimeline,
};
this.emit(RoomEvent.Timeline, event, this.room, false, false, data);
}

/**
* Replaces event with ID oldEventId with one with newEventId, if oldEventId is
* recognised. Otherwise, add to the live timeline. Used to handle remote echos.
Expand Down
39 changes: 39 additions & 0 deletions src/models/event-timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,45 @@ export class EventTimeline {
}
}

/**
* Insert a new event into the timeline, and update the state.
*
* TEMPORARY: until we have recursive relations, we need this function
* to exist to allow us to insert events in timeline order, which is our
* best guess for Sync Order.
* This is a copy of addEvent above, modified to allow inserting an event at
* a specific index.
*
* @internal
*/
public insertEvent(event: MatrixEvent, insertIndex: number, roomState: RoomState): void {
const timelineSet = this.getTimelineSet();

if (timelineSet.room) {
EventTimeline.setEventMetadata(event, roomState, false);

// modify state but only on unfiltered timelineSets
if (event.isState() && timelineSet.room.getUnfilteredTimelineSet() === timelineSet) {
roomState.setStateEvents([event], {});
// it is possible that the act of setting the state event means we
// can set more metadata (specifically sender/target props), so try
// it again if the prop wasn't previously set. It may also mean that
// the sender/target is updated (if the event set was a room member event)
// so we want to use the *updated* member (new avatar/name) instead.
//
// However, we do NOT want to do this on member events if we're going
// back in time, else we'll set the .sender value for BEFORE the given
// member event, whereas we want to set the .sender value for the ACTUAL
// member event itself.
if (!event.sender || event.getType() === EventType.RoomMember) {
EventTimeline.setEventMetadata(event, roomState!, false);
}
}
}

this.events.splice(insertIndex, 0, event); // insert element
}

/**
* Remove an event from the timeline
*
Expand Down
39 changes: 37 additions & 2 deletions src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,33 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
}
}

/**
* TEMPORARY. Only call this when MSC3981 is not available, and we have some
* late-arriving events to insert, because we recursively found them as part
* of populating a thread. When we have MSC3981 we won't need it, because
* they will all be supplied by the homeserver in one request, and they will
* already be in the right order in that response.
* This is a copy of addEventToTimeline above, modified to call
* insertEventIntoTimeline so this event is inserted into our best guess of
* the right place based on timestamp. (We should be using Sync Order but we
* don't have it.)
*
* @internal
*/
public insertEventIntoTimeline(event: MatrixEvent): void {
const eventId = event.getId();
if (!eventId) {
return;
}
if (this.findEventById(eventId)) {
return;
}
this.timelineSet.insertEventIntoTimeline(event, this.liveTimeline, this.roomState);

// As far as we know, timeline should always be the same as events
this.timeline = this.events;
}

public addEvents(events: MatrixEvent[], toStartOfTimeline: boolean): void {
events.forEach((ev) => this.addEvent(ev, toStartOfTimeline, false));
this.updateThreadMetadata();
Expand Down Expand Up @@ -281,7 +308,14 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
*/
this.replayEvents?.push(event);
} else {
this.addEventToTimeline(event, toStartOfTimeline);
const recursionSupport =
this.client.canSupport.get(Feature.RelationsRecursion) ?? ServerSupport.Unsupported;

if (recursionSupport === ServerSupport.Unsupported) {
this.insertEventIntoTimeline(event);
} else {
this.addEventToTimeline(event, toStartOfTimeline);
}
}
// Apply annotations and replace relations to the relations of the timeline only
this.timelineSet.relations?.aggregateParentEvent(event);
Expand Down Expand Up @@ -460,7 +494,7 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
// XXX: Workaround for https://github.com/matrix-org/matrix-spec-proposals/pull/2676/files#r827240084
private async fetchEditsWhereNeeded(...events: MatrixEvent[]): Promise<unknown> {
const recursionSupport = this.client.canSupport.get(Feature.RelationsRecursion) ?? ServerSupport.Unsupported;
if (recursionSupport !== ServerSupport.Unsupported) {
if (recursionSupport === ServerSupport.Unsupported) {
return Promise.all(
events
.filter((e) => e.isEncrypted())
Expand All @@ -473,6 +507,7 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
.then((relations) => {
if (relations.events.length) {
event.makeReplaced(relations.events[0]);
this.insertEventIntoTimeline(event);
}
})
.catch((e) => {
Expand Down

0 comments on commit d37a76f

Please sign in to comment.