diff --git a/cli/src/api/open-api/api.ts b/cli/src/api/open-api/api.ts index ec57c3591b..a4cb44696e 100644 --- a/cli/src/api/open-api/api.ts +++ b/cli/src/api/open-api/api.ts @@ -798,6 +798,41 @@ export interface AuthDeviceResponseDto { */ 'deviceOS': string; } +/** + * + * @export + * @interface BulkIdResponseDto + */ +export interface BulkIdResponseDto { + /** + * + * @type {string} + * @memberof BulkIdResponseDto + */ + 'id': string; + /** + * + * @type {boolean} + * @memberof BulkIdResponseDto + */ + 'success': boolean; + /** + * + * @type {string} + * @memberof BulkIdResponseDto + */ + 'error'?: BulkIdResponseDtoErrorEnum; +} + +export const BulkIdResponseDtoErrorEnum = { + Duplicate: 'duplicate', + NoPermission: 'no_permission', + NotFound: 'not_found', + Unknown: 'unknown' +} as const; + +export type BulkIdResponseDtoErrorEnum = typeof BulkIdResponseDtoErrorEnum[keyof typeof BulkIdResponseDtoErrorEnum]; + /** * * @export @@ -1686,6 +1721,19 @@ export interface MemoryLaneResponseDto { */ 'assets': Array; } +/** + * + * @export + * @interface MergePersonDto + */ +export interface MergePersonDto { + /** + * + * @type {Array} + * @memberof MergePersonDto + */ + 'ids': Array; +} /** * * @export @@ -8807,6 +8855,54 @@ export const PersonApiAxiosParamCreator = function (configuration?: Configuratio options: localVarRequestOptions, }; }, + /** + * + * @param {string} id + * @param {MergePersonDto} mergePersonDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + mergePerson: async (id: string, mergePersonDto: MergePersonDto, options: AxiosRequestConfig = {}): Promise => { + // verify required parameter 'id' is not null or undefined + assertParamExists('mergePerson', 'id', id) + // verify required parameter 'mergePersonDto' is not null or undefined + assertParamExists('mergePerson', 'mergePersonDto', mergePersonDto) + const localVarPath = `/person/{id}/merge` + .replace(`{${"id"}}`, encodeURIComponent(String(id))); + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { method: 'POST', ...baseOptions, ...options}; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + // authentication cookie required + + // authentication api_key required + await setApiKeyToObject(localVarHeaderParameter, "x-api-key", configuration) + + // authentication bearer required + // http bearer authentication required + await setBearerAuthToObject(localVarHeaderParameter, configuration) + + + + localVarHeaderParameter['Content-Type'] = 'application/json'; + + setSearchParams(localVarUrlObj, localVarQueryParameter); + let headersFromBaseOptions = baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; + localVarRequestOptions.data = serializeDataIfNeeded(mergePersonDto, localVarRequestOptions, configuration) + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, /** * * @param {string} id @@ -8904,6 +9000,17 @@ export const PersonApiFp = function(configuration?: Configuration) { const localVarAxiosArgs = await localVarAxiosParamCreator.getPersonThumbnail(id, options); return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); }, + /** + * + * @param {string} id + * @param {MergePersonDto} mergePersonDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async mergePerson(id: string, mergePersonDto: MergePersonDto, options?: AxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise>> { + const localVarAxiosArgs = await localVarAxiosParamCreator.mergePerson(id, mergePersonDto, options); + return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); + }, /** * * @param {string} id @@ -8960,6 +9067,15 @@ export const PersonApiFactory = function (configuration?: Configuration, basePat getPersonThumbnail(requestParameters: PersonApiGetPersonThumbnailRequest, options?: AxiosRequestConfig): AxiosPromise { return localVarFp.getPersonThumbnail(requestParameters.id, options).then((request) => request(axios, basePath)); }, + /** + * + * @param {PersonApiMergePersonRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + mergePerson(requestParameters: PersonApiMergePersonRequest, options?: AxiosRequestConfig): AxiosPromise> { + return localVarFp.mergePerson(requestParameters.id, requestParameters.mergePersonDto, options).then((request) => request(axios, basePath)); + }, /** * * @param {PersonApiUpdatePersonRequest} requestParameters Request parameters. @@ -9014,6 +9130,27 @@ export interface PersonApiGetPersonThumbnailRequest { readonly id: string } +/** + * Request parameters for mergePerson operation in PersonApi. + * @export + * @interface PersonApiMergePersonRequest + */ +export interface PersonApiMergePersonRequest { + /** + * + * @type {string} + * @memberof PersonApiMergePerson + */ + readonly id: string + + /** + * + * @type {MergePersonDto} + * @memberof PersonApiMergePerson + */ + readonly mergePersonDto: MergePersonDto +} + /** * Request parameters for updatePerson operation in PersonApi. * @export @@ -9085,6 +9222,17 @@ export class PersonApi extends BaseAPI { return PersonApiFp(this.configuration).getPersonThumbnail(requestParameters.id, options).then((request) => request(this.axios, this.basePath)); } + /** + * + * @param {PersonApiMergePersonRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof PersonApi + */ + public mergePerson(requestParameters: PersonApiMergePersonRequest, options?: AxiosRequestConfig) { + return PersonApiFp(this.configuration).mergePerson(requestParameters.id, requestParameters.mergePersonDto, options).then((request) => request(this.axios, this.basePath)); + } + /** * * @param {PersonApiUpdatePersonRequest} requestParameters Request parameters. diff --git a/mobile/openapi/.openapi-generator/FILES b/mobile/openapi/.openapi-generator/FILES index 9862f98c4a..86742e468c 100644 --- a/mobile/openapi/.openapi-generator/FILES +++ b/mobile/openapi/.openapi-generator/FILES @@ -32,6 +32,7 @@ doc/AssetTypeEnum.md doc/AudioCodec.md doc/AuthDeviceResponseDto.md doc/AuthenticationApi.md +doc/BulkIdResponseDto.md doc/ChangePasswordDto.md doc/CheckDuplicateAssetDto.md doc/CheckDuplicateAssetResponseDto.md @@ -64,6 +65,7 @@ doc/LoginResponseDto.md doc/LogoutResponseDto.md doc/MapMarkerResponseDto.md doc/MemoryLaneResponseDto.md +doc/MergePersonDto.md doc/OAuthApi.md doc/OAuthCallbackDto.md doc/OAuthConfigDto.md @@ -169,6 +171,7 @@ lib/model/asset_response_dto.dart lib/model/asset_type_enum.dart lib/model/audio_codec.dart lib/model/auth_device_response_dto.dart +lib/model/bulk_id_response_dto.dart lib/model/change_password_dto.dart lib/model/check_duplicate_asset_dto.dart lib/model/check_duplicate_asset_response_dto.dart @@ -200,6 +203,7 @@ lib/model/login_response_dto.dart lib/model/logout_response_dto.dart lib/model/map_marker_response_dto.dart lib/model/memory_lane_response_dto.dart +lib/model/merge_person_dto.dart lib/model/o_auth_callback_dto.dart lib/model/o_auth_config_dto.dart lib/model/o_auth_config_response_dto.dart @@ -277,6 +281,7 @@ test/asset_type_enum_test.dart test/audio_codec_test.dart test/auth_device_response_dto_test.dart test/authentication_api_test.dart +test/bulk_id_response_dto_test.dart test/change_password_dto_test.dart test/check_duplicate_asset_dto_test.dart test/check_duplicate_asset_response_dto_test.dart @@ -309,6 +314,7 @@ test/login_response_dto_test.dart test/logout_response_dto_test.dart test/map_marker_response_dto_test.dart test/memory_lane_response_dto_test.dart +test/merge_person_dto_test.dart test/o_auth_api_test.dart test/o_auth_callback_dto_test.dart test/o_auth_config_dto_test.dart diff --git a/mobile/openapi/README.md b/mobile/openapi/README.md index a78726e089..98dc3fac7e 100644 Binary files a/mobile/openapi/README.md and b/mobile/openapi/README.md differ diff --git a/mobile/openapi/doc/BulkIdResponseDto.md b/mobile/openapi/doc/BulkIdResponseDto.md new file mode 100644 index 0000000000..ce07f262d3 Binary files /dev/null and b/mobile/openapi/doc/BulkIdResponseDto.md differ diff --git a/mobile/openapi/doc/MergePersonDto.md b/mobile/openapi/doc/MergePersonDto.md new file mode 100644 index 0000000000..606f389de1 Binary files /dev/null and b/mobile/openapi/doc/MergePersonDto.md differ diff --git a/mobile/openapi/doc/PersonApi.md b/mobile/openapi/doc/PersonApi.md index aa37a294e1..ee57d0c506 100644 Binary files a/mobile/openapi/doc/PersonApi.md and b/mobile/openapi/doc/PersonApi.md differ diff --git a/mobile/openapi/lib/api.dart b/mobile/openapi/lib/api.dart index 604f07f19a..099f5615c5 100644 Binary files a/mobile/openapi/lib/api.dart and b/mobile/openapi/lib/api.dart differ diff --git a/mobile/openapi/lib/api/person_api.dart b/mobile/openapi/lib/api/person_api.dart index 37f8bf8a30..3a53bd5ebf 100644 Binary files a/mobile/openapi/lib/api/person_api.dart and b/mobile/openapi/lib/api/person_api.dart differ diff --git a/mobile/openapi/lib/api_client.dart b/mobile/openapi/lib/api_client.dart index 4ddf1833ab..5855da8e82 100644 Binary files a/mobile/openapi/lib/api_client.dart and b/mobile/openapi/lib/api_client.dart differ diff --git a/mobile/openapi/lib/model/bulk_id_response_dto.dart b/mobile/openapi/lib/model/bulk_id_response_dto.dart new file mode 100644 index 0000000000..18fb45108d Binary files /dev/null and b/mobile/openapi/lib/model/bulk_id_response_dto.dart differ diff --git a/mobile/openapi/lib/model/merge_person_dto.dart b/mobile/openapi/lib/model/merge_person_dto.dart new file mode 100644 index 0000000000..9c4cba764b Binary files /dev/null and b/mobile/openapi/lib/model/merge_person_dto.dart differ diff --git a/mobile/openapi/test/bulk_id_response_dto_test.dart b/mobile/openapi/test/bulk_id_response_dto_test.dart new file mode 100644 index 0000000000..25baca9860 Binary files /dev/null and b/mobile/openapi/test/bulk_id_response_dto_test.dart differ diff --git a/mobile/openapi/test/merge_person_dto_test.dart b/mobile/openapi/test/merge_person_dto_test.dart new file mode 100644 index 0000000000..4a22063a81 Binary files /dev/null and b/mobile/openapi/test/merge_person_dto_test.dart differ diff --git a/mobile/openapi/test/person_api_test.dart b/mobile/openapi/test/person_api_test.dart index 33e78b8a7c..95482f63d3 100644 Binary files a/mobile/openapi/test/person_api_test.dart and b/mobile/openapi/test/person_api_test.dart differ diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index 8c700d6692..88a5e74270 100644 --- a/server/immich-openapi-specs.json +++ b/server/immich-openapi-specs.json @@ -2693,6 +2693,61 @@ ] } }, + "/person/{id}/merge": { + "post": { + "operationId": "mergePerson", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "format": "uuid", + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MergePersonDto" + } + } + } + }, + "responses": { + "201": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/BulkIdResponseDto" + } + } + } + } + } + }, + "tags": [ + "Person" + ], + "security": [ + { + "bearer": [] + }, + { + "cookie": [] + }, + { + "api_key": [] + } + ] + } + }, "/person/{id}/thumbnail": { "get": { "operationId": "getPersonThumbnail", @@ -4963,6 +5018,30 @@ "deviceOS" ] }, + "BulkIdResponseDto": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "success": { + "type": "boolean" + }, + "error": { + "type": "string", + "enum": [ + "duplicate", + "no_permission", + "not_found", + "unknown" + ] + } + }, + "required": [ + "id", + "success" + ] + }, "ChangePasswordDto": { "type": "object", "properties": { @@ -5756,6 +5835,21 @@ "assets" ] }, + "MergePersonDto": { + "type": "object", + "properties": { + "ids": { + "type": "array", + "items": { + "type": "string", + "format": "uuid" + } + } + }, + "required": [ + "ids" + ] + }, "OAuthCallbackDto": { "type": "object", "properties": { diff --git a/server/src/domain/asset/response-dto/asset-ids-response.dto.ts b/server/src/domain/asset/response-dto/asset-ids-response.dto.ts index 928bed24de..81672564af 100644 --- a/server/src/domain/asset/response-dto/asset-ids-response.dto.ts +++ b/server/src/domain/asset/response-dto/asset-ids-response.dto.ts @@ -1,11 +1,26 @@ +/** @deprecated Use `BulkIdResponseDto` instead */ export enum AssetIdErrorReason { DUPLICATE = 'duplicate', NO_PERMISSION = 'no_permission', NOT_FOUND = 'not_found', } +/** @deprecated Use `BulkIdResponseDto` instead */ export class AssetIdsResponseDto { assetId!: string; success!: boolean; error?: AssetIdErrorReason; } + +export enum BulkIdErrorReason { + DUPLICATE = 'duplicate', + NO_PERMISSION = 'no_permission', + NOT_FOUND = 'not_found', + UNKNOWN = 'unknown', +} + +export class BulkIdResponseDto { + id!: string; + success!: boolean; + error?: BulkIdErrorReason; +} diff --git a/server/src/domain/person/person.dto.ts b/server/src/domain/person/person.dto.ts index 1790697e4e..b8efa65c99 100644 --- a/server/src/domain/person/person.dto.ts +++ b/server/src/domain/person/person.dto.ts @@ -1,5 +1,6 @@ import { AssetFaceEntity, PersonEntity } from '@app/infra/entities'; import { IsOptional, IsString } from 'class-validator'; +import { ValidateUUID } from '../domain.util'; export class PersonUpdateDto { /** @@ -17,6 +18,11 @@ export class PersonUpdateDto { featureFaceAssetId?: string; } +export class MergePersonDto { + @ValidateUUID({ each: true }) + ids!: string[]; +} + export class PersonResponseDto { id!: string; name!: string; diff --git a/server/src/domain/person/person.repository.ts b/server/src/domain/person/person.repository.ts index 0f05e3d98b..3c8432be1e 100644 --- a/server/src/domain/person/person.repository.ts +++ b/server/src/domain/person/person.repository.ts @@ -6,11 +6,19 @@ export interface PersonSearchOptions { minimumFaceCount: number; } +export interface UpdateFacesData { + oldPersonId: string; + newPersonId: string; +} + export interface IPersonRepository { getAll(userId: string, options: PersonSearchOptions): Promise; getAllWithoutFaces(): Promise; getById(userId: string, personId: string): Promise; - getAssets(userId: string, id: string): Promise; + + getAssets(userId: string, personId: string): Promise; + prepareReassignFaces(data: UpdateFacesData): Promise; + reassignFaces(data: UpdateFacesData): Promise; create(entity: Partial): Promise; update(entity: Partial): Promise; diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index d598f1293f..c7eb08bfdd 100644 --- a/server/src/domain/person/person.service.spec.ts +++ b/server/src/domain/person/person.service.spec.ts @@ -8,7 +8,8 @@ import { newStorageRepositoryMock, personStub, } from '@test'; -import { IJobRepository, JobName } from '..'; +import { BulkIdErrorReason } from '../asset'; +import { IJobRepository, JobName } from '../job'; import { IStorageRepository } from '../storage'; import { PersonResponseDto } from './person.dto'; import { IPersonRepository } from './person.repository'; @@ -154,4 +155,85 @@ describe(PersonService.name, () => { }); }); }); + + describe('mergePerson', () => { + it('should merge two people', async () => { + personMock.getById.mockResolvedValueOnce(personStub.primaryPerson); + personMock.getById.mockResolvedValueOnce(personStub.mergePerson); + personMock.prepareReassignFaces.mockResolvedValue([]); + personMock.delete.mockResolvedValue(personStub.mergePerson); + + await expect(sut.mergePerson(authStub.admin, 'person-1', { ids: ['person-2'] })).resolves.toEqual([ + { id: 'person-2', success: true }, + ]); + + expect(personMock.prepareReassignFaces).toHaveBeenCalledWith({ + newPersonId: personStub.primaryPerson.id, + oldPersonId: personStub.mergePerson.id, + }); + + expect(personMock.reassignFaces).toHaveBeenCalledWith({ + newPersonId: personStub.primaryPerson.id, + oldPersonId: personStub.mergePerson.id, + }); + + expect(personMock.delete).toHaveBeenCalledWith(personStub.mergePerson); + }); + + it('should delete conflicting faces before merging', async () => { + personMock.getById.mockResolvedValue(personStub.primaryPerson); + personMock.getById.mockResolvedValue(personStub.mergePerson); + personMock.prepareReassignFaces.mockResolvedValue([assetEntityStub.image.id]); + + await expect(sut.mergePerson(authStub.admin, 'person-1', { ids: ['person-2'] })).resolves.toEqual([ + { id: 'person-2', success: true }, + ]); + + expect(personMock.prepareReassignFaces).toHaveBeenCalledWith({ + newPersonId: personStub.primaryPerson.id, + oldPersonId: personStub.mergePerson.id, + }); + + expect(jobMock.queue).toHaveBeenCalledWith({ + name: JobName.SEARCH_REMOVE_FACE, + data: { assetId: assetEntityStub.image.id, personId: personStub.mergePerson.id }, + }); + }); + + it('should throw an error when the primary person is not found', async () => { + personMock.getById.mockResolvedValue(null); + + await expect(sut.mergePerson(authStub.admin, 'person-1', { ids: ['person-2'] })).rejects.toBeInstanceOf( + BadRequestException, + ); + + expect(personMock.delete).not.toHaveBeenCalled(); + }); + + it('should handle invalid merge ids', async () => { + personMock.getById.mockResolvedValueOnce(personStub.primaryPerson); + personMock.getById.mockResolvedValueOnce(null); + + await expect(sut.mergePerson(authStub.admin, 'person-1', { ids: ['person-2'] })).resolves.toEqual([ + { id: 'person-2', success: false, error: BulkIdErrorReason.NOT_FOUND }, + ]); + + expect(personMock.prepareReassignFaces).not.toHaveBeenCalled(); + expect(personMock.reassignFaces).not.toHaveBeenCalled(); + expect(personMock.delete).not.toHaveBeenCalled(); + }); + + it('should handle an error reassigning faces', async () => { + personMock.getById.mockResolvedValue(personStub.primaryPerson); + personMock.getById.mockResolvedValue(personStub.mergePerson); + personMock.prepareReassignFaces.mockResolvedValue([assetEntityStub.image.id]); + personMock.reassignFaces.mockRejectedValue(new Error('update failed')); + + await expect(sut.mergePerson(authStub.admin, 'person-1', { ids: ['person-2'] })).resolves.toEqual([ + { id: 'person-2', success: false, error: BulkIdErrorReason.UNKNOWN }, + ]); + + expect(personMock.delete).not.toHaveBeenCalled(); + }); + }); }); diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index ce009f1434..87046c50f8 100644 --- a/server/src/domain/person/person.service.ts +++ b/server/src/domain/person/person.service.ts @@ -1,12 +1,11 @@ -import { PersonEntity } from '@app/infra/entities'; import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common'; -import { AssetResponseDto, mapAsset } from '../asset'; +import { AssetResponseDto, BulkIdErrorReason, BulkIdResponseDto, mapAsset } from '../asset'; import { AuthUserDto } from '../auth'; import { mimeTypes } from '../domain.constant'; import { IJobRepository, JobName } from '../job'; import { ImmichReadStream, IStorageRepository } from '../storage'; -import { mapPerson, PersonResponseDto, PersonUpdateDto } from './person.dto'; -import { IPersonRepository } from './person.repository'; +import { mapPerson, MergePersonDto, PersonResponseDto, PersonUpdateDto } from './person.dto'; +import { IPersonRepository, UpdateFacesData } from './person.repository'; @Injectable() export class PersonService { @@ -30,17 +29,12 @@ export class PersonService { ); } - async getById(authUser: AuthUserDto, personId: string): Promise { - const person = await this.repository.getById(authUser.id, personId); - if (!person) { - throw new BadRequestException(); - } - - return mapPerson(person); + getById(authUser: AuthUserDto, id: string): Promise { + return this.findOrFail(authUser, id).then(mapPerson); } - async getThumbnail(authUser: AuthUserDto, personId: string): Promise { - const person = await this.repository.getById(authUser.id, personId); + async getThumbnail(authUser: AuthUserDto, id: string): Promise { + const person = await this.repository.getById(authUser.id, id); if (!person || !person.thumbnailPath) { throw new NotFoundException(); } @@ -48,62 +42,48 @@ export class PersonService { return this.storageRepository.createReadStream(person.thumbnailPath, mimeTypes.lookup(person.thumbnailPath)); } - async getAssets(authUser: AuthUserDto, personId: string): Promise { - const assets = await this.repository.getAssets(authUser.id, personId); + async getAssets(authUser: AuthUserDto, id: string): Promise { + const assets = await this.repository.getAssets(authUser.id, id); return assets.map(mapAsset); } - async update(authUser: AuthUserDto, personId: string, dto: PersonUpdateDto): Promise { - let person = await this.repository.getById(authUser.id, personId); - if (!person) { - throw new BadRequestException(); - } + async update(authUser: AuthUserDto, id: string, dto: PersonUpdateDto): Promise { + let person = await this.findOrFail(authUser, id); if (dto.name) { - person = await this.updateName(authUser, personId, dto.name); + person = await this.repository.update({ id, name: dto.name }); + const assets = await this.repository.getAssets(authUser.id, id); + const ids = assets.map((asset) => asset.id); + await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ASSET, data: { ids } }); } if (dto.featureFaceAssetId) { - await this.updateFaceThumbnail(personId, dto.featureFaceAssetId); + const assetId = dto.featureFaceAssetId; + const face = await this.repository.getFaceById({ personId: id, assetId }); + if (!face) { + throw new BadRequestException('Invalid assetId for feature face'); + } + + await this.jobRepository.queue({ + name: JobName.GENERATE_FACE_THUMBNAIL, + data: { + personId: id, + assetId, + boundingBox: { + x1: face.boundingBoxX1, + x2: face.boundingBoxX2, + y1: face.boundingBoxY1, + y2: face.boundingBoxY2, + }, + imageHeight: face.imageHeight, + imageWidth: face.imageWidth, + }, + }); } return mapPerson(person); } - private async updateName(authUser: AuthUserDto, personId: string, name: string): Promise { - const person = await this.repository.update({ id: personId, name }); - - const relatedAsset = await this.getAssets(authUser, personId); - const assetIds = relatedAsset.map((asset) => asset.id); - await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ASSET, data: { ids: assetIds } }); - - return person; - } - - private async updateFaceThumbnail(personId: string, assetId: string): Promise { - const face = await this.repository.getFaceById({ assetId, personId }); - - if (!face) { - throw new BadRequestException(); - } - - return await this.jobRepository.queue({ - name: JobName.GENERATE_FACE_THUMBNAIL, - data: { - assetId: assetId, - personId, - boundingBox: { - x1: face.boundingBoxX1, - x2: face.boundingBoxX2, - y1: face.boundingBoxY1, - y2: face.boundingBoxY2, - }, - imageHeight: face.imageHeight, - imageWidth: face.imageWidth, - }, - }); - } - async handlePersonCleanup() { const people = await this.repository.getAllWithoutFaces(); for (const person of people) { @@ -118,4 +98,49 @@ export class PersonService { return true; } + + async mergePerson(authUser: AuthUserDto, id: string, dto: MergePersonDto): Promise { + const mergeIds = dto.ids; + const primaryPerson = await this.findOrFail(authUser, id); + const primaryName = primaryPerson.name || primaryPerson.id; + + const results: BulkIdResponseDto[] = []; + + for (const mergeId of mergeIds) { + try { + const mergePerson = await this.repository.getById(authUser.id, mergeId); + if (!mergePerson) { + results.push({ id: mergeId, success: false, error: BulkIdErrorReason.NOT_FOUND }); + continue; + } + + const mergeName = mergePerson.name || mergePerson.id; + const mergeData: UpdateFacesData = { oldPersonId: mergeId, newPersonId: id }; + this.logger.log(`Merging ${mergeName} into ${primaryName}`); + + const assetIds = await this.repository.prepareReassignFaces(mergeData); + for (const assetId of assetIds) { + await this.jobRepository.queue({ name: JobName.SEARCH_REMOVE_FACE, data: { assetId, personId: mergeId } }); + } + await this.repository.reassignFaces(mergeData); + await this.repository.delete(mergePerson); + + this.logger.log(`Merged ${mergeName} into ${primaryName}`); + results.push({ id: mergeId, success: true }); + } catch (error: Error | any) { + this.logger.error(`Unable to merge ${mergeId} into ${id}: ${error}`, error?.stack); + results.push({ id: mergeId, success: false, error: BulkIdErrorReason.UNKNOWN }); + } + } + + return results; + } + + private async findOrFail(authUser: AuthUserDto, id: string) { + const person = await this.repository.getById(authUser.id, id); + if (!person) { + throw new BadRequestException('Person not found'); + } + return person; + } } diff --git a/server/src/immich/controllers/person.controller.ts b/server/src/immich/controllers/person.controller.ts index 6eb58844f4..106961aa89 100644 --- a/server/src/immich/controllers/person.controller.ts +++ b/server/src/immich/controllers/person.controller.ts @@ -1,12 +1,14 @@ import { AssetResponseDto, AuthUserDto, + BulkIdResponseDto, ImmichReadStream, + MergePersonDto, PersonResponseDto, PersonService, PersonUpdateDto, } from '@app/domain'; -import { Body, Controller, Get, Param, Put, StreamableFile } from '@nestjs/common'; +import { Body, Controller, Get, Param, Post, Put, StreamableFile } from '@nestjs/common'; import { ApiOkResponse, ApiTags } from '@nestjs/swagger'; import { Authenticated, AuthUser } from '../app.guard'; import { UseValidation } from '../app.utils'; @@ -56,4 +58,13 @@ export class PersonController { getPersonAssets(@AuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto): Promise { return this.service.getAssets(authUser, id); } + + @Post(':id/merge') + mergePerson( + @AuthUser() authUser: AuthUserDto, + @Param() { id }: UUIDParamDto, + @Body() dto: MergePersonDto, + ): Promise { + return this.service.mergePerson(authUser, id, dto); + } } diff --git a/server/src/infra/repositories/person.repository.ts b/server/src/infra/repositories/person.repository.ts index c4a04acab1..db4ccff06b 100644 --- a/server/src/infra/repositories/person.repository.ts +++ b/server/src/infra/repositories/person.repository.ts @@ -1,6 +1,6 @@ -import { AssetFaceId, IPersonRepository, PersonSearchOptions } from '@app/domain'; +import { AssetFaceId, IPersonRepository, PersonSearchOptions, UpdateFacesData } from '@app/domain'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { In, Repository } from 'typeorm'; import { AssetEntity, AssetFaceEntity, PersonEntity } from '../entities'; export class PersonRepository implements IPersonRepository { @@ -10,6 +10,36 @@ export class PersonRepository implements IPersonRepository { @InjectRepository(AssetFaceEntity) private assetFaceRepository: Repository, ) {} + /** + * Before reassigning faces, delete potential key violations + */ + async prepareReassignFaces({ oldPersonId, newPersonId }: UpdateFacesData): Promise { + const results = await this.assetFaceRepository + .createQueryBuilder('face') + .select('face."assetId"') + .where(`face."personId" IN (:...ids)`, { ids: [oldPersonId, newPersonId] }) + .groupBy('face."assetId"') + .having('COUNT(face."personId") > 1') + .getRawMany(); + + const assetIds = results.map(({ assetId }) => assetId); + + await this.assetFaceRepository.delete({ personId: oldPersonId, assetId: In(assetIds) }); + + return assetIds; + } + + async reassignFaces({ oldPersonId, newPersonId }: UpdateFacesData): Promise { + const result = await this.assetFaceRepository + .createQueryBuilder() + .update() + .set({ personId: newPersonId }) + .where({ personId: oldPersonId }) + .execute(); + + return result.affected ?? 0; + } + delete(entity: PersonEntity): Promise { return this.personRepository.remove(entity); } diff --git a/server/test/fixtures.ts b/server/test/fixtures.ts index 1c6f87e02f..1452294073 100644 --- a/server/test/fixtures.ts +++ b/server/test/fixtures.ts @@ -327,6 +327,39 @@ export const assetEntityStub = { fileSizeInByte: 5_000, } as ExifEntity, }), + image1: Object.freeze({ + id: 'asset-id-1', + deviceAssetId: 'device-asset-id', + fileModifiedAt: new Date('2023-02-23T05:06:29.716Z'), + fileCreatedAt: new Date('2023-02-23T05:06:29.716Z'), + owner: userEntityStub.user1, + ownerId: 'user-id', + deviceId: 'device-id', + originalPath: '/original/path.ext', + resizePath: '/uploads/user-id/thumbs/path.ext', + checksum: Buffer.from('file hash', 'utf8'), + type: AssetType.IMAGE, + webpPath: '/uploads/user-id/webp/path.ext', + thumbhash: Buffer.from('blablabla', 'base64'), + encodedVideoPath: null, + createdAt: new Date('2023-02-23T05:06:29.716Z'), + updatedAt: new Date('2023-02-23T05:06:29.716Z'), + isFavorite: true, + isArchived: false, + isReadOnly: false, + duration: null, + isVisible: true, + livePhotoVideo: null, + livePhotoVideoId: null, + tags: [], + sharedLinks: [], + originalFileName: 'asset-id.ext', + faces: [], + sidecarPath: null, + exifInfo: { + fileSizeInByte: 5_000, + } as ExifEntity, + }), video: Object.freeze({ id: 'asset-id', originalFileName: 'asset-id.ext', @@ -1158,6 +1191,26 @@ export const personStub = { thumbnailPath: '/new/path/to/thumbnail.jpg', faces: [], }), + primaryPerson: Object.freeze({ + id: 'person-1', + createdAt: new Date('2021-01-01'), + updatedAt: new Date('2021-01-01'), + ownerId: userEntityStub.admin.id, + owner: userEntityStub.admin, + name: 'Person 1', + thumbnailPath: '/path/to/thumbnail', + faces: [], + }), + mergePerson: Object.freeze({ + id: 'person-2', + createdAt: new Date('2021-01-01'), + updatedAt: new Date('2021-01-01'), + ownerId: userEntityStub.admin.id, + owner: userEntityStub.admin, + name: 'Person 2', + thumbnailPath: '/path/to/thumbnail', + faces: [], + }), }; export const partnerStub = { @@ -1193,6 +1246,45 @@ export const faceStub = { imageHeight: 1024, imageWidth: 1024, }), + primaryFace1: Object.freeze({ + assetId: assetEntityStub.image.id, + asset: assetEntityStub.image, + personId: personStub.primaryPerson.id, + person: personStub.primaryPerson, + embedding: [1, 2, 3, 4], + boundingBoxX1: 0, + boundingBoxY1: 0, + boundingBoxX2: 1, + boundingBoxY2: 1, + imageHeight: 1024, + imageWidth: 1024, + }), + mergeFace1: Object.freeze({ + assetId: assetEntityStub.image.id, + asset: assetEntityStub.image, + personId: personStub.mergePerson.id, + person: personStub.mergePerson, + embedding: [1, 2, 3, 4], + boundingBoxX1: 0, + boundingBoxY1: 0, + boundingBoxX2: 1, + boundingBoxY2: 1, + imageHeight: 1024, + imageWidth: 1024, + }), + mergeFace2: Object.freeze({ + assetId: assetEntityStub.image1.id, + asset: assetEntityStub.image1, + personId: personStub.mergePerson.id, + person: personStub.mergePerson, + embedding: [1, 2, 3, 4], + boundingBoxX1: 0, + boundingBoxY1: 0, + boundingBoxX2: 1, + boundingBoxY2: 1, + imageHeight: 1024, + imageWidth: 1024, + }), }; export const tagStub = { diff --git a/server/test/repositories/person.repository.mock.ts b/server/test/repositories/person.repository.mock.ts index 68cd833edc..99fa6de3ed 100644 --- a/server/test/repositories/person.repository.mock.ts +++ b/server/test/repositories/person.repository.mock.ts @@ -13,5 +13,7 @@ export const newPersonRepositoryMock = (): jest.Mocked => { delete: jest.fn(), getFaceById: jest.fn(), + prepareReassignFaces: jest.fn(), + reassignFaces: jest.fn(), }; }; diff --git a/web/src/api/open-api/api.ts b/web/src/api/open-api/api.ts index da6a8d174b..3ca2b95a60 100644 --- a/web/src/api/open-api/api.ts +++ b/web/src/api/open-api/api.ts @@ -798,6 +798,41 @@ export interface AuthDeviceResponseDto { */ 'deviceOS': string; } +/** + * + * @export + * @interface BulkIdResponseDto + */ +export interface BulkIdResponseDto { + /** + * + * @type {string} + * @memberof BulkIdResponseDto + */ + 'id': string; + /** + * + * @type {boolean} + * @memberof BulkIdResponseDto + */ + 'success': boolean; + /** + * + * @type {string} + * @memberof BulkIdResponseDto + */ + 'error'?: BulkIdResponseDtoErrorEnum; +} + +export const BulkIdResponseDtoErrorEnum = { + Duplicate: 'duplicate', + NoPermission: 'no_permission', + NotFound: 'not_found', + Unknown: 'unknown' +} as const; + +export type BulkIdResponseDtoErrorEnum = typeof BulkIdResponseDtoErrorEnum[keyof typeof BulkIdResponseDtoErrorEnum]; + /** * * @export @@ -1686,6 +1721,19 @@ export interface MemoryLaneResponseDto { */ 'assets': Array; } +/** + * + * @export + * @interface MergePersonDto + */ +export interface MergePersonDto { + /** + * + * @type {Array} + * @memberof MergePersonDto + */ + 'ids': Array; +} /** * * @export @@ -8852,6 +8900,54 @@ export const PersonApiAxiosParamCreator = function (configuration?: Configuratio options: localVarRequestOptions, }; }, + /** + * + * @param {string} id + * @param {MergePersonDto} mergePersonDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + mergePerson: async (id: string, mergePersonDto: MergePersonDto, options: AxiosRequestConfig = {}): Promise => { + // verify required parameter 'id' is not null or undefined + assertParamExists('mergePerson', 'id', id) + // verify required parameter 'mergePersonDto' is not null or undefined + assertParamExists('mergePerson', 'mergePersonDto', mergePersonDto) + const localVarPath = `/person/{id}/merge` + .replace(`{${"id"}}`, encodeURIComponent(String(id))); + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { method: 'POST', ...baseOptions, ...options}; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + // authentication cookie required + + // authentication api_key required + await setApiKeyToObject(localVarHeaderParameter, "x-api-key", configuration) + + // authentication bearer required + // http bearer authentication required + await setBearerAuthToObject(localVarHeaderParameter, configuration) + + + + localVarHeaderParameter['Content-Type'] = 'application/json'; + + setSearchParams(localVarUrlObj, localVarQueryParameter); + let headersFromBaseOptions = baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; + localVarRequestOptions.data = serializeDataIfNeeded(mergePersonDto, localVarRequestOptions, configuration) + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, /** * * @param {string} id @@ -8949,6 +9045,17 @@ export const PersonApiFp = function(configuration?: Configuration) { const localVarAxiosArgs = await localVarAxiosParamCreator.getPersonThumbnail(id, options); return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); }, + /** + * + * @param {string} id + * @param {MergePersonDto} mergePersonDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async mergePerson(id: string, mergePersonDto: MergePersonDto, options?: AxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise>> { + const localVarAxiosArgs = await localVarAxiosParamCreator.mergePerson(id, mergePersonDto, options); + return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); + }, /** * * @param {string} id @@ -9005,6 +9112,16 @@ export const PersonApiFactory = function (configuration?: Configuration, basePat getPersonThumbnail(id: string, options?: any): AxiosPromise { return localVarFp.getPersonThumbnail(id, options).then((request) => request(axios, basePath)); }, + /** + * + * @param {string} id + * @param {MergePersonDto} mergePersonDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + mergePerson(id: string, mergePersonDto: MergePersonDto, options?: any): AxiosPromise> { + return localVarFp.mergePerson(id, mergePersonDto, options).then((request) => request(axios, basePath)); + }, /** * * @param {string} id @@ -9060,6 +9177,27 @@ export interface PersonApiGetPersonThumbnailRequest { readonly id: string } +/** + * Request parameters for mergePerson operation in PersonApi. + * @export + * @interface PersonApiMergePersonRequest + */ +export interface PersonApiMergePersonRequest { + /** + * + * @type {string} + * @memberof PersonApiMergePerson + */ + readonly id: string + + /** + * + * @type {MergePersonDto} + * @memberof PersonApiMergePerson + */ + readonly mergePersonDto: MergePersonDto +} + /** * Request parameters for updatePerson operation in PersonApi. * @export @@ -9131,6 +9269,17 @@ export class PersonApi extends BaseAPI { return PersonApiFp(this.configuration).getPersonThumbnail(requestParameters.id, options).then((request) => request(this.axios, this.basePath)); } + /** + * + * @param {PersonApiMergePersonRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof PersonApi + */ + public mergePerson(requestParameters: PersonApiMergePersonRequest, options?: AxiosRequestConfig) { + return PersonApiFp(this.configuration).mergePerson(requestParameters.id, requestParameters.mergePersonDto, options).then((request) => request(this.axios, this.basePath)); + } + /** * * @param {PersonApiUpdatePersonRequest} requestParameters Request parameters. diff --git a/web/src/lib/components/faces-page/face-thumbnail.svelte b/web/src/lib/components/faces-page/face-thumbnail.svelte new file mode 100644 index 0000000000..61ec188a31 --- /dev/null +++ b/web/src/lib/components/faces-page/face-thumbnail.svelte @@ -0,0 +1,66 @@ + + + diff --git a/web/src/lib/components/faces-page/merge-face-selector.svelte b/web/src/lib/components/faces-page/merge-face-selector.svelte new file mode 100644 index 0000000000..71c3f6bf8f --- /dev/null +++ b/web/src/lib/components/faces-page/merge-face-selector.svelte @@ -0,0 +1,145 @@ + + + + +
+ + + {#if hasSelection} + Selected {selectedPeople.length} + {:else} + Merge faces + {/if} +
+ + + + + +
+
+
+

Choose matching faces to merge

+ +
+ {#each selectedPeople as person (person.id)} +
+ onSelect(person)} /> +
+ {/each} + + {#if hasSelection} + + {/if} + +
+
+
+
+ {#each unselectedPeople as person (person.id)} + onSelect(person)} circle border selectable /> + {/each} +
+
+
+ + {#if isShowConfirmation} + (isShowConfirmation = false)} + > + +

Are you sure you want merge these faces?
This action is irreversible.

+ + + {/if} +
+
diff --git a/web/src/routes/(user)/explore/+page.svelte b/web/src/routes/(user)/explore/+page.svelte index 68f59d8870..09b2f0710f 100644 --- a/web/src/routes/(user)/explore/+page.svelte +++ b/web/src/routes/(user)/explore/+page.svelte @@ -38,7 +38,7 @@ {#if data.people.length > MAX_ITEMS} View All {/if} diff --git a/web/src/routes/(user)/people/+page.svelte b/web/src/routes/(user)/people/+page.svelte index b0f60a6d2a..745197adfa 100644 --- a/web/src/routes/(user)/people/+page.svelte +++ b/web/src/routes/(user)/people/+page.svelte @@ -15,7 +15,7 @@ {#each data.people as person (person.id)}
-
+
= new Set(); $: isMultiSelectionMode = selectedAssets.size > 0; $: isAllArchive = Array.from(selectedAssets).every((asset) => asset.isArchived); $: isAllFavorite = Array.from(selectedAssets).every((asset) => asset.isFavorite); + $: showAssets = !showMergeFacePanel && !showFaceThumbnailSelection; + afterNavigate(({ from }) => { // Prevent setting previousRoute to the current page. if (from && from.route.id !== $page.route.id) { @@ -64,7 +67,7 @@ }; const handleSelectFeaturePhoto = async (event: CustomEvent) => { - isSelectingFace = false; + showFaceThumbnailSelection = false; const { selectedAsset }: { selectedAsset: AssetResponseDto | undefined } = event.detail; @@ -102,7 +105,8 @@ goto(previousRoute)}> - (isSelectingFace = true)} /> + (showFaceThumbnailSelection = true)} /> + (showMergeFacePanel = true)} /> @@ -117,7 +121,7 @@ on:cancel={() => (isEditingName = false)} /> {:else} -