fix(server): person repo methods (#12524)

server/src/cores/storage.core.ts

@@ -301,7 +301,7 @@ export class StorageCore {
         return this.assetRepository.update({ id, sidecarPath: newPath });
       }
       case PersonPathType.FACE: {
-        return this.personRepository.update([{ id, thumbnailPath: newPath }]);
+        return this.personRepository.update({ id, thumbnailPath: newPath });
       }
     }
   }

server/src/interfaces/person.interface.ts

@@ -54,7 +54,8 @@ export interface IPersonRepository {
 
   getAssets(personId: string): Promise<AssetEntity[]>;
 
-  create(entities: Partial<PersonEntity>[]): Promise<PersonEntity[]>;
+  create(person: Partial<PersonEntity>): Promise<PersonEntity>;
+  createAll(people: Partial<PersonEntity>[]): Promise<string[]>;
   createFaces(entities: Partial<AssetFaceEntity>[]): Promise<string[]>;
   delete(entities: PersonEntity[]): Promise<void>;
   deleteAll(): Promise<void>;
@@ -74,6 +75,7 @@ export interface IPersonRepository {
   reassignFace(assetFaceId: string, newPersonId: string): Promise<number>;
   getNumberOfPeople(userId: string): Promise<PeopleStatistics>;
   reassignFaces(data: UpdateFacesData): Promise<number>;
-  update(entities: Partial<PersonEntity>[]): Promise<PersonEntity[]>;
+  update(person: Partial<PersonEntity>): Promise<PersonEntity>;
+  updateAll(people: Partial<PersonEntity>[]): Promise<void>;
   getLatestFaceDate(): Promise<string | undefined>;
 }

server/src/repositories/person.repository.ts

@@ -280,8 +280,13 @@ export class PersonRepository implements IPersonRepository {
     return result;
   }
 
-  create(entities: Partial<PersonEntity>[]): Promise<PersonEntity[]> {
-    return this.personRepository.save(entities);
+  create(person: Partial<PersonEntity>): Promise<PersonEntity> {
+    return this.save(person);
+  }
+
+  async createAll(people: Partial<PersonEntity>[]): Promise<string[]> {
+    const results = await this.personRepository.save(people);
+    return results.map((person) => person.id);
   }
 
   async createFaces(entities: AssetFaceEntity[]): Promise<string[]> {
@@ -297,8 +302,12 @@ export class PersonRepository implements IPersonRepository {
     });
   }
 
-  async update(entities: Partial<PersonEntity>[]): Promise<PersonEntity[]> {
-    return await this.personRepository.save(entities);
+  async update(person: Partial<PersonEntity>): Promise<PersonEntity> {
+    return this.save(person);
+  }
+
+  async updateAll(people: Partial<PersonEntity>[]): Promise<void> {
+    await this.personRepository.save(people);
   }
 
   @GenerateSql({ params: [[{ assetId: DummyValue.UUID, personId: DummyValue.UUID }]] })
@@ -320,4 +329,9 @@ export class PersonRepository implements IPersonRepository {
       .getRawOne();
     return result?.latestDate;
   }
+
+  private async save(person: Partial<PersonEntity>): Promise<PersonEntity> {
+    const { id } = await this.personRepository.save(person);
+    return this.personRepository.findOneByOrFail({ id });
+  }
 }

server/src/services/audit.service.ts

@@ -115,7 +115,7 @@ export class AuditService {
         }
 
         case PersonPathType.FACE: {
-          await this.personRepository.update([{ id, thumbnailPath: pathValue }]);
+          await this.personRepository.update({ id, thumbnailPath: pathValue });
           break;
         }
 

server/src/services/media.service.ts

@@ -117,7 +117,7 @@ export class MediaService {
             continue;
           }
 
-          await this.personRepository.update([{ id: person.id, faceAssetId: face.id }]);
+          await this.personRepository.update({ id: person.id, faceAssetId: face.id });
         }
 
         jobs.push({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id: person.id } });

server/src/services/metadata.service.spec.ts

