diff --git a/server/apps/immich/src/api-v1/album/album-repository.ts b/server/apps/immich/src/api-v1/album/album-repository.ts index 070a4d31f9..a369cdfa59 100644 --- a/server/apps/immich/src/api-v1/album/album-repository.ts +++ b/server/apps/immich/src/api-v1/album/album-repository.ts @@ -6,18 +6,15 @@ import { Repository } from 'typeorm'; import { AddAssetsDto } from './dto/add-assets.dto'; import { AddUsersDto } from './dto/add-users.dto'; import { RemoveAssetsDto } from './dto/remove-assets.dto'; -import { UpdateAlbumDto } from '@app/domain'; import { AlbumCountResponseDto } from './response-dto/album-count-response.dto'; import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto'; export interface IAlbumRepository { get(albumId: string): Promise; - delete(album: AlbumEntity): Promise; addSharedUsers(album: AlbumEntity, addUsersDto: AddUsersDto): Promise; removeUser(album: AlbumEntity, userId: string): Promise; removeAssets(album: AlbumEntity, removeAssets: RemoveAssetsDto): Promise; addAssets(album: AlbumEntity, addAssetsDto: AddAssetsDto): Promise; - updateAlbum(album: AlbumEntity, updateAlbumDto: UpdateAlbumDto): Promise; updateThumbnails(): Promise; getCountByUserId(userId: string): Promise; getSharedWithUserAlbumCount(userId: string, assetId: string): Promise; @@ -62,10 +59,6 @@ export class AlbumRepository implements IAlbumRepository { }); } - async delete(album: AlbumEntity): Promise { - await this.albumRepository.delete({ id: album.id, ownerId: album.ownerId }); - } - async addSharedUsers(album: AlbumEntity, addUsersDto: AddUsersDto): Promise { album.sharedUsers.push(...addUsersDto.sharedUserIds.map((id) => ({ id } as UserEntity))); album.updatedAt = new Date().toISOString(); @@ -128,13 +121,6 @@ export class AlbumRepository implements IAlbumRepository { }; } - updateAlbum(album: AlbumEntity, updateAlbumDto: UpdateAlbumDto): Promise { - album.albumName = updateAlbumDto.albumName || album.albumName; - album.albumThumbnailAssetId = updateAlbumDto.albumThumbnailAssetId || album.albumThumbnailAssetId; - - return this.albumRepository.save(album); - } - /** * Makes sure all thumbnails for albums are updated by: * - Removing thumbnails from albums without assets diff --git a/server/apps/immich/src/api-v1/album/album.controller.ts b/server/apps/immich/src/api-v1/album/album.controller.ts index ee8214e995..df4366944e 100644 --- a/server/apps/immich/src/api-v1/album/album.controller.ts +++ b/server/apps/immich/src/api-v1/album/album.controller.ts @@ -77,12 +77,6 @@ export class AlbumController { return this.service.removeAssets(authUser, id, dto); } - @Authenticated() - @Delete(':id') - deleteAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto) { - return this.service.delete(authUser, id); - } - @Authenticated() @Delete(':id/user/:userId') removeUserFromAlbum( diff --git a/server/apps/immich/src/api-v1/album/album.service.spec.ts b/server/apps/immich/src/api-v1/album/album.service.spec.ts index 63d26178e7..e1b1def7f9 100644 --- a/server/apps/immich/src/api-v1/album/album.service.spec.ts +++ b/server/apps/immich/src/api-v1/album/album.service.spec.ts @@ -121,11 +121,9 @@ describe('Album service', () => { albumRepositoryMock = { addAssets: jest.fn(), addSharedUsers: jest.fn(), - delete: jest.fn(), get: jest.fn(), removeAssets: jest.fn(), removeUser: jest.fn(), - updateAlbum: jest.fn(), updateThumbnails: jest.fn(), getCountByUserId: jest.fn(), getSharedWithUserAlbumCount: jest.fn(), @@ -197,21 +195,6 @@ describe('Album service', () => { await expect(sut.get(authUser, '0002')).rejects.toBeInstanceOf(NotFoundException); }); - it('deletes an owned album', async () => { - const albumEntity = _getOwnedAlbum(); - albumRepositoryMock.get.mockImplementation(() => Promise.resolve(albumEntity)); - albumRepositoryMock.delete.mockImplementation(() => Promise.resolve()); - await sut.delete(authUser, albumId); - expect(albumRepositoryMock.delete).toHaveBeenCalledTimes(1); - expect(albumRepositoryMock.delete).toHaveBeenCalledWith(albumEntity); - }); - - it('prevents deleting a shared album (shared with auth user)', async () => { - const albumEntity = _getSharedWithAuthUserAlbum(); - albumRepositoryMock.get.mockImplementation(() => Promise.resolve(albumEntity)); - await expect(sut.delete(authUser, albumId)).rejects.toBeInstanceOf(ForbiddenException); - }); - it('removes a shared user from an owned album', async () => { const albumEntity = _getOwnedSharedAlbum(); albumRepositoryMock.get.mockImplementation(() => Promise.resolve(albumEntity)); diff --git a/server/apps/immich/src/api-v1/album/album.service.ts b/server/apps/immich/src/api-v1/album/album.service.ts index 78af9dc894..abebc0996e 100644 --- a/server/apps/immich/src/api-v1/album/album.service.ts +++ b/server/apps/immich/src/api-v1/album/album.service.ts @@ -3,7 +3,7 @@ import { AuthUserDto } from '../../decorators/auth-user.decorator'; import { AlbumEntity, SharedLinkType } from '@app/infra/entities'; import { AddUsersDto } from './dto/add-users.dto'; import { RemoveAssetsDto } from './dto/remove-assets.dto'; -import { AlbumResponseDto, IJobRepository, JobName, mapAlbum } from '@app/domain'; +import { AlbumResponseDto, IJobRepository, mapAlbum } from '@app/domain'; import { IAlbumRepository } from './album-repository'; import { AlbumCountResponseDto } from './response-dto/album-count-response.dto'; import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto'; @@ -64,17 +64,6 @@ export class AlbumService { return mapAlbum(updatedAlbum); } - async delete(authUser: AuthUserDto, albumId: string): Promise { - const album = await this._getAlbum({ authUser, albumId }); - - for (const sharedLink of album.sharedLinks) { - await this.shareCore.remove(authUser.id, sharedLink.id); - } - - await this.albumRepository.delete(album); - await this.jobRepository.queue({ name: JobName.SEARCH_REMOVE_ALBUM, data: { ids: [albumId] } }); - } - async removeUser(authUser: AuthUserDto, albumId: string, userId: string | 'me'): Promise { const sharedUserId = userId == 'me' ? authUser.id : userId; const album = await this._getAlbum({ authUser, albumId, validateIsOwner: false }); diff --git a/server/apps/immich/src/controllers/album.controller.ts b/server/apps/immich/src/controllers/album.controller.ts index b40d47544a..abb9447f11 100644 --- a/server/apps/immich/src/controllers/album.controller.ts +++ b/server/apps/immich/src/controllers/album.controller.ts @@ -1,6 +1,6 @@ /* */ import { AlbumService, AuthUserDto, CreateAlbumDto, UpdateAlbumDto } from '@app/domain'; import { GetAlbumsDto } from '@app/domain/album/dto/get-albums.dto'; -import { Body, Controller, Get, Param, Patch, Post, Query } from '@nestjs/common'; +import { Body, Controller, Delete, Get, Param, Patch, Post, Query } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { GetAuthUser } from '../decorators/auth-user.decorator'; import { Authenticated } from '../decorators/authenticated.decorator'; @@ -15,7 +15,7 @@ export class AlbumController { constructor(private service: AlbumService) {} @Get() - async getAllAlbums(@GetAuthUser() authUser: AuthUserDto, @Query() query: GetAlbumsDto) { + getAllAlbums(@GetAuthUser() authUser: AuthUserDto, @Query() query: GetAlbumsDto) { return this.service.getAll(authUser, query); } @@ -28,4 +28,9 @@ export class AlbumController { updateAlbumInfo(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: UpdateAlbumDto) { return this.service.update(authUser, id, dto); } + + @Delete(':id') + deleteAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto) { + return this.service.delete(authUser, id); + } } diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index d3d718606f..ce3c4d2fb3 100644 --- a/server/immich-openapi-specs.json +++ b/server/immich-openapi-specs.json @@ -143,7 +143,31 @@ }, { "api_key": [] - }, + } + ] + }, + "delete": { + "operationId": "deleteAlbum", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "format": "uuid", + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "" + } + }, + "tags": [ + "Album" + ], + "security": [ { "bearer": [] }, @@ -202,39 +226,6 @@ "api_key": [] } ] - }, - "delete": { - "operationId": "deleteAlbum", - "parameters": [ - { - "name": "id", - "required": true, - "in": "path", - "schema": { - "format": "uuid", - "type": "string" - } - } - ], - "responses": { - "200": { - "description": "" - } - }, - "tags": [ - "Album" - ], - "security": [ - { - "bearer": [] - }, - { - "cookie": [] - }, - { - "api_key": [] - } - ] } }, "/api-key": { diff --git a/server/libs/domain/src/album/album.repository.ts b/server/libs/domain/src/album/album.repository.ts index efdbcfa993..e9f2b4c368 100644 --- a/server/libs/domain/src/album/album.repository.ts +++ b/server/libs/domain/src/album/album.repository.ts @@ -20,4 +20,5 @@ export interface IAlbumRepository { getAll(): Promise; create(album: Partial): Promise; update(album: Partial): Promise; + delete(album: AlbumEntity): Promise; } diff --git a/server/libs/domain/src/album/album.service.spec.ts b/server/libs/domain/src/album/album.service.spec.ts index 18c12c1a33..81dcd9926c 100644 --- a/server/libs/domain/src/album/album.service.spec.ts +++ b/server/libs/domain/src/album/album.service.spec.ts @@ -176,7 +176,22 @@ describe(AlbumService.name, () => { ).rejects.toBeInstanceOf(ForbiddenException); }); - it('should all the owner to update the album', async () => { + it('should require a valid thumbnail asset id', async () => { + albumMock.getByIds.mockResolvedValue([albumStub.oneAsset]); + albumMock.update.mockResolvedValue(albumStub.oneAsset); + albumMock.hasAsset.mockResolvedValue(false); + + await expect( + sut.update(authStub.admin, albumStub.oneAsset.id, { + albumThumbnailAssetId: 'not-in-album', + }), + ).rejects.toBeInstanceOf(BadRequestException); + + expect(albumMock.hasAsset).toHaveBeenCalledWith(albumStub.oneAsset.id, 'not-in-album'); + expect(albumMock.update).not.toHaveBeenCalled(); + }); + + it('should allow the owner to update the album', async () => { albumMock.getByIds.mockResolvedValue([albumStub.oneAsset]); albumMock.update.mockResolvedValue(albumStub.oneAsset); @@ -195,4 +210,33 @@ describe(AlbumService.name, () => { }); }); }); + + describe('delete', () => { + it('should throw an error for an album not found', async () => { + albumMock.getByIds.mockResolvedValue([]); + + await expect(sut.delete(authStub.admin, albumStub.sharedWithAdmin.id)).rejects.toBeInstanceOf( + BadRequestException, + ); + + expect(albumMock.delete).not.toHaveBeenCalled(); + }); + + it('should not let a shared user delete the album', async () => { + albumMock.getByIds.mockResolvedValue([albumStub.sharedWithAdmin]); + + await expect(sut.delete(authStub.admin, albumStub.sharedWithAdmin.id)).rejects.toBeInstanceOf(ForbiddenException); + + expect(albumMock.delete).not.toHaveBeenCalled(); + }); + + it('should let the owner delete an album', async () => { + albumMock.getByIds.mockResolvedValue([albumStub.empty]); + + await sut.delete(authStub.admin, albumStub.empty.id); + + expect(albumMock.delete).toHaveBeenCalledTimes(1); + expect(albumMock.delete).toHaveBeenCalledWith(albumStub.empty); + }); + }); }); diff --git a/server/libs/domain/src/album/album.service.ts b/server/libs/domain/src/album/album.service.ts index d56c576d3e..c587e217e9 100644 --- a/server/libs/domain/src/album/album.service.ts +++ b/server/libs/domain/src/album/album.service.ts @@ -98,4 +98,18 @@ export class AlbumService { return mapAlbum(updatedAlbum); } + + async delete(authUser: AuthUserDto, id: string): Promise { + const [album] = await this.albumRepository.getByIds([id]); + if (!album) { + throw new BadRequestException('Album not found'); + } + + if (album.ownerId !== authUser.id) { + throw new ForbiddenException('Album not owned by user'); + } + + await this.albumRepository.delete(album); + await this.jobRepository.queue({ name: JobName.SEARCH_REMOVE_ALBUM, data: { ids: [id] } }); + } } diff --git a/server/libs/domain/test/album.repository.mock.ts b/server/libs/domain/test/album.repository.mock.ts index 944e062257..593ae61d26 100644 --- a/server/libs/domain/test/album.repository.mock.ts +++ b/server/libs/domain/test/album.repository.mock.ts @@ -14,5 +14,6 @@ export const newAlbumRepositoryMock = (): jest.Mocked => { hasAsset: jest.fn(), create: jest.fn(), update: jest.fn(), + delete: jest.fn(), }; }; diff --git a/server/libs/infra/src/entities/shared-link.entity.ts b/server/libs/infra/src/entities/shared-link.entity.ts index 1995d70f08..231b3a9660 100644 --- a/server/libs/infra/src/entities/shared-link.entity.ts +++ b/server/libs/infra/src/entities/shared-link.entity.ts @@ -53,7 +53,7 @@ export class SharedLinkEntity { assets!: AssetEntity[]; @Index('IDX_sharedlink_albumId') - @ManyToOne(() => AlbumEntity, (album) => album.sharedLinks) + @ManyToOne(() => AlbumEntity, (album) => album.sharedLinks, { onDelete: 'CASCADE', onUpdate: 'CASCADE' }) album?: AlbumEntity; } diff --git a/server/libs/infra/src/migrations/1685044328272-AddSharedLinkCascade.ts b/server/libs/infra/src/migrations/1685044328272-AddSharedLinkCascade.ts new file mode 100644 index 0000000000..3aefd3989e --- /dev/null +++ b/server/libs/infra/src/migrations/1685044328272-AddSharedLinkCascade.ts @@ -0,0 +1,16 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class AddSharedLinkCascade1685044328272 implements MigrationInterface { + name = 'AddSharedLinkCascade1685044328272' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "shared_links" DROP CONSTRAINT "FK_0c6ce9058c29f07cdf7014eac66"`); + await queryRunner.query(`ALTER TABLE "shared_links" ADD CONSTRAINT "FK_0c6ce9058c29f07cdf7014eac66" FOREIGN KEY ("albumId") REFERENCES "albums"("id") ON DELETE CASCADE ON UPDATE CASCADE`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "shared_links" DROP CONSTRAINT "FK_0c6ce9058c29f07cdf7014eac66"`); + await queryRunner.query(`ALTER TABLE "shared_links" ADD CONSTRAINT "FK_0c6ce9058c29f07cdf7014eac66" FOREIGN KEY ("albumId") REFERENCES "albums"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`); + } + +} diff --git a/server/libs/infra/src/repositories/album.repository.ts b/server/libs/infra/src/repositories/album.repository.ts index 9d8aef7b4b..6d0ba76478 100644 --- a/server/libs/infra/src/repositories/album.repository.ts +++ b/server/libs/infra/src/repositories/album.repository.ts @@ -143,10 +143,14 @@ export class AlbumRepository implements IAlbumRepository { return this.save(album); } - async update(album: Partial) { + async update(album: Partial): Promise { return this.save(album); } + async delete(album: AlbumEntity): Promise { + await this.repository.remove(album); + } + private async save(album: Partial) { const { id } = await this.repository.save(album); return this.repository.findOneOrFail({ where: { id }, relations: { owner: true } });