diff --git a/spec/unit/room-state.spec.ts b/spec/unit/room-state.spec.ts index 9e0c29f35e8..2c79440df71 100644 --- a/spec/unit/room-state.spec.ts +++ b/spec/unit/room-state.spec.ts @@ -1122,4 +1122,59 @@ describe("RoomState", function () { ).toBeFalsy(); }); }); + describe("skipWrongOrderRoomStateInserts", () => { + const idNow = "now"; + const idBefore = "before"; + const endState = new RoomState(roomId); + const startState = new RoomState(roomId, undefined, true); + + let onRoomStateEvent: (event: MatrixEvent, state: RoomState, lastStateEvent: MatrixEvent | null) => void; + const evNow = new MatrixEvent({ + type: "m.call.member", + room_id: roomId, + state_key: "@user:example.org", + content: {}, + }); + evNow.event.unsigned = { replaces_state: idBefore }; + evNow.event.event_id = idNow; + + const evBefore = new MatrixEvent({ + type: "m.call.member", + room_id: roomId, + state_key: "@user:example.org", + content: {}, + }); + + const updatedStateWithBefore = jest.fn(); + const updatedStateWithNow = jest.fn(); + + beforeEach(() => { + evBefore.event.event_id = idBefore; + onRoomStateEvent = (event, _state, _lastState) => { + if (event.event.event_id === idNow) { + updatedStateWithNow(); + } else if (event.event.event_id === idBefore) { + updatedStateWithBefore(); + } + }; + endState.on(RoomStateEvent.Events, onRoomStateEvent); + startState.on(RoomStateEvent.Events, onRoomStateEvent); + }); + afterEach(() => { + endState.off(RoomStateEvent.Events, onRoomStateEvent); + startState.off(RoomStateEvent.Events, onRoomStateEvent); + updatedStateWithNow.mockReset(); + updatedStateWithBefore.mockReset(); + }); + it("should skip inserting state to the end of the timeline if the current endState events replaces_state id is the same as the inserted events id", () => { + endState.setStateEvents([evNow, evBefore]); + expect(updatedStateWithBefore).not.toHaveBeenCalled(); + expect(updatedStateWithNow).toHaveBeenCalled(); + }); + it("should skip inserting state at the beginning of the timeline if the inserted events replaces_state is the event id of the current startState", () => { + startState.setStateEvents([evBefore, evNow]); + expect(updatedStateWithBefore).toHaveBeenCalled(); + expect(updatedStateWithNow).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts index 912dcaf2130..e8b84fd8068 100644 --- a/src/models/event-timeline.ts +++ b/src/models/event-timeline.ts @@ -127,7 +127,7 @@ export class EventTimeline { public constructor(private readonly eventTimelineSet: EventTimelineSet) { this.roomId = eventTimelineSet.room?.roomId ?? null; if (this.roomId) { - this.startState = new RoomState(this.roomId); + this.startState = new RoomState(this.roomId, undefined, true); this.endState = new RoomState(this.roomId); } @@ -267,7 +267,7 @@ export class EventTimeline { /** * Get a pagination token * - * @param direction - EventTimeline.BACKWARDS to get the pagination + * @param direction - EventTimeline.BACKWARDS to get the pagination * token for going backwards in time; EventTimeline.FORWARDS to get the * pagination token for going forwards in time. * diff --git a/src/models/room-state.ts b/src/models/room-state.ts index 92cc4fd7f9b..52a58660437 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -194,10 +194,22 @@ export class RoomState extends TypedEventEmitter * As the timeline might get reset while they are loading, this state needs to be inherited * and shared when the room state is cloned for the new timeline. * This should only be passed from clone. + * @param isStartTimelineState - Optional. This state is marked as a start state. + * This is used to skip state insertions that are + * in the wrong order. The order is determined by the `replaces_state` id. + * + * Example: + * A current state events `replaces_state` value is `1`. + * Trying to insert a state event with `event_id` `1` in its place would fail if isStartTimelineState = false. + * + * A current state events `event_id` is `2`. + * Trying to insert a state event where its `replaces_state` value is `2` would fail if isStartTimelineState = true. */ + public constructor( public readonly roomId: string, private oobMemberFlags = { status: OobStatus.NotStarted }, + public readonly isStartTimelineState = false, ) { super(); this.updateModifiedTime(); @@ -408,7 +420,7 @@ export class RoomState extends TypedEventEmitter * Fires {@link RoomStateEvent.Events} * Fires {@link RoomStateEvent.Marker} */ - public setStateEvents(stateEvents: MatrixEvent[], markerFoundOptions?: IMarkerFoundOptions): void { + public setStateEvents(stateEvents: MatrixEvent[], options?: IMarkerFoundOptions): void { this.updateModifiedTime(); // update the core event dict @@ -420,6 +432,22 @@ export class RoomState extends TypedEventEmitter } const lastStateEvent = this.getStateEventMatching(event); + + // Safety measure to not update the room (and emit the update) with older state. + // The sync loop really should not send old events but it does very regularly. + // Logging on return in those two conditions results in a large amount of logging. (on startup and when running element) + const lastReplaceId = lastStateEvent?.event.unsigned?.replaces_state; + const lastId = lastStateEvent?.event.event_id; + const newReplaceId = event.event.unsigned?.replaces_state; + const newId = event.event.event_id; + if (this.isStartTimelineState) { + // Add an event to the start of the timeline. Its replace id should not be the same as the one of the current/last start state event. + if (newReplaceId && lastId && newReplaceId === lastId) return; + } else { + // Add an event to the end of the timeline. It should not be the same as the one replaced by the current/last end state event. + if (lastReplaceId && newId && lastReplaceId === newId) return; + } + this.setStateEvent(event); if (event.getType() === EventType.RoomMember) { this.updateDisplayNameCache(event.getStateKey()!, event.getContent().displayname ?? ""); @@ -476,7 +504,7 @@ export class RoomState extends TypedEventEmitter // assume all our sentinels are now out-of-date this.sentinels = {}; } else if (UNSTABLE_MSC2716_MARKER.matches(event.getType())) { - this.emit(RoomStateEvent.Marker, event, markerFoundOptions); + this.emit(RoomStateEvent.Marker, event, options); } });