Synchronous delete call link

ts/sql/Interface.ts

@@ -44,7 +44,6 @@ import type {
 import type { SyncTaskType } from '../util/syncTasks';
 import type { AttachmentBackupJobType } from '../types/AttachmentBackup';
 import type { SingleProtoJobQueue } from '../jobs/singleProtoJobQueue';
-import type { DeleteCallLinkOptions } from './server/callLinks';
 
 export type ReadableDB = Database & { __readable_db: never };
 export type WritableDB = ReadableDB & { __writable_db: never };
@@ -584,6 +583,7 @@ type ReadableInterface = {
   getAllCallLinks: () => ReadonlyArray<CallLinkType>;
   getCallLinkByRoomId: (roomId: string) => CallLinkType | undefined;
   getCallLinkRecordByRoomId: (roomId: string) => CallLinkRecord | undefined;
+  getAllAdminCallLinks(): ReadonlyArray<CallLinkType>;
   getAllCallLinkRecordsWithAdminKey(): ReadonlyArray<CallLinkRecord>;
   getAllMarkedDeletedCallLinkRoomIds(): ReadonlyArray<string>;
   getMessagesBetween: (
@@ -813,8 +813,10 @@ type WritableInterface = {
     roomId: string,
     callLinkState: CallLinkStateType
   ): CallLinkType;
-  beginDeleteAllCallLinks(): void;
-  beginDeleteCallLink(roomId: string, options: DeleteCallLinkOptions): void;
+  beginDeleteAllCallLinks(): boolean;
+  beginDeleteCallLink(roomId: string): boolean;
+  deleteCallHistoryByRoomId(roomid: string): void;
+  deleteCallLinkAndHistory(roomId: string): void;
   finalizeDeleteCallLink(roomId: string): void;
   _removeAllCallLinks(): void;
   deleteCallLinkFromSync(roomId: string): void;

ts/sql/Server.ts

@@ -180,6 +180,9 @@ import {
   updateCallLinkAdminKeyByRoomId,
   updateCallLinkState,
   beginDeleteAllCallLinks,
+  deleteCallHistoryByRoomId,
+  deleteCallLinkAndHistory,
+  getAllAdminCallLinks,
   getAllCallLinkRecordsWithAdminKey,
   getAllMarkedDeletedCallLinkRoomIds,
   finalizeDeleteCallLink,
@@ -313,6 +316,7 @@ export const DataReader: ServerReadableInterface = {
   getAllCallLinks,
   getCallLinkByRoomId,
   getCallLinkRecordByRoomId,
+  getAllAdminCallLinks,
   getAllCallLinkRecordsWithAdminKey,
   getAllMarkedDeletedCallLinkRoomIds,
   getMessagesBetween,
@@ -451,6 +455,8 @@ export const DataWriter: ServerWritableInterface = {
   updateCallLinkState,
   beginDeleteAllCallLinks,
   beginDeleteCallLink,
+  deleteCallHistoryByRoomId,
+  deleteCallLinkAndHistory,
   finalizeDeleteCallLink,
   _removeAllCallLinks,
   deleteCallLinkFromSync,
@@ -3543,6 +3549,14 @@ function _removeAllCallHistory(db: WritableDB): void {
   db.prepare(query).run(params);
 }
 
+/**
+ * Deletes call history by marking it deleted. Tombstoning is needed in case sync messages
+ * come in around the same time, to prevent reappearance of deleted call history.
+ * Limitation: History for admin call links is skipped. Admin call links need to be
+ * deleted on the calling server first, before we can clear local history.
+ *
+ *  @returns ReadonlyArray<string>: message ids of call history messages
+ */
 function clearCallHistory(
   db: WritableDB,
   target: CallLogEventTarget
@@ -3555,17 +3569,33 @@ function clearCallHistory(
     }
     const { timestamp } = callHistory;
 
+    // Admin call links are deleted separately after server confirmation
+    const [selectAdminCallLinksQuery, selectAdminCallLinksParams] = sql`
+      SELECT roomId
+      FROM callLinks
+      WHERE callLinks.adminKey IS NOT NULL;
+    `;
+
+    const adminCallLinkIds: ReadonlyArray<string> = db
+      .prepare(selectAdminCallLinksQuery)
+      .pluck()
+      .all(selectAdminCallLinksParams);
+    const adminCallLinkIdsFragment = sqlJoin(adminCallLinkIds);
+
     const [selectCallsQuery, selectCallsParams] = sql`
       SELECT callsHistory.callId
       FROM callsHistory
       WHERE
-        -- Prior calls
-        (callsHistory.timestamp <= ${timestamp})
-        -- Unused call links
-        OR (
-          callsHistory.mode IS ${CALL_MODE_ADHOC} AND
-          callsHistory.status IS ${CALL_STATUS_PENDING}
-        );
+        (
+          -- Prior calls
+          (callsHistory.timestamp <= ${timestamp})
+          -- Unused call links
+          OR (
+            callsHistory.mode IS ${CALL_MODE_ADHOC} AND
+            callsHistory.status IS ${CALL_STATUS_PENDING}
+          )
+        ) AND
+        callsHistory.peerId NOT IN (${adminCallLinkIdsFragment});
     `;
 
     const deletedCallIds: ReadonlyArray<string> = db

ts/sql/migrations/1230-call-links-admin-key-index.ts

@@ -0,0 +1,28 @@
+// Copyright 2024 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+import type { Database } from '@signalapp/better-sqlite3';
+import type { LoggerType } from '../../types/Logging';
+
+export const version = 1230;
+
+export function updateToSchemaVersion1230(
+  currentVersion: number,
+  db: Database,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 1230) {
+    return;
+  }
+
+  db.transaction(() => {
+    db.exec(`
+      DROP INDEX IF EXISTS callLinks_adminKey;
+
+      CREATE INDEX callLinks_adminKey
+        ON callLinks (adminKey);
+    `);
+
+    db.pragma('user_version = 1230');
+  })();
+  logger.info('updateToSchemaVersion1230: success!');
+}

ts/sql/migrations/index.ts

@@ -98,10 +98,11 @@ import { updateToSchemaVersion1180 } from './1180-add-attachment-download-source
 import { updateToSchemaVersion1190 } from './1190-call-links-storage';
 import { updateToSchemaVersion1200 } from './1200-attachment-download-source-index';
 import { updateToSchemaVersion1210 } from './1210-call-history-started-id';
+import { updateToSchemaVersion1220 } from './1220-blob-sessions';
 import {
-  updateToSchemaVersion1220,
+  updateToSchemaVersion1230,
   version as MAX_VERSION,
-} from './1220-blob-sessions';
+} from './1230-call-links-admin-key-index';
 
 function updateToSchemaVersion1(
   currentVersion: number,
@@ -2069,6 +2070,7 @@ export const SCHEMA_VERSIONS = [
   updateToSchemaVersion1200,
   updateToSchemaVersion1210,
   updateToSchemaVersion1220,
+  updateToSchemaVersion1230,
 ];
 
 export class DBVersionFromFutureError extends Error {

ts/sql/server/callLinks.ts

@@ -20,7 +20,7 @@ import type { ReadableDB, WritableDB } from '../Interface';
 import { prepare } from '../Server';
 import { sql } from '../util';
 import { strictAssert } from '../../util/assert';
-import { CallStatusValue } from '../../types/CallDisposition';
+import { CallStatusValue, DirectCallStatus } from '../../types/CallDisposition';
 import { parseStrict, parseUnknown } from '../../util/schemas';
 
 export function callLinkExists(db: ReadableDB, roomId: string): boolean {
@@ -190,7 +190,10 @@ function assertRoomIdMatchesRootKey(roomId: string, rootKey: string): void {
   );
 }
 
-function deleteCallHistoryByRoomId(db: WritableDB, roomId: string) {
+export function deleteCallHistoryByRoomId(
+  db: WritableDB,
+  roomId: string
+): void {
   const [
     markCallHistoryDeleteByPeerIdQuery,
     markCallHistoryDeleteByPeerIdParams,
@@ -222,17 +225,14 @@ export function deleteCallLinkFromSync(db: WritableDB, roomId: string): void {
   })();
 }
 
-export type DeleteCallLinkOptions = {
-  storageNeedsSync: boolean;
-  deletedAt?: number;
-};
-
-export function beginDeleteCallLink(
-  db: WritableDB,
-  roomId: string,
-  options: DeleteCallLinkOptions
-): void {
-  db.transaction(() => {
+/**
+ * Deletes a non-admin call link from the local database, or if it's an admin call link,
+ * then marks it for deletion and storage sync.
+ *
+ *  @returns boolean: True if storage sync is needed; False if not
+ */
+export function beginDeleteCallLink(db: WritableDB, roomId: string): boolean {
+  return db.transaction(() => {
     // If adminKey is null, then we should delete the call link
     const [deleteNonAdminCallLinksQuery, deleteNonAdminCallLinksParams] = sql`
       DELETE FROM callLinks
@@ -244,35 +244,62 @@ export function beginDeleteCallLink(
       .prepare(deleteNonAdminCallLinksQuery)
       .run(deleteNonAdminCallLinksParams);
 
-    // Skip this query if the call is already deleted
-    if (result.changes === 0) {
-      const { storageNeedsSync } = options;
-      const deletedAt = options.deletedAt ?? new Date().getTime();
-
-      // If the admin key is not null, we should mark it for deletion
-      const [markAdminCallLinksDeletedQuery, markAdminCallLinksDeletedParams] =
-        sql`
-          UPDATE callLinks
-          SET
-            deleted = 1,
-            deletedAt = ${deletedAt},
-            storageNeedsSync = ${storageNeedsSync ? 1 : 0}
-          WHERE adminKey IS NOT NULL
-          AND roomId = ${roomId};
-        `;
-
-      db.prepare(markAdminCallLinksDeletedQuery).run(
-        markAdminCallLinksDeletedParams
-      );
+    // If we successfully deleted the call link, then it was a non-admin call link
+    // and we're done
+    if (result.changes !== 0) {
+      return false;
     }
 
-    deleteCallHistoryByRoomId(db, roomId);
+    const deletedAt = new Date().getTime();
+
+    // If the admin key is not null, we should mark it for deletion
+    const [markAdminCallLinksDeletedQuery, markAdminCallLinksDeletedParams] =
+      sql`
+        UPDATE callLinks
+        SET
+          deleted = 1,
+          deletedAt = ${deletedAt},
+          storageNeedsSync = 1
+        WHERE adminKey IS NOT NULL
+        AND deleted IS NOT 1
+        AND roomId = ${roomId};
+      `;
+
+    const deleteAdminLinkResult = db
+      .prepare(markAdminCallLinksDeletedQuery)
+      .run(markAdminCallLinksDeletedParams);
+    return deleteAdminLinkResult.changes > 0;
   })();
 }
 
-export function beginDeleteAllCallLinks(db: WritableDB): void {
-  const deletedAt = new Date().getTime();
+export function deleteCallLinkAndHistory(db: WritableDB, roomId: string): void {
   db.transaction(() => {
+    const [deleteCallLinkQuery, deleteCallLinkParams] = sql`
+      DELETE FROM callLinks
+        WHERE roomId = ${roomId};
+    `;
+    db.prepare(deleteCallLinkQuery).run(deleteCallLinkParams);
+
+    const [deleteCallHistoryQuery, clearCallHistoryParams] = sql`
+      UPDATE callsHistory
+      SET
+        status = ${DirectCallStatus.Deleted},
+        timestamp = ${Date.now()}
+      WHERE peerId = ${roomId};
+    `;
+    db.prepare(deleteCallHistoryQuery).run(clearCallHistoryParams);
+  })();
+}
+
+/**
+ * Deletes all non-admin call link from the local database, and marks all admin call links
+ * for deletion and storage sync.
+ *
+ *  @returns boolean: True if storage sync is needed; False if not
+ */
+export function beginDeleteAllCallLinks(db: WritableDB): boolean {
+  const deletedAt = new Date().getTime();
+  return db.transaction(() => {
     const [markAdminCallLinksDeletedQuery, markAdminCallLinksDeletedParams] =
       sql`
       UPDATE callLinks
@@ -280,12 +307,13 @@ export function beginDeleteAllCallLinks(db: WritableDB): void {
         deleted = 1,
         deletedAt = ${deletedAt},
         storageNeedsSync = 1
-      WHERE adminKey IS NOT NULL;
+      WHERE adminKey IS NOT NULL
+      AND deleted IS NOT 1;
     `;
 
-    db.prepare(markAdminCallLinksDeletedQuery).run(
-      markAdminCallLinksDeletedParams
-    );
+    const markAdminCallLinksDeletedResult = db
+      .prepare(markAdminCallLinksDeletedQuery)
+      .run(markAdminCallLinksDeletedParams);
 
     const [deleteNonAdminCallLinksQuery] = sql`
       DELETE FROM callLinks
@@ -293,6 +321,9 @@ export function beginDeleteAllCallLinks(db: WritableDB): void {
     `;
 
     db.prepare(deleteNonAdminCallLinksQuery).run();
+
+    // If admin call links were marked deleted, then storage will need sync
+    return markAdminCallLinksDeletedResult.changes > 0;
   })();
 }
 
@@ -311,6 +342,14 @@ export function getAllCallLinkRecordsWithAdminKey(
     .map((item: unknown) => parseUnknown(callLinkRecordSchema, item));
 }
 
+export function getAllAdminCallLinks(
+  db: ReadableDB
+): ReadonlyArray<CallLinkType> {
+  return getAllCallLinkRecordsWithAdminKey(db).map((record: CallLinkRecord) =>
+    callLinkFromRecord(record)
+  );
+}
+
 export function getAllMarkedDeletedCallLinkRoomIds(
   db: ReadableDB
 ): ReadonlyArray<string> {