From bbcf0d2572332f9e21ef778ebed231fa39cf0c98 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 16 Jun 2021 12:46:44 +0200 Subject: [PATCH] more local echo fixes for redacting a reaction + cleanup --- .../room/timeline/PendingAnnotations.js | 12 +-- src/matrix/room/timeline/Timeline.js | 81 +++++++++++-------- .../room/timeline/entries/BaseEventEntry.js | 24 ++++-- .../timeline/entries/PendingEventEntry.js | 24 +++--- 4 files changed, 78 insertions(+), 63 deletions(-) diff --git a/src/matrix/room/timeline/PendingAnnotations.js b/src/matrix/room/timeline/PendingAnnotations.js index 05495bc3..1dd32abd 100644 --- a/src/matrix/room/timeline/PendingAnnotations.js +++ b/src/matrix/room/timeline/PendingAnnotations.js @@ -14,8 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {getRelationFromContent} from "./relations.js"; - export class PendingAnnotations { constructor() { this.aggregatedAnnotations = new Map(); @@ -25,7 +23,7 @@ export class PendingAnnotations { /** adds either a pending annotation entry, or a remote annotation entry with a pending redaction */ add(entry) { - const {key} = entry.ownOrRedactedRelation; + const {key} = (entry.redactingEntry || entry).relation; if (!key) { return; } @@ -42,7 +40,7 @@ export class PendingAnnotations { return; } this._entries.splice(idx, 1); - const {key} = entry.ownOrRedactedRelation; + const {key} = (entry.redactingEntry || entry).relation; let count = this.aggregatedAnnotations.get(key); if (count !== undefined) { const addend = entry.isRedaction ? 1 : -1; @@ -56,8 +54,7 @@ export class PendingAnnotations { findForKey(key) { return this._entries.find(e => { - const relation = getRelationFromContent(e.content); - if (relation && relation.key === key) { + if (e.relation?.key === key) { return e; } }); @@ -65,8 +62,7 @@ export class PendingAnnotations { findRedactionForKey(key) { return this._entries.find(e => { - const relation = e.redactingRelation; - if (relation && relation.key === key) { + if (e.redactingEntry?.relation?.key === key) { return e; } }); diff --git a/src/matrix/room/timeline/Timeline.js b/src/matrix/room/timeline/Timeline.js index c73fe9db..76b6bf5f 100644 --- a/src/matrix/room/timeline/Timeline.js +++ b/src/matrix/room/timeline/Timeline.js @@ -22,7 +22,7 @@ import {TimelineReader} from "./persistence/TimelineReader.js"; import {PendingEventEntry} from "./entries/PendingEventEntry.js"; import {RoomMember} from "../members/RoomMember.js"; import {PowerLevels} from "./PowerLevels.js"; -import {getRelation, getRelationFromContent, ANNOTATION_RELATION_TYPE} from "./relations.js"; +import {getRelation, ANNOTATION_RELATION_TYPE} from "./relations.js"; import {REDACTION_TYPE} from "../common.js"; export class Timeline { @@ -120,42 +120,36 @@ export class Timeline { // so if we are redacting a relation, we can pass the redaction // to the relation target and the removal of the relation can // be taken into account for local echo. - let redactingRelation; + let redactingEntry; if (pe.eventType === REDACTION_TYPE) { - if (pe.relatedEventId) { - const txn = await this._storage.readWriteTxn([ - this._storage.storeNames.timelineEvents, - ]); - const redactionTargetEntry = await txn.timelineEvents.getByEventId(this._roomId, pe.relatedEventId); - if (redactionTargetEntry) { - redactingRelation = getRelation(redactionTargetEntry.event); - } - } else if (pe.relatedTxnId) { - // also look for redacting relation in pending events, in case the target is already being sent - for (const p of this._localEntries) { - if (p.id === pe.relatedTxnId) { - redactingRelation = getRelationFromContent(p.content); - break; - } - } - } + redactingEntry = await this._getOrLoadEntry(pe.relatedTxnId, pe.relatedEventId); } const pee = new PendingEventEntry({ pendingEvent: pe, member: this._ownMember, - clock: this._clock, redactingRelation + clock: this._clock, redactingEntry }); this._applyAndEmitLocalRelationChange(pee, target => target.addLocalRelation(pee)); return pee; } - _applyAndEmitLocalRelationChange(pee, updater) { + // this is the contract of findAndUpdate, used in _findAndUpdateRelatedEntry const updateOrFalse = e => { const params = updater(e); return params ? params : false; }; + this._findAndUpdateRelatedEntry(pee.pendingEvent.relatedTxnId, pee.relatedEventId, updateOrFalse); + // also look for a relation target to update with this redaction + const {redactingEntry} = pee; + if (redactingEntry) { + // redactingEntry might be a PendingEventEntry or an EventEntry, so don't assume pendingEvent + const relatedTxnId = redactingEntry.pendingEvent?.relatedTxnId; + this._findAndUpdateRelatedEntry(relatedTxnId, redactingEntry.relatedEventId, updateOrFalse); + } + } + + _findAndUpdateRelatedEntry(relatedTxnId, relatedEventId, updateOrFalse) { let found = false; - const {relatedTxnId} = pee.pendingEvent; // first, look in local entries based on txn id if (relatedTxnId) { found = this._localEntries.findAndUpdate( @@ -164,18 +158,9 @@ export class Timeline { ); } // if not found here, look in remote entries based on event id - if (!found && pee.relatedEventId) { + if (!found && relatedEventId) { this._remoteEntries.findAndUpdate( - e => e.id === pee.relatedEventId, - updateOrFalse - ); - } - // also look for a relation target to update with this redaction - if (pee.redactingRelation) { - const eventId = pee.redactingRelation.event_id; - // TODO: also support reacting to pending entries - this._remoteEntries.findAndUpdate( - e => e.id === eventId, + e => e.id === relatedEventId, updateOrFalse ); } @@ -233,8 +218,8 @@ export class Timeline { relationTarget.addLocalRelation(pee); } } - if (pee.redactingRelation) { - const eventId = pee.redactingRelation.event_id; + if (pee.redactingEntry) { + const eventId = pee.redactingEntry.relatedEventId; const relationTarget = entries.find(e => e.id === eventId); if (relationTarget) { relationTarget.addLocalRelation(pee); @@ -278,6 +263,32 @@ export class Timeline { } } + async _getOrLoadEntry(txnId, eventId) { + if (txnId) { + // also look for redacting relation in pending events, in case the target is already being sent + for (const p of this._localEntries) { + if (p.id === txnId) { + return p; + } + } + } + if (eventId) { + const loadedEntry = this.getByEventId(eventId); + if (loadedEntry) { + return loadedEntry; + } else { + const txn = await this._storage.readWriteTxn([ + this._storage.storeNames.timelineEvents, + ]); + const redactionTargetEntry = await txn.timelineEvents.getByEventId(this._roomId, eventId); + if (redactionTargetEntry) { + return new EventEntry(redactionTargetEntry, this._fragmentIdComparer); + } + } + } + return null; + } + getByEventId(eventId) { for (let i = 0; i < this._remoteEntries.length; i += 1) { const entry = this._remoteEntries.get(i); diff --git a/src/matrix/room/timeline/entries/BaseEventEntry.js b/src/matrix/room/timeline/entries/BaseEventEntry.js index 6c37457a..3ac98c8c 100644 --- a/src/matrix/room/timeline/entries/BaseEventEntry.js +++ b/src/matrix/room/timeline/entries/BaseEventEntry.js @@ -16,9 +16,11 @@ limitations under the License. import {BaseEntry} from "./BaseEntry.js"; import {REDACTION_TYPE} from "../../common.js"; -import {createAnnotation, ANNOTATION_RELATION_TYPE} from "../relations.js"; +import {createAnnotation, ANNOTATION_RELATION_TYPE, getRelationFromContent} from "../relations.js"; import {PendingAnnotations} from "../PendingAnnotations.js"; +/** Deals mainly with local echo for relations and redactions, + * so it is shared between PendingEventEntry and EventEntry */ export class BaseEventEntry extends BaseEntry { constructor(fragmentIdComparer) { super(fragmentIdComparer); @@ -59,9 +61,9 @@ export class BaseEventEntry extends BaseEntry { return "isRedacted"; } } else { - const relation = entry.ownOrRedactedRelation; - if (relation && relation.event_id === this.id) { - if (relation.rel_type === ANNOTATION_RELATION_TYPE) { + const relationEntry = entry.redactingEntry || entry; + if (relationEntry.isRelationForId(this.id)) { + if (relationEntry.relation.rel_type === ANNOTATION_RELATION_TYPE) { if (!this._pendingAnnotations) { this._pendingAnnotations = new PendingAnnotations(); } @@ -87,9 +89,9 @@ export class BaseEventEntry extends BaseEntry { } } } else { - const relation = entry.ownOrRedactedRelation; - if (relation && relation.event_id === this.id) { - if (relation.rel_type === ANNOTATION_RELATION_TYPE && this._pendingAnnotations) { + const relationEntry = entry.redactingEntry || entry; + if (relationEntry.isRelationForId(this.id)) { + if (relationEntry.relation.rel_type === ANNOTATION_RELATION_TYPE && this._pendingAnnotations) { this._pendingAnnotations.remove(entry); if (this._pendingAnnotations.isEmpty) { this._pendingAnnotations = null; @@ -121,6 +123,14 @@ export class BaseEventEntry extends BaseEntry { return createAnnotation(this.id, key); } + isRelationForId(id) { + return id && this.relation?.event_id === id; + } + + get relation() { + return getRelationFromContent(this.content); + } + get pendingAnnotations() { return this._pendingAnnotations?.aggregatedAnnotations; } diff --git a/src/matrix/room/timeline/entries/PendingEventEntry.js b/src/matrix/room/timeline/entries/PendingEventEntry.js index edb728cd..7f5a87af 100644 --- a/src/matrix/room/timeline/entries/PendingEventEntry.js +++ b/src/matrix/room/timeline/entries/PendingEventEntry.js @@ -16,16 +16,15 @@ limitations under the License. import {PENDING_FRAGMENT_ID} from "./BaseEntry.js"; import {BaseEventEntry} from "./BaseEventEntry.js"; -import {getRelationFromContent} from "../relations.js"; export class PendingEventEntry extends BaseEventEntry { - constructor({pendingEvent, member, clock, redactingRelation}) { + constructor({pendingEvent, member, clock, redactingEntry}) { super(null); this._pendingEvent = pendingEvent; /** @type {RoomMember} */ this._member = member; this._clock = clock; - this._redactingRelation = redactingRelation; + this._redactingEntry = redactingEntry; } get fragmentId() { @@ -84,19 +83,18 @@ export class PendingEventEntry extends BaseEventEntry { } + isRelationForId(id) { + if (id && id === this._pendingEvent.relatedTxnId) { + return true; + } + return super.isRelationForId(id); + } + get relatedEventId() { return this._pendingEvent.relatedEventId; } - get redactingRelation() { - return this._redactingRelation; - } - /** - * returns either the relationship on this entry, - * or the relationship this entry is redacting. - * - * Useful while aggregating relations for local echo. */ - get ownOrRedactedRelation() { - return this.redactingRelation || getRelationFromContent(this._pendingEvent.content); + get redactingEntry() { + return this._redactingEntry; } }