From 39be8ede53847f4a7b1c8cf61a738e28acd8ea0c Mon Sep 17 00:00:00 2001 From: Will Knowles Date: Sun, 25 Oct 2026 23:51:14 +0000 Subject: [PATCH] Add settings only main changes, settings checks and kit.settings.onChange() kit.start({ settings: { mainOnly, checks } }): the page's change naming a setting only main changes is refused whole, and nothing of it is stored; a setting's check refuses a value from the page or from main. Both name the app's own settings only, checked at start(). kit.settings.onChange(listener) hears every stored change as (changes, after, { from }), after the updater and the log have acted on it, and returns the function that stops it. A listener that throws or rejects is logged; the change stays stored and the others still hear it. The demo's export folder is main's alone, and a whole path when set. --- demo/src/constants.js | 10 +- demo/src/main.js | 12 ++- docs/README.md | 2 +- docs/main-process.md | 36 +++++++ e2e/log-store.spec.js | 43 +++++++- main/index.js | 48 ++++++--- main/store.js | 111 +++++++++++++++++-- test/store.test.js | 243 +++++++++++++++++++++++++++++++++++++++++- 8 files changed, 472 insertions(+), 33 deletions(-) diff --git a/demo/src/constants.js b/demo/src/constants.js index 9a3a8be..086822c 100644 --- a/demo/src/constants.js +++ b/demo/src/constants.js @@ -17,6 +17,14 @@ const ISOLATED_PARTITION = "demo-isolated"; /** The demo's own settings and their defaults, which the kit keeps beside its own. */ const SETTINGS_DEFAULTS = Object.freeze({ showAccentSample: true, + // The folder the demo saves its exports in, null for the system's + // Documents folder. Main alone changes it (MAIN_ONLY_SETTINGS), as an app + // whose folder is chosen with the system's folder picker would: the page + // may read it, and is refused a change to it. + exportFolder: null, }); -module.exports = { IPC, ISOLATED_PARTITION, SETTINGS_DEFAULTS }; +/** The demo's settings only main changes. */ +const MAIN_ONLY_SETTINGS = Object.freeze(["exportFolder"]); + +module.exports = { IPC, ISOLATED_PARTITION, SETTINGS_DEFAULTS, MAIN_ONLY_SETTINGS }; diff --git a/demo/src/main.js b/demo/src/main.js index f764bd8..77e5c81 100644 --- a/demo/src/main.js +++ b/demo/src/main.js @@ -9,21 +9,25 @@ // from kit.sessions.isolated(), where the kit's preload is never registered: // it gets electronAPI only, and its own channel is answered there only. // -// The demo passes the kit its own setting (and a migration), its credits, its -// own menu item and a log in a file, so every piece of the kit is in use. It -// logs what it was opened with, which the log keeps without any folder. +// The demo passes the kit its own settings (one only main changes, and a +// migration), its credits, its own menu item and a log in a file, so every +// piece of the kit is in use. It logs what it was opened with, which the log +// keeps without any folder. const path = require("path"); const { BrowserWindow } = require("electron"); -const { IPC, ISOLATED_PARTITION, SETTINGS_DEFAULTS } = require("./constants"); +const { IPC, ISOLATED_PARTITION, SETTINGS_DEFAULTS, MAIN_ONLY_SETTINGS } = require("./constants"); const kit = require("@diamonddigitaldev/electron-kit/main").start({ // package.json's build.appId: the Windows user model ID its shortcut carries. appId: "com.diamonddigitaldev.electronkitdemo", + // The export folder is main's alone, and a whole path when it's set. // Version 1 of the demo's settings: a store from before it loses the // setting the demo dropped, and its old key. settings: { defaults: SETTINGS_DEFAULTS, + mainOnly: MAIN_ONLY_SETTINGS, + checks: { exportFolder: (folder) => folder === null || (typeof folder === "string" && path.isAbsolute(folder)) }, version: 1, migrate: (settings) => { delete settings.showGrid; diff --git a/docs/README.md b/docs/README.md index 4aae1e7..280604a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -14,7 +14,7 @@ and links here, so these can be corrected without a new release. | [IPC](ipc.md) | The shared channels behind `window.kitAPI`, links and the allowlist, and `kit.ipc.handle()` for an app's own, each answering the app's own page only | | [The Shell, Settings and Credits](shell.md) | `kit.ui.mountShell()`: the nav rail, the header, the bottom bar and its panels, and the Settings view with its Update and Credits tabs | | [Page Components](page-components.md) | Toasts, prompts, form dialogs and the house's form row, the parts of a section of files (drop zone, progress, action bar), cards, the status line and busy buttons, the switch and the stepped slider, the key guard, `kit.format`, and the copy rules | -| [The Main Process](main-process.md) | The main window, isolated sessions, spell checking, one instance and the files an app is opened with, the settings and their migration, the redacted log, and the menu | +| [The Main Process](main-process.md) | The main window, isolated sessions, spell checking, one instance and the files an app is opened with, the settings (those only main changes, their checks, hearing a change) and their migration, the redacted log, and the menu | | [Updates](updates.md) | The updater: when it checks, Stable, Beta and Alpha, automatic downloads, its state, and a proxy | | [Building](building.md) | `config()`, the electron-builder config every app builds with; the files an app opens, on Linux; the update files every release carries; and naming an app's files | | [Theme, Accent and Tokens](theming.md) | The OS theme, an app's accent and its WCAG 2.2 AA check, the house tokens, focus rings and controls | diff --git a/docs/main-process.md b/docs/main-process.md index 160905e..5520673 100644 --- a/docs/main-process.md +++ b/docs/main-process.md @@ -103,6 +103,42 @@ only, or it's refused and nothing is stored. A setting whose default is `null` m takes any JSON value (File Converter's `concurrency: null`, the CPU count until someone picks one). The main process has the same settings as `kit.settings.get()` and `kit.settings.set(changes)`. +**Settings only main changes, and checks.** A setting that names a folder on disk, such as where downloads are +saved, shouldn't be the page's to point anywhere: it's chosen in main, with the system's folder picker. And a +setting may take only some values of its kind, such as a number in a range: + +```js +settings: { + defaults: { downloadFolder: null, transfersAtOnce: 3 }, + mainOnly: ["downloadFolder"], // the page reads it, and can't change it + checks: { transfersAtOnce: (n) => Number.isInteger(n) && n >= 1 && n <= 6 }, // true for a value it takes +}, +``` + +- **`mainOnly`:** a change from the page (`kitAPI.setSettings()`) naming one of them is refused whole ("The + setting "downloadFolder" can't be changed from the page."), and nothing of it is stored. `kit.settings.set()` + in main changes them as any other, and the page reads them as it reads the rest. +- **`checks`:** a function per setting, handed a copy of the value once its kind is checked. Unless it returns + `true`, the change is refused whole, from the page or from main ("The setting "transfersAtOnce" doesn't take + that value."), and nothing of it is stored. What's stored is read as it is, unchecked. +- Both name the app's own settings only, keys of `defaults`, never the kit's; a check is a function. `start()` + throws on anything else. + +**Hearing a change.** `kit.settings.onChange(listener)` hears every change stored, the page's and main's, and +returns the function that stops it: + +```js +const stop = kit.settings.onChange((changes, after, { from }) => { + if (Object.hasOwn(changes, "transfersAtOnce")) transfers.setAtOnce(after.transfersAtOnce); +}); +``` + +`changes` is what was changed, `after` every setting as stored, and `from` `"page"` or `"main"` (the updater +saving the running build's channel, the first time, is main's). Each listener has its own copies. It's told +once the change is stored and the kit has acted on it (the updater has its new channel, the log is on disk or +off), in the order the listeners were added. A refused change tells no one. A listener that throws, or returns +a promise that rejects, is logged as an error: the change stays stored, and the others still hear it. + **Migration.** An app whose settings change shape between versions numbers them: ```js diff --git a/e2e/log-store.spec.js b/e2e/log-store.spec.js index d69b342..e2b59af 100644 --- a/e2e/log-store.spec.js +++ b/e2e/log-store.spec.js @@ -4,7 +4,9 @@ // log in debug.log in its profile (userData), started with a banner and // emptied at each launch, with what it was opened with logged and no folder // in it; and a profile from before the demo's settings version 1 is migrated -// once, as the store is first opened, before the page reads anything. +// once, as the store is first opened, before the page reads anything. The +// demo's export folder is main's alone: the page is refused a change to it, +// main's is taken (and checked), and main hears each change. const fs = require("fs"); const path = require("path"); @@ -49,6 +51,45 @@ test("a profile from before the settings' version is migrated once, before the p expect(log).toContain("[INFO] Settings version 1: removing the store's obsolete key recentFiles."); }); +test("the page is refused a change to the export folder, which main alone changes; main's is taken, checked, and heard", async ({ demo }) => { + const main = await demo.mainWindow(); + const config = path.join(demo.profile, "config.json"); + /** What a kitAPI.setSettings() comes to in the page: the setting asked for, or the error it rejects with. */ + const pageSets = (changes, key) => main.evaluate(([changes, key]) => window.kitAPI.setSettings(changes).then((after) => after[key], (err) => err.message), [changes, key]); + /** What kit.settings.set() comes to in main: the setting asked for, or the error it throws. */ + const mainSets = (changes, key) => demo.app.evaluate((_electron, [changes, key]) => { + try { + return globalThis.demoKit.settings.set(changes)[key]; + } catch (err) { + return err.message; + } + }, [changes, key]); + const read = () => main.evaluate(() => window.kitAPI.getSettings()); + + // Main hears every change, and who made it. + await demo.app.evaluate(() => { + globalThis.heard = []; + globalThis.demoKit.settings.onChange((changes, _after, { from }) => globalThis.heard.push({ changes, from })); + }); + + expect(await pageSets({ showAccentSample: false, exportFolder: demo.profile }, "exportFolder")).toContain(`The setting "exportFolder" can't be changed from the page.`); + expect(await read(), "nothing of that change is stored").toMatchObject({ showAccentSample: true, exportFolder: null }); + expect(JSON.parse(fs.readFileSync(config, "utf8")).settings ?? {}).not.toHaveProperty("showAccentSample", false); + + // Main sets it, and the page reads it; a folder that isn't a whole path is refused by the demo's check. + expect(await mainSets({ exportFolder: demo.profile }, "exportFolder")).toBe(demo.profile); + expect(await mainSets({ exportFolder: "exports" }, "exportFolder")).toBe(`The setting "exportFolder" doesn't take that value.`); + expect((await read()).exportFolder).toBe(demo.profile); + expect(JSON.parse(fs.readFileSync(config, "utf8")).settings.exportFolder).toBe(demo.profile); + + // The page still changes the demo's other setting. + expect(await pageSets({ showAccentSample: false }, "showAccentSample")).toBe(false); + expect(await demo.app.evaluate(() => globalThis.heard)).toEqual([ + { changes: { exportFolder: demo.profile }, from: "main" }, + { changes: { showAccentSample: false }, from: "page" }, + ]); +}); + test.describe("the memory log", () => { test.use({ demoEnv: { KIT_DEMO_LOG: "memory" } }); diff --git a/main/index.js b/main/index.js index f6ec5eb..82c22d3 100644 --- a/main/index.js +++ b/main/index.js @@ -69,7 +69,7 @@ function registerPreload(ses) { /** * Start the kit. Call it once, at the top of main.js, before any window. * @param {{ - * settings?: { defaults?: Record, version?: number, migrate?: (settings: object, from: number) => object, obsoleteKeys?: string[] }, + * settings?: { defaults?: Record, mainOnly?: string[], checks?: Record boolean>, version?: number, migrate?: (settings: object, from: number) => object, obsoleteKeys?: string[] }, * log?: "file" | "memory" | { mode: "file" | "memory", lines?: number }, * appId?: string, * singleInstance?: boolean, @@ -86,9 +86,11 @@ function registerPreload(ses) { * keeps the log in debug.log in userData; "memory" keeps the run's last * lines (1000, or lines) in memory, and in debug.log too only while the * kit's keepLogOnDisk setting is on (log.js); without it, the console - * only. settings.version, migrate and obsoleteKeys: the store's migration - * (store.js). singleInstance: false lets more than one run (it's one, by - * default). appId: the app's build.appId, its user model ID on Windows + * only. settings.mainOnly: the app's settings the page may read and not + * change; settings.checks: a function per setting, true for a value it + * takes, from the page or main (store.js). settings.version, migrate and + * obsoleteKeys: the store's migration (store.js). singleInstance: false + * lets more than one run (it's one, by default). appId: the app's build.appId, its user model ID on Windows * (instance.js); the app's name without it. files: the app takes files * it's opened with (instance.js); files.except, the switches that make a * launch the app's own to handle, whose files aren't taken. @@ -97,7 +99,7 @@ function registerPreload(ses) { * (shell.js). * @returns {{ * ready: Promise, - * settings: { get(): object, set(changes: object): object }, + * settings: { get(): object, set(changes: object): object, onChange(listener: (changes: object, after: object, how: { from: "page" | "main" }) => void): () => void }, * ipc: { handle(channel: string, handler: (event: Electron.IpcMainInvokeEvent, ...args: any[]) => any, options?: { session?: Electron.Session }): void }, * sessions: { isolated(name: string, options?: { persist?: boolean }): Electron.Session }, * log: { error(...args: unknown[]): void, warn(...args: unknown[]): void, info(...args: unknown[]): void, debug(...args: unknown[]): void, lines(): string[] }, @@ -108,9 +110,11 @@ function registerPreload(ses) { * }} * ready resolves once the app is ready, the shared preload is registered and * the menu set: create windows after it. settings are the app's settings, - * for its main process. ipc.handle() answers one of the app's own channels, - * for the app's own page only (ipc.js), in the UI session or in the - * isolated session given. sessions.isolated() makes a session without the + * for its main process; settings.onChange() hears every stored change, + * after the updater and the log have, and returns the function that stops + * it. ipc.handle() answers one of the app's own channels, for the app's + * own page only (ipc.js), in the UI session or in the isolated session + * given. sessions.isolated() makes a session without the * kit's bridge (sessions.js). log is the app's log, redacted, and its * lines() the banner and the run's last lines (log.js). * windows.createMain() makes the main window (windows.js). @@ -140,6 +144,7 @@ function start(config = {}) { banner: `${config.name ?? app.getName()} ${app.getVersion()} started`, }); const settings = store.createSettings({ ...config.settings, log, memoryLog: mode === "memory" }); + const changed = store.createChangeListeners(log); const credits = info.checkInfo({ credits: config.credits, repository: config.repository, name: config.name }); const openExternal = shell.createOpener({ allow: shell.checkOpenExternal(config.openExternal), @@ -149,7 +154,16 @@ function start(config = {}) { return [...shown.credits.lines.flat().map((part) => part?.href ?? null), shown.credits.donate, shown.repository]; }, }); - const updates = updater.createUpdater({ options: updater.checkUpdates(config.updates), app, settings, send: sendUpdateStatus, log }); + // The updater saves the running build's channel itself, the first time: the app's listeners hear that too. + const updaterSettings = { + get: settings.get, + set: (changes) => { + const after = settings.set(changes); + changed.tell(changes, after, "main"); + return after; + }, + }; + const updates = updater.createUpdater({ options: updater.checkUpdates(config.updates), app, settings: updaterSettings, send: sendUpdateStatus, log }); const template = menu.menuTemplate({ items: config.menu?.items, version: app.getVersion(), @@ -163,19 +177,25 @@ function start(config = {}) { windows = windowing.createWindows({ bounds: settings.bounds, onMainCreated: files.mainCreated }); files.listen({ isPackaged: app.isPackaged, appPath: app.getAppPath() }); - /** Change some settings, from the page or the app's main, and tell the updater. */ - function setSettings(changes) { + /** + * Change some settings, from the page or the app's main; tell the updater + * and the log, then the app's listeners. + * @param {Record} changes + * @param {"page" | "main"} from + */ + function setSettings(changes, from) { const before = settings.get(); - const after = settings.set(changes); + const after = settings.set(changes, { from }); updates.settingsChanged(changes, before); if (log.mode === "memory" && Object.hasOwn(changes, "keepLogOnDisk")) log.keepOnDisk(after.keepLogOnDisk); + changed.tell(changes, after, from); return after; } ipc.handle(INVOKE.APP_GET_VERSION, () => app.getVersion()); ipc.handle(INVOKE.APP_GET_INFO, () => info.appInfo(credits)); ipc.handle(INVOKE.SETTINGS_GET, () => settings.get()); - ipc.handle(INVOKE.SETTINGS_SET, (_event, changes) => setSettings(changes)); + ipc.handle(INVOKE.SETTINGS_SET, (_event, changes) => setSettings(changes, "page")); ipc.handle(INVOKE.SHELL_OPEN_EXTERNAL, (_event, url) => openExternal(url)); ipc.handle(INVOKE.UPDATE_GET_STATUS, () => updates.status()); ipc.handle(INVOKE.UPDATE_CHECK, () => updates.check()); @@ -211,7 +231,7 @@ function start(config = {}) { }); return { ready, - settings: { get: settings.get, set: setSettings }, + settings: { get: settings.get, set: (changes) => setSettings(changes, "main"), onChange: changed.onChange }, ipc: { handle: (channel, handler, options) => ipc.handleApp(channel, handler, options, isolation.isIsolated) }, sessions: { isolated: isolation.isolated }, log: { error: log.error, warn: log.warn, info: log.info, debug: log.debug, lines: log.lines }, diff --git a/main/store.js b/main/store.js index a74b6ae..362e7b8 100644 --- a/main/store.js +++ b/main/store.js @@ -14,6 +14,15 @@ // a boolean), or it's refused and nothing is stored. A setting whose default // is null has no kind yet: it takes any JSON value. // +// Some of an app's settings are main's alone (settings: { mainOnly }): the +// page may read them, but a change from the page naming one is refused, so a +// page that went wrong can't point a setting such as a downloads folder +// anywhere. And a setting can have a check of its own (settings: { checks }), +// a function of the value that says whether it's taken: a change it refuses, +// from the page or from main, is refused whole. Reads aren't checked. The +// app's main hears every stored change, and who made it, through +// kit.settings.onChange() (createChangeListeners()). +// // Migration: an app whose settings change shape between versions gives them a // version (settings: { version, migrate, obsoleteKeys }). When the file was // last written by an older version, or by none, migrate(settings, from) is @@ -137,6 +146,29 @@ function checkDefaults(defaults) { } } +/** + * Check the settings only main changes, and the app's checks: each names one + * of the app's own settings (a key of its defaults), and each check is a + * function. Throws on anything else, when kit.start() is called. + * @param {Record} defaults - Already checked. + * @param {string[]} mainOnly + * @param {Record boolean>} checks + */ +function checkRules(defaults, mainOnly, checks) { + /** Throws unless a key, named in an option, is one of the app's own settings. */ + const ownSetting = (option, key) => { + if (Object.hasOwn(KIT_DEFAULTS, key) || Object.hasOwn(MEMORY_LOG_DEFAULTS, key)) throw new Error(`kit.start(): settings.${option} names "${key}", one of the kit's own settings; it takes the app's own only.`); + if (!Object.hasOwn(defaults, key)) throw new Error(`kit.start(): settings.${option} names "${key}", which isn't in settings.defaults.`); + }; + if (!Array.isArray(mainOnly) || !mainOnly.every((key) => typeof key === "string")) throw new Error("kit.start(): settings.mainOnly must be a list of the app's own settings, by name."); + for (const key of mainOnly) ownSetting("mainOnly", key); + if (!isPlainObject(checks)) throw new Error("kit.start(): settings.checks must be an object of functions, by setting."); + for (const [key, check] of Object.entries(checks)) { + ownSetting("checks", key); + if (typeof check !== "function") throw new Error(`kit.start(): settings.checks.${key} must be a function.`); + } +} + /** * Check the migration's options: a version (a whole number from 1), a migrate * function, and the obsolete keys (names other than the settings' own). All @@ -188,6 +220,8 @@ function migrateStore(store, { version, migrate, obsoleteKeys = [], log }) { * The app's settings. * @param {{ * defaults?: Record, + * mainOnly?: string[], + * checks?: Record boolean>, * version?: number, * migrate?: (settings: object, from: number) => object, * obsoleteKeys?: string[], @@ -195,15 +229,20 @@ function migrateStore(store, { version, migrate, obsoleteKeys = [], log }) { * memoryLog?: boolean, * open?: () => { get(key: string): unknown, set(key: string, value: unknown): void, delete(key: string): void }, * }} [options] - * defaults: the app's own settings and their defaults. version, migrate and - * obsoleteKeys: the migration (the top of this file); log: where it says - * what it removed. memoryLog: the app keeps the memory log, so it has - * keepLogOnDisk too. open: opens the store, electron-store unless a test - * stands one in. It's opened on first use, never before, and migrated then. + * defaults: the app's own settings and their defaults. mainOnly: those + * the page may not change. checks: a function per setting, true for a + * value it takes. version, migrate and obsoleteKeys: the migration (the + * top of this file); log: where it says what it removed. memoryLog: the + * app keeps the memory log, so it has keepLogOnDisk too. open: opens the + * store, electron-store unless a test stands one in. It's opened on first + * use, never before, and migrated then. */ -function createSettings({ defaults = {}, version, migrate, obsoleteKeys, log, memoryLog = false, open = openElectronStore } = {}) { +function createSettings({ defaults = {}, mainOnly = [], checks = {}, version, migrate, obsoleteKeys, log, memoryLog = false, open = openElectronStore } = {}) { checkDefaults(defaults); + checkRules(defaults, mainOnly, checks); checkMigration({ version, migrate, obsoleteKeys }); + const pageCantChange = new Set(mainOnly); + const checked = { ...checks }; const all = Object.freeze({ ...defaults, ...KIT_DEFAULTS, ...(memoryLog ? MEMORY_LOG_DEFAULTS : {}) }); let store = null; const stored = () => { @@ -229,18 +268,23 @@ function createSettings({ defaults = {}, version, migrate, obsoleteKeys, log, me /** * Change some settings, and return every setting. Refuses the whole change, - * storing nothing, if it names a setting that doesn't exist or changes a - * setting's kind. + * storing nothing, if it names a setting that doesn't exist, changes a + * setting's kind, or gives a value the setting's check refuses; or, from + * the page, if it names a setting only main changes. * @param {Record} changes + * @param {{ from?: "page" | "main" }} [options] - Who's changing them: main, unless it's the page. */ - function set(changes) { + function set(changes, { from = "main" } = {}) { if (!isPlainObject(changes)) throw new Error("Settings can only be changed with an object of settings."); for (const [key, value] of Object.entries(changes)) { if (!Object.hasOwn(all, key)) throw new Error(`There's no setting called "${key}".`); + if (from === "page" && pageCantChange.has(key)) throw new Error(`The setting "${key}" can't be changed from the page.`); if (Object.hasOwn(KIT_CHOICES, key) && !KIT_CHOICES[key].includes(value)) { throw new Error(`The setting "${key}" takes one of ${KIT_CHOICES[key].map((c) => JSON.stringify(c)).join(", ")}.`); } if (!fits(value, all[key])) throw new Error(`The setting "${key}" takes ${KIND_NAMES[kindOf(all[key])]}.`); + // A copy, so a check can't change what's stored. + if (Object.hasOwn(checked, key) && checked[key](structuredClone(value)) !== true) throw new Error(`The setting "${key}" doesn't take that value.`); } const next = { ...get(), ...changes }; store.set(STORE_KEY, next); @@ -268,10 +312,57 @@ function createSettings({ defaults = {}, version, migrate, obsoleteKeys, log, me return { get, set, defaults: all, bounds }; } +/** + * The app's listeners for its settings' changes (kit.settings.onChange()). + * Each stored change is told to every listener, as (changes, after, { from }), + * each its own copy, in the order they were added. A listener that throws, or + * returns a promise that rejects, is logged: the change stays stored, and the + * other listeners still hear it. + * @param {{ error(...args: unknown[]): void }} log + */ +function createChangeListeners(log) { + const listeners = new Set(); + + /** + * Listen for every stored change. Returns the function that stops it. + * @param {(changes: object, after: object, how: { from: "page" | "main" }) => void} listener + */ + function onChange(listener) { + if (typeof listener !== "function") throw new Error("kit.settings.onChange() takes a function."); + // Its own entry, so the same function added twice is two listeners, each stopped on its own. + const entry = { listener }; + listeners.add(entry); + return () => { + listeners.delete(entry); + }; + } + + /** + * Tell every listener of a stored change. + * @param {Record} changes + * @param {Record} after - Every setting, as stored. + * @param {"page" | "main"} from + */ + function tell(changes, after, from) { + const failed = (err) => log.error("A kit.settings.onChange() listener failed:", err); + // As they are now: one added or stopped by a listener doesn't change who hears this one. + for (const { listener } of [...listeners]) { + try { + const result = listener(structuredClone(changes), structuredClone(after), { from }); + if (typeof result?.then === "function") result.then(undefined, failed); + } catch (err) { + failed(err); + } + } + } + + return { onChange, tell }; +} + /** electron-store, in its default file. It's ESM only; Node's require() loads it as a namespace. */ function openElectronStore() { const Store = require("electron-store").default; return new Store(); } -module.exports = { createSettings, KIT_DEFAULTS, MEMORY_LOG_DEFAULTS, KIT_CHOICES, STORE_KEY, VERSION_KEY, BOUNDS_KEY }; +module.exports = { createSettings, createChangeListeners, KIT_DEFAULTS, MEMORY_LOG_DEFAULTS, KIT_CHOICES, STORE_KEY, VERSION_KEY, BOUNDS_KEY }; diff --git a/test/store.test.js b/test/store.test.js index 2c99ba5..bc66e6c 100644 --- a/test/store.test.js +++ b/test/store.test.js @@ -3,12 +3,16 @@ // The settings (main/store.js): kept under one "settings" key, read merged // over the app's defaults and the kit's own, changed a few at a time, and // never taking a setting that doesn't exist or a value of the wrong kind. And -// settings:get / settings:set, which kit.start() answers with them. +// settings:get / settings:set, which kit.start() answers with them: the +// settings only main changes, the app's checks, and kit.settings.onChange(). const test = require("node:test"); const assert = require("node:assert/strict"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); const { createSettings, KIT_DEFAULTS, KIT_CHOICES, STORE_KEY, VERSION_KEY } = require("../main/store"); -const { loadMain, appPage } = require("./helpers/main"); +const { loadMain, appPage, fakeAutoUpdater } = require("./helpers/main"); const { INVOKE } = require("../main/channels"); /** A store in memory: `data` is what's on disk. */ @@ -169,6 +173,241 @@ test("kit.start() throws on bad settings defaults, before registering anything", assert.doesNotThrow(() => main.start()); }); +// -- Settings only main changes, checks, and main hearing each change -------------- + +/** Dropgate's: the downloads folder chosen in main only, and how many transfers run at once, from 1 to 6. */ +const GUARDED = { + defaults: { downloadFolder: null, transfersAtOnce: 3, theme: "auto" }, + mainOnly: ["downloadFolder"], + checks: { transfersAtOnce: (n) => Number.isInteger(n) && n >= 1 && n <= 6 }, +}; + +test("mainOnly and checks are checked when the settings are made: the app's own settings, and functions", () => { + const defaults = { downloadFolder: null, transfersAtOnce: 3 }; + const bad = [ + [{ mainOnly: "downloadFolder" }, /settings\.mainOnly must be a list of the app's own settings/], + [{ mainOnly: [1] }, /settings\.mainOnly must be a list/], + [{ mainOnly: ["navCollapsed"] }, /settings\.mainOnly names "navCollapsed", one of the kit's own settings/], + [{ mainOnly: ["keepLogOnDisk"] }, /settings\.mainOnly names "keepLogOnDisk", one of the kit's own settings/], + [{ mainOnly: ["servers"] }, /settings\.mainOnly names "servers", which isn't in settings\.defaults/], + [{ checks: [] }, /settings\.checks must be an object of functions/], + [{ checks: () => true }, /settings\.checks must be an object of functions/], + [{ checks: { updateChannel: () => true } }, /settings\.checks names "updateChannel", one of the kit's own settings/], + [{ checks: { servers: () => true } }, /settings\.checks names "servers", which isn't in settings\.defaults/], + [{ checks: { transfersAtOnce: true } }, /settings\.checks\.transfersAtOnce must be a function/], + ]; + for (const [options, error] of bad) { + assert.throws(() => createSettings({ defaults, ...options, open: memoryStore().open }), error, String(Object.keys(options))); + } + assert.doesNotThrow(() => createSettings({ defaults, mainOnly: [], checks: {}, open: memoryStore().open })); +}); + +test("kit.start() throws on bad mainOnly or checks, with kit.start()'s name, before registering anything", () => { + const { main, handlers } = loadMain(); + assert.throws(() => main.start({ settings: { defaults: { a: 1 }, mainOnly: ["b"] } }), /^Error: kit\.start\(\): settings\.mainOnly names "b"/); + assert.throws(() => main.start({ settings: { defaults: { a: 1 }, checks: { a: "positive" } } }), /^Error: kit\.start\(\): settings\.checks\.a must be a function/); + assert.equal(handlers.size, 0); + assert.doesNotThrow(() => main.start({ settings: GUARDED })); +}); + +test("the page's change naming a setting only main changes is refused whole, and nothing is stored; main's is taken", async () => { + const { main, handlers, eventFrom, stored } = loadMain({ stored: { settings: { theme: "dark" } } }); + const kit = main.start({ settings: GUARDED }); + const page = eventFrom(appPage()); + const set = handlers.get(INVOKE.SETTINGS_SET); + for (const changes of [{ downloadFolder: "C:\\Elsewhere" }, { theme: "light", downloadFolder: null }, { downloadFolder: 7 }]) { + assert.throws(() => set(page, changes), /^Error: The setting "downloadFolder" can't be changed from the page\.$/, JSON.stringify(changes)); + } + assert.deepEqual(stored.settings, { theme: "dark" }, "nothing of any of them is stored"); + + // Main changes it as any other, and the page reads it as it is. + assert.equal(kit.settings.set({ downloadFolder: "D:\\Downloads" }).downloadFolder, "D:\\Downloads"); + assert.equal((await handlers.get(INVOKE.SETTINGS_GET)(page)).downloadFolder, "D:\\Downloads"); + // The page still changes the rest, and the kit's own. + assert.deepEqual(pick(await set(page, { theme: "light", navCollapsed: true }), ["theme", "navCollapsed", "downloadFolder"]), { theme: "light", navCollapsed: true, downloadFolder: "D:\\Downloads" }); +}); + +test("a check refuses a value from the page and from main, storing nothing of the change; it's handed a copy", async () => { + const seen = []; + const { main, handlers, eventFrom, stored } = loadMain(); + const kit = main.start({ + settings: { + ...GUARDED, + defaults: { ...GUARDED.defaults, ports: [] }, + checks: { + ...GUARDED.checks, + ports: (list) => { + const ok = list.every((n) => n > 0); + list.push(0); + seen.push(list); + return ok; + }, + }, + }, + }); + const page = eventFrom(appPage()); + const set = handlers.get(INVOKE.SETTINGS_SET); + for (const value of [0, 7, 2.5]) { + assert.throws(() => set(page, { theme: "light", transfersAtOnce: value }), /^Error: The setting "transfersAtOnce" doesn't take that value\.$/, `the page's ${value}`); + assert.throws(() => kit.settings.set({ theme: "light", transfersAtOnce: value }), /^Error: The setting "transfersAtOnce" doesn't take that value\.$/, `main's ${value}`); + } + // Its kind is checked first: the check is never handed a value of another kind. + assert.throws(() => set(page, { transfersAtOnce: "6" }), /"transfersAtOnce" takes a number/); + assert.equal(stored.settings, undefined, "nothing is stored"); + + assert.equal((await set(page, { transfersAtOnce: 6 })).transfersAtOnce, 6); + assert.equal(kit.settings.set({ transfersAtOnce: 1 }).transfersAtOnce, 1); + // A check is handed a copy: what it does to it isn't stored. + assert.deepEqual(kit.settings.set({ ports: [8080] }).ports, [8080]); + assert.deepEqual(stored.settings.ports, [8080]); + assert.deepEqual(seen, [[8080, 0]]); +}); + +test("reads aren't checked: a stored value a check would refuse is still handed out", () => { + const { main } = loadMain({ stored: { settings: { transfersAtOnce: 12 } } }); + assert.equal(main.start({ settings: GUARDED }).settings.get().transfersAtOnce, 12); +}); + +test("kit.settings.onChange() hears every stored change, as (changes, after, { from }), the page's and main's", async () => { + const { main, handlers, eventFrom } = loadMain(); + const kit = main.start({ settings: GUARDED }); + await kit.ready; + const heard = []; + kit.settings.onChange((changes, after, how) => heard.push([changes, after, how])); + + const fromPage = await handlers.get(INVOKE.SETTINGS_SET)(eventFrom(appPage()), { transfersAtOnce: 5 }); + const fromMain = kit.settings.set({ downloadFolder: "D:\\Downloads", navCollapsed: true }); + assert.deepEqual(heard, [ + [{ transfersAtOnce: 5 }, fromPage, { from: "page" }], + [{ downloadFolder: "D:\\Downloads", navCollapsed: true }, fromMain, { from: "main" }], + ]); + assert.deepEqual(heard[1][1], { ...GUARDED.defaults, ...KIT, transfersAtOnce: 5, downloadFolder: "D:\\Downloads", navCollapsed: true }); +}); + +test("a refused change is heard by no listener", async () => { + const { main, handlers, eventFrom } = loadMain(); + const kit = main.start({ settings: GUARDED }); + let heard = 0; + kit.settings.onChange(() => heard++); + assert.throws(() => handlers.get(INVOKE.SETTINGS_SET)(eventFrom(appPage()), { downloadFolder: "C:\\" })); + assert.throws(() => kit.settings.set({ transfersAtOnce: 9 })); + assert.throws(() => kit.settings.set({ nope: 1 })); + assert.equal(heard, 0); +}); + +test("each listener gets its own copy: what one does to it changes neither what's stored nor what the next one hears", () => { + const { main, stored } = loadMain(); + const kit = main.start({ settings: GUARDED }); + const heard = []; + kit.settings.onChange((changes, after) => { + changes.theme = "changed"; + after.downloadFolder = "C:\\Elsewhere"; + }); + kit.settings.onChange((changes, after) => heard.push(changes.theme, after.downloadFolder)); + const returned = kit.settings.set({ theme: "light" }); + assert.deepEqual(heard, ["light", null]); + assert.equal(stored.settings.downloadFolder, null); + assert.equal(returned.downloadFolder, null); +}); + +test("the listeners hear a change after the updater and the log have acted on it", async (t) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "kit-on-change-")); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + // An update waiting, with automatic downloads off. + const autoUpdater = fakeAutoUpdater({ + checkForUpdates: async () => ({ isUpdateAvailable: true, updateInfo: { version: "2.1.0" } }), + downloadUpdate: () => new Promise(() => {}), + }); + const { main, handlers, eventFrom, openWindow, electron } = loadMain({ version: "2.0.0", isPackaged: true, autoUpdater, userData: dir, stored: { settings: { autoDownloadUpdates: false } } }); + t.mock.method(console, "log", () => {}); + const kit = main.start({ log: "memory", updates: { checkOnLaunch: false } }); + await kit.ready; + const ui = openWindow(); + ui.webContents.session = electron.session.defaultSession; + const page = eventFrom(appPage()); + assert.equal((await handlers.get(INVOKE.UPDATE_CHECK)(page)).state, "available"); + + const seen = []; + kit.settings.onChange((changes) => seen.push({ changes, update: ui.sent.at(-1).args[0].state, onDisk: fs.existsSync(path.join(dir, "debug.log")) })); + await handlers.get(INVOKE.SETTINGS_SET)(page, { autoDownloadUpdates: true }); + await handlers.get(INVOKE.SETTINGS_SET)(page, { keepLogOnDisk: true }); + assert.deepEqual(seen, [ + { changes: { autoDownloadUpdates: true }, update: "downloading", onDisk: false }, + { changes: { keepLogOnDisk: true }, update: "downloading", onDisk: true }, + ]); +}); + +test("the updater saving the build's own channel, the first time, is heard as main's", async () => { + const { main } = loadMain({ version: "2.0.0-beta.1" }); + const kit = main.start({ updates: {} }); + const heard = []; + kit.settings.onChange((changes, after, how) => heard.push([changes, after.updateChannel, how])); + await kit.ready; + assert.deepEqual(heard, [[{ updateChannel: "beta" }, "beta", { from: "main" }]]); +}); + +test("onChange() returns the function that stops the listener; the same function added twice is two listeners", () => { + const { main } = loadMain(); + const kit = main.start({ settings: GUARDED }); + const heard = []; + const listener = (changes) => heard.push(changes.transfersAtOnce); + const stopFirst = kit.settings.onChange(listener); + const stopSecond = kit.settings.onChange(listener); + kit.settings.set({ transfersAtOnce: 2 }); + assert.equal(stopFirst(), undefined); + stopFirst(); + kit.settings.set({ transfersAtOnce: 4 }); + stopSecond(); + kit.settings.set({ transfersAtOnce: 6 }); + assert.deepEqual(heard, [2, 2, 4]); + assert.throws(() => kit.settings.onChange("listener"), /^Error: kit\.settings\.onChange\(\) takes a function\.$/); + assert.throws(() => kit.settings.onChange(), /takes a function/); +}); + +test("a listener stopped, or added, by another while a change is told hears that one as it was", () => { + const { main } = loadMain(); + const kit = main.start({ settings: GUARDED }); + const heard = []; + let stopSecond = null; + kit.settings.onChange(() => { + heard.push("first"); + stopSecond(); + kit.settings.onChange(() => heard.push("added")); + }); + stopSecond = kit.settings.onChange(() => heard.push("second")); + kit.settings.set({ theme: "light" }); + assert.deepEqual(heard, ["first", "second"]); +}); + +test("a listener that throws, or rejects, is logged: the change stays stored and the others still hear it", async (t) => { + const { main, handlers, eventFrom, stored } = loadMain(); + const errors = t.mock.method(console, "error", () => {}); + const kit = main.start({ settings: GUARDED }); + const heard = []; + kit.settings.onChange(() => { + throw new Error("the registry isn't ready"); + }); + kit.settings.onChange(async () => { + throw new Error("the async one failed"); + }); + kit.settings.onChange((_changes, _after, { from }) => heard.push(from)); + + const after = await handlers.get(INVOKE.SETTINGS_SET)(eventFrom(appPage()), { transfersAtOnce: 4 }); + assert.equal(after.transfersAtOnce, 4, "the page is answered as if nothing had failed"); + assert.equal(stored.settings.transfersAtOnce, 4); + assert.equal(kit.settings.set({ transfersAtOnce: 5 }).transfersAtOnce, 5); + await new Promise((resolve) => setImmediate(resolve)); + assert.deepEqual(heard, ["page", "main"]); + const failed = kit.log.lines().filter((line) => line.includes("[ERROR] A kit.settings.onChange() listener failed: Error: ")); + assert.equal(failed.length, 4); + assert.equal(failed.filter((line) => line.includes("the registry isn't ready")).length, 2); + assert.equal(failed.filter((line) => line.includes("the async one failed")).length, 2); + assert.equal(errors.mock.callCount(), 4); +}); + +/** Some of an object's keys. */ +const pick = (object, keys) => Object.fromEntries(keys.map((key) => [key, object[key]])); + test("a setting whose default is null takes any JSON value, and keeps it across launches", () => { // File Converter's concurrency: null until someone picks a number. const defaults = { ...APP_DEFAULTS, concurrency: null };