From b9a1b9b08746b6d4cb2128d029a3dab7cdd47677 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 2 Jul 2025 10:07:27 -0400 Subject: [PATCH] fix(api): connect config `email` validation (#1454) Followup to #1451 Empty string in email field of connect.json caused validation error on load. ## Summary by CodeRabbit * **New Features** * Added a file-watching script to streamline development workflows. * Introduced comprehensive property-based and randomized tests for configuration parsing, migration, and validation. * **Bug Fixes** * Improved handling and validation of configuration fields, including stricter email validation and robust handling of optional fields. * **Refactor** * Updated configuration change detection to buffer events for improved performance. * Made minor formatting and visibility adjustments for clarity and maintainability. * **Chores** * Added new development dependencies for testing and data generation. --- .../unraid-api/config/api-config.module.ts | 2 +- packages/unraid-api-plugin-connect/justfile | 4 + .../unraid-api-plugin-connect/package.json | 2 + .../src/model/connect-config.model.ts | 8 +- .../src/service/config.persistence.ts | 8 +- .../src/service/connection.service.ts | 4 +- .../src/test/config.persistence.test.ts | 244 ++++++++++++-- .../src/test/config.validation.test.ts | 313 ++++++++++++++++++ .../src/templates/config.persistence.ts | 37 ++- pnpm-lock.yaml | 37 ++- 10 files changed, 603 insertions(+), 56 deletions(-) create mode 100644 packages/unraid-api-plugin-connect/src/test/config.validation.test.ts diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index d3c04a717..6d001aa2d 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -4,7 +4,7 @@ import { ConfigService, registerAs } from '@nestjs/config'; import type { ApiConfig } from '@unraid/shared/services/api-config.js'; import { csvStringToArray } from '@unraid/shared/util/data.js'; import { fileExists } from '@unraid/shared/util/file.js'; -import { bufferTime, debounceTime } from 'rxjs/operators'; +import { bufferTime } from 'rxjs/operators'; import { API_VERSION } from '@app/environment.js'; import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; diff --git a/packages/unraid-api-plugin-connect/justfile b/packages/unraid-api-plugin-connect/justfile index 39f6d767d..315e1e132 100644 --- a/packages/unraid-api-plugin-connect/justfile +++ b/packages/unraid-api-plugin-connect/justfile @@ -4,6 +4,10 @@ default: @just --list +# Watch for changes in src files and run clean + build +watch: + watchexec -r -e ts,tsx -w src -- pnpm build + # Count TypeScript lines in src directory, excluding test and generated files count-lines: #!/usr/bin/env bash diff --git a/packages/unraid-api-plugin-connect/package.json b/packages/unraid-api-plugin-connect/package.json index 19709df08..9a43c9c25 100644 --- a/packages/unraid-api-plugin-connect/package.json +++ b/packages/unraid-api-plugin-connect/package.json @@ -25,6 +25,7 @@ "description": "Unraid Connect plugin for Unraid API", "devDependencies": { "@apollo/client": "^3.11.8", + "@faker-js/faker": "^9.8.0", "@graphql-codegen/cli": "^5.0.3", "@graphql-typed-document-node/core": "^3.2.0", "@ianvs/prettier-plugin-sort-imports": "^4.4.1", @@ -46,6 +47,7 @@ "class-transformer": "^0.5.1", "class-validator": "^0.14.1", "execa": "^9.5.1", + "fast-check": "^4.1.1", "got": "^14.4.6", "graphql": "^16.9.0", "graphql-scalars": "^1.23.0", diff --git a/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts b/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts index 32f2d80ed..2a83588c4 100644 --- a/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts +++ b/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts @@ -1,6 +1,7 @@ import { UsePipes, ValidationPipe } from '@nestjs/common'; import { registerAs } from '@nestjs/config'; import { Field, InputType, ObjectType } from '@nestjs/graphql'; +import { ValidateIf } from 'class-validator'; import { URL_TYPE } from '@unraid/shared/network.model.js'; import { plainToInstance } from 'class-transformer'; @@ -58,9 +59,11 @@ export class MyServersConfig { localApiKey!: string; // User Information - @Field(() => String) + @Field(() => String, { nullable: true }) + @IsOptional() + @ValidateIf((o) => o.email !== undefined && o.email !== null && o.email !== '') @IsEmail() - email!: string; + email?: string | null; @Field(() => String) @IsString() @@ -194,7 +197,6 @@ export const emptyMyServersConfig = (): MyServersConfig => ({ upnpEnabled: false, apikey: '', localApiKey: '', - email: '', username: '', avatar: '', regWizTime: '', diff --git a/packages/unraid-api-plugin-connect/src/service/config.persistence.ts b/packages/unraid-api-plugin-connect/src/service/config.persistence.ts index 0daa75239..f8aa06113 100644 --- a/packages/unraid-api-plugin-connect/src/service/config.persistence.ts +++ b/packages/unraid-api-plugin-connect/src/service/config.persistence.ts @@ -80,12 +80,14 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { * @param config - The config object to validate. * @returns The validated config instance. */ - private async validate(config: object) { + public async validate(config: object) { let instance: MyServersConfig; if (config instanceof MyServersConfig) { instance = config; } else { - instance = plainToInstance(MyServersConfig, config, { enableImplicitConversion: true }); + instance = plainToInstance(MyServersConfig, config, { + enableImplicitConversion: true, + }); } await validateOrReject(instance); return instance; @@ -103,7 +105,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { this.logger.verbose(`Config loaded from ${this.configPath}`); return true; } catch (error) { - this.logger.warn('Error loading config:', error); + this.logger.warn(error, 'Error loading config'); } try { diff --git a/packages/unraid-api-plugin-connect/src/service/connection.service.ts b/packages/unraid-api-plugin-connect/src/service/connection.service.ts index e1099724a..315e51c11 100644 --- a/packages/unraid-api-plugin-connect/src/service/connection.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/connection.service.ts @@ -4,7 +4,7 @@ import { EventEmitter2 } from '@nestjs/event-emitter'; import type { OutgoingHttpHeaders } from 'node:http2'; import { Subscription } from 'rxjs'; -import { debounceTime, filter } from 'rxjs/operators'; +import { bufferTime, filter } from 'rxjs/operators'; import { EVENTS } from '../helper/nest-tokens.js'; import { ConnectionMetadata, MinigraphStatus, MyServersConfig } from '../model/connect-config.model.js'; @@ -83,7 +83,7 @@ export class MothershipConnectionService implements OnModuleInit, OnModuleDestro this.identitySubscription = this.configService.changes$ .pipe( filter((change) => Object.values(this.configKeys).includes(change.path)), - debounceTime(25) + bufferTime(25) ) .subscribe({ next: () => { diff --git a/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts b/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts index f7d606cad..857fe6f8c 100644 --- a/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts +++ b/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts @@ -1,8 +1,10 @@ import { ConfigService } from '@nestjs/config'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { faker } from '@faker-js/faker'; +import * as fc from 'fast-check'; -import { ConfigType } from '../model/connect-config.model.js'; +import { ConfigType, DynamicRemoteAccessType } from '../model/connect-config.model.js'; import { ConnectConfigPersister } from '../service/config.persistence.js'; describe('ConnectConfigPersister', () => { @@ -21,7 +23,7 @@ describe('ConnectConfigPersister', () => { }, } as any; - service = new ConnectConfigPersister(configService); + service = new ConnectConfigPersister(configService as any); }); describe('parseLegacyConfig', () => { @@ -59,6 +61,80 @@ ssoSubIds="user1,user2" expect(result.remote.upnpEnabled).toBe('no'); expect(result.remote.ssoSubIds).toBe('user1,user2'); }); + + it('should parse various INI configs with different boolean values using fast-check', () => { + fc.assert( + fc.property( + fc.boolean(), + fc.boolean(), + fc.constantFrom('yes', 'no'), + fc.integer({ min: 1000, max: 9999 }), + fc.constant(null).map(() => faker.internet.email()), + fc.constant(null).map(() => faker.internet.username()), + (wanaccess, upnpEnabled, sandbox, port, email, username) => { + const iniContent = ` +[api] +version="6.12.0" +extraOrigins="" +[local] +sandbox="${sandbox}" +[remote] +wanaccess="${wanaccess ? 'yes' : 'no'}" +wanport="${port}" +upnpEnabled="${upnpEnabled ? 'yes' : 'no'}" +apikey="unraid_test_key" +localApiKey="test_local_key" +email="${email}" +username="${username}" +avatar="" +regWizTime="" +accesstoken="" +idtoken="" +refreshtoken="" +dynamicRemoteAccessType="DISABLED" +ssoSubIds="" + `.trim(); + + const result = service.parseLegacyConfig(iniContent); + + expect(result.api.version).toBe('6.12.0'); + expect(result.local.sandbox).toBe(sandbox); + expect(result.remote.wanaccess).toBe(wanaccess ? 'yes' : 'no'); + expect(result.remote.wanport).toBe(port.toString()); + expect(result.remote.upnpEnabled).toBe(upnpEnabled ? 'yes' : 'no'); + expect(result.remote.email).toBe(email); + expect(result.remote.username).toBe(username); + } + ), + { numRuns: 25 } + ); + }); + + it('should handle empty sections gracefully', () => { + const iniContent = ` +[api] +version="6.12.0" +[local] +[remote] +wanaccess="no" +wanport="0" +upnpEnabled="no" +apikey="test" +localApiKey="test" +email="test@example.com" +username="test" +avatar="" +regWizTime="" +dynamicRemoteAccessType="DISABLED" + `.trim(); + + const result = service.parseLegacyConfig(iniContent); + + expect(result.api.version).toBe('6.12.0'); + expect(result.local).toBeDefined(); + expect(result.remote).toBeDefined(); + expect(result.remote.wanaccess).toBe('no'); + }); }); describe('convertLegacyConfig', () => { @@ -269,31 +345,6 @@ ssoSubIds="user1,user2" expect(result.dynamicRemoteAccessType).toBe('UPNP'); }); - it('should validate the migrated config and reject invalid email', async () => { - const legacyConfig = { - api: { version: '4.8.0+9485809', extraOrigins: '' }, - local: { sandbox: 'no' }, - remote: { - wanaccess: 'yes', - wanport: '3333', - upnpEnabled: 'no', - apikey: 'unraid_test_key', - localApiKey: 'test_local_key', - email: 'invalid-email', - username: 'testuser', - avatar: '', - regWizTime: '', - accesstoken: '', - idtoken: '', - refreshtoken: '', - dynamicRemoteAccessType: 'DISABLED', - ssoSubIds: '', - }, - } as any; - - await expect(service.convertLegacyConfig(legacyConfig)).rejects.toThrow(); - }); - it('should handle integration of parsing and conversion together', async () => { const iniContent = ` [api] @@ -324,10 +375,147 @@ ssoSubIds="sub1,sub2" // Convert to new format const result = await service.convertLegacyConfig(legacyConfig); - // Verify the end-to-end conversion (extraOrigins and ssoSubIds are now handled by API config) + // Verify the end-to-end conversion expect(result.wanaccess).toBe(true); expect(result.wanport).toBe(8080); expect(result.upnpEnabled).toBe(true); }); + + it('should handle various boolean migrations consistently using property-based testing', () => { + fc.assert( + fc.asyncProperty( + fc.boolean(), + fc.boolean(), + fc.integer({ min: 1000, max: 65535 }), + fc.constant(null).map(() => faker.internet.email()), + fc.constant(null).map(() => faker.internet.username()), + fc.constant(null).map(() => faker.string.alphanumeric({ length: 32 })), + async (wanaccess, upnpEnabled, port, email, username, apikey) => { + const legacyConfig = { + api: { version: faker.system.semver(), extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: wanaccess ? 'yes' : 'no', + wanport: port.toString(), + upnpEnabled: upnpEnabled ? 'yes' : 'no', + apikey: `unraid_${apikey}`, + localApiKey: faker.string.alphanumeric({ length: 64 }), + email, + username, + avatar: faker.image.avatarGitHub(), + regWizTime: faker.date.past().toISOString(), + accesstoken: faker.string.alphanumeric({ length: 64 }), + idtoken: faker.string.alphanumeric({ length: 64 }), + refreshtoken: faker.string.alphanumeric({ length: 64 }), + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '', + }, + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + // Test migration logic, not validation + expect(result.wanaccess).toBe(wanaccess); + expect(result.upnpEnabled).toBe(upnpEnabled); + expect(result.wanport).toBe(port); + expect(typeof result.wanport).toBe('number'); + expect(result.email).toBe(email); + expect(result.username).toBe(username); + expect(result.apikey).toBe(`unraid_${apikey}`); + } + ), + { numRuns: 20 } + ); + }); + + it('should handle edge cases in port conversion', () => { + fc.assert( + fc.asyncProperty( + fc.integer({ min: 0, max: 65535 }), + async (port) => { + const legacyConfig = { + api: { version: '6.12.0', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'no', + wanport: port.toString(), + upnpEnabled: 'no', + apikey: 'unraid_test', + localApiKey: 'test_local', + email: 'test@example.com', + username: faker.internet.username(), + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '', + }, + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + // Test port conversion logic + expect(result.wanport).toBe(port); + expect(typeof result.wanport).toBe('number'); + } + ), + { numRuns: 15 } + ); + }); + + it('should handle empty port values', async () => { + const legacyConfig = { + api: { version: '6.12.0', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'no', + wanport: '', + upnpEnabled: 'no', + apikey: 'unraid_test', + localApiKey: 'test_local', + email: 'test@example.com', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '', + }, + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.wanport).toBe(0); + expect(typeof result.wanport).toBe('number'); + }); + + it('should reject invalid configurations during migration', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '3333', + upnpEnabled: 'no', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'invalid-email', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '', + }, + } as any; + + await expect(service.convertLegacyConfig(legacyConfig)).rejects.toThrow(); + }); }); }); diff --git a/packages/unraid-api-plugin-connect/src/test/config.validation.test.ts b/packages/unraid-api-plugin-connect/src/test/config.validation.test.ts new file mode 100644 index 000000000..b694e9043 --- /dev/null +++ b/packages/unraid-api-plugin-connect/src/test/config.validation.test.ts @@ -0,0 +1,313 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ConfigService } from '@nestjs/config'; +import { faker } from '@faker-js/faker'; +import * as fc from 'fast-check'; + +import { MyServersConfig, DynamicRemoteAccessType } from '../model/connect-config.model.js'; +import { ConnectConfigPersister } from '../service/config.persistence.js'; + +describe('MyServersConfig Validation', () => { + let persister: ConnectConfigPersister; + let validConfig: Partial; + + beforeEach(() => { + const configService = { + getOrThrow: vi.fn().mockReturnValue('/mock/path'), + get: vi.fn(), + set: vi.fn(), + changes$: { + pipe: vi.fn(() => ({ + subscribe: vi.fn(), + })), + }, + } as any; + + persister = new ConnectConfigPersister(configService as any); + + validConfig = { + wanaccess: false, + wanport: 0, + upnpEnabled: false, + apikey: 'test-api-key', + localApiKey: 'test-local-key', + email: 'test@example.com', + username: 'testuser', + avatar: 'https://example.com/avatar.jpg', + regWizTime: '2024-01-01T00:00:00Z', + dynamicRemoteAccessType: DynamicRemoteAccessType.DISABLED, + upnpStatus: null, + }; + }); + + describe('Email validation', () => { + it('should accept valid email addresses', async () => { + const config = { ...validConfig, email: 'user@example.com' }; + const result = await persister.validate(config); + expect(result.email).toBe('user@example.com'); + }); + + it('should accept empty string for email', async () => { + const config = { ...validConfig, email: '' }; + const result = await persister.validate(config); + expect(result.email).toBe(''); + }); + + it('should accept null for email', async () => { + const config = { ...validConfig, email: null }; + const result = await persister.validate(config); + expect(result.email).toBeNull(); + }); + + it('should reject invalid email addresses', async () => { + const config = { ...validConfig, email: 'invalid-email' }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + + it('should reject malformed email addresses', async () => { + const config = { ...validConfig, email: '@example.com' }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + }); + + describe('Boolean field validation', () => { + it('should accept boolean values for wanaccess', async () => { + const config = { ...validConfig, wanaccess: true }; + const result = await persister.validate(config); + expect(result.wanaccess).toBe(true); + }); + + it('should accept boolean values for upnpEnabled', async () => { + const config = { ...validConfig, upnpEnabled: true }; + const result = await persister.validate(config); + expect(result.upnpEnabled).toBe(true); + }); + + it('should reject non-boolean values for wanaccess', async () => { + const config = { ...validConfig, wanaccess: 'yes' as any }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + + it('should reject non-boolean values for upnpEnabled', async () => { + const config = { ...validConfig, upnpEnabled: 'no' as any }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + }); + + describe('Number field validation', () => { + it('should accept number values for wanport', async () => { + const config = { ...validConfig, wanport: 8080 }; + const result = await persister.validate(config); + expect(result.wanport).toBe(8080); + }); + + it('should accept null for optional number fields', async () => { + const config = { ...validConfig, wanport: null }; + const result = await persister.validate(config); + expect(result.wanport).toBeNull(); + }); + + it('should reject non-number values for wanport', async () => { + const config = { ...validConfig, wanport: '8080' as any }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + }); + + describe('String field validation', () => { + it('should accept string values for required string fields', async () => { + const config = { ...validConfig }; + const result = await persister.validate(config); + expect(result.apikey).toBe(validConfig.apikey); + expect(result.localApiKey).toBe(validConfig.localApiKey); + expect(result.username).toBe(validConfig.username); + }); + + it('should reject non-string values for required string fields', async () => { + const config = { ...validConfig, apikey: 123 as any }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + }); + + describe('Enum validation', () => { + it('should accept valid enum values for dynamicRemoteAccessType', async () => { + const config = { ...validConfig, dynamicRemoteAccessType: DynamicRemoteAccessType.STATIC }; + const result = await persister.validate(config); + expect(result.dynamicRemoteAccessType).toBe(DynamicRemoteAccessType.STATIC); + }); + + it('should reject invalid enum values for dynamicRemoteAccessType', async () => { + const config = { ...validConfig, dynamicRemoteAccessType: 'INVALID' as any }; + await expect(persister.validate(config)).rejects.toThrow(); + }); + }); + + describe('Property-based validation testing', () => { + it('should accept valid email addresses generated by faker', () => { + fc.assert( + fc.asyncProperty( + fc.constant(null).map(() => faker.internet.email()), + async (email) => { + const config = { ...validConfig, email }; + const result = await persister.validate(config); + expect(result.email).toBe(email); + } + ), + { numRuns: 20 } + ); + }); + + it('should handle various boolean combinations', () => { + fc.assert( + fc.asyncProperty( + fc.boolean(), + fc.boolean(), + async (wanaccess, upnpEnabled) => { + const config = { ...validConfig, wanaccess, upnpEnabled }; + const result = await persister.validate(config); + expect(result.wanaccess).toBe(wanaccess); + expect(result.upnpEnabled).toBe(upnpEnabled); + } + ), + { numRuns: 10 } + ); + }); + + it('should handle valid port numbers', () => { + fc.assert( + fc.asyncProperty( + fc.integer({ min: 0, max: 65535 }), + async (port) => { + const config = { ...validConfig, wanport: port }; + const result = await persister.validate(config); + expect(result.wanport).toBe(port); + expect(typeof result.wanport).toBe('number'); + } + ), + { numRuns: 20 } + ); + }); + + it('should handle various usernames and API keys', () => { + fc.assert( + fc.asyncProperty( + fc.constant(null).map(() => faker.internet.username()), + fc.constant(null).map(() => `unraid_${faker.string.alphanumeric({ length: 32 })}`), + fc.constant(null).map(() => faker.string.alphanumeric({ length: 64 })), + async (username, apikey, localApiKey) => { + const config = { ...validConfig, username, apikey, localApiKey }; + const result = await persister.validate(config); + expect(result.username).toBe(username); + expect(result.apikey).toBe(apikey); + expect(result.localApiKey).toBe(localApiKey); + } + ), + { numRuns: 15 } + ); + }); + + it('should handle various enum values for dynamicRemoteAccessType', () => { + fc.assert( + fc.asyncProperty( + fc.constantFrom( + DynamicRemoteAccessType.DISABLED, + DynamicRemoteAccessType.STATIC, + DynamicRemoteAccessType.UPNP + ), + async (dynamicRemoteAccessType) => { + const config = { ...validConfig, dynamicRemoteAccessType }; + const result = await persister.validate(config); + expect(result.dynamicRemoteAccessType).toBe(dynamicRemoteAccessType); + } + ), + { numRuns: 10 } + ); + }); + + it('should reject invalid enum values', () => { + fc.assert( + fc.asyncProperty( + fc.string({ minLength: 1 }).filter(s => + !Object.values(DynamicRemoteAccessType).includes(s as any) + ), + async (invalidEnumValue) => { + const config = { ...validConfig, dynamicRemoteAccessType: invalidEnumValue }; + await expect(persister.validate(config)).rejects.toThrow(); + } + ), + { numRuns: 10 } + ); + }); + + it('should reject invalid email formats using fuzzing', () => { + fc.assert( + fc.asyncProperty( + fc.string({ minLength: 1 }).filter(s => + !s.includes('@') || s.startsWith('@') || s.endsWith('@') + ), + async (invalidEmail) => { + const config = { ...validConfig, email: invalidEmail }; + await expect(persister.validate(config)).rejects.toThrow(); + } + ), + { numRuns: 15 } + ); + }); + + it('should accept any number values for wanport (range validation is done at form level)', () => { + fc.assert( + fc.asyncProperty( + fc.integer({ min: -100000, max: 100000 }), + async (port) => { + const config = { ...validConfig, wanport: port }; + const result = await persister.validate(config); + expect(result.wanport).toBe(port); + expect(typeof result.wanport).toBe('number'); + } + ), + { numRuns: 10 } + ); + }); + }); + + describe('Complete config validation', () => { + it('should validate a complete valid config', async () => { + const result = await persister.validate(validConfig); + expect(result).toBeDefined(); + expect(result.email).toBe(validConfig.email); + expect(result.username).toBe(validConfig.username); + expect(result.wanaccess).toBe(validConfig.wanaccess); + expect(result.upnpEnabled).toBe(validConfig.upnpEnabled); + }); + + it('should validate config with minimal required fields using faker data', () => { + fc.assert( + fc.asyncProperty( + fc.constant(null).map(() => ({ + email: faker.internet.email(), + username: faker.internet.username(), + apikey: `unraid_${faker.string.alphanumeric({ length: 32 })}`, + localApiKey: faker.string.alphanumeric({ length: 64 }), + avatar: faker.image.avatarGitHub(), + regWizTime: faker.date.past().toISOString(), + })), + async (fakerData) => { + const minimalConfig = { + wanaccess: false, + upnpEnabled: false, + wanport: 0, + dynamicRemoteAccessType: DynamicRemoteAccessType.DISABLED, + upnpStatus: null, + ...fakerData, + }; + + const result = await persister.validate(minimalConfig); + expect(result.email).toBe(fakerData.email); + expect(result.username).toBe(fakerData.username); + expect(result.apikey).toBe(fakerData.apikey); + expect(result.localApiKey).toBe(fakerData.localApiKey); + } + ), + { numRuns: 10 } + ); + }); + }); +}); \ No newline at end of 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 cd8b574c8..1fef4d442 100644 --- a/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts +++ b/packages/unraid-api-plugin-generator/src/templates/config.persistence.ts @@ -3,7 +3,7 @@ import { ConfigService } from "@nestjs/config"; import { existsSync, readFileSync } from "fs"; import { writeFile } from "fs/promises"; import path from "path"; -import { debounceTime } from "rxjs/operators"; +import { bufferTime } from "rxjs/operators"; import { PluginNameConfig } from "./config.entity.js"; @Injectable() @@ -31,40 +31,51 @@ export class PluginNameConfigPersister implements OnModuleInit { 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); + 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...`); + 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(debounceTime(25)).subscribe({ - next: ({ newValue, oldValue, path: changedPath }) => { - // Only persist if the change is within this plugin's config namespace - if (changedPath.startsWith("plugin-name.") && newValue !== oldValue) { - this.logger.debug(`Config changed: ${changedPath} from ${oldValue} to ${newValue}`); - // Persist the entire config object for this plugin - this.persist(); + 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 subscribing to config changes:", err); + this.logger.error("Error receiving config changes:", err); }, }); } - async persist(config = this.configService.get("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); + this.logger.error( + `Error persisting config to '${this.configPath}':`, + error + ); } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9767d9356..fb4a0ce70 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -506,6 +506,9 @@ importers: '@apollo/client': specifier: ^3.11.8 version: 3.13.8(@types/react@19.0.8)(graphql-ws@6.0.5(crossws@0.3.5)(graphql@16.11.0)(ws@8.18.2))(graphql@16.11.0)(react-dom@19.1.0(react@19.1.0))(react@19.1.0)(subscriptions-transport-ws@0.11.0(graphql@16.11.0)) + '@faker-js/faker': + specifier: ^9.8.0 + version: 9.8.0 '@graphql-codegen/cli': specifier: ^5.0.3 version: 5.0.7(@parcel/watcher@2.5.1)(@types/node@22.15.32)(crossws@0.3.5)(enquirer@2.4.1)(graphql-sock@1.0.1(graphql@16.11.0))(graphql@16.11.0)(typescript@5.8.3) @@ -569,6 +572,9 @@ importers: execa: specifier: ^9.5.1 version: 9.6.0 + fast-check: + specifier: ^4.1.1 + version: 4.1.1 got: specifier: ^14.4.6 version: 14.4.7 @@ -2474,6 +2480,10 @@ packages: resolution: {integrity: sha512-4SaFZCNfJqvk/kenHpI8xvN42DMaoycy4PzKc5otHxRswww1kAt82OlBuwRVLofCACCTZEcla2Ydxv8scMXaTg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + '@faker-js/faker@9.8.0': + resolution: {integrity: sha512-U9wpuSrJC93jZBxx/Qq2wPjCuYISBueyVUGK7qqdmj7r/nxaxwW8AQDCLeRO7wZnjj94sh3p246cAYjUKuqgfg==} + engines: {node: '>=18.0.0', npm: '>=9.0.0'} + '@fastify/ajv-compiler@4.0.2': resolution: {integrity: sha512-Rkiu/8wIjpsf46Rr+Fitd3HRP+VsxUFDDeag0hs9L0ksfnwx2g7SPQQTFL0E8Qv+rfXzQOxBJnjUB9ITUDjfWQ==} @@ -7793,6 +7803,10 @@ packages: resolution: {integrity: sha512-He2AjQGHe46svIFq5+L2Nx/eHDTI1oKgoevBP+TthnjymXiKkeJQ3+ITeWey99Y5+2OaPFbI1qEsx/5RsGtWnQ==} engines: {node: '>=18'} + fast-check@4.1.1: + resolution: {integrity: sha512-8+yQYeNYqBfWem0Nmm7BUnh27wm+qwGvI0xln60c8RPM5rVekxZf/Ildng2GNBfjaG6utIebFmVBPlNtZlBLxg==} + engines: {node: '>=12.17.0'} + fast-copy@3.0.2: resolution: {integrity: sha512-dl0O9Vhju8IrcLndv2eU4ldt1ftXMqqfgN4H1cpmGV7P6jeB9FwpN9a2c8DPGE1Ys88rNUJVYDHq73CGAGOPfQ==} @@ -11102,6 +11116,9 @@ packages: resolution: {integrity: sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==} engines: {node: '>=6'} + pure-rand@7.0.1: + resolution: {integrity: sha512-oTUZM/NAZS8p7ANR3SHh30kXB+zK2r2BPcEn/awJIbOvq82WoMN4p62AWWp3Hhw50G0xMsw1mhIBLqHw64EcNQ==} + q@1.5.1: resolution: {integrity: sha512-kV/CThkXo6xyFEZUugw/+pIOywXcDbFYgSct5cT3gqlbkBE1SJdwy6UQoZvodiWF/ckQLZyDE/Bu1M6gVu5lVw==} engines: {node: '>=0.6.0', teleport: '>=0.2.0'} @@ -13065,12 +13082,12 @@ packages: vue-component-type-helpers@2.2.0: resolution: {integrity: sha512-cYrAnv2me7bPDcg9kIcGwjJiSB6Qyi08+jLDo9yuvoFQjzHiPTzML7RnkJB1+3P6KMsX/KbCD4QE3Tv/knEllw==} - vue-component-type-helpers@2.2.10: - resolution: {integrity: sha512-iDUO7uQK+Sab2tYuiP9D1oLujCWlhHELHMgV/cB13cuGbG4qwkLHvtfWb6FzvxrIOPDnU0oHsz2MlQjhYDeaHA==} - vue-component-type-helpers@2.2.8: resolution: {integrity: sha512-4bjIsC284coDO9om4HPA62M7wfsTvcmZyzdfR0aUlFXqq4tXxM1APyXpNVxPC8QazKw9OhmZNHBVDA6ODaZsrA==} + vue-component-type-helpers@3.0.0: + resolution: {integrity: sha512-J1HtqhZIqmYoNg4SLcYVFdCdsVUkMo4Z6/Wx4sQMfY8TFIIqDmd3mS2whfBIKzAA7dHMexarwYbvtB/fOUuEsw==} + vue-demi@0.14.10: resolution: {integrity: sha512-nMZBOwuzabUO0nLgIcc6rycZEebF6eeUfaiQx9+WSk8e29IbLvPU9feI6tqW4kTo3hvoYAJkMh8n8D0fuISphg==} engines: {node: '>=12'} @@ -14752,6 +14769,8 @@ snapshots: '@eslint/core': 0.15.0 levn: 0.4.1 + '@faker-js/faker@9.8.0': {} + '@fastify/ajv-compiler@4.0.2': dependencies: ajv: 8.17.1 @@ -17208,7 +17227,7 @@ snapshots: ts-dedent: 2.2.0 type-fest: 2.19.0 vue: 3.5.17(typescript@5.8.3) - vue-component-type-helpers: 2.2.10 + vue-component-type-helpers: 3.0.0 '@stylistic/eslint-plugin@4.4.1(eslint@9.29.0(jiti@2.4.2))(typescript@5.8.3)': dependencies: @@ -21229,6 +21248,10 @@ snapshots: fake-indexeddb@6.0.1: {} + fast-check@4.1.1: + dependencies: + pure-rand: 7.0.1 + fast-copy@3.0.2: {} fast-decode-uri-component@1.0.1: {} @@ -24982,6 +25005,8 @@ snapshots: punycode@2.3.1: {} + pure-rand@7.0.1: {} + q@1.5.1: {} qs@6.13.0: @@ -27226,10 +27251,10 @@ snapshots: vue-component-type-helpers@2.2.0: {} - vue-component-type-helpers@2.2.10: {} - vue-component-type-helpers@2.2.8: {} + vue-component-type-helpers@3.0.0: {} + vue-demi@0.14.10(vue@3.5.17(typescript@5.8.3)): dependencies: vue: 3.5.17(typescript@5.8.3)