refactor: config init event for first config load (immich-app#13930)

server/src/app.module.ts

@@ -78,11 +78,11 @@ class BaseModule implements OnModuleInit, OnModuleDestroy {
     }
 
     this.eventRepository.setup({ services });
-    await this.eventRepository.emit('app.bootstrap', this.worker);
+    await this.eventRepository.emit('app.bootstrap');
   }
 
   async onModuleDestroy() {
-    await this.eventRepository.emit('app.shutdown', this.worker);
+    await this.eventRepository.emit('app.shutdown');
     await teardownTelemetry();
   }
 }

server/src/interfaces/event.interface.ts

@@ -2,22 +2,21 @@ import { ClassConstructor } from 'class-transformer';
 import { SystemConfig } from 'src/config';
 import { AssetResponseDto } from 'src/dtos/asset-response.dto';
 import { ReleaseNotification, ServerVersionResponseDto } from 'src/dtos/server.dto';
-import { ImmichWorker } from 'src/enum';
 import { JobItem, QueueName } from 'src/interfaces/job.interface';
 
 export const IEventRepository = 'IEventRepository';
 
 type EventMap = {
   // app events
-  'app.bootstrap': [ImmichWorker];
-  'app.shutdown': [ImmichWorker];
+  'app.bootstrap': [];
+  'app.shutdown': [];
 
+  'config.init': [{ newConfig: SystemConfig }];
   // config events
   'config.update': [
     {
       newConfig: SystemConfig;
-      /** When the server starts, `oldConfig` is `undefined` */
-      oldConfig?: SystemConfig;
+      oldConfig: SystemConfig;
     },
   ];
   'config.validate': [{ newConfig: SystemConfig; oldConfig: SystemConfig }];
@@ -89,6 +88,13 @@ export type EventItem<T extends EmitEvent> = {
   server: boolean;
 };
 
+export enum BootstrapEventPriority {
+  // Database service should be initialized before anything else, most other services need database access
+  DatabaseService = -200,
+  // Initialise config after other bootstrap services, stop other services from using config on bootstrap
+  SystemConfig = 100,
+}
+
 export interface IEventRepository {
   setup(options: { services: ClassConstructor<unknown>[] }): void;
   emit<T extends keyof EventMap>(event: T, ...args: ArgsOf<T>): Promise<void>;

server/src/services/backup.service.spec.ts

@@ -2,6 +2,7 @@ import { PassThrough } from 'node:stream';
 import { defaults, SystemConfig } from 'src/config';
 import { StorageCore } from 'src/cores/storage.core';
 import { ImmichWorker, StorageFolder } from 'src/enum';
+import { IConfigRepository } from 'src/interfaces/config.interface';
 import { IDatabaseRepository } from 'src/interfaces/database.interface';
 import { IJobRepository, JobStatus } from 'src/interfaces/job.interface';
 import { IProcessRepository } from 'src/interfaces/process.interface';
@@ -16,13 +17,14 @@ describe(BackupService.name, () => {
   let sut: BackupService;
 
   let databaseMock: Mocked<IDatabaseRepository>;
+  let configMock: Mocked<IConfigRepository>;
   let jobMock: Mocked<IJobRepository>;
   let processMock: Mocked<IProcessRepository>;
   let storageMock: Mocked<IStorageRepository>;
   let systemMock: Mocked<ISystemMetadataRepository>;
 
   beforeEach(() => {
-    ({ sut, databaseMock, jobMock, processMock, storageMock, systemMock } = newTestService(BackupService));
+    ({ sut, configMock, databaseMock, jobMock, processMock, storageMock, systemMock } = newTestService(BackupService));
   });
 
   it('should work', () => {
@@ -32,35 +34,32 @@ describe(BackupService.name, () => {
   describe('onBootstrapEvent', () => {
     it('should init cron job and handle config changes', async () => {
       databaseMock.tryLock.mockResolvedValue(true);
-      systemMock.get.mockResolvedValue(systemConfigStub.backupEnabled);
 
-      await sut.onBootstrap(ImmichWorker.API);
+      await sut.onConfigInit({ newConfig: systemConfigStub.backupEnabled as SystemConfig });
 
       expect(jobMock.addCronJob).toHaveBeenCalled();
-      expect(systemMock.get).toHaveBeenCalled();
     });
 
     it('should not initialize backup database cron job when lock is taken', async () => {
-      systemMock.get.mockResolvedValue(systemConfigStub.backupEnabled);
       databaseMock.tryLock.mockResolvedValue(false);
 
-      await sut.onBootstrap(ImmichWorker.API);
+      await sut.onConfigInit({ newConfig: systemConfigStub.backupEnabled as SystemConfig });
 
       expect(jobMock.addCronJob).not.toHaveBeenCalled();
     });
 
     it('should not initialise backup database job when running on microservices', async () => {
-      await sut.onBootstrap(ImmichWorker.MICROSERVICES);
+      configMock.getWorker.mockReturnValue(ImmichWorker.MICROSERVICES);
+      await sut.onConfigInit({ newConfig: systemConfigStub.backupEnabled as SystemConfig });
 
       expect(jobMock.addCronJob).not.toHaveBeenCalled();
     });
   });
 
   describe('onConfigUpdateEvent', () => {
     beforeEach(async () => {
-      systemMock.get.mockResolvedValue(defaults);
       databaseMock.tryLock.mockResolvedValue(true);
-      await sut.onBootstrap(ImmichWorker.API);
+      await sut.onConfigInit({ newConfig: defaults });
     });
 
     it('should update cron job if backup is enabled', () => {
@@ -80,14 +79,9 @@ describe(BackupService.name, () => {
       expect(jobMock.updateCronJob).toHaveBeenCalled();
     });
 
-    it('should do nothing if oldConfig is not provided', () => {
-      sut.onConfigUpdate({ newConfig: systemConfigStub.backupEnabled as SystemConfig });
-      expect(jobMock.updateCronJob).not.toHaveBeenCalled();
-    });
-
     it('should do nothing if instance does not have the backup database lock', async () => {
       databaseMock.tryLock.mockResolvedValue(false);
-      await sut.onBootstrap(ImmichWorker.API);
+      await sut.onConfigInit({ newConfig: defaults });
       sut.onConfigUpdate({ newConfig: systemConfigStub.backupEnabled as SystemConfig, oldConfig: defaults });
       expect(jobMock.updateCronJob).not.toHaveBeenCalled();
     });

server/src/services/backup.service.ts

@@ -14,14 +14,15 @@ import { validateCronExpression } from 'src/validation';
 export class BackupService extends BaseService {
   private backupLock = false;
 
-  @OnEvent({ name: 'app.bootstrap' })
-  async onBootstrap(workerType: ImmichWorker) {
-    if (workerType !== ImmichWorker.API) {
+  @OnEvent({ name: 'config.init' })
+  async onConfigInit({
+    newConfig: {
+      backup: { database },
+    },
+  }: ArgOf<'config.init'>) {
+    if (this.worker !== ImmichWorker.API) {
       return;
     }
-    const {
-      backup: { database },
-    } = await this.getConfig({ withCache: true });
 
     this.backupLock = await this.databaseRepository.tryLock(DatabaseLock.BackupDatabase);
 
@@ -36,8 +37,8 @@ export class BackupService extends BaseService {
   }
 
   @OnEvent({ name: 'config.update', server: true })
-  onConfigUpdate({ newConfig: { backup }, oldConfig }: ArgOf<'config.update'>) {
-    if (!oldConfig || !this.backupLock) {
+  onConfigUpdate({ newConfig: { backup } }: ArgOf<'config.update'>) {
+    if (!this.backupLock) {
       return;
     }
 

server/src/services/database.service.ts

@@ -9,6 +9,7 @@ import {
   VectorExtension,
   VectorIndex,
 } from 'src/interfaces/database.interface';
+import { BootstrapEventPriority } from 'src/interfaces/event.interface';
 import { BaseService } from 'src/services/base.service';
 
 type CreateFailedArgs = { name: string; extension: string; otherName: string };
@@ -64,7 +65,7 @@ const RETRY_DURATION = Duration.fromObject({ seconds: 5 });
 export class DatabaseService extends BaseService {
   private reconnection?: NodeJS.Timeout;
 
-  @OnEvent({ name: 'app.bootstrap', priority: -200 })
+  @OnEvent({ name: 'app.bootstrap', priority: BootstrapEventPriority.DatabaseService })
   async onBootstrap() {
     const version = await this.databaseRepository.getPostgresVersion();
     const current = semver.coerce(version);

server/src/services/job.service.spec.ts

@@ -31,7 +31,7 @@ describe(JobService.name, () => {
 
   describe('onConfigUpdate', () => {
     it('should update concurrency', () => {
-      sut.onConfigUpdate({ oldConfig: defaults, newConfig: defaults });
+      sut.onConfigInitOrUpdate({ newConfig: defaults });
 
       expect(jobMock.setConcurrency).toHaveBeenCalledTimes(15);
       expect(jobMock.setConcurrency).toHaveBeenNthCalledWith(5, QueueName.FACIAL_RECOGNITION, 1);

server/src/services/job.service.ts

@@ -38,8 +38,9 @@ const asJobItem = (dto: JobCreateDto): JobItem => {
 
 @Injectable()
 export class JobService extends BaseService {
+  @OnEvent({ name: 'config.init' })
   @OnEvent({ name: 'config.update', server: true })
-  onConfigUpdate({ newConfig: config }: ArgOf<'config.update'>) {
+  onConfigInitOrUpdate({ newConfig: config }: ArgOf<'config.init'>) {
     if (this.worker !== ImmichWorker.MICROSERVICES) {
       return;
     }