From 25f2b9602fbc7bfd4b99659ef45872c78c4cf6cc Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Fri, 11 Apr 2025 14:44:45 -0400 Subject: [PATCH] refactor(server): remove face, person and face search entities (#17535) * remove face, person and face search entities update tests and mappers check if face relation exists update sql unused imports * pr feedback generate sql, remove unused imports --- server/src/cores/storage.core.ts | 5 +- server/src/database.ts | 41 ++++- server/src/dtos/asset-response.dto.ts | 5 +- server/src/dtos/person.dto.ts | 20 ++- server/src/entities/asset-face.entity.ts | 21 --- server/src/entities/asset.entity.ts | 5 +- server/src/entities/face-search.entity.ts | 7 - server/src/entities/person.entity.ts | 18 --- server/src/queries/person.repository.sql | 67 +++++--- server/src/repositories/person.repository.ts | 146 +++++++++++------- server/src/repositories/search.repository.ts | 2 +- server/src/services/metadata.service.ts | 8 +- server/src/services/person.service.spec.ts | 71 ++++----- server/src/services/person.service.ts | 95 ++++-------- server/src/utils/database.ts | 24 ++- server/test/fixtures/asset.stub.ts | 2 +- server/test/fixtures/face.stub.ts | 74 ++------- server/test/fixtures/person.stub.ts | 92 +++++++++-- .../repositories/person.repository.mock.ts | 3 +- 19 files changed, 384 insertions(+), 322 deletions(-) delete mode 100644 server/src/entities/asset-face.entity.ts delete mode 100644 server/src/entities/face-search.entity.ts delete mode 100644 server/src/entities/person.entity.ts diff --git a/server/src/cores/storage.core.ts b/server/src/cores/storage.core.ts index 4cbf9631585..9985506f24b 100644 --- a/server/src/cores/storage.core.ts +++ b/server/src/cores/storage.core.ts @@ -2,7 +2,6 @@ import { randomUUID } from 'node:crypto'; import { dirname, join, resolve } from 'node:path'; import { APP_MEDIA_LOCATION } from 'src/constants'; import { AssetEntity } from 'src/entities/asset.entity'; -import { PersonEntity } from 'src/entities/person.entity'; import { AssetFileType, AssetPathType, ImageFormat, PathType, PersonPathType, StorageFolder } from 'src/enum'; import { AssetRepository } from 'src/repositories/asset.repository'; import { ConfigRepository } from 'src/repositories/config.repository'; @@ -85,7 +84,7 @@ export class StorageCore { return join(APP_MEDIA_LOCATION, folder); } - static getPersonThumbnailPath(person: PersonEntity) { + static getPersonThumbnailPath(person: { id: string; ownerId: string }) { return StorageCore.getNestedPath(StorageFolder.THUMBNAILS, person.ownerId, `${person.id}.jpeg`); } @@ -135,7 +134,7 @@ export class StorageCore { }); } - async movePersonFile(person: PersonEntity, pathType: PersonPathType) { + async movePersonFile(person: { id: string; ownerId: string; thumbnailPath: string }, pathType: PersonPathType) { const { id: entityId, thumbnailPath } = person; switch (pathType) { case PersonPathType.FACE: { diff --git a/server/src/database.ts b/server/src/database.ts index d6e63928047..38b3ca3a1db 100644 --- a/server/src/database.ts +++ b/server/src/database.ts @@ -1,6 +1,15 @@ import { Selectable } from 'kysely'; import { Exif as DatabaseExif } from 'src/db'; -import { AlbumUserRole, AssetFileType, AssetStatus, AssetType, MemoryType, Permission, UserStatus } from 'src/enum'; +import { + AlbumUserRole, + AssetFileType, + AssetStatus, + AssetType, + MemoryType, + Permission, + SourceType, + UserStatus, +} from 'src/enum'; import { OnThisDayData, UserMetadataItem } from 'src/types'; export type AuthUser = { @@ -199,6 +208,36 @@ export type Session = { export type Exif = Omit, 'updatedAt' | 'updateId'>; +export type Person = { + createdAt: Date; + id: string; + ownerId: string; + updatedAt: Date; + updateId: string; + isFavorite: boolean; + name: string; + birthDate: Date | null; + color: string | null; + faceAssetId: string | null; + isHidden: boolean; + thumbnailPath: string; +}; + +export type AssetFace = { + id: string; + deletedAt: Date | null; + assetId: string; + boundingBoxX1: number; + boundingBoxX2: number; + boundingBoxY1: number; + boundingBoxY2: number; + imageHeight: number; + imageWidth: number; + personId: string | null; + sourceType: SourceType; + person?: Person | null; +}; + const userColumns = ['id', 'name', 'email', 'profileImagePath', 'profileChangedAt'] as const; export const columns = { diff --git a/server/src/dtos/asset-response.dto.ts b/server/src/dtos/asset-response.dto.ts index b12a4378fed..985ad047290 100644 --- a/server/src/dtos/asset-response.dto.ts +++ b/server/src/dtos/asset-response.dto.ts @@ -1,4 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; +import { AssetFace } from 'src/database'; import { PropertyLifecycle } from 'src/decorators'; import { AuthDto } from 'src/dtos/auth.dto'; import { ExifResponseDto, mapExif } from 'src/dtos/exif.dto'; @@ -10,7 +11,6 @@ import { } from 'src/dtos/person.dto'; import { TagResponseDto, mapTag } from 'src/dtos/tag.dto'; import { UserResponseDto, mapUser } from 'src/dtos/user.dto'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { AssetEntity } from 'src/entities/asset.entity'; import { AssetType } from 'src/enum'; import { mimeTypes } from 'src/utils/mime-types'; @@ -71,7 +71,8 @@ export type AssetMapOptions = { auth?: AuthDto; }; -const peopleWithFaces = (faces: AssetFaceEntity[]): PersonWithFacesResponseDto[] => { +// TODO: this is inefficient +const peopleWithFaces = (faces: AssetFace[]): PersonWithFacesResponseDto[] => { const result: PersonWithFacesResponseDto[] = []; if (faces) { for (const face of faces) { diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts index 49f3416b9ad..90490715efc 100644 --- a/server/src/dtos/person.dto.ts +++ b/server/src/dtos/person.dto.ts @@ -1,11 +1,12 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { Type } from 'class-transformer'; import { IsArray, IsInt, IsNotEmpty, IsNumber, IsString, Max, Min, ValidateNested } from 'class-validator'; +import { Selectable } from 'kysely'; import { DateTime } from 'luxon'; +import { AssetFace, Person } from 'src/database'; +import { AssetFaces } from 'src/db'; import { PropertyLifecycle } from 'src/decorators'; import { AuthDto } from 'src/dtos/auth.dto'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; -import { PersonEntity } from 'src/entities/person.entity'; import { SourceType } from 'src/enum'; import { asDateString } from 'src/utils/date'; import { @@ -219,7 +220,7 @@ export class PeopleResponseDto { hasNextPage?: boolean; } -export function mapPerson(person: PersonEntity): PersonResponseDto { +export function mapPerson(person: Person): PersonResponseDto { return { id: person.id, name: person.name, @@ -232,7 +233,7 @@ export function mapPerson(person: PersonEntity): PersonResponseDto { }; } -export function mapFacesWithoutPerson(face: AssetFaceEntity): AssetFaceWithoutPersonResponseDto { +export function mapFacesWithoutPerson(face: Selectable): AssetFaceWithoutPersonResponseDto { return { id: face.id, imageHeight: face.imageHeight, @@ -245,9 +246,16 @@ export function mapFacesWithoutPerson(face: AssetFaceEntity): AssetFaceWithoutPe }; } -export function mapFaces(face: AssetFaceEntity, auth: AuthDto): AssetFaceResponseDto { +export function mapFaces(face: AssetFace, auth: AuthDto): AssetFaceResponseDto { return { - ...mapFacesWithoutPerson(face), + id: face.id, + imageHeight: face.imageHeight, + imageWidth: face.imageWidth, + boundingBoxX1: face.boundingBoxX1, + boundingBoxX2: face.boundingBoxX2, + boundingBoxY1: face.boundingBoxY1, + boundingBoxY2: face.boundingBoxY2, + sourceType: face.sourceType, person: face.person?.ownerId === auth.user.id ? mapPerson(face.person) : null, }; } diff --git a/server/src/entities/asset-face.entity.ts b/server/src/entities/asset-face.entity.ts deleted file mode 100644 index dddb6b0f3fd..00000000000 --- a/server/src/entities/asset-face.entity.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { AssetEntity } from 'src/entities/asset.entity'; -import { FaceSearchEntity } from 'src/entities/face-search.entity'; -import { PersonEntity } from 'src/entities/person.entity'; -import { SourceType } from 'src/enum'; - -export class AssetFaceEntity { - id!: string; - assetId!: string; - personId!: string | null; - faceSearch?: FaceSearchEntity; - imageWidth!: number; - imageHeight!: number; - boundingBoxX1!: number; - boundingBoxY1!: number; - boundingBoxX2!: number; - boundingBoxY2!: number; - sourceType!: SourceType; - asset!: AssetEntity; - person!: PersonEntity | null; - deletedAt!: Date | null; -} diff --git a/server/src/entities/asset.entity.ts b/server/src/entities/asset.entity.ts index 21ea8e3f08f..9cf04f8d3b9 100644 --- a/server/src/entities/asset.entity.ts +++ b/server/src/entities/asset.entity.ts @@ -1,9 +1,8 @@ import { DeduplicateJoinsPlugin, ExpressionBuilder, Kysely, SelectQueryBuilder, sql } from 'kysely'; import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres'; -import { AssetFile, Exif, Tag, User } from 'src/database'; +import { AssetFace, AssetFile, Exif, Tag, User } from 'src/database'; import { DB } from 'src/db'; import { AlbumEntity } from 'src/entities/album.entity'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { AssetJobStatusEntity } from 'src/entities/asset-job-status.entity'; import { SharedLinkEntity } from 'src/entities/shared-link.entity'; import { StackEntity } from 'src/entities/stack.entity'; @@ -49,7 +48,7 @@ export class AssetEntity { tags?: Tag[]; sharedLinks!: SharedLinkEntity[]; albums?: AlbumEntity[]; - faces!: AssetFaceEntity[]; + faces!: AssetFace[]; stackId?: string | null; stack?: StackEntity | null; jobStatus?: AssetJobStatusEntity; diff --git a/server/src/entities/face-search.entity.ts b/server/src/entities/face-search.entity.ts deleted file mode 100644 index 701fd9e5802..00000000000 --- a/server/src/entities/face-search.entity.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; - -export class FaceSearchEntity { - face?: AssetFaceEntity; - faceId!: string; - embedding!: string; -} diff --git a/server/src/entities/person.entity.ts b/server/src/entities/person.entity.ts deleted file mode 100644 index 76174443f9a..00000000000 --- a/server/src/entities/person.entity.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; - -export class PersonEntity { - id!: string; - createdAt!: Date; - updatedAt!: Date; - updateId?: string; - ownerId!: string; - name!: string; - birthDate!: Date | string | null; - thumbnailPath!: string; - faceAssetId!: string | null; - faceAsset!: AssetFaceEntity | null; - faces!: AssetFaceEntity[]; - isHidden!: boolean; - isFavorite!: boolean; - color?: string | null; -} diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index e6868ae3024..f9ba32262d0 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -23,7 +23,7 @@ REINDEX TABLE person -- PersonRepository.delete delete from "person" where - "person"."id" in ($1) + "person"."id" in $1 -- PersonRepository.deleteFaces delete from "asset_faces" @@ -95,41 +95,72 @@ where "asset_faces"."id" = $1 and "asset_faces"."deletedAt" is null --- PersonRepository.getFaceByIdWithAssets +-- PersonRepository.getFaceForFacialRecognitionJob select - "asset_faces".*, + "asset_faces"."id", + "asset_faces"."personId", + "asset_faces"."sourceType", ( select to_json(obj) from ( select - "person".* - from - "person" - where - "person"."id" = "asset_faces"."personId" - ) as obj - ) as "person", - ( - select - to_json(obj) - from - ( - select - "assets".* + "assets"."ownerId", + "assets"."isArchived", + "assets"."fileCreatedAt" from "assets" where "assets"."id" = "asset_faces"."assetId" ) as obj - ) as "asset" + ) as "asset", + ( + select + to_json(obj) + from + ( + select + "face_search".* + from + "face_search" + where + "face_search"."faceId" = "asset_faces"."id" + ) as obj + ) as "faceSearch" from "asset_faces" where "asset_faces"."id" = $1 and "asset_faces"."deletedAt" is null +-- PersonRepository.getDataForThumbnailGenerationJob +select + "person"."ownerId", + "asset_faces"."boundingBoxX1" as "x1", + "asset_faces"."boundingBoxY1" as "y1", + "asset_faces"."boundingBoxX2" as "x2", + "asset_faces"."boundingBoxY2" as "y2", + "asset_faces"."imageWidth" as "oldWidth", + "asset_faces"."imageHeight" as "oldHeight", + "exif"."exifImageWidth", + "exif"."exifImageHeight", + "assets"."type", + "assets"."originalPath", + "asset_files"."path" as "previewPath" +from + "person" + inner join "asset_faces" on "asset_faces"."id" = "person"."faceAssetId" + inner join "assets" on "asset_faces"."assetId" = "assets"."id" + inner join "exif" on "exif"."assetId" = "assets"."id" + inner join "asset_files" on "asset_files"."assetId" = "assets"."id" +where + "person"."id" = $1 + and "asset_faces"."deletedAt" is null + and "asset_files"."type" = $2 + and "exif"."exifImageWidth" > $3 + and "exif"."exifImageHeight" > $4 + -- PersonRepository.reassignFace update "asset_faces" set diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index 751f97fdeb7..d55d863ea7c 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -1,14 +1,12 @@ import { Injectable } from '@nestjs/common'; -import { ExpressionBuilder, Insertable, Kysely, Selectable, sql } from 'kysely'; +import { ExpressionBuilder, Insertable, Kysely, NotNull, Selectable, sql, Updateable } from 'kysely'; import { jsonObjectFrom } from 'kysely/helpers/postgres'; import { InjectKysely } from 'nestjs-kysely'; import { AssetFaces, DB, FaceSearch, Person } from 'src/db'; import { ChunkedArray, DummyValue, GenerateSql } from 'src/decorators'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; -import { PersonEntity } from 'src/entities/person.entity'; -import { SourceType } from 'src/enum'; +import { AssetFileType, SourceType } from 'src/enum'; import { removeUndefinedKeys } from 'src/utils/database'; -import { Paginated, PaginationOptions } from 'src/utils/pagination'; +import { PaginationOptions } from 'src/utils/pagination'; export interface PersonSearchOptions { minimumFaceCount: number; @@ -49,6 +47,19 @@ export interface DeleteFacesOptions { sourceType: SourceType; } +export interface GetAllPeopleOptions { + ownerId?: string; + thumbnailPath?: string; + faceAssetId?: string | null; + isHidden?: boolean; +} + +export interface GetAllFacesOptions { + personId?: string | null; + assetId?: string; + sourceType?: SourceType; +} + export type UnassignFacesOptions = DeleteFacesOptions; export type SelectFaceOptions = (keyof Selectable)[]; @@ -98,20 +109,13 @@ export class PersonRepository { await this.vacuum({ reindexVectors: false }); } - @GenerateSql({ params: [[{ id: DummyValue.UUID }]] }) - async delete(entities: PersonEntity[]): Promise { - if (entities.length === 0) { + @GenerateSql({ params: [DummyValue.UUID] }) + async delete(ids: string[]): Promise { + if (ids.length === 0) { return; } - await this.db - .deleteFrom('person') - .where( - 'person.id', - 'in', - entities.map(({ id }) => id), - ) - .execute(); + await this.db.deleteFrom('person').where('person.id', 'in', ids).execute(); } @GenerateSql({ params: [{ sourceType: SourceType.EXIF }] }) @@ -121,7 +125,7 @@ export class PersonRepository { await this.vacuum({ reindexVectors: sourceType === SourceType.MACHINE_LEARNING }); } - getAllFaces(options: Partial = {}): AsyncIterableIterator { + getAllFaces(options: GetAllFacesOptions = {}) { return this.db .selectFrom('asset_faces') .selectAll('asset_faces') @@ -130,10 +134,10 @@ export class PersonRepository { .$if(!!options.sourceType, (qb) => qb.where('asset_faces.sourceType', '=', options.sourceType!)) .$if(!!options.assetId, (qb) => qb.where('asset_faces.assetId', '=', options.assetId!)) .where('asset_faces.deletedAt', 'is', null) - .stream() as AsyncIterableIterator; + .stream(); } - getAll(options: Partial = {}): AsyncIterableIterator { + getAll(options: GetAllPeopleOptions = {}) { return this.db .selectFrom('person') .selectAll('person') @@ -142,15 +146,11 @@ export class PersonRepository { .$if(options.faceAssetId === null, (qb) => qb.where('person.faceAssetId', 'is', null)) .$if(!!options.faceAssetId, (qb) => qb.where('person.faceAssetId', '=', options.faceAssetId!)) .$if(options.isHidden !== undefined, (qb) => qb.where('person.isHidden', '=', options.isHidden!)) - .stream() as AsyncIterableIterator; + .stream(); } - async getAllForUser( - pagination: PaginationOptions, - userId: string, - options?: PersonSearchOptions, - ): Paginated { - const items = (await this.db + async getAllForUser(pagination: PaginationOptions, userId: string, options?: PersonSearchOptions) { + const items = await this.db .selectFrom('person') .selectAll('person') .innerJoin('asset_faces', 'asset_faces.personId', 'person.id') @@ -198,7 +198,7 @@ export class PersonRepository { .$if(!options?.withHidden, (qb) => qb.where('person.isHidden', '=', false)) .offset(pagination.skip ?? 0) .limit(pagination.take + 1) - .execute()) as PersonEntity[]; + .execute(); if (items.length > pagination.take) { return { items: items.slice(0, -1), hasNextPage: true }; @@ -208,7 +208,7 @@ export class PersonRepository { } @GenerateSql() - getAllWithoutFaces(): Promise { + getAllWithoutFaces() { return this.db .selectFrom('person') .selectAll('person') @@ -216,11 +216,11 @@ export class PersonRepository { .where('asset_faces.deletedAt', 'is', null) .having((eb) => eb.fn.count('asset_faces.assetId'), '=', 0) .groupBy('person.id') - .execute() as Promise; + .execute(); } @GenerateSql({ params: [DummyValue.UUID] }) - getFaces(assetId: string): Promise { + getFaces(assetId: string) { return this.db .selectFrom('asset_faces') .selectAll('asset_faces') @@ -228,11 +228,11 @@ export class PersonRepository { .where('asset_faces.assetId', '=', assetId) .where('asset_faces.deletedAt', 'is', null) .orderBy('asset_faces.boundingBoxX1', 'asc') - .execute() as Promise; + .execute(); } @GenerateSql({ params: [DummyValue.UUID] }) - getFaceById(id: string): Promise { + getFaceById(id: string) { // TODO return null instead of find or fail return this.db .selectFrom('asset_faces') @@ -240,25 +240,57 @@ export class PersonRepository { .select(withPerson) .where('asset_faces.id', '=', id) .where('asset_faces.deletedAt', 'is', null) - .executeTakeFirstOrThrow() as Promise; + .executeTakeFirstOrThrow(); } @GenerateSql({ params: [DummyValue.UUID] }) - getFaceByIdWithAssets( - id: string, - relations?: { faceSearch?: boolean }, - select?: SelectFaceOptions, - ): Promise { + getFaceForFacialRecognitionJob(id: string) { return this.db .selectFrom('asset_faces') - .$if(!!select, (qb) => qb.select(select!)) - .$if(!select, (qb) => qb.selectAll('asset_faces')) - .select(withPerson) - .select(withAsset) - .$if(!!relations?.faceSearch, (qb) => qb.select(withFaceSearch)) + .select(['asset_faces.id', 'asset_faces.personId', 'asset_faces.sourceType']) + .select((eb) => + jsonObjectFrom( + eb + .selectFrom('assets') + .select(['assets.ownerId', 'assets.isArchived', 'assets.fileCreatedAt']) + .whereRef('assets.id', '=', 'asset_faces.assetId'), + ).as('asset'), + ) + .select(withFaceSearch) .where('asset_faces.id', '=', id) .where('asset_faces.deletedAt', 'is', null) - .executeTakeFirst() as Promise; + .executeTakeFirst(); + } + + @GenerateSql({ params: [DummyValue.UUID] }) + getDataForThumbnailGenerationJob(id: string) { + return this.db + .selectFrom('person') + .innerJoin('asset_faces', 'asset_faces.id', 'person.faceAssetId') + .innerJoin('assets', 'asset_faces.assetId', 'assets.id') + .innerJoin('exif', 'exif.assetId', 'assets.id') + .innerJoin('asset_files', 'asset_files.assetId', 'assets.id') + .select([ + 'person.ownerId', + 'asset_faces.boundingBoxX1 as x1', + 'asset_faces.boundingBoxY1 as y1', + 'asset_faces.boundingBoxX2 as x2', + 'asset_faces.boundingBoxY2 as y2', + 'asset_faces.imageWidth as oldWidth', + 'asset_faces.imageHeight as oldHeight', + 'exif.exifImageWidth', + 'exif.exifImageHeight', + 'assets.type', + 'assets.originalPath', + 'asset_files.path as previewPath', + ]) + .where('person.id', '=', id) + .where('asset_faces.deletedAt', 'is', null) + .where('asset_files.type', '=', AssetFileType.PREVIEW) + .where('exif.exifImageWidth', '>', 0) + .where('exif.exifImageHeight', '>', 0) + .$narrowType<{ exifImageWidth: NotNull; exifImageHeight: NotNull }>() + .executeTakeFirst(); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.UUID] }) @@ -272,16 +304,16 @@ export class PersonRepository { return Number(result.numChangedRows ?? 0); } - getById(personId: string): Promise { - return (this.db // + getById(personId: string) { + return this.db // .selectFrom('person') .selectAll('person') .where('person.id', '=', personId) - .executeTakeFirst() ?? null) as Promise; + .executeTakeFirst(); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING, { withHidden: true }] }) - getByName(userId: string, personName: string, { withHidden }: PersonNameSearchOptions): Promise { + getByName(userId: string, personName: string, { withHidden }: PersonNameSearchOptions) { return this.db .selectFrom('person') .selectAll('person') @@ -296,7 +328,7 @@ export class PersonRepository { ) .limit(1000) .$if(!withHidden, (qb) => qb.where('person.isHidden', '=', false)) - .execute() as Promise; + .execute(); } @GenerateSql({ params: [DummyValue.UUID, { withHidden: true }] }) @@ -362,8 +394,8 @@ export class PersonRepository { }; } - create(person: Insertable): Promise { - return this.db.insertInto('person').values(person).returningAll().executeTakeFirst() as Promise; + create(person: Insertable) { + return this.db.insertInto('person').values(person).returningAll().executeTakeFirstOrThrow(); } async createAll(people: Insertable[]): Promise { @@ -399,13 +431,13 @@ export class PersonRepository { await query.selectFrom(sql`(select 1)`.as('dummy')).execute(); } - async update(person: Partial & { id: string }): Promise { + async update(person: Updateable & { id: string }) { return this.db .updateTable('person') .set(person) .where('person.id', '=', person.id) .returningAll() - .executeTakeFirstOrThrow() as Promise; + .executeTakeFirstOrThrow(); } async updateAll(people: Insertable[]): Promise { @@ -437,7 +469,7 @@ export class PersonRepository { @GenerateSql({ params: [[{ assetId: DummyValue.UUID, personId: DummyValue.UUID }]] }) @ChunkedArray() - getFacesByIds(ids: AssetFaceId[]): Promise { + getFacesByIds(ids: AssetFaceId[]) { if (ids.length === 0) { return Promise.resolve([]); } @@ -457,17 +489,17 @@ export class PersonRepository { .where('asset_faces.assetId', 'in', assetIds) .where('asset_faces.personId', 'in', personIds) .where('asset_faces.deletedAt', 'is', null) - .execute() as Promise; + .execute(); } @GenerateSql({ params: [DummyValue.UUID] }) - getRandomFace(personId: string): Promise { + getRandomFace(personId: string) { return this.db .selectFrom('asset_faces') .selectAll('asset_faces') .where('asset_faces.personId', '=', personId) .where('asset_faces.deletedAt', 'is', null) - .executeTakeFirst() as Promise; + .executeTakeFirst(); } @GenerateSql() diff --git a/server/src/repositories/search.repository.ts b/server/src/repositories/search.repository.ts index 736eb6dcc18..5a6785af2d4 100644 --- a/server/src/repositories/search.repository.ts +++ b/server/src/repositories/search.repository.ts @@ -162,7 +162,7 @@ export interface FaceEmbeddingSearch extends SearchEmbeddingOptions { hasPerson?: boolean; numResults: number; maxDistance: number; - minBirthDate?: Date; + minBirthDate?: Date | null; } export interface AssetDuplicateSearch { diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index 273165b5aec..3bf0c6d5c78 100644 --- a/server/src/services/metadata.service.ts +++ b/server/src/services/metadata.service.ts @@ -9,11 +9,9 @@ import { constants } from 'node:fs/promises'; import path from 'node:path'; import { JOBS_ASSET_PAGINATION_SIZE } from 'src/constants'; import { StorageCore } from 'src/cores/storage.core'; -import { Exif } from 'src/db'; +import { AssetFaces, Exif, Person } from 'src/db'; import { OnEvent, OnJob } from 'src/decorators'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { AssetEntity } from 'src/entities/asset.entity'; -import { PersonEntity } from 'src/entities/person.entity'; import { AssetType, DatabaseLock, @@ -587,10 +585,10 @@ export class MetadataService extends BaseService { return; } - const facesToAdd: (Partial & { assetId: string })[] = []; + const facesToAdd: (Insertable & { assetId: string })[] = []; const existingNames = await this.personRepository.getDistinctNames(asset.ownerId, { withHidden: true }); const existingNameMap = new Map(existingNames.map(({ id, name }) => [name.toLowerCase(), id])); - const missing: (Partial & { ownerId: string })[] = []; + const missing: (Insertable & { ownerId: string })[] = []; const missingWithFaceAsset: { id: string; ownerId: string; faceAssetId: string }[] = []; for (const region of tags.RegionInfo.RegionList) { if (!region.Name) { diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index 01093d78cf0..d907ae17149 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -1,7 +1,7 @@ import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { AssetFace } from 'src/database'; import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto'; import { mapFaces, mapPerson, PersonResponseDto } from 'src/dtos/person.dto'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { CacheControl, Colorspace, ImageFormat, JobName, JobStatus, SourceType, SystemMetadataKey } from 'src/enum'; import { WithoutProperty } from 'src/repositories/asset.repository'; import { DetectedFaces } from 'src/repositories/machine-learning.repository'; @@ -11,7 +11,7 @@ import { ImmichFileResponse } from 'src/utils/file'; import { assetStub } from 'test/fixtures/asset.stub'; import { authStub } from 'test/fixtures/auth.stub'; import { faceStub } from 'test/fixtures/face.stub'; -import { personStub } from 'test/fixtures/person.stub'; +import { personStub, personThumbnailStub } from 'test/fixtures/person.stub'; import { systemConfigStub } from 'test/fixtures/system-config.stub'; import { factory } from 'test/small.factory'; import { makeStream, newTestService, ServiceMocks } from 'test/utils'; @@ -24,6 +24,7 @@ const responseDto: PersonResponseDto = { isHidden: false, updatedAt: expect.any(Date), isFavorite: false, + color: expect.any(String), }; const statistics = { assets: 3 }; @@ -90,6 +91,7 @@ describe(PersonService.name, () => { isHidden: true, isFavorite: false, updatedAt: expect.any(Date), + color: expect.any(String), }, ], }); @@ -118,6 +120,7 @@ describe(PersonService.name, () => { isHidden: false, isFavorite: true, updatedAt: expect.any(Date), + color: personStub.isFavorite.color, }, responseDto, ], @@ -137,7 +140,6 @@ describe(PersonService.name, () => { }); it('should throw a bad request when person is not found', async () => { - mocks.person.getById.mockResolvedValue(null); mocks.access.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); await expect(sut.getById(authStub.admin, 'person-1')).rejects.toBeInstanceOf(BadRequestException); expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1'])); @@ -161,7 +163,6 @@ describe(PersonService.name, () => { }); it('should throw an error when personId is invalid', async () => { - mocks.person.getById.mockResolvedValue(null); mocks.access.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); await expect(sut.getThumbnail(authStub.admin, 'person-1')).rejects.toBeInstanceOf(NotFoundException); expect(mocks.storage.createReadStream).not.toHaveBeenCalled(); @@ -231,6 +232,7 @@ describe(PersonService.name, () => { isHidden: false, isFavorite: false, updatedAt: expect.any(Date), + color: expect.any(String), }); expect(mocks.person.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: new Date('1976-06-30') }); expect(mocks.job.queue).not.toHaveBeenCalled(); @@ -346,7 +348,6 @@ describe(PersonService.name, () => { describe('handlePersonMigration', () => { it('should not move person files', async () => { - mocks.person.getById.mockResolvedValue(null); await expect(sut.handlePersonMigration(personStub.noName)).resolves.toBe(JobStatus.FAILED); }); }); @@ -400,6 +401,7 @@ describe(PersonService.name, () => { name: personStub.noName.name, thumbnailPath: personStub.noName.thumbnailPath, updatedAt: expect.any(Date), + color: personStub.noName.color, }); expect(mocks.job.queue).not.toHaveBeenCalledWith(); @@ -438,7 +440,7 @@ describe(PersonService.name, () => { await sut.handlePersonCleanup(); - expect(mocks.person.delete).toHaveBeenCalledWith([personStub.noName]); + expect(mocks.person.delete).toHaveBeenCalledWith([personStub.noName.id]); expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.noName.thumbnailPath); }); }); @@ -480,7 +482,7 @@ describe(PersonService.name, () => { await sut.handleQueueDetectFaces({ force: true }); expect(mocks.person.deleteFaces).toHaveBeenCalledWith({ sourceType: SourceType.MACHINE_LEARNING }); - expect(mocks.person.delete).toHaveBeenCalledWith([personStub.withName]); + expect(mocks.person.delete).toHaveBeenCalledWith([personStub.withName.id]); expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.withName.thumbnailPath); expect(mocks.asset.getAll).toHaveBeenCalled(); expect(mocks.job.queueAll).toHaveBeenCalledWith([ @@ -531,7 +533,7 @@ describe(PersonService.name, () => { data: { id: assetStub.image.id }, }, ]); - expect(mocks.person.delete).toHaveBeenCalledWith([personStub.randomPerson]); + expect(mocks.person.delete).toHaveBeenCalledWith([personStub.randomPerson.id]); expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.randomPerson.thumbnailPath); }); }); @@ -698,7 +700,7 @@ describe(PersonService.name, () => { data: { id: faceStub.face1.id, deferred: false }, }, ]); - expect(mocks.person.delete).toHaveBeenCalledWith([personStub.randomPerson]); + expect(mocks.person.delete).toHaveBeenCalledWith([personStub.randomPerson.id]); expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.randomPerson.thumbnailPath); }); }); @@ -731,7 +733,7 @@ describe(PersonService.name, () => { id: 'asset-face-1', assetId: assetStub.noResizePath.id, personId: faceStub.face1.personId, - } as AssetFaceEntity, + } as AssetFace, ], }, ]); @@ -848,8 +850,8 @@ describe(PersonService.name, () => { }); it('should fail if face does not have asset', async () => { - const face = { ...faceStub.face1, asset: null } as AssetFaceEntity & { asset: null }; - mocks.person.getFaceByIdWithAssets.mockResolvedValue(face); + const face = { ...faceStub.face1, asset: null }; + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(face); expect(await sut.handleRecognizeFaces({ id: faceStub.face1.id })).toBe(JobStatus.FAILED); @@ -858,7 +860,7 @@ describe(PersonService.name, () => { }); it('should skip if face already has an assigned person', async () => { - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.face1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.face1); expect(await sut.handleRecognizeFaces({ id: faceStub.face1.id })).toBe(JobStatus.SKIPPED); @@ -880,7 +882,7 @@ describe(PersonService.name, () => { mocks.systemMetadata.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 1 } } }); mocks.search.searchFaces.mockResolvedValue(faces); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(faceStub.primaryFace1.person); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id }); @@ -910,7 +912,7 @@ describe(PersonService.name, () => { mocks.systemMetadata.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 1 } } }); mocks.search.searchFaces.mockResolvedValue(faces); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(faceStub.primaryFace1.person); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id }); @@ -940,7 +942,7 @@ describe(PersonService.name, () => { mocks.systemMetadata.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 1 } } }); mocks.search.searchFaces.mockResolvedValue(faces); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(faceStub.primaryFace1.person); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id }); @@ -965,7 +967,7 @@ describe(PersonService.name, () => { mocks.systemMetadata.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 1 } } }); mocks.search.searchFaces.mockResolvedValue(faces); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(personStub.withName); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id }); @@ -984,7 +986,7 @@ describe(PersonService.name, () => { const faces = [{ ...faceStub.noPerson1, distance: 0 }] as FaceSearchResult[]; mocks.search.searchFaces.mockResolvedValue(faces); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(personStub.withName); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id }); @@ -1003,7 +1005,7 @@ describe(PersonService.name, () => { mocks.systemMetadata.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 3 } } }); mocks.search.searchFaces.mockResolvedValue(faces); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(personStub.withName); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id }); @@ -1025,7 +1027,7 @@ describe(PersonService.name, () => { mocks.systemMetadata.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 3 } } }); mocks.search.searchFaces.mockResolvedValueOnce(faces).mockResolvedValueOnce([]); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1); + mocks.person.getFaceForFacialRecognitionJob.mockResolvedValue(faceStub.noPerson1); mocks.person.create.mockResolvedValue(personStub.withName); await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id, deferred: true }); @@ -1047,7 +1049,6 @@ describe(PersonService.name, () => { }); it('should skip a person not found', async () => { - mocks.person.getById.mockResolvedValue(null); await sut.handleGeneratePersonThumbnail({ id: 'person-1' }); expect(mocks.media.generateThumbnail).not.toHaveBeenCalled(); }); @@ -1058,30 +1059,18 @@ describe(PersonService.name, () => { expect(mocks.media.generateThumbnail).not.toHaveBeenCalled(); }); - it('should skip a person with a face asset id not found', async () => { - mocks.person.getById.mockResolvedValue({ ...personStub.primaryPerson, faceAssetId: faceStub.middle.id }); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.face1); - await sut.handleGeneratePersonThumbnail({ id: 'person-1' }); - expect(mocks.media.generateThumbnail).not.toHaveBeenCalled(); - }); - - it('should skip a person with a face asset id without a thumbnail', async () => { - mocks.person.getById.mockResolvedValue({ ...personStub.primaryPerson, faceAssetId: faceStub.middle.assetId }); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.face1); - mocks.asset.getByIds.mockResolvedValue([assetStub.noResizePath]); + it('should skip a person with face not found', async () => { await sut.handleGeneratePersonThumbnail({ id: 'person-1' }); expect(mocks.media.generateThumbnail).not.toHaveBeenCalled(); }); it('should generate a thumbnail', async () => { - mocks.person.getById.mockResolvedValue({ ...personStub.primaryPerson, faceAssetId: faceStub.middle.assetId }); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.middle); - mocks.asset.getById.mockResolvedValue(assetStub.primaryImage); + mocks.person.getDataForThumbnailGenerationJob.mockResolvedValue(personThumbnailStub.newThumbnailMiddle); mocks.media.generateThumbnail.mockResolvedValue(); await sut.handleGeneratePersonThumbnail({ id: personStub.primaryPerson.id }); - expect(mocks.asset.getById).toHaveBeenCalledWith(faceStub.middle.assetId, { exifInfo: true, files: true }); + expect(mocks.person.getDataForThumbnailGenerationJob).toHaveBeenCalledWith(personStub.primaryPerson.id); expect(mocks.storage.mkdirSync).toHaveBeenCalledWith('upload/thumbs/admin_id/pe/rs'); expect(mocks.media.generateThumbnail).toHaveBeenCalledWith( assetStub.primaryImage.originalPath, @@ -1107,9 +1096,7 @@ describe(PersonService.name, () => { }); it('should generate a thumbnail without going negative', async () => { - mocks.person.getById.mockResolvedValue({ ...personStub.primaryPerson, faceAssetId: faceStub.start.assetId }); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.start); - mocks.asset.getById.mockResolvedValue(assetStub.image); + mocks.person.getDataForThumbnailGenerationJob.mockResolvedValue(personThumbnailStub.newThumbnailStart); mocks.media.generateThumbnail.mockResolvedValue(); await sut.handleGeneratePersonThumbnail({ id: personStub.primaryPerson.id }); @@ -1134,10 +1121,8 @@ describe(PersonService.name, () => { }); it('should generate a thumbnail without overflowing', async () => { - mocks.person.getById.mockResolvedValue({ ...personStub.primaryPerson, faceAssetId: faceStub.end.assetId }); - mocks.person.getFaceByIdWithAssets.mockResolvedValue(faceStub.end); + mocks.person.getDataForThumbnailGenerationJob.mockResolvedValue(personThumbnailStub.newThumbnailEnd); mocks.person.update.mockResolvedValue(personStub.primaryPerson); - mocks.asset.getById.mockResolvedValue(assetStub.primaryImage); mocks.media.generateThumbnail.mockResolvedValue(); await sut.handleGeneratePersonThumbnail({ id: personStub.primaryPerson.id }); @@ -1220,7 +1205,6 @@ describe(PersonService.name, () => { }); it('should throw an error when the primary person is not found', async () => { - mocks.person.getById.mockResolvedValue(null); mocks.access.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); await expect(sut.mergePerson(authStub.admin, 'person-1', { ids: ['person-2'] })).rejects.toBeInstanceOf( @@ -1233,7 +1217,6 @@ describe(PersonService.name, () => { it('should handle invalid merge ids', async () => { mocks.person.getById.mockResolvedValueOnce(personStub.primaryPerson); - mocks.person.getById.mockResolvedValueOnce(null); mocks.access.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-1'])); mocks.access.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-2'])); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index ec412ad3076..ae59e2d82c9 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -1,6 +1,8 @@ import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common'; +import { Insertable, Updateable } from 'kysely'; import { FACE_THUMBNAIL_SIZE, JOBS_ASSET_PAGINATION_SIZE } from 'src/constants'; import { StorageCore } from 'src/cores/storage.core'; +import { AssetFaces, FaceSearch, Person } from 'src/db'; import { Chunked, OnJob } from 'src/decorators'; import { BulkIdErrorReason, BulkIdResponseDto } from 'src/dtos/asset-ids.response.dto'; import { AuthDto } from 'src/dtos/auth.dto'; @@ -21,10 +23,6 @@ import { PersonStatisticsResponseDto, PersonUpdateDto, } from 'src/dtos/person.dto'; -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; -import { AssetEntity } from 'src/entities/asset.entity'; -import { FaceSearchEntity } from 'src/entities/face-search.entity'; -import { PersonEntity } from 'src/entities/person.entity'; import { AssetFileType, AssetType, @@ -243,9 +241,9 @@ export class PersonService extends BaseService { } @Chunked() - private async delete(people: PersonEntity[]) { + private async delete(people: { id: string; thumbnailPath: string }[]) { await Promise.all(people.map((person) => this.storageRepository.unlink(person.thumbnailPath))); - await this.personRepository.delete(people); + await this.personRepository.delete(people.map((person) => person.id)); this.logger.debug(`Deleted ${people.length} people`); } @@ -317,8 +315,8 @@ export class PersonService extends BaseService { ); this.logger.debug(`${faces.length} faces detected in ${previewFile.path}`); - const facesToAdd: (Partial & { id: string; assetId: string })[] = []; - const embeddings: FaceSearchEntity[] = []; + const facesToAdd: (Insertable & { id: string })[] = []; + const embeddings: FaceSearch[] = []; const mlFaceIds = new Set(); for (const face of asset.faces) { if (face.sourceType === SourceType.MACHINE_LEARNING) { @@ -377,7 +375,10 @@ export class PersonService extends BaseService { return JobStatus.SUCCESS; } - private iou(face: AssetFaceEntity, newBox: BoundingBox): number { + private iou( + face: { boundingBoxX1: number; boundingBoxY1: number; boundingBoxX2: number; boundingBoxY2: number }, + newBox: BoundingBox, + ): number { const x1 = Math.max(face.boundingBoxX1, newBox.x1); const y1 = Math.max(face.boundingBoxY1, newBox.y1); const x2 = Math.min(face.boundingBoxX2, newBox.x2); @@ -453,11 +454,7 @@ export class PersonService extends BaseService { return JobStatus.SKIPPED; } - const face = await this.personRepository.getFaceByIdWithAssets(id, { faceSearch: true }, [ - 'id', - 'personId', - 'sourceType', - ]); + const face = await this.personRepository.getFaceForFacialRecognitionJob(id); if (!face || !face.asset) { this.logger.warn(`Face ${id} not found`); return JobStatus.FAILED; @@ -545,46 +542,23 @@ export class PersonService extends BaseService { } @OnJob({ name: JobName.GENERATE_PERSON_THUMBNAIL, queue: QueueName.THUMBNAIL_GENERATION }) - async handleGeneratePersonThumbnail(data: JobOf): Promise { + async handleGeneratePersonThumbnail({ id }: JobOf): Promise { const { machineLearning, metadata, image } = await this.getConfig({ withCache: true }); if (!isFacialRecognitionEnabled(machineLearning) && !isFaceImportEnabled(metadata)) { return JobStatus.SKIPPED; } - const person = await this.personRepository.getById(data.id); - if (!person?.faceAssetId) { - this.logger.error(`Could not generate person thumbnail: person ${person?.id} has no face asset`); + const data = await this.personRepository.getDataForThumbnailGenerationJob(id); + if (!data) { + this.logger.error(`Could not generate person thumbnail for ${id}: missing data`); return JobStatus.FAILED; } - const face = await this.personRepository.getFaceByIdWithAssets(person.faceAssetId); - if (!face) { - this.logger.error(`Could not generate person thumbnail: face ${person.faceAssetId} not found`); - return JobStatus.FAILED; - } + const { ownerId, x1, y1, x2, y2, oldWidth, oldHeight } = data; - const { - assetId, - boundingBoxX1: x1, - boundingBoxX2: x2, - boundingBoxY1: y1, - boundingBoxY2: y2, - imageWidth: oldWidth, - imageHeight: oldHeight, - } = face; + const { width, height, inputPath } = await this.getInputDimensions(data); - const asset = await this.assetRepository.getById(assetId, { - exifInfo: true, - files: true, - }); - if (!asset) { - this.logger.error(`Could not generate person thumbnail: asset ${assetId} does not exist`); - return JobStatus.FAILED; - } - - const { width, height, inputPath } = await this.getInputDimensions(asset, { width: oldWidth, height: oldHeight }); - - const thumbnailPath = StorageCore.getPersonThumbnailPath(person); + const thumbnailPath = StorageCore.getPersonThumbnailPath({ id, ownerId }); this.storageCore.ensureFolders(thumbnailPath); const thumbnailOptions = { @@ -597,7 +571,7 @@ export class PersonService extends BaseService { }; await this.mediaRepository.generateThumbnail(inputPath, thumbnailOptions, thumbnailPath); - await this.personRepository.update({ id: person.id, thumbnailPath }); + await this.personRepository.update({ id, thumbnailPath }); return JobStatus.SUCCESS; } @@ -634,7 +608,7 @@ export class PersonService extends BaseService { continue; } - const update: Partial = {}; + const update: Updateable & { id: string } = { id: primaryPerson.id }; if (!primaryPerson.name && mergePerson.name) { update.name = mergePerson.name; } @@ -644,7 +618,7 @@ export class PersonService extends BaseService { } if (Object.keys(update).length > 0) { - primaryPerson = await this.personRepository.update({ id: primaryPerson.id, ...update }); + primaryPerson = await this.personRepository.update(update); } const mergeName = mergePerson.name || mergePerson.id; @@ -672,27 +646,26 @@ export class PersonService extends BaseService { return person; } - private async getInputDimensions(asset: AssetEntity, oldDims: ImageDimensions): Promise { - if (!asset.exifInfo?.exifImageHeight || !asset.exifInfo.exifImageWidth) { - throw new Error(`Asset ${asset.id} dimensions are unknown`); - } - - const previewFile = getAssetFile(asset.files, AssetFileType.PREVIEW); - if (!previewFile) { - throw new Error(`Asset ${asset.id} has no preview path`); - } - + private async getInputDimensions(asset: { + type: AssetType; + exifImageWidth: number; + exifImageHeight: number; + previewPath: string; + originalPath: string; + oldWidth: number; + oldHeight: number; + }): Promise { if (asset.type === AssetType.IMAGE) { - let { exifImageWidth: width, exifImageHeight: height } = asset.exifInfo; - if (oldDims.height > oldDims.width !== height > width) { + let { exifImageWidth: width, exifImageHeight: height } = asset; + if (asset.oldHeight > asset.oldWidth !== height > width) { [width, height] = [height, width]; } return { width, height, inputPath: asset.originalPath }; } - const { width, height } = await this.mediaRepository.getImageDimensions(previewFile.path); - return { width, height, inputPath: previewFile.path }; + const { width, height } = await this.mediaRepository.getImageDimensions(asset.previewPath); + return { width, height, inputPath: asset.previewPath }; } private getCrop(dims: { old: ImageDimensions; new: ImageDimensions }, { x1, y1, x2, y2 }: BoundingBox): CropOptions { diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index 8e07f388a05..69e4acaf028 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -1,4 +1,4 @@ -import { Expression, sql } from 'kysely'; +import { Expression, ExpressionBuilder, ExpressionWrapper, Nullable, Selectable, Simplify, sql } from 'kysely'; export const asUuid = (id: string | Expression) => sql`${id}::uuid`; @@ -17,3 +17,25 @@ export const removeUndefinedKeys = (update: T, template: unkno return update; }; + +/** Modifies toJson return type to not set all properties as nullable */ +export function toJson>( + eb: ExpressionBuilder, + table: T, +) { + return eb.fn.toJson(table) as ExpressionWrapper< + DB, + TB, + Simplify< + T extends TB + ? Selectable extends Nullable + ? N | null + : Selectable + : T extends Expression + ? O extends Nullable + ? N | null + : O + : never + > + >; +} diff --git a/server/test/fixtures/asset.stub.ts b/server/test/fixtures/asset.stub.ts index 2d7bdc0fe7e..b388c31e73b 100644 --- a/server/test/fixtures/asset.stub.ts +++ b/server/test/fixtures/asset.stub.ts @@ -7,7 +7,7 @@ import { authStub } from 'test/fixtures/auth.stub'; import { fileStub } from 'test/fixtures/file.stub'; import { userStub } from 'test/fixtures/user.stub'; -const previewFile: AssetFile = { +export const previewFile: AssetFile = { id: 'file-1', type: AssetFileType.PREVIEW, path: '/uploads/user-id/thumbs/path.jpg', diff --git a/server/test/fixtures/face.stub.ts b/server/test/fixtures/face.stub.ts index 37fab86962d..fe5cbb9a567 100644 --- a/server/test/fixtures/face.stub.ts +++ b/server/test/fixtures/face.stub.ts @@ -1,15 +1,17 @@ -import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { SourceType } from 'src/enum'; import { assetStub } from 'test/fixtures/asset.stub'; import { personStub } from 'test/fixtures/person.stub'; -type NonNullableProperty = { [P in keyof T]: NonNullable }; - export const faceStub = { - face1: Object.freeze>({ + face1: Object.freeze({ id: 'assetFaceId1', assetId: assetStub.image.id, - asset: assetStub.image, + asset: { + ...assetStub.image, + libraryId: null, + updateId: '0d1173e3-4d80-4d76-b41e-57d56de21125', + stackId: null, + }, personId: personStub.withName.id, person: personStub.withName, boundingBoxX1: 0, @@ -22,7 +24,7 @@ export const faceStub = { faceSearch: { faceId: 'assetFaceId1', embedding: '[1, 2, 3, 4]' }, deletedAt: new Date(), }), - primaryFace1: Object.freeze({ + primaryFace1: Object.freeze({ id: 'assetFaceId2', assetId: assetStub.image.id, asset: assetStub.image, @@ -38,7 +40,7 @@ export const faceStub = { faceSearch: { faceId: 'assetFaceId2', embedding: '[1, 2, 3, 4]' }, deletedAt: null, }), - mergeFace1: Object.freeze({ + mergeFace1: Object.freeze({ id: 'assetFaceId3', assetId: assetStub.image.id, asset: assetStub.image, @@ -54,55 +56,7 @@ export const faceStub = { faceSearch: { faceId: 'assetFaceId3', embedding: '[1, 2, 3, 4]' }, deletedAt: null, }), - start: Object.freeze({ - id: 'assetFaceId5', - assetId: assetStub.image.id, - asset: assetStub.image, - personId: personStub.newThumbnail.id, - person: personStub.newThumbnail, - boundingBoxX1: 5, - boundingBoxY1: 5, - boundingBoxX2: 505, - boundingBoxY2: 505, - imageHeight: 2880, - imageWidth: 2160, - sourceType: SourceType.MACHINE_LEARNING, - faceSearch: { faceId: 'assetFaceId5', embedding: '[1, 2, 3, 4]' }, - deletedAt: null, - }), - middle: Object.freeze({ - id: 'assetFaceId6', - assetId: assetStub.image.id, - asset: assetStub.image, - personId: personStub.newThumbnail.id, - person: personStub.newThumbnail, - boundingBoxX1: 100, - boundingBoxY1: 100, - boundingBoxX2: 200, - boundingBoxY2: 200, - imageHeight: 500, - imageWidth: 400, - sourceType: SourceType.MACHINE_LEARNING, - faceSearch: { faceId: 'assetFaceId6', embedding: '[1, 2, 3, 4]' }, - deletedAt: null, - }), - end: Object.freeze({ - id: 'assetFaceId7', - assetId: assetStub.image.id, - asset: assetStub.image, - personId: personStub.newThumbnail.id, - person: personStub.newThumbnail, - boundingBoxX1: 300, - boundingBoxY1: 300, - boundingBoxX2: 495, - boundingBoxY2: 495, - imageHeight: 500, - imageWidth: 500, - sourceType: SourceType.MACHINE_LEARNING, - faceSearch: { faceId: 'assetFaceId7', embedding: '[1, 2, 3, 4]' }, - deletedAt: null, - }), - noPerson1: Object.freeze({ + noPerson1: Object.freeze({ id: 'assetFaceId8', assetId: assetStub.image.id, asset: assetStub.image, @@ -118,7 +72,7 @@ export const faceStub = { faceSearch: { faceId: 'assetFaceId8', embedding: '[1, 2, 3, 4]' }, deletedAt: null, }), - noPerson2: Object.freeze({ + noPerson2: Object.freeze({ id: 'assetFaceId9', assetId: assetStub.image.id, asset: assetStub.image, @@ -134,7 +88,7 @@ export const faceStub = { faceSearch: { faceId: 'assetFaceId9', embedding: '[1, 2, 3, 4]' }, deletedAt: null, }), - fromExif1: Object.freeze({ + fromExif1: Object.freeze({ id: 'assetFaceId9', assetId: assetStub.image.id, asset: assetStub.image, @@ -149,7 +103,7 @@ export const faceStub = { sourceType: SourceType.EXIF, deletedAt: null, }), - fromExif2: Object.freeze({ + fromExif2: Object.freeze({ id: 'assetFaceId9', assetId: assetStub.image.id, asset: assetStub.image, @@ -164,7 +118,7 @@ export const faceStub = { sourceType: SourceType.EXIF, deletedAt: null, }), - withBirthDate: Object.freeze({ + withBirthDate: Object.freeze({ id: 'assetFaceId10', assetId: assetStub.image.id, asset: assetStub.image, diff --git a/server/test/fixtures/person.stub.ts b/server/test/fixtures/person.stub.ts index 3d5e0312168..8457f9ddcdd 100644 --- a/server/test/fixtures/person.stub.ts +++ b/server/test/fixtures/person.stub.ts @@ -1,11 +1,15 @@ -import { PersonEntity } from 'src/entities/person.entity'; +import { AssetType } from 'src/enum'; +import { previewFile } from 'test/fixtures/asset.stub'; import { userStub } from 'test/fixtures/user.stub'; +const updateId = '0d1173e3-4d80-4d76-b41e-57d56de21125'; + export const personStub = { - noName: Object.freeze({ + noName: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: '', birthDate: null, @@ -15,11 +19,13 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - hidden: Object.freeze({ + hidden: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: '', birthDate: null, @@ -29,11 +35,13 @@ export const personStub = { faceAsset: null, isHidden: true, isFavorite: false, + color: 'red', }), - withName: Object.freeze({ + withName: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: 'Person 1', birthDate: null, @@ -43,25 +51,29 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - withBirthDate: Object.freeze({ + withBirthDate: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: 'Person 1', - birthDate: '1976-06-30', + birthDate: new Date('1976-06-30'), thumbnailPath: '/path/to/thumbnail.jpg', faces: [], faceAssetId: null, faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - noThumbnail: Object.freeze({ + noThumbnail: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: '', birthDate: null, @@ -71,11 +83,13 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - newThumbnail: Object.freeze({ + newThumbnail: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: '', birthDate: null, @@ -85,11 +99,13 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - primaryPerson: Object.freeze({ + primaryPerson: Object.freeze({ id: 'person-1', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: 'Person 1', birthDate: null, @@ -99,11 +115,13 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - mergePerson: Object.freeze({ + mergePerson: Object.freeze({ id: 'person-2', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: 'Person 2', birthDate: null, @@ -113,11 +131,13 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - randomPerson: Object.freeze({ + randomPerson: Object.freeze({ id: 'person-3', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: '', birthDate: null, @@ -127,11 +147,13 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: false, + color: 'red', }), - isFavorite: Object.freeze({ + isFavorite: Object.freeze({ id: 'person-4', createdAt: new Date('2021-01-01'), updatedAt: new Date('2021-01-01'), + updateId, ownerId: userStub.admin.id, name: 'Person 1', birthDate: null, @@ -141,5 +163,51 @@ export const personStub = { faceAsset: null, isHidden: false, isFavorite: true, + color: 'red', + }), +}; + +export const personThumbnailStub = { + newThumbnailStart: Object.freeze({ + ownerId: userStub.admin.id, + x1: 5, + y1: 5, + x2: 505, + y2: 505, + oldHeight: 2880, + oldWidth: 2160, + type: AssetType.IMAGE, + originalPath: '/original/path.jpg', + exifImageHeight: 3840, + exifImageWidth: 2160, + previewPath: previewFile.path, + }), + newThumbnailMiddle: Object.freeze({ + ownerId: userStub.admin.id, + x1: 100, + y1: 100, + x2: 200, + y2: 200, + oldHeight: 500, + oldWidth: 400, + type: AssetType.IMAGE, + originalPath: '/original/path.jpg', + exifImageHeight: 1000, + exifImageWidth: 1000, + previewPath: previewFile.path, + }), + newThumbnailEnd: Object.freeze({ + ownerId: userStub.admin.id, + x1: 300, + y1: 300, + x2: 495, + y2: 495, + oldHeight: 500, + oldWidth: 500, + type: AssetType.IMAGE, + originalPath: '/original/path.jpg', + exifImageHeight: 1000, + exifImageWidth: 1000, + previewPath: previewFile.path, }), }; diff --git a/server/test/repositories/person.repository.mock.ts b/server/test/repositories/person.repository.mock.ts index 80a6a25c749..59377576b13 100644 --- a/server/test/repositories/person.repository.mock.ts +++ b/server/test/repositories/person.repository.mock.ts @@ -14,7 +14,8 @@ export const newPersonRepositoryMock = (): Mocked