refactor(server): update album (#2562)

* refactor: update album

* fix: remove unnecessary decorator
This commit is contained in:
Jason Rasmussen 2023-05-25 15:37:19 -04:00 committed by GitHub
parent 1c293a2759
commit 4cc6e3b966
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 272 additions and 225 deletions

View File

@ -6,7 +6,7 @@ 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 './dto/update-album.dto';
import { UpdateAlbumDto } from '@app/domain';
import { AlbumCountResponseDto } from './response-dto/album-count-response.dto';
import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto';

View File

@ -1,4 +1,4 @@
import { Controller, Get, Post, Body, Patch, Param, Delete, Put, Query, Response } from '@nestjs/common';
import { Controller, Get, Post, Body, Param, Delete, Put, Query, Response } from '@nestjs/common';
import { ParseMeUUIDPipe } from '../validation/parse-me-uuid-pipe';
import { AlbumService } from './album.service';
import { Authenticated } from '../../decorators/authenticated.decorator';
@ -6,7 +6,6 @@ import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator';
import { AddAssetsDto } from './dto/add-assets.dto';
import { AddUsersDto } from './dto/add-users.dto';
import { RemoveAssetsDto } from './dto/remove-assets.dto';
import { UpdateAlbumDto } from './dto/update-album.dto';
import { ApiOkResponse, ApiTags } from '@nestjs/swagger';
import { AlbumResponseDto } from '@app/domain';
import { AlbumCountResponseDto } from './response-dto/album-count-response.dto';
@ -94,14 +93,6 @@ export class AlbumController {
return this.service.removeUser(authUser, id, userId);
}
@Authenticated()
@Patch(':id')
updateAlbumInfo(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: UpdateAlbumDto) {
// TODO: Handle nonexistent albumThumbnailAssetId.
// TODO: Disallow setting asset from other user as albumThumbnailAssetId.
return this.service.update(authUser, id, dto);
}
@Authenticated({ isShared: true })
@Get(':id/download')
@ApiOkResponse({ content: { 'application/zip': { schema: { type: 'string', format: 'binary' } } } })

View File

@ -2,7 +2,7 @@ import { AlbumService } from './album.service';
import { AuthUserDto } from '../../decorators/auth-user.decorator';
import { BadRequestException, NotFoundException, ForbiddenException } from '@nestjs/common';
import { AlbumEntity, UserEntity } from '@app/infra/entities';
import { AlbumResponseDto, ICryptoRepository, IJobRepository, JobName, mapUser } from '@app/domain';
import { AlbumResponseDto, ICryptoRepository, IJobRepository, mapUser } from '@app/domain';
import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto';
import { IAlbumRepository } from './album-repository';
import { DownloadService } from '../../modules/download/download.service';
@ -259,44 +259,6 @@ describe('Album service', () => {
await expect(sut.removeUser(authUser, albumEntity.id, authUser.id)).rejects.toBeInstanceOf(BadRequestException);
});
it('updates a owned album', async () => {
const albumEntity = _getOwnedAlbum();
const albumId = albumEntity.id;
const updatedAlbumName = 'new album name';
const updatedAlbumThumbnailAssetId = '69d2f917-0b31-48d8-9d7d-673b523f1aac';
albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
const updatedAlbum = { ...albumEntity, albumName: updatedAlbumName };
albumRepositoryMock.updateAlbum.mockResolvedValue(updatedAlbum);
const result = await sut.update(authUser, albumId, {
albumName: updatedAlbumName,
albumThumbnailAssetId: updatedAlbumThumbnailAssetId,
});
expect(result.id).toEqual(albumId);
expect(result.albumName).toEqual(updatedAlbumName);
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledWith(albumEntity, {
albumName: updatedAlbumName,
albumThumbnailAssetId: updatedAlbumThumbnailAssetId,
});
expect(jobMock.queue).toHaveBeenCalledWith({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [updatedAlbum.id] } });
});
it('prevents updating a not owned album (shared with auth user)', async () => {
const albumEntity = _getSharedWithAuthUserAlbum();
const albumId = albumEntity.id;
albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
await expect(
sut.update(authUser, albumId, {
albumName: 'new album name',
albumThumbnailAssetId: '69d2f917-0b31-48d8-9d7d-673b523f1aac',
}),
).rejects.toBeInstanceOf(ForbiddenException);
});
it('adds assets to owned album', async () => {
const albumEntity = _getOwnedAlbum();

View File

@ -3,7 +3,6 @@ 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 { UpdateAlbumDto } from './dto/update-album.dto';
import { AlbumResponseDto, IJobRepository, JobName, mapAlbum } from '@app/domain';
import { IAlbumRepository } from './album-repository';
import { AlbumCountResponseDto } from './response-dto/album-count-response.dto';
@ -116,20 +115,6 @@ export class AlbumService {
};
}
async update(authUser: AuthUserDto, albumId: string, dto: UpdateAlbumDto): Promise<AlbumResponseDto> {
const album = await this._getAlbum({ authUser, albumId });
if (authUser.id != album.ownerId) {
throw new BadRequestException('Unauthorized to change album info');
}
const updatedAlbum = await this.albumRepository.updateAlbum(album, dto);
await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [updatedAlbum.id] } });
return mapAlbum(updatedAlbum);
}
async getCountByUserId(authUser: AuthUserDto): Promise<AlbumCountResponseDto> {
return this.albumRepository.getCountByUserId(authUser.id);
}

