From 4c89b0f7dfbe1b82c70df036278be06e63951a6b Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:25:56 -0400 Subject: [PATCH] perf(server): more efficient person queries (#32523) (cherry picked from commit 38a2ce4daa678615561d2b87a8f2bb30f70fdc53) --- pnpm-workspace.yaml | 2 +- server/package.json | 2 +- server/src/database.ts | 3 + server/src/queries/person.repository.sql | 765 ++++++++++-------- server/src/repositories/asset.repository.ts | 4 +- server/src/repositories/person.repository.ts | 452 +++++------ server/src/schema/functions.ts | 29 + .../1792213725597-AddAssetFaceAssetState.ts | 77 ++ server/src/schema/migrations/ORDER | 1 + server/src/schema/tables/asset-face.table.ts | 34 +- server/src/schema/tables/asset.table.ts | 8 +- server/src/services/person.service.spec.ts | 37 +- server/src/services/person.service.ts | 20 +- server/src/utils/database.ts | 8 +- server/src/utils/editor.spec.ts | 5 +- server/test/factories/asset-face.factory.ts | 5 +- .../repositories/person.repository.spec.ts | 203 ++++- .../specs/services/person.service.spec.ts | 16 +- .../medium/specs/sync/sync-asset-face.spec.ts | 17 +- 19 files changed, 1033 insertions(+), 655 deletions(-) create mode 100644 server/src/schema/migrations/1792213725597-AddAssetFaceAssetState.ts diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 9a22c9e7d..2b92d0f9a 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -73,5 +73,5 @@ shamefullyHoist: false verifyDepsBeforeRun: install minimumReleaseAgeExclude: - '@immich/ui' - - '@immich/sql-tools@0.6.3' + - '@immich/sql-tools@0.6.3 || 0.6.6' - '@nestjs/cli@12.0.1' diff --git a/server/package.json b/server/package.json index 90f0edb81..cf907122f 100644 --- a/server/package.json +++ b/server/package.json @@ -41,7 +41,7 @@ "dependencies": { "@extism/extism": "2.0.0-rc13", "@immich/plugin-sdk": "workspace:*", - "@immich/sql-tools": "^0.6.3", + "@immich/sql-tools": "^0.6.6", "@nestjs/bullmq": "^12.0.0", "@nestjs/common": "^12.0.0", "@nestjs/core": "^12.0.0", diff --git a/server/src/database.ts b/server/src/database.ts index 31ac40e59..3f262c4f6 100644 --- a/server/src/database.ts +++ b/server/src/database.ts @@ -277,6 +277,9 @@ export type AssetFace = { updatedAt: Date; updateId: string; isVisible: boolean; + ownerId: string; + assetVisibility: AssetVisibility; + assetDeletedAt: Date | null; }; export type Plugin = Selectable; diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index 9218f6a79..3ee338a2b 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -148,168 +148,418 @@ limit -- PersonRepository.getAllForUser with - "similarity_threshold" as ( + "faces" as ( select - set_config('pg_trgm.word_similarity_threshold', '0.5', true) as "thresh" - ) -select - ( - select - to_json(obj) + "visible"."personGroupId", + count(*) as "faceCount" from ( select - "person".* + "asset_face"."personGroupId" from - "person" + "asset_face" where - "person"."personGroupId" = "person_group"."id" - and "person"."ownerId" = $1 - ) as obj - ) as "ownedPerson", - ( - select - coalesce(json_agg(agg), '[]') - from - ( - select - "person_user"."sharedById", - "person_user"."role", - "other"."name", - "other"."birthDate" - from - "person" as "other" - inner join "person_user" on "person_user"."personGroupId" = "other"."personGroupId" - and "person_user"."sharedById" = "other"."ownerId" - and "person_user"."sharedWithId" = $2 - where - "other"."personGroupId" = "person_group"."id" - ) as agg - ) as "otherPeople", - ( - select - coalesce(json_agg(agg), '[]') - from - ( - select - "id", - "name", - "email", - "avatarColor", - "profileImagePath", - "profileChangedAt", - "person_user"."role" - from - "person_user" - inner join "user" on "user"."id" = "person_user"."sharedById" - and "user"."deletedAt" is null - where - "person_user"."personGroupId" = "person_group"."id" - and "person_user"."sharedWithId" = $3 - order by - "user"."name" - ) as agg - ) as "sharedBy", - ( - select - coalesce(json_agg(agg), '[]') - from - ( - select - "id", - "name", - "email", - "avatarColor", - "profileImagePath", - "profileChangedAt", - "person_user"."role" - from - "person_user" - inner join "user" on "user"."id" = "person_user"."sharedWithId" - and "user"."deletedAt" is null - where - "person_user"."personGroupId" = "person_group"."id" - and "person_user"."sharedById" = $4 - order by - "user"."name" - ) as agg - ) as "sharedWith" -from - "similarity_threshold", - "person_group" - inner join "person" as "owned" on "owned"."personGroupId" = "person_group"."id" - and "owned"."ownerId" = $5 - inner join ( - select - "asset_face"."personGroupId", - count("asset"."id") as "faceCount" - from - "asset_face" - inner join "asset" on "asset"."id" = "asset_face"."assetId" - and "asset"."visibility" = 'timeline' - and "asset"."deletedAt" is null - and ( - "asset"."ownerId" = any ($6::uuid[]) - or exists ( - select - 1 as "exists" - from - "album_asset" - inner join "album" on "album"."id" = "album_asset"."albumId" - and "album"."deletedAt" is null - inner join "album_user" on "album_user"."albumId" = "album"."id" - and "album_user"."userId" = $7::uuid - where - "album_asset"."assetId" = "asset"."id" - ) - ) - where - "asset_face"."personGroupId" = any ( - array( - ( + "asset_face"."deletedAt" is null + and "asset_face"."isVisible" is true + and "asset_face"."assetVisibility" = 'timeline' + and "asset_face"."assetDeletedAt" is null + and "asset_face"."personGroupId" in ( select "person"."personGroupId" from "person" where - "person"."ownerId" = $8 + "person"."ownerId" = $1 ) + and "asset_face"."ownerId" = any ($2::uuid[]) + union all + select + "asset_face"."personGroupId" + from + "asset_face" + where + "asset_face"."deletedAt" is null + and "asset_face"."isVisible" is true + and "asset_face"."assetVisibility" = 'timeline' + and "asset_face"."assetDeletedAt" is null + and "asset_face"."personGroupId" in ( + select + "person"."personGroupId" + from + "person" + where + "person"."ownerId" = $3 + ) + and not "asset_face"."ownerId" = any ($4::uuid[]) + and exists ( + select + 1 as "exists" + from + "album_asset" + inner join "album" on "album"."id" = "album_asset"."albumId" + and "album"."deletedAt" is null + inner join "album_user" on "album_user"."albumId" = "album"."id" + and "album_user"."userId" = $5 + where + "album_asset"."assetId" = "asset_face"."assetId" + ) + ) as "visible" + group by + "visible"."personGroupId" + ), + "people" as ( + select + "person".*, + "faces"."faceCount" + from + "person" + inner join "faces" on "faces"."personGroupId" = "person"."personGroupId" + where + "person"."ownerId" = $6 + and ( + "person"."name" != $7 + or "faces"."faceCount" >= coalesce( + ( + select + cast(value -> 'people' ->> 'minimumFaces' as integer) as "value" + from + "user_metadata" + where + "userId" = $8 + and "key" = 'preferences' + ), + 3 ) ) - and "asset_face"."deletedAt" is null - and "asset_face"."isVisible" is true - group by - "asset_face"."personGroupId" - ) as "faces" on "faces"."personGroupId" = "person_group"."id" -where - ( - "owned"."name" != $9 - or "faces"."faceCount" >= COALESCE( - ( - SELECT - value -> 'people' ->> 'minimumFaces' - FROM - user_metadata - WHERE - "userId" = $10 - AND key = 'preferences' - ), - '3' - )::int + ), + "page" as ( + select + "people".* + from + "people" + where + "people"."isHidden" = $9 + order by + "people"."isHidden" asc, + "people"."isFavorite" desc, + "people"."name" = '', + "people"."faceCount" desc, + "people"."name", + "people"."createdAt", + "people"."personGroupId" + limit + $10 + offset + $11 ) - and "owned"."isHidden" = $11 - and 1 = 1 -order by - "owned"."isHidden" asc, - "owned"."isFavorite" desc, - NULLIF("owned"."name", '') is null asc, - "faces"."faceCount" desc, - NULLIF("owned"."name", '') asc nulls last, - "owned"."createdAt" -limit - $12 -offset - $13 +select + "counts"."total", + "counts"."hidden", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "person".*, + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "person_user"."sharedById", + "person_user"."role", + "other"."name", + "other"."birthDate" + from + "person" as "other" + inner join "person_user" on "person_user"."personGroupId" = "other"."personGroupId" + and "person_user"."sharedById" = "other"."ownerId" + and "person_user"."sharedWithId" = $12 + where + "other"."personGroupId" = "person"."personGroupId" + ) as agg + ) as "otherPeople", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "id", + "name", + "email", + "avatarColor", + "profileImagePath", + "profileChangedAt", + "person_user"."role" + from + "person_user" + inner join "user" on "user"."id" = "person_user"."sharedById" + and "user"."deletedAt" is null + where + "person_user"."personGroupId" = "person"."personGroupId" + and "person_user"."sharedWithId" = $13 + order by + "user"."name" + ) as agg + ) as "sharedBy", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "id", + "name", + "email", + "avatarColor", + "profileImagePath", + "profileChangedAt", + "person_user"."role" + from + "person_user" + inner join "user" on "user"."id" = "person_user"."sharedWithId" + and "user"."deletedAt" is null + where + "person_user"."personGroupId" = "person"."personGroupId" + and "person_user"."sharedById" = $14 + order by + "user"."name" + ) as agg + ) as "sharedWith" + from + "page" as "person" + ) as agg + ) as "people" +from + ( + select + count(*) as "total", + count(*) filter ( + where + "people"."isHidden" = $15 + ) as "hidden" + from + "people" + ) as "counts" + +-- PersonRepository.getAllForUser (closest person) +with + "faces" as ( + select + "visible"."personGroupId", + count(*) as "faceCount" + from + ( + select + "asset_face"."personGroupId" + from + "asset_face" + where + "asset_face"."deletedAt" is null + and "asset_face"."isVisible" is true + and "asset_face"."assetVisibility" = 'timeline' + and "asset_face"."assetDeletedAt" is null + and "asset_face"."personGroupId" in ( + select + "person"."personGroupId" + from + "person" + where + "person"."ownerId" = $1 + ) + and "asset_face"."ownerId" = any ($2::uuid[]) + union all + select + "asset_face"."personGroupId" + from + "asset_face" + where + "asset_face"."deletedAt" is null + and "asset_face"."isVisible" is true + and "asset_face"."assetVisibility" = 'timeline' + and "asset_face"."assetDeletedAt" is null + and "asset_face"."personGroupId" in ( + select + "person"."personGroupId" + from + "person" + where + "person"."ownerId" = $3 + ) + and not "asset_face"."ownerId" = any ($4::uuid[]) + and exists ( + select + 1 as "exists" + from + "album_asset" + inner join "album" on "album"."id" = "album_asset"."albumId" + and "album"."deletedAt" is null + inner join "album_user" on "album_user"."albumId" = "album"."id" + and "album_user"."userId" = $5 + where + "album_asset"."assetId" = "asset_face"."assetId" + ) + ) as "visible" + group by + "visible"."personGroupId" + ), + "people" as ( + select + "person".*, + "faces"."faceCount" + from + "person" + inner join "faces" on "faces"."personGroupId" = "person"."personGroupId" + where + "person"."ownerId" = $6 + and ( + "person"."name" != $7 + or "faces"."faceCount" >= coalesce( + ( + select + cast(value -> 'people' ->> 'minimumFaces' as integer) as "value" + from + "user_metadata" + where + "userId" = $8 + and "key" = 'preferences' + ), + 3 + ) + ) + ), + "page" as ( + select + "people".* + from + "people" + where + "people"."isHidden" = $9 + order by + "people"."isHidden" asc, + "people"."isFavorite" desc, + ( + select + "face_search"."embedding" + from + "face_search" + where + "face_search"."faceId" = "people"."faceAssetId" + ) <=> ( + select + "face_search"."embedding" + from + "face_search" + inner join "person" as "closest" on "closest"."faceAssetId" = "face_search"."faceId" + where + "closest"."ownerId" = $10 + and "closest"."personGroupId" = $11 + ), + "people"."name" = '', + "people"."faceCount" desc, + "people"."name", + "people"."createdAt", + "people"."personGroupId" + limit + $12 + offset + $13 + ) +select + "counts"."total", + "counts"."hidden", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "person".*, + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "person_user"."sharedById", + "person_user"."role", + "other"."name", + "other"."birthDate" + from + "person" as "other" + inner join "person_user" on "person_user"."personGroupId" = "other"."personGroupId" + and "person_user"."sharedById" = "other"."ownerId" + and "person_user"."sharedWithId" = $14 + where + "other"."personGroupId" = "person"."personGroupId" + ) as agg + ) as "otherPeople", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "id", + "name", + "email", + "avatarColor", + "profileImagePath", + "profileChangedAt", + "person_user"."role" + from + "person_user" + inner join "user" on "user"."id" = "person_user"."sharedById" + and "user"."deletedAt" is null + where + "person_user"."personGroupId" = "person"."personGroupId" + and "person_user"."sharedWithId" = $15 + order by + "user"."name" + ) as agg + ) as "sharedBy", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "id", + "name", + "email", + "avatarColor", + "profileImagePath", + "profileChangedAt", + "person_user"."role" + from + "person_user" + inner join "user" on "user"."id" = "person_user"."sharedWithId" + and "user"."deletedAt" is null + where + "person_user"."personGroupId" = "person"."personGroupId" + and "person_user"."sharedById" = $16 + order by + "user"."name" + ) as agg + ) as "sharedWith" + from + "page" as "person" + ) as agg + ) as "people" +from + ( + select + count(*) as "total", + count(*) filter ( + where + "people"."isHidden" = $17 + ) as "hidden" + from + "people" + ) as "counts" -- PersonRepository.getAllWithoutFaces select @@ -592,103 +842,6 @@ set where "asset_face"."id" = $2 --- PersonRepository.getForUser -select - ( - select - to_json(obj) - from - ( - select - "person".* - from - "person" - where - "person"."personGroupId" = "person_group"."id" - and "person"."ownerId" = $1 - ) as obj - ) as "ownedPerson", - ( - select - coalesce(json_agg(agg), '[]') - from - ( - select - "person_user"."sharedById", - "person_user"."role", - "other"."name", - "other"."birthDate" - from - "person" as "other" - inner join "person_user" on "person_user"."personGroupId" = "other"."personGroupId" - and "person_user"."sharedById" = "other"."ownerId" - and "person_user"."sharedWithId" = $2 - where - "other"."personGroupId" = "person_group"."id" - ) as agg - ) as "otherPeople", - ( - select - coalesce(json_agg(agg), '[]') - from - ( - select - "id", - "name", - "email", - "avatarColor", - "profileImagePath", - "profileChangedAt", - "person_user"."role" - from - "person_user" - inner join "user" on "user"."id" = "person_user"."sharedById" - and "user"."deletedAt" is null - where - "person_user"."personGroupId" = "person_group"."id" - and "person_user"."sharedWithId" = $3 - order by - "user"."name" - ) as agg - ) as "sharedBy", - ( - select - coalesce(json_agg(agg), '[]') - from - ( - select - "id", - "name", - "email", - "avatarColor", - "profileImagePath", - "profileChangedAt", - "person_user"."role" - from - "person_user" - inner join "user" on "user"."id" = "person_user"."sharedWithId" - and "user"."deletedAt" is null - where - "person_user"."personGroupId" = "person_group"."id" - and "person_user"."sharedById" = $4 - order by - "user"."name" - ) as agg - ) as "sharedWith" -from - "person_group" -where - "person_group"."id" = $5 - and exists ( - select - "person"."ownerId" - from - "person" - where - "person"."personGroupId" = "person_group"."id" - and "person"."ownerId" = $6 - ) - -- PersonRepository.getByGroupId select "person".*, @@ -796,11 +949,6 @@ where "person_group"."id" = $5 -- PersonRepository.getByName -with - "similarity_threshold" as ( - select - set_config('pg_trgm.word_similarity_threshold', '0.5', true) as "thresh" - ) select "person".*, ( @@ -871,11 +1019,15 @@ select ) as agg ) as "sharedWith" from - "similarity_threshold", "person" where "person"."ownerId" = $4 - and f_unaccent ("person"."name") %> f_unaccent ($5) + and f_unaccent ("person"."name") %> ( + select + f_unaccent ($5) + where + set_config('pg_trgm.word_similarity_threshold', '0.5', true) is not null + ) order by f_unaccent ("person"."name") <->>> f_unaccent ($6) limit @@ -893,90 +1045,47 @@ where and "person"."name" != $2 ) --- PersonRepository.getNumberOfPeople +-- PersonRepository.getStatistics select - coalesce(count(*), 0) as "total", - coalesce( - count(*) filter ( - where - "people"."isHidden" = $1 - ), - 0 - ) as "hidden" + count(distinct "visible"."assetId") as "assets" from ( - with - "similarity_threshold" as ( - select - set_config('pg_trgm.word_similarity_threshold', '0.5', true) as "thresh" - ) select - "owned"."isHidden" + "asset_face"."assetId" from - "similarity_threshold", - "person_group" - inner join "person" as "owned" on "owned"."personGroupId" = "person_group"."id" - and "owned"."ownerId" = $2 - inner join ( - select - "asset_face"."personGroupId", - count("asset"."id") as "faceCount" - from - "asset_face" - inner join "asset" on "asset"."id" = "asset_face"."assetId" - and "asset"."visibility" = 'timeline' - and "asset"."deletedAt" is null - and ( - "asset"."ownerId" = any ($3::uuid[]) - or exists ( - select - 1 as "exists" - from - "album_asset" - inner join "album" on "album"."id" = "album_asset"."albumId" - and "album"."deletedAt" is null - inner join "album_user" on "album_user"."albumId" = "album"."id" - and "album_user"."userId" = $4::uuid - where - "album_asset"."assetId" = "asset"."id" - ) - ) - where - "asset_face"."personGroupId" = any ( - array( - ( - select - "person"."personGroupId" - from - "person" - where - "person"."ownerId" = $5 - ) - ) - ) - and "asset_face"."deletedAt" is null - and "asset_face"."isVisible" is true - group by - "asset_face"."personGroupId" - ) as "faces" on "faces"."personGroupId" = "person_group"."id" + "asset_face" where - ( - "owned"."name" != $6 - or "faces"."faceCount" >= COALESCE( - ( - SELECT - value -> 'people' ->> 'minimumFaces' - FROM - user_metadata - WHERE - "userId" = $7 - AND key = 'preferences' - ), - '3' - )::int + "asset_face"."deletedAt" is null + and "asset_face"."isVisible" is true + and "asset_face"."assetVisibility" = 'timeline' + and "asset_face"."assetDeletedAt" is null + and "asset_face"."personGroupId" = $1 + and "asset_face"."ownerId" = any ($2::uuid[]) + union all + select + "asset_face"."assetId" + from + "asset_face" + where + "asset_face"."deletedAt" is null + and "asset_face"."isVisible" is true + and "asset_face"."assetVisibility" = 'timeline' + and "asset_face"."assetDeletedAt" is null + and "asset_face"."personGroupId" = $3 + and not "asset_face"."ownerId" = any ($4::uuid[]) + and exists ( + select + 1 as "exists" + from + "album_asset" + inner join "album" on "album"."id" = "album_asset"."albumId" + and "album"."deletedAt" is null + inner join "album_user" on "album_user"."albumId" = "album"."id" + and "album_user"."userId" = $5 + where + "album_asset"."assetId" = "asset_face"."assetId" ) - and 1 = 1 - ) as "people" + ) as "visible" -- PersonRepository.createGroup insert into diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts index e70f37cd1..f6fd27b87 100644 --- a/server/src/repositories/asset.repository.ts +++ b/server/src/repositories/asset.repository.ts @@ -795,7 +795,7 @@ export class AssetRepository { qb.where((eb) => { // TODO this should become a shared `hasAccess` style helper once implement sharing in more places const isOwner = eb('asset.ownerId', '=', anyUuid(options.userIds!)); - return options.personId ? eb.or([isOwner, inSharedAlbum(eb, auth.user.id)]) : isOwner; + return options.personId ? eb.or([isOwner, inSharedAlbum(auth.user.id, 'asset.id')]) : isOwner; }), ) .$if(options.isFavorite !== undefined, (qb) => qb.where('asset.isFavorite', '=', options.isFavorite!)) @@ -884,7 +884,7 @@ export class AssetRepository { .$if(!!options.userIds, (qb) => qb.where((eb) => { const isOwner = eb('asset.ownerId', '=', anyUuid(options.userIds!)); - return options.personId ? eb.or([isOwner, inSharedAlbum(eb, auth.user.id)]) : isOwner; + return options.personId ? eb.or([isOwner, inSharedAlbum(auth.user.id, 'asset.id')]) : isOwner; }), ) .$if(options.isFavorite !== undefined, (qb) => qb.where('asset.isFavorite', '=', options.isFavorite!)) diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index d6378d6ec..58638127f 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -1,11 +1,8 @@ import { Injectable } from '@nestjs/common'; import { - type Expression, type ExpressionBuilder, type Insertable, type Kysely, - type Selectable, - type ShallowDehydrateObject, type SqlBool, type Updateable, expressionBuilder, @@ -16,7 +13,7 @@ import { InjectKysely } from 'nestjs-kysely'; import { AssetFace, PersonUser, columns } from 'src/database.js'; import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators.js'; import { PersonUserRole } from 'src/dtos/person.dto.js'; -import { AssetFileType, AssetVisibility, SharingDirection, SourceType, UserMetadataKey } from 'src/enum.js'; +import { AssetFileType, AssetVisibility, SourceType, UserMetadataKey } from 'src/enum.js'; import { type YearMonthDay } from 'src/repositories/asset.repository.js'; import { DB } from 'src/schema/index.js'; import { AssetFaceTable } from 'src/schema/tables/asset-face.table.js'; @@ -27,13 +24,6 @@ import { anyUuid, dummy, inSharedAlbum, removeUndefinedKeys, withFilePath } from import { isLeapDayObserved } from 'src/utils/date.js'; import { type PaginationOptions, paginationHelper } from 'src/utils/pagination.js'; -type PersonGroupRow = { - ownedPerson: ShallowDehydrateObject>; - otherPeople: { sharedById: string; role: PersonUserRole; name: string; birthDate: string | null }[]; - sharedBy: PersonUser[]; - sharedWith: PersonUser[]; -}; - export interface PersonFilterOptions { sharedById?: string; sharedWithId?: string; @@ -42,13 +32,12 @@ export interface PersonFilterOptions { name?: string; } -export interface PersonCountOptions extends PersonFilterOptions { +export interface PersonSearchOptions extends PersonFilterOptions { partnerIds: string[]; -} - -export interface PersonSearchOptions extends PersonCountOptions { withHidden: boolean; closestFaceAssetId?: string; + /** order by similarity to this person's face instead; takes precedence over closestFaceAssetId */ + closestPersonId?: string; } export interface PersonNameSearchOptions { @@ -114,22 +103,9 @@ export type WithPersonOptions = { viewingUserId: string; }; -const withOwnedPerson = (userId: string) => { - return (eb: ExpressionBuilder) => - jsonObjectFrom( - eb - .selectFrom('person') - .selectAll('person') - .whereRef('person.personGroupId', '=', 'person_group.id') - .where('person.ownerId', '=', userId), - ) - .$notNull() - .as('ownedPerson'); -}; - -const withOtherPeopleFor = (userId: string, personGroupId: Expression) => +const withSharing = (userId: string) => (eb: ExpressionBuilder) => [ jsonArrayFrom( - expressionBuilder() + eb .selectFrom('person as other') .innerJoin('person_user', (join) => join @@ -138,91 +114,57 @@ const withOtherPeopleFor = (userId: string, personGroupId: Expression) = .on('person_user.sharedWithId', '=', userId), ) .select(['person_user.sharedById', 'person_user.role', 'other.name', 'other.birthDate']) - .where('other.personGroupId', '=', personGroupId), - ).as('otherPeople'); - -const withPersonUsersFor = (userId: string, personGroupId: Expression, direction: SharingDirection) => { - const [userColumn, viewerColumn] = - direction === SharingDirection.SharedBy - ? (['person_user.sharedById', 'person_user.sharedWithId'] as const) - : (['person_user.sharedWithId', 'person_user.sharedById'] as const); - - return jsonArrayFrom( - expressionBuilder() + .whereRef('other.personGroupId', '=', 'person.personGroupId'), + ).as('otherPeople'), + jsonArrayFrom( + eb .selectFrom('person_user') - .innerJoin('user', (join) => join.onRef('user.id', '=', userColumn).on('user.deletedAt', 'is', null)) + .innerJoin('user', (join) => + join.onRef('user.id', '=', 'person_user.sharedById').on('user.deletedAt', 'is', null), + ) .select(columns.user) .select('person_user.role') - .where('person_user.personGroupId', '=', personGroupId) - .where(viewerColumn, '=', userId) + .whereRef('person_user.personGroupId', '=', 'person.personGroupId') + .where('person_user.sharedWithId', '=', userId) .orderBy('user.name'), ) .$castTo() - .as(direction === SharingDirection.SharedBy ? 'sharedBy' : 'sharedWith'); -}; - -const withSharing = (userId: string, personGroupId: Expression) => [ - withOtherPeopleFor(userId, personGroupId), - withPersonUsersFor(userId, personGroupId, SharingDirection.SharedBy), - withPersonUsersFor(userId, personGroupId, SharingDirection.SharedWith), + .as('sharedBy'), + jsonArrayFrom( + eb + .selectFrom('person_user') + .innerJoin('user', (join) => + join.onRef('user.id', '=', 'person_user.sharedWithId').on('user.deletedAt', 'is', null), + ) + .select(columns.user) + .select('person_user.role') + .whereRef('person_user.personGroupId', '=', 'person.personGroupId') + .where('person_user.sharedById', '=', userId) + .orderBy('user.name'), + ) + .$castTo() + .as('sharedWith'), ]; -const withOtherPeople = (userId: string) => { - return (eb: ExpressionBuilder) => withSharing(userId, eb.ref('person_group.id')); -}; +// computing the operand as an initplan sets the threshold first while the index can still be used +const unaccentWithThreshold = (name: string) => + sql`(select f_unaccent(${name}) where set_config('pg_trgm.word_similarity_threshold', '0.5', true) is not null)`; -const withOtherPeopleForPerson = (userId: string) => { - return (eb: ExpressionBuilder) => withSharing(userId, eb.ref('person.personGroupId')); -}; +const { fn, lit, selectFrom } = expressionBuilder(); +const minimumFaces = (userId: string) => + fn.coalesce( + selectFrom('user_metadata') + .select((eb) => eb.cast(sql`value -> 'people' ->> 'minimumFaces'`, 'integer').as('value')) + .where('userId', '=', userId) + .where('key', '=', sql.lit(UserMetadataKey.Preferences)), + lit(3), + ); -const withFilters = (userId: string, options: PersonFilterOptions = {}) => { - const { sharedById, sharedWithId, isFavorite, isHidden, name } = options; - return ( - eb: ExpressionBuilder< - DB & { owned: DB['person']; similarity_threshold: unknown }, - 'person_group' | 'owned' | 'similarity_threshold' - >, - ) => { - const filters: Expression[] = []; - - if (sharedById || sharedWithId) { - filters.push( - eb.exists( - eb - .selectFrom('person_user') - .whereRef('person_user.personGroupId', '=', 'person_group.id') - // only consider shares that involve the current user - .where((eb) => - eb.or([eb('person_user.sharedById', '=', userId), eb('person_user.sharedWithId', '=', userId)]), - ) - .$if(!!sharedById, (qb) => qb.where('person_user.sharedById', '=', sharedById!)) - .$if(!!sharedWithId, (qb) => qb.where('person_user.sharedWithId', '=', sharedWithId!)), - ), - ); - } - - if (isFavorite !== undefined) { - filters.push(eb('owned.isFavorite', '=', isFavorite)); - } - - if (isHidden !== undefined) { - filters.push(eb('owned.isHidden', '=', isHidden)); - } - - if (name !== undefined) { - filters.push(sql`f_unaccent("owned"."name") %> f_unaccent(${name})`); - } - - return eb.and(filters); - }; -}; - -const asPerson = ({ ownedPerson, otherPeople, sharedBy, sharedWith }: PersonGroupRow) => ({ - ...ownedPerson, - otherPeople, - sharedBy, - sharedWith, -}); +const visibleFaces = selectFrom('asset_face') + .where('asset_face.deletedAt', 'is', null) + .where('asset_face.isVisible', 'is', true) + .where('asset_face.assetVisibility', '=', sql.lit(AssetVisibility.Timeline)) + .where('asset_face.assetDeletedAt', 'is', null); const withPerson = ({ viewingUserId }: WithPersonOptions) => { return (eb: ExpressionBuilder) => @@ -230,7 +172,7 @@ const withPerson = ({ viewingUserId }: WithPersonOptions) => { eb .selectFrom('person') .selectAll('person') - .select(withOtherPeopleForPerson(viewingUserId)) + .select(withSharing(viewingUserId)) .whereRef('person.personGroupId', '=', 'asset_face.personGroupId') .where('person.ownerId', '=', viewingUserId), ).as('person'); @@ -437,107 +379,118 @@ export class PersonRepository { .execute(); } - private getVisiblePeopleQuery(userId: string, partnerIds: string[]) { - return this.db - .with('similarity_threshold', (db) => - db.selectNoFrom(sql`set_config('pg_trgm.word_similarity_threshold', '0.5', true)`.as('thresh')), - ) - .selectFrom(['similarity_threshold', 'person_group']) - .innerJoin('person as owned', (join) => - join.onRef('owned.personGroupId', '=', 'person_group.id').on('owned.ownerId', '=', userId), - ) - .innerJoin( - (eb) => - eb - .selectFrom('asset_face') - .innerJoin('asset', (join) => - join - .onRef('asset.id', '=', 'asset_face.assetId') - .on('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) - .on('asset.deletedAt', 'is', null) - .on((eb) => - eb.or([eb('asset.ownerId', '=', anyUuid([userId, ...partnerIds])), inSharedAlbum(eb, userId)]), - ), - ) - .select('asset_face.personGroupId') - .select((eb) => eb.fn.count('asset.id').as('faceCount')) - // an array instead of a join or `in`, which the planner may turn into a scan of every face in the instance - .where('asset_face.personGroupId', '=', (eb) => { - const ownedGroupIds = eb - .selectFrom('person') - .select('person.personGroupId') - .where('person.ownerId', '=', userId); - return eb.fn.any(eb.fn('array', [ownedGroupIds])); - }) - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', 'is', true) - .groupBy('asset_face.personGroupId') - .as('faces'), - (join) => join.onRef('faces.personGroupId', '=', 'person_group.id'), - ) - .where((eb) => - eb.or([ - eb('owned.name', '!=', ''), - eb( - 'faces.faceCount', - '>=', - sql`COALESCE( - (SELECT value -> 'people' ->> 'minimumFaces' - FROM user_metadata - WHERE "userId" = ${userId} - AND key = ${sql.lit(UserMetadataKey.Preferences)}), - '3' - )::int `, - ), - ]), - ); - } - - @GenerateSql({ - params: [{ take: 1, skip: 0 }, DummyValue.UUID, { withHidden: false, partnerIds: [DummyValue.UUID] }], - }) + @GenerateSql( + { params: [{ take: 1, skip: 0 }, DummyValue.UUID, { withHidden: false, partnerIds: [DummyValue.UUID] }] }, + { + name: 'closest person', + params: [ + { take: 1, skip: 0 }, + DummyValue.UUID, + { withHidden: false, partnerIds: [], closestPersonId: DummyValue.UUID }, + ], + }, + ) async getAllForUser(pagination: PaginationOptions, userId: string, options: PersonSearchOptions) { - const items = await this.getVisiblePeopleQuery(userId, options.partnerIds) - .select(withOwnedPerson(userId)) - .select(withOtherPeople(userId)) - .orderBy('owned.isHidden', 'asc') - .orderBy('owned.isFavorite', 'desc') - .$if(!!options.closestFaceAssetId, (qb) => - qb.orderBy((eb) => - eb( - (eb) => - eb - .selectFrom('face_search') - .select('face_search.embedding') - .whereRef('face_search.faceId', '=', 'owned.faceAssetId'), - '<=>', - (eb) => - eb - .selectFrom('face_search') - .select('face_search.embedding') - .where('face_search.faceId', '=', options.closestFaceAssetId!), - ), - ), - ) - .$if(!options.closestFaceAssetId, (qb) => - qb - .orderBy(sql`NULLIF("owned"."name", '') is null`, 'asc') - .orderBy('faces.faceCount', 'desc') - .orderBy(sql`NULLIF("owned"."name", '')`, (om) => om.asc().nullsLast()) - .orderBy('owned.createdAt'), - ) - // an explicit isHidden filter takes precedence over withHidden - .$if(!options.withHidden && options.isHidden === undefined, (qb) => qb.where('owned.isHidden', '=', false)) - .where(withFilters(userId, options)) - .$if(!!options.name, (qb) => qb.orderBy(sql`f_unaccent("owned"."name") <->>> f_unaccent(${options.name!})`)) - .offset(pagination.skip ?? 0) - .limit(pagination.take + 1) - .execute(); + const ownerIds = anyUuid([userId, ...options.partnerIds]); + const groupFaces = visibleFaces + .select('asset_face.personGroupId') + .where('asset_face.personGroupId', 'in', (eb) => + eb.selectFrom('person').select('person.personGroupId').where('person.ownerId', '=', userId), + ); - return paginationHelper( - items.map((item) => asPerson(item)), - pagination.take, - ); + const { people, total, hidden } = await this.db + .with('faces', (db) => + db + .selectFrom( + groupFaces + .where('asset_face.ownerId', '=', ownerIds) + .unionAll( + groupFaces + .where((eb) => eb.not(eb('asset_face.ownerId', '=', ownerIds))) + .where(inSharedAlbum(userId, 'asset_face.assetId')), + ) + .as('visible'), + ) + .select('visible.personGroupId') + .select((eb) => eb.fn.countAll().as('faceCount')) + .groupBy('visible.personGroupId'), + ) + .with('people', (db) => + db + .selectFrom('person') + .innerJoin('faces', 'faces.personGroupId', 'person.personGroupId') + .selectAll('person') + .select('faces.faceCount') + .where('person.ownerId', '=', userId) + .where((eb) => eb.or([eb('person.name', '!=', ''), eb('faces.faceCount', '>=', minimumFaces(userId))])) + .$if(!!options.sharedById || !!options.sharedWithId, (qb) => + qb.where((eb) => + eb.exists( + eb + .selectFrom('person_user') + .whereRef('person_user.personGroupId', '=', 'person.personGroupId') + .where((eb) => + eb.or([eb('person_user.sharedById', '=', userId), eb('person_user.sharedWithId', '=', userId)]), + ) + .$if(!!options.sharedById, (qb) => qb.where('person_user.sharedById', '=', options.sharedById!)) + .$if(!!options.sharedWithId, (qb) => + qb.where('person_user.sharedWithId', '=', options.sharedWithId!), + ), + ), + ), + ) + .$if(options.isFavorite !== undefined, (qb) => qb.where('person.isFavorite', '=', options.isFavorite!)) + .$if(options.isHidden !== undefined, (qb) => qb.where('person.isHidden', '=', options.isHidden!)) + .$if(options.name !== undefined, (qb) => + qb.where(sql`f_unaccent("person"."name") %> ${unaccentWithThreshold(options.name!)}`), + ), + ) + .with('page', (db) => + db + .selectFrom('people') + .selectAll('people') + .$if(!options.withHidden && options.isHidden === undefined, (qb) => qb.where('people.isHidden', '=', false)) + .orderBy('people.isHidden', 'asc') + .orderBy('people.isFavorite', 'desc') + .$if(!!options.closestPersonId || !!options.closestFaceAssetId, (qb) => + qb.orderBy((eb) => { + const embedding = eb.selectFrom('face_search').select('face_search.embedding'); + return eb( + embedding.whereRef('face_search.faceId', '=', 'people.faceAssetId'), + '<=>', + options.closestPersonId + ? embedding + .innerJoin('person as closest', 'closest.faceAssetId', 'face_search.faceId') + .where('closest.ownerId', '=', userId) + .where('closest.personGroupId', '=', options.closestPersonId) + : embedding.where('face_search.faceId', '=', options.closestFaceAssetId!), + ); + }), + ) + .orderBy((eb) => eb('people.name', '=', sql.lit(''))) + .orderBy('people.faceCount', 'desc') + .orderBy('people.name') + .orderBy('people.createdAt') + .orderBy('people.personGroupId') + .offset(pagination.skip ?? 0) + .limit(pagination.take + 1), + ) + .selectFrom((eb) => + eb + .selectFrom('people') + .select((eb) => [ + eb.fn.countAll().as('total'), + eb.fn.countAll().filterWhere('people.isHidden', '=', true).as('hidden'), + ]) + .as('counts'), + ) + .select(['counts.total', 'counts.hidden']) + .select((eb) => + jsonArrayFrom(eb.selectFrom('page as person').selectAll('person').select(withSharing(userId))).as('people'), + ) + .executeTakeFirstOrThrow(); + + return { ...paginationHelper(people, pagination.take), total, hidden }; } @GenerateSql() @@ -637,33 +590,12 @@ export class PersonRepository { return Number(result.numChangedRows ?? 0); } - @GenerateSql({ params: [{ userId: DummyValue.UUID, personGroupId: DummyValue.UUID }] }) - async getForUser({ userId, personGroupId }: { userId: string; personGroupId: string }) { - const group = await this.db - .selectFrom('person_group') - .select(withOwnedPerson(userId)) - .select(withOtherPeople(userId)) - .where('person_group.id', '=', personGroupId) - .where((eb) => - eb.exists( - eb - .selectFrom('person') - .select('person.ownerId') - .whereRef('person.personGroupId', '=', 'person_group.id') - .where('person.ownerId', '=', userId), - ), - ) - .executeTakeFirst(); - - return group && asPerson(group); - } - @GenerateSql({ params: [{ ownerId: DummyValue.UUID, personGroupId: DummyValue.UUID }] }) getByGroupId({ ownerId, personGroupId }: PersonId) { return this.db .selectFrom('person') .selectAll('person') - .select(withOtherPeopleForPerson(ownerId)) + .select(withSharing(ownerId)) .where('person.personGroupId', '=', personGroupId) .where('person.ownerId', '=', ownerId) .executeTakeFirst(); @@ -701,14 +633,11 @@ export class PersonRepository { @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING, { withHidden: true }] }) getByName(userId: string, personName: string, { withHidden }: PersonNameSearchOptions) { return this.db - .with('similarity_threshold', (db) => - db.selectNoFrom(sql`set_config('pg_trgm.word_similarity_threshold', '0.5', true)`.as('thresh')), - ) - .selectFrom(['similarity_threshold', 'person']) + .selectFrom('person') .selectAll('person') - .select(withOtherPeopleForPerson(userId)) + .select(withSharing(userId)) .where('person.ownerId', '=', userId) - .where(() => sql`f_unaccent("person"."name") %> f_unaccent(${personName})`) + .where(sql`f_unaccent("person"."name") %> ${unaccentWithThreshold(personName)}`) .orderBy(sql`f_unaccent("person"."name") <->>> f_unaccent(${personName})`) .limit(100) .$if(!withHidden, (qb) => qb.where('person.isHidden', '=', false)) @@ -726,44 +655,25 @@ export class PersonRepository { .execute(); } - @GenerateSql({ params: [DummyValue.UUID, DummyValue.UUID] }) - async getStatistics( + @GenerateSql({ params: [DummyValue.UUID, { ownerId: DummyValue.UUID, partnerIds: [DummyValue.UUID] }] }) + getStatistics( personGroupId: string, { ownerId, partnerIds }: { ownerId: string; partnerIds: string[] }, ): Promise { - const result = await this.db - .selectFrom('asset_face') - .leftJoin('asset', (join) => - join - .onRef('asset.id', '=', 'asset_face.assetId') - .on('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) - .on('asset.deletedAt', 'is', null) - .on((eb) => eb.or([eb('asset.ownerId', '=', anyUuid([ownerId, ...partnerIds])), inSharedAlbum(eb, ownerId)])), - ) - .select((eb) => eb.fn.count(eb.fn('distinct', ['asset.id'])).as('count')) - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', 'is', true) - .where('asset_face.personGroupId', '=', personGroupId) - .executeTakeFirst(); - - return { - assets: result ? Number(result.count) : 0, - }; - } - @GenerateSql({ params: [DummyValue.UUID, { partnerIds: [DummyValue.UUID] }] }) - getNumberOfPeople(userId: string, options: PersonCountOptions) { - const zero = sql.lit(0); + const ownerIds = anyUuid([ownerId, ...partnerIds]); + const groupFaces = visibleFaces.select('asset_face.assetId').where('asset_face.personGroupId', '=', personGroupId); return this.db .selectFrom( - this.getVisiblePeopleQuery(userId, options.partnerIds) - .select('owned.isHidden') - .where(withFilters(userId, options)) - .as('people'), - ) - .select((eb) => eb.fn.coalesce(eb.fn.countAll(), zero).as('total')) - .select((eb) => - eb.fn.coalesce(eb.fn.countAll().filterWhere('people.isHidden', '=', true), zero).as('hidden'), + groupFaces + .where('asset_face.ownerId', '=', ownerIds) + .unionAll( + groupFaces + .where((eb) => eb.not(eb('asset_face.ownerId', '=', ownerIds))) + .where(inSharedAlbum(ownerId, 'asset_face.assetId')), + ) + .as('visible'), ) + .select((eb) => eb.fn.count('visible.assetId').distinct().as('assets')) .executeTakeFirstOrThrow(); } @@ -772,7 +682,7 @@ export class PersonRepository { .insertInto('person') .values(person) .returningAll() - .returning(withOtherPeopleForPerson(person.ownerId)) + .returning(withSharing(person.ownerId)) .executeTakeFirstOrThrow(); } @@ -904,12 +814,16 @@ export class PersonRepository { ): Promise { let query = this.db; if (facesToAdd.length > 0) { - (query as any) = query.with('added', (db) => db.insertInto('asset_face').values(facesToAdd)); + (query as any) = query.with('added', (db) => db.insertInto('asset_face').values(facesToAdd).returning('id')); } if (faceIdsToRemove.length > 0) { (query as any) = query.with('removed', (db) => - db.deleteFrom('asset_face').where('asset_face.id', '=', (eb) => eb.fn.any(eb.val(faceIdsToRemove))), + db + .deleteFrom('asset_face') + .where('asset_face.id', '=', (eb) => eb.fn.any(eb.val(faceIdsToRemove))) + // ensure insert happens first to prevent deadlock on concurrent updates + .$if(facesToAdd.length > 0, (qb) => qb.where(sql`exists (select from added)`)), ); } @@ -927,7 +841,7 @@ export class PersonRepository { .where('person.ownerId', '=', person.ownerId) .where('person.personGroupId', '=', person.personGroupId) .returningAll() - .returning(withOtherPeopleForPerson(person.ownerId)) + .returning(withSharing(person.ownerId)) .executeTakeFirstOrThrow(); } diff --git a/server/src/schema/functions.ts b/server/src/schema/functions.ts index 83db76910..9266ad363 100644 --- a/server/src/schema/functions.ts +++ b/server/src/schema/functions.ts @@ -308,6 +308,35 @@ export const asset_face_audit = registerFunction({ END`, }); +export const asset_face_set_asset_state = registerFunction({ + name: 'asset_face_set_asset_state', + returnType: 'TRIGGER', + language: 'PLPGSQL', + body: ` + BEGIN + -- FOR SHARE so a concurrent update of the asset cannot leave this face with stale state + SELECT "ownerId", "visibility", "deletedAt" + INTO NEW."ownerId", NEW."assetVisibility", NEW."assetDeletedAt" + FROM asset + WHERE "id" = NEW."assetId" + FOR SHARE; + RETURN NEW; + END`, +}); + +export const asset_face_update_asset_state = registerFunction({ + name: 'asset_face_update_asset_state', + returnType: 'TRIGGER', + language: 'PLPGSQL', + body: ` + BEGIN + UPDATE asset_face + SET "ownerId" = NEW."ownerId", "assetVisibility" = NEW."visibility", "assetDeletedAt" = NEW."deletedAt" + WHERE "assetId" = NEW."id"; + RETURN NULL; + END`, +}); + export const asset_edit_insert = registerFunction({ name: 'asset_edit_insert', returnType: 'TRIGGER', diff --git a/server/src/schema/migrations/1792213725597-AddAssetFaceAssetState.ts b/server/src/schema/migrations/1792213725597-AddAssetFaceAssetState.ts new file mode 100644 index 000000000..ebaa84ac3 --- /dev/null +++ b/server/src/schema/migrations/1792213725597-AddAssetFaceAssetState.ts @@ -0,0 +1,77 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql`ALTER TABLE "asset_face" ADD "ownerId" uuid;`.execute(db); + await sql`ALTER TABLE "asset_face" ADD "assetVisibility" asset_visibility_enum;`.execute(db); + await sql`ALTER TABLE "asset_face" ADD "assetDeletedAt" timestamp with time zone;`.execute(db); + await sql`CREATE OR REPLACE FUNCTION asset_face_set_asset_state() + RETURNS TRIGGER + LANGUAGE PLPGSQL + AS $$ + BEGIN + -- FOR SHARE so a concurrent update of the asset cannot leave this face with stale state + SELECT "ownerId", "visibility", "deletedAt" + INTO NEW."ownerId", NEW."assetVisibility", NEW."assetDeletedAt" + FROM asset + WHERE "id" = NEW."assetId" + FOR SHARE; + RETURN NEW; + END + $$;`.execute(db); + await sql`CREATE OR REPLACE FUNCTION asset_face_update_asset_state() + RETURNS TRIGGER + LANGUAGE PLPGSQL + AS $$ + BEGIN + UPDATE asset_face + SET "ownerId" = NEW."ownerId", "assetVisibility" = NEW."visibility", "assetDeletedAt" = NEW."deletedAt" + WHERE "assetId" = NEW."id"; + RETURN NULL; + END + $$;`.execute(db); + await sql`CREATE OR REPLACE TRIGGER "asset_face_updatedAt" + BEFORE UPDATE ON "asset_face" + FOR EACH ROW + WHEN ((OLD."ownerId", OLD."assetVisibility", OLD."assetDeletedAt") IS NOT DISTINCT FROM (NEW."ownerId", NEW."assetVisibility", NEW."assetDeletedAt")) + EXECUTE FUNCTION updated_at();`.execute(db); + await sql`CREATE OR REPLACE TRIGGER "asset_face_update_asset_state" + AFTER UPDATE ON "asset" + FOR EACH ROW + WHEN ((OLD."ownerId", OLD."visibility", OLD."deletedAt") IS DISTINCT FROM (NEW."ownerId", NEW."visibility", NEW."deletedAt")) + EXECUTE FUNCTION asset_face_update_asset_state();`.execute(db); + await sql`CREATE OR REPLACE TRIGGER "asset_face_set_asset_state" + BEFORE INSERT ON "asset_face" + FOR EACH ROW + EXECUTE FUNCTION asset_face_set_asset_state();`.execute(db); + await sql`UPDATE "asset_face" SET "ownerId" = "asset"."ownerId", "assetVisibility" = "asset"."visibility", "assetDeletedAt" = "asset"."deletedAt" FROM "asset" WHERE "asset"."id" = "asset_face"."assetId";`.execute(db); + await sql`ALTER TABLE "asset_face" ALTER COLUMN "ownerId" SET NOT NULL, ALTER COLUMN "assetVisibility" SET NOT NULL;`.execute(db); + await sql`CREATE INDEX "asset_face_personGroupId_ownerId_assetId_timeline_idx" ON "asset_face" ("personGroupId", "ownerId", "assetId") WHERE ("deletedAt" IS NULL AND "isVisible" IS TRUE AND "assetVisibility" = 'timeline' AND "assetDeletedAt" IS NULL);`.execute(db); + await sql`ANALYZE "asset_face";`.execute(db); + await sql`UPDATE "migration_overrides" SET "value" = '{"type":"trigger","name":"asset_face_updatedAt","sql":"CREATE OR REPLACE TRIGGER \\"asset_face_updatedAt\\"\\n BEFORE UPDATE ON \\"asset_face\\"\\n FOR EACH ROW\\n WHEN ((OLD.\\"ownerId\\", OLD.\\"assetVisibility\\", OLD.\\"assetDeletedAt\\") IS NOT DISTINCT FROM (NEW.\\"ownerId\\", NEW.\\"assetVisibility\\", NEW.\\"assetDeletedAt\\"))\\n EXECUTE FUNCTION updated_at();"}'::jsonb WHERE "name" = 'trigger_asset_face_updatedAt';`.execute(db); + await sql`INSERT INTO "migration_overrides" ("name", "value") VALUES ('function_asset_face_set_asset_state', '{"type":"function","name":"asset_face_set_asset_state","sql":"CREATE OR REPLACE FUNCTION asset_face_set_asset_state()\\n RETURNS TRIGGER\\n LANGUAGE PLPGSQL\\n AS $$\\n BEGIN\\n -- FOR SHARE so a concurrent update of the asset cannot leave this face with stale state\\n SELECT \\"ownerId\\", \\"visibility\\", \\"deletedAt\\"\\n INTO NEW.\\"ownerId\\", NEW.\\"assetVisibility\\", NEW.\\"assetDeletedAt\\"\\n FROM asset\\n WHERE \\"id\\" = NEW.\\"assetId\\"\\n FOR SHARE;\\n RETURN NEW;\\n END\\n $$;"}'::jsonb);`.execute(db); + await sql`INSERT INTO "migration_overrides" ("name", "value") VALUES ('function_asset_face_update_asset_state', '{"type":"function","name":"asset_face_update_asset_state","sql":"CREATE OR REPLACE FUNCTION asset_face_update_asset_state()\\n RETURNS TRIGGER\\n LANGUAGE PLPGSQL\\n AS $$\\n BEGIN\\n UPDATE asset_face\\n SET \\"ownerId\\" = NEW.\\"ownerId\\", \\"assetVisibility\\" = NEW.\\"visibility\\", \\"assetDeletedAt\\" = NEW.\\"deletedAt\\"\\n WHERE \\"assetId\\" = NEW.\\"id\\";\\n RETURN NULL;\\n END\\n $$;"}'::jsonb);`.execute(db); + await sql`INSERT INTO "migration_overrides" ("name", "value") VALUES ('trigger_asset_face_update_asset_state', '{"type":"trigger","name":"asset_face_update_asset_state","sql":"CREATE OR REPLACE TRIGGER \\"asset_face_update_asset_state\\"\\n AFTER UPDATE ON \\"asset\\"\\n FOR EACH ROW\\n WHEN ((OLD.\\"ownerId\\", OLD.\\"visibility\\", OLD.\\"deletedAt\\") IS DISTINCT FROM (NEW.\\"ownerId\\", NEW.\\"visibility\\", NEW.\\"deletedAt\\"))\\n EXECUTE FUNCTION asset_face_update_asset_state();"}'::jsonb);`.execute(db); + await sql`INSERT INTO "migration_overrides" ("name", "value") VALUES ('trigger_asset_face_set_asset_state', '{"type":"trigger","name":"asset_face_set_asset_state","sql":"CREATE OR REPLACE TRIGGER \\"asset_face_set_asset_state\\"\\n BEFORE INSERT ON \\"asset_face\\"\\n FOR EACH ROW\\n EXECUTE FUNCTION asset_face_set_asset_state();"}'::jsonb);`.execute(db); + await sql`INSERT INTO "migration_overrides" ("name", "value") VALUES ('index_asset_face_personGroupId_ownerId_assetId_timeline_idx', '{"type":"index","name":"asset_face_personGroupId_ownerId_assetId_timeline_idx","sql":"CREATE INDEX \\"asset_face_personGroupId_ownerId_assetId_timeline_idx\\" ON \\"asset_face\\" (\\"personGroupId\\", \\"ownerId\\", \\"assetId\\") WHERE (\\"deletedAt\\" IS NULL AND \\"isVisible\\" IS TRUE AND \\"assetVisibility\\" = ''timeline'' AND \\"assetDeletedAt\\" IS NULL);"}'::jsonb);`.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`DROP TRIGGER "asset_face_set_asset_state" ON "asset_face";`.execute(db); + await sql`DROP FUNCTION asset_face_set_asset_state;`.execute(db); + await sql`DROP TRIGGER "asset_face_update_asset_state" ON "asset";`.execute(db); + await sql`DROP FUNCTION asset_face_update_asset_state;`.execute(db); + await sql`CREATE OR REPLACE TRIGGER "asset_face_updatedAt" + BEFORE UPDATE ON "asset_face" + FOR EACH ROW + EXECUTE FUNCTION updated_at();`.execute(db); + await sql`DROP INDEX "asset_face_personGroupId_ownerId_assetId_timeline_idx";`.execute(db); + await sql`ALTER TABLE "asset_face" DROP COLUMN "ownerId";`.execute(db); + await sql`ALTER TABLE "asset_face" DROP COLUMN "assetVisibility";`.execute(db); + await sql`ALTER TABLE "asset_face" DROP COLUMN "assetDeletedAt";`.execute(db); + await sql`UPDATE "migration_overrides" SET "value" = '{"sql":"CREATE OR REPLACE TRIGGER \\"asset_face_updatedAt\\"\\n BEFORE UPDATE ON \\"asset_face\\"\\n FOR EACH ROW\\n EXECUTE FUNCTION updated_at();","name":"asset_face_updatedAt","type":"trigger"}'::jsonb WHERE "name" = 'trigger_asset_face_updatedAt';`.execute(db); + await sql`DELETE FROM "migration_overrides" WHERE "name" = 'function_asset_face_set_asset_state';`.execute(db); + await sql`DELETE FROM "migration_overrides" WHERE "name" = 'function_asset_face_update_asset_state';`.execute(db); + await sql`DELETE FROM "migration_overrides" WHERE "name" = 'trigger_asset_face_update_asset_state';`.execute(db); + await sql`DELETE FROM "migration_overrides" WHERE "name" = 'trigger_asset_face_set_asset_state';`.execute(db); + await sql`DELETE FROM "migration_overrides" WHERE "name" = 'index_asset_face_personGroupId_ownerId_assetId_timeline_idx';`.execute(db); +} diff --git a/server/src/schema/migrations/ORDER b/server/src/schema/migrations/ORDER index 9f46fc499..2619cbe44 100644 --- a/server/src/schema/migrations/ORDER +++ b/server/src/schema/migrations/ORDER @@ -98,3 +98,4 @@ 1790587508209-RenameGeoNamesCountries 1790616293884-PersonSharing 1790693088454-AddPersonUserTableSharedBySharedWithConstraint +1792213725597-AddAssetFaceAssetState diff --git a/server/src/schema/tables/asset-face.table.ts b/server/src/schema/tables/asset-face.table.ts index 85437a129..a318cc728 100644 --- a/server/src/schema/tables/asset-face.table.ts +++ b/server/src/schema/tables/asset-face.table.ts @@ -1,5 +1,6 @@ import { AfterDeleteTrigger, + BeforeUpdateTrigger, Column, DeleteDateColumn, ForeignKeyColumn, @@ -8,17 +9,25 @@ import { PrimaryGeneratedColumn, Table, Timestamp, + TriggerFunction, UpdateDateColumn, } from '@immich/sql-tools'; -import { UpdateIdColumn, UpdatedAtTrigger } from 'src/decorators.js'; -import { SourceType } from 'src/enum.js'; -import { asset_face_source_type } from 'src/schema/enums.js'; -import { asset_face_audit } from 'src/schema/functions.js'; +import { UpdateIdColumn } from 'src/decorators.js'; +import { AssetVisibility, SourceType } from 'src/enum.js'; +import { asset_face_source_type, asset_visibility_enum } from 'src/schema/enums.js'; +import { asset_face_audit, asset_face_set_asset_state, updated_at } from 'src/schema/functions.js'; import { AssetTable } from 'src/schema/tables/asset.table.js'; import { PersonGroupTable } from 'src/schema/tables/person-group.table.js'; @Table({ name: 'asset_face' }) -@UpdatedAtTrigger('asset_face_updatedAt') +@BeforeUpdateTrigger({ + name: 'asset_face_updatedAt', + scope: 'row', + function: updated_at, + // these are internal optimizations, not to be synced to the client + when: '(OLD."ownerId", OLD."assetVisibility", OLD."assetDeletedAt") IS NOT DISTINCT FROM (NEW."ownerId", NEW."assetVisibility", NEW."assetDeletedAt")', +}) +@TriggerFunction({ timing: 'before', actions: ['insert'], scope: 'row', function: asset_face_set_asset_state }) @AfterDeleteTrigger({ scope: 'statement', function: asset_face_audit, @@ -33,6 +42,11 @@ import { PersonGroupTable } from 'src/schema/tables/person-group.table.js'; where: '"deletedAt" IS NULL AND "isVisible" IS TRUE', }) @Index({ columns: ['personGroupId', 'assetId'] }) +@Index({ + name: 'asset_face_personGroupId_ownerId_assetId_timeline_idx', + columns: ['personGroupId', 'ownerId', 'assetId'], + where: `"deletedAt" IS NULL AND "isVisible" IS TRUE AND "assetVisibility" = 'timeline' AND "assetDeletedAt" IS NULL`, +}) export class AssetFaceTable { @PrimaryGeneratedColumn() id!: Generated; @@ -86,4 +100,14 @@ export class AssetFaceTable { @Column({ type: 'boolean', default: true }) isVisible!: Generated; + + // copied from the asset by triggers, so face queries can filter on them without joining asset + @Column({ type: 'uuid' }) + ownerId!: Generated; + + @Column({ enum: asset_visibility_enum }) + assetVisibility!: Generated; + + @Column({ type: 'timestamp with time zone', nullable: true }) + assetDeletedAt!: Timestamp | null; } diff --git a/server/src/schema/tables/asset.table.ts b/server/src/schema/tables/asset.table.ts index 9053aec48..ff8e1fcba 100644 --- a/server/src/schema/tables/asset.table.ts +++ b/server/src/schema/tables/asset.table.ts @@ -1,5 +1,6 @@ import { AfterDeleteTrigger, + AfterUpdateTrigger, Column, CreateDateColumn, DeleteDateColumn, @@ -14,7 +15,7 @@ import { import { UpdateIdColumn, UpdatedAtTrigger } from 'src/decorators.js'; import { AssetStatus, AssetType, AssetVisibility, ChecksumAlgorithm } from 'src/enum.js'; import { asset_checksum_algorithm_enum, asset_visibility_enum, assets_status_enum } from 'src/schema/enums.js'; -import { asset_delete_audit } from 'src/schema/functions.js'; +import { asset_delete_audit, asset_face_update_asset_state } from 'src/schema/functions.js'; import { LibraryTable } from 'src/schema/tables/library.table.js'; import { StackTable } from 'src/schema/tables/stack.table.js'; import { UserTable } from 'src/schema/tables/user.table.js'; @@ -28,6 +29,11 @@ import { ASSET_CHECKSUM_CONSTRAINT } from 'src/utils/database.js'; referencingOldTableAs: 'old', when: 'pg_trigger_depth() = 0', }) +@AfterUpdateTrigger({ + scope: 'row', + function: asset_face_update_asset_state, + when: '(OLD."ownerId", OLD."visibility", OLD."deletedAt") IS DISTINCT FROM (NEW."ownerId", NEW."visibility", NEW."deletedAt")', +}) // Checksums must be unique per user and library @Index({ name: ASSET_CHECKSUM_CONSTRAINT, diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index 6428e8c39..29946004c 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -47,10 +47,11 @@ describe(PersonService.name, () => { const [person, hiddenPerson] = [PersonFactory.create(), PersonFactory.create({ isHidden: true })]; mocks.person.getAllForUser.mockResolvedValue({ - items: [getDehydrated(person), getDehydrated(hiddenPerson)], + items: [getDehydrated({ ...person, faceCount: 3 }), getDehydrated({ ...hiddenPerson, faceCount: 3 })], hasNextPage: false, + total: 2, + hidden: 1, }); - mocks.person.getNumberOfPeople.mockResolvedValue({ total: 2, hidden: 1 }); mocks.partner.getAll.mockResolvedValue([]); await expect(sut.getAll(auth, { withHidden: true, page: 1, size: 10 })).resolves.toEqual({ hasNextPage: false, @@ -75,10 +76,11 @@ describe(PersonService.name, () => { const [isFavorite, person] = [PersonFactory.create({ isFavorite: true }), PersonFactory.create()]; mocks.person.getAllForUser.mockResolvedValue({ - items: [getDehydrated(isFavorite), getDehydrated(person)], + items: [getDehydrated({ ...isFavorite, faceCount: 3 }), getDehydrated({ ...person, faceCount: 3 })], hasNextPage: false, + total: 2, + hidden: 1, }); - mocks.person.getNumberOfPeople.mockResolvedValue({ total: 2, hidden: 1 }); mocks.partner.getAll.mockResolvedValue([]); await expect(sut.getAll(auth, { withHidden: false, page: 1, size: 10 })).resolves.toEqual({ hasNextPage: false, @@ -97,13 +99,32 @@ describe(PersonService.name, () => { partnerIds: [], }); }); + + it('should pass the closest person to the query', async () => { + const auth = AuthFactory.create(); + const closestPersonId = newUuid(); + + mocks.person.getAllForUser.mockResolvedValue({ items: [], hasNextPage: false, total: 0, hidden: 0 }); + mocks.partner.getAll.mockResolvedValue([]); + await expect(sut.getAll(auth, { closestPersonId, page: 1, size: 10 })).resolves.toEqual({ + people: [], + hasNextPage: false, + total: 0, + hidden: 0, + }); + expect(mocks.person.getAllForUser).toHaveBeenCalledWith({ skip: 0, take: 10 }, auth.user.id, { + withHidden: false, + partnerIds: [], + closestPersonId, + }); + }); }); describe('getById', () => { it('should require person.read permission', async () => { const auth = AuthFactory.create(); const person = PersonFactory.create(); - mocks.person.getForUser.mockResolvedValue(getDehydrated(person)); + mocks.person.getByGroupId.mockResolvedValue(person); await expect(sut.getById(auth, person.personGroupId)).rejects.toBeInstanceOf(BadRequestException); expect(mocks.access.person.checkAccess).toHaveBeenCalledWith( auth.user.id, @@ -127,12 +148,12 @@ describe(PersonService.name, () => { const ids = [{ personGroupId: person.personGroupId, ownerId: auth.user.id }]; mocks.access.person.checkAccess.mockResolvedValue(new Set(ids)); - mocks.person.getForUser.mockResolvedValue(getDehydrated(person)); + mocks.person.getByGroupId.mockResolvedValue(person); await expect(sut.getById(auth, person.personGroupId)).resolves.toEqual( expect.objectContaining({ id: person.personGroupId }), ); - expect(mocks.person.getForUser).toHaveBeenCalledWith({ - userId: auth.user.id, + expect(mocks.person.getByGroupId).toHaveBeenCalledWith({ + ownerId: auth.user.id, personGroupId: person.personGroupId, }); expect(mocks.access.person.checkAccess).toHaveBeenCalledWith(auth.user.id, new Set(ids), PERSON_READ_ROLES); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index 2e20cd01c..3aaa34dea 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -62,33 +62,21 @@ const personKey = ({ ownerId, personGroupId }: PersonId) => `${ownerId}/${person export class PersonService extends BaseService { async getAll(auth: AuthDto, dto: PersonSearchDto): Promise { const { withHidden = false, closestAssetId, closestPersonId, page, size, ...filters } = dto; - let closestFaceAssetId = closestAssetId; const pagination = { take: size, skip: (page - 1) * size, }; - - if (closestPersonId) { - const person = await this.personRepository.getByGroupId({ - ownerId: auth.user.id, - personGroupId: closestPersonId, - }); - if (!person?.faceAssetId) { - throw new NotFoundException('Person not found'); - } - closestFaceAssetId = person.faceAssetId; - } const partnerIds = await getMyPartnerIds({ userId: auth.user.id, repository: this.partnerRepository, }); - const { items, hasNextPage } = await this.personRepository.getAllForUser(pagination, auth.user.id, { + const { items, hasNextPage, total, hidden } = await this.personRepository.getAllForUser(pagination, auth.user.id, { withHidden, partnerIds, - closestFaceAssetId, + closestFaceAssetId: closestAssetId, + closestPersonId, ...filters, }); - const { total, hidden } = await this.personRepository.getNumberOfPeople(auth.user.id, { partnerIds, ...filters }); return { people: items.map((person) => mapPerson(person)), @@ -199,7 +187,7 @@ export class PersonService extends BaseService { ids: [{ personGroupId, ownerId: auth.user.id }], }); return mapPerson( - await findOrFail(() => this.personRepository.getForUser({ userId: auth.user.id, personGroupId }), 'Person'), + await findOrFail(() => this.personRepository.getByGroupId({ ownerId: auth.user.id, personGroupId }), 'Person'), ); } diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index e760e7714..985d7bb40 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -13,6 +13,7 @@ import { Selectable, ShallowDehydrateObject, SqlBool, + expressionBuilder, sql, } from 'kysely'; import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres'; @@ -287,7 +288,8 @@ export function hasPeople(qb: SelectQueryBuilder, personGroup ); } -export function inSharedAlbum(eb: ExpressionBuilder, userId: string) { +export function inSharedAlbum(userId: string, assetId: 'asset.id' | 'asset_face.assetId') { + const eb = expressionBuilder(); return eb.exists( eb .selectFrom('album_asset') @@ -296,9 +298,9 @@ export function inSharedAlbum(eb: ExpressionBuilder, userId: string join.onRef('album.id', '=', 'album_asset.albumId').on('album.deletedAt', 'is', null), ) .innerJoin('album_user', (join) => - join.onRef('album_user.albumId', '=', 'album.id').on('album_user.userId', '=', asUuid(userId)), + join.onRef('album_user.albumId', '=', 'album.id').on('album_user.userId', '=', userId), ) - .whereRef('album_asset.assetId', '=', 'asset.id'), + .whereRef('album_asset.assetId', '=', assetId), ); } diff --git a/server/src/utils/editor.spec.ts b/server/src/utils/editor.spec.ts index 2f1081d8b..14560e554 100644 --- a/server/src/utils/editor.spec.ts +++ b/server/src/utils/editor.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import { AssetFace } from 'src/database.js'; import { AssetOcrResponseDto } from 'src/dtos/ocr.dto.js'; -import { SourceType } from 'src/enum.js'; +import { AssetVisibility, SourceType } from 'src/enum.js'; import { boundingBoxOverlap, checkFaceVisibility, checkOcrVisibility } from 'src/utils/editor.js'; describe('boundingBoxOverlap', () => { @@ -70,6 +70,9 @@ const createFace = (params: Partial = {}): AssetFace => ({ updatedAt: new Date(), updateId: 'update-id', isVisible: true, + ownerId: 'owner-id', + assetVisibility: AssetVisibility.Timeline, + assetDeletedAt: null, ...params, }); diff --git a/server/test/factories/asset-face.factory.ts b/server/test/factories/asset-face.factory.ts index 8d49bc5d2..eb1ec4ece 100644 --- a/server/test/factories/asset-face.factory.ts +++ b/server/test/factories/asset-face.factory.ts @@ -1,5 +1,5 @@ import { Selectable } from 'kysely'; -import { SourceType } from 'src/enum.js'; +import { AssetVisibility, SourceType } from 'src/enum.js'; import { AssetFaceTable } from 'src/schema/tables/asset-face.table.js'; import { build } from 'test/factories/builder.factory.js'; import { PersonFactory } from 'test/factories/person.factory.js'; @@ -31,6 +31,9 @@ export class AssetFaceFactory { sourceType: SourceType.MachineLearning, updatedAt: newDate(), updateId: newUuidV7(), + ownerId: newUuid(), + assetVisibility: AssetVisibility.Timeline, + assetDeletedAt: null, ...dto, }); } diff --git a/server/test/medium/specs/repositories/person.repository.spec.ts b/server/test/medium/specs/repositories/person.repository.spec.ts index b91e30ebf..c400fe2b0 100644 --- a/server/test/medium/specs/repositories/person.repository.spec.ts +++ b/server/test/medium/specs/repositories/person.repository.spec.ts @@ -1,6 +1,6 @@ import { Kysely } from 'kysely'; import { PersonUserRole } from 'src/dtos/person.dto.js'; -import { AssetFileType } from 'src/enum.js'; +import { AssetFileType, AssetVisibility, UserMetadataKey } from 'src/enum.js'; import { LoggingRepository } from 'src/repositories/logging.repository.js'; import { PersonUserRepository } from 'src/repositories/person-user.repository.js'; import { PersonRepository, type PersonSearchOptions } from 'src/repositories/person.repository.js'; @@ -29,6 +29,15 @@ const listFor = async (sut: PersonRepository, userId: string, options?: Partial< return items; }; +const countFor = async (sut: PersonRepository, userId: string, options?: Partial) => { + const { total, hidden } = await sut.getAllForUser({ take: 100, skip: 0 }, userId, { + withHidden: false, + partnerIds: [], + ...options, + }); + return { total, hidden }; +}; + // a named person with at least one visible face clears the minimum-faces rule const newNamedPerson = async (ctx: MediumTestContext, ownerId: string, name: string) => { const { result: person } = await ctx.newPerson({ ownerId, name }); @@ -46,11 +55,9 @@ const otherPeopleOf = async ( ) => { const listItems = await listFor(sut, userId, { partnerIds }); const listed = listItems.find((item) => item.personGroupId === groupId); - const forUser = await sut.getForUser({ userId, personGroupId: groupId }); const byGroupId = await sut.getByGroupId({ ownerId: userId, personGroupId: groupId }); return { getAllForUser: listed?.otherPeople, - getForUser: forUser?.otherPeople, getByGroupId: byGroupId?.otherPeople, }; }; @@ -184,7 +191,6 @@ describe(PersonRepository.name, () => { const results = await otherPeopleOf(sut, ctx, recipient.id, person.personGroupId, [owner.id]); expect(results.getAllForUser).toEqual(expected); - expect(results.getForUser).toEqual(expected); expect(results.getByGroupId).toEqual(expected); }); @@ -216,7 +222,6 @@ describe(PersonRepository.name, () => { const results = await otherPeopleOf(sut, ctx, first.id, person.personGroupId, [owner.id]); expect(results.getAllForUser).toEqual(expected); - expect(results.getForUser).toEqual(expected); expect(results.getByGroupId).toEqual(expected); }); @@ -232,7 +237,6 @@ describe(PersonRepository.name, () => { const results = await otherPeopleOf(sut, ctx, other.id, person.personGroupId, [owner.id]); - expect(results.getForUser).toEqual([]); expect(results.getByGroupId).toEqual([]); }); }); @@ -550,10 +554,170 @@ describe(PersonRepository.name, () => { await ctx.newAssetFace({ assetId: asset.id, personGroupId: onPartnerAssets.personGroupId }); } - await expect(sut.getNumberOfPeople(owner.id, { partnerIds: [] })).resolves.toEqual({ total: 1, hidden: 1 }); - await expect(sut.getNumberOfPeople(owner.id, { partnerIds: [partner.id] })).resolves.toEqual({ - total: 2, - hidden: 1, + await expect(countFor(sut, owner.id)).resolves.toEqual({ total: 1, hidden: 1 }); + await expect(countFor(sut, owner.id, { partnerIds: [partner.id] })).resolves.toEqual({ total: 2, hidden: 1 }); + }); + + it('should count everyone who matches the filters, hidden people included', async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const { user } = await ctx.newUser(); + await newNamedPerson(ctx, user.id, 'Alice'); + await newNamedPerson(ctx, user.id, 'Bob'); + const hidden = await newNamedPerson(ctx, user.id, 'Carol'); + await ctx.database + .updateTable('person') + .set({ isHidden: true }) + .where('personGroupId', '=', hidden.personGroupId) + .execute(); + + await expect(listFor(sut, user.id)).resolves.toHaveLength(2); + await expect(countFor(sut, user.id)).resolves.toEqual({ total: 3, hidden: 1 }); + await expect(countFor(sut, user.id, { name: 'alice' })).resolves.toEqual({ total: 1, hidden: 0 }); + // an empty page still has the counts + await expect(countFor(sut, user.id, { isFavorite: true })).resolves.toEqual({ total: 0, hidden: 0 }); + const { items, total } = await sut.getAllForUser({ take: 10, skip: 10 }, user.id, { + withHidden: false, + partnerIds: [], + }); + expect({ items, total }).toEqual({ items: [], total: 3 }); + }); + + it('should page through people who tie on every other sort key in a stable order', async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const { user } = await ctx.newUser(); + const ids: string[] = []; + for (let i = 0; i < 6; i++) { + const person = await newNamedPerson(ctx, user.id, 'Twin'); + ids.push(person.personGroupId); + } + await ctx.database + .updateTable('person') + .set({ createdAt: new Date('2024-01-01') }) + .where('ownerId', '=', user.id) + .execute(); + + const pages = []; + for (let skip = 0; skip < 6; skip += 2) { + const { items } = await sut.getAllForUser({ take: 2, skip }, user.id, { withHidden: false, partnerIds: [] }); + pages.push(...items.map(({ personGroupId }) => personGroupId)); + } + expect(pages).toEqual(ids.toSorted()); + }); + + it('should order people by similarity to the closest person', async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const { user } = await ctx.newUser(); + // alphabetical order is the reverse of the order by similarity to Zed + const axes = { Zed: [1, 0], Mid: [1, 1], Abe: [0, 1] }; + const people: Record = {}; + for (const [name, [x, y]] of Object.entries(axes)) { + const { result: person } = await ctx.newPerson({ ownerId: user.id, name }); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + const { assetFace } = await ctx.newAssetFace({ assetId: asset.id, personGroupId: person.personGroupId }); + const embedding = `[${[x, y, ...Array.from({ length: 510 }, () => 0)]}]`; + await ctx.database.insertInto('face_search').values({ faceId: assetFace.id, embedding }).execute(); + await ctx.database + .updateTable('person') + .set({ faceAssetId: assetFace.id }) + .where('personGroupId', '=', person.personGroupId) + .execute(); + people[name] = person.personGroupId; + } + const { result: faceless } = await ctx.newPerson({ ownerId: user.id, name: 'Faceless' }); + const closestTo = async (closestPersonId: string) => { + const { items } = await sut.getAllForUser({ take: 10, skip: 0 }, user.id, { + withHidden: false, + partnerIds: [], + closestPersonId, + }); + return items.map(({ name }) => name); + }; + + await expect(listFor(sut, user.id)).resolves.toEqual( + ['Abe', 'Mid', 'Zed'].map((name) => expect.objectContaining({ name })), + ); + await expect(closestTo(people.Zed)).resolves.toEqual(['Zed', 'Mid', 'Abe']); + // a person without a face has nothing to compare against, so the default order applies + await expect(closestTo(faceless.personGroupId)).resolves.toEqual(['Abe', 'Mid', 'Zed']); + }); + + it('should match names with a word similarity of at least 0.5', async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const { user } = await ctx.newUser(); + const person = await newNamedPerson(ctx, user.id, 'Alison'); + // 'alisty' is 0.57 similar to 'Alison', below the default threshold of 0.6 + const match = [expect.objectContaining({ personGroupId: person.personGroupId })]; + + await expect(listFor(sut, user.id, { name: 'alisty' })).resolves.toEqual(match); + await expect(sut.getByName(user.id, 'alisty', { withHidden: true })).resolves.toEqual(match); + }); + + it('should stop counting faces on an asset while it is archived or trashed', async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const { user } = await ctx.newUser(); + const { result: person } = await ctx.newPerson({ ownerId: user.id, name: 'Alice' }); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person.personGroupId }); + const { asset: archived } = await ctx.newAsset({ ownerId: user.id, visibility: AssetVisibility.Archive }); + await ctx.newAssetFace({ assetId: archived.id, personGroupId: person.personGroupId }); + const updateAsset = (values: { visibility?: AssetVisibility; deletedAt?: Date | null }) => + ctx.database.updateTable('asset').set(values).where('id', '=', asset.id).execute(); + const statistics = () => sut.getStatistics(person.personGroupId, { ownerId: user.id, partnerIds: [] }); + + await expect(statistics()).resolves.toEqual({ assets: 1 }); + + await updateAsset({ visibility: AssetVisibility.Archive }); + await expect(listFor(sut, user.id)).resolves.toEqual([]); + await expect(statistics()).resolves.toEqual({ assets: 0 }); + + await updateAsset({ visibility: AssetVisibility.Timeline, deletedAt: new Date() }); + await expect(listFor(sut, user.id)).resolves.toEqual([]); + + await updateAsset({ deletedAt: null }); + await expect(listFor(sut, user.id)).resolves.toEqual([ + expect.objectContaining({ personGroupId: person.personGroupId }), + ]); + await expect(statistics()).resolves.toEqual({ assets: 1 }); + }); + + it("should count a partner's face once when it is also in an album shared with the user", async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const [{ user }, { user: partner }] = [await ctx.newUser(), await ctx.newUser()]; + const person = await newNamedPerson(ctx, user.id, 'Alice'); + const { asset } = await ctx.newAsset({ ownerId: partner.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person.personGroupId }); + const { album } = await ctx.newAlbum({ ownerId: partner.id }, [asset.id]); + await ctx.newAlbumUser({ albumId: album.id, userId: user.id }); + + await expect(listFor(sut, user.id, { partnerIds: [partner.id] })).resolves.toEqual([ + expect.objectContaining({ personGroupId: person.personGroupId, faceCount: 2 }), + ]); + }); + + it("should count a face once, and faces on other users' assets only in albums shared with the user", async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const [{ user }, { user: other }] = [await ctx.newUser(), await ctx.newUser()]; + const { result: person } = await ctx.newPerson({ ownerId: user.id, name: '' }); + const ownAssetIds: string[] = []; + for (let i = 0; i < 2; i++) { + const { asset } = await ctx.newAsset({ ownerId: user.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person.personGroupId }); + ownAssetIds.push(asset.id); + } + await ctx.newAlbum({ ownerId: user.id }, ownAssetIds); + const { asset } = await ctx.newAsset({ ownerId: other.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person.personGroupId }); + const { album } = await ctx.newAlbum({ ownerId: other.id }, [asset.id]); + + // two faces are below the minimum of three, even though their assets are also in the user's album + await expect(listFor(sut, user.id)).resolves.toEqual([]); + + await ctx.newAlbumUser({ albumId: album.id, userId: user.id }); + await expect(listFor(sut, user.id)).resolves.toEqual([ + expect.objectContaining({ personGroupId: person.personGroupId }), + ]); + await expect(sut.getStatistics(person.personGroupId, { ownerId: user.id, partnerIds: [] })).resolves.toEqual({ + assets: 3, }); }); @@ -567,6 +731,25 @@ describe(PersonRepository.name, () => { await expect(listFor(sut, user.id)).resolves.toEqual([]); }); + it("should use the user's minimum face count", async () => { + const { ctx, sut } = setup(await getKyselyDB()); + const { user } = await ctx.newUser(); + const { result: person } = await ctx.newPerson({ ownerId: user.id, name: '' }); + for (let i = 0; i < 2; i++) { + const { asset } = await ctx.newAsset({ ownerId: user.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person.personGroupId }); + } + + await expect(listFor(sut, user.id)).resolves.toEqual([]); + await ctx.database + .insertInto('user_metadata') + .values({ userId: user.id, key: UserMetadataKey.Preferences, value: { people: { minimumFaces: 2 } } }) + .execute(); + await expect(listFor(sut, user.id)).resolves.toEqual([ + expect.objectContaining({ personGroupId: person.personGroupId, faceCount: 2 }), + ]); + }); + it('should stop returning a shared person once the user deletes their copy', async () => { const { ctx, sut } = setup(await getKyselyDB()); const [{ user: owner }, { user: sharedWith }] = [await ctx.newUser(), await ctx.newUser()]; diff --git a/server/test/medium/specs/services/person.service.spec.ts b/server/test/medium/specs/services/person.service.spec.ts index a6a1f88e5..2750cdd3f 100644 --- a/server/test/medium/specs/services/person.service.spec.ts +++ b/server/test/medium/specs/services/person.service.spec.ts @@ -344,13 +344,13 @@ describe(PersonService.name, () => { sut.update(factory.auth({ user }), personGroupId, { name: 'New name', birthDate: '2000-01-01' }), ).resolves.toEqual(expect.objectContaining({ name: 'New name', birthDate: '2000-01-01' })); - await expect(personRepo.getForUser({ userId: user.id, personGroupId })).resolves.toEqual( - expect.objectContaining({ name: 'New name', birthDate: '2000-01-01' }), + await expect(personRepo.getByGroupId({ ownerId: user.id, personGroupId })).resolves.toEqual( + expect.objectContaining({ name: 'New name', birthDate: new Date('2000-01-01') }), ); - await expect(personRepo.getForUser({ userId: writer.id, personGroupId })).resolves.toEqual( - expect.objectContaining({ name: 'New name', birthDate: '2000-01-01' }), + await expect(personRepo.getByGroupId({ ownerId: writer.id, personGroupId })).resolves.toEqual( + expect.objectContaining({ name: 'New name', birthDate: new Date('2000-01-01') }), ); - await expect(personRepo.getForUser({ userId: reader.id, personGroupId })).resolves.toEqual( + await expect(personRepo.getByGroupId({ ownerId: reader.id, personGroupId })).resolves.toEqual( expect.objectContaining({ name: 'Old name', birthDate: null }), ); }); @@ -373,10 +373,10 @@ describe(PersonService.name, () => { sut.update(factory.auth({ user }), personGroupId, { name: 'New name', userId: writer.id }), ).resolves.toEqual(expect.objectContaining({ name: 'New name' })); - await expect(personRepo.getForUser({ userId: writer.id, personGroupId })).resolves.toEqual( + await expect(personRepo.getByGroupId({ ownerId: writer.id, personGroupId })).resolves.toEqual( expect.objectContaining({ name: 'New name' }), ); - await expect(personRepo.getForUser({ userId: user.id, personGroupId })).resolves.toEqual( + await expect(personRepo.getByGroupId({ ownerId: user.id, personGroupId })).resolves.toEqual( expect.objectContaining({ name: 'Old name' }), ); }); @@ -399,7 +399,7 @@ describe(PersonService.name, () => { sut.update(factory.auth({ user }), personGroupId, { name: 'New name', isFavorite: true, isHidden: true }), ).resolves.toEqual(expect.objectContaining({ name: 'New name', isFavorite: true, isHidden: true })); - await expect(personRepo.getForUser({ userId: writer.id, personGroupId })).resolves.toEqual( + await expect(personRepo.getByGroupId({ ownerId: writer.id, personGroupId })).resolves.toEqual( expect.objectContaining({ name: 'New name', isFavorite: false, isHidden: false }), ); }); diff --git a/server/test/medium/specs/sync/sync-asset-face.spec.ts b/server/test/medium/specs/sync/sync-asset-face.spec.ts index 4248fe9bf..28208a4c8 100644 --- a/server/test/medium/specs/sync/sync-asset-face.spec.ts +++ b/server/test/medium/specs/sync/sync-asset-face.spec.ts @@ -1,5 +1,5 @@ import { Kysely } from 'kysely'; -import { SyncEntityType, SyncRequestType } from 'src/enum.js'; +import { AssetVisibility, SyncEntityType, SyncRequestType } from 'src/enum.js'; import { PersonRepository } from 'src/repositories/person.repository.js'; import { DB } from 'src/schema/index.js'; import { SyncTestContext } from 'test/medium.factory.js'; @@ -399,6 +399,21 @@ describe(SyncEntityType.AssetFaceV3, () => { await ctx.assertSyncIsComplete(auth, [SyncRequestType.AssetFacesV3]); }); + it.each([ + { state: 'archived', values: { visibility: AssetVisibility.Archive } }, + { state: 'trashed', values: { deletedAt: new Date() } }, + { state: 'favorited', values: { isFavorite: true } }, + ])('should not sync an asset face again when its asset is $state', async ({ values }) => { + const { auth, ctx } = await setup(); + const { asset } = await ctx.newAsset({ ownerId: auth.user.id }); + await ctx.newAssetFace({ assetId: asset.id }); + await ctx.syncAckAll(auth, await ctx.syncStream(auth, [SyncRequestType.AssetFacesV3])); + + await ctx.database.updateTable('asset').set(values).where('id', '=', asset.id).execute(); + + await ctx.assertSyncIsComplete(auth, [SyncRequestType.AssetFacesV3]); + }); + it('should detect and sync a deleted asset face without a person', async () => { const { auth, ctx } = await setup(); const personRepo = ctx.get(PersonRepository);