From d827a6182be10f1d9362e599ddb33a61c3775be5 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Wed, 24 May 2023 22:10:45 -0400 Subject: [PATCH] refactor: create album (#2555) --- .../src/api-v1/album/album-repository.ts | 15 ----- .../src/api-v1/album/album.controller.ts | 8 --- .../src/api-v1/album/album.service.spec.ts | 14 ----- .../immich/src/api-v1/album/album.service.ts | 7 --- .../src/controllers/album.controller.ts | 11 +++- server/apps/immich/test/album.e2e-spec.ts | 2 +- server/immich-openapi-specs.json | 50 ++++++++-------- .../libs/domain/src/album/album.repository.ts | 1 + .../domain/src/album/album.service.spec.ts | 60 +++++++++++++++---- server/libs/domain/src/album/album.service.ts | 22 ++++++- .../domain/src/album/dto/album-create.dto.ts} | 2 +- .../domain/src/album/dto/get-albums.dto.ts | 6 +- server/libs/domain/src/album/dto/index.ts | 2 + server/libs/domain/src/album/index.ts | 1 + .../libs/domain/test/album.repository.mock.ts | 1 + .../src/repositories/album.repository.ts | 6 +- 16 files changed, 117 insertions(+), 91 deletions(-) rename server/{apps/immich/src/api-v1/album/dto/create-album.dto.ts => libs/domain/src/album/dto/album-create.dto.ts} (78%) create mode 100644 server/libs/domain/src/album/dto/index.ts 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 b6c40271cd..e349948011 100644 --- a/server/apps/immich/src/api-v1/album/album-repository.ts +++ b/server/apps/immich/src/api-v1/album/album-repository.ts @@ -5,14 +5,12 @@ import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { AddAssetsDto } from './dto/add-assets.dto'; import { AddUsersDto } from './dto/add-users.dto'; -import { CreateAlbumDto } from './dto/create-album.dto'; import { RemoveAssetsDto } from './dto/remove-assets.dto'; import { UpdateAlbumDto } from './dto/update-album.dto'; import { AlbumCountResponseDto } from './response-dto/album-count-response.dto'; import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto'; export interface IAlbumRepository { - create(ownerId: string, createAlbumDto: CreateAlbumDto): Promise; get(albumId: string): Promise; delete(album: AlbumEntity): Promise; addSharedUsers(album: AlbumEntity, addUsersDto: AddUsersDto): Promise; @@ -45,19 +43,6 @@ export class AlbumRepository implements IAlbumRepository { return new AlbumCountResponseDto(ownedAlbums.length, sharedAlbums, sharedAlbumCount); } - async create(ownerId: string, dto: CreateAlbumDto): Promise { - const album = await this.albumRepository.save({ - ownerId, - albumName: dto.albumName, - sharedUsers: dto.sharedWithUserIds?.map((value) => ({ id: value } as UserEntity)) ?? [], - assets: dto.assetIds?.map((value) => ({ id: value } as AssetEntity)) ?? [], - albumThumbnailAssetId: dto.assetIds?.[0] || null, - }); - - // need to re-load the relations - return this.get(album.id) as Promise; - } - async get(albumId: string): Promise { return this.albumRepository.findOne({ where: { id: albumId }, 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 373c58a177..846154f557 100644 --- a/server/apps/immich/src/api-v1/album/album.controller.ts +++ b/server/apps/immich/src/api-v1/album/album.controller.ts @@ -1,7 +1,6 @@ import { Controller, Get, Post, Body, Patch, Param, Delete, Put, Query, Response } from '@nestjs/common'; import { ParseMeUUIDPipe } from '../validation/parse-me-uuid-pipe'; import { AlbumService } from './album.service'; -import { CreateAlbumDto } from './dto/create-album.dto'; import { Authenticated } from '../../decorators/authenticated.decorator'; import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; import { AddAssetsDto } from './dto/add-assets.dto'; @@ -44,13 +43,6 @@ export class AlbumController { return this.service.getCountByUserId(authUser); } - @Authenticated() - @Post() - createAlbum(@GetAuthUser() authUser: AuthUserDto, @Body() dto: CreateAlbumDto) { - // TODO: Handle nonexistent sharedWithUserIds and assetIds. - return this.service.create(authUser, dto); - } - @Authenticated() @Put(':id/users') addUsersToAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: AddUsersDto) { 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 95e5fadda1..6ecc675833 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,7 +121,6 @@ describe('Album service', () => { albumRepositoryMock = { addAssets: jest.fn(), addSharedUsers: jest.fn(), - create: jest.fn(), delete: jest.fn(), get: jest.fn(), removeAssets: jest.fn(), @@ -150,19 +149,6 @@ describe('Album service', () => { ); }); - it('creates album', async () => { - const albumEntity = _getOwnedAlbum(); - albumRepositoryMock.create.mockImplementation(() => Promise.resolve(albumEntity)); - - const result = await sut.create(authUser, { - albumName: albumEntity.albumName, - }); - - expect(result.id).toEqual(albumEntity.id); - expect(result.albumName).toEqual(albumEntity.albumName); - expect(jobMock.queue).toHaveBeenCalledWith({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [albumEntity.id] } }); - }); - it('gets an owned album', async () => { const albumId = 'f19ab956-4761-41ea-a5d6-bae948308d58'; 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 651b87b59c..8f49e19a8a 100644 --- a/server/apps/immich/src/api-v1/album/album.service.ts +++ b/server/apps/immich/src/api-v1/album/album.service.ts @@ -1,6 +1,5 @@ import { BadRequestException, Inject, Injectable, NotFoundException, ForbiddenException, Logger } from '@nestjs/common'; import { AuthUserDto } from '../../decorators/auth-user.decorator'; -import { CreateAlbumDto } from './dto/create-album.dto'; import { AlbumEntity, SharedLinkType } from '@app/infra/entities'; import { AddUsersDto } from './dto/add-users.dto'; import { RemoveAssetsDto } from './dto/remove-assets.dto'; @@ -55,12 +54,6 @@ export class AlbumService { return album; } - async create(authUser: AuthUserDto, createAlbumDto: CreateAlbumDto): Promise { - const albumEntity = await this.albumRepository.create(authUser.id, createAlbumDto); - await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [albumEntity.id] } }); - return mapAlbum(albumEntity); - } - async get(authUser: AuthUserDto, albumId: string): Promise { const album = await this._getAlbum({ authUser, albumId, validateIsOwner: false }); return mapAlbum(album); diff --git a/server/apps/immich/src/controllers/album.controller.ts b/server/apps/immich/src/controllers/album.controller.ts index ec8ecd28e1..5b883b3b64 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 } from '@app/domain'; +import { AlbumService, AuthUserDto, CreateAlbumDto } from '@app/domain'; import { GetAlbumsDto } from '@app/domain/album/dto/get-albums.dto'; -import { Controller, Get, Query } from '@nestjs/common'; +import { Body, Controller, Get, Post, Query } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { GetAuthUser } from '../decorators/auth-user.decorator'; import { Authenticated } from '../decorators/authenticated.decorator'; @@ -15,6 +15,11 @@ export class AlbumController { @Get() async getAllAlbums(@GetAuthUser() authUser: AuthUserDto, @Query() query: GetAlbumsDto) { - return this.service.getAllAlbums(authUser, query); + return this.service.getAll(authUser, query); + } + + @Post() + createAlbum(@GetAuthUser() authUser: AuthUserDto, @Body() dto: CreateAlbumDto) { + return this.service.create(authUser, dto); } } diff --git a/server/apps/immich/test/album.e2e-spec.ts b/server/apps/immich/test/album.e2e-spec.ts index 7346fc6efd..36bce93d28 100644 --- a/server/apps/immich/test/album.e2e-spec.ts +++ b/server/apps/immich/test/album.e2e-spec.ts @@ -2,7 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { INestApplication } from '@nestjs/common'; import request from 'supertest'; import { clearDb, getAuthUser, authCustom } from './test-utils'; -import { CreateAlbumDto } from '../src/api-v1/album/dto/create-album.dto'; +import { CreateAlbumDto } from '@app/domain'; import { CreateAlbumShareLinkDto } from '../src/api-v1/album/dto/create-album-shared-link.dto'; import { AuthUserDto } from '../src/decorators/auth-user.decorator'; import { AlbumResponseDto, AuthService, SharedLinkResponseDto, UserService } from '@app/domain'; diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index 9d971801c5..64a586d23c 100644 --- a/server/immich-openapi-specs.json +++ b/server/immich-openapi-specs.json @@ -4553,6 +4553,31 @@ "owner" ] }, + "CreateAlbumDto": { + "type": "object", + "properties": { + "albumName": { + "type": "string" + }, + "sharedWithUserIds": { + "type": "array", + "items": { + "type": "string", + "format": "uuid" + } + }, + "assetIds": { + "type": "array", + "items": { + "type": "string", + "format": "uuid" + } + } + }, + "required": [ + "albumName" + ] + }, "APIKeyCreateDto": { "type": "object", "properties": { @@ -6280,31 +6305,6 @@ "sharing" ] }, - "CreateAlbumDto": { - "type": "object", - "properties": { - "albumName": { - "type": "string" - }, - "sharedWithUserIds": { - "type": "array", - "items": { - "type": "string", - "format": "uuid" - } - }, - "assetIds": { - "type": "array", - "items": { - "type": "string", - "format": "uuid" - } - } - }, - "required": [ - "albumName" - ] - }, "AddUsersDto": { "type": "object", "properties": { diff --git a/server/libs/domain/src/album/album.repository.ts b/server/libs/domain/src/album/album.repository.ts index c6cc40a05d..55c7fa84fa 100644 --- a/server/libs/domain/src/album/album.repository.ts +++ b/server/libs/domain/src/album/album.repository.ts @@ -17,5 +17,6 @@ export interface IAlbumRepository { getNotShared(ownerId: string): Promise; deleteAll(userId: string): Promise; getAll(): Promise; + create(album: Partial): Promise; save(album: Partial): Promise; } diff --git a/server/libs/domain/src/album/album.service.spec.ts b/server/libs/domain/src/album/album.service.spec.ts index 7555065004..4e902241ab 100644 --- a/server/libs/domain/src/album/album.service.spec.ts +++ b/server/libs/domain/src/album/album.service.spec.ts @@ -1,5 +1,6 @@ -import { albumStub, authStub, newAlbumRepositoryMock, newAssetRepositoryMock } from '../../test'; +import { albumStub, authStub, newAlbumRepositoryMock, newAssetRepositoryMock, newJobRepositoryMock } from '../../test'; import { IAssetRepository } from '../asset'; +import { IJobRepository, JobName } from '../job'; import { IAlbumRepository } from './album.repository'; import { AlbumService } from './album.service'; @@ -7,19 +8,21 @@ describe(AlbumService.name, () => { let sut: AlbumService; let albumMock: jest.Mocked; let assetMock: jest.Mocked; + let jobMock: jest.Mocked; beforeEach(async () => { albumMock = newAlbumRepositoryMock(); assetMock = newAssetRepositoryMock(); + jobMock = newJobRepositoryMock(); - sut = new AlbumService(albumMock, assetMock); + sut = new AlbumService(albumMock, assetMock, jobMock); }); it('should work', () => { expect(sut).toBeDefined(); }); - describe('get list of albums', () => { + describe('getAll', () => { it('gets list of albums for auth user', async () => { albumMock.getOwned.mockResolvedValue([albumStub.empty, albumStub.sharedWithUser]); albumMock.getAssetCountForIds.mockResolvedValue([ @@ -28,7 +31,7 @@ describe(AlbumService.name, () => { ]); albumMock.getInvalidThumbnail.mockResolvedValue([]); - const result = await sut.getAllAlbums(authStub.admin, {}); + const result = await sut.getAll(authStub.admin, {}); expect(result).toHaveLength(2); expect(result[0].id).toEqual(albumStub.empty.id); expect(result[1].id).toEqual(albumStub.sharedWithUser.id); @@ -39,7 +42,7 @@ describe(AlbumService.name, () => { albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]); albumMock.getInvalidThumbnail.mockResolvedValue([]); - const result = await sut.getAllAlbums(authStub.admin, { assetId: albumStub.oneAsset.id }); + const result = await sut.getAll(authStub.admin, { assetId: albumStub.oneAsset.id }); expect(result).toHaveLength(1); expect(result[0].id).toEqual(albumStub.oneAsset.id); expect(albumMock.getByAssetId).toHaveBeenCalledTimes(1); @@ -50,7 +53,7 @@ describe(AlbumService.name, () => { albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.sharedWithUser.id, assetCount: 0 }]); albumMock.getInvalidThumbnail.mockResolvedValue([]); - const result = await sut.getAllAlbums(authStub.admin, { shared: true }); + const result = await sut.getAll(authStub.admin, { shared: true }); expect(result).toHaveLength(1); expect(result[0].id).toEqual(albumStub.sharedWithUser.id); expect(albumMock.getShared).toHaveBeenCalledTimes(1); @@ -61,7 +64,7 @@ describe(AlbumService.name, () => { albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.empty.id, assetCount: 0 }]); albumMock.getInvalidThumbnail.mockResolvedValue([]); - const result = await sut.getAllAlbums(authStub.admin, { shared: false }); + const result = await sut.getAll(authStub.admin, { shared: false }); expect(result).toHaveLength(1); expect(result[0].id).toEqual(albumStub.empty.id); expect(albumMock.getNotShared).toHaveBeenCalledTimes(1); @@ -73,7 +76,7 @@ describe(AlbumService.name, () => { albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]); albumMock.getInvalidThumbnail.mockResolvedValue([]); - const result = await sut.getAllAlbums(authStub.admin, {}); + const result = await sut.getAll(authStub.admin, {}); expect(result).toHaveLength(1); expect(result[0].assetCount).toEqual(1); @@ -89,7 +92,7 @@ describe(AlbumService.name, () => { albumMock.save.mockResolvedValue(albumStub.oneAssetValidThumbnail); assetMock.getFirstAssetForAlbumId.mockResolvedValue(albumStub.oneAssetInvalidThumbnail.assets[0]); - const result = await sut.getAllAlbums(authStub.admin, {}); + const result = await sut.getAll(authStub.admin, {}); expect(result).toHaveLength(1); expect(albumMock.getInvalidThumbnail).toHaveBeenCalledTimes(1); @@ -105,10 +108,47 @@ describe(AlbumService.name, () => { albumMock.save.mockResolvedValue(albumStub.emptyWithValidThumbnail); assetMock.getFirstAssetForAlbumId.mockResolvedValue(null); - const result = await sut.getAllAlbums(authStub.admin, {}); + const result = await sut.getAll(authStub.admin, {}); expect(result).toHaveLength(1); expect(albumMock.getInvalidThumbnail).toHaveBeenCalledTimes(1); expect(albumMock.save).toHaveBeenCalledTimes(1); }); + + describe('create', () => { + it('creates album', async () => { + albumMock.create.mockResolvedValue(albumStub.empty); + + await expect(sut.create(authStub.admin, { albumName: 'Empty album' })).resolves.toEqual({ + albumName: 'Empty album', + albumThumbnailAssetId: null, + assetCount: 0, + assets: [], + createdAt: expect.anything(), + id: 'album-1', + owner: { + createdAt: '2021-01-01', + email: 'admin@test.com', + firstName: 'admin_first_name', + id: 'admin_id', + isAdmin: true, + lastName: 'admin_last_name', + oauthId: '', + profileImagePath: '', + shouldChangePassword: false, + storageLabel: 'admin', + updatedAt: '2021-01-01', + }, + ownerId: 'admin_id', + shared: false, + sharedUsers: [], + updatedAt: expect.anything(), + }); + + expect(jobMock.queue).toHaveBeenCalledWith({ + name: JobName.SEARCH_INDEX_ALBUM, + data: { ids: [albumStub.empty.id] }, + }); + }); + }); }); diff --git a/server/libs/domain/src/album/album.service.ts b/server/libs/domain/src/album/album.service.ts index 16b9184563..a368fbecca 100644 --- a/server/libs/domain/src/album/album.service.ts +++ b/server/libs/domain/src/album/album.service.ts @@ -1,19 +1,22 @@ -import { AlbumEntity } from '@app/infra/entities'; +import { AlbumEntity, AssetEntity, UserEntity } from '@app/infra/entities'; import { Inject, Injectable } from '@nestjs/common'; import { IAssetRepository } from '../asset'; import { AuthUserDto } from '../auth'; +import { IJobRepository, JobName } from '../job'; import { IAlbumRepository } from './album.repository'; +import { CreateAlbumDto } from './dto/album-create.dto'; import { GetAlbumsDto } from './dto/get-albums.dto'; -import { AlbumResponseDto } from './response-dto'; +import { AlbumResponseDto, mapAlbum } from './response-dto'; @Injectable() export class AlbumService { constructor( @Inject(IAlbumRepository) private albumRepository: IAlbumRepository, @Inject(IAssetRepository) private assetRepository: IAssetRepository, + @Inject(IJobRepository) private jobRepository: IJobRepository, ) {} - async getAllAlbums({ id: ownerId }: AuthUserDto, { assetId, shared }: GetAlbumsDto): Promise { + async getAll({ id: ownerId }: AuthUserDto, { assetId, shared }: GetAlbumsDto): Promise { await this.updateInvalidThumbnails(); let albums: AlbumEntity[]; @@ -55,4 +58,17 @@ export class AlbumService { return invalidAlbumIds.length; } + + async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise { + // TODO: Handle nonexistent sharedWithUserIds and assetIds. + const album = await this.albumRepository.create({ + ownerId: authUser.id, + albumName: dto.albumName, + sharedUsers: dto.sharedWithUserIds?.map((value) => ({ id: value } as UserEntity)) ?? [], + assets: (dto.assetIds || []).map((id) => ({ id } as AssetEntity)), + albumThumbnailAssetId: dto.assetIds?.[0] || null, + }); + await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [album.id] } }); + return mapAlbum(album); + } } diff --git a/server/apps/immich/src/api-v1/album/dto/create-album.dto.ts b/server/libs/domain/src/album/dto/album-create.dto.ts similarity index 78% rename from server/apps/immich/src/api-v1/album/dto/create-album.dto.ts rename to server/libs/domain/src/album/dto/album-create.dto.ts index 9857c26a3f..45e5faa446 100644 --- a/server/apps/immich/src/api-v1/album/dto/create-album.dto.ts +++ b/server/libs/domain/src/album/dto/album-create.dto.ts @@ -1,5 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; -import { ValidateUUID } from 'apps/immich/src/decorators/validate-uuid.decorator'; +import { ValidateUUID } from '../../../../../apps/immich/src/decorators/validate-uuid.decorator'; import { IsNotEmpty, IsString } from 'class-validator'; export class CreateAlbumDto { diff --git a/server/libs/domain/src/album/dto/get-albums.dto.ts b/server/libs/domain/src/album/dto/get-albums.dto.ts index ea62ffc204..ccf5098b01 100644 --- a/server/libs/domain/src/album/dto/get-albums.dto.ts +++ b/server/libs/domain/src/album/dto/get-albums.dto.ts @@ -1,8 +1,8 @@ +import { ApiProperty } from '@nestjs/swagger'; import { Transform } from 'class-transformer'; import { IsBoolean, IsOptional } from 'class-validator'; -import { toBoolean } from 'apps/immich/src/utils/transform.util'; -import { ApiProperty } from '@nestjs/swagger'; -import { ValidateUUID } from 'apps/immich/src/decorators/validate-uuid.decorator'; +import { ValidateUUID } from '../../../../../apps/immich/src/decorators/validate-uuid.decorator'; +import { toBoolean } from '../../../../../apps/immich/src/utils/transform.util'; export class GetAlbumsDto { @IsOptional() diff --git a/server/libs/domain/src/album/dto/index.ts b/server/libs/domain/src/album/dto/index.ts new file mode 100644 index 0000000000..2a7e8df243 --- /dev/null +++ b/server/libs/domain/src/album/dto/index.ts @@ -0,0 +1,2 @@ +export * from './album-create.dto'; +export * from './get-albums.dto'; diff --git a/server/libs/domain/src/album/index.ts b/server/libs/domain/src/album/index.ts index 5dd1b2e677..f80717b257 100644 --- a/server/libs/domain/src/album/index.ts +++ b/server/libs/domain/src/album/index.ts @@ -1,3 +1,4 @@ export * from './album.repository'; export * from './album.service'; +export * from './dto'; export * from './response-dto'; diff --git a/server/libs/domain/test/album.repository.mock.ts b/server/libs/domain/test/album.repository.mock.ts index ad9d9ce984..74ddd8aec4 100644 --- a/server/libs/domain/test/album.repository.mock.ts +++ b/server/libs/domain/test/album.repository.mock.ts @@ -11,6 +11,7 @@ export const newAlbumRepositoryMock = (): jest.Mocked => { getNotShared: jest.fn(), deleteAll: jest.fn(), getAll: jest.fn(), + create: jest.fn(), save: jest.fn(), }; }; diff --git a/server/libs/infra/src/repositories/album.repository.ts b/server/libs/infra/src/repositories/album.repository.ts index 94eb17ad76..cab574ed48 100644 --- a/server/libs/infra/src/repositories/album.repository.ts +++ b/server/libs/infra/src/repositories/album.repository.ts @@ -123,8 +123,12 @@ export class AlbumRepository implements IAlbumRepository { }); } + create(album: Partial): Promise { + return this.save(album); + } + async save(album: Partial) { const { id } = await this.repository.save(album); - return this.repository.findOneOrFail({ where: { id } }); + return this.repository.findOneOrFail({ where: { id }, relations: { owner: true } }); } }