Refactor target message for delivery receipt processing

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<MessageAttributesType>
-): Promise<MessageModel | null> {
+function getTargetMessage({
+  sourceConversationId,
+  messages,
+  targetTimestamp,
+}: {
+  sourceConversationId: string;
+  messages: ReadonlyArray<MessageAttributesType>;
+  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);

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, number> = {
   [SendStatus.Failed]: 0,
   [SendStatus.Pending]: 1,

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<T extends keyof EditHistoryType>({
   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<T extends keyof EditHistoryType>({
     if (editHistory) {
       log.warn(`${logId}: No edit found, using top-level data`);
     }
-    return message.get(prop);
+    return attributes[prop];
   }
 
   return targetEdit[prop];