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.

Sign up for GitHub

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

Include redundant members in most timeline filters #4329

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Include redundant members in most timeline filters #4329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c2786a
Include redundant members in most timeline filters
morguldir Jul 25, 2024
f6683f0
Lint
morguldir Jul 25, 2024
25608e0
Add integration test for missing sender profile edge case
morguldir Jul 25, 2024
980b256
Merge branch 'develop' into redundant-member-state-scrollback
t3chguy Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 149 additions & 1 deletion spec/integ/matrix-client-event-timeline.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

const userId = "@alice:localhost";
const userName = "Alice";
const altName = "meow";
const accessToken = "aseukfgwef";
const roomId = "!foo:bar";
const otherUserId = "@bob:localhost";
Expand Down Expand Up @@ -747,7 +748,9 @@
};
});
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([
Expand Down Expand Up @@ -1076,6 +1079,150 @@
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();

Check failure on line 1120 in spec/integ/matrix-client-event-timeline.spec.ts

View workflow job for this annotation

GitHub Actions / Jest [integ] (Node lts/*)

MatrixClient event timelines › paginateEventTimeline › event will have a sender profile if /context and /messages returns state 

expect(received).toBeNull()

Received: undefined

    at Array.toBeNull (spec/integ/matrix-client-event-timeline.spec.ts:1120:43)
    at HttpBackend._takeFromQueue (node_modules/matrix-mock-request/lib/index.js:249:34)
    at _tryFlush (node_modules/matrix-mock-request/lib/index.js:134:20)
    at Timeout.tryFlush [as _onTimeout] (node_modules/matrix-mock-request/lib/index.js:124:13)

Check failure on line 1120 in spec/integ/matrix-client-event-timeline.spec.ts

View workflow job for this annotation

GitHub Actions / Jest [integ] (Node 22)

MatrixClient event timelines › paginateEventTimeline › event will have a sender profile if /context and /messages returns state 

expect(received).toBeNull()

Received: undefined

    at Array.toBeNull (spec/integ/matrix-client-event-timeline.spec.ts:1120:43)
    at HttpBackend._takeFromQueue (node_modules/matrix-mock-request/lib/index.js:249:34)
    at _tryFlush (node_modules/matrix-mock-request/lib/index.js:134:20)
    at Timeout.tryFlush [as _onTimeout] (node_modules/matrix-mock-request/lib/index.js:124:13)
})
.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();

Check failure on line 1221 in spec/integ/matrix-client-event-timeline.spec.ts

View workflow job for this annotation

GitHub Actions / Jest [integ] (Node lts/*)

MatrixClient event timelines › paginateEventTimeline › event will sometimes have no sender profile if /context but not /messages doesn't return state 

expect(received).toBeNull()

Received: {"_events": {}, "_eventsCount": 0, "_isOutOfBand": false, "_maxListeners": undefined, "disambiguate": false, "events": {"member": {"content": {"displayname": "meow", "membership": "join"}, "event_id": "$15-0.8340423255739344-0.13842104613837392", "origin_server_ts": 0, "room_id": "!foo:bar", "sender": "@alice:localhost", "state_key": "@alice:localhost", "txn_id": "~0.8280236893722641", "type": "m.room.member", "unsigned": {}}}, "membership": "join", "modified": 1722360403661, "name": "meow", "powerLevel": 0, "powerLevelNorm": 0, "rawDisplayName": "meow", "requestedProfileInfo": false, "roomId": "!foo:bar", "typing": false, "user": undefined, "userId": "@alice:localhost", Symbol(shapeMode): false, Symbol(kCapture): false}

    at toBeNull (spec/integ/matrix-client-event-timeline.spec.ts:1221:59)
    at async Promise.all (index 0)

Check failure on line 1221 in spec/integ/matrix-client-event-timeline.spec.ts

View workflow job for this annotation

GitHub Actions / Jest [integ] (Node 22)

MatrixClient event timelines › paginateEventTimeline › event will sometimes have no sender profile if /context but not /messages doesn't return state 

expect(received).toBeNull()

Received: {"_events": {}, "_eventsCount": 0, "_isOutOfBand": false, "_maxListeners": undefined, "disambiguate": false, "events": {"member": {"content": {"displayname": "meow", "membership": "join"}, "event_id": "$15-0.5603748785913731-0.11070096114260042", "origin_server_ts": 0, "room_id": "!foo:bar", "sender": "@alice:localhost", "state_key": "@alice:localhost", "txn_id": "~0.14906772460991702", "type": "m.room.member", "unsigned": {}}}, "membership": "join", "modified": 1722360405903, "name": "meow", "powerLevel": 0, "powerLevelNorm": 0, "rawDisplayName": "meow", "requestedProfileInfo": false, "roomId": "!foo:bar", "typing": false, "user": undefined, "userId": "@alice:localhost", Symbol(shapeMode): false, Symbol(kCapture): false}

    at toBeNull (spec/integ/matrix-client-event-timeline.spec.ts:1221:59)
    at async Promise.all (index 0)
}),
httpBackend.flushAllExpected(),
]);
});
});

it("should ensure thread events are ordered correctly", async () => {
Expand Down Expand Up @@ -1781,6 +1928,7 @@
expect(request.queryParams?.filter).toEqual(
JSON.stringify({
lazy_load_members: true,
include_redundant_members: true,
}),
);
});
Expand Down
12 changes: 6 additions & 6 deletions src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6131,7 +6131,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

let params: Record<string, string | string[]> | 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.
Expand Down Expand Up @@ -6209,7 +6209,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
limit: "0",
};
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.
Expand Down Expand Up @@ -6389,7 +6389,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
dir: "b",
};
if (this.clientOpts?.lazyLoadMembers) {
params.filter = JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER);
params.filter = JSON.stringify(Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER);
}

const res = await this.http.authedRequest<IMessagesResponse>(Method.Get, messagesPath, params);
Expand Down Expand Up @@ -6434,7 +6434,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
if (this.clientOpts?.lazyLoadMembers) {
// create a shallow copy of LAZY_LOADING_MESSAGES_FILTER,
// so the timelineFilter doesn't get written into it below
filter = Object.assign({}, Filter.LAZY_LOADING_MESSAGES_FILTER);
filter = Object.assign({}, Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER);
}
if (timelineFilter) {
// XXX: it's horrific that /messages' filter parameter doesn't match
Expand Down Expand Up @@ -6480,10 +6480,10 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

let filter: IRoomEventFilter = {};
if (this.clientOpts?.lazyLoadMembers) {
// create a shallow copy of LAZY_LOADING_MESSAGES_FILTER,
// create a shallow copy of LAZY_LOADING_MESSAGES_REDUNDANT_FILTER,
// so the timelineFilter doesn't get written into it below
filter = {
...Filter.LAZY_LOADING_MESSAGES_FILTER,
...Filter.LAZY_LOADING_MESSAGES_REDUNDANT_FILTER,
};
}
if (timelineFilter) {
Expand Down
5 changes: 5 additions & 0 deletions src/filter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ export class Filter {
lazy_load_members: true,
};

public static LAZY_LOADING_MESSAGES_REDUNDANT_FILTER = {
Copy link
Member

Choose a reason for hiding this comment

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

Things exported in the public API should have documentation

lazy_load_members: true,
include_redundant_members: true,
};

/**
* Create a filter from existing data.
*/
Expand Down
Loading