@@ -1002,13 +1002,12 @@ describe(MetadataService.name, () => {
       systemMock.get.mockResolvedValue({ metadata: { faces: { import: true } } });
       metadataMock.readTags.mockResolvedValue(metadataStub.withFaceNoName);
       personMock.getDistinctNames.mockResolvedValue([]);
-      personMock.create.mockResolvedValue([]);
+      personMock.createAll.mockResolvedValue([]);
       personMock.replaceFaces.mockResolvedValue([]);
-      personMock.update.mockResolvedValue([]);
       await sut.handleMetadataExtraction({ id: assetStub.image.id });
-      expect(personMock.create).toHaveBeenCalledWith([]);
+      expect(personMock.createAll).toHaveBeenCalledWith([]);
       expect(personMock.replaceFaces).toHaveBeenCalledWith(assetStub.primaryImage.id, [], SourceType.EXIF);
-      expect(personMock.update).toHaveBeenCalledWith([]);
+      expect(personMock.updateAll).toHaveBeenCalledWith([]);
     });
 
     it('should skip importing faces with empty name', async () => {
@@ -1016,13 +1015,12 @@ describe(MetadataService.name, () => {
       systemMock.get.mockResolvedValue({ metadata: { faces: { import: true } } });
       metadataMock.readTags.mockResolvedValue(metadataStub.withFaceEmptyName);
       personMock.getDistinctNames.mockResolvedValue([]);
-      personMock.create.mockResolvedValue([]);
+      personMock.createAll.mockResolvedValue([]);
       personMock.replaceFaces.mockResolvedValue([]);
-      personMock.update.mockResolvedValue([]);
       await sut.handleMetadataExtraction({ id: assetStub.image.id });
-      expect(personMock.create).toHaveBeenCalledWith([]);
+      expect(personMock.createAll).toHaveBeenCalledWith([]);
       expect(personMock.replaceFaces).toHaveBeenCalledWith(assetStub.primaryImage.id, [], SourceType.EXIF);
-      expect(personMock.update).toHaveBeenCalledWith([]);
+      expect(personMock.updateAll).toHaveBeenCalledWith([]);
     });
 
     it('should apply metadata face tags creating new persons', async () => {
@@ -1030,13 +1028,13 @@ describe(MetadataService.name, () => {
       systemMock.get.mockResolvedValue({ metadata: { faces: { import: true } } });
       metadataMock.readTags.mockResolvedValue(metadataStub.withFace);
       personMock.getDistinctNames.mockResolvedValue([]);
-      personMock.create.mockResolvedValue([personStub.withName]);
+      personMock.createAll.mockResolvedValue([personStub.withName.id]);
       personMock.replaceFaces.mockResolvedValue(['face-asset-uuid']);
-      personMock.update.mockResolvedValue([personStub.withName]);
+      personMock.update.mockResolvedValue(personStub.withName);
       await sut.handleMetadataExtraction({ id: assetStub.primaryImage.id });
       expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.primaryImage.id]);
       expect(personMock.getDistinctNames).toHaveBeenCalledWith(assetStub.primaryImage.ownerId, { withHidden: true });
-      expect(personMock.create).toHaveBeenCalledWith([expect.objectContaining({ name: personStub.withName.name })]);
+      expect(personMock.createAll).toHaveBeenCalledWith([expect.objectContaining({ name: personStub.withName.name })]);
       expect(personMock.replaceFaces).toHaveBeenCalledWith(
         assetStub.primaryImage.id,
         [
@@ -1055,7 +1053,7 @@ describe(MetadataService.name, () => {
         ],
         SourceType.EXIF,
       );
-      expect(personMock.update).toHaveBeenCalledWith([{ id: 'random-uuid', faceAssetId: 'random-uuid' }]);
+      expect(personMock.updateAll).toHaveBeenCalledWith([{ id: 'random-uuid', faceAssetId: 'random-uuid' }]);
       expect(jobMock.queueAll).toHaveBeenCalledWith([
         {
           name: JobName.GENERATE_PERSON_THUMBNAIL,
@@ -1069,13 +1067,13 @@ describe(MetadataService.name, () => {
       systemMock.get.mockResolvedValue({ metadata: { faces: { import: true } } });
       metadataMock.readTags.mockResolvedValue(metadataStub.withFace);
       personMock.getDistinctNames.mockResolvedValue([{ id: personStub.withName.id, name: personStub.withName.name }]);
-      personMock.create.mockResolvedValue([]);
+      personMock.createAll.mockResolvedValue([]);
       personMock.replaceFaces.mockResolvedValue(['face-asset-uuid']);
-      personMock.update.mockResolvedValue([personStub.withName]);
+      personMock.update.mockResolvedValue(personStub.withName);
       await sut.handleMetadataExtraction({ id: assetStub.primaryImage.id });
       expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.primaryImage.id]);
       expect(personMock.getDistinctNames).toHaveBeenCalledWith(assetStub.primaryImage.ownerId, { withHidden: true });
-      expect(personMock.create).toHaveBeenCalledWith([]);
+      expect(personMock.createAll).toHaveBeenCalledWith([]);
       expect(personMock.replaceFaces).toHaveBeenCalledWith(
         assetStub.primaryImage.id,
         [
@@ -1094,7 +1092,7 @@ describe(MetadataService.name, () => {
         ],
         SourceType.EXIF,
       );
-      expect(personMock.update).toHaveBeenCalledWith([]);
+      expect(personMock.updateAll).toHaveBeenCalledWith([]);
       expect(jobMock.queueAll).toHaveBeenCalledWith([]);
     });
   });

server/src/services/metadata.service.ts

@@ -584,18 +584,15 @@ export class MetadataService {
       this.logger.debug(`Creating missing persons: ${missing.map((p) => `${p.name}/${p.id}`)}`);
     }
 
-    const newPersons = await this.personRepository.create(missing);
+    const newPersonIds = await this.personRepository.createAll(missing);
 
     const faceIds = await this.personRepository.replaceFaces(asset.id, discoveredFaces, SourceType.EXIF);
     this.logger.debug(`Created ${faceIds.length} faces for asset ${asset.id}`);
 
-    await this.personRepository.update(missingWithFaceAsset);
+    await this.personRepository.updateAll(missingWithFaceAsset);
 
     await this.jobRepository.queueAll(
-      newPersons.map((person) => ({
-        name: JobName.GENERATE_PERSON_THUMBNAIL,
-        data: { id: person.id },
-      })),
+      newPersonIds.map((id) => ({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id } })),
     );
   }
 

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

@@ -241,18 +241,18 @@ describe(PersonService.name, () => {
     });
 
     it("should update a person's name", async () => {
-      personMock.update.mockResolvedValue([personStub.withName]);
+      personMock.update.mockResolvedValue(personStub.withName);
       personMock.getAssets.mockResolvedValue([assetStub.image]);
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
 
       await expect(sut.update(authStub.admin, 'person-1', { name: 'Person 1' })).resolves.toEqual(responseDto);
 
-      expect(personMock.update).toHaveBeenCalledWith([{ id: 'person-1', name: 'Person 1' }]);
+      expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', name: 'Person 1' });
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });
 
     it("should update a person's date of birth", async () => {
-      personMock.update.mockResolvedValue([personStub.withBirthDate]);
+      personMock.update.mockResolvedValue(personStub.withBirthDate);
       personMock.getAssets.mockResolvedValue([assetStub.image]);
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
 
@@ -264,25 +264,25 @@ describe(PersonService.name, () => {
         isHidden: false,
         updatedAt: expect.any(Date),
       });
-      expect(personMock.update).toHaveBeenCalledWith([{ id: 'person-1', birthDate: '1976-06-30' }]);
+      expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: '1976-06-30' });
       expect(jobMock.queue).not.toHaveBeenCalled();
       expect(jobMock.queueAll).not.toHaveBeenCalled();
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });
 
     it('should update a person visibility', async () => {
-      personMock.update.mockResolvedValue([personStub.withName]);
+      personMock.update.mockResolvedValue(personStub.withName);
       personMock.getAssets.mockResolvedValue([assetStub.image]);
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
 
       await expect(sut.update(authStub.admin, 'person-1', { isHidden: false })).resolves.toEqual(responseDto);
 
-      expect(personMock.update).toHaveBeenCalledWith([{ id: 'person-1', isHidden: false }]);
+      expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', isHidden: false });
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });
 
     it("should update a person's thumbnailPath", async () => {
-      personMock.update.mockResolvedValue([personStub.withName]);
+      personMock.update.mockResolvedValue(personStub.withName);
       personMock.getFacesByIds.mockResolvedValue([faceStub.face1]);
       accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id]));
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
@@ -291,7 +291,7 @@ describe(PersonService.name, () => {
         sut.update(authStub.admin, 'person-1', { featureFaceAssetId: faceStub.face1.assetId }),
       ).resolves.toEqual(responseDto);
 
-      expect(personMock.update).toHaveBeenCalledWith([{ id: 'person-1', faceAssetId: faceStub.face1.id }]);
+      expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', faceAssetId: faceStub.face1.id });
       expect(personMock.getFacesByIds).toHaveBeenCalledWith([
         {
           assetId: faceStub.face1.assetId,
@@ -441,11 +441,11 @@ describe(PersonService.name, () => {
 
   describe('createPerson', () => {
     it('should create a new person', async () => {
-      personMock.create.mockResolvedValue([personStub.primaryPerson]);
+      personMock.create.mockResolvedValue(personStub.primaryPerson);
 
       await expect(sut.create(authStub.admin, {})).resolves.toBe(personStub.primaryPerson);
 
-      expect(personMock.create).toHaveBeenCalledWith([{ ownerId: authStub.admin.user.id }]);
+      expect(personMock.create).toHaveBeenCalledWith({ ownerId: authStub.admin.user.id });
     });
   });
 
@@ -819,7 +819,7 @@ describe(PersonService.name, () => {
       systemMock.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 1 } } });
       searchMock.searchFaces.mockResolvedValue(faces);
       personMock.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1);
-      personMock.create.mockResolvedValue([faceStub.primaryFace1.person]);
+      personMock.create.mockResolvedValue(faceStub.primaryFace1.person);
 
       await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id });
 
@@ -844,16 +844,14 @@ describe(PersonService.name, () => {
       systemMock.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 1 } } });
       searchMock.searchFaces.mockResolvedValue(faces);
       personMock.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1);
