fix(server): regenerate missing person thumbnails (#3970)

* Regenerate missing person thumbnails

* Check for empty string instead of zero length

* Remember asset used as person face

* Define entity relation between person and asset via faceAssetId

* Typo

* Fix entity relation

* Tests

* Tests

* Fix code formatting

* Fix import path

* Fix migration

* format

* Fix entity and migration

* Linting

* Remove unneeded cast

* Conventions

* Simplify queries

* Simplify queries

* Remove unneeded typings from entity

* Remove unneeded cast

---------

Co-authored-by: Alex <alex.tran1502@gmail.com>

server/src/domain/facial-recognition/facial-recognition.service.spec.ts

@@ -296,6 +296,7 @@ describe(FacialRecognitionService.name, () => {
         colorspace: Colorspace.P3,
       });
       expect(personMock.update).toHaveBeenCalledWith({
+        faceAssetId: 'asset-1',
         id: 'person-1',
         thumbnailPath: 'upload/thumbs/user-id/person-1.jpeg',
       });

server/src/domain/facial-recognition/facial-recognition.services.ts

@@ -171,7 +171,7 @@ export class FacialRecognitionService {
       quality: thumbnail.quality,
     } as const;
     await this.mediaRepository.resize(croppedOutput, output, thumbnailOptions);
-    await this.personRepository.update({ id: personId, thumbnailPath: output });
+    await this.personRepository.update({ id: personId, thumbnailPath: output, faceAssetId: data.assetId });
 
     return true;
   }

server/src/domain/job/job.constants.ts

