From 03eaa9eb3e6320bed6b6b1c7357a6821906d1a9f Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Thu, 13 May 2021 12:10:20 -0700 Subject: [PATCH] Apply reactions optimistically --- js/reactions.js | 11 +++++++---- ts/model-types.d.ts | 21 ++++++++++++--------- ts/models/conversations.ts | 24 ++++++++++++++++++++++-- ts/models/messages.ts | 14 ++++++++------ ts/state/ducks/conversations.ts | 9 --------- ts/window.d.ts | 8 +++++--- 6 files changed, 54 insertions(+), 33 deletions(-) diff --git a/js/reactions.js b/js/reactions.js index 8f8d01c738..7a71c80ae7 100644 --- a/js/reactions.js +++ b/js/reactions.js @@ -61,11 +61,11 @@ reaction.get('targetAuthorUuid'), reaction.get('targetTimestamp') ); - return; + return undefined; } // awaiting is safe since `onReaction` is never called from inside the queue - await targetConversation.queueJob(async () => { + return await targetConversation.queueJob(async () => { window.log.info( 'Handling reaction for', reaction.get('targetTimestamp') @@ -112,7 +112,7 @@ oldReaction.forEach(r => this.remove(r)); } - return; + return undefined; } const message = MessageController.register( @@ -120,15 +120,18 @@ targetMessage ); - await message.handleReaction(reaction); + const oldReaction = await message.handleReaction(reaction); this.remove(reaction); + + return oldReaction; }); } catch (error) { window.log.error( 'Reactions.onReaction error:', error && error.stack ? error.stack : error ); + return undefined; } }, }))(); diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts index 79deda1da1..246e05de2c 100644 --- a/ts/model-types.d.ts +++ b/ts/model-types.d.ts @@ -109,15 +109,6 @@ export type MessageAttributesType = { quote?: QuotedMessageType; reactions?: Array<{ emoji: string; - from: { - id: string; - color?: string; - avatarPath?: string; - name?: string; - profileName?: string; - isMe?: boolean; - phoneNumber?: string; - }; fromId: string; targetAuthorUuid: string; targetTimestamp: number; @@ -348,3 +339,15 @@ export declare class ConversationModelCollectionType extends Backbone.Collection } export declare class MessageModelCollectionType extends Backbone.Collection {} + +export type ReactionAttributesType = { + emoji: string; + remove?: boolean; + targetAuthorUuid: string; + targetTimestamp: number; + fromId?: string; + timestamp: number; + fromSync?: boolean; +}; + +export declare class ReactionModelType extends Backbone.Model {} diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 81ec350916..121807e9f5 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -9,6 +9,7 @@ import { MessageModelCollectionType, WhatIsThis, MessageAttributesType, + ReactionModelType, ConversationAttributesType, VerificationOptions, } from '../model-types.d'; @@ -3067,6 +3068,11 @@ export class ConversationModel extends window.Backbone fromSync: true, }); + // Apply reaction optimistically + const oldReaction = await window.Whisper.Reactions.onReaction( + reactionModel + ); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const destination = this.getSendTarget()!; const recipients = this.getRecipients(); @@ -3176,9 +3182,23 @@ export class ConversationModel extends window.Backbone throw new Error('No successful delivery for reaction'); } - window.Whisper.Reactions.onReaction(reactionModel); - return result; + }).catch(() => { + let reverseReaction: ReactionModelType; + if (oldReaction) { + // Either restore old reaction + reverseReaction = window.Whisper.Reactions.add({ + ...oldReaction, + fromId: window.ConversationController.getOurConversationId(), + timestamp, + }); + } else { + // Or remove a new one on failure + reverseReaction = reactionModel.clone(); + reverseReaction.set('remove', !reverseReaction.get('remove')); + } + + window.Whisper.Reactions.onReaction(reverseReaction); }); } diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 088a5b9cf3..a9304c6065 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -5,6 +5,7 @@ import { CustomError, MessageAttributesType, RetryOptions, + ReactionAttributesType, ShallowChallengeError, QuotedMessageType, WhatIsThis, @@ -4084,9 +4085,9 @@ export class MessageModel extends window.Backbone.Model { async handleReaction( reaction: typeof window.WhatIsThis, shouldPersist = true - ): Promise { + ): Promise { if (this.get('deletedForEveryone')) { - return; + return undefined; } // We allow you to react to messages with outgoing errors only if it has sent @@ -4095,7 +4096,7 @@ export class MessageModel extends window.Backbone.Model { this.hasErrors() && (this.isIncoming() || this.getMessagePropStatus() !== 'partial-sent') ) { - return; + return undefined; } const reactions = this.get('reactions') || []; @@ -4108,6 +4109,7 @@ export class MessageModel extends window.Backbone.Model { let reactionToRemove: Partial | undefined; + let oldReaction: ReactionAttributesType | undefined; if (reaction.get('remove')) { window.log.info('Removing reaction for message', messageId); const newReactions = reactions.filter( @@ -4137,9 +4139,7 @@ export class MessageModel extends window.Backbone.Model { newReactions.push(reaction.toJSON()); this.set({ reactions: newReactions }); - const oldReaction = reactions.find( - re => re.fromId === reaction.get('fromId') - ); + oldReaction = reactions.find(re => re.fromId === reaction.get('fromId')); if (oldReaction) { reactionToRemove = { emoji: oldReaction.emoji, @@ -4177,6 +4177,8 @@ export class MessageModel extends window.Backbone.Model { Message: window.Whisper.Message, }); } + + return oldReaction; } async handleDeleteForEveryone( diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 6eb5a70065..1a44df4413 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -179,15 +179,6 @@ export type MessageType = { reactions?: Array<{ emoji: string; timestamp: number; - from: { - id: string; - color?: string; - avatarPath?: string; - name?: string; - profileName?: string; - isMe?: boolean; - phoneNumber?: string; - }; }>; deletedForEveryone?: boolean; diff --git a/ts/window.d.ts b/ts/window.d.ts index 305f4c5353..7601d10271 100644 --- a/ts/window.d.ts +++ b/ts/window.d.ts @@ -16,6 +16,8 @@ import { ConversationModelCollectionType, MessageModelCollectionType, MessageAttributesType, + ReactionAttributesType, + ReactionModelType, } from './model-types.d'; import { ContactRecordIdentityState, TextSecureType } from './textsecure.d'; import { @@ -724,9 +726,9 @@ export type WhisperType = { }; Reactions: { - forMessage: (message: unknown) => Array; - add: (reaction: unknown) => WhatIsThis; - onReaction: (reactionModel: unknown) => unknown; + forMessage: (message: unknown) => Array; + add: (reaction: ReactionAttributesType) => ReactionModelType; + onReaction: (reactionModel: ReactionModelType) => ReactionAttributesType; }; Deletes: {