diff --git a/server/src/database.ts b/server/src/database.ts index c3fb4cbab4..46d33d916d 100644 --- a/server/src/database.ts +++ b/server/src/database.ts @@ -52,5 +52,6 @@ export const columns = { 'shared_links.password', ], userDto: ['id', 'name', 'email', 'profileImagePath', 'profileChangedAt'], + tagDto: ['id', 'value', 'createdAt', 'updatedAt', 'color', 'parentId'], apiKey: ['id', 'name', 'userId', 'createdAt', 'updatedAt', 'permissions'], } as const; diff --git a/server/src/dtos/tag.dto.ts b/server/src/dtos/tag.dto.ts index 17200a8874..e62cf21636 100644 --- a/server/src/dtos/tag.dto.ts +++ b/server/src/dtos/tag.dto.ts @@ -1,6 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsHexColor, IsNotEmpty, IsString } from 'class-validator'; import { TagEntity } from 'src/entities/tag.entity'; +import { TagItem } from 'src/types'; import { Optional, ValidateHexColor, ValidateUUID } from 'src/validation'; export class TagCreateDto { @@ -51,7 +52,7 @@ export class TagResponseDto { color?: string; } -export function mapTag(entity: TagEntity): TagResponseDto { +export function mapTag(entity: TagItem | TagEntity): TagResponseDto { return { id: entity.id, parentId: entity.parentId ?? undefined, diff --git a/server/src/queries/tag.repository.sql b/server/src/queries/tag.repository.sql index 580344c597..6c97d7843f 100644 --- a/server/src/queries/tag.repository.sql +++ b/server/src/queries/tag.repository.sql @@ -1,10 +1,118 @@ -- NOTE: This file is auto generated by ./sql-generator --- TagRepository.getAssetIds -SELECT - "tag_asset"."assetsId" AS "assetId" -FROM - "tag_asset" "tag_asset" -WHERE - "tag_asset"."tagsId" = $1 - AND "tag_asset"."assetsId" IN ($2) +-- TagRepository.get +select + "id", + "value", + "createdAt", + "updatedAt", + "color", + "parentId" +from + "tags" +where + "id" = $1 + +-- TagRepository.getByValue +select + "id", + "value", + "createdAt", + "updatedAt", + "color", + "parentId" +from + "tags" +where + "userId" = $1 + and "value" = $2 + +-- TagRepository.upsertValue +begin +insert into + "tags" ("userId", "value", "parentId") +values + ($1, $2, $3) +on conflict ("userId", "value") do update +set + "parentId" = $4 +returning + * +rollback + +-- TagRepository.getAll +select + "id", + "value", + "createdAt", + "updatedAt", + "color", + "parentId" +from + "tags" +where + "userId" = $1 +order by + "value" asc + +-- TagRepository.create +insert into + "tags" ("userId", "color", "value") +values + ($1, $2, $3) +returning + * + +-- TagRepository.update +update "tags" +set + "color" = $1 +where + "id" = $2 +returning + * + +-- TagRepository.delete +delete from "tags" +where + "id" = $1 + +-- TagRepository.addAssetIds +insert into + "tag_asset" ("tagsId", "assetsId") +values + ($1, $2) + +-- TagRepository.removeAssetIds +delete from "tag_asset" +where + "tagsId" = $1 + and "assetsId" in ($2) + +-- TagRepository.replaceAssetTags +begin +delete from "tag_asset" +where + "assetsId" = $1 +insert into + "tag_asset" ("tagsId", "assetsId") +values + ($1, $2) +on conflict do nothing +returning + * +rollback + +-- TagRepository.deleteEmptyTags +begin +select + "tags"."id", + count("assets"."id") as "count" +from + "assets" + inner join "tag_asset" on "tag_asset"."assetsId" = "assets"."id" + inner join "tags_closure" on "tags_closure"."id_descendant" = "tag_asset"."tagsId" + inner join "tags" on "tags"."id" = "tags_closure"."id_descendant" +group by + "tags"."id" +commit diff --git a/server/src/repositories/tag.repository.ts b/server/src/repositories/tag.repository.ts index c5156e1837..c0ca6ebf37 100644 --- a/server/src/repositories/tag.repository.ts +++ b/server/src/repositories/tag.repository.ts @@ -1,209 +1,188 @@ import { Injectable } from '@nestjs/common'; -import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; +import { Insertable, Kysely, sql, Updateable } from 'kysely'; +import { InjectKysely } from 'nestjs-kysely'; +import { columns } from 'src/database'; +import { DB, TagAsset, Tags } from 'src/db'; import { Chunked, ChunkedSet, DummyValue, GenerateSql } from 'src/decorators'; -import { TagEntity } from 'src/entities/tag.entity'; import { LoggingRepository } from 'src/repositories/logging.repository'; -import { DataSource, In, Repository } from 'typeorm'; - -export type AssetTagItem = { assetId: string; tagId: string }; @Injectable() export class TagRepository { constructor( - @InjectDataSource() private dataSource: DataSource, - @InjectRepository(TagEntity) private repository: Repository, + @InjectKysely() private db: Kysely, private logger: LoggingRepository, ) { this.logger.setContext(TagRepository.name); } - get(id: string): Promise { - return this.repository.findOne({ where: { id } }); + @GenerateSql({ params: [DummyValue.UUID] }) + get(id: string) { + return this.db.selectFrom('tags').select(columns.tagDto).where('id', '=', id).executeTakeFirst(); } - getByValue(userId: string, value: string): Promise { - return this.repository.findOne({ where: { userId, value } }); + @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING] }) + getByValue(userId: string, value: string) { + return this.db + .selectFrom('tags') + .select(columns.tagDto) + .where('userId', '=', userId) + .where('value', '=', value) + .executeTakeFirst(); } - async upsertValue({ - userId, - value, - parent, - }: { - userId: string; - value: string; - parent?: TagEntity; - }): Promise { - return this.dataSource.transaction(async (manager) => { - // upsert tag - const { identifiers } = await manager.upsert( - TagEntity, - { userId, value, parentId: parent?.id }, - { conflictPaths: { userId: true, value: true } }, - ); - const id = identifiers[0]?.id; - if (!id) { - throw new Error('Failed to upsert tag'); - } + @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; + return this.db.transaction().execute(async (tx) => { + const tag = await this.db + .insertInto('tags') + .values({ userId, value, parentId }) + .onConflict((oc) => oc.columns(['userId', 'value']).doUpdateSet({ parentId })) + .returningAll() + .executeTakeFirstOrThrow(); // update closure table - await manager.query( - `INSERT INTO tags_closure (id_ancestor, id_descendant) - VALUES ($1, $1) - ON CONFLICT DO NOTHING;`, - [id], - ); + await tx + .insertInto('tags_closure') + .values({ id_ancestor: tag.id, id_descendant: tag.id }) + .onConflict((oc) => oc.doNothing()) + .execute(); - if (parent) { - await manager.query( - `INSERT INTO tags_closure (id_ancestor, id_descendant) - SELECT id_ancestor, '${id}' as id_descendant FROM tags_closure WHERE id_descendant = $1 - ON CONFLICT DO NOTHING`, - [parent.id], - ); + if (parentId) { + await tx + .insertInto('tags_closure') + .columns(['id_ancestor', 'id_descendant']) + .expression( + this.db + .selectFrom('tags_closure') + .select(['id_ancestor', sql.raw(`'${tag.id}'`).as('id_descendant')]) + .where('id_descendant', '=', parentId), + ) + .onConflict((oc) => oc.doNothing()) + .execute(); } - return manager.findOneOrFail(TagEntity, { where: { id } }); + return tag; }); } - async getAll(userId: string): Promise { - const tags = await this.repository.find({ - where: { userId }, - order: { - value: 'ASC', - }, - }); - - return tags; + @GenerateSql({ params: [DummyValue.UUID] }) + getAll(userId: string) { + return this.db + .selectFrom('tags') + .select(columns.tagDto) + .where('userId', '=', userId) + .orderBy('value asc') + .execute(); } - create(tag: Partial): Promise { - return this.save(tag); + @GenerateSql({ params: [{ userId: DummyValue.UUID, color: DummyValue.STRING, value: DummyValue.STRING }] }) + create(tag: Insertable) { + return this.db.insertInto('tags').values(tag).returningAll().executeTakeFirstOrThrow(); } - update(tag: Partial): Promise { - return this.save(tag); + @GenerateSql({ params: [DummyValue.UUID, { color: DummyValue.STRING }] }) + update(id: string, dto: Updateable) { + return this.db.updateTable('tags').set(dto).where('id', '=', id).returningAll().executeTakeFirstOrThrow(); } - async delete(id: string): Promise { - await this.repository.delete(id); + @GenerateSql({ params: [DummyValue.UUID] }) + async delete(id: string) { + await this.db.deleteFrom('tags').where('id', '=', id).execute(); } - @GenerateSql({ params: [DummyValue.UUID, [DummyValue.UUID]] }) @ChunkedSet({ paramIndex: 1 }) + @GenerateSql({ params: [DummyValue.UUID, [DummyValue.UUID]] }) async getAssetIds(tagId: string, assetIds: string[]): Promise> { if (assetIds.length === 0) { return new Set(); } - const results = await this.dataSource - .createQueryBuilder() - .select('tag_asset.assetsId', 'assetId') - .from('tag_asset', 'tag_asset') - .where('"tag_asset"."tagsId" = :tagId', { tagId }) - .andWhere('"tag_asset"."assetsId" IN (:...assetIds)', { assetIds }) - .getRawMany<{ assetId: string }>(); + const results = await this.db + .selectFrom('tag_asset') + .select(['assetsId as assetId']) + .where('tagsId', '=', tagId) + .where('assetsId', 'in', assetIds) + .execute(); return new Set(results.map(({ assetId }) => assetId)); } + @GenerateSql({ params: [DummyValue.UUID, [DummyValue.UUID]] }) + @Chunked({ paramIndex: 1 }) async addAssetIds(tagId: string, assetIds: string[]): Promise { if (assetIds.length === 0) { return; } - await this.dataSource.manager - .createQueryBuilder() - .insert() - .into('tag_asset', ['tagsId', 'assetsId']) + await this.db + .insertInto('tag_asset') .values(assetIds.map((assetId) => ({ tagsId: tagId, assetsId: assetId }))) .execute(); } + @GenerateSql({ params: [DummyValue.UUID, [DummyValue.UUID]] }) @Chunked({ paramIndex: 1 }) async removeAssetIds(tagId: string, assetIds: string[]): Promise { if (assetIds.length === 0) { return; } - await this.dataSource - .createQueryBuilder() - .delete() - .from('tag_asset') - .where({ - tagsId: tagId, - assetsId: In(assetIds), - }) - .execute(); + await this.db.deleteFrom('tag_asset').where('tagsId', '=', tagId).where('assetsId', 'in', assetIds).execute(); } + @GenerateSql({ params: [{ assetId: DummyValue.UUID, tagsIds: [DummyValue.UUID] }] }) @Chunked() - async upsertAssetIds(items: AssetTagItem[]): Promise { + upsertAssetIds(items: Insertable[]) { if (items.length === 0) { - return []; + return Promise.resolve([]); } - const { identifiers } = await this.dataSource - .createQueryBuilder() - .insert() - .into('tag_asset', ['assetsId', 'tagsId']) - .values(items.map(({ assetId, tagId }) => ({ assetsId: assetId, tagsId: tagId }))) + return this.db + .insertInto('tag_asset') + .values(items) + .onConflict((oc) => oc.doNothing()) + .returningAll() .execute(); - - return (identifiers as Array<{ assetsId: string; tagsId: string }>).map(({ assetsId, tagsId }) => ({ - assetId: assetsId, - tagId: tagsId, - })); } - async upsertAssetTags({ assetId, tagIds }: { assetId: string; tagIds: string[] }) { - await this.dataSource.transaction(async (manager) => { - await manager.createQueryBuilder().delete().from('tag_asset').where({ assetsId: assetId }).execute(); + @GenerateSql({ params: [DummyValue.UUID, [DummyValue.UUID]] }) + @Chunked({ paramIndex: 1 }) + replaceAssetTags(assetId: string, tagIds: string[]) { + return this.db.transaction().execute(async (tx) => { + await tx.deleteFrom('tag_asset').where('assetsId', '=', assetId).execute(); if (tagIds.length === 0) { return; } - await manager - .createQueryBuilder() - .insert() - .into('tag_asset', ['tagsId', 'assetsId']) + return tx + .insertInto('tag_asset') .values(tagIds.map((tagId) => ({ tagsId: tagId, assetsId: assetId }))) + .onConflict((oc) => oc.doNothing()) + .returningAll() .execute(); }); } + @GenerateSql() async deleteEmptyTags() { - await this.dataSource.transaction(async (manager) => { - const ids = new Set(); - const tags = await manager.find(TagEntity); - for (const tag of tags) { - const count = await manager - .createQueryBuilder('assets', 'asset') - .innerJoin( - 'asset.tags', - 'asset_tags', - 'asset_tags.id IN (SELECT id_descendant FROM tags_closure WHERE id_ancestor = :tagId)', - { tagId: tag.id }, - ) - .getCount(); + // TODO rewrite as a single statement + await this.db.transaction().execute(async (tx) => { + const result = await tx + .selectFrom('assets') + .innerJoin('tag_asset', 'tag_asset.assetsId', 'assets.id') + .innerJoin('tags_closure', 'tags_closure.id_descendant', 'tag_asset.tagsId') + .innerJoin('tags', 'tags.id', 'tags_closure.id_descendant') + .select((eb) => ['tags.id', eb.fn.count('assets.id').as('count')]) + .groupBy('tags.id') + .execute(); - if (count === 0) { - this.logger.debug(`Found empty tag: ${tag.id} - ${tag.value}`); - ids.add(tag.id); - } - } - - if (ids.size > 0) { - await manager.delete(TagEntity, { id: In([...ids]) }); - this.logger.log(`Deleted ${ids.size} empty tags`); + const ids = result.filter(({ count }) => count === 0).map(({ id }) => id); + if (ids.length > 0) { + await this.db.deleteFrom('tags').where('id', 'in', ids).execute(); + this.logger.log(`Deleted ${ids.length} empty tags`); } }); } - - private async save(partial: Partial): Promise { - const { id } = await this.repository.save(partial); - return this.repository.findOneOrFail({ where: { id } }); - } } diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 4769281367..e8ffd4a04f 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -241,7 +241,7 @@ describe(MetadataService.name, () => { it('should extract tags from TagsList', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ TagsList: ['Parent'] }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -251,27 +251,27 @@ describe(MetadataService.name, () => { it('should extract hierarchy from TagsList', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ TagsList: ['Parent/Child'] }); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parent); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.child); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { userId: 'user-id', value: 'Parent', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { userId: 'user-id', value: 'Parent/Child', - parent: tagStub.parent, + parentId: 'tag-parent', }); }); it('should extract tags from Keywords as a string', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ Keywords: 'Parent' }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -281,7 +281,7 @@ describe(MetadataService.name, () => { it('should extract tags from Keywords as a list', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ Keywords: ['Parent'] }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -291,7 +291,7 @@ describe(MetadataService.name, () => { it('should extract tags from Keywords as a list with a number', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ Keywords: ['Parent', 2024] }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -302,58 +302,58 @@ describe(MetadataService.name, () => { it('should extract hierarchal tags from Keywords', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ Keywords: 'Parent/Child' }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { userId: 'user-id', value: 'Parent', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { userId: 'user-id', value: 'Parent/Child', - parent: tagStub.parent, + parentId: 'tag-parent', }); }); it('should ignore Keywords when TagsList is present', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ Keywords: 'Child', TagsList: ['Parent/Child'] }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { userId: 'user-id', value: 'Parent', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { userId: 'user-id', value: 'Parent/Child', - parent: tagStub.parent, + parentId: 'tag-parent', }); }); it('should extract hierarchy from HierarchicalSubject', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ HierarchicalSubject: ['Parent|Child', 'TagA'] }); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parent); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.child); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { userId: 'user-id', value: 'Parent', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { userId: 'user-id', value: 'Parent/Child', - parent: tagStub.parent, + parentId: 'tag-parent', }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(3, { userId: 'user-id', value: 'TagA', parent: undefined }); }); @@ -361,7 +361,7 @@ describe(MetadataService.name, () => { it('should extract tags from HierarchicalSubject as a list with a number', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ HierarchicalSubject: ['Parent', 2024] }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -372,7 +372,7 @@ describe(MetadataService.name, () => { it('should extract ignore / characters in a HierarchicalSubject tag', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ HierarchicalSubject: ['Mom/Dad'] }); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -386,19 +386,19 @@ describe(MetadataService.name, () => { it('should ignore HierarchicalSubject when TagsList is present', async () => { mocks.asset.getByIds.mockResolvedValue([assetStub.image]); mockReadTags({ HierarchicalSubject: ['Parent2|Child2'], TagsList: ['Parent/Child'] }); - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { userId: 'user-id', value: 'Parent', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { userId: 'user-id', value: 'Parent/Child', - parent: tagStub.parent, + parentId: 'tag-parent', }); }); @@ -408,7 +408,7 @@ describe(MetadataService.name, () => { await sut.handleMetadataExtraction({ id: assetStub.image.id }); - expect(mocks.tag.upsertAssetTags).toHaveBeenCalledWith({ assetId: 'asset-id', tagIds: [] }); + expect(mocks.tag.replaceAssetTags).toHaveBeenCalledWith('asset-id', []); }); it('should not apply motion photos if asset is video', async () => { diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index 958aa76f89..14513c738a 100644 --- a/server/src/services/metadata.service.ts +++ b/server/src/services/metadata.service.ts @@ -390,7 +390,10 @@ export class MetadataService extends BaseService { } const results = await upsertTags(this.tagRepository, { userId: asset.ownerId, tags }); - await this.tagRepository.upsertAssetTags({ assetId: asset.id, tagIds: results.map((tag) => tag.id) }); + await this.tagRepository.replaceAssetTags( + asset.id, + results.map((tag) => tag.id), + ); } private async applyMotionPhotos(asset: AssetEntity, tags: ImmichTags) { diff --git a/server/src/services/tag.service.spec.ts b/server/src/services/tag.service.spec.ts index f1385eb8c8..22d7747d0d 100644 --- a/server/src/services/tag.service.spec.ts +++ b/server/src/services/tag.service.spec.ts @@ -22,7 +22,7 @@ describe(TagService.name, () => { describe('getAll', () => { it('should return all tags for a user', async () => { - mocks.tag.getAll.mockResolvedValue([tagStub.tag1]); + mocks.tag.getAll.mockResolvedValue([tagStub.tag]); await expect(sut.getAll(authStub.admin)).resolves.toEqual([tagResponseStub.tag1]); expect(mocks.tag.getAll).toHaveBeenCalledWith(authStub.admin.user.id); }); @@ -30,13 +30,12 @@ describe(TagService.name, () => { describe('get', () => { it('should throw an error for an invalid id', async () => { - mocks.tag.get.mockResolvedValue(null); await expect(sut.get(authStub.admin, 'tag-1')).rejects.toBeInstanceOf(BadRequestException); expect(mocks.tag.get).toHaveBeenCalledWith('tag-1'); }); it('should return a tag for a user', async () => { - mocks.tag.get.mockResolvedValue(tagStub.tag1); + mocks.tag.get.mockResolvedValue(tagStub.tag); await expect(sut.get(authStub.admin, 'tag-1')).resolves.toEqual(tagResponseStub.tag1); expect(mocks.tag.get).toHaveBeenCalledWith('tag-1'); }); @@ -53,9 +52,9 @@ describe(TagService.name, () => { it('should create a tag with a parent', async () => { mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-parent'])); - mocks.tag.create.mockResolvedValue(tagStub.tag1); - mocks.tag.get.mockResolvedValueOnce(tagStub.parent); - mocks.tag.get.mockResolvedValueOnce(tagStub.child); + mocks.tag.create.mockResolvedValue(tagStub.tagCreate); + mocks.tag.get.mockResolvedValueOnce(tagStub.parentUpsert); + mocks.tag.get.mockResolvedValueOnce(tagStub.childUpsert); await expect(sut.create(authStub.admin, { name: 'tagA', parentId: 'tag-parent' })).resolves.toBeDefined(); expect(mocks.tag.create).toHaveBeenCalledWith(expect.objectContaining({ value: 'Parent/tagA' })); }); @@ -71,14 +70,14 @@ describe(TagService.name, () => { describe('create', () => { it('should throw an error for a duplicate tag', async () => { - mocks.tag.getByValue.mockResolvedValue(tagStub.tag1); + mocks.tag.getByValue.mockResolvedValue(tagStub.tag); await expect(sut.create(authStub.admin, { name: 'tag-1' })).rejects.toBeInstanceOf(BadRequestException); expect(mocks.tag.getByValue).toHaveBeenCalledWith(authStub.admin.user.id, 'tag-1'); expect(mocks.tag.create).not.toHaveBeenCalled(); }); it('should create a new tag', async () => { - mocks.tag.create.mockResolvedValue(tagStub.tag1); + mocks.tag.create.mockResolvedValue(tagStub.tagCreate); await expect(sut.create(authStub.admin, { name: 'tag-1' })).resolves.toEqual(tagResponseStub.tag1); expect(mocks.tag.create).toHaveBeenCalledWith({ userId: authStub.admin.user.id, @@ -87,7 +86,7 @@ describe(TagService.name, () => { }); it('should create a new tag with optional color', async () => { - mocks.tag.create.mockResolvedValue(tagStub.color1); + mocks.tag.create.mockResolvedValue(tagStub.colorCreate); await expect(sut.create(authStub.admin, { name: 'tag-1', color: '#000000' })).resolves.toEqual( tagResponseStub.color1, ); @@ -110,15 +109,15 @@ describe(TagService.name, () => { it('should update a tag', async () => { mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-1'])); - mocks.tag.update.mockResolvedValue(tagStub.color1); + mocks.tag.update.mockResolvedValue(tagStub.colorCreate); await expect(sut.update(authStub.admin, 'tag-1', { color: '#000000' })).resolves.toEqual(tagResponseStub.color1); - expect(mocks.tag.update).toHaveBeenCalledWith({ id: 'tag-1', color: '#000000' }); + expect(mocks.tag.update).toHaveBeenCalledWith('tag-1', { color: '#000000' }); }); }); describe('upsert', () => { it('should upsert a new tag', async () => { - mocks.tag.upsertValue.mockResolvedValue(tagStub.parent); + mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); await expect(sut.upsert(authStub.admin, { tags: ['Parent'] })).resolves.toBeDefined(); expect(mocks.tag.upsertValue).toHaveBeenCalledWith({ value: 'Parent', @@ -128,36 +127,34 @@ describe(TagService.name, () => { }); it('should upsert a nested tag', async () => { - mocks.tag.getByValue.mockResolvedValueOnce(null); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parent); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.child); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert); await expect(sut.upsert(authStub.admin, { tags: ['Parent/Child'] })).resolves.toBeDefined(); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { value: 'Parent', userId: 'admin_id', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { value: 'Parent/Child', userId: 'admin_id', - parent: expect.objectContaining({ id: 'tag-parent' }), + parentId: 'tag-parent', }); }); it('should upsert a tag and ignore leading and trailing slashes', async () => { - mocks.tag.getByValue.mockResolvedValueOnce(null); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parent); - mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.child); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); + mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert); await expect(sut.upsert(authStub.admin, { tags: ['/Parent/Child/'] })).resolves.toBeDefined(); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(1, { value: 'Parent', userId: 'admin_id', - parent: undefined, + parentId: undefined, }); expect(mocks.tag.upsertValue).toHaveBeenNthCalledWith(2, { value: 'Parent/Child', userId: 'admin_id', - parent: expect.objectContaining({ id: 'tag-parent' }), + parentId: 'tag-parent', }); }); }); @@ -170,7 +167,7 @@ describe(TagService.name, () => { }); it('should remove a tag', async () => { - mocks.tag.get.mockResolvedValue(tagStub.tag1); + mocks.tag.get.mockResolvedValue(tagStub.tag); await sut.remove(authStub.admin, 'tag-1'); expect(mocks.tag.delete).toHaveBeenCalledWith('tag-1'); }); @@ -190,12 +187,12 @@ describe(TagService.name, () => { 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.tag.upsertAssetIds.mockResolvedValue([ - { tagId: 'tag-1', assetId: 'asset-1' }, - { tagId: 'tag-1', assetId: 'asset-2' }, - { tagId: 'tag-1', assetId: 'asset-3' }, - { tagId: 'tag-2', assetId: 'asset-1' }, - { tagId: 'tag-2', assetId: 'asset-2' }, - { tagId: 'tag-2', assetId: 'asset-3' }, + { tagsId: 'tag-1', assetsId: 'asset-1' }, + { tagsId: 'tag-1', assetsId: 'asset-2' }, + { tagsId: 'tag-1', assetsId: 'asset-3' }, + { tagsId: 'tag-2', assetsId: 'asset-1' }, + { tagsId: 'tag-2', assetsId: 'asset-2' }, + { tagsId: 'tag-2', assetsId: 'asset-3' }, ]); await expect( sut.bulkTagAssets(authStub.admin, { tagIds: ['tag-1', 'tag-2'], assetIds: ['asset-1', 'asset-2', 'asset-3'] }), @@ -203,19 +200,18 @@ describe(TagService.name, () => { count: 6, }); expect(mocks.tag.upsertAssetIds).toHaveBeenCalledWith([ - { tagId: 'tag-1', assetId: 'asset-1' }, - { tagId: 'tag-1', assetId: 'asset-2' }, - { tagId: 'tag-1', assetId: 'asset-3' }, - { tagId: 'tag-2', assetId: 'asset-1' }, - { tagId: 'tag-2', assetId: 'asset-2' }, - { tagId: 'tag-2', assetId: 'asset-3' }, + { tagsId: 'tag-1', assetsId: 'asset-1' }, + { tagsId: 'tag-1', assetsId: 'asset-2' }, + { tagsId: 'tag-1', assetsId: 'asset-3' }, + { tagsId: 'tag-2', assetsId: 'asset-1' }, + { tagsId: 'tag-2', assetsId: 'asset-2' }, + { tagsId: 'tag-2', assetsId: 'asset-3' }, ]); }); }); describe('addAssets', () => { it('should handle invalid ids', async () => { - mocks.tag.get.mockResolvedValue(null); mocks.tag.getAssetIds.mockResolvedValue(new Set([])); await expect(sut.addAssets(authStub.admin, 'tag-1', { ids: ['asset-1'] })).resolves.toEqual([ { id: 'asset-1', success: false, error: 'no_permission' }, @@ -225,7 +221,7 @@ describe(TagService.name, () => { }); it('should accept accept ids that are new and reject the rest', async () => { - mocks.tag.get.mockResolvedValue(tagStub.tag1); + mocks.tag.get.mockResolvedValue(tagStub.tag); mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1'])); mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-2'])); @@ -245,7 +241,6 @@ describe(TagService.name, () => { describe('removeAssets', () => { it('should throw an error for an invalid id', async () => { - mocks.tag.get.mockResolvedValue(null); mocks.tag.getAssetIds.mockResolvedValue(new Set()); await expect(sut.removeAssets(authStub.admin, 'tag-1', { ids: ['asset-1'] })).resolves.toEqual([ { id: 'asset-1', success: false, error: 'not_found' }, @@ -253,7 +248,7 @@ describe(TagService.name, () => { }); it('should accept accept ids that are tagged and reject the rest', async () => { - mocks.tag.get.mockResolvedValue(tagStub.tag1); + mocks.tag.get.mockResolvedValue(tagStub.tag); mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1'])); await expect( diff --git a/server/src/services/tag.service.ts b/server/src/services/tag.service.ts index c241f59a80..ecf4d6e9fb 100644 --- a/server/src/services/tag.service.ts +++ b/server/src/services/tag.service.ts @@ -1,4 +1,6 @@ import { BadRequestException, Injectable } from '@nestjs/common'; +import { Insertable } from 'kysely'; +import { TagAsset } from 'src/db'; import { OnJob } from 'src/decorators'; import { BulkIdResponseDto, BulkIdsDto } from 'src/dtos/asset-ids.response.dto'; import { AuthDto } from 'src/dtos/auth.dto'; @@ -11,9 +13,7 @@ import { TagUpsertDto, mapTag, } from 'src/dtos/tag.dto'; -import { TagEntity } from 'src/entities/tag.entity'; import { JobName, JobStatus, Permission, QueueName } from 'src/enum'; -import { AssetTagItem } from 'src/repositories/tag.repository'; import { BaseService } from 'src/services/base.service'; import { addAssets, removeAssets } from 'src/utils/asset.util'; import { upsertTags } from 'src/utils/tag'; @@ -32,10 +32,10 @@ export class TagService extends BaseService { } async create(auth: AuthDto, dto: TagCreateDto) { - let parent: TagEntity | undefined; + let parent; if (dto.parentId) { await this.requireAccess({ auth, permission: Permission.TAG_READ, ids: [dto.parentId] }); - parent = (await this.tagRepository.get(dto.parentId)) || undefined; + parent = await this.tagRepository.get(dto.parentId); if (!parent) { throw new BadRequestException('Tag not found'); } @@ -49,7 +49,7 @@ export class TagService extends BaseService { } const { color } = dto; - const tag = await this.tagRepository.create({ userId, value, color, parent }); + const tag = await this.tagRepository.create({ userId, value, color, parentId: parent?.id }); return mapTag(tag); } @@ -58,7 +58,7 @@ export class TagService extends BaseService { await this.requireAccess({ auth, permission: Permission.TAG_UPDATE, ids: [id] }); const { color } = dto; - const tag = await this.tagRepository.update({ id, color }); + const tag = await this.tagRepository.update(id, { color }); return mapTag(tag); } @@ -81,15 +81,15 @@ export class TagService extends BaseService { this.checkAccess({ auth, permission: Permission.ASSET_UPDATE, ids: dto.assetIds }), ]); - const items: AssetTagItem[] = []; - for (const tagId of tagIds) { - for (const assetId of assetIds) { - items.push({ tagId, assetId }); + const items: Insertable[] = []; + for (const tagsId of tagIds) { + for (const assetsId of assetIds) { + items.push({ tagsId, assetsId }); } } const results = await this.tagRepository.upsertAssetIds(items); - for (const assetId of new Set(results.map((item) => item.assetId))) { + for (const assetId of new Set(results.map((item) => item.assetsId))) { await this.eventRepository.emit('asset.tag', { assetId }); } diff --git a/server/src/types.ts b/server/src/types.ts index e9865b6a5a..41d98f9a02 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -37,6 +37,15 @@ export type MemoryItem = export type SessionItem = Awaited>[0]; +export type TagItem = { + id: string; + value: string; + createdAt: Date; + updatedAt: Date; + color: string | null; + parentId: string | null; +}; + export interface CropOptions { top: number; left: number; diff --git a/server/src/utils/tag.ts b/server/src/utils/tag.ts index 4b3b360a8b..b095fcfd85 100644 --- a/server/src/utils/tag.ts +++ b/server/src/utils/tag.ts @@ -1,19 +1,19 @@ -import { TagEntity } from 'src/entities/tag.entity'; import { TagRepository } from 'src/repositories/tag.repository'; +import { TagItem } from 'src/types'; type UpsertRequest = { userId: string; tags: string[] }; export const upsertTags = async (repository: TagRepository, { userId, tags }: UpsertRequest) => { tags = [...new Set(tags)]; - const results: TagEntity[] = []; + const results: TagItem[] = []; for (const tag of tags) { const parts = tag.split('/').filter(Boolean); - let parent: TagEntity | undefined; + let parent: TagItem | undefined; for (const part of parts) { const value = parent ? `${parent.value}/${part}` : part; - parent = await repository.upsertValue({ userId, value, parent }); + parent = await repository.upsertValue({ userId, value, parentId: parent?.id }); } if (parent) { diff --git a/server/test/fixtures/tag.stub.ts b/server/test/fixtures/tag.stub.ts index b245bfe9e5..1a19c2a002 100644 --- a/server/test/fixtures/tag.stub.ts +++ b/server/test/fixtures/tag.stub.ts @@ -1,49 +1,51 @@ import { TagResponseDto } from 'src/dtos/tag.dto'; -import { TagEntity } from 'src/entities/tag.entity'; -import { userStub } from 'test/fixtures/user.stub'; +import { TagItem } from 'src/types'; -const parent = Object.freeze({ +const parent = Object.freeze({ id: 'tag-parent', createdAt: new Date('2021-01-01T00:00:00Z'), updatedAt: new Date('2021-01-01T00:00:00Z'), value: 'Parent', color: null, - userId: userStub.admin.id, - user: userStub.admin, + parentId: null, }); -const child = Object.freeze({ +const child = Object.freeze({ id: 'tag-child', createdAt: new Date('2021-01-01T00:00:00Z'), updatedAt: new Date('2021-01-01T00:00:00Z'), value: 'Parent/Child', color: null, - parent, - userId: userStub.admin.id, - user: userStub.admin, + parentId: parent.id, }); +const tag = { + id: 'tag-1', + createdAt: new Date('2021-01-01T00:00:00Z'), + updatedAt: new Date('2021-01-01T00:00:00Z'), + value: 'Tag1', + color: null, + parentId: null, +}; + +const color = { + id: 'tag-1', + createdAt: new Date('2021-01-01T00:00:00Z'), + updatedAt: new Date('2021-01-01T00:00:00Z'), + value: 'Tag1', + color: '#000000', + parentId: null, +}; + +const upsert = { userId: 'tag-user', updateId: 'uuid-v7' }; + export const tagStub = { - tag1: Object.freeze({ - id: 'tag-1', - createdAt: new Date('2021-01-01T00:00:00Z'), - updatedAt: new Date('2021-01-01T00:00:00Z'), - value: 'Tag1', - color: null, - userId: userStub.admin.id, - user: userStub.admin, - }), - parent, - child, - color1: Object.freeze({ - id: 'tag-1', - createdAt: new Date('2021-01-01T00:00:00Z'), - updatedAt: new Date('2021-01-01T00:00:00Z'), - value: 'Tag1', - color: '#000000', - userId: userStub.admin.id, - user: userStub.admin, - }), + tag, + tagCreate: { ...tag, ...upsert }, + color, + colorCreate: { ...color, ...upsert }, + parentUpsert: { ...parent, ...upsert }, + childUpsert: { ...child, ...upsert }, }; export const tagResponseStub = { diff --git a/server/test/repositories/tag.repository.mock.ts b/server/test/repositories/tag.repository.mock.ts index 7f6f1b6c53..b6313ca798 100644 --- a/server/test/repositories/tag.repository.mock.ts +++ b/server/test/repositories/tag.repository.mock.ts @@ -7,7 +7,7 @@ export const newTagRepositoryMock = (): Mocked