diff --git a/server/src/queries/asset.repository.sql b/server/src/queries/asset.repository.sql index 82f867beb6..b124814433 100644 --- a/server/src/queries/asset.repository.sql +++ b/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 diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts index 65fff74595..ec348a5ebc 100644 --- a/server/src/repositories/asset.repository.ts +++ b/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(); + } } diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 942817a213..51fa16dfc0 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/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); diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index f74f9f4cec..d212c103ff 100644 --- a/server/src/services/metadata.service.ts +++ b/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, diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index b57a5e1072..a20341b569 100644 --- a/server/src/services/person.service.spec.ts +++ b/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), ]); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index dfbb56bd1e..f1f89054a4 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -127,10 +127,10 @@ export class PersonService extends BaseService { async getFacesById(auth: AuthDto, dto: FaceDto): Promise { 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[]) { diff --git a/server/src/services/tag.service.spec.ts b/server/src/services/tag.service.spec.ts index f42f40940d..6fc472bb87 100644 --- a/server/src/services/tag.service.spec.ts +++ b/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', { diff --git a/server/src/services/tag.service.ts b/server/src/services/tag.service.ts index 20303421c1..d34cd84ecd 100644 --- a/server/src/services/tag.service.ts +++ b/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', + }); } } diff --git a/server/test/repositories/asset.repository.mock.ts b/server/test/repositories/asset.repository.mock.ts index 2d51030ee9..4926ac1f5b 100644 --- a/server/test/repositories/asset.repository.mock.ts +++ b/server/test/repositories/asset.repository.mock.ts @@ -55,5 +55,8 @@ export const newAssetRepositoryMock = (): Mocked