refactor(server): user core (#4722)

server/src/domain/auth/auth.service.ts

@@ -23,7 +23,7 @@ import {
   IUserTokenRepository,
 } from '../repositories';
 import { SystemConfigCore } from '../system-config/system-config.core';
-import { UserCore, UserResponseDto } from '../user';
+import { UserCore, UserResponseDto, mapUser } from '../user';
 import {
   AuthType,
   IMMICH_ACCESS_COOKIE,
@@ -73,7 +73,7 @@ export class AuthService {
     @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository,
     @Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository,
     @Inject(ILibraryRepository) libraryRepository: ILibraryRepository,
-    @Inject(IUserRepository) userRepository: IUserRepository,
+    @Inject(IUserRepository) private userRepository: IUserRepository,
     @Inject(IUserTokenRepository) private userTokenRepository: IUserTokenRepository,
     @Inject(ISharedLinkRepository) private sharedLinkRepository: ISharedLinkRepository,
     @Inject(IKeyRepository) private keyRepository: IKeyRepository,
@@ -91,7 +91,7 @@ export class AuthService {
       throw new UnauthorizedException('Password login has been disabled');
     }
 
-    let user = await this.userCore.getByEmail(dto.email, true);
+    let user = await this.userRepository.getByEmail(dto.email, true);
     if (user) {
       const isAuthenticated = this.validatePassword(dto.password, user);
       if (!isAuthenticated) {
@@ -120,7 +120,7 @@ export class AuthService {
 
   async changePassword(authUser: AuthUserDto, dto: ChangePasswordDto) {
     const { password, newPassword } = dto;
-    const user = await this.userCore.getByEmail(authUser.email, true);
+    const user = await this.userRepository.getByEmail(authUser.email, true);
     if (!user) {
       throw new UnauthorizedException();
     }
@@ -134,7 +134,7 @@ export class AuthService {
   }
 
   async adminSignUp(dto: SignUpDto): Promise<AdminSignupResponseDto> {
-    const adminUser = await this.userCore.getAdmin();
+    const adminUser = await this.userRepository.getAdmin();
 
     if (adminUser) {
       throw new BadRequestException('The server already has an admin');
@@ -243,13 +243,13 @@ export class AuthService {
     const config = await this.configCore.getConfig();
     const profile = await this.getOAuthProfile(config, dto.url);
     this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
-    let user = await this.userCore.getByOAuthId(profile.sub);
+    let user = await this.userRepository.getByOAuthId(profile.sub);
 
     // link existing user
     if (!user) {
-      const emailUser = await this.userCore.getByEmail(profile.email);
+      const emailUser = await this.userRepository.getByEmail(profile.email);
       if (emailUser) {
-        user = await this.userCore.updateUser(emailUser, emailUser.id, { oauthId: profile.sub });
+        user = await this.userRepository.update(emailUser.id, { oauthId: profile.sub });
       }
     }
 
@@ -285,16 +285,16 @@ export class AuthService {
   async link(user: AuthUserDto, dto: OAuthCallbackDto): Promise<UserResponseDto> {
     const config = await this.configCore.getConfig();
     const { sub: oauthId } = await this.getOAuthProfile(config, dto.url);
-    const duplicate = await this.userCore.getByOAuthId(oauthId);
+    const duplicate = await this.userRepository.getByOAuthId(oauthId);
     if (duplicate && duplicate.id !== user.id) {
       this.logger.warn(`OAuth link account failed: sub is already linked to another user (${duplicate.email}).`);
       throw new BadRequestException('This OAuth account has already been linked to another user.');
     }
-    return this.userCore.updateUser(user, user.id, { oauthId });
+    return mapUser(await this.userRepository.update(user.id, { oauthId }));
   }
 
   async unlink(user: AuthUserDto): Promise<UserResponseDto> {
-    return this.userCore.updateUser(user, user.id, { oauthId: '' });
+    return mapUser(await this.userRepository.update(user.id, { oauthId: '' }));
   }
 
   private async getLogoutEndpoint(authType: AuthType): Promise<string> {

server/src/domain/user/user.core.ts

@@ -1,17 +1,9 @@
 import { LibraryType, UserEntity } from '@app/infra/entities';
-import {
+import { BadRequestException, ForbiddenException, InternalServerErrorException, Logger } from '@nestjs/common';
-  BadRequestException,
-  ForbiddenException,
-  InternalServerErrorException,
-  Logger,
-  NotFoundException,
-} from '@nestjs/common';
-import { ReadStream, constants, createReadStream } from 'fs';
-import fs from 'fs/promises';
 import path from 'path';
 import sanitize from 'sanitize-filename';
 import { AuthUserDto } from '../auth';
-import { ICryptoRepository, ILibraryRepository, IUserRepository, UserListFilter } from '../repositories';
+import { ICryptoRepository, ILibraryRepository, IUserRepository } from '../repositories';
 
 const SALT_ROUNDS = 10;
 
@@ -131,34 +123,6 @@ export class UserCore {
     }
   }
 
-  async get(userId: string, withDeleted?: boolean): Promise<UserEntity | null> {
-    return this.userRepository.get(userId, withDeleted);
-  }
-
-  async getAdmin(): Promise<UserEntity | null> {
-    return this.userRepository.getAdmin();
-  }
-
-  async getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null> {
-    return this.userRepository.getByEmail(email, withPassword);
-  }
-
-  async getByOAuthId(oauthId: string): Promise<UserEntity | null> {
-    return this.userRepository.getByOAuthId(oauthId);
-  }
-
-  async getUserProfileImage(user: UserEntity): Promise<ReadStream> {
-    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);
-  }
-
-  async getList(filter?: UserListFilter): Promise<UserEntity[]> {
-    return this.userRepository.getList(filter);
-  }
-
   async createProfileImage(authUser: AuthUserDto, filePath: string): Promise<UserEntity> {
     try {
       return this.userRepository.update(authUser.id, { profileImagePath: filePath });

server/src/domain/user/user.service.spec.ts

@@ -149,9 +149,7 @@ describe(UserService.name, () => {
     sut = new UserService(albumMock, assetMock, cryptoRepositoryMock, jobMock, libraryMock, storageMock, userMock);
 
     when(userMock.get).calledWith(adminUser.id).mockResolvedValue(adminUser);
-    when(userMock.get).calledWith(adminUser.id, undefined).mockResolvedValue(adminUser);
     when(userMock.get).calledWith(immichUser.id).mockResolvedValue(immichUser);
-    when(userMock.get).calledWith(immichUser.id, undefined).mockResolvedValue(immichUser);
   });
 
   describe('getAll', () => {
@@ -207,7 +205,7 @@ describe(UserService.name, () => {
 
       const response = await sut.getMe(adminUser);
 
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id, undefined);
+      expect(userMock.get).toHaveBeenCalledWith(adminUser.id);
       expect(response).toEqual(adminUserResponse);
     });
 
@@ -216,7 +214,7 @@ describe(UserService.name, () => {
 
       await expect(sut.getMe(adminUser)).rejects.toBeInstanceOf(BadRequestException);
 
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id, undefined);
+      expect(userMock.get).toHaveBeenCalledWith(adminUser.id);
     });
   });
 
@@ -256,7 +254,7 @@ describe(UserService.name, () => {
 
     it('user can only update its information', async () => {
       when(userMock.get)
-        .calledWith('not_immich_auth_user_id', undefined)
+        .calledWith('not_immich_auth_user_id')
         .mockResolvedValueOnce({
           ...immichUser,
           id: 'not_immich_auth_user_id',
@@ -321,7 +319,7 @@ describe(UserService.name, () => {
     });
 
     it('update user information should throw error if user not found', async () => {
-      when(userMock.get).calledWith(immichUser.id, undefined).mockResolvedValueOnce(null);
+      when(userMock.get).calledWith(immichUser.id).mockResolvedValueOnce(null);
 
       const result = sut.update(adminUser, {
         id: immichUser.id,
@@ -334,7 +332,6 @@ describe(UserService.name, () => {
     it('should let the admin update himself', async () => {
       const dto = { id: adminUser.id, shouldChangePassword: true, isAdmin: true };
 
-      when(userMock.get).calledWith(adminUser.id).mockResolvedValueOnce(null);
       when(userMock.update).calledWith(adminUser.id, dto).mockResolvedValueOnce(adminUser);
 
       await sut.update(adminUser, dto);
@@ -398,7 +395,7 @@ describe(UserService.name, () => {
       userMock.delete.mockResolvedValue(immichUser);
 
       await expect(sut.delete(adminUserAuth, immichUser.id)).resolves.toEqual(mapUser(immichUser));
-      expect(userMock.get).toHaveBeenCalledWith(immichUser.id, undefined);
+      expect(userMock.get).toHaveBeenCalledWith(immichUser.id);
       expect(userMock.delete).toHaveBeenCalledWith(immichUser);
     });
   });
@@ -466,7 +463,7 @@ describe(UserService.name, () => {
 
       await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException);
 
-      expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id, undefined);
+      expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id);
     });
 
     it('should throw an error if the user does not have a picture', async () => {
@@ -474,7 +471,7 @@ describe(UserService.name, () => {
 
       await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException);
 
-      expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id, undefined);
+      expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id);
     });
   });
 

server/src/domain/user/user.service.ts

@@ -1,7 +1,8 @@
 import { UserEntity } from '@app/infra/entities';
 import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common';
 import { randomBytes } from 'crypto';
-import { ReadStream } from 'fs';
+import { ReadStream, constants, createReadStream } from 'fs';
+import fs from 'fs/promises';
 import { AuthUserDto } from '../auth';
 import { IEntityJob, JobName } from '../job';
 import {
@@ -36,12 +37,12 @@ export class UserService {
   }
 
   async getAll(authUser: AuthUserDto, isAll: boolean): Promise<UserResponseDto[]> {
-    const users = await this.userCore.getList({ withDeleted: !isAll });
+    const users = await this.userRepository.getList({ withDeleted: !isAll });
     return users.map(mapUser);
   }
 
   async get(userId: string, withDeleted = false): Promise<UserResponseDto> {
-    const user = await this.userCore.get(userId, withDeleted);
+    const user = await this.userRepository.get(userId, withDeleted);
     if (!user) {
       throw new NotFoundException('User not found');
     }
@@ -50,7 +51,7 @@ export class UserService {
   }
 
   async getMe(authUser: AuthUserDto): Promise<UserResponseDto> {
-    const user = await this.userCore.get(authUser.id);
+    const user = await this.userRepository.get(authUser.id);
     if (!user) {
       throw new BadRequestException('User not found');
     }
@@ -63,7 +64,7 @@ export class UserService {
   }
 
   async update(authUser: AuthUserDto, dto: UpdateUserDto): Promise<UserResponseDto> {
-    const user = await this.userCore.get(dto.id);
+    const user = await this.userRepository.get(dto.id);
     if (!user) {
       throw new NotFoundException('User not found');
     }
@@ -73,7 +74,7 @@ export class UserService {
   }
 
   async delete(authUser: AuthUserDto, userId: string): Promise<UserResponseDto> {
-    const user = await this.userCore.get(userId);
+    const user = await this.userRepository.get(userId);
     if (!user) {
       throw new BadRequestException('User not found');
     }
@@ -83,7 +84,7 @@ export class UserService {
   }
 
   async restore(authUser: AuthUserDto, userId: string): Promise<UserResponseDto> {
-    const user = await this.userCore.get(userId, true);
+    const user = await this.userRepository.get(userId, true);
     if (!user) {
       throw new BadRequestException('User not found');
     }
@@ -101,15 +102,19 @@ export class UserService {
   }
 
   async getProfileImage(userId: string): Promise<ReadStream> {
-    const user = await this.userCore.get(userId);
+    const user = await this.userRepository.get(userId);
     if (!user) {
       throw new NotFoundException('User not found');
     }
-    return this.userCore.getUserProfileImage(user);
+    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);
   }
 
   async resetAdminPassword(ask: (admin: UserResponseDto) => Promise<string | undefined>) {
-    const admin = await this.userCore.getAdmin();
+    const admin = await this.userRepository.getAdmin();
     if (!admin) {
       throw new BadRequestException('Admin account does not exist');
     }