Skip to content

Commit 7265105

Browse files
fix: reload var state when emhttp writes temp files (#1950)
## Summary - normalize `*.ini.new` state-file events back to their underlying state file key in the state watcher - stop ignoring the watched root state directory when chokidar calls the ignore callback without stats - add unit and integration coverage for the temp-file watcher path ## Why On the Unraid server, emhttp writes temp files like `var.ini.new` and swaps them in. The existing watcher only recognized exact `*.ini` basenames, so `var.ini.new` updates were ignored and the API store could stay stale even when disk state was correct. ## Testing - `pnpm --filter ./api test src/__test__/store/watch/state-watch.test.ts src/__test__/store/watch/state-watch.integration.test.ts src/unraid-api/graph/resolvers/servers/server.service.spec.ts` - `pnpm build` - deployed to `192.168.1.176` with `pnpm unraid:deploy 192.168.1.176` - verified live GraphQL returned `Gamer5` after restart - verified live rename flow `Gamer5 -> Gamer5tmp -> Gamer5` and saw GraphQL follow both changes correctly <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * StateManager now provides a public method to gracefully close and clean up file watchers. * **Tests** * Added comprehensive integration test suite for StateManager that validates state reload behavior triggered by filesystem events in the state directory. * Extended directory watcher tests with validation for temporary state file variants and proper event handling during state updates. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 5be53a4 commit 7265105

File tree

3 files changed

+173
-8
lines changed

3 files changed

+173
-8
lines changed
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
4+
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
6+
7+
import { StateFileKey } from '@app/store/types.js';
8+
9+
const testContext = vi.hoisted(() => ({
10+
states: '',
11+
}));
12+
13+
const mockedStore = vi.hoisted(() => ({
14+
dispatch: vi.fn().mockResolvedValue(undefined),
15+
}));
16+
17+
const mockedGetters = vi.hoisted(() => ({
18+
paths: vi.fn(() => ({
19+
states: testContext.states,
20+
})),
21+
}));
22+
23+
const mockedLoadSingleStateFile = vi.hoisted(() =>
24+
vi.fn((key: StateFileKey) => ({ type: 'emhttp/load-single-state-file', payload: key }))
25+
);
26+
27+
const mockedLoadRegistrationKey = vi.hoisted(() =>
28+
vi.fn(() => ({ type: 'registration/load-registration-key' }))
29+
);
30+
31+
vi.mock('@app/environment.js', () => ({
32+
CHOKIDAR_USEPOLLING: false,
33+
}));
34+
35+
vi.mock('@app/store/index.js', () => ({
36+
store: mockedStore,
37+
getters: mockedGetters,
38+
}));
39+
40+
vi.mock('@app/store/modules/emhttp.js', () => ({
41+
loadSingleStateFile: mockedLoadSingleStateFile,
42+
}));
43+
44+
vi.mock('@app/store/modules/registration.js', () => ({
45+
loadRegistrationKey: mockedLoadRegistrationKey,
46+
}));
47+
48+
vi.mock('@app/core/log.js', () => ({
49+
emhttpLogger: {
50+
trace: vi.fn(),
51+
debug: vi.fn(),
52+
error: vi.fn(),
53+
},
54+
}));
55+
56+
describe('StateManager integration', () => {
57+
let tempRoot: string;
58+
let statesDirectory: string;
59+
60+
beforeEach(async () => {
61+
vi.resetModules();
62+
vi.clearAllMocks();
63+
64+
tempRoot = await mkdtemp(join(tmpdir(), 'state-watch-'));
65+
statesDirectory = join(tempRoot, 'state');
66+
await mkdir(statesDirectory);
67+
testContext.states = statesDirectory;
68+
69+
const { StateManager } = await import('@app/store/watch/state-watch.js');
70+
const { store } = await import('@app/store/index.js');
71+
72+
await StateManager.getInstance().ready;
73+
await new Promise((resolve) => setTimeout(resolve, 500));
74+
vi.mocked(store.dispatch).mockClear();
75+
});
76+
77+
afterEach(async () => {
78+
const { StateManager } = await import('@app/store/watch/state-watch.js');
79+
80+
if (StateManager.instance) {
81+
await StateManager.instance.close();
82+
}
83+
84+
await rm(tempRoot, { recursive: true, force: true });
85+
});
86+
87+
it('reloads var state when emhttp writes var.ini.new into the state directory', async () => {
88+
const { store } = await import('@app/store/index.js');
89+
const { loadSingleStateFile } = await import('@app/store/modules/emhttp.js');
90+
const { loadRegistrationKey } = await import('@app/store/modules/registration.js');
91+
92+
await writeFile(join(statesDirectory, 'var.ini.new'), 'NAME="Gamer5"\n');
93+
94+
await vi.waitFor(() => {
95+
expect(store.dispatch).toHaveBeenNthCalledWith(1, loadSingleStateFile(StateFileKey.var));
96+
expect(store.dispatch).toHaveBeenNthCalledWith(2, loadRegistrationKey());
97+
});
98+
});
99+
100+
it('does not route disks.ini.new through the directory watcher', async () => {
101+
const { store } = await import('@app/store/index.js');
102+
103+
await writeFile(join(statesDirectory, 'disks.ini.new'), '[disk1]\nname=disk1\n');
104+
await new Promise((resolve) => setTimeout(resolve, 250));
105+
106+
expect(store.dispatch).not.toHaveBeenCalled();
107+
});
108+
});

api/src/__test__/store/watch/state-watch.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,16 @@ describe('StateManager', () => {
157157
const ignored = standardWatcher?.options.ignored;
158158

159159
expect(ignored).toBeTypeOf('function');
160+
expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state')).toBe(false);
160161
expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/README.txt')).toBe(true);
161162
expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/devs.ini')).toBe(false);
163+
expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/var.ini.new')).toBe(
164+
false
165+
);
162166
expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/disks.ini')).toBe(true);
167+
expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/disks.ini.new')).toBe(
168+
true
169+
);
163170
});
164171

