From 7adb35e59e5c8e00e5391abfb69bee7acb068bb2 Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Mon, 30 Sep 2024 00:29:35 -0400 Subject: [PATCH] fix(server): `/search/random` failing with certain options (#13040) * fix relation handling, remove pagination * update api, sql * update mock --- mobile/openapi/lib/api/search_api.dart | Bin 14825 -> 14927 bytes .../openapi/lib/model/random_search_dto.dart | Bin 22131 -> 21494 bytes open-api/immich-openapi-specs.json | 9 +- open-api/typescript-sdk/src/fetch-client.ts | 3 +- server/src/controllers/search.controller.ts | 2 +- server/src/dtos/search.dto.ts | 18 +- server/src/interfaces/search.interface.ts | 2 +- server/src/queries/search.repository.sql | 189 +++++++++++++++++- server/src/repositories/search.repository.ts | 41 +++- server/src/services/search.service.ts | 16 +- server/src/utils/database.ts | 2 +- .../repositories/search.repository.mock.ts | 1 + 12 files changed, 243 insertions(+), 40 deletions(-) diff --git a/mobile/openapi/lib/api/search_api.dart b/mobile/openapi/lib/api/search_api.dart index 3b981e0ccb5bb3ba65e54ed547adbdb1f847916a..985029f106d275bb686ce3bbc958dd4d11f9043c 100644 GIT binary patch delta 37 vcmV+=0NVfQbI)|JhA@-QDFu@=1QfHHFj)eVoELhN@CKHXm>3PSuQNIZJJt_S delta 25 hcmX?K^0IhCo55s%bE(Zs4T6~_Z?~Mb`Mq%h8vviA3u*uW diff --git a/mobile/openapi/lib/model/random_search_dto.dart b/mobile/openapi/lib/model/random_search_dto.dart index 419cb451e2a0251c057fb3ed2766f8a14def33b0..3fcab05bbb275632cbc55c2a6689d043946cbeae 100644 GIT binary patch delta 48 zcmV-00MGyPtO53?0kFXWv(N&U5wqzPhZM7lD#8V`b}()Pv+y+f36sr43A1896AiPX GOwkE3WfhtL delta 198 zcmeyiobmG-#tjFVm=n`eH*aNHEI7GXL~8SLk$6Uy0+3L!NRNn6eo01ZksgSzU~8)Y z7Qe1>id7UUkXM?MqhPOKpkM`(n|x7EWb*>Od=^fiw5%_TP$X1wF0jSJMA+IzSsHCVkRYP5+I;%K8 QFS("/search/random", oazapfts.json({ ...opts, method: "POST", diff --git a/server/src/controllers/search.controller.ts b/server/src/controllers/search.controller.ts index 5b6deb298..9fdb2746f 100644 --- a/server/src/controllers/search.controller.ts +++ b/server/src/controllers/search.controller.ts @@ -32,7 +32,7 @@ export class SearchController { @Post('random') @HttpCode(HttpStatus.OK) @Authenticated() - searchRandom(@Auth() auth: AuthDto, @Body() dto: RandomSearchDto): Promise { + searchRandom(@Auth() auth: AuthDto, @Body() dto: RandomSearchDto): Promise { return this.service.searchRandom(auth, dto); } diff --git a/server/src/dtos/search.dto.ts b/server/src/dtos/search.dto.ts index ddc6c192c..5c5dce1a1 100644 --- a/server/src/dtos/search.dto.ts +++ b/server/src/dtos/search.dto.ts @@ -99,12 +99,6 @@ class BaseSearchDto { @Optional({ nullable: true, emptyToNull: true }) lensModel?: string | null; - @IsInt() - @Min(1) - @Type(() => Number) - @Optional() - page?: number; - @IsInt() @Min(1) @Max(1000) @@ -170,12 +164,24 @@ export class MetadataSearchDto extends RandomSearchDto { @Optional() @ApiProperty({ enumName: 'AssetOrder', enum: AssetOrder }) order?: AssetOrder; + + @IsInt() + @Min(1) + @Type(() => Number) + @Optional() + page?: number; } export class SmartSearchDto extends BaseSearchDto { @IsString() @IsNotEmpty() query!: string; + + @IsInt() + @Min(1) + @Type(() => Number) + @Optional() + page?: number; } export class SearchPlacesDto { diff --git a/server/src/interfaces/search.interface.ts b/server/src/interfaces/search.interface.ts index 0ba524c00..63d74a35f 100644 --- a/server/src/interfaces/search.interface.ts +++ b/server/src/interfaces/search.interface.ts @@ -116,7 +116,6 @@ export interface SearchPeopleOptions { export interface SearchOrderOptions { orderDirection?: 'ASC' | 'DESC'; - random?: boolean; } export interface SearchPaginationOptions { @@ -177,6 +176,7 @@ export interface ISearchRepository { searchSmart(pagination: SearchPaginationOptions, options: SmartSearchOptions): Paginated; searchDuplicates(options: AssetDuplicateSearch): Promise; searchFaces(search: FaceEmbeddingSearch): Promise; + searchRandom(size: number, options: AssetSearchOptions): Promise; upsert(assetId: string, embedding: number[]): Promise; searchPlaces(placeName: string): Promise; getAssetsByCity(userIds: string[]): Promise; diff --git a/server/src/queries/search.repository.sql b/server/src/queries/search.repository.sql index 58b299901..cd9a84b01 100644 --- a/server/src/queries/search.repository.sql +++ b/server/src/queries/search.repository.sql @@ -77,10 +77,11 @@ FROM "asset"."fileCreatedAt" >= $1 AND "exifInfo"."lensModel" = $2 AND 1 = 1 + AND "asset"."ownerId" IN ($3) AND 1 = 1 AND ( - "asset"."isFavorite" = $3 - AND "asset"."isArchived" = $4 + "asset"."isFavorite" = $4 + AND "asset"."isArchived" = $5 ) ) AND ("asset"."deletedAt" IS NULL) @@ -91,6 +92,190 @@ ORDER BY LIMIT 101 +-- SearchRepository.searchRandom +SELECT DISTINCT + "distinctAlias"."asset_id" AS "ids_asset_id", + "distinctAlias"."asset_id" +FROM + ( + SELECT + "asset"."id" AS "asset_id", + "asset"."deviceAssetId" AS "asset_deviceAssetId", + "asset"."ownerId" AS "asset_ownerId", + "asset"."libraryId" AS "asset_libraryId", + "asset"."deviceId" AS "asset_deviceId", + "asset"."type" AS "asset_type", + "asset"."status" AS "asset_status", + "asset"."originalPath" AS "asset_originalPath", + "asset"."thumbhash" AS "asset_thumbhash", + "asset"."encodedVideoPath" AS "asset_encodedVideoPath", + "asset"."createdAt" AS "asset_createdAt", + "asset"."updatedAt" AS "asset_updatedAt", + "asset"."deletedAt" AS "asset_deletedAt", + "asset"."fileCreatedAt" AS "asset_fileCreatedAt", + "asset"."localDateTime" AS "asset_localDateTime", + "asset"."fileModifiedAt" AS "asset_fileModifiedAt", + "asset"."isFavorite" AS "asset_isFavorite", + "asset"."isArchived" AS "asset_isArchived", + "asset"."isExternal" AS "asset_isExternal", + "asset"."isOffline" AS "asset_isOffline", + "asset"."checksum" AS "asset_checksum", + "asset"."duration" AS "asset_duration", + "asset"."isVisible" AS "asset_isVisible", + "asset"."livePhotoVideoId" AS "asset_livePhotoVideoId", + "asset"."originalFileName" AS "asset_originalFileName", + "asset"."sidecarPath" AS "asset_sidecarPath", + "asset"."stackId" AS "asset_stackId", + "asset"."duplicateId" AS "asset_duplicateId", + "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", + "stack"."primaryAssetId" AS "stack_primaryAssetId", + "stackedAssets"."id" AS "stackedAssets_id", + "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", + "stackedAssets"."ownerId" AS "stackedAssets_ownerId", + "stackedAssets"."libraryId" AS "stackedAssets_libraryId", + "stackedAssets"."deviceId" AS "stackedAssets_deviceId", + "stackedAssets"."type" AS "stackedAssets_type", + "stackedAssets"."status" AS "stackedAssets_status", + "stackedAssets"."originalPath" AS "stackedAssets_originalPath", + "stackedAssets"."thumbhash" AS "stackedAssets_thumbhash", + "stackedAssets"."encodedVideoPath" AS "stackedAssets_encodedVideoPath", + "stackedAssets"."createdAt" AS "stackedAssets_createdAt", + "stackedAssets"."updatedAt" AS "stackedAssets_updatedAt", + "stackedAssets"."deletedAt" AS "stackedAssets_deletedAt", + "stackedAssets"."fileCreatedAt" AS "stackedAssets_fileCreatedAt", + "stackedAssets"."localDateTime" AS "stackedAssets_localDateTime", + "stackedAssets"."fileModifiedAt" AS "stackedAssets_fileModifiedAt", + "stackedAssets"."isFavorite" AS "stackedAssets_isFavorite", + "stackedAssets"."isArchived" AS "stackedAssets_isArchived", + "stackedAssets"."isExternal" AS "stackedAssets_isExternal", + "stackedAssets"."isOffline" AS "stackedAssets_isOffline", + "stackedAssets"."checksum" AS "stackedAssets_checksum", + "stackedAssets"."duration" AS "stackedAssets_duration", + "stackedAssets"."isVisible" AS "stackedAssets_isVisible", + "stackedAssets"."livePhotoVideoId" AS "stackedAssets_livePhotoVideoId", + "stackedAssets"."originalFileName" AS "stackedAssets_originalFileName", + "stackedAssets"."sidecarPath" AS "stackedAssets_sidecarPath", + "stackedAssets"."stackId" AS "stackedAssets_stackId", + "stackedAssets"."duplicateId" AS "stackedAssets_duplicateId" + FROM + "assets" "asset" + LEFT JOIN "exif" "exifInfo" ON "exifInfo"."assetId" = "asset"."id" + LEFT JOIN "asset_stack" "stack" ON "stack"."id" = "asset"."stackId" + LEFT JOIN "assets" "stackedAssets" ON "stackedAssets"."stackId" = "stack"."id" + AND ("stackedAssets"."deletedAt" IS NULL) + WHERE + ( + "asset"."fileCreatedAt" >= $1 + AND "exifInfo"."lensModel" = $2 + AND 1 = 1 + AND "asset"."ownerId" IN ($3) + AND 1 = 1 + AND ( + "asset"."isFavorite" = $4 + AND "asset"."isArchived" = $5 + ) + AND "asset"."id" > $6 + ) + AND ("asset"."deletedAt" IS NULL) + ) "distinctAlias" +ORDER BY + "distinctAlias"."asset_id" ASC, + "asset_id" ASC +LIMIT + 100 +SELECT DISTINCT + "distinctAlias"."asset_id" AS "ids_asset_id", + "distinctAlias"."asset_id" +FROM + ( + SELECT + "asset"."id" AS "asset_id", + "asset"."deviceAssetId" AS "asset_deviceAssetId", + "asset"."ownerId" AS "asset_ownerId", + "asset"."libraryId" AS "asset_libraryId", + "asset"."deviceId" AS "asset_deviceId", + "asset"."type" AS "asset_type", + "asset"."status" AS "asset_status", + "asset"."originalPath" AS "asset_originalPath", + "asset"."thumbhash" AS "asset_thumbhash", + "asset"."encodedVideoPath" AS "asset_encodedVideoPath", + "asset"."createdAt" AS "asset_createdAt", + "asset"."updatedAt" AS "asset_updatedAt", + "asset"."deletedAt" AS "asset_deletedAt", + "asset"."fileCreatedAt" AS "asset_fileCreatedAt", + "asset"."localDateTime" AS "asset_localDateTime", + "asset"."fileModifiedAt" AS "asset_fileModifiedAt", + "asset"."isFavorite" AS "asset_isFavorite", + "asset"."isArchived" AS "asset_isArchived", + "asset"."isExternal" AS "asset_isExternal", + "asset"."isOffline" AS "asset_isOffline", + "asset"."checksum" AS "asset_checksum", + "asset"."duration" AS "asset_duration", + "asset"."isVisible" AS "asset_isVisible", + "asset"."livePhotoVideoId" AS "asset_livePhotoVideoId", + "asset"."originalFileName" AS "asset_originalFileName", + "asset"."sidecarPath" AS "asset_sidecarPath", + "asset"."stackId" AS "asset_stackId", + "asset"."duplicateId" AS "asset_duplicateId", + "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", + "stack"."primaryAssetId" AS "stack_primaryAssetId", + "stackedAssets"."id" AS "stackedAssets_id", + "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", + "stackedAssets"."ownerId" AS "stackedAssets_ownerId", + "stackedAssets"."libraryId" AS "stackedAssets_libraryId", + "stackedAssets"."deviceId" AS "stackedAssets_deviceId", + "stackedAssets"."type" AS "stackedAssets_type", + "stackedAssets"."status" AS "stackedAssets_status", + "stackedAssets"."originalPath" AS "stackedAssets_originalPath", + "stackedAssets"."thumbhash" AS "stackedAssets_thumbhash", + "stackedAssets"."encodedVideoPath" AS "stackedAssets_encodedVideoPath", + "stackedAssets"."createdAt" AS "stackedAssets_createdAt", + "stackedAssets"."updatedAt" AS "stackedAssets_updatedAt", + "stackedAssets"."deletedAt" AS "stackedAssets_deletedAt", + "stackedAssets"."fileCreatedAt" AS "stackedAssets_fileCreatedAt", + "stackedAssets"."localDateTime" AS "stackedAssets_localDateTime", + "stackedAssets"."fileModifiedAt" AS "stackedAssets_fileModifiedAt", + "stackedAssets"."isFavorite" AS "stackedAssets_isFavorite", + "stackedAssets"."isArchived" AS "stackedAssets_isArchived", + "stackedAssets"."isExternal" AS "stackedAssets_isExternal", + "stackedAssets"."isOffline" AS "stackedAssets_isOffline", + "stackedAssets"."checksum" AS "stackedAssets_checksum", + "stackedAssets"."duration" AS "stackedAssets_duration", + "stackedAssets"."isVisible" AS "stackedAssets_isVisible", + "stackedAssets"."livePhotoVideoId" AS "stackedAssets_livePhotoVideoId", + "stackedAssets"."originalFileName" AS "stackedAssets_originalFileName", + "stackedAssets"."sidecarPath" AS "stackedAssets_sidecarPath", + "stackedAssets"."stackId" AS "stackedAssets_stackId", + "stackedAssets"."duplicateId" AS "stackedAssets_duplicateId" + FROM + "assets" "asset" + LEFT JOIN "exif" "exifInfo" ON "exifInfo"."assetId" = "asset"."id" + LEFT JOIN "asset_stack" "stack" ON "stack"."id" = "asset"."stackId" + LEFT JOIN "assets" "stackedAssets" ON "stackedAssets"."stackId" = "stack"."id" + AND ("stackedAssets"."deletedAt" IS NULL) + WHERE + ( + "asset"."fileCreatedAt" >= $1 + AND "exifInfo"."lensModel" = $2 + AND 1 = 1 + AND "asset"."ownerId" IN ($3) + AND 1 = 1 + AND ( + "asset"."isFavorite" = $4 + AND "asset"."isArchived" = $5 + ) + AND "asset"."id" < $6 + ) + AND ("asset"."deletedAt" IS NULL) + ) "distinctAlias" +ORDER BY + "distinctAlias"."asset_id" ASC, + "asset_id" ASC +LIMIT + 100 + -- SearchRepository.searchSmart START TRANSACTION SET diff --git a/server/src/repositories/search.repository.ts b/server/src/repositories/search.repository.ts index 60694b6bf..cb80c8d2f 100644 --- a/server/src/repositories/search.repository.ts +++ b/server/src/repositories/search.repository.ts @@ -1,5 +1,6 @@ import { Inject, Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; +import { randomUUID } from 'node:crypto'; import { getVectorExtension } from 'src/database.config'; import { DummyValue, GenerateSql } from 'src/decorators'; import { AssetFaceEntity } from 'src/entities/asset-face.entity'; @@ -63,22 +64,15 @@ export class SearchRepository implements ISearchRepository { { takenAfter: DummyValue.DATE, lensModel: DummyValue.STRING, - ownerId: DummyValue.UUID, withStacked: true, isFavorite: true, - ownerIds: [DummyValue.UUID], + userIds: [DummyValue.UUID], }, ], }) async searchMetadata(pagination: SearchPaginationOptions, options: AssetSearchOptions): Paginated { let builder = this.assetRepository.createQueryBuilder('asset'); - builder = searchAssetBuilder(builder, options); - builder.orderBy('asset.fileCreatedAt', options.orderDirection ?? 'DESC'); - - if (options.random) { - // TODO replace with complicated SQL magic after kysely migration - builder.addSelect('RANDOM() as r').orderBy('r'); - } + builder = searchAssetBuilder(builder, options).orderBy('asset.fileCreatedAt', options.orderDirection ?? 'DESC'); return paginatedBuilder(builder, { mode: PaginationMode.SKIP_TAKE, @@ -87,6 +81,35 @@ export class SearchRepository implements ISearchRepository { }); } + @GenerateSql({ + params: [ + 100, + { + takenAfter: DummyValue.DATE, + lensModel: DummyValue.STRING, + withStacked: true, + isFavorite: true, + userIds: [DummyValue.UUID], + }, + ], + }) + async searchRandom(size: number, options: AssetSearchOptions): Promise { + const builder1 = searchAssetBuilder(this.assetRepository.createQueryBuilder('asset'), options); + const builder2 = builder1.clone(); + + const uuid = randomUUID(); + builder1.andWhere('asset.id > :uuid', { uuid }).orderBy('asset.id').take(size); + builder2.andWhere('asset.id < :uuid', { uuid }).orderBy('asset.id').take(size); + + const [assets1, assets2] = await Promise.all([builder1.getMany(), builder2.getMany()]); + const missingCount = size - assets1.length; + for (let i = 0; i < missingCount && i < assets2.length; i++) { + assets1.push(assets2[i]); + } + + return assets1; + } + private createPersonFilter(builder: SelectQueryBuilder, personIds: string[]) { return builder .select(`${builder.alias}."assetId"`) diff --git a/server/src/services/search.service.ts b/server/src/services/search.service.ts index dc6e71f34..c3cc5399c 100644 --- a/server/src/services/search.service.ts +++ b/server/src/services/search.service.ts @@ -94,20 +94,10 @@ export class SearchService { return this.mapResponse(items, hasNextPage ? (page + 1).toString() : null, { auth }); } - async searchRandom(auth: AuthDto, dto: RandomSearchDto): Promise { + async searchRandom(auth: AuthDto, dto: RandomSearchDto): Promise { const userIds = await this.getUserIdsToSearch(auth); - const page = dto.page ?? 1; - const size = dto.size || 250; - const { hasNextPage, items } = await this.searchRepository.searchMetadata( - { page, size }, - { - ...dto, - userIds, - random: true, - }, - ); - - return this.mapResponse(items, hasNextPage ? (page + 1).toString() : null, { auth }); + const items = await this.searchRepository.searchRandom(dto.size || 250, { ...dto, userIds }); + return items.map((item) => mapAsset(item, { auth })); } async searchSmart(auth: AuthDto, dto: SmartSearchDto): Promise { diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index 5f4577f4d..498dd3456 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -120,7 +120,7 @@ export function searchAssetBuilder( } if (withPeople) { - builder.leftJoinAndSelect(`${builder.alias}.person`, 'person'); + builder.leftJoinAndSelect('faces.person', 'person'); } if (withSmartInfo) { diff --git a/server/test/repositories/search.repository.mock.ts b/server/test/repositories/search.repository.mock.ts index 5426316b6..be0e753e3 100644 --- a/server/test/repositories/search.repository.mock.ts +++ b/server/test/repositories/search.repository.mock.ts @@ -7,6 +7,7 @@ export const newSearchRepositoryMock = (): Mocked => { searchSmart: vitest.fn(), searchDuplicates: vitest.fn(), searchFaces: vitest.fn(), + searchRandom: vitest.fn(), upsert: vitest.fn(), searchPlaces: vitest.fn(), getAssetsByCity: vitest.fn(),