From 0273e1ac1d0a79530e68817d3b8b08b2d889ff87 Mon Sep 17 00:00:00 2001
From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com>
Date: Thu, 17 Aug 2023 15:29:02 -0700
Subject: [PATCH] Fix call migration for unregistered conversations without
 serviceId

---
 ts/sql/migrations/89-call-history.ts  | 33 +++++++++------------
 ts/test-node/sql/migration_89_test.ts | 42 ++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/ts/sql/migrations/89-call-history.ts b/ts/sql/migrations/89-call-history.ts
index 76d2a434a1f8..d6be6e459a14 100644
--- a/ts/sql/migrations/89-call-history.ts
+++ b/ts/sql/migrations/89-call-history.ts
@@ -21,7 +21,6 @@ import { CallMode } from '../../types/Calling';
 import type { MessageType, ConversationType } from '../Interface';
 import { strictAssert } from '../../util/assert';
 import { missingCaseError } from '../../util/missingCaseError';
-import type { ServiceIdString } from '../../types/ServiceId';
 import { isAciString } from '../../types/ServiceId';
 
 // Legacy type for calls that never had a call id
@@ -87,9 +86,16 @@ function upcastCallHistoryDetailsFromDiskType(
 }
 
 function getPeerIdFromConversation(
-  conversation: Pick<ConversationType, 'type' | 'serviceId' | 'groupId'>
+  conversation: ConversationType,
+  logger: LoggerType
 ): string {
   if (conversation.type === 'private') {
+    if (conversation.serviceId == null) {
+      logger.warn(
+        `updateToSchemaVersion89: Private conversation (${conversation.id}) was missing serviceId (discoveredUnregisteredAt: ${conversation.discoveredUnregisteredAt})`
+      );
+      return conversation.id;
+    }
     strictAssert(
       isAciString(conversation.serviceId),
       'ACI must exist for direct chat'
@@ -228,9 +234,7 @@ export default function updateToSchemaVersion89(
     const [selectQuery] = sql`
       SELECT
         messages.json AS messageJson,
-        conversations.type AS conversationType,
-        conversations.serviceId AS conversationServiceId,
-        conversations.groupId AS conversationGroupId
+        conversations.json AS conversationJson
       FROM messages
       LEFT JOIN conversations ON conversations.id = messages.conversationId
       WHERE messages.type = 'call-history'
@@ -243,28 +247,19 @@ export default function updateToSchemaVersion89(
     // Must match query above
     type CallHistoryRow = {
       messageJson: string;
-      conversationType: ConversationType['type'];
-      conversationServiceId: ServiceIdString | undefined;
-      conversationGroupId: string | undefined;
+      conversationJson: string;
     };
 
     const rows: Array<CallHistoryRow> = db.prepare(selectQuery).all();
 
     for (const row of rows) {
-      const {
-        messageJson,
-        conversationType,
-        conversationServiceId,
-        conversationGroupId,
-      } = row;
+      const { messageJson, conversationJson } = row;
       const message = jsonToObject<MessageWithCallHistoryDetails>(messageJson);
+      const conversation = jsonToObject<ConversationType>(conversationJson);
+
       const details = message.callHistoryDetails;
 
-      const peerId = getPeerIdFromConversation({
-        type: conversationType,
-        serviceId: conversationServiceId,
-        groupId: conversationGroupId,
-      });
+      const peerId = getPeerIdFromConversation(conversation, logger);
 
       const callHistory = convertLegacyCallDetails(
         ourUuid,
diff --git a/ts/test-node/sql/migration_89_test.ts b/ts/test-node/sql/migration_89_test.ts
index 10dab08b6616..b1e518b369a2 100644
--- a/ts/test-node/sql/migration_89_test.ts
+++ b/ts/test-node/sql/migration_89_test.ts
@@ -99,16 +99,31 @@ describe('SQL/updateToSchemaVersion89', () => {
     return message;
   }
 
-  function createConversation(type: 'private' | 'group') {
+  function createConversation(
+    type: 'private' | 'group',
+    discoveredUnregisteredAt?: number
+  ) {
     const id = generateGuid();
-    const serviceId = type === 'private' ? generateGuid() : null;
+    const serviceId =
+      // Emulate older unregistered conversations
+      type === 'private' && discoveredUnregisteredAt == null
+        ? generateGuid()
+        : null;
     const groupId = type === 'group' ? generateGuid() : null;
 
+    const json = JSON.stringify({
+      type,
+      id,
+      serviceId,
+      groupId,
+      discoveredUnregisteredAt,
+    });
+
     const [query, params] = sql`
       INSERT INTO conversations
-        (id, type, serviceId, groupId)
+        (id, type, serviceId, groupId, json)
       VALUES
-        (${id}, ${type}, ${serviceId}, ${groupId});
+        (${id}, ${type}, ${serviceId}, ${groupId}, ${json});
     `;
 
     db.prepare(query).run(params);
@@ -266,6 +281,25 @@ describe('SQL/updateToSchemaVersion89', () => {
     assert.strictEqual(callHistory[1].peerId, conversation2.groupId);
   });
 
+  it('migrates older unregistered conversations with no serviceId', () => {
+    updateToVersion(db, 88);
+
+    const conversation = createConversation('private', Date.now());
+    createCallHistoryMessage({
+      messageId: generateGuid(),
+      conversationId: conversation.id,
+      callHistoryDetails: getDirectCallHistoryDetails({
+        callId: '123',
+      }),
+    });
+
+    updateToVersion(db, 89);
+
+    const callHistory = getAllCallHistory();
+    assert.strictEqual(callHistory.length, 1);
+    assert.strictEqual(callHistory[0].peerId, conversation.id);
+  });
+
   describe('clients with schema version 87', () => {
     function createCallHistoryTable() {
       const [query] = sql`