From ab2799e98219e758b1766ef95dd9a9ac4b33fec3 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Wed, 20 Oct 2021 13:31:14 -0700 Subject: [PATCH] Cache system-tray-setting in ephemeral config Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> --- app/SystemTraySettingCache.ts | 31 ++++++--- app/main.ts | 17 +++-- ts/main/settingsChannel.ts | 17 ++++- .../app/SystemTraySettingCache_test.ts | 69 +++++++++++++++++-- 4 files changed, 108 insertions(+), 26 deletions(-) diff --git a/app/SystemTraySettingCache.ts b/app/SystemTraySettingCache.ts index 7fb2314c0c9..6b49d22a644 100644 --- a/app/SystemTraySettingCache.ts +++ b/app/SystemTraySettingCache.ts @@ -8,6 +8,7 @@ import { } from '../ts/types/SystemTraySetting'; import { isSystemTraySupported } from '../ts/types/Settings'; import type { MainSQL } from '../ts/sql/main'; +import type { ConfigType } from './base_config'; /** * A small helper class to get and cache the `system-tray-setting` preference in the main @@ -20,6 +21,7 @@ export class SystemTraySettingCache { constructor( private readonly sql: Pick, + private readonly ephemeralConfig: Pick, private readonly argv: Array, private readonly appVersion: string ) {} @@ -53,20 +55,25 @@ export class SystemTraySettingCache { `getSystemTraySetting saw --use-tray-icon flag. Returning ${result}` ); } else if (isSystemTraySupported(this.appVersion)) { - const { value } = (await this.sql.sqlCall('getItemById', [ - 'system-tray-setting', - ])) || { value: undefined }; + const fastValue = this.ephemeralConfig.get('system-tray-setting'); + if (fastValue !== undefined) { + log.info('getSystemTraySetting got fast value', fastValue); + } + + const value = + fastValue ?? + (await this.sql.sqlCall('getItemById', ['system-tray-setting']))?.value; if (value !== undefined) { result = parseSystemTraySetting(value); - log.info( - `getSystemTraySetting returning value from database, ${result}` - ); + log.info(`getSystemTraySetting returning ${result}`); } else { result = SystemTraySetting.DoNotUseSystemTray; - log.info( - `getSystemTraySetting got no value from database, returning ${result}` - ); + log.info(`getSystemTraySetting got no value, returning ${result}`); + } + + if (result !== fastValue) { + this.ephemeralConfig.set('system-tray-setting', result); } } else { result = SystemTraySetting.DoNotUseSystemTray; @@ -75,10 +82,14 @@ export class SystemTraySettingCache { ); } + return this.updateCachedValue(result); + } + + private updateCachedValue(value: SystemTraySetting): SystemTraySetting { // If there's a value in the cache, someone has updated the value "out from under us", // so we should return that because it's newer. this.cachedValue = - this.cachedValue === undefined ? result : this.cachedValue; + this.cachedValue === undefined ? value : this.cachedValue; return this.cachedValue; } diff --git a/app/main.ts b/app/main.ts index a15c8c524f3..fbf36cd10f8 100644 --- a/app/main.ts +++ b/app/main.ts @@ -118,6 +118,7 @@ const heicConverter = getHeicConverter(); let systemTrayService: SystemTrayService | undefined; const systemTraySettingCache = new SystemTraySettingCache( sql, + ephemeralConfig, process.argv, app.getVersion() ); @@ -1230,11 +1231,9 @@ function showPermissionsPopupWindow(forCalling: boolean, forCamera: boolean) { }); } -async function initializeSQL(): Promise< - { ok: true; error: undefined } | { ok: false; error: Error } -> { - const userDataPath = await getRealPath(app.getPath('userData')); - +async function initializeSQL( + userDataPath: string +): Promise<{ ok: true; error: undefined } | { ok: false; error: Error }> { let key: string | undefined; const keyFromConfig = userConfig.get('key'); if (typeof keyFromConfig === 'string') { @@ -1255,6 +1254,9 @@ async function initializeSQL(): Promise< sqlInitTimeStart = Date.now(); try { + // This should be the first awaited call in this function, otherwise + // `sql.sqlCall` will throw an uninitialized error instead of waiting for + // init to finish. await sql.initialize({ configDir: userDataPath, key, @@ -1339,9 +1341,11 @@ ipc.on('database-error', (_event: Electron.Event, error: string) => { // Some APIs can only be used after this event occurs. let ready = false; app.on('ready', async () => { + const userDataPath = await getRealPath(app.getPath('userData')); + logger = await logging.initialize(getMainWindow); - sqlInitPromise = initializeSQL(); + sqlInitPromise = initializeSQL(userDataPath); const startTime = Date.now(); @@ -1377,7 +1381,6 @@ app.on('ready', async () => { }); }); - const userDataPath = await getRealPath(app.getPath('userData')); const installPath = await getRealPath(app.getAppPath()); addSensitivePath(userDataPath); diff --git a/ts/main/settingsChannel.ts b/ts/main/settingsChannel.ts index e576791b07c..7872e8f97c5 100644 --- a/ts/main/settingsChannel.ts +++ b/ts/main/settingsChannel.ts @@ -12,6 +12,11 @@ import { IPCEventsCallbacksType, } from '../util/createIPCEvents'; +const EPHEMERAL_NAME_MAP = new Map([ + ['spellCheck', 'spell-check'], + ['systemTraySetting', 'system-tray-setting'], +]); + export class SettingsChannel { private mainWindow?: BrowserWindow; @@ -50,7 +55,9 @@ export class SettingsChannel { this.installSetting('themeSetting'); this.installSetting('hideMenuBar'); - this.installSetting('systemTraySetting'); + this.installSetting('systemTraySetting', { + isEphemeral: true, + }); this.installSetting('notificationSetting'); this.installSetting('notificationDrawAttention'); @@ -199,8 +206,12 @@ export class SettingsChannel { ipc.on(`settings:set:${name}`, (event, value) => { if (isEphemeral) { - strictAssert(name === 'spellCheck', 'Only spellCheck is ephemeral'); - ephemeralConfig.set('spell-check', value); + const ephemeralName = EPHEMERAL_NAME_MAP.get(name); + strictAssert( + ephemeralName !== undefined, + `${name} is not an ephemeral setting` + ); + ephemeralConfig.set(ephemeralName, value); } const { mainWindow } = this; diff --git a/ts/test-node/app/SystemTraySettingCache_test.ts b/ts/test-node/app/SystemTraySettingCache_test.ts index 8d16c6d2458..2c9f9abbb82 100644 --- a/ts/test-node/app/SystemTraySettingCache_test.ts +++ b/ts/test-node/app/SystemTraySettingCache_test.ts @@ -6,19 +6,27 @@ import * as sinon from 'sinon'; import { MainSQL } from '../../sql/main'; import { SystemTraySetting } from '../../types/SystemTraySetting'; +import type { ConfigType } from '../../../app/base_config'; import { SystemTraySettingCache } from '../../../app/SystemTraySettingCache'; describe('SystemTraySettingCache', () => { let sandbox: sinon.SinonSandbox; let sqlCallStub: sinon.SinonStub; + let configGetStub: sinon.SinonStub; + let configSetStub: sinon.SinonStub; let sql: Pick; + let config: Pick; beforeEach(() => { sandbox = sinon.createSandbox(); sqlCallStub = sandbox.stub().resolves(); sql = { sqlCall: sqlCallStub }; + + configGetStub = sandbox.stub().returns(undefined); + configSetStub = sandbox.stub().returns(undefined); + config = { get: configGetStub, set: configSetStub }; }); afterEach(() => { @@ -28,6 +36,7 @@ describe('SystemTraySettingCache', () => { it('returns MinimizeToAndStartInSystemTray if passed the --start-in-tray argument', async () => { const justOneArg = new SystemTraySettingCache( sql, + config, ['--start-in-tray'], '1.2.3' ); @@ -38,6 +47,7 @@ describe('SystemTraySettingCache', () => { const bothArgs = new SystemTraySettingCache( sql, + config, ['--start-in-tray', '--use-tray-icon'], '1.2.3' ); @@ -47,23 +57,39 @@ describe('SystemTraySettingCache', () => { ); sinon.assert.notCalled(sqlCallStub); + sinon.assert.notCalled(configGetStub); + sinon.assert.notCalled(configSetStub); }); it('returns MinimizeToSystemTray if passed the --use-tray-icon argument', async () => { - const cache = new SystemTraySettingCache(sql, ['--use-tray-icon'], '1.2.3'); + const cache = new SystemTraySettingCache( + sql, + config, + ['--use-tray-icon'], + '1.2.3' + ); assert.strictEqual( await cache.get(), SystemTraySetting.MinimizeToSystemTray ); sinon.assert.notCalled(sqlCallStub); + sinon.assert.notCalled(configGetStub); + sinon.assert.notCalled(configSetStub); }); it('returns DoNotUseSystemTray if system tray is supported but no preference is stored', async () => { sandbox.stub(process, 'platform').value('win32'); - const cache = new SystemTraySettingCache(sql, [], '1.2.3'); + const cache = new SystemTraySettingCache(sql, config, [], '1.2.3'); assert.strictEqual(await cache.get(), SystemTraySetting.DoNotUseSystemTray); + assert(configGetStub.calledOnceWith('system-tray-setting')); + assert( + configSetStub.calledOnceWith( + 'system-tray-setting', + SystemTraySetting.DoNotUseSystemTray + ) + ); }); it('returns DoNotUseSystemTray if system tray is supported but the stored preference is invalid', async () => { @@ -71,8 +97,15 @@ describe('SystemTraySettingCache', () => { sqlCallStub.resolves({ value: 'garbage' }); - const cache = new SystemTraySettingCache(sql, [], '1.2.3'); + const cache = new SystemTraySettingCache(sql, config, [], '1.2.3'); assert.strictEqual(await cache.get(), SystemTraySetting.DoNotUseSystemTray); + assert(configGetStub.calledOnceWith('system-tray-setting')); + assert( + configSetStub.calledOnceWith( + 'system-tray-setting', + SystemTraySetting.DoNotUseSystemTray + ) + ); }); it('returns the stored preference if system tray is supported and something is stored', async () => { @@ -80,29 +113,53 @@ describe('SystemTraySettingCache', () => { sqlCallStub.resolves({ value: 'MinimizeToSystemTray' }); - const cache = new SystemTraySettingCache(sql, [], '1.2.3'); + const cache = new SystemTraySettingCache(sql, config, [], '1.2.3'); assert.strictEqual( await cache.get(), SystemTraySetting.MinimizeToSystemTray ); + assert(configGetStub.calledOnceWith('system-tray-setting')); + assert( + configSetStub.calledOnceWith( + 'system-tray-setting', + SystemTraySetting.MinimizeToSystemTray + ) + ); + }); + + it('returns the cached preference if system tray is supported and something is stored', async () => { + sandbox.stub(process, 'platform').value('win32'); + + configGetStub.returns('MinimizeToSystemTray'); + + const cache = new SystemTraySettingCache(sql, config, [], '1.2.3'); + assert.strictEqual( + await cache.get(), + SystemTraySetting.MinimizeToSystemTray + ); + assert(configGetStub.calledOnceWith('system-tray-setting')); + sinon.assert.notCalled(sqlCallStub); }); it('only kicks off one request to the database if multiple sources ask at once', async () => { sandbox.stub(process, 'platform').value('win32'); - const cache = new SystemTraySettingCache(sql, [], '1.2.3'); + const cache = new SystemTraySettingCache(sql, config, [], '1.2.3'); await Promise.all([cache.get(), cache.get(), cache.get()]); + assert(configGetStub.calledOnceWith('system-tray-setting')); sinon.assert.calledOnce(sqlCallStub); }); it('returns DoNotUseSystemTray if system tray is unsupported and there are no CLI flags', async () => { sandbox.stub(process, 'platform').value('darwin'); - const cache = new SystemTraySettingCache(sql, [], '1.2.3'); + const cache = new SystemTraySettingCache(sql, config, [], '1.2.3'); assert.strictEqual(await cache.get(), SystemTraySetting.DoNotUseSystemTray); + sinon.assert.notCalled(configGetStub); + sinon.assert.notCalled(configSetStub); sinon.assert.notCalled(sqlCallStub); }); });