Skip to content

Commit

Permalink
use events for config validation
Browse files Browse the repository at this point in the history
  • Loading branch information
danieldietzler committed Mar 15, 2024
1 parent abedfd1 commit d8c74bd
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 81 deletions.
31 changes: 31 additions & 0 deletions server/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions server/src/domain/domain.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import { TrashService } from './trash';
import { UserService } from './user';

const providers: Provider[] = [
APIKeyService,
ActivityService,
AlbumService,
APIKeyService,
AssetService,
AuditService,
AuthService,
Expand All @@ -39,8 +39,8 @@ const providers: Provider[] = [
LibraryService,
MediaService,
MetadataService,
PersonService,
PartnerService,
PersonService,
SearchService,
ServerInfoService,
SharedLinkService,
Expand Down
18 changes: 11 additions & 7 deletions server/src/domain/library/library.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { AssetType, LibraryEntity, LibraryType } from '@app/infra/entities';
import { AssetType, LibraryEntity, LibraryType, SystemConfig } 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';
Expand All @@ -22,6 +23,7 @@ import {
ILibraryRepository,
IStorageRepository,
ISystemConfigRepository,
InternalEvent,
JobStatus,
StorageEventType,
WithProperty,
Expand Down Expand Up @@ -64,12 +66,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() {
Expand Down Expand Up @@ -109,6 +105,14 @@ export class LibraryService extends EventEmitter {
});
}

@OnEvent(InternalEvent.VALIDATE_CONFIG)
validateConfig(newConfig: SystemConfig) {
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;
Expand Down
6 changes: 6 additions & 0 deletions server/src/domain/repositories/communication.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export enum ServerEvent {
CONFIG_UPDATE = 'config:update',
}

export enum InternalEvent {
VALIDATE_CONFIG = 'validate_config',
}

export interface ClientEventMap {
[ClientEvent.UPLOAD_SUCCESS]: AssetResponseDto;
[ClientEvent.USER_DELETE]: string;
Expand All @@ -45,4 +49,6 @@ export interface ICommunicationRepository {
on(event: 'connect', callback: OnConnectCallback): void;
on(event: ServerEvent, callback: OnServerEventCallback): void;
sendServerEvent(event: ServerEvent): void;
emit(eventName: string, ...parameters: any[]): boolean;
emitAsync(eventName: string, ...parameters: any[]): Promise<any[]>;
}
44 changes: 23 additions & 21 deletions server/src/domain/storage-template/storage-template.service.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -18,6 +19,7 @@ import {
IStorageRepository,
ISystemConfigRepository,
IUserRepository,
InternalEvent,
JobStatus,
} from '../repositories';
import { StorageCore, StorageFolder } from '../storage';
Expand Down Expand Up @@ -74,7 +76,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,
Expand All @@ -86,6 +87,27 @@ export class StorageTemplateService {
);
}

@OnEvent(InternalEvent.VALIDATE_CONFIG)
validate(newConfig: SystemConfig) {
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;
Expand Down Expand Up @@ -259,26 +281,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) {
Expand Down
16 changes: 0 additions & 16 deletions server/src/domain/system-config/system-config.core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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[] = [];

Expand Down
22 changes: 1 addition & 21 deletions server/src/domain/system-config/system-config.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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({}));
Expand Down
32 changes: 19 additions & 13 deletions server/src/domain/system-config/system-config.service.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
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 {
ClientEvent,
ICommunicationRepository,
ISearchRepository,
ISystemConfigRepository,
InternalEvent,
ServerEvent,
} from '../repositories';
import { SystemConfigDto, mapConfig } from './dto/system-config.dto';
Expand All @@ -22,7 +24,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 {
Expand All @@ -37,7 +39,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() {
Expand All @@ -59,8 +60,23 @@ export class SystemConfigService {
return mapConfig(config);
}

@OnEvent(InternalEvent.VALIDATE_CONFIG)
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.');
}
}

async updateConfig(dto: SystemConfigDto): Promise<SystemConfigDto> {
const oldConfig = await this.core.getConfig();

try {
await this.communicationRepository.emitAsync(InternalEvent.VALIDATE_CONFIG, 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, {});
Expand All @@ -79,10 +95,6 @@ export class SystemConfigService {
return true;
}

addValidator(validator: SystemConfigValidator) {
this.core.addValidator(validator);
}

getStorageTemplateOptions(): SystemConfigTemplateStorageOptionDto {
const options = new SystemConfigTemplateStorageOptionDto();

Expand Down Expand Up @@ -129,10 +141,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.');
}
}
}

0 comments on commit d8c74bd

Please sign in to comment.