[MVC 구현하기 - 2단계] 유콩(김유빈) 미션 제출합니다 #251
Conversation
| @@ -16,8 +16,8 @@ public class HandlerExecution { | |||
| private final Object clazz; | |||
There was a problem hiding this comment.
이 멤버필드는 실제 요청을 처리하는 객체일텐데요. 필드이름을 좀 더 명확하게 지어보는건 어떨까요?
There was a problem hiding this comment.
앗 생성자에서 받아오는건 controller 로 변경했는데 여기는 빠트렸네요 😅
| final HandlerKey handlerKey = new HandlerKey(request.getRequestURI(), RequestMethod.from(request.getMethod())); | ||
| log.info("request handler [{}]", handlerKey); | ||
| if (!handlerExecutions.containsKey(handlerKey)) { | ||
| throw new IllegalArgumentException(String.format("요청한 핸들러가 존재하지 않습니다. [%s]", handlerKey)); |
There was a problem hiding this comment.
현재 ManualHandlerMapping의 경우 핸들러가 없는 경우 null이 반환되고 여기서는 예외가 발생하는군요!!
물론 ManualHandlerMapping을 먼저 등록해주기 때문에 별 문제가 없지만 만약 이 순서가 바뀐다면 ManualHandlerMapping의 Handler는 동작하지 않을 것 같네요!!
지금도 좋지만 일관적으로 null 이나 Optional을 반환하도록 하고 registry에서 예외를 던지는건 어떨까요?!
There was a problem hiding this comment.
꼼꼼한 리뷰 너무 감사해요 HandlerMapping 을 관리하는 Registry 에서 처리하도록 수정했습니다!
| return request; | ||
| } | ||
|
|
||
| public static HttpServletRequest getRequest(final String path) { |
There was a problem hiding this comment.
| public static HttpServletRequest getRequest(final String path) { | |
| public static HttpServletRequest getRequest(final String path, final RequestMethod method) |
이런식으로 클라이언트에서 인자를 넣도록하는건 어떨까요?!
그러면 하나의 Fixture를 범용적으로 사용할 수 있을것 같아서요!
|
|
||
| @DisplayName("annotation 기반 handler adapter 초기화") | ||
| @Test | ||
| void annotationInit() { |
There was a problem hiding this comment.
이 테스트는 요청을 처리하는 핸들러가 잘 동작했는가 를 검증하는 테스트 같은데 이름을 좀 더 명확하게 하는게 어떨까요?!
|
|
||
| @DisplayName("annotation 기반 handler mapping 초기화") | ||
| @Test | ||
| void annotationInit() { |
There was a problem hiding this comment.
요청을 처리할 수 있는 핸들러를 반환하는지 검증한다 를 테스트하는 것 같은데 테스트 이름을 명확하게 지어준다면 좋을 것 같아요!!
여기에 더해서 요청을 처리할 핸들러가 없는 경우도 검증해준다면 좋을 것 같다는 생각이 들었습니다!!
안녕하세요 칙촉 잘 지내고 계신가요ㅎㅎ
2단계 미션 제출합니다
1단계 미션의 피드백도 일부 적용하였고 답변도 달아두었습니다!
이번에도 잘 부탁드려요 😊
✓ 체크리스트