diff --git a/server/src/domain/album/album.service.spec.ts b/server/src/domain/album/album.service.spec.ts index 453539129d..25b616ad6d 100644 --- a/server/src/domain/album/album.service.spec.ts +++ b/server/src/domain/album/album.service.spec.ts @@ -460,7 +460,7 @@ describe(AlbumService.name, () => { accessMock.album.hasOwnerAccess.mockResolvedValue(true); accessMock.asset.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.oneAsset)); - albumMock.hasAsset.mockResolvedValue(false); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect( sut.addAssets(authStub.admin, 'album-123', { ids: ['asset-1', 'asset-2', 'asset-3'] }), @@ -485,6 +485,7 @@ describe(AlbumService.name, () => { accessMock.album.hasOwnerAccess.mockResolvedValue(true); accessMock.asset.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep({ ...albumStub.empty, albumThumbnailAssetId: 'asset-id' })); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect(sut.addAssets(authStub.admin, 'album-123', { ids: ['asset-1'] })).resolves.toEqual([ { success: true, id: 'asset-1' }, @@ -503,6 +504,7 @@ describe(AlbumService.name, () => { accessMock.album.hasSharedAlbumAccess.mockResolvedValue(true); accessMock.asset.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.sharedWithUser)); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect( sut.addAssets(authStub.user1, 'album-123', { ids: ['asset-1', 'asset-2', 'asset-3'] }), @@ -529,7 +531,7 @@ describe(AlbumService.name, () => { accessMock.album.hasSharedLinkAccess.mockResolvedValue(true); accessMock.asset.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.oneAsset)); - albumMock.hasAsset.mockResolvedValue(false); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect( sut.addAssets(authStub.adminSharedLink, 'album-123', { ids: ['asset-1', 'asset-2', 'asset-3'] }), @@ -560,7 +562,7 @@ describe(AlbumService.name, () => { accessMock.asset.hasOwnerAccess.mockResolvedValue(false); accessMock.asset.hasPartnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.oneAsset)); - albumMock.hasAsset.mockResolvedValue(false); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect(sut.addAssets(authStub.admin, 'album-123', { ids: ['asset-1'] })).resolves.toEqual([ { success: true, id: 'asset-1' }, @@ -578,7 +580,7 @@ describe(AlbumService.name, () => { accessMock.album.hasOwnerAccess.mockResolvedValue(true); accessMock.asset.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.oneAsset)); - albumMock.hasAsset.mockResolvedValue(true); + albumMock.getAssetIds.mockResolvedValueOnce(new Set(['asset-id'])); await expect(sut.addAssets(authStub.admin, 'album-123', { ids: ['asset-id'] })).resolves.toEqual([ { success: false, id: 'asset-id', error: BulkIdErrorReason.DUPLICATE }, @@ -592,6 +594,7 @@ describe(AlbumService.name, () => { accessMock.asset.hasOwnerAccess.mockResolvedValue(false); accessMock.asset.hasPartnerAccess.mockResolvedValue(false); albumMock.getById.mockResolvedValue(albumStub.oneAsset); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect(sut.addAssets(authStub.admin, 'album-123', { ids: ['asset-1'] })).resolves.toEqual([ { success: false, id: 'asset-1', error: BulkIdErrorReason.NO_PERMISSION }, @@ -630,7 +633,7 @@ describe(AlbumService.name, () => { it('should allow the owner to remove assets', async () => { accessMock.album.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.oneAsset)); - albumMock.hasAsset.mockResolvedValue(true); + albumMock.getAssetIds.mockResolvedValueOnce(new Set(['asset-id'])); await expect(sut.removeAssets(authStub.admin, 'album-123', { ids: ['asset-id'] })).resolves.toEqual([ { success: true, id: 'asset-id' }, @@ -643,6 +646,7 @@ describe(AlbumService.name, () => { it('should skip assets not in the album', async () => { accessMock.album.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.empty)); + albumMock.getAssetIds.mockResolvedValueOnce(new Set()); await expect(sut.removeAssets(authStub.admin, 'album-123', { ids: ['asset-id'] })).resolves.toEqual([ { success: false, id: 'asset-id', error: BulkIdErrorReason.NOT_FOUND }, @@ -654,7 +658,7 @@ describe(AlbumService.name, () => { it('should skip assets without user permission to remove', async () => { accessMock.album.hasSharedAlbumAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.oneAsset)); - albumMock.hasAsset.mockResolvedValue(true); + albumMock.getAssetIds.mockResolvedValueOnce(new Set(['asset-id'])); await expect(sut.removeAssets(authStub.admin, 'album-123', { ids: ['asset-id'] })).resolves.toEqual([ { @@ -670,7 +674,7 @@ describe(AlbumService.name, () => { it('should reset the thumbnail if it is removed', async () => { accessMock.album.hasOwnerAccess.mockResolvedValue(true); albumMock.getById.mockResolvedValue(_.cloneDeep(albumStub.twoAssets)); - albumMock.hasAsset.mockResolvedValue(true); + albumMock.getAssetIds.mockResolvedValueOnce(new Set(['asset-id'])); await expect(sut.removeAssets(authStub.admin, 'album-123', { ids: ['asset-id'] })).resolves.toEqual([ { success: true, id: 'asset-id' }, diff --git a/server/src/domain/album/album.service.ts b/server/src/domain/album/album.service.ts index e6f1edfa77..ecff9f49d7 100644 --- a/server/src/domain/album/album.service.ts +++ b/server/src/domain/album/album.service.ts @@ -152,9 +152,11 @@ export class AlbumService { await this.access.requirePermission(authUser, Permission.ALBUM_READ, id); + const existingAssetIds = await this.albumRepository.getAssetIds(id, dto.ids); + const results: BulkIdResponseDto[] = []; for (const assetId of dto.ids) { - const hasAsset = await this.albumRepository.hasAsset({ albumId: id, assetId }); + const hasAsset = existingAssetIds.has(assetId); if (hasAsset) { results.push({ id: assetId, success: false, error: BulkIdErrorReason.DUPLICATE }); continue; @@ -187,9 +189,11 @@ export class AlbumService { await this.access.requirePermission(authUser, Permission.ALBUM_READ, id); + const existingAssetIds = await this.albumRepository.getAssetIds(id, dto.ids); + const results: BulkIdResponseDto[] = []; for (const assetId of dto.ids) { - const hasAsset = await this.albumRepository.hasAsset({ albumId: id, assetId }); + const hasAsset = existingAssetIds.has(assetId); if (!hasAsset) { results.push({ id: assetId, success: false, error: BulkIdErrorReason.NOT_FOUND }); continue; diff --git a/server/src/domain/repositories/album.repository.ts b/server/src/domain/repositories/album.repository.ts index 276ab796bd..d3ca62da12 100644 --- a/server/src/domain/repositories/album.repository.ts +++ b/server/src/domain/repositories/album.repository.ts @@ -26,6 +26,7 @@ export interface IAlbumRepository { getByIds(ids: string[]): Promise; getByAssetId(ownerId: string, assetId: string): Promise; addAssets(assets: AlbumAssets): Promise; + getAssetIds(albumId: string, assetIds?: string[]): Promise>; hasAsset(asset: AlbumAsset): Promise; removeAsset(assetId: string): Promise; removeAssets(assets: AlbumAssets): Promise; diff --git a/server/src/infra/repositories/album.repository.ts b/server/src/infra/repositories/album.repository.ts index a8cd504146..69df226859 100644 --- a/server/src/infra/repositories/album.repository.ts +++ b/server/src/infra/repositories/album.repository.ts @@ -183,6 +183,28 @@ export class AlbumRepository implements IAlbumRepository { .execute(); } + /** + * Get asset IDs for the given album ID. + * + * @param albumId Album ID to get asset IDs for. + * @param assetIds Optional list of asset IDs to filter on. + * @returns Set of Asset IDs for the given album ID. + */ + async getAssetIds(albumId: string, assetIds?: string[]): Promise> { + const query = this.dataSource + .createQueryBuilder() + .select('albums_assets.assetsId', 'assetId') + .from('albums_assets_assets', 'albums_assets') + .where('"albums_assets"."albumsId" = :albumId', { albumId }); + + if (assetIds?.length) { + query.andWhere('"albums_assets"."assetsId" IN (:...assetIds)', { assetIds }); + } + + const result = await query.getRawMany(); + return new Set(result.map((row) => row['assetId'])); + } + hasAsset(asset: AlbumAsset): Promise { return this.repository.exist({ where: { diff --git a/server/test/repositories/album.repository.mock.ts b/server/test/repositories/album.repository.mock.ts index 20c3552699..7cd0a846b3 100644 --- a/server/test/repositories/album.repository.mock.ts +++ b/server/test/repositories/album.repository.mock.ts @@ -17,6 +17,7 @@ export const newAlbumRepositoryMock = (): jest.Mocked => { addAssets: jest.fn(), removeAsset: jest.fn(), removeAssets: jest.fn(), + getAssetIds: jest.fn(), hasAsset: jest.fn(), create: jest.fn(), update: jest.fn(),