refactor(main): improve config file handling

Simplify config file watcher by debouncing events instead of throttling.

Make sure to flush config changes on application exit.

Signed-off-by: Kristóf Marussy <kristof@marussy.com>

1 files changed, 42 insertions, 52 deletions

diff --git a/packages/main/src/infrastructure/config/impl/__tests__/ConfigFile.integ.test.ts b/packages/main/src/infrastructure/config/impl/__tests__/ConfigFile.integ.test.ts
index b61e85a..dd4aaaa 100644
--- a/packages/main/src/infrastructure/config/impl/__tests__/ConfigFile.integ.test.ts
+++ b/packages/main/src/infrastructure/config/impl/__tests__/ConfigFile.integ.test.ts
@@ -26,6 +26,7 @@ import {
   rename,
   rm,
   stat,
+  utimes,
   writeFile,
 } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
@@ -37,7 +38,7 @@ import { mocked } from 'jest-mock';
 import Disposer from '../../../../utils/Disposer.js';
 import ConfigFile, { CONFIG_FILE_NAME } from '../ConfigFile.js';
 
-const THROTTLE_MS = 1000;
+const CONFIG_CHANGE_DEBOUNCE_MS = 10;
 
 let filesystemDelay = 100;
 let realSetTimeout: typeof setTimeout;
@@ -45,35 +46,29 @@ let userDataDir: string | undefined;
 let configFilePath: string;
 let repository: ConfigFile;
 
+async function realDelay(ms: number): Promise<void> {
+  return new Promise<void>((resolve) => {
+    realSetTimeout(() => resolve(), ms);
+  });
+}
+
 /**
  * Wait for a short (real-time) delay to let node be notified of file changes.
  *
- * Will also run all pending microtasks to completion.
- *
  * Use the `SOPHIE_INTEG_TEST_DELAY` environmental variable to customize the delay
  * (e.g., quicker test execution on faster computers or longer wait time in CI).
  * The default delay is 100 ms, but as little as 10 ms might be enough, depending on your system.
  *
- * @returns A promise that resolves in a short while.
+ * @param ms The time that should elapse after noticing the filesystem change.
+ * @returns A promise that resolves in a `filesystemDelay` ms.
  */
