From 32485abf06bc33da05bb04490a55440e4997e9d1 Mon Sep 17 00:00:00 2001 From: ayumi-signal <143036029+ayumi-signal@users.noreply.github.com> Date: Fri, 20 Sep 2024 12:17:46 -0700 Subject: [PATCH] Async refresh call links on CallLinkUpdate sync --- ts/jobs/callLinkRefreshJobQueue.ts | 12 ++++-- ts/services/storageRecordOps.ts | 10 ++++- ts/state/ducks/calling.ts | 41 +++++++------------- ts/test-electron/state/ducks/calling_test.ts | 33 ++++++---------- ts/util/callLinks.ts | 5 ++- 5 files changed, 45 insertions(+), 56 deletions(-) diff --git a/ts/jobs/callLinkRefreshJobQueue.ts b/ts/jobs/callLinkRefreshJobQueue.ts index 9272b2c708..c8ef3c7d61 100644 --- a/ts/jobs/callLinkRefreshJobQueue.ts +++ b/ts/jobs/callLinkRefreshJobQueue.ts @@ -24,6 +24,8 @@ const DEFAULT_SLEEP_TIME = 20 * SECOND; const callLinkRefreshJobDataSchema = z.object({ roomId: z.string(), + deleteLocallyIfMissingOnCallingServer: z.boolean(), + source: z.string(), }); export type CallLinkRefreshJobData = z.infer< @@ -54,8 +56,8 @@ export class CallLinkRefreshJobQueue extends JobQueue { }: Readonly<{ data: CallLinkRefreshJobData; timestamp: number }>, { attempt, log }: Readonly<{ attempt: number; log: LoggerType }> ): Promise { - const { roomId } = data; - const logId = `callLinkRefreshJobQueue(${roomId}).run`; + const { roomId, deleteLocallyIfMissingOnCallingServer, source } = data; + const logId = `callLinkRefreshJobQueue(${roomId}, source=${source}).run`; log.info(`${logId}: Starting`); const timeRemaining = timestamp + MAX_RETRY_TIME - Date.now(); @@ -89,9 +91,9 @@ export class CallLinkRefreshJobQueue extends JobQueue { }; await DataWriter.updateCallLinkState(roomId, freshCallLinkState); window.reduxActions.calling.handleCallLinkUpdateLocal(callLink); - } else { + } else if (deleteLocallyIfMissingOnCallingServer) { log.info( - `${logId}: Call link not found on server, deleting local call link` + `${logId}: Call link not found on server and deleteLocallyIfMissingOnCallingServer; deleting local call link` ); // This will leave a storage service record, and it's up to primary to delete it await DataWriter.beginDeleteCallLink(roomId, { @@ -99,6 +101,8 @@ export class CallLinkRefreshJobQueue extends JobQueue { }); await DataWriter.finalizeDeleteCallLink(roomId); window.reduxActions.calling.handleCallLinkDelete({ roomId }); + } else { + log.info(`${logId}: Call link not found on server, ignoring`); } } catch (err) { error = err; diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 5d56e2855f..dee4348c5e 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -2025,7 +2025,15 @@ export async function mergeCallLinkRecord( DataWriter.saveCallHistory(callHistory), ]); - drop(callLinkRefreshJobQueue.add({ roomId: callLink.roomId })); + // The local DB record is a placeholder until confirmed refreshed. If it's gone from + // the calling server then delete the local record. + drop( + callLinkRefreshJobQueue.add({ + roomId: callLink.roomId, + deleteLocallyIfMissingOnCallingServer: true, + source: 'storage.mergeCallLinkRecord', + }) + ); window.reduxActions.callHistory.addCallHistory(callHistory); } diff --git a/ts/state/ducks/calling.ts b/ts/state/ducks/calling.ts index de29ccc0b2..8471ef77c3 100644 --- a/ts/state/ducks/calling.ts +++ b/ts/state/ducks/calling.ts @@ -107,6 +107,7 @@ import type { StartCallData } from '../../components/ConfirmLeaveCallModal'; import { getCallLinksByRoomId } from '../selectors/calling'; import { storageServiceUploadJob } from '../../services/storage'; import { CallLinkDeleteManager } from '../../jobs/CallLinkDeleteManager'; +import { callLinkRefreshJobQueue } from '../../jobs/callLinkRefreshJobQueue'; // State @@ -1508,33 +1509,12 @@ function handleCallLinkUpdate( const roomId = getRoomIdFromRootKey(callLinkRootKey); const logId = `handleCallLinkUpdate(${roomId})`; - const freshCallLinkState = await calling.readCallLink(callLinkRootKey); const existingCallLink = await DataReader.getCallLinkByRoomId(roomId); - // Only give up when server confirms the call link is gone. If we fail to fetch - // state due to unexpected errors, continue to save rootKey and adminKey. - if (freshCallLinkState == null) { - log.info(`${logId}: Call link not found on server`); - if (!existingCallLink) { - return; - } - - // If the call link is gone remotely (for example if it expired on the server), - // then delete local call link. - log.info(`${logId}: Deleting existing call link`); - await DataWriter.beginDeleteCallLink(roomId, { - storageNeedsSync: true, - }); - storageServiceUploadJob(); - handleCallLinkDelete({ roomId }); - return; - } - const callLink: CallLinkType = { ...CALL_LINK_DEFAULT_STATE, storageNeedsSync: false, ...existingCallLink, - ...freshCallLinkState, roomId, rootKey, adminKey, @@ -1544,21 +1524,16 @@ function handleCallLinkUpdate( if (existingCallLink) { if (adminKey && adminKey !== existingCallLink.adminKey) { + log.info(`${logId}: Updating existing call link with new adminKey`); await DataWriter.updateCallLinkAdminKeyByRoomId(roomId, adminKey); - log.info(`${logId}: Updated existing call link with new adminKey`); - } - - if (freshCallLinkState) { - await DataWriter.updateCallLinkState(roomId, freshCallLinkState); - log.info(`${logId}: Updated existing call link state`); } } else { + log.info(`${logId}: Saving new call link`); await DataWriter.insertCallLink(callLink); if (adminKey != null) { callHistory = toCallHistoryFromUnusedCallLink(callLink); await DataWriter.saveCallHistory(callHistory); } - log.info(`${logId}: Saved new call link`); } dispatch({ @@ -1569,6 +1544,16 @@ function handleCallLinkUpdate( if (callHistory != null) { dispatch(addCallHistory(callHistory)); } + + // Schedule async refresh. It's possible to get a big batch of sync messages. + // This job will throttle requests to the calling server. + drop( + callLinkRefreshJobQueue.add({ + roomId: callLink.roomId, + deleteLocallyIfMissingOnCallingServer: false, + source: 'handleCallLinkUpdate', + }) + ); }; } diff --git a/ts/test-electron/state/ducks/calling_test.ts b/ts/test-electron/state/ducks/calling_test.ts index 91836abc6a..f121ba6eab 100644 --- a/ts/test-electron/state/ducks/calling_test.ts +++ b/ts/test-electron/state/ducks/calling_test.ts @@ -45,6 +45,8 @@ import { getCallLinkState, } from '../../../test-both/helpers/fakeCallLink'; import { strictAssert } from '../../../util/assert'; +import { callLinkRefreshJobQueue } from '../../../jobs/callLinkRefreshJobQueue'; +import { CALL_LINK_DEFAULT_STATE } from '../../../util/callLinks'; const ACI_1 = generateAci(); const NOW = new Date('2020-01-23T04:56:00.000'); @@ -1427,20 +1429,13 @@ describe('calling duck', () => { }); describe('handleCallLinkUpdate', () => { - const { - roomId, - name, - restrictions, - expiration, - revoked, - rootKey, - adminKey, - } = FAKE_CALL_LINK; + const { roomId, rootKey, adminKey } = FAKE_CALL_LINK; beforeEach(function (this: Mocha.Context) { - this.callingServiceReadCallLink = this.sandbox - .stub(callingService, 'readCallLink') - .resolves(getCallLinkState(FAKE_CALL_LINK)); + this.callLinkRefreshJobQueueAdd = this.sandbox.stub( + callLinkRefreshJobQueue, + 'add' + ); }); const doAction = async ( @@ -1452,10 +1447,10 @@ describe('calling duck', () => { return { dispatch }; }; - it('reads the call link from calling service', async function (this: Mocha.Context) { + it('queues call link refresh', async function (this: Mocha.Context) { await doAction({ rootKey, adminKey: null }); - sinon.assert.calledOnce(this.callingServiceReadCallLink); + sinon.assert.calledOnce(this.callLinkRefreshJobQueueAdd); }); it('dispatches HANDLE_CALL_LINK_UPDATE', async () => { @@ -1466,10 +1461,7 @@ describe('calling duck', () => { type: 'calling/HANDLE_CALL_LINK_UPDATE', payload: { callLink: { - name, - restrictions, - expiration, - revoked, + ...CALL_LINK_DEFAULT_STATE, roomId, rootKey, adminKey, @@ -1490,10 +1482,7 @@ describe('calling duck', () => { type: 'calling/HANDLE_CALL_LINK_UPDATE', payload: { callLink: { - name, - restrictions, - expiration, - revoked, + ...CALL_LINK_DEFAULT_STATE, roomId, rootKey, adminKey: 'banana', diff --git a/ts/util/callLinks.ts b/ts/util/callLinks.ts index c878026c2a..4627406918 100644 --- a/ts/util/callLinks.ts +++ b/ts/util/callLinks.ts @@ -17,7 +17,10 @@ import { } from '../types/CallDisposition'; import { DAY } from './durations'; -export const CALL_LINK_DEFAULT_STATE: Partial = { +export const CALL_LINK_DEFAULT_STATE: Pick< + CallLinkType, + 'name' | 'restrictions' | 'revoked' | 'expiration' | 'storageNeedsSync' +> = { name: '', restrictions: CallLinkRestrictions.Unknown, revoked: false,