Skip to content

Commit fee7d46

Browse files
pujitmpujitm
authored
refactor: add & use ConfigFilePersister primitive (#1534)
Add `ConfigFilePersister<T>` to provide automatic JSON file persistence for configs. It bridges the gap between in-memory configuration (via `ConfigService`) and persistent file storage, with minimal developer effort. ## Key Features - **Reactive Persistence**: Automatically saves config changes to disk when `ConfigService` updates - **NestJS Integration**: Implements lifecycle hooks for proper initialization and cleanup - **Standalone Operations**: Provides direct file access via `getFileHandler()` for non-reactive use cases - **Change Detection**: Only writes to disk when configuration actually changes (performance optimization) - **Error Handling**: Includes logging and graceful error handling throughout ## How to Implement Extend the class and implement these required methods: ```typescript @Injectable() class MyConfigPersister extends ConfigFilePersister<MyConfigType> { constructor(configService: ConfigService) { super(configService); } // Required: JSON filename in config directory fileName(): string { return "my-config.json"; } // Required: ConfigService key for reactive updates configKey(): string { return "myConfig"; } // Required: Default values for new installations defaultConfig(): MyConfigType { return { enabled: false, timeout: 5000 }; } // optionally, override validate() and/or migrateConfig() } ``` ## Lifecycle Behavior - **Initialization** (`onModuleInit`): Loads config from disk → sets in ConfigService → starts reactive subscription - **Runtime**: Automatically persists to disk when ConfigService changes (buffered every 25ms) - **Shutdown** (`onModuleDestroy`): Final persistence and cleanup <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit * **New Features** * Introduced a unified, robust configuration file management system with automatic migration, validation, and efficient persistence for plugins and services. * **Refactor** * Centralized configuration persistence logic into a shared base class, simplifying and standardizing config handling. * Refactored multiple config persisters to extend the new base class, removing redundant manual file and lifecycle management. * Removed legacy config state management, persistence helpers, and related modules, streamlining the codebase. * Simplified test suites to focus on core functionality and error handling. * **Chores** * Updated dependencies to support the new configuration management system. * Integrated the new API config module into plugin modules for consistent configuration handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent b6acf50 commit fee7d46

27 files changed

+1726
-1259
lines changed

‎api/src/__test__/config/api-config.test.ts‎

Lines changed: 0 additions & 137 deletions
This file was deleted.

‎api/src/unraid-api/config/api-config.module.ts‎

Lines changed: 55 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
import { Injectable, Logger, Module } from '@nestjs/common';
22
import { ConfigService, registerAs } from '@nestjs/config';
3+
import path from 'path';
34

45
import type { ApiConfig } from '@unraid/shared/services/api-config.js';
6+
import { ConfigFilePersister } from '@unraid/shared/services/config-file.js';
57
import { csvStringToArray } from '@unraid/shared/util/data.js';
6-
import { fileExists } from '@unraid/shared/util/file.js';
7-
import { bufferTime } from 'rxjs/operators';
88

9-
import { API_VERSION } from '@app/environment.js';
10-
import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js';
11-
import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js';
9+
import { API_VERSION, PATHS_CONFIG_MODULES } from '@app/environment.js';
1210

1311
export { type ApiConfig };
1412

@@ -22,123 +20,72 @@ const createDefaultConfig = (): ApiConfig => ({
2220
plugins: [],
2321
});
2422

25-
export const persistApiConfig = async (config: ApiConfig) => {
26-
const apiConfig = new ApiStateConfig<ApiConfig>(
27-
{
28-
name: 'api',
29-
defaultConfig: config,
30-
parse: (data) => data as ApiConfig,
31-
},
32-
new ConfigPersistenceHelper()
33-
);
34-
return await apiConfig.persist(config);
35-
};
36-
23+
/**
24+
* Simple file-based config loading for plugin discovery (outside of nestjs DI container).
25+
* This avoids complex DI container instantiation during module loading.
26+
*/
3727
export const loadApiConfig = async () => {
38-
try {
39-
const defaultConfig = createDefaultConfig();
40-
const apiConfig = new ApiStateConfig<ApiConfig>(
41-
{
42-
name: 'api',
43-
defaultConfig,
44-
parse: (data) => data as ApiConfig,
45-
},
46-
new ConfigPersistenceHelper()
47-
);
48-
49-
let diskConfig: ApiConfig | undefined;
50-
try {
51-
diskConfig = await apiConfig.parseConfig();
52-
} catch (error) {
53-
logger.error('Failed to load API config from disk, using defaults:', error);
54-
diskConfig = undefined;
55-
56-
// Try to overwrite the invalid config with defaults to fix the issue
57-
try {
58-
const configToWrite = {
59-
...defaultConfig,
60-
version: API_VERSION,
61-
};
62-
63-
const writeSuccess = await apiConfig.persist(configToWrite);
64-
if (writeSuccess) {
65-
logger.log('Successfully overwrote invalid config file with defaults.');
66-
} else {
67-
logger.error(
68-
'Failed to overwrite invalid config file. Continuing with defaults in memory only.'
69-
);
70-
}
71-
} catch (persistError) {
72-
logger.error('Error during config file repair:', persistError);
73-
}
74-
}
28+
const defaultConfig = createDefaultConfig();
29+
const apiHandler = new ApiConfigPersistence(new ConfigService()).getFileHandler();
7530

76-
return {
77-
...defaultConfig,
78-
...diskConfig,
79-
version: API_VERSION,
80-
};
81-
} catch (outerError) {
82-
// This should never happen, but ensures the config factory never throws
83-
logger.error('Critical error in loadApiConfig, using minimal defaults:', outerError);
84-
return createDefaultConfig();
31+
let diskConfig: Partial<ApiConfig> = {};
32+
try {
33+
diskConfig = await apiHandler.loadConfig();
34+
} catch (error) {
35+
logger.warn('Failed to load API config from disk:', error);
8536
}
37+
38+
return {
39+
...defaultConfig,
40+
...diskConfig,
41+
// diskConfig's version may be older, but we still want to use the correct version
42+
version: API_VERSION,
43+
};
8644
};
8745

8846
/**
8947
* Loads the API config from disk. If not found, returns the default config, but does not persist it.
48+
* This is used in the root config module to register the api config.
9049
*/
9150
export const apiConfig = registerAs<ApiConfig>('api', loadApiConfig);
9251

9352
@Injectable()
94-
export class ApiConfigPersistence {
95-
private configModel: ApiStateConfig<ApiConfig>;
96-
private logger = new Logger(ApiConfigPersistence.name);
97-
get filePath() {
98-
return this.configModel.filePath;
53+
export class ApiConfigPersistence extends ConfigFilePersister<ApiConfig> {
54+
constructor(configService: ConfigService) {
55+
super(configService);
9956
}
100-
get config() {
101-
return this.configService.getOrThrow('api');
57+
58+
fileName(): string {
59+
return 'api.json';
10260
}
10361

104-
constructor(
105-
private readonly configService: ConfigService,
106-
private readonly persistenceHelper: ConfigPersistenceHelper
107-
) {
108-
this.configModel = new ApiStateConfig<ApiConfig>(
109-
{
110-
name: 'api',
111-
defaultConfig: createDefaultConfig(),
112-
parse: (data) => data as ApiConfig,
113-
},
114-
this.persistenceHelper
115-
);
62+
configKey(): string {
63+
return 'api';
11664
}
11765

118-
async onModuleInit() {
119-
try {
120-
if (!(await fileExists(this.filePath))) {
121-
this.migrateFromMyServersConfig();
122-
}
123-
await this.persistenceHelper.persistIfChanged(this.filePath, this.config);
124-
this.configService.changes$.pipe(bufferTime(25)).subscribe({
125-
next: async (changes) => {
126-
if (changes.some((change) => change.path.startsWith('api'))) {
127-
this.logger.verbose(`API Config changed ${JSON.stringify(changes)}`);
128-
try {
129-
await this.persistenceHelper.persistIfChanged(this.filePath, this.config);
130-
} catch (persistError) {
131-
this.logger.error('Error persisting config changes:', persistError);
132-
}
133-
}
134-
},
135-
error: (err) => {
136-
this.logger.error('Error receiving config changes:', err);
137-
},
138-
});
139-
} catch (error) {
140-
this.logger.error('Error during API config module initialization:', error);
141-
}
66+
/**
67+
* @override
68+
* Since the api config is read outside of the nestjs DI container,
69+
* we need to provide an explicit path instead of relying on the
70+
* default prefix from the configService.
71+
*
72+
* @returns The path to the api config file
73+
*/
74+
configPath(): string {
75+
return path.join(PATHS_CONFIG_MODULES, this.fileName());
76+
}
77+
78+
defaultConfig(): ApiConfig {
79+
return createDefaultConfig();
80+
}
81+
82+
async migrateConfig(): Promise<ApiConfig> {
83+
const legacyConfig = this.configService.get('store.config', {});
84+
const migrated = this.convertLegacyConfig(legacyConfig);
85+
return {
86+
...this.defaultConfig(),
87+
...migrated,
88+
};
14289
}
14390

14491
convertLegacyConfig(
@@ -156,18 +103,11 @@ export class ApiConfigPersistence {
156103
ssoSubIds: csvStringToArray(config?.remote?.ssoSubIds ?? ''),
157104
};
158105
}
159-
160-
migrateFromMyServersConfig() {
161-
const legacyConfig = this.configService.get('store.config', {});
162-
const { sandbox, extraOrigins, ssoSubIds } = this.convertLegacyConfig(legacyConfig);
163-
this.configService.set('api.sandbox', sandbox);
164-
this.configService.set('api.extraOrigins', extraOrigins);
165-
this.configService.set('api.ssoSubIds', ssoSubIds);
166-
}
167106
}
168107

169108
// apiConfig should be registered in root config in app.module.ts, not here.
170109
@Module({
171-
providers: [ApiConfigPersistence, ConfigPersistenceHelper],
110+
providers: [ApiConfigPersistence],
111+
exports: [ApiConfigPersistence],
172112
})
173113
export class ApiConfigModule {}

0 commit comments

Comments
 (0)