From 3d9c677c4a98b13f99afdaa529793aab59e97ba6 Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Thu, 29 Feb 2024 23:36:05 +0100 Subject: [PATCH] refactor event type mocks --- .../jobs/specs/library-watcher.e2e-spec.ts | 16 +++++------ .../domain/library/library.service.spec.ts | 27 ++++++++++++------- server/src/domain/library/library.service.ts | 18 +++++++------ .../domain/repositories/storage.repository.ts | 2 +- .../infra/repositories/filesystem.provider.ts | 12 ++++----- server/src/test-utils/utils.ts | 6 ++--- .../repositories/storage.repository.mock.ts | 20 +++++++++----- 7 files changed, 60 insertions(+), 41 deletions(-) diff --git a/server/e2e/jobs/specs/library-watcher.e2e-spec.ts b/server/e2e/jobs/specs/library-watcher.e2e-spec.ts index 0199933723..95112c8dd2 100644 --- a/server/e2e/jobs/specs/library-watcher.e2e-spec.ts +++ b/server/e2e/jobs/specs/library-watcher.e2e-spec.ts @@ -1,4 +1,4 @@ -import { LibraryResponseDto, LibraryService, LoginResponseDto, StorageEvent } from '@app/domain'; +import { LibraryResponseDto, LibraryService, LoginResponseDto, StorageEventType } from '@app/domain'; import { AssetType, LibraryType } from '@app/infra/entities'; import fs from 'node:fs/promises'; import path from 'node:path'; @@ -57,7 +57,7 @@ describe(`Library watcher (e2e)`, () => { `${IMMICH_TEST_ASSET_TEMP_PATH}/file.jpg`, ); - await waitForEvent(libraryService, StorageEvent.ADD); + await waitForEvent(libraryService, StorageEventType.ADD); const afterAssets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(afterAssets.length).toEqual(1); @@ -84,7 +84,7 @@ describe(`Library watcher (e2e)`, () => { `${IMMICH_TEST_ASSET_TEMP_PATH}/file5.jPg`, ); - await waitForEvent(libraryService, StorageEvent.ADD, 4); + await waitForEvent(libraryService, StorageEventType.ADD, 4); const afterAssets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(afterAssets.length).toEqual(4); @@ -96,7 +96,7 @@ describe(`Library watcher (e2e)`, () => { `${IMMICH_TEST_ASSET_TEMP_PATH}/file.jpg`, ); - await waitForEvent(libraryService, StorageEvent.ADD); + await waitForEvent(libraryService, StorageEventType.ADD); const originalAssets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(originalAssets.length).toEqual(1); @@ -106,7 +106,7 @@ describe(`Library watcher (e2e)`, () => { `${IMMICH_TEST_ASSET_TEMP_PATH}/file.jpg`, ); - await waitForEvent(libraryService, StorageEvent.CHANGE); + await waitForEvent(libraryService, StorageEventType.CHANGE); const afterAssets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(afterAssets).toEqual([ @@ -158,7 +158,7 @@ describe(`Library watcher (e2e)`, () => { `${IMMICH_TEST_ASSET_TEMP_PATH}/dir3/file4.jpg`, ); - await waitForEvent(libraryService, StorageEvent.ADD, 3); + await waitForEvent(libraryService, StorageEventType.ADD, 3); const assets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(assets.length).toEqual(3); @@ -170,14 +170,14 @@ describe(`Library watcher (e2e)`, () => { `${IMMICH_TEST_ASSET_TEMP_PATH}/dir1/file.jpg`, ); - await waitForEvent(libraryService, StorageEvent.ADD); + await waitForEvent(libraryService, StorageEventType.ADD); const addedAssets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(addedAssets.length).toEqual(1); await fs.unlink(`${IMMICH_TEST_ASSET_TEMP_PATH}/dir1/file.jpg`); - await waitForEvent(libraryService, StorageEvent.UNLINK); + await waitForEvent(libraryService, StorageEventType.UNLINK); const afterAssets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(afterAssets[0].isOffline).toEqual(true); diff --git a/server/src/domain/library/library.service.spec.ts b/server/src/domain/library/library.service.spec.ts index 45980f286e..9c4d5cce51 100644 --- a/server/src/domain/library/library.service.spec.ts +++ b/server/src/domain/library/library.service.spec.ts @@ -29,6 +29,7 @@ import { IStorageRepository, ISystemConfigRepository, IUserRepository, + StorageEventType, } from '../repositories'; import { SystemConfigCore } from '../system-config/system-config.core'; import { mapLibrary } from './library.dto'; @@ -1187,7 +1188,9 @@ describe(LibraryService.name, () => { it('should handle a new file event', async () => { libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1); libraryMock.getAll.mockResolvedValue([libraryStub.externalLibraryWithImportPaths1]); - storageMock.watch.mockImplementation(makeMockWatcher({ items: [{ event: 'add', value: '/foo/photo.jpg' }] })); + storageMock.watch.mockImplementation( + makeMockWatcher({ items: [{ event: StorageEventType.ADD, value: '/foo/photo.jpg' }] }), + ); await sut.watchAll(); @@ -1208,7 +1211,7 @@ describe(LibraryService.name, () => { libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1); libraryMock.getAll.mockResolvedValue([libraryStub.externalLibraryWithImportPaths1]); storageMock.watch.mockImplementation( - makeMockWatcher({ items: [{ event: 'change', value: '/foo/photo.jpg' }] }), + makeMockWatcher({ items: [{ event: StorageEventType.CHANGE, value: '/foo/photo.jpg' }] }), ); await sut.watchAll(); @@ -1231,7 +1234,7 @@ describe(LibraryService.name, () => { libraryMock.getAll.mockResolvedValue([libraryStub.externalLibraryWithImportPaths1]); assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.external); storageMock.watch.mockImplementation( - makeMockWatcher({ items: [{ event: 'unlink', value: '/foo/photo.jpg' }] }), + makeMockWatcher({ items: [{ event: StorageEventType.UNLINK, value: '/foo/photo.jpg' }] }), ); await sut.watchAll(); @@ -1245,17 +1248,19 @@ describe(LibraryService.name, () => { libraryMock.getAll.mockResolvedValue([libraryStub.externalLibraryWithImportPaths1]); storageMock.watch.mockImplementation( makeMockWatcher({ - items: [{ event: 'error', value: 'Error!' }], + items: [{ event: StorageEventType.ERROR, value: 'Error!' }], }), ); - await sut.watchAll(); + await expect(sut.watchAll()).rejects.toEqual([expect.stringMatching('/Error: Error/')]); }); it('should ignore unknown extensions', async () => { libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1); libraryMock.getAll.mockResolvedValue([libraryStub.externalLibraryWithImportPaths1]); - storageMock.watch.mockImplementation(makeMockWatcher({ items: [{ event: 'add', value: '/foo/photo.jpg' }] })); + storageMock.watch.mockImplementation( + makeMockWatcher({ items: [{ event: StorageEventType.ADD, value: '/foo/photo.jpg' }] }), + ); await sut.watchAll(); @@ -1265,7 +1270,9 @@ describe(LibraryService.name, () => { it('should ignore excluded paths', async () => { libraryMock.get.mockResolvedValue(libraryStub.patternPath); libraryMock.getAll.mockResolvedValue([libraryStub.patternPath]); - storageMock.watch.mockImplementation(makeMockWatcher({ items: [{ event: 'add', value: '/dir1/photo.txt' }] })); + storageMock.watch.mockImplementation( + makeMockWatcher({ items: [{ event: StorageEventType.ADD, value: '/dir1/photo.txt' }] }), + ); await sut.watchAll(); @@ -1275,7 +1282,9 @@ describe(LibraryService.name, () => { it('should ignore excluded paths without case sensitivity', async () => { libraryMock.get.mockResolvedValue(libraryStub.patternPath); libraryMock.getAll.mockResolvedValue([libraryStub.patternPath]); - storageMock.watch.mockImplementation(makeMockWatcher({ items: [{ event: 'add', value: '/DIR1/photo.txt' }] })); + storageMock.watch.mockImplementation( + makeMockWatcher({ items: [{ event: StorageEventType.ADD, value: '/DIR1/photo.txt' }] }), + ); await sut.watchAll(); @@ -1284,7 +1293,7 @@ describe(LibraryService.name, () => { }); }); - describe('tearDown', () => { + describe('teardown', () => { it('should tear down all watchers', async () => { libraryMock.getAll.mockResolvedValue([ libraryStub.externalLibraryWithImportPaths1, diff --git a/server/src/domain/library/library.service.ts b/server/src/domain/library/library.service.ts index 34660a30d5..fe276fdff9 100644 --- a/server/src/domain/library/library.service.ts +++ b/server/src/domain/library/library.service.ts @@ -23,7 +23,7 @@ import { IStorageRepository, ISystemConfigRepository, IUserRepository, - StorageEvent, + StorageEventType, WithProperty, } from '../repositories'; import { SystemConfigCore } from '../system-config'; @@ -97,9 +97,11 @@ export class LibraryService extends EventEmitter { this.configCore.config$.subscribe(async ({ library }) => { this.jobRepository.updateCronJob('libraryScan', library.scan.cronExpression, library.scan.enabled); - if (this.watchLock && library.watch.enabled !== this.watchLibraries) { - this.watchLibraries = library.watch.enabled; - await (this.watchLibraries ? this.watchAll() : this.unwatchAll()); + if (this.watchLock) { + if (library.watch.enabled !== this.watchLibraries) { + this.watchLibraries = library.watch.enabled; + await (this.watchLibraries ? this.watchAll() : this.unwatchAll()); + } } }); } @@ -142,7 +144,7 @@ export class LibraryService extends EventEmitter { if (matcher(path)) { await this.scanAssets(library.id, [path], library.ownerId, false); } - this.emit(StorageEvent.ADD, path); + this.emit(StorageEventType.ADD, path); }, onChange: async (path) => { this.logger.debug(`Detected file change for ${path} in library ${library.id}`); @@ -150,7 +152,7 @@ export class LibraryService extends EventEmitter { // Note: if the changed file was not previously imported, it will be imported now. await this.scanAssets(library.id, [path], library.ownerId, false); } - this.emit(StorageEvent.CHANGE, path); + this.emit(StorageEventType.CHANGE, path); }, onUnlink: async (path) => { this.logger.debug(`Detected deleted file at ${path} in library ${library.id}`); @@ -158,12 +160,12 @@ export class LibraryService extends EventEmitter { if (asset && matcher(path)) { await this.assetRepository.save({ id: asset.id, isOffline: true }); } - this.emit(StorageEvent.UNLINK, path); + this.emit(StorageEventType.UNLINK, path); }, onError: (error) => { // TODO: should we log, or throw an exception? this.logger.error(`Library watcher for library ${library.id} encountered error: ${error}`); - this.emit(StorageEvent.ERROR, error); + this.emit(StorageEventType.ERROR, error); }, }, ); diff --git a/server/src/domain/repositories/storage.repository.ts b/server/src/domain/repositories/storage.repository.ts index b07fa78f82..d9f8c27ef2 100644 --- a/server/src/domain/repositories/storage.repository.ts +++ b/server/src/domain/repositories/storage.repository.ts @@ -31,7 +31,7 @@ export interface WatchEvents { onError(error: Error): void; } -export enum StorageEvent { +export enum StorageEventType { READY = 'ready', ADD = 'add', CHANGE = 'change', diff --git a/server/src/infra/repositories/filesystem.provider.ts b/server/src/infra/repositories/filesystem.provider.ts index 6b306d8f2d..fef184992d 100644 --- a/server/src/infra/repositories/filesystem.provider.ts +++ b/server/src/infra/repositories/filesystem.provider.ts @@ -4,7 +4,7 @@ import { IStorageRepository, ImmichReadStream, ImmichZipStream, - StorageEvent, + StorageEventType, WatchEvents, mimeTypes, } from '@app/domain'; @@ -142,11 +142,11 @@ export class FilesystemProvider implements IStorageRepository { watch(paths: string[], options: WatchOptions, events: Partial) { const watcher = chokidar.watch(paths, options); - watcher.on(StorageEvent.READY, () => events.onReady?.()); - watcher.on(StorageEvent.ADD, (path) => events.onAdd?.(path)); - watcher.on(StorageEvent.CHANGE, (path) => events.onChange?.(path)); - watcher.on(StorageEvent.UNLINK, (path) => events.onUnlink?.(path)); - watcher.on(StorageEvent.ERROR, (error) => events.onError?.(error)); + watcher.on(StorageEventType.READY, () => events.onReady?.()); + watcher.on(StorageEventType.ADD, (path) => events.onAdd?.(path)); + watcher.on(StorageEventType.CHANGE, (path) => events.onChange?.(path)); + watcher.on(StorageEventType.UNLINK, (path) => events.onUnlink?.(path)); + watcher.on(StorageEventType.ERROR, (error) => events.onError?.(error)); return () => watcher.close(); } diff --git a/server/src/test-utils/utils.ts b/server/src/test-utils/utils.ts index 1517448ef0..62bf69df46 100644 --- a/server/src/test-utils/utils.ts +++ b/server/src/test-utils/utils.ts @@ -1,4 +1,4 @@ -import { IJobRepository, IMediaRepository, JobItem, JobItemHandler, QueueName, StorageEvent } from '@app/domain'; +import { IJobRepository, IMediaRepository, JobItem, JobItemHandler, QueueName, StorageEventType } from '@app/domain'; import { AppModule } from '@app/immich'; import { InfraModule, InfraTestModule, dataSource } from '@app/infra'; import { MediaRepository } from '@app/infra/repositories'; @@ -145,7 +145,7 @@ export function waitForEvent(emitter: EventEmitter, event: string, times = 1): P promises.push( new Promise((resolve, reject) => { const success = (value: any) => { - emitter.off(StorageEvent.ERROR, fail); + emitter.off(StorageEventType.ERROR, fail); resolve(value); }; const fail = (error: Error) => { @@ -153,7 +153,7 @@ export function waitForEvent(emitter: EventEmitter, event: string, times = 1): P reject(error); }; emitter.once(event, success); - emitter.once(StorageEvent.ERROR, fail); + emitter.once(StorageEventType.ERROR, fail); }), ); } diff --git a/server/test/repositories/storage.repository.mock.ts b/server/test/repositories/storage.repository.mock.ts index 1ee57b78dd..1066d4f688 100644 --- a/server/test/repositories/storage.repository.mock.ts +++ b/server/test/repositories/storage.repository.mock.ts @@ -1,8 +1,16 @@ -import { IStorageRepository, StorageCore, WatchEvents } from '@app/domain'; +import { IStorageRepository, StorageCore, StorageEventType, WatchEvents } from '@app/domain'; import { WatchOptions } from 'chokidar'; interface MockWatcherOptions { - items?: Array<{ event: 'change' | 'add' | 'unlink' | 'error'; value: string }>; + items?: Array<{ + event: + | StorageEventType.READY + | StorageEventType.ADD + | StorageEventType.CHANGE + | StorageEventType.UNLINK + | StorageEventType.ERROR; + value: string; + }>; close?: () => void; } @@ -12,19 +20,19 @@ export const makeMockWatcher = events.onReady?.(); for (const item of items || []) { switch (item.event) { - case 'add': { + case StorageEventType.ADD: { events.onAdd?.(item.value); break; } - case 'change': { + case StorageEventType.CHANGE: { events.onChange?.(item.value); break; } - case 'unlink': { + case StorageEventType.UNLINK: { events.onUnlink?.(item.value); break; } - case 'error': { + case StorageEventType.ERROR: { events.onError?.(new Error(item.value)); } }