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

@@ -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, QueueName> = {
   [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,

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 }

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');
     });
 

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 });

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(),