165172
it('reloads registration key when var.ini is replaced after boot', async () => {
@@ -183,6 +190,27 @@ describe('StateManager', () => {
183190
expect(store.dispatch).toHaveBeenNthCalledWith(2, loadRegistrationKey());
184191
});
185192

193+
it('reloads registration key when var.ini.new is observed after boot', async () => {
194+
const { StateManager } = await import('@app/store/watch/state-watch.js');
195+
const { store } = await import('@app/store/index.js');
196+
const { loadSingleStateFile } = await import('@app/store/modules/emhttp.js');
197+
const { loadRegistrationKey } = await import('@app/store/modules/registration.js');
198+
199+
await StateManager.getInstance().ready;
200+
vi.mocked(store.dispatch).mockClear();
201+
202+
const standardWatcher = watchRegistrations.find(
203+
(registration) => registration.options.usePolling === false
204+
);
205+
const addHandler = standardWatcher?.handlers.add;
206+
expect(addHandler).toBeDefined();
207+
208+
await addHandler?.('/usr/local/emhttp/state/var.ini.new');
209+
210+
expect(store.dispatch).toHaveBeenNthCalledWith(1, loadSingleStateFile(StateFileKey.var));
211+
expect(store.dispatch).toHaveBeenNthCalledWith(2, loadRegistrationKey());
212+
});
213+
186214
it('routes polled state files through the polling directory watcher', async () => {
187215
const { StateManager } = await import('@app/store/watch/state-watch.js');
188216
const { store } = await import('@app/store/index.js');

api/src/store/watch/state-watch.ts

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,37 @@ const POLLED_STATE_KEYS = [StateFileKey.disks, StateFileKey.shares] as const;
1414
const POLLED_STATE_KEY_SET = new Set<StateFileKey>(POLLED_STATE_KEYS);
1515
const STATE_FILE_NAMES = new Set(Object.values(StateFileKey).map((key) => `${key}.ini`));
1616

17-
const shouldIgnoreStatePath = (path: string): boolean => {
17+
const getNormalizedStateFileName = (path: string): string | null => {
1818
const parsed = parse(path);
19-
const isStateFile = parsed.ext === '.ini' && STATE_FILE_NAMES.has(parsed.base);
19+
if (STATE_FILE_NAMES.has(parsed.base)) {
20+
return parsed.base;
21+
}
22+
23+
if (!parsed.base.endsWith('.new')) {
24+
return null;
25+
}
26+
27+
const originalStateFileName = parsed.base.slice(0, -'.new'.length);
28+
return STATE_FILE_NAMES.has(originalStateFileName) ? originalStateFileName : null;
29+
};
30+
31+
const shouldIgnoreStatePath = (path: string): boolean => {
32+
const normalizedStateFileName = getNormalizedStateFileName(path);
2033

21-
if (!isStateFile) {
34+
if (!normalizedStateFileName) {
2235
return true;
2336
}
2437

25-
const stateFileKey = StateFileKey[parsed.name];
38+
const stateFileKey = StateFileKey[normalizedStateFileName.slice(0, -'.ini'.length)];
2639
return POLLED_STATE_KEY_SET.has(stateFileKey);
2740
};
2841

29-
const chokidarOptionsForStateDirectory = (): ChokidarOptions => ({
42+
const chokidarOptionsForStateDirectory = (statesPath: string): ChokidarOptions => ({
3043
ignoreInitial: true,
3144
ignored: (path, stats) => {
45+
if (path === statesPath) {
46+
return false;
47+
}
3248
if (stats?.isDirectory()) {
3349
return false;
3450
}
@@ -54,9 +70,22 @@ export class StateManager {
5470
return StateManager.instance;
5571
}
5672

73+
public async close(): Promise<void> {
74+
await Promise.all(this.fileWatchers.map(async (watcher) => watcher.close()));
75+
this.fileWatchers.length = 0;
76+
77+
if (StateManager.instance === this) {
78+
StateManager.instance = null;
79+
}
80+
}
81+
5782
private getStateFileKeyFromPath(path: string): StateFileKey | undefined {
58-
const parsed = parse(path);
59-
return StateFileKey[parsed.name];
83+
const normalizedStateFileName = getNormalizedStateFileName(path);
84+
if (!normalizedStateFileName) {
85+
return undefined;
86+
}
87+
88+
return StateFileKey[normalizedStateFileName.slice(0, -'.ini'.length)];
6089
}
6190

6291
private async reloadStateFile(stateFile: StateFileKey, reason: 'add' | 'change' | 'startup-sync') {
@@ -96,7 +125,7 @@ export class StateManager {
96125
const { states } = getters.paths();
97126

98127
emhttpLogger.debug('Setting up watch for path: %s', states);
99-
const directoryWatch = watch(states, chokidarOptionsForStateDirectory());
128+
const directoryWatch = watch(states, chokidarOptionsForStateDirectory(states));
100129
directoryWatch.on('add', async (path) => this.handleStateFileUpdate(path, 'add'));
101130
directoryWatch.on('change', async (path) => this.handleStateFileUpdate(path, 'change'));
102131
this.fileWatchers.push(directoryWatch);

0 commit comments

Comments
 (0)