diff options
author | Kristóf Marussy <kristof@marussy.com> | 2022-04-26 16:55:57 +0200 |
---|---|---|
committer | Kristóf Marussy <kristof@marussy.com> | 2022-05-16 00:55:02 +0200 |
commit | b971de0717a66ae6085670fe5d3a469e236a9446 (patch) | |
tree | c64352908648512bad713fc1e91dd5f07a66968c /packages | |
parent | refactor: remove json5 dependency (diff) | |
download | sophie-b971de0717a66ae6085670fe5d3a469e236a9446.tar.gz sophie-b971de0717a66ae6085670fe5d3a469e236a9446.tar.zst sophie-b971de0717a66ae6085670fe5d3a469e236a9446.zip |
refactor: config file saving and debugging
Reduce the number of dependencies and the amount of code running in a
security sensitive context.
Instead of a deep comparison, we just compare the serialized versions
of the config files.
Signed-off-by: Kristóf Marussy <kristof@marussy.com>
Diffstat (limited to 'packages')
-rw-r--r-- | packages/main/esbuild.config.js | 6 | ||||
-rw-r--r-- | packages/main/package.json | 6 | ||||
-rw-r--r-- | packages/main/src/index.ts | 15 | ||||
-rw-r--r-- | packages/main/src/infrastructure/config/ConfigRepository.ts | 5 | ||||
-rw-r--r-- | packages/main/src/infrastructure/config/impl/ConfigFile.ts | 12 | ||||
-rw-r--r-- | packages/main/src/infrastructure/electron/impl/devTools.ts | 43 | ||||
-rw-r--r-- | packages/main/src/infrastructure/electron/impl/hardenSession.ts | 14 | ||||
-rw-r--r-- | packages/main/src/reactions/__tests__/synchronizeConfig.spec.ts | 56 | ||||
-rw-r--r-- | packages/main/src/reactions/synchronizeConfig.ts | 40 |
9 files changed, 103 insertions, 94 deletions
diff --git a/packages/main/esbuild.config.js b/packages/main/esbuild.config.js index ae8565d..dddda75 100644 --- a/packages/main/esbuild.config.js +++ b/packages/main/esbuild.config.js | |||
@@ -13,7 +13,11 @@ const thisDir = fileUrlToDirname(import.meta.url); | |||
13 | const externalPackages = ['electron']; | 13 | const externalPackages = ['electron']; |
14 | 14 | ||
15 | if (process.env.MODE !== 'development') { | 15 | if (process.env.MODE !== 'development') { |
16 | externalPackages.push('electron-devtools-installer', 'source-map-support'); | 16 | externalPackages.push( |
17 | 'electron-devtools-installer', | ||
18 | 'mkdirp', | ||
19 | 'source-map-support', | ||
20 | ); | ||
17 | } | 21 | } |
18 | 22 | ||
19 | const gitInfo = getRepoInfo(); | 23 | const gitInfo = getRepoInfo(); |
diff --git a/packages/main/package.json b/packages/main/package.json index fceb392..fc972ba 100644 --- a/packages/main/package.json +++ b/packages/main/package.json | |||
@@ -11,9 +11,7 @@ | |||
11 | "@sophie/service-shared": "workspace:*", | 11 | "@sophie/service-shared": "workspace:*", |
12 | "@sophie/shared": "workspace:*", | 12 | "@sophie/shared": "workspace:*", |
13 | "chalk": "^5.0.1", | 13 | "chalk": "^5.0.1", |
14 | "deep-equal": "^2.0.5", | ||
15 | "electron": "^19.0.0-alpha.1", | 14 | "electron": "^19.0.0-alpha.1", |
16 | "fs-extra": "^10.1.0", | ||
17 | "i18next": "^21.6.16", | 15 | "i18next": "^21.6.16", |
18 | "lodash-es": "^4.17.21", | 16 | "lodash-es": "^4.17.21", |
19 | "loglevel": "^1.8.0", | 17 | "loglevel": "^1.8.0", |
@@ -21,15 +19,14 @@ | |||
21 | "mobx": "^6.5.0", | 19 | "mobx": "^6.5.0", |
22 | "mobx-state-tree": "^5.1.3", | 20 | "mobx-state-tree": "^5.1.3", |
23 | "nanoid": "^3.3.3", | 21 | "nanoid": "^3.3.3", |
24 | "os-name": "^5.0.1", | ||
25 | "slug": "^5.3.0" | 22 | "slug": "^5.3.0" |
26 | }, | 23 | }, |
27 | "devDependencies": { | 24 | "devDependencies": { |
28 | "@jest/globals": "^27.5.1", | 25 | "@jest/globals": "^27.5.1", |
29 | "@sophie/test-utils": "workspace:*", | 26 | "@sophie/test-utils": "workspace:*", |
30 | "@types/deep-equal": "^1.0.1", | ||
31 | "@types/electron-devtools-installer": "^2.2.2", | 27 | "@types/electron-devtools-installer": "^2.2.2", |
32 | "@types/lodash-es": "^4.17.6", | 28 | "@types/lodash-es": "^4.17.6", |
29 | "@types/mkdirp": "^1", | ||
33 | "@types/node": "^17.0.25", | 30 | "@types/node": "^17.0.25", |
34 | "@types/slug": "^5.0.3", | 31 | "@types/slug": "^5.0.3", |
35 | "@types/source-map-support": "^0.5.4", | 32 | "@types/source-map-support": "^0.5.4", |
@@ -38,6 +35,7 @@ | |||
38 | "git-repo-info": "^2.1.1", | 35 | "git-repo-info": "^2.1.1", |
39 | "jest": "^27.5.1", | 36 | "jest": "^27.5.1", |
40 | "jest-mock": "^27.5.1", | 37 | "jest-mock": "^27.5.1", |
38 | "mkdirp": "^1.0.4", | ||
41 | "source-map-support": "^0.5.21" | 39 | "source-map-support": "^0.5.21" |
42 | } | 40 | } |
43 | } | 41 | } |
diff --git a/packages/main/src/index.ts b/packages/main/src/index.ts index 29a8cca..78861c2 100644 --- a/packages/main/src/index.ts +++ b/packages/main/src/index.ts | |||
@@ -22,10 +22,11 @@ | |||
22 | import os from 'node:os'; | 22 | import os from 'node:os'; |
23 | 23 | ||
24 | import { app } from 'electron'; | 24 | import { app } from 'electron'; |
25 | import { ensureDirSync } from 'fs-extra'; | ||
26 | import osName from 'os-name'; | ||
27 | 25 | ||
28 | import { enableStacktraceSourceMaps } from './infrastructure/electron/impl/devTools'; | 26 | import { |
27 | enableStacktraceSourceMaps, | ||
28 | ensureDevDataDir, | ||
29 | } from './infrastructure/electron/impl/devTools'; | ||
29 | import electronShell from './infrastructure/electron/impl/electronShell'; | 30 | import electronShell from './infrastructure/electron/impl/electronShell'; |
30 | import initReactions from './initReactions'; | 31 | import initReactions from './initReactions'; |
31 | import MainStore from './stores/MainStore'; | 32 | import MainStore from './stores/MainStore'; |
@@ -39,11 +40,7 @@ const log = getLogger('index'); | |||
39 | app.enableSandbox(); | 40 | app.enableSandbox(); |
40 | 41 | ||
41 | if (isDevelopment) { | 42 | if (isDevelopment) { |
42 | // Use alternative directory when debugging to avoid clobbering the main installation. | 43 | ensureDevDataDir(); |
43 | app.setPath('userData', `${app.getPath('userData')}-dev`); | ||
44 | ensureDirSync(app.getPath('userData')); | ||
45 | |||
46 | // Use source maps in stack traces. | ||
47 | enableStacktraceSourceMaps(); | 44 | enableStacktraceSourceMaps(); |
48 | } | 45 | } |
49 | 46 | ||
@@ -67,7 +64,7 @@ app.setAboutPanelOptions({ | |||
67 | `Electron: ${process.versions.electron}`, | 64 | `Electron: ${process.versions.electron}`, |
68 | `Chrome: ${process.versions.chrome}`, | 65 | `Chrome: ${process.versions.chrome}`, |
69 | `Node.js: ${process.versions.node}`, | 66 | `Node.js: ${process.versions.node}`, |
70 | `Platform: ${osName()}`, | 67 | `Platform: ${os.platform()} ${os.release()}`, |
71 | `Arch: ${os.arch()}`, | 68 | `Arch: ${os.arch()}`, |
72 | `Build date: ${new Date( | 69 | `Build date: ${new Date( |
73 | Number(import.meta.env.BUILD_DATE), | 70 | Number(import.meta.env.BUILD_DATE), |
diff --git a/packages/main/src/infrastructure/config/ConfigRepository.ts b/packages/main/src/infrastructure/config/ConfigRepository.ts index e00f5a0..67bffb0 100644 --- a/packages/main/src/infrastructure/config/ConfigRepository.ts +++ b/packages/main/src/infrastructure/config/ConfigRepository.ts | |||
@@ -18,17 +18,16 @@ | |||
18 | * SPDX-License-Identifier: AGPL-3.0-only | 18 | * SPDX-License-Identifier: AGPL-3.0-only |
19 | */ | 19 | */ |
20 | 20 | ||
21 | import type Config from '../../stores/config/Config'; | ||
22 | import type Disposer from '../../utils/Disposer'; | 21 | import type Disposer from '../../utils/Disposer'; |
23 | 22 | ||
24 | export type ReadConfigResult = | 23 | export type ReadConfigResult = |
25 | | { found: true; data: unknown } | 24 | | { found: true; contents: string } |
26 | | { found: false }; | 25 | | { found: false }; |
27 | 26 | ||
28 | export default interface ConfigPersistence { | 27 | export default interface ConfigPersistence { |
29 | readConfig(): Promise<ReadConfigResult>; | 28 | readConfig(): Promise<ReadConfigResult>; |
30 | 29 | ||
31 | writeConfig(config: Config): Promise<void>; | 30 | writeConfig(contents: string): Promise<void>; |
32 | 31 | ||
33 | watchConfig(callback: () => Promise<void>, throttleMs: number): Disposer; | 32 | watchConfig(callback: () => Promise<void>, throttleMs: number): Disposer; |
34 | } | 33 | } |
diff --git a/packages/main/src/infrastructure/config/impl/ConfigFile.ts b/packages/main/src/infrastructure/config/impl/ConfigFile.ts index c4e6d22..8b110a2 100644 --- a/packages/main/src/infrastructure/config/impl/ConfigFile.ts +++ b/packages/main/src/infrastructure/config/impl/ConfigFile.ts | |||
@@ -24,7 +24,6 @@ import path from 'node:path'; | |||
24 | 24 | ||
25 | import { throttle } from 'lodash-es'; | 25 | import { throttle } from 'lodash-es'; |
26 | 26 | ||
27 | import type Config from '../../../stores/config/Config'; | ||
28 | import type Disposer from '../../../utils/Disposer'; | 27 | import type Disposer from '../../../utils/Disposer'; |
29 | import isErrno from '../../../utils/isErrno'; | 28 | import isErrno from '../../../utils/isErrno'; |
30 | import { getLogger } from '../../../utils/log'; | 29 | import { getLogger } from '../../../utils/log'; |
@@ -48,9 +47,9 @@ export default class ConfigFile implements ConfigRepository { | |||
48 | } | 47 | } |
49 | 48 | ||
50 | async readConfig(): Promise<ReadConfigResult> { | 49 | async readConfig(): Promise<ReadConfigResult> { |
51 | let configStr: string; | 50 | let contents: string; |
52 | try { | 51 | try { |
53 | configStr = await readFile(this.configFilePath, 'utf8'); | 52 | contents = await readFile(this.configFilePath, 'utf8'); |
54 | } catch (error) { | 53 | } catch (error) { |
55 | if (isErrno(error, 'ENOENT')) { | 54 | if (isErrno(error, 'ENOENT')) { |
56 | log.debug('Config file', this.configFilePath, 'was not found'); | 55 | log.debug('Config file', this.configFilePath, 'was not found'); |
@@ -61,15 +60,14 @@ export default class ConfigFile implements ConfigRepository { | |||
61 | log.info('Read config file', this.configFilePath); | 60 | log.info('Read config file', this.configFilePath); |
62 | return { | 61 | return { |
63 | found: true, | 62 | found: true, |
64 | data: JSON.parse(configStr), | 63 | contents, |
65 | }; | 64 | }; |
66 | } | 65 | } |
67 | 66 | ||
68 | async writeConfig(configSnapshot: Config): Promise<void> { | 67 | async writeConfig(contents: string): Promise<void> { |
69 | const configJson = JSON.stringify(configSnapshot, undefined, 2); | ||
70 | this.writingConfig = true; | 68 | this.writingConfig = true; |
71 | try { | 69 | try { |
72 | await writeFile(this.configFilePath, configJson, 'utf8'); | 70 | await writeFile(this.configFilePath, contents, 'utf8'); |
73 | const { mtime } = await stat(this.configFilePath); | 71 | const { mtime } = await stat(this.configFilePath); |
74 | log.trace('Config file', this.configFilePath, 'last written at', mtime); | 72 | log.trace('Config file', this.configFilePath, 'last written at', mtime); |
75 | this.timeLastWritten = mtime; | 73 | this.timeLastWritten = mtime; |
diff --git a/packages/main/src/infrastructure/electron/impl/devTools.ts b/packages/main/src/infrastructure/electron/impl/devTools.ts index 10f4545..6db88d1 100644 --- a/packages/main/src/infrastructure/electron/impl/devTools.ts +++ b/packages/main/src/infrastructure/electron/impl/devTools.ts | |||
@@ -18,34 +18,32 @@ | |||
18 | * SPDX-License-Identifier: AGPL-3.0-only | 18 | * SPDX-License-Identifier: AGPL-3.0-only |
19 | */ | 19 | */ |
20 | 20 | ||
21 | import type { BrowserWindow } from 'electron'; | 21 | import { app, type BrowserWindow } from 'electron'; |
22 | |||
23 | /* eslint-disable | ||
24 | import/no-extraneous-dependencies, | ||
25 | global-require, | ||
26 | @typescript-eslint/no-var-requires, | ||
27 | unicorn/prefer-module -- | ||
28 | Hack to lazily require a CJS module from an ES module transpiled into a CJS module. | ||
29 | */ | ||
22 | 30 | ||
23 | /** | 31 | /** |
24 | * URL prefixes Sophie is allowed load in dev mode. | 32 | * Makes sure we use a separate data dir for development. |
25 | * | ||
26 | * In dev mode, in addition to the application itself, | ||
27 | * Sophie must be able do download and load the devtools and related extensions, | ||
28 | * so we have to make exceptions in the UI process request filter. | ||
29 | */ | 33 | */ |
30 | export const DEVMODE_ALLOWED_URL_PREFIXES = [ | 34 | export function ensureDevDataDir(): void { |
31 | 'chrome-extension:', | 35 | // Use alternative directory when debugging to avoid clobbering the main installation. |
32 | 'devtools:', | 36 | app.setPath('userData', `${app.getPath('userData')}-dev`); |
33 | 'https://clients2.google.com/service/update2/crx', | 37 | const userData = app.getPath('userData'); |
34 | 'https://clients2.googleusercontent.com/crx', | 38 | const mkdirp = require('mkdirp') as typeof import('mkdirp'); |
35 | ]; | 39 | mkdirp.sync(userData); |
40 | } | ||
36 | 41 | ||
37 | /** | 42 | /** |
38 | * Enables using source maps for node stack traces. | 43 | * Enables using source maps for node stack traces. |
39 | */ | 44 | */ |
40 | export function enableStacktraceSourceMaps(): void { | 45 | export function enableStacktraceSourceMaps(): void { |
41 | const sourceMapSupport = | 46 | const sourceMapSupport = |
42 | /* eslint-disable-next-line | ||
43 | import/no-extraneous-dependencies, | ||
44 | global-require, | ||
45 | @typescript-eslint/no-var-requires, | ||
46 | unicorn/prefer-module -- | ||
47 | Hack to lazily require a CJS module from an ES module transpiled into a CJS module. | ||
48 | */ | ||
49 | require('source-map-support') as typeof import('source-map-support'); | 47 | require('source-map-support') as typeof import('source-map-support'); |
50 | sourceMapSupport.install(); | 48 | sourceMapSupport.install(); |
51 | } | 49 | } |
@@ -61,13 +59,6 @@ export async function installDevToolsExtensions(): Promise<void> { | |||
61 | default: installExtension, | 59 | default: installExtension, |
62 | REACT_DEVELOPER_TOOLS, | 60 | REACT_DEVELOPER_TOOLS, |
63 | REDUX_DEVTOOLS, | 61 | REDUX_DEVTOOLS, |
64 | /* eslint-disable-next-line | ||
65 | import/no-extraneous-dependencies, | ||
66 | global-require, | ||
67 | @typescript-eslint/no-var-requires, | ||
68 | unicorn/prefer-module -- | ||
69 | Hack to lazily require a CJS module from an ES module transpiled into a CJS module. | ||
70 | */ | ||
71 | } = require('electron-devtools-installer') as typeof import('electron-devtools-installer'); | 62 | } = require('electron-devtools-installer') as typeof import('electron-devtools-installer'); |
72 | await installExtension([REACT_DEVELOPER_TOOLS, REDUX_DEVTOOLS], { | 63 | await installExtension([REACT_DEVELOPER_TOOLS, REDUX_DEVTOOLS], { |
73 | forceDownload: false, | 64 | forceDownload: false, |
diff --git a/packages/main/src/infrastructure/electron/impl/hardenSession.ts b/packages/main/src/infrastructure/electron/impl/hardenSession.ts index 10b694a..53675a7 100644 --- a/packages/main/src/infrastructure/electron/impl/hardenSession.ts +++ b/packages/main/src/infrastructure/electron/impl/hardenSession.ts | |||
@@ -25,7 +25,19 @@ import type { Session } from 'electron'; | |||
25 | import { getLogger } from '../../../utils/log'; | 25 | import { getLogger } from '../../../utils/log'; |
26 | import type Resources from '../../resources/Resources'; | 26 | import type Resources from '../../resources/Resources'; |
27 | 27 | ||
28 | import { DEVMODE_ALLOWED_URL_PREFIXES } from './devTools'; | 28 | /** |
29 | * URL prefixes Sophie is allowed load in dev mode. | ||
30 | * | ||
31 | * In dev mode, in addition to the application itself, | ||
32 | * Sophie must be able do download and load the devtools and related extensions, | ||
33 | * so we have to make exceptions in the UI process request filter. | ||
34 | */ | ||
35 | export const DEVMODE_ALLOWED_URL_PREFIXES = [ | ||
36 | 'chrome-extension:', | ||
37 | 'devtools:', | ||
38 | 'https://clients2.google.com/service/update2/crx', | ||
39 | 'https://clients2.googleusercontent.com/crx', | ||
40 | ]; | ||
29 | 41 | ||
30 | const log = getLogger('hardenSession'); | 42 | const log = getLogger('hardenSession'); |
31 | 43 | ||
diff --git a/packages/main/src/reactions/__tests__/synchronizeConfig.spec.ts b/packages/main/src/reactions/__tests__/synchronizeConfig.spec.ts index b5013ea..d3338d0 100644 --- a/packages/main/src/reactions/__tests__/synchronizeConfig.spec.ts +++ b/packages/main/src/reactions/__tests__/synchronizeConfig.spec.ts | |||
@@ -25,7 +25,7 @@ import type ConfigRepository from '../../infrastructure/config/ConfigRepository' | |||
25 | import SharedStore from '../../stores/SharedStore'; | 25 | import SharedStore from '../../stores/SharedStore'; |
26 | import type Disposer from '../../utils/Disposer'; | 26 | import type Disposer from '../../utils/Disposer'; |
27 | import { silenceLogger } from '../../utils/log'; | 27 | import { silenceLogger } from '../../utils/log'; |
28 | import synchronizeConfig from '../synchronizeConfig'; | 28 | import synchronizeConfig, { serializeConfig } from '../synchronizeConfig'; |
29 | 29 | ||
30 | let store: SharedStore; | 30 | let store: SharedStore; |
31 | const repository: ConfigRepository = { | 31 | const repository: ConfigRepository = { |
@@ -70,11 +70,11 @@ describe('when synchronizeializing', () => { | |||
70 | beforeEach(() => { | 70 | beforeEach(() => { |
71 | mocked(repository.readConfig).mockResolvedValueOnce({ | 71 | mocked(repository.readConfig).mockResolvedValueOnce({ |
72 | found: true, | 72 | found: true, |
73 | data: { | 73 | contents: serializeConfig({ |
74 | // Use a default empty config file to not trigger config rewrite. | 74 | // Use a default empty config file to not trigger config rewrite. |
75 | ...store.config, | 75 | ...store.config, |
76 | themeSource: 'dark', | 76 | themeSource: 'dark', |
77 | }, | 77 | }), |
78 | }); | 78 | }); |
79 | }); | 79 | }); |
80 | 80 | ||
@@ -97,12 +97,15 @@ describe('when synchronizeializing', () => { | |||
97 | it('should update the config file if new details are added during read', async () => { | 97 | it('should update the config file if new details are added during read', async () => { |
98 | mocked(repository.readConfig).mockResolvedValueOnce({ | 98 | mocked(repository.readConfig).mockResolvedValueOnce({ |
99 | found: true, | 99 | found: true, |
100 | data: { | 100 | contents: `{ |
101 | themeSource: 'light', | 101 | "themeSource": "light", |
102 | profile: { | 102 | "profiles": [ |
103 | name: 'Test profile', | 103 | { |
104 | }, | 104 | "name": "Test profile" |
105 | }, | 105 | } |
106 | ] | ||
107 | } | ||
108 | `, | ||
106 | }); | 109 | }); |
107 | await synchronizeConfig(store, repository); | 110 | await synchronizeConfig(store, repository); |
108 | expect(repository.writeConfig).toHaveBeenCalledTimes(1); | 111 | expect(repository.writeConfig).toHaveBeenCalledTimes(1); |
@@ -111,9 +114,10 @@ describe('when synchronizeializing', () => { | |||
111 | it('should not apply an invalid config file but should not overwrite it', async () => { | 114 | it('should not apply an invalid config file but should not overwrite it', async () => { |
112 | mocked(repository.readConfig).mockResolvedValueOnce({ | 115 | mocked(repository.readConfig).mockResolvedValueOnce({ |
113 | found: true, | 116 | found: true, |
114 | data: { | 117 | contents: `{ |
115 | themeSource: -1, | 118 | "themeSource": -1 |
116 | }, | 119 | } |
120 | `, | ||
117 | }); | 121 | }); |
118 | await synchronizeConfig(store, repository); | 122 | await synchronizeConfig(store, repository); |
119 | expect(store.settings.themeSource).not.toBe(-1); | 123 | expect(store.settings.themeSource).not.toBe(-1); |
@@ -136,7 +140,7 @@ describe('when it has loaded the config', () => { | |||
136 | beforeEach(async () => { | 140 | beforeEach(async () => { |
137 | mocked(repository.readConfig).mockResolvedValueOnce({ | 141 | mocked(repository.readConfig).mockResolvedValueOnce({ |
138 | found: true, | 142 | found: true, |
139 | data: store.config, | 143 | contents: serializeConfig(store.config), |
140 | }); | 144 | }); |
141 | mocked(repository.watchConfig).mockReturnValueOnce(watcherDisposer); | 145 | mocked(repository.watchConfig).mockReturnValueOnce(watcherDisposer); |
142 | sutDisposer = await synchronizeConfig(store, repository, throttleMs); | 146 | sutDisposer = await synchronizeConfig(store, repository, throttleMs); |
@@ -163,11 +167,11 @@ describe('when it has loaded the config', () => { | |||
163 | it('should read the config file when it has changed', async () => { | 167 | it('should read the config file when it has changed', async () => { |
164 | mocked(repository.readConfig).mockResolvedValueOnce({ | 168 | mocked(repository.readConfig).mockResolvedValueOnce({ |
165 | found: true, | 169 | found: true, |
166 | data: { | 170 | contents: serializeConfig({ |
167 | // Use a default empty config file to not trigger config rewrite. | 171 | // Use a default empty config file to not trigger config rewrite. |
168 | ...store.config, | 172 | ...store.config, |
169 | themeSource: 'dark', | 173 | themeSource: 'dark', |
170 | }, | 174 | }), |
171 | }); | 175 | }); |
172 | await configChangedCallback(); | 176 | await configChangedCallback(); |
173 | // Do not write back the changes we have just read. | 177 | // Do not write back the changes we have just read. |
@@ -178,12 +182,15 @@ describe('when it has loaded the config', () => { | |||
178 | it('should update the config file if new details are added', async () => { | 182 | it('should update the config file if new details are added', async () => { |
179 | mocked(repository.readConfig).mockResolvedValueOnce({ | 183 | mocked(repository.readConfig).mockResolvedValueOnce({ |
180 | found: true, | 184 | found: true, |
181 | data: { | 185 | contents: `{ |
182 | themeSource: 'light', | 186 | "themeSource": "light", |
183 | profile: { | 187 | "profiles": [ |
184 | name: 'Test profile', | 188 | { |
185 | }, | 189 | "name": "Test profile" |
186 | }, | 190 | } |
191 | ] | ||
192 | } | ||
193 | `, | ||
187 | }); | 194 | }); |
188 | await configChangedCallback(); | 195 | await configChangedCallback(); |
189 | expect(repository.writeConfig).toHaveBeenCalledTimes(1); | 196 | expect(repository.writeConfig).toHaveBeenCalledTimes(1); |
@@ -192,9 +199,10 @@ describe('when it has loaded the config', () => { | |||
192 | it('should not apply an invalid config file when it has changed but should not overwrite it', async () => { | 199 | it('should not apply an invalid config file when it has changed but should not overwrite it', async () => { |
193 | mocked(repository.readConfig).mockResolvedValueOnce({ | 200 | mocked(repository.readConfig).mockResolvedValueOnce({ |
194 | found: true, | 201 | found: true, |
195 | data: { | 202 | contents: `{ |
196 | themeSource: -1, | 203 | "themeSource": -1 |
197 | }, | 204 | } |
205 | `, | ||
198 | }); | 206 | }); |
199 | await configChangedCallback(); | 207 | await configChangedCallback(); |
200 | expect(store.settings.themeSource).not.toBe(-1); | 208 | expect(store.settings.themeSource).not.toBe(-1); |
diff --git a/packages/main/src/reactions/synchronizeConfig.ts b/packages/main/src/reactions/synchronizeConfig.ts index 7e366e2..247c2e2 100644 --- a/packages/main/src/reactions/synchronizeConfig.ts +++ b/packages/main/src/reactions/synchronizeConfig.ts | |||
@@ -18,7 +18,6 @@ | |||
18 | * SPDX-License-Identifier: AGPL-3.0-only | 18 | * SPDX-License-Identifier: AGPL-3.0-only |
19 | */ | 19 | */ |
20 | 20 | ||
21 | import deepEqual from 'deep-equal'; | ||
22 | import { debounce } from 'lodash-es'; | 21 | import { debounce } from 'lodash-es'; |
23 | import { reaction } from 'mobx'; | 22 | import { reaction } from 'mobx'; |
24 | 23 | ||
@@ -32,36 +31,38 @@ const DEFAULT_CONFIG_DEBOUNCE_TIME_MS = 1000; | |||
32 | 31 | ||
33 | const log = getLogger('synchronizeConfig'); | 32 | const log = getLogger('synchronizeConfig'); |
34 | 33 | ||
34 | export function serializeConfig(config: Config): string { | ||
35 | return `${JSON.stringify(config, undefined, 2)}\n`; | ||
36 | } | ||
37 | |||
35 | export default async function synchronizeConfig( | 38 | export default async function synchronizeConfig( |
36 | sharedStore: SharedStore, | 39 | sharedStore: SharedStore, |
37 | repository: ConfigRepository, | 40 | repository: ConfigRepository, |
38 | debounceTime: number = DEFAULT_CONFIG_DEBOUNCE_TIME_MS, | 41 | debounceTime: number = DEFAULT_CONFIG_DEBOUNCE_TIME_MS, |
39 | ): Promise<Disposer> { | 42 | ): Promise<Disposer> { |
40 | let lastConfigOnDisk: Config | undefined; | 43 | let lastConfigOnDisk: string | undefined; |
41 | 44 | ||
42 | async function writeConfig(): Promise<void> { | 45 | async function writeConfig(serializedConfig: string): Promise<void> { |
43 | const { config } = sharedStore; | 46 | await repository.writeConfig(serializedConfig); |
44 | await repository.writeConfig(config); | 47 | lastConfigOnDisk = serializedConfig; |
45 | lastConfigOnDisk = config; | ||
46 | } | 48 | } |
47 | 49 | ||
48 | async function readConfig(): Promise<boolean> { | 50 | async function readConfig(): Promise<boolean> { |
49 | const result = await repository.readConfig(); | 51 | const result = await repository.readConfig(); |
50 | if (result.found) { | 52 | if (result.found) { |
53 | const { contents } = result; | ||
51 | try { | 54 | try { |
52 | // This cast is unsound if the config file is invalid, | 55 | // This cast is unsound if the config file is invalid, |
53 | // but we'll throw an error in the end anyways. | 56 | // but we'll throw an error in the end anyways. |
54 | sharedStore.loadConfig(result.data as Config); | 57 | const data = JSON.parse(contents) as Config; |
58 | sharedStore.loadConfig(data); | ||
55 | } catch (error) { | 59 | } catch (error) { |
56 | log.error('Failed to apply config snapshot', result.data, error); | 60 | log.error('Failed to apply config snapshot', contents, error); |
57 | return true; | 61 | return true; |
58 | } | 62 | } |
59 | lastConfigOnDisk = sharedStore.config; | 63 | lastConfigOnDisk = serializeConfig(sharedStore.config); |
60 | // We can't use `comparer.structural` from `mobx`, because | 64 | if (contents !== lastConfigOnDisk) { |
61 | // it handles missing values and `undefined` values differently, | 65 | await writeConfig(lastConfigOnDisk); |
62 | // but JSON is unable to distinguish them. | ||
63 | if (!deepEqual(result.data, lastConfigOnDisk, { strict: true })) { | ||
64 | await writeConfig(); | ||
65 | } | 66 | } |
66 | } | 67 | } |
67 | return result.found; | 68 | return result.found; |
@@ -69,16 +70,17 @@ export default async function synchronizeConfig( | |||
69 | 70 | ||
70 | if (!(await readConfig())) { | 71 | if (!(await readConfig())) { |
71 | log.info('Config file was not found'); | 72 | log.info('Config file was not found'); |
72 | await writeConfig(); | 73 | const serializedConfig = serializeConfig(sharedStore.config); |
74 | await writeConfig(serializedConfig); | ||
73 | log.info('Created config file'); | 75 | log.info('Created config file'); |
74 | } | 76 | } |
75 | 77 | ||
76 | const disposeReaction = reaction( | 78 | const disposeReaction = reaction( |
77 | () => sharedStore.config, | 79 | () => sharedStore.config, |
78 | debounce((config) => { | 80 | debounce(() => { |
79 | // We can compare snapshots by reference, since it is only recreated on store changes. | 81 | const serializedConfig = serializeConfig(sharedStore.config); |
80 | if (!deepEqual(config, lastConfigOnDisk, { strict: true })) { | 82 | if (serializedConfig !== lastConfigOnDisk) { |
81 | writeConfig().catch((error) => { | 83 | writeConfig(serializedConfig).catch((error) => { |
82 | log.error('Failed to write config on config change', error); | 84 | log.error('Failed to write config on config change', error); |
83 | }); | 85 | }); |
84 | } | 86 | } |