From 2f13db51df15d90221cb4f964936482003da21f2 Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Mon, 30 Sep 2024 00:29:14 -0400 Subject: [PATCH] fix(server): "all" button for facial recognition deleting faces instead of unassigning them (#13042) * unassign faces instead of deleting them * formatting --- server/src/interfaces/person.interface.ts | 10 ++++-- server/src/repositories/person.repository.ts | 36 +++++++++++++------ server/src/services/person.service.spec.ts | 5 +-- server/src/services/person.service.ts | 14 ++------ .../repositories/person.repository.mock.ts | 3 +- 5 files changed, 39 insertions(+), 29 deletions(-) diff --git a/server/src/interfaces/person.interface.ts b/server/src/interfaces/person.interface.ts index 5708274a6e..65814e0046 100644 --- a/server/src/interfaces/person.interface.ts +++ b/server/src/interfaces/person.interface.ts @@ -1,6 +1,7 @@ import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { AssetEntity } from 'src/entities/asset.entity'; import { PersonEntity } from 'src/entities/person.entity'; +import { SourceType } from 'src/enum'; import { Paginated, PaginationOptions } from 'src/utils/pagination'; import { FindManyOptions, FindOptionsRelations, FindOptionsSelect } from 'typeorm'; @@ -40,10 +41,12 @@ export interface PeopleStatistics { hidden: number; } -export interface DeleteAllFacesOptions { - sourceType?: string; +export interface DeleteFacesOptions { + sourceType: SourceType; } +export type UnassignFacesOptions = DeleteFacesOptions; + export interface IPersonRepository { getAll(pagination: PaginationOptions, options?: FindManyOptions): Paginated; getAllForUser(pagination: PaginationOptions, userId: string, options: PersonSearchOptions): Paginated; @@ -59,7 +62,7 @@ export interface IPersonRepository { createFaces(entities: Partial[]): Promise; delete(entities: PersonEntity[]): Promise; deleteAll(): Promise; - deleteAllFaces(options: DeleteAllFacesOptions): Promise; + deleteFaces(options: DeleteFacesOptions): Promise; replaceFaces(assetId: string, entities: Partial[], sourceType?: string): Promise; getAllFaces(pagination: PaginationOptions, options?: FindManyOptions): Paginated; getFaceById(id: string): Promise; @@ -75,6 +78,7 @@ export interface IPersonRepository { reassignFace(assetFaceId: string, newPersonId: string): Promise; getNumberOfPeople(userId: string): Promise; reassignFaces(data: UpdateFacesData): Promise; + unassignFaces(options: UnassignFacesOptions): Promise; update(person: Partial): Promise; updateAll(people: Partial[]): Promise; getLatestFaceDate(): Promise; diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index 2607d2a9ec..0350e8a953 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -9,13 +9,14 @@ import { PersonEntity } from 'src/entities/person.entity'; import { PaginationMode, SourceType } from 'src/enum'; import { AssetFaceId, - DeleteAllFacesOptions, + DeleteFacesOptions, IPersonRepository, PeopleStatistics, PersonNameResponse, PersonNameSearchOptions, PersonSearchOptions, PersonStatistics, + UnassignFacesOptions, UpdateFacesData, } from 'src/interfaces/person.interface'; import { Instrumentation } from 'src/utils/instrumentation'; @@ -39,12 +40,23 @@ export class PersonRepository implements IPersonRepository { .createQueryBuilder() .update() .set({ personId: newPersonId }) - .where(_.omitBy({ personId: oldPersonId ?? undefined, id: faceIds ? In(faceIds) : undefined }, _.isUndefined)) + .where(_.omitBy({ personId: oldPersonId, id: faceIds ? In(faceIds) : undefined }, _.isUndefined)) .execute(); return result.affected ?? 0; } + async unassignFaces({ sourceType }: UnassignFacesOptions): Promise { + await this.assetFaceRepository + .createQueryBuilder() + .update() + .set({ personId: null }) + .where({ sourceType }) + .execute(); + + await this.vacuum({ reindexVectors: false }); + } + async delete(entities: PersonEntity[]): Promise { await this.personRepository.remove(entities); } @@ -53,21 +65,14 @@ export class PersonRepository implements IPersonRepository { await this.personRepository.clear(); } - async deleteAllFaces({ sourceType }: DeleteAllFacesOptions): Promise { - if (!sourceType) { - return this.assetFaceRepository.query('TRUNCATE TABLE asset_faces CASCADE'); - } - + async deleteFaces({ sourceType }: DeleteFacesOptions): Promise { await this.assetFaceRepository .createQueryBuilder('asset_faces') .delete() .andWhere('sourceType = :sourceType', { sourceType }) .execute(); - await this.assetFaceRepository.query('VACUUM ANALYZE asset_faces, face_search'); - if (sourceType === SourceType.MACHINE_LEARNING) { - await this.assetFaceRepository.query('REINDEX INDEX face_index'); - } + await this.vacuum({ reindexVectors: sourceType === SourceType.MACHINE_LEARNING }); } getAllFaces( @@ -331,4 +336,13 @@ export class PersonRepository implements IPersonRepository { const { id } = await this.personRepository.save(person); return this.personRepository.findOneByOrFail({ id }); } + + private async vacuum({ reindexVectors }: { reindexVectors: boolean }): Promise { + await this.assetFaceRepository.query('VACUUM ANALYZE asset_faces, face_search, person'); + await this.assetFaceRepository.query('REINDEX TABLE asset_faces'); + await this.assetFaceRepository.query('REINDEX TABLE person'); + if (reindexVectors) { + await this.assetFaceRepository.query('REINDEX TABLE face_search'); + } + } } diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index c2b8f18221..5214808de0 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -660,7 +660,7 @@ describe(PersonService.name, () => { expect(systemMock.set).not.toHaveBeenCalled(); }); - it('should delete existing people and faces if forced', async () => { + it('should delete existing people if forced', async () => { jobMock.getJobCounts.mockResolvedValue({ active: 1, waiting: 0, paused: 0, completed: 0, failed: 0, delayed: 0 }); personMock.getAll.mockResolvedValue({ items: [faceStub.face1.person, personStub.randomPerson], @@ -675,7 +675,8 @@ describe(PersonService.name, () => { await sut.handleQueueRecognizeFaces({ force: true }); - expect(personMock.deleteAllFaces).toHaveBeenCalledWith({ sourceType: SourceType.MACHINE_LEARNING }); + expect(personMock.deleteFaces).not.toHaveBeenCalled(); + expect(personMock.unassignFaces).toHaveBeenCalledWith({ sourceType: SourceType.MACHINE_LEARNING }); expect(jobMock.queueAll).toHaveBeenCalledWith([ { name: JobName.FACIAL_RECOGNITION, diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index e8e16adb17..b009696b63 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -276,16 +276,6 @@ export class PersonService { this.logger.debug(`Deleted ${people.length} people`); } - private async deleteAllPeople() { - const personPagination = usePagination(JOBS_ASSET_PAGINATION_SIZE, (pagination) => - this.repository.getAll({ ...pagination, skip: 0 }), - ); - - for await (const people of personPagination) { - await this.delete(people); // deletes thumbnails too - } - } - async handlePersonCleanup(): Promise { const people = await this.repository.getAllWithoutFaces(); await this.delete(people); @@ -299,7 +289,7 @@ export class PersonService { } if (force) { - await this.repository.deleteAllFaces({ sourceType: SourceType.MACHINE_LEARNING }); + await this.repository.deleteFaces({ sourceType: SourceType.MACHINE_LEARNING }); await this.handlePersonCleanup(); } @@ -407,7 +397,7 @@ export class PersonService { const { waiting } = await this.jobRepository.getJobCounts(QueueName.FACIAL_RECOGNITION); if (force) { - await this.repository.deleteAllFaces({ sourceType: SourceType.MACHINE_LEARNING }); + await this.repository.unassignFaces({ sourceType: SourceType.MACHINE_LEARNING }); await this.handlePersonCleanup(); } else if (waiting) { this.logger.debug( diff --git a/server/test/repositories/person.repository.mock.ts b/server/test/repositories/person.repository.mock.ts index 77e8ccf010..6ffe7bf97b 100644 --- a/server/test/repositories/person.repository.mock.ts +++ b/server/test/repositories/person.repository.mock.ts @@ -18,7 +18,7 @@ export const newPersonRepositoryMock = (): Mocked => { updateAll: vitest.fn(), delete: vitest.fn(), deleteAll: vitest.fn(), - deleteAllFaces: vitest.fn(), + deleteFaces: vitest.fn(), getStatistics: vitest.fn(), getAllFaces: vitest.fn(), @@ -26,6 +26,7 @@ export const newPersonRepositoryMock = (): Mocked => { getRandomFace: vitest.fn(), reassignFaces: vitest.fn(), + unassignFaces: vitest.fn(), createFaces: vitest.fn(), replaceFaces: vitest.fn(), getFaces: vitest.fn(),