diff --git a/server/package-lock.json b/server/package-lock.json index d4ecf0e666..eecc75bb2c 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -15,6 +15,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", @@ -2640,6 +2641,18 @@ } } }, + "node_modules/@nestjs/event-emitter": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/@nestjs/event-emitter/-/event-emitter-2.0.4.tgz", + "integrity": "sha512-quMiw8yOwoSul0pp3mOonGz8EyXWHSBTqBy8B0TbYYgpnG1Ix2wGUnuTksLWaaBiiOTDhciaZ41Y5fJZsSJE1Q==", + "dependencies": { + "eventemitter2": "6.4.9" + }, + "peerDependencies": { + "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0", + "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0" + } + }, "node_modules/@nestjs/mapped-types": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/@nestjs/mapped-types/-/mapped-types-2.0.5.tgz", @@ -7637,6 +7650,11 @@ "node": ">=6" } }, + "node_modules/eventemitter2": { + "version": "6.4.9", + "resolved": "https://registry.npmjs.org/eventemitter2/-/eventemitter2-6.4.9.tgz", + "integrity": "sha512-JEPTiaOt9f04oa6NOkc4aH+nVp5I3wEjpHbIPqfgCdD5v5bUzy7xQqwcVO2aDQgOWhI28da57HksMrzK9HlRxg==" + }, "node_modules/events": { "version": "3.3.0", "resolved": "https://registry.npmjs.org/events/-/events-3.3.0.tgz", @@ -16201,6 +16219,14 @@ "uid": "2.0.2" } }, + "@nestjs/event-emitter": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/@nestjs/event-emitter/-/event-emitter-2.0.4.tgz", + "integrity": "sha512-quMiw8yOwoSul0pp3mOonGz8EyXWHSBTqBy8B0TbYYgpnG1Ix2wGUnuTksLWaaBiiOTDhciaZ41Y5fJZsSJE1Q==", + "requires": { + "eventemitter2": "6.4.9" + } + }, "@nestjs/mapped-types": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/@nestjs/mapped-types/-/mapped-types-2.0.5.tgz", @@ -19910,6 +19936,11 @@ "resolved": "https://registry.npmjs.org/event-target-shim/-/event-target-shim-5.0.1.tgz", "integrity": "sha512-i/2XbnSz/uxRCU6+NdVJgKWDTM427+MqYbkQzD321DuCQJUqOuJKIA0IM2+W2xtYHdKOmZ4dR6fExsd4SXL+WQ==" }, + "eventemitter2": { + "version": "6.4.9", + "resolved": "https://registry.npmjs.org/eventemitter2/-/eventemitter2-6.4.9.tgz", + "integrity": "sha512-JEPTiaOt9f04oa6NOkc4aH+nVp5I3wEjpHbIPqfgCdD5v5bUzy7xQqwcVO2aDQgOWhI28da57HksMrzK9HlRxg==" + }, "events": { "version": "3.3.0", "resolved": "https://registry.npmjs.org/events/-/events-3.3.0.tgz", diff --git a/server/package.json b/server/package.json index 94bc3f0907..c575571d07 100644 --- a/server/package.json +++ b/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", diff --git a/server/src/domain/domain.module.ts b/server/src/domain/domain.module.ts index 37faa09c9f..c3e62edb55 100644 --- a/server/src/domain/domain.module.ts +++ b/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, diff --git a/server/src/domain/library/library.service.spec.ts b/server/src/domain/library/library.service.spec.ts index ffcb80df0c..57bdf7373d 100644 --- a/server/src/domain/library/library.service.spec.ts +++ b/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 = { diff --git a/server/src/domain/library/library.service.ts b/server/src/domain/library/library.service.ts index d24c9cdc1a..12d135fa30 100644 --- a/server/src/domain/library/library.service.ts +++ b/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 { if (!this.watchLibraries) { return false; diff --git a/server/src/domain/repositories/communication.repository.ts b/server/src/domain/repositories/communication.repository.ts index 65e322702f..3efbbcb5ef 100644 --- a/server/src/domain/repositories/communication.repository.ts +++ b/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(event: E, data: InternalEventMap[E]): boolean; + emitAsync(event: E, data: InternalEventMap[E]): Promise; } diff --git a/server/src/domain/storage-template/storage-template.service.spec.ts b/server/src/domain/storage-template/storage-template.service.spec.ts index 388b9c4d6c..21fa6ef7df 100644 --- a/server/src/domain/storage-template/storage-template.service.spec.ts +++ b/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 }]); diff --git a/server/src/domain/storage-template/storage-template.service.ts b/server/src/domain/storage-template/storage-template.service.ts index e2b74c0da2..ffdbfbefbd 100644 --- a/server/src/domain/storage-template/storage-template.service.ts +++ b/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 { 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) { diff --git a/server/src/domain/system-config/system-config.core.ts b/server/src/domain/system-config/system-config.core.ts index 4a45de93e9..93a4937cb3 100644 --- a/server/src/domain/system-config/system-config.core.ts +++ b/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[] | null = null; public config$ = new Subject(); @@ -245,10 +244,6 @@ export class SystemConfigCore { return defaults; } - public addValidator(validator: SystemConfigValidator) { - this.validators.push(validator); - } - public async getConfig(force = false): Promise { 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[] = []; diff --git a/server/src/domain/system-config/system-config.service.spec.ts b/server/src/domain/system-config/system-config.service.spec.ts index 5602c378be..fd9c164638 100644 --- a/server/src/domain/system-config/system-config.service.spec.ts +++ b/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({})); diff --git a/server/src/domain/system-config/system-config.service.ts b/server/src/domain/system-config/system-config.service.ts index 54d113cf60..7e68cf0b9b 100644 --- a/server/src/domain/system-config/system-config.service.ts +++ b/server/src/domain/system-config/system-config.service.ts @@ -1,6 +1,7 @@ import { LogLevel, SystemConfig } from '@app/infra/entities'; import { ImmichLogger } from '@app/infra/logger'; -import { Inject, Injectable } from '@nestjs/common'; +import { BadRequestException, Inject, Injectable } from '@nestjs/common'; +import { OnEvent } from '@nestjs/event-emitter'; import { instanceToPlain } from 'class-transformer'; import _ from 'lodash'; import { @@ -8,6 +9,8 @@ import { ICommunicationRepository, ISearchRepository, ISystemConfigRepository, + InternalEvent, + InternalEventMap, ServerEvent, } from '../repositories'; import { SystemConfigDto, mapConfig } from './dto/system-config.dto'; @@ -22,7 +25,7 @@ import { supportedWeekTokens, supportedYearTokens, } from './system-config.constants'; -import { SystemConfigCore, SystemConfigValidator } from './system-config.core'; +import { SystemConfigCore } from './system-config.core'; @Injectable() export class SystemConfigService { @@ -37,7 +40,6 @@ export class SystemConfigService { this.core = SystemConfigCore.create(repository); this.communicationRepository.on(ServerEvent.CONFIG_UPDATE, () => this.handleConfigUpdate()); this.core.config$.subscribe((config) => this.setLogLevel(config)); - this.core.addValidator((newConfig, oldConfig) => this.validateConfig(newConfig, oldConfig)); } async init() { @@ -59,8 +61,23 @@ export class SystemConfigService { return mapConfig(config); } + @OnEvent(InternalEvent.VALIDATE_CONFIG) + validateConfig({ newConfig, oldConfig }: InternalEventMap[InternalEvent.VALIDATE_CONFIG]) { + if (!_.isEqual(instanceToPlain(newConfig.logging), oldConfig.logging) && this.getEnvLogLevel()) { + throw new Error('Logging cannot be changed while the environment variable LOG_LEVEL is set.'); + } + } + async updateConfig(dto: SystemConfigDto): Promise { const oldConfig = await this.core.getConfig(); + + try { + await this.communicationRepository.emitAsync(InternalEvent.VALIDATE_CONFIG, { newConfig: dto, 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 newConfig = await this.core.updateConfig(dto); this.communicationRepository.broadcast(ClientEvent.CONFIG_UPDATE, {}); @@ -79,10 +96,6 @@ export class SystemConfigService { return true; } - addValidator(validator: SystemConfigValidator) { - this.core.addValidator(validator); - } - getStorageTemplateOptions(): SystemConfigTemplateStorageOptionDto { const options = new SystemConfigTemplateStorageOptionDto(); @@ -129,10 +142,4 @@ export class SystemConfigService { private getEnvLogLevel() { return process.env.LOG_LEVEL as LogLevel; } - - private validateConfig(newConfig: SystemConfig, oldConfig: SystemConfig) { - if (!_.isEqual(instanceToPlain(newConfig.logging), oldConfig.logging) && this.getEnvLogLevel()) { - throw new Error('Logging cannot be changed while the environment variable LOG_LEVEL is set.'); - } - } } diff --git a/server/src/infra/infra.module.ts b/server/src/infra/infra.module.ts index cdd5ab442c..df3773deb2 100644 --- a/server/src/infra/infra.module.ts +++ b/server/src/infra/infra.module.ts @@ -31,6 +31,7 @@ import { import { BullModule } from '@nestjs/bullmq'; import { Global, Module, Provider } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; +import { EventEmitterModule } from '@nestjs/event-emitter'; import { ScheduleModule, SchedulerRegistry } from '@nestjs/schedule'; import { TypeOrmModule } from '@nestjs/typeorm'; import { OpenTelemetryModule } from 'nestjs-otel'; @@ -103,6 +104,7 @@ const providers: Provider[] = [ @Module({ imports: [ ConfigModule.forRoot(immichAppConfig), + EventEmitterModule.forRoot(), TypeOrmModule.forRoot(databaseConfig), TypeOrmModule.forFeature(databaseEntities), ScheduleModule, @@ -119,6 +121,7 @@ export class InfraModule {} @Module({ imports: [ ConfigModule.forRoot(immichAppConfig), + EventEmitterModule.forRoot(), TypeOrmModule.forRoot(databaseConfig), TypeOrmModule.forFeature(databaseEntities), ScheduleModule, diff --git a/server/src/infra/repositories/communication.repository.ts b/server/src/infra/repositories/communication.repository.ts index b7ac6ac99f..6429b6e191 100644 --- a/server/src/infra/repositories/communication.repository.ts +++ b/server/src/infra/repositories/communication.repository.ts @@ -2,11 +2,13 @@ import { AuthService, ClientEvent, ICommunicationRepository, + InternalEventMap, OnConnectCallback, OnServerEventCallback, ServerEvent, } from '@app/domain'; import { ImmichLogger } from '@app/infra/logger'; +import { EventEmitter2 } from '@nestjs/event-emitter'; import { OnGatewayConnection, OnGatewayDisconnect, @@ -35,7 +37,10 @@ export class CommunicationRepository @WebSocketServer() private server?: Server; - constructor(private authService: AuthService) {} + constructor( + private authService: AuthService, + private eventEmitter: EventEmitter2, + ) {} afterInit(server: Server) { this.logger.log('Initialized websocket server'); @@ -97,4 +102,12 @@ export class CommunicationRepository this.logger.debug(`Server event: ${event} (send)`); this.server?.serverSideEmit(event); } + + emit(event: E, data: InternalEventMap[E]): boolean { + return this.eventEmitter.emit(event, data); + } + + emitAsync(event: E, data: InternalEventMap[E]): Promise { + return this.eventEmitter.emitAsync(event, data) as Promise; + } } diff --git a/server/test/repositories/communication.repository.mock.ts b/server/test/repositories/communication.repository.mock.ts index 6fb95bffd9..e98e0a68f7 100644 --- a/server/test/repositories/communication.repository.mock.ts +++ b/server/test/repositories/communication.repository.mock.ts @@ -6,5 +6,7 @@ export const newCommunicationRepositoryMock = (): jest.Mocked