matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-08-09 10:22:46 +03:00
Code Activity

Fix issues with /search and /context API handling for threads (#2261)

Browse Source
This commit is contained in:
Michael Telatynski
2022-03-29 09:24:45 +01:00
committed by GitHub
parent bdc3da1fac
commit 85b8d4f83a
8 changed files with 371 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
spec/integ/matrix-client-event-timeline.spec.js
View File

@@ -2,6 +2,7 @@ import * as utils from "../test-utils/test-utils";
import { EventTimeline } from "../../src/matrix";
import { logger } from "../../src/logger";
import { TestClient } from "../TestClient";
import { Thread, THREAD_RELATION_TYPE } from "../../src/models/thread";
const userId = "@alice:localhost";
const userName = "Alice";
@@ -69,6 +70,27 @@ const EVENTS = [
}),
];
const THREAD_ROOT = utils.mkMessage({
room: roomId,
user: userId,
msg: "thread root",
});
const THREAD_REPLY = utils.mkEvent({
room: roomId,
user: userId,
type: "m.room.message",
content: {
"body": "thread reply",
"msgtype": "m.text",
"m.relates_to": {
// We can't use the const here because we change server support mode for test
rel_type: "io.element.thread",
event_id: THREAD_ROOT.event_id,
},
},
});
// start the client, and wait for it to initialise
function startClient(httpBackend, client) {
httpBackend.when("GET", "/versions").respond(200, {});
@@ -116,9 +138,7 @@ describe("getEventTimeline support", function() {
return startClient(httpBackend, client).then(function() {
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
expect(function() {
client.getEventTimeline(timelineSet, "event");
}).toThrow();
expect(client.getEventTimeline(timelineSet, "event")).rejects.toBeTruthy();
});
});
@@ -136,16 +156,12 @@ describe("getEventTimeline support", function() {
return startClient(httpBackend, client).then(() => {
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
expect(function() {
client.getEventTimeline(timelineSet, "event");
}).not.toThrow();
expect(client.getEventTimeline(timelineSet, "event")).rejects.toBeFalsy();
});
});
it("scrollback should be able to scroll back to before a gappy /sync",
function() {
it("scrollback should be able to scroll back to before a gappy /sync", function() {
// need a client with timelineSupport disabled to make this work
let room;
return startClient(httpBackend, client).then(function() {
@@ -229,6 +245,7 @@ describe("MatrixClient event timelines", function() {
afterEach(function() {
httpBackend.verifyNoOutstandingExpectation();
client.stopClient();
Thread.setServerSideSupport(false);
});
describe("getEventTimeline", function() {
@@ -355,8 +372,7 @@ describe("MatrixClient event timelines", function() {
]);
});
it("should join timelines where they overlap a previous /context",
function() {
it("should join timelines where they overlap a previous /context", function() {
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
@@ -478,6 +494,50 @@ describe("MatrixClient event timelines", function() {
httpBackend.flushAllExpected(),
]);
});
it("should handle thread replies with server support by fetching a contiguous thread timeline", async () => {
Thread.setServerSideSupport(true);
client.stopClient(); // we don't need the client to be syncing at this time
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
httpBackend.when("GET", "/rooms/!foo%3Abar/context/" + encodeURIComponent(THREAD_REPLY.event_id))
.respond(200, function() {
return {
start: "start_token0",
events_before: [],
event: THREAD_REPLY,
events_after: [],
end: "end_token0",
state: [],
};
});
httpBackend.when("GET", "/rooms/!foo%3Abar/event/" + encodeURIComponent(THREAD_ROOT.event_id))
.respond(200, function() {
return THREAD_ROOT;
});
httpBackend.when("GET", "/rooms/!foo%3Abar/relations/" +
encodeURIComponent(THREAD_ROOT.event_id) + "/" +
encodeURIComponent(THREAD_RELATION_TYPE.name) + "?limit=20")
.respond(200, function() {
return {
original_event: THREAD_ROOT,
chunk: [THREAD_REPLY],
next_batch: "next_batch_token0",
prev_batch: "prev_batch_token0",
};
});
const timelinePromise = client.getEventTimeline(timelineSet, THREAD_REPLY.event_id);
await httpBackend.flushAllExpected();
const timeline = await timelinePromise;
expect(timeline.getEvents().find(e => e.getId() === THREAD_ROOT.event_id));
expect(timeline.getEvents().find(e => e.getId() === THREAD_REPLY.event_id));
});
});
describe("paginateEventTimeline", function() {

191
spec/integ/matrix-client-methods.spec.js
View File

@@ -3,6 +3,7 @@ import { CRYPTO_ENABLED } from "../../src/client";
import { MatrixEvent } from "../../src/models/event";
import { Filter, MemoryStore, Room } from "../../src/matrix";
import { TestClient } from "../TestClient";
import { THREAD_RELATION_TYPE } from "../../src/models/thread";
describe("MatrixClient", function() {
let client = null;
@@ -14,9 +15,7 @@ describe("MatrixClient", function() {
beforeEach(function() {
store = new MemoryStore();
const testClient = new TestClient(userId, "aliceDevice", accessToken, undefined, {
store: store,
});
const testClient = new TestClient(userId, "aliceDevice", accessToken, undefined, { store });
httpBackend = testClient.httpBackend;
client = testClient.client;
});
@@ -244,14 +243,15 @@ describe("MatrixClient", function() {
});
describe("searching", function() {
const response = {
search_categories: {
room_events: {
count: 24,
results: {
"$flibble:localhost": {
it("searchMessageText should perform a /search for room_events", function() {
const response = {
search_categories: {
room_events: {
count: 24,
results: [{
rank: 0.1,
result: {
event_id: "$flibble:localhost",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
@@ -260,13 +260,11 @@ describe("MatrixClient", function() {
msgtype: "m.text",
},
},
},
}],
},
},
},
};
};
it("searchMessageText should perform a /search for room_events", function(done) {
client.searchMessageText({
query: "monkeys",
});
@@ -280,8 +278,171 @@ describe("MatrixClient", function() {
});
}).respond(200, response);
httpBackend.flush().then(function() {
done();
return httpBackend.flush();
});
describe("should filter out context from different timelines (threads)", () => {
it("filters out thread replies when result is in the main timeline", async () => {
const response = {
search_categories: {
room_events: {
count: 24,
results: [{
rank: 0.1,
result: {
event_id: "$flibble:localhost",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
body: "main timeline",
msgtype: "m.text",
},
},
context: {
events_after: [{
event_id: "$ev-after:server",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
"body": "thread reply",
"msgtype": "m.text",
"m.relates_to": {
"event_id": "$some-thread:server",
"rel_type": THREAD_RELATION_TYPE.name,
},
},
}],
events_before: [{
event_id: "$ev-before:server",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
body: "main timeline again",
msgtype: "m.text",
},
}],
},
}],
},
},
};
const data = {
results: [],
highlights: [],
};
client.processRoomEventsSearch(data, response);
expect(data.results).toHaveLength(1);
expect(data.results[0].context.timeline).toHaveLength(2);
expect(data.results[0].context.timeline.find(e => e.getId() === "$ev-after:server")).toBeFalsy();
});
it("filters out thread replies from threads other than the thread the result replied to", () => {
const response = {
search_categories: {
room_events: {
count: 24,
results: [{
rank: 0.1,
result: {
event_id: "$flibble:localhost",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
"body": "thread 1 reply 1",
"msgtype": "m.text",
"m.relates_to": {
"event_id": "$thread1:server",
"rel_type": THREAD_RELATION_TYPE.name,
},
},
},
context: {
events_after: [{
event_id: "$ev-after:server",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
"body": "thread 2 reply 2",
"msgtype": "m.text",
"m.relates_to": {
"event_id": "$thread2:server",
"rel_type": THREAD_RELATION_TYPE.name,
},
},
}],
events_before: [],
},
}],
},
},
};
const data = {
results: [],
highlights: [],
};
client.processRoomEventsSearch(data, response);
expect(data.results).toHaveLength(1);
expect(data.results[0].context.timeline).toHaveLength(1);
expect(data.results[0].context.timeline.find(e => e.getId() === "$flibble:localhost")).toBeTruthy();
});
it("filters out main timeline events when result is a thread reply", () => {
const response = {
search_categories: {
room_events: {
count: 24,
results: [{
rank: 0.1,
result: {
event_id: "$flibble:localhost",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
"body": "thread 1 reply 1",
"msgtype": "m.text",
"m.relates_to": {
"event_id": "$thread1:server",
"rel_type": THREAD_RELATION_TYPE.name,
},
},
},
context: {
events_after: [{
event_id: "$ev-after:server",
type: "m.room.message",
user_id: "@alice:localhost",
room_id: "!feuiwhf:localhost",
content: {
"body": "main timeline",
"msgtype": "m.text",
},
}],
events_before: [],
},
}],
},
},
};
const data = {
results: [],
highlights: [],
};
client.processRoomEventsSearch(data, response);
expect(data.results).toHaveLength(1);
expect(data.results[0].context.timeline).toHaveLength(1);
expect(data.results[0].context.timeline.find(e => e.getId() === "$flibble:localhost")).toBeTruthy();
});
});
});

118
src/client.ts
View File

@@ -5231,19 +5231,17 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @param {string} eventId The ID of the event to look for
*
* @return {Promise} Resolves:
* {@link module:models/event-timeline~EventTimeline} including the given
* event
* {@link module:models/event-timeline~EventTimeline} including the given event
*/
public getEventTimeline(timelineSet: EventTimelineSet, eventId: string): Promise<EventTimeline> {
public async getEventTimeline(timelineSet: EventTimelineSet, eventId: string): 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.");
" parameter to true when creating MatrixClient to enable it.");
}
if (timelineSet.getTimelineForEvent(eventId)) {
return Promise.resolve(timelineSet.getTimelineForEvent(eventId));
return timelineSet.getTimelineForEvent(eventId);
}
const path = utils.encodeUri(
@@ -5253,56 +5251,82 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
},
);
let params = undefined;
let params: Record<string, string | string[]> = undefined;
if (this.clientOpts.lazyLoadMembers) {
params = { filter: JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER) };
}
// TODO: we should implement a backoff (as per scrollback()) to deal more
// nicely with HTTP errors.
const promise = this.http.authedRequest<any>(undefined, Method.Get, path, params).then(async (res) => { // TODO types
if (!res.event) {
throw new Error("'event' not in '/context' result - homeserver too old?");
}
// TODO: we should implement a backoff (as per scrollback()) to deal more nicely with HTTP errors.
const res = await this.http.authedRequest<any>(undefined, Method.Get, path, params); // TODO types
if (!res.event) {
throw new Error("'event' not in '/context' result - homeserver too old?");
}
// by the time the request completes, the event might have ended up in
// the timeline.
if (timelineSet.getTimelineForEvent(eventId)) {
return timelineSet.getTimelineForEvent(eventId);
}
// by the time the request completes, the event might have ended up in the timeline.
if (timelineSet.getTimelineForEvent(eventId)) {
return timelineSet.getTimelineForEvent(eventId);
}
// we start with the last event, since that's the point at which we
// have known state.
const mapper = this.getEventMapper();
const event = mapper(res.event);
const events = [
// 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.
res.events_after.reverse();
const events = res.events_after
.concat([res.event])
.concat(res.events_before);
const matrixEvents = events.map(this.getEventMapper());
...res.events_after.reverse().map(mapper),
event,
...res.events_before.map(mapper),
];
let timeline = timelineSet.getTimelineForEvent(matrixEvents[0].getId());
if (!timeline) {
timeline = timelineSet.addTimeline();
timeline.initialiseState(res.state.map(this.getEventMapper()));
timeline.getState(EventTimeline.FORWARDS).paginationToken = res.end;
} else {
const stateEvents = res.state.map(this.getEventMapper());
timeline.getState(EventTimeline.BACKWARDS).setUnknownStateEvents(stateEvents);
// Where the event is a thread reply (not a root) and running in MSC-enabled mode the Thread timeline only
// functions contiguously, so we have to jump through some hoops to get our target event in it.
// XXX: workaround for https://github.com/vector-im/element-meta/issues/150
if (Thread.hasServerSideSupport && event.isRelation(THREAD_RELATION_TYPE.name)) {
const [, threadedEvents] = this.partitionThreadedEvents(events);
const thread = await timelineSet.room.createThreadFetchRoot(event.threadRootId, threadedEvents, true);
let nextBatch: string;
const response = await thread.fetchInitialEvents();
if (response?.nextBatch) {
nextBatch = response.nextBatch;
}
const [timelineEvents, threadedEvents] = this.partitionThreadedEvents(matrixEvents);
const opts: IRelationsRequestOpts = {
limit: 50,
};
timelineSet.addEventsToTimeline(timelineEvents, true, timeline, res.start);
await this.processThreadEvents(timelineSet.room, threadedEvents, true);
// Fetch events until we find the one we were asked for
while (!thread.findEventById(eventId)) {
if (nextBatch) {
opts.from = nextBatch;
}
// there is no guarantee that the event ended up in "timeline" (we
// might have switched to a neighbouring timeline) - so check the
// room's index again. On the other hand, there's no guarantee the
// event ended up anywhere, if it was later redacted, so we just
// return the timeline we first thought of.
return timelineSet.getTimelineForEvent(eventId) || timeline;
});
return promise;
({ nextBatch } = await thread.fetchEvents(opts));
}
return thread.liveTimeline;
}
// Here we handle non-thread timelines only, but still process any thread events to populate thread summaries.
let timeline = timelineSet.getTimelineForEvent(events[0].getId());
if (timeline) {
timeline.getState(EventTimeline.BACKWARDS).setUnknownStateEvents(res.state.map(mapper));
} else {
timeline = timelineSet.addTimeline();
timeline.initialiseState(res.state.map(mapper));
timeline.getState(EventTimeline.FORWARDS).paginationToken = res.end;
}
const [timelineEvents, threadedEvents] = this.partitionThreadedEvents(events);
timelineSet.addEventsToTimeline(timelineEvents, true, timeline, res.start);
// The target event is not in a thread but process the contextual events, so we can show any threads around it.
await this.processThreadEvents(timelineSet.room, threadedEvents, true);
// There is no guarantee that the event ended up in "timeline" (we might have switched to a neighbouring
// timeline) - so check the room's index again. On the other hand, there's no guarantee the event ended up
// anywhere, if it was later redacted, so we just return the timeline we first thought of.
return timelineSet.getTimelineForEvent(eventId)
?? timelineSet.room.findThreadForEvent(event)?.liveTimeline // for Threads degraded support
?? timeline;
}
/**
@@ -6026,10 +6050,12 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
// turn it back into a list.
searchResults.highlights = Array.from(highlights);
const mapper = this.getEventMapper();
// append the new results to our existing results
const resultsLength = roomEvents.results ? roomEvents.results.length : 0;
const resultsLength = roomEvents.results?.length ?? 0;
for (let i = 0; i < resultsLength; i++) {
const sr = SearchResult.fromJson(roomEvents.results[i], this.getEventMapper());
const sr = SearchResult.fromJson(roomEvents.results[i], mapper);
const room = this.getRoom(sr.context.getEvent().getRoomId());
if (room) {
// Copy over a known event sender if we can

2
src/models/event-context.ts
View File

@@ -42,7 +42,7 @@ export class EventContext {
*
* @constructor
*/
constructor(ourEvent: MatrixEvent) {
constructor(public readonly ourEvent: MatrixEvent) {
this.timeline = [ourEvent];
}

48
src/models/room.ts
View File

@@ -48,7 +48,6 @@ import {
} from "./thread";
import { Method } from "../http-api";
import { TypedEventEmitter } from "./typed-event-emitter";
import { IMinimalEvent } from "../sync-accumulator";
// 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
@@ -1578,24 +1577,18 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
}
}
/**
* Add an event to a thread's timeline. Will fire "Thread.update"
* @experimental
*/
public async addThreadedEvent(event: MatrixEvent, toStartOfTimeline: boolean): Promise<void> {
this.applyRedaction(event);
let thread = this.findThreadForEvent(event);
if (thread) {
thread.addEvent(event, toStartOfTimeline);
} else {
const events = [event];
let rootEvent = this.findEventById(event.threadRootId);
// If the rootEvent does not exist in the current sync, then look for it over the network.
public async createThreadFetchRoot(
threadId: string,
events?: MatrixEvent[],
toStartOfTimeline?: boolean,
): Promise<Thread> {
let thread = this.getThread(threadId);
if (!thread) {
let rootEvent = this.findEventById(threadId);
// If the rootEvent does not exist in the local stores, then fetch it from the server.
try {
let eventData: IMinimalEvent;
if (event.threadRootId) {
eventData = await this.client.fetchRoomEvent(this.roomId, event.threadRootId);
}
const eventData = await this.client.fetchRoomEvent(this.roomId, threadId);
if (!rootEvent) {
rootEvent = new MatrixEvent(eventData);
@@ -1613,6 +1606,22 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
}
}
return thread;
}
/**
* Add an event to a thread's timeline. Will fire "Thread.update"
* @experimental
*/
public async addThreadedEvent(event: MatrixEvent, toStartOfTimeline: boolean): Promise<void> {
this.applyRedaction(event);
let thread = this.findThreadForEvent(event);
if (thread) {
await thread.addEvent(event, toStartOfTimeline);
} else {
thread = await this.createThreadFetchRoot(event.threadRootId, [event], toStartOfTimeline);
}
this.emit(ThreadEvent.Update, thread);
}
@@ -1634,8 +1643,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
room: this,
client: this.client,
});
// If we managed to create a thread and figure out its `id`
// then we can use it
// If we managed to create a thread and figure out its `id` then we can use it
if (thread.id) {
this.threads.set(thread.id, thread);
this.reEmitter.reEmit(thread, [

13
src/models/search-result.ts
View File

@@ -33,14 +33,19 @@ export class SearchResult {
public static fromJson(jsonObj: ISearchResult, eventMapper: EventMapper): SearchResult {
const jsonContext = jsonObj.context || {} as IResultContext;
const eventsBefore = jsonContext.events_before || [];
const eventsAfter = jsonContext.events_after || [];
let eventsBefore = (jsonContext.events_before || []).map(eventMapper);
let eventsAfter = (jsonContext.events_after || []).map(eventMapper);
const context = new EventContext(eventMapper(jsonObj.result));
// Filter out any contextual events which do not correspond to the same timeline (thread or room)
const threadRootId = context.ourEvent.threadRootId;
eventsBefore = eventsBefore.filter(e => e.threadRootId === threadRootId);
eventsAfter = eventsAfter.filter(e => e.threadRootId === threadRootId);
context.setPaginateToken(jsonContext.start, true);
context.addEvents(eventsBefore.map(eventMapper), true);
context.addEvents(eventsAfter.map(eventMapper), false);
context.addEvents(eventsBefore, true);
context.addEvents(eventsAfter, false);
context.setPaginateToken(jsonContext.end, false);
return new SearchResult(jsonObj.rank, context);

10
src/models/thread.ts
View File

@@ -165,12 +165,12 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
this.addEventToTimeline(event, toStartOfTimeline);
await this.client.decryptEventIfNeeded(event, {});
} else {
} else if (!toStartOfTimeline &&
this.initialEventsFetched &&
event.localTimestamp > this.lastReply().localTimestamp
) {
await this.fetchEditsWhereNeeded(event);
if (this.initialEventsFetched && event.localTimestamp > this.lastReply().localTimestamp) {
this.addEventToTimeline(event, false);
}
this.addEventToTimeline(event, false);
}
if (!this._currentUserParticipated && event.getSender() === this.client.getUserId()) {

27
src/timeline-window.ts
View File

@@ -99,11 +99,11 @@ export class TimelineWindow {
*
* @return {Promise}
*/
public load(initialEventId?: string, initialWindowSize = 20): Promise<any> {
public load(initialEventId?: string, initialWindowSize = 20): Promise<void> {
// given an EventTimeline, find the event we were looking for, and initialise our
// fields so that the event in question is in the middle of the window.
const initFields = (timeline: EventTimeline) => {
let eventIndex;
let eventIndex: number;
const events = timeline.getEvents();
@@ -111,40 +111,31 @@ export class TimelineWindow {
// we were looking for the live timeline: initialise to the end
eventIndex = events.length;
} else {
for (let i = 0; i < events.length; i++) {
if (events[i].getId() == initialEventId) {
eventIndex = i;
break;
}
}
eventIndex = events.findIndex(e => e.getId() === initialEventId);
if (eventIndex === undefined) {
if (eventIndex < 0) {
throw new Error("getEventTimeline result didn't include requested event");
}
}
const endIndex = Math.min(events.length,
eventIndex + Math.ceil(initialWindowSize / 2));
const endIndex = Math.min(events.length, eventIndex + Math.ceil(initialWindowSize / 2));
const startIndex = Math.max(0, endIndex - initialWindowSize);
this.start = new TimelineIndex(timeline, startIndex - timeline.getBaseIndex());
this.end = new TimelineIndex(timeline, endIndex - timeline.getBaseIndex());
this.eventCount = endIndex - startIndex;
};
// We avoid delaying the resolution of the promise by a reactor tick if
// we already have the data we need, which is important to keep room-switching
// feeling snappy.
//
// We avoid delaying the resolution of the promise by a reactor tick if we already have the data we need,
// which is important to keep room-switching feeling snappy.
if (initialEventId) {
const timeline = this.timelineSet.getTimelineForEvent(initialEventId);
if (timeline) {
// hot-path optimization to save a reactor tick by replicating the sync check getTimelineForEvent does.
initFields(timeline);
return Promise.resolve(timeline);
return Promise.resolve();
}
const prom = this.client.getEventTimeline(this.timelineSet, initialEventId);
return prom.then(initFields);
return this.client.getEventTimeline(this.timelineSet, initialEventId).then(initFields);
} else {
const tl = this.timelineSet.getLiveTimeline();
initFields(tl);