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.

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<ApiConfig>('api', loadApiConfig);
 @Injectable()
 export class ApiConfigPersistence
     extends ConfigFilePersister<ApiConfig>
-    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<string>('store.emhttp.var.version');
-    }
-
-    async onApplicationShutdown() {
-        if (!this.currentOsVersion) {
-            return;
-        }
-
-        const apiConfig = this.configService.get<ApiConfig>('api');
-        if (apiConfig) {
-            apiConfig.lastSeenOsVersion = this.currentOsVersion;
-            await this.persist(apiConfig);
-        }
     }
 
     async migrateConfig(): Promise<ApiConfig> {
@@ -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 {}

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<typeof vi.fn>;
+    let getMock: ReturnType<typeof vi.fn>;
 
     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<typeof vi.fn>;
+
+    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,
         });
     });
 });

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<string | undefined> {
+        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<OsVersionTrackerState | undefined> {
+        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<void> {
+        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 {}