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.

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

Fix issue with sentinels being incorrect on m.room.member events #4609

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions spec/unit/event-timeline-set.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
EventTimelineSet,
EventType,
Filter,
KnownMembership,
MatrixClient,
MatrixEvent,
MatrixEventEvent,
Expand Down Expand Up @@ -138,6 +139,31 @@ describe("EventTimelineSet", () => {
expect(eventsInLiveTimeline.length).toStrictEqual(1);
expect(eventsInLiveTimeline[0]).toStrictEqual(duplicateMessageEvent);
});

it("should set event.target after applying the membership state update", () => {
const eventTimelineSet = room.getUnfilteredTimelineSet();

const ev1 = utils.mkMembership({
room: roomId,
mship: KnownMembership.Invite,
user: "@sender:server",
skey: userA,
event: true,
});
const ev2 = utils.mkMembership({
room: roomId,
mship: KnownMembership.Join,
user: userA,
skey: userA,
name: "This is my displayname",
event: true,
});

eventTimelineSet.addLiveEvent(ev1, { addToState: true });
expect(ev1.target?.name).toBe(userA);
eventTimelineSet.addLiveEvent(ev2, { addToState: true });
expect(ev2.target?.name).toBe("This is my displayname");
});
});

describe("addEventToTimeline", () => {
Expand Down
4 changes: 1 addition & 3 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7348,10 +7348,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
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
for (const ev of sr.context.getTimeline()) {
const sender = room.getMember(ev.getSender()!);
if (!ev.sender && sender) ev.sender = sender;
ev.setMetadata(room.currentState, false);
}
}
searchResults.results.push(sr);
Expand Down
22 changes: 1 addition & 21 deletions src/models/event-timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,27 +68,7 @@ export class EventTimeline {
* @param toStartOfTimeline - if true the event's forwardLooking flag is set false
*/
public static setEventMetadata(event: MatrixEvent, stateContext: RoomState, toStartOfTimeline: boolean): void {
// When we try to generate a sentinel member before we have that member
// in the members object, we still generate a sentinel but it doesn't
// have a membership event, so test to see if events.member is set. We
// check this to avoid overriding non-sentinel members by sentinel ones
// when adding the event to a filtered timeline
if (!event.sender?.events?.member) {
event.sender = stateContext.getSentinelMember(event.getSender()!);
}
if (!event.target?.events?.member && event.getType() === EventType.RoomMember) {
event.target = stateContext.getSentinelMember(event.getStateKey()!);
}

if (event.isState()) {
// room state has no concept of 'old' or 'current', but we want the
// room state to regress back to previous values if toStartOfTimeline
// is set, which means inspecting prev_content if it exists. This
// is done by toggling the forwardLooking flag.
if (toStartOfTimeline) {
event.forwardLooking = false;
}
}
event.setMetadata(stateContext, toStartOfTimeline);
}

private readonly roomId: string | null;
Expand Down
49 changes: 49 additions & 0 deletions src/models/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { Room } from "./room.ts";
import { EventTimeline } from "./event-timeline.ts";
import { Membership } from "../@types/membership.ts";
import { DecryptionFailureCode } from "../crypto-api/index.ts";
import { RoomState } from "./room-state.ts";

export { EventStatus } from "./event-status.ts";

Expand Down Expand Up @@ -232,6 +233,7 @@ export enum MatrixEventEvent {
Status = "Event.status",
Replaced = "Event.replaced",
RelationsCreated = "Event.relationsCreated",
SentinelUpdated = "Event.sentinelUpdated",
}

export type MatrixEventEmittedEvents = MatrixEventEvent | ThreadEvent.Update;
Expand All @@ -244,6 +246,7 @@ export type MatrixEventHandlerMap = {
[MatrixEventEvent.Status]: (event: MatrixEvent, status: EventStatus | null) => void;
[MatrixEventEvent.Replaced]: (event: MatrixEvent) => void;
[MatrixEventEvent.RelationsCreated]: (relationType: string, eventType: string) => void;
[MatrixEventEvent.SentinelUpdated]: () => void;
} & Pick<ThreadEventHandlerMap, ThreadEvent.Update>;

export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, MatrixEventHandlerMap> {
Expand Down Expand Up @@ -328,13 +331,59 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
* Should be read-only
*/
public sender: RoomMember | null = null;

/**
* The room member who is the target of this event, e.g.
* the invitee, the person being banned, etc.
* @privateRemarks
* Should be read-only
*/
public target: RoomMember | null = null;

/**
* Update the sentinels and forwardLooking flag for this event.
* @param stateContext - the room state to be queried
* @param toStartOfTimeline - if true the event's forwardLooking flag is set false
* @internal
*/
public setMetadata(stateContext: RoomState, toStartOfTimeline: boolean): void {
// If an event is an m.room.member state event then we should set the sentinels again in case setEventMetadata
// was already called before the state was applied and thus the sentinel points at the member from before this event.
const affectsSelf =
this.isState() && this.getType() === EventType.RoomMember && this.getSender() === this.getStateKey();

let changed = false;
// When we try to generate a sentinel member before we have that member
// in the members object, we still generate a sentinel but it doesn't
// have a membership event, so test to see if events.member is set. We
// check this to avoid overriding non-sentinel members by sentinel ones
// when adding the event to a filtered timeline
if (affectsSelf || !this.sender?.events?.member) {
const newSender = stateContext.getSentinelMember(this.getSender()!);
if (newSender !== this.sender) changed = true;
this.sender = newSender;
}
if (affectsSelf || (!this.target?.events?.member && this.getType() === EventType.RoomMember)) {
const newTarget = stateContext.getSentinelMember(this.getStateKey()!);
if (newTarget !== this.target) changed = true;
this.target = newTarget;
}

if (this.isState()) {
// room state has no concept of 'old' or 'current', but we want the
// room state to regress back to previous values if toStartOfTimeline
// is set, which means inspecting prev_content if it exists. This
// is done by toggling the forwardLooking flag.
if (toStartOfTimeline) {
this.forwardLooking = false;
}
}

if (changed) {
this.emit(MatrixEventEvent.SentinelUpdated);
}
}

/**
* The sending status of the event.
* @privateRemarks
Expand Down
Loading