Improve message content cleanup behavior

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

ts/messages/handleDataMessage.preload.ts

@@ -613,7 +613,7 @@ export async function handleDataMessage(
 
       const isSupported = !isUnsupportedMessage(message.attributes);
       if (!isSupported) {
-        await eraseMessageContents(message);
+        await eraseMessageContents(message, 'unsupported-message');
       }
 
       if (isSupported) {
@@ -735,7 +735,7 @@ export async function handleDataMessage(
         }
 
         if (isTapToView(message.attributes) && type === 'outgoing') {
-          await eraseMessageContents(message);
+          await eraseMessageContents(message, 'view-once-viewed');
         }
 
         if (
@@ -749,7 +749,7 @@ export async function handleDataMessage(
           message.set({
             isTapToViewInvalid: true,
           });
-          await eraseMessageContents(message);
+          await eraseMessageContents(message, 'view-once-invalid');
         }
       }
 

ts/models/messages.preload.ts

@@ -35,6 +35,10 @@ export class MessageModel {
 
     window.MessageCache._updateCaches(this);
   }
+  public resetAllAttributes(attributes: MessageAttributesType): void {
+    this.#_attributes = { ...attributes };
+    window.MessageCache._updateCaches(this);
+  }
 
   public get attributes(): Readonly<MessageAttributesType> {
     return this.#_attributes;

ts/services/MessageUpdater.preload.ts

@@ -94,7 +94,7 @@ export async function markViewOnceMessageViewed(
     message.set(markViewed(message.attributes));
   }
 
-  await eraseMessageContents(message);
+  await eraseMessageContents(message, 'view-once-viewed');
 
   if (!fromSync) {
     const senderE164 = getSource(message.attributes);

ts/services/tapToViewMessagesDeletionService.preload.ts

@@ -43,7 +43,7 @@ async function eraseTapToViewMessages() {
         // We do this to update the UI, if this message is being displayed somewhere
         window.reduxActions.conversations.messageExpired(message.id);
 
-        await eraseMessageContents(message);
+        await eraseMessageContents(message, 'view-once-expired');
       })
     );
   } catch (error) {

ts/test-electron/util/cleanup_message_test.preload.ts

@@ -0,0 +1,59 @@
+// Copyright 2025 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { v7 } from 'uuid';
+import { assert } from 'chai';
+
+import { eraseMessageContents } from '../../util/cleanup.preload.js';
+import { MessageModel } from '../../models/messages.preload.js';
+import type { PollMessageAttribute } from '../../types/Polls.dom.js';
+import { DataWriter } from '../../sql/Client.preload.js';
+import { itemStorage } from '../../textsecure/Storage.preload.js';
+import { generateAci } from '../../types/ServiceId.std.js';
+import type { MessageAttributesType } from '../../model-types.js';
+import { IMAGE_BMP } from '../../types/MIME.std.js';
+import { SendStatus } from '../../messages/MessageSendState.std.js';
+
+describe('eraseMessageContents', () => {
+  beforeEach(async () => {
+    await DataWriter.removeAll();
+    await itemStorage.user.setAciAndDeviceId(generateAci(), 1);
+    await window.ConversationController.load();
+  });
+  it('only preserves explicitly preserved fields', async () => {
+    const now = Date.now();
+    const attributes: MessageAttributesType = {
+      id: v7(),
+      type: 'incoming',
+      sent_at: now,
+      received_at: now,
+      conversationId: 'convoId',
+      timestamp: now,
+      schemaVersion: 12,
+      body: 'body',
+      poll: {
+        question: 'poll question',
+      } as PollMessageAttribute,
+      sendStateByConversationId: { aci: { status: SendStatus.Delivered } },
+      storyReplyContext: {
+        attachment: { contentType: IMAGE_BMP, size: 128 },
+        messageId: 'messageId',
+      },
+    };
+    const message = new MessageModel(attributes);
+
+    await eraseMessageContents(message, 'unsupported-message');
+
+    assert.deepEqual(message.attributes, {
+      id: attributes.id,
+      type: attributes.type,
+      sent_at: attributes.sent_at,
+      received_at: attributes.received_at,
+      conversationId: 'convoId',
+      timestamp: attributes.timestamp,
+      schemaVersion: 12,
+      sendStateByConversationId: { aci: { status: SendStatus.Delivered } },
+      isErased: true,
+    });
+  });
+});

ts/types/Message.std.ts

@@ -1,9 +1,12 @@
 // Copyright 2018 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
+import type { MessageAttributesType } from '../model-types.js';
+import { strictAssert } from '../util/assert.std.js';
 import type { DurationInSeconds } from '../util/durations/index.std.js';
 import type { AttachmentType } from './Attachment.std.js';
 import type { EmbeddedContactType } from './EmbeddedContact.std.js';
+import type { ErrorIfOverlapping, ExactKeys } from './Util.std.js';
 
 export function getMentionsRegex(): RegExp {
   return /\uFFFC/g;
@@ -92,3 +95,115 @@ export type MessageSchemaVersion6 = Partial<
     contact: Array<EmbeddedContactType>;
   }>
 >;
+
+// NB: see `eraseMessageContents` for all scenarios in which message content can be erased
+export const messageAttrsToPreserveAfterErase = [
+  // TS required fields
+  'id',
+  'timestamp',
+  'conversationId',
+  'type',
+  'sent_at',
+  'received_at',
+
+  // all other, non-TS-required fields to preserve
+  'canReplyToStory',
+  'deletedForEveryone',
+  'deletedForEveryoneFailed',
+  'deletedForEveryoneSendStatus',
+  'deletedForEveryoneTimestamp',
+  'editMessageReceivedAt',
+  'editMessageReceivedAtMs',
+  'editMessageTimestamp',
+  'errors',
+  'expirationStartTimestamp',
+  'expireTimer',
+  'isErased',
+  'isTapToViewInvalid',
+  'isViewOnce',
+  'readAt',
+  'readStatus',
+  'received_at_ms',
+  'requiredProtocolVersion',
+  'schemaMigrationAttempts',
+  'schemaVersion',
+  'seenStatus',
+  'sendStateByConversationId',
+  'serverGuid',
+  'serverTimestamp',
+  'source',
+  'sourceDevice',
+  'sourceServiceId',
+  'storyId',
+  'synced',
+  'unidentifiedDeliveries',
+] as const;
+
+const messageAttrsToErase = [
+  'attachments',
+  'body',
+  'bodyAttachment',
+  'bodyRanges',
+  'callId',
+  'changedId',
+  'contact',
+  'conversationMerge',
+  'dataMessage',
+  'decrypted_at',
+  'droppedGV2MemberIds',
+  'editHistory',
+  'expirationTimerUpdate',
+  'flags',
+  'giftBadge',
+  'group_update',
+  'groupMigration',
+  'groupV2Change',
+  'hasUnreadPollVotes',
+  'invitedGV2Members',
+  'unidentifiedDeliveryReceived',
+  'key_changed',
+  'local',
+  'logger',
+  'mentionsMe',
+  'message',
+  'messageRequestResponseEvent',
+  'messageTimer',
+  'payment',
+  'phoneNumberDiscovery',
+  'pinnedMessageId',
+  'poll',
+  'pollTerminateNotification',
+  'preview',
+  'profileChange',
+  'quote',
+  'reactions',
+  'sendHQImages',
+  'sms',
+  'sticker',
+  'storyDistributionListId',
+  'storyReaction',
+  'storyRecipientsVersion',
+  'storyReplyContext',
+  'supportedVersionAtReceive',
+  'titleTransition',
+  'verified',
+  'verifiedChanged',
+] as const;
+
+const allKeys = [
+  ...messageAttrsToPreserveAfterErase,
+  ...messageAttrsToErase,
+] as const;
+
+// Note: if this errors, it's likely that the keys of MessageAttributesType have changed
+// and you need to update messageAttrsToPreserveAfterErase or
+// messageAttributesToEraseIfMessageContentsAreErased as needed
+const _enforceTypeCheck: ExactKeys<MessageAttributesType, typeof allKeys> =
+  {} as MessageAttributesType;
+strictAssert(_enforceTypeCheck != null, 'type check');
+
+const _checkKeys: ErrorIfOverlapping<
+  typeof messageAttrsToPreserveAfterErase,
+  typeof messageAttrsToErase
+> = undefined;
+strictAssert(_checkKeys === undefined, 'type check');

ts/types/Util.std.ts

@@ -115,3 +115,19 @@ export type WithRequiredProperties<T, P extends keyof T> = Omit<T, P> &
 
 export type WithOptionalProperties<T, P extends keyof T> = Omit<T, P> &
   Partial<Pick<T, P>>;
+
+// Check that two const arrays do not have overlapping values
+export type ErrorIfOverlapping<
+  T1 extends ReadonlyArray<unknown>,
+  T2 extends ReadonlyArray<unknown>,
+> = T1[number] & T2[number] extends never
+  ? void
+  : 'Error: Arrays have overlapping values';
+
+// Check that T has all the fields (and only those fields) from K
+export type ExactKeys<T, K extends ReadonlyArray<string>> =
+  Exclude<keyof T, K[number]> extends never
+    ? Exclude<K[number], keyof T> extends never
+      ? T
+      : 'Error: Array has fields not present in object type'
+    : 'Error: Object type has keys not present in array';

ts/util/cleanup.preload.ts

@@ -3,6 +3,7 @@
 
 import PQueue from 'p-queue';
 import { batch } from 'react-redux';
+import { pick } from 'lodash';
 
 import type { MessageAttributesType } from '../model-types.d.ts';
 import { MessageModel } from '../models/messages.preload.js';
@@ -30,6 +31,7 @@ import { hydrateStoryContext } from './hydrateStoryContext.preload.js';
 import { update as updateExpiringMessagesService } from '../services/expiringMessagesDeletion.preload.js';
 import { tapToViewMessagesDeletionService } from '../services/tapToViewMessagesDeletionService.preload.js';
 import { throttledUpdateBackupMediaDownloadProgress } from './updateBackupMediaDownloadProgress.preload.js';
+import { messageAttrsToPreserveAfterErase } from '../types/Message.std.js';
 
 const log = createLogger('cleanup');
 
@@ -40,11 +42,16 @@ export async function postSaveUpdates(): Promise<void> {
 
 export async function eraseMessageContents(
   message: MessageModel,
-  additionalProperties = {},
-  shouldPersist = true
+  reason:
+    | 'view-once-viewed'
+    | 'view-once-invalid'
+    | 'view-once-expired'
+    | 'unsupported-message'
+    | 'delete-for-everyone',
+  additionalProperties = {}
 ): Promise<void> {
   log.info(
-    `Erasing data for message ${getMessageIdForLogging(message.attributes)}`
+    `Erasing data for message ${getMessageIdForLogging(message.attributes)}: ${reason}`
   );
 
   // Note: There are cases where we want to re-erase a given message. For example, when
@@ -59,27 +66,22 @@ export async function eraseMessageContents(
     );
   }
 
-  message.set({
-    attachments: [],
-    body: '',
-    bodyRanges: undefined,
-    contact: [],
-    editHistory: undefined,
-    hasUnreadPollVotes: undefined,
+  const preservedAttributes = pick(
+    message.attributes,
+    ...messageAttrsToPreserveAfterErase
+  );
+
+  message.resetAllAttributes({
+    ...preservedAttributes,
     isErased: true,
-    preview: [],
-    poll: undefined,
-    quote: undefined,
-    sticker: undefined,
     ...additionalProperties,
   });
+
   window.ConversationController.get(
     message.attributes.conversationId
   )?.debouncedUpdateLastMessage();
 
-  if (shouldPersist) {
   await window.MessageCache.saveMessage(message.attributes);
-  }
 
   await DataWriter.deleteSentProtoByMessageId(message.id);
 }

ts/util/deleteForEveryone.preload.ts

@@ -8,9 +8,9 @@ import { isMe } from './whatTypeOfConversation.dom.js';
 import { getAuthorId } from '../messages/sources.preload.js';
 import { isStory } from '../state/selectors/message.preload.js';
 import { isTooOldToModifyMessage } from './isTooOldToModifyMessage.std.js';
-import { drop } from './drop.std.js';
 import { eraseMessageContents } from './cleanup.preload.js';
 import { notificationService } from '../services/notifications.preload.js';
+import { DataWriter } from '../sql/Client.preload.js';
 
 const log = createLogger('deleteForEveryone');
 
@@ -19,8 +19,7 @@ export async function deleteForEveryone(
   doe: Pick<
     DeleteAttributesType,
     'fromId' | 'targetSentTimestamp' | 'serverTimestamp'
-  >,
-  shouldPersist = true
+  >
 ): Promise<void> {
   if (isDeletionByMe(message, doe)) {
     const conversation = window.ConversationController.get(
@@ -36,7 +35,7 @@ export async function deleteForEveryone(
       return;
     }
 
-    await handleDeleteForEveryone(message, doe, shouldPersist);
+    await handleDeleteForEveryone(message, doe);
     return;
   }
 
@@ -51,7 +50,7 @@ export async function deleteForEveryone(
     return;
   }
 
-  await handleDeleteForEveryone(message, doe, shouldPersist);
+  await handleDeleteForEveryone(message, doe);
 }
 
 function isDeletionByMe(
@@ -71,8 +70,7 @@ export async function handleDeleteForEveryone(
   del: Pick<
     DeleteAttributesType,
     'fromId' | 'targetSentTimestamp' | 'serverTimestamp'
-  >,
-  shouldPersist = true
+  >
 ): Promise<void> {
   if (message.deletingForEveryone || message.get('deletedForEveryone')) {
     return;
@@ -94,18 +92,21 @@ export async function handleDeleteForEveryone(
     notificationService.removeBy({ messageId: message.get('id') });
 
     // Erase the contents of this message
-    await eraseMessageContents(
-      message,
-      { deletedForEveryone: true, reactions: [] },
-      shouldPersist
-    );
+    await eraseMessageContents(message, 'delete-for-everyone', {
+      deletedForEveryone: true,
+      reactions: [],
+    });
 
-    // Update the conversation's last message in case this was the last message
-    drop(
-      window.ConversationController.get(
-        message.attributes.conversationId
-      )?.updateLastMessage()
-    );
+    // We delete the message first, before re-saving it -- this causes any foreign key ON
+    // DELETE CASCADE and messages_on_delete triggers to run, which is important
+    await DataWriter.removeMessage(message.attributes.id, {
+      cleanupMessages: async () => {
+        // We don't actually want to remove this message up from in-memory caches
+      },
+    });
+    await window.MessageCache.saveMessage(message.attributes, {
+      forceSave: true,
+    });
   } finally {
     // eslint-disable-next-line no-param-reassign
     message.deletingForEveryone = undefined;

ts/util/hydrateStoryContext.preload.ts

@@ -74,7 +74,7 @@ export async function hydrateStoryContext(
     );
     const newMessageAttributes: Partial<MessageAttributesType> = {
       storyReplyContext: {
-        ...context,
+        authorAci: context?.authorAci,
         attachment: undefined,
         // No messageId = referenced story not found
         messageId: '',

ts/util/modifyTargetMessage.preload.ts

@@ -355,7 +355,7 @@ export async function modifyTargetMessage(
   const deletes = Deletes.forMessage(message.attributes);
   await Promise.all(
     deletes.map(async del => {
-      await deleteForEveryone(message, del, false);
+      await deleteForEveryone(message, del);
       changed = true;
     })
   );