From 89d34760466ed95be2599ae45ebc1a93b39f993b Mon Sep 17 00:00:00 2001 From: Lee-won-hyeok Date: Tue, 24 Sep 2024 06:07:55 +0000 Subject: [PATCH 1/5] refactor(be): improve client userr & client notice module error handling logic --- .../client/src/notice/notice.controller.ts | 35 +++-------- .../apps/client/src/notice/notice.service.ts | 58 +++++++++++-------- .../apps/client/src/user/user.controller.ts | 29 +--------- .../apps/client/src/user/user.service.ts | 42 ++++++++------ 4 files changed, 69 insertions(+), 95 deletions(-) diff --git a/apps/backend/apps/client/src/notice/notice.controller.ts b/apps/backend/apps/client/src/notice/notice.controller.ts index c656931e88..f8aa67f129 100644 --- a/apps/backend/apps/client/src/notice/notice.controller.ts +++ b/apps/backend/apps/client/src/notice/notice.controller.ts @@ -3,13 +3,10 @@ import { Get, Query, Param, - NotFoundException, - InternalServerErrorException, Logger, DefaultValuePipe, ParseBoolPipe } from '@nestjs/common' -import { Prisma } from '@prisma/client' import { AuthNotNeededIfOpenSpace } from '@libs/auth' import { CursorValidationPipe, GroupIDPipe, RequiredIntPipe } from '@libs/pipe' import { NoticeService } from './notice.service' @@ -30,18 +27,13 @@ export class NoticeController { @Query('fixed', new DefaultValuePipe(false), ParseBoolPipe) fixed: boolean, @Query('search') search?: string ) { - try { - return await this.noticeService.getNotices({ - cursor, - take, - fixed, - search, - groupId - }) - } catch (error) { - this.logger.error(error) - throw new InternalServerErrorException() - } + return await this.noticeService.getNotices({ + cursor, + take, + fixed, + search, + groupId + }) } @Get(':id') @@ -49,17 +41,6 @@ export class NoticeController { @Query('groupId', GroupIDPipe) groupId: number, @Param('id', new RequiredIntPipe('id')) id: number ) { - try { - return await this.noticeService.getNoticeByID(id, groupId) - } catch (error) { - if ( - error instanceof Prisma.PrismaClientKnownRequestError && - error.name === 'NotFoundError' - ) { - throw new NotFoundException(error.message) - } - this.logger.error(error) - throw new InternalServerErrorException() - } + return await this.noticeService.getNoticeByID(id, groupId) } } diff --git a/apps/backend/apps/client/src/notice/notice.service.ts b/apps/backend/apps/client/src/notice/notice.service.ts index 4acb4b86bd..a1cadb00d7 100644 --- a/apps/backend/apps/client/src/notice/notice.service.ts +++ b/apps/backend/apps/client/src/notice/notice.service.ts @@ -1,5 +1,6 @@ import { Injectable } from '@nestjs/common' import { OPEN_SPACE_ID } from '@libs/constants' +import { EntityNotExistException } from '@libs/exception' import { PrismaService } from '@libs/prisma' @Injectable() @@ -20,7 +21,6 @@ export class NoticeService { groupId?: number }) { const paginator = this.prisma.getPaginator(cursor) - const notices = await this.prisma.notice.findMany({ ...paginator, where: { @@ -53,7 +53,6 @@ export class NoticeService { createdBy: notice.createdBy?.username } }) - const total = await this.prisma.notice.count({ where: { groupId, @@ -70,28 +69,29 @@ export class NoticeService { } async getNoticeByID(id: number, groupId = OPEN_SPACE_ID) { - const current = await this.prisma.notice - .findUniqueOrThrow({ - where: { - id, - groupId, - isVisible: true - }, - select: { - title: true, - content: true, - createTime: true, - updateTime: true, - createdBy: { - select: { - username: true - } + const notice = await this.prisma.notice.findUnique({ + where: { + id, + groupId, + isVisible: true + }, + select: { + title: true, + content: true, + createTime: true, + updateTime: true, + createdBy: { + select: { + username: true } } - }) - .then((notice) => { - return { ...notice, createdBy: notice.createdBy?.username } - }) + } + }) + + if (!notice) { + throw new EntityNotExistException('notice') + } + const current = { ...notice, createdBy: notice.createdBy?.username } const navigate = (pos: 'prev' | 'next') => { type Order = 'asc' | 'desc' @@ -115,10 +115,20 @@ export class NoticeService { } } + const prev = await this.prisma.notice.findFirst(navigate('prev')) + const next = await this.prisma.notice.findFirst(navigate('next')) + + if (!prev) { + throw new EntityNotExistException('prev notice') + } + if (!next) { + throw new EntityNotExistException('next notice') + } + return { current, - prev: await this.prisma.notice.findFirst(navigate('prev')), - next: await this.prisma.notice.findFirst(navigate('next')) + prev, + next } } } diff --git a/apps/backend/apps/client/src/user/user.controller.ts b/apps/backend/apps/client/src/user/user.controller.ts index c4a63e2e40..68f75ab13b 100644 --- a/apps/backend/apps/client/src/user/user.controller.ts +++ b/apps/backend/apps/client/src/user/user.controller.ts @@ -8,18 +8,10 @@ import { Controller, Logger, Delete, - Query, - NotFoundException, - InternalServerErrorException + Query } from '@nestjs/common' -import { Prisma } from '@prisma/client' import { Request, type Response } from 'express' import { AuthenticatedRequest, AuthNotNeededIfOpenSpace } from '@libs/auth' -import { - EntityNotExistException, - UnidentifiedException, - UnprocessableDataException -} from '@libs/exception' import { DeleteUserDto } from './dto/deleteUser.dto' import { EmailAuthenticationPinDto } from './dto/email-auth-pin.dto' import { NewPasswordDto } from './dto/newPassword.dto' @@ -96,24 +88,7 @@ export class UserController { @Req() req: AuthenticatedRequest, @Body() updateUserDto: UpdateUserDto ) { - try { - return await this.userService.updateUser(req, updateUserDto) - } catch (error) { - if ( - error instanceof Prisma.PrismaClientKnownRequestError && - error.name == 'NotFoundError' - ) { - throw new NotFoundException(error.message) - } else if ( - error instanceof EntityNotExistException || - error instanceof UnprocessableDataException || - error instanceof UnidentifiedException - ) { - throw error.convert2HTTPException() - } - this.logger.error(error) - throw new InternalServerErrorException() - } + return await this.userService.updateUser(req, updateUserDto) } } diff --git a/apps/backend/apps/client/src/user/user.service.ts b/apps/backend/apps/client/src/user/user.service.ts index 3e1485b828..99df1434b1 100644 --- a/apps/backend/apps/client/src/user/user.service.ts +++ b/apps/backend/apps/client/src/user/user.service.ts @@ -583,10 +583,15 @@ export class UserService { if (!this.isValidPassword(updateUserDto.newPassword)) { throw new UnprocessableDataException('Bad new password') } - encryptedNewPassword = await hash( - updateUserDto.newPassword, - ARGON2_HASH_OPTION - ) + + try { + encryptedNewPassword = await hash( + updateUserDto.newPassword, + ARGON2_HASH_OPTION + ) + } catch (error) { + throw new UnprocessableDataException(error.message) + } } const updateData = { @@ -598,20 +603,23 @@ export class UserService { } } - const updatedUser = await this.prisma.user.update({ - where: { id: req.user.id }, - data: updateData, - select: { - // don't select password for security - studentId: true, - major: true, - userProfile: { - select: { - realName: true + try { + return await this.prisma.user.update({ + where: { id: req.user.id }, + data: updateData, + select: { + // don't select password for security + studentId: true, + major: true, + userProfile: { + select: { + realName: true + } } } - } - }) - return updatedUser + }) + } catch (error) { + throw new UnprocessableDataException(error.message) + } } } From c01bd918973d158244912d57d0d6b4fe7096355e Mon Sep 17 00:00:00 2001 From: Lee-won-hyeok Date: Tue, 24 Sep 2024 06:20:13 +0000 Subject: [PATCH 2/5] refactor(be): minor change --- .../apps/client/src/notice/notice.controller.ts | 3 --- .../apps/client/src/notice/notice.service.ts | 14 ++------------ .../apps/client/src/user/user.controller.ts | 5 ----- apps/backend/apps/client/src/user/user.service.ts | 12 ++++-------- 4 files changed, 6 insertions(+), 28 deletions(-) diff --git a/apps/backend/apps/client/src/notice/notice.controller.ts b/apps/backend/apps/client/src/notice/notice.controller.ts index f8aa67f129..0dec24412e 100644 --- a/apps/backend/apps/client/src/notice/notice.controller.ts +++ b/apps/backend/apps/client/src/notice/notice.controller.ts @@ -3,7 +3,6 @@ import { Get, Query, Param, - Logger, DefaultValuePipe, ParseBoolPipe } from '@nestjs/common' @@ -14,8 +13,6 @@ import { NoticeService } from './notice.service' @Controller('notice') @AuthNotNeededIfOpenSpace() export class NoticeController { - private readonly logger = new Logger(NoticeController.name) - constructor(private readonly noticeService: NoticeService) {} @Get() diff --git a/apps/backend/apps/client/src/notice/notice.service.ts b/apps/backend/apps/client/src/notice/notice.service.ts index a1cadb00d7..c4ebfd1e6e 100644 --- a/apps/backend/apps/client/src/notice/notice.service.ts +++ b/apps/backend/apps/client/src/notice/notice.service.ts @@ -115,20 +115,10 @@ export class NoticeService { } } - const prev = await this.prisma.notice.findFirst(navigate('prev')) - const next = await this.prisma.notice.findFirst(navigate('next')) - - if (!prev) { - throw new EntityNotExistException('prev notice') - } - if (!next) { - throw new EntityNotExistException('next notice') - } - return { current, - prev, - next + prev: await this.prisma.notice.findFirst(navigate('prev')), + next: await this.prisma.notice.findFirst(navigate('next')) } } } diff --git a/apps/backend/apps/client/src/user/user.controller.ts b/apps/backend/apps/client/src/user/user.controller.ts index 68f75ab13b..39df929500 100644 --- a/apps/backend/apps/client/src/user/user.controller.ts +++ b/apps/backend/apps/client/src/user/user.controller.ts @@ -6,7 +6,6 @@ import { Req, Res, Controller, - Logger, Delete, Query } from '@nestjs/common' @@ -25,8 +24,6 @@ import { UserService } from './user.service' @Controller('user') export class UserController { - private readonly logger = new Logger(UserController.name) - constructor(private readonly userService: UserService) {} @Patch('password-reset') @@ -95,8 +92,6 @@ export class UserController { @Controller('email-auth') @AuthNotNeededIfOpenSpace() export class EmailAuthenticationController { - private readonly logger = new Logger(EmailAuthenticationController.name) - constructor(private readonly userService: UserService) {} setJwtInHeader(res: Response, jwt: string) { diff --git a/apps/backend/apps/client/src/user/user.service.ts b/apps/backend/apps/client/src/user/user.service.ts index 99df1434b1..74b6fc448d 100644 --- a/apps/backend/apps/client/src/user/user.service.ts +++ b/apps/backend/apps/client/src/user/user.service.ts @@ -584,14 +584,10 @@ export class UserService { throw new UnprocessableDataException('Bad new password') } - try { - encryptedNewPassword = await hash( - updateUserDto.newPassword, - ARGON2_HASH_OPTION - ) - } catch (error) { - throw new UnprocessableDataException(error.message) - } + encryptedNewPassword = await hash( + updateUserDto.newPassword, + ARGON2_HASH_OPTION + ) } const updateData = { From fbe3567179e2667e44ab0c7119b64911fb96476d Mon Sep 17 00:00:00 2001 From: Jaehyeon Kim <65964601+Jaehyeon1020@users.noreply.github.com> Date: Sat, 4 Jan 2025 03:59:54 +0000 Subject: [PATCH 3/5] refactor(be): remove unused exception handling logic --- .../apps/client/src/notice/notice.service.ts | 6 +--- .../apps/client/src/user/user.service.ts | 34 ++++++------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/apps/backend/apps/client/src/notice/notice.service.ts b/apps/backend/apps/client/src/notice/notice.service.ts index c4ebfd1e6e..09080f0762 100644 --- a/apps/backend/apps/client/src/notice/notice.service.ts +++ b/apps/backend/apps/client/src/notice/notice.service.ts @@ -1,6 +1,5 @@ import { Injectable } from '@nestjs/common' import { OPEN_SPACE_ID } from '@libs/constants' -import { EntityNotExistException } from '@libs/exception' import { PrismaService } from '@libs/prisma' @Injectable() @@ -69,7 +68,7 @@ export class NoticeService { } async getNoticeByID(id: number, groupId = OPEN_SPACE_ID) { - const notice = await this.prisma.notice.findUnique({ + const notice = await this.prisma.notice.findUniqueOrThrow({ where: { id, groupId, @@ -88,9 +87,6 @@ export class NoticeService { } }) - if (!notice) { - throw new EntityNotExistException('notice') - } const current = { ...notice, createdBy: notice.createdBy?.username } const navigate = (pos: 'prev' | 'next') => { diff --git a/apps/backend/apps/client/src/user/user.service.ts b/apps/backend/apps/client/src/user/user.service.ts index 74b6fc448d..8e19d35a18 100644 --- a/apps/backend/apps/client/src/user/user.service.ts +++ b/apps/backend/apps/client/src/user/user.service.ts @@ -55,7 +55,7 @@ export class UserService { ) {} async getUsernameByEmail({ email }: UserEmailDto) { - const username = await this.prisma.user.findUnique({ + const username = await this.prisma.user.findUniqueOrThrow({ where: { email }, @@ -63,9 +63,6 @@ export class UserService { username: true } }) - if (!username) { - throw new EntityNotExistException('User') - } this.logger.debug(username, 'getUsernameByEmail') return username @@ -180,26 +177,17 @@ export class UserService { email: string, newPassword: string ): Promise { - try { - const user = await this.prisma.user.update({ - where: { - email - }, - data: { - password: await hash(newPassword, ARGON2_HASH_OPTION) - } - }) - this.logger.debug(user, 'updateUserPasswordInPrisma') + const user = await this.prisma.user.update({ + where: { + email + }, + data: { + password: await hash(newPassword, ARGON2_HASH_OPTION) + } + }) + this.logger.debug(user, 'updateUserPasswordInPrisma') - return user - } catch (error) { - if ( - error instanceof PrismaClientKnownRequestError && - error.code == 'P2025' - ) - throw new EntityNotExistException('User') - throw error - } + return user } async verifyPinAndIssueJwt({ From fc384ef98aef4f7f52d14773c7e8fc4cca329458 Mon Sep 17 00:00:00 2001 From: Jaehyeon Kim <65964601+Jaehyeon1020@users.noreply.github.com> Date: Sat, 4 Jan 2025 04:22:36 +0000 Subject: [PATCH 4/5] test(be): resolve test error --- .../backend/apps/client/src/user/user.service.spec.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/backend/apps/client/src/user/user.service.spec.ts b/apps/backend/apps/client/src/user/user.service.spec.ts index 681b12e163..80cb5b6521 100644 --- a/apps/backend/apps/client/src/user/user.service.spec.ts +++ b/apps/backend/apps/client/src/user/user.service.spec.ts @@ -141,7 +141,7 @@ describe('UserService', () => { describe('getUsernameByEmail', () => { const { email, username } = user it('return username', async () => { - db.user.findUnique.resolves({ username }) + db.user.findUniqueOrThrow.resolves({ username }) expect(await service.getUsernameByEmail({ email })).to.be.deep.equal({ username @@ -149,7 +149,7 @@ describe('UserService', () => { }) it('should not return username', async () => { - db.user.findUnique.resolves(null) + db.user.findUniqueOrThrow.throws(new EntityNotExistException('user')) await expect(service.getUsernameByEmail({ email })).to.be.rejectedWith( EntityNotExistException @@ -400,6 +400,13 @@ describe('UserService', () => { createUserProfileSpy = spy(service, 'createUserProfile') registerUserToPublicGroupSpy = spy(service, 'registerUserToPublicGroup') deletePinFromCacheSpy = stub(service, 'deletePinFromCache') + + db.user.findUniqueOrThrow.resolves({ username: signUpDto.username }) + }) + + after(() => { + db.user.findUnique = stub() + db.user.findUniqueOrThrow = stub() }) it('carry sign up process', async () => { From 42303399ab919ce8ef8b0e9f9a29e2dc35159c5a Mon Sep 17 00:00:00 2001 From: Jaehyeon Kim <65964601+Jaehyeon1020@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:37:55 +0000 Subject: [PATCH 5/5] chore(be): remove unused error handling logic --- .../apps/client/src/user/user.service.ts | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/apps/backend/apps/client/src/user/user.service.ts b/apps/backend/apps/client/src/user/user.service.ts index 8e19d35a18..b83349a227 100644 --- a/apps/backend/apps/client/src/user/user.service.ts +++ b/apps/backend/apps/client/src/user/user.service.ts @@ -587,23 +587,19 @@ export class UserService { } } - try { - return await this.prisma.user.update({ - where: { id: req.user.id }, - data: updateData, - select: { - // don't select password for security - studentId: true, - major: true, - userProfile: { - select: { - realName: true - } + return await this.prisma.user.update({ + where: { id: req.user.id }, + data: updateData, + select: { + // don't select password for security + studentId: true, + major: true, + userProfile: { + select: { + realName: true } } - }) - } catch (error) { - throw new UnprocessableDataException(error.message) - } + } + }) } }