-      personMock.create.mockResolvedValue([personStub.withName]);
+      personMock.create.mockResolvedValue(personStub.withName);
 
       await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id });
 
-      expect(personMock.create).toHaveBeenCalledWith([
-        {
-          ownerId: faceStub.noPerson1.asset.ownerId,
-          faceAssetId: faceStub.noPerson1.id,
-        },
-      ]);
+      expect(personMock.create).toHaveBeenCalledWith({
+        ownerId: faceStub.noPerson1.asset.ownerId,
+        faceAssetId: faceStub.noPerson1.id,
+      });
       expect(personMock.reassignFaces).toHaveBeenCalledWith({
         faceIds: [faceStub.noPerson1.id],
         newPersonId: personStub.withName.id,
@@ -865,7 +863,7 @@ describe(PersonService.name, () => {
 
       searchMock.searchFaces.mockResolvedValue(faces);
       personMock.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1);
-      personMock.create.mockResolvedValue([personStub.withName]);
+      personMock.create.mockResolvedValue(personStub.withName);
 
       await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id });
 
@@ -884,7 +882,7 @@ describe(PersonService.name, () => {
       systemMock.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 3 } } });
       searchMock.searchFaces.mockResolvedValue(faces);
       personMock.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1);
-      personMock.create.mockResolvedValue([personStub.withName]);
+      personMock.create.mockResolvedValue(personStub.withName);
 
       await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id });
 
