refactor!: remove asset faces from AssetResponseDto (#27779)

* refactor!: remove faces from `people` in AssetResposnseDto

* chore: tests

* chore: e2e generator

* chore: code review readonly

* chore: code review changes

* chore: cleanup

* fix: openapi

* chore: format

---------

Co-authored-by: Jason Rasmussen <jason@rasm.me>

server/src/dtos/asset-response.dto.spec.ts

@@ -1,192 +0,0 @@
-import { mapAsset } from 'src/dtos/asset-response.dto';
-import { AssetEditAction } from 'src/dtos/editing.dto';
-import { AssetFaceFactory } from 'test/factories/asset-face.factory';
-import { AssetFactory } from 'test/factories/asset.factory';
-import { PersonFactory } from 'test/factories/person.factory';
-import { getForAsset } from 'test/mappers';
-
-describe('mapAsset', () => {
-  describe('peopleWithFaces', () => {
-    it('should transform all faces when a person has multiple faces in the same image', () => {
-      const person = PersonFactory.create();
-      const face1 = {
-        boundingBoxX1: 100,
-        boundingBoxY1: 100,
-        boundingBoxX2: 200,
-        boundingBoxY2: 200,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const face2 = {
-        boundingBoxX1: 300,
-        boundingBoxY1: 400,
-        boundingBoxX2: 400,
-        boundingBoxY2: 500,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const asset = AssetFactory.from()
-        .face(face1, (builder) => builder.person(person))
-        .face(face2, (builder) => builder.person(person))
-        .exif({ exifImageWidth: 1000, exifImageHeight: 800 })
-        .edit({
-          action: AssetEditAction.Crop,
-          parameters: {
-            width: 1512,
-            height: 1152,
-            x: 216,
-            y: 1512,
-          },
-        })
-        .build();
-
-      const result = mapAsset(getForAsset(asset));
-
-      expect(result.people).toBeDefined();
-      expect(result.people).toHaveLength(1);
-      expect(result.people![0].faces).toHaveLength(2);
-
-      // Verify that both faces have been transformed (bounding boxes adjusted for crop)
-      const firstFace = result.people![0].faces[0];
-      const secondFace = result.people![0].faces[1];
-
-      // After crop (x: 216, y: 1512), the coordinates should be adjusted
-      // Faces outside the crop area will be clamped
-      expect(firstFace.boundingBoxX1).toBe(-116); // 100 - 216 = -116
-      expect(firstFace.boundingBoxY1).toBe(-1412); // 100 - 1512 = -1412
-      expect(firstFace.boundingBoxX2).toBe(-16); // 200 - 216 = -16
-      expect(firstFace.boundingBoxY2).toBe(-1312); // 200 - 1512 = -1312
-
-      expect(secondFace.boundingBoxX1).toBe(84); // 300 - 216
-      expect(secondFace.boundingBoxY1).toBe(-1112); // 400 - 1512 = -1112
-      expect(secondFace.boundingBoxX2).toBe(184); // 400 - 216
-      expect(secondFace.boundingBoxY2).toBe(-1012); // 500 - 1512 = -1012
-    });
-
-    it('should transform unassigned faces with edits and dimensions', () => {
-      const unassignedFace = AssetFaceFactory.create({
-        boundingBoxX1: 100,
-        boundingBoxY1: 100,
-        boundingBoxX2: 200,
-        boundingBoxY2: 200,
-        imageWidth: 1000,
-        imageHeight: 800,
-      });
-
-      const asset = AssetFactory.from()
-        .face(unassignedFace)
-        .exif({ exifImageWidth: 1000, exifImageHeight: 800 })
-        .edit({ action: AssetEditAction.Crop, parameters: { x: 50, y: 50, width: 500, height: 400 } })
-        .build();
-
-      const result = mapAsset(getForAsset(asset));
-
-      expect(result.unassignedFaces).toBeDefined();
-      expect(result.unassignedFaces).toHaveLength(1);
-
-      // Verify that unassigned face has been transformed
-      const face = result.unassignedFaces![0];
-      expect(face.boundingBoxX1).toBe(50); // 100 - 50
-      expect(face.boundingBoxY1).toBe(50); // 100 - 50
-      expect(face.boundingBoxX2).toBe(150); // 200 - 50
-      expect(face.boundingBoxY2).toBe(150); // 200 - 50
-    });
-
-    it('should handle multiple people each with multiple faces', () => {
-      const person1Face1 = {
-        boundingBoxX1: 100,
-        boundingBoxY1: 100,
-        boundingBoxX2: 200,
-        boundingBoxY2: 200,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const person1Face2 = {
-        boundingBoxX1: 300,
-        boundingBoxY1: 300,
-        boundingBoxX2: 400,
-        boundingBoxY2: 400,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const person2Face1 = {
-        boundingBoxX1: 500,
-        boundingBoxY1: 100,
-        boundingBoxX2: 600,
-        boundingBoxY2: 200,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const person = PersonFactory.create({ id: 'person-1' });
-
-      const asset = AssetFactory.from()
-        .face(person1Face1, (builder) => builder.person(person))
-        .face(person1Face2, (builder) => builder.person(person))
-        .face(person2Face1, (builder) => builder.person({ id: 'person-2' }))
-        .exif({ exifImageWidth: 1000, exifImageHeight: 800 })
-        .build();
-
-      const result = mapAsset(getForAsset(asset));
-
-      expect(result.people).toBeDefined();
-      expect(result.people).toHaveLength(2);
-
-      const person1 = result.people!.find((p) => p.id === 'person-1');
-      const person2 = result.people!.find((p) => p.id === 'person-2');
-
-      expect(person1).toBeDefined();
-      expect(person1!.faces).toHaveLength(2);
-      // No edits, so coordinates should be unchanged
-      expect(person1!.faces[0].boundingBoxX1).toBe(100);
-      expect(person1!.faces[0].boundingBoxY1).toBe(100);
-      expect(person1!.faces[1].boundingBoxX1).toBe(300);
-      expect(person1!.faces[1].boundingBoxY1).toBe(300);
-
-      expect(person2).toBeDefined();
-      expect(person2!.faces).toHaveLength(1);
-      expect(person2!.faces[0].boundingBoxX1).toBe(500);
-      expect(person2!.faces[0].boundingBoxY1).toBe(100);
-    });
-
-    it('should combine faces of the same person into a single entry', () => {
-      const face1 = {
-        boundingBoxX1: 100,
-        boundingBoxY1: 100,
-        boundingBoxX2: 200,
-        boundingBoxY2: 200,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const face2 = {
-        boundingBoxX1: 300,
-        boundingBoxY1: 300,
-        boundingBoxX2: 400,
-        boundingBoxY2: 400,
-        imageWidth: 1000,
-        imageHeight: 800,
-      };
-
-      const person = PersonFactory.create();
-
-      const asset = AssetFactory.from()
-        .face(face1, (builder) => builder.person(person))
-        .face(face2, (builder) => builder.person(person))
-        .exif({ exifImageWidth: 1000, exifImageHeight: 800 })
-        .build();
-
-      const result = mapAsset(getForAsset(asset));
-
-      expect(result.people).toBeDefined();
-      expect(result.people).toHaveLength(1);
-
-      expect(result.people![0].id).toBe(person.id);
-      expect(result.people![0].faces).toHaveLength(2);
-    });
-  });
-});

server/src/dtos/asset-response.dto.ts

@@ -5,13 +5,7 @@ import { HistoryBuilder } from 'src/decorators';
 import { AuthDto } from 'src/dtos/auth.dto';
 import { AssetEditActionItem } from 'src/dtos/editing.dto';
 import { ExifResponseSchema, mapExif } from 'src/dtos/exif.dto';
-import {
-  AssetFaceWithoutPersonResponseSchema,
-  PersonWithFacesResponseDto,
-  PersonWithFacesResponseSchema,
-  mapFacesWithoutPerson,
-  mapPerson,
-} from 'src/dtos/person.dto';
+import { PersonResponseDto, PersonResponseSchema, mapPerson } from 'src/dtos/person.dto';
 import { TagResponseSchema, mapTag } from 'src/dtos/tag.dto';
 import { UserResponseSchema, mapUser } from 'src/dtos/user.dto';
 import {
@@ -22,8 +16,7 @@ import {
   AssetVisibilitySchema,
   ChecksumAlgorithm,
 } from 'src/enum';
-import { ImageDimensions, MaybeDehydrated } from 'src/types';
-import { getDimensions } from 'src/utils/asset.util';
+import { MaybeDehydrated } from 'src/types';
 import { hexOrBufferToBase64 } from 'src/utils/bytes';
 import { asDateString } from 'src/utils/date';
 import { mimeTypes } from 'src/utils/mime-types';
@@ -107,8 +100,7 @@ export const AssetResponseSchema = SanitizedAssetResponseSchema.extend(
     visibility: AssetVisibilitySchema,
     exifInfo: ExifResponseSchema.optional(),
     tags: z.array(TagResponseSchema).optional(),
-    people: z.array(PersonWithFacesResponseSchema).optional(),
-    unassignedFaces: z.array(AssetFaceWithoutPersonResponseSchema).optional(),
+    people: z.array(PersonResponseSchema).optional(),
     checksum: z.string().describe('Base64 encoded SHA1 hash'),
     stack: AssetStackResponseSchema.nullish(),
     duplicateId: z.string().nullish().describe('Duplicate group ID'),
@@ -170,33 +162,20 @@ export type AssetMapOptions = {
   auth?: AuthDto;
 };
 
-const peopleWithFaces = (
-  faces?: MaybeDehydrated<AssetFace>[],
-  edits?: AssetEditActionItem[],
-  assetDimensions?: ImageDimensions,
-): PersonWithFacesResponseDto[] => {
+const peopleFromFaces = (faces?: MaybeDehydrated<AssetFace>[]): PersonResponseDto[] => {
   if (!faces) {
     return [];
   }
 
-  const peopleFaces: Map<string, PersonWithFacesResponseDto> = new Map();
+  const peopleMap: Map<string, PersonResponseDto> = new Map();
 
   for (const face of faces) {
-    if (!face.person) {
-      continue;
+    if (face.person && !peopleMap.has(face.person.id)) {
+      peopleMap.set(face.person.id, mapPerson(face.person));
     }
-
-    if (!peopleFaces.has(face.person.id)) {
-      peopleFaces.set(face.person.id, {
-        ...mapPerson(face.person),
-        faces: [],
-      });
-    }
-    const mappedFace = mapFacesWithoutPerson(face, edits, assetDimensions);
-    peopleFaces.get(face.person.id)!.faces.push(mappedFace);
   }
 
-  return [...peopleFaces.values()];
+  return [...peopleMap.values()];
 };
 
 const mapStack = (entity: { stack?: Stack | null }) => {
@@ -230,8 +209,6 @@ export function mapAsset(entity: MaybeDehydrated<MapAsset>, options: AssetMapOpt
     return sanitizedAssetResponse as AssetResponseDto;
   }
 
-  const assetDimensions = entity.exifInfo ? getDimensions(entity.exifInfo) : undefined;
-
   return {
     id: entity.id,
     createdAt: asDateString(entity.createdAt),
@@ -255,10 +232,7 @@ export function mapAsset(entity: MaybeDehydrated<MapAsset>, options: AssetMapOpt
     exifInfo: entity.exifInfo ? mapExif(entity.exifInfo) : undefined,
     livePhotoVideoId: entity.livePhotoVideoId,
     tags: entity.tags?.map((tag) => mapTag(tag)),
-    people: peopleWithFaces(entity.faces, entity.edits, assetDimensions),
-    unassignedFaces: entity.faces
-      ?.filter((face) => !face.person)
-      .map((face) => mapFacesWithoutPerson(face, entity.edits, assetDimensions)),
+    people: peopleFromFaces(entity.faces),
     checksum: hexOrBufferToBase64(entity.checksum)!,
     stack: withStack ? mapStack(entity) : undefined,
     isOffline: entity.isOffline,

server/src/dtos/person.dto.ts

@@ -56,7 +56,7 @@ const PersonSearchSchema = z
   })
   .meta({ id: 'PersonSearchDto' });
 
-const PersonResponseSchema = z
+export const PersonResponseSchema = z
   .object({
     id: z.string().describe('Person ID'),
     name: z.string().describe('Person name'),
@@ -91,7 +91,7 @@ export class MergePersonDto extends createZodDto(MergePersonSchema) {}
 export class PersonSearchDto extends createZodDto(PersonSearchSchema) {}
 export class PersonResponseDto extends createZodDto(PersonResponseSchema) {}
 
-export const AssetFaceWithoutPersonResponseSchema = z
+export const AssetFaceResponseSchema = z
   .object({
     id: z.uuidv4().describe('Face ID'),
     imageHeight: z.int().min(0).describe('Image height in pixels'),
@@ -101,21 +101,10 @@ export const AssetFaceWithoutPersonResponseSchema = z
     boundingBoxY1: z.int().describe('Bounding box Y1 coordinate'),
     boundingBoxY2: z.int().describe('Bounding box Y2 coordinate'),
     sourceType: SourceTypeSchema.optional(),
+    person: PersonResponseSchema.nullable(),
   })
-  .describe('Asset face without person')
-  .meta({ id: 'AssetFaceWithoutPersonResponseDto' });
-
-class AssetFaceWithoutPersonResponseDto extends createZodDto(AssetFaceWithoutPersonResponseSchema) {}
-
-export const PersonWithFacesResponseSchema = PersonResponseSchema.extend({
-  faces: z.array(AssetFaceWithoutPersonResponseSchema),
-}).meta({ id: 'PersonWithFacesResponseDto' });
-
-export class PersonWithFacesResponseDto extends createZodDto(PersonWithFacesResponseSchema) {}
-
-const AssetFaceResponseSchema = AssetFaceWithoutPersonResponseSchema.extend({
-  person: PersonResponseSchema.nullable(),
-}).meta({ id: 'AssetFaceResponseDto' });
+  .describe('Asset face with person')
+  .meta({ id: 'AssetFaceResponseDto' });
 
 export class AssetFaceResponseDto extends createZodDto(AssetFaceResponseSchema) {}
 
@@ -193,11 +182,11 @@ export function mapPerson(person: MaybeDehydrated<Person>): PersonResponseDto {
   };
 }
 
-export function mapFacesWithoutPerson(
+function mapFacesWithoutPerson(
   face: MaybeDehydrated<Selectable<AssetFaceTable>>,
   edits?: AssetEditActionItem[],
   assetDimensions?: ImageDimensions,
-): AssetFaceWithoutPersonResponseDto {
+) {
   return {
     id: face.id,
     ...transformFaceBoundingBox(