1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2024-12-28 22:51:59 +00:00

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>
This commit is contained in:
Daniel Dietzler 2024-03-17 20:16:02 +01:00 committed by GitHub
parent 14da671bf9
commit 148428a564
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 170 additions and 81 deletions

View file

@ -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",

View file

@ -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",

View file

@ -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,

View file

@ -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 = {

View file

@ -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;

View file

@ -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>;
}

View file

@ -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 }]);

View file

@ -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) {

View file

@ -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[] = [];

View file

@ -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({}));

View file

@ -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<SystemConfigDto> {
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.');
}
}
}

View file

@ -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,

View file

@ -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<E extends keyof InternalEventMap>(event: E, data: InternalEventMap[E]): boolean {
return this.eventEmitter.emit(event, data);
}
emitAsync<E extends keyof InternalEventMap, R = any[]>(event: E, data: InternalEventMap[E]): Promise<R> {
return this.eventEmitter.emitAsync(event, data) as Promise<R>;
}
}

View file

@ -6,5 +6,7 @@ export const newCommunicationRepositoryMock = (): jest.Mocked<ICommunicationRepo
broadcast: jest.fn(),
on: jest.fn(),
sendServerEvent: jest.fn(),
emit: jest.fn(),
emitAsync: jest.fn(),
};
};