diff --git a/spec/test-utils/test-utils.js b/spec/test-utils/test-utils.js index df137ba6f..b2c180205 100644 --- a/spec/test-utils/test-utils.js +++ b/spec/test-utils/test-utils.js @@ -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) { diff --git a/spec/unit/event-mapper.spec.ts b/spec/unit/event-mapper.spec.ts new file mode 100644 index 000000000..a46c955b7 --- /dev/null +++ b/spec/unit/event-mapper.spec.ts @@ -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 + }); +}); diff --git a/spec/unit/event.spec.js b/spec/unit/event.spec.js index a28b9224f..897d469b6 100644 --- a/spec/unit/event.spec.js +++ b/spec/unit/event.spec.js @@ -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. diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 67b922991..251ade94c 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -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"; diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index dbb5f33d5..6977c862f 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -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); - }); }); }); }); diff --git a/src/client.ts b/src/client.ts index 640fb724d..09d610b94 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3951,7 +3951,6 @@ export class MatrixClient extends TypedEventEmitter this.decryptEventIfNeeded(e))); @@ -6633,6 +6631,7 @@ export class MatrixClient extends TypedEventEmitter 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 ( + 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) { - 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, diff --git a/src/models/event.ts b/src/models/event.ts index 5d1dc784e..d630c17ac 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -287,7 +287,7 @@ export class MatrixEvent extends TypedEventEmitter } /** - * 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 } } } + + return event; } /** @@ -1204,10 +1205,7 @@ export class Room extends TypedEventEmitter 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 } 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 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 } } - 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 } } + 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 */ 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 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 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 } 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); diff --git a/src/models/thread.ts b/src/models/thread.ts index 3f9266e69..6ace74042 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -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 { 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 { 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 { 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 { + 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 { this.initialEventsFetched = true; return null; } + try { const response = await this.fetchEvents(); this.initialEventsFetched = true; @@ -253,6 +268,11 @@ export class Thread extends TypedEventEmitter { * 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 { events = [...events, originalEvent]; } + await this.fetchEditsWhereNeeded(...events); + await Promise.all(events.map(event => { this.setEventMetadata(event); return this.client.decryptEventIfNeeded(event);