diff --git a/ts/messageModifiers/MessageReceipts.ts b/ts/messageModifiers/MessageReceipts.ts index f495e0d8fa..f343037a3f 100644 --- a/ts/messageModifiers/MessageReceipts.ts +++ b/ts/messageModifiers/MessageReceipts.ts @@ -15,6 +15,7 @@ import * as Errors from '../types/errors'; import { SendActionType, SendStatus, + UNDELIVERED_SEND_STATUSES, sendStateReducer, } from '../messages/MessageSendState'; import type { DeleteSentProtoRecipientOptionsType } from '../sql/Interface'; @@ -25,6 +26,7 @@ import { queueUpdateMessage } from '../util/messageBatcher'; import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp'; import { getMessageIdForLogging } from '../util/idForLogging'; import { generateCacheKey } from './generateCacheKey'; +import { getPropForTimestamp } from '../util/editHelpers'; const { deleteSentProtoRecipient } = dataInterface; @@ -38,7 +40,7 @@ export type MessageReceiptAttributesType = { envelopeId: string; messageSentAt: number; receiptTimestamp: number; - removeFromMessageReceiverCache: () => unknown; + removeFromMessageReceiverCache: () => void; sourceConversationId: string; sourceDevice: number; sourceServiceId: ServiceIdString; @@ -89,45 +91,74 @@ function remove(receipt: MessageReceiptAttributesType): void { receipt.removeFromMessageReceiverCache(); } -async function getTargetMessage( - sourceId: string, - serviceId: ServiceIdString, - messages: ReadonlyArray -): Promise { +function getTargetMessage({ + sourceConversationId, + messages, + targetTimestamp, +}: { + sourceConversationId: string; + messages: ReadonlyArray; + targetTimestamp: number; +}): MessageModel | null { if (messages.length === 0) { return null; } - const message = messages.find( - item => - (isOutgoing(item) || isStory(item)) && sourceId === item.conversationId - ); - if (message) { - return window.MessageCache.__DEPRECATED$register( - message.id, - message, - 'MessageReceipts.getTargetMessage 1' - ); - } - const groups = await window.Signal.Data.getAllGroupsInvolvingServiceId( - serviceId - ); + const matchingMessages = messages + .filter(msg => isOutgoing(msg) || isStory(msg)) + .filter(msg => { + const sendStateByConversationId = getPropForTimestamp({ + message: msg, + prop: 'sendStateByConversationId', + targetTimestamp, + log, + }); - const ids = groups.map(item => item.id); - ids.push(sourceId); + const isRecipient = Object.hasOwn( + sendStateByConversationId ?? {}, + sourceConversationId + ); + if (!isRecipient) { + return false; + } - const target = messages.find( - item => - (isOutgoing(item) || isStory(item)) && ids.includes(item.conversationId) - ); - if (!target) { + const sendStatus = + sendStateByConversationId?.[sourceConversationId]?.status; + + if ( + sendStatus === undefined || + UNDELIVERED_SEND_STATUSES.includes(sendStatus) + ) { + log.warn(` + MessageReceipts.getTargetMessage: received receipt for undelivered message, + status: ${sendStatus}, + sourceConversationId: ${sourceConversationId}, + message: ${getMessageIdForLogging(message)}. + `); + return false; + } + + return true; + }); + + if (matchingMessages.length === 0) { return null; } + if (matchingMessages.length > 1) { + log.warn(` + MessageReceipts.getTargetMessage: multiple (${matchingMessages.length}) + matching messages for receipt, + sentAt=${targetTimestamp}, + sourceConversationId=${sourceConversationId} + `); + } + + const message = matchingMessages[0]; return window.MessageCache.__DEPRECATED$register( - target.id, - target, - 'MessageReceipts.getTargetMessage 2' + message.id, + message, + 'MessageReceipts.getTargetMessage' ); } @@ -367,11 +398,11 @@ export async function onReceipt( messageSentAt ); - const message = await getTargetMessage( + const message = getTargetMessage({ sourceConversationId, - sourceServiceId, - messages - ); + messages, + targetTimestamp: receipt.messageSentAt, + }); if (message) { await updateMessageSendState(receipt, message); diff --git a/ts/messages/MessageSendState.ts b/ts/messages/MessageSendState.ts index 7f1173fc48..09f5bc99a6 100644 --- a/ts/messages/MessageSendState.ts +++ b/ts/messages/MessageSendState.ts @@ -39,6 +39,11 @@ export const parseMessageSendStatus = makeEnumParser( SendStatus.Pending ); +export const UNDELIVERED_SEND_STATUSES = [ + SendStatus.Pending, + SendStatus.Failed, +]; + const STATUS_NUMBERS: Record = { [SendStatus.Failed]: 0, [SendStatus.Pending]: 1, diff --git a/ts/util/editHelpers.ts b/ts/util/editHelpers.ts index 5306d4ee06..9cd1ecb23a 100644 --- a/ts/util/editHelpers.ts +++ b/ts/util/editHelpers.ts @@ -5,9 +5,10 @@ import { isNumber, sortBy } from 'lodash'; import { strictAssert } from './assert'; -import type { EditHistoryType } from '../model-types'; -import type { MessageModel } from '../models/messages'; +import type { EditHistoryType, MessageAttributesType } from '../model-types'; import type { LoggerType } from '../types/Logging'; +import { getMessageIdForLogging } from './idForLogging'; +import type { MessageModel } from '../models/messages'; // The tricky bit for this function is if we are on our second+ attempt to send a given // edit, we're still sending that edit. @@ -51,13 +52,18 @@ export function getPropForTimestamp({ targetTimestamp, }: { log: LoggerType; - message: MessageModel; + message: MessageModel | MessageAttributesType; prop: T; targetTimestamp: number; }): EditHistoryType[T] { - const logId = `getPropForTimestamp(${message.idForLogging()}, target=${targetTimestamp}})`; + const attributes = + message instanceof window.Whisper.Message ? message.attributes : message; - const editHistory = message.get('editHistory'); + const logId = `getPropForTimestamp(${getMessageIdForLogging( + attributes + )}, target=${targetTimestamp}})`; + + const { editHistory } = attributes; const targetEdit = editHistory?.find( item => item.timestamp === targetTimestamp ); @@ -65,7 +71,7 @@ export function getPropForTimestamp({ if (editHistory) { log.warn(`${logId}: No edit found, using top-level data`); } - return message.get(prop); + return attributes[prop]; } return targetEdit[prop];