fix(server): write tag updates to exif table (#31425)

Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>

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 (

server/src/repositories/tag.repository.ts

@@ -32,6 +32,19 @@ export class TagRepository {
       .executeTakeFirst();
   }
 
+  @GenerateSql({ params: [DummyValue.UUID] })
+  async getAssetIdsByTagId(tagId: string): Promise<string[]> {
+    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;

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' } }]);
     });
   });
 

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<void> {
     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<TagBulkAssetsResponseDto> {
@@ -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 } })));
+  }
 }

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<DB>) => {
   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', () => {