From 366d3761b8bbc01ef511ac3970d786cfe3199c53 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 24 Jun 2021 13:35:59 +0200 Subject: [PATCH] remove waiting for update event (it might not come in case of dupe) also remove duplicate logging impl for re(d)action at cost of double haveAnnotation call --- .../room/timeline/tiles/BaseMessageTile.js | 76 ++++--------------- 1 file changed, 15 insertions(+), 61 deletions(-) diff --git a/src/domain/session/room/timeline/tiles/BaseMessageTile.js b/src/domain/session/room/timeline/tiles/BaseMessageTile.js index d2f13692..ba0198fe 100644 --- a/src/domain/session/room/timeline/tiles/BaseMessageTile.js +++ b/src/domain/session/room/timeline/tiles/BaseMessageTile.js @@ -27,7 +27,6 @@ export class BaseMessageTile extends SimpleTile { if (this._entry.annotations || this._entry.pendingAnnotations) { this._updateReactions(); } - this._pendingReactionChangeCallback = null; } get _mediaRepository() { @@ -124,79 +123,45 @@ export class BaseMessageTile extends SimpleTile { } react(key, log = null) { - return this.logger.wrapOrRun(log, "react", log => { + return this.logger.wrapOrRun(log, "react", async log => { if (this._entry.haveAnnotation(key)) { log.set("already_reacted", true); return; } - return this._react(key, log); - }); - } - - async _react(key, log) { - // This will also block concurently adding multiple reactions, - // but in practice it happens fast enough. - if (this._pendingReactionChangeCallback) { - log.set("ongoing", true); - return; - } - const redaction = this._entry.pendingAnnotations?.get(key)?.redactionEntry; - try { - const updatePromise = new Promise(resolve => this._pendingReactionChangeCallback = resolve); + const redaction = this._entry.pendingAnnotations?.get(key)?.redactionEntry; if (redaction && !redaction.pendingEvent.hasStartedSending) { log.set("abort_redaction", true); await redaction.pendingEvent.abort(); } else { await this._room.sendEvent("m.reaction", this._entry.annotate(key), null, log); } - await updatePromise; - } finally { - this._pendingReactionChangeCallback = null; - } + }); } redactReaction(key, log = null) { - return this.logger.wrapOrRun(log, "redactReaction", log => { + return this.logger.wrapOrRun(log, "redactReaction", async log => { if (!this._entry.haveAnnotation(key)) { log.set("not_yet_reacted", true); return; } - return this._redactReaction(key, log); - }); - } - - async _redactReaction(key, log) { - // This will also block concurently removing multiple reactions, - // but in practice it happens fast enough. - - // TODO: remove this as we'll protect against reentry in the SendQueue - if (this._pendingReactionChangeCallback) { - log.set("ongoing", true); - return; - } - let entry = this._entry.pendingAnnotations?.get(key)?.annotationEntry; - if (!entry) { - entry = await this._timeline.getOwnAnnotationEntry(this._entry.id, key); - } - if (entry) { - try { - const updatePromise = new Promise(resolve => this._pendingReactionChangeCallback = resolve); - await this._room.sendRedaction(entry.id, null, log); - await updatePromise; - } finally { - this._pendingReactionChangeCallback = null; + let entry = this._entry.pendingAnnotations?.get(key)?.annotationEntry; + if (!entry) { + entry = await this._timeline.getOwnAnnotationEntry(this._entry.id, key); } - } else { - log.set("no_reaction", true); - } + if (entry) { + await this._room.sendRedaction(entry.id, null, log); + } else { + log.set("no_reaction", true); + } + }); } toggleReaction(key, log = null) { return this.logger.wrapOrRun(log, "toggleReaction", async log => { if (this._entry.haveAnnotation(key)) { - await log.wrap("redactReaction", log => this._redactReaction(key, log)); + await this.redactReaction(key, log); } else { - await log.wrap("react", log => this._react(key, log)); + await this.react(key, log); } }); } @@ -206,23 +171,12 @@ export class BaseMessageTile extends SimpleTile { if (!annotations && !pendingAnnotations) { if (this._reactions) { this._reactions = null; - // The update comes in async because pending events are mapped in the timeline - // to pending event entries using an AsyncMappedMap, because in rare cases, the target - // of a redaction needs to be loaded from storage in order to know for which message - // the reaction needs to be removed. The SendQueue also only adds pending events after - // storing them first. - // This makes that if we want to know the local echo for either react or redactReaction is available, - // we need to async wait for the update call. In theory the update can also be triggered - // by something else than the reaction local echo changing (e.g. from sync), - // but this is very unlikely and deemed good enough for now. - this._pendingReactionChangeCallback && this._pendingReactionChangeCallback(); } } else { if (!this._reactions) { this._reactions = new ReactionsViewModel(this); } this._reactions.update(annotations, pendingAnnotations); - this._pendingReactionChangeCallback && this._pendingReactionChangeCallback(); } } }