From 68cd5e4f472755f4cfa4f977288a9f4ea98d6cc1 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Mon, 13 Oct 2025 10:28:26 -0400 Subject: [PATCH] feat(api-config): integrate OS version tracking with new OsVersionTracker module - Introduced `OsVersionTracker` to manage and persist the current OS version during application bootstrap. - Updated `ApiConfigPersistence` to remove shutdown handling for OS version tracking, streamlining the logic. - Enhanced tests for `ApiConfigPersistence` and added tests for `OsVersionTracker` to ensure proper functionality. - Registered `OsVersionTrackerModule` in `ApiConfigModule` to facilitate OS version management. This update improves the reliability of OS version tracking and simplifies the configuration persistence logic. --- .../unraid-api/config/api-config.module.ts | 23 +- api/src/unraid-api/config/api-config.test.ts | 274 +++++++++--------- .../config/os-version-tracker.module.ts | 83 ++++++ 3 files changed, 220 insertions(+), 160 deletions(-) create mode 100644 api/src/unraid-api/config/os-version-tracker.module.ts diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index 0911db54c..ce182ad55 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -1,4 +1,4 @@ -import { Injectable, Module, OnApplicationBootstrap, OnApplicationShutdown } from '@nestjs/common'; +import { Injectable, Module, OnApplicationBootstrap } from '@nestjs/common'; import { ConfigService, registerAs } from '@nestjs/config'; import path from 'path'; @@ -8,6 +8,7 @@ import { csvStringToArray } from '@unraid/shared/util/data.js'; import { isConnectPluginInstalled } from '@app/connect-plugin-cleanup.js'; import { API_VERSION, PATHS_CONFIG_MODULES } from '@app/environment.js'; +import { OsVersionTrackerModule } from '@app/unraid-api/config/os-version-tracker.module.js'; export { type ApiConfig }; @@ -56,10 +57,8 @@ export const apiConfig = registerAs('api', loadApiConfig); @Injectable() export class ApiConfigPersistence extends ConfigFilePersister - implements OnApplicationBootstrap, OnApplicationShutdown + implements OnApplicationBootstrap { - private currentOsVersion: string | undefined; - constructor(configService: ConfigService) { super(configService); } @@ -90,19 +89,6 @@ export class ApiConfigPersistence async onApplicationBootstrap() { this.configService.set('api.version', API_VERSION); - this.currentOsVersion = this.configService.get('store.emhttp.var.version'); - } - - async onApplicationShutdown() { - if (!this.currentOsVersion) { - return; - } - - const apiConfig = this.configService.get('api'); - if (apiConfig) { - apiConfig.lastSeenOsVersion = this.currentOsVersion; - await this.persist(apiConfig); - } } async migrateConfig(): Promise { @@ -134,7 +120,8 @@ export class ApiConfigPersistence // apiConfig should be registered in root config in app.module.ts, not here. @Module({ + imports: [OsVersionTrackerModule], providers: [ApiConfigPersistence], - exports: [ApiConfigPersistence], + exports: [ApiConfigPersistence, OsVersionTrackerModule], }) 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 ede0b1514..bcd7d143e 100644 --- a/api/src/unraid-api/config/api-config.test.ts +++ b/api/src/unraid-api/config/api-config.test.ts @@ -1,11 +1,16 @@ import { ConfigService } from '@nestjs/config'; +import { readFile } from 'fs/promises'; +import path from 'path'; +import type { ApiConfig } from '@unraid/shared/services/api-config.js'; +import { writeFile as atomicWriteFile } from 'atomically'; +import { Subject } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { fileExists } from '@app/core/utils/files/file-exists.js'; +import { API_VERSION, PATHS_CONFIG_MODULES } from '@app/environment.js'; import { ApiConfigPersistence, loadApiConfig } from '@app/unraid-api/config/api-config.module.js'; +import { OsVersionTracker } from '@app/unraid-api/config/os-version-tracker.module.js'; -// Mock file utilities vi.mock('@app/core/utils/files/file-exists.js', () => ({ fileExists: vi.fn(), })); @@ -14,185 +19,168 @@ vi.mock('@unraid/shared/util/file.js', () => ({ fileExists: vi.fn(), })); -// Mock fs/promises for file I/O operations vi.mock('fs/promises', () => ({ readFile: vi.fn(), +})); + +vi.mock('atomically', () => ({ writeFile: vi.fn(), })); +const mockReadFile = vi.mocked(readFile); +const mockAtomicWriteFile = vi.mocked(atomicWriteFile); + describe('ApiConfigPersistence', () => { let service: ApiConfigPersistence; let configService: ConfigService; + let configChanges$: Subject<{ path?: string }>; + let setMock: ReturnType; + let getMock: ReturnType; beforeEach(() => { + configChanges$ = new Subject<{ path?: string }>(); + setMock = vi.fn(); + getMock = vi.fn(); + configService = { - get: vi.fn(), - set: vi.fn(), + get: getMock, + set: setMock, getOrThrow: vi.fn().mockReturnValue('test-config-path'), + changes$: configChanges$, } as any; service = new ApiConfigPersistence(configService); }); - describe('required ConfigFilePersister methods', () => { - it('should return correct file name', () => { - expect(service.fileName()).toBe('api.json'); - }); + 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 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: [], - }); + it('should return default config', () => { + const defaultConfig = service.defaultConfig(); + expect(defaultConfig).toEqual({ + version: API_VERSION, + extraOrigins: [], + sandbox: false, + ssoSubIds: [], + plugins: [], + lastSeenOsVersion: undefined, }); }); - describe('convertLegacyConfig', () => { - it('should migrate sandbox from string "yes" to boolean true', () => { - const legacyConfig = { - local: { sandbox: 'yes' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; + it('should migrate config from legacy format', async () => { + const legacyConfig = { + local: { sandbox: 'yes' }, + api: { extraOrigins: 'https://example.com,https://test.com' }, + remote: { ssoSubIds: 'sub1,sub2' }, + }; - const result = service.convertLegacyConfig(legacyConfig); - - expect(result.sandbox).toBe(true); + getMock.mockImplementation((key: string) => { + if (key === 'store.config') { + return legacyConfig; + } + return undefined; }); - it('should migrate sandbox from string "no" to boolean false', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; + const result = await service.migrateConfig(); - const result = service.convertLegacyConfig(legacyConfig); + expect(result).toEqual({ + version: API_VERSION, + extraOrigins: ['https://example.com', 'https://test.com'], + sandbox: true, + ssoSubIds: ['sub1', 'sub2'], + plugins: [], + lastSeenOsVersion: undefined, + }); + }); - expect(result.sandbox).toBe(false); + it('sets api.version on bootstrap', async () => { + await service.onApplicationBootstrap(); + expect(setMock).toHaveBeenCalledWith('api.version', API_VERSION); + }); +}); + +describe('OsVersionTracker', () => { + const trackerPath = path.join(PATHS_CONFIG_MODULES, 'os-version-tracker.json'); + let configService: ConfigService; + let setMock: ReturnType; + + beforeEach(() => { + setMock = vi.fn(); + configService = { + set: setMock, + get: vi.fn(), + getOrThrow: vi.fn(), + } as any; + + mockReadFile.mockReset(); + mockAtomicWriteFile.mockReset(); + }); + + it('persists current version when tracker is missing', async () => { + mockReadFile.mockImplementation(async (filePath) => { + if (filePath === '/etc/unraid-version') { + return 'version="7.2.0-beta.3.4"\n'; + } + throw Object.assign(new Error('Not found'), { code: 'ENOENT' }); }); - 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 tracker = new OsVersionTracker(configService); + await tracker.onApplicationBootstrap(); - const result = service.convertLegacyConfig(legacyConfig); + expect(setMock).toHaveBeenCalledWith('api.currentOsVersion', '7.2.0-beta.3.4'); + expect(setMock).toHaveBeenCalledWith('store.emhttp.var.version', '7.2.0-beta.3.4'); + expect(setMock).toHaveBeenCalledWith('api.lastSeenOsVersion', undefined); - expect(result.extraOrigins).toEqual(['https://example.com', 'https://test.com']); + expect(mockAtomicWriteFile).toHaveBeenCalledWith( + trackerPath, + expect.stringContaining('"lastTrackedVersion": "7.2.0-beta.3.4"'), + { mode: 0o644 } + ); + }); + + it('does not rewrite when version has not changed', async () => { + mockReadFile.mockImplementation(async (filePath) => { + if (filePath === '/etc/unraid-version') { + return 'version="6.12.0"\n'; + } + if (filePath === trackerPath) { + return JSON.stringify({ + lastTrackedVersion: '6.12.0', + updatedAt: '2024-01-01T00:00:00.000Z', + }); + } + throw Object.assign(new Error('Not found'), { code: 'ENOENT' }); }); - 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 tracker = new OsVersionTracker(configService); + await tracker.onApplicationBootstrap(); - const result = service.convertLegacyConfig(legacyConfig); + expect(setMock).toHaveBeenCalledWith('api.currentOsVersion', '6.12.0'); + expect(setMock).toHaveBeenCalledWith('store.emhttp.var.version', '6.12.0'); + expect(setMock).toHaveBeenCalledWith('api.lastSeenOsVersion', '6.12.0'); + expect(mockAtomicWriteFile).not.toHaveBeenCalled(); + }); - expect(result.extraOrigins).toEqual(['https://example.com', 'http://test.com']); - }); + it('handles missing version file gracefully', async () => { + mockReadFile.mockRejectedValue(new Error('permission denied')); - it('should handle empty extraOrigins string', () => { - const legacyConfig = { - local: { sandbox: 'no' }, - api: { extraOrigins: '' }, - remote: { ssoSubIds: '' }, - }; + const tracker = new OsVersionTracker(configService); + await tracker.onApplicationBootstrap(); - 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']); - }); + expect(setMock).toHaveBeenCalledWith('api.currentOsVersion', undefined); + expect(setMock).toHaveBeenCalledWith('store.emhttp.var.version', undefined); + expect(setMock).toHaveBeenCalledWith('api.lastSeenOsVersion', undefined); + expect(mockAtomicWriteFile).not.toHaveBeenCalled(); }); }); describe('loadApiConfig', () => { - beforeEach(async () => { + beforeEach(() => { vi.clearAllMocks(); }); @@ -200,11 +188,12 @@ describe('loadApiConfig', () => { const result = await loadApiConfig(); expect(result).toEqual({ - version: expect.any(String), + version: API_VERSION, extraOrigins: [], sandbox: false, ssoSubIds: [], plugins: [], + lastSeenOsVersion: undefined, }); }); @@ -212,11 +201,12 @@ describe('loadApiConfig', () => { const result = await loadApiConfig(); expect(result).toEqual({ - version: expect.any(String), + version: API_VERSION, extraOrigins: [], sandbox: false, ssoSubIds: [], plugins: [], + lastSeenOsVersion: undefined, }); }); }); diff --git a/api/src/unraid-api/config/os-version-tracker.module.ts b/api/src/unraid-api/config/os-version-tracker.module.ts new file mode 100644 index 000000000..c0c1f7e10 --- /dev/null +++ b/api/src/unraid-api/config/os-version-tracker.module.ts @@ -0,0 +1,83 @@ +import { Injectable, Logger, Module, OnApplicationBootstrap } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { readFile } from 'fs/promises'; +import path from 'path'; + +import { writeFile } from 'atomically'; + +import { PATHS_CONFIG_MODULES } from '@app/environment.js'; + +const OS_VERSION_FILE_PATH = '/etc/unraid-version'; +const VERSION_TRACKER_FILE = 'os-version-tracker.json'; + +type OsVersionTrackerState = { + lastTrackedVersion?: string; + updatedAt?: string; +}; + +@Injectable() +export class OsVersionTracker implements OnApplicationBootstrap { + private readonly logger = new Logger(OsVersionTracker.name); + private readonly trackerPath = path.join(PATHS_CONFIG_MODULES, VERSION_TRACKER_FILE); + + constructor(private readonly configService: ConfigService) {} + + async onApplicationBootstrap() { + const currentVersion = await this.readCurrentVersion(); + if (!currentVersion) { + this.configService.set('api.currentOsVersion', undefined); + this.configService.set('store.emhttp.var.version', undefined); + this.configService.set('api.lastSeenOsVersion', undefined); + return; + } + + const previousState = await this.readTrackerState(); + const lastTrackedVersion = previousState?.lastTrackedVersion; + + this.configService.set('api.currentOsVersion', currentVersion); + this.configService.set('store.emhttp.var.version', currentVersion); + this.configService.set('api.lastSeenOsVersion', lastTrackedVersion); + + if (lastTrackedVersion !== currentVersion) { + await this.writeTrackerState({ + lastTrackedVersion: currentVersion, + updatedAt: new Date().toISOString(), + }); + } + } + + private async readCurrentVersion(): Promise { + try { + const contents = await readFile(OS_VERSION_FILE_PATH, 'utf8'); + const match = contents.match(/^\s*version\s*=\s*"([^"]+)"\s*$/m); + return match?.[1]?.trim() || undefined; + } catch (error) { + this.logger.error(error, `Failed to read current OS version from ${OS_VERSION_FILE_PATH}`); + return undefined; + } + } + + private async readTrackerState(): Promise { + try { + const content = await readFile(this.trackerPath, 'utf8'); + return JSON.parse(content) as OsVersionTrackerState; + } catch (error) { + this.logger.debug(error, `Unable to read OS version tracker state at ${this.trackerPath}`); + return undefined; + } + } + + private async writeTrackerState(state: OsVersionTrackerState): Promise { + try { + await writeFile(this.trackerPath, JSON.stringify(state, null, 2), { mode: 0o644 }); + } catch (error) { + this.logger.error(error, 'Failed to persist OS version tracker state'); + } + } +} + +@Module({ + providers: [OsVersionTracker], + exports: [OsVersionTracker], +}) +export class OsVersionTrackerModule {}