diff --git a/server/src/queries/tag.repository.sql b/server/src/queries/tag.repository.sql index 0670b5ab1..9314738a7 100644 --- a/server/src/queries/tag.repository.sql +++ b/server/src/queries/tag.repository.sql @@ -27,6 +27,15 @@ where "userId" = $1 and "value" = $2 +-- TagRepository.getAssetIdsByTagId +select distinct + "tag_asset"."assetId" +from + "tag_closure" + inner join "tag_asset" on "tag_asset"."tagId" = "tag_closure"."id_descendant" +where + "tag_closure"."id_ancestor" = $1 + -- TagRepository.upsertValue with "created_tag" as ( diff --git a/server/src/repositories/tag.repository.ts b/server/src/repositories/tag.repository.ts index 204dab973..2dc103048 100644 --- a/server/src/repositories/tag.repository.ts +++ b/server/src/repositories/tag.repository.ts @@ -32,6 +32,19 @@ export class TagRepository { .executeTakeFirst(); } + @GenerateSql({ params: [DummyValue.UUID] }) + async getAssetIdsByTagId(tagId: string): Promise { + const rows = await this.db + .selectFrom('tag_closure') + .innerJoin('tag_asset', 'tag_asset.tagId', 'tag_closure.id_descendant') + .select('tag_asset.assetId') + .distinct() + .where('tag_closure.id_ancestor', '=', tagId) + .execute(); + + return rows.map(({ assetId }) => assetId); + } + @GenerateSql({ params: [{ userId: DummyValue.UUID, value: DummyValue.STRING, parentId: DummyValue.UUID }] }) async upsertValue({ userId, value, parentId: _parentId }: { userId: string; value: string; parentId?: string }) { const parentId = _parentId ?? null; diff --git a/server/src/services/tag.service.spec.ts b/server/src/services/tag.service.spec.ts index f6fcf4b16..cefdf6bbf 100644 --- a/server/src/services/tag.service.spec.ts +++ b/server/src/services/tag.service.spec.ts @@ -1,6 +1,6 @@ import { BadRequestException } from '@nestjs/common'; import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto.js'; -import { JobStatus } from 'src/enum.js'; +import { JobName, JobStatus } from 'src/enum.js'; import { TagService } from 'src/services/tag.service.js'; import { authStub } from 'test/fixtures/auth.stub.js'; import { tagResponseStub, tagStub } from 'test/fixtures/tag.stub.js'; @@ -122,10 +122,30 @@ describe(TagService.name, () => { mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-1'])); mocks.tag.update.mockResolvedValue(tagStub.colorCreate); mocks.tag.get.mockResolvedValue(tagStub.tag); + mocks.tag.getAssetIdsByTagId.mockResolvedValue(['asset-1']); + mocks.asset.getForUpdateTags.mockResolvedValue({ tags: [{ value: 'tag' }] }); await expect(sut.update(authStub.admin, 'tag-1', { name: 'tag', color: '#000000' })).resolves.toEqual( tagResponseStub.color1, ); + expect(mocks.tag.getAssetIdsByTagId).toHaveBeenCalledWith('tag-1'); expect(mocks.tag.update).toHaveBeenCalledWith('tag-1', { value: 'tag', color: '#000000' }); + expect(mocks.asset.upsertExif).toHaveBeenCalledWith({ + exif: { assetId: 'asset-1', lockedProperties: ['tags'], tags: ['tag'] }, + lockedPropertiesBehavior: 'append', + }); + expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.SidecarWrite, data: { id: 'asset-1' } }]); + }); + + it('should not sync assets when only the tag color changes', async () => { + mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-1'])); + mocks.tag.update.mockResolvedValue(tagStub.colorCreate); + mocks.tag.get.mockResolvedValue(tagStub.tag); + + await sut.update(authStub.admin, 'tag-1', { color: '#000000' }); + + expect(mocks.tag.getAssetIdsByTagId).not.toHaveBeenCalled(); + expect(mocks.asset.upsertExif).not.toHaveBeenCalled(); + expect(mocks.job.queueAll).not.toHaveBeenCalled(); }); }); @@ -183,9 +203,17 @@ describe(TagService.name, () => { it('should remove a tag', async () => { mocks.tag.get.mockResolvedValue(tagStub.tag); mocks.tag.delete.mockResolvedValue(); + mocks.tag.getAssetIdsByTagId.mockResolvedValue(['asset-1']); + mocks.asset.getForUpdateTags.mockResolvedValue({ tags: [] }); await sut.remove(authStub.admin, 'tag-1'); + expect(mocks.tag.getAssetIdsByTagId).toHaveBeenCalledWith('tag-1'); expect(mocks.tag.delete).toHaveBeenCalledWith('tag-1'); + expect(mocks.asset.upsertExif).toHaveBeenCalledWith({ + exif: { assetId: 'asset-1', lockedProperties: ['tags'], tags: [] }, + lockedPropertiesBehavior: 'append', + }); + expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.SidecarWrite, data: { id: 'asset-1' } }]); }); }); diff --git a/server/src/services/tag.service.ts b/server/src/services/tag.service.ts index 48e49c4a9..be7dda4b0 100644 --- a/server/src/services/tag.service.ts +++ b/server/src/services/tag.service.ts @@ -71,7 +71,9 @@ export class TagService extends BaseService { value = existing.value; } + const assetIds = value === existing.value ? [] : await this.tagRepository.getAssetIdsByTagId(id); const tag = await this.tagRepository.update(id, { value, color }); + await this.syncAssetTags(assetIds); return mapTag(tag); } @@ -83,9 +85,9 @@ export class TagService extends BaseService { async remove(auth: AuthDto, id: string): Promise { await this.requireAccess({ auth, permission: Permission.TagDelete, ids: [id] }); - // TODO sync tag changes for affected assets - + const assetIds = await this.tagRepository.getAssetIdsByTagId(id); await this.tagRepository.delete(id); + await this.syncAssetTags(assetIds); } async bulkTagAssets(auth: AuthDto, dto: TagBulkAssetsDto): Promise { @@ -169,4 +171,16 @@ export class TagService extends BaseService { lockedPropertiesBehavior: 'append', }); } + + private async syncAssetTags(assetIds: string[]) { + if (assetIds.length === 0) { + return; + } + + for (const assetId of assetIds) { + await this.updateTags(assetId); + } + + await this.jobRepository.queueAll(assetIds.map((id) => ({ name: JobName.SidecarWrite, data: { id } }))); + } } diff --git a/server/test/medium/specs/services/tag.service.spec.ts b/server/test/medium/specs/services/tag.service.spec.ts index 731664add..2ed6ee063 100644 --- a/server/test/medium/specs/services/tag.service.spec.ts +++ b/server/test/medium/specs/services/tag.service.spec.ts @@ -1,9 +1,10 @@ import { Kysely } from 'kysely'; import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto.js'; -import { JobStatus } from 'src/enum.js'; +import { JobName, JobStatus } from 'src/enum.js'; import { AccessRepository } from 'src/repositories/access.repository.js'; import { AssetRepository } from 'src/repositories/asset.repository.js'; import { EventRepository } from 'src/repositories/event.repository.js'; +import { JobRepository } from 'src/repositories/job.repository.js'; import { LoggingRepository } from 'src/repositories/logging.repository.js'; import { TagRepository } from 'src/repositories/tag.repository.js'; import { DB } from 'src/schema/index.js'; @@ -19,7 +20,7 @@ const setup = (db?: Kysely) => { return newMediumService(TagService, { database: db || defaultDatabase, real: [AssetRepository, TagRepository, AccessRepository], - mock: [EventRepository, LoggingRepository], + mock: [EventRepository, JobRepository, LoggingRepository], }); }; @@ -55,6 +56,45 @@ describe(TagService.name, () => { 'Not found or no tag.update access', ); }); + + it('should sync renamed tags and descendants to the exif table', async () => { + const { sut, ctx } = setup(); + ctx.getMock(JobRepository).queueAll.mockResolvedValue(); + const { user } = await ctx.newUser(); + const { asset: parentAsset } = await ctx.newAsset({ ownerId: user.id }); + const { asset: childAsset } = await ctx.newAsset({ ownerId: user.id }); + const tagRepo = ctx.get(TagRepository); + const [parentTag, childTag] = await upsertTags(tagRepo, { + userId: user.id, + tags: ['parent', 'parent/child'], + }); + await ctx.newTagAsset({ tagIds: [parentTag.id], assetIds: [parentAsset.id] }); + await ctx.newTagAsset({ tagIds: [childTag.id], assetIds: [childAsset.id] }); + await ctx.newExif({ assetId: parentAsset.id, tags: ['parent'] }); + await ctx.newExif({ assetId: childAsset.id, tags: ['parent/child'] }); + + await sut.update(factory.auth({ user }), parentTag.id, { name: 'renamed' }); + + await expect( + ctx.database + .selectFrom('asset_exif') + .select(['assetId', 'lockedProperties', 'tags']) + .where('assetId', 'in', [parentAsset.id, childAsset.id]) + .orderBy('assetId') + .execute(), + ).resolves.toEqual( + expect.arrayContaining([ + { assetId: parentAsset.id, lockedProperties: ['tags'], tags: ['renamed'] }, + { assetId: childAsset.id, lockedProperties: ['tags'], tags: ['renamed/child'] }, + ]), + ); + expect(ctx.getMock(JobRepository).queueAll).toHaveBeenCalledWith( + expect.arrayContaining([ + { name: JobName.SidecarWrite, data: { id: parentAsset.id } }, + { name: JobName.SidecarWrite, data: { id: childAsset.id } }, + ]), + ); + }); }); describe('remove', () => {