Skip to content

Commit

Permalink
Remove m.thread filter from relations API call
Browse files Browse the repository at this point in the history
We used MSC3981 to pass the recurse param to the /relations
endpoint so that we could get relations to events in a thread, but
we kept the rel_type filter on (as m.thread) so no second-order relations
would ever have been returned (a nested thread isn't a thing).

This removes the filter and does some filtering on the client side to
remove any events that shouldn't live in the threaded timeline (ie.
non-thread relations to the thread root event).

This should help fix stuck unreads because it will avoid the event that
the receipt refers to going missing (but only on HSes that support MSC3981).

For element-hq/element-web#26718
  • Loading branch information
dbkr committed Dec 12, 2023
1 parent b03dc6a commit c0de03e
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 9 deletions.
51 changes: 51 additions & 0 deletions spec/unit/thread-utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import { IEvent } from "../../src";
import { randomString } from "../../src/randomstring";
import { getRelationsThreadFilter } from "../../src/thread-utils";

function makeEvent(relatesToEvent: string, relType: string): Partial<IEvent> {
return {
event_id: randomString(10),
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "foo",
"m.relates_to": {
rel_type: relType,
event_id: relatesToEvent,
},
},
};
}

describe("getRelationsThreadFilter", () => {
it("should filter out relations directly to the thread root event", () => {
const threadId = "thisIsMyThreadRoot";

const reactionToRoot = makeEvent(threadId, "m.annotation");
const editToRoot = makeEvent(threadId, "m.replace");
const firstThreadedReply = makeEvent(threadId, "m.thread");
const reactionToThreadedEvent = makeEvent(firstThreadedReply.event_id!, "m.annotation");

const filteredEvents = [reactionToRoot, editToRoot, firstThreadedReply, reactionToThreadedEvent].filter(
getRelationsThreadFilter(threadId),
);

expect(filteredEvents).toEqual([firstThreadedReply, reactionToThreadedEvent]);
});
});
22 changes: 14 additions & 8 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ import {
} from "./secret-storage";
import { RegisterRequest, RegisterResponse } from "./@types/registration";
import { MatrixRTCSessionManager } from "./matrixrtc/MatrixRTCSessionManager";
import { getRelationsThreadFilter } from "./thread-utils";

export type Store = IStore;

Expand Down Expand Up @@ -5968,25 +5969,26 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
const resOlder: IRelationsResponse = await this.fetchRelations(
timelineSet.room.roomId,
thread.id,
THREAD_RELATION_TYPE.name,
null,
null,
{ dir: Direction.Backward, from: res.start, recurse: recurse || undefined },
);
const resNewer: IRelationsResponse = await this.fetchRelations(
timelineSet.room.roomId,
thread.id,
THREAD_RELATION_TYPE.name,
null,
null,
{ dir: Direction.Forward, from: res.end, recurse: recurse || undefined },
);
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.
...resNewer.chunk.reverse().map(mapper),
...resNewer.chunk.reverse().filter(getRelationsThreadFilter(thread.id)).map(mapper),
event,
...resOlder.chunk.map(mapper),
...resOlder.chunk.filter(getRelationsThreadFilter(thread.id)).map(mapper),
];

for (const event of events) {
await timelineSet.thread?.processEvent(event);
}
Expand Down Expand Up @@ -6361,6 +6363,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
const stateEvents = res.state.filter(noUnsafeEventProps).map(this.getEventMapper());
roomState.setUnknownStateEvents(stateEvents);
}

const token = res.end;
const matrixEvents = res.chunk.filter(noUnsafeEventProps).map(this.getEventMapper());

Expand Down Expand Up @@ -6388,15 +6391,18 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
}

const recurse = this.canSupport.get(Feature.RelationsRecursion) !== ServerSupport.Unsupported;
promise = this.fetchRelations(eventTimeline.getRoomId() ?? "", thread.id, THREAD_RELATION_TYPE.name, null, {
promise = this.fetchRelations(eventTimeline.getRoomId() ?? "", thread.id, null, null, {
dir,
limit: opts.limit,
from: token ?? undefined,
recurse: recurse || undefined,
})
.then(async (res) => {
const mapper = this.getEventMapper();
const matrixEvents = res.chunk.filter(noUnsafeEventProps).map(mapper);
const matrixEvents = res.chunk
.filter(noUnsafeEventProps)
.filter(getRelationsThreadFilter(thread.id))
.map(mapper);

// Process latest events first
for (const event of matrixEvents.slice().reverse()) {
Expand Down Expand Up @@ -7591,7 +7597,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
public async relations(
roomId: string,
eventId: string,
relationType?: RelationType | string | null,
relationType: RelationType | string | null,
eventType?: EventType | string | null,
opts: IRelationsRequestOpts = { dir: Direction.Backward },
): Promise<{
Expand Down Expand Up @@ -8114,7 +8120,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
public fetchRelations(
roomId: string,
eventId: string,
relationType?: RelationType | string | null,
relationType: RelationType | string | null,
eventType?: EventType | string | null,
opts: IRelationsRequestOpts = { dir: Direction.Backward },
): Promise<IRelationsResponse> {
Expand Down
1 change: 0 additions & 1 deletion src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,6 @@ export class Thread extends ReadReceipt<ThreadEmittedEvents, ThreadEventHandlerM
} else {
await this.client.paginateEventTimeline(this.liveTimeline, {
backwards: true,
limit: Math.max(1, this.length),
});
}
for (const event of this.replayEvents!) {
Expand Down
30 changes: 30 additions & 0 deletions src/thread-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import { IEvent, THREAD_RELATION_TYPE } from "./matrix";

/**
* Returns a filter function for the /relations endpoint to filter out relations directly
* to the thread root event that should not live in the thread timeline
*
* @param threadId - the thread ID (ie. the event ID of the root event of the thread)
* @returns the filtered list of events
*/
export function getRelationsThreadFilter(threadId: string): (e: Partial<IEvent>) => boolean {
return (e: Partial<IEvent>) =>
e.content?.["m.relates_to"]?.event_id !== threadId ||
e.content?.["m.relates_to"]?.rel_type === THREAD_RELATION_TYPE.name;
}

0 comments on commit c0de03e

Please sign in to comment.