1
0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-07-31 15:24:23 +03:00

Fix issues with duplicated MatrixEvent objects around threads (#2256)

This commit is contained in:
Michael Telatynski
2022-03-24 12:24:19 +00:00
committed by GitHub
parent 6192325fe0
commit c541b3f1ce
10 changed files with 301 additions and 117 deletions

View File

@ -85,7 +85,7 @@ export function mkEvent(opts) {
room_id: opts.room,
sender: opts.sender || opts.user, // opts.user for backwards-compat
content: opts.content,
unsigned: opts.unsigned,
unsigned: opts.unsigned || {},
event_id: "$" + Math.random() + "-" + Math.random(),
};
if (opts.skey !== undefined) {

View File

@ -0,0 +1,180 @@
/*
Copyright 2022 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 { MatrixClient, MatrixEvent, MatrixEventEvent, MatrixScheduler, Room } from "../../src";
import { eventMapperFor } from "../../src/event-mapper";
import { IStore } from "../../src/store";
describe("eventMapperFor", function() {
let rooms: Room[] = [];
const userId = "@test:example.org";
let client: MatrixClient;
beforeEach(() => {
client = new MatrixClient({
baseUrl: "https://my.home.server",
accessToken: "my.access.token",
request: function() {} as any, // NOP
store: {
getRoom(roomId: string): Room | null {
return rooms.find(r => r.roomId === roomId);
},
} as IStore,
scheduler: {
setProcessFunction: jest.fn(),
} as unknown as MatrixScheduler,
userId: userId,
});
rooms = [];
});
it("should de-duplicate MatrixEvent instances by means of findEventById on the room object", async () => {
const roomId = "!room:example.org";
const room = new Room(roomId, client, userId);
rooms.push(room);
const mapper = eventMapperFor(client, {
preventReEmit: true,
decrypt: false,
});
const eventId = "$event1:server";
const eventDefinition = {
type: "m.room.message",
room_id: roomId,
sender: userId,
content: {
body: "body",
},
unsigned: {},
event_id: eventId,
};
const event = mapper(eventDefinition);
expect(event).toBeInstanceOf(MatrixEvent);
room.addLiveEvents([event]);
expect(room.findEventById(eventId)).toBe(event);
const event2 = mapper(eventDefinition);
expect(event).toBe(event2);
});
it("should not de-duplicate state events due to directionality of sentinel members", async () => {
const roomId = "!room:example.org";
const room = new Room(roomId, client, userId);
rooms.push(room);
const mapper = eventMapperFor(client, {
preventReEmit: true,
decrypt: false,
});
const eventId = "$event1:server";
const eventDefinition = {
type: "m.room.name",
room_id: roomId,
sender: userId,
content: {
name: "Room name",
},
unsigned: {},
event_id: eventId,
state_key: "",
};
const event = mapper(eventDefinition);
expect(event).toBeInstanceOf(MatrixEvent);
room.oldState.setStateEvents([event]);
room.currentState.setStateEvents([event]);
room.addLiveEvents([event]);
expect(room.findEventById(eventId)).toBe(event);
const event2 = mapper(eventDefinition);
expect(event).not.toBe(event2);
});
it("should decrypt appropriately", async () => {
const roomId = "!room:example.org";
const room = new Room(roomId, client, userId);
rooms.push(room);
const eventId = "$event1:server";
const eventDefinition = {
type: "m.room.encrypted",
room_id: roomId,
sender: userId,
content: {
ciphertext: "",
},
unsigned: {},
event_id: eventId,
};
const decryptEventIfNeededSpy = jest.spyOn(client, "decryptEventIfNeeded");
decryptEventIfNeededSpy.mockResolvedValue(); // stub it out
const mapper = eventMapperFor(client, {
decrypt: true,
});
const event = mapper(eventDefinition);
expect(event).toBeInstanceOf(MatrixEvent);
expect(decryptEventIfNeededSpy).toHaveBeenCalledWith(event);
});
it("should configure re-emitter appropriately", async () => {
const roomId = "!room:example.org";
const room = new Room(roomId, client, userId);
rooms.push(room);
const eventId = "$event1:server";
const eventDefinition = {
type: "m.room.message",
room_id: roomId,
sender: userId,
content: {
body: "body",
},
unsigned: {},
event_id: eventId,
};
const evListener = jest.fn();
client.on(MatrixEventEvent.Replaced, evListener);
const noReEmitMapper = eventMapperFor(client, {
preventReEmit: true,
});
const event1 = noReEmitMapper(eventDefinition);
expect(event1).toBeInstanceOf(MatrixEvent);
event1.emit(MatrixEventEvent.Replaced, event1);
expect(evListener).not.toHaveBeenCalled();
const reEmitMapper = eventMapperFor(client, {
preventReEmit: false,
});
const event2 = reEmitMapper(eventDefinition);
expect(event2).toBeInstanceOf(MatrixEvent);
event2.emit(MatrixEventEvent.Replaced, event2);
expect(evListener.mock.calls[0][0]).toEqual(event2);
expect(event1).not.toBe(event2); // the event wasn't added to a room so de-duplication wouldn't occur
});
});

View File

@ -1,6 +1,6 @@
/*
Copyright 2017 New Vector Ltd
Copyright 2019 The Matrix.org Foundaction C.I.C.
Copyright 2019 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.

View File

@ -1,3 +1,19 @@
/*
Copyright 2022 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 { logger } from "../../src/logger";
import { MatrixClient } from "../../src/client";
import { Filter } from "../../src/filter";

View File

@ -26,7 +26,6 @@ import { Room } from "../../src/models/room";
import { RoomState } from "../../src/models/room-state";
import { UNSTABLE_ELEMENT_FUNCTIONAL_USERS } from "../../src/@types/event";
import { TestClient } from "../TestClient";
import { Thread } from "../../src/models/thread";
describe("Room", function() {
const roomId = "!foo:bar";
@ -1867,54 +1866,6 @@ describe("Room", function() {
expect(() => room.createThread(rootEvent, [])).not.toThrow();
});
it("should not add events before server supports is known", function() {
Thread.hasServerSideSupport = undefined;
const rootEvent = new MatrixEvent({
event_id: "$666",
room_id: roomId,
content: {},
unsigned: {
"age": 1,
"m.relations": {
"m.thread": {
latest_event: null,
count: 1,
current_user_participated: false,
},
},
},
});
let age = 1;
function mkEvt(id): MatrixEvent {
return new MatrixEvent({
event_id: id,
room_id: roomId,
content: {
"m.relates_to": {
"rel_type": "m.thread",
"event_id": "$666",
},
},
unsigned: {
"age": age++,
},
});
}
const thread = room.createThread(rootEvent, []);
expect(thread.length).toBe(0);
thread.addEvent(mkEvt("$1"));
expect(thread.length).toBe(0);
Thread.hasServerSideSupport = true;
thread.addEvent(mkEvt("$2"));
expect(thread.length).toBeGreaterThan(0);
});
});
});
});

View File

@ -3951,7 +3951,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
/**
* Returns the eventType that should be used taking encryption into account
* for a given eventType.
* @param {MatrixClient} client the client
* @param {string} roomId the room for the events `eventType` relates to
* @param {string} eventType the event type
* @return {string} the event type taking encryption into account
@ -6621,11 +6620,10 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
fetchedEventType,
opts);
const mapper = this.getEventMapper();
let originalEvent: MatrixEvent;
if (result.original_event) {
originalEvent = mapper(result.original_event);
}
const originalEvent = result.original_event ? mapper(result.original_event) : undefined;
let events = result.chunk.map(mapper);
if (fetchedEventType === EventType.RoomMessageEncrypted) {
const allEvents = originalEvent ? events.concat(originalEvent) : events;
await Promise.all(allEvents.map(e => this.decryptEventIfNeeded(e)));
@ -6633,6 +6631,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
events = events.filter(e => e.getType() === eventType);
}
}
if (originalEvent && relationType === RelationType.Replace) {
events = events.filter(e => e.getSender() === originalEvent.getSender());
}
@ -8866,12 +8865,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
}
const parentEventId = event.getAssociatedId();
const parentEvent = room?.findEventById(parentEventId) || events.find((mxEv: MatrixEvent) => (
const parentEvent = room?.findEventById(parentEventId) ?? events.find((mxEv: MatrixEvent) => (
mxEv.getId() === parentEventId
));
// A reaction targetting the thread root needs to be routed to both the
// the main timeline and the associated thread
// A reaction targeting the thread root needs to be routed to both the main timeline and the associated thread
const targetingThreadRoot = parentEvent?.isThreadRoot || roots.has(event.relationEventId);
if (targetingThreadRoot) {
return {
@ -8887,18 +8885,21 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
// we want that redaction to be pushed to both timeline
if (parentEvent?.getAssociatedId()) {
return this.eventShouldLiveIn(parentEvent, room, events, roots);
} else {
// We've exhausted all scenarios, can safely assume that this event
// should live in the room timeline
return {
shouldLiveInRoom: true,
shouldLiveInThread: false,
};
}
// We've exhausted all scenarios, can safely assume that this event
// should live in the room timeline
return {
shouldLiveInRoom: true,
shouldLiveInThread: false,
};
}
public partitionThreadedEvents(events: MatrixEvent[]): [MatrixEvent[], MatrixEvent[]] {
// Indices to the events array, for readibility
public partitionThreadedEvents(events: MatrixEvent[]): [
timelineEvents: MatrixEvent[],
threadedEvents: MatrixEvent[],
] {
// Indices to the events array, for readability
const ROOM = 0;
const THREAD = 1;
if (this.supportsExperimentalThreads()) {

View File

@ -29,9 +29,22 @@ export function eventMapperFor(client: MatrixClient, options: MapperOpts): Event
const decrypt = options.decrypt !== false;
function mapper(plainOldJsObject: Partial<IEvent>) {
const event = new MatrixEvent(plainOldJsObject);
const room = client.getRoom(plainOldJsObject.room_id);
let event: MatrixEvent;
// If the event is already known to the room, let's re-use the model rather than duplicating.
// We avoid doing this to state events as they may be forward or backwards looking which tweaks behaviour.
if (room && plainOldJsObject.state_key === undefined) {
event = room.findEventById(plainOldJsObject.event_id);
}
if (!event || event.status) {
event = new MatrixEvent(plainOldJsObject);
} else {
// merge the latest unsigned data from the server
event.setUnsigned({ ...event.getUnsigned(), ...plainOldJsObject.unsigned });
}
const room = client.getRoom(event.getRoomId());
if (room?.threads.has(event.getId())) {
event.setThread(room.threads.get(event.getId()));
}
@ -46,6 +59,7 @@ export function eventMapperFor(client: MatrixClient, options: MapperOpts): Event
client.decryptEventIfNeeded(event);
}
}
if (!preventReEmit) {
client.reEmitter.reEmit(event, [
MatrixEventEvent.Replaced,

View File

@ -287,7 +287,7 @@ export class MatrixEvent extends TypedEventEmitter<EmittedEvents, MatrixEventHan
public target: RoomMember = null;
public status: EventStatus = null;
public error: MatrixError = null;
public forwardLooking = true;
public forwardLooking = true; // only state events may be backwards looking
/* If the event is a `m.key.verification.request` (or to_device `m.key.verification.start`) event,
* `Crypto` will set this the `VerificationRequest` for the event

View File

@ -48,6 +48,7 @@ import {
} from "./thread";
import { Method } from "../http-api";
import { TypedEventEmitter } from "./typed-event-emitter";
import { IMinimalEvent } from "../sync-accumulator";
// These constants are used as sane defaults when the homeserver doesn't support
// the m.room_versions capability. In practice, KNOWN_SAFE_ROOM_VERSION should be
@ -1005,17 +1006,15 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
}
/**
* Get an event which is stored in our unfiltered timeline set or in a thread
* Get an event which is stored in our unfiltered timeline set, or in a thread
*
* @param {string} eventId event ID to look for
* @param {string} eventId event ID to look for
* @return {?module:models/event.MatrixEvent} the given event, or undefined if unknown
*/
public findEventById(eventId: string): MatrixEvent | undefined {
let event = this.getUnfilteredTimelineSet().findEventById(eventId);
if (event) {
return event;
} else {
if (!event) {
const threads = this.getThreads();
for (let i = 0; i < threads.length; i++) {
const thread = threads[i];
@ -1025,6 +1024,8 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
}
}
}
return event;
}
/**
@ -1204,10 +1205,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
timeline: EventTimeline,
paginationToken?: string,
): void {
timeline.getTimelineSet().addEventsToTimeline(
events, toStartOfTimeline,
timeline, paginationToken,
);
timeline.getTimelineSet().addEventsToTimeline(events, toStartOfTimeline, timeline, paginationToken);
}
/**
@ -1592,10 +1590,9 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
} else {
const events = [event];
let rootEvent = this.findEventById(event.threadRootId);
// If the rootEvent does not exist in the current sync, then look for
// it over the network
// If the rootEvent does not exist in the current sync, then look for it over the network.
try {
let eventData;
let eventData: IMinimalEvent;
if (event.threadRootId) {
eventData = await this.client.fetchRoomEvent(this.roomId, event.threadRootId);
}
@ -1606,11 +1603,13 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
rootEvent.setUnsigned(eventData.unsigned);
}
} finally {
// The root event might be not be visible to the person requesting
// it. If it wasn't fetched successfully the thread will work
// in "limited" mode and won't benefit from all the APIs a homeserver
// can provide to enhance the thread experience
// The root event might be not be visible to the person requesting it.
// If it wasn't fetched successfully the thread will work in "limited" mode and won't
// benefit from all the APIs a homeserver can provide to enhance the thread experience
thread = this.createThread(rootEvent, events, toStartOfTimeline);
if (thread) {
rootEvent.setThread(thread);
}
}
}
@ -1672,7 +1671,7 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
}
}
applyRedaction(event: MatrixEvent): void {
private applyRedaction(event: MatrixEvent): void {
if (event.isRedaction()) {
const redactId = event.event.redacts;
@ -1888,6 +1887,14 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
}
}
private shouldAddEventToMainTimeline(thread: Thread, event: MatrixEvent): boolean {
if (!thread) {
return true;
}
return !event.isThreadRelation && thread.id === event.getAssociatedId();
}
/**
* Used to aggregate the local echo for a relation, and also
* for re-applying a relation after it's redaction has been cancelled,
@ -1900,11 +1907,9 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
*/
private aggregateNonLiveRelation(event: MatrixEvent): void {
const thread = this.findThreadForEvent(event);
if (thread) {
thread.timelineSet.aggregateRelations(event);
}
thread?.timelineSet.aggregateRelations(event);
if (thread?.id === event.getAssociatedId() || !thread) {
if (this.shouldAddEventToMainTimeline(thread, event)) {
// TODO: We should consider whether this means it would be a better
// design to lift the relations handling up to the room instead.
for (let i = 0; i < this.timelineSets.length; i++) {
@ -1961,11 +1966,9 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
localEvent.handleRemoteEcho(remoteEvent.event);
const thread = this.findThreadForEvent(remoteEvent);
if (thread) {
thread.timelineSet.handleRemoteEcho(localEvent, oldEventId, newEventId);
}
thread?.timelineSet.handleRemoteEcho(localEvent, oldEventId, newEventId);
if (thread?.id === remoteEvent.getAssociatedId() || !thread) {
if (this.shouldAddEventToMainTimeline(thread, remoteEvent)) {
for (let i = 0; i < this.timelineSets.length; i++) {
const timelineSet = this.timelineSets[i];
@ -2032,10 +2035,9 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
event.replaceLocalEventId(newEventId);
const thread = this.findThreadForEvent(event);
if (thread) {
thread.timelineSet.replaceEventId(oldEventId, newEventId);
}
if (thread?.id === event.getAssociatedId() || !thread) {
thread?.timelineSet.replaceEventId(oldEventId, newEventId);
if (this.shouldAddEventToMainTimeline(thread, event)) {
// if the event was already in the timeline (which will be the case if
// opts.pendingEventOrdering==chronological), we need to update the
// timeline map.
@ -2046,12 +2048,10 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
} else if (newStatus == EventStatus.CANCELLED) {
// remove it from the pending event list, or the timeline.
if (this.pendingEventList) {
const idx = this.pendingEventList.findIndex(ev => ev.getId() === oldEventId);
if (idx !== -1) {
const [removedEvent] = this.pendingEventList.splice(idx, 1);
if (removedEvent.isRedaction()) {
this.revertRedactionLocalEcho(removedEvent);
}
const removedEvent = this.getPendingEvent(oldEventId);
this.removePendingEvent(oldEventId);
if (removedEvent.isRedaction()) {
this.revertRedactionLocalEcho(removedEvent);
}
}
this.removeEvent(oldEventId);

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { MatrixClient, RoomEvent } from "../matrix";
import { MatrixClient, RelationType, RoomEvent } from "../matrix";
import { TypedReEmitter } from "../ReEmitter";
import { IRelationsRequestOpts } from "../@types/requests";
import { IThreadBundledRelationship, MatrixEvent } from "./event";
@ -24,6 +24,7 @@ import { Room } from './room';
import { TypedEventEmitter } from "./typed-event-emitter";
import { RoomState } from "./room-state";
import { ServerControlledNamespacedValue } from "../NamespacedValue";
import { logger } from "../logger";
export enum ThreadEvent {
New = "Thread.new",
@ -93,14 +94,9 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
RoomEvent.TimelineReset,
]);
// If we weren't able to find the root event, it's probably missing
// If we weren't able to find the root event, it's probably missing,
// and we define the thread ID from one of the thread relation
if (!rootEvent) {
this.id = opts?.initialEvents
?.find(event => event.isThreadRelation)?.relationEventId;
} else {
this.id = rootEvent.getId();
}
this.id = rootEvent?.getId() ?? opts?.initialEvents?.find(event => event.isThreadRelation)?.relationEventId;
this.initialiseThread(this.rootEvent);
opts?.initialEvents?.forEach(event => this.addEvent(event, false));
@ -169,10 +165,10 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
this.addEventToTimeline(event, toStartOfTimeline);
await this.client.decryptEventIfNeeded(event, {});
}
} else {
await this.fetchEditsWhereNeeded(event);
if (Thread.hasServerSideSupport && this.initialEventsFetched) {
if (event.localTimestamp > this.lastReply().localTimestamp) {
if (this.initialEventsFetched && event.localTimestamp > this.lastReply().localTimestamp) {
this.addEventToTimeline(event, false);
}
}
@ -221,10 +217,28 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
const event = new MatrixEvent(bundledRelationship.latest_event);
this.setEventMetadata(event);
event.setThread(this);
this.lastEvent = event;
this.fetchEditsWhereNeeded(event);
}
}
// XXX: Workaround for https://github.com/matrix-org/matrix-spec-proposals/pull/2676/files#r827240084
private async fetchEditsWhereNeeded(...events: MatrixEvent[]): Promise<unknown> {
return Promise.all(events.filter(e => e.isEncrypted()).map((event: MatrixEvent) => {
return this.client.relations(this.roomId, event.getId(), RelationType.Replace, event.getType(), {
limit: 1,
}).then(relations => {
if (relations.events.length) {
event.makeReplaced(relations.events[0]);
}
}).catch(e => {
logger.error("Failed to load edits for encrypted thread event", e);
});
}));
}
public async fetchInitialEvents(): Promise<{
originalEvent: MatrixEvent;
events: MatrixEvent[];
@ -235,6 +249,7 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
this.initialEventsFetched = true;
return null;
}
try {
const response = await this.fetchEvents();
this.initialEventsFetched = true;
@ -253,6 +268,11 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
* Finds an event by ID in the current thread
*/
public findEventById(eventId: string) {
// Check the lastEvent as it may have been created based on a bundled relationship and not in a timeline
if (this.lastEvent?.getId() === eventId) {
return this.lastEvent;
}
return this.timelineSet.findEventById(eventId);
}
@ -329,6 +349,8 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
events = [...events, originalEvent];
}
await this.fetchEditsWhereNeeded(...events);
await Promise.all(events.map(event => {
this.setEventMetadata(event);
return this.client.decryptEventIfNeeded(event);