From 6063209fff19fe39d2669830ec9397e28b78c40c Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 7 May 2025 12:24:43 +0100 Subject: [PATCH] Cache the key backup status whether enabled or not (#29886) --- src/DeviceListener.ts | 38 +++++++++++++++++++------- test/unit-tests/DeviceListener-test.ts | 4 ++- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 751e71dd9f..91ee0f284e 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -132,7 +132,7 @@ export default class DeviceListener { this.dismissedThisDeviceToast = false; this.keyBackupInfo = null; this.keyBackupFetchedAt = null; - this.keyBackupStatusChecked = false; + this.cachedKeyBackupStatus = undefined; this.ourDeviceIdsAtStart = null; this.displayingToastsForDeviceIds = new Set(); this.client = undefined; @@ -512,18 +512,36 @@ export default class DeviceListener { * trigger an auto-rageshake). */ private checkKeyBackupStatus = async (): Promise => { - if (this.keyBackupStatusChecked || !this.client) { - return; - } - const activeKeyBackupVersion = await this.client.getCrypto()?.getActiveSessionBackupVersion(); - // if key backup is enabled, no need to check this ever again (XXX: why only when it is enabled?) - this.keyBackupStatusChecked = !!activeKeyBackupVersion; - - if (!activeKeyBackupVersion) { + if (!(await this.getKeyBackupStatus())) { dis.dispatch({ action: Action.ReportKeyBackupNotEnabled }); } }; - private keyBackupStatusChecked = false; + + /** + * Is key backup enabled? Use a cached answer if we have one. + */ + private getKeyBackupStatus = async (): Promise => { + if (!this.client) { + // To preserve existing behaviour, if there is no client, we + // pretend key storage is on. + // + // Someone looking to improve this code could try throwing an error + // here since we don't expect client to be undefined. + return true; + } + + // If we've already cached the answer, return it. + if (this.cachedKeyBackupStatus !== undefined) { + return this.cachedKeyBackupStatus; + } + + // Fetch the answer and cache it + const activeKeyBackupVersion = await this.client.getCrypto()?.getActiveSessionBackupVersion(); + this.cachedKeyBackupStatus = !!activeKeyBackupVersion; + + return this.cachedKeyBackupStatus; + }; + private cachedKeyBackupStatus: boolean | undefined = undefined; private onRecordClientInformationSettingChange: CallbackFn = ( _originalSettingName, diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index f58eccf586..09bc4cd18a 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -445,7 +445,9 @@ describe("DeviceListener", () => { it("dispatches keybackup event when key backup is not enabled", async () => { mockCrypto.getActiveSessionBackupVersion.mockResolvedValue(null); await createAndStart(); - expect(mockDispatcher.dispatch).toHaveBeenCalledWith({ action: Action.ReportKeyBackupNotEnabled }); + expect(mockDispatcher.dispatch).toHaveBeenCalledWith({ + action: Action.ReportKeyBackupNotEnabled, + }); }); it("does not check key backup status again after check is complete", async () => {