chore: refactor more queries

server/src/queries/asset.repository.sql

@@ -663,3 +663,57 @@ from
   inner join "asset_exif" on "asset_exif"."assetId" = "asset"."id"
 where
   "asset"."id" = $1
+
+-- AssetRepository.getForMetadataExtractionTags
+select
+  "asset_exif"."tags"
+from
+  "asset_exif"
+where
+  "asset_exif"."assetId" = $1
+
+-- AssetRepository.getForFaces
+select
+  "asset_exif"."exifImageHeight",
+  "asset_exif"."exifImageWidth",
+  "asset_exif"."orientation",
+  (
+    select
+      coalesce(json_agg(agg), '[]')
+    from
+      (
+        select
+          "asset_edit"."action",
+          "asset_edit"."parameters"
+        from
+          "asset_edit"
+        where
+          "asset_edit"."assetId" = "asset"."id"
+      ) as agg
+  ) as "edits"
+from
+  "asset"
+  inner join "asset_exif" on "asset_exif"."assetId" = "asset"."id"
+where
+  "asset"."id" = $1
+
+-- AssetRepository.getForUpdateTags
+select
+  (
+    select
+      coalesce(json_agg(agg), '[]')
+    from
+      (
+        select
+          "tag"."value"
+        from
+          "tag"
+          inner join "tag_asset" on "tag"."id" = "tag_asset"."tagId"
+        where
+          "asset"."id" = "tag_asset"."assetId"
+      ) as agg
+  ) as "tags"
+from
+  "asset"
+where
+  "asset"."id" = $1

server/src/repositories/asset.repository.ts

@@ -1,5 +1,6 @@
 import { Injectable } from '@nestjs/common';
 import { ExpressionBuilder, Insertable, Kysely, NotNull, Selectable, sql, Updateable, UpdateResult } from 'kysely';
+import { jsonArrayFrom } from 'kysely/helpers/postgres';
 import { isEmpty, isUndefined, omitBy } from 'lodash';
 import { InjectKysely } from 'nestjs-kysely';
 import { LockableProperty, Stack } from 'src/database';
@@ -1080,4 +1081,41 @@ export class AssetRepository {
       ])
       .executeTakeFirst();
   }
+
+  @GenerateSql({ params: [DummyValue.UUID] })
+  async getForMetadataExtractionTags(id: string) {
+    return this.db
+      .selectFrom('asset_exif')
+      .select('asset_exif.tags')
+      .where('asset_exif.assetId', '=', id)
+      .executeTakeFirst();
+  }
+
+  @GenerateSql({ params: [DummyValue.UUID] })
+  async getForFaces(id: string) {
+    return this.db
+      .selectFrom('asset')
+      .innerJoin('asset_exif', (join) => join.onRef('asset_exif.assetId', '=', 'asset.id'))
+      .select(['asset_exif.exifImageHeight', 'asset_exif.exifImageWidth', 'asset_exif.orientation'])
+      .select(withEdits)
+      .where('asset.id', '=', id)
+      .executeTakeFirstOrThrow();
+  }
+
+  @GenerateSql({ params: [DummyValue.UUID] })
+  async getForUpdateTags(id: string) {
+    return this.db
+      .selectFrom('asset')
+      .select((eb) =>
+        jsonArrayFrom(
+          eb
+            .selectFrom('tag')
+            .select('tag.value')
+            .innerJoin('tag_asset', 'tag.id', 'tag_asset.tagId')
+            .whereRef('asset.id', '=', 'tag_asset.assetId'),
+        ).as('tags'),
+      )
+      .where('asset.id', '=', id)
+      .executeTakeFirstOrThrow();
+  }
 }

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