View File

@ -1,10 +1,11 @@
import { AlbumService, AuthUserDto, CreateAlbumDto } from '@app/domain';
/* */ import { AlbumService, AuthUserDto, CreateAlbumDto, UpdateAlbumDto } from '@app/domain';
import { GetAlbumsDto } from '@app/domain/album/dto/get-albums.dto';
import { Body, Controller, Get, Post, Query } from '@nestjs/common';
import { Body, Controller, 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';
import { UseValidation } from '../decorators/use-validation.decorator';
import { UUIDParamDto } from './dto/uuid-param.dto';
@ApiTags('Album')
@Controller('album')
@ -22,4 +23,9 @@ export class AlbumController {
createAlbum(@GetAuthUser() authUser: AuthUserDto, @Body() dto: CreateAlbumDto) {
return this.service.create(authUser, dto);
}
@Patch(':id')
updateAlbumInfo(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: UpdateAlbumDto) {
return this.service.update(authUser, id, dto);
}
}

View File

@ -95,6 +95,148 @@
]
}
},
"/album/{id}": {
"patch": {
"operationId": "updateAlbumInfo",
"parameters": [
{
"name": "id",
"required": true,
"in": "path",
"schema": {
"format": "uuid",
"type": "string"
}
}
],
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/UpdateAlbumDto"
}
}
}
},
"responses": {
"200": {
"description": "",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/AlbumResponseDto"
}
}
}
}
},
"tags": [
"Album"
],
"security": [
{
"bearer": []
},
{
"cookie": []
},
{
"api_key": []
},
{
"bearer": []
},
{
"cookie": []
},
{
"api_key": []
}
]
},
"get": {
"operationId": "getAlbumInfo",
"parameters": [
{
"name": "id",
"required": true,
"in": "path",
"schema": {
"format": "uuid",
"type": "string"
}
},
{
"name": "key",
"required": false,
"in": "query",
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": "",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/AlbumResponseDto"
}
}
}
}
},
"tags": [
"Album"
],
"security": [
{
"bearer": []
},
{
"cookie": []
},
{
"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": {
"post": {
"operationId": "createKey",
@ -3859,139 +4001,6 @@
]
}
},
"/album/{id}": {
"get": {
"operationId": "getAlbumInfo",
"parameters": [
{
"name": "id",
"required": true,
"in": "path",
"schema": {
"format": "uuid",
"type": "string"
}
},
{
"name": "key",
"required": false,
"in": "query",
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": "",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/AlbumResponseDto"
}
}
}
}
},
"tags": [
"Album"
],
"security": [
{
"bearer": []
},
{
"cookie": []
},
{
"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": []
}
]
},
"patch": {
"operationId": "updateAlbumInfo",
"parameters": [
{
"name": "id",
"required": true,
"in": "path",
"schema": {
"format": "uuid",
"type": "string"
}
}
],
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/UpdateAlbumDto"
}
}
}
},
"responses": {
"200": {
"description": "",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/AlbumResponseDto"
}
}
}
}
},
"tags": [
"Album"
],
"security": [
{
"bearer": []
},
{
"cookie": []
},
{
"api_key": []
}
]
}
},
"/album/{id}/user/{userId}": {
"delete": {
"operationId": "removeUserFromAlbum",
@ -4605,6 +4614,18 @@
"albumName"
]
},
"UpdateAlbumDto": {
"type": "object",
"properties": {
"albumName": {
"type": "string"
},
"albumThumbnailAssetId": {
"type": "string",
"format": "uuid"
}
}
},
"APIKeyCreateDto": {
"type": "object",
"properties": {
@ -6372,18 +6393,6 @@
"alreadyInAlbum"
]
},
"UpdateAlbumDto": {
"type": "object",
"properties": {
"albumName": {
"type": "string"
},
"albumThumbnailAssetId": {
"type": "string",
"format": "uuid"
}
}
},
"CreateAlbumShareLinkDto": {
"type": "object",
"properties": {

View File

@ -10,6 +10,7 @@ export interface AlbumAssetCount {
export interface IAlbumRepository {
getByIds(ids: string[]): Promise<AlbumEntity[]>;
getByAssetId(ownerId: string, assetId: string): Promise<AlbumEntity[]>;
hasAsset(id: string, assetId: string): Promise<boolean>;
getAssetCountForIds(ids: string[]): Promise<AlbumAssetCount[]>;
getInvalidThumbnail(): Promise<string[]>;
getOwned(ownerId: string): Promise<AlbumEntity[]>;
@ -18,5 +19,5 @@ export interface IAlbumRepository {
deleteAll(userId: string): Promise<void>;
getAll(): Promise<AlbumEntity[]>;
create(album: Partial<AlbumEntity>): Promise<AlbumEntity>;
save(album: Partial<AlbumEntity>): Promise<AlbumEntity>;
update(album: Partial<AlbumEntity>): Promise<AlbumEntity>;
}

View File

@ -1,3 +1,4 @@
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { albumStub, authStub, newAlbumRepositoryMock, newAssetRepositoryMock, newJobRepositoryMock } from '../../test';
import { IAssetRepository } from '../asset';
import { IJobRepository, JobName } from '../job';
@ -89,14 +90,14 @@ describe(AlbumService.name, () => {
{ albumId: albumStub.oneAssetInvalidThumbnail.id, assetCount: 1 },
]);
albumMock.getInvalidThumbnail.mockResolvedValue([albumStub.oneAssetInvalidThumbnail.id]);
albumMock.save.mockResolvedValue(albumStub.oneAssetValidThumbnail);
albumMock.update.mockResolvedValue(albumStub.oneAssetValidThumbnail);
assetMock.getFirstAssetForAlbumId.mockResolvedValue(albumStub.oneAssetInvalidThumbnail.assets[0]);
const result = await sut.getAll(authStub.admin, {});
expect(result).toHaveLength(1);
expect(albumMock.getInvalidThumbnail).toHaveBeenCalledTimes(1);
expect(albumMock.save).toHaveBeenCalledTimes(1);
expect(albumMock.update).toHaveBeenCalledTimes(1);
});
it('removes the thumbnail for an empty album', async () => {
@ -105,14 +106,14 @@ describe(AlbumService.name, () => {
{ albumId: albumStub.emptyWithInvalidThumbnail.id, assetCount: 1 },
]);
albumMock.getInvalidThumbnail.mockResolvedValue([albumStub.emptyWithInvalidThumbnail.id]);
albumMock.save.mockResolvedValue(albumStub.emptyWithValidThumbnail);
albumMock.update.mockResolvedValue(albumStub.emptyWithValidThumbnail);
assetMock.getFirstAssetForAlbumId.mockResolvedValue(null);
const result = await sut.getAll(authStub.admin, {});
expect(result).toHaveLength(1);
expect(albumMock.getInvalidThumbnail).toHaveBeenCalledTimes(1);
expect(albumMock.save).toHaveBeenCalledTimes(1);
expect(albumMock.update).toHaveBeenCalledTimes(1);
});
describe('create', () => {
@ -151,4 +152,47 @@ describe(AlbumService.name, () => {
});
});
});
describe('update', () => {
it('should prevent updating an album that does not exist', async () => {
albumMock.getByIds.mockResolvedValue([]);
await expect(
sut.update(authStub.user1, 'invalid-id', {
albumName: 'new album name',
}),
).rejects.toBeInstanceOf(BadRequestException);
expect(albumMock.update).not.toHaveBeenCalled();
});
it('should prevent updating a not owned album (shared with auth user)', async () => {
albumMock.getByIds.mockResolvedValue([albumStub.sharedWithAdmin]);
await expect(
sut.update(authStub.admin, albumStub.sharedWithAdmin.id, {
albumName: 'new album name',
}),
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should all the owner to update the album', async () => {
albumMock.getByIds.mockResolvedValue([albumStub.oneAsset]);
albumMock.update.mockResolvedValue(albumStub.oneAsset);
await sut.update(authStub.admin, albumStub.oneAsset.id, {
albumName: 'new album name',
});
expect(albumMock.update).toHaveBeenCalledTimes(1);
expect(albumMock.update).toHaveBeenCalledWith({
id: 'album-4',
albumName: 'new album name',
});
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.SEARCH_INDEX_ALBUM,
data: { ids: [albumStub.oneAsset.id] },
});
});
});
});

