Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix refreshLiveTimeline(...) not working when the latest event in the room is a threaded message #2852

191 changes: 191 additions & 0 deletions spec/integ/matrix-client-event-timeline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,197 @@ describe("MatrixClient event timelines", function() {
});
});

describe("fetchLatestLiveTimeline", function() {
beforeEach(() => {
// @ts-ignore
client.clientOpts.experimentalThreadSupport = true;
});

it("timeline support must be enabled to work", async function() {
await client.stopClient();

const testClient = new TestClient(
userId,
"DEVICE",
accessToken,
undefined,
{ timelineSupport: false },
);
client = testClient.client;
httpBackend = testClient.httpBackend;
await startClient(httpBackend, client);

const room = client.getRoom(roomId)!;
const timelineSet = room.getTimelineSets()[0]!;
await expect(client.fetchLatestLiveTimeline(timelineSet)).rejects.toBeTruthy();
Comment on lines +920 to +935
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a little room for deduplicating code in these first 3 tests

});

it("timeline support works when enabled", async function() {
await client.stopClient();

const testClient = new TestClient(
userId,
"DEVICE",
accessToken,
undefined,
{ timelineSupport: true },
);
client = testClient.client;
httpBackend = testClient.httpBackend;

return startClient(httpBackend, client).then(() => {
const room = client.getRoom(roomId)!;
const timelineSet = room.getTimelineSets()[0];
expect(client.fetchLatestLiveTimeline(timelineSet)).rejects.toBeFalsy();
});
});

it("only works with room timelines", async function() {
await client.stopClient();

const testClient = new TestClient(
userId,
"DEVICE",
accessToken,
undefined,
{ timelineSupport: true },
);
client = testClient.client;
httpBackend = testClient.httpBackend;
await startClient(httpBackend, client);

const timelineSet = new EventTimelineSet(undefined);
await expect(client.fetchLatestLiveTimeline(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 = EVENTS[2].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: [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.fetchLatestLiveTimeline(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
// `fetchLatestLiveTimeline` 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 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.fetchLatestLiveTimeline(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];

httpBackend.when("GET", "/rooms/!foo%3Abar/messages")
.respond(200, () => {
return {
chunk: [
// No messages to return
],
};
});

return Promise.all([
expect(client.fetchLatestLiveTimeline(timelineSet)).rejects.toThrow(),
httpBackend.flushAllExpected(),
]);
});
});

describe("paginateEventTimeline", function() {
it("should allow you to paginate backwards", function() {
const room = client.getRoom(roomId)!;
Expand Down
4 changes: 2 additions & 2 deletions spec/integ/matrix-client-room-timeline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
// `fetchLatestLiveTimeline()` to construct a new timeline from.
httpBackend!.when("GET", `/rooms/${encodeURIComponent(roomId)}/messages`)
.respond(200, function() {
return {
Expand All @@ -840,7 +840,7 @@ describe("MatrixClient room timelines", function() {
};
});
// `/context` request for `refreshLiveTimeline()` ->
// `getLatestTimeline()` -> `getEventTimeline()` to construct a new
// `fetchLatestLiveTimeline()` -> `getEventTimeline()` to construct a new
// timeline from.
httpBackend!.when("GET", contextUrl)
.respond(200, function() {
Expand Down
116 changes: 116 additions & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5564,6 +5564,122 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
return this.getEventTimeline(timelineSet, event.event_id);
}

/**
* Get an EventTimeline for the latest events in the room. This will just
* call `/messages` to get the latest message in the room, then use
* `client.getEventTimeline(...)` to construct a new timeline from it.
* Always returns timeline in the given `timelineSet`.
*
* @param {EventTimelineSet} timelineSet The timelineSet to find or add the timeline to
*
* @return {Promise} Resolves:
* {@link module:models/event-timeline~EventTimeline} timeline with the latest events in the room
*/
public async fetchLatestLiveTimeline(timelineSet: EventTimelineSet): Promise<EventTimeline> {
// 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.");
}

if (!timelineSet.room) {
throw new Error("fetchLatestLiveTimeline only supports room timelines");
}

if (timelineSet.threadListType !== null || timelineSet.thread && Thread.hasServerSideSupport) {
throw new Error("fetchLatestLiveTimeline only supports live timelines");
}

const messagesPath = utils.encodeUri(
"/rooms/$roomId/messages", {
$roomId: timelineSet.room.roomId,
},
);
const messageRequestParams: Record<string, string | string[]> = {
dir: 'b',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direction.Backward

// 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<IMessagesResponse>(
Method.Get,
messagesPath,
messageRequestParams,
);
Comment on lines +5598 to +5616
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this reuse createMessagesRequest instead?

const latestEventInTimeline = messagesRes.chunk?.[0];
const latestEventIdInTimeline = latestEventInTimeline?.event_id;
if (!latestEventIdInTimeline) {
throw new Error("No message returned when trying to construct fetchLatestLiveTimeline");
}

const contextPath = utils.encodeUri(
"/rooms/$roomId/context/$eventId", {
$roomId: timelineSet.room.roomId,
$eventId: latestEventIdInTimeline,
},
);
let contextRequestParams: Record<string, string | string[]> | undefined = undefined;
if (this.clientOpts?.lazyLoadMembers) {
contextRequestParams = { filter: JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER) };
}
const contextRes = await this.http.authedRequest<IContextResponse>(
Method.Get,
contextPath,
contextRequestParams,
);
Comment on lines +5623 to +5637
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be extracted into a function and reused in the other identical call sites?

if (!contextRes.event || contextRes.event.event_id !== latestEventIdInTimeline) {
throw new Error(
`fetchLatestLiveTimeline: \`/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;
}

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;
}

/**
* 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),
Expand Down
Loading