add member authentication and error monitoring - #1
Conversation
| CHECK (membership_grade IN ('BASIC', 'VIP')), | ||
| CONSTRAINT ck_members_role | ||
| CHECK (role IN ('USER', 'ADMIN')), | ||
|
|
There was a problem hiding this comment.
[생각해보면 좋을 포인트]
실제 서비스들에서는 pk정도만 사용하고 fk, ck들은 가급적 사용하지 않고, application level에서 처리하는데
이유는 constraint들이 많아질수록 결국 db도 해당 정보들에 대한 관리를 위한 비용들이 증가하기 때문에, 보통 꼭 필요한 경우가 아니면 최대한 application level에서 처리하고 있습니다.
해당 스키마기준으로 만약 나중에 가입자가 수십만,수백만이상으로 늘어날 경우 일일 가입현황, 어떤 유형의 유저들이 가장 티켓을 많이 구매하는지와 같은 패턴을 분석해보고 싶다면 어떻게 해야할까요?
| request.getRequestId(), | ||
| exception | ||
| ); | ||
|
|
There was a problem hiding this comment.
[확장성 고민해보기]
만약, 사용자에게 좀더 자세하고, 친절하게 어떤 상황과 이슈로 오류가 발생하였는지 알려줄수 있다면
or
특정오류의 경우 어떻게 하라는 대응 가이드를 알려줄수 있다면 현재 코드들이 어떤 형태로 변하게 될까요?
| import com.example.baseballticketing.member.domain.LoginId; | ||
|
|
||
| /** 정규화된 로그인 ID가 이미 사용 중임을 표현하는 애플리케이션 예외다. */ | ||
| public final class DuplicateLoginIdException extends RuntimeException { |
There was a problem hiding this comment.
현재 이 exception도 아래 advice에 의해 예상하지 못한 http 오류입니다로 handling될거 같아보이는데 의도한 동작 결과이실까요?
@ExceptionHandler(Exception.class)
public ResponseEntity<ApiErrorResponse> handleUnexpectedException(
Exception exception,
HttpServletRequest request
) {
// 상세 스택은 서버 로그에서 requestId로 찾고, Discord에는 아래의 제한된 이벤트만 보낸다.
log.error(
"예상하지 못한 HTTP 오류입니다. method={}, path={}, requestId={}",
request.getMethod(),
request.getRequestURI(),
request.getRequestId(),
exception
);
| MemberRepository memberRepository, | ||
| TeamRepository teamRepository, | ||
| PasswordEncoder passwordEncoder | ||
| ) { |
There was a problem hiding this comment.
@Bean
public MemberRegistrationService memberRegistrationService(
MemberRepository memberRepository,
TeamRepository teamRepository,
PasswordEncoder passwordEncoder
) {
return new MemberRegistrationService(
memberRepository,
teamRepository,
passwordEncoder
);
}
와 같이 스프링에게 해당 클래스의 생성/관리/주입의 책임을 다 전달하였기 떄문에 실제 작성한 코드레벨에 도달하기전에
생성자 매개변수들이 존재하는지 체크가 다 되기 떄문에 사실상 garbage 코드
Bean annotation을 통한 명시적 선언을 하지않고 단순히 현재 class에 @service annotation선언만으로 동일한 결과를 볼수있음 -> 현 클래스가 항상 스프링 빈으로 생성/관리되는 목적의 클래스가 아니라면 @bean 으로 선언해서 필요한 모듈에서만 사용할수 있게 할 경우에만 유의미한 의미가 존재
| Team favoriteTeam = findFavoriteTeam(favoriteTeamId); | ||
| String passwordHash = passwordEncoder.encode(rawPassword); | ||
| validateEncodedPassword(rawPassword, passwordHash); | ||
|
|
There was a problem hiding this comment.
[생각해보기]
현재와 같이 코드작성되어 동작할때
begin transaction
...
commit or rollback
과 같은 트랜잭션 처리는 어떻게 동작될거 같으신가요?
| private void validateEncodedPassword( | ||
| String rawPassword, | ||
| String passwordHash | ||
| ) { |
There was a problem hiding this comment.
[tip]
apache stringutils, spring stringutils와 같은 library들을 이용하면
passwordHash == null || passwordHash.isBlank()
이러한 코드들이 짧고 가독성있게 표현 가능
| teamJpaRepository | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
[생각해보기]
composition이 정말 유의미한 경우가 맞는지?에 대해 한번 생각해보시면 좋을거 같아요
대부분의 패턴, 기술은 적절하게 사용되어야 유의미하고, 불필요한 over engineering / showing off적인 따라하기는 오히려 anti pattern이 될수 있다.
(하지만 이러한 것들도 많이 경험해보셔야지만 느낄수 있기 떄문에 지금하고 계신거를 갑자기 확 수정하실필요는 없으니 코멘트들은 참고만 하시면서 고민과 함께 계속 해보시길 추천드립니다)
| throw new IllegalArgumentException( | ||
| "로그인 ID는 4자 이상 50자 이하여야 합니다." | ||
| ); | ||
| } |
There was a problem hiding this comment.
[고민해보기]
어떠한 경우의 validation은 application level에
어떠한 경우의 validation은 constraint를 통해 DB에
-> 어떠한 기준을 가지고 있는가?
-> 장단점은 무엇인가?
-> 장기적으로 관리가 잘될것인가?
와 같은 부분들 한번 고민해보시면 좋을거같아요
| /** | ||
| * MemberRepository port를 JPA로 구현한다. | ||
| * Entity와 Domain을 변환하고 DB가 발견한 로그인 ID 동시 충돌을 애플리케이션 예외로 번역한다. | ||
| */ |
There was a problem hiding this comment.
[생각해보기]
composition 관련해서 코멘트 드린것과 유사한 경우로 생각해볼수 있을거 같은데
port adapter pattern 을 사용했다 vs Member 와 관련된 처리 잘 관리/확장/개선할수 있다
중 어느 쪽에 더 가까운지...?
대부분 패턴/기술들이 목적을 달성하기 위한 수단인데 늘 수단이 목적을 앞서버리는 경우가 over engineering / anti pattern화가 됩니다.
과한건 없는지 ROI가 괜찮은것일지와 같은 부분들을 같이 생각하면서 개발을 하시면 도움이 많이 되실거 같아요
| * 원문 비밀번호나 생성된 해시를 로그로 남기지 않는다. | ||
| */ | ||
| public final class SpringPasswordEncoderAdapter implements PasswordEncoder { | ||
|
|
There was a problem hiding this comment.
port와 adapter도 기본적으로는 Interface Segregation Principle (ISP)
그런데 port/adapter에 대한 강박(?)이 느껴지는거 같네요 ㅎ
2 participants
목적
브라우저 기반 티켓팅 서비스에 회원가입과 서버 세션 인증을 추가하고,
401·403·500 오류를 안전하게 관제할 수 있는 기반을 구축한다.
주요 변경
설계 판단
검증