Backfill missing expire times for incoming messages

Co-authored-by: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com>

ts/background.ts

@@ -19,6 +19,8 @@ import { DEFAULT_CONVERSATION_COLOR } from './types/Colors';
 import { ChallengeHandler } from './challenge';
 import { isWindowDragElement } from './util/isWindowDragElement';
 import { assert } from './util/assert';
+import { filter } from './util/iterables';
+import { isNotNil } from './util/isNotNil';
 import { senderCertificateService } from './services/senderCertificate';
 import { routineProfileRefresh } from './routineProfileRefresh';
 import { isMoreRecentThan, isOlderThan } from './util/timestamp';
@@ -43,7 +45,7 @@ import { isDirectConversation, isGroupV2 } from './util/whatTypeOfConversation';
 import { getSendOptions } from './util/getSendOptions';
 import { BackOff } from './util/BackOff';
 import { AppViewType } from './state/ducks/app';
-import { hasErrors, isIncoming } from './state/selectors/message';
+import { isIncoming } from './state/selectors/message';
 import { actionCreators } from './state/actions';
 import { Deletes } from './messageModifiers/Deletes';
 import { DeliveryReceipts } from './messageModifiers/DeliveryReceipts';
@@ -1652,51 +1654,43 @@ export async function startApp(): Promise<void> {
 
     window.dispatchEvent(new Event('storage_ready'));
 
-    window.log.info('Cleanup: starting...');
-    const messagesForCleanup = await window.Signal.Data.getOutgoingWithoutExpirationStartTimestamp(
-      {
-        MessageCollection: window.Whisper.MessageCollection,
-      }
-    );
+    window.log.info('Expiration start timestamp cleanup: starting...');
+    const messagesUnexpectedlyMissingExpirationStartTimestamp = await window.Signal.Data.getMessagesUnexpectedlyMissingExpirationStartTimestamp();
     window.log.info(
-      `Cleanup: Found ${messagesForCleanup.length} messages for cleanup`
+      `Expiration start timestamp cleanup: Found ${messagesUnexpectedlyMissingExpirationStartTimestamp.length} messages for cleanup`
     );
-    await Promise.all(
-      messagesForCleanup.map(async message => {
-        assert(
-          !message.get('expirationStartTimestamp'),
-          'Cleanup should not have messages with an expirationStartTimestamp'
-        );
-
-        const delivered = message.get('delivered');
-        const sentAt = message.get('sent_at');
-
-        if (hasErrors(message.attributes)) {
-          return;
-        }
-
-        if (delivered) {
-          window.log.info(
-            `Cleanup: Starting timer for delivered message ${sentAt}`
+    if (messagesUnexpectedlyMissingExpirationStartTimestamp.length) {
+      const newMessageAttributes = messagesUnexpectedlyMissingExpirationStartTimestamp.map(
+        message => {
+          const expirationStartTimestamp = Math.min(
+            ...filter(
+              [
+                // These messages should always have a sent_at, but we have fallbacks
+                //   just in case.
+                message.sent_at,
+                Date.now(),
+                // The query shouldn't return messages with expiration start timestamps,
+                //   but we're trying to be extra careful.
+                message.expirationStartTimestamp,
+              ],
+              isNotNil
+            )
           );
-          message.set('expirationStartTimestamp', sentAt);
-          return;
+          window.log.info(
+            `Expiration start timestamp cleanup: starting timer for ${message.type} message sent at ${message.sent_at}. Starting timer at ${message.expirationStartTimestamp}`
+          );
+          return {
+            ...message,
+            expirationStartTimestamp,
+          };
         }
+      );
 
-        window.log.info(`Cleanup: Deleting unsent message ${sentAt}`);
-        await window.Signal.Data.removeMessage(message.id, {
-          Message: window.Whisper.Message,
-        });
-        const conversation = message.getConversation();
-        if (conversation) {
-          await conversation.updateLastMessage();
-        }
-      })
-    );
-    if (messagesForCleanup.length) {
-      window.Whisper.ExpiringMessagesListener.update();
+      await window.Signal.Data.saveMessages(newMessageAttributes, {
+        Message: MessageModel,
+      });
     }
-    window.log.info('Cleanup: complete');
+    window.log.info('Expiration start timestamp cleanup: complete');
 
     window.log.info('listening for registration events');
     window.Whisper.events.on('registration_done', () => {

ts/sql/Client.ts

@@ -189,7 +189,7 @@ const dataInterface: ClientInterface = {
   getAllMessageIds,
   getMessagesBySentAt,
   getExpiredMessages,
-  getOutgoingWithoutExpirationStartTimestamp,
+  getMessagesUnexpectedlyMissingExpirationStartTimestamp,
   getSoonestMessageExpiry,
   getNextTapToViewMessageTimestampToAgeOut,
   getTapToViewMessagesNeedingErase,
@@ -1301,14 +1301,8 @@ async function getExpiredMessages({
   return new MessageCollection(messages);
 }
 
-async function getOutgoingWithoutExpirationStartTimestamp({
-  MessageCollection,
-}: {
-  MessageCollection: typeof MessageModelCollectionType;
-}) {
-  const messages = await channels.getOutgoingWithoutExpirationStartTimestamp();
-
-  return new MessageCollection(messages);
+function getMessagesUnexpectedlyMissingExpirationStartTimestamp() {
+  return channels.getMessagesUnexpectedlyMissingExpirationStartTimestamp();
 }
 
 function getSoonestMessageExpiry() {

ts/sql/Interface.ts

@@ -322,6 +322,9 @@ export type DataInterface = {
   getMessageServerGuidsForSpam: (
     conversationId: string
   ) => Promise<Array<string>>;
+  getMessagesUnexpectedlyMissingExpirationStartTimestamp: () => Promise<
+    Array<MessageType>
+  >;
   getSoonestMessageExpiry: () => Promise<undefined | number>;
 
   getJobsInQueue(queueType: string): Promise<Array<StoredJob>>;
@@ -380,7 +383,6 @@ export type ServerInterface = DataInterface & {
     conversationId: string;
     ourConversationId: string;
   }) => Promise<MessageType | undefined>;
-  getOutgoingWithoutExpirationStartTimestamp: () => Promise<Array<MessageType>>;
   getTapToViewMessagesNeedingErase: () => Promise<Array<MessageType>>;
   getUnreadCountForConversation: (conversationId: string) => Promise<number>;
   getUnreadByConversationAndMarkRead: (
@@ -518,9 +520,6 @@ export type ClientInterface = DataInterface & {
     ourConversationId: string;
     Message: typeof MessageModel;
   }) => Promise<MessageModel | undefined>;
-  getOutgoingWithoutExpirationStartTimestamp: (options: {
-    MessageCollection: typeof MessageModelCollectionType;
-  }) => Promise<MessageModelCollectionType>;
   getTapToViewMessagesNeedingErase: (options: {
     MessageCollection: typeof MessageModelCollectionType;
   }) => Promise<MessageModelCollectionType>;

ts/sql/Server.ts

@@ -178,7 +178,7 @@ const dataInterface: ServerInterface = {
   getAllMessageIds,
   getMessagesBySentAt,
   getExpiredMessages,
-  getOutgoingWithoutExpirationStartTimestamp,
+  getMessagesUnexpectedlyMissingExpirationStartTimestamp,
   getSoonestMessageExpiry,
   getNextTapToViewMessageTimestampToAgeOut,
   getTapToViewMessagesNeedingErase,
@@ -1889,6 +1889,27 @@ function updateToSchemaVersion33(currentVersion: number, db: Database) {
   console.log('updateToSchemaVersion33: success!');
 }
 
+function updateToSchemaVersion34(currentVersion: number, db: Database) {
+  if (currentVersion >= 34) {
+    return;
+  }
+
+  db.transaction(() => {
+    db.exec(`
+      -- This index should exist, but we add "IF EXISTS" for safety.
+      DROP INDEX IF EXISTS outgoing_messages_without_expiration_start_timestamp;
+
+      CREATE INDEX messages_unexpectedly_missing_expiration_start_timestamp ON messages (
+        expireTimer, expirationStartTimestamp, type
+      )
+      WHERE expireTimer IS NOT NULL AND expirationStartTimestamp IS NULL;
+    `);
+
+    db.pragma('user_version = 34');
+  })();
+  console.log('updateToSchemaVersion34: success!');
+}
+
 const SCHEMA_VERSIONS = [
   updateToSchemaVersion1,
   updateToSchemaVersion2,
@@ -1923,6 +1944,7 @@ const SCHEMA_VERSIONS = [
   updateToSchemaVersion31,
   updateToSchemaVersion32,
   updateToSchemaVersion33,
+  updateToSchemaVersion34,
 ];
 
 function updateSchema(db: Database): void {
@@ -3906,7 +3928,7 @@ async function getExpiredMessages(): Promise<Array<MessageType>> {
   return rows.map(row => jsonToObject(row.json));
 }
 
-async function getOutgoingWithoutExpirationStartTimestamp(): Promise<
+async function getMessagesUnexpectedlyMissingExpirationStartTimestamp(): Promise<
   Array<MessageType>
 > {
   const db = getInstance();
@@ -3914,11 +3936,17 @@ async function getOutgoingWithoutExpirationStartTimestamp(): Promise<
     .prepare<EmptyQuery>(
       `
       SELECT json FROM messages
-      INDEXED BY outgoing_messages_without_expiration_start_timestamp
+      INDEXED BY messages_unexpectedly_missing_expiration_start_timestamp
       WHERE
         expireTimer > 0 AND
         expirationStartTimestamp IS NULL AND
-        type IS 'outgoing';
+        (
+          type IS 'outgoing' OR
+          (type IS 'incoming' AND (
+            unread = 0 OR
+            unread IS NULL
+          ))
+        );
       `
     )
     .all();