tbelway/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2025-10-31 10:49:11 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(server): ensure new exclusion patterns work (#12102)

Browse Source 
* add test for bug

* find excluded paths when checking offline

* fix filename

* fix unit tests

* bump picomatch

* fix e2e paths

* improve e2e

* add unit tests

* cleanup e2e

* set correct asset count

* fix e2e test

* fix lint
This commit is contained in:
Jonathan Jogenfors 2024-08-29 01:51:25 +02:00 committed by GitHub
parent c6c7c54fa5
commit bab5ad7ebd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 85 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
e2e/src/api/specs/library.e2e-spec.ts
View File

@ -353,7 +353,7 @@ describe('/libraries', () => {
expect(assets.count).toBe(2);
utils.createImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
utils.createImageFile(`${testAssetDir}/temp/directoryA/assetC.png`);
await scan(admin.accessToken, library.id);
await utils.waitForWebsocketEvent({ event: 'assetUpload', total: 3 });
@ -361,11 +361,11 @@ describe('/libraries', () => {
const { assets: newAssets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
expect(newAssets.count).toBe(3);
utils.removeImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
utils.removeImageFile(`${testAssetDir}/temp/directoryA/assetC.png`);
});
it('should offline a file missing from disk', async () => {
utils.createImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
utils.createImageFile(`${testAssetDir}/temp/directoryA/assetC.png`);
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp`],
@ -374,26 +374,28 @@ describe('/libraries', () => {
await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
utils.removeImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
expect(assets.count).toBe(3);
utils.removeImageFile(`${testAssetDir}/temp/directoryA/assetC.png`);
await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
const { assets: newAssets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
expect(newAssets.count).toBe(3);
expect(assets.items).toEqual(
expect(newAssets.items).toEqual(
expect.arrayContaining([
expect.objectContaining({
isOffline: true,
originalFileName: 'assetB.png',
originalFileName: 'assetC.png',
}),
]),
);
});
it('should offline a file outside of import paths', async () => {
utils.createImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
utils.createImageFile(`${testAssetDir}/temp/directoryB/assetC.png`);
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp`],
@ -416,16 +418,49 @@ describe('/libraries', () => {
expect.arrayContaining([
expect.objectContaining({
isOffline: false,
originalFileName: 'assetB.png',
originalFileName: 'assetA.png',
}),
expect.objectContaining({
isOffline: true,
originalFileName: 'assetC.png',
originalFileName: 'assetB.png',
}),
]),
);
});
utils.removeImageFile(`${testAssetDir}/temp/directoryB/assetC.png`);
it('should offline a file covered by an exclusion pattern', async () => {
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp`],
});
await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
await request(app)
.put(`/libraries/${library.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ exclusionPatterns: ['**/directoryB/**'] });
await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
expect(assets.count).toBe(2);
expect(assets.items).toEqual(
expect.arrayContaining([
expect.objectContaining({
isOffline: false,
originalFileName: 'assetA.png',
}),
expect.objectContaining({
isOffline: true,
originalFileName: 'assetB.png',
}),
]),
);
});
it('should not try to delete offline files', async () => {
@ -471,6 +506,8 @@ describe('/libraries', () => {
await utils.waitForWebsocketEvent({ event: 'assetDelete', total: 1 });
expect(existsSync(`${testAssetDir}/temp/offline1/assetA.png`)).toBe(true);
utils.removeImageFile(`${testAssetDir}/temp/offline1/assetA.png`);
});
it('should scan new files', async () => {
@ -482,14 +519,14 @@ describe('/libraries', () => {
await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
utils.createImageFile(`${testAssetDir}/temp/directoryA/assetC.png`);
utils.createImageFile(`${testAssetDir}/temp/directoryC/assetC.png`);
await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
utils.removeImageFile(`${testAssetDir}/temp/directoryA/assetC.png`);
const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
expect(assets.count).toBe(3);
expect(assets.items).toEqual(
expect.arrayContaining([
expect.objectContaining({
@ -497,6 +534,8 @@ describe('/libraries', () => {
}),
]),
);
utils.removeImageFile(`${testAssetDir}/temp/directoryC/assetC.png`);
});
describe('with refreshModifiedFiles=true', () => {

3
server/package-lock.json generated
View File

@ -51,7 +51,7 @@
"nodemailer": "^6.9.13",
"openid-client": "^5.4.3",
"pg": "^8.11.3",
"picomatch": "^4.0.0",
"picomatch": "^4.0.2",
"react": "^18.3.1",
"react-email": "^3.0.0",
"reflect-metadata": "^0.2.0",
@ -11403,6 +11403,7 @@
"version": "4.0.2",
"resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.2.tgz",
"integrity": "sha512-M7BAV6Rlcy5u+m6oPhAPFgJTzAioX/6B0DxyvDlo9l8+T3nLKbrczg2WLUyzd45L8RqfUMyGPzekbMvX2Ldkwg==",
"license": "MIT",
"engines": {
"node": ">=12"
},

2
server/package.json
View File

@ -77,7 +77,7 @@
"nodemailer": "^6.9.13",
"openid-client": "^5.4.3",
"pg": "^8.11.3",
"picomatch": "^4.0.0",
"picomatch": "^4.0.2",
"react": "^18.3.1",
"react-email": "^3.0.0",
"reflect-metadata": "^0.2.0",

1
server/src/interfaces/job.interface.ts
View File

@ -133,6 +133,7 @@ export interface ILibraryFileJob extends IEntityJob {
export interface ILibraryOfflineJob extends IEntityJob {
importPaths: string[];
exclusionPatterns: string[];
}
export interface ILibraryRefreshJob extends IEntityJob {

21
server/src/services/library.service.spec.ts
View File

@ -301,6 +301,7 @@ describe(LibraryService.name, () => {
const mockAssetJob: ILibraryOfflineJob = {
id: assetStub.external.id,
importPaths: ['/'],
exclusionPatterns: [],
};
assetMock.getById.mockResolvedValue(null);
@ -314,6 +315,7 @@ describe(LibraryService.name, () => {
const mockAssetJob: ILibraryOfflineJob = {
id: assetStub.external.id,
importPaths: ['/'],
exclusionPatterns: [],
};
assetMock.getById.mockResolvedValue(assetStub.offline);
@ -323,10 +325,25 @@ describe(LibraryService.name, () => {
expect(assetMock.update).not.toHaveBeenCalled();
});
it('should offline assets no longer on disk or matching exclusion pattern', async () => {
it('should offline assets no longer on disk', async () => {
const mockAssetJob: ILibraryOfflineJob = {
id: assetStub.external.id,
importPaths: ['/'],
exclusionPatterns: [],
};
assetMock.getById.mockResolvedValue(assetStub.external);
await expect(sut.handleOfflineCheck(mockAssetJob)).resolves.toBe(JobStatus.SUCCESS);
expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.external.id, isOffline: true });
});
it('should offline assets matching an exclusion pattern', async () => {
const mockAssetJob: ILibraryOfflineJob = {
id: assetStub.external.id,
importPaths: ['/'],
exclusionPatterns: ['**/user1/**'],
};
assetMock.getById.mockResolvedValue(assetStub.external);
@ -340,6 +357,7 @@ describe(LibraryService.name, () => {
const mockAssetJob: ILibraryOfflineJob = {
id: assetStub.external.id,
importPaths: ['/data/user2'],
exclusionPatterns: [],
};
assetMock.getById.mockResolvedValue(assetStub.external);
@ -354,6 +372,7 @@ describe(LibraryService.name, () => {
const mockAssetJob: ILibraryOfflineJob = {
id: assetStub.external.id,
importPaths: ['/'],
exclusionPatterns: [],
};
assetMock.getById.mockResolvedValue(assetStub.external);

11
server/src/services/library.service.ts
View File

@ -556,11 +556,16 @@ export class LibraryService {
return JobStatus.SUCCESS;
}
const isExcluded = job.exclusionPatterns.some((pattern) => picomatch.isMatch(asset.originalPath, pattern));
if (isExcluded) {
this.logger.debug(`Asset is covered by an exclusion pattern, marking offline: ${asset.originalPath}`);
await this.assetRepository.update({ id: asset.id, isOffline: true });
return JobStatus.SUCCESS;
}
const fileExists = await this.storageRepository.checkFileExists(asset.originalPath, R_OK);
if (!fileExists) {
this.logger.debug(
`Asset is no longer found on disk or is covered by exclusion pattern, marking offline: ${asset.originalPath}`,
);
this.logger.debug(`Asset is no longer found on disk, marking offline: ${asset.originalPath}`);
await this.assetRepository.update({ id: asset.id, isOffline: true });
return JobStatus.SUCCESS;
}