feat(web/server): merge faces (#3121)

* feat(server/web): Merge faces

* get parent id

* update

* query to get identical asset and change controller

* change delete asset signature

* delete identical assets

* gaming time

* delete merge person

* query

* query

* generate api

* pr feedback

* generate api

* naming

* remove unused method

* Update server/src/domain/person/person.service.ts

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

* Update server/src/domain/person/person.service.ts

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

* better method signature

* cleaning up

* fix bug

* added interfaces

* added tests

* merge main

* api

* build merge face interface

* api

* selector interface

* style

* more style

* clean up import

* styling

* styling

* better

* styling

* styling

* add merge face diablog

* finished

* refactor: merge person endpoint

* refactor: merge person component

* chore: open api

* fix: tests

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
This commit is contained in:
Alex
2023-07-11 16:52:41 -05:00
committed by GitHub
parent 848ba685eb
commit c86b2ae500
30 changed files with 1478 additions and 71 deletions

View File

@@ -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": {

View File

@@ -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;
}

View File

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

View File

@@ -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<PersonEntity[]>;
getAllWithoutFaces(): Promise<PersonEntity[]>;
getById(userId: string, personId: string): Promise<PersonEntity | null>;
getAssets(userId: string, id: string): Promise<AssetEntity[]>;
getAssets(userId: string, personId: string): Promise<AssetEntity[]>;
prepareReassignFaces(data: UpdateFacesData): Promise<string[]>;
reassignFaces(data: UpdateFacesData): Promise<number>;
create(entity: Partial<PersonEntity>): Promise<PersonEntity>;
update(entity: Partial<PersonEntity>): Promise<PersonEntity>;

View File

@@ -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();
});
});
});

View File

@@ -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<PersonResponseDto> {
const person = await this.repository.getById(authUser.id, personId);
if (!person) {
throw new BadRequestException();
}
return mapPerson(person);
getById(authUser: AuthUserDto, id: string): Promise<PersonResponseDto> {
return this.findOrFail(authUser, id).then(mapPerson);
}
async getThumbnail(authUser: AuthUserDto, personId: string): Promise<ImmichReadStream> {
const person = await this.repository.getById(authUser.id, personId);
async getThumbnail(authUser: AuthUserDto, id: string): Promise<ImmichReadStream> {
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<AssetResponseDto[]> {
const assets = await this.repository.getAssets(authUser.id, personId);
async getAssets(authUser: AuthUserDto, id: string): Promise<AssetResponseDto[]> {
const assets = await this.repository.getAssets(authUser.id, id);
return assets.map(mapAsset);
}
async update(authUser: AuthUserDto, personId: string, dto: PersonUpdateDto): Promise<PersonResponseDto> {
let person = await this.repository.getById(authUser.id, personId);
if (!person) {
throw new BadRequestException();
}
async update(authUser: AuthUserDto, id: string, dto: PersonUpdateDto): Promise<PersonResponseDto> {
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<PersonEntity> {
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<void> {
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<BulkIdResponseDto[]> {
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;
}
}

View File

@@ -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<AssetResponseDto[]> {
return this.service.getAssets(authUser, id);
}
@Post(':id/merge')
mergePerson(
@AuthUser() authUser: AuthUserDto,
@Param() { id }: UUIDParamDto,
@Body() dto: MergePersonDto,
): Promise<BulkIdResponseDto[]> {
return this.service.mergePerson(authUser, id, dto);
}
}

View File

@@ -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<AssetFaceEntity>,
) {}
/**
* Before reassigning faces, delete potential key violations
*/
async prepareReassignFaces({ oldPersonId, newPersonId }: UpdateFacesData): Promise<string[]> {
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<number> {
const result = await this.assetFaceRepository
.createQueryBuilder()
.update()
.set({ personId: newPersonId })
.where({ personId: oldPersonId })
.execute();
return result.affected ?? 0;
}
delete(entity: PersonEntity): Promise<PersonEntity | null> {
return this.personRepository.remove(entity);
}

View File

@@ -327,6 +327,39 @@ export const assetEntityStub = {
fileSizeInByte: 5_000,
} as ExifEntity,
}),
image1: Object.freeze<AssetEntity>({
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<AssetEntity>({
id: 'asset-id',
originalFileName: 'asset-id.ext',
@@ -1158,6 +1191,26 @@ export const personStub = {
thumbnailPath: '/new/path/to/thumbnail.jpg',
faces: [],
}),
primaryPerson: Object.freeze<PersonEntity>({
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<PersonEntity>({
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<AssetFaceEntity>({
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<AssetFaceEntity>({
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<AssetFaceEntity>({
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 = {

View File

@@ -13,5 +13,7 @@ export const newPersonRepositoryMock = (): jest.Mocked<IPersonRepository> => {
delete: jest.fn(),
getFaceById: jest.fn(),
prepareReassignFaces: jest.fn(),
reassignFaces: jest.fn(),
};
};