[Spring MVC] 한경준 미션 제출합니다. #42
Conversation
…iguration), ReservationRequest record 변경
There was a problem hiding this comment.
안녕하세요 경준님 리뷰어 이동호입니다.
리뷰가 늦은 이유는 MVC 미션 관련해서 많이 부족하여
추가적으로 공부한 뒤 리뷰해야 될거 같아서 오늘 리뷰드렸습니다.
늦은점 죄송합니다
추가적으로 패키지 분리에 대해서 말해보고 싶습니다.
auth라는 패키지 안에 인터셉터/web/LoginRequest/Token 등 많은 클래스가 있다고 생각됩니다.
auth패키지 안에 추가적으로 web.pakage / Token.pakage 등 세부적으로 나눠서 관리하는게 어떨지 아니면 다르 경로에 여러개의 패키지를 만들어 관리할 지 경준님의 의견의 궁금합니다 : )
| public class AdminInterceptor implements HandlerInterceptor { | ||
| private final MemberDao memberDao; | ||
| private final MemberService memberService; | ||
|
|
||
| public AdminInterceptor(MemberDao memberDao, MemberService memberService) { | ||
| this.memberDao = memberDao; | ||
| this.memberService = memberService; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { | ||
| Long memberId = memberService.checkLogin(request).getId(); | ||
| Member member = memberDao.findById(memberId); | ||
|
|
There was a problem hiding this comment.
MemberDao 는 데이터베이스와 직접적으로 소통하는 객채입니다.
Interceptor 클래스에서 MemberDao를 직접 소환하는게 아닌 memberService만 소환하여
member 객체를 찾는 방법은 어떨까요? : )
| private String secretKey = "Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5E="; | ||
|
|
There was a problem hiding this comment.
직접적으로 토큰키를 명시하기 보다는 application.yaml 파일에 있는 내용을 가지고 오는건 어떨가요??
hint : @value 어노테이션 : )
| if (reservationRequest.name() == null) { | ||
| reservationRequest = new ReservationRequest( | ||
| loginMember.getName(), | ||
| reservationRequest.date(), | ||
| reservationRequest.theme(), | ||
| reservationRequest.time() | ||
| ); | ||
| } | ||
|
|
||
| if (reservationRequest.name() == null | ||
| || reservationRequest.date() == null | ||
| || reservationRequest.theme() == null | ||
| || reservationRequest.time() == null) { | ||
| return ResponseEntity.badRequest().build(); | ||
| } |
There was a problem hiding this comment.
순서를 바꾸고 코드를 작게 수정하면
중복되는 코드없이 더 효율적으로 작성할 수 있을거 같아요 :)
| @GetMapping("/login/check") | ||
| public ResponseEntity<LoginMember> loginCheck(HttpServletRequest request) { | ||
|
|
||
| LoginMember loginMember = memberService.checkLogin(request); | ||
|
|
||
| return ResponseEntity.ok().body(loginMember); | ||
| } |
There was a problem hiding this comment.
단계가 넘어가서 수정이 안된거 같지만
경준님께서 만드신 @authentication 을 활용해보는게 어떨가요??
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target(ElementType.PARAMETER) | ||
| public @interface Authentication { | ||
| } |
There was a problem hiding this comment.
이 부분은 궁금하네요
@annotation 으로 만들지 않고 @interface 로 만드신 이유가 있나요?
안녕하세요 리뷰어님 step2~3 완료 후 다시 PR을 생성하는게 리뷰하기 편할 것 같아서 새로 PR을 만들었습니다.
중점적으로 봐주셨으면 하는 내용