From 2b029c9b15fc0c6b2f62c4b13ea6ea53b080462f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 7 Apr 2022 22:16:26 -0500 Subject: [PATCH 01/20] Refresh room timeline when we see a MSC2716 marker event See https://github.com/matrix-org/matrix-spec-proposals/pull/2716#discussion_r782629097 > In a [sync meeting with @ara4n](https://docs.google.com/document/d/1KCEmpnGr4J-I8EeaVQ8QJZKBDu53ViI7V62y5BzfXr0/edit#bookmark=id.67nio1ka8znc), we came up with the idea to make the `marker` events as state events. When the client sees that the `m.room.marker` state changed to a different event ID, it can throw away all of the timeline and re-fetch as needed. > > For homeservers where the [same problem](https://github.com/matrix-org/matrix-doc/pull/2716#discussion_r782499674) can happen, we probably don't want to throw away the whole timeline but it can go up the `unsigned.replaces_state` chain of the `m.room.marker` state events to get them all. > > In terms of state performance, there could be thousands of `marker` events in a room but it's no different than room members joining and leaving over and over like an IRC room. --- src/@types/event.ts | 2 ++ src/client.ts | 1 + src/models/room-state.ts | 4 ++++ src/models/room.ts | 33 +++++++++++++++++++++++++++++++++ src/sync.ts | 29 +++++++++++++++++++++++++++++ 5 files changed, 69 insertions(+) diff --git a/src/@types/event.ts b/src/@types/event.ts index 25946a5ac9..a3e9d17ec4 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -87,6 +87,8 @@ export enum EventType { RoomKeyRequest = "m.room_key_request", ForwardedRoomKey = "m.forwarded_room_key", Dummy = "m.dummy", + + Marker = "org.matrix.msc2716.marker", // MSC2716 } export enum RelationType { diff --git a/src/client.ts b/src/client.ts index 11ec7fa4d5..37e3eee478 100644 --- a/src/client.ts +++ b/src/client.ts @@ -799,6 +799,7 @@ type RoomStateEvents = RoomStateEvent.Events | RoomStateEvent.Members | RoomStateEvent.NewMember | RoomStateEvent.Update + | RoomStateEvent.Marker ; type CryptoEvents = CryptoEvent.KeySignatureUploadFailure diff --git a/src/models/room-state.ts b/src/models/room-state.ts index 56e27be3d5..de996db381 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -42,6 +42,7 @@ export enum RoomStateEvent { NewMember = "RoomState.newMember", Update = "RoomState.update", // signals batches of updates without specificity BeaconLiveness = "RoomState.BeaconLiveness", + Marker = "RoomState.org.matrix.msc2716.marker", } export type RoomStateEventHandlerMap = { @@ -51,6 +52,7 @@ export type RoomStateEventHandlerMap = { [RoomStateEvent.Update]: (state: RoomState) => void; [RoomStateEvent.BeaconLiveness]: (state: RoomState, hasLiveBeacons: boolean) => void; [BeaconEvent.New]: (event: MatrixEvent, beacon: Beacon) => void; + [RoomStateEvent.Marker]: (event: MatrixEvent, state: RoomState) => void; }; type EmittedEvents = RoomStateEvent | BeaconEvent; @@ -398,6 +400,8 @@ export class RoomState extends TypedEventEmitter // assume all our sentinels are now out-of-date this.sentinels = {}; + } else if (event.getType() === EventType.Marker) { + this.emit(RoomStateEvent.Marker, event, this); } }); diff --git a/src/models/room.ts b/src/models/room.ts index f6008d7b14..359414110a 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -204,6 +204,7 @@ export class Room extends TypedEventEmitter public readonly threadsTimelineSets: EventTimelineSet[] = []; // any filtered timeline sets we're maintaining for this room private readonly filteredTimelineSets: Record = {}; // filter_id: timelineSet + private lastMarkerEventIdProcessed: string = null; private readonly pendingEventList?: MatrixEvent[]; // read by megolm via getter; boolean value - null indicates "use global value" private blacklistUnverifiedDevices: boolean = null; @@ -439,6 +440,19 @@ export class Room extends TypedEventEmitter return Promise.allSettled(decryptionPromises) as unknown as Promise; } + /** + * Gets the creator of the room + * @returns {string} The creator of the room, or null if it could not be determined + */ + public getRoomCreator(): string | null { + const createEvent = this.currentState.getStateEvents(EventType.RoomCreate, ""); + if (!createEvent) { + return null; + } + const roomCreator = createEvent.getContent()['sender']; + return roomCreator; + } + /** * Gets the version of the room * @returns {string} The version of the room, or null if it could not be determined @@ -1004,6 +1018,25 @@ export class Room extends TypedEventEmitter return this.getUnfilteredTimelineSet().addTimeline(); } + /** + * Get the last marker event ID proccessed + * + * @return {String} the last marker event ID proccessed or null if none have + * been processed + */ + public getLastMarkerEventIdProcessed(): string | null { + return this.lastMarkerEventIdProcessed; + } + + /** + * Set the last marker event ID proccessed + * + * @param {String} eventId The marker event ID to set + */ + public setLastMarkerEventIdProcessed(eventId: string): void { + this.lastMarkerEventIdProcessed = eventId; + } + /** * Get an event which is stored in our unfiltered timeline set, or in a thread * diff --git a/src/sync.ts b/src/sync.ts index bab1da9704..2c97fa3ce2 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -77,6 +77,13 @@ export enum SyncState { Reconnecting = "RECONNECTING", } +// Room versions where "insertion", "batch", and "marker" events are controlled +// by power-levels. MSC2716 is supported in existing room versions but they +// should only have special meaning when the room creator sends them. +const MSC2716_ROOM_VERSIONS = [ + 'org.matrix.msc2716v3' +]; + function getFilterName(userId: string, suffix?: string): string { // scope this on the user ID because people may login on many accounts // and they all need to be stored! @@ -239,6 +246,27 @@ export class SyncApi { RoomMemberEvent.Membership, ]); }); + + room.currentState.on(RoomStateEvent.Marker, function(event, state) { + const isValidMsc2716Event = MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || + // MSC2716 is supported in all existing room versions but special + // meaning should only be given to "insertion", "batch", and + // "marker" events when they come from the room creator + event.getSender() === room.getRoomCreator(); + console.log(`On marker state isValidMsc2716Event=${isValidMsc2716Event}, event_id=${event.getId()} room.getLastMarkerEventIdProcessed()=${room.getLastMarkerEventIdProcessed()}`); + if( + isValidMsc2716Event && + // We only need to throw the timeline away once when we see a + // marker. All of the historical content will be in the + // `/messsages` responses from here on out. + event.getId() !== room.getLastMarkerEventIdProcessed() + ) { + // Saw new marker event, refreshing the timeline + console.log('Saw new marker event, refreshing the timeline'); + room.resetLiveTimeline(null, null); + room.setLastMarkerEventIdProcessed(event.getId()) + } + }); } /** @@ -250,6 +278,7 @@ export class SyncApi { room.currentState.removeAllListeners(RoomStateEvent.Events); room.currentState.removeAllListeners(RoomStateEvent.Members); room.currentState.removeAllListeners(RoomStateEvent.NewMember); + room.currentState.removeAllListeners(RoomStateEvent.Marker); } /** From ffbcbff97fc9f08136d0bdd18e956051e3c93feb Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 Apr 2022 01:39:55 -0500 Subject: [PATCH 02/20] Don't refresh timeline when first sync or from cache Context: https://github.com/matrix-org/matrix-js-sdk/pull/2282#discussion_r845711835 --- src/models/event-timeline-set.ts | 57 ++++++++++++++++++++++++++------ src/models/event-timeline.ts | 27 ++++++++++++--- src/models/room-state.ts | 20 +++++++---- src/models/room.ts | 42 +++++++++++++---------- src/sync.ts | 24 +++++++++++--- 5 files changed, 126 insertions(+), 44 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 1fda0d977a..5f34d060cc 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -56,6 +56,20 @@ export interface IRoomTimelineData { liveEvent?: boolean; } +export interface IAddLiveEventOptions { + duplicateStrategy?: DuplicateStrategy; + fromCache?: boolean; + roomState?: RoomState; + fromInitialState?: boolean; +} + +export interface IAddEventToTimelineOptions { + toStartOfTimeline: boolean; + fromCache?: boolean; + roomState?: RoomState; + fromInitialState?: boolean; +} + type EmittedEvents = RoomEvent.Timeline | RoomEvent.TimelineReset; export type EventTimelineSetHandlerMap = { @@ -431,7 +445,9 @@ export class EventTimelineSet extends TypedEventEmitter void; [RoomStateEvent.BeaconLiveness]: (state: RoomState, hasLiveBeacons: boolean) => void; [BeaconEvent.New]: (event: MatrixEvent, beacon: Beacon) => void; - [RoomStateEvent.Marker]: (event: MatrixEvent, state: RoomState) => void; + [RoomStateEvent.Marker]: (event: MatrixEvent, setStateOptions: ISetStateOptions) => void; }; type EmittedEvents = RoomStateEvent | BeaconEvent; @@ -311,16 +315,18 @@ export class RoomState extends TypedEventEmitter } /** - * Add an array of one or more state MatrixEvents, overwriting - * any existing state with the same {type, stateKey} tuple. Will fire - * "RoomState.events" for every event added. May fire "RoomState.members" - * if there are m.room.member events. + * Add an array of one or more state MatrixEvents, overwriting any existing + * state with the same {type, stateKey} tuple. Will fire "RoomState.events" + * for every event added. May fire "RoomState.members" if there are + * m.room.member events. * @param {MatrixEvent[]} stateEvents a list of state events for this room. + * @param {Bool} fromInitialState whether the stateEvents are from the first + * sync in the room or a sync we already know about (syncFromCache) * @fires module:client~MatrixClient#event:"RoomState.members" * @fires module:client~MatrixClient#event:"RoomState.newMember" * @fires module:client~MatrixClient#event:"RoomState.events" */ - public setStateEvents(stateEvents: MatrixEvent[]) { + public setStateEvents(stateEvents: MatrixEvent[], setStateOptions?: ISetStateOptions) { this.updateModifiedTime(); // update the core event dict @@ -401,7 +407,7 @@ export class RoomState extends TypedEventEmitter // assume all our sentinels are now out-of-date this.sentinels = {}; } else if (event.getType() === EventType.Marker) { - this.emit(RoomStateEvent.Marker, event, this); + this.emit(RoomStateEvent.Marker, event, setStateOptions); } }); diff --git a/src/models/room.ts b/src/models/room.ts index 359414110a..e427ea7ecb 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -48,6 +48,7 @@ import { } from "./thread"; import { Method } from "../http-api"; import { TypedEventEmitter } from "./typed-event-emitter"; +import { IAddLiveEventOptions } from "./event-timeline-set"; // These constants are used as sane defaults when the homeserver doesn't support // the m.room_versions capability. In practice, KNOWN_SAFE_ROOM_VERSION should be @@ -1493,7 +1494,9 @@ export class Room extends TypedEventEmitter return event.getSender() === this.client.getUserId(); }); if (filterType !== ThreadFilterType.My || currentUserParticipated) { - timelineSet.getLiveTimeline().addEvent(thread.rootEvent, false); + timelineSet.getLiveTimeline().addEvent(thread.rootEvent, { + atStart: false + }); } }); } @@ -1540,22 +1543,20 @@ export class Room extends TypedEventEmitter let latestMyThreadsRootEvent: MatrixEvent; const roomState = this.getLiveTimeline().getState(EventTimeline.FORWARDS); for (const rootEvent of threadRoots) { - this.threadsTimelineSets[0].addLiveEvent( - rootEvent, - DuplicateStrategy.Ignore, - false, + this.threadsTimelineSets[0].addLiveEvent(rootEvent, { + duplicateStrategy: DuplicateStrategy.Ignore, + fromCache: false, roomState, - ); + }); const threadRelationship = rootEvent .getServerAggregatedRelation(RelationType.Thread); if (threadRelationship.current_user_participated) { - this.threadsTimelineSets[1].addLiveEvent( - rootEvent, - DuplicateStrategy.Ignore, - false, + this.threadsTimelineSets[1].addLiveEvent(rootEvent, { + duplicateStrategy: DuplicateStrategy.Ignore, + fromCache: false, roomState, - ); + }); latestMyThreadsRootEvent = rootEvent; } @@ -1750,7 +1751,7 @@ export class Room extends TypedEventEmitter timelineSet.addEventToTimeline( thread.rootEvent, timelineSet.getLiveTimeline(), - toStartOfTimeline, + {toStartOfTimeline} ); } } @@ -1817,7 +1818,7 @@ export class Room extends TypedEventEmitter * @fires module:client~MatrixClient#event:"Room.timeline" * @private */ - private addLiveEvent(event: MatrixEvent, duplicateStrategy?: DuplicateStrategy, fromCache = false): void { + private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions = {}): void { this.applyRedaction(event); // Implement MSC3531: hiding messages. @@ -1840,7 +1841,7 @@ export class Room extends TypedEventEmitter // add to our timeline sets for (let i = 0; i < this.timelineSets.length; i++) { - this.timelineSets[i].addLiveEvent(event, duplicateStrategy, fromCache); + this.timelineSets[i].addLiveEvent(event, addLiveEventOptions); } // synthesize and inject implicit read receipts @@ -1926,11 +1927,15 @@ export class Room extends TypedEventEmitter if (timelineSet.getFilter()) { if (timelineSet.getFilter().filterRoomTimeline([event]).length) { timelineSet.addEventToTimeline(event, - timelineSet.getLiveTimeline(), false); + timelineSet.getLiveTimeline(), { + toStartOfTimeline: false + }); } } else { timelineSet.addEventToTimeline(event, - timelineSet.getLiveTimeline(), false); + timelineSet.getLiveTimeline(), { + toStartOfTimeline: false + }); } } } @@ -2181,7 +2186,8 @@ export class Room extends TypedEventEmitter * @param {boolean} fromCache whether the sync response came from cache * @throws If duplicateStrategy is not falsey, 'replace' or 'ignore'. */ - public addLiveEvents(events: MatrixEvent[], duplicateStrategy?: DuplicateStrategy, fromCache = false): void { + public addLiveEvents(events: MatrixEvent[], addLiveEventOptions: IAddLiveEventOptions = {}): void { + const { duplicateStrategy } = addLiveEventOptions; if (duplicateStrategy && ["replace", "ignore"].indexOf(duplicateStrategy) === -1) { throw new Error("duplicateStrategy MUST be either 'replace' or 'ignore'"); } @@ -2202,7 +2208,7 @@ export class Room extends TypedEventEmitter for (let i = 0; i < events.length; i++) { // TODO: We should have a filter to say "only add state event types X Y Z to the timeline". - this.addLiveEvent(events[i], duplicateStrategy, fromCache); + this.addLiveEvent(events[i], addLiveEventOptions); } } diff --git a/src/sync.ts b/src/sync.ts index 2c97fa3ce2..af5241da60 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -54,6 +54,7 @@ import { IPushRules } from "./@types/PushRules"; import { RoomStateEvent } from "./models/room-state"; import { RoomMemberEvent } from "./models/room-member"; import { BeaconEvent } from "./models/beacon"; +import { ISetStateOptions } from "./models/room-state"; const DEBUG = true; @@ -247,7 +248,17 @@ export class SyncApi { ]); }); - room.currentState.on(RoomStateEvent.Marker, function(event, state) { + room.currentState.on(RoomStateEvent.Marker, function(event, {fromInitialState}: ISetStateOptions = {}) { + // We don't want to refresh the timeline: + // 1. If it's persons first time syncing the room, they won't have + // any old events cached to refresh. + // 1. If we're re-hydrating from `syncFromCache` because we already + // processed any marker event state that was in the cache + if(fromInitialState) { + console.log('fromInitialState ignoring'); + return; + } + const isValidMsc2716Event = MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || // MSC2716 is supported in all existing room versions but special // meaning should only be given to "insertion", "batch", and @@ -256,13 +267,13 @@ export class SyncApi { console.log(`On marker state isValidMsc2716Event=${isValidMsc2716Event}, event_id=${event.getId()} room.getLastMarkerEventIdProcessed()=${room.getLastMarkerEventIdProcessed()}`); if( isValidMsc2716Event && - // We only need to throw the timeline away once when we see a + // We only need to throw the timeline away once, when we see a // marker. All of the historical content will be in the // `/messsages` responses from here on out. event.getId() !== room.getLastMarkerEventIdProcessed() ) { // Saw new marker event, refreshing the timeline - console.log('Saw new marker event, refreshing the timeline'); + console.log('Saw new marker event, refreshing the timeline', new Error().stack); room.resetLiveTimeline(null, null); room.setLastMarkerEventIdProcessed(event.getId()) } @@ -1618,6 +1629,7 @@ export class SyncApi { // the given state events const liveTimeline = room.getLiveTimeline(); const timelineWasEmpty = liveTimeline.getEvents().length == 0; + console.log('timelineWasEmpty', timelineWasEmpty) if (timelineWasEmpty) { // Passing these events into initialiseState will freeze them, so we need // to compute and cache the push actions for them now, otherwise sync dies @@ -1669,7 +1681,11 @@ export class SyncApi { // This also needs to be done before running push rules on the events as they need // to be decorated with sender etc. const [mainTimelineEvents, threadedEvents] = this.client.partitionThreadedEvents(room, timelineEventList || []); - room.addLiveEvents(mainTimelineEvents, null, fromCache); + room.addLiveEvents(mainTimelineEvents, { + duplicateStrategy: null, + fromCache, + fromInitialState: timelineWasEmpty + }); await this.processThreadEvents(room, threadedEvents, false); } From 39cd6e906c7eee2745bf4c4eaf8236c102974991 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 8 Apr 2022 15:27:14 -0500 Subject: [PATCH 03/20] Fix docstring typo --- src/models/room-state.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/room-state.ts b/src/models/room-state.ts index f823027cf4..1800b95250 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -320,7 +320,7 @@ export class RoomState extends TypedEventEmitter * for every event added. May fire "RoomState.members" if there are * m.room.member events. * @param {MatrixEvent[]} stateEvents a list of state events for this room. - * @param {Bool} fromInitialState whether the stateEvents are from the first + * @param {boolean} fromInitialState whether the stateEvents are from the first * sync in the room or a sync we already know about (syncFromCache) * @fires module:client~MatrixClient#event:"RoomState.members" * @fires module:client~MatrixClient#event:"RoomState.newMember" From e5f34db621d9b53fded1823ae8a3f023d8c1522f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 20:10:38 -0500 Subject: [PATCH 04/20] Only notify client that timeline should be refreshed instead of doing it automatically See https://github.com/matrix-org/matrix-js-sdk/pull/2282#discussion_r846515012 --- src/client.ts | 1 + src/models/room.ts | 24 ++++++++++++++++++++++++ src/models/thread.ts | 8 +++++--- src/sync.ts | 26 ++++++++++++++++++-------- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/client.ts b/src/client.ts index 37e3eee478..193aa23fac 100644 --- a/src/client.ts +++ b/src/client.ts @@ -790,6 +790,7 @@ type RoomEvents = RoomEvent.Name | RoomEvent.Receipt | RoomEvent.Tags | RoomEvent.LocalEchoUpdated + | RoomEvent.historyImportedWithinTimeline | RoomEvent.AccountData | RoomEvent.MyMembership | RoomEvent.Timeline diff --git a/src/models/room.ts b/src/models/room.ts index e427ea7ecb..6cab2f4283 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -163,6 +163,7 @@ export enum RoomEvent { Redaction = "Room.redaction", RedactionCancelled = "Room.redactionCancelled", LocalEchoUpdated = "Room.localEchoUpdated", + historyImportedWithinTimeline = "Room.historyImportedWithinTimeline", Timeline = "Room.timeline", TimelineReset = "Room.timelineReset", } @@ -188,6 +189,10 @@ export type RoomEventHandlerMap = { oldEventId?: string, oldStatus?: EventStatus, ) => void; + [RoomEvent.historyImportedWithinTimeline]: ( + markerEvent: MatrixEvent, + room: Room, + ) => void; [ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void; } & ThreadHandlerMap; @@ -205,6 +210,7 @@ export class Room extends TypedEventEmitter public readonly threadsTimelineSets: EventTimelineSet[] = []; // any filtered timeline sets we're maintaining for this room private readonly filteredTimelineSets: Record = {}; // filter_id: timelineSet + private timelineNeedsRefresh: boolean = false; private lastMarkerEventIdProcessed: string = null; private readonly pendingEventList?: MatrixEvent[]; // read by megolm via getter; boolean value - null indicates "use global value" @@ -1019,6 +1025,24 @@ export class Room extends TypedEventEmitter return this.getUnfilteredTimelineSet().addTimeline(); } + /** + * Whether the timeline needs to be refreshed in order to pull in new + * historical messages that were imported. + * @param {Boolean} value The value to set + */ + public setTimelineNeedsRefresh(value: boolean): void { + this.timelineNeedsRefresh = value; + } + + /** + * Whether the timeline needs to be refreshed in order to pull in new + * historical messages that were imported. + * @return {Boolean} . + */ + public getTimelineNeedsRefresh(): boolean { + return this.timelineNeedsRefresh; + } + /** * Get the last marker event ID proccessed * diff --git a/src/models/thread.ts b/src/models/thread.ts index 7879cc89f9..187b316047 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -138,9 +138,11 @@ export class Thread extends TypedEventEmitter { this.timelineSet.addEventToTimeline( event, this.liveTimeline, - toStartOfTimeline, - false, - this.roomState, + { + toStartOfTimeline, + fromCache: false, + roomState: this.roomState, + } ); } } diff --git a/src/sync.ts b/src/sync.ts index af5241da60..887b08aef9 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -210,6 +210,7 @@ export class SyncApi { RoomEvent.Receipt, RoomEvent.Tags, RoomEvent.LocalEchoUpdated, + RoomEvent.historyImportedWithinTimeline, RoomEvent.AccountData, RoomEvent.MyMembership, RoomEvent.Timeline, @@ -248,7 +249,8 @@ export class SyncApi { ]); }); - room.currentState.on(RoomStateEvent.Marker, function(event, {fromInitialState}: ISetStateOptions = {}) { + // TODO: Should we just move this to `room.ts`? + room.currentState.on(RoomStateEvent.Marker, async function(markerEvent, {fromInitialState}: ISetStateOptions = {}) { // We don't want to refresh the timeline: // 1. If it's persons first time syncing the room, they won't have // any old events cached to refresh. @@ -263,19 +265,27 @@ export class SyncApi { // MSC2716 is supported in all existing room versions but special // meaning should only be given to "insertion", "batch", and // "marker" events when they come from the room creator - event.getSender() === room.getRoomCreator(); - console.log(`On marker state isValidMsc2716Event=${isValidMsc2716Event}, event_id=${event.getId()} room.getLastMarkerEventIdProcessed()=${room.getLastMarkerEventIdProcessed()}`); + markerEvent.getSender() === room.getRoomCreator(); + console.log(`On marker state isValidMsc2716Event=${isValidMsc2716Event}, event_id=${markerEvent.getId()} room.getLastMarkerEventIdProcessed()=${room.getLastMarkerEventIdProcessed()}`); if( isValidMsc2716Event && // We only need to throw the timeline away once, when we see a // marker. All of the historical content will be in the // `/messsages` responses from here on out. - event.getId() !== room.getLastMarkerEventIdProcessed() + markerEvent.getId() !== room.getLastMarkerEventIdProcessed() ) { - // Saw new marker event, refreshing the timeline - console.log('Saw new marker event, refreshing the timeline', new Error().stack); - room.resetLiveTimeline(null, null); - room.setLastMarkerEventIdProcessed(event.getId()) + // Saw new marker event, let's let the clients know they should + // refresh the timeline. + // + // FIXME: Is there a way we could lookup + // `markerEvent.getContent()['m.insertion_id']` and get the + // prev_events of that event to see exactly where the history + // was imported and whether we have it locally? Because we only + // need to refresh the timeline if the timeline includes the + // prev_events of the base insertion event. + console.log('Saw new marker event', new Error().stack); + this.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); + room.setLastMarkerEventIdProcessed(markerEvent.getId()) } }); } From e070b3c0a562f11e55796a24d42492eb2ea8e9ef Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 20:16:17 -0500 Subject: [PATCH 05/20] Remove console.log --- src/sync.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sync.ts b/src/sync.ts index 887b08aef9..65be38b4ef 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1639,7 +1639,6 @@ export class SyncApi { // the given state events const liveTimeline = room.getLiveTimeline(); const timelineWasEmpty = liveTimeline.getEvents().length == 0; - console.log('timelineWasEmpty', timelineWasEmpty) if (timelineWasEmpty) { // Passing these events into initialiseState will freeze them, so we need // to compute and cache the push actions for them now, otherwise sync dies From 21f6aa2ac2c9fd91fc2530b816a7baf032c58333 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 20:46:18 -0500 Subject: [PATCH 06/20] Fixup docstrings --- src/models/event-timeline-set.ts | 25 ++++++++++++++++++++----- src/models/event-timeline.ts | 2 +- src/models/room-state.ts | 5 +++++ src/models/room.ts | 15 +++------------ 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 5f34d060cc..b1a758fe20 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -57,9 +57,22 @@ export interface IRoomTimelineData { } export interface IAddLiveEventOptions { + /** Applies to events in the timeline only. If this is 'replace' then if a + * duplicate is encountered, the event passed to this function will replace + * the existing event in the timeline. If this is not specified, or is + * 'ignore', then the event passed to this function will be ignored + * entirely, preserving the existing event in the timeline. Events are + * identical based on their event ID only. */ duplicateStrategy?: DuplicateStrategy; + /** Whether the sync response came from cache */ fromCache?: boolean; + /** The state events to reconcile metadata from */ roomState?: RoomState; + /** Whether the state is part of the first state snapshot we're seeing in + * the room. This could be happen in a variety of cases: + * 1. From the initial sync + * 2. It's the first state we're seeing after joining the room + * 3. Or whether it's coming from `syncFromCache` */ fromInitialState?: boolean; } @@ -67,6 +80,11 @@ export interface IAddEventToTimelineOptions { toStartOfTimeline: boolean; fromCache?: boolean; roomState?: RoomState; + /** Whether the state is part of the first state snapshot we're seeing in + * the room. This could be happen in a variety of cases: + * 1. From the initial sync + * 2. It's the first state we're seeing after joining the room + * 3. Or whether it's coming from `syncFromCache` */ fromInitialState?: boolean; } @@ -539,9 +557,7 @@ export class EventTimelineSet extends TypedEventEmitter }); if (filterType !== ThreadFilterType.My || currentUserParticipated) { timelineSet.getLiveTimeline().addEvent(thread.rootEvent, { - atStart: false + atStart: false, }); } }); @@ -1837,9 +1837,7 @@ export class Room extends TypedEventEmitter * "Room.timeline". * * @param {MatrixEvent} event Event to be added - * @param {string?} duplicateStrategy 'ignore' or 'replace' - * @param {boolean} fromCache whether the sync response came from cache - * @fires module:client~MatrixClient#event:"Room.timeline" + * @param {IAddLiveEventOptions} options addLiveEvent options * @private */ private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions = {}): void { @@ -2199,15 +2197,8 @@ export class Room extends TypedEventEmitter * they will go to the end of the timeline. * * @param {MatrixEvent[]} events A list of events to add. + * @param {IAddLiveEventOptions} options addLiveEvent options * - * @param {string} duplicateStrategy Optional. Applies to events in the - * timeline only. If this is 'replace' then if a duplicate is encountered, the - * event passed to this function will replace the existing event in the - * timeline. If this is not specified, or is 'ignore', then the event passed to - * this function will be ignored entirely, preserving the existing event in the - * timeline. Events are identical based on their event ID only. - * - * @param {boolean} fromCache whether the sync response came from cache * @throws If duplicateStrategy is not falsey, 'replace' or 'ignore'. */ public addLiveEvents(events: MatrixEvent[], addLiveEventOptions: IAddLiveEventOptions = {}): void { From 9bfcf4b01b74c4539874c5da9b76226639b4d56d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 21:15:54 -0500 Subject: [PATCH 07/20] Fix some lints --- src/models/event-timeline-set.ts | 16 +++++++++------- src/models/event-timeline.ts | 6 +++--- src/models/room.ts | 4 ++-- src/models/thread.ts | 2 +- src/sync.ts | 17 ++++++++++------- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index b1a758fe20..690fc39616 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -78,7 +78,9 @@ export interface IAddLiveEventOptions { export interface IAddEventToTimelineOptions { toStartOfTimeline: boolean; + /** Whether the sync response came from cache */ fromCache?: boolean; + /** The state events to reconcile metadata from */ roomState?: RoomState; /** Whether the state is part of the first state snapshot we're seeing in * the room. This could be happen in a variety of cases: @@ -464,7 +466,7 @@ export class EventTimelineSet extends TypedEventEmitter public readonly threadsTimelineSets: EventTimelineSet[] = []; // any filtered timeline sets we're maintaining for this room private readonly filteredTimelineSets: Record = {}; // filter_id: timelineSet - private timelineNeedsRefresh: boolean = false; + private timelineNeedsRefresh = false; private lastMarkerEventIdProcessed: string = null; private readonly pendingEventList?: MatrixEvent[]; // read by megolm via getter; boolean value - null indicates "use global value" @@ -1030,7 +1030,7 @@ export class Room extends TypedEventEmitter * historical messages that were imported. * @param {Boolean} value The value to set */ - public setTimelineNeedsRefresh(value: boolean): void { + public setTimelineNeedsRefresh(value: boolean): void { this.timelineNeedsRefresh = value; } diff --git a/src/models/thread.ts b/src/models/thread.ts index 187b316047..9b1119fc18 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -142,7 +142,7 @@ export class Thread extends TypedEventEmitter { toStartOfTimeline, fromCache: false, roomState: this.roomState, - } + }, ); } } diff --git a/src/sync.ts b/src/sync.ts index 65be38b4ef..d66262849e 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -82,7 +82,7 @@ export enum SyncState { // by power-levels. MSC2716 is supported in existing room versions but they // should only have special meaning when the room creator sends them. const MSC2716_ROOM_VERSIONS = [ - 'org.matrix.msc2716v3' + 'org.matrix.msc2716v3', ]; function getFilterName(userId: string, suffix?: string): string { @@ -250,24 +250,27 @@ export class SyncApi { }); // TODO: Should we just move this to `room.ts`? - room.currentState.on(RoomStateEvent.Marker, async function(markerEvent, {fromInitialState}: ISetStateOptions = {}) { + room.currentState.on(RoomStateEvent.Marker, async function( + markerEvent, + {fromInitialState}: ISetStateOptions = {}, + ) { // We don't want to refresh the timeline: // 1. If it's persons first time syncing the room, they won't have // any old events cached to refresh. // 1. If we're re-hydrating from `syncFromCache` because we already // processed any marker event state that was in the cache - if(fromInitialState) { + if (fromInitialState) { console.log('fromInitialState ignoring'); return; } - const isValidMsc2716Event = MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || + const isValidMsc2716Event = MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || // MSC2716 is supported in all existing room versions but special // meaning should only be given to "insertion", "batch", and // "marker" events when they come from the room creator markerEvent.getSender() === room.getRoomCreator(); console.log(`On marker state isValidMsc2716Event=${isValidMsc2716Event}, event_id=${markerEvent.getId()} room.getLastMarkerEventIdProcessed()=${room.getLastMarkerEventIdProcessed()}`); - if( + if ( isValidMsc2716Event && // We only need to throw the timeline away once, when we see a // marker. All of the historical content will be in the @@ -284,8 +287,8 @@ export class SyncApi { // need to refresh the timeline if the timeline includes the // prev_events of the base insertion event. console.log('Saw new marker event', new Error().stack); - this.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); - room.setLastMarkerEventIdProcessed(markerEvent.getId()) + room.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); + room.setLastMarkerEventIdProcessed(markerEvent.getId()); } }); } From 05b98000953b6d7c5754b3add1c3d355dcdeb750 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 21:37:52 -0500 Subject: [PATCH 08/20] Fix some lints --- src/models/room.ts | 4 ++-- src/sync.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index 092177f484..08e8c37fe1 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -212,7 +212,7 @@ export class Room extends TypedEventEmitter public readonly threadsTimelineSets: EventTimelineSet[] = []; // any filtered timeline sets we're maintaining for this room private readonly filteredTimelineSets: Record = {}; // filter_id: timelineSet - private timelineNeedsRefresh: boolean = false; + private timelineNeedsRefresh = false; private lastMarkerEventIdProcessed: string = null; private readonly pendingEventList?: MatrixEvent[]; // read by megolm via getter; boolean value - null indicates "use global value" @@ -1815,7 +1815,7 @@ export class Room extends TypedEventEmitter timelineSet.addEventToTimeline( thread.rootEvent, timelineSet.getLiveTimeline(), - {toStartOfTimeline}, + { toStartOfTimeline }, ); } } diff --git a/src/sync.ts b/src/sync.ts index 32827eb352..1bba80e3fa 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -253,7 +253,7 @@ export class SyncApi { // TODO: Should we just move this to `room.ts`? room.currentState.on(RoomStateEvent.Marker, async function( markerEvent, - {fromInitialState}: ISetStateOptions = {}, + { fromInitialState }: ISetStateOptions = {}, ) { // We don't want to refresh the timeline: // 1. If it's persons first time syncing the room, they won't have From 8b82bb519927913496e6680a17fb55803a4d232f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 21:42:48 -0500 Subject: [PATCH 09/20] Rename to roomState to align with other options objects --- src/models/event-timeline-set.ts | 2 +- src/models/event-timeline.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 4246636471..be22bbbbc0 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -640,7 +640,7 @@ export class EventTimelineSet extends TypedEventEmitter Date: Tue, 12 Apr 2022 21:47:55 -0500 Subject: [PATCH 10/20] Rename to toStartOfTimeline to align with other options objects --- src/crypto/index.ts | 2 +- src/models/event-timeline-set.ts | 2 +- src/models/event-timeline.ts | 23 +++++++++++++++-------- src/models/room.ts | 2 +- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 116d3ea99a..0515fc2eb0 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -3240,7 +3240,7 @@ export class Crypto extends TypedEventEmitter { diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index be22bbbbc0..0c1b7d15dc 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -639,7 +639,7 @@ export class EventTimelineSet extends TypedEventEmitter }); if (filterType !== ThreadFilterType.My || currentUserParticipated) { timelineSet.getLiveTimeline().addEvent(thread.rootEvent, { - atStart: false, + toStartOfTimeline: false, }); } }); From b2636c366d19c99fe56ef2ca268fe72292073eb6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 22:38:48 -0500 Subject: [PATCH 11/20] Revert arg rename change for unused --- src/crypto/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 0515fc2eb0..116d3ea99a 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -3240,7 +3240,7 @@ export class Crypto extends TypedEventEmitter { From da57a740047206a3065f6c0f88dbd68a7ee42a8f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 12 Apr 2022 23:22:27 -0500 Subject: [PATCH 12/20] Fix up tests --- spec/unit/event-timeline.spec.js | 66 +++++++++++++++++++------------- spec/unit/room.spec.ts | 16 ++++++-- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/spec/unit/event-timeline.spec.js b/spec/unit/event-timeline.spec.js index c9311d0e38..d32287e3e1 100644 --- a/spec/unit/event-timeline.spec.js +++ b/spec/unit/event-timeline.spec.js @@ -49,10 +49,14 @@ describe("EventTimeline", function() { ]; timeline.initialiseState(events); expect(timeline.startState.setStateEvents).toHaveBeenCalledWith( - events, + events, { + fromInitialState: true, + }, ); expect(timeline.endState.setStateEvents).toHaveBeenCalledWith( - events, + events, { + fromInitialState: true, + }, ); }); @@ -73,7 +77,7 @@ describe("EventTimeline", function() { expect(function() { timeline.initialiseState(state); }).not.toThrow(); - timeline.addEvent(event, false); + timeline.addEvent(event, { toStartOfTimeline: false }); expect(function() { timeline.initialiseState(state); }).toThrow(); @@ -149,9 +153,9 @@ describe("EventTimeline", function() { ]; it("should be able to add events to the end", function() { - timeline.addEvent(events[0], false); + timeline.addEvent(events[0], { toStartOfTimeline: false }); const initialIndex = timeline.getBaseIndex(); - timeline.addEvent(events[1], false); + timeline.addEvent(events[1], { toStartOfTimeline: false }); expect(timeline.getBaseIndex()).toEqual(initialIndex); expect(timeline.getEvents().length).toEqual(2); expect(timeline.getEvents()[0]).toEqual(events[0]); @@ -159,9 +163,9 @@ describe("EventTimeline", function() { }); it("should be able to add events to the start", function() { - timeline.addEvent(events[0], true); + timeline.addEvent(events[0], { toStartOfTimeline: true }); const initialIndex = timeline.getBaseIndex(); - timeline.addEvent(events[1], true); + timeline.addEvent(events[1], { toStartOfTimeline: true }); expect(timeline.getBaseIndex()).toEqual(initialIndex + 1); expect(timeline.getEvents().length).toEqual(2); expect(timeline.getEvents()[0]).toEqual(events[1]); @@ -203,9 +207,9 @@ describe("EventTimeline", function() { content: { name: "Old Room Name" }, }); - timeline.addEvent(newEv, false); + timeline.addEvent(newEv, { toStartOfTimeline: false }); expect(newEv.sender).toEqual(sentinel); - timeline.addEvent(oldEv, true); + timeline.addEvent(oldEv, { toStartOfTimeline: true }); expect(oldEv.sender).toEqual(oldSentinel); }); @@ -242,9 +246,9 @@ describe("EventTimeline", function() { const oldEv = utils.mkMembership({ room: roomId, mship: "ban", user: userB, skey: userA, event: true, }); - timeline.addEvent(newEv, false); + timeline.addEvent(newEv, { toStartOfTimeline: false }); expect(newEv.target).toEqual(sentinel); - timeline.addEvent(oldEv, true); + timeline.addEvent(oldEv, { toStartOfTimeline: true }); expect(oldEv.target).toEqual(oldSentinel); }); @@ -262,13 +266,17 @@ describe("EventTimeline", function() { }), ]; - timeline.addEvent(events[0], false); - timeline.addEvent(events[1], false); + timeline.addEvent(events[0], { toStartOfTimeline: false }); + timeline.addEvent(events[1], { toStartOfTimeline: false }); expect(timeline.getState(EventTimeline.FORWARDS).setStateEvents). - toHaveBeenCalledWith([events[0]]); + toHaveBeenCalledWith([events[0]], { + fromInitialState: undefined, + }); expect(timeline.getState(EventTimeline.FORWARDS).setStateEvents). - toHaveBeenCalledWith([events[1]]); + toHaveBeenCalledWith([events[1]], { + fromInitialState: undefined, + }); expect(events[0].forwardLooking).toBe(true); expect(events[1].forwardLooking).toBe(true); @@ -291,13 +299,17 @@ describe("EventTimeline", function() { }), ]; - timeline.addEvent(events[0], true); - timeline.addEvent(events[1], true); + timeline.addEvent(events[0], { toStartOfTimeline: true }); + timeline.addEvent(events[1], { toStartOfTimeline: true }); expect(timeline.getState(EventTimeline.BACKWARDS).setStateEvents). - toHaveBeenCalledWith([events[0]]); + toHaveBeenCalledWith([events[0]], { + fromInitialState: undefined, + }); expect(timeline.getState(EventTimeline.BACKWARDS).setStateEvents). - toHaveBeenCalledWith([events[1]]); + toHaveBeenCalledWith([events[1]], { + fromInitialState: undefined, + }); expect(events[0].forwardLooking).toBe(false); expect(events[1].forwardLooking).toBe(false); @@ -324,8 +336,8 @@ describe("EventTimeline", function() { ]; it("should remove events", function() { - timeline.addEvent(events[0], false); - timeline.addEvent(events[1], false); + timeline.addEvent(events[0], { toStartOfTimeline: false }); + timeline.addEvent(events[1], { toStartOfTimeline: false }); expect(timeline.getEvents().length).toEqual(2); let ev = timeline.removeEvent(events[0].getId()); @@ -338,9 +350,9 @@ describe("EventTimeline", function() { }); it("should update baseIndex", function() { - timeline.addEvent(events[0], false); - timeline.addEvent(events[1], true); - timeline.addEvent(events[2], false); + timeline.addEvent(events[0], { toStartOfTimeline: false }); + timeline.addEvent(events[1], { toStartOfTimeline: true }); + timeline.addEvent(events[2], { toStartOfTimeline: false }); expect(timeline.getEvents().length).toEqual(3); expect(timeline.getBaseIndex()).toEqual(1); @@ -358,11 +370,11 @@ describe("EventTimeline", function() { // further addEvent(ev, false) calls made the index increase. it("should not make baseIndex assplode when removing the last event", function() { - timeline.addEvent(events[0], true); + timeline.addEvent(events[0], { toStartOfTimeline: true }); timeline.removeEvent(events[0].getId()); const initialIndex = timeline.getBaseIndex(); - timeline.addEvent(events[1], false); - timeline.addEvent(events[2], false); + timeline.addEvent(events[1], { toStartOfTimeline: false }); + timeline.addEvent(events[2], { toStartOfTimeline: false }); expect(timeline.getBaseIndex()).toEqual(initialIndex); expect(timeline.getEvents().length).toEqual(2); }); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 85f4c21d57..bb7b3887ea 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -209,7 +209,9 @@ describe("Room", function() { it("should throw if duplicateStrategy isn't 'replace' or 'ignore'", function() { expect(function() { - room.addLiveEvents(events, "foo"); + room.addLiveEvents(events, { + duplicateStrategy: "foo", + }); }).toThrow(); }); @@ -221,7 +223,9 @@ describe("Room", function() { dupe.event.event_id = events[0].getId(); room.addLiveEvents(events); expect(room.timeline[0]).toEqual(events[0]); - room.addLiveEvents([dupe], DuplicateStrategy.Replace); + room.addLiveEvents([dupe], { + duplicateStrategy: DuplicateStrategy.Replace, + }); expect(room.timeline[0]).toEqual(dupe); }); @@ -233,7 +237,9 @@ describe("Room", function() { dupe.event.event_id = events[0].getId(); room.addLiveEvents(events); expect(room.timeline[0]).toEqual(events[0]); - room.addLiveEvents([dupe], "ignore"); + room.addLiveEvents([dupe], { + duplicateStrategy: "ignore", + }); expect(room.timeline[0]).toEqual(events[0]); }); @@ -266,9 +272,11 @@ describe("Room", function() { room.addLiveEvents(events); expect(room.currentState.setStateEvents).toHaveBeenCalledWith( [events[0]], + { fromInitialState: false }, ); expect(room.currentState.setStateEvents).toHaveBeenCalledWith( [events[1]], + { fromInitialState: false }, ); expect(events[0].forwardLooking).toBe(true); expect(events[1].forwardLooking).toBe(true); @@ -470,9 +478,11 @@ describe("Room", function() { room.addEventsToTimeline(events, true, room.getLiveTimeline()); expect(room.oldState.setStateEvents).toHaveBeenCalledWith( [events[0]], + { fromInitialState: false }, ); expect(room.oldState.setStateEvents).toHaveBeenCalledWith( [events[1]], + { fromInitialState: false }, ); expect(events[0].forwardLooking).toBe(false); expect(events[1].forwardLooking).toBe(false); From c539b647de590d75858a8ef60634c0fefc1fc2d3 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 01:22:04 -0500 Subject: [PATCH 13/20] Fix timeline-window specs --- spec/unit/timeline-window.spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/unit/timeline-window.spec.js b/spec/unit/timeline-window.spec.js index c9466412c8..f118dc1215 100644 --- a/spec/unit/timeline-window.spec.js +++ b/spec/unit/timeline-window.spec.js @@ -35,13 +35,15 @@ function createTimeline(numEvents, baseIndex) { return timeline; } -function addEventsToTimeline(timeline, numEvents, atStart) { +function addEventsToTimeline(timeline, numEvents, toStartOfTimeline) { for (let i = 0; i < numEvents; i++) { timeline.addEvent( utils.mkMessage({ room: ROOM_ID, user: USER_ID, event: true, - }), atStart, + }), { + toStartOfTimeline + }, ); } } From 11040e593efe7932bdd6481d7831f56ae10aa9b6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 01:48:26 -0500 Subject: [PATCH 14/20] Stop prompting to refresh timeline when syncing from cache --- src/models/event-timeline-set.ts | 8 +++++++- src/sync.ts | 9 ++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 0c1b7d15dc..9973dc0873 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -641,7 +641,13 @@ export class EventTimelineSet extends TypedEventEmitter Date: Wed, 13 Apr 2022 02:27:08 -0500 Subject: [PATCH 15/20] Fix lints and move to logger --- spec/unit/timeline-window.spec.js | 2 +- src/sync.ts | 52 ++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/spec/unit/timeline-window.spec.js b/spec/unit/timeline-window.spec.js index f118dc1215..a0d6120730 100644 --- a/spec/unit/timeline-window.spec.js +++ b/spec/unit/timeline-window.spec.js @@ -42,7 +42,7 @@ function addEventsToTimeline(timeline, numEvents, toStartOfTimeline) { room: ROOM_ID, user: USER_ID, event: true, }), { - toStartOfTimeline + toStartOfTimeline, }, ); } diff --git a/src/sync.ts b/src/sync.ts index baeb08572e..3b337bd597 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -262,22 +262,46 @@ export class SyncApi { // 2. If we're re-hydrating from `syncFromCache` because we already // processed any marker event state that was in the cache if (fromInitialState) { - console.log('fromInitialState ignoring'); + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} ` + + `because it's from initial state.`, + ); return; } - const isValidMsc2716Event = MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || - // MSC2716 is supported in all existing room versions but special - // meaning should only be given to "insertion", "batch", and - // "marker" events when they come from the room creator + const isValidMsc2716Event = + // Check whether the room version directly supports MSC2716, in + // which case, "marker" events are already auth'ed by + // power_levels + MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || + // MSC2716 is also supported in all existing room versions but + // special meaning should only be given to "insertion", "batch", + // and "marker" events when they come from the room creator markerEvent.getSender() === room.getRoomCreator(); - console.log(`On marker state isValidMsc2716Event=${isValidMsc2716Event}, event_id=${markerEvent.getId()} room.getLastMarkerEventIdProcessed()=${room.getLastMarkerEventIdProcessed()}`); + + if (!isValidMsc2716Event) { + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} because ` + + `MSC2716 is not supported in the room version or for any room version, the marker wasn't sent ` + + `by the room creator.`, + ); + } + + // Don't process a marker event multiple times; we only need to + // throw the timeline away once, when we see a marker. All of the + // historical content will be in the `/messsages` responses from + // here on out. + const markerAlreadyProcessed = markerEvent.getId() === room.getLastMarkerEventIdProcessed(); + if (markerAlreadyProcessed) { + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} because ` + + `it has already been processed.`, + ); + } + if ( isValidMsc2716Event && - // We only need to throw the timeline away once, when we see a - // marker. All of the historical content will be in the - // `/messsages` responses from here on out. - markerEvent.getId() !== room.getLastMarkerEventIdProcessed() + !markerAlreadyProcessed ) { // Saw new marker event, let's let the clients know they should // refresh the timeline. @@ -288,8 +312,14 @@ export class SyncApi { // was imported and whether we have it locally? Because we only // need to refresh the timeline if the timeline includes the // prev_events of the base insertion event. + const originalStackTraceLimit = Error.stackTraceLimit; Error.stackTraceLimit = Infinity; - console.log(`Saw new marker event roomId=${room.roomId}`, new Error().stack); + logger.debug( + `MarkerState: Timeline needs to be refreshed because ` + + `a new markerEventId=${markerEvent.getId()} was sent in roomId=${room.roomId}`, + new Error().stack, + ); + Error.stackTraceLimit = originalStackTraceLimit; room.setTimelineNeedsRefresh(true); room.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); room.setLastMarkerEventIdProcessed(markerEvent.getId()); From 701bf34ca4d48786eecfef839573d3081b19b2fc Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 15:39:22 -0500 Subject: [PATCH 16/20] Remove todo --- src/sync.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sync.ts b/src/sync.ts index 3b337bd597..e6e48e6534 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -250,7 +250,6 @@ export class SyncApi { ]); }); - // TODO: Should we just move this to `room.ts`? room.currentState.on(RoomStateEvent.Marker, async function( markerEvent, { fromInitialState }: ISetStateOptions = {}, From c080c6c42373833ab8d6341cdf45d224c6144290 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 15:45:42 -0500 Subject: [PATCH 17/20] Remove fixme in favor of aspirational explanation of what we could do better --- src/sync.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index e6e48e6534..f9f828a955 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -305,20 +305,16 @@ export class SyncApi { // Saw new marker event, let's let the clients know they should // refresh the timeline. // - // FIXME: Is there a way we could lookup - // `markerEvent.getContent()['m.insertion_id']` and get the - // prev_events of that event to see exactly where the history - // was imported and whether we have it locally? Because we only + // It would be nice if we could lookup the base insertion event + // that the marker event was pointing to because we really only // need to refresh the timeline if the timeline includes the - // prev_events of the base insertion event. - const originalStackTraceLimit = Error.stackTraceLimit; - Error.stackTraceLimit = Infinity; + // prev_events of the base insertion event and won't re-request + // `/messages` over that range in the timeline. But the problem + // is we can't see prev_events from the client API. logger.debug( `MarkerState: Timeline needs to be refreshed because ` + `a new markerEventId=${markerEvent.getId()} was sent in roomId=${room.roomId}`, - new Error().stack, ); - Error.stackTraceLimit = originalStackTraceLimit; room.setTimelineNeedsRefresh(true); room.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); room.setLastMarkerEventIdProcessed(markerEvent.getId()); From 0694b8473582571737a7750a84db0a1b780ff2cc Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 16:05:07 -0500 Subject: [PATCH 18/20] Add some explanation comments --- src/sync.ts | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index f9f828a955..51e34e7cfe 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -250,6 +250,11 @@ export class SyncApi { ]); }); + // When we see the marker state change in the room, we know there is + // some new historical messages imported by MSC2716 `/batch_send` + // somewhere in the room and we need to throw away the timeline to make + // sure the historical messages are shown when we paginate `/messages` + // again. room.currentState.on(RoomStateEvent.Marker, async function( markerEvent, { fromInitialState }: ISetStateOptions = {}, @@ -298,19 +303,24 @@ export class SyncApi { ); } + // It would be nice if we could also specifically tell whether the + // historical messages actually affected the locally cached client + // timeline or not. The problem is we can't see the prev_events of + // the base insertion event that the marker was pointing to because + // prev_events aren't available in the client API's. In most cases, + // the history won't be in people's locally cached timelines in the + // client, so we don't need to bother everyone about refreshing + // their timeline. This works for a v1 though and there are use + // cases like initially bootstrapping your bridged room where people + // are likely to encounter the historical messages affecting their + // current timeline (think someone signing up for Beeper and + // importing their Whatsapp history). if ( isValidMsc2716Event && !markerAlreadyProcessed ) { // Saw new marker event, let's let the clients know they should // refresh the timeline. - // - // It would be nice if we could lookup the base insertion event - // that the marker event was pointing to because we really only - // need to refresh the timeline if the timeline includes the - // prev_events of the base insertion event and won't re-request - // `/messages` over that range in the timeline. But the problem - // is we can't see prev_events from the client API. logger.debug( `MarkerState: Timeline needs to be refreshed because ` + `a new markerEventId=${markerEvent.getId()} was sent in roomId=${room.roomId}`, From 26fb2b6cbd59f8c5252d57d1343437b8ae6e43fd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 18:48:48 -0500 Subject: [PATCH 19/20] Add some tests --- spec/integ/matrix-client-syncing.spec.js | 99 ++++++++++++++ spec/unit/room-state.spec.js | 24 ++++ src/models/room-state.ts | 4 +- src/models/room.ts | 2 +- src/sync.ts | 163 ++++++++++++----------- 5 files changed, 213 insertions(+), 79 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 6adb35a50c..a54194d31a 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -1,5 +1,6 @@ import { MatrixEvent } from "../../src/models/event"; import { EventTimeline } from "../../src/models/event-timeline"; +import { EventType } from "../../src/@types/event"; import * as utils from "../test-utils/test-utils"; import { TestClient } from "../TestClient"; @@ -461,6 +462,104 @@ describe("MatrixClient syncing", function() { xit("should update the room topic", function() { }); + + describe("onMarkerStateEvent", () => { + it('no marker means the timeline does not need a refresh (check a sane default)', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [ + utils.mkMessage({ + room: roomOne, user: otherUserId, msg: "hello", + }), + ], + prev_batch: "pagTok", + }, + state: { + events: [ + utils.mkEvent({ + type: "m.room.create", room: roomOne, user: otherUserId, + content: { + creator: otherUserId, + }, + }), + ], + }, + }; + + httpBackend.when("GET", "/sync").respond(200, syncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + const room = client.getRoom(roomOne); + expect(room.getTimelineNeedsRefresh()).toEqual(false); + }); + + it('a new marker event should mark the timeline as needing a refresh', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [ + utils.mkEvent({ + type: EventType.Marker, room: roomOne, user: otherUserId, + skey: "", + content: { + "m.insertion_id": "$abc", + }, + }), + ], + prev_batch: "pagTok", + }, + state: { + events: [ + utils.mkEvent({ + type: "m.room.create", room: roomOne, user: otherUserId, + content: { + creator: otherUserId, + }, + }), + ], + }, + }; + + const markerEventId = syncData.rooms.join[roomOne].timeline.events[0].event_id; + + let emitCount = 0; + client.on("Room.historyImportedWithinTimeline", function(markerEvent, room) { + expect(markerEvent.getId()).toEqual(markerEventId); + expect(room.roomId).toEqual(roomOne); + emitCount += 1; + }); + + httpBackend.when("GET", "/sync").respond(200, syncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + const room = client.getRoom(roomOne); + expect(room.getTimelineNeedsRefresh()).toEqual(true); + // Make sure "Room.historyImportedWithinTimeline" was emitted + expect(emitCount).toEqual(1); + expect(room.getLastMarkerEventIdProcessed()).toEqual(markerEventId); + }); + }); }); describe("timeline", function() { diff --git a/spec/unit/room-state.spec.js b/spec/unit/room-state.spec.js index 261f7572d9..cefa3c5fe5 100644 --- a/spec/unit/room-state.spec.js +++ b/spec/unit/room-state.spec.js @@ -3,6 +3,7 @@ import { makeBeaconEvent, makeBeaconInfoEvent } from "../test-utils/beacon"; import { filterEmitCallsByEventType } from "../test-utils/emitter"; import { RoomState, RoomStateEvent } from "../../src/models/room-state"; import { BeaconEvent, getBeaconInfoIdentifier } from "../../src/models/beacon"; +import { EventType } from "../../src/@types/event"; describe("RoomState", function() { const roomId = "!foo:bar"; @@ -252,6 +253,29 @@ describe("RoomState", function() { ); }); + it("should emit 'RoomStateEvent.Marker' for each marker event", function() { + const events = [ + utils.mkEvent({ + event: true, + type: EventType.Marker, + room: roomId, + user: userA, + skey: "", + content: { + "m.insertion_id": "$abc", + }, + }), + ]; + let emitCount = 0; + state.on("RoomStateEvent.Marker", function(markerEvent, setStateOptions) { + expect(markerEvent).toEqual(events[emitCount]); + expect(setStateOptions).toEqual({ fromInitialState: true }); + emitCount += 1; + }); + state.setStateEvents(events, { fromInitialState: true }); + expect(emitCount).toEqual(1); + }); + describe('beacon events', () => { it('adds new beacon info events to state and emits', () => { const beaconEvent = makeBeaconInfoEvent(userA, roomId); diff --git a/src/models/room-state.ts b/src/models/room-state.ts index 8272043e8e..fc8a727f27 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -326,13 +326,15 @@ export class RoomState extends TypedEventEmitter * Add an array of one or more state MatrixEvents, overwriting any existing * state with the same {type, stateKey} tuple. Will fire "RoomState.events" * for every event added. May fire "RoomState.members" if there are - * m.room.member events. + * m.room.member events. May fire "RoomStateEvent.Marker" if there are + * EventType.Marker events. * @param {MatrixEvent[]} stateEvents a list of state events for this room. * @param {boolean} fromInitialState whether the stateEvents are from the first * sync in the room or a sync we already know about (syncFromCache) * @fires module:client~MatrixClient#event:"RoomState.members" * @fires module:client~MatrixClient#event:"RoomState.newMember" * @fires module:client~MatrixClient#event:"RoomState.events" + * @fires module:client~MatrixClient#event:"RoomStateEvent.Marker" */ public setStateEvents(stateEvents: MatrixEvent[], setStateOptions?: ISetStateOptions) { this.updateModifiedTime(); diff --git a/src/models/room.ts b/src/models/room.ts index 7db386026e..e6f566c153 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -460,7 +460,7 @@ export class Room extends TypedEventEmitter if (!createEvent) { return null; } - const roomCreator = createEvent.getContent()['sender']; + const roomCreator = createEvent.getContent()['creator']; return roomCreator; } diff --git a/src/sync.ts b/src/sync.ts index 51e34e7cfe..33b87d4806 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -250,86 +250,95 @@ export class SyncApi { ]); }); - // When we see the marker state change in the room, we know there is - // some new historical messages imported by MSC2716 `/batch_send` - // somewhere in the room and we need to throw away the timeline to make - // sure the historical messages are shown when we paginate `/messages` - // again. - room.currentState.on(RoomStateEvent.Marker, async function( - markerEvent, - { fromInitialState }: ISetStateOptions = {}, - ) { - // We don't want to refresh the timeline: - // 1. If it's persons first time syncing the room, they won't have - // any old events cached to refresh. This could be from initial - // sync or just the first time syncing the room since joining. - // 2. If we're re-hydrating from `syncFromCache` because we already - // processed any marker event state that was in the cache - if (fromInitialState) { - logger.debug( - `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} ` + - `because it's from initial state.`, - ); - return; - } + room.currentState.on(RoomStateEvent.Marker, (markerEvent, setStateOptions) => { + this.onMarkerStateEvent(room, markerEvent, setStateOptions); + }); + } - const isValidMsc2716Event = - // Check whether the room version directly supports MSC2716, in - // which case, "marker" events are already auth'ed by - // power_levels - MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || - // MSC2716 is also supported in all existing room versions but - // special meaning should only be given to "insertion", "batch", - // and "marker" events when they come from the room creator - markerEvent.getSender() === room.getRoomCreator(); - - if (!isValidMsc2716Event) { - logger.debug( - `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} because ` + - `MSC2716 is not supported in the room version or for any room version, the marker wasn't sent ` + - `by the room creator.`, - ); - } + /** When we see the marker state change in the room, we know there is some + * new historical messages imported by MSC2716 `/batch_send` somewhere in + * the room and we need to throw away the timeline to make sure the + * historical messages are shown when we paginate `/messages` again. + * @param {Room} room The room where the marker event was sent + * @param {MatrixEvent} markerEvent The new marker event + * @param {ISetStateOptions} setStateOptions When `fromInitialState` is set + * as `true`, the given marker event will be ignored + */ + private onMarkerStateEvent( + room: Room, + markerEvent: MatrixEvent, + { fromInitialState }: ISetStateOptions = {}, + ): void { + // We don't want to refresh the timeline: + // 1. If it's persons first time syncing the room, they won't have + // any old events cached to refresh. This could be from initial + // sync or just the first time syncing the room since joining. + // 2. If we're re-hydrating from `syncFromCache` because we already + // processed any marker event state that was in the cache + if (fromInitialState) { + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} ` + + `because it's from initial state.`, + ); + return; + } - // Don't process a marker event multiple times; we only need to - // throw the timeline away once, when we see a marker. All of the - // historical content will be in the `/messsages` responses from - // here on out. - const markerAlreadyProcessed = markerEvent.getId() === room.getLastMarkerEventIdProcessed(); - if (markerAlreadyProcessed) { - logger.debug( - `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} because ` + - `it has already been processed.`, - ); - } + const isValidMsc2716Event = + // Check whether the room version directly supports MSC2716, in + // which case, "marker" events are already auth'ed by + // power_levels + MSC2716_ROOM_VERSIONS.includes(room.getVersion()) || + // MSC2716 is also supported in all existing room versions but + // special meaning should only be given to "insertion", "batch", + // and "marker" events when they come from the room creator + markerEvent.getSender() === room.getRoomCreator(); + + if (!isValidMsc2716Event) { + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} because ` + + `MSC2716 is not supported in the room version or for any room version, the marker wasn't sent ` + + `by the room creator.`, + ); + } - // It would be nice if we could also specifically tell whether the - // historical messages actually affected the locally cached client - // timeline or not. The problem is we can't see the prev_events of - // the base insertion event that the marker was pointing to because - // prev_events aren't available in the client API's. In most cases, - // the history won't be in people's locally cached timelines in the - // client, so we don't need to bother everyone about refreshing - // their timeline. This works for a v1 though and there are use - // cases like initially bootstrapping your bridged room where people - // are likely to encounter the historical messages affecting their - // current timeline (think someone signing up for Beeper and - // importing their Whatsapp history). - if ( - isValidMsc2716Event && - !markerAlreadyProcessed - ) { - // Saw new marker event, let's let the clients know they should - // refresh the timeline. - logger.debug( - `MarkerState: Timeline needs to be refreshed because ` + - `a new markerEventId=${markerEvent.getId()} was sent in roomId=${room.roomId}`, - ); - room.setTimelineNeedsRefresh(true); - room.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); - room.setLastMarkerEventIdProcessed(markerEvent.getId()); - } - }); + // Don't process a marker event multiple times; we only need to + // throw the timeline away once, when we see a marker. All of the + // historical content will be in the `/messsages` responses from + // here on out. + const markerAlreadyProcessed = markerEvent.getId() === room.getLastMarkerEventIdProcessed(); + if (markerAlreadyProcessed) { + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} because ` + + `it has already been processed.`, + ); + } + + // It would be nice if we could also specifically tell whether the + // historical messages actually affected the locally cached client + // timeline or not. The problem is we can't see the prev_events of + // the base insertion event that the marker was pointing to because + // prev_events aren't available in the client API's. In most cases, + // the history won't be in people's locally cached timelines in the + // client, so we don't need to bother everyone about refreshing + // their timeline. This works for a v1 though and there are use + // cases like initially bootstrapping your bridged room where people + // are likely to encounter the historical messages affecting their + // current timeline (think someone signing up for Beeper and + // importing their Whatsapp history). + if ( + isValidMsc2716Event && + !markerAlreadyProcessed + ) { + // Saw new marker event, let's let the clients know they should + // refresh the timeline. + logger.debug( + `MarkerState: Timeline needs to be refreshed because ` + + `a new markerEventId=${markerEvent.getId()} was sent in roomId=${room.roomId}`, + ); + room.setTimelineNeedsRefresh(true); + room.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); + room.setLastMarkerEventIdProcessed(markerEvent.getId()); + } } /** From 8ca2645bfafcb4564fcb697d4f911e47dd13f864 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 13 Apr 2022 20:13:08 -0500 Subject: [PATCH 20/20] WIP: messy timelineWasEmpty --- spec/integ/matrix-client-syncing.spec.js | 115 ++++++++++++++++++++++- src/models/event-timeline.ts | 3 +- src/models/room-state.ts | 6 ++ src/sync.ts | 1 + 4 files changed, 120 insertions(+), 5 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index a54194d31a..8703ef2958 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -463,7 +463,7 @@ describe("MatrixClient syncing", function() { }); - describe("onMarkerStateEvent", () => { + fdescribe("onMarkerStateEvent", () => { it('no marker means the timeline does not need a refresh (check a sane default)', async () => { const syncData = { next_batch: "batch_token", @@ -504,7 +504,7 @@ describe("MatrixClient syncing", function() { expect(room.getTimelineNeedsRefresh()).toEqual(false); }); - it('a new marker event should mark the timeline as needing a refresh', async () => { + fit('when this is our first sync for the room, there is no timeline to refresh', async () => { const syncData = { next_batch: "batch_token", rooms: { @@ -536,7 +536,68 @@ describe("MatrixClient syncing", function() { }, }; - const markerEventId = syncData.rooms.join[roomOne].timeline.events[0].event_id; + httpBackend.when("GET", "/sync").respond(200, syncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + const room = client.getRoom(roomOne); + expect(room.getTimelineNeedsRefresh()).toEqual(false); + }); + + it('a new marker event should mark the timeline as needing a refresh', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [ + utils.mkMessage({ + room: roomOne, user: otherUserId, msg: "hello", + }), + ], + prev_batch: "pagTok", + }, + state: { + events: [ + utils.mkEvent({ + type: "m.room.create", room: roomOne, user: otherUserId, + content: { + creator: otherUserId, + }, + }), + ], + }, + }; + + const nextSyncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + nextSyncData.rooms.join[roomOne] = { + timeline: { + events: [ + utils.mkEvent({ + type: EventType.Marker, room: roomOne, user: otherUserId, + skey: "", + content: { + "m.insertion_id": "$abc", + }, + }), + ], + prev_batch: "pagTok", + }, + }; + + const markerEventId = nextSyncData.rooms.join[roomOne].timeline.events[0].event_id; let emitCount = 0; client.on("Room.historyImportedWithinTimeline", function(markerEvent, room) { @@ -546,11 +607,12 @@ describe("MatrixClient syncing", function() { }); httpBackend.when("GET", "/sync").respond(200, syncData); + httpBackend.when("GET", "/sync").respond(200, nextSyncData); client.startClient(); await Promise.all([ httpBackend.flushAllExpected(), - awaitSyncEvent(), + awaitSyncEvent(2), ]); const room = client.getRoom(roomOne); @@ -559,6 +621,51 @@ describe("MatrixClient syncing", function() { expect(emitCount).toEqual(1); expect(room.getLastMarkerEventIdProcessed()).toEqual(markerEventId); }); + + it('marker event sent far back in the scroll back but since our last sync will cause the timeline to refresh', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [ + // TODO: Update this scenario to match test title + utils.mkEvent({ + type: EventType.Marker, room: roomOne, user: otherUserId, + skey: "", + content: { + "m.insertion_id": "$abc", + }, + }), + ], + prev_batch: "pagTok", + }, + state: { + events: [ + utils.mkEvent({ + type: "m.room.create", room: roomOne, user: otherUserId, + content: { + creator: otherUserId, + }, + }), + ], + }, + }; + + httpBackend.when("GET", "/sync").respond(200, syncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + const room = client.getRoom(roomOne); + expect(room.getTimelineNeedsRefresh()).toEqual(true); + }); }); }); diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts index 5cc337e372..7e9f3df369 100644 --- a/src/models/event-timeline.ts +++ b/src/models/event-timeline.ts @@ -146,6 +146,7 @@ export class EventTimeline { * @throws {Error} if an attempt is made to call this after addEvent is called. */ public initialiseState(stateEvents: MatrixEvent[]): void { + console.log('initialiseState', stateEvents.map((ev) => { return ev.getType(); })); if (this.events.length > 0) { throw new Error("Cannot initialise state after events are added"); } @@ -387,7 +388,7 @@ export class EventTimeline { timelineSet.room.getUnfilteredTimelineSet() === timelineSet ) { roomState.setStateEvents([event], { - fromInitialState, + fromInitialState: fromInitialState, }); // it is possible that the act of setting the state event means we // can set more metadata (specifically sender/target props), so try diff --git a/src/models/room-state.ts b/src/models/room-state.ts index fc8a727f27..248c7428d3 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -40,6 +40,8 @@ enum OobStatus { } export interface ISetStateOptions { + /** Whether the sync response came from cache */ + fromCache?: boolean; /** Whether the state is part of the first state snapshot we're seeing in * the room. This could be happen in a variety of cases: * 1. From the initial sync @@ -337,6 +339,10 @@ export class RoomState extends TypedEventEmitter * @fires module:client~MatrixClient#event:"RoomStateEvent.Marker" */ public setStateEvents(stateEvents: MatrixEvent[], setStateOptions?: ISetStateOptions) { + Error.stackTraceLimit = Infinity; + console.log(`setStateEvents fromInitialState=${setStateOptions && setStateOptions.fromInitialState} stateEvents:\n`, stateEvents.map((ev) => { + return `\t${ev.getType()} ${ev.getId()}`; + }).join('\n'), new Error().stack); this.updateModifiedTime(); // update the core event dict diff --git a/src/sync.ts b/src/sync.ts index 33b87d4806..df3b658206 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1738,6 +1738,7 @@ export class SyncApi { room.addLiveEvents(timelineEventList || [], { fromCache, + fromInitialState: timelineWasEmpty || fromCache }); this.client.processBeaconEvents(room, timelineEventList); }