View File

@ -1,11 +1,10 @@
import { AlbumEntity, AssetEntity, UserEntity } from '@app/infra/entities';
import { Inject, Injectable } from '@nestjs/common';
import { BadRequestException, ForbiddenException, 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 { CreateAlbumDto, GetAlbumsDto, UpdateAlbumDto } from './dto';
import { AlbumResponseDto, mapAlbum } from './response-dto';
@Injectable()
@ -53,7 +52,7 @@ export class AlbumService {
for (const albumId of invalidAlbumIds) {
const newThumbnail = await this.assetRepository.getFirstAssetForAlbumId(albumId);
await this.albumRepository.save({ id: albumId, albumThumbnailAsset: newThumbnail });
await this.albumRepository.update({ id: albumId, albumThumbnailAsset: newThumbnail });
}
return invalidAlbumIds.length;
@ -71,4 +70,32 @@ export class AlbumService {
await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [album.id] } });
return mapAlbum(album);
}
async update(authUser: AuthUserDto, id: string, dto: UpdateAlbumDto): Promise<AlbumResponseDto> {
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');
}
if (dto.albumThumbnailAssetId) {
const valid = await this.albumRepository.hasAsset(id, dto.albumThumbnailAssetId);
if (!valid) {
throw new BadRequestException('Invalid album thumbnail');
}
}
const updatedAlbum = await this.albumRepository.update({
id: album.id,
albumName: dto.albumName,
albumThumbnailAssetId: dto.albumThumbnailAssetId,
});
await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [updatedAlbum.id] } });
return mapAlbum(updatedAlbum);
}
}