@@ -906,7 +904,7 @@ describe(PersonService.name, () => {
       systemMock.get.mockResolvedValue({ machineLearning: { facialRecognition: { minFaces: 3 } } });
       searchMock.searchFaces.mockResolvedValueOnce(faces).mockResolvedValueOnce([]);
       personMock.getFaceByIdWithAssets.mockResolvedValue(faceStub.noPerson1);
-      personMock.create.mockResolvedValue([personStub.withName]);
+      personMock.create.mockResolvedValue(personStub.withName);
 
       await sut.handleRecognizeFaces({ id: faceStub.noPerson1.id, deferred: true });
 
@@ -979,12 +977,10 @@ describe(PersonService.name, () => {
           processInvalidImages: false,
         },
       );
-      expect(personMock.update).toHaveBeenCalledWith([
-        {
-          id: 'person-1',
-          thumbnailPath: 'upload/thumbs/admin_id/pe/rs/person-1.jpeg',
-        },
-      ]);
+      expect(personMock.update).toHaveBeenCalledWith({
+        id: 'person-1',
+        thumbnailPath: 'upload/thumbs/admin_id/pe/rs/person-1.jpeg',
+      });
     });
 
     it('should generate a thumbnail without going negative', async () => {
@@ -1103,7 +1099,7 @@ describe(PersonService.name, () => {
     it('should merge two people with smart merge', async () => {
       personMock.getById.mockResolvedValueOnce(personStub.randomPerson);
       personMock.getById.mockResolvedValueOnce(personStub.primaryPerson);
-      personMock.update.mockResolvedValue([{ ...personStub.randomPerson, name: personStub.primaryPerson.name }]);
+      personMock.update.mockResolvedValue({ ...personStub.randomPerson, name: personStub.primaryPerson.name });
       accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-3']));
       accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-1']));
 
@@ -1116,12 +1112,10 @@ describe(PersonService.name, () => {
         oldPersonId: personStub.primaryPerson.id,
       });
 
-      expect(personMock.update).toHaveBeenCalledWith([
-        {
-          id: personStub.randomPerson.id,
-          name: personStub.primaryPerson.name,
-        },
-      ]);
+      expect(personMock.update).toHaveBeenCalledWith({
+        id: personStub.randomPerson.id,
+        name: personStub.primaryPerson.name,
+      });
 
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });

server/src/services/person.service.ts

@@ -173,7 +173,7 @@ export class PersonService {
       const assetFace = await this.repository.getRandomFace(personId);
 
       if (assetFace !== null) {
-        await this.repository.update([{ id: personId, faceAssetId: assetFace.id }]);
+        await this.repository.update({ id: personId, faceAssetId: assetFace.id });
         jobs.push({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id: personId } });
       }
     }
@@ -211,16 +211,13 @@ export class PersonService {
     return assets.map((asset) => mapAsset(asset));
   }
 
-  async create(auth: AuthDto, dto: PersonCreateDto): Promise<PersonResponseDto> {
-    const [created] = await this.repository.create([
-      {
-        ownerId: auth.user.id,
-        name: dto.name,
-        birthDate: dto.birthDate,
-        isHidden: dto.isHidden,
-      },
-    ]);
-    return created;
+  create(auth: AuthDto, dto: PersonCreateDto): Promise<PersonResponseDto> {
+    return this.repository.create({
+      ownerId: auth.user.id,
+      name: dto.name,
+      birthDate: dto.birthDate,
+      isHidden: dto.isHidden,
+    });
   }
 
   async update(auth: AuthDto, id: string, dto: PersonUpdateDto): Promise<PersonResponseDto> {
@@ -239,7 +236,7 @@ export class PersonService {
       faceId = face.id;
     }
 
-    const [person] = await this.repository.update([{ id, faceAssetId: faceId, name, birthDate, isHidden }]);
+    const person = await this.repository.update({ id, faceAssetId: faceId, name, birthDate, isHidden });
 
     if (assetId) {
       await this.jobRepository.queue({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id } });
@@ -501,7 +498,7 @@ export class PersonService {
 
     if (isCore && !personId) {
       this.logger.log(`Creating new person for face ${id}`);
-      const [newPerson] = await this.repository.create([{ ownerId: face.asset.ownerId, faceAssetId: face.id }]);
+      const newPerson = await this.repository.create({ ownerId: face.asset.ownerId, faceAssetId: face.id });
       await this.jobRepository.queue({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id: newPerson.id } });
       personId = newPerson.id;
     }
@@ -577,7 +574,7 @@ export class PersonService {
     } as const;
 
     await this.mediaRepository.generateThumbnail(inputPath, thumbnailPath, thumbnailOptions);
-    await this.repository.update([{ id: person.id, thumbnailPath }]);
+    await this.repository.update({ id: person.id, thumbnailPath });
 
     return JobStatus.SUCCESS;
   }
@@ -624,7 +621,7 @@ export class PersonService {
         }
 
         if (Object.keys(update).length > 0) {
-          [primaryPerson] = await this.repository.update([{ id: primaryPerson.id, ...update }]);
+          primaryPerson = await this.repository.update({ id: primaryPerson.id, ...update });
         }
 
         const mergeName = mergePerson.name || mergePerson.id;

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

@@ -13,9 +13,11 @@ export const newPersonRepositoryMock = (): Mocked<IPersonRepository> => {
     getDistinctNames: vitest.fn(),
 
     create: vitest.fn(),
+    createAll: vitest.fn(),
     update: vitest.fn(),
-    deleteAll: vitest.fn(),
+    updateAll: vitest.fn(),
     delete: vitest.fn(),
+    deleteAll: vitest.fn(),
     deleteAllFaces: vitest.fn(),
 
     getStatistics: vitest.fn(),