From d37fa8103116f2583da23cb3d08239cc0700d6a1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 14 Apr 2022 00:06:02 -0500 Subject: [PATCH 01/51] Timeline needs to refresh when we see a MSC2716 marker event > 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. > > *-- https://github.com/matrix-org/matrix-spec-proposals/pull/2716#discussion_r782629097* --- spec/integ/matrix-client-syncing.spec.js | 299 +++++++++++++++++++++++ spec/unit/room-state.spec.js | 24 ++ src/@types/event.ts | 1 + src/client.ts | 2 + src/models/event-timeline-set.ts | 83 +++++-- src/models/event-timeline.ts | 59 +++-- src/models/room-state.ts | 26 +- src/models/room.ts | 113 ++++++--- src/sync.ts | 108 +++++++- 9 files changed, 650 insertions(+), 65 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 6adb35a50c0..b499001a548 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,304 @@ describe("MatrixClient syncing", function() { xit("should update the room topic", function() { }); + + describe("onMarkerStateEvent", () => { + const normalMessageEvent = utils.mkMessage({ + room: roomOne, user: otherUserId, msg: "hello", + }); + + it('new marker event *NOT* from the room creator in a subsequent syncs ' + + 'should *NOT* mark the timeline as needing a refresh', async () => { + const roomCreateEvent = utils.mkEvent({ + type: "m.room.create", room: roomOne, user: otherUserId, + content: { + creator: otherUserId, + room_version: '9', + }, + }); + const normalFirstSync = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + normalFirstSync.rooms.join[roomOne] = { + timeline: { + events: [normalMessageEvent], + prev_batch: "pagTok", + }, + state: { + events: [roomCreateEvent], + }, + }; + + const nextSyncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + nextSyncData.rooms.join[roomOne] = { + timeline: { + events: [ + // In subsequent syncs, a marker event in timeline + // range should normally trigger + // `timelineNeedsRefresh=true` but this marker isn't + // being sent by the room creator so it has no + // special meaning in existing room versions. + utils.mkEvent({ + type: EventType.Marker, + room: roomOne, + // The important part we're testing is here! + // `userC` is not the room creator. + user: userC, + skey: "", + content: { + "m.insertion_id": "$abc", + }, + }), + ], + prev_batch: "pagTok", + }, + }; + + // Ensure the marker is being sent by someone who is not the room creator + // because this is the main thing we're testing in this spec. + const markerEvent = nextSyncData.rooms.join[roomOne].timeline.events[0]; + expect(markerEvent.sender).toBeDefined(); + expect(markerEvent.sender).not.toEqual(roomCreateEvent.sender); + + httpBackend.when("GET", "/sync").respond(200, normalFirstSync); + httpBackend.when("GET", "/sync").respond(200, nextSyncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(2), + ]); + + const room = client.getRoom(roomOne); + expect(room.getTimelineNeedsRefresh()).toEqual(false); + }); + + [{ + label: 'In existing room versions (when the room creator sends the MSC2716 events)', + roomVersion: '9', + }, { + label: 'In a MSC2716 supported room version', + roomVersion: 'org.matrix.msc2716v3', + }].forEach((testMeta) => { + describe(testMeta.label, () => { + const roomCreateEvent = utils.mkEvent({ + type: "m.room.create", room: roomOne, user: otherUserId, + content: { + creator: otherUserId, + room_version: testMeta.roomVersion, + }, + }); + + const markerEventFromRoomCreator = utils.mkEvent({ + type: EventType.Marker, room: roomOne, user: otherUserId, + skey: "", + content: { + "m.insertion_id": "$abc", + }, + }); + + const normalFirstSync = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + normalFirstSync.rooms.join[roomOne] = { + timeline: { + events: [normalMessageEvent], + prev_batch: "pagTok", + }, + state: { + events: [roomCreateEvent], + }, + }; + + it('no marker event in sync response '+ + 'should *NOT* mark the timeline as needing a refresh (check for a sane default)', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [normalMessageEvent], + prev_batch: "pagTok", + }, + state: { + events: [roomCreateEvent], + }, + }; + + 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('marker event already sent within timeline range when you join ' + + 'should *NOT* mark the timeline as needing a refresh (timelineWasEmpty)', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [markerEventFromRoomCreator], + prev_batch: "pagTok", + }, + state: { + events: [roomCreateEvent], + }, + }; + + 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('marker event already sent before joining (in state) ' + + 'should *NOT* mark the timeline as needing a refresh (timelineWasEmpty)', async () => { + const syncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomOne] = { + timeline: { + events: [normalMessageEvent], + prev_batch: "pagTok", + }, + state: { + events: [ + roomCreateEvent, + markerEventFromRoomCreator, + ], + }, + }; + + 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('new marker event in a subsequent syncs timeline range ' + + 'should mark the timeline as needing a refresh', async () => { + const nextSyncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + nextSyncData.rooms.join[roomOne] = { + timeline: { + events: [ + // In subsequent syncs, a marker event in timeline + // range should trigger `timelineNeedsRefresh=true` + markerEventFromRoomCreator, + ], + prev_batch: "pagTok", + }, + }; + + const markerEventId = nextSyncData.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, normalFirstSync); + httpBackend.when("GET", "/sync").respond(200, nextSyncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(2), + ]); + + 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); + }); + + // Mimic a marker event being sent far back in the scroll back but since our last sync + it('new marker event in sync state should mark the timeline as needing a refresh', async () => { + const nextSyncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + nextSyncData.rooms.join[roomOne] = { + timeline: { + events: [ + utils.mkMessage({ + room: roomOne, user: otherUserId, msg: "hello again", + }), + ], + prev_batch: "pagTok", + }, + state: { + events: [ + // In subsequent syncs, a marker event in state + // should trigger `timelineNeedsRefresh=true` + markerEventFromRoomCreator, + ], + }, + }; + + httpBackend.when("GET", "/sync").respond(200, normalFirstSync); + httpBackend.when("GET", "/sync").respond(200, nextSyncData); + + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(2), + ]); + + const room = client.getRoom(roomOne); + expect(room.getTimelineNeedsRefresh()).toEqual(true); + }); + }); + }); + }); }); describe("timeline", function() { diff --git a/spec/unit/room-state.spec.js b/spec/unit/room-state.spec.js index 261f7572d91..71ccd52417c 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("RoomState.Marker", function(markerEvent, markerFoundOptions) { + expect(markerEvent).toEqual(events[emitCount]); + expect(markerFoundOptions).toEqual({ timelineWasEmpty: true }); + emitCount += 1; + }); + state.setStateEvents(events, { timelineWasEmpty: 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/@types/event.ts b/src/@types/event.ts index e5eac34f948..6ef74b92ff5 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -33,6 +33,7 @@ export enum EventType { RoomGuestAccess = "m.room.guest_access", RoomServerAcl = "m.room.server_acl", RoomTombstone = "m.room.tombstone", + Marker = "org.matrix.msc2716.marker", // MSC2716 /** * @deprecated Should not be used. */ diff --git a/src/client.ts b/src/client.ts index bc55145e557..5b26cd3c95b 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 @@ -799,6 +800,7 @@ type RoomStateEvents = RoomStateEvent.Events | RoomStateEvent.Members | RoomStateEvent.NewMember | RoomStateEvent.Update + | RoomStateEvent.Marker ; type CryptoEvents = CryptoEvent.KeySignatureUploadFailure diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 13ea8c458f2..c33b8dbe69a 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -56,6 +56,40 @@ export interface IRoomTimelineData { liveEvent?: boolean; } +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 timeline was empty before the marker arrived 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` */ + timelineWasEmpty?: boolean; +} + +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 timeline was empty before the marker arrived 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` */ + timelineWasEmpty?: boolean; +} + type EmittedEvents = RoomEvent.Timeline | RoomEvent.TimelineReset; export type EventTimelineSetHandlerMap = { @@ -431,7 +465,9 @@ export class EventTimelineSet extends TypedEventEmitter 0) { throw new Error("Cannot initialise state after events are added"); } @@ -152,8 +170,12 @@ export class EventTimeline { Object.freeze(e); } - this.startState.setStateEvents(stateEvents); - this.endState.setStateEvents(stateEvents); + this.startState.setStateEvents(stateEvents, { + timelineWasEmpty, + }); + this.endState.setStateEvents(stateEvents, { + timelineWasEmpty, + }); } /** @@ -342,27 +364,36 @@ export class EventTimeline { } /** - * Add a new event to the timeline, and update the state + * Add a new event to the timeline, and update the state * * @param {MatrixEvent} event new event - * @param {boolean} atStart true to insert new event at the start + * @param {IAddEventOptions} options addEvent options */ - public addEvent(event: MatrixEvent, atStart: boolean, stateContext?: RoomState): void { - if (!stateContext) { - stateContext = atStart ? this.startState : this.endState; + public addEvent( + event: MatrixEvent, + { + toStartOfTimeline, + roomState, + timelineWasEmpty, + }: IAddEventOptions, + ): void { + if (!roomState) { + roomState = toStartOfTimeline ? this.startState : this.endState; } const timelineSet = this.getTimelineSet(); if (timelineSet.room) { - EventTimeline.setEventMetadata(event, stateContext, atStart); + EventTimeline.setEventMetadata(event, roomState, toStartOfTimeline); // modify state but only on unfiltered timelineSets if ( event.isState() && timelineSet.room.getUnfilteredTimelineSet() === timelineSet ) { - stateContext.setStateEvents([event]); + roomState.setStateEvents([event], { + timelineWasEmpty, + }); // 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 @@ -373,22 +404,22 @@ export class EventTimeline { // 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() === "m.room.member" && !atStart)) { - EventTimeline.setEventMetadata(event, stateContext, atStart); + if (!event.sender || (event.getType() === "m.room.member" && !toStartOfTimeline)) { + EventTimeline.setEventMetadata(event, roomState, toStartOfTimeline); } } } let insertIndex; - if (atStart) { + if (toStartOfTimeline) { insertIndex = 0; } else { insertIndex = this.events.length; } this.events.splice(insertIndex, 0, event); // insert element - if (atStart) { + if (toStartOfTimeline) { this.baseIndex++; } } diff --git a/src/models/room-state.ts b/src/models/room-state.ts index 5719034397c..e9df01c8011 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -32,6 +32,15 @@ import { M_BEACON_INFO } from "../@types/beacon"; import { getBeaconInfoIdentifier } from "./beacon"; import { BeaconIdentifier } from ".."; +export interface IMarkerFoundOptions { + /** Whether the timeline was empty before the marker arrived 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` */ + timelineWasEmpty?: boolean; +} + // possible statuses for out-of-band member loading enum OobStatus { NotStarted, @@ -45,6 +54,7 @@ export enum RoomStateEvent { NewMember = "RoomState.newMember", Update = "RoomState.update", // signals batches of updates without specificity BeaconLiveness = "RoomState.BeaconLiveness", + Marker = "RoomState.Marker", } export type RoomStateEventHandlerMap = { @@ -53,6 +63,7 @@ export type RoomStateEventHandlerMap = { [RoomStateEvent.NewMember]: (event: MatrixEvent, state: RoomState, member: RoomMember) => void; [RoomStateEvent.Update]: (state: RoomState) => void; [RoomStateEvent.BeaconLiveness]: (state: RoomState, hasLiveBeacons: boolean) => void; + [RoomStateEvent.Marker]: (event: MatrixEvent, setStateOptions: IMarkerFoundOptions) => void; [BeaconEvent.New]: (event: MatrixEvent, beacon: Beacon) => void; }; @@ -312,16 +323,19 @@ 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. May fire "RoomStateEvent.Marker" if there are + * EventType.Marker events. * @param {MatrixEvent[]} stateEvents a list of state events for this room. + * @param {IMarkerFoundOptions} markerFoundOptions * @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[]) { + public setStateEvents(stateEvents: MatrixEvent[], markerFoundOptions?: IMarkerFoundOptions) { this.updateModifiedTime(); // update the core event dict @@ -401,6 +415,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, markerFoundOptions); } }); diff --git a/src/models/room.ts b/src/models/room.ts index af3dd4758b8..e6f566c1533 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 @@ -164,6 +165,7 @@ export enum RoomEvent { LocalEchoUpdated = "Room.localEchoUpdated", Timeline = "Room.timeline", TimelineReset = "Room.timelineReset", + historyImportedWithinTimeline = "Room.historyImportedWithinTimeline", } type EmittedEvents = RoomEvent @@ -172,6 +174,7 @@ type EmittedEvents = RoomEvent | ThreadEvent.NewReply | RoomEvent.Timeline | RoomEvent.TimelineReset + | RoomEvent.historyImportedWithinTimeline | MatrixEventEvent.BeforeRedaction; export type RoomEventHandlerMap = { @@ -188,6 +191,10 @@ export type RoomEventHandlerMap = { oldEventId?: string, oldStatus?: EventStatus, ) => void; + [RoomEvent.historyImportedWithinTimeline]: ( + markerEvent: MatrixEvent, + room: Room, + ) => void; [ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void; } & ThreadHandlerMap & MatrixEventHandlerMap; @@ -205,6 +212,8 @@ 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 = false; + private lastMarkerEventIdProcessed: string = null; private readonly pendingEventList?: MatrixEvent[]; // read by megolm via getter; boolean value - null indicates "use global value" private blacklistUnverifiedDevices: boolean = null; @@ -442,6 +451,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()['creator']; + return roomCreator; + } + /** * Gets the version of the room * @returns {string} The version of the room, or null if it could not be determined @@ -1007,6 +1029,43 @@ 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 + * + * @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 * @@ -1463,7 +1522,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, { + toStartOfTimeline: false, + }); } }); } @@ -1510,22 +1571,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; } @@ -1756,7 +1815,7 @@ export class Room extends TypedEventEmitter timelineSet.addEventToTimeline( thread.rootEvent, timelineSet.getLiveTimeline(), - toStartOfTimeline, + { toStartOfTimeline }, ); } } @@ -1839,15 +1898,14 @@ 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 + * @param {IAddLiveEventOptions} options addLiveEvent options * @fires module:client~MatrixClient#event:"Room.timeline" * @private */ - private addLiveEvent(event: MatrixEvent, duplicateStrategy: DuplicateStrategy, fromCache = false): void { + private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions = {}): void { // 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 @@ -1933,11 +1991,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, + }); } } } @@ -2174,18 +2236,11 @@ export class Room extends TypedEventEmitter * they will go to the end of the timeline. * * @param {MatrixEvent[]} events A list of events to add. - * - * @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 + * @param {IAddLiveEventOptions} options addLiveEvent options * @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'"); } @@ -2226,7 +2281,7 @@ export class Room extends TypedEventEmitter } if (shouldLiveInRoom) { - this.addLiveEvent(events[i], duplicateStrategy, fromCache); + this.addLiveEvent(events[i], addLiveEventOptions); } } diff --git a/src/sync.ts b/src/sync.ts index 30df2cb7e6c..272d341de88 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 { IMarkerFoundOptions } from "./models/room-state"; const DEBUG = true; @@ -77,6 +78,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! @@ -202,6 +210,7 @@ export class SyncApi { RoomEvent.Receipt, RoomEvent.Tags, RoomEvent.LocalEchoUpdated, + RoomEvent.historyImportedWithinTimeline, RoomEvent.AccountData, RoomEvent.MyMembership, RoomEvent.Timeline, @@ -240,6 +249,10 @@ export class SyncApi { RoomMemberEvent.Membership, ]); }); + + room.currentState.on(RoomStateEvent.Marker, (markerEvent, markerFoundOptions) => { + this.onMarkerStateEvent(room, markerEvent, markerFoundOptions); + }); } /** @@ -251,6 +264,92 @@ export class SyncApi { room.currentState.removeAllListeners(RoomStateEvent.Events); room.currentState.removeAllListeners(RoomStateEvent.Members); room.currentState.removeAllListeners(RoomStateEvent.NewMember); + room.currentState.removeAllListeners(RoomStateEvent.Marker); + } + + /** 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 `timelineWasEmpty` is set + * as `true`, the given marker event will be ignored + */ + private onMarkerStateEvent( + room: Room, + markerEvent: MatrixEvent, + { timelineWasEmpty }: IMarkerFoundOptions = {}, + ): void { + // We don't need to refresh the timeline if it was empty before the + // marker arrived. This could be happen in a variety of cases: + // 1. From the initial sync + // 2. If it's from the first state we're seeing after joining the room + // 3. Or whether it's coming from `syncFromCache` + if (timelineWasEmpty) { + logger.debug( + `MarkerState: Ignoring markerEventId=${markerEvent.getId()} in roomId=${room.roomId} ` + + `because the timeline was empty before the marker arrived which means there is nothing to refresh.`, + ); + return; + } + + 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.`, + ); + } + + // 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()); + } } /** @@ -1597,7 +1696,9 @@ export class SyncApi { for (const ev of stateEventList) { this.client.getPushActionsForEvent(ev); } - liveTimeline.initialiseState(stateEventList); + liveTimeline.initialiseState(stateEventList, { + timelineWasEmpty, + }); } this.resolveInvites(room); @@ -1635,7 +1736,10 @@ export class SyncApi { // if the timeline has any state events in it. // This also needs to be done before running push rules on the events as they need // to be decorated with sender etc. - room.addLiveEvents(timelineEventList || [], null, fromCache); + room.addLiveEvents(timelineEventList || [], { + fromCache, + timelineWasEmpty, + }); this.client.processBeaconEvents(room, timelineEventList); } From dbc7d664335bc2a4e2bc1d85acd997ee9f9f5073 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 14 Apr 2022 00:31:29 -0500 Subject: [PATCH 02/51] Fix tests failing from function signature changes --- spec/unit/event-timeline.spec.js | 52 ++++++++++++++++--------------- spec/unit/room.spec.ts | 16 ++++++++-- spec/unit/timeline-window.spec.js | 5 +-- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/spec/unit/event-timeline.spec.js b/spec/unit/event-timeline.spec.js index c9311d0e387..caacf55fc99 100644 --- a/spec/unit/event-timeline.spec.js +++ b/spec/unit/event-timeline.spec.js @@ -50,9 +50,11 @@ describe("EventTimeline", function() { timeline.initialiseState(events); expect(timeline.startState.setStateEvents).toHaveBeenCalledWith( events, + { timelineWasEmpty: undefined }, ); expect(timeline.endState.setStateEvents).toHaveBeenCalledWith( events, + { timelineWasEmpty: undefined }, ); }); @@ -73,7 +75,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 +151,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 +161,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 +205,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 +244,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 +264,13 @@ 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]], { timelineWasEmpty: undefined }); expect(timeline.getState(EventTimeline.FORWARDS).setStateEvents). - toHaveBeenCalledWith([events[1]]); + toHaveBeenCalledWith([events[1]], { timelineWasEmpty: undefined }); expect(events[0].forwardLooking).toBe(true); expect(events[1].forwardLooking).toBe(true); @@ -291,13 +293,13 @@ 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]], { timelineWasEmpty: undefined }); expect(timeline.getState(EventTimeline.BACKWARDS).setStateEvents). - toHaveBeenCalledWith([events[1]]); + toHaveBeenCalledWith([events[1]], { timelineWasEmpty: undefined }); expect(events[0].forwardLooking).toBe(false); expect(events[1].forwardLooking).toBe(false); @@ -324,8 +326,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 +340,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 +360,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 85f4c21d572..390e0946b46 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]], + { timelineWasEmpty: undefined }, ); expect(room.currentState.setStateEvents).toHaveBeenCalledWith( [events[1]], + { timelineWasEmpty: undefined }, ); 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]], + { timelineWasEmpty: undefined }, ); expect(room.oldState.setStateEvents).toHaveBeenCalledWith( [events[1]], + { timelineWasEmpty: undefined }, ); expect(events[0].forwardLooking).toBe(false); expect(events[1].forwardLooking).toBe(false); diff --git a/spec/unit/timeline-window.spec.js b/spec/unit/timeline-window.spec.js index c9466412c83..4fc78234446 100644 --- a/spec/unit/timeline-window.spec.js +++ b/spec/unit/timeline-window.spec.js @@ -35,13 +35,14 @@ 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 ee39d6c4f0dac164332cf58769eecf04a8e62e93 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 14 Apr 2022 01:25:01 -0500 Subject: [PATCH 03/51] Fix argument mismatch --- src/models/thread.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/models/thread.ts b/src/models/thread.ts index 14a036b3046..60c1d013b85 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -172,9 +172,11 @@ export class Thread extends TypedEventEmitter { this.timelineSet.addEventToTimeline( event, this.liveTimeline, - toStartOfTimeline, - false, - this.roomState, + { + toStartOfTimeline, + fromCache: false, + roomState: this.roomState, + }, ); } } From 246869081819407fab8ca44fffa2ab27ef535b8a Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 14 Apr 2022 01:53:29 -0500 Subject: [PATCH 04/51] Add missing docstrings --- src/models/event-timeline-set.ts | 1 + src/models/event-timeline.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index c33b8dbe69a..788e7989e13 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -77,6 +77,7 @@ export interface IAddLiveEventOptions { } export interface IAddEventToTimelineOptions { + /** Whether to insert the new event at the start of the timeline */ toStartOfTimeline: boolean; /** Whether the sync response came from cache */ fromCache?: boolean; diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts index 91d29f68e95..0ac492484c7 100644 --- a/src/models/event-timeline.ts +++ b/src/models/event-timeline.ts @@ -38,7 +38,11 @@ export interface IAddEventOptions { toStartOfTimeline: boolean; /** The state events to reconcile metadata from */ roomState?: RoomState; - /** TODO */ + /** Whether the timeline was empty before the marker arrived 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` */ timelineWasEmpty?: boolean; } @@ -364,7 +368,7 @@ export class EventTimeline { } /** - * Add a new event to the timeline, and update the state + * Add a new event to the timeline, and update the state * * @param {MatrixEvent} event new event * @param {IAddEventOptions} options addEvent options From f44489d468ecd2ad803e340afaecb5072e0e416b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 16 Apr 2022 02:17:35 -0500 Subject: [PATCH 05/51] Raw refreshLiveTimeline function that seems to work --- src/client.ts | 9 +++ src/models/event-timeline-set.ts | 9 +++ src/models/room.ts | 100 +++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/src/client.ts b/src/client.ts index 5b26cd3c95b..30e309cca8e 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5135,6 +5135,7 @@ export class MatrixClient extends TypedEventEmitter { + console.log('client: scrollback'); if (utils.isFunction(limit)) { callback = limit as any as Callback; // legacy limit = undefined; @@ -5150,11 +5151,13 @@ export class MatrixClient extends TypedEventEmitter void; + [RoomEvent.TimelineRefresh]: (room: Room, eventTimelineSet: EventTimelineSet) => void; [ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void; } & ThreadHandlerMap & MatrixEventHandlerMap; @@ -926,6 +929,98 @@ export class Room extends TypedEventEmitter }); } + // TODO + public async refreshLiveTimeline(): Promise { + const liveTimelineBefore = this.getLiveTimeline(); + const forwardPaginationToken = liveTimelineBefore.getPaginationToken(EventTimeline.FORWARDS); + const eventsBefore = liveTimelineBefore.getEvents(); + const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; + + // Empty out all of `this.timelineSets` but still keeps the same + // `timelineSet` references around so the React code updates properly + // and doesn't ignore the room events we emit because it checks that the + // `timelineSet` references are the same. We need the `timelineSet` + // empty so that the `client.getEventTimeline(...)` call later, will + // call `/context` and create a new timeline instead of returning the + // same one. + this.resetLiveTimeline(null, null); + //this.resetLiveTimeline(forwardPaginationToken, forwardPaginationToken); + + // Get a reference to the emptied out `timelineSet` + // + // TODO: Do we want to use `this.getLiveTimeline().getTimelineSet()` instead? + // I think it's the same thing but what's more right? + const timelineSet = this.getUnfilteredTimelineSet(); + + console.log(`refreshLiveTimeline: after resetLiveTimeline timelineSets=${this.getTimelineSets().length} getUnfilteredTimelineSet=`, timelineSet.getTimelines().length, timelineSet.getTimelines().map((timeline) => { + return timeline.getEvents().length; + })); + + // Use `client.getEventTimeline(...)` to construct a new timeline from a + // `/context` response state and events for the most recent event before + // we reset everything. The `timelineSet` needs to be empty in order for + // this function to call `/context` + const newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); + console.log('refreshLiveTimeline: after getEventTimeline', timelineSet.getTimelines().length, timelineSet.getTimelines().map((timeline) => { + return timeline.getEvents().length; + })); + // Set the pagination token back to the live sync token instead of using + // the /context historical token so that it matches the next response from `/sync` + // and we can properly continue the timeline. + newTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); + + timelineSet.setLiveTimeline(newTimeline); + //timelineSet.getLiveTimeline().setNeighbouringTimeline(newTimeline, EventTimeline.FORWARDS); + // Fixup `this.oldstate` so that `scrollback` has the pagination tokens available + this.fixUpLegacyTimelineFields(); + + // TODO: Set timelineNeedsRefresh = false + + // const liveTimeline = this.getLiveTimeline(); + // liveTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.BACKWARDS); + // liveTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); + // await new Promise((resolve) => { + // setTimeout(async () => { + // await this.client.scrollback(this, 30); + // resolve(null); + // }, 2000); + // }); + //await this.client.scrollback(this, 30); + this.emit(RoomEvent.TimelineRefresh, this, timelineSet); + + // const timelineSet = new EventTimelineSet(this, this.opts); + + // const liveTimeline = this.getLiveTimeline(); + // const events = liveTimeline.getEvents(); + // const mostRecentEventInTimeline = events[events.length - 1]; + + // // This will create a timeline with the given event and add it to the timelineSet + // const newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); + + // console.log('refreshTimeline: timelineSet', + // timelineSet, + // timelineSet.getTimelines().map((timeline) => { + // return timeline.getEvents().length; + // }), + // ); + + // // Since timelineSets is a readonly property, clear it out this way + // this.timelineSets.length = 0; + // // Keep track of the new timelineSet + // this.timelineSets.push(timelineSet); + // // Set our new timeline as the liveTimeline + // timelineSet.setLiveTimeline(newTimeline); + // // Fixup `this.oldstate` so that `scrollback` has the pagination tokens available + // this.fixUpLegacyTimelineFields(); + + + // //this.emit(RoomEvent.Timeline, event, this.room, Boolean(toStartOfTimeline), false, data); + // // setTimeout(() => { + // // this.emit(RoomEvent.TimelineReset, this, timelineSet, true); + // // }, 1000); + // this.emit(RoomEvent.TimelineRefresh, this, timelineSet); + } + /** * Reset the live timeline of all timelineSets, and start new ones. * @@ -953,6 +1048,11 @@ export class Room extends TypedEventEmitter * @private */ private fixUpLegacyTimelineFields(): void { + console.log( + 'fixUpLegacyTimelineFields', + this.getLiveTimeline().getState(EventTimeline.BACKWARDS), + this.getLiveTimeline().getState(EventTimeline.FORWARDS) + ); // maintain this.timeline as a reference to the live timeline, // and this.oldState and this.currentState as references to the // state at the start and end of that timeline. These are more From 1ab5460b45a6926a841f59a65d5f11f7baf135df Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 18 Apr 2022 18:43:07 -0500 Subject: [PATCH 06/51] Clean up raw commit --- src/client.ts | 9 --- src/models/event-timeline-set.ts | 2 +- src/models/room.ts | 96 ++++++++++---------------------- 3 files changed, 31 insertions(+), 76 deletions(-) diff --git a/src/client.ts b/src/client.ts index 30e309cca8e..5b26cd3c95b 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5135,7 +5135,6 @@ export class MatrixClient extends TypedEventEmitter { - console.log('client: scrollback'); if (utils.isFunction(limit)) { callback = limit as any as Callback; // legacy limit = undefined; @@ -5151,13 +5150,11 @@ export class MatrixClient extends TypedEventEmitter }); } - // TODO + /** + * Empty out the current live timeline and re-request it. This is used when + * historical messages are imported into the room via MSC2716 `/batch_send + * because the client may already have that section of the timeline loaded. + * We need to force the client to throw away their current timeline so that + * when they back paginate over the area again with the historical messages + * in between, it grabs the newly imported messages. We can listen for + * `EventType.Marker`, in order to tell when historical messages are ready + * to be discovered in the room and the timeline needs a refresh. The SDK + * emits a `RoomEvent.historyImportedWithinTimeline` event when we detect a + * valid marker and can check the needs refresh status via + * `room.getTimelineNeedsRefresh()`. + */ public async refreshLiveTimeline(): Promise { const liveTimelineBefore = this.getLiveTimeline(); const forwardPaginationToken = liveTimelineBefore.getPaginationToken(EventTimeline.FORWARDS); const eventsBefore = liveTimelineBefore.getEvents(); const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; + logger.log(`[refreshLiveTimeline] for room ${this.roomId} at mostRecentEventInTimeline=${mostRecentEventInTimeline.getId()} forwardPaginationToken+${forwardPaginationToken}`) // Empty out all of `this.timelineSets` but still keeps the same // `timelineSet` references around so the React code updates properly @@ -944,81 +957,37 @@ export class Room extends TypedEventEmitter // call `/context` and create a new timeline instead of returning the // same one. this.resetLiveTimeline(null, null); - //this.resetLiveTimeline(forwardPaginationToken, forwardPaginationToken); // Get a reference to the emptied out `timelineSet` // - // TODO: Do we want to use `this.getLiveTimeline().getTimelineSet()` instead? - // I think it's the same thing but what's more right? + // TODO: Do we want to use `this.getLiveTimeline().getTimelineSet()` + // instead? I think it's the same thing but what's more right? const timelineSet = this.getUnfilteredTimelineSet(); - - console.log(`refreshLiveTimeline: after resetLiveTimeline timelineSets=${this.getTimelineSets().length} getUnfilteredTimelineSet=`, timelineSet.getTimelines().length, timelineSet.getTimelines().map((timeline) => { - return timeline.getEvents().length; - })); // Use `client.getEventTimeline(...)` to construct a new timeline from a // `/context` response state and events for the most recent event before - // we reset everything. The `timelineSet` needs to be empty in order for - // this function to call `/context` + // we reset everything. The `timelineSet` we pass in needs to be empty + // in order for this function to call `/context` and generate a new + // timeline. const newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); - console.log('refreshLiveTimeline: after getEventTimeline', timelineSet.getTimelines().length, timelineSet.getTimelines().map((timeline) => { - return timeline.getEvents().length; - })); // Set the pagination token back to the live sync token instead of using - // the /context historical token so that it matches the next response from `/sync` - // and we can properly continue the timeline. + // the `/context` historical token so that it matches the next response + // from `/sync` and we can properly continue the timeline. newTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); + // Set our new fresh timeline as the live timeline to continue syncing + // forwards and back paginating from. timelineSet.setLiveTimeline(newTimeline); - //timelineSet.getLiveTimeline().setNeighbouringTimeline(newTimeline, EventTimeline.FORWARDS); - // Fixup `this.oldstate` so that `scrollback` has the pagination tokens available + // Fixup `this.oldstate` so that `scrollback` has the pagination tokens + // available this.fixUpLegacyTimelineFields(); - // TODO: Set timelineNeedsRefresh = false - - // const liveTimeline = this.getLiveTimeline(); - // liveTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.BACKWARDS); - // liveTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); - // await new Promise((resolve) => { - // setTimeout(async () => { - // await this.client.scrollback(this, 30); - // resolve(null); - // }, 2000); - // }); - //await this.client.scrollback(this, 30); - this.emit(RoomEvent.TimelineRefresh, this, timelineSet); - - // const timelineSet = new EventTimelineSet(this, this.opts); - - // const liveTimeline = this.getLiveTimeline(); - // const events = liveTimeline.getEvents(); - // const mostRecentEventInTimeline = events[events.length - 1]; - - // // This will create a timeline with the given event and add it to the timelineSet - // const newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); + // The timeline has now been refreshed ✅ + this.setTimelineNeedsRefresh(false); - // console.log('refreshTimeline: timelineSet', - // timelineSet, - // timelineSet.getTimelines().map((timeline) => { - // return timeline.getEvents().length; - // }), - // ); - - // // Since timelineSets is a readonly property, clear it out this way - // this.timelineSets.length = 0; - // // Keep track of the new timelineSet - // this.timelineSets.push(timelineSet); - // // Set our new timeline as the liveTimeline - // timelineSet.setLiveTimeline(newTimeline); - // // Fixup `this.oldstate` so that `scrollback` has the pagination tokens available - // this.fixUpLegacyTimelineFields(); - - - // //this.emit(RoomEvent.Timeline, event, this.room, Boolean(toStartOfTimeline), false, data); - // // setTimeout(() => { - // // this.emit(RoomEvent.TimelineReset, this, timelineSet, true); - // // }, 1000); - // this.emit(RoomEvent.TimelineRefresh, this, timelineSet); + // Emit an event which clients can react to and re-load the timeline + // from the SDK + this.emit(RoomEvent.TimelineRefresh, this, timelineSet); } /** @@ -1048,11 +1017,6 @@ export class Room extends TypedEventEmitter * @private */ private fixUpLegacyTimelineFields(): void { - console.log( - 'fixUpLegacyTimelineFields', - this.getLiveTimeline().getState(EventTimeline.BACKWARDS), - this.getLiveTimeline().getState(EventTimeline.FORWARDS) - ); // maintain this.timeline as a reference to the live timeline, // and this.oldState and this.currentState as references to the // state at the start and end of that timeline. These are more From 52ce1843a9ba1e5934d614801a1fec41b76a2115 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 18 Apr 2022 23:37:22 -0500 Subject: [PATCH 07/51] Remove event remitters See https://github.com/matrix-org/matrix-react-sdk/pull/8303#discussion_r850192080 --- spec/integ/matrix-client-syncing.spec.js | 23 ++++++++++++++++------- src/models/room.ts | 2 +- src/sync.ts | 12 +++++++++++- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index b499001a548..9e662b89ff6 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -1,4 +1,5 @@ import { MatrixEvent } from "../../src/models/event"; +import { RoomEvent } from "../../src/models/room"; import { EventTimeline } from "../../src/models/event-timeline"; import { EventType } from "../../src/@types/event"; import * as utils from "../test-utils/test-utils"; @@ -696,25 +697,33 @@ describe("MatrixClient syncing", function() { const markerEventId = nextSyncData.rooms.join[roomOne].timeline.events[0].event_id; + // Only do the first sync + httpBackend.when("GET", "/sync").respond(200, normalFirstSync); + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + // Get the room after the first sync so the room is created + const room = client.getRoom(roomOne); + let emitCount = 0; - client.on("Room.historyImportedWithinTimeline", function(markerEvent, room) { + room.on(RoomEvent.historyImportedWithinTimeline, function(markerEvent, room) { expect(markerEvent.getId()).toEqual(markerEventId); expect(room.roomId).toEqual(roomOne); emitCount += 1; }); - httpBackend.when("GET", "/sync").respond(200, normalFirstSync); + // Now do a subsequent sync with the marker event httpBackend.when("GET", "/sync").respond(200, nextSyncData); - - client.startClient(); await Promise.all([ httpBackend.flushAllExpected(), - awaitSyncEvent(2), + awaitSyncEvent(), ]); - const room = client.getRoom(roomOne); expect(room.getTimelineNeedsRefresh()).toEqual(true); - // Make sure "Room.historyImportedWithinTimeline" was emitted + // Make sure `RoomEvent.historyImportedWithinTimeline` was emitted expect(emitCount).toEqual(1); expect(room.getLastMarkerEventIdProcessed()).toEqual(markerEventId); }); diff --git a/src/models/room.ts b/src/models/room.ts index a5169654081..fede8b2efe7 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -165,7 +165,7 @@ export enum RoomEvent { LocalEchoUpdated = "Room.localEchoUpdated", Timeline = "Room.timeline", TimelineReset = "Room.timelineReset", - TimelineRefresh = "RoomEvent.TimelineRefresh", + TimelineRefresh = "Room.TimelineRefresh", historyImportedWithinTimeline = "Room.historyImportedWithinTimeline", } diff --git a/src/sync.ts b/src/sync.ts index 272d341de88..f0b4289675e 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -70,11 +70,22 @@ const BUFFER_PERIOD_MS = 80 * 1000; const FAILED_SYNC_ERROR_THRESHOLD = 3; export enum SyncState { + /** Emitted after we try to sync more than `FAILED_SYNC_ERROR_THRESHOLD` + * times and are still failing. Or when we enounter a hard error like the + * token being invalid. */ Error = "ERROR", + /** Emitted after the first sync events are ready (this could even be sync + * events from the cache) */ Prepared = "PREPARED", + /** Emitted when the sync loop is no longer running */ Stopped = "STOPPED", + /** Emitted after each sync request happens */ Syncing = "SYNCING", + /** Emitted after a connectivity error and we're ready to start syncing again */ Catchup = "CATCHUP", + /** Emitted for each time we try reconnecting. Will switch to `Error` after + * we reach the `FAILED_SYNC_ERROR_THRESHOLD` + */ Reconnecting = "RECONNECTING", } @@ -210,7 +221,6 @@ export class SyncApi { RoomEvent.Receipt, RoomEvent.Tags, RoomEvent.LocalEchoUpdated, - RoomEvent.historyImportedWithinTimeline, RoomEvent.AccountData, RoomEvent.MyMembership, RoomEvent.Timeline, From 26d4ecdde03a6981194fa6ce8f5f8036babcc60e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 18 Apr 2022 23:43:45 -0500 Subject: [PATCH 08/51] Fix up lints --- src/models/event-timeline-set.ts | 2 +- src/models/room.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 41424a59ae4..9963eb4d74e 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -221,7 +221,7 @@ export class EventTimelineSet extends TypedEventEmitter const forwardPaginationToken = liveTimelineBefore.getPaginationToken(EventTimeline.FORWARDS); const eventsBefore = liveTimelineBefore.getEvents(); const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; - logger.log(`[refreshLiveTimeline] for room ${this.roomId} at mostRecentEventInTimeline=${mostRecentEventInTimeline.getId()} forwardPaginationToken+${forwardPaginationToken}`) + logger.log( + `[refreshLiveTimeline] for room ${this.roomId} at ` + + `mostRecentEventInTimeline=${mostRecentEventInTimeline.getId()} forwardPaginationToken+${forwardPaginationToken}` + ); // Empty out all of `this.timelineSets` but still keeps the same // `timelineSet` references around so the React code updates properly @@ -979,7 +982,7 @@ export class Room extends TypedEventEmitter // forwards and back paginating from. timelineSet.setLiveTimeline(newTimeline); // Fixup `this.oldstate` so that `scrollback` has the pagination tokens - // available + // available this.fixUpLegacyTimelineFields(); // The timeline has now been refreshed ✅ From f1979d25073befd7da358650d4e82d9375e69714 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 18 Apr 2022 23:46:46 -0500 Subject: [PATCH 09/51] Fix lints --- src/models/room.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index b9e1c707abc..8715a46df15 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -948,8 +948,9 @@ export class Room extends TypedEventEmitter const eventsBefore = liveTimelineBefore.getEvents(); const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; logger.log( - `[refreshLiveTimeline] for room ${this.roomId} at ` + - `mostRecentEventInTimeline=${mostRecentEventInTimeline.getId()} forwardPaginationToken+${forwardPaginationToken}` + `[refreshLiveTimeline] for room ${this.roomId} at ` + + `mostRecentEventInTimeline=${mostRecentEventInTimeline.getId()} ` + + `forwardPaginationToken+${forwardPaginationToken}`, ); // Empty out all of `this.timelineSets` but still keeps the same From c863ad1d07da278a055c1f968b8670fcd5e2d582 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Apr 2022 02:38:02 -0500 Subject: [PATCH 10/51] Use UnstableValue See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r850188598 --- spec/integ/matrix-client-syncing.spec.js | 6 +++--- spec/unit/room-state.spec.js | 4 ++-- src/@types/event.ts | 9 ++++++++- src/models/room-state.ts | 6 +++--- src/models/room.ts | 2 +- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 9e662b89ff6..4bcbf03335a 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -1,7 +1,7 @@ import { MatrixEvent } from "../../src/models/event"; import { RoomEvent } from "../../src/models/room"; import { EventTimeline } from "../../src/models/event-timeline"; -import { EventType } from "../../src/@types/event"; +import { UNSTABLE_MSC2716_MARKER } from "../../src/@types/event"; import * as utils from "../test-utils/test-utils"; import { TestClient } from "../TestClient"; @@ -509,7 +509,7 @@ describe("MatrixClient syncing", function() { // being sent by the room creator so it has no // special meaning in existing room versions. utils.mkEvent({ - type: EventType.Marker, + type: UNSTABLE_MSC2716_MARKER.name, room: roomOne, // The important part we're testing is here! // `userC` is not the room creator. @@ -560,7 +560,7 @@ describe("MatrixClient syncing", function() { }); const markerEventFromRoomCreator = utils.mkEvent({ - type: EventType.Marker, room: roomOne, user: otherUserId, + type: UNSTABLE_MSC2716_MARKER.name, room: roomOne, user: otherUserId, skey: "", content: { "m.insertion_id": "$abc", diff --git a/spec/unit/room-state.spec.js b/spec/unit/room-state.spec.js index 380a0aead99..69e7277b134 100644 --- a/spec/unit/room-state.spec.js +++ b/spec/unit/room-state.spec.js @@ -3,7 +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"; +import { UNSTABLE_MSC2716_MARKER } from "../../src/@types/event"; describe("RoomState", function() { const roomId = "!foo:bar"; @@ -257,7 +257,7 @@ describe("RoomState", function() { const events = [ utils.mkEvent({ event: true, - type: EventType.Marker, + type: UNSTABLE_MSC2716_MARKER.name, room: roomId, user: userA, skey: "", diff --git a/src/@types/event.ts b/src/@types/event.ts index 6ef74b92ff5..dac2770ade3 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -33,7 +33,6 @@ export enum EventType { RoomGuestAccess = "m.room.guest_access", RoomServerAcl = "m.room.server_acl", RoomTombstone = "m.room.tombstone", - Marker = "org.matrix.msc2716.marker", // MSC2716 /** * @deprecated Should not be used. */ @@ -152,6 +151,14 @@ export const UNSTABLE_MSC3089_LEAF = new UnstableValue("m.leaf", "org.matrix.msc */ export const UNSTABLE_MSC3089_BRANCH = new UnstableValue("m.branch", "org.matrix.msc3089.branch"); +/** + * Marker event type to point back at imported historical content in a room. See + * [MSC2716](https://github.com/matrix-org/matrix-spec-proposals/pull/2716). + * Note that this reference is UNSTABLE and subject to breaking changes, + * including its eventual removal. + */ +export const UNSTABLE_MSC2716_MARKER = new UnstableValue("m.room.marker", "org.matrix.msc2716.marker"); + /** * Functional members type for declaring a purpose of room members (e.g. helpful bots). * Note that this reference is UNSTABLE and subject to breaking changes, including its diff --git a/src/models/room-state.ts b/src/models/room-state.ts index 9b02a2327d9..6f7ade6911c 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -21,7 +21,7 @@ limitations under the License. import { RoomMember } from "./room-member"; import { logger } from '../logger'; import * as utils from "../utils"; -import { EventType } from "../@types/event"; +import { EventType, UNSTABLE_MSC2716_MARKER } from "../@types/event"; import { MatrixEvent } from "./event"; import { MatrixClient } from "../client"; import { GuestAccess, HistoryVisibility, IJoinRuleEventContent, JoinRule } from "../@types/partials"; @@ -331,7 +331,7 @@ export class RoomState extends TypedEventEmitter * 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. May fire "RoomStateEvent.Marker" if there are - * EventType.Marker events. + * UNSTABLE_MSC2716_MARKER events. * @param {MatrixEvent[]} stateEvents a list of state events for this room. * @param {IMarkerFoundOptions} markerFoundOptions * @fires module:client~MatrixClient#event:"RoomState.members" @@ -419,7 +419,7 @@ export class RoomState extends TypedEventEmitter // assume all our sentinels are now out-of-date this.sentinels = {}; - } else if (event.getType() === EventType.Marker) { + } else if (event.getType() === UNSTABLE_MSC2716_MARKER.name) { this.emit(RoomStateEvent.Marker, event, markerFoundOptions); } }); diff --git a/src/models/room.ts b/src/models/room.ts index 8715a46df15..68dbc02a720 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -936,7 +936,7 @@ export class Room extends TypedEventEmitter * We need to force the client to throw away their current timeline so that * when they back paginate over the area again with the historical messages * in between, it grabs the newly imported messages. We can listen for - * `EventType.Marker`, in order to tell when historical messages are ready + * `UNSTABLE_MSC2716_MARKER`, in order to tell when historical messages are ready * to be discovered in the room and the timeline needs a refresh. The SDK * emits a `RoomEvent.historyImportedWithinTimeline` event when we detect a * valid marker and can check the needs refresh status via From b58e198d85b6f0ee2b8e3d8396526f9120691d11 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Apr 2022 02:40:53 -0500 Subject: [PATCH 11/51] Clarify toStartOfTimeline order See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r850949505 --- src/models/event-timeline-set.ts | 4 +++- src/models/event-timeline.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 9963eb4d74e..7c8d6d0fbf0 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -77,7 +77,9 @@ export interface IAddLiveEventOptions { } export interface IAddEventToTimelineOptions { - /** Whether to insert the new event at the start of the timeline */ + /** Whether to insert the new event at the start of the timeline where the + * oldest events are (timeline is in chronological order, oldest to most + * recent) */ toStartOfTimeline: boolean; /** Whether the sync response came from cache */ fromCache?: boolean; diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts index 0ac492484c7..0924c5f0884 100644 --- a/src/models/event-timeline.ts +++ b/src/models/event-timeline.ts @@ -34,7 +34,9 @@ export interface IInitialiseStateOptions { } export interface IAddEventOptions { - /** Whether to insert the new event at the start of the timeline */ + /** Whether to insert the new event at the start of the timeline where the + * oldest events are (timeline is in chronological order, oldest to most + * recent) */ toStartOfTimeline: boolean; /** The state events to reconcile metadata from */ roomState?: RoomState; From 81785ce3e621c63117c6fb487cfbca95eca883f8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Apr 2022 03:43:28 -0500 Subject: [PATCH 12/51] Add backwards compatible overloads See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r850128422 --- src/models/event-timeline-set.ts | 50 ++++++++++++++++++++++++++---- src/models/event-timeline.ts | 16 ++++++++++ src/models/room.ts | 52 +++++++++++++++++++++++++++++--- 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 7c8d6d0fbf0..3ca0b2ced8e 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -574,15 +574,33 @@ export class EventTimelineSet extends TypedEventEmitter * @fires module:client~MatrixClient#event:"Room.timeline" * @private */ - private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions = {}): void { + private addLiveEvent(event: MatrixEvent, duplicateStrategy: DuplicateStrategy, fromCache?): void; + private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions): void; + private addLiveEvent( + event: MatrixEvent, + duplicateStrategyOrOpts: DuplicateStrategy | IAddLiveEventOptions, + fromCache = false, + ): void { + let duplicateStrategy = duplicateStrategyOrOpts as DuplicateStrategy; + let timelineWasEmpty; + if (typeof (duplicateStrategyOrOpts) === 'object') { + ({ + duplicateStrategy = DuplicateStrategy.Ignore, + fromCache = false, + /* roomState, (not used here) */ + timelineWasEmpty, + } = duplicateStrategyOrOpts); + } + // add to our timeline sets for (let i = 0; i < this.timelineSets.length; i++) { - this.timelineSets[i].addLiveEvent(event, addLiveEventOptions); + this.timelineSets[i].addLiveEvent(event, { + duplicateStrategy, + fromCache, + timelineWasEmpty, + }); } // synthesize and inject implicit read receipts @@ -2307,8 +2328,25 @@ export class Room extends TypedEventEmitter * @param {IAddLiveEventOptions} options addLiveEvent options * @throws If duplicateStrategy is not falsey, 'replace' or 'ignore'. */ - public addLiveEvents(events: MatrixEvent[], addLiveEventOptions: IAddLiveEventOptions = {}): void { - const { duplicateStrategy } = addLiveEventOptions; + public addLiveEvents(events: MatrixEvent[], duplicateStrategy?: DuplicateStrategy, fromCache?: boolean): void; + public addLiveEvents(events: MatrixEvent[], addLiveEventOptions?: IAddLiveEventOptions): void + public addLiveEvents( + events: MatrixEvent[], + duplicateStrategyOrOpts?: DuplicateStrategy | IAddLiveEventOptions, + fromCache = false, + ): void { + let duplicateStrategy = duplicateStrategyOrOpts as (DuplicateStrategy | null); + let timelineWasEmpty; + if (typeof (duplicateStrategyOrOpts) === 'object') { + ({ + duplicateStrategy, + fromCache = false, + /* roomState, (not used here) */ + timelineWasEmpty, + } = duplicateStrategyOrOpts); + } + + if (duplicateStrategy && ["replace", "ignore"].indexOf(duplicateStrategy) === -1) { throw new Error("duplicateStrategy MUST be either 'replace' or 'ignore'"); } @@ -2349,7 +2387,11 @@ export class Room extends TypedEventEmitter } if (shouldLiveInRoom) { - this.addLiveEvent(events[i], addLiveEventOptions); + this.addLiveEvent(events[i], { + duplicateStrategy, + fromCache, + timelineWasEmpty, + }); } } From a98df621a46b5cec0fd4beb6ee4dfca97e388822 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Apr 2022 03:47:47 -0500 Subject: [PATCH 13/51] Fix lints --- src/models/event-timeline-set.ts | 10 +++++++--- src/models/event-timeline.ts | 2 +- src/models/room.ts | 3 +-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 3ca0b2ced8e..680d7865733 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -574,13 +574,13 @@ export class EventTimelineSet extends TypedEventEmitter * @throws If duplicateStrategy is not falsey, 'replace' or 'ignore'. */ public addLiveEvents(events: MatrixEvent[], duplicateStrategy?: DuplicateStrategy, fromCache?: boolean): void; - public addLiveEvents(events: MatrixEvent[], addLiveEventOptions?: IAddLiveEventOptions): void + public addLiveEvents(events: MatrixEvent[], addLiveEventOptions?: IAddLiveEventOptions): void; public addLiveEvents( events: MatrixEvent[], duplicateStrategyOrOpts?: DuplicateStrategy | IAddLiveEventOptions, @@ -2346,7 +2346,6 @@ export class Room extends TypedEventEmitter } = duplicateStrategyOrOpts); } - if (duplicateStrategy && ["replace", "ignore"].indexOf(duplicateStrategy) === -1) { throw new Error("duplicateStrategy MUST be either 'replace' or 'ignore'"); } From a8e4d070511597ab1f740146aa5be081f6c14b3e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Apr 2022 03:54:24 -0500 Subject: [PATCH 14/51] Fix lints --- src/models/event-timeline-set.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 680d7865733..c434b0a5a53 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -678,7 +678,7 @@ export class EventTimelineSet extends TypedEventEmitter Date: Wed, 20 Apr 2022 00:49:30 -0500 Subject: [PATCH 15/51] Match case of other events See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r852884948 --- spec/integ/matrix-client-syncing.spec.js | 4 ++-- src/client.ts | 2 +- src/models/room.ts | 8 ++++---- src/sync.ts | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 4bcbf03335a..9fe34e0625c 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -709,7 +709,7 @@ describe("MatrixClient syncing", function() { const room = client.getRoom(roomOne); let emitCount = 0; - room.on(RoomEvent.historyImportedWithinTimeline, function(markerEvent, room) { + room.on(RoomEvent.HistoryImportedWithinTimeline, function(markerEvent, room) { expect(markerEvent.getId()).toEqual(markerEventId); expect(room.roomId).toEqual(roomOne); emitCount += 1; @@ -723,7 +723,7 @@ describe("MatrixClient syncing", function() { ]); expect(room.getTimelineNeedsRefresh()).toEqual(true); - // Make sure `RoomEvent.historyImportedWithinTimeline` was emitted + // Make sure `RoomEvent.HistoryImportedWithinTimeline` was emitted expect(emitCount).toEqual(1); expect(room.getLastMarkerEventIdProcessed()).toEqual(markerEventId); }); diff --git a/src/client.ts b/src/client.ts index f543567b4b4..cf0da7347e4 100644 --- a/src/client.ts +++ b/src/client.ts @@ -790,7 +790,7 @@ type RoomEvents = RoomEvent.Name | RoomEvent.Receipt | RoomEvent.Tags | RoomEvent.LocalEchoUpdated - | RoomEvent.historyImportedWithinTimeline + | RoomEvent.HistoryImportedWithinTimeline | RoomEvent.AccountData | RoomEvent.MyMembership | RoomEvent.Timeline diff --git a/src/models/room.ts b/src/models/room.ts index d4a108116b0..bd9de1bbdba 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -166,7 +166,7 @@ export enum RoomEvent { Timeline = "Room.timeline", TimelineReset = "Room.timelineReset", TimelineRefresh = "Room.TimelineRefresh", - historyImportedWithinTimeline = "Room.historyImportedWithinTimeline", + HistoryImportedWithinTimeline = "Room.historyImportedWithinTimeline", } type EmittedEvents = RoomEvent @@ -176,7 +176,7 @@ type EmittedEvents = RoomEvent | RoomEvent.Timeline | RoomEvent.TimelineReset | RoomEvent.TimelineRefresh - | RoomEvent.historyImportedWithinTimeline + | RoomEvent.HistoryImportedWithinTimeline | MatrixEventEvent.BeforeRedaction; export type RoomEventHandlerMap = { @@ -193,7 +193,7 @@ export type RoomEventHandlerMap = { oldEventId?: string, oldStatus?: EventStatus, ) => void; - [RoomEvent.historyImportedWithinTimeline]: ( + [RoomEvent.HistoryImportedWithinTimeline]: ( markerEvent: MatrixEvent, room: Room, ) => void; @@ -938,7 +938,7 @@ export class Room extends TypedEventEmitter * in between, it grabs the newly imported messages. We can listen for * `UNSTABLE_MSC2716_MARKER`, in order to tell when historical messages are ready * to be discovered in the room and the timeline needs a refresh. The SDK - * emits a `RoomEvent.historyImportedWithinTimeline` event when we detect a + * emits a `RoomEvent.HistoryImportedWithinTimeline` event when we detect a * valid marker and can check the needs refresh status via * `room.getTimelineNeedsRefresh()`. */ diff --git a/src/sync.ts b/src/sync.ts index f0b4289675e..f48441b7ee8 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -357,7 +357,7 @@ export class SyncApi { `a new markerEventId=${markerEvent.getId()} was sent in roomId=${room.roomId}`, ); room.setTimelineNeedsRefresh(true); - room.emit(RoomEvent.historyImportedWithinTimeline, markerEvent, room); + room.emit(RoomEvent.HistoryImportedWithinTimeline, markerEvent, room); room.setLastMarkerEventIdProcessed(markerEvent.getId()); } } From 4acc712b0baac6c6a926e2fb3b6038258effebcd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 20 Apr 2022 00:53:10 -0500 Subject: [PATCH 16/51] Add some missing type hints --- src/models/event-timeline-set.ts | 4 ++-- src/models/event-timeline.ts | 2 +- src/models/room.ts | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index c434b0a5a53..ac8752811d8 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -596,7 +596,7 @@ export class EventTimelineSet extends TypedEventEmitter fromCache = false, ): void { let duplicateStrategy = duplicateStrategyOrOpts as DuplicateStrategy; - let timelineWasEmpty; + let timelineWasEmpty: boolean; if (typeof (duplicateStrategyOrOpts) === 'object') { ({ duplicateStrategy = DuplicateStrategy.Ignore, @@ -2335,8 +2335,8 @@ export class Room extends TypedEventEmitter duplicateStrategyOrOpts?: DuplicateStrategy | IAddLiveEventOptions, fromCache = false, ): void { - let duplicateStrategy = duplicateStrategyOrOpts as (DuplicateStrategy | null); - let timelineWasEmpty; + let duplicateStrategy = duplicateStrategyOrOpts as DuplicateStrategy; + let timelineWasEmpty: boolean; if (typeof (duplicateStrategyOrOpts) === 'object') { ({ duplicateStrategy, From 15d0e8adc58129a2ecd8c1131664960dbcbcdfa2 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 20 Apr 2022 00:53:41 -0500 Subject: [PATCH 17/51] Use built-in matches function See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r852888987 --- 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 6f7ade6911c..68990c64ee5 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -419,7 +419,7 @@ export class RoomState extends TypedEventEmitter // assume all our sentinels are now out-of-date this.sentinels = {}; - } else if (event.getType() === UNSTABLE_MSC2716_MARKER.name) { + } else if (UNSTABLE_MSC2716_MARKER.matches(event.getType())) { this.emit(RoomStateEvent.Marker, event, markerFoundOptions); } }); From 64a6e3814842f7873d862d4abe3a49ef2f84e51f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 20 Apr 2022 00:55:10 -0500 Subject: [PATCH 18/51] Add more missing types --- src/models/event-timeline-set.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index ac8752811d8..608ed054041 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -666,7 +666,7 @@ export class EventTimelineSet extends TypedEventEmitter Date: Fri, 20 May 2022 02:51:22 -0500 Subject: [PATCH 19/51] Add method to call MSC2716 /batch_send endpoint --- src/client.ts | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/client.ts b/src/client.ts index 4a94645d969..ab58552620e 100644 --- a/src/client.ts +++ b/src/client.ts @@ -776,6 +776,25 @@ interface ITimestampToEventResponse { event_id: string; origin_server_ts: string; } + +interface IBatchSendResponse { + /** List of state event ID's we inserted */ + state_event_ids: string[]; + /** List of historical event ID's we inserted */ + event_ids: string[]; + next_batch_id: string; + insertion_event_id: string; + batch_event_id: string; + /** When `?batch_id` isn't provided, the homeserver automatically creates an + * insertion event as a starting place to hang the history off of. This + * automatic insertion event ID is returned in this field. + * + * When `?batch_id` is provided, this field is not present because we can + * hang the history off the insertion event specified and associated by the + * batch ID. + */ + base_insertion_event_id?: string; +} /* eslint-enable camelcase */ // We're using this constant for methods overloading and inspect whether a variable @@ -8961,6 +8980,40 @@ export class MatrixClient extends TypedEventEmitter { + const path = utils.encodeUri("/rooms/$roomId/batch_send", { + $roomId: roomId, + }); + + const queryParams: { prev_event_id: string, batch_id?: string } = { + prev_event_id, + }; + // Avoid passing literally "null" as the query parameter + if (batch_id !== null) { + queryParams.batch_id = batch_id; + } + + return this.http.authedRequest( + undefined, + Method.Post, + path, + queryParams, + payload, + { + prefix: "/_matrix/client/unstable/org.matrix.msc2716", + }, + ); + } } /** From 148434dcfa3ee4787433f02703d4d9ae46b4440d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 23 May 2022 23:55:12 -0500 Subject: [PATCH 20/51] Move batchSend to e2e test in matrix-react-sdk See: - https://github.com/matrix-org/matrix-js-sdk/commit/d267136d26dc6b8fbaf8d764e25e194a9119b04e#r74092517 - https://github.com/matrix-org/matrix-react-sdk/pull/8354#discussion_r878660862 --- src/client.ts | 53 --------------------------------------------------- 1 file changed, 53 deletions(-) diff --git a/src/client.ts b/src/client.ts index ab58552620e..4a94645d969 100644 --- a/src/client.ts +++ b/src/client.ts @@ -776,25 +776,6 @@ interface ITimestampToEventResponse { event_id: string; origin_server_ts: string; } - -interface IBatchSendResponse { - /** List of state event ID's we inserted */ - state_event_ids: string[]; - /** List of historical event ID's we inserted */ - event_ids: string[]; - next_batch_id: string; - insertion_event_id: string; - batch_event_id: string; - /** When `?batch_id` isn't provided, the homeserver automatically creates an - * insertion event as a starting place to hang the history off of. This - * automatic insertion event ID is returned in this field. - * - * When `?batch_id` is provided, this field is not present because we can - * hang the history off the insertion event specified and associated by the - * batch ID. - */ - base_insertion_event_id?: string; -} /* eslint-enable camelcase */ // We're using this constant for methods overloading and inspect whether a variable @@ -8980,40 +8961,6 @@ export class MatrixClient extends TypedEventEmitter { - const path = utils.encodeUri("/rooms/$roomId/batch_send", { - $roomId: roomId, - }); - - const queryParams: { prev_event_id: string, batch_id?: string } = { - prev_event_id, - }; - // Avoid passing literally "null" as the query parameter - if (batch_id !== null) { - queryParams.batch_id = batch_id; - } - - return this.http.authedRequest( - undefined, - Method.Post, - path, - queryParams, - payload, - { - prefix: "/_matrix/client/unstable/org.matrix.msc2716", - }, - ); - } } /** From 3973e34cad674828c9878756877a125c61fd75f2 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 24 May 2022 03:39:20 -0500 Subject: [PATCH 21/51] Extra TimelineRefresh to make the timeline blank and have something to key off in the test but this also gives user feedback after pushing the refresh timeline button --- src/client.ts | 2 ++ src/models/room.ts | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/src/client.ts b/src/client.ts index 4a94645d969..d21f55e1196 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5271,7 +5271,9 @@ export class MatrixClient extends TypedEventEmitter(undefined, Method.Get, path, params); + console.log("getEventTimeline: after /context"); if (!res.event) { throw new Error("'event' not in '/context' result - homeserver too old?"); } diff --git a/src/models/room.ts b/src/models/room.ts index d52339bdaf1..a0872471305 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -955,18 +955,27 @@ export class Room extends TypedEventEmitter // same one. this.resetLiveTimeline(null, null); + // await new Promise((resolve) => { + // setTimeout(resolve, 2000); + // }) + // Get a reference to the emptied out `timelineSet` // // TODO: Do we want to use `this.getLiveTimeline().getTimelineSet()` // instead? I think it's the same thing but what's more right? const timelineSet = this.getUnfilteredTimelineSet(); + // TODO: Testing this + this.emit(RoomEvent.TimelineRefresh, this, timelineSet); + // Use `client.getEventTimeline(...)` to construct a new timeline from a // `/context` response state and events for the most recent event before // we reset everything. The `timelineSet` we pass in needs to be empty // in order for this function to call `/context` and generate a new // timeline. + console.log('refreshLiveTimeline before getEventTimeline'); const newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); + console.log('refreshLiveTimeline after getEventTimeline'); // Set the pagination token back to the live sync token instead of using // the `/context` historical token so that it matches the next response // from `/sync` and we can properly continue the timeline. From a08648957018f2493addb9e19772f32d0fabe7bb Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 25 May 2022 00:30:52 -0500 Subject: [PATCH 22/51] getLatestTimeline for when no events exist in the timeline already --- src/client.ts | 48 +++++++++++++++++++++++++++++-- src/models/room.ts | 70 +++++++++++++++++++++++++++------------------- 2 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/client.ts b/src/client.ts index d21f55e1196..5000bb5acf4 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5271,9 +5271,7 @@ export class MatrixClient extends TypedEventEmitter(undefined, Method.Get, path, params); - console.log("getEventTimeline: after /context"); if (!res.event) { throw new Error("'event' not in '/context' result - homeserver too old?"); } @@ -5286,13 +5284,15 @@ export class MatrixClient extends TypedEventEmitter { + // don't allow any timeline support unless it's been enabled. + if (!this.timelineSupport) { + throw new Error("timeline support is disabled. Set the 'timelineSupport'" + + " parameter to true when creating MatrixClient to enable it."); + } + + const messagesPath = utils.encodeUri( + "/rooms/$roomId/messages", { + $roomId: timelineSet.room.roomId + }, + ); + + let params: Record = { + dir: 'b', + }; + if (this.clientOpts.lazyLoadMembers) { + params.filter = JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER); + } + + const res = await this.http.authedRequest(undefined, Method.Get, messagesPath, params); + const event = res.chunk[0]; + if (!event) { + throw new Error("No message returned from /messages when trying to construct getLatestTimeline"); + } + + return this.getEventTimeline(timelineSet, event.event_id); + } + /** * Makes a request to /messages with the appropriate lazy loading filter set. * XXX: if we do get rid of scrollback (as it's not used at the moment), diff --git a/src/models/room.ts b/src/models/room.ts index a0872471305..eee2b4c66fb 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -938,44 +938,56 @@ export class Room extends TypedEventEmitter public async refreshLiveTimeline(): Promise { const liveTimelineBefore = this.getLiveTimeline(); const forwardPaginationToken = liveTimelineBefore.getPaginationToken(EventTimeline.FORWARDS); + const backwardPaginationToken = liveTimelineBefore.getPaginationToken(EventTimeline.BACKWARDS); const eventsBefore = liveTimelineBefore.getEvents(); const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; logger.log( `[refreshLiveTimeline] for room ${this.roomId} at ` + - `mostRecentEventInTimeline=${mostRecentEventInTimeline.getId()} ` + - `forwardPaginationToken+${forwardPaginationToken}`, + `mostRecentEventInTimeline=${mostRecentEventInTimeline && mostRecentEventInTimeline.getId()} ` + + `liveTimelineBefore=${liveTimelineBefore.toString()} ` + + `forwardPaginationToken=${forwardPaginationToken} ` + + `backwardPaginationToken=${backwardPaginationToken}`, ); - // Empty out all of `this.timelineSets` but still keeps the same - // `timelineSet` references around so the React code updates properly - // and doesn't ignore the room events we emit because it checks that the - // `timelineSet` references are the same. We need the `timelineSet` - // empty so that the `client.getEventTimeline(...)` call later, will - // call `/context` and create a new timeline instead of returning the - // same one. - this.resetLiveTimeline(null, null); - - // await new Promise((resolve) => { - // setTimeout(resolve, 2000); - // }) - - // Get a reference to the emptied out `timelineSet` - // - // TODO: Do we want to use `this.getLiveTimeline().getTimelineSet()` - // instead? I think it's the same thing but what's more right? + // Get the main TimelineSet const timelineSet = this.getUnfilteredTimelineSet(); - // TODO: Testing this - this.emit(RoomEvent.TimelineRefresh, this, timelineSet); + let newTimeline; + // If there isn't any event in the timeline, let's go fetch the latest + // event and construct a timeline from it. + // + // This should only really happen if the user ran into an error + // with refreshing the timeline before which left them in a blank + // timeline from `resetLiveTimeline`. + if (!mostRecentEventInTimeline) { + newTimeline = await this.client.getLatestTimeline(timelineSet); + } else { + // Empty out all of `this.timelineSets`. But we also need to keep the + // same `timelineSet` references around so the React code updates + // properly and doesn't ignore the room events we emit because it checks + // that the `timelineSet` references are the same. We need the + // `timelineSet` empty so that the `client.getEventTimeline(...)` call + // later, will call `/context` and create a new timeline instead of + // returning the same one. + this.resetLiveTimeline(null, null); + + // await new Promise((resolve) => { + // setTimeout(resolve, 2000); + // }) + + // TODO: Testing this, this makes the TimelinePanel show the new blank timeline + this.emit(RoomEvent.TimelineRefresh, this, timelineSet); + + // Use `client.getEventTimeline(...)` to construct a new timeline from a + // `/context` response state and events for the most recent event before + // we reset everything. The `timelineSet` we pass in needs to be empty + // in order for this function to call `/context` and generate a new + // timeline. + console.log('refreshLiveTimeline before getEventTimeline'); + newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); + console.log('refreshLiveTimeline after getEventTimeline'); + } - // Use `client.getEventTimeline(...)` to construct a new timeline from a - // `/context` response state and events for the most recent event before - // we reset everything. The `timelineSet` we pass in needs to be empty - // in order for this function to call `/context` and generate a new - // timeline. - console.log('refreshLiveTimeline before getEventTimeline'); - const newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); - console.log('refreshLiveTimeline after getEventTimeline'); // Set the pagination token back to the live sync token instead of using // the `/context` historical token so that it matches the next response // from `/sync` and we can properly continue the timeline. From d241f139584e4a4f7d38e397a45cf9344609e856 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 25 May 2022 00:45:57 -0500 Subject: [PATCH 23/51] Some cleanup --- src/models/room.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index eee2b4c66fb..00d46cead68 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -971,11 +971,11 @@ export class Room extends TypedEventEmitter // returning the same one. this.resetLiveTimeline(null, null); - // await new Promise((resolve) => { - // setTimeout(resolve, 2000); - // }) - - // TODO: Testing this, this makes the TimelinePanel show the new blank timeline + // Make the UI timeline show the new blank live timeline we just + // reset so that if the network fails below it's showing the + // accurate state of what we're working with instead of the + // disconnected one in the TimelineWindow which is just hanging + // around by reference. this.emit(RoomEvent.TimelineRefresh, this, timelineSet); // Use `client.getEventTimeline(...)` to construct a new timeline from a @@ -983,9 +983,7 @@ export class Room extends TypedEventEmitter // we reset everything. The `timelineSet` we pass in needs to be empty // in order for this function to call `/context` and generate a new // timeline. - console.log('refreshLiveTimeline before getEventTimeline'); newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); - console.log('refreshLiveTimeline after getEventTimeline'); } // Set the pagination token back to the live sync token instead of using From 0e3a5dbce713b79066105285406a304c8209bed7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 25 May 2022 00:53:01 -0500 Subject: [PATCH 24/51] Fix lints --- src/client.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client.ts b/src/client.ts index 5000bb5acf4..e06cab8f19e 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5292,7 +5292,6 @@ export class MatrixClient extends TypedEventEmitter { + public async getLatestTimeline(timelineSet: EventTimelineSet): Promise { // don't allow any timeline support unless it's been enabled. if (!this.timelineSupport) { throw new Error("timeline support is disabled. Set the 'timelineSupport'" + @@ -5373,11 +5372,11 @@ export class MatrixClient extends TypedEventEmitter = { + const params: Record = { dir: 'b', }; if (this.clientOpts.lazyLoadMembers) { From 3dff724a56b80ebecbf989972692930774761153 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 25 May 2022 21:48:12 -0500 Subject: [PATCH 25/51] Better comment --- src/models/room.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index 00d46cead68..758ca6a062c 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -986,9 +986,10 @@ export class Room extends TypedEventEmitter newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); } - // Set the pagination token back to the live sync token instead of using - // the `/context` historical token so that it matches the next response - // from `/sync` and we can properly continue the timeline. + // Set the pagination token back to the live sync token (`null`) instead + // of using the `/context` historical token (ex. `t12-13_0_0_0_0_0_0_0_0`) + // so that it matches the next response from `/sync` and we can properly + // continue the timeline. newTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); // Set our new fresh timeline as the live timeline to continue syncing From d37f788694566d13935f5f9814a2ee6cc8db823d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 25 May 2022 23:29:18 -0500 Subject: [PATCH 26/51] Re-register state listeners after `fixUpLegacyTimelineFields` changes the reference Important so that you can refresh the timeline again after refreshing it once --- src/models/room.ts | 13 +++++++++++++ src/sync.ts | 26 ++++++++++++++++---------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index 758ca6a062c..9de9aa38177 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -167,6 +167,8 @@ export enum RoomEvent { Timeline = "Room.timeline", TimelineReset = "Room.timelineReset", TimelineRefresh = "Room.TimelineRefresh", + OldStateUpdated = "Room.OldStateUpdated", + CurrentStateUpdated = "Room.CurrentStateUpdated", HistoryImportedWithinTimeline = "Room.historyImportedWithinTimeline", } @@ -178,6 +180,8 @@ type EmittedEvents = RoomEvent | RoomEvent.TimelineReset | RoomEvent.TimelineRefresh | RoomEvent.HistoryImportedWithinTimeline + | RoomEvent.OldStateUpdated + | RoomEvent.CurrentStateUpdated | MatrixEventEvent.BeforeRedaction; export type RoomEventHandlerMap = { @@ -194,6 +198,8 @@ export type RoomEventHandlerMap = { oldEventId?: string, oldStatus?: EventStatus, ) => void; + [RoomEvent.OldStateUpdated]: (room: Room, previousRoomState: RoomState, roomState: RoomState) => void; + [RoomEvent.CurrentStateUpdated]: (room: Room, previousRoomState: RoomState, roomState: RoomState) => void; [RoomEvent.HistoryImportedWithinTimeline]: ( markerEvent: MatrixEvent, room: Room, @@ -1034,6 +1040,9 @@ export class Room extends TypedEventEmitter * @private */ private fixUpLegacyTimelineFields(): void { + const previousOldState = this.oldState; + const previousCurrentState = this.currentState; + // maintain this.timeline as a reference to the live timeline, // and this.oldState and this.currentState as references to the // state at the start and end of that timeline. These are more @@ -1043,6 +1052,10 @@ export class Room extends TypedEventEmitter .getState(EventTimeline.BACKWARDS); this.currentState = this.getLiveTimeline() .getState(EventTimeline.FORWARDS); + + // Let people know to register listeners for the new state references + this.emit(RoomEvent.OldStateUpdated, this, previousOldState, this.currentState); + this.emit(RoomEvent.CurrentStateUpdated, this, previousCurrentState, this.currentState); } /** diff --git a/src/sync.ts b/src/sync.ts index aa0fb49c187..1cb24e3e788 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -51,7 +51,7 @@ import { MatrixError, Method } from "./http-api"; import { ISavedSync } from "./store"; import { EventType } from "./@types/event"; import { IPushRules } from "./@types/PushRules"; -import { RoomStateEvent } from "./models/room-state"; +import { RoomState, RoomStateEvent } from "./models/room-state"; import { RoomMemberEvent } from "./models/room-member"; import { BeaconEvent } from "./models/beacon"; import { IEventsResponse } from "./@types/requests"; @@ -228,6 +228,15 @@ export class SyncApi { RoomEvent.TimelineReset, ]); this.registerStateListeners(room); + // Register listeners again after the state reference changes + room.on(RoomEvent.CurrentStateUpdated, (targetRoom, previousCurrentState) => { + if (targetRoom !== room) { + return; + } + + this.deregisterStateListeners(previousCurrentState); + this.registerStateListeners(room); + }); return room; } @@ -267,15 +276,15 @@ export class SyncApi { } /** - * @param {Room} room + * @param {RoomState} roomState The roomState to clear listeners from * @private */ - private deregisterStateListeners(room: Room): void { + private deregisterStateListeners(roomState: RoomState): void { // could do with a better way of achieving this. - room.currentState.removeAllListeners(RoomStateEvent.Events); - room.currentState.removeAllListeners(RoomStateEvent.Members); - room.currentState.removeAllListeners(RoomStateEvent.NewMember); - room.currentState.removeAllListeners(RoomStateEvent.Marker); + roomState.removeAllListeners(RoomStateEvent.Events); + roomState.removeAllListeners(RoomStateEvent.Members); + roomState.removeAllListeners(RoomStateEvent.NewMember); + roomState.removeAllListeners(RoomStateEvent.Marker); } /** When we see the marker state change in the room, we know there is some @@ -1363,7 +1372,6 @@ export class SyncApi { } if (limited) { - this.deregisterStateListeners(room); room.resetLiveTimeline( joinObj.timeline.prev_batch, this.opts.canResetEntireTimeline(room.roomId) ? @@ -1374,8 +1382,6 @@ export class SyncApi { // reason to stop incrementally tracking notifications and // reset the timeline. client.resetNotifTimelineSet(); - - this.registerStateListeners(room); } } From 6b8aa6c33e705513f8bc428586020817ddedc456 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 26 May 2022 00:37:22 -0500 Subject: [PATCH 27/51] Remove lastMarkerEventProcessed logic I don't think it's necessary, https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r882329556 --- spec/integ/matrix-client-syncing.spec.js | 1 - src/models/room.ts | 20 -------------------- src/sync.ts | 18 +----------------- 3 files changed, 1 insertion(+), 38 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 891cfcbbc8c..805daca8283 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -845,7 +845,6 @@ describe("MatrixClient syncing", function() { expect(room.getTimelineNeedsRefresh()).toEqual(true); // Make sure `RoomEvent.HistoryImportedWithinTimeline` was emitted expect(emitCount).toEqual(1); - expect(room.getLastMarkerEventIdProcessed()).toEqual(markerEventId); }); // Mimic a marker event being sent far back in the scroll back but since our last sync diff --git a/src/models/room.ts b/src/models/room.ts index 9de9aa38177..33df792d69c 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -223,7 +223,6 @@ export class Room extends TypedEventEmitter // any filtered timeline sets we're maintaining for this room private readonly filteredTimelineSets: Record = {}; // filter_id: timelineSet private timelineNeedsRefresh = false; - private lastMarkerEventIdProcessed: string = null; private readonly pendingEventList?: MatrixEvent[]; // read by megolm via getter; boolean value - null indicates "use global value" private blacklistUnverifiedDevices: boolean = null; @@ -1141,25 +1140,6 @@ export class Room extends TypedEventEmitter return this.timelineNeedsRefresh; } - /** - * 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 1cb24e3e788..c80fb83cfb3 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -332,18 +332,6 @@ export class SyncApi { ); } - // 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 @@ -356,10 +344,7 @@ export class SyncApi { // 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 - ) { + if (isValidMsc2716Event) { // Saw new marker event, let's let the clients know they should // refresh the timeline. logger.debug( @@ -368,7 +353,6 @@ export class SyncApi { ); room.setTimelineNeedsRefresh(true); room.emit(RoomEvent.HistoryImportedWithinTimeline, markerEvent, room); - room.setLastMarkerEventIdProcessed(markerEvent.getId()); } } From 4bb1d148967ac8b92e50bdadfed82974b53e2c6b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 26 May 2022 16:14:14 -0500 Subject: [PATCH 28/51] Add some missing test coverage Added tests for the following - `getRoomCreator` - `getLatestTimeline` - `RoomEvent.OldStateUpdated` and `RoomEvent.CurrentStateUpdated` --- .../matrix-client-event-timeline.spec.js | 71 +++++++++++++++++++ spec/unit/room.spec.ts | 42 +++++++++++ src/client.ts | 2 +- src/models/room.ts | 8 ++- src/sync.ts | 2 +- 5 files changed, 121 insertions(+), 4 deletions(-) diff --git a/spec/integ/matrix-client-event-timeline.spec.js b/spec/integ/matrix-client-event-timeline.spec.js index 6df4a9a813c..52a029343b6 100644 --- a/spec/integ/matrix-client-event-timeline.spec.js +++ b/spec/integ/matrix-client-event-timeline.spec.js @@ -540,6 +540,77 @@ describe("MatrixClient event timelines", function() { }); }); + describe("getLatestTimeline", function() { + it("should create a new timeline for new events", function() { + const room = client.getRoom(roomId); + const timelineSet = room.getTimelineSets()[0]; + + const latestMessageId = 'event1:bar'; + + httpBackend.when("GET", "/rooms/!foo%3Abar/messages") + .respond(200, function() { + return { + chunk: [{ + event_id: latestMessageId, + }], + }; + }); + + httpBackend.when("GET", `/rooms/!foo%3Abar/context/${encodeURIComponent(latestMessageId)}`) + .respond(200, function() { + return { + start: "start_token", + events_before: [EVENTS[1], EVENTS[0]], + event: EVENTS[2], + events_after: [EVENTS[3]], + state: [ + ROOM_NAME_EVENT, + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + }); + + return Promise.all([ + client.getLatestTimeline(timelineSet).then(function(tl) { + // Instead of this assertion logic, we could just add a spy + // for `getEventTimeline` and make sure it's called with the + // correct parameters. This doesn't feel to bad to make sure + // `getLatestTimeline` is doing the right thing though. + expect(tl.getEvents().length).toEqual(4); + for (let i = 0; i < 4; i++) { + expect(tl.getEvents()[i].event).toEqual(EVENTS[i]); + expect(tl.getEvents()[i].sender.name).toEqual(userName); + } + expect(tl.getPaginationToken(EventTimeline.BACKWARDS)) + .toEqual("start_token"); + expect(tl.getPaginationToken(EventTimeline.FORWARDS)) + .toEqual("end_token"); + }), + httpBackend.flushAllExpected(), + ]); + }); + + it("should throw error when /messages does not return a message", () => { + const room = client.getRoom(roomId); + const timelineSet = room.getTimelineSets()[0]; + + httpBackend.when("GET", "/rooms/!foo%3Abar/messages") + .respond(200, () => { + return { + chunk: [ + // No messages to return + ], + }; + }); + + return Promise.all([ + expect(client.getLatestTimeline(timelineSet)).rejects.toThrow(), + httpBackend.flushAllExpected(), + ]); + }); + }); + describe("paginateEventTimeline", function() { it("should allow you to paginate backwards", function() { const room = client.getRoom(roomId); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index feb0b6618fb..2e9803bf017 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -133,6 +133,27 @@ describe("Room", function() { room.currentState = room.getLiveTimeline().endState = utils.mock(RoomState, "currentState"); }); + describe('getCreator', () => { + it("should return the creator from m.room.create", function() { + room.currentState.getStateEvents.mockImplementation(function(type, key) { + if (type === EventType.RoomCreate && key === "") { + return utils.mkEvent({ + event: true, + type: EventType.RoomCreate, + skey: "", + room: roomId, + user: userA, + content: { + creator: userA, + }, + }); + } + }); + const roomCreator = room.getCreator(); + expect(roomCreator).toStrictEqual(userA); + }); + }); + describe("getAvatarUrl", function() { const hsUrl = "https://my.home.server"; @@ -530,6 +551,23 @@ describe("Room", function() { it("should reset the legacy timeline fields", function() { room.addLiveEvents([events[0], events[1]]); expect(room.timeline.length).toEqual(2); + + const currentStateBeforeRunningReset = room.currentState; + let currentStateUpdateEmitCount = 0; + room.on(RoomEvent.CurrentStateUpdated, function(room, previousCurrentState, currentState) { + expect(previousCurrentState).toBe(currentStateBeforeRunningReset); + expect(currentState).toBe(room.currentState); + currentStateUpdateEmitCount += 1; + }); + + const oldStateBeforeRunningReset = room.oldState; + let oldStateUpdateEmitCount = 0; + room.on(RoomEvent.OldStateUpdated, function(room, previousOldState, oldState) { + expect(previousOldState).toBe(oldStateBeforeRunningReset); + expect(oldState).toBe(room.oldState); + oldStateUpdateEmitCount += 1; + }); + room.resetLiveTimeline('sometoken', 'someothertoken'); room.addLiveEvents([events[2]]); @@ -539,6 +577,10 @@ describe("Room", function() { newLiveTimeline.getState(EventTimeline.BACKWARDS)); expect(room.currentState).toEqual( newLiveTimeline.getState(EventTimeline.FORWARDS)); + // Make sure `RoomEvent.CurrentStateUpdated` was emitted + expect(currentStateUpdateEmitCount).toEqual(1); + // Make sure `RoomEvent.OldStateUpdated` was emitted + expect(oldStateUpdateEmitCount).toEqual(1); }); it("should emit Room.timelineReset event and set the correct " + diff --git a/src/client.ts b/src/client.ts index 42df9b66d81..6cc52be7439 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5397,7 +5397,7 @@ export class MatrixClient extends TypedEventEmitter(undefined, Method.Get, messagesPath, params); - const event = res.chunk[0]; + const event = res.chunk?.[0]; if (!event) { throw new Error("No message returned from /messages when trying to construct getLatestTimeline"); } diff --git a/src/models/room.ts b/src/models/room.ts index 33df792d69c..6d33a0f385f 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -462,7 +462,7 @@ export class Room extends TypedEventEmitter * 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 { + public getCreator(): string | null { const createEvent = this.currentState.getStateEvents(EventType.RoomCreate, ""); if (!createEvent) { return null; @@ -954,6 +954,10 @@ export class Room extends TypedEventEmitter `backwardPaginationToken=${backwardPaginationToken}`, ); + await new Promise((resolve) => { + setTimeout(resolve, 12000); + }); + // Get the main TimelineSet const timelineSet = this.getUnfilteredTimelineSet(); @@ -1053,7 +1057,7 @@ export class Room extends TypedEventEmitter .getState(EventTimeline.FORWARDS); // Let people know to register listeners for the new state references - this.emit(RoomEvent.OldStateUpdated, this, previousOldState, this.currentState); + this.emit(RoomEvent.OldStateUpdated, this, previousOldState, this.oldState); this.emit(RoomEvent.CurrentStateUpdated, this, previousCurrentState, this.currentState); } diff --git a/src/sync.ts b/src/sync.ts index c80fb83cfb3..1f178240ac2 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -322,7 +322,7 @@ export class SyncApi { // 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(); + markerEvent.getSender() === room.getCreator(); if (!isValidMsc2716Event) { logger.debug( From 34a943b8617c240e7bd96d2f8312a5786bf069a3 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 26 May 2022 22:14:08 -0500 Subject: [PATCH 29/51] Add tests for sync state listeners and RoomEvent.CurrentStateUpdated --- spec/integ/matrix-client-syncing.spec.js | 172 ++++++++++++++++++++++- src/models/room.ts | 17 ++- 2 files changed, 178 insertions(+), 11 deletions(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 805daca8283..222ab834d69 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventTimeline, MatrixEvent, RoomEvent } from "../../src"; +import { EventTimeline, MatrixEvent, RoomEvent, RoomStateEvent, RoomMemberEvent } from "../../src"; import { UNSTABLE_MSC2716_MARKER } from "../../src/@types/event"; import * as utils from "../test-utils/test-utils"; import { TestClient } from "../TestClient"; @@ -77,7 +77,7 @@ describe("MatrixClient syncing", function() { }); }); - it("should emit Room.myMembership for invite->leave->invite cycles", async () => { + it("should emit RoomEvent.MyMembership for invite->leave->invite cycles", async () => { const roomId = "!cycles:example.org"; // First sync: an invite @@ -299,7 +299,7 @@ describe("MatrixClient syncing", function() { httpBackend.when("GET", "/sync").respond(200, syncData); let latestFiredName = null; - client.on("RoomMember.name", function(event, m) { + client.on(RoomMemberEvent.Name, function(event, m) { if (m.userId === userC && m.roomId === roomOne) { latestFiredName = m.name; } @@ -888,6 +888,170 @@ describe("MatrixClient syncing", function() { }); }); }); + + // Make sure the state listeners work and events are re-emitted properly from + // the client regardless if we reset and refresh the timeline. + describe('state listeners and re-registered when RoomEvent.CurrentStateUpdated is fired', () => { + const EVENTS = [ + utils.mkMessage({ + room: roomOne, user: userA, msg: "we", + }), + utils.mkMessage({ + room: roomOne, user: userA, msg: "could", + }), + utils.mkMessage({ + room: roomOne, user: userA, msg: "be", + }), + utils.mkMessage({ + room: roomOne, user: userA, msg: "heroes", + }), + ]; + + const SOME_STATE_EVENT = utils.mkEvent({ + event: true, + type: 'org.matrix.test_state', + room: roomOne, + user: userA, + skey: "", + content: { + "foo": "bar", + }, + }); + + const USER_MEMBERSHIP_EVENT = utils.mkMembership({ + room: roomOne, mship: "join", user: userA, + }); + + // This appears to work even if we comment out + // `RoomEvent.CurrentStateUpdated` part which triggers everything to + // re-listen after the `room.currentState` reference changes. I'm + // not sure how it's getting re-emitted. + it("should be able to listen to state events even after " + + "the timeline is reset during `limited` sync response", async () => { + // Create a room from the sync + httpBackend.when("GET", "/sync").respond(200, syncData); + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + // Get the room after the first sync so the room is created + const room = client.getRoom(roomOne); + expect(room).toBeDefined(); + + let stateEventEmitCount = 0; + client.on(RoomStateEvent.Update, () => { + stateEventEmitCount += 1; + }); + + // Cause `RoomStateEvent.Update` to be fired + room.currentState.setStateEvents([SOME_STATE_EVENT]); + // Make sure we can listen to the room state events before the reset + expect(stateEventEmitCount).toEqual(1); + + // Make a `limited` sync which will cause a `room.resetLiveTimeline` + const limitedSyncData = { + next_batch: "batch_token", + rooms: { + join: {}, + }, + }; + limitedSyncData.rooms.join[roomOne] = { + timeline: { + events: [ + utils.mkMessage({ + room: roomOne, user: otherUserId, msg: "world", + }), + ], + // The important part, make the sync `limited` + limited: true, + prev_batch: "newerTok", + }, + }; + httpBackend.when("GET", "/sync").respond(200, limitedSyncData); + + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + // This got incremented again from processing the sync response + expect(stateEventEmitCount).toEqual(2); + + // Cause `RoomStateEvent.Update` to be fired + room.currentState.setStateEvents([SOME_STATE_EVENT]); + // Make sure we can still listen to the room state events after the reset + expect(stateEventEmitCount).toEqual(3); + }); + + // Make sure it re-registers the state listeners after the + // `room.currentState` reference changes + it("should be able to listen to state events even after " + + "refreshing the timeline", async () => { + const testClientWithTimelineSupport = new TestClient( + selfUserId, + "DEVICE", + selfAccessToken, + undefined, + { timelineSupport: true }, + ); + httpBackend = testClientWithTimelineSupport.httpBackend; + httpBackend.when("GET", "/versions").respond(200, {}); + httpBackend.when("GET", "/pushrules").respond(200, {}); + httpBackend.when("POST", "/filter").respond(200, { filter_id: "a filter id" }); + client = testClientWithTimelineSupport.client; + + // Create a room from the sync + httpBackend.when("GET", "/sync").respond(200, syncData); + client.startClient(); + await Promise.all([ + httpBackend.flushAllExpected(), + awaitSyncEvent(), + ]); + + // Get the room after the first sync so the room is created + const room = client.getRoom(roomOne); + expect(room).toBeDefined(); + + let stateEventEmitCount = 0; + client.on(RoomStateEvent.Update, () => { + stateEventEmitCount += 1; + }); + + // Cause `RoomStateEvent.Update` to be fired + room.currentState.setStateEvents([SOME_STATE_EVENT]); + // Make sure we can listen to the room state events before the reset + expect(stateEventEmitCount).toEqual(1); + + const eventsInRoom = syncData.rooms.join[roomOne].timeline.events; + httpBackend.when("GET", `/rooms/${encodeURIComponent(roomOne)}/context/${encodeURIComponent(eventsInRoom[0].event_id)}`) + .respond(200, function() { + return { + start: "start_token", + events_before: [EVENTS[1], EVENTS[0]], + event: EVENTS[2], + events_after: [EVENTS[3]], + state: [ + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + }); + + // Refresh the timeline. This will cause the `room.currentState` + // reference to change + await Promise.all([ + room.refreshLiveTimeline(), + httpBackend.flushAllExpected(), + ]); + + // Cause `RoomStateEvent.Update` to be fired + room.currentState.setStateEvents([SOME_STATE_EVENT]); + // Make sure we can still listen to the room state events after the reset + expect(stateEventEmitCount).toEqual(2); + }); + }); }); describe("timeline", function() { @@ -972,7 +1136,7 @@ describe("MatrixClient syncing", function() { let resetCallCount = 0; // the token should be set *before* timelineReset is emitted - client.on("Room.timelineReset", function(room) { + client.on(RoomEvent.TimelineReset, function(room) { resetCallCount++; const tl = room.getLiveTimeline(); diff --git a/src/models/room.ts b/src/models/room.ts index 6d33a0f385f..3f92a679479 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -954,10 +954,6 @@ export class Room extends TypedEventEmitter `backwardPaginationToken=${backwardPaginationToken}`, ); - await new Promise((resolve) => { - setTimeout(resolve, 12000); - }); - // Get the main TimelineSet const timelineSet = this.getUnfilteredTimelineSet(); @@ -1056,9 +1052,16 @@ export class Room extends TypedEventEmitter this.currentState = this.getLiveTimeline() .getState(EventTimeline.FORWARDS); - // Let people know to register listeners for the new state references - this.emit(RoomEvent.OldStateUpdated, this, previousOldState, this.oldState); - this.emit(RoomEvent.CurrentStateUpdated, this, previousCurrentState, this.currentState); + // Let people know to register new listeners for the new state + // references. The reference won't necessarily change every time so only + // emit when we see a change. + if (previousOldState !== this.oldState) { + this.emit(RoomEvent.OldStateUpdated, this, previousOldState, this.oldState); + } + + if (previousCurrentState !== this.currentState) { + this.emit(RoomEvent.CurrentStateUpdated, this, previousCurrentState, this.currentState); + } } /** From e412aa17d77f0df6641d4ae2d0f56b4ff3145d82 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 26 May 2022 22:18:08 -0500 Subject: [PATCH 30/51] Fix lint --- spec/integ/matrix-client-syncing.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 222ab834d69..39735420f4d 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -1025,7 +1025,9 @@ describe("MatrixClient syncing", function() { expect(stateEventEmitCount).toEqual(1); const eventsInRoom = syncData.rooms.join[roomOne].timeline.events; - httpBackend.when("GET", `/rooms/${encodeURIComponent(roomOne)}/context/${encodeURIComponent(eventsInRoom[0].event_id)}`) + const contextUrl = `/rooms/${encodeURIComponent(roomOne)}/context/` + + `${encodeURIComponent(eventsInRoom[0].event_id)}`; + httpBackend.when("GET", contextUrl) .respond(200, function() { return { start: "start_token", From 9d1c7c7308dd1c6cfba320c1ac00ecba93035fec Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 26 May 2022 22:33:53 -0500 Subject: [PATCH 31/51] Fix test when we only emit on reference change --- spec/unit/room.spec.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 2e9803bf017..5f6a43a2e11 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -552,14 +552,6 @@ describe("Room", function() { room.addLiveEvents([events[0], events[1]]); expect(room.timeline.length).toEqual(2); - const currentStateBeforeRunningReset = room.currentState; - let currentStateUpdateEmitCount = 0; - room.on(RoomEvent.CurrentStateUpdated, function(room, previousCurrentState, currentState) { - expect(previousCurrentState).toBe(currentStateBeforeRunningReset); - expect(currentState).toBe(room.currentState); - currentStateUpdateEmitCount += 1; - }); - const oldStateBeforeRunningReset = room.oldState; let oldStateUpdateEmitCount = 0; room.on(RoomEvent.OldStateUpdated, function(room, previousOldState, oldState) { @@ -568,6 +560,14 @@ describe("Room", function() { oldStateUpdateEmitCount += 1; }); + const currentStateBeforeRunningReset = room.currentState; + let currentStateUpdateEmitCount = 0; + room.on(RoomEvent.CurrentStateUpdated, function(room, previousCurrentState, currentState) { + expect(previousCurrentState).toBe(currentStateBeforeRunningReset); + expect(currentState).toBe(room.currentState); + currentStateUpdateEmitCount += 1; + }); + room.resetLiveTimeline('sometoken', 'someothertoken'); room.addLiveEvents([events[2]]); @@ -577,10 +577,11 @@ describe("Room", function() { newLiveTimeline.getState(EventTimeline.BACKWARDS)); expect(room.currentState).toEqual( newLiveTimeline.getState(EventTimeline.FORWARDS)); - // Make sure `RoomEvent.CurrentStateUpdated` was emitted - expect(currentStateUpdateEmitCount).toEqual(1); // Make sure `RoomEvent.OldStateUpdated` was emitted expect(oldStateUpdateEmitCount).toEqual(1); + // `RoomEvent.CurrentStateUpdated` wasn't emitted because it has the + // same reference (timeline didn't change) + expect(currentStateUpdateEmitCount).toEqual(0); }); it("should emit Room.timelineReset event and set the correct " + From c36e1b9e8d3773db2861ad97fac7790bbb8523b7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 26 May 2022 22:41:37 -0500 Subject: [PATCH 32/51] Fix test passing with and without timelineSupport --- spec/unit/room.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 5f6a43a2e11..f092e6e0dd5 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -579,9 +579,8 @@ describe("Room", function() { newLiveTimeline.getState(EventTimeline.FORWARDS)); // Make sure `RoomEvent.OldStateUpdated` was emitted expect(oldStateUpdateEmitCount).toEqual(1); - // `RoomEvent.CurrentStateUpdated` wasn't emitted because it has the - // same reference (timeline didn't change) - expect(currentStateUpdateEmitCount).toEqual(0); + // Make sure `RoomEvent.OldStateUpdated` was emitted if necessary + expect(currentStateUpdateEmitCount).toEqual(timelineSupport ? 1 : 0); }); it("should emit Room.timelineReset event and set the correct " + From f663d6b272079c54e6d233fa731ba016ee923cf9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 May 2022 17:53:55 -0500 Subject: [PATCH 33/51] Consolidate imports --- src/models/room.ts | 3 +-- src/sync.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index 3f92a679479..646c1615e09 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -18,7 +18,7 @@ limitations under the License. * @module models/room */ -import { EventTimelineSet, DuplicateStrategy } from "./event-timeline-set"; +import { EventTimelineSet, DuplicateStrategy, IAddLiveEventOptions } from "./event-timeline-set"; import { Direction, EventTimeline } from "./event-timeline"; import { getHttpUriForMxc } from "../content-repo"; import * as utils from "../utils"; @@ -49,7 +49,6 @@ import { import { TypedEventEmitter } from "./typed-event-emitter"; import { ReceiptType } from "../@types/read_receipts"; import { IStateEventWithRoomId } from "../@types/search"; -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 diff --git a/src/sync.ts b/src/sync.ts index 1f178240ac2..05a7bc9cd8d 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -51,11 +51,10 @@ import { MatrixError, Method } from "./http-api"; import { ISavedSync } from "./store"; import { EventType } from "./@types/event"; import { IPushRules } from "./@types/PushRules"; -import { RoomState, RoomStateEvent } from "./models/room-state"; +import { RoomState, RoomStateEvent, IMarkerFoundOptions } from "./models/room-state"; import { RoomMemberEvent } from "./models/room-member"; import { BeaconEvent } from "./models/beacon"; import { IEventsResponse } from "./@types/requests"; -import { IMarkerFoundOptions } from "./models/room-state"; const DEBUG = true; From 313df51ce33a269a41ba095a075e23f4808902e9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 May 2022 18:18:16 -0500 Subject: [PATCH 34/51] Remove unnecessary overloads for private method See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r883505721 --- src/models/room.ts | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index 646c1615e09..f445a8042b1 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -1928,23 +1928,8 @@ export class Room extends TypedEventEmitter * @fires module:client~MatrixClient#event:"Room.timeline" * @private */ - private addLiveEvent(event: MatrixEvent, duplicateStrategy: DuplicateStrategy, fromCache?): void; - private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions): void; - private addLiveEvent( - event: MatrixEvent, - duplicateStrategyOrOpts: DuplicateStrategy | IAddLiveEventOptions, - fromCache = false, - ): void { - let duplicateStrategy = duplicateStrategyOrOpts as DuplicateStrategy; - let timelineWasEmpty: boolean; - if (typeof (duplicateStrategyOrOpts) === 'object') { - ({ - duplicateStrategy = DuplicateStrategy.Ignore, - fromCache = false, - /* roomState, (not used here) */ - timelineWasEmpty, - } = duplicateStrategyOrOpts); - } + private addLiveEvent(event: MatrixEvent, addLiveEventOptions: IAddLiveEventOptions): void { + const { duplicateStrategy, timelineWasEmpty, fromCache } = addLiveEventOptions; // add to our timeline sets for (let i = 0; i < this.timelineSets.length; i++) { From 90db6fd860db97e7468736e0c25d418d638973c5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 May 2022 23:40:41 -0500 Subject: [PATCH 35/51] Add barebones tests for overloads See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r883500595 --- spec/unit/event-timeline-set.spec.ts | 71 ++++++++++++++++++++++++++++ spec/unit/event-timeline.spec.js | 5 ++ spec/unit/room.spec.ts | 30 +++++++----- 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index 82cadddf8f5..eaa723940b9 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -23,6 +23,7 @@ import { MatrixEvent, MatrixEventEvent, Room, + DuplicateStrategy, } from '../../src'; describe('EventTimelineSet', () => { @@ -73,6 +74,76 @@ describe('EventTimelineSet', () => { }) as MatrixEvent; }); + describe('addLiveEvent', () => { + it("Adds event to the live timeline in the timeline set", () => { + const liveTimeline = eventTimelineSet.getLiveTimeline(); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + eventTimelineSet.addLiveEvent(messageEvent); + expect(liveTimeline.getEvents().length).toStrictEqual(1); + }); + + it("should replace a timeline event if dupe strategy is 'replace'", () => { + const liveTimeline = eventTimelineSet.getLiveTimeline(); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + eventTimelineSet.addLiveEvent(messageEvent, { + duplicateStrategy: DuplicateStrategy.Replace, + }); + expect(liveTimeline.getEvents().length).toStrictEqual(1); + + // make a duplicate + const duplicateMessageEvent = utils.mkMessage({ + room: roomId, user: userA, msg: "dupe", event: true, + }) as MatrixEvent; + duplicateMessageEvent.event.event_id = messageEvent.getId(); + + // Adding the duplicate event should replace the `messageEvent` + // because it has the same `event_id` and duplicate strategy is + // replace. + eventTimelineSet.addLiveEvent(duplicateMessageEvent, { + duplicateStrategy: DuplicateStrategy.Replace, + }); + + const eventsInLiveTimeline = liveTimeline.getEvents(); + expect(eventsInLiveTimeline.length).toStrictEqual(1); + expect(eventsInLiveTimeline[0]).toStrictEqual(duplicateMessageEvent); + }); + + it("Make sure legacy overload passing options directly as parameters still works", () => { + expect(() => eventTimelineSet.addLiveEvent(messageEvent, DuplicateStrategy.Replace, false)).not.toThrow(); + expect(() => eventTimelineSet.addLiveEvent(messageEvent, DuplicateStrategy.Ignore, true)).not.toThrow(); + }); + }); + + describe('addEventToTimeline', () => { + it("Adds event to timeline", () => { + const liveTimeline = eventTimelineSet.getLiveTimeline(); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline, { + toStartOfTimeline: true, + }); + expect(liveTimeline.getEvents().length).toStrictEqual(1); + }); + + it("Make sure legacy overload passing options directly as parameters still works", () => { + const liveTimeline = eventTimelineSet.getLiveTimeline(); + expect(() => { + eventTimelineSet.addEventToTimeline( + messageEvent, + liveTimeline, + true, + ); + }).not.toThrow(); + expect(() => { + eventTimelineSet.addEventToTimeline( + messageEvent, + liveTimeline, + true, + false, + ); + }).not.toThrow(); + }); + }); + describe('aggregateRelations', () => { describe('with unencrypted events', () => { beforeEach(() => { diff --git a/spec/unit/event-timeline.spec.js b/spec/unit/event-timeline.spec.js index caacf55fc99..ed5047c111e 100644 --- a/spec/unit/event-timeline.spec.js +++ b/spec/unit/event-timeline.spec.js @@ -307,6 +307,11 @@ describe("EventTimeline", function() { expect(timeline.getState(EventTimeline.FORWARDS).setStateEvents). not.toHaveBeenCalled(); }); + + it("Make sure legacy overload passing options directly as parameters still works", () => { + expect(() => timeline.addEvent(events[0], { toStartOfTimeline: true })).not.toThrow(); + expect(() => timeline.addEvent(events[0], { stateContext: new RoomState() })).not.toThrow(); + }); }); describe("removeEvent", function() { diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index f092e6e0dd5..921feae1af4 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -217,17 +217,10 @@ describe("Room", function() { }) as MatrixEvent, ]; - it("should call RoomState.setTypingEvent on m.typing events", function() { - const typing = utils.mkEvent({ - room: roomId, - type: EventType.Typing, - event: true, - content: { - user_ids: [userA], - }, - }); - room.addEphemeralEvents([typing]); - expect(room.currentState.setTypingEvent).toHaveBeenCalledWith(typing); + it("Make sure legacy overload passing options directly as parameters still works", () => { + expect(() => room.addLiveEvents(events, DuplicateStrategy.Replace, false)).not.toThrow(); + expect(() => room.addLiveEvents(events, DuplicateStrategy.Ignore, true)).not.toThrow(); + expect(() => room.addLiveEvents(events, "shouldfailbecauseinvalidduplicatestrategy", false)).toThrow(); }); it("should throw if duplicateStrategy isn't 'replace' or 'ignore'", function() { @@ -370,6 +363,21 @@ describe("Room", function() { }); }); + describe('addEphemeralEvents', () => { + it("should call RoomState.setTypingEvent on m.typing events", function() { + const typing = utils.mkEvent({ + room: roomId, + type: EventType.Typing, + event: true, + content: { + user_ids: [userA], + }, + }); + room.addEphemeralEvents([typing]); + expect(room.currentState.setTypingEvent).toHaveBeenCalledWith(typing); + }); + }); + describe("addEventsToTimeline", function() { const events = [ utils.mkMessage({ From 9f3df87ced3ebb3036269ac6b2f9f8bed9196805 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 May 2022 23:54:13 -0500 Subject: [PATCH 36/51] More straight-forward if-else See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r883495388 --- src/sync.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 05a7bc9cd8d..7236679b745 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -323,13 +323,6 @@ export class SyncApi { // and "marker" events when they come from the room creator markerEvent.getSender() === room.getCreator(); - 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 @@ -352,6 +345,12 @@ export class SyncApi { ); room.setTimelineNeedsRefresh(true); room.emit(RoomEvent.HistoryImportedWithinTimeline, markerEvent, room); + } else { + 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.`, + ); } } From 8fc837f08b5501a7b37f7ccd75fc906def7847ab Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 28 May 2022 00:11:52 -0500 Subject: [PATCH 37/51] Added extra explanation what marker event means See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r883499258 --- src/models/room-state.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/models/room-state.ts b/src/models/room-state.ts index 098eabb899d..02632b1aece 100644 --- a/src/models/room-state.ts +++ b/src/models/room-state.ts @@ -31,11 +31,18 @@ import { TypedReEmitter } from "../ReEmitter"; import { M_BEACON, M_BEACON_INFO } from "../@types/beacon"; export interface IMarkerFoundOptions { - /** Whether the timeline was empty before the marker arrived in - * the room. This could be happen in a variety of cases: + /** Whether the timeline was empty before the marker event arrived 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` */ + * 3. Or whether it's coming from `syncFromCache` + * + * A marker event refers to `UNSTABLE_MSC2716_MARKER` and indicates that + * history was imported somewhere back in time. It specifically points to an + * MSC2716 insertion event where the history was imported at. Marker events + * are sent as state events so they are easily discoverable by clients and + * homeservers and don't get lost in timeline gaps. + */ timelineWasEmpty?: boolean; } From 78689a9f9fb96dc4478b335e3b0e03f21e639612 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 28 May 2022 00:24:50 -0500 Subject: [PATCH 38/51] Fix lint --- src/sync.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sync.ts b/src/sync.ts index 7236679b745..9cbea0b48a2 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -323,7 +323,6 @@ export class SyncApi { // and "marker" events when they come from the room creator markerEvent.getSender() === room.getCreator(); - // 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 From 58fa63c27a21aa2721b2c178caa7b2ea68cfca77 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 28 May 2022 00:29:28 -0500 Subject: [PATCH 39/51] Dry interfaces by extending (OOP) See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r883502814 --- src/models/event-timeline-set.ts | 36 +++++++------------------------- src/models/event-timeline.ts | 23 +++++++------------- 2 files changed, 15 insertions(+), 44 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index ae718baf8f4..e9d87729dbd 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -18,7 +18,7 @@ limitations under the License. * @module models/event-timeline-set */ -import { EventTimeline } from "./event-timeline"; +import { EventTimeline, IAddEventOptions } from "./event-timeline"; import { EventStatus, MatrixEvent, MatrixEventEvent } from "./event"; import { logger } from '../logger'; import { Relations } from './relations'; @@ -55,7 +55,12 @@ export interface IRoomTimelineData { liveEvent?: boolean; } -export interface IAddLiveEventOptions { +export interface IAddEventToTimelineOptions extends IAddEventOptions { + /** Whether the sync response came from cache */ + fromCache?: boolean; +} + +export interface IAddLiveEventOptions extends Omit { /** 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 @@ -63,33 +68,6 @@ export interface IAddLiveEventOptions { * 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 timeline was empty before the marker arrived 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` */ - timelineWasEmpty?: boolean; -} - -export interface IAddEventToTimelineOptions { - /** Whether to insert the new event at the start of the timeline where the - * oldest events are (timeline is in chronological order, oldest to most - * recent) */ - toStartOfTimeline: boolean; - /** Whether the sync response came from cache */ - fromCache?: boolean; - /** The state events to reconcile metadata from */ - roomState?: RoomState; - /** Whether the timeline was empty before the marker arrived 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` */ - timelineWasEmpty?: boolean; } type EmittedEvents = RoomEvent.Timeline | RoomEvent.TimelineReset; diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts index 3837521454c..f240336ef50 100644 --- a/src/models/event-timeline.ts +++ b/src/models/event-timeline.ts @@ -18,34 +18,27 @@ limitations under the License. * @module models/event-timeline */ -import { RoomState } from "./room-state"; +import { RoomState, IMarkerFoundOptions } from "./room-state"; import { EventTimelineSet } from "./event-timeline-set"; import { MatrixEvent } from "./event"; import { Filter } from "../filter"; import { EventType } from "../@types/event"; -export interface IInitialiseStateOptions { - /** Whether the timeline was empty before the marker arrived 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` */ - timelineWasEmpty?: boolean; +export interface IInitialiseStateOptions extends IMarkerFoundOptions { + // This is a separate interface without any extra stuff currently added on + // top of `IMarkerFoundOptions` just because it feels like they have + // different concerns. One shouldn't necessarily look to add to + // `IMarkerFoundOptions` just because they want to add an extra option to + // `initialiseState`. } -export interface IAddEventOptions { +export interface IAddEventOptions extends IMarkerFoundOptions { /** Whether to insert the new event at the start of the timeline where the * oldest events are (timeline is in chronological order, oldest to most * recent) */ toStartOfTimeline: boolean; /** The state events to reconcile metadata from */ roomState?: RoomState; - /** Whether the timeline was empty before the marker arrived 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` */ - timelineWasEmpty?: boolean; } export enum Direction { From d0466a1605d4ed68d49db5c4157c608a97137db0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 28 May 2022 00:40:06 -0500 Subject: [PATCH 40/51] Use Pick<> to be more composable and obvious what we're extending See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r883502814 --- src/models/event-timeline-set.ts | 6 ++++-- src/models/event-timeline.ts | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index e9d87729dbd..dcb1a0ca7c1 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -55,12 +55,14 @@ export interface IRoomTimelineData { liveEvent?: boolean; } -export interface IAddEventToTimelineOptions extends IAddEventOptions { +export interface IAddEventToTimelineOptions + extends Pick { /** Whether the sync response came from cache */ fromCache?: boolean; } -export interface IAddLiveEventOptions extends Omit { +export interface IAddLiveEventOptions + extends Pick { /** 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 diff --git a/src/models/event-timeline.ts b/src/models/event-timeline.ts index f240336ef50..64078739924 100644 --- a/src/models/event-timeline.ts +++ b/src/models/event-timeline.ts @@ -24,7 +24,7 @@ import { MatrixEvent } from "./event"; import { Filter } from "../filter"; import { EventType } from "../@types/event"; -export interface IInitialiseStateOptions extends IMarkerFoundOptions { +export interface IInitialiseStateOptions extends Pick { // This is a separate interface without any extra stuff currently added on // top of `IMarkerFoundOptions` just because it feels like they have // different concerns. One shouldn't necessarily look to add to @@ -32,7 +32,7 @@ export interface IInitialiseStateOptions extends IMarkerFoundOptions { // `initialiseState`. } -export interface IAddEventOptions extends IMarkerFoundOptions { +export interface IAddEventOptions extends Pick { /** Whether to insert the new event at the start of the timeline where the * oldest events are (timeline is in chronological order, oldest to most * recent) */ From e67b620f4c38d5c39af3999e9b5ae9bca2fcafbb Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 28 May 2022 00:47:31 -0500 Subject: [PATCH 41/51] Use optional chaining and null coalescing for more accurate and standard code Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/models/room.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index f445a8042b1..9c6dbf4aee5 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -463,11 +463,7 @@ export class Room extends TypedEventEmitter */ public getCreator(): string | null { const createEvent = this.currentState.getStateEvents(EventType.RoomCreate, ""); - if (!createEvent) { - return null; - } - const roomCreator = createEvent.getContent()['creator']; - return roomCreator; + return createEvent?.getContent()['creator'] ?? null; } /** From 2a4a4304c0f87f826e21fdb50972d9c4764319e5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 28 May 2022 00:49:17 -0500 Subject: [PATCH 42/51] Add type --- src/models/room.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/room.ts b/src/models/room.ts index 9c6dbf4aee5..be8690d3cde 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -952,7 +952,7 @@ export class Room extends TypedEventEmitter // Get the main TimelineSet const timelineSet = this.getUnfilteredTimelineSet(); - let newTimeline; + let newTimeline: EventTimeline; // If there isn't any event in the timeline, let's go fetch the latest // event and construct a timeline from it. // From e4bd5188735e614a814f19ce4fa616243b052481 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 31 May 2022 22:02:20 -0500 Subject: [PATCH 43/51] Add deprecation warnings for old overloads See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r884037961 - Move the preferred overload to the top so it's suggested first - Add `@deprecated` decorators to the deprecated overload - Add deprecation log warning when we see usage of the deprecated function --- src/models/event-timeline-set.ts | 54 +++++++++++++++++++++++--------- src/models/event-timeline.ts | 22 ++++++++++--- src/models/room.ts | 13 +++++++- 3 files changed, 68 insertions(+), 21 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index dcb1a0ca7c1..9b3273d929e 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -551,14 +551,7 @@ export class EventTimelineSet extends TypedEventEmitter * @param {IAddLiveEventOptions} options addLiveEvent options * @throws If duplicateStrategy is not falsey, 'replace' or 'ignore'. */ - public addLiveEvents(events: MatrixEvent[], duplicateStrategy?: DuplicateStrategy, fromCache?: boolean): void; public addLiveEvents(events: MatrixEvent[], addLiveEventOptions?: IAddLiveEventOptions): void; + /** + * @deprecated In favor of the overload with `IAddLiveEventOptions` + */ + public addLiveEvents(events: MatrixEvent[], duplicateStrategy?: DuplicateStrategy, fromCache?: boolean): void; public addLiveEvents( events: MatrixEvent[], duplicateStrategyOrOpts?: DuplicateStrategy | IAddLiveEventOptions, @@ -2283,6 +2286,14 @@ export class Room extends TypedEventEmitter /* roomState, (not used here) */ timelineWasEmpty, } = duplicateStrategyOrOpts); + } else if (duplicateStrategyOrOpts !== undefined) { + // Deprecation warning + // FIXME: Remove after 2023-06-01 (technical debt) + logger.warn( + 'Overload deprecated: ' + + '`Room.addLiveEvents(events, duplicateStrategy?, fromCache?)` ' + + 'is deprecated in favor of the overload with `Room.addLiveEvents(events, IAddLiveEventOptions)`', + ); } if (duplicateStrategy && ["replace", "ignore"].indexOf(duplicateStrategy) === -1) { From 310423a78125ae4593abe73f32c7b9d372a5ec64 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 31 May 2022 22:32:01 -0500 Subject: [PATCH 44/51] Better test name --- spec/integ/matrix-client-room-timeline.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index edb38175b36..95ec2dc8989 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -579,7 +579,7 @@ describe("MatrixClient room timelines", function() { }); }); - it("should emit a 'Room.timelineReset' event", function() { + it("should emit a `RoomEvent.TimelineReset` event when the sync response is `limited`", function() { const eventData = [ utils.mkMessage({ user: userId, room: roomId }), ]; From ac07eaf717473e98704357ecaba06f11fabcc6c6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 1 Jun 2022 01:25:35 -0500 Subject: [PATCH 45/51] Add refresh timeline tests to matrix-js-sdk See https://github.com/matrix-org/matrix-js-sdk/pull/2299#discussion_r885024383 --- .../integ/matrix-client-room-timeline.spec.js | 261 ++++++++++++++++++ spec/integ/matrix-client-syncing.spec.js | 6 +- 2 files changed, 264 insertions(+), 3 deletions(-) diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index 95ec2dc8989..79b0130cdec 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -608,4 +608,265 @@ describe("MatrixClient room timelines", function() { }); }); }); + + describe('Refresh live timeline', () => { + it('should clear and refresh messages in timeline', async () => { + const eventData = [ + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + ]; + setNextSyncData(eventData); + + // Create a room from the sync + await Promise.all([ + httpBackend.flushAllExpected(), + utils.syncPromise(client, 1), + ]); + + // Get the room after the first sync so the room is created + const room = client.getRoom(roomId); + expect(room).toBeTruthy(); + + // `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()` + // to construct a new timeline from. + const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + + `${encodeURIComponent(eventData[2].event_id)}`; + httpBackend.when("GET", contextUrl) + .respond(200, function() { + // The timeline should be cleared at this point in the refresh + expect(room.timeline.length).toEqual(0); + + return { + start: "start_token", + events_before: [eventData[1], eventData[0]], + event: eventData[2], + events_after: [], + state: [ + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + }); + + // Refresh the timeline. + await Promise.all([ + room.refreshLiveTimeline(), + httpBackend.flushAllExpected(), + ]); + + // Make sure the message are visible + expect(room.timeline.length).toEqual(3); + expect(room.timeline[0].event).toEqual(eventData[0]); + expect(room.timeline[1].event).toEqual(eventData[1]); + expect(room.timeline[2].event).toEqual(eventData[2]); + }); + + it.skip('Perfectly merges timelines if a sync finishes while refreshing the timeline', async () => { + const eventData = [ + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + ]; + setNextSyncData(eventData); + + // Create a room from the sync + await Promise.all([ + httpBackend.flushAllExpected(), + utils.syncPromise(client, 1), + ]); + + // Get the room after the first sync so the room is created + const room = client.getRoom(roomId); + expect(room).toBeTruthy(); + + const racingSyncEventData = [ + utils.mkMessage({ user: userId, room: roomId }), + ]; + + // `/context` request for `refreshLiveTimeline()` -> + // `getEventTimeline()` to construct a new timeline from. + // + // We only resolve this request after we detect that the timeline + // was reset(when it goes blank) and force a sync to happen in the + // middle of all of this refresh timeline logic. We want to make + // sure the sync pagination still works as expected after messing + // the refresh timline logic messes with the pagination tokens. + const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + + `${encodeURIComponent(eventData[2].event_id)}`; + httpBackend.when("GET", contextUrl) + // FIXME: This currently does not work because + // `matrix-mock-request` does not support responding + // asynchronously. + .respond(200, async () => { + // The timeline should be cleared at this point in the refresh + expect(room.timeline.length).toEqual(0); + + // Then make a `/sync` happen by sending a message and seeing that it + // shows up (simulate a /sync naturally racing with us). + setNextSyncData(racingSyncEventData); + httpBackend.when("GET", "/sync").respond(200, function() { + return NEXT_SYNC_DATA; + }); + await Promise.all([ + await httpBackend.flush("/sync", 1), + utils.syncPromise(client, 1), + ]); + // Make sure the timeline has the racey sync data + expect(room.timeline.length).toEqual(1); + expect(room.timeline[0].event).toEqual(racingSyncEventData[0]); + + // Now finally return and make the `/context` request respond + return { + start: "start_token", + events_before: [eventData[1], eventData[0]], + event: eventData[2], + events_after: [], + state: [ + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + }); + + // Refresh the timeline. + await Promise.all([ + room.refreshLiveTimeline(), + httpBackend.flushAllExpected(), + ]); + + // Make sure sync pagination still works by seeing a new message show up + // after refreshing the timeline. + const afterRefreshEventData = [ + utils.mkMessage({ user: userId, room: roomId }), + ]; + setNextSyncData(afterRefreshEventData); + httpBackend.when("GET", "/sync").respond(200, function() { + return NEXT_SYNC_DATA; + }); + await Promise.all([ + httpBackend.flushAllExpected(), + utils.syncPromise(client, 1), + ]); + + // Make sure the timeline includes the the events from the refresh + // and the `/sync` that raced us in the middle of everything. + expect(room.timeline.length).toEqual(5); + expect(room.timeline[0].event).toEqual(eventData[0]); + expect(room.timeline[1].event).toEqual(eventData[1]); + expect(room.timeline[2].event).toEqual(eventData[2]); + expect(room.timeline[3].event).toEqual(racingSyncEventData[0]); + expect(room.timeline[3].event).toEqual(afterRefreshEventData[0]); + }); + + it('Timeline recovers after `/context` request to generate new timeline fails', async () => { + const eventData = [ + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + ]; + setNextSyncData(eventData); + + // Create a room from the sync + await Promise.all([ + httpBackend.flushAllExpected(), + utils.syncPromise(client, 1), + ]); + + // Get the room after the first sync so the room is created + const room = client.getRoom(roomId); + expect(room).toBeTruthy(); + + // `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()` + // to construct a new timeline from. + const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + + `${encodeURIComponent(eventData[2].event_id)}`; + httpBackend.when("GET", contextUrl) + .respond(500, function() { + // The timeline should be cleared at this point in the refresh + expect(room.timeline.length).toEqual(0); + + return { + errcode: 'TEST_FAKE_ERROR', + error: 'We purposely intercepted this /context request to make it fail ' + + 'in order to test whether the refresh timeline code is resilient', + }; + }); + + // Refresh the timeline and expect it to fail + const settledFailedRefreshPromises = await Promise.allSettled([ + room.refreshLiveTimeline(), + httpBackend.flushAllExpected(), + ]); + // We only expect `TEST_FAKE_ERROR` here. Anything else is + // unexpected and should fail the test. + if (settledFailedRefreshPromises[0].status === 'fulfilled') { + throw new Error('Expected the /context request to fail with a 500'); + } else if (settledFailedRefreshPromises[0].reason.errcode !== 'TEST_FAKE_ERROR') { + throw settledFailedRefreshPromises[0].reason; + } + + // The timeline will be empty after we refresh the timeline and fail + // to construct a new timeline. + expect(room.timeline.length).toEqual(0); + + // `/messages` request for `refreshLiveTimeline()` -> + // `getLatestTimeline()` to construct a new timeline from. + httpBackend.when("GET", `/rooms/${encodeURIComponent(roomId)}/messages`) + .respond(200, function() { + return { + chunk: [{ + // The latest message in the room + event_id: eventData[2].event_id, + }], + }; + }); + // `/context` request for `refreshLiveTimeline()` -> + // `getLatestTimeline()` -> `getEventTimeline()` to construct a new + // timeline from. + httpBackend.when("GET", contextUrl) + .respond(200, function() { + // The timeline should be cleared at this point in the refresh + expect(room.timeline.length).toEqual(0); + + return { + start: "start_token", + events_before: [eventData[1], eventData[0]], + event: eventData[2], + events_after: [], + state: [ + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + }); + + // Refresh the timeline again but this time it should pass + await Promise.all([ + room.refreshLiveTimeline(), + httpBackend.flushAllExpected(), + ]); + + // Make sure sync pagination still works by seeing a new message show up + // after refreshing the timeline. + const afterRefreshEventData = [ + utils.mkMessage({ user: userId, room: roomId }), + ]; + setNextSyncData(afterRefreshEventData); + httpBackend.when("GET", "/sync").respond(200, function() { + return NEXT_SYNC_DATA; + }); + await Promise.all([ + httpBackend.flushAllExpected(), + utils.syncPromise(client, 1), + ]); + + // Make sure the message are visible + expect(room.timeline.length).toEqual(4); + expect(room.timeline[0].event).toEqual(eventData[0]); + expect(room.timeline[1].event).toEqual(eventData[1]); + expect(room.timeline[2].event).toEqual(eventData[2]); + expect(room.timeline[3].event).toEqual(afterRefreshEventData[0]); + }); + }); }); diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 39735420f4d..0c571707ad3 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -938,7 +938,7 @@ describe("MatrixClient syncing", function() { // Get the room after the first sync so the room is created const room = client.getRoom(roomOne); - expect(room).toBeDefined(); + expect(room).toBeTruthy(); let stateEventEmitCount = 0; client.on(RoomStateEvent.Update, () => { @@ -1012,7 +1012,7 @@ describe("MatrixClient syncing", function() { // Get the room after the first sync so the room is created const room = client.getRoom(roomOne); - expect(room).toBeDefined(); + expect(room).toBeTruthy(); let stateEventEmitCount = 0; client.on(RoomStateEvent.Update, () => { @@ -1109,7 +1109,7 @@ describe("MatrixClient syncing", function() { awaitSyncEvent(), ]).then(function() { const room = client.getRoom(roomTwo); - expect(room).toBeDefined(); + expect(room).toBeTruthy(); const tok = room.getLiveTimeline() .getPaginationToken(EventTimeline.BACKWARDS); expect(tok).toEqual("roomtwotok"); From 4915aef80174bef8be60e1382671932d28ffb617 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 1 Jun 2022 02:26:24 -0500 Subject: [PATCH 46/51] WIP: almost working perfect merge test --- .../integ/matrix-client-room-timeline.spec.js | 203 ++++++++++-------- 1 file changed, 110 insertions(+), 93 deletions(-) diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index 79b0130cdec..eff88e11da6 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -1,5 +1,6 @@ import * as utils from "../test-utils/test-utils"; import { EventStatus } from "../../src/models/event"; +import { RoomEvent } from "../../src"; import { TestClient } from "../TestClient"; describe("MatrixClient room timelines", function() { @@ -609,14 +610,16 @@ describe("MatrixClient room timelines", function() { }); }); - describe('Refresh live timeline', () => { - it('should clear and refresh messages in timeline', async () => { - const eventData = [ - utils.mkMessage({ user: userId, room: roomId }), - utils.mkMessage({ user: userId, room: roomId }), - utils.mkMessage({ user: userId, room: roomId }), - ]; - setNextSyncData(eventData); + describe.only('Refresh live timeline', () => { + const initialSyncEventData = [ + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + utils.mkMessage({ user: userId, room: roomId }), + ]; + + let room; + beforeEach(async () => { + setNextSyncData(initialSyncEventData); // Create a room from the sync await Promise.all([ @@ -625,13 +628,15 @@ describe("MatrixClient room timelines", function() { ]); // Get the room after the first sync so the room is created - const room = client.getRoom(roomId); + room = client.getRoom(roomId); expect(room).toBeTruthy(); + }); + it('should clear and refresh messages in timeline', async () => { // `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()` // to construct a new timeline from. const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + - `${encodeURIComponent(eventData[2].event_id)}`; + `${encodeURIComponent(initialSyncEventData[2].event_id)}`; httpBackend.when("GET", contextUrl) .respond(200, function() { // The timeline should be cleared at this point in the refresh @@ -639,8 +644,8 @@ describe("MatrixClient room timelines", function() { return { start: "start_token", - events_before: [eventData[1], eventData[0]], - event: eventData[2], + events_before: [initialSyncEventData[1], initialSyncEventData[0]], + event: initialSyncEventData[2], events_after: [], state: [ USER_MEMBERSHIP_EVENT, @@ -657,33 +662,12 @@ describe("MatrixClient room timelines", function() { // Make sure the message are visible expect(room.timeline.length).toEqual(3); - expect(room.timeline[0].event).toEqual(eventData[0]); - expect(room.timeline[1].event).toEqual(eventData[1]); - expect(room.timeline[2].event).toEqual(eventData[2]); + expect(room.timeline[0].event).toEqual(initialSyncEventData[0]); + expect(room.timeline[1].event).toEqual(initialSyncEventData[1]); + expect(room.timeline[2].event).toEqual(initialSyncEventData[2]); }); - it.skip('Perfectly merges timelines if a sync finishes while refreshing the timeline', async () => { - const eventData = [ - utils.mkMessage({ user: userId, room: roomId }), - utils.mkMessage({ user: userId, room: roomId }), - utils.mkMessage({ user: userId, room: roomId }), - ]; - setNextSyncData(eventData); - - // Create a room from the sync - await Promise.all([ - httpBackend.flushAllExpected(), - utils.syncPromise(client, 1), - ]); - - // Get the room after the first sync so the room is created - const room = client.getRoom(roomId); - expect(room).toBeTruthy(); - - const racingSyncEventData = [ - utils.mkMessage({ user: userId, room: roomId }), - ]; - + it.only('Perfectly merges timelines if a sync finishes while refreshing the timeline', async () => { // `/context` request for `refreshLiveTimeline()` -> // `getEventTimeline()` to construct a new timeline from. // @@ -693,34 +677,14 @@ describe("MatrixClient room timelines", function() { // sure the sync pagination still works as expected after messing // the refresh timline logic messes with the pagination tokens. const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + - `${encodeURIComponent(eventData[2].event_id)}`; + `${encodeURIComponent(initialSyncEventData[2].event_id)}`; httpBackend.when("GET", contextUrl) - // FIXME: This currently does not work because - // `matrix-mock-request` does not support responding - // asynchronously. - .respond(200, async () => { - // The timeline should be cleared at this point in the refresh - expect(room.timeline.length).toEqual(0); - - // Then make a `/sync` happen by sending a message and seeing that it - // shows up (simulate a /sync naturally racing with us). - setNextSyncData(racingSyncEventData); - httpBackend.when("GET", "/sync").respond(200, function() { - return NEXT_SYNC_DATA; - }); - await Promise.all([ - await httpBackend.flush("/sync", 1), - utils.syncPromise(client, 1), - ]); - // Make sure the timeline has the racey sync data - expect(room.timeline.length).toEqual(1); - expect(room.timeline[0].event).toEqual(racingSyncEventData[0]); - + .respond(200, () => { // Now finally return and make the `/context` request respond return { start: "start_token", - events_before: [eventData[1], eventData[0]], - event: eventData[2], + events_before: [initialSyncEventData[1], initialSyncEventData[0]], + event: initialSyncEventData[2], events_after: [], state: [ USER_MEMBERSHIP_EVENT, @@ -729,9 +693,76 @@ describe("MatrixClient room timelines", function() { }; }); - // Refresh the timeline. + // Wait for the timeline to reset(when it goes blank) which means + // it's in the middle of the refrsh logic right before the + // `getEventTimeline()` -> `/context`. Then simulate a racey `/sync` + // to happen in the middle of all of this refresh timeline logic. We + // want to make sure the sync pagination still works as expected + // after messing the refresh timline logic messes with the + // pagination tokens. + // + // We define this here so the event listener is in place before we + // call `room.refreshLiveTimeline()`. + const racingSyncEventData = [ + utils.mkMessage({ user: userId, room: roomId }), + ]; + const waitForRaceySyncAfterResetPromise = new Promise((resolve, reject) => { + let eventFired = false; + // Throw a more descriptive error if this part of the test times out. + const failTimeout = setTimeout(() => { + if (eventFired) { + reject(new Error( + 'TestError: `RoomEvent.TimelineReset` fired but we timed out trying to make' + + 'a `/sync` happen in time.' + )); + } else { + reject(new Error( + 'TestError: Timed out while waiting for `RoomEvent.TimelineReset` to fire.' + )); + } + }, 4000 /* FIXME: Is there a way to reference the current timeout of this test in Jest? */); + + room.on(RoomEvent.TimelineReset, async () => { + try { + eventFired = true; + + // The timeline should be cleared at this point in the refresh + expect(room.getUnfilteredTimelineSet().getLiveTimeline().getEvents().length).toEqual(0); + + // Then make a `/sync` happen by sending a message and seeing that it + // shows up (simulate a /sync naturally racing with us). + setNextSyncData(racingSyncEventData); + httpBackend.when("GET", "/sync").respond(200, function() { + return NEXT_SYNC_DATA; + }); + await Promise.all([ + httpBackend.flush("/sync", 1), + utils.syncPromise(client, 1), + ]); + // Make sure the timeline has the racey sync data + const afterRaceySyncTimelineEvents = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents(); + const afterRaceySyncTimelineEventIds = afterRaceySyncTimelineEvents.map((event) => event.getId()); + expect(afterRaceySyncTimelineEventIds).toEqual([ + racingSyncEventData[0].event_id, + ]); + + clearTimeout(failTimeout); + resolve(); + } catch(err) { + reject(err); + } + }); + }); + + // Refresh the timeline. Just start the function, we will wait for + // it to finish after the racey sync. + const refreshLiveTimelinePromise = room.refreshLiveTimeline(); + + await waitForRaceySyncAfterResetPromise; + await Promise.all([ - room.refreshLiveTimeline(), + refreshLiveTimelinePromise, + // Then flush the remaining `/context` to left the refresh logic complete httpBackend.flushAllExpected(), ]); @@ -751,36 +782,22 @@ describe("MatrixClient room timelines", function() { // Make sure the timeline includes the the events from the refresh // and the `/sync` that raced us in the middle of everything. - expect(room.timeline.length).toEqual(5); - expect(room.timeline[0].event).toEqual(eventData[0]); - expect(room.timeline[1].event).toEqual(eventData[1]); - expect(room.timeline[2].event).toEqual(eventData[2]); - expect(room.timeline[3].event).toEqual(racingSyncEventData[0]); - expect(room.timeline[3].event).toEqual(afterRefreshEventData[0]); + const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents(); + const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId()); + expect(resultantEventIdsInTimeline).toEqual([ + initialSyncEventData[0].event_id, + initialSyncEventData[1].event_id, + initialSyncEventData[2].event_id, + racingSyncEventData[0].event_id, + afterRefreshEventData[0].event_id, + ]); }); it('Timeline recovers after `/context` request to generate new timeline fails', async () => { - const eventData = [ - utils.mkMessage({ user: userId, room: roomId }), - utils.mkMessage({ user: userId, room: roomId }), - utils.mkMessage({ user: userId, room: roomId }), - ]; - setNextSyncData(eventData); - - // Create a room from the sync - await Promise.all([ - httpBackend.flushAllExpected(), - utils.syncPromise(client, 1), - ]); - - // Get the room after the first sync so the room is created - const room = client.getRoom(roomId); - expect(room).toBeTruthy(); - // `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()` // to construct a new timeline from. const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + - `${encodeURIComponent(eventData[2].event_id)}`; + `${encodeURIComponent(initialSyncEventData[2].event_id)}`; httpBackend.when("GET", contextUrl) .respond(500, function() { // The timeline should be cleared at this point in the refresh @@ -817,7 +834,7 @@ describe("MatrixClient room timelines", function() { return { chunk: [{ // The latest message in the room - event_id: eventData[2].event_id, + event_id: initialSyncEventData[2].event_id, }], }; }); @@ -831,8 +848,8 @@ describe("MatrixClient room timelines", function() { return { start: "start_token", - events_before: [eventData[1], eventData[0]], - event: eventData[2], + events_before: [initialSyncEventData[1], initialSyncEventData[0]], + event: initialSyncEventData[2], events_after: [], state: [ USER_MEMBERSHIP_EVENT, @@ -863,9 +880,9 @@ describe("MatrixClient room timelines", function() { // Make sure the message are visible expect(room.timeline.length).toEqual(4); - expect(room.timeline[0].event).toEqual(eventData[0]); - expect(room.timeline[1].event).toEqual(eventData[1]); - expect(room.timeline[2].event).toEqual(eventData[2]); + expect(room.timeline[0].event).toEqual(initialSyncEventData[0]); + expect(room.timeline[1].event).toEqual(initialSyncEventData[1]); + expect(room.timeline[2].event).toEqual(initialSyncEventData[2]); expect(room.timeline[3].event).toEqual(afterRefreshEventData[0]); }); }); From 460e887d582fbc0a079bc250763e6a10f98d9d83 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 1 Jun 2022 03:37:20 -0500 Subject: [PATCH 47/51] Working tests --- .../integ/matrix-client-room-timeline.spec.js | 88 ++++++++----------- src/models/room.ts | 43 ++++++--- 2 files changed, 67 insertions(+), 64 deletions(-) diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index eff88e11da6..4a97e87ddc1 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -617,6 +617,19 @@ describe("MatrixClient room timelines", function() { utils.mkMessage({ user: userId, room: roomId }), ]; + const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + + `${encodeURIComponent(initialSyncEventData[2].event_id)}`; + const contextResponse = { + start: "start_token", + events_before: [initialSyncEventData[1], initialSyncEventData[0]], + event: initialSyncEventData[2], + events_after: [], + state: [ + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + let room; beforeEach(async () => { setNextSyncData(initialSyncEventData); @@ -635,23 +648,12 @@ describe("MatrixClient room timelines", function() { it('should clear and refresh messages in timeline', async () => { // `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()` // to construct a new timeline from. - const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + - `${encodeURIComponent(initialSyncEventData[2].event_id)}`; httpBackend.when("GET", contextUrl) .respond(200, function() { // The timeline should be cleared at this point in the refresh expect(room.timeline.length).toEqual(0); - return { - start: "start_token", - events_before: [initialSyncEventData[1], initialSyncEventData[0]], - event: initialSyncEventData[2], - events_after: [], - state: [ - USER_MEMBERSHIP_EVENT, - ], - end: "end_token", - }; + return contextResponse; }); // Refresh the timeline. @@ -661,13 +663,16 @@ describe("MatrixClient room timelines", function() { ]); // Make sure the message are visible - expect(room.timeline.length).toEqual(3); - expect(room.timeline[0].event).toEqual(initialSyncEventData[0]); - expect(room.timeline[1].event).toEqual(initialSyncEventData[1]); - expect(room.timeline[2].event).toEqual(initialSyncEventData[2]); + const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents(); + const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId()); + expect(resultantEventIdsInTimeline).toEqual([ + initialSyncEventData[0].event_id, + initialSyncEventData[1].event_id, + initialSyncEventData[2].event_id, + ]); }); - it.only('Perfectly merges timelines if a sync finishes while refreshing the timeline', async () => { + it('Perfectly merges timelines if a sync finishes while refreshing the timeline', async () => { // `/context` request for `refreshLiveTimeline()` -> // `getEventTimeline()` to construct a new timeline from. // @@ -676,21 +681,10 @@ describe("MatrixClient room timelines", function() { // middle of all of this refresh timeline logic. We want to make // sure the sync pagination still works as expected after messing // the refresh timline logic messes with the pagination tokens. - const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + - `${encodeURIComponent(initialSyncEventData[2].event_id)}`; httpBackend.when("GET", contextUrl) .respond(200, () => { // Now finally return and make the `/context` request respond - return { - start: "start_token", - events_before: [initialSyncEventData[1], initialSyncEventData[0]], - event: initialSyncEventData[2], - events_after: [], - state: [ - USER_MEMBERSHIP_EVENT, - ], - end: "end_token", - }; + return contextResponse; }); // Wait for the timeline to reset(when it goes blank) which means @@ -780,14 +774,14 @@ describe("MatrixClient room timelines", function() { utils.syncPromise(client, 1), ]); - // Make sure the timeline includes the the events from the refresh - // and the `/sync` that raced us in the middle of everything. + // Make sure the timeline includes the the events from the `/sync` + // that raced and beat us in the middle of everything and the + // `/sync` after the refresh. Since the `/sync` beat us to create + // the timeline, `initialSyncEventData` won't be visible unless we + // paginate backwards with `/messages`. const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents(); const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId()); expect(resultantEventIdsInTimeline).toEqual([ - initialSyncEventData[0].event_id, - initialSyncEventData[1].event_id, - initialSyncEventData[2].event_id, racingSyncEventData[0].event_id, afterRefreshEventData[0].event_id, ]); @@ -796,8 +790,6 @@ describe("MatrixClient room timelines", function() { it('Timeline recovers after `/context` request to generate new timeline fails', async () => { // `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()` // to construct a new timeline from. - const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` + - `${encodeURIComponent(initialSyncEventData[2].event_id)}`; httpBackend.when("GET", contextUrl) .respond(500, function() { // The timeline should be cleared at this point in the refresh @@ -846,16 +838,7 @@ describe("MatrixClient room timelines", function() { // The timeline should be cleared at this point in the refresh expect(room.timeline.length).toEqual(0); - return { - start: "start_token", - events_before: [initialSyncEventData[1], initialSyncEventData[0]], - event: initialSyncEventData[2], - events_after: [], - state: [ - USER_MEMBERSHIP_EVENT, - ], - end: "end_token", - }; + return contextResponse; }); // Refresh the timeline again but this time it should pass @@ -879,11 +862,14 @@ describe("MatrixClient room timelines", function() { ]); // Make sure the message are visible - expect(room.timeline.length).toEqual(4); - expect(room.timeline[0].event).toEqual(initialSyncEventData[0]); - expect(room.timeline[1].event).toEqual(initialSyncEventData[1]); - expect(room.timeline[2].event).toEqual(initialSyncEventData[2]); - expect(room.timeline[3].event).toEqual(afterRefreshEventData[0]); + const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents(); + const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId()); + expect(resultantEventIdsInTimeline).toEqual([ + initialSyncEventData[0].event_id, + initialSyncEventData[1].event_id, + initialSyncEventData[2].event_id, + afterRefreshEventData[0].event_id, + ]); }); }); }); diff --git a/src/models/room.ts b/src/models/room.ts index 4f69a7714a5..78d8e6dde37 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -942,7 +942,7 @@ export class Room extends TypedEventEmitter const eventsBefore = liveTimelineBefore.getEvents(); const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; logger.log( - `[refreshLiveTimeline] for room ${this.roomId} at ` + + `[refreshLiveTimeline for ${this.roomId}] at ` + `mostRecentEventInTimeline=${mostRecentEventInTimeline && mostRecentEventInTimeline.getId()} ` + `liveTimelineBefore=${liveTimelineBefore.toString()} ` + `forwardPaginationToken=${forwardPaginationToken} ` + @@ -986,18 +986,35 @@ export class Room extends TypedEventEmitter newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()); } - // Set the pagination token back to the live sync token (`null`) instead - // of using the `/context` historical token (ex. `t12-13_0_0_0_0_0_0_0_0`) - // so that it matches the next response from `/sync` and we can properly - // continue the timeline. - newTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); - - // Set our new fresh timeline as the live timeline to continue syncing - // forwards and back paginating from. - timelineSet.setLiveTimeline(newTimeline); - // Fixup `this.oldstate` so that `scrollback` has the pagination tokens - // available - this.fixUpLegacyTimelineFields(); + // If a racing `/sync` beat us to creating a new timeline, use that + // instead because it's the latest in the room and any new messages in + // the scrollback will include the history. + const liveTimeline = timelineSet.getLiveTimeline(); + if (!liveTimeline || ( + liveTimeline.getPaginationToken(Direction.Forward) === null && + liveTimeline.getPaginationToken(Direction.Backward) === null && + liveTimeline.getEvents().length === 0 + )) { + logger.log(`[refreshLiveTimeline for ${this.roomId}] using our new live timeline`); + // Set the pagination token back to the live sync token (`null`) instead + // of using the `/context` historical token (ex. `t12-13_0_0_0_0_0_0_0_0`) + // so that it matches the next response from `/sync` and we can properly + // continue the timeline. + newTimeline.setPaginationToken(forwardPaginationToken, EventTimeline.FORWARDS); + + // Set our new fresh timeline as the live timeline to continue syncing + // forwards and back paginating from. + timelineSet.setLiveTimeline(newTimeline); + // Fixup `this.oldstate` so that `scrollback` has the pagination tokens + // available + this.fixUpLegacyTimelineFields(); + } else { + logger.log( + `[refreshLiveTimeline for ${this.roomId}] \`/sync\` or some other request beat us to creating a new ` + + `live timeline after we reset it. We'll use that instead since any events in the scrollback from this ` + + `timeline will include the history.`, + ); + } // The timeline has now been refreshed ✅ this.setTimelineNeedsRefresh(false); From 741a4fa2f2542de91c8554dccfab7000ab362a85 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 1 Jun 2022 03:41:01 -0500 Subject: [PATCH 48/51] Fix lints --- spec/integ/matrix-client-room-timeline.spec.js | 16 ++++++++++------ src/models/room.ts | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index 4a97e87ddc1..acf751a8c09 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -610,7 +610,7 @@ describe("MatrixClient room timelines", function() { }); }); - describe.only('Refresh live timeline', () => { + describe('Refresh live timeline', () => { const initialSyncEventData = [ utils.mkMessage({ user: userId, room: roomId }), utils.mkMessage({ user: userId, room: roomId }), @@ -707,11 +707,11 @@ describe("MatrixClient room timelines", function() { if (eventFired) { reject(new Error( 'TestError: `RoomEvent.TimelineReset` fired but we timed out trying to make' + - 'a `/sync` happen in time.' + 'a `/sync` happen in time.', )); } else { reject(new Error( - 'TestError: Timed out while waiting for `RoomEvent.TimelineReset` to fire.' + 'TestError: Timed out while waiting for `RoomEvent.TimelineReset` to fire.', )); } }, 4000 /* FIXME: Is there a way to reference the current timeout of this test in Jest? */); @@ -734,15 +734,19 @@ describe("MatrixClient room timelines", function() { utils.syncPromise(client, 1), ]); // Make sure the timeline has the racey sync data - const afterRaceySyncTimelineEvents = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents(); - const afterRaceySyncTimelineEventIds = afterRaceySyncTimelineEvents.map((event) => event.getId()); + const afterRaceySyncTimelineEvents = room + .getUnfilteredTimelineSet() + .getLiveTimeline() + .getEvents(); + const afterRaceySyncTimelineEventIds = afterRaceySyncTimelineEvents + .map((event) => event.getId()); expect(afterRaceySyncTimelineEventIds).toEqual([ racingSyncEventData[0].event_id, ]); clearTimeout(failTimeout); resolve(); - } catch(err) { + } catch (err) { reject(err); } }); diff --git a/src/models/room.ts b/src/models/room.ts index 78d8e6dde37..eeb2325f536 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -1011,8 +1011,8 @@ export class Room extends TypedEventEmitter } else { logger.log( `[refreshLiveTimeline for ${this.roomId}] \`/sync\` or some other request beat us to creating a new ` + - `live timeline after we reset it. We'll use that instead since any events in the scrollback from this ` + - `timeline will include the history.`, + `live timeline after we reset it. We'll use that instead since any events in the scrollback from ` + + `this timeline will include the history.`, ); } From 2bda3d3e0aeef2205ad26001ea7372c2403e0140 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 1 Jun 2022 16:14:32 -0500 Subject: [PATCH 49/51] Accurate return description Previously copy-paste from `client.getEventTimeline()` --- src/client.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/client.ts b/src/client.ts index b4167b04b6d..44977cfd69c 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5387,8 +5387,7 @@ export class MatrixClient extends TypedEventEmitter { // don't allow any timeline support unless it's been enabled. From 150a49852802b46e6cf2bfdf0c4120ed7b93dc1d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 1 Jun 2022 16:15:16 -0500 Subject: [PATCH 50/51] Correct function parameters Previously copy-paste from `client.getEventTimeline()` --- src/client.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/client.ts b/src/client.ts index 44977cfd69c..e0f40a8a2ba 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5382,9 +5382,7 @@ export class MatrixClient extends TypedEventEmitter Date: Wed, 1 Jun 2022 16:16:15 -0500 Subject: [PATCH 51/51] Small formatting updates --- spec/integ/matrix-client-event-timeline.spec.js | 2 +- spec/unit/room-state.spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integ/matrix-client-event-timeline.spec.js b/spec/integ/matrix-client-event-timeline.spec.js index 52a029343b6..454a4d30323 100644 --- a/spec/integ/matrix-client-event-timeline.spec.js +++ b/spec/integ/matrix-client-event-timeline.spec.js @@ -575,7 +575,7 @@ describe("MatrixClient event timelines", function() { client.getLatestTimeline(timelineSet).then(function(tl) { // Instead of this assertion logic, we could just add a spy // for `getEventTimeline` and make sure it's called with the - // correct parameters. This doesn't feel to bad to make sure + // correct parameters. This doesn't feel too bad to make sure // `getLatestTimeline` is doing the right thing though. expect(tl.getEvents().length).toEqual(4); for (let i = 0; i < 4; i++) { diff --git a/spec/unit/room-state.spec.js b/spec/unit/room-state.spec.js index 7aae3d12240..b54121431bb 100644 --- a/spec/unit/room-state.spec.js +++ b/spec/unit/room-state.spec.js @@ -258,7 +258,7 @@ describe("RoomState", function() { ); }); - it("should emit 'RoomStateEvent.Marker' for each marker event", function() { + it("should emit `RoomStateEvent.Marker` for each marker event", function() { const events = [ utils.mkEvent({ event: true,