[Spring MVC (인증)] 김진학 미션 제출합니다. #121
Conversation
만료된 토큰을 테스트하기 위해 토큰 생성 객체에서 날짜 처리를 분리해 발행날짜를 주입받도록 함 BREAKING CHANGE: 토큰 서명 테스트는 제거됨
로그인 확인 또는 어드민 권한 검사에 중복되는 인증 토큰 추출 로직을 외부 객체로 분리
로그인 확인 또는 어드민 권한 검사에 중복되는 인증 객체 생성 로직을 AuthService 로 분리
키 값이 잘못 사용될 수 있는 Map 객체 대신 Claim 객체 사용
Jackson은 isXXX 메서드를 JSON 필드로 간주하여 직렬화/역직렬화 작업에 포함하므로 @JsonIgnore 로 무시하도록 함
| import org.springframework.web.servlet.HandlerInterceptor; | ||
|
|
||
| @Component | ||
| public class AdminInterceptor implements HandlerInterceptor { |
There was a problem hiding this comment.
Interceptor 를 사용했네요.
Interceptor 와 Filter 의 차이점이 뭔가요?
| public record LoginMember( | ||
| String email, | ||
| String name, | ||
| String role |
There was a problem hiding this comment.
클린코드 관점에서 Role 이라는 ENUM 을 만들어도 될 거 같네요.
isAdmin 역시도 Role 에서 메소드로 가지고 있고 LoginMember 는 호출만 해도 되겠네용
| reservationRequest.put("theme", "1"); | ||
|
|
||
| //when | ||
| ReservationResponse reservationResponse = sendCreateReservationsRequest(reservationRequest, token).as( |
There was a problem hiding this comment.
직접 DTO 를 넘긴게 아니라 MAP 으로 넘겼는데 의도가 있을까요?
( 틀린게 아님. 의견 묻는거임 )
| @Test | ||
| void 비로그인_상태에서_예약할_경우_예약에_실패한다() { | ||
| //given | ||
| Map<String, String> reservationRequest = new HashMap<>(); |
There was a problem hiding this comment.
request 를 만드는 부분도 private 메소드로 더 간편하게 해도 되겠네요.
| throws Exception { | ||
|
|
||
| String token = authorizationExtractor.extract(request); | ||
| LoginMember loginMember = authService.createAuthentication(token); |
There was a problem hiding this comment.
LoginMember.from() 부분에서 Claims 에 해당하는 값이 없으면 예외가 터질거 같은데 이 경우 어떻게 되나요?
| import org.springframework.web.method.support.ModelAndViewContainer; | ||
|
|
||
| @Component | ||
| public class LoginMemberArgumentResolver implements HandlerMethodArgumentResolver { |
There was a problem hiding this comment.
HandlerMethodArgumentResolver 가 정확하게 하는게 뭔가요?
| if (reservationRequest.getName() == null | ||
| || reservationRequest.getDate() == null | ||
| public ResponseEntity create(@RequestBody ReservationRequest reservationRequest, LoginMember loginMember) { | ||
| if (reservationRequest.getDate() == null |
There was a problem hiding this comment.
물론, 기존 코드에서 검증을 여기서 하지만
DTO 가 스스로가 메소드로 자신의 값들이 타당한지validXXX 와 같은 메소드를 가지는것도 좋을거 같은데 어떤가요??
Controller 가 DTO 의 특정 요소가 null 인걸 알 필요가 있을까? 라고 생각해서 입니다.
| return ResponseEntity.badRequest().body("Invalid reservation request"); | ||
| } | ||
|
|
||
| if (reservationRequest.getName() == null) { |
There was a problem hiding this comment.
이 부분도 위와 비슷하게 getName 이 null 인지 + 새로운 객체 생성을 DTO 내부로 은닉해도 될 거 같아요.
🏃♂️ 과정
이전 단계의 리뷰를 추가로 반영하고 새 단계의 요구사항을 추가했습니다. 역할 분리와 테스트에 대한 관점이 조금씩 만들어지는 것 같습니다. 이번 리뷰도 잘 부탁드립니다.
📃 요구사항
로그인 리팩터링
로직
예약 생성 기능 변경
로직
예외
관리자 기능
로직
🔎 궁금한점
로그인 상태가 필요한 모든 기능들에 대해서 로그인 요청 로직이 중복되었습니다. 이에 따라 아래와 같은 방법을 생각해보았습니다.
연쇄적으로 일어나야 하는 테스트에 대해서 이전 단계가 성공했다고 가정하고 단위 테스트로 쪼개거나, 통합 테스트에서 중복을 해결해야할 것으로 생각되는데 생각의 방향이 맞는지 궁금합니다.