From 631945084e05e64ada1a961ca36482eb29cfe475 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 09:30:51 +0100 Subject: [PATCH 01/12] Refactor Relations to not be per-EventTimelineSet --- src/models/event-timeline-set.ts | 138 +++++++-------------------- src/models/relations-container.ts | 151 ++++++++++++++++++++++++++++++ src/models/relations.ts | 14 +-- src/models/room.ts | 28 ++---- src/models/thread.ts | 4 +- 5 files changed, 201 insertions(+), 134 deletions(-) create mode 100644 src/models/relations-container.ts diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 8e5049c5f09..17d81b36810 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -19,7 +19,7 @@ limitations under the License. */ import { EventTimeline } from "./event-timeline"; -import { EventStatus, MatrixEvent, MatrixEventEvent } from "./event"; +import { MatrixEvent } from "./event"; import { logger } from '../logger'; import { Relations } from './relations'; import { Room, RoomEvent } from "./room"; @@ -27,6 +27,8 @@ import { Filter } from "../filter"; import { EventType, RelationType } from "../@types/event"; import { RoomState } from "./room-state"; import { TypedEventEmitter } from "./typed-event-emitter"; +import { RelationsContainer } from "./relations-container"; +import { MatrixClient } from "../client"; const DEBUG = true; @@ -64,14 +66,13 @@ export type EventTimelineSetHandlerMap = { }; export class EventTimelineSet extends TypedEventEmitter { + public readonly relations?: RelationsContainer; private readonly timelineSupport: boolean; - private unstableClientRelationAggregation: boolean; - private displayPendingEvents: boolean; + private readonly displayPendingEvents: boolean; private liveTimeline: EventTimeline; private timelines: EventTimeline[]; private _eventIdToTimeline: Record; private filter?: Filter; - private relations: Record>>; /** * Construct a set of EventTimeline objects, typically on behalf of a given @@ -108,13 +109,18 @@ export class EventTimelineSet extends TypedEventEmitter { - if (event.isDecryptionFailure()) { - // This could for example happen if the encryption keys are not yet available. - // The event may still be decrypted later. Register the listener again. - event.once(MatrixEventEvent.Decrypted, onEventDecrypted); - return; - } - - this.aggregateRelations(event); - }; - - // If the event is currently encrypted, wait until it has been decrypted. - if (event.isBeingDecrypted() || event.shouldAttemptDecryption()) { - event.once(MatrixEventEvent.Decrypted, onEventDecrypted); - return; - } - - const relation = event.getRelation(); - if (!relation) { - return; - } - - const relatesToEventId = relation.event_id; - const relationType = relation.rel_type; - const eventType = event.getType(); - - // debuglog("Aggregating relation: ", event.getId(), eventType, relation); - - let relationsForEvent: Record>> = this.relations[relatesToEventId]; - if (!relationsForEvent) { - relationsForEvent = this.relations[relatesToEventId] = {}; - } - let relationsWithRelType = relationsForEvent[relationType]; - if (!relationsWithRelType) { - relationsWithRelType = relationsForEvent[relationType] = {}; - } - let relationsWithEventType = relationsWithRelType[eventType]; - - if (!relationsWithEventType) { - relationsWithEventType = relationsWithRelType[eventType] = new Relations( - relationType, - eventType, - this.room, - ); - const relatesToEvent = this.findEventById(relatesToEventId) || this.room.getPendingEvent(relatesToEventId); - if (relatesToEvent) { - relationsWithEventType.setTargetEvent(relatesToEvent); - } - } - - relationsWithEventType.addEvent(event); + this.relations?.aggregateRelations(event, this); } } diff --git a/src/models/relations-container.ts b/src/models/relations-container.ts new file mode 100644 index 00000000000..8f3d19f448d --- /dev/null +++ b/src/models/relations-container.ts @@ -0,0 +1,151 @@ +/* +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 { Relations } from "./relations"; +import { EventType, RelationType } from "../@types/event"; +import { EventStatus, MatrixEvent, MatrixEventEvent } from "./event"; +import { EventTimelineSet } from "./event-timeline-set"; +import { MatrixClient } from "../client"; + +export class RelationsContainer { + // A tree of objects to access a set of relations for an event, as in: + // this.relations[relatesToEventId][relationType][relationEventType] + private relations: { + [relatesToEventId: string]: { + [relationType: RelationType | string]: { + [eventType: EventType | string]: Relations; + }; + }; + }; + + constructor(private readonly client: MatrixClient) { + } + + /** + * Get a collection of relations to a given event in this timeline set. + * + * @param {String} eventId + * The ID of the event that you'd like to access relation events for. + * For example, with annotations, this would be the ID of the event being annotated. + * @param {String} relationType + * The type of relation involved, such as "m.annotation", "m.reference", "m.replace", etc. + * @param {String} eventType + * The relation event's type, such as "m.reaction", etc. + * @throws If eventId, relationType or eventType + * are not valid. + * + * @returns {?Relations} + * A container for relation events or undefined if there are no relation events for + * the relationType. + */ + public getRelationsForEvent( + eventId: string, + relationType: RelationType | string, + eventType: EventType | string, + ): Relations | undefined { + return this.relations[eventId]?.[relationType]?.[eventType]; + } + + public getAllRelationsEventForEvent(eventId: string): MatrixEvent[] { + const relationsForEvent = this.relations?.[eventId] ?? {}; + const events: MatrixEvent[] = []; + for (const relationsRecord of Object.values(relationsForEvent)) { + for (const relations of Object.values(relationsRecord)) { + events.push(...relations.getRelations()); + } + } + return events; + } + + /** + * Set an event as the target event if any Relations exist for it already + * + * @param {MatrixEvent} event The event to check as relation target. + */ + public setRelationsTarget(event: MatrixEvent): void { + const relationsForEvent = this.relations[event.getId()]; + if (!relationsForEvent) return; + + for (const relationsWithRelType of Object.values(relationsForEvent)) { + for (const relationsWithEventType of Object.values(relationsWithRelType)) { + relationsWithEventType.setTargetEvent(event); + } + } + } + + /** + * Add relation events to the relevant relation collection. + * + * @param {MatrixEvent} event The new relation event to be aggregated. + * @param {EventTimelineSet} timelineSet The event timeline set within which to search for the related event if any. + */ + public async aggregateRelations(event: MatrixEvent, timelineSet?: EventTimelineSet): Promise { + if (event.isRedacted() || event.status === EventStatus.CANCELLED) { + return; + } + + const onEventDecrypted = (event: MatrixEvent) => { + if (event.isDecryptionFailure()) { + // This could for example happen if the encryption keys are not yet available. + // The event may still be decrypted later. Register the listener again. + event.once(MatrixEventEvent.Decrypted, onEventDecrypted); + return; + } + + this.aggregateRelations(event, timelineSet); + }; + + // If the event is currently encrypted, wait until it has been decrypted. + if (event.isBeingDecrypted() || event.shouldAttemptDecryption()) { + event.once(MatrixEventEvent.Decrypted, onEventDecrypted); + return; + } + + const relation = event.getRelation(); + if (!relation) return; + + const relatesToEventId = relation.event_id; + const relationType = relation.rel_type; + const eventType = event.getType(); + + let relationsForEvent = this.relations[relatesToEventId]; + if (!relationsForEvent) { + relationsForEvent = this.relations[relatesToEventId] = {}; + } + + let relationsWithRelType = relationsForEvent[relationType]; + if (!relationsWithRelType) { + relationsWithRelType = relationsForEvent[relationType] = {}; + } + + let relationsWithEventType = relationsWithRelType[eventType]; + if (!relationsWithEventType) { + relationsWithEventType = relationsWithRelType[eventType] = new Relations( + relationType, + eventType, + this.client, + ); + const relatesToEvent = timelineSet.findEventById(relatesToEventId) + ?? timelineSet.room?.findEventById(relatesToEventId) + ?? timelineSet.room?.getPendingEvent(relatesToEventId); + if (relatesToEvent) { + await relationsWithEventType.setTargetEvent(relatesToEvent); + } + } + + await relationsWithEventType.addEvent(event); + } +} diff --git a/src/models/relations.ts b/src/models/relations.ts index b3d0d235172..21d32390797 100644 --- a/src/models/relations.ts +++ b/src/models/relations.ts @@ -15,10 +15,11 @@ limitations under the License. */ import { EventStatus, IAggregatedRelation, MatrixEvent, MatrixEventEvent } from './event'; -import { Room } from './room'; import { logger } from '../logger'; import { RelationType } from "../@types/event"; import { TypedEventEmitter } from "./typed-event-emitter"; +import { MatrixClient } from "../client"; +import { Room } from "./room"; export enum RelationsEvent { Add = "Relations.add", @@ -48,6 +49,7 @@ export class Relations extends TypedEventEmitter][] = []; private targetEvent: MatrixEvent = null; private creationEmitted = false; + private readonly client: MatrixClient; /** * @param {RelationType} relationType @@ -55,16 +57,16 @@ export class Relations extends TypedEventEmitter * prefer getLiveTimeline().getState(EventTimeline.FORWARDS). */ public currentState: RoomState; + public readonly relations?: RelationsContainer; /** * @experimental @@ -375,6 +377,10 @@ export class Room extends TypedEventEmitter } else { this.membersPromise = null; } + + if (this.opts.unstableClientRelationAggregation) { + this.relations = new RelationsContainer(this.client); + } } private threadTimelineSetsPromise: Promise<[EventTimelineSet, EventTimelineSet]> | null = null; @@ -1657,8 +1663,7 @@ export class Room extends TypedEventEmitter toStartOfTimeline: boolean, ): Thread { if (rootEvent) { - const tl = this.getTimelineForEvent(rootEvent.getId()); - const relatedEvents = tl?.getTimelineSet().getAllRelationsEventForEvent(rootEvent.getId()); + const relatedEvents = this.relations?.getAllRelationsEventForEvent(rootEvent.getId()); if (relatedEvents?.length) { // Include all relations of the root event, given it'll be visible in both timelines, // except `m.replace` as that will already be applied atop the event using `MatrixEvent::makeReplaced` @@ -1934,24 +1939,7 @@ export class Room extends TypedEventEmitter * @param {module:models/event.MatrixEvent} event the relation event that needs to be aggregated. */ private aggregateNonLiveRelation(event: MatrixEvent): void { - const { shouldLiveInRoom, threadId } = this.eventShouldLiveIn(event); - const thread = this.getThread(threadId); - thread?.timelineSet.aggregateRelations(event); - - if (shouldLiveInRoom) { - // 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++) { - const timelineSet = this.timelineSets[i]; - if (timelineSet.getFilter()) { - if (timelineSet.getFilter().filterRoomTimeline([event]).length) { - timelineSet.aggregateRelations(event); - } - } else { - timelineSet.aggregateRelations(event); - } - } - } + this.relations?.aggregateRelations(event); } public getEventForTxnId(txnId: string): MatrixEvent { diff --git a/src/models/thread.ts b/src/models/thread.ts index 6ac5c985d4d..3326d3cc14d 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -229,8 +229,8 @@ export class Thread extends TypedEventEmitter { if ([RelationType.Annotation, RelationType.Replace].includes(event.getRelation()?.rel_type as RelationType)) { // Apply annotations and replace relations to the relations of the timeline only - this.timelineSet.setRelationsTarget(event); - this.timelineSet.aggregateRelations(event); + this.room.relations?.setRelationsTarget(event); + this.room.relations?.aggregateRelations(event, this.timelineSet); return; } From d471269ff2c82767c8d9cb59b20ea495c7f5e49e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 09:40:43 +0100 Subject: [PATCH 02/12] Fix comment and relations-container init --- src/models/event-timeline-set.ts | 2 +- src/models/relations-container.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 17d81b36810..acc483f708a 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -107,7 +107,7 @@ export class EventTimelineSet extends TypedEventEmitter Date: Mon, 30 May 2022 10:05:22 +0100 Subject: [PATCH 03/12] Revert timing tweaks --- spec/unit/event-timeline-set.spec.ts | 12 ++-- spec/unit/relations.spec.ts | 8 +-- spec/unit/room.spec.ts | 4 +- src/client.ts | 9 --- src/models/event-timeline-set.ts | 84 ++-------------------------- src/models/relations-container.ts | 19 +++---- src/models/room.ts | 13 +---- src/models/thread.ts | 5 +- src/sync.ts | 2 - 9 files changed, 29 insertions(+), 127 deletions(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index 82cadddf8f5..97f05b06690 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -39,8 +39,8 @@ describe('EventTimelineSet', () => { const itShouldReturnTheRelatedEvents = () => { it('should return the related events', () => { - eventTimelineSet.aggregateRelations(messageEvent); - const relations = eventTimelineSet.getRelationsForEvent( + eventTimelineSet.relations.aggregate(messageEvent); + const relations = eventTimelineSet.relations.getRelationsForEvent( messageEvent.getId(), "m.in_reply_to", EventType.RoomMessage, @@ -54,9 +54,7 @@ describe('EventTimelineSet', () => { beforeEach(() => { client = utils.mock(MatrixClient, 'MatrixClient'); room = new Room(roomId, client, userA); - eventTimelineSet = new EventTimelineSet(room, { - unstableClientRelationAggregation: true, - }); + eventTimelineSet = new EventTimelineSet(room); eventTimeline = new EventTimeline(eventTimelineSet); messageEvent = utils.mkMessage({ room: roomId, @@ -118,8 +116,8 @@ describe('EventTimelineSet', () => { }); it('should not return the related events', () => { - eventTimelineSet.aggregateRelations(messageEvent); - const relations = eventTimelineSet.getRelationsForEvent( + eventTimelineSet.relations.aggregate(messageEvent); + const relations = eventTimelineSet.relations.getRelationsForEvent( messageEvent.getId(), "m.in_reply_to", EventType.RoomMessage, diff --git a/spec/unit/relations.spec.ts b/spec/unit/relations.spec.ts index c1e1dc8271e..95f242d25ac 100644 --- a/spec/unit/relations.spec.ts +++ b/spec/unit/relations.spec.ts @@ -106,9 +106,7 @@ describe("Relations", function() { targetEvent.once(MatrixEventEvent.RelationsCreated, resolve); }); - const timelineSet = new EventTimelineSet(room, { - unstableClientRelationAggregation: true, - }); + const timelineSet = new EventTimelineSet(room); timelineSet.addLiveEvent(targetEvent); timelineSet.addLiveEvent(relationEvent); @@ -121,9 +119,7 @@ describe("Relations", function() { targetEvent.once(MatrixEventEvent.RelationsCreated, resolve); }); - const timelineSet = new EventTimelineSet(room, { - unstableClientRelationAggregation: true, - }); + const timelineSet = new EventTimelineSet(room); timelineSet.addLiveEvent(relationEvent); timelineSet.addLiveEvent(targetEvent); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 54d0f41dcb4..3ec1f6b6916 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -2274,7 +2274,7 @@ describe("Room", function() { const thread = threadRoot.getThread(); expect(thread.rootEvent).toBe(threadRoot); - const rootRelations = thread.timelineSet.getRelationsForEvent( + const rootRelations = thread.timelineSet.relations.getRelationsForEvent( threadRoot.getId(), RelationType.Annotation, EventType.Reaction, @@ -2284,7 +2284,7 @@ describe("Room", function() { expect(rootRelations[0][1].size).toEqual(1); expect(rootRelations[0][1].has(rootReaction)).toBeTruthy(); - const responseRelations = thread.timelineSet.getRelationsForEvent( + const responseRelations = thread.timelineSet.relations.getRelationsForEvent( threadResponse.getId(), RelationType.Annotation, EventType.Reaction, diff --git a/src/client.ts b/src/client.ts index 1e97cc80325..abf7dddace0 100644 --- a/src/client.ts +++ b/src/client.ts @@ -323,13 +323,6 @@ export interface ICreateClientOpts { */ sessionStore?: SessionStore; - /** - * Set to true to enable client-side aggregation of event relations - * via `EventTimelineSet#getRelationsForEvent`. - * This feature is currently unstable and the API may change without notice. - */ - unstableClientRelationAggregation?: boolean; - verificationMethods?: Array; /** @@ -905,7 +898,6 @@ export class MatrixClient extends TypedEventEmitter } = {}; - public unstableClientRelationAggregation = false; public identityServer: IIdentityServerProvider; public sessionStore: SessionStore; // XXX: Intended private, used in code. public http: MatrixHttpApi; // XXX: Intended private, used in code. @@ -1037,7 +1029,6 @@ export class MatrixClient extends TypedEventEmittereventId, relationType or eventType - * are not valid. - * - * @returns {?Relations} - * A container for relation events or undefined if there are no relation events for - * the relationType. - * @deprecated use `.relations.getRelationsForEvent` instead. - */ - public getRelationsForEvent( - eventId: string, - relationType: RelationType | string, - eventType: EventType | string, - ): Relations | undefined { - if (!this.relations) { - throw new Error("Client-side relation aggregation is disabled"); - } - - return this.relations?.getRelationsForEvent(eventId, relationType, eventType); - } - - /** - * @deprecated use `.relations.getAllRelationsEventForEvent` instead. - */ - public getAllRelationsEventForEvent(eventId: string): MatrixEvent[] | undefined { - return this.relations?.getAllRelationsEventForEvent(eventId); - } - - /** - * Set an event as the target event if any Relations exist for it already - * - * @param {MatrixEvent} event - * The event to check as relation target. - * @deprecated use `.relations.setRelationsTarget` instead. - */ - public setRelationsTarget(event: MatrixEvent): void { - this.relations?.setRelationsTarget(event); - } - - /** - * Add relation events to the relevant relation collection. - * - * @param {MatrixEvent} event - * The new relation event to be aggregated. - * @deprecated use `.relations.aggregateRelations` instead. - */ - public aggregateRelations(event: MatrixEvent): void { - this.relations?.aggregateRelations(event, this); - } } /** diff --git a/src/models/relations-container.ts b/src/models/relations-container.ts index 3813ec26b36..c7166d13d58 100644 --- a/src/models/relations-container.ts +++ b/src/models/relations-container.ts @@ -60,7 +60,7 @@ export class RelationsContainer { } public getAllRelationsEventForEvent(eventId: string): MatrixEvent[] { - const relationsForEvent = this.relations?.[eventId] ?? {}; + const relationsForEvent = this.relations[eventId] ?? {}; const events: MatrixEvent[] = []; for (const relationsRecord of Object.values(relationsForEvent)) { for (const relations of Object.values(relationsRecord)) { @@ -92,11 +92,14 @@ export class RelationsContainer { * @param {MatrixEvent} event The new relation event to be aggregated. * @param {EventTimelineSet} timelineSet The event timeline set within which to search for the related event if any. */ - public async aggregateRelations(event: MatrixEvent, timelineSet?: EventTimelineSet): Promise { + public async aggregate(event: MatrixEvent, timelineSet?: EventTimelineSet): Promise { if (event.isRedacted() || event.status === EventStatus.CANCELLED) { return; } + const relation = event.getRelation(); + if (!relation) return; + const onEventDecrypted = (event: MatrixEvent) => { if (event.isDecryptionFailure()) { // This could for example happen if the encryption keys are not yet available. @@ -105,7 +108,7 @@ export class RelationsContainer { return; } - this.aggregateRelations(event, timelineSet); + this.aggregate(event, timelineSet); }; // If the event is currently encrypted, wait until it has been decrypted. @@ -114,11 +117,7 @@ export class RelationsContainer { return; } - const relation = event.getRelation(); - if (!relation) return; - - const relatesToEventId = relation.event_id; - const relationType = relation.rel_type; + const { event_id: relatesToEventId, rel_type: relationType } = relation; const eventType = event.getType(); let relationsForEvent = this.relations[relatesToEventId]; @@ -142,10 +141,10 @@ export class RelationsContainer { ?? timelineSet.room?.findEventById(relatesToEventId) ?? timelineSet.room?.getPendingEvent(relatesToEventId); if (relatesToEvent) { - await relationsWithEventType.setTargetEvent(relatesToEvent); + relationsWithEventType.setTargetEvent(relatesToEvent); } } - await relationsWithEventType.addEvent(event); + relationsWithEventType.addEvent(event); } } diff --git a/src/models/room.ts b/src/models/room.ts index b926d13c445..d77c3d3b147 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -81,7 +81,6 @@ interface IOpts { storageToken?: string; pendingEventOrdering?: PendingEventOrdering; timelineSupport?: boolean; - unstableClientRelationAggregation?: boolean; lazyLoadMembers?: boolean; } @@ -324,10 +323,6 @@ export class Room extends TypedEventEmitter * "chronological". * @param {boolean} [opts.timelineSupport = false] Set to true to enable improved * timeline support. - * @param {boolean} [opts.unstableClientRelationAggregation = false] - * Optional. Set to true to enable client-side aggregation of event relations - * via `EventTimelineSet#getRelationsForEvent`. - * This feature is currently unstable and the API may change without notice. */ constructor( public readonly roomId: string, @@ -378,9 +373,7 @@ export class Room extends TypedEventEmitter this.membersPromise = null; } - if (this.opts.unstableClientRelationAggregation) { - this.relations = new RelationsContainer(this.client); - } + this.relations = new RelationsContainer(this.client); } private threadTimelineSetsPromise: Promise<[EventTimelineSet, EventTimelineSet]> | null = null; @@ -1663,7 +1656,7 @@ export class Room extends TypedEventEmitter toStartOfTimeline: boolean, ): Thread { if (rootEvent) { - const relatedEvents = this.relations?.getAllRelationsEventForEvent(rootEvent.getId()); + const relatedEvents = this.relations.getAllRelationsEventForEvent(rootEvent.getId()); if (relatedEvents?.length) { // Include all relations of the root event, given it'll be visible in both timelines, // except `m.replace` as that will already be applied atop the event using `MatrixEvent::makeReplaced` @@ -1939,7 +1932,7 @@ export class Room extends TypedEventEmitter * @param {module:models/event.MatrixEvent} event the relation event that needs to be aggregated. */ private aggregateNonLiveRelation(event: MatrixEvent): void { - this.relations?.aggregateRelations(event); + this.relations.aggregate(event); } public getEventForTxnId(txnId: string): MatrixEvent { diff --git a/src/models/thread.ts b/src/models/thread.ts index 3326d3cc14d..96584b27029 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -88,7 +88,6 @@ export class Thread extends TypedEventEmitter { this.room = opts.room; this.client = opts.client; this.timelineSet = new EventTimelineSet(this.room, { - unstableClientRelationAggregation: true, timelineSupport: true, pendingEvents: true, }); @@ -229,8 +228,8 @@ export class Thread extends TypedEventEmitter { if ([RelationType.Annotation, RelationType.Replace].includes(event.getRelation()?.rel_type as RelationType)) { // Apply annotations and replace relations to the relations of the timeline only - this.room.relations?.setRelationsTarget(event); - this.room.relations?.aggregateRelations(event, this.timelineSet); + this.room.relations.setRelationsTarget(event); + this.room.relations.aggregate(event, this.timelineSet); return; } diff --git a/src/sync.ts b/src/sync.ts index 794fd88b250..d2dd1d5df51 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -184,13 +184,11 @@ export class SyncApi { const client = this.client; const { timelineSupport, - unstableClientRelationAggregation, } = client; const room = new Room(roomId, client, client.getUserId(), { lazyLoadMembers: this.opts.lazyLoadMembers, pendingEventOrdering: this.opts.pendingEventOrdering, timelineSupport, - unstableClientRelationAggregation, }); client.reEmitter.reEmit(room, [ RoomEvent.Name, From 51a77341d6465cbcbd0b1381e23ebc35e3185df4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 10:12:51 +0100 Subject: [PATCH 04/12] Fix relations order test --- spec/unit/relations.spec.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/unit/relations.spec.ts b/spec/unit/relations.spec.ts index 95f242d25ac..b35ce5d2f3b 100644 --- a/spec/unit/relations.spec.ts +++ b/spec/unit/relations.spec.ts @@ -96,12 +96,9 @@ describe("Relations", function() { }, }); - // Stub the room - - const room = new Room("room123", null, null); - // Add the target event first, then the relation event { + const room = new Room("room123", null, null); const relationsCreated = new Promise(resolve => { targetEvent.once(MatrixEventEvent.RelationsCreated, resolve); }); @@ -115,6 +112,7 @@ describe("Relations", function() { // Add the relation event first, then the target event { + const room = new Room("room123", null, null); const relationsCreated = new Promise(resolve => { targetEvent.once(MatrixEventEvent.RelationsCreated, resolve); }); From 8c187406128d005331a05c1b8710216b79c9a4ef Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 10:36:08 +0100 Subject: [PATCH 05/12] Add test and simplify thread relations handling --- spec/unit/relations.spec.ts | 7 +++++++ src/models/relations-container.ts | 4 ++-- src/models/thread.ts | 9 ++------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/spec/unit/relations.spec.ts b/spec/unit/relations.spec.ts index b35ce5d2f3b..007c0d78157 100644 --- a/spec/unit/relations.spec.ts +++ b/spec/unit/relations.spec.ts @@ -125,6 +125,13 @@ describe("Relations", function() { } }); + it("should re-use Relations between all timeline sets in a room", async () => { + const room = new Room("room123", null, null); + const timelineSet1 = new EventTimelineSet(room); + const timelineSet2 = new EventTimelineSet(room); + expect(timelineSet1.relations).toBe(timelineSet2.relations); + }); + it("should ignore m.replace for state events", async () => { const userId = "@bob:example.com"; const room = new Room("room123", null, userId); diff --git a/src/models/relations-container.ts b/src/models/relations-container.ts index c7166d13d58..09cb32572ff 100644 --- a/src/models/relations-container.ts +++ b/src/models/relations-container.ts @@ -92,7 +92,7 @@ export class RelationsContainer { * @param {MatrixEvent} event The new relation event to be aggregated. * @param {EventTimelineSet} timelineSet The event timeline set within which to search for the related event if any. */ - public async aggregate(event: MatrixEvent, timelineSet?: EventTimelineSet): Promise { + public aggregate(event: MatrixEvent, timelineSet?: EventTimelineSet): void { if (event.isRedacted() || event.status === EventStatus.CANCELLED) { return; } @@ -100,7 +100,7 @@ export class RelationsContainer { const relation = event.getRelation(); if (!relation) return; - const onEventDecrypted = (event: MatrixEvent) => { + const onEventDecrypted = () => { if (event.isDecryptionFailure()) { // This could for example happen if the encryption keys are not yet available. // The event may still be decrypted later. Register the listener again. diff --git a/src/models/thread.ts b/src/models/thread.ts index 96584b27029..e6f27cbf553 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -165,6 +165,7 @@ export class Thread extends TypedEventEmitter { private onEcho = (event: MatrixEvent) => { if (event.threadRootId !== this.id) return; // ignore echoes for other timelines if (this.lastEvent === event) return; + if (!event.isRelation(THREAD_RELATION_TYPE.name)) return; // There is a risk that the `localTimestamp` approximation will not be accurate // when threads are used over federation. That could result in the reply @@ -226,13 +227,6 @@ export class Thread extends TypedEventEmitter { this._currentUserParticipated = true; } - if ([RelationType.Annotation, RelationType.Replace].includes(event.getRelation()?.rel_type as RelationType)) { - // Apply annotations and replace relations to the relations of the timeline only - this.room.relations.setRelationsTarget(event); - this.room.relations.aggregate(event, this.timelineSet); - return; - } - // Add all incoming events to the thread's timeline set when there's no server support if (!Thread.hasServerSideSupport) { // all the relevant membership info to hydrate events with a sender @@ -290,6 +284,7 @@ export class Thread extends TypedEventEmitter { // 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) => { + if (event.isRelation()) return; // skip - relations don't get edits return this.client.relations(this.roomId, event.getId(), RelationType.Replace, event.getType(), { limit: 1, }).then(relations => { From a2a423b5d3db5b5b9258b766ac82545079e59b34 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 10:49:35 +0100 Subject: [PATCH 06/12] Fix order of initialising a room object --- src/models/room.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index d77c3d3b147..b95be70d150 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -261,7 +261,7 @@ export class Room extends TypedEventEmitter * prefer getLiveTimeline().getState(EventTimeline.FORWARDS). */ public currentState: RoomState; - public readonly relations?: RelationsContainer; + public readonly relations = new RelationsContainer(this.client); /** * @experimental @@ -372,8 +372,6 @@ export class Room extends TypedEventEmitter } else { this.membersPromise = null; } - - this.relations = new RelationsContainer(this.client); } private threadTimelineSetsPromise: Promise<[EventTimelineSet, EventTimelineSet]> | null = null; From 3d7f8c2be360df1e133433021f3518a1f2d610e5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 10:50:05 +0100 Subject: [PATCH 07/12] Fix test --- spec/unit/relations.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/unit/relations.spec.ts b/spec/unit/relations.spec.ts index 007c0d78157..091d95ea914 100644 --- a/spec/unit/relations.spec.ts +++ b/spec/unit/relations.spec.ts @@ -129,7 +129,8 @@ describe("Relations", function() { const room = new Room("room123", null, null); const timelineSet1 = new EventTimelineSet(room); const timelineSet2 = new EventTimelineSet(room); - expect(timelineSet1.relations).toBe(timelineSet2.relations); + expect(room.relations).toBe(timelineSet1.relations); + expect(room.relations).toBe(timelineSet2.relations); }); it("should ignore m.replace for state events", async () => { From acd32b467145401773154f006d75da42f6a3fdab Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 10:58:01 +0100 Subject: [PATCH 08/12] Re-add thread handling for relations of unloaded threads --- src/models/thread.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/models/thread.ts b/src/models/thread.ts index e6f27cbf553..32a9cd06d1c 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -242,6 +242,11 @@ export class Thread extends TypedEventEmitter { ) { this.fetchEditsWhereNeeded(event); this.addEventToTimeline(event, false); + } else if (event.isRelation(RelationType.Annotation) || event.isRelation(RelationType.Replace)) { + // Apply annotations and replace relations to the relations of the timeline only + this.timelineSet.relations.setRelationsTarget(event); + this.timelineSet.relations.aggregate(event, this.timelineSet); + return; } // If no thread support exists we want to count all thread relation From 6711fbe562e9a2d7982add0ed60d08e8ce831b1a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 11:31:58 +0100 Subject: [PATCH 09/12] Ditch confusing experimental getter `MatrixEvent::isThreadRelation` --- src/models/event.ts | 7 ------- src/models/relations-container.ts | 2 +- src/models/room.ts | 4 ++-- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/models/event.ts b/src/models/event.ts index 7e4de8e2251..15d0649c48d 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -514,13 +514,6 @@ export class MatrixEvent extends TypedEventEmitter } // A thread relation is always only shown in a thread - if (event.isThreadRelation) { + if (event.isRelation(THREAD_RELATION_TYPE.name)) { return { shouldLiveInRoom: false, shouldLiveInThread: true, @@ -2192,7 +2192,7 @@ export class Room extends TypedEventEmitter private findThreadRoots(events: MatrixEvent[]): Set { const threadRoots = new Set(); for (const event of events) { - if (event.isThreadRelation) { + if (event.isRelation(THREAD_RELATION_TYPE.name)) { threadRoots.add(event.relationEventId); } } From fa1a01d39e0e9f85407584ba0885ae7509267f3e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 30 May 2022 11:52:20 +0100 Subject: [PATCH 10/12] Fix room handling in RelationsContainer --- src/models/relations-container.ts | 9 ++++++--- src/models/room.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/models/relations-container.ts b/src/models/relations-container.ts index c9253253ba3..889d8944d33 100644 --- a/src/models/relations-container.ts +++ b/src/models/relations-container.ts @@ -19,6 +19,7 @@ import { EventType, RelationType } from "../@types/event"; import { EventStatus, MatrixEvent, MatrixEventEvent } from "./event"; import { EventTimelineSet } from "./event-timeline-set"; import { MatrixClient } from "../client"; +import { Room } from "./room"; export class RelationsContainer { // A tree of objects to access a set of relations for an event, as in: @@ -31,7 +32,7 @@ export class RelationsContainer { }; } = {}; - constructor(private readonly client: MatrixClient) { + constructor(private readonly client: MatrixClient, private readonly room?: Room) { } /** @@ -137,9 +138,11 @@ export class RelationsContainer { eventType, this.client, ); + + const room = this.room ?? timelineSet?.room; const relatesToEvent = timelineSet?.findEventById(relatesToEventId) - ?? timelineSet.room?.findEventById(relatesToEventId) - ?? timelineSet.room?.getPendingEvent(relatesToEventId); + ?? room?.findEventById(relatesToEventId) + ?? room?.getPendingEvent(relatesToEventId); if (relatesToEvent) { relationsWithEventType.setTargetEvent(relatesToEvent); } diff --git a/src/models/room.ts b/src/models/room.ts index ffa04a9f77c..993fd2b21ef 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -261,7 +261,7 @@ export class Room extends TypedEventEmitter * prefer getLiveTimeline().getState(EventTimeline.FORWARDS). */ public currentState: RoomState; - public readonly relations = new RelationsContainer(this.client); + public readonly relations = new RelationsContainer(this.client, this); /** * @experimental From 9720d8fcda2bbf8b550d7cff3d51193062718c36 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jun 2022 15:13:00 +0100 Subject: [PATCH 11/12] Iterate PR --- src/models/relations-container.ts | 2 +- src/models/room.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/models/relations-container.ts b/src/models/relations-container.ts index 889d8944d33..60b1a84b37f 100644 --- a/src/models/relations-container.ts +++ b/src/models/relations-container.ts @@ -60,7 +60,7 @@ export class RelationsContainer { return this.relations[eventId]?.[relationType]?.[eventType]; } - public getAllRelationsEventForEvent(eventId: string): MatrixEvent[] { + public getAllRelationsEventsForEvent(eventId: string): MatrixEvent[] { const relationsForEvent = this.relations[eventId] ?? {}; const events: MatrixEvent[] = []; for (const relationsRecord of Object.values(relationsForEvent)) { diff --git a/src/models/room.ts b/src/models/room.ts index 993fd2b21ef..6c4608c7a38 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -1654,7 +1654,7 @@ export class Room extends TypedEventEmitter toStartOfTimeline: boolean, ): Thread { if (rootEvent) { - const relatedEvents = this.relations.getAllRelationsEventForEvent(rootEvent.getId()); + const relatedEvents = this.relations.getAllRelationsEventsForEvent(rootEvent.getId()); if (relatedEvents?.length) { // Include all relations of the root event, given it'll be visible in both timelines, // except `m.replace` as that will already be applied atop the event using `MatrixEvent::makeReplaced` From 68a7e8e1d001728c0e5e2cd25e2a9c9bcd372dd5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jun 2022 15:26:46 +0100 Subject: [PATCH 12/12] Tweak method naming to closer match spec --- spec/unit/event-timeline-set.spec.ts | 8 ++++---- spec/unit/room.spec.ts | 4 ++-- src/models/event-timeline-set.ts | 4 ++-- src/models/relations-container.ts | 30 +++++++++++++++------------- src/models/room.ts | 4 ++-- src/models/thread.ts | 4 ++-- 6 files changed, 28 insertions(+), 26 deletions(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index 1add1789eac..053a78e46c5 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -40,8 +40,8 @@ describe('EventTimelineSet', () => { const itShouldReturnTheRelatedEvents = () => { it('should return the related events', () => { - eventTimelineSet.relations.aggregate(messageEvent); - const relations = eventTimelineSet.relations.getRelationsForEvent( + eventTimelineSet.relations.aggregateChildEvent(messageEvent); + const relations = eventTimelineSet.relations.getChildEventsForEvent( messageEvent.getId(), "m.in_reply_to", EventType.RoomMessage, @@ -187,8 +187,8 @@ describe('EventTimelineSet', () => { }); it('should not return the related events', () => { - eventTimelineSet.relations.aggregate(messageEvent); - const relations = eventTimelineSet.relations.getRelationsForEvent( + eventTimelineSet.relations.aggregateChildEvent(messageEvent); + const relations = eventTimelineSet.relations.getChildEventsForEvent( messageEvent.getId(), "m.in_reply_to", EventType.RoomMessage, diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 5de9f43247a..3807795d1f3 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -2334,7 +2334,7 @@ describe("Room", function() { const thread = threadRoot.getThread(); expect(thread.rootEvent).toBe(threadRoot); - const rootRelations = thread.timelineSet.relations.getRelationsForEvent( + const rootRelations = thread.timelineSet.relations.getChildEventsForEvent( threadRoot.getId(), RelationType.Annotation, EventType.Reaction, @@ -2344,7 +2344,7 @@ describe("Room", function() { expect(rootRelations[0][1].size).toEqual(1); expect(rootRelations[0][1].has(rootReaction)).toBeTruthy(); - const responseRelations = thread.timelineSet.relations.getRelationsForEvent( + const responseRelations = thread.timelineSet.relations.getChildEventsForEvent( threadResponse.getId(), RelationType.Annotation, EventType.Reaction, diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 03cbed7ad9f..aae2f999733 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -696,8 +696,8 @@ export class EventTimelineSet extends TypedEventEmittereventId, relationType or eventType @@ -52,7 +52,7 @@ export class RelationsContainer { * A container for relation events or undefined if there are no relation events for * the relationType. */ - public getRelationsForEvent( + public getChildEventsForEvent( eventId: string, relationType: RelationType | string, eventType: EventType | string, @@ -60,8 +60,8 @@ export class RelationsContainer { return this.relations[eventId]?.[relationType]?.[eventType]; } - public getAllRelationsEventsForEvent(eventId: string): MatrixEvent[] { - const relationsForEvent = this.relations[eventId] ?? {}; + public getAllChildEventsForEvent(parentEventId: string): MatrixEvent[] { + const relationsForEvent = this.relations[parentEventId] ?? {}; const events: MatrixEvent[] = []; for (const relationsRecord of Object.values(relationsForEvent)) { for (const relations of Object.values(relationsRecord)) { @@ -72,11 +72,13 @@ export class RelationsContainer { } /** - * Set an event as the target event if any Relations exist for it already + * Set an event as the target event if any Relations exist for it already. + * Child events can point to other child events as their parent, so this method may be + * called for events which are also logically child events. * * @param {MatrixEvent} event The event to check as relation target. */ - public setRelationsTarget(event: MatrixEvent): void { + public aggregateParentEvent(event: MatrixEvent): void { const relationsForEvent = this.relations[event.getId()]; if (!relationsForEvent) return; @@ -90,10 +92,10 @@ export class RelationsContainer { /** * Add relation events to the relevant relation collection. * - * @param {MatrixEvent} event The new relation event to be aggregated. + * @param {MatrixEvent} event The new child event to be aggregated. * @param {EventTimelineSet} timelineSet The event timeline set within which to search for the related event if any. */ - public aggregate(event: MatrixEvent, timelineSet?: EventTimelineSet): void { + public aggregateChildEvent(event: MatrixEvent, timelineSet?: EventTimelineSet): void { if (event.isRedacted() || event.status === EventStatus.CANCELLED) { return; } @@ -109,7 +111,7 @@ export class RelationsContainer { return; } - this.aggregate(event, timelineSet); + this.aggregateChildEvent(event, timelineSet); }; // If the event is currently encrypted, wait until it has been decrypted. diff --git a/src/models/room.ts b/src/models/room.ts index 1a30428be2d..e2e09e4773b 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -1813,7 +1813,7 @@ export class Room extends TypedEventEmitter toStartOfTimeline: boolean, ): Thread { if (rootEvent) { - const relatedEvents = this.relations.getAllRelationsEventsForEvent(rootEvent.getId()); + const relatedEvents = this.relations.getAllChildEventsForEvent(rootEvent.getId()); if (relatedEvents?.length) { // Include all relations of the root event, given it'll be visible in both timelines, // except `m.replace` as that will already be applied atop the event using `MatrixEvent::makeReplaced` @@ -2098,7 +2098,7 @@ export class Room extends TypedEventEmitter * @param {module:models/event.MatrixEvent} event the relation event that needs to be aggregated. */ private aggregateNonLiveRelation(event: MatrixEvent): void { - this.relations.aggregate(event); + this.relations.aggregateChildEvent(event); } public getEventForTxnId(txnId: string): MatrixEvent { diff --git a/src/models/thread.ts b/src/models/thread.ts index 08e3314035b..28bb9933d92 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -246,8 +246,8 @@ export class Thread extends TypedEventEmitter { this.addEventToTimeline(event, false); } else if (event.isRelation(RelationType.Annotation) || event.isRelation(RelationType.Replace)) { // Apply annotations and replace relations to the relations of the timeline only - this.timelineSet.relations.setRelationsTarget(event); - this.timelineSet.relations.aggregate(event, this.timelineSet); + this.timelineSet.relations.aggregateParentEvent(event); + this.timelineSet.relations.aggregateChildEvent(event, this.timelineSet); return; }