perf(server): optimize getByIds query (#7918)

* clean up usage

* i'm not updating all these tests

* update tests

* add indices

* add indices to entities

remove index from person entity

add to face entity

fix

* simplify query

* update sql

* missing await

* remove synchronize false
This commit is contained in:
Mert 2024-03-14 01:58:09 -04:00 committed by GitHub
parent d67cc00e4e
commit ee8e8a0c0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 120 additions and 43 deletions

View File

@ -164,7 +164,7 @@ describe(DownloadService.name, () => {
const assetIds = ['asset-1', 'asset-2'];
await expect(sut.getDownloadInfo(authStub.admin, { assetIds })).resolves.toEqual(downloadResponse);
expect(assetMock.getByIds).toHaveBeenCalledWith(['asset-1', 'asset-2']);
expect(assetMock.getByIds).toHaveBeenCalledWith(['asset-1', 'asset-2'], { exifInfo: true });
});
it('should return a list of archives (albumId)', async () => {
@ -228,10 +228,10 @@ describe(DownloadService.name, () => {
accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(assetIds));
when(assetMock.getByIds)
.calledWith([assetStub.livePhotoStillAsset.id])
.calledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true })
.mockResolvedValue([assetStub.livePhotoStillAsset]);
when(assetMock.getByIds)
.calledWith([assetStub.livePhotoMotionAsset.id])
.calledWith([assetStub.livePhotoMotionAsset.id], { exifInfo: true })
.mockResolvedValue([assetStub.livePhotoMotionAsset]);
await expect(sut.getDownloadInfo(authStub.admin, { assetIds })).resolves.toEqual({

View File

@ -50,7 +50,7 @@ export class DownloadService {
// motion part of live photos
const motionIds = assets.map((asset) => asset.livePhotoVideoId).filter<string>((id): id is string => !!id);
if (motionIds.length > 0) {
assets.push(...(await this.assetRepository.getByIds(motionIds)));
assets.push(...(await this.assetRepository.getByIds(motionIds, { exifInfo: true })));
}
for (const asset of assets) {
@ -114,7 +114,7 @@ export class DownloadService {
if (dto.assetIds) {
const assetIds = dto.assetIds;
await this.access.requirePermission(auth, Permission.ASSET_DOWNLOAD, assetIds);
const assets = await this.assetRepository.getByIds(assetIds);
const assets = await this.assetRepository.getByIds(assetIds, { exifInfo: true });
return usePagination(PAGINATION_SIZE, () => ({ hasNextPage: false, items: assets }));
}

View File

@ -330,8 +330,6 @@ describe(JobService.name, () => {
} else {
assetMock.getByIds.mockResolvedValue([assetStub.livePhotoMotionAsset]);
}
} else {
assetMock.getByIds.mockResolvedValue([]);
}
await sut.init(makeMockHandlers(true));

View File

@ -214,7 +214,7 @@ export class JobService {
case JobName.METADATA_EXTRACTION: {
if (item.data.source === 'sidecar-write') {
const [asset] = await this.assetRepository.getByIds([item.data.id]);
const [asset] = await this.assetRepository.getByIdsWithAllRelations([item.data.id]);
if (asset) {
this.communicationRepository.send(ClientEvent.ASSET_UPDATE, asset.ownerId, mapAsset(asset));
}
@ -272,7 +272,7 @@ export class JobService {
break;
}
const [asset] = await this.assetRepository.getByIds([item.data.id]);
const [asset] = await this.assetRepository.getByIdsWithAllRelations([item.data.id]);
// Only live-photo motion part will be marked as not visible immediately on upload. Skip notifying clients
if (asset && asset.isVisible) {

View File

@ -165,7 +165,7 @@ export class MediaService {
}
async handleGenerateJpegThumbnail({ id }: IEntityJob) {
const [asset] = await this.assetRepository.getByIds([id]);
const [asset] = await this.assetRepository.getByIds([id], { exifInfo: true });
if (!asset) {
return false;
}
@ -215,7 +215,7 @@ export class MediaService {
}
async handleGenerateWebpThumbnail({ id }: IEntityJob) {
const [asset] = await this.assetRepository.getByIds([id]);
const [asset] = await this.assetRepository.getByIds([id], { exifInfo: true });
if (!asset) {
return false;
}

View File

@ -114,7 +114,7 @@ describe(MetadataService.name, () => {
describe('handleLivePhotoLinking', () => {
it('should handle an asset that could not be found', async () => {
await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(false);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(assetMock.findLivePhotoMatch).not.toHaveBeenCalled();
expect(assetMock.save).not.toHaveBeenCalled();
expect(albumMock.removeAsset).not.toHaveBeenCalled();
@ -124,7 +124,7 @@ describe(MetadataService.name, () => {
assetMock.getByIds.mockResolvedValue([{ ...assetStub.image, exifInfo: undefined }]);
await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(false);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(assetMock.findLivePhotoMatch).not.toHaveBeenCalled();
expect(assetMock.save).not.toHaveBeenCalled();
expect(albumMock.removeAsset).not.toHaveBeenCalled();
@ -134,7 +134,7 @@ describe(MetadataService.name, () => {
assetMock.getByIds.mockResolvedValue([{ ...assetStub.image }]);
await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(assetMock.findLivePhotoMatch).not.toHaveBeenCalled();
expect(assetMock.save).not.toHaveBeenCalled();
expect(albumMock.removeAsset).not.toHaveBeenCalled();
@ -149,7 +149,7 @@ describe(MetadataService.name, () => {
]);
await expect(sut.handleLivePhotoLinking({ id: assetStub.livePhotoMotionAsset.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id], { exifInfo: true });
expect(assetMock.findLivePhotoMatch).toHaveBeenCalledWith({
livePhotoCID: assetStub.livePhotoStillAsset.id,
ownerId: assetStub.livePhotoMotionAsset.ownerId,
@ -170,7 +170,7 @@ describe(MetadataService.name, () => {
assetMock.findLivePhotoMatch.mockResolvedValue(assetStub.livePhotoMotionAsset);
await expect(sut.handleLivePhotoLinking({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true });
expect(assetMock.findLivePhotoMatch).toHaveBeenCalledWith({
livePhotoCID: assetStub.livePhotoMotionAsset.id,
ownerId: assetStub.livePhotoStillAsset.ownerId,

View File

@ -153,7 +153,7 @@ export class MetadataService {
async handleLivePhotoLinking(job: IEntityJob) {
const { id } = job;
const [asset] = await this.assetRepository.getByIds([id]);
const [asset] = await this.assetRepository.getByIds([id], { exifInfo: true });
if (!asset?.exifInfo) {
return false;
}

View File

@ -121,6 +121,7 @@ export interface IAssetRepository {
relations?: FindOptionsRelations<AssetEntity>,
select?: FindOptionsSelect<AssetEntity>,
): Promise<AssetEntity[]>;
getByIdsWithAllRelations(ids: string[]): Promise<AssetEntity[]>;
getByDayOfYear(ownerId: string, monthDay: MonthDay): Promise<AssetEntity[]>;
getByChecksum(userId: string, checksum: Buffer): Promise<AssetEntity | null>;
getByAlbumId(pagination: PaginationOptions, albumId: string): Paginated<AssetEntity>;

View File

@ -76,7 +76,7 @@ describe(SearchService.name, () => {
fieldName: 'smartInfo.tags',
items: [{ value: 'train', data: assetStub.imageFrom2015.id }],
});
assetMock.getByIds.mockResolvedValueOnce([assetStub.image, assetStub.imageFrom2015]);
assetMock.getByIdsWithAllRelations.mockResolvedValueOnce([assetStub.image, assetStub.imageFrom2015]);
const expectedResponse = [
{ fieldName: 'exifInfo.city', items: [{ value: 'Paris', data: mapAsset(assetStub.image) }] },
{ fieldName: 'smartInfo.tags', items: [{ value: 'train', data: mapAsset(assetStub.imageFrom2015) }] },

View File

@ -60,7 +60,7 @@ export class SearchService {
this.assetRepository.getAssetIdByTag(auth.user.id, options),
]);
const assetIds = new Set<string>(results.flatMap((field) => field.items.map((item) => item.data)));
const assets = await this.assetRepository.getByIds([...assetIds]);
const assets = await this.assetRepository.getByIdsWithAllRelations([...assetIds]);
const assetMap = new Map<string, AssetResponseDto>(assets.map((asset) => [asset.id, mapAsset(asset)]));
return results.map(({ fieldName, items }) => ({

View File

@ -76,6 +76,10 @@ export class SmartInfoService {
}
const [asset] = await this.assetRepository.getByIds([id]);
if (!asset) {
return false;
}
if (!asset.resizePath) {
return false;
}

View File

@ -101,11 +101,11 @@ describe(StorageTemplateService.name, () => {
.mockResolvedValue(assetStub.livePhotoMotionAsset);
when(assetMock.getByIds)
.calledWith([assetStub.livePhotoStillAsset.id])
.calledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true })
.mockResolvedValue([assetStub.livePhotoStillAsset]);
when(assetMock.getByIds)
.calledWith([assetStub.livePhotoMotionAsset.id])
.calledWith([assetStub.livePhotoMotionAsset.id], { exifInfo: true })
.mockResolvedValue([assetStub.livePhotoMotionAsset]);
when(moveMock.create)
@ -140,8 +140,8 @@ describe(StorageTemplateService.name, () => {
await expect(sut.handleMigrationSingle({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true });
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id], { exifInfo: true });
expect(storageMock.checkFileExists).toHaveBeenCalledTimes(2);
expect(assetMock.save).toHaveBeenCalledWith({
id: assetStub.livePhotoStillAsset.id,
@ -172,7 +172,9 @@ describe(StorageTemplateService.name, () => {
.calledWith({ id: assetStub.image.id, originalPath: newPath })
.mockResolvedValue(assetStub.image);
when(assetMock.getByIds).calledWith([assetStub.image.id]).mockResolvedValue([assetStub.image]);
when(assetMock.getByIds)
.calledWith([assetStub.image.id], { exifInfo: true })
.mockResolvedValue([assetStub.image]);
when(moveMock.update)
.calledWith({
@ -190,7 +192,7 @@ describe(StorageTemplateService.name, () => {
await expect(sut.handleMigrationSingle({ id: assetStub.image.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(storageMock.checkFileExists).toHaveBeenCalledTimes(3);
expect(storageMock.rename).toHaveBeenCalledWith(assetStub.image.originalPath, newPath);
expect(moveMock.update).toHaveBeenCalledWith({
@ -227,7 +229,9 @@ describe(StorageTemplateService.name, () => {
.calledWith({ id: assetStub.image.id, originalPath: newPath })
.mockResolvedValue(assetStub.image);
when(assetMock.getByIds).calledWith([assetStub.image.id]).mockResolvedValue([assetStub.image]);
when(assetMock.getByIds)
.calledWith([assetStub.image.id], { exifInfo: true })
.mockResolvedValue([assetStub.image]);
when(moveMock.update)
.calledWith({
@ -245,7 +249,7 @@ describe(StorageTemplateService.name, () => {
await expect(sut.handleMigrationSingle({ id: assetStub.image.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(storageMock.checkFileExists).toHaveBeenCalledTimes(3);
expect(storageMock.stat).toHaveBeenCalledWith(previousFailedNewPath);
expect(storageMock.rename).toHaveBeenCalledWith(previousFailedNewPath, newPath);
@ -275,7 +279,9 @@ describe(StorageTemplateService.name, () => {
.calledWith({ id: assetStub.image.id, originalPath: newPath })
.mockResolvedValue(assetStub.image);
when(assetMock.getByIds).calledWith([assetStub.image.id]).mockResolvedValue([assetStub.image]);
when(assetMock.getByIds)
.calledWith([assetStub.image.id], { exifInfo: true })
.mockResolvedValue([assetStub.image]);
when(moveMock.create)
.calledWith({
@ -294,7 +300,7 @@ describe(StorageTemplateService.name, () => {
await expect(sut.handleMigrationSingle({ id: assetStub.image.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(storageMock.checkFileExists).toHaveBeenCalledTimes(1);
expect(storageMock.stat).toHaveBeenCalledWith(newPath);
expect(moveMock.create).toHaveBeenCalledWith({
@ -340,7 +346,9 @@ describe(StorageTemplateService.name, () => {
.calledWith({ id: assetStub.image.id, originalPath: newPath })
.mockResolvedValue(assetStub.image);
when(assetMock.getByIds).calledWith([assetStub.image.id]).mockResolvedValue([assetStub.image]);
when(assetMock.getByIds)
.calledWith([assetStub.image.id], { exifInfo: true })
.mockResolvedValue([assetStub.image]);
when(moveMock.update)
.calledWith({
@ -358,7 +366,7 @@ describe(StorageTemplateService.name, () => {
await expect(sut.handleMigrationSingle({ id: assetStub.image.id })).resolves.toBe(true);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
expect(storageMock.checkFileExists).toHaveBeenCalledTimes(3);
expect(storageMock.stat).toHaveBeenCalledWith(previousFailedNewPath);
expect(storageMock.rename).not.toHaveBeenCalled();

View File

@ -92,7 +92,10 @@ export class StorageTemplateService {
return true;
}
const [asset] = await this.assetRepository.getByIds([id]);
const [asset] = await this.assetRepository.getByIds([id], { exifInfo: true });
if (!asset) {
return false;
}
const user = await this.userRepository.get(asset.ownerId, {});
const storageLabel = user?.storageLabel || null;
@ -101,7 +104,10 @@ export class StorageTemplateService {
// move motion part of live photo
if (asset.livePhotoVideoId) {
const [livePhotoVideo] = await this.assetRepository.getByIds([asset.livePhotoVideoId]);
const [livePhotoVideo] = await this.assetRepository.getByIds([asset.livePhotoVideoId], { exifInfo: true });
if (!livePhotoVideo) {
return false;
}
const motionFilename = getLivePhotoMotionFilename(filename, livePhotoVideo.originalPath);
await this.moveAsset(livePhotoVideo, { storageLabel, filename: motionFilename });
}

View File

@ -3,6 +3,7 @@ import { AssetEntity } from './asset.entity';
import { PersonEntity } from './person.entity';
@Entity('asset_faces', { synchronize: false })
@Index('IDX_asset_faces_assetId_personId', ['assetId', 'personId'])
@Index(['personId', 'assetId'])
export class AssetFaceEntity {
@PrimaryGeneratedColumn('uuid')

View File

@ -35,6 +35,7 @@ export const ASSET_CHECKSUM_CONSTRAINT = 'UQ_assets_owner_library_checksum';
@Index('IDX_day_of_month', { synchronize: false })
@Index('IDX_month', { synchronize: false })
@Index('IDX_originalPath_libraryId', ['originalPath', 'libraryId'])
@Index('IDX_asset_id_stackId', ['id', 'stackId'])
@Index('idx_originalFileName_trigram', { synchronize: false })
// For all assets, each originalpath must be unique per user and library
export class AssetEntity {
@ -145,7 +146,7 @@ export class AssetEntity {
smartSearch?: SmartSearchEntity;
@ManyToMany(() => TagEntity, (tag) => tag.assets, { cascade: true })
@JoinTable({ name: 'tag_asset' })
@JoinTable({ name: 'tag_asset', synchronize: false })
tags!: TagEntity[];
@ManyToMany(() => SharedLinkEntity, (link) => link.assets, { cascade: true })

View File

@ -0,0 +1,15 @@
import { MigrationInterface, QueryRunner } from "typeorm";
export class AddAssetRelationIndices1710293990203 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`CREATE INDEX "IDX_asset_id_stackId" on assets ("id", "stackId")`);
await queryRunner.query(`CREATE INDEX "IDX_tag_asset_assetsId_tagsId" on tag_asset ("assetsId", "tagsId")`);
await queryRunner.query(`CREATE INDEX "IDX_asset_faces_assetId_personId" on asset_faces ("assetId", "personId")`);
}
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`DROP INDEX "IDX_asset_id_stackId" on assets ("id", "stackId")`);
await queryRunner.query(`DROP INDEX "IDX_tag_asset_assetsId_tagsId" on tag_asset ("assetsId", "tagsId")`);
await queryRunner.query(`DROP INDEX "IDX_asset_faces_assetId_personId" on asset_faces ("assetId", "personId")`);
}
}

View File

@ -137,8 +137,20 @@ export class AssetRepository implements IAssetRepository {
relations?: FindOptionsRelations<AssetEntity>,
select?: FindOptionsSelect<AssetEntity>,
): Promise<AssetEntity[]> {
if (!relations) {
relations = {
return this.repository.find({
where: { id: In(ids) },
relations,
select,
withDeleted: true,
});
}
@GenerateSql({ params: [[DummyValue.UUID]] })
@ChunkedArray()
getByIdsWithAllRelations(ids: string[]): Promise<AssetEntity[]> {
return this.repository.find({
where: { id: In(ids) },
relations: {
exifInfo: true,
smartInfo: true,
tags: true,
@ -148,13 +160,7 @@ export class AssetRepository implements IAssetRepository {
stack: {
assets: true,
},
};
}
return this.repository.find({
where: { id: In(ids) },
relations,
select,
},
withDeleted: true,
});
}

View File

@ -160,6 +160,42 @@ ORDER BY
"entity"."localDateTime" DESC
-- AssetRepository.getByIds
SELECT
"AssetEntity"."id" AS "AssetEntity_id",
"AssetEntity"."deviceAssetId" AS "AssetEntity_deviceAssetId",
"AssetEntity"."ownerId" AS "AssetEntity_ownerId",
"AssetEntity"."libraryId" AS "AssetEntity_libraryId",
"AssetEntity"."deviceId" AS "AssetEntity_deviceId",
"AssetEntity"."type" AS "AssetEntity_type",
"AssetEntity"."originalPath" AS "AssetEntity_originalPath",
"AssetEntity"."resizePath" AS "AssetEntity_resizePath",
"AssetEntity"."webpPath" AS "AssetEntity_webpPath",
"AssetEntity"."thumbhash" AS "AssetEntity_thumbhash",
"AssetEntity"."encodedVideoPath" AS "AssetEntity_encodedVideoPath",
"AssetEntity"."createdAt" AS "AssetEntity_createdAt",
"AssetEntity"."updatedAt" AS "AssetEntity_updatedAt",
"AssetEntity"."deletedAt" AS "AssetEntity_deletedAt",
"AssetEntity"."fileCreatedAt" AS "AssetEntity_fileCreatedAt",
"AssetEntity"."localDateTime" AS "AssetEntity_localDateTime",
"AssetEntity"."fileModifiedAt" AS "AssetEntity_fileModifiedAt",
"AssetEntity"."isFavorite" AS "AssetEntity_isFavorite",
"AssetEntity"."isArchived" AS "AssetEntity_isArchived",
"AssetEntity"."isExternal" AS "AssetEntity_isExternal",
"AssetEntity"."isReadOnly" AS "AssetEntity_isReadOnly",
"AssetEntity"."isOffline" AS "AssetEntity_isOffline",
"AssetEntity"."checksum" AS "AssetEntity_checksum",
"AssetEntity"."duration" AS "AssetEntity_duration",
"AssetEntity"."isVisible" AS "AssetEntity_isVisible",
"AssetEntity"."livePhotoVideoId" AS "AssetEntity_livePhotoVideoId",
"AssetEntity"."originalFileName" AS "AssetEntity_originalFileName",
"AssetEntity"."sidecarPath" AS "AssetEntity_sidecarPath",
"AssetEntity"."stackId" AS "AssetEntity_stackId"
FROM
"assets" "AssetEntity"
WHERE
(("AssetEntity"."id" IN ($1)))
-- AssetRepository.getByIdsWithAllRelations
SELECT
"AssetEntity"."id" AS "AssetEntity_id",
"AssetEntity"."deviceAssetId" AS "AssetEntity_deviceAssetId",

View File

@ -8,6 +8,7 @@ export const newAssetRepositoryMock = (): jest.Mocked<IAssetRepository> => {
getByDate: jest.fn(),
getByDayOfYear: jest.fn(),
getByIds: jest.fn().mockResolvedValue([]),
getByIdsWithAllRelations: jest.fn().mockResolvedValue([]),
getByAlbumId: jest.fn(),
getByUserId: jest.fn(),
getById: jest.fn(),