@@ -387,7 +387,7 @@ describe(MetadataService.name, () => {
 
     it('should extract tags from TagsList', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({ ...factory.asset(), exifInfo: factory.exif({ tags: ['Parent'] }) });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent'] });
       mockReadTags({ TagsList: ['Parent'] });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -398,7 +398,7 @@ describe(MetadataService.name, () => {
 
     it('should extract hierarchy from TagsList', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({ ...factory.asset(), exifInfo: factory.exif({ tags: ['Parent/Child'] }) });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent/Child'] });
       mockReadTags({ TagsList: ['Parent/Child'] });
       mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert);
       mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert);
@@ -419,7 +419,7 @@ describe(MetadataService.name, () => {
 
     it('should extract tags from Keywords as a string', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({ ...factory.asset(), exifInfo: factory.exif({ tags: ['Parent'] }) });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent'] });
       mockReadTags({ Keywords: 'Parent' });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -430,7 +430,7 @@ describe(MetadataService.name, () => {
 
     it('should extract tags from Keywords as a list', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({ ...factory.asset(), exifInfo: factory.exif({ tags: ['Parent'] }) });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent'] });
       mockReadTags({ Keywords: ['Parent'] });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -441,10 +441,7 @@ describe(MetadataService.name, () => {
 
     it('should extract tags from Keywords as a list with a number', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        exifInfo: factory.exif({ tags: ['Parent', '2024'] }),
-      });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent', '2024'] });
       mockReadTags({ Keywords: ['Parent', 2024] });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -456,7 +453,7 @@ describe(MetadataService.name, () => {
 
     it('should extract hierarchal tags from Keywords', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({ ...factory.asset(), exifInfo: factory.exif({ tags: ['Parent/Child'] }) });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent/Child'] });
       mockReadTags({ Keywords: 'Parent/Child' });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -476,10 +473,7 @@ describe(MetadataService.name, () => {
 
     it('should ignore Keywords when TagsList is present', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        exifInfo: factory.exif({ tags: ['Parent/Child', 'Child'] }),
-      });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent/Child', 'Child'] });
       mockReadTags({ Keywords: 'Child', TagsList: ['Parent/Child'] });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -499,10 +493,7 @@ describe(MetadataService.name, () => {
 
     it('should extract hierarchy from HierarchicalSubject', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        exifInfo: factory.exif({ tags: ['Parent/Child', 'TagA'] }),
-      });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent/Child', 'TagA'] });
       mockReadTags({ HierarchicalSubject: ['Parent|Child', 'TagA'] });
       mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert);
       mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert);
@@ -524,10 +515,7 @@ describe(MetadataService.name, () => {
 
     it('should extract tags from HierarchicalSubject as a list with a number', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image));
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        exifInfo: factory.exif({ tags: ['Parent', '2024'] }),
-      });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent', '2024'] });
       mockReadTags({ HierarchicalSubject: ['Parent', 2024] });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 
@@ -539,7 +527,7 @@ describe(MetadataService.name, () => {
 
     it('should extract ignore / characters in a HierarchicalSubject tag', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image);
-      mocks.asset.getById.mockResolvedValue({ ...factory.asset(), exifInfo: factory.exif({ tags: ['Mom|Dad'] }) });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Mom|Dad'] });
       mockReadTags({ HierarchicalSubject: ['Mom/Dad'] });
       mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert);
 
@@ -554,10 +542,7 @@ describe(MetadataService.name, () => {
 
     it('should ignore HierarchicalSubject when TagsList is present', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image);
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        exifInfo: factory.exif({ tags: ['Parent/Child', 'Parent2/Child2'] }),
-      });
+      mocks.asset.getForMetadataExtractionTags.mockResolvedValue({ tags: ['Parent/Child', 'Parent2/Child2'] });
       mockReadTags({ HierarchicalSubject: ['Parent2|Child2'], TagsList: ['Parent/Child'] });
       mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert);
 

server/src/services/metadata.service.ts

@@ -566,10 +566,10 @@ export class MetadataService extends BaseService {
   }
 
   private async applyTagList({ id, ownerId }: { id: string; ownerId: string }) {
-    const asset = await this.assetRepository.getById(id, { exifInfo: true });
+    const asset = await this.assetRepository.getForMetadataExtractionTags(id);
     const results = await upsertTags(this.tagRepository, {
       userId: ownerId,
-      tags: asset?.exifInfo?.tags ?? [],
+      tags: asset?.tags ?? [],
     });
     await this.tagRepository.replaceAssetTags(
       id,

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

@@ -354,7 +354,7 @@ describe(PersonService.name, () => {
     it('should get the bounding boxes for an asset', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([faceStub.face1.assetId]));
       mocks.person.getFaces.mockResolvedValue([faceStub.primaryFace1]);
-      mocks.asset.getById.mockResolvedValue(assetStub.image);
+      mocks.asset.getForFaces.mockResolvedValue({ edits: [], ...factory.exif() });
       await expect(sut.getFacesById(authStub.admin, { id: faceStub.face1.assetId })).resolves.toStrictEqual([
         mapFaces(faceStub.primaryFace1, authStub.admin),
       ]);

server/src/services/person.service.ts

@@ -127,10 +127,10 @@ export class PersonService extends BaseService {
   async getFacesById(auth: AuthDto, dto: FaceDto): Promise<AssetFaceResponseDto[]> {
     await this.requireAccess({ auth, permission: Permission.AssetRead, ids: [dto.id] });
     const faces = await this.personRepository.getFaces(dto.id);
-    const asset = await this.assetRepository.getById(dto.id, { edits: true, exifInfo: true });
-    const assetDimensions = getDimensions(asset!.exifInfo!);
+    const asset = await this.assetRepository.getForFaces(dto.id);
+    const assetDimensions = getDimensions(asset);
 
-    return faces.map((face) => mapFaces(face, auth, asset!.edits!, assetDimensions));
+    return faces.map((face) => mapFaces(face, auth, asset.edits, assetDimensions));
   }
 
   async createNewFeaturePhoto(changeFeaturePhoto: string[]) {

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

@@ -4,7 +4,6 @@ import { JobStatus } from 'src/enum';
 import { TagService } from 'src/services/tag.service';
 import { authStub } from 'test/fixtures/auth.stub';
 import { tagResponseStub, tagStub } from 'test/fixtures/tag.stub';
-import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 describe(TagService.name, () => {
@@ -192,10 +191,7 @@ describe(TagService.name, () => {
     it('should upsert records', async () => {
       mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-1', 'tag-2']));
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1', 'asset-2', 'asset-3']));
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        tags: [factory.tag({ value: 'tag-1' }), factory.tag({ value: 'tag-2' })],
-      });
+      mocks.asset.getForUpdateTags.mockResolvedValue({ tags: [{ value: 'tag-1' }, { value: 'tag-2' }] });
       mocks.tag.upsertAssetIds.mockResolvedValue([
         { tagId: 'tag-1', assetId: 'asset-1' },
         { tagId: 'tag-1', assetId: 'asset-2' },
@@ -246,10 +242,7 @@ describe(TagService.name, () => {
       mocks.tag.get.mockResolvedValue(tagStub.tag);
       mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1']));
       mocks.tag.addAssetIds.mockResolvedValue();
-      mocks.asset.getById.mockResolvedValue({
-        ...factory.asset(),
-        tags: [factory.tag({ value: 'tag-1' })],
-      });
+      mocks.asset.getForUpdateTags.mockResolvedValue({ tags: [{ value: 'tag-1' }] });
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-2']));
 
       await expect(
@@ -278,6 +271,7 @@ describe(TagService.name, () => {
     it('should throw an error for an invalid id', async () => {
       mocks.tag.getAssetIds.mockResolvedValue(new Set());
       mocks.tag.removeAssetIds.mockResolvedValue();
+      mocks.asset.getForUpdateTags.mockResolvedValue({ tags: [] });
 
       await expect(sut.removeAssets(authStub.admin, 'tag-1', { ids: ['asset-1'] })).resolves.toEqual([
         { id: 'asset-1', success: false, error: 'not_found' },
@@ -288,6 +282,7 @@ describe(TagService.name, () => {
       mocks.tag.get.mockResolvedValue(tagStub.tag);
       mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1']));
       mocks.tag.removeAssetIds.mockResolvedValue();
+      mocks.asset.getForUpdateTags.mockResolvedValue({ tags: [] });
 
       await expect(
         sut.removeAssets(authStub.admin, 'tag-1', {

server/src/services/tag.service.ts

@@ -151,10 +151,9 @@ export class TagService extends BaseService {
   }
 
   private async updateTags(assetId: string) {
-    const asset = await this.assetRepository.getById(assetId, { tags: true });
-    await this.assetRepository.upsertExif(
-      updateLockedColumns({ assetId, tags: asset?.tags?.map(({ value }) => value) ?? [] }),
-      { lockedPropertiesBehavior: 'append' },
-    );
+    const { tags } = await this.assetRepository.getForUpdateTags(assetId);
+    await this.assetRepository.upsertExif(updateLockedColumns({ assetId, tags: tags.map(({ value }) => value) }), {
+      lockedPropertiesBehavior: 'append',
+    });
   }
 }

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

@@ -55,5 +55,8 @@ export const newAssetRepositoryMock = (): Mocked<RepositoryInterface<AssetReposi
     getForVideo: vitest.fn(),
     getForEdit: vitest.fn(),
     getForOcr: vitest.fn(),
+    getForMetadataExtractionTags: vitest.fn(),
+    getForFaces: vitest.fn(),
+    getForUpdateTags: vitest.fn(),
   };
 };