From 1c2786ac1576a23d15bd1cb5012d2c85dce24355 Mon Sep 17 00:00:00 2001 From: morguldir Date: Thu, 25 Jul 2024 19:59:28 +0200 Subject: [PATCH 1/3] Include redundant members in most timeline filters Synapse never tries to avoid redundant members anyway, so it's hard to ensure that it works correctly (https://github.com/element-hq/synapse/blob/568051c0f07393b786b9d813a1db53dd332c9fc2/synapse/handlers/pagination.py#L639) It also seems like it would be difficult when the "Show current profile picture and name for users in message history" option is turned off Conduwuit and Dendrite do attempt to remove redundant members, and it seems to cause some problems, so this commit tells them to include redundant members Similar to element-hq/element-web#7417, which is also why there might still be missing profiles in rare cases after this Signed-off-by: morguldir --- spec/integ/matrix-client-event-timeline.spec.ts | 3 ++- src/client.ts | 12 ++++++------ src/filter.ts | 5 +++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/spec/integ/matrix-client-event-timeline.spec.ts b/spec/integ/matrix-client-event-timeline.spec.ts index 5785732422..05997b2f47 100644 --- a/spec/integ/matrix-client-event-timeline.spec.ts +++ b/spec/integ/matrix-client-event-timeline.spec.ts @@ -747,7 +747,7 @@ describe("MatrixClient event timelines", function () { }; }); req.check((request) => { - expect(request.queryParams?.filter).toEqual(JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER)); + expect(request.queryParams?.filter).toEqual(JSON.stringify(Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER)); }); await Promise.all([ @@ -1781,6 +1781,7 @@ describe("MatrixClient event timelines", function () { expect(request.queryParams?.filter).toEqual( JSON.stringify({ lazy_load_members: true, + include_redundant_members: true, }), ); }); diff --git a/src/client.ts b/src/client.ts index 32ce1b2685..7c297459cc 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5946,7 +5946,7 @@ export class MatrixClient extends TypedEventEmitter | undefined = undefined; if (this.clientOpts?.lazyLoadMembers) { - params = { filter: JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER) }; + params = { filter: JSON.stringify(Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER) }; } // TODO: we should implement a backoff (as per scrollback()) to deal more nicely with HTTP errors. @@ -6024,7 +6024,7 @@ export class MatrixClient extends TypedEventEmitter(Method.Get, messagesPath, params); @@ -6249,7 +6249,7 @@ export class MatrixClient extends TypedEventEmitter Date: Thu, 25 Jul 2024 20:35:15 +0200 Subject: [PATCH 2/3] Lint Signed-off-by: morguldir --- src/filter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filter.ts b/src/filter.ts index 0cdd44d041..8da663434b 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -76,7 +76,7 @@ export class Filter { public static LAZY_LOADING_MESSAGES_REDUNDANT_FILTER = { lazy_load_members: true, - include_redundant_members: true + include_redundant_members: true, }; /** From 25608e019a9f723d28ad932b09bdfde6d66bd2cc Mon Sep 17 00:00:00 2001 From: morguldir Date: Fri, 26 Jul 2024 00:01:30 +0200 Subject: [PATCH 3/3] Add integration test for missing sender profile edge case --- .../matrix-client-event-timeline.spec.ts | 149 +++++++++++++++++- 1 file changed, 148 insertions(+), 1 deletion(-) diff --git a/spec/integ/matrix-client-event-timeline.spec.ts b/spec/integ/matrix-client-event-timeline.spec.ts index 05997b2f47..90d270836a 100644 --- a/spec/integ/matrix-client-event-timeline.spec.ts +++ b/spec/integ/matrix-client-event-timeline.spec.ts @@ -40,6 +40,7 @@ import { KnownMembership } from "../../src/@types/membership"; const userId = "@alice:localhost"; const userName = "Alice"; +const altName = "meow"; const accessToken = "aseukfgwef"; const roomId = "!foo:bar"; const otherUserId = "@bob:localhost"; @@ -747,7 +748,9 @@ describe("MatrixClient event timelines", function () { }; }); req.check((request) => { - expect(request.queryParams?.filter).toEqual(JSON.stringify(Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER)); + expect(request.queryParams?.filter).toEqual( + JSON.stringify(Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER), + ); }); await Promise.all([ @@ -1076,6 +1079,150 @@ describe("MatrixClient event timelines", function () { httpBackend.flushAllExpected(), ]); }); + it("event will have a sender profile if /context and /messages returns state", async () => { + // @ts-ignore + client.clientOpts.lazyLoadMembers = false; + const room = client.getRoom(roomId)!; + const timelineSet = room.getTimelineSets()[0]; + + httpBackend + .when("GET", "/rooms/!foo%3Abar/context/" + encodeURIComponent(EVENTS[0].event_id!)) + .respond(200, function () { + return { + start: "start_token0", + events_before: [], + event: EVENTS[0], + events_after: [], + end: "end_token0", + state: [ + utils.mkMembership({ + mship: KnownMembership.Join, + room: roomId, + user: userId, + name: altName, + event: true, + }), + utils.mkMembership({ + mship: KnownMembership.Join, + room: roomId, + user: otherUserId, + event: true, + }), + ], + }; + }); + + // Server thinks the client has cached the member event + httpBackend + .when("GET", "/rooms/!foo%3Abar/messages") + .check(function (req) { + const params = req.queryParams!; + expect(params.filter).toBeNull(); + }) + .respond(200, function () { + return { + chunk: [EVENTS[1], EVENTS[2]], + end: "start_token1", + state: [ + utils.mkMembership({ + mship: KnownMembership.Join, + room: roomId, + user: userId, + name: altName, + event: true, + }), + utils.mkMembership({ + mship: KnownMembership.Join, + room: roomId, + user: otherUserId, + event: true, + }), + ], + }; + }); + + let tl: EventTimeline; + return Promise.all([ + client + .getEventTimeline(timelineSet, EVENTS[0].event_id!) + .then(function (tl0) { + tl = tl0!; + return client.paginateEventTimeline(tl, { backwards: true }); + }) + .then(function (success) { + expect(success).toBeTruthy(); + expect(tl!.getEvents()[0].sender?.name).toEqual(altName); + expect(tl!.getEvents()[1].sender?.name).toEqual(altName); + expect(tl!.getEvents()[2].sender?.name).toEqual(altName); + }), + httpBackend.flushAllExpected(), + ]); + }); + + it("event will sometimes have no sender profile if /context but not /messages doesn't return state", async () => { + // @ts-ignore + client.clientOpts.lazyLoadMembers = true; + const room = client.getRoom(roomId)!; + const timelineSet = room.getTimelineSets()[0]; + + httpBackend + .when("GET", "/rooms/!foo%3Abar/context/" + encodeURIComponent(EVENTS[0].event_id!)) + .respond(200, function () { + return { + start: "start_token0", + events_before: [], + event: EVENTS[0], + events_after: [], + end: "end_token0", + state: [ + utils.mkMembership({ + mship: KnownMembership.Join, + room: roomId, + user: userId, + name: altName, + event: true, + }), + ], + }; + }); + + // Lazy loaded members with redundant members removed since the server just sent it above + + httpBackend + .when("GET", "/rooms/!foo%3Abar/messages") + .check(function (req) { + const params = req.queryParams!; + expect(params.filter).toEqual(JSON.stringify(Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER)); + }) + .respond(200, function () { + return { + chunk: [EVENTS[1], EVENTS[2]], + end: "start_token1", + }; + }); + + logger.log("hello world"); + let tl: EventTimeline; + return Promise.all([ + client + .getEventTimeline(timelineSet, EVENTS[0].event_id!) + .then(function (tl0) { + tl = tl0!; + return client.paginateEventTimeline(tl, { backwards: true }); + }) + .then(function (success) { + logger.log("abcd, ", tl!.getEvents()[0].sender); + expect(success).toBeTruthy(); + expect(tl!.getEvents().length).toEqual(3); + expect(tl!.getEvents()[0].sender?.name).toEqual(altName); + expect(tl!.getEvents()[1].sender?.name).toEqual(altName); + + // Lost track of the member state? + expect(tl!.getEvents()[2].sender).toBeNull(); + }), + httpBackend.flushAllExpected(), + ]); + }); }); it("should ensure thread events are ordered correctly", async () => {