mirror of
https://github.com/immich-app/immich.git
synced 2026-09-30 13:23:21 +08:00
fix: library watcher should respect ignore patterns (#31418)
* fix: chokidar should respect ignore patterns * fix: chokidar mock --------- Co-authored-by: Jason Rasmussen <jason@rasm.me> Co-authored-by: Daniel Dietzler <mail@ddietzler.dev>
This commit is contained in:
co-authored by
Jason Rasmussen
Daniel Dietzler
parent
c12d5e6d35
commit
76c08fa230
@@ -1,9 +1,30 @@
|
||||
import mockfs from 'mock-fs';
|
||||
import { vitest } from 'vitest';
|
||||
import { CrawlOptionsDto } from 'src/dtos/library.dto.js';
|
||||
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
||||
import { StorageRepository } from 'src/repositories/storage.repository.js';
|
||||
import { automock } from 'test/utils.js';
|
||||
|
||||
const mocks = vitest.hoisted(() => {
|
||||
const watcher = {
|
||||
close: vitest.fn(),
|
||||
on: vitest.fn(),
|
||||
};
|
||||
watcher.on.mockReturnValue(watcher);
|
||||
|
||||
return { watch: vitest.fn(() => watcher), watcher };
|
||||
});
|
||||
|
||||
vitest.mock('chokidar', () => ({ watch: mocks.watch }));
|
||||
|
||||
const getHandler = (event: string) => {
|
||||
const handler = mocks.watcher.on.mock.calls.find(([name]) => name === event)?.[1];
|
||||
if (!handler) {
|
||||
throw new Error(`Missing ${event} handler`);
|
||||
}
|
||||
return handler;
|
||||
};
|
||||
|
||||
interface Test {
|
||||
test: string;
|
||||
options: CrawlOptionsDto;
|
||||
@@ -185,6 +206,9 @@ describe(StorageRepository.name, () => {
|
||||
beforeEach(() => {
|
||||
// eslint-disable-next-line no-sparse-arrays
|
||||
sut = new StorageRepository(automock(LoggingRepository, { args: [, { getEnv: () => ({}) }], strict: false }));
|
||||
mocks.watch.mockReset().mockReturnValue(mocks.watcher);
|
||||
mocks.watcher.on.mockReset().mockReturnValue(mocks.watcher);
|
||||
mocks.watcher.close.mockReset().mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -206,4 +230,55 @@ describe(StorageRepository.name, () => {
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('watch', () => {
|
||||
it('should register event handlers and close the watcher', async () => {
|
||||
const onReady = vitest.fn();
|
||||
const onAdd = vitest.fn();
|
||||
const onChange = vitest.fn();
|
||||
const onUnlink = vitest.fn();
|
||||
const onError = vitest.fn();
|
||||
const close = sut.watch(['/photos'], {}, { onAdd, onChange, onError, onReady, onUnlink });
|
||||
|
||||
expect(mocks.watch).toHaveBeenCalledWith(['/photos'], expect.objectContaining({ ignored: expect.any(Function) }));
|
||||
|
||||
const error = new Error('watch error');
|
||||
getHandler('ready')();
|
||||
getHandler('add')('/photos/add.jpg');
|
||||
getHandler('change')('/photos/change.jpg');
|
||||
getHandler('unlink')('/photos/unlink.jpg');
|
||||
getHandler('error')(error);
|
||||
|
||||
expect(onReady).toHaveBeenCalledWith();
|
||||
expect(onAdd).toHaveBeenCalledWith('/photos/add.jpg');
|
||||
expect(onChange).toHaveBeenCalledWith('/photos/change.jpg');
|
||||
expect(onUnlink).toHaveBeenCalledWith('/photos/unlink.jpg');
|
||||
expect(onError).toHaveBeenCalledWith(error);
|
||||
|
||||
await close();
|
||||
expect(mocks.watcher.close).toHaveBeenCalledWith();
|
||||
});
|
||||
|
||||
it('should convert ignored glob arrays to a case-insensitive matcher', () => {
|
||||
sut.watch(['/photos'], { ignored: ['**/excluded/**'] }, {});
|
||||
const [, options] = mocks.watch.mock.lastCall as unknown as [string[], { ignored?: unknown }];
|
||||
const ignored = options.ignored as (path: string) => boolean;
|
||||
|
||||
expect(typeof ignored).toBe('function');
|
||||
expect(ignored('/photos/EXCLUDED/photo.jpg')).toBe(true);
|
||||
expect(ignored('/photos/included/photo.jpg')).toBe(false);
|
||||
});
|
||||
|
||||
it('should tolerate missing event callbacks', () => {
|
||||
sut.watch(['/photos'], {}, {});
|
||||
|
||||
expect(() => {
|
||||
getHandler('ready')();
|
||||
getHandler('add')('/photos/add.jpg');
|
||||
getHandler('change')('/photos/change.jpg');
|
||||
getHandler('unlink')('/photos/unlink.jpg');
|
||||
getHandler('error')(new Error('watch error'));
|
||||
}).not.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,6 +16,7 @@ import fs from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
import { PassThrough, Readable, Writable } from 'node:stream';
|
||||
import { createGunzip, createGzip } from 'node:zlib';
|
||||
import picomatch from 'picomatch';
|
||||
import { CrawlOptionsDto, WalkOptionsDto } from 'src/dtos/library.dto.js';
|
||||
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
||||
import { mimeTypes } from 'src/utils/mime-types.js';
|
||||
@@ -28,6 +29,8 @@ export interface WatchEvents {
|
||||
onError(error: Error): void;
|
||||
}
|
||||
|
||||
export type WatchOptions = Omit<ChokidarOptions, 'ignored'> & { ignored?: string[] };
|
||||
|
||||
export interface ImmichReadStream {
|
||||
stream: Readable;
|
||||
type?: string;
|
||||
@@ -273,8 +276,15 @@ export class StorageRepository {
|
||||
}
|
||||
}
|
||||
|
||||
watch(paths: string[], options: ChokidarOptions, events: Partial<WatchEvents>) {
|
||||
const watcher = chokidarWatch(paths, options);
|
||||
watch(paths: string[], options: WatchOptions, events: Partial<WatchEvents>) {
|
||||
const matchesIgnoredPath = picomatch(options.ignored ?? [], {
|
||||
dot: true, // Match the behavior of fast-glob's micromatch by using these settings
|
||||
nocase: true,
|
||||
posix: true,
|
||||
strictSlashes: false,
|
||||
});
|
||||
|
||||
const watcher = chokidarWatch(paths, { ...options, ignored: (path) => matchesIgnoredPath(path) });
|
||||
|
||||
watcher.on('ready', () => events.onReady?.());
|
||||
watcher.on('add', (path) => events.onAdd?.(path));
|
||||
|
||||
@@ -942,6 +942,24 @@ describe(LibraryService.name, () => {
|
||||
expect(mocks.storage.watch).toHaveBeenCalledWith(library.importPaths, expect.anything(), expect.anything());
|
||||
});
|
||||
|
||||
it('should exclude paths from the watcher', async () => {
|
||||
const library = factory.library({
|
||||
importPaths: ['/foo', '/bar'],
|
||||
exclusionPatterns: ['**/excluded/**'],
|
||||
});
|
||||
|
||||
mocks.library.get.mockResolvedValue(library);
|
||||
mocks.library.getAll.mockResolvedValue([library]);
|
||||
|
||||
await sut.watchAll();
|
||||
|
||||
expect(mocks.storage.watch).toHaveBeenCalledWith(
|
||||
library.importPaths,
|
||||
expect.objectContaining({ ignored: library.exclusionPatterns }),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('should watch and unwatch library', async () => {
|
||||
const library = factory.library({ importPaths: ['/foo', '/bar'] });
|
||||
|
||||
|
||||
@@ -135,6 +135,7 @@ export class LibraryService extends BaseService {
|
||||
{
|
||||
usePolling: false,
|
||||
ignoreInitial: true,
|
||||
ignored: library.exclusionPatterns,
|
||||
awaitWriteFinish: {
|
||||
stabilityThreshold: 5000,
|
||||
pollInterval: 1000,
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
||||
import { StorageRepository, type WatchOptions } from 'src/repositories/storage.repository.js';
|
||||
import { automock } from 'test/utils.js';
|
||||
|
||||
describe(StorageRepository.name, () => {
|
||||
const annoyingExclusionPatterns = ['@', '#', '$', '%', '^', '&', '='];
|
||||
let sut: StorageRepository;
|
||||
let tempDir: string;
|
||||
let includedFile: string;
|
||||
let excludedFile: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
// eslint-disable-next-line no-sparse-arrays
|
||||
sut = new StorageRepository(automock(LoggingRepository, { args: [, { getEnv: () => ({}) }], strict: false }));
|
||||
tempDir = await mkdtemp(join(tmpdir(), 'immich-storage-watch-'));
|
||||
includedFile = join(tempDir, 'included/photo.jpg');
|
||||
excludedFile = join(tempDir, 'excluded/photo.jpg');
|
||||
|
||||
await Promise.all([
|
||||
mkdir(join(tempDir, 'included'), { recursive: true }),
|
||||
mkdir(join(tempDir, 'excluded'), { recursive: true }),
|
||||
]);
|
||||
await Promise.all([writeFile(includedFile, 'included'), writeFile(excludedFile, 'excluded')]);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const startWatching = (options: WatchOptions) => {
|
||||
const paths: string[] = [];
|
||||
let resolveReady!: () => void;
|
||||
|
||||
const ready = new Promise<void>((resolve) => (resolveReady = resolve));
|
||||
const close = sut.watch([tempDir], options, {
|
||||
onAdd: (path) => {
|
||||
paths.push(path);
|
||||
},
|
||||
onReady: resolveReady,
|
||||
});
|
||||
|
||||
return { close, paths, ready };
|
||||
};
|
||||
|
||||
it('should emit add events for initial files', async () => {
|
||||
const watcher = startWatching({ ignoreInitial: false });
|
||||
|
||||
try {
|
||||
await watcher.ready;
|
||||
|
||||
expect(watcher.paths.toSorted()).toEqual([includedFile, excludedFile].toSorted());
|
||||
} finally {
|
||||
await watcher.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('should not emit add events for ignored initial files', async () => {
|
||||
const watcher = startWatching({ ignoreInitial: false, ignored: ['**/excluded/**'] });
|
||||
|
||||
try {
|
||||
await watcher.ready;
|
||||
|
||||
expect(watcher.paths).toEqual([includedFile]);
|
||||
} finally {
|
||||
await watcher.close();
|
||||
}
|
||||
});
|
||||
|
||||
it.each(annoyingExclusionPatterns)('should ignore folders with %s in their name', async (char) => {
|
||||
const ignoredFolder = `${char}folder`;
|
||||
const ignoredFile = join(tempDir, ignoredFolder, 'photo.jpg');
|
||||
await mkdir(join(tempDir, ignoredFolder), { recursive: true });
|
||||
await writeFile(ignoredFile, 'ignored');
|
||||
|
||||
const watcher = startWatching({ ignoreInitial: false, ignored: [`**/${ignoredFolder}/**`] });
|
||||
|
||||
try {
|
||||
await watcher.ready;
|
||||
|
||||
expect(watcher.paths.toSorted()).toEqual([includedFile, excludedFile].toSorted());
|
||||
} finally {
|
||||
await watcher.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -3,11 +3,14 @@ import { existsSync } from 'node:fs';
|
||||
import { copyFile, mkdir, mkdtemp, rm, utimes, writeFile } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname, join, resolve } from 'node:path';
|
||||
import type { SystemConfig } from 'src/dtos/config.dto.js';
|
||||
import { StorageCore } from 'src/cores/storage.core.js';
|
||||
import { AssetStatus, JobName, JobStatus } from 'src/enum.js';
|
||||
import { AssetJobRepository } from 'src/repositories/asset-job.repository.js';
|
||||
import { AssetRepository } from 'src/repositories/asset.repository.js';
|
||||
import { CronRepository } from 'src/repositories/cron.repository.js';
|
||||
import { CryptoRepository } from 'src/repositories/crypto.repository.js';
|
||||
import { DatabaseRepository } from 'src/repositories/database.repository.js';
|
||||
import { EventRepository } from 'src/repositories/event.repository.js';
|
||||
import { JobRepository } from 'src/repositories/job.repository.js';
|
||||
import { LibraryRepository } from 'src/repositories/library.repository.js';
|
||||
@@ -15,6 +18,7 @@ import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
||||
import { StorageRepository } from 'src/repositories/storage.repository.js';
|
||||
import { DB } from 'src/schema/index.js';
|
||||
import { LibraryService } from 'src/services/library.service.js';
|
||||
import { systemConfigStub } from 'test/fixtures/system-config.stub.js';
|
||||
import { MediumTestContext, testAssetsDir } from 'test/medium.factory.js';
|
||||
import { newUuid } from 'test/small.factory.js';
|
||||
import { getKyselyDB } from 'test/utils.js';
|
||||
@@ -45,7 +49,7 @@ class LibraryTestContext extends MediumTestContext<typeof LibraryService> {
|
||||
super(LibraryService, {
|
||||
database,
|
||||
real: [AssetRepository, AssetJobRepository, CryptoRepository, LibraryRepository, StorageRepository],
|
||||
mock: [EventRepository, JobRepository, LoggingRepository],
|
||||
mock: [CronRepository, DatabaseRepository, EventRepository, JobRepository, LoggingRepository],
|
||||
});
|
||||
|
||||
const jobs = this.getMock(JobRepository);
|
||||
@@ -518,6 +522,32 @@ describe(LibraryService.name, () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('watch', () => {
|
||||
it('should pass exclusion patterns to the library watcher', async () => {
|
||||
const { sut, ctx } = setup();
|
||||
const library = await ctx.createLibrary({
|
||||
importPaths: [importRoot],
|
||||
exclusionPatterns: ['**/excluded/**'],
|
||||
});
|
||||
const storage = ctx.get(StorageRepository);
|
||||
const watch = vitest.spyOn(storage, 'watch');
|
||||
|
||||
ctx.getMock(DatabaseRepository).tryLock.mockResolvedValue(true);
|
||||
|
||||
try {
|
||||
await sut.onConfigInit({ newConfig: systemConfigStub.libraryWatchEnabled as SystemConfig });
|
||||
|
||||
expect(watch).toHaveBeenCalledWith(
|
||||
library.importPaths,
|
||||
expect.objectContaining({ ignored: library.exclusionPatterns }),
|
||||
expect.anything(),
|
||||
);
|
||||
} finally {
|
||||
await sut.onShutdown();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('handleQueueSyncAssets', () => {
|
||||
it('should set an asset offline if its file is not in any import path', async () => {
|
||||
const { sut, ctx } = setup();
|
||||
|
||||
Reference in New Issue
Block a user