-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[COZY-232] feat : 메일 인증 기능 추가 #100
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그냥 하나 제안입니다~
|
||
public void sendUniversityAuthenticationCode(String clientId, String mailAddress) { | ||
SimpleMailMessage message = new SimpleMailMessage(); | ||
String authCode = UUID.randomUUID().toString().substring(0, 6); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UUID를 randomUUID로 생성해서 6자리를 가져오는건 정말 좋은 방식이라고 생각합니다.
추가로 제안드리고 싶은 부분은 Base64 인코딩입니다.
uuid는 16진수로 authCode의 가지수가 대략 1600만 정도가 존재하는데요. 엄청 큰 수이긴 하지만, 좀 더 가지수를 늘린다면 Base64 인코딩을 하는게 좋겠다는 생각입니다. Base64로 6자리 코드를 만들어 제공한다면 대략 690억 정도의 가지수가 존재하게 됩니다. ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 감사합니다아 적용해볼게여~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~한가지 궁금증 있어서 남겼습니다 고생하셨어요!!
message.setText("COZYMATE 대학교 메일인증 코드입니다 : " + authCode); | ||
mailSender.send(message); | ||
|
||
memberAuthenticationCodeMap.put(clientId, authCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 아직 메일 관련 서비스를 만들어보지 않아서 생긴 궁금증인데, 디비 말고 메모리 안에 저장한 이유가 있나요??
제가 생각한 장점은 메모리 접근이 빠르고, 디비 쿼리를 날릴 필요가 없어서 오버헤드가 적다이고, 일정 시간동안만 유효한 데이터라 그런거 같은데, 이런 이유가 맞나요??
만약에 서버가 재시작되는 시간에, 메일 인증을 받고 있다면, 사용중인 인증코드가 날아갈 수도 있을 거 같은데, 이런 경우도 유지가 되는지 궁금합니다~~!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이 부분 생각하긴 했는데,
일단 단일 서버에서는 효율적이라고 생각합니다.
말해주신 이야기대로
- 메모리 접근을 하기 때문에 빠르며
- DB에 접근을 하지 않아도 되기 때문에 통신 속도를 고려하지 않아도 되며
- 마찬가지로 DB와 연관이 없기 때문에 부하를 주지 않는다.
가 해당 방식에서 볼 수 잇는 장점이라고 봅니다.
여기서 생길 수 있는 단점은
- 서버가 다중 서버가 되었을 때 해당 정보를 통해서 인증을 하기 어렵다. (어떤 서버에는 데이터가 존재하겠지만, 그 서버에만 있을 것이기 때문에)
- 서버가 터지거나 꺼지면 사라진다. (사실 이건 크게 상관이 없다고 느끼는데, 인증코드를 거치는 과정이 사용자당 한번이라 데이터가 사라지면 다시 인증과정을 거치도록 해도 문제 없을 것 같습니다.)
- 인증코드의 만료기간이 지났을 때 삭제과정을 거치지 않는다면, 사용자가 엄청나게 많아졌을 때(비약이긴 하지만) 메모리가 넘치는 현상도 생길 수 있을 것이다.
정도라고 생각합니다.
그래서 이런 문제를 해결하기 위해 Redis 도입을 고려하셨고, Redis를 도입하게 되면 모든 문제가 해결되기는 합니다...ㅎ
하지만 이 인증과정을 위해 Redis를 도입하는 것은 지나치게 오버엔지니어링이라 말을 전달드렸고...
(한번 경험을 해보고싶은 목적이 있다면 써도 되지만) 이 과정을 효율적으로 처리하기 위한 다른 방법을 찾아보긴 해야할 것 같습니다.
그래서 저는 그냥 DB에 데이터를 저장하고 (1, 2 해결) 이벤트 브릿지나 스케줄러를 통해서 정기적으로 삭제 (3번도 해결)하는 방식이 좋지않을까 생각합니다.
장점 중 빠르고, DB에 부하를 주지 않는다는것은 해당 기능에서는 그렇게 중요한 요소라고 보이지는 않기 때문입니다. (사용자당 1번의 인증 과정만을 거치는게 대부분일테니 DB에 부하는 눈에 띄지도 않는 수준일 뿐더러, 빠른 응답이 필수적인 그런 부분도 아니라고 생각합니다)
주절주절...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
대리답변 감사드립니다..
아마 분산서버 도입에 대비해서 추후에 DB에 저장해두려구요
그리고 굳이 쓸데없는 데이터 저장하고 있는 건 별로라 생각돼서 스케줄러돌리면서 삭제하려고 해요~
36c599b
to
ff8d3c2
Compare
[REVIEW]안녕하세요. 코드 리뷰 감사합니다. 아래와 같이 개선할 점을 제안드립니다.
전반적으로 잘 구현되었습니다. 일부 성능 및 가독성 개선, 유효성 검사 추가 등의 피드백을 드렸습니다. 전반적인 코드 구조와 로직은 매우 좋다고 생각합니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!! 고생하셨습니다~
ff8d3c2
to
8b42227
Compare
리뷰해드려요~
|
8b42227
to
7617c62
Compare
리뷰해드려요~
|
#️⃣ 요약 설명
메일 보내기 기능을 추가했습니다.
아직 메일 패턴적용은 안했구여.. 인증코드 생성해서 메일 보내고, 검증하는 api만 만들었습니다.
일단 지금은 자바 맵으로 구현했습니다.
📝 작업 내용
6자리 코드를 만들어 메일을 보내는 코드입니다.
메일로 보낸 인증코드와 사용자가 보낸 코드의 일치여부를 판단하는 메소드 입니다.
Response는 다음과 같습니다.
동작 확인
💬 리뷰 요구사항(선택)
도메인의 역할이 단순해서 멤버 도메인에 포함시킬까 고민했습니다.
역할이 많지 않아 Converter를 적용하지 않았는데 적용하라고 하면 하겠습니다!!
추후에 Redis를 사용해볼까 고민중입니다!