-
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
Feature/member #1
Conversation
CookieUtil cookieUtil; | ||
TokenUtil tokenUtil; | ||
SignUpUseCase signupUseCase; | ||
LoginUseCase loginUseCase; |
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.
UseCase 가 직관적인 네이밍은 아닌거 같습니다.
계층 구조를 만들고자 한다면 Processor 나 Application 이 좋을 것 같습니다.
판단해주시면 좋을 것 같습니다.
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.
이 부분은 일단 UseCase로 유지해서 사용해보도록 하겠습니다
|
||
ResponseCookie responseCookie = createLoginTokenCookie(member); | ||
|
||
response.addHeader("Set-Cookie", responseCookie.toString()); |
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.
응답이 200 으로만 나가는게 확장성이 떨어져보입니다.
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.
void 로 주는 부분들 전체 포함입니다.
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.
ResponseDto를 사용해서 반환하도록 수정했습니다!
} | ||
|
||
public Member create(CreateMemberDto createDto) { | ||
return memberRepository.save(createDto.toModel(encodePassword(createDto.password()))); |
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.
Salt 껴서 구현하시는걸 권고드립니다.
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.
salt를 포함해서 비밀번호 암호화 로직 수정했습니다!
import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
||
@ResponseStatus(value = HttpStatus.UNAUTHORIZED) | ||
public class UnAuthorized extends CustomException { |
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.
Exception 달아주시면 좋을거 같아요!
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.
예외 클래스명을 전부 ~Exception으로 수정햇습니다!
import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
||
@ResponseStatus(value = HttpStatus.CONFLICT) | ||
public class ConflicException extends CustomException { |
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.
Conflict 오타입니다
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.
확인했습니다. 핸드폰 번호가 유니크인 점에 대해서는 스펙으로 결정하실거죠?
예를들어 핸드폰 번호가 변경되면 다른 사용자가 되니까요.
넵! 회원관리 부분은 간단하게 구현하기 위해서 핸드포번호 변경등은 고려하지 않았습니다! |
Quality Gate passedIssues Measures |
개요
회원 관련 기능 개발
상세정보