From 3212a47720b9b6351bca37c2cd312159e0c11ea8 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Mon, 30 Oct 2023 19:38:34 -0400 Subject: [PATCH] refactor(server): user profile picture (#4728) --- server/src/domain/user/user.core.ts | 9 ------- server/src/domain/user/user.service.spec.ts | 15 ++++++++++- server/src/domain/user/user.service.ts | 27 ++++++++++--------- .../src/immich/controllers/user.controller.ts | 11 +++----- server/test/fixtures/user.stub.ts | 17 ++++++++++++ 5 files changed, 49 insertions(+), 30 deletions(-) diff --git a/server/src/domain/user/user.core.ts b/server/src/domain/user/user.core.ts index 7ed550a784..a262477819 100644 --- a/server/src/domain/user/user.core.ts +++ b/server/src/domain/user/user.core.ts @@ -123,15 +123,6 @@ export class UserCore { } } - async createProfileImage(authUser: AuthUserDto, filePath: string): Promise { - try { - return this.userRepository.update(authUser.id, { profileImagePath: filePath }); - } catch (e) { - Logger.error(e, 'Create User Profile Image'); - throw new InternalServerErrorException('Failed to create new user profile image'); - } - } - async restoreUser(authUser: AuthUserDto, userToRestore: UserEntity): Promise { if (!authUser.isAdmin) { throw new ForbiddenException('Unauthorized'); diff --git a/server/src/domain/user/user.service.spec.ts b/server/src/domain/user/user.service.spec.ts index e78faedb1b..180337dff5 100644 --- a/server/src/domain/user/user.service.spec.ts +++ b/server/src/domain/user/user.service.spec.ts @@ -16,6 +16,7 @@ import { userStub, } from '@test'; import { when } from 'jest-when'; +import { Readable } from 'stream'; import { AuthUserDto } from '../auth'; import { JobName } from '../job'; import { @@ -461,7 +462,7 @@ describe(UserService.name, () => { it('should throw an error if the user does not exist', async () => { userMock.get.mockResolvedValue(null); - await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException); + await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(BadRequestException); expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id); }); @@ -473,6 +474,18 @@ describe(UserService.name, () => { expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id); }); + + it('should return the profile picture', async () => { + const stream = new Readable(); + + userMock.get.mockResolvedValue(userStub.profilePath); + storageMock.createReadStream.mockResolvedValue({ stream }); + + await expect(sut.getProfileImage(userStub.profilePath.id)).resolves.toEqual({ stream }); + + expect(userMock.get).toHaveBeenCalledWith(userStub.profilePath.id); + expect(storageMock.createReadStream).toHaveBeenCalledWith('/path/to/profile.jpg', 'image/jpeg'); + }); }); describe('resetAdminPassword', () => { diff --git a/server/src/domain/user/user.service.ts b/server/src/domain/user/user.service.ts index 3e4c79ab67..3f36f2bec1 100644 --- a/server/src/domain/user/user.service.ts +++ b/server/src/domain/user/user.service.ts @@ -1,8 +1,6 @@ import { UserEntity } from '@app/infra/entities'; import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common'; import { randomBytes } from 'crypto'; -import { ReadStream, constants, createReadStream } from 'fs'; -import fs from 'fs/promises'; import { AuthUserDto } from '../auth'; import { IEntityJob, JobName } from '../job'; import { @@ -13,6 +11,7 @@ import { ILibraryRepository, IStorageRepository, IUserRepository, + ImmichReadStream, } from '../repositories'; import { StorageCore, StorageFolder } from '../storage'; import { CreateUserDto, UpdateUserDto } from './dto'; @@ -41,8 +40,8 @@ export class UserService { return users.map(mapUser); } - async get(userId: string, withDeleted = false): Promise { - const user = await this.userRepository.get(userId, withDeleted); + async get(userId: string): Promise { + const user = await this.userRepository.get(userId, false); if (!user) { throw new NotFoundException('User not found'); } @@ -97,20 +96,16 @@ export class UserService { authUser: AuthUserDto, fileInfo: Express.Multer.File, ): Promise { - const updatedUser = await this.userCore.createProfileImage(authUser, fileInfo.path); + const updatedUser = await this.userRepository.update(authUser.id, { profileImagePath: fileInfo.path }); return mapCreateProfileImageResponse(updatedUser.id, updatedUser.profileImagePath); } - async getProfileImage(userId: string): Promise { - const user = await this.userRepository.get(userId); - if (!user) { - throw new NotFoundException('User not found'); - } + async getProfileImage(id: string): Promise { + const user = await this.findOrFail(id); if (!user.profileImagePath) { throw new NotFoundException('User does not have a profile image'); } - await fs.access(user.profileImagePath, constants.R_OK); - return createReadStream(user.profileImagePath); + return this.storageRepository.createReadStream(user.profileImagePath, 'image/jpeg'); } async resetAdminPassword(ask: (admin: UserResponseDto) => Promise) { @@ -185,4 +180,12 @@ export class UserService { return msSinceDelete >= msDeleteWait; } + + private async findOrFail(id: string) { + const user = await this.userRepository.get(id); + if (!user) { + throw new BadRequestException('User not found'); + } + return user; + } } diff --git a/server/src/immich/controllers/user.controller.ts b/server/src/immich/controllers/user.controller.ts index 254aa0525e..7ca8b1c6e2 100644 --- a/server/src/immich/controllers/user.controller.ts +++ b/server/src/immich/controllers/user.controller.ts @@ -17,16 +17,13 @@ import { Post, Put, Query, - Response, - StreamableFile, UploadedFile, UseInterceptors, } from '@nestjs/common'; import { ApiBody, ApiConsumes, ApiTags } from '@nestjs/swagger'; -import { Response as Res } from 'express'; import { AdminRoute, AuthUser, Authenticated } from '../app.guard'; import { FileUploadInterceptor, Route } from '../app.interceptor'; -import { UseValidation } from '../app.utils'; +import { UseValidation, asStreamableFile } from '../app.utils'; import { UUIDParamDto } from './dto/uuid-param.dto'; @ApiTags('User') @@ -88,9 +85,7 @@ export class UserController { @Get('profile-image/:id') @Header('Cache-Control', 'private, no-cache, no-transform') - async getProfileImage(@Param() { id }: UUIDParamDto, @Response({ passthrough: true }) res: Res): Promise { - const readableStream = await this.service.getProfileImage(id); - res.header('Content-Type', 'image/jpeg'); - return new StreamableFile(readableStream); + getProfileImage(@Param() { id }: UUIDParamDto): Promise { + return this.service.getProfileImage(id).then(asStreamableFile); } } diff --git a/server/test/fixtures/user.stub.ts b/server/test/fixtures/user.stub.ts index 34d1353293..af5515bafc 100644 --- a/server/test/fixtures/user.stub.ts +++ b/server/test/fixtures/user.stub.ts @@ -104,4 +104,21 @@ export const userStub = { assets: [], memoriesEnabled: true, }), + profilePath: Object.freeze({ + ...authStub.user1, + password: 'immich_password', + firstName: 'immich_first_name', + lastName: 'immich_last_name', + storageLabel: 'label-1', + externalPath: null, + oauthId: '', + shouldChangePassword: false, + profileImagePath: '/path/to/profile.jpg', + createdAt: new Date('2021-01-01'), + deletedAt: null, + updatedAt: new Date('2021-01-01'), + tags: [], + assets: [], + memoriesEnabled: true, + }), };