diff --git a/server/src/dtos/system-config.dto.ts b/server/src/dtos/system-config.dto.ts index ec1e272ab3..7a2e0632b4 100644 --- a/server/src/dtos/system-config.dto.ts +++ b/server/src/dtos/system-config.dto.ts @@ -12,11 +12,8 @@ import { IsUrl, Max, Min, - Validate, ValidateIf, ValidateNested, - ValidatorConstraint, - ValidatorConstraintInterface, } from 'class-validator'; import { SystemConfig } from 'src/config'; import { CLIPConfig, DuplicateDetectionConfig, FacialRecognitionConfig } from 'src/dtos/model-config.dto'; @@ -33,14 +30,7 @@ import { VideoContainer, } from 'src/enum'; import { ConcurrentQueueName, QueueName } from 'src/interfaces/job.interface'; -import { ValidateBoolean, validateCronExpression } from 'src/validation'; - -@ValidatorConstraint({ name: 'cronValidator' }) -class CronValidator implements ValidatorConstraintInterface { - validate(expression: string): boolean { - return validateCronExpression(expression); - } -} +import { IsCronExpression, ValidateBoolean } from 'src/validation'; const isLibraryScanEnabled = (config: SystemConfigLibraryScanDto) => config.enabled; const isOAuthEnabled = (config: SystemConfigOAuthDto) => config.enabled; @@ -54,7 +44,7 @@ export class DatabaseBackupConfig { @ValidateIf(isDatabaseBackupEnabled) @IsNotEmpty() - @Validate(CronValidator, { message: 'Invalid cron expression' }) + @IsCronExpression() @IsString() cronExpression!: string; @@ -244,7 +234,7 @@ class SystemConfigLibraryScanDto { @ValidateIf(isLibraryScanEnabled) @IsNotEmpty() - @Validate(CronValidator, { message: 'Invalid cron expression' }) + @IsCronExpression() @IsString() cronExpression!: string; } diff --git a/server/src/services/backup.service.spec.ts b/server/src/services/backup.service.spec.ts index 82537cb9f6..5f00596622 100644 --- a/server/src/services/backup.service.spec.ts +++ b/server/src/services/backup.service.spec.ts @@ -88,26 +88,6 @@ describe(BackupService.name, () => { }); }); - describe('onConfigValidateEvent', () => { - it('should allow a valid cron expression', () => { - expect(() => - sut.onConfigValidate({ - newConfig: { backup: { database: { cronExpression: '0 0 * * *' } } } as SystemConfig, - oldConfig: {} as SystemConfig, - }), - ).not.toThrow(expect.stringContaining('Invalid cron expression')); - }); - - it('should fail for an invalid cron expression', () => { - expect(() => - sut.onConfigValidate({ - newConfig: { backup: { database: { cronExpression: 'foo' } } } as SystemConfig, - oldConfig: {} as SystemConfig, - }), - ).toThrow(/Invalid cron expression.*/); - }); - }); - describe('cleanupDatabaseBackups', () => { it('should do nothing if not reached keepLastAmount', async () => { systemMock.get.mockResolvedValue(systemConfigStub.backupEnabled); diff --git a/server/src/services/backup.service.ts b/server/src/services/backup.service.ts index ef76d4fb27..4be0e27879 100644 --- a/server/src/services/backup.service.ts +++ b/server/src/services/backup.service.ts @@ -8,7 +8,6 @@ import { ArgOf } from 'src/interfaces/event.interface'; import { JobName, JobStatus, QueueName } from 'src/interfaces/job.interface'; import { BaseService } from 'src/services/base.service'; import { handlePromiseError } from 'src/utils/misc'; -import { validateCronExpression } from 'src/validation'; @Injectable() export class BackupService extends BaseService { @@ -49,14 +48,6 @@ export class BackupService extends BaseService { }); } - @OnEvent({ name: 'config.validate' }) - onConfigValidate({ newConfig }: ArgOf<'config.validate'>) { - const { database } = newConfig.backup; - if (!validateCronExpression(database.cronExpression)) { - throw new Error(`Invalid cron expression ${database.cronExpression}`); - } - } - async cleanupDatabaseBackups() { this.logger.debug(`Database Backup Cleanup Started`); const { diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index 37bddab136..96fa127e3c 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -177,26 +177,6 @@ describe(LibraryService.name, () => { }); }); - describe('onConfigValidateEvent', () => { - it('should allow a valid cron expression', () => { - expect(() => - sut.onConfigValidate({ - newConfig: { library: { scan: { cronExpression: '0 0 * * *' } } } as SystemConfig, - oldConfig: {} as SystemConfig, - }), - ).not.toThrow(expect.stringContaining('Invalid cron expression')); - }); - - it('should fail for an invalid cron expression', () => { - expect(() => - sut.onConfigValidate({ - newConfig: { library: { scan: { cronExpression: 'foo' } } } as SystemConfig, - oldConfig: {} as SystemConfig, - }), - ).toThrow(/Invalid cron expression.*/); - }); - }); - describe('handleQueueSyncFiles', () => { it('should queue refresh of a new asset', async () => { libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1); diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts index bd32be9a2d..6c5d400c84 100644 --- a/server/src/services/library.service.ts +++ b/server/src/services/library.service.ts @@ -24,7 +24,6 @@ import { BaseService } from 'src/services/base.service'; import { mimeTypes } from 'src/utils/mime-types'; import { handlePromiseError } from 'src/utils/misc'; import { usePagination } from 'src/utils/pagination'; -import { validateCronExpression } from 'src/validation'; @Injectable() export class LibraryService extends BaseService { @@ -81,14 +80,6 @@ export class LibraryService extends BaseService { } } - @OnEvent({ name: 'config.validate' }) - onConfigValidate({ newConfig }: ArgOf<'config.validate'>) { - const { scan } = newConfig.library; - if (!validateCronExpression(scan.cronExpression)) { - throw new Error(`Invalid cron expression ${scan.cronExpression}`); - } - } - private async watch(id: string): Promise { if (!this.watchLibraries) { return false; diff --git a/server/src/services/system-config.service.spec.ts b/server/src/services/system-config.service.spec.ts index 26284d52b5..f9ee42eb03 100644 --- a/server/src/services/system-config.service.spec.ts +++ b/server/src/services/system-config.service.spec.ts @@ -261,6 +261,29 @@ describe(SystemConfigService.name, () => { }); }); + it('should accept valid cron expressions', async () => { + configMock.getEnv.mockReturnValue(mockEnvData({ configFile: 'immich-config.json' })); + systemMock.readFile.mockResolvedValue(JSON.stringify({ library: { scan: { cronExpression: '0 0 * * *' } } })); + + await expect(sut.getSystemConfig()).resolves.toMatchObject({ + library: { + scan: { + enabled: true, + cronExpression: '0 0 * * *', + }, + }, + }); + }); + + it('should reject invalid cron expressions', async () => { + configMock.getEnv.mockReturnValue(mockEnvData({ configFile: 'immich-config.json' })); + systemMock.readFile.mockResolvedValue(JSON.stringify({ library: { scan: { cronExpression: 'foo' } } })); + + await expect(sut.getSystemConfig()).rejects.toThrow( + 'library.scan.cronExpression has failed the following constraints: cronValidator', + ); + }); + it('should log errors with the config file', async () => { configMock.getEnv.mockReturnValue(mockEnvData({ configFile: 'immich-config.json' })); diff --git a/server/src/validation.ts b/server/src/validation.ts index 180d53ed56..177e439919 100644 --- a/server/src/validation.ts +++ b/server/src/validation.ts @@ -16,9 +16,12 @@ import { IsOptional, IsString, IsUUID, + Validate, ValidateBy, ValidateIf, ValidationOptions, + ValidatorConstraint, + ValidatorConstraintInterface, buildMessage, isDateString, } from 'class-validator'; @@ -156,16 +159,20 @@ export const ValidateBoolean = (options?: BooleanOptions) => { return applyDecorators(...decorators); }; -export function validateCronExpression(expression: string) { - try { - new CronJob(expression, () => {}); - } catch { - return false; +@ValidatorConstraint({ name: 'cronValidator' }) +class CronValidator implements ValidatorConstraintInterface { + validate(expression: string): boolean { + try { + new CronJob(expression, () => {}); + return true; + } catch { + return false; + } } - - return true; } +export const IsCronExpression = () => Validate(CronValidator, { message: 'Invalid cron expression' }); + type IValue = { value: unknown }; export const toEmail = ({ value }: IValue) => (typeof value === 'string' ? value.toLowerCase() : value);