From 81b6b37d524140ed4c6762acce4f17d10c8d7384 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 17:27:50 -0400 Subject: [PATCH 01/15] refactor: add `ConfigFilePersister` primitive to unraid-shared --- packages/unraid-shared/package.json | 4 +- .../unraid-shared/src/services/config-file.ts | 165 ++++++++++++++++++ pnpm-lock.yaml | 7 + 3 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 packages/unraid-shared/src/services/config-file.ts diff --git a/packages/unraid-shared/package.json b/packages/unraid-shared/package.json index 387eabe0a6..14f1421b11 100644 --- a/packages/unraid-shared/package.json +++ b/packages/unraid-shared/package.json @@ -47,11 +47,13 @@ "@graphql-tools/utils": "10.8.6", "@jsonforms/core": "3.6.0", "@nestjs/common": "11.1.3", + "@nestjs/config": "4.0.2", "@nestjs/graphql": "13.1.0", "class-validator": "0.14.2", "graphql": "16.11.0", "graphql-scalars": "1.24.2", "lodash-es": "4.17.21", - "nest-authz": "2.17.0" + "nest-authz": "2.17.0", + "rxjs": "7.8.2" } } \ No newline at end of file diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts new file mode 100644 index 0000000000..ed83a5c4e8 --- /dev/null +++ b/packages/unraid-shared/src/services/config-file.ts @@ -0,0 +1,165 @@ +import { + Logger, + type OnModuleDestroy, + type OnModuleInit, +} from "@nestjs/common"; +import type { ConfigService } from "@nestjs/config"; +import path from "node:path"; + +import { isEqual } from "lodash-es"; +import { bufferTime } from "rxjs/operators"; +import { fileExists } from "../util/file.js"; +import { readFile, writeFile } from "node:fs/promises"; +import type { Subscription } from "rxjs"; + +export abstract class ConfigFilePersister + implements OnModuleInit, OnModuleDestroy +{ + constructor(protected readonly configService: ConfigService) { + this.logger = new Logger(`ConfigFilePersister:${this.fileName()}`); + } + + private readonly logger: Logger; + private configObserver?: Subscription; + + /** + * @returns The name of the config file. + */ + abstract fileName(): string; // defined as function so it can be used in constructor + + /** + * @returns The key of the config in the config service. + */ + abstract configKey(): string; + + /** + * @returns The default config object. + */ + abstract defaultConfig(): T; + + /** + * @returns Absolute path to the config file. + */ + configPath(): string { + // PATHS_CONFIG_MODULES is a required environment variable. + // It is the directory where custom config files are stored. + return path.join( + this.configService.getOrThrow("PATHS_CONFIG_MODULES"), + this.fileName() + ); + } + + async onModuleDestroy() { + this.configObserver?.unsubscribe(); + await this.persist(); + } + + async onModuleInit() { + this.logger.verbose(`Config path: ${this.configPath()}`); + await this.loadOrMigrateConfig(); + // Persist changes to the config. + this.configObserver = this.configService.changes$ + .pipe(bufferTime(25)) + .subscribe({ + next: async (changes) => { + const configChanged = changes.some(({ path }) => + path.startsWith(this.configKey()) + ); + if (configChanged) { + await this.persist(); + } + }, + error: (err) => { + this.logger.error("Error receiving config changes:", err); + }, + }); + } + + /** + * Persist the config to disk if the given data is different from the data on-disk. + * This helps preserve the boot flash drive's life by avoiding unnecessary writes. + * + * @param config - The config object to persist. + * @returns `true` if the config was persisted, `false` otherwise. + */ + async persist(config = this.configService.get(this.configKey())) { + try { + if (isEqual(config, await this.getConfigFromFile())) { + this.logger.verbose(`Config is unchanged, skipping persistence`); + return false; + } + } catch (error) { + this.logger.error(error, `Error loading config (will overwrite file)`); + } + const data = JSON.stringify(config, null, 2); + this.logger.verbose(`Persisting config to ${this.configPath()}: ${data}`); + try { + await writeFile(this.configPath(), data); + this.logger.verbose(`Config persisted to ${this.configPath()}`); + return true; + } catch (error) { + this.logger.error( + error, + `Error persisting config to '${this.configPath()}'` + ); + return false; + } + } + + /** + * Validate the config object. + * @param config - The config object to validate. + * @returns The validated config instance. + * @throws If the config is not valid. Defaults to passthrough. + */ + public async validate(config: object): Promise { + return config as T; + } + + /** + * 1. Loads and validates config from disk, or migrates if there is an error. + * 2. Merges loaded/migrated config with defaults, sets it in the config service, and persists it to disk. + * + * When unable to load or migrate the config (e.g. file system corruption, first load for a fresh config), + * messages are logged at WARN level and defaults are used/persisted. + * + * @returns true if the config was persisted to disk, false if migration failed or persistence was skipped/failed. + */ + private async loadOrMigrateConfig() { + const config = this.defaultConfig(); + try { + Object.assign(config, await this.getConfigFromFile()); + } catch (error) { + this.logger.warn(error, "Error loading config. Attempting to migrate..."); + try { + Object.assign(config, await this.migrateConfig()); + } catch (error) { + this.logger.warn("Migration failed. Using defaults in-memory.", error); + return false; + } + } + this.configService.set(this.configKey(), config); + return this.persist(config); + } + + /** + * Load the JSON config from the filesystem + * @throws {Error} - If the config file does not exist. + * @throws {Error} - If the config file is not parse-able. + * @throws {Error} - If the config file is not valid. + */ + async getConfigFromFile(configFilePath = this.configPath()): Promise { + if (!(await fileExists(configFilePath))) { + throw new Error(`Config file does not exist at '${configFilePath}'`); + } + return this.validate(JSON.parse(await readFile(configFilePath, "utf8"))); + } + + /** + * Migrate config file to a new format. + * @returns A config object in the new format. + */ + async migrateConfig(): Promise { + throw new Error("Not implemented"); + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 267f04a525..b031127909 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -707,6 +707,13 @@ importers: version: 5.8.3 packages/unraid-shared: + dependencies: + '@nestjs/config': + specifier: 4.0.2 + version: 4.0.2(@nestjs/common@11.1.3(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.1.14)(rxjs@7.8.2))(rxjs@7.8.2) + rxjs: + specifier: 7.8.2 + version: 7.8.2 devDependencies: '@graphql-tools/utils': specifier: 10.8.6 From d68bc268ec6ad92bb617ba243190d837a0e71a36 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 17:29:02 -0400 Subject: [PATCH 02/15] refactor(connect): config persister to use shared primitive --- .../src/config/config.persistence.ts | 126 ++++-------------- 1 file changed, 29 insertions(+), 97 deletions(-) diff --git a/packages/unraid-api-plugin-connect/src/config/config.persistence.ts b/packages/unraid-api-plugin-connect/src/config/config.persistence.ts index 2948769c5a..cb7d270161 100644 --- a/packages/unraid-api-plugin-connect/src/config/config.persistence.ts +++ b/packages/unraid-api-plugin-connect/src/config/config.persistence.ts @@ -1,82 +1,43 @@ -import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; +import { Injectable } from '@nestjs/common'; import { existsSync, readFileSync } from 'fs'; -import { writeFile } from 'fs/promises'; -import path from 'path'; +import { ConfigFilePersister } from '@unraid/shared/services/config-file.js'; import { plainToInstance } from 'class-transformer'; import { validateOrReject } from 'class-validator'; import { parse as parseIni } from 'ini'; -import { isEqual } from 'lodash-es'; -import { bufferTime } from 'rxjs/operators'; import type { MyServersConfig as LegacyConfig } from './my-servers.config.js'; -import { ConfigType, MyServersConfig } from './connect.config.js'; +import { emptyMyServersConfig, MyServersConfig } from './connect.config.js'; @Injectable() -export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { - constructor(private readonly configService: ConfigService) {} - - private logger = new Logger(ConnectConfigPersister.name); - get configPath() { - // PATHS_CONFIG_MODULES is a required environment variable. - // It is the directory where custom config files are stored. - return path.join(this.configService.getOrThrow('PATHS_CONFIG_MODULES'), 'connect.json'); - } - - async onModuleDestroy() { - await this.persist(); +export class ConnectConfigPersister extends ConfigFilePersister { + /** + * @override + * @returns The name of the config file. + */ + fileName(): string { + return 'connect.json'; } - async onModuleInit() { - this.logger.verbose(`Config path: ${this.configPath}`); - await this.loadOrMigrateConfig(); - // Persist changes to the config. - this.configService.changes$.pipe(bufferTime(25)).subscribe({ - next: async (changes) => { - const connectConfigChanged = changes.some(({ path }) => - path.startsWith('connect.config') - ); - if (connectConfigChanged) { - await this.persist(); - } - }, - error: (err) => { - this.logger.error('Error receiving config changes:', err); - }, - }); + /** + * @override + * @returns The key of the config in the config service. + */ + configKey(): string { + return 'connect.config'; } /** - * Persist the config to disk if the given data is different from the data on-disk. - * This helps preserve the boot flash drive's life by avoiding unnecessary writes. - * - * @param config - The config object to persist. - * @returns `true` if the config was persisted, `false` otherwise. + * @override + * @returns The default config object. */ - async persist(config = this.configService.get('connect.config')) { - try { - if (isEqual(config, await this.loadConfig())) { - this.logger.verbose(`Config is unchanged, skipping persistence`); - return false; - } - } catch (error) { - this.logger.error(error, `Error loading config (will overwrite file)`); - } - const data = JSON.stringify(config, null, 2); - this.logger.verbose(`Persisting config to ${this.configPath}: ${data}`); - try { - await writeFile(this.configPath, data); - this.logger.verbose(`Config persisted to ${this.configPath}`); - return true; - } catch (error) { - this.logger.error(error, `Error persisting config to '${this.configPath}'`); - return false; - } + defaultConfig(): MyServersConfig { + return emptyMyServersConfig(); } /** * Validate the config object. + * @override * @param config - The config object to validate. * @returns The validated config instance. */ @@ -94,44 +55,16 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { } /** - * Load the config from the filesystem, or migrate the legacy config file to the new config format. - * When unable to load or migrate the config, messages are logged at WARN level, but no other action is taken. - * @returns true if the config was loaded successfully, false otherwise. + * @override + * @returns The migrated config object. */ - private async loadOrMigrateConfig() { - try { - const config = await this.loadConfig(); - this.configService.set('connect.config', config); - this.logger.verbose(`Config loaded from ${this.configPath}`); - return true; - } catch (error) { - this.logger.warn(error, 'Error loading config'); - } - - try { - await this.migrateLegacyConfig(); - return this.persist(); - } catch (error) { - this.logger.warn('Error migrating legacy config:', error); - } - - this.logger.error( - 'Failed to load or migrate config from filesystem. Config is not persisted. Using defaults in-memory.' - ); - return false; + async migrateConfig(): Promise { + return await this.migrateLegacyConfig(); } - /** - * Load the JSON config from the filesystem - * @throws {Error} - If the config file does not exist. - * @throws {Error} - If the config file is not parse-able. - * @throws {Error} - If the config file is not valid. - */ - private async loadConfig(configFilePath = this.configPath) { - if (!existsSync(configFilePath)) - throw new Error(`Config file does not exist at '${configFilePath}'`); - return this.validate(JSON.parse(readFileSync(configFilePath, 'utf8'))); - } + /**----------------------------------------------------- + * Helpers for migrating myservers.cfg to connect.json + *------------------------------------------------------**/ /** * Migrate the legacy config file to the new config format. @@ -143,8 +76,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { private async migrateLegacyConfig(filePath?: string) { const myServersCfgFile = await this.readLegacyConfig(filePath); const legacyConfig = this.parseLegacyConfig(myServersCfgFile); - const newConfig = await this.convertLegacyConfig(legacyConfig); - this.configService.set('connect.config', newConfig); + return await this.convertLegacyConfig(legacyConfig); } /** From 1550dfd3a13205ae918bdc4962ffe7d66cb5eda9 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 17:36:31 -0400 Subject: [PATCH 03/15] refactor: api-config persistence --- .../unraid-api/config/api-config.module.ts | 73 +++++-------------- api/src/unraid-api/config/api-config.test.ts | 47 +++++++++++- 2 files changed, 61 insertions(+), 59 deletions(-) diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index e456ace466..739c4cd1f8 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -1,10 +1,9 @@ import { Injectable, Logger, Module } from '@nestjs/common'; -import { ConfigService, registerAs } from '@nestjs/config'; +import { registerAs } from '@nestjs/config'; import type { ApiConfig } from '@unraid/shared/services/api-config.js'; +import { ConfigFilePersister } from '@unraid/shared/services/config-file.js'; import { csvStringToArray } from '@unraid/shared/util/data.js'; -import { fileExists } from '@unraid/shared/util/file.js'; -import { bufferTime } from 'rxjs/operators'; import { API_VERSION } from '@app/environment.js'; import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; @@ -91,54 +90,26 @@ export const loadApiConfig = async () => { export const apiConfig = registerAs('api', loadApiConfig); @Injectable() -export class ApiConfigPersistence { - private configModel: ApiStateConfig; - private logger = new Logger(ApiConfigPersistence.name); - get filePath() { - return this.configModel.filePath; +export class ApiConfigPersistence extends ConfigFilePersister { + fileName(): string { + return 'api.json'; } - get config() { - return this.configService.getOrThrow('api'); + + configKey(): string { + return 'api'; } - constructor( - private readonly configService: ConfigService, - private readonly persistenceHelper: ConfigPersistenceHelper - ) { - this.configModel = new ApiStateConfig( - { - name: 'api', - defaultConfig: createDefaultConfig(), - parse: (data) => data as ApiConfig, - }, - this.persistenceHelper - ); + defaultConfig(): ApiConfig { + return createDefaultConfig(); } - async onModuleInit() { - try { - if (!(await fileExists(this.filePath))) { - this.migrateFromMyServersConfig(); - } - await this.persistenceHelper.persistIfChanged(this.filePath, this.config); - this.configService.changes$.pipe(bufferTime(25)).subscribe({ - next: async (changes) => { - if (changes.some((change) => change.path.startsWith('api'))) { - this.logger.verbose(`API Config changed ${JSON.stringify(changes)}`); - try { - await this.persistenceHelper.persistIfChanged(this.filePath, this.config); - } catch (persistError) { - this.logger.error('Error persisting config changes:', persistError); - } - } - }, - error: (err) => { - this.logger.error('Error receiving config changes:', err); - }, - }); - } catch (error) { - this.logger.error('Error during API config module initialization:', error); - } + async migrateConfig(): Promise { + const legacyConfig = this.configService.get('store.config', {}); + const migrated = this.convertLegacyConfig(legacyConfig); + return { + ...this.defaultConfig(), + ...migrated, + }; } convertLegacyConfig( @@ -156,18 +127,10 @@ export class ApiConfigPersistence { ssoSubIds: csvStringToArray(config?.remote?.ssoSubIds ?? ''), }; } - - migrateFromMyServersConfig() { - const legacyConfig = this.configService.get('store.config', {}); - const { sandbox, extraOrigins, ssoSubIds } = this.convertLegacyConfig(legacyConfig); - this.configService.set('api.sandbox', sandbox); - this.configService.set('api.extraOrigins', extraOrigins); - this.configService.set('api.ssoSubIds', ssoSubIds); - } } // apiConfig should be registered in root config in app.module.ts, not here. @Module({ - providers: [ApiConfigPersistence, ConfigPersistenceHelper], + providers: [ApiConfigPersistence], }) export class ApiConfigModule {} diff --git a/api/src/unraid-api/config/api-config.test.ts b/api/src/unraid-api/config/api-config.test.ts index 2638856e0d..17ebe4ebdf 100644 --- a/api/src/unraid-api/config/api-config.test.ts +++ b/api/src/unraid-api/config/api-config.test.ts @@ -4,7 +4,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { fileExists } from '@app/core/utils/files/file-exists.js'; import { ApiConfigPersistence, loadApiConfig } from '@app/unraid-api/config/api-config.module.js'; -import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; // Mock the core file-exists utility used by ApiStateConfig vi.mock('@app/core/utils/files/file-exists.js', () => ({ @@ -25,16 +24,56 @@ vi.mock('fs/promises', () => ({ describe('ApiConfigPersistence', () => { let service: ApiConfigPersistence; let configService: ConfigService; - let persistenceHelper: ConfigPersistenceHelper; beforeEach(() => { configService = { get: vi.fn(), set: vi.fn(), + getOrThrow: vi.fn().mockReturnValue('test-config-path'), } as any; - persistenceHelper = {} as ConfigPersistenceHelper; - service = new ApiConfigPersistence(configService, persistenceHelper); + service = new ApiConfigPersistence(configService); + }); + + describe('required ConfigFilePersister methods', () => { + it('should return correct file name', () => { + expect(service.fileName()).toBe('api.json'); + }); + + it('should return correct config key', () => { + expect(service.configKey()).toBe('api'); + }); + + it('should return default config', () => { + const defaultConfig = service.defaultConfig(); + expect(defaultConfig).toEqual({ + version: expect.any(String), + extraOrigins: [], + sandbox: false, + ssoSubIds: [], + plugins: [], + }); + }); + + it('should migrate config from legacy format', async () => { + const mockLegacyConfig = { + local: { sandbox: 'yes' }, + api: { extraOrigins: 'https://example.com,https://test.com' }, + remote: { ssoSubIds: 'sub1,sub2' }, + }; + + vi.mocked(configService.get).mockReturnValue(mockLegacyConfig); + + const result = await service.migrateConfig(); + + expect(result).toEqual({ + version: expect.any(String), + extraOrigins: ['https://example.com', 'https://test.com'], + sandbox: true, + ssoSubIds: ['sub1', 'sub2'], + plugins: [], + }); + }); }); describe('convertLegacyConfig', () => { From dce7d00923c5264eae09af0f9b5268e43f848426 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 17:37:03 -0400 Subject: [PATCH 04/15] refactor: config persistence in api plugin generator --- .../src/templates/config.persistence.ts | 83 +++---------------- 1 file changed, 11 insertions(+), 72 deletions(-) diff --git a/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts b/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts index 1fef4d4423..6e4a895ffb 100644 --- a/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts +++ b/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts @@ -1,81 +1,20 @@ -import { Logger, Injectable, OnModuleInit } from "@nestjs/common"; -import { ConfigService } from "@nestjs/config"; -import { existsSync, readFileSync } from "fs"; -import { writeFile } from "fs/promises"; -import path from "path"; -import { bufferTime } from "rxjs/operators"; +import { Injectable } from "@nestjs/common"; +import { ConfigFilePersister } from "@unraid/shared/services/config-file.js"; // npm install @unraid/shared import { PluginNameConfig } from "./config.entity.js"; @Injectable() -export class PluginNameConfigPersister implements OnModuleInit { - constructor(private readonly configService: ConfigService) {} - - private logger = new Logger(PluginNameConfigPersister.name); - - /** the file path to the config file for this plugin */ - get configPath() { - return path.join( - this.configService.get("PATHS_CONFIG_MODULES")!, - "plugin-name.json" // Use kebab-case for the filename - ); +export class PluginNameConfigPersister extends ConfigFilePersister { + fileName(): string { + return "plugin-name.json"; // Use kebab-case for the filename } - onModuleInit() { - this.logger.debug(`Config path: ${this.configPath}`); - // Load the config from the file if it exists, otherwise initialize it with defaults. - if (existsSync(this.configPath)) { - try { - const configFromFile = JSON.parse( - readFileSync(this.configPath, "utf8") - ); - this.configService.set("plugin-name", configFromFile); - this.logger.verbose(`Config loaded from ${this.configPath}`); - } catch (error) { - this.logger.error( - `Error reading or parsing config file at ${this.configPath}. Using defaults.`, - error - ); - // If loading fails, ensure default config is set and persisted - this.persist(); - } - } else { - this.logger.log( - `Config file ${this.configPath} does not exist. Writing default config...` - ); - // Persist the default configuration provided by configFeature - this.persist(); - } - - // Automatically persist changes to the config file after a short delay. - this.configService.changes$.pipe(bufferTime(25)).subscribe({ - next: async (changes) => { - const pluginNameConfigChanged = changes.some(({ path }) => - path.startsWith("plugin-name.") - ); - if (pluginNameConfigChanged) { - this.logger.verbose("Plugin config changed"); - await this.persist(); - } - }, - error: (err) => { - this.logger.error("Error receiving config changes:", err); - }, - }); + configKey(): string { + return "plugin-name"; } - async persist( - config = this.configService.get("plugin-name") - ) { - const data = JSON.stringify(config, null, 2); - this.logger.verbose(`Persisting config to ${this.configPath}: ${data}`); - try { - await writeFile(this.configPath, data); - this.logger.verbose(`Config change persisted to ${this.configPath}`); - } catch (error) { - this.logger.error( - `Error persisting config to '${this.configPath}':`, - error - ); - } + defaultConfig(): PluginNameConfig { + // Return the default configuration for your plugin + // This should match the structure defined in your config.entity.ts + return {} as PluginNameConfig; } } From 02e16b89b479d389394d90125e6badc187c1b213 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 17:54:41 -0400 Subject: [PATCH 05/15] test: ConfigFilePersister --- .../services/__tests__/config-file.test.ts | 450 ++++++++++++++++++ .../unraid-shared/src/services/config-file.ts | 6 +- 2 files changed, 455 insertions(+), 1 deletion(-) create mode 100644 packages/unraid-shared/src/services/__tests__/config-file.test.ts diff --git a/packages/unraid-shared/src/services/__tests__/config-file.test.ts b/packages/unraid-shared/src/services/__tests__/config-file.test.ts new file mode 100644 index 0000000000..751b194c83 --- /dev/null +++ b/packages/unraid-shared/src/services/__tests__/config-file.test.ts @@ -0,0 +1,450 @@ +import { expect, test, describe, beforeEach, afterEach } from "bun:test"; +import { Subject } from "rxjs"; +import { readFile, writeFile, mkdir, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { ConfigFilePersister } from "../config-file.js"; + +interface TestConfig { + name: string; + version: number; + enabled: boolean; + settings: { + timeout: number; + retries: number; + }; +} + +class TestConfigFilePersister extends ConfigFilePersister { + constructor(configService: any) { + super(configService); + } + + fileName(): string { + return "test-config.json"; + } + + configKey(): string { + return "testConfig"; + } + + defaultConfig(): TestConfig { + return { + name: "test", + version: 1, + enabled: false, + settings: { + timeout: 5000, + retries: 3, + }, + }; + } + + async validate(config: object): Promise { + const testConfig = config as TestConfig; + if (testConfig.version < 1) { + throw new Error("Invalid version: must be >= 1"); + } + if (testConfig.settings.timeout < 1000) { + throw new Error("Invalid timeout: must be >= 1000"); + } + return testConfig; + } + + async migrateConfig(): Promise { + return { + name: "migrated", + version: 2, + enabled: true, + settings: { + timeout: 3000, + retries: 5, + }, + }; + } +} + +describe("ConfigFilePersister Integration Tests", () => { + let configService: any; + let persister: TestConfigFilePersister; + let testDir: string; + let configPath: string; + let changesSubject: Subject; + let configStore: Record; + + beforeEach(async () => { + // Setup test directory + testDir = join(tmpdir(), `config-test-${Date.now()}`); + await mkdir(testDir, { recursive: true }); + configPath = join(testDir, "test-config.json"); + + // Setup config store + configStore = {}; + + // Setup rxjs subject for config changes + changesSubject = new Subject(); + + // Mock ConfigService + configService = { + get: (key: string) => configStore[key], + set: (key: string, value: any) => { + configStore[key] = value; + }, + getOrThrow: (key: string) => { + if (key === "PATHS_CONFIG_MODULES") return testDir; + throw new Error(`Config key ${key} not found`); + }, + changes$: changesSubject.asObservable(), + }; + + persister = new TestConfigFilePersister(configService); + }); + + afterEach(async () => { + // Proper cleanup + changesSubject.complete(); + await persister.onModuleDestroy?.(); + await rm(testDir, { recursive: true, force: true }); + }); + + test("configPath returns correct path", () => { + expect(persister.configPath()).toBe(configPath); + }); + + test("loads existing config from file", async () => { + const existingConfig = { + name: "existing", + version: 2, + enabled: true, + settings: { + timeout: 3000, + retries: 5, + }, + }; + await writeFile(configPath, JSON.stringify(existingConfig, null, 2)); + + await persister.onModuleInit(); + + // Should load existing config + expect(configStore.testConfig).toEqual(existingConfig); + }); + + test("handles invalid config by attempting migration", async () => { + const invalidConfig = { + name: "invalid", + version: 0, // Invalid version + enabled: true, + settings: { + timeout: 500, // Invalid timeout + retries: 5, + }, + }; + await writeFile(configPath, JSON.stringify(invalidConfig, null, 2)); + + await persister.onModuleInit(); + + // Should call migrate and set migrated config + expect(configStore.testConfig).toEqual({ + name: "migrated", + version: 2, + enabled: true, + settings: { + timeout: 3000, + retries: 5, + }, + }); + }); + + test("persists config to file", async () => { + const config = { + name: "persist-test", + version: 2, + enabled: true, + settings: { + timeout: 4000, + retries: 4, + }, + }; + + const result = await persister.persist(config); + + expect(result).toBe(true); + const fileContent = await readFile(configPath, "utf8"); + const parsedConfig = JSON.parse(fileContent); + expect(parsedConfig).toEqual(config); + }); + + test("skips persistence when config is unchanged", async () => { + const config = { + name: "unchanged", + version: 1, + enabled: false, + settings: { + timeout: 5000, + retries: 3, + }, + }; + + // Write initial config + await writeFile(configPath, JSON.stringify(config, null, 2)); + + const result = await persister.persist(config); + + expect(result).toBe(false); + }); + + test("loads and validates config from file", async () => { + const config = { + name: "file-test", + version: 3, + enabled: true, + settings: { + timeout: 2000, + retries: 1, + }, + }; + await writeFile(configPath, JSON.stringify(config)); + + const result = await persister.getConfigFromFile(); + + expect(result).toEqual(config); + }); + + test("throws error when file doesn't exist", async () => { + await expect(persister.getConfigFromFile()).rejects.toThrow( + "Config file does not exist" + ); + }); + + test("throws error when file contains invalid JSON", async () => { + await writeFile(configPath, "{ invalid json"); + + await expect(persister.getConfigFromFile()).rejects.toThrow(); + }); + + test("throws error when config is invalid", async () => { + const invalidConfig = { + name: "invalid", + version: -1, + enabled: true, + settings: { + timeout: 100, + retries: 1, + }, + }; + await writeFile(configPath, JSON.stringify(invalidConfig)); + + await expect(persister.getConfigFromFile()).rejects.toThrow( + "Invalid version" + ); + }); + + test("base class migration throws not implemented error", async () => { + const basePersister = new class extends ConfigFilePersister { + fileName() { return "base-test.json"; } + configKey() { return "baseTest"; } + defaultConfig() { return persister.defaultConfig(); } + }(configService); + + await expect(basePersister.migrateConfig()).rejects.toThrow( + "Not implemented" + ); + }); + + test("unsubscribes from config changes and persists final state", async () => { + await persister.onModuleInit(); + + // Setup final config state + configStore["testConfig"] = { + name: "final", + version: 4, + enabled: false, + settings: { + timeout: 1000, + retries: 10, + }, + }; + + await persister.onModuleDestroy(); + + // Should persist final state + const fileContent = await readFile(configPath, "utf8"); + const parsedConfig = JSON.parse(fileContent); + expect(parsedConfig.name).toBe("final"); + }); + + test("handles destroy when not initialized", async () => { + // Should not throw error + await expect(persister.onModuleDestroy()).resolves.toBeUndefined(); + }); + + test("config change subscription is properly set up", async () => { + // Pre-create config file to avoid migration + const initialConfig = persister.defaultConfig(); + await writeFile(configPath, JSON.stringify(initialConfig, null, 2)); + + await persister.onModuleInit(); + + // Verify that the config observer is active by checking internal state + // This tests that the subscription was created without relying on timing + expect((persister as any).configObserver).toBeDefined(); + expect((persister as any).configObserver.closed).toBe(false); + + // Test that non-matching changes are ignored (synchronous test) + configStore["testConfig"] = persister.defaultConfig(); + const initialFileContent = await readFile(configPath, "utf8"); + + // Emit a non-matching config change + changesSubject.next([{ path: "otherConfig.setting" }]); + + // Wait briefly to ensure no processing occurs + await new Promise(resolve => setTimeout(resolve, 30)); + + // File should remain unchanged + const afterFileContent = await readFile(configPath, "utf8"); + expect(afterFileContent).toBe(initialFileContent); + }); + + test("ignores non-matching config changes", async () => { + // Pre-create config file + const initialConfig = persister.defaultConfig(); + await writeFile(configPath, JSON.stringify(initialConfig, null, 2)); + + await persister.onModuleInit(); + + // Set initial config and write to file + configStore["testConfig"] = persister.defaultConfig(); + + // Get initial modification time + const stats1 = await import('fs/promises').then(fs => fs.stat(configPath)); + + // Wait a bit to ensure timestamp difference + await new Promise(resolve => setTimeout(resolve, 10)); + + // Emit change for different config key + changesSubject.next([{ path: "otherConfig.setting" }]); + + // Wait for buffer time + await new Promise(resolve => setTimeout(resolve, 50)); + + // File should remain unchanged (same modification time) + const stats2 = await import('fs/promises').then(fs => fs.stat(configPath)); + expect(stats2.mtime).toEqual(stats1.mtime); + }); + + test("handles config service errors gracefully", async () => { + // Mock config service to throw error on get + const errorConfigService = { + ...configService, + get: () => { + throw new Error("Config service error"); + } + }; + + const errorPersister = new TestConfigFilePersister(errorConfigService); + + // Should still initialize (migration will be called due to no file) + await errorPersister.onModuleInit(); + + // Should have migrated config since get failed + const expectedMigrated = await errorPersister.migrateConfig(); + expect(configStore.testConfig).toEqual(expectedMigrated); + }); + + test("handles persistence errors gracefully", async () => { + await persister.onModuleInit(); + + // Create a persister that points to invalid directory + const invalidPersister = new TestConfigFilePersister({ + ...configService, + getOrThrow: (key: string) => { + if (key === "PATHS_CONFIG_MODULES") return "/invalid/path/that/does/not/exist"; + throw new Error(`Config key ${key} not found`); + }, + }); + + const config = { ...persister.defaultConfig(), name: "error-test" }; + + // Should not throw despite write error + const result = await invalidPersister.persist(config); + expect(result).toBe(false); + }); + + test("migration priority over defaults when file doesn't exist", async () => { + // No file exists, should trigger migration path + await persister.onModuleInit(); + + // ConfigFilePersister prioritizes migration over defaults when file doesn't exist + expect(configStore.testConfig).toEqual({ + name: "migrated", + version: 2, + enabled: true, + settings: { + timeout: 3000, + retries: 5, + }, + }); + + // Should persist migrated config to file + const fileContent = await readFile(configPath, "utf8"); + const parsedConfig = JSON.parse(fileContent); + expect(parsedConfig).toEqual({ + name: "migrated", + version: 2, + enabled: true, + settings: { + timeout: 3000, + retries: 5, + }, + }); + }); + + test("full lifecycle integration", async () => { + // Initialize - will use migration since no file exists + await persister.onModuleInit(); + + // Verify initial state (migrated, not defaults) + expect(configStore.testConfig).toEqual({ + name: "migrated", + version: 2, + enabled: true, + settings: { + timeout: 3000, + retries: 5, + }, + }); + + // Simulate config change + configStore["testConfig"] = { + name: "lifecycle-test", + version: 5, + enabled: true, + settings: { + timeout: 1500, + retries: 7, + }, + }; + + // Trigger change notification + changesSubject.next([{ path: "testConfig.enabled" }]); + + // Wait for persistence + await new Promise(resolve => setTimeout(resolve, 50)); + + // Cleanup + await persister.onModuleDestroy(); + + // Verify final persisted state + const fileContent = await readFile(configPath, "utf8"); + const parsedConfig = JSON.parse(fileContent); + expect(parsedConfig).toEqual({ + name: "lifecycle-test", + version: 5, + enabled: true, + settings: { + timeout: 1500, + retries: 7, + }, + }); + }); +}); \ No newline at end of file diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index ed83a5c4e8..42792ea0e7 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -63,7 +63,7 @@ export abstract class ConfigFilePersister .subscribe({ next: async (changes) => { const configChanged = changes.some(({ path }) => - path.startsWith(this.configKey()) + path?.startsWith(this.configKey()) ); if (configChanged) { await this.persist(); @@ -83,6 +83,10 @@ export abstract class ConfigFilePersister * @returns `true` if the config was persisted, `false` otherwise. */ async persist(config = this.configService.get(this.configKey())) { + if (!config) { + this.logger.warn(`Cannot persist undefined config`); + return false; + } try { if (isEqual(config, await this.getConfigFromFile())) { this.logger.verbose(`Config is unchanged, skipping persistence`); From 4858e20468ed2ad232ec7160c92a12a6140ed8bd Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 17:59:54 -0400 Subject: [PATCH 06/15] document the ConfigFilePersister --- .../unraid-shared/src/services/config-file.ts | 257 ++++++++++++++++-- 1 file changed, 234 insertions(+), 23 deletions(-) diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index 42792ea0e7..6f3019e96e 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -12,6 +12,43 @@ import { fileExists } from "../util/file.js"; import { readFile, writeFile } from "node:fs/promises"; import type { Subscription } from "rxjs"; +/** + * Abstract base class for persisting configuration objects to JSON files on disk. + * + * This class provides a robust configuration persistence layer with the following features: + * - **Migration Priority**: When files don't exist, migration is attempted before falling back to defaults + * - **Change Detection**: Uses deep equality checks to avoid unnecessary disk writes (flash drive optimization) + * - **Reactive Updates**: Subscribes to config changes with 25ms buffering to reduce I/O operations + * - **Error Resilience**: Graceful handling of file system errors, JSON parsing failures, and validation errors + * - **Lifecycle Management**: Proper cleanup of subscriptions and final persistence on module destruction + * + * @template T The configuration object type that extends object + * + * @example + * ```typescript + * interface MyConfig { + * enabled: boolean; + * timeout: number; + * } + * + * class MyConfigPersister extends ConfigFilePersister { + * fileName() { return "my-config.json"; } + * configKey() { return "myConfig"; } + * defaultConfig() { return { enabled: false, timeout: 5000 }; } + * + * async validate(config: object): Promise { + * const myConfig = config as MyConfig; + * if (myConfig.timeout < 1000) throw new Error("Timeout too low"); + * return myConfig; + * } + * + * async migrateConfig(): Promise { + * // Custom migration logic for legacy configs + * return { enabled: true, timeout: 3000 }; + * } + * } + * ``` + */ export abstract class ConfigFilePersister implements OnModuleInit, OnModuleDestroy { @@ -23,22 +60,54 @@ export abstract class ConfigFilePersister private configObserver?: Subscription; /** - * @returns The name of the config file. + * Returns the filename for the configuration file. + * + * @returns The name of the config file (e.g., "my-config.json") + * @example "user-preferences.json" */ abstract fileName(): string; // defined as function so it can be used in constructor /** - * @returns The key of the config in the config service. + * Returns the configuration key used in the ConfigService. + * + * This key is used to: + * - Store/retrieve config from the ConfigService + * - Filter config change events to only process relevant changes + * + * @returns The config key string (e.g., "userPreferences") + * @example "myModuleConfig" */ abstract configKey(): string; /** - * @returns The default config object. + * Returns the default configuration object. + * + * **Important**: This is used as a fallback when migration fails or as a base + * for merging with loaded/migrated configurations. + * + * @returns The default configuration object + * @example + * ```typescript + * defaultConfig(): MyConfig { + * return { + * enabled: false, + * timeout: 5000, + * retries: 3 + * }; + * } + * ``` */ abstract defaultConfig(): T; /** - * @returns Absolute path to the config file. + * Returns the absolute path to the configuration file. + * + * Combines the `PATHS_CONFIG_MODULES` environment variable with the filename + * to create the full path where the config file should be stored. + * + * @returns Absolute path to the config file + * @throws Error if `PATHS_CONFIG_MODULES` environment variable is not set + * @example "/usr/local/etc/unraid/config/my-config.json" */ configPath(): string { // PATHS_CONFIG_MODULES is a required environment variable. @@ -49,11 +118,31 @@ export abstract class ConfigFilePersister ); } + /** + * NestJS lifecycle hook called when the module is being destroyed. + * + * Performs cleanup by: + * 1. Unsubscribing from config change notifications + * 2. Persisting any final configuration state to disk + * + * This ensures no data loss and prevents memory leaks. + */ async onModuleDestroy() { this.configObserver?.unsubscribe(); await this.persist(); } + /** + * NestJS lifecycle hook called when the module is initialized. + * + * Performs initialization by: + * 1. Loading existing config from disk or attempting migration + * 2. Setting up reactive config change subscription with 25ms buffering + * 3. Filtering changes to only process events matching this config's key + * + * **Key Behavior**: Migration is prioritized over defaults when files don't exist. + * The 25ms buffer reduces disk I/O by batching rapid config changes. + */ async onModuleInit() { this.logger.verbose(`Config path: ${this.configPath()}`); await this.loadOrMigrateConfig(); @@ -76,11 +165,29 @@ export abstract class ConfigFilePersister } /** - * Persist the config to disk if the given data is different from the data on-disk. - * This helps preserve the boot flash drive's life by avoiding unnecessary writes. + * Persists the configuration to disk with intelligent change detection. * - * @param config - The config object to persist. - * @returns `true` if the config was persisted, `false` otherwise. + * **Flash Drive Optimization**: Uses deep equality checks to avoid unnecessary writes, + * helping preserve the lifespan of boot flash drives by preventing redundant I/O operations. + * + * **Process**: + * 1. Validates that config is not undefined + * 2. Compares new config with existing file content using deep equality + * 3. Skips write if content is identical + * 4. Writes pretty-printed JSON (2-space indentation) if changes detected + * 5. Handles file system errors gracefully + * + * @param config - The config object to persist (defaults to current config from service) + * @returns `true` if the config was persisted to disk, `false` if skipped or failed + * + * @example + * ```typescript + * // Persist current config + * const persisted = await persister.persist(); + * + * // Persist specific config + * const persisted = await persister.persist(myConfig); + * ``` */ async persist(config = this.configService.get(this.configKey())) { if (!config) { @@ -111,23 +218,61 @@ export abstract class ConfigFilePersister } /** - * Validate the config object. - * @param config - The config object to validate. - * @returns The validated config instance. - * @throws If the config is not valid. Defaults to passthrough. + * Validates and transforms a configuration object. + * + * **Default Implementation**: Simple type cast (passthrough validation). + * Override this method to implement custom validation logic. + * + * **Common Use Cases**: + * - Schema validation (e.g., using Joi, Yup, or Zod) + * - Range checking for numeric values + * - Required field validation + * - Data transformation/normalization + * + * @param config - The raw config object to validate + * @returns The validated and potentially transformed config + * @throws Error if the config is invalid (stops loading/migration process) + * + * @example + * ```typescript + * async validate(config: object): Promise { + * const myConfig = config as MyConfig; + * + * if (typeof myConfig.timeout !== 'number' || myConfig.timeout < 1000) { + * throw new Error('Invalid timeout: must be number >= 1000'); + * } + * + * if (!['low', 'medium', 'high'].includes(myConfig.priority)) { + * throw new Error('Invalid priority level'); + * } + * + * return myConfig; + * } + * ``` */ public async validate(config: object): Promise { return config as T; } /** - * 1. Loads and validates config from disk, or migrates if there is an error. - * 2. Merges loaded/migrated config with defaults, sets it in the config service, and persists it to disk. + * Core initialization logic that loads, migrates, or creates configuration. + * + * **Migration Priority Strategy**: + * 1. **Load**: Attempts to load and validate existing config from disk + * 2. **Migrate**: If loading fails, attempts migration using `migrateConfig()` + * 3. **Default**: If migration fails, falls back to `defaultConfig()` + * 4. **Merge**: Merges result with defaults to ensure all properties exist + * 5. **Store**: Sets final config in ConfigService + * 6. **Persist**: Writes final config to disk + * + * **Key Insight**: Migration is attempted before using defaults, making this suitable + * for upgrading legacy configurations or handling first-time installations. * - * When unable to load or migrate the config (e.g. file system corruption, first load for a fresh config), - * messages are logged at WARN level and defaults are used/persisted. + * **Error Handling**: All errors are logged at WARN level, ensuring the system + * continues to function with sensible defaults even if file system issues occur. * - * @returns true if the config was persisted to disk, false if migration failed or persistence was skipped/failed. + * @returns `true` if config was successfully persisted, `false` if persistence failed + * @private */ private async loadOrMigrateConfig() { const config = this.defaultConfig(); @@ -147,10 +292,32 @@ export abstract class ConfigFilePersister } /** - * Load the JSON config from the filesystem - * @throws {Error} - If the config file does not exist. - * @throws {Error} - If the config file is not parse-able. - * @throws {Error} - If the config file is not valid. + * Loads and validates configuration from a JSON file. + * + * **Process**: + * 1. Checks if file exists using `fileExists()` utility + * 2. Reads file content as UTF-8 text + * 3. Parses JSON content + * 4. Validates result using `validate()` method + * + * **Error Cases**: + * - File doesn't exist → Error (triggers migration in caller) + * - Invalid JSON syntax → Error (triggers migration in caller) + * - Validation failure → Error (triggers migration in caller) + * + * @param configFilePath - Path to config file (defaults to `configPath()`) + * @returns Validated configuration object + * @throws Error if file doesn't exist, contains invalid JSON, or fails validation + * + * @example + * ```typescript + * try { + * const config = await persister.getConfigFromFile(); + * console.log('Loaded config:', config); + * } catch (error) { + * console.log('Config loading failed, will attempt migration'); + * } + * ``` */ async getConfigFromFile(configFilePath = this.configPath()): Promise { if (!(await fileExists(configFilePath))) { @@ -160,8 +327,52 @@ export abstract class ConfigFilePersister } /** - * Migrate config file to a new format. - * @returns A config object in the new format. + * Migrates legacy or corrupted configuration to the current format. + * + * **Default Implementation**: Throws "Not implemented" error. + * Override this method to provide custom migration logic. + * + * **When Called**: + * - Config file doesn't exist (first-time setup) + * - Config file contains invalid JSON + * - Config validation fails + * - File system read errors + * + * **Migration Strategies**: + * - **Legacy Format**: Convert old config structure to new format + * - **Partial Config**: Fill missing properties with sensible defaults + * - **Corrupted Data**: Attempt to salvage usable parts + * - **Fresh Install**: Return initial setup configuration + * + * **Priority**: Migration is attempted before falling back to `defaultConfig()`, + * making this ideal for handling upgrades and first-time installations. + * + * @returns Migrated configuration object + * @throws Error if migration is not possible (falls back to defaults) + * + * @example + * ```typescript + * async migrateConfig(): Promise { + * // Try to load legacy config format + * try { + * const legacyPath = path.join(this.configDir, 'old-config.ini'); + * const legacyData = await readLegacyConfig(legacyPath); + * + * return { + * enabled: legacyData.isEnabled ?? true, + * timeout: legacyData.timeoutMs ?? 5000, + * newFeature: 'default-value' // New property not in legacy + * }; + * } catch (error) { + * // First-time installation + * return { + * enabled: true, + * timeout: 3000, + * newFeature: 'initial-setup' + * }; + * } + * } + * ``` */ async migrateConfig(): Promise { throw new Error("Not implemented"); From d57d9e582a83e5b6d45f3a2aec9d531ae217c015 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 21 Jul 2025 18:34:49 -0400 Subject: [PATCH 07/15] rm obsoleted code --- api/src/__test__/config/api-config.test.ts | 137 ------- .../unraid-api/config/api-config.module.ts | 84 ++-- api/src/unraid-api/config/api-config.test.ts | 101 +---- .../config/factory/api-state.model.test.ts | 364 ------------------ .../config/factory/api-state.model.ts | 122 ------ .../config/factory/api-state.register.ts | 54 --- .../config/factory/api-state.service.ts | 82 ---- .../config/factory/config.injection.ts | 22 -- .../config/factory/config.interface.ts | 15 - .../unraid-api/config/persistence.helper.ts | 70 ---- .../plugin/plugin-management.service.ts | 7 +- api/src/unraid-api/plugin/plugin.module.ts | 5 +- .../downloaded/.login.php.last-download-time | 2 +- .../DefaultPageLayout.php.last-download-time | 2 +- .../Notifications.page.last-download-time | 2 +- .../auth-request.php.last-download-time | 2 +- .../downloaded/rc.nginx.last-download-time | 2 +- 17 files changed, 40 insertions(+), 1033 deletions(-) delete mode 100644 api/src/__test__/config/api-config.test.ts delete mode 100644 api/src/unraid-api/config/factory/api-state.model.test.ts delete mode 100644 api/src/unraid-api/config/factory/api-state.model.ts delete mode 100644 api/src/unraid-api/config/factory/api-state.register.ts delete mode 100644 api/src/unraid-api/config/factory/api-state.service.ts delete mode 100644 api/src/unraid-api/config/factory/config.injection.ts delete mode 100644 api/src/unraid-api/config/factory/config.interface.ts delete mode 100644 api/src/unraid-api/config/persistence.helper.ts diff --git a/api/src/__test__/config/api-config.test.ts b/api/src/__test__/config/api-config.test.ts deleted file mode 100644 index 58f9603c23..0000000000 --- a/api/src/__test__/config/api-config.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { ConfigService } from '@nestjs/config'; - -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js'; -import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; - -describe('ApiConfigPersistence', () => { - let service: ApiConfigPersistence; - let configService: ConfigService; - let persistenceHelper: ConfigPersistenceHelper; - - beforeEach(() => { - configService = { - get: vi.fn(), - set: vi.fn(), - } as any; - - persistenceHelper = {} as ConfigPersistenceHelper; - service = new ApiConfigPersistence(configService, persistenceHelper); - }); - - describe('convertLegacyConfig', () => { - it('should migrate sandbox from string "yes" to boolean true', () => { - const legacyConfig = { - local: { sandbox: 'yes' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.sandbox).toBe(true); - }); - - it('should migrate sandbox from string "no" to boolean false', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.sandbox).toBe(false); - }); - - it('should migrate extraOrigins from comma-separated string to array', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: 'https://example.com,https://test.com' }, - remote: { ssoSubIds: '' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.extraOrigins).toEqual(['https://example.com', 'https://test.com']); - }); - - it('should filter out non-HTTP origins from extraOrigins', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { - extraOrigins: 'https://example.com,invalid-origin,http://test.com,ftp://bad.com', - }, - remote: { ssoSubIds: '' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.extraOrigins).toEqual(['https://example.com', 'http://test.com']); - }); - - it('should handle empty extraOrigins string', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.extraOrigins).toEqual([]); - }); - - it('should migrate ssoSubIds from comma-separated string to array', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: 'user1,user2,user3' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.ssoSubIds).toEqual(['user1', 'user2', 'user3']); - }); - - it('should handle empty ssoSubIds string', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.ssoSubIds).toEqual([]); - }); - - it('should handle undefined config sections', () => { - const legacyConfig = {}; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.sandbox).toBe(false); - expect(result.extraOrigins).toEqual([]); - expect(result.ssoSubIds).toEqual([]); - }); - - it('should handle complete migration with all fields', () => { - const legacyConfig = { - local: { sandbox: 'yes' }, - api: { extraOrigins: 'https://app1.example.com,https://app2.example.com' }, - remote: { ssoSubIds: 'sub1,sub2,sub3' }, - }; - - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.sandbox).toBe(true); - expect(result.extraOrigins).toEqual([ - 'https://app1.example.com', - 'https://app2.example.com', - ]); - expect(result.ssoSubIds).toEqual(['sub1', 'sub2', 'sub3']); - }); - }); -}); diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index 739c4cd1f8..557faf730a 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -1,13 +1,14 @@ import { Injectable, Logger, Module } from '@nestjs/common'; import { registerAs } from '@nestjs/config'; +import { readFile } from 'fs/promises'; +import path from 'path'; import type { ApiConfig } from '@unraid/shared/services/api-config.js'; import { ConfigFilePersister } from '@unraid/shared/services/config-file.js'; import { csvStringToArray } from '@unraid/shared/util/data.js'; +import { fileExists } from '@unraid/shared/util/file.js'; -import { API_VERSION } from '@app/environment.js'; -import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; -import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; +import { API_VERSION, PATHS_CONFIG_MODULES } from '@app/environment.js'; export { type ApiConfig }; @@ -21,67 +22,32 @@ const createDefaultConfig = (): ApiConfig => ({ plugins: [], }); -export const persistApiConfig = async (config: ApiConfig) => { - const apiConfig = new ApiStateConfig( - { - name: 'api', - defaultConfig: config, - parse: (data) => data as ApiConfig, - }, - new ConfigPersistenceHelper() - ); - return await apiConfig.persist(config); -}; - +/** + * Simple file-based config loading for plugin discovery (outside of nestjs DI container). + * This avoids complex DI container instantiation during module loading. + */ export const loadApiConfig = async () => { + const defaultConfig = createDefaultConfig(); + const configPath = path.join(PATHS_CONFIG_MODULES, 'api.json'); + + let diskConfig: Partial = {}; + try { - const defaultConfig = createDefaultConfig(); - const apiConfig = new ApiStateConfig( - { - name: 'api', - defaultConfig, - parse: (data) => data as ApiConfig, - }, - new ConfigPersistenceHelper() - ); - - let diskConfig: ApiConfig | undefined; - try { - diskConfig = await apiConfig.parseConfig(); - } catch (error) { - logger.error('Failed to load API config from disk, using defaults:', error); - diskConfig = undefined; - - // Try to overwrite the invalid config with defaults to fix the issue - try { - const configToWrite = { - ...defaultConfig, - version: API_VERSION, - }; - - const writeSuccess = await apiConfig.persist(configToWrite); - if (writeSuccess) { - logger.log('Successfully overwrote invalid config file with defaults.'); - } else { - logger.error( - 'Failed to overwrite invalid config file. Continuing with defaults in memory only.' - ); - } - } catch (persistError) { - logger.error('Error during config file repair:', persistError); + if (await fileExists(configPath)) { + const fileContent = await readFile(configPath, 'utf8'); + if (fileContent.trim()) { + diskConfig = JSON.parse(fileContent); } } - - return { - ...defaultConfig, - ...diskConfig, - version: API_VERSION, - }; - } catch (outerError) { - // This should never happen, but ensures the config factory never throws - logger.error('Critical error in loadApiConfig, using minimal defaults:', outerError); - return createDefaultConfig(); + } catch (error) { + logger.warn('Failed to load API config from disk:', error); } + + return { + ...defaultConfig, + ...diskConfig, + version: API_VERSION, + }; }; /** diff --git a/api/src/unraid-api/config/api-config.test.ts b/api/src/unraid-api/config/api-config.test.ts index 17ebe4ebdf..ede0b15149 100644 --- a/api/src/unraid-api/config/api-config.test.ts +++ b/api/src/unraid-api/config/api-config.test.ts @@ -5,12 +5,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { fileExists } from '@app/core/utils/files/file-exists.js'; import { ApiConfigPersistence, loadApiConfig } from '@app/unraid-api/config/api-config.module.js'; -// Mock the core file-exists utility used by ApiStateConfig +// Mock file utilities vi.mock('@app/core/utils/files/file-exists.js', () => ({ fileExists: vi.fn(), })); -// Mock the shared file-exists utility used by ConfigPersistenceHelper vi.mock('@unraid/shared/util/file.js', () => ({ fileExists: vi.fn(), })); @@ -193,67 +192,13 @@ describe('ApiConfigPersistence', () => { }); describe('loadApiConfig', () => { - let readFile: any; - let writeFile: any; - beforeEach(async () => { vi.clearAllMocks(); - // Reset modules to ensure fresh imports - vi.resetModules(); - - // Get mocked functions - const fsMocks = await import('fs/promises'); - readFile = fsMocks.readFile; - writeFile = fsMocks.writeFile; - }); - - it('should return default config when file does not exist', async () => { - vi.mocked(fileExists).mockResolvedValue(false); - - const result = await loadApiConfig(); - - expect(result).toEqual({ - version: expect.any(String), - extraOrigins: [], - sandbox: false, - ssoSubIds: [], - plugins: [], - }); - }); - - it('should merge disk config with defaults when file exists', async () => { - const diskConfig = { - extraOrigins: ['https://example.com'], - sandbox: true, - ssoSubIds: ['sub1', 'sub2'], - }; - - vi.mocked(fileExists).mockResolvedValue(true); - vi.mocked(readFile).mockResolvedValue(JSON.stringify(diskConfig)); - - const result = await loadApiConfig(); - - expect(result).toEqual({ - version: expect.any(String), - extraOrigins: ['https://example.com'], - sandbox: true, - ssoSubIds: ['sub1', 'sub2'], - plugins: [], - }); }); - it('should use default config and overwrite file when JSON parsing fails', async () => { - const { fileExists: sharedFileExists } = await import('@unraid/shared/util/file.js'); - - vi.mocked(fileExists).mockResolvedValue(true); - vi.mocked(readFile).mockResolvedValue('{ invalid json }'); - vi.mocked(sharedFileExists).mockResolvedValue(false); // For persist operation - vi.mocked(writeFile).mockResolvedValue(undefined); - + it('should return default config with current API_VERSION', async () => { const result = await loadApiConfig(); - // Error logging is handled by NestJS Logger, just verify the config is returned - expect(writeFile).toHaveBeenCalled(); expect(result).toEqual({ version: expect.any(String), extraOrigins: [], @@ -263,18 +208,9 @@ describe('loadApiConfig', () => { }); }); - it('should handle write failure gracefully when JSON parsing fails', async () => { - const { fileExists: sharedFileExists } = await import('@unraid/shared/util/file.js'); - - vi.mocked(fileExists).mockResolvedValue(true); - vi.mocked(readFile).mockResolvedValue('{ invalid json }'); - vi.mocked(sharedFileExists).mockResolvedValue(false); // For persist operation - vi.mocked(writeFile).mockRejectedValue(new Error('Permission denied')); - + it('should handle errors gracefully and return defaults', async () => { const result = await loadApiConfig(); - // Error logging is handled by NestJS Logger, just verify the config is returned - expect(writeFile).toHaveBeenCalled(); expect(result).toEqual({ version: expect.any(String), extraOrigins: [], @@ -283,35 +219,4 @@ describe('loadApiConfig', () => { plugins: [], }); }); - - it('should use default config when file is empty', async () => { - vi.mocked(fileExists).mockResolvedValue(true); - vi.mocked(readFile).mockResolvedValue(''); - - const result = await loadApiConfig(); - - // No error logging expected for empty files - expect(result).toEqual({ - version: expect.any(String), - extraOrigins: [], - sandbox: false, - ssoSubIds: [], - plugins: [], - }); - }); - - it('should always override version with current API_VERSION', async () => { - const diskConfig = { - version: 'old-version', - extraOrigins: ['https://example.com'], - }; - - vi.mocked(fileExists).mockResolvedValue(true); - vi.mocked(readFile).mockResolvedValue(JSON.stringify(diskConfig)); - - const result = await loadApiConfig(); - - expect(result.version).not.toBe('old-version'); - expect(result.version).toBeTruthy(); - }); }); diff --git a/api/src/unraid-api/config/factory/api-state.model.test.ts b/api/src/unraid-api/config/factory/api-state.model.test.ts deleted file mode 100644 index 09fb777250..0000000000 --- a/api/src/unraid-api/config/factory/api-state.model.test.ts +++ /dev/null @@ -1,364 +0,0 @@ -import { Logger } from '@nestjs/common'; -import { readFile } from 'node:fs/promises'; -import { join } from 'path'; - -import type { Mock } from 'vitest'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -import { fileExists } from '@app/core/utils/files/file-exists.js'; -import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; -import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; - -vi.mock('node:fs/promises'); -vi.mock('@app/core/utils/files/file-exists.js'); -vi.mock('@app/environment.js', () => ({ - PATHS_CONFIG_MODULES: '/test/config/path', -})); - -describe('ApiStateConfig', () => { - let mockPersistenceHelper: ConfigPersistenceHelper; - let mockLogger: Logger; - - interface TestConfig { - name: string; - value: number; - enabled: boolean; - } - - const defaultConfig: TestConfig = { - name: 'test', - value: 42, - enabled: true, - }; - - const parseFunction = (data: unknown): TestConfig => { - if (!data || typeof data !== 'object') { - throw new Error('Invalid config format'); - } - return data as TestConfig; - }; - - beforeEach(() => { - vi.clearAllMocks(); - - mockPersistenceHelper = { - persistIfChanged: vi.fn().mockResolvedValue(true), - } as any; - - mockLogger = { - log: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - } as any; - - vi.spyOn(Logger.prototype, 'log').mockImplementation(mockLogger.log); - vi.spyOn(Logger.prototype, 'warn').mockImplementation(mockLogger.warn); - vi.spyOn(Logger.prototype, 'error').mockImplementation(mockLogger.error); - vi.spyOn(Logger.prototype, 'debug').mockImplementation(mockLogger.debug); - }); - - describe('constructor', () => { - it('should initialize with cloned default config', () => { - const config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - - expect(config.config).toEqual(defaultConfig); - expect(config.config).not.toBe(defaultConfig); - }); - }); - - describe('token', () => { - it('should generate correct token', () => { - const config = new ApiStateConfig( - { - name: 'my-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - - expect(config.token).toBe('ApiConfig.my-config'); - }); - }); - - describe('file paths', () => { - it('should generate correct file name', () => { - const config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - - expect(config.fileName).toBe('test-config.json'); - }); - - it('should generate correct file path', () => { - const config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - - expect(config.filePath).toBe(join('/test/config/path', 'test-config.json')); - }); - }); - - describe('parseConfig', () => { - let config: ApiStateConfig; - - beforeEach(() => { - config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - }); - - it('should return undefined when file does not exist', async () => { - (fileExists as Mock).mockResolvedValue(false); - - const result = await config.parseConfig(); - - expect(result).toBeUndefined(); - expect(readFile).not.toHaveBeenCalled(); - }); - - it('should parse valid JSON config', async () => { - const validConfig = { name: 'custom', value: 100, enabled: false }; - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue(JSON.stringify(validConfig)); - - const result = await config.parseConfig(); - - expect(result).toEqual(validConfig); - expect(readFile).toHaveBeenCalledWith(config.filePath, 'utf8'); - }); - - it('should return undefined for empty file', async () => { - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue(''); - - const result = await config.parseConfig(); - - expect(result).toBeUndefined(); - expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('is empty')); - }); - - it('should return undefined for whitespace-only file', async () => { - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue(' \n\t '); - - const result = await config.parseConfig(); - - expect(result).toBeUndefined(); - expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('is empty')); - }); - - it('should throw error for invalid JSON', async () => { - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue('{ invalid json }'); - - await expect(config.parseConfig()).rejects.toThrow(); - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Failed to parse JSON') - ); - expect(mockLogger.debug).toHaveBeenCalledWith(expect.stringContaining('{ invalid json }')); - }); - - it('should throw error for incomplete JSON', async () => { - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue('{ "name": "test"'); - - await expect(config.parseConfig()).rejects.toThrow(); - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Failed to parse JSON') - ); - }); - - it('should use custom file path when provided', async () => { - const customPath = '/custom/path/config.json'; - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue(JSON.stringify(defaultConfig)); - - await config.parseConfig({ filePath: customPath }); - - expect(fileExists).toHaveBeenCalledWith(customPath); - expect(readFile).toHaveBeenCalledWith(customPath, 'utf8'); - }); - }); - - describe('persist', () => { - let config: ApiStateConfig; - - beforeEach(() => { - config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - }); - - it('should persist current config when no argument provided', async () => { - const result = await config.persist(); - - expect(result).toBe(true); - expect(mockPersistenceHelper.persistIfChanged).toHaveBeenCalledWith( - config.filePath, - defaultConfig - ); - }); - - it('should persist provided config', async () => { - const customConfig = { name: 'custom', value: 999, enabled: false }; - - const result = await config.persist(customConfig); - - expect(result).toBe(true); - expect(mockPersistenceHelper.persistIfChanged).toHaveBeenCalledWith( - config.filePath, - customConfig - ); - }); - - it('should return false and log error on persistence failure', async () => { - (mockPersistenceHelper.persistIfChanged as Mock).mockResolvedValue(false); - - const result = await config.persist(); - - expect(result).toBe(false); - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Could not write config') - ); - }); - }); - - describe('load', () => { - let config: ApiStateConfig; - - beforeEach(() => { - config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - }); - - it('should load config from file when it exists', async () => { - const savedConfig = { name: 'saved', value: 200, enabled: true }; - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue(JSON.stringify(savedConfig)); - - await config.load(); - - expect(config.config).toEqual(savedConfig); - }); - - it('should create default config when file does not exist', async () => { - (fileExists as Mock).mockResolvedValue(false); - - await config.load(); - - expect(config.config).toEqual(defaultConfig); - expect(mockLogger.log).toHaveBeenCalledWith( - expect.stringContaining('Config file does not exist') - ); - expect(mockPersistenceHelper.persistIfChanged).toHaveBeenCalledWith( - config.filePath, - defaultConfig - ); - }); - - it('should not modify config when file is invalid', async () => { - (fileExists as Mock).mockResolvedValue(true); - (readFile as Mock).mockResolvedValue('invalid json'); - - await config.load(); - - expect(config.config).toEqual(defaultConfig); - expect(mockLogger.warn).toHaveBeenCalledWith( - expect.any(Error), - expect.stringContaining('is invalid') - ); - }); - - it('should not throw even when persist fails', async () => { - (fileExists as Mock).mockResolvedValue(false); - (mockPersistenceHelper.persistIfChanged as Mock).mockResolvedValue(false); - - await expect(config.load()).resolves.not.toThrow(); - - expect(config.config).toEqual(defaultConfig); - }); - }); - - describe('update', () => { - let config: ApiStateConfig; - - beforeEach(() => { - config = new ApiStateConfig( - { - name: 'test-config', - defaultConfig, - parse: parseFunction, - }, - mockPersistenceHelper - ); - }); - - it('should update config with partial values', () => { - config.update({ value: 123 }); - - expect(config.config).toEqual({ - name: 'test', - value: 123, - enabled: true, - }); - }); - - it('should return self for chaining', () => { - const result = config.update({ enabled: false }); - - expect(result).toBe(config); - }); - - it('should validate updated config through parse function', () => { - const badParseFunction = vi.fn().mockImplementation(() => { - throw new Error('Validation failed'); - }); - - const strictConfig = new ApiStateConfig( - { - name: 'strict-config', - defaultConfig, - parse: badParseFunction, - }, - mockPersistenceHelper - ); - - expect(() => strictConfig.update({ value: -1 })).toThrow('Validation failed'); - }); - }); -}); diff --git a/api/src/unraid-api/config/factory/api-state.model.ts b/api/src/unraid-api/config/factory/api-state.model.ts deleted file mode 100644 index 487f80f0d4..0000000000 --- a/api/src/unraid-api/config/factory/api-state.model.ts +++ /dev/null @@ -1,122 +0,0 @@ -import { Logger } from '@nestjs/common'; -import { readFile } from 'node:fs/promises'; -import { join } from 'path'; - -import { fileExists } from '@app/core/utils/files/file-exists.js'; -import { PATHS_CONFIG_MODULES } from '@app/environment.js'; -import { makeConfigToken } from '@app/unraid-api/config/factory/config.injection.js'; -import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; - -export interface ApiStateConfigOptions { - /** - * The name of the config. - * - * - Must be unique. - * - Should be the key representing this config in the `ConfigFeatures` interface. - * - Used for logging and dependency injection. - */ - name: string; - defaultConfig: T; - parse: (data: unknown) => T; -} - -export class ApiStateConfig { - #config: T; - private logger: Logger; - - constructor( - readonly options: ApiStateConfigOptions, - readonly persistenceHelper: ConfigPersistenceHelper - ) { - // avoid sharing a reference with the given default config. This allows us to re-use it. - this.#config = structuredClone(options.defaultConfig); - this.logger = new Logger(this.token); - } - - /** Unique token for this config. Used for Dependency Injection & logging. */ - get token() { - return makeConfigToken(this.options.name); - } - - get fileName() { - return `${this.options.name}.json`; - } - - get filePath() { - return join(PATHS_CONFIG_MODULES, this.fileName); - } - - get config() { - return this.#config; - } - - /** - * Persists the config to the file system. Will never throw. - * @param config - The config to persist. - * @returns True if the config was written successfully, false otherwise. - */ - async persist(config = this.#config) { - const success = await this.persistenceHelper.persistIfChanged(this.filePath, config); - if (!success) { - this.logger.error(`Could not write config to ${this.filePath}.`); - } - return success; - } - - /** - * Reads the config from a path (defaults to the default file path of the config). - * @param opts - The options for the read operation. - * @param opts.filePath - The path to the config file. - * @returns The parsed config or undefined if the file does not exist. - * @throws If the file exists but is invalid. - */ - async parseConfig(opts: { filePath?: string } = {}): Promise { - const { filePath = this.filePath } = opts; - if (!(await fileExists(filePath))) return undefined; - - const fileContent = await readFile(filePath, 'utf8'); - - if (!fileContent || fileContent.trim() === '') { - this.logger.warn(`Config file '${filePath}' is empty.`); - return undefined; - } - - try { - const rawConfig = JSON.parse(fileContent); - return this.options.parse(rawConfig); - } catch (error) { - this.logger.error( - `Failed to parse JSON from '${filePath}': ${error instanceof Error ? error.message : String(error)}` - ); - this.logger.debug(`File content: ${fileContent.substring(0, 100)}...`); - throw error; - } - } - - /** - * Loads config from the file system. If the file does not exist, it will be created with the default config. - * If the config is invalid or corrupt, no action will be taken. The error will be logged. - * - * Will never throw. - */ - async load() { - try { - const config = await this.parseConfig(); - if (config) { - this.#config = config; - } else { - this.logger.log(`Config file does not exist. Writing default config.`); - this.#config = this.options.defaultConfig; - await this.persist(); - } - } catch (error) { - this.logger.warn(error, `Config file '${this.filePath}' is invalid. Not modifying config.`); - } - } - - update(config: Partial) { - const proposedConfig = this.options.parse({ ...this.#config, ...config }); - this.#config = proposedConfig; - return this; - } -} diff --git a/api/src/unraid-api/config/factory/api-state.register.ts b/api/src/unraid-api/config/factory/api-state.register.ts deleted file mode 100644 index 9b1af5438e..0000000000 --- a/api/src/unraid-api/config/factory/api-state.register.ts +++ /dev/null @@ -1,54 +0,0 @@ -import type { DynamicModule, Provider } from '@nestjs/common'; -import { SchedulerRegistry } from '@nestjs/schedule'; - -import type { ApiStateConfigOptions } from '@app/unraid-api/config/factory/api-state.model.js'; -import type { ApiStateConfigPersistenceOptions } from '@app/unraid-api/config/factory/api-state.service.js'; -import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; -import { ScheduledConfigPersistence } from '@app/unraid-api/config/factory/api-state.service.js'; -import { makeConfigToken } from '@app/unraid-api/config/factory/config.injection.js'; -import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; - -type ApiStateRegisterOptions = ApiStateConfigOptions & { - persistence?: ApiStateConfigPersistenceOptions; -}; - -export class ApiStateConfigModule { - static async register( - options: ApiStateRegisterOptions - ): Promise { - const { persistence, ...configOptions } = options; - const configToken = makeConfigToken(options.name); - const persistenceToken = makeConfigToken(options.name, ScheduledConfigPersistence.name); - const ConfigProvider = { - provide: configToken, - useFactory: async (helper: ConfigPersistenceHelper) => { - const config = new ApiStateConfig(configOptions, helper); - await config.load(); - return config; - }, - inject: [ConfigPersistenceHelper], - }; - - const providers: Provider[] = [ConfigProvider, ConfigPersistenceHelper]; - const exports = [configToken]; - if (persistence) { - providers.push({ - provide: persistenceToken, - useFactory: ( - schedulerRegistry: SchedulerRegistry, - config: ApiStateConfig - ) => { - return new ScheduledConfigPersistence(schedulerRegistry, config, persistence); - }, - inject: [SchedulerRegistry, configToken], - }); - exports.push(persistenceToken); - } - - return { - module: ApiStateConfigModule, - providers, - exports, - }; - } -} diff --git a/api/src/unraid-api/config/factory/api-state.service.ts b/api/src/unraid-api/config/factory/api-state.service.ts deleted file mode 100644 index f6de4e5e84..0000000000 --- a/api/src/unraid-api/config/factory/api-state.service.ts +++ /dev/null @@ -1,82 +0,0 @@ -import type { OnModuleDestroy, OnModuleInit } from '@nestjs/common'; -import { Logger } from '@nestjs/common'; -import { SchedulerRegistry } from '@nestjs/schedule'; - -import type { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; -import { makeConfigToken } from '@app/unraid-api/config/factory/config.injection.js'; - -export interface ApiStateConfigPersistenceOptions { - /** How often to persist the config to the file system, in milliseconds. Defaults to 10 seconds. */ - intervalMs?: number; - /** How many consecutive failed persistence attempts to tolerate before stopping. Defaults to 5. */ - maxConsecutiveFailures?: number; - /** By default, the config will be persisted to the file system when the module is initialized and destroyed. - * Set this to true to disable this behavior. - */ - disableLifecycleHooks?: boolean; -} - -export class ScheduledConfigPersistence implements OnModuleInit, OnModuleDestroy { - private consecutiveFailures = 0; - private logger: Logger; - - constructor( - private readonly schedulerRegistry: SchedulerRegistry, - private readonly config: ApiStateConfig, - private readonly options: ApiStateConfigPersistenceOptions - ) { - this.logger = new Logger(this.token); - } - - get token() { - return makeConfigToken(this.configName, ScheduledConfigPersistence.name); - } - - get configName() { - return this.config.options.name; - } - - onModuleInit() { - if (this.options.disableLifecycleHooks) return; - this.setup(); - } - - async onModuleDestroy() { - if (this.options.disableLifecycleHooks) return; - this.stop(); - await this.config.persist(); - } - - stop() { - if (this.schedulerRegistry.getInterval(this.token)) { - this.schedulerRegistry.deleteInterval(this.token); - } - } - - setup() { - const interval = this.schedulerRegistry.getInterval(this.token); - if (interval) { - this.logger.warn(`Persistence interval for '${this.token}' already exists. Aborting setup.`); - return; - } - const ONE_MINUTE = 60_000; - const { intervalMs = ONE_MINUTE, maxConsecutiveFailures = 3 } = this.options; - - const callback = async () => { - const success = await this.config.persist(); - if (success) { - this.consecutiveFailures = 0; - return; - } - this.consecutiveFailures++; - if (this.consecutiveFailures > maxConsecutiveFailures) { - this.logger.warn( - `Failed to persist '${this.configName}' too many times in a row (${this.consecutiveFailures} attempts). Disabling persistence.` - ); - this.schedulerRegistry.deleteInterval(this.token); - } - }; - - this.schedulerRegistry.addInterval(this.token, setInterval(callback, intervalMs)); - } -} diff --git a/api/src/unraid-api/config/factory/config.injection.ts b/api/src/unraid-api/config/factory/config.injection.ts deleted file mode 100644 index d4a57f1fea..0000000000 --- a/api/src/unraid-api/config/factory/config.injection.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Inject } from '@nestjs/common'; - -import type { ConfigFeatures } from '@app/unraid-api/config/factory/config.interface.js'; - -/** - * Creates a string token representation of the arguements. Pure function. - * - * @param configName - The name of the config. - * @returns A colon-separated string - */ -export function makeConfigToken(configName: string, ...details: string[]) { - return ['ApiConfig', configName, ...details].join('.'); -} - -/** - * Custom decorator to inject a config by name. - * @param feature - The name of the config to inject. - * @returns Dependency injector for the config. - */ -export function InjectConfig(feature: K) { - return Inject(makeConfigToken(feature)); -} diff --git a/api/src/unraid-api/config/factory/config.interface.ts b/api/src/unraid-api/config/factory/config.interface.ts deleted file mode 100644 index 5ac1558cb5..0000000000 --- a/api/src/unraid-api/config/factory/config.interface.ts +++ /dev/null @@ -1,15 +0,0 @@ -/** - * Container record of config names to their types. Used for type completion on registered configs. - * Config authors should redeclare/merge this interface with their config names as the keys - * and implementation models as the types. - */ -export interface ConfigFeatures {} - -export interface ConfigMetadata { - /** Unique token for this config. Used for Dependency Injection, logging, etc. */ - token: string; - /** The path to the config file. */ - filePath?: string; - /** Validates a config of type `T`. */ - validate: (config: unknown) => Promise; -} diff --git a/api/src/unraid-api/config/persistence.helper.ts b/api/src/unraid-api/config/persistence.helper.ts deleted file mode 100644 index 0a40c841b8..0000000000 --- a/api/src/unraid-api/config/persistence.helper.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import { readFile, writeFile } from 'fs/promises'; - -import { fileExists } from '@unraid/shared/util/file.js'; -import { isEqual } from 'lodash-es'; - -@Injectable() -export class ConfigPersistenceHelper { - /** - * Persist the config to disk if the given data is different from the data on-disk. - * This helps preserve the boot flash drive's life by avoiding unnecessary writes. - * - * @param filePath - The path to the config file. - * @param data - The data to persist. - * @returns `true` if the config was persisted, `false` if no changes were needed or if persistence failed. - * - * This method is designed to never throw errors. If the existing file is corrupted or unreadable, - * it will attempt to overwrite it with the new data. If write operations fail, it returns false - * but does not crash the application. - */ - async persistIfChanged(filePath: string, data: unknown): Promise { - if (!(await fileExists(filePath))) { - try { - const jsonString = JSON.stringify(data ?? {}, null, 2); - await writeFile(filePath, jsonString); - return true; - } catch (error) { - // JSON serialization or write failed, but don't crash - just return false - return false; - } - } - - let currentData: unknown; - try { - const fileContent = await readFile(filePath, 'utf8'); - currentData = JSON.parse(fileContent); - } catch (error) { - // If existing file is corrupted, treat it as if it doesn't exist - // and write the new data - try { - const jsonString = JSON.stringify(data ?? {}, null, 2); - await writeFile(filePath, jsonString); - return true; - } catch (writeError) { - // JSON serialization or write failed, but don't crash - just return false - return false; - } - } - - let stagedData: unknown; - try { - stagedData = JSON.parse(JSON.stringify(data)); - } catch (error) { - // If data can't be serialized to JSON, we can't persist it - return false; - } - - if (isEqual(currentData, stagedData)) { - return false; - } - - try { - await writeFile(filePath, JSON.stringify(stagedData, null, 2)); - return true; - } catch (error) { - // Write failed, but don't crash - just return false - return false; - } - } -} diff --git a/api/src/unraid-api/plugin/plugin-management.service.ts b/api/src/unraid-api/plugin/plugin-management.service.ts index 78e4f81aca..d1c20db71d 100644 --- a/api/src/unraid-api/plugin/plugin-management.service.ts +++ b/api/src/unraid-api/plugin/plugin-management.service.ts @@ -4,13 +4,14 @@ import { ConfigService } from '@nestjs/config'; import { ApiConfig } from '@unraid/shared/services/api-config.js'; import { DependencyService } from '@app/unraid-api/app/dependency.service.js'; -import { persistApiConfig } from '@app/unraid-api/config/api-config.module.js'; +import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js'; @Injectable() export class PluginManagementService { constructor( private readonly configService: ConfigService<{ api: ApiConfig }, true>, - private readonly dependencyService: DependencyService + private readonly dependencyService: DependencyService, + private readonly apiConfigPersistence: ApiConfigPersistence ) {} get plugins() { @@ -111,6 +112,6 @@ export class PluginManagementService { } private async persistConfig() { - return await persistApiConfig(this.configService.get('api', { infer: true })); + return await this.apiConfigPersistence.persist(); } } diff --git a/api/src/unraid-api/plugin/plugin.module.ts b/api/src/unraid-api/plugin/plugin.module.ts index acd2f79a38..800630b9aa 100644 --- a/api/src/unraid-api/plugin/plugin.module.ts +++ b/api/src/unraid-api/plugin/plugin.module.ts @@ -1,6 +1,7 @@ import { DynamicModule, Logger, Module } from '@nestjs/common'; import { DependencyService } from '@app/unraid-api/app/dependency.service.js'; +import { ApiConfigModule } from '@app/unraid-api/config/api-config.module.js'; import { ResolversModule } from '@app/unraid-api/graph/resolvers/resolvers.module.js'; import { GlobalDepsModule } from '@app/unraid-api/plugin/global-deps.module.js'; import { PluginManagementService } from '@app/unraid-api/plugin/plugin-management.service.js'; @@ -22,7 +23,7 @@ export class PluginModule { return { module: PluginModule, - imports: [GlobalDepsModule, ResolversModule, ...apiModules], + imports: [GlobalDepsModule, ResolversModule, ApiConfigModule, ...apiModules], providers: [PluginService, PluginManagementService, DependencyService, PluginResolver], exports: [PluginService, PluginManagementService, DependencyService, GlobalDepsModule], }; @@ -44,7 +45,7 @@ export class PluginCliModule { return { module: PluginCliModule, - imports: [GlobalDepsModule, ...cliModules], + imports: [GlobalDepsModule, ApiConfigModule, ...cliModules], providers: [PluginManagementService, DependencyService], exports: [PluginManagementService, DependencyService, GlobalDepsModule], }; diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/.login.php.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/.login.php.last-download-time index 57c42a2ef2..2501c07e1f 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/.login.php.last-download-time +++ b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/.login.php.last-download-time @@ -1 +1 @@ -1752524464371 \ No newline at end of file +1753135397044 \ No newline at end of file diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time index c25e06d501..e78e6e7002 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time +++ b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time @@ -1 +1 @@ -1752524464066 \ No newline at end of file +1753135396799 \ No newline at end of file diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/Notifications.page.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/Notifications.page.last-download-time index 478d8e7f73..e59db3ffde 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/Notifications.page.last-download-time +++ b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/Notifications.page.last-download-time @@ -1 +1 @@ -1752524464213 \ No newline at end of file +1753135396931 \ No newline at end of file diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/auth-request.php.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/auth-request.php.last-download-time index e8c55463f0..d67c709c4d 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/auth-request.php.last-download-time +++ b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/auth-request.php.last-download-time @@ -1 +1 @@ -1752524464631 \ No newline at end of file +1753135397144 \ No newline at end of file diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/rc.nginx.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/rc.nginx.last-download-time index d3a765f5b3..d22c02199a 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/rc.nginx.last-download-time +++ b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/rc.nginx.last-download-time @@ -1 +1 @@ -1752524464761 \ No newline at end of file +1753135397303 \ No newline at end of file From d3619113a362c7afbc9a40fbaf3417e8a7404e76 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 06:42:58 -0400 Subject: [PATCH 08/15] validate before persisting --- packages/unraid-shared/src/services/config-file.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index 6f3019e96e..3604d475e5 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -189,11 +189,17 @@ export abstract class ConfigFilePersister * const persisted = await persister.persist(myConfig); * ``` */ - async persist(config = this.configService.get(this.configKey())) { + async persist(config = this.configService.get(this.configKey())): Promise { if (!config) { this.logger.warn(`Cannot persist undefined config`); return false; } + try { + config = await this.validate(config); + } catch (error) { + this.logger.error(error, `Cannot persist invalid config`); + return false; + } try { if (isEqual(config, await this.getConfigFromFile())) { this.logger.verbose(`Config is unchanged, skipping persistence`); @@ -206,7 +212,6 @@ export abstract class ConfigFilePersister this.logger.verbose(`Persisting config to ${this.configPath()}: ${data}`); try { await writeFile(this.configPath(), data); - this.logger.verbose(`Config persisted to ${this.configPath()}`); return true; } catch (error) { this.logger.error( From 8e5696b20e813b33f793cf2c1d900d1b20627900 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 06:57:26 -0400 Subject: [PATCH 09/15] plumb config service through --- api/src/unraid-api/config/api-config.module.ts | 7 ++++++- .../src/config/config.persistence.ts | 5 +++++ .../src/templates/config.persistence.ts | 5 +++++ packages/unraid-shared/src/services/config-file.ts | 9 +++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index 557faf730a..fa2adbe1d1 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -1,5 +1,5 @@ import { Injectable, Logger, Module } from '@nestjs/common'; -import { registerAs } from '@nestjs/config'; +import { ConfigService, registerAs } from '@nestjs/config'; import { readFile } from 'fs/promises'; import path from 'path'; @@ -57,6 +57,10 @@ export const apiConfig = registerAs('api', loadApiConfig); @Injectable() export class ApiConfigPersistence extends ConfigFilePersister { + constructor(configService: ConfigService) { + super(configService); + } + fileName(): string { return 'api.json'; } @@ -98,5 +102,6 @@ export class ApiConfigPersistence extends ConfigFilePersister { // apiConfig should be registered in root config in app.module.ts, not here. @Module({ providers: [ApiConfigPersistence], + exports: [ApiConfigPersistence], }) export class ApiConfigModule {} diff --git a/packages/unraid-api-plugin-connect/src/config/config.persistence.ts b/packages/unraid-api-plugin-connect/src/config/config.persistence.ts index cb7d270161..25bce7c027 100644 --- a/packages/unraid-api-plugin-connect/src/config/config.persistence.ts +++ b/packages/unraid-api-plugin-connect/src/config/config.persistence.ts @@ -1,4 +1,5 @@ import { Injectable } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { existsSync, readFileSync } from 'fs'; import { ConfigFilePersister } from '@unraid/shared/services/config-file.js'; @@ -11,6 +12,10 @@ import { emptyMyServersConfig, MyServersConfig } from './connect.config.js'; @Injectable() export class ConnectConfigPersister extends ConfigFilePersister { + constructor(configService: ConfigService) { + super(configService); + } + /** * @override * @returns The name of the config file. diff --git a/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts b/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts index 6e4a895ffb..fc747301d5 100644 --- a/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts +++ b/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts @@ -1,9 +1,14 @@ import { Injectable } from "@nestjs/common"; import { ConfigFilePersister } from "@unraid/shared/services/config-file.js"; // npm install @unraid/shared import { PluginNameConfig } from "./config.entity.js"; +import { ConfigService } from "@nestjs/config"; @Injectable() export class PluginNameConfigPersister extends ConfigFilePersister { + constructor(configService: ConfigService) { + super(configService); + } + fileName(): string { return "plugin-name.json"; // Use kebab-case for the filename } diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index 3604d475e5..609e1b8963 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -1,4 +1,5 @@ import { + Injectable, Logger, type OnModuleDestroy, type OnModuleInit, @@ -32,6 +33,10 @@ import type { Subscription } from "rxjs"; * } * * class MyConfigPersister extends ConfigFilePersister { + * constructor(configService: ConfigService) { + * super(configService); + * } + * * fileName() { return "my-config.json"; } * configKey() { return "myConfig"; } * defaultConfig() { return { enabled: false, timeout: 5000 }; } @@ -52,6 +57,10 @@ import type { Subscription } from "rxjs"; export abstract class ConfigFilePersister implements OnModuleInit, OnModuleDestroy { + /** + * Sub-classes must provide a configService. This is not auto-injectable here. + * @param configService + */ constructor(protected readonly configService: ConfigService) { this.logger = new Logger(`ConfigFilePersister:${this.fileName()}`); } From 156e4a6649513899db2a56b86fa2e01976e01646 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 14:19:08 -0400 Subject: [PATCH 10/15] tmp: doc new pattern --- .../unraid-shared/src/services/config-file.ts | 776 +++++++++++++----- 1 file changed, 559 insertions(+), 217 deletions(-) diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index 609e1b8963..ceb02453cf 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -14,14 +14,14 @@ import { readFile, writeFile } from "node:fs/promises"; import type { Subscription } from "rxjs"; /** - * Abstract base class for persisting configuration objects to JSON files on disk. + * Abstract base class that defines configuration behavior without NestJS dependencies. + * This can be extended and used both standalone and with NestJS integration. * - * This class provides a robust configuration persistence layer with the following features: - * - **Migration Priority**: When files don't exist, migration is attempted before falling back to defaults - * - **Change Detection**: Uses deep equality checks to avoid unnecessary disk writes (flash drive optimization) - * - **Reactive Updates**: Subscribes to config changes with 25ms buffering to reduce I/O operations - * - **Error Resilience**: Graceful handling of file system errors, JSON parsing failures, and validation errors - * - **Lifecycle Management**: Proper cleanup of subscriptions and final persistence on module destruction + * This class provides the core configuration logic including: + * - **File Path Resolution**: Abstract `configPath()` method for flexible path handling + * - **Default Configuration**: Fallback values when files don't exist or are corrupted + * - **Validation Logic**: Optional validation and transformation of config data + * - **Migration Support**: Legacy config conversion during upgrades * * @template T The configuration object type that extends object * @@ -30,16 +30,23 @@ import type { Subscription } from "rxjs"; * interface MyConfig { * enabled: boolean; * timeout: number; + * apiKey?: string; * } * - * class MyConfigPersister extends ConfigFilePersister { - * constructor(configService: ConfigService) { - * super(configService); + * class MyConfigDefinition extends ConfigDefinition { + * constructor(private configDir: string) { + * super('MyConfig'); * } * * fileName() { return "my-config.json"; } - * configKey() { return "myConfig"; } - * defaultConfig() { return { enabled: false, timeout: 5000 }; } + * + * configPath() { + * return path.join(this.configDir, this.fileName()); + * } + * + * defaultConfig(): MyConfig { + * return { enabled: false, timeout: 5000 }; + * } * * async validate(config: object): Promise { * const myConfig = config as MyConfig; @@ -48,45 +55,44 @@ import type { Subscription } from "rxjs"; * } * * async migrateConfig(): Promise { - * // Custom migration logic for legacy configs + * // Try to load from legacy location + * const legacyPath = path.join(this.configDir, 'old-config.ini'); + * // ... migration logic * return { enabled: true, timeout: 3000 }; * } * } * ``` */ -export abstract class ConfigFilePersister - implements OnModuleInit, OnModuleDestroy -{ +export abstract class ConfigDefinition { + protected logger: Logger; + /** - * Sub-classes must provide a configService. This is not auto-injectable here. - * @param configService + * @param loggerName Optional custom logger name (defaults to generic name) */ - constructor(protected readonly configService: ConfigService) { - this.logger = new Logger(`ConfigFilePersister:${this.fileName()}`); + constructor(loggerName?: string) { + this.logger = new Logger(loggerName ?? `ConfigDefinition`); } - private readonly logger: Logger; - private configObserver?: Subscription; - /** * Returns the filename for the configuration file. * * @returns The name of the config file (e.g., "my-config.json") * @example "user-preferences.json" */ - abstract fileName(): string; // defined as function so it can be used in constructor + abstract fileName(): string; /** - * Returns the configuration key used in the ConfigService. + * Returns the absolute path to the configuration file. * - * This key is used to: - * - Store/retrieve config from the ConfigService - * - Filter config change events to only process relevant changes + * This is abstract to allow different path resolution strategies: + * - NestJS: Use ConfigService to get environment-based paths + * - Standalone: Use constructor parameters or environment variables + * - Testing: Use temporary directories * - * @returns The config key string (e.g., "userPreferences") - * @example "myModuleConfig" + * @returns Absolute path to the config file + * @example "/usr/local/etc/unraid/config/my-config.json" */ - abstract configKey(): string; + abstract configPath(): string; /** * Returns the default configuration object. @@ -101,136 +107,17 @@ export abstract class ConfigFilePersister * return { * enabled: false, * timeout: 5000, - * retries: 3 + * retries: 3, + * features: { + * autoBackup: true, + * notifications: false + * } * }; * } * ``` */ abstract defaultConfig(): T; - /** - * Returns the absolute path to the configuration file. - * - * Combines the `PATHS_CONFIG_MODULES` environment variable with the filename - * to create the full path where the config file should be stored. - * - * @returns Absolute path to the config file - * @throws Error if `PATHS_CONFIG_MODULES` environment variable is not set - * @example "/usr/local/etc/unraid/config/my-config.json" - */ - configPath(): string { - // PATHS_CONFIG_MODULES is a required environment variable. - // It is the directory where custom config files are stored. - return path.join( - this.configService.getOrThrow("PATHS_CONFIG_MODULES"), - this.fileName() - ); - } - - /** - * NestJS lifecycle hook called when the module is being destroyed. - * - * Performs cleanup by: - * 1. Unsubscribing from config change notifications - * 2. Persisting any final configuration state to disk - * - * This ensures no data loss and prevents memory leaks. - */ - async onModuleDestroy() { - this.configObserver?.unsubscribe(); - await this.persist(); - } - - /** - * NestJS lifecycle hook called when the module is initialized. - * - * Performs initialization by: - * 1. Loading existing config from disk or attempting migration - * 2. Setting up reactive config change subscription with 25ms buffering - * 3. Filtering changes to only process events matching this config's key - * - * **Key Behavior**: Migration is prioritized over defaults when files don't exist. - * The 25ms buffer reduces disk I/O by batching rapid config changes. - */ - async onModuleInit() { - this.logger.verbose(`Config path: ${this.configPath()}`); - await this.loadOrMigrateConfig(); - // Persist changes to the config. - this.configObserver = this.configService.changes$ - .pipe(bufferTime(25)) - .subscribe({ - next: async (changes) => { - const configChanged = changes.some(({ path }) => - path?.startsWith(this.configKey()) - ); - if (configChanged) { - await this.persist(); - } - }, - error: (err) => { - this.logger.error("Error receiving config changes:", err); - }, - }); - } - - /** - * Persists the configuration to disk with intelligent change detection. - * - * **Flash Drive Optimization**: Uses deep equality checks to avoid unnecessary writes, - * helping preserve the lifespan of boot flash drives by preventing redundant I/O operations. - * - * **Process**: - * 1. Validates that config is not undefined - * 2. Compares new config with existing file content using deep equality - * 3. Skips write if content is identical - * 4. Writes pretty-printed JSON (2-space indentation) if changes detected - * 5. Handles file system errors gracefully - * - * @param config - The config object to persist (defaults to current config from service) - * @returns `true` if the config was persisted to disk, `false` if skipped or failed - * - * @example - * ```typescript - * // Persist current config - * const persisted = await persister.persist(); - * - * // Persist specific config - * const persisted = await persister.persist(myConfig); - * ``` - */ - async persist(config = this.configService.get(this.configKey())): Promise { - if (!config) { - this.logger.warn(`Cannot persist undefined config`); - return false; - } - try { - config = await this.validate(config); - } catch (error) { - this.logger.error(error, `Cannot persist invalid config`); - return false; - } - try { - if (isEqual(config, await this.getConfigFromFile())) { - this.logger.verbose(`Config is unchanged, skipping persistence`); - return false; - } - } catch (error) { - this.logger.error(error, `Error loading config (will overwrite file)`); - } - const data = JSON.stringify(config, null, 2); - this.logger.verbose(`Persisting config to ${this.configPath()}: ${data}`); - try { - await writeFile(this.configPath(), data); - return true; - } catch (error) { - this.logger.error( - error, - `Error persisting config to '${this.configPath()}'` - ); - return false; - } - } - /** * Validates and transforms a configuration object. * @@ -242,6 +129,7 @@ export abstract class ConfigFilePersister * - Range checking for numeric values * - Required field validation * - Data transformation/normalization + * - Environment-specific validation * * @param config - The raw config object to validate * @returns The validated and potentially transformed config @@ -252,32 +140,135 @@ export abstract class ConfigFilePersister * async validate(config: object): Promise { * const myConfig = config as MyConfig; * + * // Type validation * if (typeof myConfig.timeout !== 'number' || myConfig.timeout < 1000) { * throw new Error('Invalid timeout: must be number >= 1000'); * } * + * // Enum validation * if (!['low', 'medium', 'high'].includes(myConfig.priority)) { * throw new Error('Invalid priority level'); * } * + * // Data transformation + * myConfig.apiKey = myConfig.apiKey?.trim() || undefined; + * * return myConfig; * } * ``` */ - public async validate(config: object): Promise { + async validate(config: object): Promise { return config as T; } /** - * Core initialization logic that loads, migrates, or creates configuration. + * Migrates legacy or corrupted configuration to the current format. + * + * **Default Implementation**: Throws "Migration not implemented" error. + * Override this method to provide custom migration logic. + * + * **When Called**: + * - Config file doesn't exist (first-time setup) + * - Config file contains invalid JSON + * - Config validation fails + * - File system read errors + * + * **Migration Strategies**: + * - **Legacy Format**: Convert old config structure to new format + * - **Partial Config**: Fill missing properties with sensible defaults + * - **Corrupted Data**: Attempt to salvage usable parts + * - **Fresh Install**: Return initial setup configuration + * - **Version Upgrades**: Handle breaking changes between config versions + * + * **Priority**: Migration is attempted before falling back to `defaultConfig()`, + * making this ideal for handling upgrades and first-time installations. + * + * @returns Migrated configuration object + * @throws Error if migration is not possible (falls back to defaults) + * + * @example + * ```typescript + * async migrateConfig(): Promise { + * try { + * // Try to load legacy config format + * const legacyPath = path.join(this.configDir, 'old-config.ini'); + * const legacyData = await this.readLegacyIniFile(legacyPath); + * + * return { + * enabled: legacyData.isEnabled === 'true', + * timeout: parseInt(legacyData.timeoutMs) || 5000, + * apiKey: legacyData.secret, + * newFeature: 'default-value' // New property not in legacy + * }; + * } catch (legacyError) { + * // Try environment variables for first-time setup + * if (process.env.MY_CONFIG_ENABLED) { + * return { + * enabled: process.env.MY_CONFIG_ENABLED === 'true', + * timeout: parseInt(process.env.MY_CONFIG_TIMEOUT) || 3000, + * newFeature: 'env-setup' + * }; + * } + * + * // Last resort: throw to use defaults + * throw new Error('No migration path available'); + * } + * } + * ``` + */ + async migrateConfig(): Promise { + throw new Error("Migration not implemented"); + } +} + +/** + * Standalone configuration file handler that works with any ConfigDefinition. + * Can be used independently of NestJS DI container. + * + * This class provides robust file operations with the following features: + * - **Migration Priority**: When files don't exist, migration is attempted before falling back to defaults + * - **Change Detection**: Uses deep equality checks to avoid unnecessary disk writes (flash drive optimization) + * - **Error Resilience**: Graceful handling of file system errors, JSON parsing failures, and validation errors + * - **Atomic Operations**: Individual methods for specific file operations (read, write, update) + * + * @template T The configuration object type that extends object + * + * @example + * ```typescript + * // Standalone usage + * const configDef = new MyConfigDefinition('/etc/myapp'); + * const fileHandler = new ConfigFileHandler(configDef); + * + * // Load config with migration fallback + * const config = await fileHandler.loadConfig(); + * + * // Update specific properties + * await fileHandler.updateConfig({ enabled: true, timeout: 8000 }); + * + * // Direct file operations + * const currentConfig = await fileHandler.readConfigFile(); + * await fileHandler.writeConfigFile(newConfig); + * ``` + */ +export class ConfigFileHandler { + private readonly logger: Logger; + + /** + * @param definition The configuration definition that provides behavior + */ + constructor(private readonly definition: ConfigDefinition) { + this.logger = new Logger(`ConfigFileHandler:${definition.fileName()}`); + } + + /** + * Loads configuration from file, with migration fallback. * * **Migration Priority Strategy**: * 1. **Load**: Attempts to load and validate existing config from disk * 2. **Migrate**: If loading fails, attempts migration using `migrateConfig()` * 3. **Default**: If migration fails, falls back to `defaultConfig()` * 4. **Merge**: Merges result with defaults to ensure all properties exist - * 5. **Store**: Sets final config in ConfigService - * 6. **Persist**: Writes final config to disk + * 5. **Persist**: If migration occurred, writes final config to disk * * **Key Insight**: Migration is attempted before using defaults, making this suitable * for upgrading legacy configurations or handling first-time installations. @@ -285,110 +276,461 @@ export abstract class ConfigFilePersister * **Error Handling**: All errors are logged at WARN level, ensuring the system * continues to function with sensible defaults even if file system issues occur. * - * @returns `true` if config was successfully persisted, `false` if persistence failed - * @private + * @returns Complete configuration object (defaults + loaded/migrated data) + * + * @example + * ```typescript + * // Load config - handles all error cases gracefully + * const config = await fileHandler.loadConfig(); + * console.log('Loaded config:', config); + * + * // Always returns valid config object, even if file doesn't exist + * const alwaysValid = await fileHandler.loadConfig(); + * console.log('Timeout:', alwaysValid.timeout); // Safe to access + * ``` */ - private async loadOrMigrateConfig() { - const config = this.defaultConfig(); + async loadConfig(): Promise { + const config = this.definition.defaultConfig(); + try { - Object.assign(config, await this.getConfigFromFile()); + const fileConfig = await this.readConfigFile(); + Object.assign(config, fileConfig); + return config; } catch (error) { this.logger.warn(error, "Error loading config. Attempting to migrate..."); + try { - Object.assign(config, await this.migrateConfig()); - } catch (error) { - this.logger.warn("Migration failed. Using defaults in-memory.", error); - return false; + const migratedConfig = await this.definition.migrateConfig(); + Object.assign(config, migratedConfig); + // Persist migrated config for future loads + await this.writeConfigFile(config); + return config; + } catch (migrationError) { + this.logger.warn("Migration failed. Using defaults.", migrationError); + return config; } } - this.configService.set(this.configKey(), config); - return this.persist(config); } /** - * Loads and validates configuration from a JSON file. + * Reads and validates configuration from file. * * **Process**: * 1. Checks if file exists using `fileExists()` utility * 2. Reads file content as UTF-8 text * 3. Parses JSON content - * 4. Validates result using `validate()` method + * 4. Validates result using `validate()` method from definition * * **Error Cases**: * - File doesn't exist → Error (triggers migration in caller) * - Invalid JSON syntax → Error (triggers migration in caller) * - Validation failure → Error (triggers migration in caller) * - * @param configFilePath - Path to config file (defaults to `configPath()`) - * @returns Validated configuration object + * @returns Validated configuration object from disk * @throws Error if file doesn't exist, contains invalid JSON, or fails validation * * @example * ```typescript * try { - * const config = await persister.getConfigFromFile(); - * console.log('Loaded config:', config); + * const config = await fileHandler.readConfigFile(); + * console.log('Successfully loaded config from disk:', config); * } catch (error) { - * console.log('Config loading failed, will attempt migration'); + * console.log('Config file issue, will attempt migration/defaults'); * } * ``` */ - async getConfigFromFile(configFilePath = this.configPath()): Promise { - if (!(await fileExists(configFilePath))) { - throw new Error(`Config file does not exist at '${configFilePath}'`); + async readConfigFile(): Promise { + const configPath = this.definition.configPath(); + + if (!(await fileExists(configPath))) { + throw new Error(`Config file does not exist at '${configPath}'`); } - return this.validate(JSON.parse(await readFile(configFilePath, "utf8"))); + + const content = await readFile(configPath, "utf8"); + const parsed = JSON.parse(content); + return await this.definition.validate(parsed); } /** - * Migrates legacy or corrupted configuration to the current format. + * Writes configuration to file with intelligent change detection. * - * **Default Implementation**: Throws "Not implemented" error. - * Override this method to provide custom migration logic. + * **Flash Drive Optimization**: Uses deep equality checks to avoid unnecessary writes, + * helping preserve the lifespan of boot flash drives by preventing redundant I/O operations. * - * **When Called**: - * - Config file doesn't exist (first-time setup) - * - Config file contains invalid JSON - * - Config validation fails - * - File system read errors + * **Process**: + * 1. Validates config using definition's `validate()` method + * 2. Compares new config with existing file content using deep equality + * 3. Skips write if content is identical (logs at verbose level) + * 4. Writes pretty-printed JSON (2-space indentation) if changes detected + * 5. Handles file system errors gracefully * - * **Migration Strategies**: - * - **Legacy Format**: Convert old config structure to new format - * - **Partial Config**: Fill missing properties with sensible defaults - * - **Corrupted Data**: Attempt to salvage usable parts - * - **Fresh Install**: Return initial setup configuration + * @param config - The config object to write to disk + * @returns `true` if the config was written to disk, `false` if skipped (no changes) or failed * - * **Priority**: Migration is attempted before falling back to `defaultConfig()`, - * making this ideal for handling upgrades and first-time installations. + * @example + * ```typescript + * const newConfig = { enabled: true, timeout: 8000 }; + * + * // Write config - automatically skips if unchanged + * const written = await fileHandler.writeConfigFile(newConfig); + * if (written) { + * console.log('Config updated on disk'); + * } else { + * console.log('Config unchanged, skipped write'); + * } + * ``` + */ + async writeConfigFile(config: T): Promise { + try { + config = await this.definition.validate(config); + } catch (error) { + this.logger.error(error, `Cannot write invalid config`); + return false; + } + + // Skip write if config is unchanged (flash drive optimization) + try { + const existingConfig = await this.readConfigFile(); + if (isEqual(config, existingConfig)) { + this.logger.verbose(`Config is unchanged, skipping write`); + return false; + } + } catch (error) { + // File doesn't exist or is invalid, proceed with write + this.logger.verbose(`Existing config unreadable, proceeding with write`); + } + + const data = JSON.stringify(config, null, 2); + this.logger.verbose(`Writing config to ${this.definition.configPath()}: ${data}`); + + try { + await writeFile(this.definition.configPath(), data); + return true; + } catch (error) { + this.logger.error(error, `Error writing config to '${this.definition.configPath()}'`); + return false; + } + } + + /** + * Updates configuration by merging with existing config. * - * @returns Migrated configuration object - * @throws Error if migration is not possible (falls back to defaults) + * **Process**: + * 1. Loads current config from disk (with migration/defaults fallback) + * 2. Merges provided updates with current config + * 3. Writes merged result back to disk + * + * This is ideal for making partial updates without losing other configuration values. + * + * @param updates - Partial configuration object with properties to update + * @returns `true` if the config was updated on disk, `false` if failed or no changes * * @example * ```typescript - * async migrateConfig(): Promise { - * // Try to load legacy config format - * try { - * const legacyPath = path.join(this.configDir, 'old-config.ini'); - * const legacyData = await readLegacyConfig(legacyPath); + * // Update just the timeout, keep other settings unchanged + * const updated = await fileHandler.updateConfig({ timeout: 10000 }); + * + * // Update multiple properties + * const multiUpdate = await fileHandler.updateConfig({ + * enabled: true, + * timeout: 8000, + * features: { autoBackup: false } + * }); + * ``` + */ + async updateConfig(updates: Partial): Promise { + try { + const currentConfig = await this.loadConfig(); + const newConfig = { ...currentConfig, ...updates } as T; + return await this.writeConfigFile(newConfig); + } catch (error) { + this.logger.error("Failed to update config", error); + return false; + } + } +} + +/** + * Abstract base class for persisting configuration objects to JSON files on disk. + * + * This class provides a robust configuration persistence layer that integrates with NestJS + * while also providing standalone file operations through ConfigFileHandler delegation. + * + * **Key Features**: + * - **NestJS Integration**: Integrates with ConfigService and lifecycle hooks + * - **Reactive Updates**: Subscribes to config changes with 25ms buffering to reduce I/O operations + * - **Standalone Access**: Provides `getFileHandler()` for direct file operations outside NestJS + * - **Method Overrides**: Supports overriding `configPath()`, `validate()`, `migrateConfig()` etc. + * - **Flash Drive Optimization**: Uses change detection to minimize unnecessary writes + * - **Error Resilience**: Graceful handling of all error conditions + * - **Lifecycle Management**: Proper cleanup of subscriptions and final persistence on destruction + * + * @template T The configuration object type that extends object + * + * @example + * ```typescript + * interface MyConfig { + * enabled: boolean; + * timeout: number; + * apiEndpoint: string; + * } + * + * @Injectable() + * class MyConfigPersister extends ConfigFilePersister { + * constructor(configService: ConfigService) { + * super(configService); + * } + * + * fileName() { return "my-config.json"; } + * configKey() { return "myConfig"; } + * + * defaultConfig(): MyConfig { + * return { + * enabled: false, + * timeout: 5000, + * apiEndpoint: 'https://api.example.com' + * }; + * } + * + * // Override path resolution if needed + * configPath(): string { + * return path.join('/custom/path', this.fileName()); + * } + * + * async validate(config: object): Promise { + * const myConfig = config as MyConfig; + * if (myConfig.timeout < 1000) throw new Error("Timeout too low"); + * return myConfig; + * } + * + * async migrateConfig(): Promise { + * // Load from legacy location or format + * const legacyConfig = await this.loadLegacyConfig(); + * return { + * enabled: legacyConfig.isEnabled ?? true, + * timeout: legacyConfig.timeoutMs ?? 3000, + * apiEndpoint: legacyConfig.endpoint ?? 'https://api.example.com' + * }; + * } + * } + * + * // Usage in NestJS: + * const myConfig = configService.get('myConfig'); + * + * // Usage standalone (outside NestJS): + * const fileHandler = myConfigPersister.getFileHandler(); + * await fileHandler.updateConfig({ timeout: 8000 }); + * ``` + */ +export abstract class ConfigFilePersister + extends ConfigDefinition + implements OnModuleInit, OnModuleDestroy +{ + private configObserver?: Subscription; + private fileHandler: ConfigFileHandler; + + /** + * Creates a new ConfigFilePersister instance. + * + * **Note**: The fileHandler is initialized after the constructor completes + * to ensure all abstract methods are available. + * + * @param configService The NestJS ConfigService instance for reactive config management + */ + constructor(protected readonly configService: ConfigService) { + super(`ConfigFilePersister`); + // Update logger name after fileName() is available + this.logger = new Logger(`ConfigFilePersister:${this.fileName()}`); + this.fileHandler = new ConfigFileHandler(this); + } + + /** + * Returns the configuration key used in the ConfigService. * - * return { - * enabled: legacyData.isEnabled ?? true, - * timeout: legacyData.timeoutMs ?? 5000, - * newFeature: 'default-value' // New property not in legacy - * }; - * } catch (error) { - * // First-time installation - * return { - * enabled: true, - * timeout: 3000, - * newFeature: 'initial-setup' - * }; + * This key is used to: + * - Store/retrieve config from the ConfigService + * - Filter config change events to only process relevant changes + * - Namespace configuration to avoid conflicts + * + * @returns The config key string (e.g., "userPreferences", "apiSettings") + * @example "myModuleConfig" + */ + abstract configKey(): string; + + /** + * Returns the absolute path to the configuration file. + * Overrides parent to use NestJS ConfigService for path resolution. + * + * **Default Implementation**: Combines the `PATHS_CONFIG_MODULES` environment variable + * with the filename to create the full path where the config file should be stored. + * + * **Override Examples**: + * - Custom directory: `path.join('/custom/config/dir', this.fileName())` + * - User-specific: `path.join(os.homedir(), '.myapp', this.fileName())` + * - Environment-based: `path.join(process.env.CONFIG_DIR, this.fileName())` + * + * @returns Absolute path to the config file + * @throws Error if `PATHS_CONFIG_MODULES` environment variable is not set + * @example "/usr/local/etc/unraid/config/my-config.json" + */ + configPath(): string { + return path.join( + this.configService.getOrThrow("PATHS_CONFIG_MODULES"), + this.fileName() + ); + } + + /** + * Returns a standalone ConfigFileHandler for direct file operations. + * This allows the same config logic to be used outside of NestJS lifecycle. + * + * **Use Cases**: + * - Reading config during application startup (before NestJS is ready) + * - Standalone scripts that need config access + * - Testing without full NestJS context + * - Background jobs that run outside the main application + * + * @returns ConfigFileHandler instance for direct file operations + * + * @example + * ```typescript + * // In a NestJS service + * @Injectable() + * class MyService { + * constructor(private configPersister: MyConfigPersister) {} + * + * async updateConfigDirectly() { + * const fileHandler = this.configPersister.getFileHandler(); + * await fileHandler.updateConfig({ enabled: true }); * } * } + * + * // In a standalone script + * const configPersister = new MyConfigPersister(mockConfigService); + * const fileHandler = configPersister.getFileHandler(); + * const config = await fileHandler.loadConfig(); * ``` */ - async migrateConfig(): Promise { - throw new Error("Not implemented"); + getFileHandler(): ConfigFileHandler { + return this.fileHandler; + } + + /** + * NestJS lifecycle hook called when the module is being destroyed. + * + * Performs cleanup by: + * 1. Unsubscribing from config change notifications to prevent memory leaks + * 2. Persisting any final configuration state to disk to ensure no data loss + * + * This ensures that configuration changes made just before shutdown are not lost. + */ + async onModuleDestroy() { + this.configObserver?.unsubscribe(); + await this.persist(); + } + + /** + * NestJS lifecycle hook called when the module is initialized. + * + * Performs initialization by: + * 1. Loading existing config from disk (with migration fallback) + * 2. Setting loaded config in the ConfigService for reactive access + * 3. Setting up reactive config change subscription with 25ms buffering + * 4. Filtering changes to only process events matching this config's key + * + * **Key Behavior**: Migration is prioritized over defaults when files don't exist. + * The 25ms buffer reduces disk I/O by batching rapid config changes. + * + * **Config Change Detection**: Only persists when changes occur to this specific + * config key, preventing unnecessary writes when other configs change. + */ + async onModuleInit() { + this.logger.verbose(`Config path: ${this.configPath()}`); + await this.loadOrMigrateConfig(); + + this.configObserver = this.configService.changes$ + .pipe(bufferTime(25)) + .subscribe({ + next: async (changes) => { + const configChanged = changes.some(({ path }) => + path?.startsWith(this.configKey()) + ); + if (configChanged) { + await this.persist(); + } + }, + error: (err) => { + this.logger.error("Error receiving config changes:", err); + }, + }); + } + + /** + * Persists the configuration to disk using the underlying file handler. + * + * **Features**: + * - Validates config before writing + * - Uses change detection to avoid unnecessary writes (flash drive optimization) + * - Handles all error conditions gracefully + * - Logs operations at appropriate levels + * + * @param config - The config object to persist (defaults to current config from service) + * @returns `true` if the config was persisted to disk, `false` if skipped or failed + * + * @example + * ```typescript + * // Persist current config from ConfigService + * const persisted = await persister.persist(); + * + * // Persist specific config object + * const customConfig = { enabled: true, timeout: 8000 }; + * const persisted = await persister.persist(customConfig); + * ``` + */ + async persist(config = this.configService.get(this.configKey())): Promise { + if (!config) { + this.logger.warn(`Cannot persist undefined config`); + return false; + } + return await this.fileHandler.writeConfigFile(config); + } + + /** + * Load or migrate configuration and set it in ConfigService. + * + * This is the core initialization method that: + * 1. Uses the file handler to load config (with migration fallback) + * 2. Sets the loaded config in the NestJS ConfigService + * 3. Persists the final config to disk (important for migrations) + * + * @returns `true` if config was successfully persisted, `false` if persistence failed + * @private + */ + private async loadOrMigrateConfig() { + const config = await this.fileHandler.loadConfig(); + this.configService.set(this.configKey(), config); + return this.persist(config); + } + + /** + * Loads and validates configuration from a JSON file. + * + * **Legacy Method**: This method is kept for backward compatibility. + * For new code, prefer using `getFileHandler().readConfigFile()` for consistency. + * + * @param configFilePath - Path to config file (defaults to `configPath()`) + * @returns Validated configuration object + * @throws Error if file doesn't exist, contains invalid JSON, or fails validation + * @deprecated Use getFileHandler().readConfigFile() for consistency + */ + async getConfigFromFile(configFilePath = this.configPath()): Promise { + if (configFilePath !== this.configPath()) { + // For custom paths, use the original implementation + if (!(await fileExists(configFilePath))) { + throw new Error(`Config file does not exist at '${configFilePath}'`); + } + return this.validate(JSON.parse(await readFile(configFilePath, "utf8"))); + } + return await this.fileHandler.readConfigFile(); } } From 04c334423357e1ef0f271bbb478803249cc5fbfc Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 15:13:42 -0400 Subject: [PATCH 11/15] refactor: split into definition, file-handler, and nest js abstractions --- .../services/__tests__/config-file.test.ts | 109 ++-- .../unraid-shared/src/services/config-file.ts | 487 +----------------- .../util/__tests__/config-definition.test.ts | 192 +++++++ .../__tests__/config-file-handler.test.ts | 446 ++++++++++++++++ .../src/util/config-definition.ts | 204 ++++++++ .../src/util/config-file-handler.ts | 242 +++++++++ 6 files changed, 1175 insertions(+), 505 deletions(-) create mode 100644 packages/unraid-shared/src/util/__tests__/config-definition.test.ts create mode 100644 packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts create mode 100644 packages/unraid-shared/src/util/config-definition.ts create mode 100644 packages/unraid-shared/src/util/config-file-handler.ts diff --git a/packages/unraid-shared/src/services/__tests__/config-file.test.ts b/packages/unraid-shared/src/services/__tests__/config-file.test.ts index 751b194c83..b5821b0990 100644 --- a/packages/unraid-shared/src/services/__tests__/config-file.test.ts +++ b/packages/unraid-shared/src/services/__tests__/config-file.test.ts @@ -5,6 +5,40 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { ConfigFilePersister } from "../config-file.js"; +/** + * TEST SCOPE: ConfigFilePersister NestJS Integration + * + * BEHAVIORS TESTED: + * • NestJS lifecycle integration (OnModuleInit, OnModuleDestroy) + * • Reactive config change subscription with 25ms buffering + * • ConfigService integration for path resolution and config storage + * • Automatic config loading with migration priority over defaults + * • Config change detection and selective persistence (matching configKey only) + * • Graceful error handling for all failure scenarios + * • Flash drive optimization through change detection + * • Standalone file access via getFileHandler() delegation + * • Proper cleanup of subscriptions and final state persistence + * + * INTEGRATION SCENARIOS: + * ✓ Module initialization with existing/missing/invalid config files + * ✓ Reactive config change processing with proper filtering + * ✓ Module destruction with subscription cleanup and final persistence + * ✓ Error resilience (file system errors, validation failures, service errors) + * ✓ Migration vs defaults priority during initialization + * ✓ Full application lifecycle from startup to shutdown + * + * COVERAGE FOCUS: + * • NestJS framework integration correctness + * • Reactive configuration management + * • Production-like error scenarios + * • Memory leak prevention (subscription management) + * • Data persistence guarantees during shutdown + * + * NOT TESTED (covered in other files): + * • Low-level file operations (ConfigFileHandler) + * • Abstract class behavior (ConfigDefinition) + */ + interface TestConfig { name: string; version: number; @@ -80,7 +114,7 @@ describe("ConfigFilePersister Integration Tests", () => { // Setup config store configStore = {}; - + // Setup rxjs subject for config changes changesSubject = new Subject(); @@ -205,13 +239,13 @@ describe("ConfigFilePersister Integration Tests", () => { }; await writeFile(configPath, JSON.stringify(config)); - const result = await persister.getConfigFromFile(); + const result = await persister.getFileHandler().readConfigFile(); expect(result).toEqual(config); }); test("throws error when file doesn't exist", async () => { - await expect(persister.getConfigFromFile()).rejects.toThrow( + await expect(persister.getFileHandler().readConfigFile()).rejects.toThrow( "Config file does not exist" ); }); @@ -219,7 +253,7 @@ describe("ConfigFilePersister Integration Tests", () => { test("throws error when file contains invalid JSON", async () => { await writeFile(configPath, "{ invalid json"); - await expect(persister.getConfigFromFile()).rejects.toThrow(); + await expect(persister.getFileHandler().readConfigFile()).rejects.toThrow(); }); test("throws error when config is invalid", async () => { @@ -234,17 +268,23 @@ describe("ConfigFilePersister Integration Tests", () => { }; await writeFile(configPath, JSON.stringify(invalidConfig)); - await expect(persister.getConfigFromFile()).rejects.toThrow( + await expect(persister.getFileHandler().readConfigFile()).rejects.toThrow( "Invalid version" ); }); test("base class migration throws not implemented error", async () => { - const basePersister = new class extends ConfigFilePersister { - fileName() { return "base-test.json"; } - configKey() { return "baseTest"; } - defaultConfig() { return persister.defaultConfig(); } - }(configService); + const basePersister = new (class extends ConfigFilePersister { + fileName() { + return "base-test.json"; + } + configKey() { + return "baseTest"; + } + defaultConfig() { + return persister.defaultConfig(); + } + })(configService); await expect(basePersister.migrateConfig()).rejects.toThrow( "Not implemented" @@ -282,24 +322,24 @@ describe("ConfigFilePersister Integration Tests", () => { // Pre-create config file to avoid migration const initialConfig = persister.defaultConfig(); await writeFile(configPath, JSON.stringify(initialConfig, null, 2)); - + await persister.onModuleInit(); // Verify that the config observer is active by checking internal state // This tests that the subscription was created without relying on timing expect((persister as any).configObserver).toBeDefined(); expect((persister as any).configObserver.closed).toBe(false); - + // Test that non-matching changes are ignored (synchronous test) configStore["testConfig"] = persister.defaultConfig(); const initialFileContent = await readFile(configPath, "utf8"); - + // Emit a non-matching config change changesSubject.next([{ path: "otherConfig.setting" }]); - + // Wait briefly to ensure no processing occurs - await new Promise(resolve => setTimeout(resolve, 30)); - + await new Promise((resolve) => setTimeout(resolve, 30)); + // File should remain unchanged const afterFileContent = await readFile(configPath, "utf8"); expect(afterFileContent).toBe(initialFileContent); @@ -309,26 +349,30 @@ describe("ConfigFilePersister Integration Tests", () => { // Pre-create config file const initialConfig = persister.defaultConfig(); await writeFile(configPath, JSON.stringify(initialConfig, null, 2)); - + await persister.onModuleInit(); // Set initial config and write to file configStore["testConfig"] = persister.defaultConfig(); - + // Get initial modification time - const stats1 = await import('fs/promises').then(fs => fs.stat(configPath)); - + const stats1 = await import("fs/promises").then((fs) => + fs.stat(configPath) + ); + // Wait a bit to ensure timestamp difference - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Emit change for different config key changesSubject.next([{ path: "otherConfig.setting" }]); - + // Wait for buffer time - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, 50)); // File should remain unchanged (same modification time) - const stats2 = await import('fs/promises').then(fs => fs.stat(configPath)); + const stats2 = await import("fs/promises").then((fs) => + fs.stat(configPath) + ); expect(stats2.mtime).toEqual(stats1.mtime); }); @@ -338,14 +382,14 @@ describe("ConfigFilePersister Integration Tests", () => { ...configService, get: () => { throw new Error("Config service error"); - } + }, }; const errorPersister = new TestConfigFilePersister(errorConfigService); - + // Should still initialize (migration will be called due to no file) await errorPersister.onModuleInit(); - + // Should have migrated config since get failed const expectedMigrated = await errorPersister.migrateConfig(); expect(configStore.testConfig).toEqual(expectedMigrated); @@ -358,13 +402,14 @@ describe("ConfigFilePersister Integration Tests", () => { const invalidPersister = new TestConfigFilePersister({ ...configService, getOrThrow: (key: string) => { - if (key === "PATHS_CONFIG_MODULES") return "/invalid/path/that/does/not/exist"; + if (key === "PATHS_CONFIG_MODULES") + return "/invalid/path/that/does/not/exist"; throw new Error(`Config key ${key} not found`); }, }); const config = { ...persister.defaultConfig(), name: "error-test" }; - + // Should not throw despite write error const result = await invalidPersister.persist(config); expect(result).toBe(false); @@ -427,9 +472,9 @@ describe("ConfigFilePersister Integration Tests", () => { // Trigger change notification changesSubject.next([{ path: "testConfig.enabled" }]); - + // Wait for persistence - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, 50)); // Cleanup await persister.onModuleDestroy(); @@ -447,4 +492,4 @@ describe("ConfigFilePersister Integration Tests", () => { }, }); }); -}); \ No newline at end of file +}); diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index ceb02453cf..55f9929200 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -1,5 +1,4 @@ import { - Injectable, Logger, type OnModuleDestroy, type OnModuleInit, @@ -7,452 +6,14 @@ import { import type { ConfigService } from "@nestjs/config"; import path from "node:path"; -import { isEqual } from "lodash-es"; import { bufferTime } from "rxjs/operators"; -import { fileExists } from "../util/file.js"; -import { readFile, writeFile } from "node:fs/promises"; import type { Subscription } from "rxjs"; - -/** - * Abstract base class that defines configuration behavior without NestJS dependencies. - * This can be extended and used both standalone and with NestJS integration. - * - * This class provides the core configuration logic including: - * - **File Path Resolution**: Abstract `configPath()` method for flexible path handling - * - **Default Configuration**: Fallback values when files don't exist or are corrupted - * - **Validation Logic**: Optional validation and transformation of config data - * - **Migration Support**: Legacy config conversion during upgrades - * - * @template T The configuration object type that extends object - * - * @example - * ```typescript - * interface MyConfig { - * enabled: boolean; - * timeout: number; - * apiKey?: string; - * } - * - * class MyConfigDefinition extends ConfigDefinition { - * constructor(private configDir: string) { - * super('MyConfig'); - * } - * - * fileName() { return "my-config.json"; } - * - * configPath() { - * return path.join(this.configDir, this.fileName()); - * } - * - * defaultConfig(): MyConfig { - * return { enabled: false, timeout: 5000 }; - * } - * - * async validate(config: object): Promise { - * const myConfig = config as MyConfig; - * if (myConfig.timeout < 1000) throw new Error("Timeout too low"); - * return myConfig; - * } - * - * async migrateConfig(): Promise { - * // Try to load from legacy location - * const legacyPath = path.join(this.configDir, 'old-config.ini'); - * // ... migration logic - * return { enabled: true, timeout: 3000 }; - * } - * } - * ``` - */ -export abstract class ConfigDefinition { - protected logger: Logger; - - /** - * @param loggerName Optional custom logger name (defaults to generic name) - */ - constructor(loggerName?: string) { - this.logger = new Logger(loggerName ?? `ConfigDefinition`); - } - - /** - * Returns the filename for the configuration file. - * - * @returns The name of the config file (e.g., "my-config.json") - * @example "user-preferences.json" - */ - abstract fileName(): string; - - /** - * Returns the absolute path to the configuration file. - * - * This is abstract to allow different path resolution strategies: - * - NestJS: Use ConfigService to get environment-based paths - * - Standalone: Use constructor parameters or environment variables - * - Testing: Use temporary directories - * - * @returns Absolute path to the config file - * @example "/usr/local/etc/unraid/config/my-config.json" - */ - abstract configPath(): string; - - /** - * Returns the default configuration object. - * - * **Important**: This is used as a fallback when migration fails or as a base - * for merging with loaded/migrated configurations. - * - * @returns The default configuration object - * @example - * ```typescript - * defaultConfig(): MyConfig { - * return { - * enabled: false, - * timeout: 5000, - * retries: 3, - * features: { - * autoBackup: true, - * notifications: false - * } - * }; - * } - * ``` - */ - abstract defaultConfig(): T; - - /** - * Validates and transforms a configuration object. - * - * **Default Implementation**: Simple type cast (passthrough validation). - * Override this method to implement custom validation logic. - * - * **Common Use Cases**: - * - Schema validation (e.g., using Joi, Yup, or Zod) - * - Range checking for numeric values - * - Required field validation - * - Data transformation/normalization - * - Environment-specific validation - * - * @param config - The raw config object to validate - * @returns The validated and potentially transformed config - * @throws Error if the config is invalid (stops loading/migration process) - * - * @example - * ```typescript - * async validate(config: object): Promise { - * const myConfig = config as MyConfig; - * - * // Type validation - * if (typeof myConfig.timeout !== 'number' || myConfig.timeout < 1000) { - * throw new Error('Invalid timeout: must be number >= 1000'); - * } - * - * // Enum validation - * if (!['low', 'medium', 'high'].includes(myConfig.priority)) { - * throw new Error('Invalid priority level'); - * } - * - * // Data transformation - * myConfig.apiKey = myConfig.apiKey?.trim() || undefined; - * - * return myConfig; - * } - * ``` - */ - async validate(config: object): Promise { - return config as T; - } - - /** - * Migrates legacy or corrupted configuration to the current format. - * - * **Default Implementation**: Throws "Migration not implemented" error. - * Override this method to provide custom migration logic. - * - * **When Called**: - * - Config file doesn't exist (first-time setup) - * - Config file contains invalid JSON - * - Config validation fails - * - File system read errors - * - * **Migration Strategies**: - * - **Legacy Format**: Convert old config structure to new format - * - **Partial Config**: Fill missing properties with sensible defaults - * - **Corrupted Data**: Attempt to salvage usable parts - * - **Fresh Install**: Return initial setup configuration - * - **Version Upgrades**: Handle breaking changes between config versions - * - * **Priority**: Migration is attempted before falling back to `defaultConfig()`, - * making this ideal for handling upgrades and first-time installations. - * - * @returns Migrated configuration object - * @throws Error if migration is not possible (falls back to defaults) - * - * @example - * ```typescript - * async migrateConfig(): Promise { - * try { - * // Try to load legacy config format - * const legacyPath = path.join(this.configDir, 'old-config.ini'); - * const legacyData = await this.readLegacyIniFile(legacyPath); - * - * return { - * enabled: legacyData.isEnabled === 'true', - * timeout: parseInt(legacyData.timeoutMs) || 5000, - * apiKey: legacyData.secret, - * newFeature: 'default-value' // New property not in legacy - * }; - * } catch (legacyError) { - * // Try environment variables for first-time setup - * if (process.env.MY_CONFIG_ENABLED) { - * return { - * enabled: process.env.MY_CONFIG_ENABLED === 'true', - * timeout: parseInt(process.env.MY_CONFIG_TIMEOUT) || 3000, - * newFeature: 'env-setup' - * }; - * } - * - * // Last resort: throw to use defaults - * throw new Error('No migration path available'); - * } - * } - * ``` - */ - async migrateConfig(): Promise { - throw new Error("Migration not implemented"); - } -} - -/** - * Standalone configuration file handler that works with any ConfigDefinition. - * Can be used independently of NestJS DI container. - * - * This class provides robust file operations with the following features: - * - **Migration Priority**: When files don't exist, migration is attempted before falling back to defaults - * - **Change Detection**: Uses deep equality checks to avoid unnecessary disk writes (flash drive optimization) - * - **Error Resilience**: Graceful handling of file system errors, JSON parsing failures, and validation errors - * - **Atomic Operations**: Individual methods for specific file operations (read, write, update) - * - * @template T The configuration object type that extends object - * - * @example - * ```typescript - * // Standalone usage - * const configDef = new MyConfigDefinition('/etc/myapp'); - * const fileHandler = new ConfigFileHandler(configDef); - * - * // Load config with migration fallback - * const config = await fileHandler.loadConfig(); - * - * // Update specific properties - * await fileHandler.updateConfig({ enabled: true, timeout: 8000 }); - * - * // Direct file operations - * const currentConfig = await fileHandler.readConfigFile(); - * await fileHandler.writeConfigFile(newConfig); - * ``` - */ -export class ConfigFileHandler { - private readonly logger: Logger; - - /** - * @param definition The configuration definition that provides behavior - */ - constructor(private readonly definition: ConfigDefinition) { - this.logger = new Logger(`ConfigFileHandler:${definition.fileName()}`); - } - - /** - * Loads configuration from file, with migration fallback. - * - * **Migration Priority Strategy**: - * 1. **Load**: Attempts to load and validate existing config from disk - * 2. **Migrate**: If loading fails, attempts migration using `migrateConfig()` - * 3. **Default**: If migration fails, falls back to `defaultConfig()` - * 4. **Merge**: Merges result with defaults to ensure all properties exist - * 5. **Persist**: If migration occurred, writes final config to disk - * - * **Key Insight**: Migration is attempted before using defaults, making this suitable - * for upgrading legacy configurations or handling first-time installations. - * - * **Error Handling**: All errors are logged at WARN level, ensuring the system - * continues to function with sensible defaults even if file system issues occur. - * - * @returns Complete configuration object (defaults + loaded/migrated data) - * - * @example - * ```typescript - * // Load config - handles all error cases gracefully - * const config = await fileHandler.loadConfig(); - * console.log('Loaded config:', config); - * - * // Always returns valid config object, even if file doesn't exist - * const alwaysValid = await fileHandler.loadConfig(); - * console.log('Timeout:', alwaysValid.timeout); // Safe to access - * ``` - */ - async loadConfig(): Promise { - const config = this.definition.defaultConfig(); - - try { - const fileConfig = await this.readConfigFile(); - Object.assign(config, fileConfig); - return config; - } catch (error) { - this.logger.warn(error, "Error loading config. Attempting to migrate..."); - - try { - const migratedConfig = await this.definition.migrateConfig(); - Object.assign(config, migratedConfig); - // Persist migrated config for future loads - await this.writeConfigFile(config); - return config; - } catch (migrationError) { - this.logger.warn("Migration failed. Using defaults.", migrationError); - return config; - } - } - } - - /** - * Reads and validates configuration from file. - * - * **Process**: - * 1. Checks if file exists using `fileExists()` utility - * 2. Reads file content as UTF-8 text - * 3. Parses JSON content - * 4. Validates result using `validate()` method from definition - * - * **Error Cases**: - * - File doesn't exist → Error (triggers migration in caller) - * - Invalid JSON syntax → Error (triggers migration in caller) - * - Validation failure → Error (triggers migration in caller) - * - * @returns Validated configuration object from disk - * @throws Error if file doesn't exist, contains invalid JSON, or fails validation - * - * @example - * ```typescript - * try { - * const config = await fileHandler.readConfigFile(); - * console.log('Successfully loaded config from disk:', config); - * } catch (error) { - * console.log('Config file issue, will attempt migration/defaults'); - * } - * ``` - */ - async readConfigFile(): Promise { - const configPath = this.definition.configPath(); - - if (!(await fileExists(configPath))) { - throw new Error(`Config file does not exist at '${configPath}'`); - } - - const content = await readFile(configPath, "utf8"); - const parsed = JSON.parse(content); - return await this.definition.validate(parsed); - } - - /** - * Writes configuration to file with intelligent change detection. - * - * **Flash Drive Optimization**: Uses deep equality checks to avoid unnecessary writes, - * helping preserve the lifespan of boot flash drives by preventing redundant I/O operations. - * - * **Process**: - * 1. Validates config using definition's `validate()` method - * 2. Compares new config with existing file content using deep equality - * 3. Skips write if content is identical (logs at verbose level) - * 4. Writes pretty-printed JSON (2-space indentation) if changes detected - * 5. Handles file system errors gracefully - * - * @param config - The config object to write to disk - * @returns `true` if the config was written to disk, `false` if skipped (no changes) or failed - * - * @example - * ```typescript - * const newConfig = { enabled: true, timeout: 8000 }; - * - * // Write config - automatically skips if unchanged - * const written = await fileHandler.writeConfigFile(newConfig); - * if (written) { - * console.log('Config updated on disk'); - * } else { - * console.log('Config unchanged, skipped write'); - * } - * ``` - */ - async writeConfigFile(config: T): Promise { - try { - config = await this.definition.validate(config); - } catch (error) { - this.logger.error(error, `Cannot write invalid config`); - return false; - } - - // Skip write if config is unchanged (flash drive optimization) - try { - const existingConfig = await this.readConfigFile(); - if (isEqual(config, existingConfig)) { - this.logger.verbose(`Config is unchanged, skipping write`); - return false; - } - } catch (error) { - // File doesn't exist or is invalid, proceed with write - this.logger.verbose(`Existing config unreadable, proceeding with write`); - } - - const data = JSON.stringify(config, null, 2); - this.logger.verbose(`Writing config to ${this.definition.configPath()}: ${data}`); - - try { - await writeFile(this.definition.configPath(), data); - return true; - } catch (error) { - this.logger.error(error, `Error writing config to '${this.definition.configPath()}'`); - return false; - } - } - - /** - * Updates configuration by merging with existing config. - * - * **Process**: - * 1. Loads current config from disk (with migration/defaults fallback) - * 2. Merges provided updates with current config - * 3. Writes merged result back to disk - * - * This is ideal for making partial updates without losing other configuration values. - * - * @param updates - Partial configuration object with properties to update - * @returns `true` if the config was updated on disk, `false` if failed or no changes - * - * @example - * ```typescript - * // Update just the timeout, keep other settings unchanged - * const updated = await fileHandler.updateConfig({ timeout: 10000 }); - * - * // Update multiple properties - * const multiUpdate = await fileHandler.updateConfig({ - * enabled: true, - * timeout: 8000, - * features: { autoBackup: false } - * }); - * ``` - */ - async updateConfig(updates: Partial): Promise { - try { - const currentConfig = await this.loadConfig(); - const newConfig = { ...currentConfig, ...updates } as T; - return await this.writeConfigFile(newConfig); - } catch (error) { - this.logger.error("Failed to update config", error); - return false; - } - } -} +import { ConfigFileHandler } from "../util/config-file-handler.js"; +import { ConfigDefinition } from "../util/config-definition.js"; /** * Abstract base class for persisting configuration objects to JSON files on disk. - * + * * This class provides a robust configuration persistence layer that integrates with NestJS * while also providing standalone file operations through ConfigFileHandler delegation. * @@ -483,10 +44,10 @@ export class ConfigFileHandler { * * fileName() { return "my-config.json"; } * configKey() { return "myConfig"; } - * + * * defaultConfig(): MyConfig { - * return { - * enabled: false, + * return { + * enabled: false, * timeout: 5000, * apiEndpoint: 'https://api.example.com' * }; @@ -531,10 +92,10 @@ export abstract class ConfigFilePersister /** * Creates a new ConfigFilePersister instance. - * + * * **Note**: The fileHandler is initialized after the constructor completes * to ensure all abstract methods are available. - * + * * @param configService The NestJS ConfigService instance for reactive config management */ constructor(protected readonly configService: ConfigService) { @@ -561,7 +122,7 @@ export abstract class ConfigFilePersister * Returns the absolute path to the configuration file. * Overrides parent to use NestJS ConfigService for path resolution. * - * **Default Implementation**: Combines the `PATHS_CONFIG_MODULES` environment variable + * **Default Implementation**: Combines the `PATHS_CONFIG_MODULES` environment variable * with the filename to create the full path where the config file should be stored. * * **Override Examples**: @@ -647,7 +208,7 @@ export abstract class ConfigFilePersister async onModuleInit() { this.logger.verbose(`Config path: ${this.configPath()}`); await this.loadOrMigrateConfig(); - + this.configObserver = this.configService.changes$ .pipe(bufferTime(25)) .subscribe({ @@ -687,7 +248,9 @@ export abstract class ConfigFilePersister * const persisted = await persister.persist(customConfig); * ``` */ - async persist(config = this.configService.get(this.configKey())): Promise { + async persist( + config = this.configService.get(this.configKey()) + ): Promise { if (!config) { this.logger.warn(`Cannot persist undefined config`); return false; @@ -697,7 +260,7 @@ export abstract class ConfigFilePersister /** * Load or migrate configuration and set it in ConfigService. - * + * * This is the core initialization method that: * 1. Uses the file handler to load config (with migration fallback) * 2. Sets the loaded config in the NestJS ConfigService @@ -711,26 +274,4 @@ export abstract class ConfigFilePersister this.configService.set(this.configKey(), config); return this.persist(config); } - - /** - * Loads and validates configuration from a JSON file. - * - * **Legacy Method**: This method is kept for backward compatibility. - * For new code, prefer using `getFileHandler().readConfigFile()` for consistency. - * - * @param configFilePath - Path to config file (defaults to `configPath()`) - * @returns Validated configuration object - * @throws Error if file doesn't exist, contains invalid JSON, or fails validation - * @deprecated Use getFileHandler().readConfigFile() for consistency - */ - async getConfigFromFile(configFilePath = this.configPath()): Promise { - if (configFilePath !== this.configPath()) { - // For custom paths, use the original implementation - if (!(await fileExists(configFilePath))) { - throw new Error(`Config file does not exist at '${configFilePath}'`); - } - return this.validate(JSON.parse(await readFile(configFilePath, "utf8"))); - } - return await this.fileHandler.readConfigFile(); - } } diff --git a/packages/unraid-shared/src/util/__tests__/config-definition.test.ts b/packages/unraid-shared/src/util/__tests__/config-definition.test.ts new file mode 100644 index 0000000000..cdea6f4a02 --- /dev/null +++ b/packages/unraid-shared/src/util/__tests__/config-definition.test.ts @@ -0,0 +1,192 @@ +import { expect, test, describe, beforeEach } from "bun:test"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { ConfigDefinition } from "../config-definition.js"; + +/** + * TEST SCOPE: ConfigDefinition Abstract Base Class + * + * BEHAVIORS TESTED: + * • Core abstract method implementations (fileName, configPath, defaultConfig) + * • Default validation behavior (passthrough without transformation) + * • Custom validation with data transformation and error throwing + * • Default migration behavior (throws "Not implemented" error) + * • Custom migration implementation with success and failure scenarios + * • Error propagation from validation and migration methods + * + * COVERAGE FOCUS: + * • Abstract class contract enforcement + * • Extension point behavior (validate, migrate methods) + * • Error handling patterns for implementors + * • Type safety and configuration structure validation + * + * NOT TESTED (covered in other files): + * • File I/O operations (ConfigFileHandler) + * • NestJS integration (ConfigFilePersister) + * • Reactive config changes + */ + +interface TestConfig { + name: string; + version: number; + enabled: boolean; + timeout: number; +} + +class TestConfigDefinition extends ConfigDefinition { + constructor(private configDir: string, loggerName?: string) { + super(loggerName); + } + + fileName(): string { + return "test-config.json"; + } + + configPath(): string { + return join(this.configDir, this.fileName()); + } + + defaultConfig(): TestConfig { + return { + name: "test", + version: 1, + enabled: false, + timeout: 5000, + }; + } +} + +class ValidatingConfigDefinition extends TestConfigDefinition { + async validate(config: object): Promise { + const testConfig = config as TestConfig; + + if (typeof testConfig.name !== "string" || testConfig.name.length === 0) { + throw new Error("Name must be a non-empty string"); + } + + if (typeof testConfig.version !== "number" || testConfig.version < 1) { + throw new Error("Version must be a number >= 1"); + } + + if (typeof testConfig.timeout !== "number" || testConfig.timeout < 1000) { + throw new Error("Timeout must be a number >= 1000"); + } + + // Test data transformation + return { + ...testConfig, + name: testConfig.name.trim(), + timeout: Math.max(testConfig.timeout, 1000), + }; + } +} + +class MigratingConfigDefinition extends TestConfigDefinition { + public migrationShouldFail = false; + public migrationCallCount = 0; + + async migrateConfig(): Promise { + this.migrationCallCount++; + + if (this.migrationShouldFail) { + throw new Error("Migration failed"); + } + + return { + name: "migrated", + version: 2, + enabled: true, + timeout: 3000, + }; + } +} + +describe("ConfigDefinition", () => { + let testDir: string; + let configDefinition: TestConfigDefinition; + + beforeEach(() => { + testDir = join(tmpdir(), `config-def-test-${Date.now()}`); + configDefinition = new TestConfigDefinition(testDir); + }); + + describe("Core Functionality", () => { + test("abstract methods are implemented correctly", () => { + expect(configDefinition.fileName()).toBe("test-config.json"); + expect(configDefinition.configPath()).toBe( + join(testDir, "test-config.json") + ); + expect(configDefinition.defaultConfig()).toEqual({ + name: "test", + version: 1, + enabled: false, + timeout: 5000, + }); + }); + + test("default validation is passthrough", async () => { + const config = { name: "test", version: 2, enabled: true, timeout: 3000 }; + const result = await configDefinition.validate(config); + expect(result).toEqual(config); + }); + }); + + describe("Validation Behavior", () => { + test("validation can transform and validate config", async () => { + const validatingDefinition = new ValidatingConfigDefinition(testDir); + const config = { + name: " test-name ", // Should be trimmed + version: 2, + enabled: true, + timeout: 1500, + }; + + const result = await validatingDefinition.validate(config); + expect(result.name).toBe("test-name"); // Trimmed + expect(result.timeout).toBe(1500); + }); + + test("validation errors are thrown for invalid configs", async () => { + const validatingDefinition = new ValidatingConfigDefinition(testDir); + const invalidConfig = { + name: "", + version: 0, + enabled: false, + timeout: 500, + }; + + await expect( + validatingDefinition.validate(invalidConfig) + ).rejects.toThrow(); + }); + }); + + describe("Migration Behavior", () => { + test("default migration throws not implemented error", async () => { + await expect(configDefinition.migrateConfig()).rejects.toThrow( + "Not implemented" + ); + }); + + test("custom migration works when implemented", async () => { + const migratingDefinition = new MigratingConfigDefinition(testDir); + const result = await migratingDefinition.migrateConfig(); + + expect(result).toEqual({ + name: "migrated", + version: 2, + enabled: true, + timeout: 3000, + }); + }); + + test("migration failures are propagated as errors", async () => { + const migratingDefinition = new MigratingConfigDefinition(testDir); + migratingDefinition.migrationShouldFail = true; + + await expect(migratingDefinition.migrateConfig()).rejects.toThrow( + "Migration failed" + ); + }); + }); +}); diff --git a/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts b/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts new file mode 100644 index 0000000000..e839f50631 --- /dev/null +++ b/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts @@ -0,0 +1,446 @@ +import { expect, test, describe, beforeEach, afterEach } from "bun:test"; +import { readFile, writeFile, mkdir, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { ConfigFileHandler } from "../config-file-handler.js"; +import { ConfigDefinition } from "../config-definition.js"; + +/** + * TEST SCOPE: ConfigFileHandler Standalone File Operations + * + * BEHAVIORS TESTED: + * • Configuration loading with error recovery cascade: + * - File exists & valid → load directly + * - File read fails → attempt migration → fallback to defaults + * - File valid but merged config fails validation → attempt migration + * - Migration succeeds but merged result fails validation → fallback to defaults + * - Migration fails → fallback to defaults + * • File I/O operations (read, write) with validation + * • Flash drive optimization (skip writes when config unchanged) + * • Partial config updates with deep merging + * • Error resilience (invalid JSON, validation failures, file system errors) + * • End-to-end workflows (load → update → reload cycles) + * + * CRITICAL ERROR RECOVERY PATHS: + * ✓ read failed → migration failed → defaults written + * ✓ read failed → migration succeeded but combo validation failed → defaults written + * ✓ read succeeded but merged validation failed → migration → recovery + * + * COVERAGE FOCUS: + * • Data integrity during all error scenarios + * • Performance optimization (change detection) + * • Configuration persistence reliability + * • Validation error handling at all stages + * + * NOT TESTED (covered in other files): + * • NestJS integration and reactive changes (ConfigFilePersister) + * • Abstract class behavior (ConfigDefinition) + */ + +interface TestConfig { + name: string; + version: number; + enabled: boolean; + timeout: number; + maxRetries?: number; // Optional field for testing merge validation +} + +class TestConfigDefinition extends ConfigDefinition { + public migrationCallCount = 0; + public migrationShouldFail = false; + public validationShouldFail = false; + public mergeValidationShouldFail = false; // New flag for the edge case + + constructor(private configDir: string) { + super("TestConfigDefinition"); + } + + fileName(): string { + return "test-config.json"; + } + + configPath(): string { + return join(this.configDir, this.fileName()); + } + + defaultConfig(): TestConfig { + return { + name: "test", + version: 1, + enabled: false, + timeout: 5000, + maxRetries: 3, // Default includes maxRetries + }; + } + + async validate(config: object): Promise { + if (this.validationShouldFail) { + throw new Error("Validation failed"); + } + + const testConfig = config as TestConfig; + + // Basic validation + if (typeof testConfig.version !== "number" || testConfig.version < 1) { + throw new Error("Invalid version: must be >= 1"); + } + + if (typeof testConfig.timeout !== "number" || testConfig.timeout < 1000) { + throw new Error("Invalid timeout: must be >= 1000"); + } + + // Critical edge case: maxRetries validation that could fail after merge + if (testConfig.maxRetries !== undefined && testConfig.maxRetries < 0) { + throw new Error("Invalid maxRetries: must be >= 0"); + } + + // Simulate a validation that fails specifically for merged configs + if (this.mergeValidationShouldFail && testConfig.maxRetries === -1) { + throw new Error("Merged validation failed: maxRetries cannot be -1"); + } + + return testConfig; + } + + async migrateConfig(): Promise { + this.migrationCallCount++; + + if (this.migrationShouldFail) { + throw new Error("Migration failed"); + } + + return { + name: "migrated", + version: 2, + enabled: true, + timeout: 3000, + maxRetries: 5, + }; + } +} + +describe("ConfigFileHandler", () => { + let testDir: string; + let configPath: string; + let configDefinition: TestConfigDefinition; + let fileHandler: ConfigFileHandler; + + beforeEach(async () => { + testDir = join(tmpdir(), `config-handler-test-${Date.now()}`); + await mkdir(testDir, { recursive: true }); + configPath = join(testDir, "test-config.json"); + + configDefinition = new TestConfigDefinition(testDir); + fileHandler = new ConfigFileHandler(configDefinition); + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + describe("Critical loadConfig Scenarios", () => { + test("loads valid config from file successfully", async () => { + const validConfig = { + name: "existing", + version: 2, + enabled: true, + timeout: 3000, + maxRetries: 2, + }; + await writeFile(configPath, JSON.stringify(validConfig)); + + const result = await fileHandler.loadConfig(); + expect(result.name).toBe("existing"); + expect(result.version).toBe(2); + expect(result.maxRetries).toBe(2); + }); + + test("falls back to migration when file doesn't exist", async () => { + const result = await fileHandler.loadConfig(); + + expect(configDefinition.migrationCallCount).toBe(1); + expect(result.name).toBe("migrated"); + expect(result.version).toBe(2); + + // Should persist migrated config + const persistedContent = await readFile(configPath, "utf8"); + const persistedConfig = JSON.parse(persistedContent); + expect(persistedConfig.name).toBe("migrated"); + }); + + test("falls back to defaults when migration fails", async () => { + configDefinition.migrationShouldFail = true; + + const result = await fileHandler.loadConfig(); + + expect(result.name).toBe("test"); // From defaults + expect(result.version).toBe(1); + }); + + test("CRITICAL: file valid but merged config fails validation - triggers migration", async () => { + // File contains valid config but defaults have invalid maxRetries + const fileConfig = { + name: "file-valid", + version: 2, + enabled: true, + timeout: 2000, + // Note: no maxRetries in file + }; + await writeFile(configPath, JSON.stringify(fileConfig)); + + // Override defaults to include invalid value that fails after merge + const originalDefaults = configDefinition.defaultConfig; + configDefinition.defaultConfig = () => ({ + name: "test", + version: 1, + enabled: false, + timeout: 5000, + maxRetries: -1, // This will cause merged validation to fail! + }); + + configDefinition.mergeValidationShouldFail = true; + + // This should NOT throw - should catch validation error and migrate + const result = await fileHandler.loadConfig(); + + // Should have triggered migration due to validation failure + expect(configDefinition.migrationCallCount).toBe(1); + expect(result.name).toBe("migrated"); + expect(result.maxRetries).toBe(5); // From migration + + // Restore original method + configDefinition.defaultConfig = originalDefaults; + }); + + test("handles invalid JSON by migrating", async () => { + await writeFile(configPath, "{ invalid json"); + + const result = await fileHandler.loadConfig(); + expect(configDefinition.migrationCallCount).toBe(1); + expect(result.name).toBe("migrated"); + }); + + test("CRITICAL: read failed → migration succeeded but merged validation fails → defaults used", async () => { + // No file exists (read will fail) + // Migration will succeed but return config that passes its own validation + // But when merged with defaults, the result fails validation + + // Create a special definition for this edge case + class SpecialMigrationDefinition extends TestConfigDefinition { + async migrateConfig(): Promise { + this.migrationCallCount++; + // Return a config that's valid on its own + return { + name: "migration-success", + version: 2, + enabled: true, + timeout: 2000, + // Missing maxRetries - will be merged from defaults + }; + } + + async validate(config: object): Promise { + const testConfig = config as TestConfig; + + // Basic validation + if ( + typeof testConfig.version !== "number" || + testConfig.version < 1 + ) { + throw new Error("Invalid version: must be >= 1"); + } + + if ( + typeof testConfig.timeout !== "number" || + testConfig.timeout < 1000 + ) { + throw new Error("Invalid timeout: must be >= 1000"); + } + + // This validation will fail after merge when maxRetries comes from defaults + if ( + testConfig.maxRetries !== undefined && + testConfig.name === "migration-success" && + testConfig.maxRetries === 3 + ) { + throw new Error( + "Special validation failure: migration + defaults combo invalid" + ); + } + + return testConfig; + } + } + + const specialDefinition = new SpecialMigrationDefinition(testDir); + const specialHandler = new ConfigFileHandler(specialDefinition); + + // Should NOT throw - should catch validation error and fall back to defaults + const result = await specialHandler.loadConfig(); + + // Should have attempted migration + expect(specialDefinition.migrationCallCount).toBe(1); + + // But result should be from defaults due to validation failure + expect(result.name).toBe("test"); // From defaults + expect(result.version).toBe(1); // From defaults + expect(result.maxRetries).toBe(3); // From defaults + }); + }); + + describe("File Operations", () => { + test("readConfigFile validates config from disk", async () => { + const config = { + name: "read-test", + version: 2, + enabled: true, + timeout: 2000, + }; + await writeFile(configPath, JSON.stringify(config)); + + const result = await fileHandler.readConfigFile(); + expect(result).toEqual(config); + }); + + test("readConfigFile throws for invalid config", async () => { + const invalidConfig = { + name: "invalid", + version: -1, + enabled: true, + timeout: 2000, + }; + await writeFile(configPath, JSON.stringify(invalidConfig)); + + await expect(fileHandler.readConfigFile()).rejects.toThrow( + "Invalid version" + ); + }); + + test("writeConfigFile persists config to disk", async () => { + const config = { + name: "write-test", + version: 3, + enabled: true, + timeout: 4000, + }; + + const success = await fileHandler.writeConfigFile(config); + expect(success).toBe(true); + + const fileContent = await readFile(configPath, "utf8"); + expect(JSON.parse(fileContent)).toEqual(config); + }); + + test("writeConfigFile skips write when config unchanged (flash drive optimization)", async () => { + const config = { + name: "unchanged", + version: 1, + enabled: false, + timeout: 5000, + }; + await writeFile(configPath, JSON.stringify(config, null, 2)); + + const success = await fileHandler.writeConfigFile(config); + expect(success).toBe(false); // Skipped + }); + + test("writeConfigFile handles validation errors", async () => { + configDefinition.validationShouldFail = true; + const config = { + name: "invalid", + version: 1, + enabled: false, + timeout: 5000, + }; + + const success = await fileHandler.writeConfigFile(config); + expect(success).toBe(false); + }); + }); + + describe("updateConfig Operations", () => { + test("updates existing config with partial changes", async () => { + const existing = { + name: "existing", + version: 1, + enabled: false, + timeout: 5000, + }; + await writeFile(configPath, JSON.stringify(existing)); + + const success = await fileHandler.updateConfig({ + enabled: true, + timeout: 8000, + }); + expect(success).toBe(true); + + const updated = JSON.parse(await readFile(configPath, "utf8")); + expect(updated.name).toBe("existing"); // Preserved + expect(updated.enabled).toBe(true); // Updated + expect(updated.timeout).toBe(8000); // Updated + }); + + test("creates config when file doesn't exist (via migration)", async () => { + const updates = { name: "new", enabled: true }; + + const success = await fileHandler.updateConfig(updates); + expect(success).toBe(true); + + const created = JSON.parse(await readFile(configPath, "utf8")); + expect(created.name).toBe("new"); // From update + expect(created.version).toBe(2); // From migration (no file existed) + }); + + test("handles validation errors during update", async () => { + const existing = { + name: "existing", + version: 1, + enabled: false, + timeout: 5000, + }; + await writeFile(configPath, JSON.stringify(existing)); + + const success = await fileHandler.updateConfig({ version: -1 }); // Invalid + expect(success).toBe(false); + + // Original should be unchanged + const unchanged = JSON.parse(await readFile(configPath, "utf8")); + expect(unchanged.version).toBe(1); + }); + }); + + describe("Error Resilience", () => { + test("handles write errors gracefully", async () => { + const invalidDefinition = new TestConfigDefinition( + "/invalid/readonly/path" + ); + const invalidHandler = new ConfigFileHandler(invalidDefinition); + + const config = { + name: "error-test", + version: 1, + enabled: false, + timeout: 5000, + }; + const success = await invalidHandler.writeConfigFile(config); + expect(success).toBe(false); + }); + }); + + describe("End-to-End Workflow", () => { + test("complete workflow: load -> update -> reload", async () => { + // 1. Load (triggers migration since no file) + let config = await fileHandler.loadConfig(); + expect(config.name).toBe("migrated"); + + // 2. Update + await fileHandler.updateConfig({ name: "workflow-test", timeout: 6000 }); + + // 3. Reload from disk + config = await fileHandler.readConfigFile(); + expect(config.name).toBe("workflow-test"); + expect(config.timeout).toBe(6000); + expect(config.version).toBe(2); // Preserved from migration + }); + }); +}); diff --git a/packages/unraid-shared/src/util/config-definition.ts b/packages/unraid-shared/src/util/config-definition.ts new file mode 100644 index 0000000000..ba4778d58c --- /dev/null +++ b/packages/unraid-shared/src/util/config-definition.ts @@ -0,0 +1,204 @@ +import { Logger } from "@nestjs/common"; + +/** + * Abstract base class that defines configuration behavior without NestJS dependencies. + * This can be extended and used both standalone and with NestJS integration. + * + * This class provides the core configuration logic including: + * - **File Path Resolution**: Abstract `configPath()` method for flexible path handling + * - **Default Configuration**: Fallback values when files don't exist or are corrupted + * - **Validation Logic**: Optional validation and transformation of config data + * - **Migration Support**: Legacy config conversion during upgrades + * + * @template T The configuration object type that extends object + * + * @example + * ```typescript + * interface MyConfig { + * enabled: boolean; + * timeout: number; + * apiKey?: string; + * } + * + * class MyConfigDefinition extends ConfigDefinition { + * constructor(private configDir: string) { + * super('MyConfig'); + * } + * + * fileName() { return "my-config.json"; } + * + * configPath() { + * return path.join(this.configDir, this.fileName()); + * } + * + * defaultConfig(): MyConfig { + * return { enabled: false, timeout: 5000 }; + * } + * + * async validate(config: object): Promise { + * const myConfig = config as MyConfig; + * if (myConfig.timeout < 1000) throw new Error("Timeout too low"); + * return myConfig; + * } + * + * async migrateConfig(): Promise { + * // Try to load from legacy location + * const legacyPath = path.join(this.configDir, 'old-config.ini'); + * // ... migration logic + * return { enabled: true, timeout: 3000 }; + * } + * } + * ``` + */ +export abstract class ConfigDefinition { + protected logger: Logger; + + /** + * @param loggerName Optional custom logger name (defaults to generic name) + */ + constructor(loggerName?: string) { + this.logger = new Logger(loggerName ?? `ConfigDefinition`); + } + + /** + * Returns the filename for the configuration file. + * + * @returns The name of the config file (e.g., "my-config.json") + * @example "user-preferences.json" + */ + abstract fileName(): string; + + /** + * Returns the absolute path to the configuration file. + * + * @returns Absolute path to the config file + * @example "/usr/local/etc/unraid/config/my-config.json" + */ + abstract configPath(): string; + + /** + * Returns the default configuration object. + * + * **Important**: This is used as a fallback when migration fails or as a base + * for merging with loaded/migrated configurations. + * + * @returns The default configuration object + * @example + * ```typescript + * defaultConfig(): MyConfig { + * return { + * enabled: false, + * timeout: 5000, + * retries: 3, + * features: { + * autoBackup: true, + * notifications: false + * } + * }; + * } + * ``` + */ + abstract defaultConfig(): T; + + /** + * Validates and transforms a configuration object. + * + * **Default Implementation**: Simple type cast (passthrough validation). + * Override this method to implement custom validation logic. + * + * **Common Use Cases**: + * - Schema validation (e.g., using Joi, Yup, or Zod) + * - Range checking for numeric values + * - Required field validation + * - Data transformation/normalization + * - Environment-specific validation + * + * @param config - The raw config object to validate + * @returns The validated and potentially transformed config + * @throws Error if the config is invalid (stops loading/migration process) + * + * @example + * ```typescript + * async validate(config: object): Promise { + * const myConfig = config as MyConfig; + * + * // Type validation + * if (typeof myConfig.timeout !== 'number' || myConfig.timeout < 1000) { + * throw new Error('Invalid timeout: must be number >= 1000'); + * } + * + * // Enum validation + * if (!['low', 'medium', 'high'].includes(myConfig.priority)) { + * throw new Error('Invalid priority level'); + * } + * + * // Data transformation + * myConfig.apiKey = myConfig.apiKey?.trim() || undefined; + * + * return myConfig; + * } + * ``` + */ + async validate(config: object): Promise { + return config as T; + } + + /** + * Migrates legacy or corrupted configuration to the current format. + * + * **Default Implementation**: Throws "Migration not implemented" error. + * Override this method to provide custom migration logic. + * + * **When Called**: + * - Config file doesn't exist (first-time setup) + * - Config file contains invalid JSON + * - Config validation fails + * - File system read errors + * + * **Migration Strategies**: + * - **Legacy Format**: Convert old config structure to new format + * - **Partial Config**: Fill missing properties with sensible defaults + * - **Corrupted Data**: Attempt to salvage usable parts + * - **Fresh Install**: Return initial setup configuration + * - **Version Upgrades**: Handle breaking changes between config versions + * + * **Priority**: Migration is attempted before falling back to `defaultConfig()`, + * making this ideal for handling upgrades and first-time installations. + * + * @returns Migrated configuration object + * @throws Error if migration is not possible (falls back to defaults) + * + * @example + * ```typescript + * async migrateConfig(): Promise { + * try { + * // Try to load legacy config format + * const legacyPath = path.join(this.configDir, 'old-config.ini'); + * const legacyData = await this.readLegacyIniFile(legacyPath); + * + * return { + * enabled: legacyData.isEnabled === 'true', + * timeout: parseInt(legacyData.timeoutMs) || 5000, + * apiKey: legacyData.secret, + * newFeature: 'default-value' // New property not in legacy + * }; + * } catch (legacyError) { + * // Try environment variables for first-time setup + * if (process.env.MY_CONFIG_ENABLED) { + * return { + * enabled: process.env.MY_CONFIG_ENABLED === 'true', + * timeout: parseInt(process.env.MY_CONFIG_TIMEOUT) || 3000, + * newFeature: 'env-setup' + * }; + * } + * + * // Last resort: throw to use defaults + * throw new Error('No migration path available'); + * } + * } + * ``` + */ + async migrateConfig(): Promise { + throw new Error("Not implemented"); + } +} diff --git a/packages/unraid-shared/src/util/config-file-handler.ts b/packages/unraid-shared/src/util/config-file-handler.ts new file mode 100644 index 0000000000..fba97258b8 --- /dev/null +++ b/packages/unraid-shared/src/util/config-file-handler.ts @@ -0,0 +1,242 @@ +import { Logger } from "@nestjs/common"; +import { readFile, writeFile } from "node:fs/promises"; +import { isEqual } from "lodash-es"; +import { ConfigDefinition } from "./config-definition.js"; +import { fileExists } from "./file.js"; + +/** + * Standalone configuration file handler that works with any ConfigDefinition. + * Can be used independently of NestJS DI container. + * + * This class provides robust file operations with the following features: + * - **Migration Priority**: When files don't exist, migration is attempted before falling back to defaults + * - **Change Detection**: Uses deep equality checks to avoid unnecessary disk writes (flash drive optimization) + * - **Error Resilience**: Graceful handling of file system errors, JSON parsing failures, and validation errors + * - **Atomic Operations**: Individual methods for specific file operations (read, write, update) + * + * @template T The configuration object type that extends object + * + * @example + * ```typescript + * // Standalone usage + * const configDef = new MyConfigDefinition('/etc/myapp'); + * const fileHandler = new ConfigFileHandler(configDef); + * + * // Load config with migration fallback + * const config = await fileHandler.loadConfig(); + * + * // Update specific properties + * await fileHandler.updateConfig({ enabled: true, timeout: 8000 }); + * + * // Direct file operations + * const currentConfig = await fileHandler.readConfigFile(); + * await fileHandler.writeConfigFile(newConfig); + * ``` + */ +export class ConfigFileHandler { + private readonly logger: Logger; + + /** + * @param definition The configuration definition that provides behavior + */ + constructor(private readonly definition: ConfigDefinition) { + this.logger = new Logger(`ConfigFileHandler:${definition.fileName()}`); + } + + /** + * Loads configuration from file, with migration fallback. + * + * **Migration Priority Strategy**: + * 1. **Load**: Attempts to load and validate existing config from disk + * 2. **Migrate**: If loading fails, attempts migration using `migrateConfig()` + * 3. **Default**: If migration fails, falls back to `defaultConfig()` + * 4. **Merge**: Merges result with defaults to ensure all properties exist + * 5. **Persist**: If migration occurred, writes final config to disk + * + * **Key Insight**: Migration is attempted before using defaults, making this suitable + * for upgrading legacy configurations or handling first-time installations. + * + * **Error Handling**: All errors are logged at WARN level, ensuring the system + * continues to function with sensible defaults even if file system issues occur. + * + * @returns Complete configuration object (defaults + loaded/migrated data) + * + * @example + * ```typescript + * // Load config - handles all error cases gracefully + * const config = await fileHandler.loadConfig(); + * console.log('Loaded config:', config); + * + * // Always returns valid config object, even if file doesn't exist + * const alwaysValid = await fileHandler.loadConfig(); + * console.log('Timeout:', alwaysValid.timeout); // Safe to access + * ``` + */ + async loadConfig(): Promise { + const defaultConfig = this.definition.defaultConfig(); + + try { + const fileConfig = await this.readConfigFile(); + return await this.definition.validate({ ...defaultConfig, ...fileConfig }); + } catch (error) { + this.logger.warn(error, "Error loading config. Attempting to migrate..."); + + try { + const migratedConfig = await this.definition.migrateConfig(); + const mergedConfig = await this.definition.validate({ + ...defaultConfig, + ...migratedConfig, + }); + // Persist migrated config for future loads + await this.writeConfigFile(mergedConfig); + return mergedConfig; + } catch (migrationError) { + this.logger.warn("Migration failed. Using defaults.", migrationError); + return defaultConfig; + } + } + } + + /** + * Reads and validates configuration from file. + * + * **Process**: + * 1. Checks if file exists using `fileExists()` utility + * 2. Reads file content as UTF-8 text + * 3. Parses JSON content + * 4. Validates result using `validate()` method from definition + * + * **Error Cases**: + * - File doesn't exist → Error (triggers migration in caller) + * - Invalid JSON syntax → Error (triggers migration in caller) + * - Validation failure → Error (triggers migration in caller) + * + * @param configPath - Path to config file (defaults to `configPath()`) + * @returns Validated configuration object from disk + * @throws Error if file doesn't exist, contains invalid JSON, or fails validation + * + * @example + * ```typescript + * try { + * const config = await fileHandler.readConfigFile(); + * console.log('Successfully loaded config from disk:', config); + * } catch (error) { + * console.log('Config file issue, will attempt migration/defaults'); + * } + * ``` + */ + async readConfigFile(configPath = this.definition.configPath()): Promise { + if (!(await fileExists(configPath))) { + throw new Error(`Config file does not exist at '${configPath}'`); + } + const content = await readFile(configPath, "utf8"); + const parsed = JSON.parse(content); + return await this.definition.validate(parsed); + } + + /** + * Writes configuration to file with intelligent change detection. + * + * **Flash Drive Optimization**: Uses deep equality checks to avoid unnecessary writes, + * helping preserve the lifespan of boot flash drives by preventing redundant I/O operations. + * + * **Process**: + * 1. Validates config using definition's `validate()` method + * 2. Compares new config with existing file content using deep equality + * 3. Skips write if content is identical (logs at verbose level) + * 4. Writes pretty-printed JSON (2-space indentation) if changes detected + * 5. Handles file system errors gracefully + * + * @param config - The config object to write to disk + * @returns `true` if the config was written to disk, `false` if skipped (no changes) or failed + * + * @example + * ```typescript + * const newConfig = { enabled: true, timeout: 8000 }; + * + * // Write config - automatically skips if unchanged + * const written = await fileHandler.writeConfigFile(newConfig); + * if (written) { + * console.log('Config updated on disk'); + * } else { + * console.log('Config unchanged, skipped write'); + * } + * ``` + */ + async writeConfigFile(config: T): Promise { + try { + config = await this.definition.validate(config); + } catch (error) { + this.logger.error(error, `Cannot write invalid config`); + return false; + } + + // Skip write if config is unchanged (flash drive optimization) + try { + const existingConfig = await this.readConfigFile(); + if (isEqual(config, existingConfig)) { + this.logger.verbose(`Config is unchanged, skipping write`); + return false; + } + } catch (error) { + // File doesn't exist or is invalid, proceed with write + this.logger.verbose(`Existing config unreadable, proceeding with write`); + } + + const data = JSON.stringify(config, null, 2); + this.logger.verbose( + `Writing config to ${this.definition.configPath()}: ${data}` + ); + + try { + await writeFile(this.definition.configPath(), data); + return true; + } catch (error) { + this.logger.error( + error, + `Error writing config to '${this.definition.configPath()}'` + ); + return false; + } + } + + /** + * Updates configuration by merging with existing config. + * + * **Process**: + * 1. Loads current config from disk (with migration/defaults fallback) + * 2. Merges provided updates with current config + * 3. Writes merged result back to disk + * + * This is ideal for making partial updates without losing other configuration values. + * + * @param updates - Partial configuration object with properties to update + * @returns `true` if the config was updated on disk, `false` if failed or no changes + * + * @example + * ```typescript + * // Update just the timeout, keep other settings unchanged + * const updated = await fileHandler.updateConfig({ timeout: 10000 }); + * + * // Update multiple properties + * const multiUpdate = await fileHandler.updateConfig({ + * enabled: true, + * timeout: 8000, + * features: { autoBackup: false } + * }); + * ``` + */ + async updateConfig(updates: Partial): Promise { + try { + const currentConfig = await this.loadConfig(); + const newConfig = await this.definition.validate({ + ...currentConfig, + ...updates, + }); + return await this.writeConfigFile(newConfig); + } catch (error) { + this.logger.error("Failed to update config", error); + return false; + } + } +} From 95f7d551b92f164bd386a830d8f04a8dcd650dca Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 15:31:00 -0400 Subject: [PATCH 12/15] refactor: loadApiConfig --- .../unraid-api/config/api-config.module.ts | 26 ++++++++++++------- .../unraid-shared/src/services/config-file.ts | 3 +-- .../src/util/config-definition.ts | 2 +- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index fa2adbe1d1..642ebef2c2 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -1,12 +1,10 @@ import { Injectable, Logger, Module } from '@nestjs/common'; import { ConfigService, registerAs } from '@nestjs/config'; -import { readFile } from 'fs/promises'; import path from 'path'; import type { ApiConfig } from '@unraid/shared/services/api-config.js'; import { ConfigFilePersister } from '@unraid/shared/services/config-file.js'; import { csvStringToArray } from '@unraid/shared/util/data.js'; -import { fileExists } from '@unraid/shared/util/file.js'; import { API_VERSION, PATHS_CONFIG_MODULES } from '@app/environment.js'; @@ -28,17 +26,11 @@ const createDefaultConfig = (): ApiConfig => ({ */ export const loadApiConfig = async () => { const defaultConfig = createDefaultConfig(); - const configPath = path.join(PATHS_CONFIG_MODULES, 'api.json'); + const apiHandler = new ApiConfigPersistence(new ConfigService()).getFileHandler(); let diskConfig: Partial = {}; - try { - if (await fileExists(configPath)) { - const fileContent = await readFile(configPath, 'utf8'); - if (fileContent.trim()) { - diskConfig = JSON.parse(fileContent); - } - } + diskConfig = await apiHandler.loadConfig(); } catch (error) { logger.warn('Failed to load API config from disk:', error); } @@ -46,12 +38,14 @@ export const loadApiConfig = async () => { return { ...defaultConfig, ...diskConfig, + // diskConfig's version may be older, but we still want to use the correct version version: API_VERSION, }; }; /** * Loads the API config from disk. If not found, returns the default config, but does not persist it. + * This is used in the root config module to register the api config. */ export const apiConfig = registerAs('api', loadApiConfig); @@ -69,6 +63,18 @@ export class ApiConfigPersistence extends ConfigFilePersister { return 'api'; } + /** + * @override + * Since the api config is read outside of the nestjs DI container, + * we need to provide an explicit path instead of relying on the + * default prefix from the configService. + * + * @returns The path to the api config file + */ + configPath(): string { + return path.join(PATHS_CONFIG_MODULES, this.fileName()); + } + defaultConfig(): ApiConfig { return createDefaultConfig(); } diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index 55f9929200..ed2e16b122 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -99,8 +99,7 @@ export abstract class ConfigFilePersister * @param configService The NestJS ConfigService instance for reactive config management */ constructor(protected readonly configService: ConfigService) { - super(`ConfigFilePersister`); - // Update logger name after fileName() is available + super(); this.logger = new Logger(`ConfigFilePersister:${this.fileName()}`); this.fileHandler = new ConfigFileHandler(this); } diff --git a/packages/unraid-shared/src/util/config-definition.ts b/packages/unraid-shared/src/util/config-definition.ts index ba4778d58c..1d6c8ffd77 100644 --- a/packages/unraid-shared/src/util/config-definition.ts +++ b/packages/unraid-shared/src/util/config-definition.ts @@ -57,7 +57,7 @@ export abstract class ConfigDefinition { * @param loggerName Optional custom logger name (defaults to generic name) */ constructor(loggerName?: string) { - this.logger = new Logger(loggerName ?? `ConfigDefinition`); + this.logger = new Logger(loggerName ?? `ConfigDefinition:${this.fileName()}`); } /** From c37ae5db27793cac41a543d4c6381e62ce519879 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 15:47:54 -0400 Subject: [PATCH 13/15] improve code doc clarity --- .../unraid-shared/src/services/config-file.ts | 159 ++---------------- .../src/util/config-definition.ts | 136 ++------------- .../src/util/config-file-handler.ts | 111 ++---------- 3 files changed, 44 insertions(+), 362 deletions(-) diff --git a/packages/unraid-shared/src/services/config-file.ts b/packages/unraid-shared/src/services/config-file.ts index ed2e16b122..ab4764e0a3 100644 --- a/packages/unraid-shared/src/services/config-file.ts +++ b/packages/unraid-shared/src/services/config-file.ts @@ -12,30 +12,15 @@ import { ConfigFileHandler } from "../util/config-file-handler.js"; import { ConfigDefinition } from "../util/config-definition.js"; /** - * Abstract base class for persisting configuration objects to JSON files on disk. + * Abstract base class for persisting configuration objects to JSON files. * - * This class provides a robust configuration persistence layer that integrates with NestJS - * while also providing standalone file operations through ConfigFileHandler delegation. - * - * **Key Features**: - * - **NestJS Integration**: Integrates with ConfigService and lifecycle hooks - * - **Reactive Updates**: Subscribes to config changes with 25ms buffering to reduce I/O operations - * - **Standalone Access**: Provides `getFileHandler()` for direct file operations outside NestJS - * - **Method Overrides**: Supports overriding `configPath()`, `validate()`, `migrateConfig()` etc. - * - **Flash Drive Optimization**: Uses change detection to minimize unnecessary writes - * - **Error Resilience**: Graceful handling of all error conditions - * - **Lifecycle Management**: Proper cleanup of subscriptions and final persistence on destruction + * Provides NestJS integration with reactive config updates, standalone file operations, + * and lifecycle management with automatic persistence. * * @template T The configuration object type that extends object * * @example * ```typescript - * interface MyConfig { - * enabled: boolean; - * timeout: number; - * apiEndpoint: string; - * } - * * @Injectable() * class MyConfigPersister extends ConfigFilePersister { * constructor(configService: ConfigService) { @@ -44,43 +29,10 @@ import { ConfigDefinition } from "../util/config-definition.js"; * * fileName() { return "my-config.json"; } * configKey() { return "myConfig"; } - * * defaultConfig(): MyConfig { - * return { - * enabled: false, - * timeout: 5000, - * apiEndpoint: 'https://api.example.com' - * }; - * } - * - * // Override path resolution if needed - * configPath(): string { - * return path.join('/custom/path', this.fileName()); - * } - * - * async validate(config: object): Promise { - * const myConfig = config as MyConfig; - * if (myConfig.timeout < 1000) throw new Error("Timeout too low"); - * return myConfig; - * } - * - * async migrateConfig(): Promise { - * // Load from legacy location or format - * const legacyConfig = await this.loadLegacyConfig(); - * return { - * enabled: legacyConfig.isEnabled ?? true, - * timeout: legacyConfig.timeoutMs ?? 3000, - * apiEndpoint: legacyConfig.endpoint ?? 'https://api.example.com' - * }; + * return { enabled: false, timeout: 5000 }; * } * } - * - * // Usage in NestJS: - * const myConfig = configService.get('myConfig'); - * - * // Usage standalone (outside NestJS): - * const fileHandler = myConfigPersister.getFileHandler(); - * await fileHandler.updateConfig({ timeout: 8000 }); * ``` */ export abstract class ConfigFilePersister @@ -92,10 +44,7 @@ export abstract class ConfigFilePersister /** * Creates a new ConfigFilePersister instance. - * - * **Note**: The fileHandler is initialized after the constructor completes - * to ensure all abstract methods are available. - * + * * @param configService The NestJS ConfigService instance for reactive config management */ constructor(protected readonly configService: ConfigService) { @@ -119,19 +68,9 @@ export abstract class ConfigFilePersister /** * Returns the absolute path to the configuration file. - * Overrides parent to use NestJS ConfigService for path resolution. - * - * **Default Implementation**: Combines the `PATHS_CONFIG_MODULES` environment variable - * with the filename to create the full path where the config file should be stored. - * - * **Override Examples**: - * - Custom directory: `path.join('/custom/config/dir', this.fileName())` - * - User-specific: `path.join(os.homedir(), '.myapp', this.fileName())` - * - Environment-based: `path.join(process.env.CONFIG_DIR, this.fileName())` - * - * @returns Absolute path to the config file + * Combines `PATHS_CONFIG_MODULES` environment variable with the filename. + * * @throws Error if `PATHS_CONFIG_MODULES` environment variable is not set - * @example "/usr/local/etc/unraid/config/my-config.json" */ configPath(): string { return path.join( @@ -141,48 +80,15 @@ export abstract class ConfigFilePersister } /** - * Returns a standalone ConfigFileHandler for direct file operations. - * This allows the same config logic to be used outside of NestJS lifecycle. - * - * **Use Cases**: - * - Reading config during application startup (before NestJS is ready) - * - Standalone scripts that need config access - * - Testing without full NestJS context - * - Background jobs that run outside the main application - * - * @returns ConfigFileHandler instance for direct file operations - * - * @example - * ```typescript - * // In a NestJS service - * @Injectable() - * class MyService { - * constructor(private configPersister: MyConfigPersister) {} - * - * async updateConfigDirectly() { - * const fileHandler = this.configPersister.getFileHandler(); - * await fileHandler.updateConfig({ enabled: true }); - * } - * } - * - * // In a standalone script - * const configPersister = new MyConfigPersister(mockConfigService); - * const fileHandler = configPersister.getFileHandler(); - * const config = await fileHandler.loadConfig(); - * ``` + * Returns a standalone ConfigFileHandler for direct file operations outside NestJS. */ getFileHandler(): ConfigFileHandler { return this.fileHandler; } /** - * NestJS lifecycle hook called when the module is being destroyed. - * - * Performs cleanup by: - * 1. Unsubscribing from config change notifications to prevent memory leaks - * 2. Persisting any final configuration state to disk to ensure no data loss - * - * This ensures that configuration changes made just before shutdown are not lost. + * NestJS lifecycle hook for cleanup. + * Unsubscribes from config changes and persists final state. */ async onModuleDestroy() { this.configObserver?.unsubscribe(); @@ -190,19 +96,8 @@ export abstract class ConfigFilePersister } /** - * NestJS lifecycle hook called when the module is initialized. - * - * Performs initialization by: - * 1. Loading existing config from disk (with migration fallback) - * 2. Setting loaded config in the ConfigService for reactive access - * 3. Setting up reactive config change subscription with 25ms buffering - * 4. Filtering changes to only process events matching this config's key - * - * **Key Behavior**: Migration is prioritized over defaults when files don't exist. - * The 25ms buffer reduces disk I/O by batching rapid config changes. - * - * **Config Change Detection**: Only persists when changes occur to this specific - * config key, preventing unnecessary writes when other configs change. + * NestJS lifecycle hook for initialization. + * Loads config from disk and sets up reactive change subscription. */ async onModuleInit() { this.logger.verbose(`Config path: ${this.configPath()}`); @@ -226,26 +121,10 @@ export abstract class ConfigFilePersister } /** - * Persists the configuration to disk using the underlying file handler. - * - * **Features**: - * - Validates config before writing - * - Uses change detection to avoid unnecessary writes (flash drive optimization) - * - Handles all error conditions gracefully - * - Logs operations at appropriate levels - * + * Persists configuration to disk with change detection optimization. + * * @param config - The config object to persist (defaults to current config from service) - * @returns `true` if the config was persisted to disk, `false` if skipped or failed - * - * @example - * ```typescript - * // Persist current config from ConfigService - * const persisted = await persister.persist(); - * - * // Persist specific config object - * const customConfig = { enabled: true, timeout: 8000 }; - * const persisted = await persister.persist(customConfig); - * ``` + * @returns `true` if persisted to disk, `false` if skipped or failed */ async persist( config = this.configService.get(this.configKey()) @@ -259,14 +138,6 @@ export abstract class ConfigFilePersister /** * Load or migrate configuration and set it in ConfigService. - * - * This is the core initialization method that: - * 1. Uses the file handler to load config (with migration fallback) - * 2. Sets the loaded config in the NestJS ConfigService - * 3. Persists the final config to disk (important for migrations) - * - * @returns `true` if config was successfully persisted, `false` if persistence failed - * @private */ private async loadOrMigrateConfig() { const config = await this.fileHandler.loadConfig(); diff --git a/packages/unraid-shared/src/util/config-definition.ts b/packages/unraid-shared/src/util/config-definition.ts index 1d6c8ffd77..5ce761834a 100644 --- a/packages/unraid-shared/src/util/config-definition.ts +++ b/packages/unraid-shared/src/util/config-definition.ts @@ -1,14 +1,9 @@ import { Logger } from "@nestjs/common"; /** - * Abstract base class that defines configuration behavior without NestJS dependencies. - * This can be extended and used both standalone and with NestJS integration. - * - * This class provides the core configuration logic including: - * - **File Path Resolution**: Abstract `configPath()` method for flexible path handling - * - **Default Configuration**: Fallback values when files don't exist or are corrupted - * - **Validation Logic**: Optional validation and transformation of config data - * - **Migration Support**: Legacy config conversion during upgrades + * Abstract base class for configuration behavior without NestJS dependencies. + * Provides core configuration logic including file path resolution, defaults, + * validation, and migration support. * * @template T The configuration object type that extends object * @@ -17,7 +12,6 @@ import { Logger } from "@nestjs/common"; * interface MyConfig { * enabled: boolean; * timeout: number; - * apiKey?: string; * } * * class MyConfigDefinition extends ConfigDefinition { @@ -26,27 +20,14 @@ import { Logger } from "@nestjs/common"; * } * * fileName() { return "my-config.json"; } - * - * configPath() { - * return path.join(this.configDir, this.fileName()); - * } - * - * defaultConfig(): MyConfig { - * return { enabled: false, timeout: 5000 }; - * } + * configPath() { return path.join(this.configDir, this.fileName()); } + * defaultConfig(): MyConfig { return { enabled: false, timeout: 5000 }; } * * async validate(config: object): Promise { * const myConfig = config as MyConfig; * if (myConfig.timeout < 1000) throw new Error("Timeout too low"); * return myConfig; * } - * - * async migrateConfig(): Promise { - * // Try to load from legacy location - * const legacyPath = path.join(this.configDir, 'old-config.ini'); - * // ... migration logic - * return { enabled: true, timeout: 3000 }; - * } * } * ``` */ @@ -70,74 +51,26 @@ export abstract class ConfigDefinition { /** * Returns the absolute path to the configuration file. - * - * @returns Absolute path to the config file - * @example "/usr/local/etc/unraid/config/my-config.json" */ abstract configPath(): string; /** * Returns the default configuration object. - * - * **Important**: This is used as a fallback when migration fails or as a base - * for merging with loaded/migrated configurations. - * - * @returns The default configuration object - * @example - * ```typescript - * defaultConfig(): MyConfig { - * return { - * enabled: false, - * timeout: 5000, - * retries: 3, - * features: { - * autoBackup: true, - * notifications: false - * } - * }; - * } - * ``` + * Used as fallback when migration fails or as base for merging. */ abstract defaultConfig(): T; /** * Validates and transforms a configuration object. * - * **Default Implementation**: Simple type cast (passthrough validation). - * Override this method to implement custom validation logic. - * - * **Common Use Cases**: - * - Schema validation (e.g., using Joi, Yup, or Zod) + * Override to implement custom validation logic such as: + * - Schema validation * - Range checking for numeric values - * - Required field validation * - Data transformation/normalization - * - Environment-specific validation * * @param config - The raw config object to validate * @returns The validated and potentially transformed config - * @throws Error if the config is invalid (stops loading/migration process) - * - * @example - * ```typescript - * async validate(config: object): Promise { - * const myConfig = config as MyConfig; - * - * // Type validation - * if (typeof myConfig.timeout !== 'number' || myConfig.timeout < 1000) { - * throw new Error('Invalid timeout: must be number >= 1000'); - * } - * - * // Enum validation - * if (!['low', 'medium', 'high'].includes(myConfig.priority)) { - * throw new Error('Invalid priority level'); - * } - * - * // Data transformation - * myConfig.apiKey = myConfig.apiKey?.trim() || undefined; - * - * return myConfig; - * } - * ``` + * @throws Error if the config is invalid */ async validate(config: object): Promise { return config as T; @@ -146,57 +79,20 @@ export abstract class ConfigDefinition { /** * Migrates legacy or corrupted configuration to the current format. * - * **Default Implementation**: Throws "Migration not implemented" error. - * Override this method to provide custom migration logic. - * - * **When Called**: + * Called when: * - Config file doesn't exist (first-time setup) * - Config file contains invalid JSON * - Config validation fails - * - File system read errors - * - * **Migration Strategies**: - * - **Legacy Format**: Convert old config structure to new format - * - **Partial Config**: Fill missing properties with sensible defaults - * - **Corrupted Data**: Attempt to salvage usable parts - * - **Fresh Install**: Return initial setup configuration - * - **Version Upgrades**: Handle breaking changes between config versions * - * **Priority**: Migration is attempted before falling back to `defaultConfig()`, - * making this ideal for handling upgrades and first-time installations. + * Override to provide custom migration logic for legacy formats, + * version upgrades, or first-time installations. + * + * Note: + * - Backwards-compatible updates such as field additions are better handled via `defaultConfig()` + * because `defaultConfig()` is merged with the loaded config. * * @returns Migrated configuration object * @throws Error if migration is not possible (falls back to defaults) - * - * @example - * ```typescript - * async migrateConfig(): Promise { - * try { - * // Try to load legacy config format - * const legacyPath = path.join(this.configDir, 'old-config.ini'); - * const legacyData = await this.readLegacyIniFile(legacyPath); - * - * return { - * enabled: legacyData.isEnabled === 'true', - * timeout: parseInt(legacyData.timeoutMs) || 5000, - * apiKey: legacyData.secret, - * newFeature: 'default-value' // New property not in legacy - * }; - * } catch (legacyError) { - * // Try environment variables for first-time setup - * if (process.env.MY_CONFIG_ENABLED) { - * return { - * enabled: process.env.MY_CONFIG_ENABLED === 'true', - * timeout: parseInt(process.env.MY_CONFIG_TIMEOUT) || 3000, - * newFeature: 'env-setup' - * }; - * } - * - * // Last resort: throw to use defaults - * throw new Error('No migration path available'); - * } - * } - * ``` */ async migrateConfig(): Promise { throw new Error("Not implemented"); diff --git a/packages/unraid-shared/src/util/config-file-handler.ts b/packages/unraid-shared/src/util/config-file-handler.ts index fba97258b8..9d7ffecd9f 100644 --- a/packages/unraid-shared/src/util/config-file-handler.ts +++ b/packages/unraid-shared/src/util/config-file-handler.ts @@ -18,7 +18,6 @@ import { fileExists } from "./file.js"; * * @example * ```typescript - * // Standalone usage * const configDef = new MyConfigDefinition('/etc/myapp'); * const fileHandler = new ConfigFileHandler(configDef); * @@ -26,11 +25,7 @@ import { fileExists } from "./file.js"; * const config = await fileHandler.loadConfig(); * * // Update specific properties - * await fileHandler.updateConfig({ enabled: true, timeout: 8000 }); - * - * // Direct file operations - * const currentConfig = await fileHandler.readConfigFile(); - * await fileHandler.writeConfigFile(newConfig); + * await fileHandler.updateConfig({ enabled: true }); * ``` */ export class ConfigFileHandler { @@ -46,31 +41,13 @@ export class ConfigFileHandler { /** * Loads configuration from file, with migration fallback. * - * **Migration Priority Strategy**: - * 1. **Load**: Attempts to load and validate existing config from disk - * 2. **Migrate**: If loading fails, attempts migration using `migrateConfig()` - * 3. **Default**: If migration fails, falls back to `defaultConfig()` - * 4. **Merge**: Merges result with defaults to ensure all properties exist - * 5. **Persist**: If migration occurred, writes final config to disk - * - * **Key Insight**: Migration is attempted before using defaults, making this suitable - * for upgrading legacy configurations or handling first-time installations. - * - * **Error Handling**: All errors are logged at WARN level, ensuring the system - * continues to function with sensible defaults even if file system issues occur. - * - * @returns Complete configuration object (defaults + loaded/migrated data) - * - * @example - * ```typescript - * // Load config - handles all error cases gracefully - * const config = await fileHandler.loadConfig(); - * console.log('Loaded config:', config); + * Strategy: + * 1. Load and validate existing config + * 2. If loading fails, attempt migration + * 3. If migration fails, use defaults + * 4. Merge result with defaults and persist if migrated * - * // Always returns valid config object, even if file doesn't exist - * const alwaysValid = await fileHandler.loadConfig(); - * console.log('Timeout:', alwaysValid.timeout); // Safe to access - * ``` + * @returns Complete configuration object */ async loadConfig(): Promise { const defaultConfig = this.definition.defaultConfig(); @@ -99,31 +76,10 @@ export class ConfigFileHandler { /** * Reads and validates configuration from file. - * - * **Process**: - * 1. Checks if file exists using `fileExists()` utility - * 2. Reads file content as UTF-8 text - * 3. Parses JSON content - * 4. Validates result using `validate()` method from definition - * - * **Error Cases**: - * - File doesn't exist → Error (triggers migration in caller) - * - Invalid JSON syntax → Error (triggers migration in caller) - * - Validation failure → Error (triggers migration in caller) - * + * * @param configPath - Path to config file (defaults to `configPath()`) * @returns Validated configuration object from disk * @throws Error if file doesn't exist, contains invalid JSON, or fails validation - * - * @example - * ```typescript - * try { - * const config = await fileHandler.readConfigFile(); - * console.log('Successfully loaded config from disk:', config); - * } catch (error) { - * console.log('Config file issue, will attempt migration/defaults'); - * } - * ``` */ async readConfigFile(configPath = this.definition.configPath()): Promise { if (!(await fileExists(configPath))) { @@ -135,33 +91,11 @@ export class ConfigFileHandler { } /** - * Writes configuration to file with intelligent change detection. - * - * **Flash Drive Optimization**: Uses deep equality checks to avoid unnecessary writes, - * helping preserve the lifespan of boot flash drives by preventing redundant I/O operations. - * - * **Process**: - * 1. Validates config using definition's `validate()` method - * 2. Compares new config with existing file content using deep equality - * 3. Skips write if content is identical (logs at verbose level) - * 4. Writes pretty-printed JSON (2-space indentation) if changes detected - * 5. Handles file system errors gracefully + * Writes configuration to file with change detection optimization. + * Uses deep equality checks to avoid unnecessary writes. * * @param config - The config object to write to disk - * @returns `true` if the config was written to disk, `false` if skipped (no changes) or failed - * - * @example - * ```typescript - * const newConfig = { enabled: true, timeout: 8000 }; - * - * // Write config - automatically skips if unchanged - * const written = await fileHandler.writeConfigFile(newConfig); - * if (written) { - * console.log('Config updated on disk'); - * } else { - * console.log('Config unchanged, skipped write'); - * } - * ``` + * @returns `true` if written to disk, `false` if skipped or failed */ async writeConfigFile(config: T): Promise { try { @@ -202,29 +136,10 @@ export class ConfigFileHandler { /** * Updates configuration by merging with existing config. - * - * **Process**: - * 1. Loads current config from disk (with migration/defaults fallback) - * 2. Merges provided updates with current config - * 3. Writes merged result back to disk - * - * This is ideal for making partial updates without losing other configuration values. + * Loads current config, shallow merges updates, and writes back to disk. * * @param updates - Partial configuration object with properties to update - * @returns `true` if the config was updated on disk, `false` if failed or no changes - * - * @example - * ```typescript - * // Update just the timeout, keep other settings unchanged - * const updated = await fileHandler.updateConfig({ timeout: 10000 }); - * - * // Update multiple properties - * const multiUpdate = await fileHandler.updateConfig({ - * enabled: true, - * timeout: 8000, - * features: { autoBackup: false } - * }); - * ``` + * @returns `true` if updated on disk, `false` if failed or no changes */ async updateConfig(updates: Partial): Promise { try { From f0085623d31850a7efff8f1a0ea6a18348bdff90 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 22 Jul 2025 16:00:37 -0400 Subject: [PATCH 14/15] add test case for writing when existing file has invalid json --- .../__tests__/config-file-handler.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts b/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts index e839f50631..38a56f989e 100644 --- a/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts +++ b/packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts @@ -344,6 +344,26 @@ describe("ConfigFileHandler", () => { expect(success).toBe(false); // Skipped }); + test("writeConfigFile proceeds with write when existing file has invalid JSON", async () => { + // Pre-existing file with invalid JSON + await writeFile(configPath, "{ invalid json"); + + const config = { + name: "write-despite-invalid", + version: 2, + enabled: true, + timeout: 4000, + }; + + // Should proceed with write despite invalid existing file + const success = await fileHandler.writeConfigFile(config); + expect(success).toBe(true); + + // Should have written valid config + const fileContent = await readFile(configPath, "utf8"); + expect(JSON.parse(fileContent)).toEqual(config); + }); + test("writeConfigFile handles validation errors", async () => { configDefinition.validationShouldFail = true; const config = { From abcafb9fe86235a953ade8d17731fefd22b77efc Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 23 Jul 2025 10:14:43 -0400 Subject: [PATCH 15/15] fix: catch potential exceptions from JSON.stringify in writeConfigFile --- .../unraid-shared/src/util/config-file-handler.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/unraid-shared/src/util/config-file-handler.ts b/packages/unraid-shared/src/util/config-file-handler.ts index 9d7ffecd9f..29946e0f3d 100644 --- a/packages/unraid-shared/src/util/config-file-handler.ts +++ b/packages/unraid-shared/src/util/config-file-handler.ts @@ -54,7 +54,10 @@ export class ConfigFileHandler { try { const fileConfig = await this.readConfigFile(); - return await this.definition.validate({ ...defaultConfig, ...fileConfig }); + return await this.definition.validate({ + ...defaultConfig, + ...fileConfig, + }); } catch (error) { this.logger.warn(error, "Error loading config. Attempting to migrate..."); @@ -76,7 +79,7 @@ export class ConfigFileHandler { /** * Reads and validates configuration from file. - * + * * @param configPath - Path to config file (defaults to `configPath()`) * @returns Validated configuration object from disk * @throws Error if file doesn't exist, contains invalid JSON, or fails validation @@ -117,12 +120,9 @@ export class ConfigFileHandler { this.logger.verbose(`Existing config unreadable, proceeding with write`); } - const data = JSON.stringify(config, null, 2); - this.logger.verbose( - `Writing config to ${this.definition.configPath()}: ${data}` - ); - try { + const data = JSON.stringify(config, null, 2); + this.logger.verbose("Writing config"); await writeFile(this.definition.configPath(), data); return true; } catch (error) {