JongHoon #1
JongHoon #1
Conversation
JunJongHun
commented
Nov 21, 2023
•
JunJongHun
commented
- test코드 작성하기
| import { ApiProperty } from '@nestjs/swagger'; | ||
|
|
||
| export class CreateCertificationPhoneResponseDto { | ||
| // TODO: 반환할 때 유효성 검사를 해야하나? 한다면 어떻게 하나? validation pipe가 먹지 않는다. |
There was a problem hiding this comment.
응답 데이터는 서비스 레이어에서 만들기 때문에 유효성 검사를 하지 않아도 됩니다
There was a problem hiding this comment.
응답 데이터에 유효성 검사를 안하는 이유가 서비스 레이어 에서 값을 잘 만들어서 보내기 때문인가요?!
개인적으로 백엔드에서 값을 당연히 잘 만들어서 보내겠지만 혹시나? 잘못 만들어서 보낼 수도 있으니, 응답이 나가기 전에도 유효성 검사를 하면 좋지 않을까 생각해봤습니다~
만약 응답값에 대해서 유효성 검사를 한다고 하면 서비스 로직에서 검사하는게 좋을까요?!
There was a problem hiding this comment.
제가 생각하는 유효성 검사는 비즈니스로직을 처리하기 이전에 일어나는 행위로, 요청 데이터가 정해진 규약에 따라 들어오는지를 확인하는 작업입니다.
응답 데이터는 비즈니스 로직에서 만들어지기에 이를 검사하는 것은 비즈니스 로직이 2번 수행되는 것과 다름이 없지 않을까 생각합니다.
만약 특정 데이터가 들어왔을 때, 반환되는 응답 데이터를 확인 혹은 검증하고 싶은 거라면 테스트 코드가 그 역할을 해줄 것이라 생각합니다.
| @Column() | ||
| certificationCode: string; | ||
|
|
||
| //TODO: 유효시간 default 설정하기 ? 아니면 서비스에서 설정하기 ? |
There was a problem hiding this comment.
생성자 방식으로 엔티티를 생성한다면,
CertificationPhoneEntity(phoneNumber: string, certificationCode: string) {
CertificationPhoneEntity entity = new CertificationPhoneEntity();
entity.phoneNumber = phoneNumber;
entity.certificationCode = certificationCode;
entity.expiredAt = new Date.now().plusDays(1);
}이런 식으로 기본 값 할당을 할 수 있을 거 같아요. ts 문법 기억이 잘 안나서 .. 코틀린과 자바 그 사이 어딘가의 문법입니다.
There was a problem hiding this comment.
유효시간이 정해져 있는 데이터는 다른 서버를 사용해 볼 수도 있을 거 같아요. 가령 Redis라던가 아니면 로컬 캐싱이라던가?
유저가 인증 요청을 여러번 보내면, 그 인증코드를 모두 디비에 저장할건가요..? 서비스 레이어에서 방어로직이 있지 않다면 데이터가 계속 쌓이게 될 거고 나중에 검증 시, 어떤 코드값으로 할지도 모호해질 거 같아요.
There was a problem hiding this comment.
유효시간이 정해져 있는 데이터는 다른 서버를 사용해 볼 수도 있을 거 같아요. 가령 Redis라던가 아니면 로컬 캐싱이라던가?
- 그렇군요? redis 또는 로컬 캐싱에 저장을 고려해보는 이유는 유효기간이 지난 데이터는 더 이상 사용하지 않을 것이기 때문에 redis나 로컬 캐싱을 고려해보는게 맞을까요? 아니면 다른 이유가 또 있을까요?
유저가 인증 요청을 여러번 보내면, 그 인증코드를 모두 디비에 저장할건가요..? 서비스 레이어에서 방어로직이 있지 않다면 데이터가 계속 쌓이게 될 거고 나중에 검증 시, 어떤 코드값으로 할지도 모호해질 거 같아요.
-
저는 인증 코드를 모두 디비에 저장하는 방식을 선택했습니다! 서비스 레이어에서는 데이터가 계속 쌓이는 걸 방지하기 위해 기존에 인증요청한 핸드폰 번호가 있다면 소프트로 지우고 새로 만드는 방식을 생각해봤습니다. 데이터를 소프트로 삭제하더라도 db에는 남아있기 때문에 좋은 방법은 아니라고 볼 수 있을까요?!
-
인증 코드값은 최신 순으로 생성된 값을 불러오도록 작성했는데 이렇게 한다면 코드값을 선택하는 방법은 괜찮지 않을까요?
There was a problem hiding this comment.
만일 시나리오가 아래와 같은 상황이라면 어떻게 동작할까요??
유저 A : 결제를 위해 휴대폰 인증 요청을 한다.
시스템 : 인증코드를 생성하고, 유저 A에게 반환한다.
해커 : 유저 A 전화번호를 해킹해서 휴대폰 인증 요청을 한다.
시스템 : 인증코드를 생성하고, 해커에게 반환한다.
유저 A : 인증코드를 검증 요청한다.
시스템 : 가장 마지막에 전송된 코드가 아니므로 실패 처리한다.
해커 : 인증코드를 검증 요청한다.
시스템 : 가장 마지막에 전송된 코드이므로 성공 처리한다.
만약 제 3자가 인증 요청을 1번 이상 연속적으로 보내게 되면 실제 요청에 대한 응답이 성공으로 이루어지지 않을 수도 있으며, 요청 만큼 응답 코드를 문자로 전송하기에 비용이 발생하게 됩니다.
-> 연속 요청을 막거나, 최초 생성된 인증코드만 유효한 값으로 보거나 등의 서비스 정책에 따라 처리할 수 있을 거 같아요..!
레디스나 로컬 캐싱이 반드시 정답은 아니지만, 한 번 사용되고 더 이상 사용하지 않을 인증코드 값을 영구적으로 갖고 있을 이유는 없다고 생각합니다. 이 역시 메모리를 차지하기에 비용이 발생한다고 볼 수 있기 때문이죠.
| ); | ||
| } | ||
| } catch (e) { | ||
| throw new BadRequestException('DB 데이터 삭제에 실패하였습니다.'); |
There was a problem hiding this comment.
데이터 삭제 실패 오류는 BadRequest로 보는 것보다는 다른 오류가 맞지 않을까 싶어요.
데이터 삭제 실패의 원인이 데이터가 존재하지 않아서 일 수도 있기에 좀더 로직을 명시적으로 작성하는 편이 좋을 거 같아요.
가령 올바른 전화번호를 넘겼으나, 해당 데이터를 조회하는 시점에 이미 삭제된 데이터라면?
NotFound 가 적절할 수도 있겠죠?
There was a problem hiding this comment.
오.. 여기까지는 생각을 못 해봤는데, 여러가지 경우가 있겠군요! 또한, 명시적으로 코드를 작성하도록 노력해봐야겠어요😄
| try { | ||
| await this.certificationPhoneRepository.save(certificationPhone); | ||
| } catch (e) { | ||
| throw new BadRequestException('DB 데이터 저장에 실패하였습니다.'); |
There was a problem hiding this comment.
저라면 internal server exception을 쓸 거 같아요..! 서버 간 통신에서 발생한 오류이기도 하고, 잘못된 요청으로 인한 저장 실패로 보기엔 모호한 거 같아요.
There was a problem hiding this comment.
맞네요😂 서버에서 에러가 난건데 front에서 400번 에러를 받고 message에는 'db 저장' 실패라고 하면 이상하네요..ㅎㅎ
에러 처리에 대해서 좀 신경을 써보도록 해야겠어요! 감사합니다👍
| throw new BadRequestException('이미 인증이 완료된 휴대폰 번호입니다.'); | ||
| } | ||
|
|
||
| certificationPhone.isVerified = true; |
There was a problem hiding this comment.
엔티티 클래스 안에 함수로 인증 상태 변경을 정의하면 좋을 거 같아요.
certificationPhone.verify();certification.entity.ts
verify() {
certificationEntity.isVerified = true;
}There was a problem hiding this comment.
오 이런 방법도 있군요! 보통 이렇게 많이 하나요??
There was a problem hiding this comment.
저의 경우, 엔티티에 수정이 발생하면 엔티티 클래스 내부에 정의해서 사용하는 편입니다.
setter 사용을 지양하고, 수정되어야 하는 데이터들을 묶어 한 번에 함수로 정의합니다.
이러면 다른 부분에서 엔티티에 변경을 가하고자 할 때, 함수 호출로 대체할 수 있으니 중복 코드를 줄이고 코드의 가독성도 높아집니다.
setter를 쭉 나열하면, 비즈니스 로직이 변경되었을 때 유지보수에 큰 비용이 발생합니다. ( 변경점을 개발자가 모두 찾으러 다녀야 하는 ... )