-function catchUpWithFilesystem(): Promise<void> {
-  return new Promise((resolve, reject) => {
-    realSetTimeout(() => {
-      try {
-        jest.runAllTicks();
-      } catch (error) {
-        reject(error);
-        return;
-      }
-      resolve();
-    }, filesystemDelay);
-  });
-}
-
-async function catchUpWithFilesystemAndTimers(): Promise<void> {
-  await catchUpWithFilesystem();
-  jest.runAllTimers();
-  await catchUpWithFilesystem();
+async function catchUpWithFilesystem(
+  ms = CONFIG_CHANGE_DEBOUNCE_MS,
+): Promise<void> {
+  await realDelay(filesystemDelay);
+  jest.advanceTimersByTime(ms);
+  // Some extra real time is needed after advancing the timer to make sure the callback runs.
+  await realDelay(Math.max(filesystemDelay / 10, 1));
 }
 
 beforeAll(() => {
@@ -91,7 +86,11 @@ beforeEach(async () => {
   try {
     userDataDir = await mkdtemp(path.join(tmpdir(), 'sophie-configFile-'));
     configFilePath = path.join(userDataDir, CONFIG_FILE_NAME);
-    repository = new ConfigFile(userDataDir);
+    repository = new ConfigFile(
+      userDataDir,
+      CONFIG_FILE_NAME,
+      CONFIG_CHANGE_DEBOUNCE_MS,
+    );
   } catch (error) {
     userDataDir = undefined;
     throw error;
@@ -180,7 +179,7 @@ describe('watchConfig', () => {
 
   describe('when the config file does not exist', () => {
     beforeEach(() => {
-      watcher = repository.watchConfig(callback, THROTTLE_MS);
+      watcher = repository.watchConfig(callback);
     });
 
     test('notifies when the config file is created externally', async () => {
@@ -191,7 +190,7 @@ describe('watchConfig', () => {
 
     test('does not notify when the config file is created by the repository', async () => {
       await repository.writeConfig('Hello World!');
-      await catchUpWithFilesystemAndTimers();
+      await catchUpWithFilesystem();
       expect(callback).not.toHaveBeenCalled();
     });
   });
@@ -199,7 +198,7 @@ describe('watchConfig', () => {
   describe('when the config file already exists', () => {
     beforeEach(async () => {
       await writeFile(configFilePath, 'Hello World!', 'utf8');
-      watcher = repository.watchConfig(callback, THROTTLE_MS);
+      watcher = repository.watchConfig(callback);
     });
 
     test('notifies when the config file is updated externally', async () => {
@@ -210,35 +209,23 @@ describe('watchConfig', () => {
 
     test('does not notify when the config file is created by the repository', async () => {
       await repository.writeConfig('Hi Mars!');
-      await catchUpWithFilesystemAndTimers();
+      await catchUpWithFilesystem();
       expect(callback).not.toHaveBeenCalled();
     });
 
-    test('throttles notifications of external changes to the config file', async () => {
+    test('debounces changes to the config file', async () => {
       await writeFile(configFilePath, 'Hi Mars!', 'utf8');
-      await catchUpWithFilesystem();
-      expect(callback).toHaveBeenCalledTimes(1);
-      mocked(callback).mockClear();
-
-      jest.advanceTimersByTime(100);
-      await writeFile(configFilePath, 'Howdy Venus!', 'utf8');
-      await catchUpWithFilesystem();
-
-      jest.advanceTimersByTime(100);
-      await catchUpWithFilesystem();
+      await catchUpWithFilesystem(5);
       expect(callback).not.toHaveBeenCalled();
 
-      jest.advanceTimersByTime(THROTTLE_MS);
+      await writeFile(configFilePath, 'Hi Mars!', 'utf8');
       await catchUpWithFilesystem();
       expect(callback).toHaveBeenCalledTimes(1);
     });
 
     test('handles the config file being deleted', async () => {
-      await writeFile(configFilePath, 'Hi Mars!', 'utf8');
-      // Clear the mock if we inadverently set off `callback`.
-      mocked(callback).mockClear();
       await rm(configFilePath);
-      await catchUpWithFilesystemAndTimers();
+      await catchUpWithFilesystem();
       expect(callback).not.toHaveBeenCalled();
 
       await writeFile(configFilePath, 'Hello World!', 'utf8');
@@ -249,15 +236,12 @@ describe('watchConfig', () => {
     test('handles the config file being renamed', async () => {
       const renamedPath = `${configFilePath}.renamed`;
 
-      await writeFile(configFilePath, 'Hi Mars!', 'utf8');
-      // Clear the mock if we inadverently set off `callback`.
-      mocked(callback).mockClear();
       await rename(configFilePath, renamedPath);
-      await catchUpWithFilesystemAndTimers();
+      await catchUpWithFilesystem();
       expect(callback).not.toHaveBeenCalled();
 
       await writeFile(renamedPath, 'Hello World!', 'utf8');
-      await catchUpWithFilesystemAndTimers();
+      await catchUpWithFilesystem();
       expect(callback).not.toHaveBeenCalled();
 
       await writeFile(configFilePath, 'Hello World!', 'utf8');
@@ -266,21 +250,27 @@ describe('watchConfig', () => {
     });
 
     test('handles other filesystem errors', async () => {
-      await writeFile(configFilePath, 'Hi Mars!', 'utf8');
-      // Clear the mock if we inadverently set off `callback`.
-      mocked(callback).mockClear();
       const { mode } = await stat(userDataDir!);
+      await writeFile(configFilePath, 'Hi Mars!', 'utf8');
       // Remove permission to force a filesystem error.
       // eslint-disable-next-line no-bitwise -- Compute reduced permissions.
       await chmod(userDataDir!, mode & 0o666);
       try {
-        await catchUpWithFilesystemAndTimers();
+        await catchUpWithFilesystem();
         expect(callback).not.toHaveBeenCalled();
       } finally {
         await chmod(userDataDir!, mode);
       }
     });
 
+    test('does not notify when the modification date is prior to the last write', async () => {
+      await repository.writeConfig('Hello World!');
+      const date = new Date(Date.now() - 3_600_000);
+      await utimes(configFilePath, date, date);
+      await catchUpWithFilesystem();
+      expect(callback).not.toHaveBeenCalled();
+    });
+
     test('handles callback errors', async () => {
       mocked(callback).mockRejectedValue(new Error('Test error'));
       await writeFile(configFilePath, 'Hi Mars!', 'utf8');