feat(server): don't re-run face recognition on assets without any faces (#4854)

* Add AssetJobStatus

* fentity

* Add jobStatus field to AssetEntity

* Fix the migration doc paths

* Filter on facesRecognizedAt

* Set facesRecognizedAt field

* Test for facesRecognizedAt

* Done testing

* Adjust FK properties

* Add tests for WithoutProperty.FACES

* chore: non-nullable

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

docs/docs/developer/database-migrations.md

@@ -9,6 +9,6 @@ npm run typeorm:migrations:generate ./src/infra/<migration-name>
 ```
 
 2. Check if the migration file makes sense.
-3. Move the migration file to folder `./src/infra/database/migrations` in your code editor.
+3. Move the migration file to folder `./server/src/infra/migrations` in your code editor.
 
 The server will automatically detect `*.ts` file changes and restart. Part of the server start-up process includes running any new migrations, so it will be applied immediately.

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

@@ -467,6 +467,8 @@ describe(PersonService.name, () => {
     });
 
     it('should handle no results', async () => {
+      const start = Date.now();
+
       machineLearningMock.detectFaces.mockResolvedValue([]);
       assetMock.getByIds.mockResolvedValue([assetStub.image]);
       await sut.handleRecognizeFaces({ id: assetStub.image.id });
@@ -485,6 +487,12 @@ describe(PersonService.name, () => {
       );
       expect(personMock.createFace).not.toHaveBeenCalled();
       expect(jobMock.queue).not.toHaveBeenCalled();
+
+      expect(assetMock.upsertJobStatus).toHaveBeenCalledWith({
+        assetId: assetStub.image.id,
+        facesRecognizedAt: expect.any(Date),
+      });
+      expect(assetMock.upsertJobStatus.mock.calls[0][0].facesRecognizedAt?.getTime()).toBeGreaterThan(start);
     });
 
     it('should match existing people', async () => {

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

@@ -274,6 +274,11 @@ export class PersonService {
       }
     }
 
+    await this.assetRepository.upsertJobStatus({
+      assetId: asset.id,
+      facesRecognizedAt: new Date(),
+    });
+
     return true;
   }
 

server/src/domain/repositories/asset.repository.ts

@@ -1,4 +1,4 @@
-import { AssetEntity, AssetType, ExifEntity } from '@app/infra/entities';
+import { AssetEntity, AssetJobStatusEntity, AssetType, ExifEntity } from '@app/infra/entities';
 import { FindOptionsRelations } from 'typeorm';
 import { Paginated, PaginationOptions } from '../domain.util';
 
@@ -126,4 +126,5 @@ export interface IAssetRepository {
   getTimeBuckets(options: TimeBucketOptions): Promise<TimeBucketItem[]>;
   getTimeBucket(timeBucket: string, options: TimeBucketOptions): Promise<AssetEntity[]>;
   upsertExif(exif: Partial<ExifEntity>): Promise<void>;
+  upsertJobStatus(jobStatus: Partial<AssetJobStatusEntity>): Promise<void>;
 }

server/src/infra/entities/asset-job-status.entity.ts

@@ -0,0 +1,15 @@
+import { Column, Entity, JoinColumn, OneToOne, PrimaryColumn } from 'typeorm';
+import { AssetEntity } from './asset.entity';
+
+@Entity('asset_job_status')
+export class AssetJobStatusEntity {
+  @OneToOne(() => AssetEntity, { onDelete: 'CASCADE', onUpdate: 'CASCADE' })
+  @JoinColumn()
+  asset!: AssetEntity;
+
+  @PrimaryColumn()
+  assetId!: string;
+
+  @Column({ type: 'timestamptz', nullable: true })
+  facesRecognizedAt!: Date | null;
+}

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

@@ -15,6 +15,7 @@ import {
 } from 'typeorm';
 import { AlbumEntity } from './album.entity';
 import { AssetFaceEntity } from './asset-face.entity';
+import { AssetJobStatusEntity } from './asset-job-status.entity';
 import { ExifEntity } from './exif.entity';
 import { LibraryEntity } from './library.entity';
 import { SharedLinkEntity } from './shared-link.entity';
@@ -158,6 +159,9 @@ export class AssetEntity {
 
   @OneToMany(() => AssetEntity, (asset) => asset.stackParent)
   stack?: AssetEntity[];
+
+  @OneToOne(() => AssetJobStatusEntity, (jobStatus) => jobStatus.asset, { nullable: true })
+  jobStatus?: AssetJobStatusEntity;
 }
 
 export enum AssetType {

server/src/infra/entities/index.ts

@@ -2,6 +2,7 @@ import { ActivityEntity } from './activity.entity';
 import { AlbumEntity } from './album.entity';
 import { APIKeyEntity } from './api-key.entity';
 import { AssetFaceEntity } from './asset-face.entity';
+import { AssetJobStatusEntity } from './asset-job-status.entity';
 import { AssetEntity } from './asset.entity';
 import { AuditEntity } from './audit.entity';
 import { ExifEntity } from './exif.entity';
@@ -20,6 +21,7 @@ export * from './activity.entity';
 export * from './album.entity';
 export * from './api-key.entity';
 export * from './asset-face.entity';
+export * from './asset-job-status.entity';
 export * from './asset.entity';
 export * from './audit.entity';
 export * from './exif.entity';
@@ -40,6 +42,7 @@ export const databaseEntities = [
   APIKeyEntity,
   AssetEntity,
   AssetFaceEntity,
+  AssetJobStatusEntity,
   AuditEntity,
   ExifEntity,
   MoveEntity,

server/src/infra/migrations/1699345863886-AddJobStatus.ts

@@ -0,0 +1,16 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+
+export class AddJobStatus1699345863886 implements MigrationInterface {
+    name = 'AddJobStatus1699345863886'
+
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(`CREATE TABLE "asset_job_status" ("assetId" uuid NOT NULL, "facesRecognizedAt" TIMESTAMP WITH TIME ZONE, CONSTRAINT "PK_420bec36fc02813bddf5c8b73d4" PRIMARY KEY ("assetId"))`);
+        await queryRunner.query(`ALTER TABLE "asset_job_status" ADD CONSTRAINT "FK_420bec36fc02813bddf5c8b73d4" FOREIGN KEY ("assetId") REFERENCES "assets"("id") ON DELETE CASCADE ON UPDATE CASCADE`);
+    }
+
+    public async down(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(`ALTER TABLE "asset_job_status" DROP CONSTRAINT "FK_420bec36fc02813bddf5c8b73d4"`);
+        await queryRunner.query(`DROP TABLE "asset_job_status"`);
+    }
+
+}

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

@@ -20,7 +20,7 @@ import { Injectable } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { DateTime } from 'luxon';
 import { And, FindOptionsRelations, FindOptionsWhere, In, IsNull, LessThan, Not, Repository } from 'typeorm';
-import { AssetEntity, AssetType, ExifEntity } from '../entities';
+import { AssetEntity, AssetJobStatusEntity, AssetType, ExifEntity } from '../entities';
 import OptionalBetween from '../utils/optional-between.util';
 import { paginate } from '../utils/pagination.util';
 
@@ -39,12 +39,17 @@ export class AssetRepository implements IAssetRepository {
   constructor(
     @InjectRepository(AssetEntity) private repository: Repository<AssetEntity>,
     @InjectRepository(ExifEntity) private exifRepository: Repository<ExifEntity>,
+    @InjectRepository(AssetJobStatusEntity) private jobStatusRepository: Repository<AssetJobStatusEntity>,
   ) {}
 
   async upsertExif(exif: Partial<ExifEntity>): Promise<void> {
     await this.exifRepository.upsert(exif, { conflictPaths: ['assetId'] });
   }
 
+  async upsertJobStatus(jobStatus: Partial<AssetJobStatusEntity>): Promise<void> {
+    await this.jobStatusRepository.upsert(jobStatus, { conflictPaths: ['assetId'] });
+  }
+
   create(asset: AssetCreate): Promise<AssetEntity> {
     return this.repository.save(asset);
   }
@@ -323,6 +328,7 @@ export class AssetRepository implements IAssetRepository {
       case WithoutProperty.FACES:
         relations = {
           faces: true,
+          jobStatus: true,
         };
         where = {
           resizePath: Not(IsNull()),
@@ -331,6 +337,9 @@ export class AssetRepository implements IAssetRepository {
             assetId: IsNull(),
             personId: IsNull(),
           },
+          jobStatus: {
+            facesRecognizedAt: IsNull(),
+          },
         };
         break;
 

server/test/e2e/asset.e2e-spec.ts

@@ -6,9 +6,12 @@ import {
   LoginResponseDto,
   SharedLinkResponseDto,
   TimeBucketSize,
+  WithoutProperty,
+  usePagination,
 } from '@app/domain';
 import { AssetController } from '@app/immich';
 import { AssetEntity, AssetType, SharedLinkType } from '@app/infra/entities';
+import { AssetRepository } from '@app/infra/repositories';
 import { INestApplication } from '@nestjs/common';
 import { api } from '@test/api';
 import { errorStub, uuidStub } from '@test/fixtures';
@@ -788,4 +791,55 @@ describe(`${AssetController.name} (e2e)`, () => {
       expect(asset.stack).toEqual(expect.arrayContaining([expect.objectContaining({ id: asset3.id })]));
     });
   });
+
+  describe(AssetRepository.name, () => {
+    describe('getWithout', () => {
+      describe('WithoutProperty.FACES', () => {
+        const getAssetIdsWithoutFaces = async () => {
+          const assetPagination = usePagination(10, (pagination) =>
+            assetRepository.getWithout(pagination, WithoutProperty.FACES),
+          );
+          let assets: AssetEntity[] = [];
+          for await (const assetsPage of assetPagination) {
+            assets = [...assets, ...assetsPage];
+          }
+          return assets.map((a) => a.id);
+        };
+
+        beforeEach(async () => {
+          await assetRepository.save({ id: asset1.id, resizePath: '/path/to/resize' });
+          expect(await getAssetIdsWithoutFaces()).toContain(asset1.id);
+        });
+
+        describe('with recognized faces', () => {
+          beforeEach(async () => {
+            const personRepository = app.get<IPersonRepository>(IPersonRepository);
+            const person = await personRepository.create({ ownerId: asset1.ownerId, name: 'Test Person' });
+            await personRepository.createFace({ assetId: asset1.id, personId: person.id });
+          });
+
+          it('should not return asset with facesRecognizedAt unset', async () => {
+            expect(await getAssetIdsWithoutFaces()).not.toContain(asset1.id);
+          });
+
+          it('should not return asset with facesRecognizedAt set', async () => {
+            await assetRepository.upsertJobStatus({ assetId: asset1.id, facesRecognizedAt: new Date() });
+            expect(await getAssetIdsWithoutFaces()).not.toContain(asset1.id);
+          });
+        });
+
+        describe('without recognized faces', () => {
+          it('should return asset with facesRecognizedAt unset', async () => {
+            expect(await getAssetIdsWithoutFaces()).toContain(asset1.id);
+          });
+
+          it('should not return asset with facesRecognizedAt set', async () => {
+            expect(await getAssetIdsWithoutFaces()).toContain(asset1.id);
+            await assetRepository.upsertJobStatus({ assetId: asset1.id, facesRecognizedAt: new Date() });
+            expect(await getAssetIdsWithoutFaces()).not.toContain(asset1.id);
+          });
+        });
+      });
+    });
+  });
 });

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

@@ -4,6 +4,7 @@ export const newAssetRepositoryMock = (): jest.Mocked<IAssetRepository> => {
   return {
     create: jest.fn(),
     upsertExif: jest.fn(),
+    upsertJobStatus: jest.fn(),
     getByDate: jest.fn(),
     getByDayOfYear: jest.fn(),
     getByIds: jest.fn().mockResolvedValue([]),