Add logging for deleted prekeys and other records

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/SignalProtocolStore.ts

@@ -64,6 +64,7 @@ import {
   KYBER_KEY_ID_KEY,
   SIGNED_PRE_KEY_ID_KEY,
 } from './textsecure/AccountManager';
+import { formatGroups, groupWhile } from './util/groupWhile';
 
 const TIMESTAMP_THRESHOLD = 5 * 1000; // 5 seconds
 const LOW_KEYS_THRESHOLD = 25;
@@ -526,7 +527,8 @@ export class SignalProtocolStore extends EventEmitter {
 
     const ids = keyIds.map(keyId => this._getKeyId(ourServiceId, keyId));
 
-    await window.Signal.Data.removeKyberPreKeyById(ids);
+    const changes = await window.Signal.Data.removeKyberPreKeyById(ids);
+    log.info(`removeKyberPreKeys: Removed ${changes} kyber prekeys`);
     ids.forEach(id => {
       kyberPreKeyCache.delete(id);
     });
@@ -543,7 +545,8 @@ export class SignalProtocolStore extends EventEmitter {
     if (this.kyberPreKeys) {
       this.kyberPreKeys.clear();
     }
-    await window.Signal.Data.removeAllKyberPreKeys();
+    const changes = await window.Signal.Data.removeAllKyberPreKeys();
+    log.info(`clearKyberPreKeyStore: Removed ${changes} kyber prekeys`);
   }
 
   // PreKeys
@@ -623,6 +626,7 @@ export class SignalProtocolStore extends EventEmitter {
       toSave.push(preKey);
     });
 
+    log.info(`storePreKeys: Saving ${toSave.length} prekeys`);
     await window.Signal.Data.bulkAddPreKeys(toSave);
     toSave.forEach(preKey => {
       preKeyCache.set(preKey.id, {
@@ -643,7 +647,22 @@ export class SignalProtocolStore extends EventEmitter {
 
     const ids = keyIds.map(keyId => this._getKeyId(ourServiceId, keyId));
 
-    await window.Signal.Data.removePreKeyById(ids);
+    log.info(
+      'removePreKeys: Removing prekeys:',
+      // Potentially hundreds of items, so we'll group together sequences,
+      // take the first 10 of the sequences, format them as ranges,
+      // and log that once.
+      // => '1-10, 12, 14-20'
+      formatGroups(
+        groupWhile(keyIds.sort(), (a, b) => a + 1 === b).slice(0, 10),
+        '-',
+        ', ',
+        String
+      )
+    );
+
+    const changes = await window.Signal.Data.removePreKeyById(ids);
+    log.info(`removePreKeys: Removed ${changes} prekeys`);
     ids.forEach(id => {
       preKeyCache.delete(id);
     });
@@ -657,7 +676,8 @@ export class SignalProtocolStore extends EventEmitter {
     if (this.preKeys) {
       this.preKeys.clear();
     }
-    await window.Signal.Data.removeAllPreKeys();
+    const changes = await window.Signal.Data.removeAllPreKeys();
+    log.info(`clearPreKeyStore: Removed ${changes} prekeys`);
   }
 
   // Signed PreKeys
@@ -797,7 +817,8 @@ export class SignalProtocolStore extends EventEmitter {
     if (this.signedPreKeys) {
       this.signedPreKeys.clear();
     }
-    await window.Signal.Data.removeAllSignedPreKeys();
+    const changes = await window.Signal.Data.removeAllSignedPreKeys();
+    log.info(`clearSignedPreKeysStore: Removed ${changes} signed prekeys`);
   }
 
   // Sender Key
@@ -1728,7 +1749,8 @@ export class SignalProtocolStore extends EventEmitter {
         this.sessions.clear();
       }
       this.pendingSessions.clear();
-      await window.Signal.Data.removeAllSessions();
+      const changes = await window.Signal.Data.removeAllSessions();
+      log.info(`clearSessionStore: Removed ${changes} sessions`);
     });
   }
 
@@ -1848,7 +1870,13 @@ export class SignalProtocolStore extends EventEmitter {
     await this._saveIdentityKey(newRecord);
 
     this.identityKeys.delete(record.fromDB.id);
-    await window.Signal.Data.removeIdentityKeyById(record.fromDB.id);
+    const changes = await window.Signal.Data.removeIdentityKeyById(
+      record.fromDB.id
+    );
+
+    log.info(
+      `getOrMigrateIdentityRecord: Removed ${changes} old identity keys for ${record.fromDB.id}`
+    );
 
     return newRecord;
   }

ts/sql/Interface.ts

@@ -428,27 +428,27 @@ export type DataInterface = {
   removeDB: () => Promise<void>;
   removeIndexedDBFiles: () => Promise<void>;
 
-  removeIdentityKeyById: (id: IdentityKeyIdType) => Promise<void>;
+  removeIdentityKeyById: (id: IdentityKeyIdType) => Promise<number>;
-  removeAllIdentityKeys: () => Promise<void>;
+  removeAllIdentityKeys: () => Promise<number>;
 
   removeKyberPreKeyById: (
     id: PreKeyIdType | Array<PreKeyIdType>
-  ) => Promise<void>;
+  ) => Promise<number>;
   removeKyberPreKeysByServiceId: (serviceId: ServiceIdString) => Promise<void>;
-  removeAllKyberPreKeys: () => Promise<void>;
+  removeAllKyberPreKeys: () => Promise<number>;
 
-  removePreKeyById: (id: PreKeyIdType | Array<PreKeyIdType>) => Promise<void>;
+  removePreKeyById: (id: PreKeyIdType | Array<PreKeyIdType>) => Promise<number>;
   removePreKeysByServiceId: (serviceId: ServiceIdString) => Promise<void>;
-  removeAllPreKeys: () => Promise<void>;
+  removeAllPreKeys: () => Promise<number>;
 
   removeSignedPreKeyById: (
     id: SignedPreKeyIdType | Array<SignedPreKeyIdType>
-  ) => Promise<void>;
+  ) => Promise<number>;
   removeSignedPreKeysByServiceId: (serviceId: ServiceIdString) => Promise<void>;
-  removeAllSignedPreKeys: () => Promise<void>;
+  removeAllSignedPreKeys: () => Promise<number>;
 
-  removeAllItems: () => Promise<void>;
+  removeAllItems: () => Promise<number>;
-  removeItemById: (id: ItemKeyType | Array<ItemKeyType>) => Promise<void>;
+  removeItemById: (id: ItemKeyType | Array<ItemKeyType>) => Promise<number>;
 
   createOrUpdateSenderKey: (key: SenderKeyType) => Promise<void>;
   getSenderKeyById: (id: SenderKeyIdType) => Promise<SenderKeyType | undefined>;
@@ -494,10 +494,10 @@ export type DataInterface = {
     unprocessed: Array<UnprocessedType>;
   }): Promise<void>;
   bulkAddSessions: (array: Array<SessionType>) => Promise<void>;
