From f604ab2f6339e4cd2e20bf12bd0b208c2e1722f0 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Dec 2023 10:39:43 +0000 Subject: [PATCH] Remove m.thread filter from relations API call (#3959) * Remove m.thread filter from relations API call 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 https://github.com/vector-im/element-web/issues/26718 * Fix import cycle * Remove params from expected calls in tests to match * Unused import --- .../matrix-client-event-timeline.spec.ts | 24 ++------- spec/unit/thread-utils.spec.ts | 51 +++++++++++++++++++ src/client.ts | 22 +++++--- src/models/thread.ts | 1 - src/thread-utils.ts | 31 +++++++++++ 5 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 spec/unit/thread-utils.spec.ts create mode 100644 src/thread-utils.ts diff --git a/spec/integ/matrix-client-event-timeline.spec.ts b/spec/integ/matrix-client-event-timeline.spec.ts index 89cfdb486..1d80cdd98 100644 --- a/spec/integ/matrix-client-event-timeline.spec.ts +++ b/spec/integ/matrix-client-event-timeline.spec.ts @@ -33,7 +33,7 @@ import { import { logger } from "../../src/logger"; import { encodeParams, encodeUri, QueryDict, replaceParam } from "../../src/utils"; import { TestClient } from "../TestClient"; -import { FeatureSupport, Thread, THREAD_RELATION_TYPE, ThreadEvent } from "../../src/models/thread"; +import { FeatureSupport, Thread, ThreadEvent } from "../../src/models/thread"; import { emitPromise } from "../test-utils/test-utils"; import { Feature, ServerSupport } from "../../src/feature"; @@ -623,9 +623,7 @@ describe("MatrixClient event timelines", function () { "GET", "/rooms/!foo%3Abar/relations/" + encodeURIComponent(THREAD_ROOT.event_id!) + - "/" + - encodeURIComponent(THREAD_RELATION_TYPE.name) + - buildRelationPaginationQuery({ dir: Direction.Backward, limit: 1 }), + buildRelationPaginationQuery({ dir: Direction.Backward }), ) .respond(200, function () { return { @@ -1154,10 +1152,7 @@ describe("MatrixClient event timelines", function () { httpBackend .when( "GET", - "/_matrix/client/v1/rooms/!foo%3Abar/relations/" + - encodeURIComponent(THREAD_ROOT_UPDATED.event_id!) + - "/" + - encodeURIComponent(THREAD_RELATION_TYPE.name), + "/_matrix/client/v1/rooms/!foo%3Abar/relations/" + encodeURIComponent(THREAD_ROOT_UPDATED.event_id!), ) .respond(200, { chunk: [THREAD_REPLY3.event, THREAD_REPLY2.event, THREAD_REPLY], @@ -1262,11 +1257,8 @@ describe("MatrixClient event timelines", function () { "GET", "/_matrix/client/v1/rooms/!foo%3Abar/relations/" + encodeURIComponent(THREAD_ROOT_UPDATED.event_id!) + - "/" + - encodeURIComponent(THREAD_RELATION_TYPE.name) + buildRelationPaginationQuery({ dir: Direction.Backward, - limit: 3, recurse: true, }), ) @@ -1321,11 +1313,7 @@ describe("MatrixClient event timelines", function () { function respondToThread(root: Partial, replies: Partial[]): ExpectedHttpRequest { const request = httpBackend.when( "GET", - "/_matrix/client/v1/rooms/!foo%3Abar/relations/" + - encodeURIComponent(root.event_id!) + - "/" + - encodeURIComponent(THREAD_RELATION_TYPE.name) + - "?dir=b&limit=1", + "/_matrix/client/v1/rooms/!foo%3Abar/relations/" + encodeURIComponent(root.event_id!) + "?dir=b", ); request.respond(200, function () { return { @@ -2034,9 +2022,7 @@ describe("MatrixClient event timelines", function () { "GET", "/_matrix/client/v1/rooms/!foo%3Abar/relations/" + encodeURIComponent(THREAD_ROOT.event_id!) + - "/" + - encodeURIComponent(THREAD_RELATION_TYPE.name) + - buildRelationPaginationQuery({ dir: Direction.Backward, limit: 1 }), + buildRelationPaginationQuery({ dir: Direction.Backward }), ) .respond(200, function () { return { diff --git a/spec/unit/thread-utils.spec.ts b/spec/unit/thread-utils.spec.ts new file mode 100644 index 000000000..eba63d86e --- /dev/null +++ b/spec/unit/thread-utils.spec.ts @@ -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 { + 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]); + }); +}); diff --git a/src/client.ts b/src/client.ts index 9ef4550d4..b10949e22 100644 --- a/src/client.ts +++ b/src/client.ts @@ -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; @@ -5968,14 +5969,14 @@ export class MatrixClient extends TypedEventEmitter { 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()) { @@ -7591,7 +7597,7 @@ export class MatrixClient extends TypedEventEmitter { diff --git a/src/models/thread.ts b/src/models/thread.ts index e4cfdaadb..71e5d749d 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -609,7 +609,6 @@ export class Thread extends ReadReceipt) => boolean { + return (e: Partial) => + e.content?.["m.relates_to"]?.event_id !== threadId || + e.content?.["m.relates_to"]?.rel_type === THREAD_RELATION_TYPE.name; +}