immich-app/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2025-12-25 17:24:58 +03:00
Code Issues Packages Projects Releases 18 Wiki Activity

refactor: migration tag repository to kysely (#16398)

Browse Source
This commit is contained in:
Jason Rasmussen
2025-03-03 13:41:19 -05:00
committed by GitHub
parent ff19502035
commit d1fd0076cc
12 changed files with 343 additions and 245 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
server/src/services/metadata.service.spec.ts
View File

@@ -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 () => {

5
server/src/services/metadata.service.ts
View File

@@ -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) {

73
server/src/services/tag.service.spec.ts
View File

@@ -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(

22
server/src/services/tag.service.ts
View File

@@ -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<TagAsset>[] = [];
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 });
}