-  removeSessionById: (id: SessionIdType) => Promise<void>;
+  removeSessionById: (id: SessionIdType) => Promise<number>;
   removeSessionsByConversation: (conversationId: string) => Promise<void>;
   removeSessionsByServiceId: (serviceId: ServiceIdString) => Promise<void>;
-  removeAllSessions: () => Promise<void>;
+  removeAllSessions: () => Promise<number>;
   getAllSessions: () => Promise<Array<SessionType>>;
 
   getConversationCount: () => Promise<number>;
@@ -704,8 +704,8 @@ export type DataInterface = {
     id: string,
     pending: boolean
   ) => Promise<void>;
-  removeAttachmentDownloadJob: (id: string) => Promise<void>;
+  removeAttachmentDownloadJob: (id: string) => Promise<number>;
-  removeAllAttachmentDownloadJobs: () => Promise<void>;
+  removeAllAttachmentDownloadJobs: () => Promise<number>;
 
   createOrUpdateStickerPack: (pack: StickerPackType) => Promise<void>;
   updateStickerPackStatus: (

ts/sql/Server.ts

@@ -742,10 +742,10 @@ async function bulkAddIdentityKeys(
 ): Promise<void> {
   return bulkAdd(await getWritableInstance(), IDENTITY_KEYS_TABLE, array);
 }
-async function removeIdentityKeyById(id: IdentityKeyIdType): Promise<void> {
+async function removeIdentityKeyById(id: IdentityKeyIdType): Promise<number> {
   return removeById(await getWritableInstance(), IDENTITY_KEYS_TABLE, id);
 }
-async function removeAllIdentityKeys(): Promise<void> {
+async function removeAllIdentityKeys(): Promise<number> {
   return removeAllFromTable(await getWritableInstance(), IDENTITY_KEYS_TABLE);
 }
 async function getAllIdentityKeys(): Promise<Array<StoredIdentityKeyType>> {
@@ -774,7 +774,7 @@ async function bulkAddKyberPreKeys(
 }
 async function removeKyberPreKeyById(
   id: PreKeyIdType | Array<PreKeyIdType>
-): Promise<void> {
+): Promise<number> {
   return removeById(await getWritableInstance(), KYBER_PRE_KEYS_TABLE, id);
 }
 async function removeKyberPreKeysByServiceId(
@@ -787,7 +787,7 @@ async function removeKyberPreKeysByServiceId(
     serviceId,
   });
 }
-async function removeAllKyberPreKeys(): Promise<void> {
+async function removeAllKyberPreKeys(): Promise<number> {
   return removeAllFromTable(await getWritableInstance(), KYBER_PRE_KEYS_TABLE);
 }
 async function getAllKyberPreKeys(): Promise<Array<StoredKyberPreKeyType>> {
@@ -808,7 +808,7 @@ async function bulkAddPreKeys(array: Array<StoredPreKeyType>): Promise<void> {
 }
 async function removePreKeyById(
   id: PreKeyIdType | Array<PreKeyIdType>
-): Promise<void> {
+): Promise<number> {
   return removeById(await getWritableInstance(), PRE_KEYS_TABLE, id);
 }
 async function removePreKeysByServiceId(
@@ -821,7 +821,7 @@ async function removePreKeysByServiceId(
     serviceId,
   });
 }
-async function removeAllPreKeys(): Promise<void> {
+async function removeAllPreKeys(): Promise<number> {
   return removeAllFromTable(await getWritableInstance(), PRE_KEYS_TABLE);
 }
 async function getAllPreKeys(): Promise<Array<StoredPreKeyType>> {
@@ -850,7 +850,7 @@ async function bulkAddSignedPreKeys(
 }
 async function removeSignedPreKeyById(
   id: SignedPreKeyIdType | Array<SignedPreKeyIdType>
-): Promise<void> {
+): Promise<number> {
   return removeById(await getWritableInstance(), SIGNED_PRE_KEYS_TABLE, id);
 }
 async function removeSignedPreKeysByServiceId(
@@ -863,7 +863,7 @@ async function removeSignedPreKeysByServiceId(
     serviceId,
   });
 }
-async function removeAllSignedPreKeys(): Promise<void> {
+async function removeAllSignedPreKeys(): Promise<number> {
   return removeAllFromTable(await getWritableInstance(), SIGNED_PRE_KEYS_TABLE);
 }
 async function getAllSignedPreKeys(): Promise<Array<StoredSignedPreKeyType>> {
@@ -912,10 +912,10 @@ async function getAllItems(): Promise<StoredAllItemsType> {
 }
 async function removeItemById(
   id: ItemKeyType | Array<ItemKeyType>
-): Promise<void> {
+): Promise<number> {
   return removeById(await getWritableInstance(), ITEMS_TABLE, id);
 }
-async function removeAllItems(): Promise<void> {
+async function removeAllItems(): Promise<number> {
   return removeAllFromTable(await getWritableInstance(), ITEMS_TABLE);
 }
 
@@ -1421,7 +1421,7 @@ async function commitDecryptResult({
 async function bulkAddSessions(array: Array<SessionType>): Promise<void> {
   return bulkAdd(await getWritableInstance(), SESSIONS_TABLE, array);
 }
-async function removeSessionById(id: SessionIdType): Promise<void> {
+async function removeSessionById(id: SessionIdType): Promise<number> {
   return removeById(await getWritableInstance(), SESSIONS_TABLE, id);
 }
 async function removeSessionsByConversation(
@@ -1450,7 +1450,7 @@ async function removeSessionsByServiceId(
     serviceId,
   });
 }
-async function removeAllSessions(): Promise<void> {
+async function removeAllSessions(): Promise<number> {
   return removeAllFromTable(await getWritableInstance(), SESSIONS_TABLE);
 }
 async function getAllSessions(): Promise<Array<SessionType>> {
@@ -4331,14 +4331,14 @@ async function resetAttachmentDownloadPending(): Promise<void> {
     `
   ).run();
 }
-function removeAttachmentDownloadJobSync(db: Database, id: string): void {
+function removeAttachmentDownloadJobSync(db: Database, id: string): number {
   return removeById(db, ATTACHMENT_DOWNLOADS_TABLE, id);
 }
-async function removeAttachmentDownloadJob(id: string): Promise<void> {
+async function removeAttachmentDownloadJob(id: string): Promise<number> {
   const db = await getWritableInstance();
   return removeAttachmentDownloadJobSync(db, id);
 }
-async function removeAllAttachmentDownloadJobs(): Promise<void> {
+async function removeAllAttachmentDownloadJobs(): Promise<number> {
   return removeAllFromTable(
     await getWritableInstance(),
     ATTACHMENT_DOWNLOADS_TABLE

ts/sql/util.ts

@@ -323,37 +323,39 @@ export function getById<Key extends string | number, Result = unknown>(
 
 export function removeById<Key extends string | number>(
   db: Database,
-  table: TableType,
+  tableName: TableType,
   id: Key | Array<Key>
-): void {
+): number {
+  const table = sqlConstant(tableName);
   if (!Array.isArray(id)) {
-    db.prepare<Query>(
+    const [query, params] = sql`
-      `
       DELETE FROM ${table}
-      WHERE id = $id;
+      WHERE id = ${id};
-      `
+    `;
-    ).run({ id });
+    return db.prepare(query).run(params).changes;
-    return;
   }
 
   if (!id.length) {
     throw new Error('removeById: No ids to delete!');
   }
 
+  let totalChanges = 0;
+
   const removeByIdsSync = (ids: ReadonlyArray<string | number>): void => {
-    db.prepare<ArrayQuery>(
+    const [query, params] = sql`
-      `
       DELETE FROM ${table}
-      WHERE id IN ( ${id.map(() => '?').join(', ')} );
+      WHERE id IN (${sqlJoin(ids, ', ')});
-      `
+    `;
-    ).run(ids);
+    totalChanges += db.prepare(query).run(params).changes;
   };
 
   batchMultiVarQuery(db, id, removeByIdsSync);
+
+  return totalChanges;
 }
 
-export function removeAllFromTable(db: Database, table: TableType): void {
+export function removeAllFromTable(db: Database, table: TableType): number {
-  db.prepare<EmptyQuery>(`DELETE FROM ${table};`).run();
+  return db.prepare<EmptyQuery>(`DELETE FROM ${table};`).run().changes;
 }
 
 export function getAllFromTable<T>(db: Database, table: TableType): Array<T> {

ts/test-node/util/groupBy_test.ts

@@ -0,0 +1,63 @@
+// Copyright 2023 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { groupWhile, formatGroups } from '../../util/groupWhile';
+
+describe('groupWhile/formatGroups', () => {
+  function check(
+    input: Array<number>,
+    expected: Array<Array<number>>,
+    expectedFormatted: string
+  ) {
+    const result = groupWhile(input, (item, prev) => {
+      return prev + 1 === item;
+    });
+    assert.deepEqual(result, expected);
+    const formatted = formatGroups(result, '-', ', ', String);
+    assert.equal(formatted, expectedFormatted);
+  }
+
+  it('empty', () => {
+    check([], [], '');
+  });
+
+  it('one', () => {
+    check([1], [[1]], '1');
+  });
+
+  it('sequential', () => {
+    check([1, 2, 3, 4, 5, 6], [[1, 2, 3, 4, 5, 6]], '1-6');
+  });
+
+  it('non-sequential', () => {
+    check(
+      [1, 2, 4, 5],
+      [
+        [1, 2],
+        [4, 5],
+      ],
+      '1-2, 4-5'
+    );
+  });
+
+  it('multiple non-sequential', () => {
+    check(
+      [1, 2, 4, 5, 7, 9, 10],
+      [[1, 2], [4, 5], [7], [9, 10]],
+      '1-2, 4-5, 7, 9-10'
+    );
+  });
+
+  function range(start: number, end: number) {
+    return Array.from({ length: end - start + 1 }, (_, index) => start + index);
+  }
+
+  it('huge', () => {
+    check(
+      [...range(1, 100), ...range(102, 200), ...range(202, 300)],
+      [range(1, 100), range(102, 200), range(202, 300)],
+      '1-100, 102-200, 202-300'
+    );
+  });
+});

ts/util/groupWhile.ts

@@ -0,0 +1,69 @@
+// Copyright 2023 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+/**
+ * Accepts an array and a predicate function. Returns an array of arrays, where
+ * each time the predicate returns false, a new sub-array is started.
+ *
+ * Useful for grouping sequential items in an array.
+ *
+ * @example
+ * ```ts
+ * groupWhile([1, 2, 3, 4, 5, 6], (item, prev) => {
+ *   return prev + 1 === item
+ * })
+ * // => [[1, 2, 3], [4, 5, 6]]
+ * ```
+ */
+export function groupWhile<T>(
+  array: ReadonlyArray<T>,
+  iteratee: (item: T, prev: T) => boolean
+): Array<Array<T>> {
+  const groups: Array<Array<T>> = [];
+  let cursor = 0;
+  while (cursor < array.length) {
+    const group: Array<T> = [];
+    do {
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      group.push(array[cursor]!);
+      cursor += 1;
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      if (!iteratee(array[cursor]!, array[cursor - 1]!)) {
+        break;
+      }
+    } while (cursor < array.length);
+    groups.push(group);
+  }
+  return groups;
+}
+
+/**
+ * @example
+ * ```ts
+ * let result = [[1, 2], [4, 5], [7], [9, 10]]
+ * let formatted = formatGroups(result, "-", ", ", String)
+ * // => "1-2, 4-5, 7, 9-10"
+ * ```
+ */
+export function formatGroups<T>(
+  groups: Array<Array<T>>,
+  rangeSeparator: string,
+  groupSeparator: string,
+  formatItem: (value: T) => string
+): string {
+  return groups
+    .map(group => {
+      if (group.length === 0) {
+        return '';
+      }
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      const start = formatItem(group.at(0)!);
+      if (group.length === 1) {
+        return start;
+      }
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      const end = formatItem(group.at(-1)!);
+      return `${start}${rangeSeparator}${end}`;
+    })
+    .join(groupSeparator);
+}