From 66fae76af24c78ae525ce1c6e5f54cf46d376c9b Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Wed, 17 Jul 2024 07:43:35 -0400 Subject: [PATCH] fix(server): delete large album (#11042) fix: large album asset operations --- server/src/decorators.ts | 11 +++-- server/src/interfaces/album.interface.ts | 2 +- server/src/repositories/album.repository.ts | 51 +++++++++++++++++---- server/src/services/album.service.spec.ts | 5 +- server/src/services/album.service.ts | 5 +- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/server/src/decorators.ts b/server/src/decorators.ts index f4fce96cdc..1c632e549a 100644 --- a/server/src/decorators.ts +++ b/server/src/decorators.ts @@ -49,23 +49,26 @@ function chunks(collection: Array | Set, size: number): Array> * @param options.paramIndex The index of the function parameter to chunk. Defaults to 0. * @param options.flatten Whether to flatten the results. Defaults to false. */ -export function Chunked(options: { paramIndex?: number; mergeFn?: (results: any) => any } = {}): MethodDecorator { +export function Chunked( + options: { paramIndex?: number; chunkSize?: number; mergeFn?: (results: any) => any } = {}, +): MethodDecorator { return (target: any, propertyKey: string | symbol, descriptor: PropertyDescriptor) => { const originalMethod = descriptor.value; const parameterIndex = options.paramIndex ?? 0; + const chunkSize = options.chunkSize || DATABASE_PARAMETER_CHUNK_SIZE; descriptor.value = async function (...arguments_: any[]) { const argument = arguments_[parameterIndex]; // Early return if argument length is less than or equal to the chunk size. if ( - (Array.isArray(argument) && argument.length <= DATABASE_PARAMETER_CHUNK_SIZE) || - (argument instanceof Set && argument.size <= DATABASE_PARAMETER_CHUNK_SIZE) + (Array.isArray(argument) && argument.length <= chunkSize) || + (argument instanceof Set && argument.size <= chunkSize) ) { return await originalMethod.apply(this, arguments_); } return Promise.all( - chunks(argument, DATABASE_PARAMETER_CHUNK_SIZE).map(async (chunk) => { + chunks(argument, chunkSize).map(async (chunk) => { await Reflect.apply(originalMethod, this, [ ...arguments_.slice(0, parameterIndex), chunk, diff --git a/server/src/interfaces/album.interface.ts b/server/src/interfaces/album.interface.ts index 9963612fd6..091442ff05 100644 --- a/server/src/interfaces/album.interface.ts +++ b/server/src/interfaces/album.interface.ts @@ -30,6 +30,6 @@ export interface IAlbumRepository extends IBulkAsset { getAll(): Promise; create(album: Partial): Promise; update(album: Partial): Promise; - delete(album: AlbumEntity): Promise; + delete(id: string): Promise; updateThumbnails(): Promise; } diff --git a/server/src/repositories/album.repository.ts b/server/src/repositories/album.repository.ts index a44b8dc071..fd3a89993a 100644 --- a/server/src/repositories/album.repository.ts +++ b/server/src/repositories/album.repository.ts @@ -5,7 +5,16 @@ import { AlbumEntity } from 'src/entities/album.entity'; import { AssetEntity } from 'src/entities/asset.entity'; import { AlbumAssetCount, AlbumInfoOptions, IAlbumRepository } from 'src/interfaces/album.interface'; import { Instrumentation } from 'src/utils/instrumentation'; -import { DataSource, FindOptionsOrder, FindOptionsRelations, In, IsNull, Not, Repository } from 'typeorm'; +import { + DataSource, + EntityManager, + FindOptionsOrder, + FindOptionsRelations, + In, + IsNull, + Not, + Repository, +} from 'typeorm'; const withoutDeletedUsers = (album: T) => { if (album) { @@ -255,24 +264,46 @@ export class AlbumRepository implements IAlbumRepository { @GenerateSql({ params: [DummyValue.UUID, [DummyValue.UUID]] }) async addAssetIds(albumId: string, assetIds: string[]): Promise { - await this.dataSource - .createQueryBuilder() - .insert() - .into('albums_assets_assets', ['albumsId', 'assetsId']) - .values(assetIds.map((assetId) => ({ albumsId: albumId, assetsId: assetId }))) - .execute(); + await this.addAssets(this.dataSource.manager, albumId, assetIds); } create(album: Partial): Promise { - return this.save(album); + return this.dataSource.transaction(async (manager) => { + const { id } = await manager.save(AlbumEntity, { ...album, assets: [] }); + const assetIds = (album.assets || []).map((asset) => asset.id); + await this.addAssets(manager, id, assetIds); + return manager.findOneOrFail(AlbumEntity, { + where: { id }, + relations: { + owner: true, + albumUsers: { user: true }, + sharedLinks: true, + assets: true, + }, + }); + }); } update(album: Partial): Promise { return this.save(album); } - async delete(album: AlbumEntity): Promise { - await this.repository.remove(album); + async delete(id: string): Promise { + await this.repository.delete({ id }); + } + + @Chunked({ paramIndex: 2, chunkSize: 30_000 }) + private async addAssets(manager: EntityManager, albumId: string, assetIds: string[]): Promise { + if (assetIds.length === 0) { + return; + } + + await manager + .createQueryBuilder() + .insert() + .into('albums_assets_assets', ['albumsId', 'assetsId']) + .values(assetIds.map((assetId) => ({ albumsId: albumId, assetsId: assetId }))) + .execute(); } private async save(album: Partial) { diff --git a/server/src/services/album.service.spec.ts b/server/src/services/album.service.spec.ts index 97549d89f1..4d6aca7a8b 100644 --- a/server/src/services/album.service.spec.ts +++ b/server/src/services/album.service.spec.ts @@ -302,8 +302,7 @@ describe(AlbumService.name, () => { describe('delete', () => { it('should throw an error for an album not found', async () => { - accessMock.album.checkOwnerAccess.mockResolvedValue(new Set([albumStub.sharedWithAdmin.id])); - albumMock.getById.mockResolvedValue(null); + accessMock.album.checkOwnerAccess.mockResolvedValue(new Set()); await expect(sut.delete(authStub.admin, albumStub.sharedWithAdmin.id)).rejects.toBeInstanceOf( BadRequestException, @@ -329,7 +328,7 @@ describe(AlbumService.name, () => { await sut.delete(authStub.admin, albumStub.empty.id); expect(albumMock.delete).toHaveBeenCalledTimes(1); - expect(albumMock.delete).toHaveBeenCalledWith(albumStub.empty); + expect(albumMock.delete).toHaveBeenCalledWith(albumStub.empty.id); }); }); diff --git a/server/src/services/album.service.ts b/server/src/services/album.service.ts index 7c86d74d45..9cd750e6b1 100644 --- a/server/src/services/album.service.ts +++ b/server/src/services/album.service.ts @@ -165,10 +165,7 @@ export class AlbumService { async delete(auth: AuthDto, id: string): Promise { await this.access.requirePermission(auth, Permission.ALBUM_DELETE, id); - - const album = await this.findOrFail(id, { withAssets: false }); - - await this.albumRepository.delete(album); + await this.albumRepository.delete(id); } async addAssets(auth: AuthDto, id: string, dto: BulkIdsDto): Promise {