From 98db9331d83263d7637e42851d01ba58324c69e6 Mon Sep 17 00:00:00 2001 From: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> Date: Tue, 3 Oct 2023 03:15:11 +0200 Subject: [PATCH] fix(server): delete face thumbnails when merging people (#4310) * new job for person deletion, including face thumbnail deletion * fix tests, delete files directly instead queueing jobs --- server/src/domain/job/job.constants.ts | 4 ++- server/src/domain/job/job.repository.ts | 1 + .../src/domain/person/person.service.spec.ts | 17 ++++++----- server/src/domain/person/person.service.ts | 30 ++++++++++++++----- server/src/microservices/app.service.ts | 1 + 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/server/src/domain/job/job.constants.ts b/server/src/domain/job/job.constants.ts index 20c566b51a..3c422aaf7e 100644 --- a/server/src/domain/job/job.constants.ts +++ b/server/src/domain/job/job.constants.ts @@ -56,9 +56,10 @@ export enum JobName { CLASSIFY_IMAGE = 'classify-image', // facial recognition + PERSON_CLEANUP = 'person-cleanup', + PERSON_DELETE = 'person-delete', QUEUE_RECOGNIZE_FACES = 'queue-recognize-faces', RECOGNIZE_FACES = 'recognize-faces', - PERSON_CLEANUP = 'person-cleanup', // library managment LIBRARY_SCAN = 'library-refresh', @@ -103,6 +104,7 @@ export const JOBS_TO_QUEUE: Record = { [JobName.DELETE_FILES]: QueueName.BACKGROUND_TASK, [JobName.CLEAN_OLD_AUDIT_LOGS]: QueueName.BACKGROUND_TASK, [JobName.PERSON_CLEANUP]: QueueName.BACKGROUND_TASK, + [JobName.PERSON_DELETE]: QueueName.BACKGROUND_TASK, // conversion [JobName.QUEUE_VIDEO_CONVERSION]: QueueName.VIDEO_CONVERSION, diff --git a/server/src/domain/job/job.repository.ts b/server/src/domain/job/job.repository.ts index d353b0cb75..072d48af1b 100644 --- a/server/src/domain/job/job.repository.ts +++ b/server/src/domain/job/job.repository.ts @@ -68,6 +68,7 @@ export type JobItem = | { name: JobName.QUEUE_RECOGNIZE_FACES; data: IBaseJob } | { name: JobName.RECOGNIZE_FACES; data: IEntityJob } | { name: JobName.GENERATE_PERSON_THUMBNAIL; data: IEntityJob } + | { name: JobName.PERSON_DELETE; data: IEntityJob } // Clip Embedding | { name: JobName.QUEUE_ENCODE_CLIP; data: IBaseJob } diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index d1e2c436da..3bfea3b689 100644 --- a/server/src/domain/person/person.service.spec.ts +++ b/server/src/domain/person/person.service.spec.ts @@ -373,11 +373,7 @@ describe(PersonService.name, () => { await sut.handlePersonCleanup(); - expect(personMock.delete).toHaveBeenCalledWith(personStub.noName); - expect(jobMock.queue).toHaveBeenCalledWith({ - name: JobName.DELETE_FILES, - data: { files: ['/path/to/thumbnail.jpg'] }, - }); + expect(jobMock.queue).toHaveBeenCalledWith({ name: JobName.PERSON_DELETE, data: { id: personStub.noName.id } }); }); }); @@ -409,7 +405,7 @@ describe(PersonService.name, () => { items: [assetStub.image], hasNextPage: false, }); - personMock.deleteAll.mockResolvedValue(5); + personMock.getAll.mockResolvedValue([personStub.withName]); searchMock.deleteAllFaces.mockResolvedValue(100); await sut.handleQueueRecognizeFaces({ force: true }); @@ -419,6 +415,10 @@ describe(PersonService.name, () => { name: JobName.RECOGNIZE_FACES, data: { id: assetStub.image.id }, }); + expect(jobMock.queue).toHaveBeenCalledWith({ + name: JobName.PERSON_DELETE, + data: { id: personStub.withName.id }, + }); }); }); @@ -650,7 +650,10 @@ describe(PersonService.name, () => { oldPersonId: personStub.mergePerson.id, }); - expect(personMock.delete).toHaveBeenCalledWith(personStub.mergePerson); + expect(jobMock.queue).toHaveBeenCalledWith({ + name: JobName.PERSON_DELETE, + data: { id: personStub.mergePerson.id }, + }); expect(accessMock.person.hasOwnerAccess).toHaveBeenCalledWith(authStub.admin.id, 'person-1'); }); diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index 0143abc96f..11177fce7e 100644 --- a/server/src/domain/person/person.service.ts +++ b/server/src/domain/person/person.service.ts @@ -139,16 +139,27 @@ export class PersonService { return results; } + async handlePersonDelete({ id }: IEntityJob) { + const person = await this.repository.getById(id); + if (!person) { + return false; + } + + try { + await this.repository.delete(person); + await this.storageRepository.unlink(person.thumbnailPath); + } catch (error: Error | any) { + this.logger.error(`Unable to delete person: ${error}`, error?.stack); + } + + return true; + } + async handlePersonCleanup() { const people = await this.repository.getAllWithoutFaces(); for (const person of people) { this.logger.debug(`Person ${person.name || person.id} no longer has any faces, deleting.`); - try { - await this.repository.delete(person); - await this.jobRepository.queue({ name: JobName.DELETE_FILES, data: { files: [person.thumbnailPath] } }); - } catch (error: Error | any) { - this.logger.error(`Unable to delete person: ${error}`, error?.stack); - } + await this.jobRepository.queue({ name: JobName.PERSON_DELETE, data: { id: person.id } }); } return true; @@ -167,7 +178,10 @@ export class PersonService { }); if (force) { - const people = await this.repository.deleteAll(); + const people = await this.repository.getAll(); + for (const person of people) { + await this.jobRepository.queue({ name: JobName.PERSON_DELETE, data: { id: person.id } }); + } const faces = await this.searchRepository.deleteAllFaces(); this.logger.debug(`Deleted ${people} people and ${faces} faces`); } @@ -363,7 +377,7 @@ export class PersonService { await this.jobRepository.queue({ name: JobName.SEARCH_REMOVE_FACE, data: { assetId, personId: mergeId } }); } await this.repository.reassignFaces(mergeData); - await this.repository.delete(mergePerson); + await this.jobRepository.queue({ name: JobName.PERSON_DELETE, data: { id: mergePerson.id } }); this.logger.log(`Merged ${mergeName} into ${primaryName}`); results.push({ id: mergeId, success: true }); diff --git a/server/src/microservices/app.service.ts b/server/src/microservices/app.service.ts index abe4b1fba0..eb3c57d4ba 100644 --- a/server/src/microservices/app.service.ts +++ b/server/src/microservices/app.service.ts @@ -74,6 +74,7 @@ export class AppService { [JobName.RECOGNIZE_FACES]: (data) => this.personService.handleRecognizeFaces(data), [JobName.GENERATE_PERSON_THUMBNAIL]: (data) => this.personService.handleGeneratePersonThumbnail(data), [JobName.PERSON_CLEANUP]: () => this.personService.handlePersonCleanup(), + [JobName.PERSON_DELETE]: (data) => this.personService.handlePersonDelete(data), [JobName.QUEUE_SIDECAR]: (data) => this.metadataService.handleQueueSidecar(data), [JobName.SIDECAR_DISCOVERY]: (data) => this.metadataService.handleSidecarDiscovery(data), [JobName.SIDECAR_SYNC]: () => this.metadataService.handleSidecarSync(),