diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index 899f2f56fb6..f52be0be1d6 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -787,8 +787,8 @@ type WritableInterface = { markCallHistoryDeleted: (callId: string) => void; cleanupCallHistoryMessages: () => void; markCallHistoryRead(callId: string): void; - markAllCallHistoryRead(target: CallLogEventTarget): void; - markAllCallHistoryReadInConversation(target: CallLogEventTarget): void; + markAllCallHistoryRead(target: CallLogEventTarget): number; + markAllCallHistoryReadInConversation(target: CallLogEventTarget): number; saveCallHistory(callHistory: CallHistoryDetails): void; markCallHistoryMissed(callIds: ReadonlyArray): void; getRecentStaleRingsAndMarkOlderMissed(): ReadonlyArray; diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index 25974f5fbc2..c27157f9c2e 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -3491,7 +3491,12 @@ function clearCallHistory( target: CallLogEventTarget ): ReadonlyArray { return db.transaction(() => { - const timestamp = getMessageTimestampForCallLogEventTarget(db, target); + const callHistory = getCallHistoryForCallLogEventTarget(db, target); + if (callHistory == null) { + logger.error('clearCallHistory: Target call not found'); + return []; + } + const { timestamp } = callHistory; const [selectCallsQuery, selectCallsParams] = sql` SELECT callsHistory.callId @@ -3650,12 +3655,13 @@ function markCallHistoryRead(db: WritableDB, callId: string): void { db.prepare(query).run(params); } -function getMessageTimestampForCallLogEventTarget( +function getCallHistoryForCallLogEventTarget( db: ReadableDB, target: CallLogEventTarget -): number { - let { callId, peerId } = target; - const { timestamp } = target; +): CallHistoryDetails | null { + const { callId, peerId, timestamp } = target; + + let row: unknown; if (callId == null || peerId == null) { const predicate = @@ -3665,7 +3671,7 @@ function getMessageTimestampForCallLogEventTarget( // Get the most recent call history timestamp for the target.timestamp const [selectQuery, selectParams] = sql` - SELECT callsHistory.callId, callsHistory.peerId + SELECT * FROM callsHistory WHERE ${predicate} AND callsHistory.timestamp <= ${timestamp} @@ -3673,54 +3679,130 @@ function getMessageTimestampForCallLogEventTarget( LIMIT 1 `; - const row = db.prepare(selectQuery).get(selectParams); - if (row == null) { - logger.warn('getTimestampForCallLogEventTarget: Target call not found'); - return timestamp; - } - callId = row.callId as string; - peerId = row.peerId as AciString; + row = db.prepare(selectQuery).get(selectParams); + } else { + const [selectQuery, selectParams] = sql` + SELECT * + FROM callsHistory + WHERE callsHistory.peerId IS ${target.peerId} + AND callsHistory.callId IS ${target.callId} + LIMIT 1 + `; + + row = db.prepare(selectQuery).get(selectParams); } + if (row == null) { + return null; + } + + return callHistoryDetailsSchema.parse(row); +} + +function getConversationIdForCallHistory( + db: ReadableDB, + callHistory: CallHistoryDetails +): string | null { + const { peerId, mode } = callHistory; + + if (mode === CallMode.Adhoc) { + throw new Error( + 'getConversationIdForCallHistory: Adhoc calls do not have conversations' + ); + } + + const predicate = + mode === CallMode.Direct + ? sqlFragment`serviceId IS ${peerId}` + : sqlFragment`groupId IS ${peerId}`; + + const [selectConversationIdQuery, selectConversationIdParams] = sql` + SELECT id FROM conversations + WHERE ${predicate} + `; + + const conversationId = db + .prepare(selectConversationIdQuery) + .pluck() + .get(selectConversationIdParams); + + if (typeof conversationId !== 'string') { + logger.warn('getConversationIdForCallHistory: Unknown conversation'); + return null; + } + + return conversationId ?? null; +} + +function getMessageReceivedAtForCall( + db: ReadableDB, + callId: string, + conversationId: string +): number | null { const [selectQuery, selectParams] = sql` - SELECT messages.sent_at + SELECT messages.received_at FROM messages WHERE messages.type IS 'call-history' - AND messages.conversationId IS ${peerId} + AND messages.conversationId IS ${conversationId} AND messages.callId IS ${callId} LIMIT 1 `; - const messageTimestamp = db.prepare(selectQuery).pluck().get(selectParams); - - if (messageTimestamp == null) { - logger.warn( - 'getTimestampForCallLogEventTarget: Target call message not found' - ); + const receivedAt = db.prepare(selectQuery).pluck().get(selectParams); + if (receivedAt == null) { + logger.warn('getMessageReceivedAtForCall: Target call message not found'); } - return messageTimestamp ?? target.timestamp; + return receivedAt ?? null; } export function markAllCallHistoryRead( db: WritableDB, target: CallLogEventTarget, inConversation = false -): void { +): number { if (inConversation) { strictAssert(target.peerId, 'peerId is required'); } - db.transaction(() => { + return db.transaction(() => { + const callHistory = getCallHistoryForCallLogEventTarget(db, target); + if (callHistory == null) { + logger.warn('markAllCallHistoryRead: Target call not found'); + return 0; + } + + const { callId } = callHistory; + + strictAssert( + target.callId == null || callId === target.callId, + 'Call ID must be the same as target if supplied' + ); + + const conversationId = getConversationIdForCallHistory(db, callHistory); + if (conversationId == null) { + logger.warn('markAllCallHistoryRead: Conversation not found for call'); + return 0; + } + logger.info(`markAllCallHistoryRead: Found conversation ${conversationId}`); + const receivedAt = getMessageReceivedAtForCall(db, callId, conversationId); + + if (receivedAt == null) { + logger.warn('markAllCallHistoryRead: Message not found for call'); + return 0; + } + + const predicate = inConversation + ? sqlFragment`messages.conversationId IS ${conversationId}` + : sqlFragment`TRUE`; + const jsonPatch = JSON.stringify({ seenStatus: SeenStatus.Seen, }); - const timestamp = getMessageTimestampForCallLogEventTarget(db, target); - - const predicate = inConversation - ? sqlFragment`messages.conversationId IS ${target.peerId}` - : sqlFragment`TRUE`; + logger.warn( + `markAllCallHistoryRead: Marking calls before ${receivedAt} read` + ); const [updateQuery, updateParams] = sql` UPDATE messages @@ -3730,19 +3812,20 @@ export function markAllCallHistoryRead( WHERE messages.type IS 'call-history' AND ${predicate} AND messages.seenStatus IS ${SEEN_STATUS_UNSEEN} - AND messages.sent_at <= ${timestamp} + AND messages.received_at <= ${receivedAt}; `; - db.prepare(updateQuery).run(updateParams); + const result = db.prepare(updateQuery).run(updateParams); + return result.changes; })(); } function markAllCallHistoryReadInConversation( db: WritableDB, target: CallLogEventTarget -): void { +): number { strictAssert(target.peerId, 'peerId is required'); - markAllCallHistoryRead(db, target, true); + return markAllCallHistoryRead(db, target, true); } function getCallHistoryGroupData( diff --git a/ts/sql/migrations/1170-update-call-history-unread-index.ts b/ts/sql/migrations/1170-update-call-history-unread-index.ts new file mode 100644 index 00000000000..7dc7c0d23a8 --- /dev/null +++ b/ts/sql/migrations/1170-update-call-history-unread-index.ts @@ -0,0 +1,29 @@ +// 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'; +import { sql } from '../util'; + +export const version = 1170; +export function updateToSchemaVersion1170( + currentVersion: number, + db: Database, + logger: LoggerType +): void { + if (currentVersion >= 1170) { + return; + } + + db.transaction(() => { + const [query] = sql` + DROP INDEX IF EXISTS messages_callHistory_markReadBefore; + CREATE INDEX messages_callHistory_markReadBefore + ON messages (type, seenStatus, received_at DESC) + WHERE type IS 'call-history'; + `; + db.exec(query); + + db.pragma('user_version = 1170'); + })(); + logger.info('updateToSchemaVersion1170: success!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index c9323e2823b..41de8a23034 100644 --- a/ts/sql/migrations/index.ts +++ b/ts/sql/migrations/index.ts @@ -92,10 +92,11 @@ import { updateToSchemaVersion1120 } from './1120-messages-foreign-keys-indexes' import { updateToSchemaVersion1130 } from './1130-isStory-index'; import { updateToSchemaVersion1140 } from './1140-call-links-deleted-column'; import { updateToSchemaVersion1150 } from './1150-expire-timer-version'; +import { updateToSchemaVersion1160 } from './1160-optimize-calls-unread-count'; import { - updateToSchemaVersion1160, + updateToSchemaVersion1170, version as MAX_VERSION, -} from './1160-optimize-calls-unread-count'; +} from './1170-update-call-history-unread-index'; function updateToSchemaVersion1( currentVersion: number, @@ -2056,6 +2057,7 @@ export const SCHEMA_VERSIONS = [ updateToSchemaVersion1140, updateToSchemaVersion1150, updateToSchemaVersion1160, + updateToSchemaVersion1170, ]; export class DBVersionFromFutureError extends Error { diff --git a/ts/test-node/sql/migration_1100_test.ts b/ts/test-node/sql/migration_1100_test.ts index 7239b3fdf31..529a9cd2556 100644 --- a/ts/test-node/sql/migration_1100_test.ts +++ b/ts/test-node/sql/migration_1100_test.ts @@ -19,7 +19,8 @@ describe('SQL/updateToSchemaVersion1100', () => { let db: WritableDB; beforeEach(() => { db = createDB(); - updateToVersion(db, 1100); + // index updated in 1170 + updateToVersion(db, 1170); }); afterEach(() => { @@ -29,14 +30,26 @@ describe('SQL/updateToSchemaVersion1100', () => { describe('Optimize markAllCallHistoryReadInConversation', () => { it('is fast', () => { const COUNT = 10_000; + const CONVERSATIONS = 30; + + const conversations = Array.from( + { length: CONVERSATIONS }, + (_, index) => { + return { + id: `test-conversation-${index}`, + groupId: `test-conversation-${index}`, + serviceId: `test-conversation-${index}`, + }; + } + ); const messages = Array.from({ length: COUNT }, (_, index) => { return { id: `test-message-${index}`, type: 'call-history', seenStatus: SeenStatus.Unseen, - conversationId: `test-conversation-${index % 30}`, - sent_at: index, + conversationId: `test-conversation-${index % CONVERSATIONS}`, + received_at: index, json: { callId: `test-call-${index}`, }, @@ -46,7 +59,7 @@ describe('SQL/updateToSchemaVersion1100', () => { const callsHistory = Array.from({ length: COUNT }, (_, index) => { return { callId: `test-call-${index}`, - peerId: `test-conversation-${index % 30}`, + peerId: `test-conversation-${index % CONVERSATIONS}`, timestamp: index, ringerId: null, mode: CallMode.Direct, @@ -56,6 +69,7 @@ describe('SQL/updateToSchemaVersion1100', () => { }; }); + insertData(db, 'conversations', conversations); insertData(db, 'messages', messages); insertData(db, 'callsHistory', callsHistory); @@ -72,8 +86,9 @@ describe('SQL/updateToSchemaVersion1100', () => { }; const start = performance.now(); - markAllCallHistoryRead(db, target, true); + const changes = markAllCallHistoryRead(db, target, true); const end = performance.now(); + assert.equal(changes, Math.ceil(COUNT / CONVERSATIONS)); assert.isBelow(end - start, 50); }); }); diff --git a/ts/util/callDisposition.ts b/ts/util/callDisposition.ts index 4bfb44616b4..a11b31de740 100644 --- a/ts/util/callDisposition.ts +++ b/ts/util/callDisposition.ts @@ -1324,12 +1324,17 @@ export async function markAllCallHistoryReadAndSync( log.info( `markAllCallHistoryReadAndSync: Marking call history read before (${latestCall.callId}, ${latestCall.timestamp})` ); + let count: number; if (inConversation) { - await DataWriter.markAllCallHistoryReadInConversation(latestCall); + count = await DataWriter.markAllCallHistoryReadInConversation(latestCall); } else { - await DataWriter.markAllCallHistoryRead(latestCall); + count = await DataWriter.markAllCallHistoryRead(latestCall); } + log.info( + `markAllCallHistoryReadAndSync: Marked ${count} call history messages read` + ); + const ourAci = window.textsecure.storage.user.getCheckedAci(); const callLogEvent = new Proto.SyncMessage.CallLogEvent({ diff --git a/ts/util/onCallLogEventSync.ts b/ts/util/onCallLogEventSync.ts index 3ad5d3f594e..54efbb12aa1 100644 --- a/ts/util/onCallLogEventSync.ts +++ b/ts/util/onCallLogEventSync.ts @@ -39,7 +39,10 @@ export async function onCallLogEventSync( } else if (type === CallLogEvent.MarkedAsRead) { log.info('onCallLogEventSync: Marking call history read'); try { - await DataWriter.markAllCallHistoryRead(target); + const count = await DataWriter.markAllCallHistoryRead(target); + log.info( + `onCallLogEventSync: Marked ${count} call history messages read` + ); } finally { window.reduxActions.callHistory.updateCallHistoryUnreadCount(); } @@ -48,7 +51,11 @@ export async function onCallLogEventSync( log.info('onCallLogEventSync: Marking call history read in conversation'); try { strictAssert(peerId, 'Missing peerId'); - await DataWriter.markAllCallHistoryReadInConversation(target); + const count = + await DataWriter.markAllCallHistoryReadInConversation(target); + log.info( + `onCallLogEventSync: Marked ${count} call history messages read` + ); } finally { window.reduxActions.callHistory.updateCallHistoryUnreadCount(); }