diff --git a/server/src/repositories/tag.repository.ts b/server/src/repositories/tag.repository.ts index 9bbb62bd8b..d9c44f4ba4 100644 --- a/server/src/repositories/tag.repository.ts +++ b/server/src/repositories/tag.repository.ts @@ -163,22 +163,22 @@ export class TagRepository { } async deleteEmptyTags() { - // TODO rewrite as a single statement - await this.db.transaction().execute(async (tx) => { - const result = await tx - .selectFrom('asset') - .innerJoin('tag_asset', 'tag_asset.assetsId', 'asset.id') - .innerJoin('tag_closure', 'tag_closure.id_descendant', 'tag_asset.tagsId') - .innerJoin('tag', 'tag.id', 'tag_closure.id_descendant') - .select((eb) => ['tag.id', eb.fn.count('asset.id').as('count')]) - .groupBy('tag.id') - .execute(); + const result = await this.db + .deleteFrom('tag') + .where(({ not, exists, selectFrom }) => + not( + exists( + selectFrom('tag_closure') + .whereRef('tag.id', '=', 'tag_closure.id_ancestor') + .innerJoin('tag_asset', 'tag_closure.id_descendant', 'tag_asset.tagsId'), + ), + ), + ) + .executeTakeFirst(); - const ids = result.filter(({ count }) => count === 0).map(({ id }) => id); - if (ids.length > 0) { - await this.db.deleteFrom('tag').where('id', 'in', ids).execute(); - this.logger.log(`Deleted ${ids.length} empty tags`); - } - }); + const deletedRows = Number(result.numDeletedRows); + if (deletedRows > 0) { + this.logger.log(`Deleted ${deletedRows} empty tags`); + } } } diff --git a/server/test/medium.factory.ts b/server/test/medium.factory.ts index c7356e2f1b..f802e3113e 100644 --- a/server/test/medium.factory.ts +++ b/server/test/medium.factory.ts @@ -39,6 +39,7 @@ import { StorageRepository } from 'src/repositories/storage.repository'; import { SyncCheckpointRepository } from 'src/repositories/sync-checkpoint.repository'; import { SyncRepository } from 'src/repositories/sync.repository'; import { SystemMetadataRepository } from 'src/repositories/system-metadata.repository'; +import { TagRepository } from 'src/repositories/tag.repository'; import { TelemetryRepository } from 'src/repositories/telemetry.repository'; import { UserRepository } from 'src/repositories/user.repository'; import { VersionHistoryRepository } from 'src/repositories/version-history.repository'; @@ -52,6 +53,8 @@ import { MemoryTable } from 'src/schema/tables/memory.table'; import { PersonTable } from 'src/schema/tables/person.table'; import { SessionTable } from 'src/schema/tables/session.table'; import { StackTable } from 'src/schema/tables/stack.table'; +import { TagAssetTable } from 'src/schema/tables/tag-asset.table'; +import { TagTable } from 'src/schema/tables/tag.table'; import { UserTable } from 'src/schema/tables/user.table'; import { BASE_SERVICE_DEPENDENCIES, BaseService } from 'src/services/base.service'; import { SyncService } from 'src/services/sync.service'; @@ -240,6 +243,18 @@ export class MediumTestContext { user, }; } + + async newTagAsset(tagBulkAssets: { tagIds: string[]; assetIds: string[] }) { + const tagsAssets: Insertable[] = []; + for (const tagsId of tagBulkAssets.tagIds) { + for (const assetsId of tagBulkAssets.assetIds) { + tagsAssets.push({ tagsId, assetsId }); + } + } + + const result = await this.get(TagRepository).upsertAssetIds(tagsAssets); + return { tagsAssets, result }; + } } export class SyncTestContext extends MediumTestContext { @@ -318,6 +333,10 @@ const newRealRepository = (key: ClassConstructor, db: Kysely): T => { return new key(LoggingRepository.create()); } + case TagRepository: { + return new key(db, LoggingRepository.create()); + } + case LoggingRepository as unknown as ClassConstructor: { return new key() as unknown as T; } @@ -345,7 +364,8 @@ const newMockRepository = (key: ClassConstructor) => { case SyncCheckpointRepository: case SystemMetadataRepository: case UserRepository: - case VersionHistoryRepository: { + case VersionHistoryRepository: + case TagRepository: { return automock(key); } @@ -567,6 +587,23 @@ const memoryInsert = (memory: Partial> = {}) => { return { ...defaults, ...memory, id }; }; +const tagInsert = (tag: Partial>) => { + const id = tag.id || newUuid(); + + const defaults: Insertable = { + id, + userId: '', + value: '', + createdAt: newDate(), + updatedAt: newDate(), + color: '', + parentId: null, + updateId: newUuid(), + }; + + return { ...defaults, ...tag, id }; +}; + class CustomWritable extends Writable { private data = ''; @@ -619,4 +656,5 @@ export const mediumFactory = { memoryInsert, loginDetails, loginResponse, + tagInsert, }; diff --git a/server/test/medium/specs/services/tag.service.spec.ts b/server/test/medium/specs/services/tag.service.spec.ts new file mode 100644 index 0000000000..2ec498e56d --- /dev/null +++ b/server/test/medium/specs/services/tag.service.spec.ts @@ -0,0 +1,116 @@ +import { Kysely } from 'kysely'; +import { JobStatus } from 'src/enum'; +import { AccessRepository } from 'src/repositories/access.repository'; +import { LoggingRepository } from 'src/repositories/logging.repository'; +import { TagRepository } from 'src/repositories/tag.repository'; +import { DB } from 'src/schema'; +import { TagService } from 'src/services/tag.service'; +import { upsertTags } from 'src/utils/tag'; +import { newMediumService } from 'test/medium.factory'; +import { getKyselyDB } from 'test/utils'; + +let defaultDatabase: Kysely; + +const setup = (db?: Kysely) => { + return newMediumService(TagService, { + database: db || defaultDatabase, + real: [TagRepository, AccessRepository], + mock: [LoggingRepository], + }); +}; + +beforeAll(async () => { + defaultDatabase = await getKyselyDB(); +}); + +describe(TagService.name, () => { + describe('deleteEmptyTags', () => { + it('single tag exists, not connected to any assets, and is deleted', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const tagRepo = ctx.get(TagRepository); + const [tag] = await upsertTags(tagRepo, { userId: user.id, tags: ['tag-1'] }); + + await expect(tagRepo.getByValue(user.id, 'tag-1')).resolves.toEqual(expect.objectContaining({ id: tag.id })); + await expect(sut.handleTagCleanup()).resolves.toBe(JobStatus.Success); + await expect(tagRepo.getByValue(user.id, 'tag-1')).resolves.toBeUndefined(); + }); + + it('single tag exists, connected to one asset, and is not deleted', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + const tagRepo = ctx.get(TagRepository); + const [tag] = await upsertTags(tagRepo, { userId: user.id, tags: ['tag-1'] }); + + await ctx.newTagAsset({ tagIds: [tag.id], assetIds: [asset.id] }); + + await expect(tagRepo.getByValue(user.id, 'tag-1')).resolves.toEqual(expect.objectContaining({ id: tag.id })); + await expect(sut.handleTagCleanup()).resolves.toBe(JobStatus.Success); + await expect(tagRepo.getByValue(user.id, 'tag-1')).resolves.toEqual(expect.objectContaining({ id: tag.id })); + }); + + it('hierarchical tag exists, and the parent is connected to an asset, and the child is deleted', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const { asset } = 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: [asset.id] }); + + await expect(tagRepo.getByValue(user.id, 'parent')).resolves.toEqual( + expect.objectContaining({ id: parentTag.id }), + ); + await expect(tagRepo.getByValue(user.id, 'parent/child')).resolves.toEqual( + expect.objectContaining({ id: childTag.id }), + ); + await expect(sut.handleTagCleanup()).resolves.toBe(JobStatus.Success); + await expect(tagRepo.getByValue(user.id, 'parent')).resolves.toEqual( + expect.objectContaining({ id: parentTag.id }), + ); + await expect(tagRepo.getByValue(user.id, 'parent/child')).resolves.toBeUndefined(); + }); + + it('hierarchical tag exists, and only the child is connected to an asset, and nothing is deleted', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const { asset } = 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: [childTag.id], assetIds: [asset.id] }); + + await expect(tagRepo.getByValue(user.id, 'parent')).resolves.toEqual( + expect.objectContaining({ id: parentTag.id }), + ); + await expect(tagRepo.getByValue(user.id, 'parent/child')).resolves.toEqual( + expect.objectContaining({ id: childTag.id }), + ); + await expect(sut.handleTagCleanup()).resolves.toBe(JobStatus.Success); + await expect(tagRepo.getByValue(user.id, 'parent')).resolves.toEqual( + expect.objectContaining({ id: parentTag.id }), + ); + await expect(tagRepo.getByValue(user.id, 'parent/child')).resolves.toEqual( + expect.objectContaining({ id: childTag.id }), + ); + }); + + it('hierarchical tag exists, and neither parent nor child is connected to an asset, and both are deleted', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const tagRepo = ctx.get(TagRepository); + const [parentTag, childTag] = await upsertTags(tagRepo, { userId: user.id, tags: ['parent', 'parent/child'] }); + + await expect(tagRepo.getByValue(user.id, 'parent')).resolves.toEqual( + expect.objectContaining({ id: parentTag.id }), + ); + await expect(tagRepo.getByValue(user.id, 'parent/child')).resolves.toEqual( + expect.objectContaining({ id: childTag.id }), + ); + await expect(sut.handleTagCleanup()).resolves.toBe(JobStatus.Success); + await expect(tagRepo.getByValue(user.id, 'parent/child')).resolves.toBeUndefined(); + await expect(tagRepo.getByValue(user.id, 'parent')).resolves.toBeUndefined(); + }); + }); +});