From 5b00bc499ff71307cab7300086edfdace1721d51 Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Mon, 7 Oct 2024 21:43:21 +0200 Subject: [PATCH 1/2] fix(server): Allow commas and braces in import paths (#13259) fix commas and braces in paths --- e2e/src/api/specs/library.e2e-spec.ts | 56 +++++++++++++++++++ server/src/repositories/storage.repository.ts | 15 +++-- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/e2e/src/api/specs/library.e2e-spec.ts b/e2e/src/api/specs/library.e2e-spec.ts index 20bd230159..9f5adc4e27 100644 --- a/e2e/src/api/specs/library.e2e-spec.ts +++ b/e2e/src/api/specs/library.e2e-spec.ts @@ -347,6 +347,62 @@ describe('/libraries', () => { expect(assets.items.find((asset) => asset.originalPath.includes('directoryB'))).toBeDefined(); }); + it('should scan multiple import paths with commas', async () => { + // https://github.com/immich-app/immich/issues/10699 + const library = await utils.createLibrary(admin.accessToken, { + ownerId: admin.userId, + importPaths: [`${testAssetDirInternal}/temp/folder, a`, `${testAssetDirInternal}/temp/folder, b`], + }); + + utils.createImageFile(`${testAssetDir}/temp/folder, a/assetA.png`); + utils.createImageFile(`${testAssetDir}/temp/folder, b/assetB.png`); + + const { status } = await request(app) + .post(`/libraries/${library.id}/scan`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send(); + expect(status).toBe(204); + + await utils.waitForQueueFinish(admin.accessToken, 'library'); + + const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id }); + + expect(assets.count).toBe(2); + expect(assets.items.find((asset) => asset.originalPath.includes('folder, a'))).toBeDefined(); + expect(assets.items.find((asset) => asset.originalPath.includes('folder, b'))).toBeDefined(); + + utils.removeImageFile(`${testAssetDir}/temp/folder, a/assetA.png`); + utils.removeImageFile(`${testAssetDir}/temp/folder, b/assetB.png`); + }); + + it('should scan multiple import paths with braces', async () => { + // https://github.com/immich-app/immich/issues/10699 + const library = await utils.createLibrary(admin.accessToken, { + ownerId: admin.userId, + importPaths: [`${testAssetDirInternal}/temp/folder{ a`, `${testAssetDirInternal}/temp/folder} b`], + }); + + utils.createImageFile(`${testAssetDir}/temp/folder{ a/assetA.png`); + utils.createImageFile(`${testAssetDir}/temp/folder} b/assetB.png`); + + const { status } = await request(app) + .post(`/libraries/${library.id}/scan`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send(); + expect(status).toBe(204); + + await utils.waitForQueueFinish(admin.accessToken, 'library'); + + const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id }); + + expect(assets.count).toBe(2); + expect(assets.items.find((asset) => asset.originalPath.includes('folder{ a'))).toBeDefined(); + expect(assets.items.find((asset) => asset.originalPath.includes('folder} b'))).toBeDefined(); + + utils.removeImageFile(`${testAssetDir}/temp/folder{ a/assetA.png`); + utils.removeImageFile(`${testAssetDir}/temp/folder} b/assetB.png`); + }); + it('should reimport a modified file', async () => { const library = await utils.createLibrary(admin.accessToken, { ownerId: admin.userId, diff --git a/server/src/repositories/storage.repository.ts b/server/src/repositories/storage.repository.ts index 6fd9bb8b04..b957449984 100644 --- a/server/src/repositories/storage.repository.ts +++ b/server/src/repositories/storage.repository.ts @@ -156,7 +156,9 @@ export class StorageRepository implements IStorageRepository { return Promise.resolve([]); } - return glob(this.asGlob(pathsToCrawl), { + const globbedPaths = pathsToCrawl.map((path) => this.asGlob(path)); + + return glob(globbedPaths, { absolute: true, caseSensitiveMatch: false, onlyFiles: true, @@ -172,7 +174,9 @@ export class StorageRepository implements IStorageRepository { return emptyGenerator(); } - const stream = globStream(this.asGlob(pathsToCrawl), { + const globbedPaths = pathsToCrawl.map((path) => this.asGlob(path)); + + const stream = globStream(globbedPaths, { absolute: true, caseSensitiveMatch: false, onlyFiles: true, @@ -206,10 +210,9 @@ export class StorageRepository implements IStorageRepository { return () => watcher.close(); } - private asGlob(pathsToCrawl: string[]): string { - const escapedPaths = pathsToCrawl.map((path) => escapePath(path)); - const base = escapedPaths.length === 1 ? escapedPaths[0] : `{${escapedPaths.join(',')}}`; + private asGlob(pathToCrawl: string): string { + const escapedPath = escapePath(pathToCrawl); const extensions = `*{${mimeTypes.getSupportedFileExtensions().join(',')}}`; - return `${base}/**/${extensions}`; + return `${escapedPath}/**/${extensions}`; } } From 063969ca052ac6615e1e003b9c6a13342efc097c Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Mon, 7 Oct 2024 15:44:04 -0400 Subject: [PATCH 2/2] fix(server): searching with both `personIds` and `withPeople` (#13254) * use cte * linting --- server/src/repositories/search.repository.ts | 21 ++------------------ server/src/utils/database.ts | 19 +++++++++--------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/server/src/repositories/search.repository.ts b/server/src/repositories/search.repository.ts index 1e4b32357c..882a2634bd 100644 --- a/server/src/repositories/search.repository.ts +++ b/server/src/repositories/search.repository.ts @@ -26,7 +26,7 @@ import { asVector, searchAssetBuilder } from 'src/utils/database'; import { Instrumentation } from 'src/utils/instrumentation'; import { Paginated, PaginationResult, paginatedBuilder } from 'src/utils/pagination'; import { isValidInteger } from 'src/validation'; -import { Repository, SelectQueryBuilder } from 'typeorm'; +import { Repository } from 'typeorm'; @Instrumentation() @Injectable() @@ -113,14 +113,6 @@ export class SearchRepository implements ISearchRepository { return assets1; } - private createPersonFilter(builder: SelectQueryBuilder, personIds: string[]) { - return builder - .select(`${builder.alias}."assetId"`) - .where(`${builder.alias}."personId" IN (:...personIds)`, { personIds }) - .groupBy(`${builder.alias}."assetId"`) - .having(`COUNT(DISTINCT ${builder.alias}."personId") = :personCount`, { personCount: personIds.length }); - } - @GenerateSql({ params: [ { page: 1, size: 100 }, @@ -136,21 +128,12 @@ export class SearchRepository implements ISearchRepository { }) async searchSmart( pagination: SearchPaginationOptions, - { embedding, userIds, personIds, ...options }: SmartSearchOptions, + { embedding, userIds, ...options }: SmartSearchOptions, ): Paginated { let results: PaginationResult = { items: [], hasNextPage: false }; await this.assetRepository.manager.transaction(async (manager) => { let builder = manager.createQueryBuilder(AssetEntity, 'asset'); - - if (personIds?.length) { - const assetFaceBuilder = manager.createQueryBuilder(AssetFaceEntity, 'asset_face'); - const cte = this.createPersonFilter(assetFaceBuilder, personIds); - builder - .addCommonTableExpression(cte, 'asset_face_ids') - .innerJoin('asset_face_ids', 'a', 'a."assetId" = asset.id'); - } - builder = searchAssetBuilder(builder, options); builder .innerJoin('asset.smartSearch', 'search') diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index 498dd3456b..45fb6c6dae 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -1,4 +1,5 @@ import _ from 'lodash'; +import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { AssetEntity } from 'src/entities/asset.entity'; import { AssetSearchBuilderOptions } from 'src/interfaces/search.interface'; import { Between, IsNull, LessThanOrEqual, MoreThanOrEqual, Not, SelectQueryBuilder } from 'typeorm'; @@ -91,7 +92,6 @@ export function searchAssetBuilder( withPeople, withSmartInfo, personIds, - withExif, withStacked, trashedAfter, trashedBefore, @@ -128,15 +128,14 @@ export function searchAssetBuilder( } if (personIds && personIds.length > 0) { - builder - .leftJoin(`${builder.alias}.faces`, 'faces') - .andWhere('faces.personId IN (:...personIds)', { personIds }) - .addGroupBy(`${builder.alias}.id`) - .having('COUNT(DISTINCT faces.personId) = :personCount', { personCount: personIds.length }); - - if (withExif) { - builder.addGroupBy('exifInfo.assetId'); - } + const cte = builder + .createQueryBuilder() + .select('faces."assetId"') + .from(AssetFaceEntity, 'faces') + .where('faces."personId" IN (:...personIds)', { personIds }) + .groupBy(`faces."assetId"`) + .having(`COUNT(DISTINCT faces."personId") = :personCount`, { personCount: personIds.length }); + builder.addCommonTableExpression(cte, 'face_ids').innerJoin('face_ids', 'a', 'a."assetId" = asset.id'); } if (withStacked) {