View File

@ -1,6 +1,6 @@
import { ApiProperty } from '@nestjs/swagger';
import { ValidateUUID } from 'apps/immich/src/decorators/validate-uuid.decorator';
import { IsOptional } from 'class-validator';
import { ValidateUUID } from '../../../../../apps/immich/src/decorators/validate-uuid.decorator';
export class UpdateAlbumDto {
@IsOptional()

View File

@ -1,2 +1,3 @@
export * from './album-create.dto';
export * from './album-update.dto';
export * from './get-albums.dto';

View File

@ -11,7 +11,8 @@ export const newAlbumRepositoryMock = (): jest.Mocked<IAlbumRepository> => {
getNotShared: jest.fn(),
deleteAll: jest.fn(),
getAll: jest.fn(),
hasAsset: jest.fn(),
create: jest.fn(),
save: jest.fn(),
update: jest.fn(),
};
};

View File

@ -123,11 +123,31 @@ export class AlbumRepository implements IAlbumRepository {
});
}
create(album: Partial<AlbumEntity>): Promise<AlbumEntity> {
async hasAsset(id: string, assetId: string): Promise<boolean> {
const count = await this.repository.count({
where: {
id,
assets: {
id: assetId,
},
},
relations: {
assets: true,
},
});
return Boolean(count);
}
async create(album: Partial<AlbumEntity>): Promise<AlbumEntity> {
return this.save(album);
}
async save(album: Partial<AlbumEntity>) {
async update(album: Partial<AlbumEntity>) {
return this.save(album);
}
private async save(album: Partial<AlbumEntity>) {
const { id } = await this.repository.save(album);
return this.repository.findOneOrFail({ where: { id }, relations: { owner: true } });
}