From 2451e10f5c52a09cf83286f1d29d223cdd431d40 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 4 Nov 2022 05:14:17 -0500 Subject: [PATCH] Add `getLatestLiveTimeline` A v2 of https://github.com/matrix-org/matrix-js-sdk/pull/2521 --- .../matrix-client-event-timeline.spec.ts | 83 +++++++++-- .../integ/matrix-client-room-timeline.spec.ts | 4 +- src/client.ts | 129 ++++++++++++------ src/models/room.ts | 30 +++- 4 files changed, 188 insertions(+), 58 deletions(-) diff --git a/spec/integ/matrix-client-event-timeline.spec.ts b/spec/integ/matrix-client-event-timeline.spec.ts index 4e732184749..15ce6e5316c 100644 --- a/spec/integ/matrix-client-event-timeline.spec.ts +++ b/spec/integ/matrix-client-event-timeline.spec.ts @@ -780,7 +780,12 @@ describe("MatrixClient event timelines", function() { }); }); - describe("getLatestTimeline", function() { + describe("getLatestLiveTimeline", function() { + beforeEach(() => { + // @ts-ignore + client.clientOpts.experimentalThreadSupport = true; + }); + it("timeline support must be enabled to work", async function() { await client.stopClient(); @@ -797,7 +802,7 @@ describe("MatrixClient event timelines", function() { const room = client.getRoom(roomId)!; const timelineSet = room.getTimelineSets()[0]!; - await expect(client.getLatestTimeline(timelineSet)).rejects.toBeTruthy(); + await expect(client.getLatestLiveTimeline(timelineSet)).rejects.toBeTruthy(); }); it("timeline support works when enabled", async function() { @@ -816,7 +821,7 @@ describe("MatrixClient event timelines", function() { return startClient(httpBackend, client).then(() => { const room = client.getRoom(roomId)!; const timelineSet = room.getTimelineSets()[0]; - expect(client.getLatestTimeline(timelineSet)).rejects.toBeFalsy(); + expect(client.getLatestLiveTimeline(timelineSet)).rejects.toBeFalsy(); }); }); @@ -835,14 +840,14 @@ describe("MatrixClient event timelines", function() { await startClient(httpBackend, client); const timelineSet = new EventTimelineSet(undefined); - await expect(client.getLatestTimeline(timelineSet)).rejects.toBeTruthy(); + await expect(client.getLatestLiveTimeline(timelineSet)).rejects.toBeTruthy(); }); it("should create a new timeline for new events", function() { const room = client.getRoom(roomId)!; const timelineSet = room.getTimelineSets()[0]; - const latestMessageId = 'event1:bar'; + const latestMessageId = EVENTS[2].event_id!; httpBackend.when("GET", "/rooms/!foo%3Abar/messages") .respond(200, function() { @@ -869,11 +874,11 @@ describe("MatrixClient event timelines", function() { }); return Promise.all([ - client.getLatestTimeline(timelineSet).then(function(tl) { + client.getLatestLiveTimeline(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 too bad to make sure - // `getLatestTimeline` is doing the right thing though. + // `getLatestLiveTimeline` 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]); @@ -888,6 +893,64 @@ describe("MatrixClient event timelines", function() { ]); }); + it("should successfully create a new timeline even when the latest event is a threaded reply", function() { + const room = client.getRoom(roomId); + const timelineSet = room!.getTimelineSets()[0]; + expect(timelineSet.thread).toBeUndefined(); + + const latestMessageId = THREAD_REPLY.event_id; + + 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: [THREAD_ROOT, EVENTS[0]], + event: THREAD_REPLY, + events_after: [], + state: [ + ROOM_NAME_EVENT, + USER_MEMBERSHIP_EVENT, + ], + end: "end_token", + }; + }); + + // Make it easy to debug when there is a mismatch of events. We care + // about the event ID for direct comparison and the content for a + // human readable description. + const eventPropertiesToCompare = (event) => { + return { + eventId: event.event_id || event.getId(), + contentBody: event.content?.body || event.getContent()?.body, + }; + }; + return Promise.all([ + client.getLatestLiveTimeline(timelineSet).then(function(tl) { + const events = tl!.getEvents(); + const expectedEvents = [EVENTS[0], THREAD_ROOT]; + expect(events.map(event => eventPropertiesToCompare(event))) + .toEqual(expectedEvents.map(event => eventPropertiesToCompare(event))); + // Sanity check: The threaded reply should not be in the timeline + expect(events.find(e => e.getId() === THREAD_REPLY.event_id)).toBeFalsy(); + + 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]; @@ -902,7 +965,7 @@ describe("MatrixClient event timelines", function() { }); return Promise.all([ - expect(client.getLatestTimeline(timelineSet)).rejects.toThrow(), + expect(client.getLatestLiveTimeline(timelineSet)).rejects.toThrow(), httpBackend.flushAllExpected(), ]); }); @@ -1122,7 +1185,7 @@ describe("MatrixClient event timelines", function() { respondToContext(); await flushHttp(client.getEventTimeline(timelineSet, THREAD_ROOT.event_id!)); respondToThreads(); - const timeline = await flushHttp(client.getLatestTimeline(timelineSet)); + const timeline = await flushHttp(client.getLatestLiveTimeline(timelineSet)); expect(timeline).not.toBeNull(); respondToThreads(); @@ -1178,7 +1241,7 @@ describe("MatrixClient event timelines", function() { await flushHttp(client.getEventTimeline(timelineSet, THREAD_ROOT.event_id!)); respondToMessagesRequest(); - const timeline = await flushHttp(client.getLatestTimeline(timelineSet)); + const timeline = await flushHttp(client.getLatestLiveTimeline(timelineSet)); expect(timeline).not.toBeNull(); respondToMessagesRequest(); diff --git a/spec/integ/matrix-client-room-timeline.spec.ts b/spec/integ/matrix-client-room-timeline.spec.ts index f89ab04e2b8..0fd88c0f447 100644 --- a/spec/integ/matrix-client-room-timeline.spec.ts +++ b/spec/integ/matrix-client-room-timeline.spec.ts @@ -829,7 +829,7 @@ describe("MatrixClient room timelines", function() { expect(room.timeline.length).toEqual(0); // `/messages` request for `refreshLiveTimeline()` -> - // `getLatestTimeline()` to construct a new timeline from. + // `getLatestLiveTimeline()` to construct a new timeline from. httpBackend!.when("GET", `/rooms/${encodeURIComponent(roomId)}/messages`) .respond(200, function() { return { @@ -840,7 +840,7 @@ describe("MatrixClient room timelines", function() { }; }); // `/context` request for `refreshLiveTimeline()` -> - // `getLatestTimeline()` -> `getEventTimeline()` to construct a new + // `getLatestLiveTimeline()` -> `getEventTimeline()` to construct a new // timeline from. httpBackend!.when("GET", contextUrl) .respond(200, function() { diff --git a/src/client.ts b/src/client.ts index 341b2cbecd7..feb8fc5d78c 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5400,13 +5400,14 @@ export class MatrixClient extends TypedEventEmitter> { + public async getLatestLiveTimeline(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'" + @@ -5414,51 +5415,101 @@ export class MatrixClient extends TypedEventEmitter = { - dir: 'b', - }; - if (this.clientOpts?.lazyLoadMembers) { - params.filter = JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER); - } + const messagesPath = utils.encodeUri( + "/rooms/$roomId/messages", { + $roomId: timelineSet.room.roomId, + }, + ); + const messageRequestParams: Record = { + dir: 'b', + // Since we only use the latest message in the response, we only need to + // fetch the one message here. + limit: "1", + }; + if (this.clientOpts?.lazyLoadMembers) { + messageRequestParams.filter = JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER); + } + const messagesRes = await this.http.authedRequest( + Method.Get, + messagesPath, + messageRequestParams, + ); + const latestEventInTimeline = messagesRes.chunk?.[0]; + const latestEventIdInTimeline = latestEventInTimeline?.event_id; + if (!latestEventIdInTimeline) { + throw new Error("No message returned when trying to construct getLatestLiveTimeline"); + } - const res = await this.http.authedRequest(Method.Get, messagesPath, params); - event = res.chunk?.[0]; + const contextPath = utils.encodeUri( + "/rooms/$roomId/context/$eventId", { + $roomId: timelineSet.room.roomId, + $eventId: latestEventIdInTimeline, + }, + ); + let contextRequestParams: Record | undefined = undefined; + if (this.clientOpts?.lazyLoadMembers) { + contextRequestParams = { filter: JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER) }; } - if (!event) { - throw new Error("No message returned when trying to construct getLatestTimeline"); + const contextRes = await this.http.authedRequest( + Method.Get, + contextPath, + contextRequestParams, + ); + if (!contextRes.event || contextRes.event.event_id !== latestEventIdInTimeline) { + throw new Error( + `getLatestLiveTimeline: \`/context\` response did not include latestEventIdInTimeline=` + + `${latestEventIdInTimeline} which we were asking about. This is probably a bug in the ` + + `homeserver since we just saw the event with the other request above and now the server ` + + `claims it does not exist.`, + ); + } + + // By the time the request completes, the event might have ended up in the timeline. + const shortcutTimelineForEvent = timelineSet.getTimelineForEvent(latestEventIdInTimeline); + if (shortcutTimelineForEvent) { + return shortcutTimelineForEvent; + } + + const mapper = this.getEventMapper(); + const latestMatrixEventInTimeline = mapper(contextRes.event); + const events = [ + // Order events from most recent to oldest (reverse-chronological). + // We start with the last event, since that's the point at which we have known state. + // events_after is already backwards; events_before is forwards. + ...contextRes.events_after.reverse().map(mapper), + latestMatrixEventInTimeline, + ...contextRes.events_before.map(mapper), + ]; + + // This function handles non-thread timelines only, but we still process any + // thread events to populate thread summaries. + let timeline = timelineSet.getTimelineForEvent(events[0].getId()!); + if (timeline) { + timeline.getState(EventTimeline.BACKWARDS)!.setUnknownStateEvents(contextRes.state.map(mapper)); + } else { + // If the `latestEventIdInTimeline` does not belong to this `timelineSet` + // then it will be ignored and not added to the `timelineSet`. We'll instead + // just create a new blank timeline in the `timelineSet` with the proper + // pagination tokens setup to continue paginating. + timeline = timelineSet.addTimeline(); + timeline.initialiseState(contextRes.state.map(mapper)); + timeline.getState(EventTimeline.FORWARDS)!.paginationToken = contextRes.end; } - return this.getEventTimeline(timelineSet, event.event_id); + const [timelineEvents, threadedEvents] = timelineSet.room.partitionThreadedEvents(events); + timelineSet.addEventsToTimeline(timelineEvents, true, timeline, contextRes.start); + // The target event is not in a thread but process the contextual events, so we can show any threads around it. + this.processThreadEvents(timelineSet.room, threadedEvents, true); + this.processBeaconEvents(timelineSet.room, timelineEvents); + + return timelineSet.getTimelineForEvent(latestEventIdInTimeline) ?? timeline; } /** diff --git a/src/models/room.ts b/src/models/room.ts index 3bfa308c5c0..2216342aa0a 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -913,9 +913,10 @@ export class Room extends ReadReceipt { const backwardPaginationToken = liveTimelineBefore.getPaginationToken(EventTimeline.BACKWARDS); const eventsBefore = liveTimelineBefore.getEvents(); const mostRecentEventInTimeline = eventsBefore[eventsBefore.length - 1]; + const mostRecentEventIdInTimeline = mostRecentEventInTimeline.getId(); logger.log( `[refreshLiveTimeline for ${this.roomId}] at ` + - `mostRecentEventInTimeline=${mostRecentEventInTimeline && mostRecentEventInTimeline.getId()} ` + + `mostRecentEventIdInTimeline=${mostRecentEventIdInTimeline} ` + `liveTimelineBefore=${liveTimelineBefore.toString()} ` + `forwardPaginationToken=${forwardPaginationToken} ` + `backwardPaginationToken=${backwardPaginationToken}`, @@ -924,15 +925,15 @@ export class Room extends ReadReceipt { // Get the main TimelineSet const timelineSet = this.getUnfilteredTimelineSet(); - let newTimeline: Optional; + 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. // // 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); + if (!mostRecentEventIdInTimeline) { + newTimeline = await this.client.getLatestLiveTimeline(timelineSet); } else { // Empty out all of `this.timelineSets`. But we also need to keep the // same `timelineSet` references around so the React code updates @@ -955,7 +956,22 @@ export class Room extends ReadReceipt { // 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. - newTimeline = await this.client.getEventTimeline(timelineSet, mostRecentEventInTimeline.getId()!); + const timelineForMostRecentEvent = await this.client.getEventTimeline( + timelineSet, + mostRecentEventIdInTimeline, + ); + + if (!timelineForMostRecentEvent) { + throw new Error( + `refreshLiveTimeline: No new timeline was returned by \`getEventTimeline(...)\`. ` + + `This probably means that mostRecentEventIdInTimeline=${mostRecentEventIdInTimeline}` + + `which was in the live timeline before, wasn't supposed to be there in the first place ` + + `(maybe a problem with threads leaking into the main live timeline). ` + + `This is a problem with Element, please report this error.`, + ); + } + + newTimeline = timelineForMostRecentEvent; } // If a racing `/sync` beat us to creating a new timeline, use that @@ -972,11 +988,11 @@ export class Room extends ReadReceipt { // 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); + 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.setLiveTimeline(newTimeline); // Fixup `this.oldstate` so that `scrollback` has the pagination tokens // available this.fixUpLegacyTimelineFields();