feat(server): use nestjs events to validate config (#7986)

* use events for config validation

* chore: better types

* add unit tests

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

server/package.json

@@ -39,6 +39,7 @@
     "@nestjs/common": "^10.2.2",
     "@nestjs/config": "^3.0.0",
     "@nestjs/core": "^10.2.2",
+    "@nestjs/event-emitter": "^2.0.4",
     "@nestjs/platform-express": "^10.2.2",
     "@nestjs/platform-socket.io": "^10.2.2",
     "@nestjs/schedule": "^4.0.0",

server/src/domain/domain.module.ts

@@ -26,9 +26,9 @@ import { TrashService } from './trash';
 import { UserService } from './user';
 
 const providers: Provider[] = [
+  APIKeyService,
   ActivityService,
   AlbumService,
-  APIKeyService,
   AssetService,
   AuditService,
   AuthService,
@@ -39,8 +39,8 @@ const providers: Provider[] = [
   LibraryService,
   MediaService,
   MetadataService,
-  PersonService,
   PartnerService,
+  PersonService,
   SearchService,
   ServerInfoService,
   SharedLinkService,

server/src/domain/library/library.service.spec.ts

@@ -148,6 +148,26 @@ describe(LibraryService.name, () => {
     });
   });
 
+  describe('validateConfig', () => {
+    it('should allow a valid cron expression', () => {
+      expect(() =>
+        sut.validateConfig({
+          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.validateConfig({
+          newConfig: { library: { scan: { cronExpression: 'foo' } } } as SystemConfig,
+          oldConfig: {} as SystemConfig,
+        }),
+      ).toThrow(/Invalid cron expression.*/);
+    });
+  });
+
   describe('handleQueueAssetRefresh', () => {
     it('should queue new assets', async () => {
       const mockLibraryJob: ILibraryRefreshJob = {

server/src/domain/library/library.service.ts

@@ -1,6 +1,7 @@
 import { AssetType, LibraryEntity, LibraryType } from '@app/infra/entities';
 import { ImmichLogger } from '@app/infra/logger';
 import { BadRequestException, Inject, Injectable } from '@nestjs/common';
+import { OnEvent } from '@nestjs/event-emitter';
 import { Trie } from 'mnemonist';
 import { R_OK } from 'node:constants';
 import { EventEmitter } from 'node:events';
@@ -22,6 +23,8 @@ import {
   ILibraryRepository,
   IStorageRepository,
   ISystemConfigRepository,
+  InternalEvent,
+  InternalEventMap,
   JobStatus,
   StorageEventType,
   WithProperty,
@@ -65,12 +68,6 @@ export class LibraryService extends EventEmitter {
     super();
     this.access = AccessCore.create(accessRepository);
     this.configCore = SystemConfigCore.create(configRepository);
-    this.configCore.addValidator((config) => {
-      const { scan } = config.library;
-      if (!validateCronExpression(scan.cronExpression)) {
-        throw new Error(`Invalid cron expression ${scan.cronExpression}`);
-      }
-    });
   }
 
   async init() {
@@ -110,6 +107,14 @@ export class LibraryService extends EventEmitter {
     });
   }
 
+  @OnEvent(InternalEvent.VALIDATE_CONFIG)
+  validateConfig({ newConfig }: InternalEventMap[InternalEvent.VALIDATE_CONFIG]) {
+    const { scan } = newConfig.library;
+    if (!validateCronExpression(scan.cronExpression)) {
+      throw new Error(`Invalid cron expression ${scan.cronExpression}`);
+    }
+  }
+
   private async watch(id: string): Promise<boolean> {
     if (!this.watchLibraries) {
       return false;

server/src/domain/repositories/communication.repository.ts

@@ -1,4 +1,5 @@
 import { AssetResponseDto, ReleaseNotification, ServerVersionResponseDto } from '@app/domain';
+import { SystemConfig } from '@app/infra/entities';
 
 export const ICommunicationRepository = 'ICommunicationRepository';
 
@@ -21,6 +22,14 @@ export enum ServerEvent {
   CONFIG_UPDATE = 'config:update',
 }
 
+export enum InternalEvent {
+  VALIDATE_CONFIG = 'validate_config',
+}
+
+export interface InternalEventMap {
+  [InternalEvent.VALIDATE_CONFIG]: { newConfig: SystemConfig; oldConfig: SystemConfig };
+}
+
 export interface ClientEventMap {
   [ClientEvent.UPLOAD_SUCCESS]: AssetResponseDto;
   [ClientEvent.USER_DELETE]: string;
@@ -45,4 +54,6 @@ export interface ICommunicationRepository {
   on(event: 'connect', callback: OnConnectCallback): void;
   on(event: ServerEvent, callback: OnServerEventCallback): void;
   sendServerEvent(event: ServerEvent): void;
+  emit<E extends keyof InternalEventMap>(event: E, data: InternalEventMap[E]): boolean;
+  emitAsync<E extends keyof InternalEventMap>(event: E, data: InternalEventMap[E]): Promise<any>;
 }

server/src/domain/storage-template/storage-template.service.spec.ts

@@ -12,7 +12,7 @@ import {
   StorageTemplateService,
   defaults,
 } from '@app/domain';
-import { AssetPathType, SystemConfigKey } from '@app/infra/entities';
+import { AssetPathType, SystemConfig, SystemConfigKey } from '@app/infra/entities';
 import {
   assetStub,
   newAlbumRepositoryMock,
@@ -74,6 +74,35 @@ describe(StorageTemplateService.name, () => {
     SystemConfigCore.create(configMock).config$.next(defaults);
   });
 
+  describe('validate', () => {
+    it('should allow valid templates', () => {
+      expect(() =>
+        sut.validate({
+          newConfig: {
+            storageTemplate: {
+              template:
+                '{{y}}{{M}}{{W}}{{d}}{{h}}{{m}}{{s}}{{filename}}{{ext}}{{filetype}}{{filetypefull}}{{assetId}}{{album}}',
+            },
+          } as SystemConfig,
+          oldConfig: {} as SystemConfig,
+        }),
+      ).not.toThrow();
+    });
+
+    it('should fail for an invalid template', () => {
+      expect(() =>
+        sut.validate({
+          newConfig: {
+            storageTemplate: {
+              template: '{{foo}}',
+            },
+          } as SystemConfig,
+          oldConfig: {} as SystemConfig,
+        }),
+      ).toThrow(/Invalid storage template.*/);
+    });
+  });
+
   describe('handleMigrationSingle', () => {
     it('should skip when storage template is disabled', async () => {
       configMock.load.mockResolvedValue([{ key: SystemConfigKey.STORAGE_TEMPLATE_ENABLED, value: false }]);

server/src/domain/storage-template/storage-template.service.ts

@@ -1,6 +1,7 @@
 import { AssetEntity, AssetPathType, AssetType, SystemConfig } from '@app/infra/entities';
 import { ImmichLogger } from '@app/infra/logger';
 import { Inject, Injectable } from '@nestjs/common';
+import { OnEvent } from '@nestjs/event-emitter';
 import handlebar from 'handlebars';
 import * as luxon from 'luxon';
 import path from 'node:path';
@@ -18,6 +19,8 @@ import {
   IStorageRepository,
   ISystemConfigRepository,
   IUserRepository,
+  InternalEvent,
+  InternalEventMap,
   JobStatus,
 } from '../repositories';
 import { StorageCore, StorageFolder } from '../storage';
@@ -74,7 +77,6 @@ export class StorageTemplateService {
     @Inject(IDatabaseRepository) private databaseRepository: IDatabaseRepository,
   ) {
     this.configCore = SystemConfigCore.create(configRepository);
-    this.configCore.addValidator((config) => this.validate(config));
     this.configCore.config$.subscribe((config) => this.onConfig(config));
     this.storageCore = StorageCore.create(
       assetRepository,
@@ -86,6 +88,27 @@ export class StorageTemplateService {
     );
   }
 
+  @OnEvent(InternalEvent.VALIDATE_CONFIG)
+  validate({ newConfig }: InternalEventMap[InternalEvent.VALIDATE_CONFIG]) {
+    try {
+      const { compiled } = this.compile(newConfig.storageTemplate.template);
+      this.render(compiled, {
+        asset: {
+          fileCreatedAt: new Date(),
+          originalPath: '/upload/test/IMG_123.jpg',
+          type: AssetType.IMAGE,
+          id: 'd587e44b-f8c0-4832-9ba3-43268bbf5d4e',
+        } as AssetEntity,
+        filename: 'IMG_123',
+        extension: 'jpg',
+        albumName: 'album',
+      });
+    } catch (error) {
+      this.logger.warn(`Storage template validation failed: ${JSON.stringify(error)}`);
+      throw new Error(`Invalid storage template: ${error}`);
+    }
+  }
+
   async handleMigrationSingle({ id }: IEntityJob): Promise<JobStatus> {
     const config = await this.configCore.getConfig();
     const storageTemplateEnabled = config.storageTemplate.enabled;
@@ -259,26 +282,6 @@ export class StorageTemplateService {
     }
   }
 
-  private validate(config: SystemConfig) {
-    try {
-      const { compiled } = this.compile(config.storageTemplate.template);
-      this.render(compiled, {
-        asset: {
-          fileCreatedAt: new Date(),
-          originalPath: '/upload/test/IMG_123.jpg',
-          type: AssetType.IMAGE,
-          id: 'd587e44b-f8c0-4832-9ba3-43268bbf5d4e',
-        } as AssetEntity,
-        filename: 'IMG_123',
-        extension: 'jpg',
-        albumName: 'album',
-      });
-    } catch (error) {
-      this.logger.warn(`Storage template validation failed: ${JSON.stringify(error)}`);
-      throw new Error(`Invalid storage template: ${error}`);
-    }
-  }
-
   private onConfig(config: SystemConfig) {
     const template = config.storageTemplate.template;
     if (!this._template || template !== this.template.raw) {

server/src/domain/system-config/system-config.core.ts

@@ -167,7 +167,6 @@ let instance: SystemConfigCore | null;
 @Injectable()
 export class SystemConfigCore {
   private logger = new ImmichLogger(SystemConfigCore.name);
-  private validators: SystemConfigValidator[] = [];
   private configCache: SystemConfigEntity<SystemConfigValue>[] | null = null;
 
   public config$ = new Subject<SystemConfig>();
@@ -245,10 +244,6 @@ export class SystemConfigCore {
     return defaults;
   }
 
-  public addValidator(validator: SystemConfigValidator) {
-    this.validators.push(validator);
-  }
-
   public async getConfig(force = false): Promise<SystemConfig> {
     const configFilePath = process.env.IMMICH_CONFIG_FILE;
     const config = _.cloneDeep(defaults);
@@ -283,17 +278,6 @@ export class SystemConfigCore {
       throw new BadRequestException('Cannot update configuration while IMMICH_CONFIG_FILE is in use');
     }
 
-    const oldConfig = await this.getConfig();
-
-    try {
-      for (const validator of this.validators) {
-        await validator(newConfig, oldConfig);
-      }
-    } catch (error) {
-      this.logger.warn(`Unable to save system config due to a validation error: ${error}`);
-      throw new BadRequestException(error instanceof Error ? error.message : error);
-    }
-
     const updates: SystemConfigEntity[] = [];
     const deletes: SystemConfigEntity[] = [];
 

server/src/domain/system-config/system-config.service.spec.ts

@@ -16,7 +16,7 @@ import { BadRequestException } from '@nestjs/common';
 import { newCommunicationRepositoryMock, newSystemConfigRepositoryMock } from '@test';
 import { QueueName } from '../job';
 import { ICommunicationRepository, ISearchRepository, ISystemConfigRepository, ServerEvent } from '../repositories';
-import { defaults, SystemConfigValidator } from './system-config.core';
+import { defaults } from './system-config.core';
 import { SystemConfigService } from './system-config.service';
 
 const updates: SystemConfigEntity[] = [
@@ -172,15 +172,6 @@ describe(SystemConfigService.name, () => {
     });
   });
 
-  describe('addValidator', () => {
-    it('should call the validator on config changes', async () => {
-      const validator: SystemConfigValidator = jest.fn();
-      sut.addValidator(validator);
-      await sut.updateConfig(defaults);
-      expect(validator).toHaveBeenCalledWith(defaults, defaults);
-    });
-  });
-
   describe('getConfig', () => {
     let warnLog: jest.SpyInstance;
 
@@ -341,17 +332,6 @@ describe(SystemConfigService.name, () => {
       expect(configMock.saveAll).toHaveBeenCalledWith(updates);
     });
 
-    it('should throw an error if the config is not valid', async () => {
-      const validator = jest.fn().mockRejectedValue('invalid config');
-
-      sut.addValidator(validator);
-
-      await expect(sut.updateConfig(updatedConfig)).rejects.toBeInstanceOf(BadRequestException);
-
-      expect(validator).toHaveBeenCalledWith(updatedConfig, defaults);
-      expect(configMock.saveAll).not.toHaveBeenCalled();
-    });
-
     it('should throw an error if a config file is in use', async () => {
       process.env.IMMICH_CONFIG_FILE = 'immich-config.json';
       configMock.readFile.mockResolvedValue(JSON.stringify({}));