@@ -28,6 +28,7 @@ export enum JobName {
   GENERATE_JPEG_THUMBNAIL = 'generate-jpeg-thumbnail',
   GENERATE_WEBP_THUMBNAIL = 'generate-webp-thumbnail',
   GENERATE_THUMBHASH_THUMBNAIL = 'generate-thumbhash-thumbnail',
+  GENERATE_FACE_THUMBNAIL = 'generate-face-thumbnail',
 
   // metadata
   QUEUE_METADATA_EXTRACTION = 'queue-metadata-extraction',
@@ -50,7 +51,6 @@ export enum JobName {
   // facial recognition
   QUEUE_RECOGNIZE_FACES = 'queue-recognize-faces',
   RECOGNIZE_FACES = 'recognize-faces',
-  GENERATE_FACE_THUMBNAIL = 'generate-face-thumbnail',
   PERSON_CLEANUP = 'person-cleanup',
 
   // cleanup
@@ -97,6 +97,7 @@ export const JOBS_TO_QUEUE: Record<JobName, QueueName> = {
   [JobName.GENERATE_JPEG_THUMBNAIL]: QueueName.THUMBNAIL_GENERATION,
   [JobName.GENERATE_WEBP_THUMBNAIL]: QueueName.THUMBNAIL_GENERATION,
   [JobName.GENERATE_THUMBHASH_THUMBNAIL]: QueueName.THUMBNAIL_GENERATION,
+  [JobName.GENERATE_FACE_THUMBNAIL]: QueueName.THUMBNAIL_GENERATION,
 
   // metadata
   [JobName.QUEUE_METADATA_EXTRACTION]: QueueName.METADATA_EXTRACTION,
@@ -115,7 +116,6 @@ export const JOBS_TO_QUEUE: Record<JobName, QueueName> = {
   // facial recognition
   [JobName.QUEUE_RECOGNIZE_FACES]: QueueName.RECOGNIZE_FACES,
   [JobName.RECOGNIZE_FACES]: QueueName.RECOGNIZE_FACES,
-  [JobName.GENERATE_FACE_THUMBNAIL]: QueueName.RECOGNIZE_FACES,
 
   // clip
   [JobName.QUEUE_ENCODE_CLIP]: QueueName.CLIP_ENCODING,

server/src/domain/media/media.service.spec.ts

@@ -9,15 +9,19 @@ import {
 } from '@app/infra/entities';
 import {
   assetStub,
+  faceStub,
   newAssetRepositoryMock,
   newJobRepositoryMock,
   newMediaRepositoryMock,
+  newPersonRepositoryMock,
   newStorageRepositoryMock,
   newSystemConfigRepositoryMock,
+  personStub,
   probeStub,
 } from '@test';
 import { IAssetRepository, WithoutProperty } from '../asset';
 import { IJobRepository, JobName } from '../job';
+import { IPersonRepository } from '../person';
 import { IStorageRepository } from '../storage';
 import { ISystemConfigRepository } from '../system-config';
 import { IMediaRepository } from './media.repository';
@@ -29,6 +33,7 @@ describe(MediaService.name, () => {
   let configMock: jest.Mocked<ISystemConfigRepository>;
   let jobMock: jest.Mocked<IJobRepository>;
   let mediaMock: jest.Mocked<IMediaRepository>;
+  let personMock: jest.Mocked<IPersonRepository>;
   let storageMock: jest.Mocked<IStorageRepository>;
 
   beforeEach(async () => {
@@ -36,9 +41,10 @@ describe(MediaService.name, () => {
     configMock = newSystemConfigRepositoryMock();
     jobMock = newJobRepositoryMock();
     mediaMock = newMediaRepositoryMock();
+    personMock = newPersonRepositoryMock();
     storageMock = newStorageRepositoryMock();
 
-    sut = new MediaService(assetMock, jobMock, mediaMock, storageMock, configMock);
+    sut = new MediaService(assetMock, personMock, jobMock, mediaMock, storageMock, configMock);
   });
 
   it('should be defined', () => {
@@ -51,6 +57,8 @@ describe(MediaService.name, () => {
         items: [assetStub.image],
         hasNextPage: false,
       });
+      personMock.getAll.mockResolvedValue([personStub.newThumbnail]);
+      personMock.getFaceById.mockResolvedValue(faceStub.face1);
 
       await sut.handleQueueGenerateThumbnails({ force: true });
 
@@ -60,6 +68,57 @@ describe(MediaService.name, () => {
         name: JobName.GENERATE_JPEG_THUMBNAIL,
         data: { id: assetStub.image.id },
       });
+
+      expect(personMock.getAll).toHaveBeenCalled();
+      expect(personMock.getAllWithoutThumbnail).not.toHaveBeenCalled();
+      expect(jobMock.queue).toHaveBeenCalledWith({
+        name: JobName.GENERATE_FACE_THUMBNAIL,
+        data: {
+          imageWidth: faceStub.face1.imageWidth,
+          imageHeight: faceStub.face1.imageHeight,
+          boundingBox: {
+            x1: faceStub.face1.boundingBoxX1,
+            x2: faceStub.face1.boundingBoxX2,
+            y1: faceStub.face1.boundingBoxY1,
+            y2: faceStub.face1.boundingBoxY2,
+          },
+          assetId: faceStub.face1.assetId,
+          personId: personStub.newThumbnail.id,
+        },
+      });
+    });
+
+    it('should queue all people with missing thumbnail path', async () => {
+      assetMock.getWithout.mockResolvedValue({
+        items: [assetStub.image],
+        hasNextPage: false,
+      });
+      personMock.getAllWithoutThumbnail.mockResolvedValue([personStub.noThumbnail]);
+      personMock.getRandomFace.mockResolvedValue(faceStub.face1);
+
+      await sut.handleQueueGenerateThumbnails({ force: false });
+
+      expect(assetMock.getAll).not.toHaveBeenCalled();
+      expect(assetMock.getWithout).toHaveBeenCalledWith({ skip: 0, take: 1000 }, WithoutProperty.THUMBNAIL);
+
+      expect(personMock.getAll).not.toHaveBeenCalled();
+      expect(personMock.getAllWithoutThumbnail).toHaveBeenCalled();
+      expect(personMock.getRandomFace).toHaveBeenCalled();
+      expect(jobMock.queue).toHaveBeenCalledWith({
+        name: JobName.GENERATE_FACE_THUMBNAIL,
+        data: {
+          imageWidth: faceStub.face1.imageWidth,
+          imageHeight: faceStub.face1.imageHeight,
+          boundingBox: {
+            x1: faceStub.face1.boundingBoxX1,
+            x2: faceStub.face1.boundingBoxX2,
+            y1: faceStub.face1.boundingBoxY1,
+            y2: faceStub.face1.boundingBoxY2,
+          },
+          assetId: faceStub.face1.assetId,
+          personId: personStub.newThumbnail.id,
+        },
+      });
     });
 
     it('should queue all assets with missing resize path', async () => {
@@ -67,6 +126,7 @@ describe(MediaService.name, () => {
         items: [assetStub.noResizePath],
         hasNextPage: false,
       });
+      personMock.getAllWithoutThumbnail.mockResolvedValue([]);
 
       await sut.handleQueueGenerateThumbnails({ force: false });
 
@@ -76,6 +136,9 @@ describe(MediaService.name, () => {
         name: JobName.GENERATE_JPEG_THUMBNAIL,
         data: { id: assetStub.image.id },
       });
+
+      expect(personMock.getAll).not.toHaveBeenCalled();
+      expect(personMock.getAllWithoutThumbnail).toHaveBeenCalled();
     });
 
     it('should queue all assets with missing webp path', async () => {
@@ -83,6 +146,7 @@ describe(MediaService.name, () => {
         items: [assetStub.noWebpPath],
         hasNextPage: false,
       });
+      personMock.getAllWithoutThumbnail.mockResolvedValue([]);
 
       await sut.handleQueueGenerateThumbnails({ force: false });
 
@@ -92,6 +156,9 @@ describe(MediaService.name, () => {
         name: JobName.GENERATE_WEBP_THUMBNAIL,
         data: { id: assetStub.image.id },
       });
+
+      expect(personMock.getAll).not.toHaveBeenCalled();
+      expect(personMock.getAllWithoutThumbnail).toHaveBeenCalled();
     });
 
     it('should queue all assets with missing thumbhash', async () => {
@@ -99,6 +166,7 @@ describe(MediaService.name, () => {
         items: [assetStub.noThumbhash],
         hasNextPage: false,
       });
+      personMock.getAllWithoutThumbnail.mockResolvedValue([]);
 
       await sut.handleQueueGenerateThumbnails({ force: false });
 
@@ -108,6 +176,9 @@ describe(MediaService.name, () => {
         name: JobName.GENERATE_THUMBHASH_THUMBNAIL,
         data: { id: assetStub.image.id },
       });
+
+      expect(personMock.getAll).not.toHaveBeenCalled();
+      expect(personMock.getAllWithoutThumbnail).toHaveBeenCalled();
     });
   });
 
@@ -245,6 +316,7 @@ describe(MediaService.name, () => {
         items: [assetStub.video],
         hasNextPage: false,
       });
+      personMock.getAll.mockResolvedValue([]);
 
       await sut.handleQueueVideoConversion({ force: true });
 

server/src/domain/media/media.service.ts

@@ -4,11 +4,13 @@ import { join } from 'path';
 import { IAssetRepository, WithoutProperty } from '../asset';
 import { usePagination } from '../domain.util';
 import { IBaseJob, IEntityJob, IJobRepository, JOBS_ASSET_PAGINATION_SIZE, JobName } from '../job';
+import { IPersonRepository } from '../person';
 import { IStorageRepository, StorageCore, StorageFolder } from '../storage';
 import { ISystemConfigRepository, SystemConfigFFmpegDto } from '../system-config';
 import { SystemConfigCore } from '../system-config/system-config.core';
 import { AudioStreamInfo, IMediaRepository, VideoCodecHWConfig, VideoStreamInfo } from './media.repository';
 import { H264Config, HEVCConfig, NVENCConfig, QSVConfig, ThumbnailConfig, VAAPIConfig, VP9Config } from './media.util';
+
 @Injectable()
 export class MediaService {
   private logger = new Logger(MediaService.name);
@@ -17,6 +19,7 @@ export class MediaService {
 
   constructor(
     @Inject(IAssetRepository) private assetRepository: IAssetRepository,
+    @Inject(IPersonRepository) private personRepository: IPersonRepository,
     @Inject(IJobRepository) private jobRepository: IJobRepository,
     @Inject(IMediaRepository) private mediaRepository: IMediaRepository,
     @Inject(IStorageRepository) private storageRepository: IStorageRepository,
@@ -49,6 +52,32 @@ export class MediaService {
       }
     }
 
+    const people = force ? await this.personRepository.getAll() : await this.personRepository.getAllWithoutThumbnail();
+
+    for (const person of people) {
+      // use stored asset for generating thumbnail or pick a random one if not present
+      const face = person.faceAssetId
+        ? await this.personRepository.getFaceById({ personId: person.id, assetId: person.faceAssetId })
+        : await this.personRepository.getRandomFace(person.id);
+      if (face) {
+        await this.jobRepository.queue({
+          name: JobName.GENERATE_FACE_THUMBNAIL,
+          data: {
+            imageWidth: face.imageWidth,
+            imageHeight: face.imageHeight,
+            boundingBox: {
+              x1: face.boundingBoxX1,
+              x2: face.boundingBoxX2,
+              y1: face.boundingBoxY1,
+              y2: face.boundingBoxY2,
+            },
+            assetId: face.assetId,
+            personId: person.id,
+          },
+        });
+      }
+    }
+
     return true;
   }
 

server/src/domain/person/person.repository.ts

@@ -13,7 +13,9 @@ export interface UpdateFacesData {
 }
 
 export interface IPersonRepository {
-  getAll(userId: string, options: PersonSearchOptions): Promise<PersonEntity[]>;
+  getAll(): Promise<PersonEntity[]>;
+  getAllWithoutThumbnail(): Promise<PersonEntity[]>;
+  getAllForUser(userId: string, options: PersonSearchOptions): Promise<PersonEntity[]>;
   getAllWithoutFaces(): Promise<PersonEntity[]>;
   getById(userId: string, personId: string): Promise<PersonEntity | null>;
 
@@ -27,4 +29,5 @@ export interface IPersonRepository {
   deleteAll(): Promise<number>;
 
   getFaceById(payload: AssetFaceId): Promise<AssetFaceEntity | null>;
+  getRandomFace(personId: string): Promise<AssetFaceEntity | null>;
 }

server/src/domain/person/person.service.spec.ts

@@ -42,25 +42,31 @@ describe(PersonService.name, () => {
 
   describe('getAll', () => {
     it('should get all people with thumbnails', async () => {
-      personMock.getAll.mockResolvedValue([personStub.withName, personStub.noThumbnail]);
+      personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.noThumbnail]);
       await expect(sut.getAll(authStub.admin, { withHidden: undefined })).resolves.toEqual({
         total: 1,
         visible: 1,
         people: [responseDto],
       });
-      expect(personMock.getAll).toHaveBeenCalledWith(authStub.admin.id, { minimumFaceCount: 1, withHidden: false });
+      expect(personMock.getAllForUser).toHaveBeenCalledWith(authStub.admin.id, {
+        minimumFaceCount: 1,
+        withHidden: false,
+      });
     });
     it('should get all visible people with thumbnails', async () => {
-      personMock.getAll.mockResolvedValue([personStub.withName, personStub.hidden]);
+      personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.hidden]);
       await expect(sut.getAll(authStub.admin, { withHidden: false })).resolves.toEqual({
         total: 2,
         visible: 1,
         people: [responseDto],
       });
-      expect(personMock.getAll).toHaveBeenCalledWith(authStub.admin.id, { minimumFaceCount: 1, withHidden: false });
+      expect(personMock.getAllForUser).toHaveBeenCalledWith(authStub.admin.id, {
+        minimumFaceCount: 1,
+        withHidden: false,
+      });
     });
     it('should get all hidden and visible people with thumbnails', async () => {
-      personMock.getAll.mockResolvedValue([personStub.withName, personStub.hidden]);
+      personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.hidden]);
       await expect(sut.getAll(authStub.admin, { withHidden: true })).resolves.toEqual({
         total: 2,
         visible: 1,
@@ -75,7 +81,10 @@ describe(PersonService.name, () => {
           },
         ],
       });
-      expect(personMock.getAll).toHaveBeenCalledWith(authStub.admin.id, { minimumFaceCount: 1, withHidden: true });
+      expect(personMock.getAllForUser).toHaveBeenCalledWith(authStub.admin.id, {
+        minimumFaceCount: 1,
+        withHidden: true,
+      });
     });
   });
 

server/src/domain/person/person.service.ts

@@ -26,7 +26,7 @@ export class PersonService {
   ) {}
 
   async getAll(authUser: AuthUserDto, dto: PersonSearchDto): Promise<PeopleResponseDto> {
-    const people = await this.repository.getAll(authUser.id, {
+    const people = await this.repository.getAllForUser(authUser.id, {
       minimumFaceCount: 1,
       withHidden: dto.withHidden || false,
     });

server/src/infra/entities/person.entity.ts

@@ -8,6 +8,7 @@ import {
   UpdateDateColumn,
 } from 'typeorm';
 import { AssetFaceEntity } from './asset-face.entity';
+import { AssetEntity } from './asset.entity';
 import { UserEntity } from './user.entity';
 
 @Entity('person')
@@ -36,6 +37,12 @@ export class PersonEntity {
   @Column({ default: '' })
   thumbnailPath!: string;
 
+  @Column({ nullable: true })
+  faceAssetId!: string | null;
+
+  @ManyToOne(() => AssetEntity, { onDelete: 'SET NULL', nullable: true })
+  faceAsset!: AssetEntity | null;
+
   @OneToMany(() => AssetFaceEntity, (assetFace) => assetFace.person)
   faces!: AssetFaceEntity[];
 

server/src/infra/migrations/1693833336881-AddPersonFaceAssetId.ts

@@ -0,0 +1,15 @@
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class AddPersonFaceAssetId1693833336881 implements MigrationInterface {
+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`ALTER TABLE "person" ADD "faceAssetId" uuid`);
+    await queryRunner.query(`ALTER TABLE "person" ADD CONSTRAINT "FK_2bbabe31656b6778c6b87b61023" FOREIGN KEY ("faceAssetId") REFERENCES "assets"("id") ON DELETE SET NULL ON UPDATE NO ACTION`);
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`ALTER TABLE "person" DROP CONSTRAINT "FK_2bbabe31656b6778c6b87b61023"`);
+    await queryRunner.query(`ALTER TABLE "person" DROP COLUMN "faceAssetId"`);
+  }
+
+}

server/src/infra/repositories/person.repository.ts

@@ -50,7 +50,15 @@ export class PersonRepository implements IPersonRepository {
     return people.length;
   }
 
-  getAll(userId: string, options?: PersonSearchOptions): Promise<PersonEntity[]> {
+  getAll(): Promise<PersonEntity[]> {
+    return this.personRepository.find();
+  }
+
+  getAllWithoutThumbnail(): Promise<PersonEntity[]> {
+    return this.personRepository.findBy({ thumbnailPath: '' });
+  }
+
+  getAllForUser(userId: string, options?: PersonSearchOptions): Promise<PersonEntity[]> {
     const queryBuilder = this.personRepository
       .createQueryBuilder('person')
       .leftJoin('person.faces', 'face')
@@ -118,4 +126,8 @@ export class PersonRepository implements IPersonRepository {
   async getFaceById({ personId, assetId }: AssetFaceId): Promise<AssetFaceEntity | null> {
     return this.assetFaceRepository.findOneBy({ assetId, personId });
   }
+
+  async getRandomFace(personId: string): Promise<AssetFaceEntity | null> {
+    return this.assetFaceRepository.findOneBy({ personId });
+  }
 }

server/test/fixtures/person.stub.ts

@@ -1,4 +1,5 @@
 import { PersonEntity } from '@app/infra/entities';
+import { assetStub } from '@test/fixtures/asset.stub';
 import { userStub } from './user.stub';
 
 export const personStub = {
@@ -12,6 +13,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/path/to/thumbnail.jpg',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
   hidden: Object.freeze<PersonEntity>({
@@ -24,6 +27,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/path/to/thumbnail.jpg',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: true,
   }),
   withName: Object.freeze<PersonEntity>({
@@ -36,6 +41,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/path/to/thumbnail.jpg',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
   noBirthDate: Object.freeze<PersonEntity>({
@@ -48,6 +55,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/path/to/thumbnail.jpg',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
   withBirthDate: Object.freeze<PersonEntity>({
@@ -60,6 +69,8 @@ export const personStub = {
     birthDate: new Date('1976-06-30'),
     thumbnailPath: '/path/to/thumbnail.jpg',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
   noThumbnail: Object.freeze<PersonEntity>({
@@ -72,6 +83,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
   newThumbnail: Object.freeze<PersonEntity>({
@@ -84,6 +97,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/new/path/to/thumbnail.jpg',
     faces: [],
+    faceAssetId: assetStub.image.id,
+    faceAsset: assetStub.image,
     isHidden: false,
   }),
   primaryPerson: Object.freeze<PersonEntity>({
@@ -96,6 +111,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/path/to/thumbnail',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
   mergePerson: Object.freeze<PersonEntity>({
@@ -108,6 +125,8 @@ export const personStub = {
     birthDate: null,
     thumbnailPath: '/path/to/thumbnail',
     faces: [],
+    faceAssetId: null,
+    faceAsset: null,
     isHidden: false,
   }),
 };

server/test/repositories/person.repository.mock.ts

@@ -4,6 +4,8 @@ export const newPersonRepositoryMock = (): jest.Mocked<IPersonRepository> => {
   return {
     getById: jest.fn(),
     getAll: jest.fn(),
+    getAllWithoutThumbnail: jest.fn(),
+    getAllForUser: jest.fn(),
     getAssets: jest.fn(),
     getAllWithoutFaces: jest.fn(),
 
@@ -13,6 +15,7 @@ export const newPersonRepositoryMock = (): jest.Mocked<IPersonRepository> => {
     delete: jest.fn(),
 
     getFaceById: jest.fn(),
+    getRandomFace: jest.fn(),
     prepareReassignFaces: jest.fn(),
     reassignFaces: jest.fn(),
   };