From b359dc3cb6ea606ad7230a3dbe4b1985cfec726f Mon Sep 17 00:00:00 2001 From: Jaime Baez Date: Mon, 6 Jun 2022 18:16:03 +0200 Subject: [PATCH] Fix user e2e tests (#194) * WIP fix user e2e tests The e2e tests still don't seem to work due to migrations not running. Changes: - update user.e2e tests to use new `userService.createUser` method - fix server `typeorm` command to use ORM config - update make test-e2e to re-create database volume every time - add User DTO - update auth.service and user.service to use User DTO - update CreateUserDto making optional properties that are optional * Fix migrations - add missing `.ts` extension to migrations path - update user e2e test for the new returned User resource --- Makefile | 2 +- docker/.env.test | 10 +++- docker/docker-compose.test.yml | 4 +- server/package.json | 2 +- server/src/api-v1/auth/auth.service.ts | 42 +++++++++------- server/src/api-v1/user/dto/create-user.dto.ts | 8 +-- server/src/api-v1/user/response-dto/user.ts | 19 +++++++ server/src/api-v1/user/user.service.ts | 49 +++++++----------- server/src/config/database.config.ts | 4 +- server/test/user.e2e-spec.ts | 50 ++++++++++++++----- 10 files changed, 114 insertions(+), 76 deletions(-) create mode 100644 server/src/api-v1/user/response-dto/user.ts diff --git a/Makefile b/Makefile index ae56b3b464..d6efb9b206 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ stage: docker-compose -f ./docker/docker-compose.staging.yml up --build -V --remove-orphans test-e2e: - docker-compose -f ./docker/docker-compose.test.yml --env-file ./docker/.env.test up --abort-on-container-exit --exit-code-from immich_server_test + docker-compose -f ./docker/docker-compose.test.yml --env-file ./docker/.env.test up --renew-anon-volumes --abort-on-container-exit --exit-code-from immich_server_test --remove-orphans prod: docker-compose -f ./docker/docker-compose.yml up --build -V --remove-orphans diff --git a/docker/.env.test b/docker/.env.test index f5438bb2b1..7171aa3822 100644 --- a/docker/.env.test +++ b/docker/.env.test @@ -1,9 +1,12 @@ -DB_HOSTNAME=immich_postgres_test # Database +DB_HOSTNAME=immich_postgres_test DB_USERNAME=postgres DB_PASSWORD=postgres DB_DATABASE_NAME=e2e_test +# Redis +REDIS_HOSTNAME=immich_redis_test + # Upload File Config UPLOAD_LOCATION=./upload @@ -13,4 +16,7 @@ JWT_SECRET=randomstringthatissolongandpowerfulthatnoonecanguess # MAPBOX ## ENABLE_MAPBOX is either true of false -> if true, you have to provide MAPBOX_KEY ENABLE_MAPBOX=false -MAPBOX_KEY= \ No newline at end of file + +# WEB +MAPBOX_KEY= +VITE_SERVER_ENDPOINT=http://localhost:2283 \ No newline at end of file diff --git a/docker/docker-compose.test.yml b/docker/docker-compose.test.yml index 6d7a83700b..603e05d601 100644 --- a/docker/docker-compose.test.yml +++ b/docker/docker-compose.test.yml @@ -40,7 +40,7 @@ services: POSTGRES_DB: ${DB_DATABASE_NAME} PG_DATA: /var/lib/postgresql/data volumes: - - pgdata-test:/var/lib/postgresql/data + - /var/lib/postgresql/data ports: - 5432:5432 networks: @@ -48,5 +48,3 @@ services: networks: immich_network_test: -volumes: - pgdata-test: diff --git a/server/package.json b/server/package.json index c5749fe66b..480e6ea572 100644 --- a/server/package.json +++ b/server/package.json @@ -19,7 +19,7 @@ "test:cov": "jest --coverage", "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand", "test:e2e": "jest --config ./test/jest-e2e.json", - "typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js" + "typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js --config src/config/database.config.ts" }, "dependencies": { "@mapbox/mapbox-sdk": "^0.13.3", diff --git a/server/src/api-v1/auth/auth.service.ts b/server/src/api-v1/auth/auth.service.ts index 07eb932530..8bc2f36231 100644 --- a/server/src/api-v1/auth/auth.service.ts +++ b/server/src/api-v1/auth/auth.service.ts @@ -7,6 +7,7 @@ import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service'; import { JwtPayloadDto } from './dto/jwt-payload.dto'; import { SignUpDto } from './dto/sign-up.dto'; import * as bcrypt from 'bcrypt'; +import { mapUser, User } from '../user/response-dto/user'; @Injectable() export class AuthService { @@ -14,12 +15,24 @@ export class AuthService { @InjectRepository(UserEntity) private userRepository: Repository, private immichJwtService: ImmichJwtService, - ) { } + ) {} private async validateUser(loginCredential: LoginCredentialDto): Promise { const user = await this.userRepository.findOne( { email: loginCredential.email }, - { select: ['id', 'email', 'password', 'salt', 'firstName', 'lastName', 'isAdmin', 'profileImagePath', 'isFirstLoggedIn'] }, + { + select: [ + 'id', + 'email', + 'password', + 'salt', + 'firstName', + 'lastName', + 'isAdmin', + 'profileImagePath', + 'isFirstLoggedIn', + ], + }, ); const isAuthenticated = await this.validatePassword(user.password, loginCredential.password, user.salt); @@ -48,38 +61,29 @@ export class AuthService { lastName: validatedUser.lastName, isAdmin: validatedUser.isAdmin, profileImagePath: validatedUser.profileImagePath, - isFirstLogin: validatedUser.isFirstLoggedIn + isFirstLogin: validatedUser.isFirstLoggedIn, }; } - - public async adminSignUp(signUpCrendential: SignUpDto) { + public async adminSignUp(signUpCredential: SignUpDto): Promise { const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } }); if (adminUser) { - throw new BadRequestException('The server already has an admin') + throw new BadRequestException('The server already has an admin'); } - const newAdminUser = new UserEntity(); - newAdminUser.email = signUpCrendential.email; + newAdminUser.email = signUpCredential.email; newAdminUser.salt = await bcrypt.genSalt(); - newAdminUser.password = await this.hashPassword(signUpCrendential.password, newAdminUser.salt); - newAdminUser.firstName = signUpCrendential.firstName; - newAdminUser.lastName = signUpCrendential.lastName; + newAdminUser.password = await this.hashPassword(signUpCredential.password, newAdminUser.salt); + newAdminUser.firstName = signUpCredential.firstName; + newAdminUser.lastName = signUpCredential.lastName; newAdminUser.isAdmin = true; try { const savedNewAdminUserUser = await this.userRepository.save(newAdminUser); - return { - id: savedNewAdminUserUser.id, - email: savedNewAdminUserUser.email, - firstName: savedNewAdminUserUser.firstName, - lastName: savedNewAdminUserUser.lastName, - createdAt: savedNewAdminUserUser.createdAt, - }; - + return mapUser(savedNewAdminUserUser); } catch (e) { Logger.error('e', 'signUp'); throw new InternalServerErrorException('Failed to register new admin user'); diff --git a/server/src/api-v1/user/dto/create-user.dto.ts b/server/src/api-v1/user/dto/create-user.dto.ts index 5b94c95f6b..fbd63a22f4 100644 --- a/server/src/api-v1/user/dto/create-user.dto.ts +++ b/server/src/api-v1/user/dto/create-user.dto.ts @@ -14,14 +14,14 @@ export class CreateUserDto { lastName: string; @IsOptional() - profileImagePath: string; + profileImagePath?: string; @IsOptional() - isAdmin: boolean; + isAdmin?: boolean; @IsOptional() - isFirstLoggedIn: boolean; + isFirstLoggedIn?: boolean; @IsOptional() - id: string; + id?: string; } diff --git a/server/src/api-v1/user/response-dto/user.ts b/server/src/api-v1/user/response-dto/user.ts new file mode 100644 index 0000000000..9f96380a9a --- /dev/null +++ b/server/src/api-v1/user/response-dto/user.ts @@ -0,0 +1,19 @@ +import { UserEntity } from '../entities/user.entity'; + +export interface User { + id: string; + email: string; + firstName: string; + lastName: string; + createdAt: string; +} + +export function mapUser(entity: UserEntity): User { + return { + id: entity.id, + email: entity.email, + firstName: entity.firstName, + lastName: entity.lastName, + createdAt: entity.createdAt, + }; +} diff --git a/server/src/api-v1/user/user.service.ts b/server/src/api-v1/user/user.service.ts index cc5954b8ba..e24bb1784e 100644 --- a/server/src/api-v1/user/user.service.ts +++ b/server/src/api-v1/user/user.service.ts @@ -6,19 +6,18 @@ import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; import { UserEntity } from './entities/user.entity'; import * as bcrypt from 'bcrypt'; -import sharp from 'sharp'; -import { createReadStream, unlink, unlinkSync } from 'fs'; +import { createReadStream } from 'fs'; import { Response as Res } from 'express'; +import { mapUser, User } from './response-dto/user'; @Injectable() export class UserService { constructor( @InjectRepository(UserEntity) private userRepository: Repository, - ) { } + ) {} async getAllUsers(authUser: AuthUserDto, isAll: boolean) { - if (isAll) { return await this.userRepository.find(); } @@ -26,8 +25,8 @@ export class UserService { return await this.userRepository.find({ where: { id: Not(authUser.id) }, order: { - createdAt: 'DESC' - } + createdAt: 'DESC', + }, }); } @@ -40,14 +39,12 @@ export class UserService { users = await this.userRepository.find(); } - return { - userCount: users.length - } - + userCount: users.length, + }; } - async createUser(createUserDto: CreateUserDto) { + async createUser(createUserDto: CreateUserDto): Promise { const user = await this.userRepository.findOne({ where: { email: createUserDto.email } }); if (user) { @@ -62,18 +59,10 @@ export class UserService { newUser.lastName = createUserDto.lastName; newUser.isAdmin = false; - try { const savedUser = await this.userRepository.save(newUser); - return { - id: savedUser.id, - email: savedUser.email, - firstName: savedUser.firstName, - lastName: savedUser.lastName, - createdAt: savedUser.createdAt, - }; - + return mapUser(savedUser); } catch (e) { Logger.error(e, 'Create new user'); throw new InternalServerErrorException('Failed to register new user'); @@ -84,7 +73,6 @@ export class UserService { return bcrypt.hash(password, salt); } - async updateUser(updateUserDto: UpdateUserDto) { const user = await this.userRepository.findOne(updateUserDto.id); @@ -100,10 +88,10 @@ export class UserService { } if (updateUserDto.isAdmin) { - const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } }) + const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } }); if (adminUser) { - throw new BadRequestException("Admin user exists") + throw new BadRequestException('Admin user exists'); } user.isAdmin = true; @@ -120,7 +108,6 @@ export class UserService { isAdmin: updatedUser.isAdmin, profileImagePath: updatedUser.profileImagePath, }; - } catch (e) { Logger.error(e, 'Create new user'); throw new InternalServerErrorException('Failed to register new user'); @@ -130,13 +117,12 @@ export class UserService { async createProfileImage(authUser: AuthUserDto, fileInfo: Express.Multer.File) { try { await this.userRepository.update(authUser.id, { - profileImagePath: fileInfo.path - }) - + profileImagePath: fileInfo.path, + }); return { userId: authUser.id, - profileImagePath: fileInfo.path + profileImagePath: fileInfo.path, }; } catch (e) { Logger.error(e, 'Create User Profile Image'); @@ -146,7 +132,7 @@ export class UserService { async getUserProfileImage(userId: string, res: Res) { try { - const user = await this.userRepository.findOne({ id: userId }) + const user = await this.userRepository.findOne({ id: userId }); if (!user.profileImagePath) { // throw new BadRequestException('User does not have a profile image'); @@ -157,11 +143,10 @@ export class UserService { res.set({ 'Content-Type': 'image/jpeg', }); - const fileStream = createReadStream(user.profileImagePath) + const fileStream = createReadStream(user.profileImagePath); return new StreamableFile(fileStream); } catch (e) { - console.log("error getting user profile") + console.log('error getting user profile'); } - } } diff --git a/server/src/config/database.config.ts b/server/src/config/database.config.ts index 90a09f5ae8..d635b5113a 100644 --- a/server/src/config/database.config.ts +++ b/server/src/config/database.config.ts @@ -9,9 +9,11 @@ export const databaseConfig: TypeOrmModuleOptions = { database: process.env.DB_DATABASE_NAME, entities: [__dirname + '/../**/*.entity.{js,ts}'], synchronize: false, - migrations: [__dirname + '/../migration/*.js'], + migrations: [__dirname + '/../migration/*.{js,ts}'], cli: { migrationsDir: __dirname + '/../migration', }, migrationsRun: true, }; + +export default databaseConfig; diff --git a/server/test/user.e2e-spec.ts b/server/test/user.e2e-spec.ts index 4d6d373eb0..5657c94efd 100644 --- a/server/test/user.e2e-spec.ts +++ b/server/test/user.e2e-spec.ts @@ -5,14 +5,13 @@ import request from 'supertest'; import { clearDb, authCustom } from './test-utils'; import { databaseConfig } from '../src/config/database.config'; import { UserModule } from '../src/api-v1/user/user.module'; -import { AuthModule } from '../src/api-v1/auth/auth.module'; -import { AuthService } from '../src/api-v1/auth/auth.service'; import { ImmichJwtModule } from '../src/modules/immich-jwt/immich-jwt.module'; -import { SignUpDto } from '../src/api-v1/auth/dto/sign-up.dto'; -import { AuthUserDto } from '../src/decorators/auth-user.decorator'; +import { UserService } from '../src/api-v1/user/user.service'; +import { CreateUserDto } from '../src/api-v1/user/dto/create-user.dto'; +import { User } from '../src/api-v1/user/response-dto/user'; -function _createUser(authService: AuthService, data: SignUpDto) { - return authService.signUp(data); +function _createUser(userService: UserService, data: CreateUserDto) { + return userService.createUser(data); } describe('User', () => { @@ -44,17 +43,17 @@ describe('User', () => { }); describe('with auth', () => { - let authService: AuthService; - let authUser: AuthUserDto; + let userService: UserService; + let authUser: User; beforeAll(async () => { const builder = Test.createTestingModule({ - imports: [UserModule, AuthModule, TypeOrmModule.forRoot(databaseConfig)], + imports: [UserModule, TypeOrmModule.forRoot(databaseConfig)], }); const moduleFixture: TestingModule = await authCustom(builder, () => authUser).compile(); app = moduleFixture.createNestApplication(); - authService = app.get(AuthService); + userService = app.get(UserService); await app.init(); }); @@ -65,9 +64,24 @@ describe('User', () => { beforeAll(async () => { await Promise.allSettled([ - _createUser(authService, { email: authUserEmail, password: '1234' }).then((user) => (authUser = user)), - _createUser(authService, { email: userOneEmail, password: '1234' }), - _createUser(authService, { email: userTwoEmail, password: '1234' }), + _createUser(userService, { + firstName: 'auth-user', + lastName: 'test', + email: authUserEmail, + password: '1234', + }).then((user) => (authUser = user)), + _createUser(userService, { + firstName: 'one', + lastName: 'test', + email: userOneEmail, + password: '1234', + }), + _createUser(userService, { + firstName: 'two', + lastName: 'test', + email: userTwoEmail, + password: '1234', + }), ]); }); @@ -79,13 +93,23 @@ describe('User', () => { expect.arrayContaining([ { email: userOneEmail, + firstName: 'one', + lastName: 'test', id: expect.anything(), createdAt: expect.anything(), + isAdmin: false, + isFirstLoggedIn: true, + profileImagePath: '', }, { email: userTwoEmail, + firstName: 'two', + lastName: 'test', id: expect.anything(), createdAt: expect.anything(), + isAdmin: false, + isFirstLoggedIn: true, + profileImagePath: '', }, ]), );