[MVC 구현하기 - 2단계] 디우(김동규) 미션 제출합니다. #214
Conversation
There was a problem hiding this comment.
좋은 아침입니다 디우 !
1단계 마지막 때 주고받았던 내용이 잘 반영된 코드 덕분에
리뷰하면서 재미있었어요 ㅎㅎㅎ
특히, 모든 클래스에 대해서 테스트 작성하신 부분이 인상깊었습니다. 💯 💯
나누고 싶은 내용들 코멘트로 남겨두었으니 시간 나실 때 보시고 의견 남겨주세요.
크게 변경할 부분은 없어서 선택 반영하시고 다시 리뷰 요청보내주시면
바로 머지 때리겠습니다 !!!
ps. 갑자기 궁금한데 왜 닉네임이 디우인가요.. ?
인도에 도시 이름이 디우라네요 ㅋㅋㅋ
https://ko.wikipedia.org/wiki/%EB%94%94%EC%9A%B0
mvc/src/main/java/nextstep/mvc/controller/tobe/ControllerScanner.java
Outdated
Show resolved
Hide resolved
| try { | ||
| return controllerClass.getDeclaredConstructor().newInstance(); | ||
| } catch(NoSuchMethodException | InvocationTargetException | InstantiationException | IllegalAccessException e) { | ||
| throw new NotFoundControllerException(); |
There was a problem hiding this comment.
NotFoundControllerException 네이밍
해당 try-catch 절에서는 어노테이션으로 찾은 controller class 를 인스턴스 화 할 때
리플랙션으로 발생하는 에러 처리를 하는 곳으로 보여집니다!
위 4개의 에러가 생기는 이유에 대해서 고민해보고 네이밍에 대해서 조금 더 고민해보면 어떨까요 ~??
- NoSuchMethodException ( reflections 으로 메서드를 찾을 수 없는 경우)
- InvocationTargetException ( reflections 으로 호출된 메서드나 생성자에 의해 발생하는 에러)
- InstantiationException (reflections 으로 인스턴스 화 할 수 없을 때 )
- IllegalAccessException (reflections 으로 접근하려는 클래스에 접근 제어로 접근할 수 없을 경우)
참고 자료
There was a problem hiding this comment.
오!! 이렇게 예외들 정리해주시고 참고자료 까지 올려주셔서 감사합니다!!
음~ 저는 ControllerScanner 가 하는일에 집중해서 예외 클래스를 생성했어요..!!
ControllerScanner 가 던지는 예외는 컨트롤러를 제대로 스캔하지 못했을 때라고 생각했구 createController() 에서 발생한 예외가 발생하려면 매개변수로 넘겨진 controllerClass 가 없을 때라고 생각해서 NotFoundControllerException 이라고 네이밍했어요 ㅎ.ㅎ
조금 더 구체적으로 네이밍을 한다면 InvalidReflectionException 이라고 네이밍 할 수 있을 것 같은데, 수달 생각은 어떠신지요..ㅎ.ㅎ
There was a problem hiding this comment.
아하! 클래스에서 원하는 행위가 안되었을 때에 대해서 네이밍을 하신거라니
공감이 가네요. 그렇지만 왜 행위가 안됐는지 exception 이 발생한 구체적인 상황을 묘사하는 의미에 네이밍이라면 InvalidReflectionException 이 좋을 것 같네요 ! 설명 감사합니다 ㅎㅎ
| final HandlerAdapter handlerAdapter = handlerAdapters.getHandlerAdapter(handler); | ||
| handleRequest(request, response, handler, handlerAdapter); | ||
| } | ||
|
|
||
| private static void handleAnnotationHandler(final HttpServletRequest request, final HttpServletResponse response, | ||
| final HandlerExecution handler) { | ||
| private static void handleRequest(final HttpServletRequest request, final HttpServletResponse response, | ||
| final Object handler, final HandlerAdapter handlerAdapter) throws ServletException { | ||
| try { | ||
| ModelAndView modelAndView = handler.handle(request, response); | ||
| renderView(modelAndView, request, response); | ||
| final ModelAndView modelAndView = handlerAdapter.handle(request, response, handler); | ||
| modelAndView.renderView(request, response); | ||
| } catch (Exception e) { | ||
| log.error("Exception : {}", e.getMessage(), e); | ||
| throw new RuntimeException(e.getMessage()); | ||
| throw new ServletException(e.getMessage()); |
There was a problem hiding this comment.
HandlerExecutor 를 만들어볼까요 ?
현재 DispatcherServlet 이 하는 일이 많은 것 같아요 !
<하고 있는 일>
- handlerMappingRegistry.initialize();
- request 로 handler 찾기
- 찾은 handler 로 adapter 찾기
- adapter 로 작업 수행하기
- 수행한 결과 modelAndView 반환하기
- view render 해주기
DispatcherServlet 의 역할을 생각해본다면 요청을 확인하고, 해당 요청을 처리할 수 있는 객체에게
책임을 위함하는 frontController 라는 특성이 떠오르네요. ㅎㅎ
따라서 아래와 같은 부분은 새로운 객체를 만들어서 역할을 위임해봐도 좋을 것 같아요!
(지금도 충분히 너무 너무 잘하셨어요. )
- adapter 로 작업 수행하기
- 수행한 결과 modelAndView 반환하기
- view render 해주기
참고자료
There was a problem hiding this comment.
어,,현재 request 로 handler 찾기 는 handlerMappingRegistry 으로, 찾은 handler 로 adapter 찾기 는 handlerAdapters 로 위임해주고 있다고 생각해요..!! 마지막으로 수행한 결과 modelAndView 반환하기 는 Adapter가 수행해주고 있다고생각해요!!
그리고 위와 같이 생각하고 보면, 다음과 같은 일을 수행하고 있고 충분하다고 생각합니다!
- handlerMappingRegistry.initialize();
- adapter 로 작업 수행하기
- view render 해주기
혹시 제가 수달 코멘트의 의도를 파악하지 못한 것이라면...조금만 추가적으로 설명해주시면 감사할 것 같습니다...ㅠ.ㅠ
There was a problem hiding this comment.
엇 이부분은 미션 힌트에서 제공해준 구조라서 한번 체험해보면 좋겠다는 생각에서 제안드린 부분이고
디우 설명을 통해서 의도를 알게 되어서 이대로 유지해도 좋다고 생각해요.
| class HandlerMappingRegistryTest { | ||
|
|
||
| @DisplayName("HandlerMapping 인터페이스를 구현한 인스턴스를 추가하고 핸들러를 찾을 수 있다.") | ||
| @Test | ||
| void addHandlerMappingAndGetHandler() { | ||
| // given | ||
| final HandlerMappingRegistry handlerMappingRegistry = new HandlerMappingRegistry(); | ||
| final AnnotationHandlerMapping annotationHandlerMapping = new AnnotationHandlerMapping("samples"); | ||
| annotationHandlerMapping.initialize(); | ||
|
|
||
| // when | ||
| handlerMappingRegistry.addHandlerMapping(annotationHandlerMapping); |
There was a problem hiding this comment.
테스트 작성에 대해서
새로 생성한 모든 클래스에 대해서 테스트를 작성하신 부분 💯
비지니스 로직 만큼 테스트 코드 작성도 중요한 것 같아요. (바쁘면 제일 먼저 미루는 작업이기도 하지만 .. ㅎㅎ)
저는 개인적으로 방어로직(예외 케이스에 대해서 생각해보고 검증)에 대해서 많이 고민해봐야겠다고
이번 프로젝트를 하면서 느꼈어요.
디우가 작성해주신 테스트 코드의 대부분은 성공하는 로직에 대해서 였던것 같은데, 실패하는 케이스에 대해서도 고민해보고
검증한다면 정말 믿음직한 코드가 될 것 같네요.
There was a problem hiding this comment.
저도 팀플 중 QA 진행하면서 잡히는 버그들을 보며 최대한 꼼꼼하게 보이는 예외 케이스들을 테스트하는 코드가 필요하다고 느껴요..!!
하지만, 뭔가 모든 예외 케이스들을 잡긴 현실적으로 힘들다는 것도 함께 느끼게 되는 것 같아요...ㅠ.ㅠ
수달이 리뷰 주신 내용대로 최대한 단위 테스트에서 예외 케이스들을 검증하는 로직을 추가해보았어요~_~😃
아직 제가 생각하지 못한 예외 케이스들은 좀 더 생각 나거나 3단계 진행하면서 고려하며 추가해보도록 할게요!!😄
| } | ||
|
|
||
| public void addHandlerMapping(final HandlerMapping handlerMapping) { | ||
| handlerMappings.add(handlerMapping); | ||
| handlerMappingRegistry.addHandlerMapping(handlerMapping); | ||
| } |
안녕하세요 수달!!
중간에 데모데이가 껴있어서 1단계 미션 이후 좀 시간이 많이 흐른 후에 2단계 제출하게 되었네요..
데모데이는 잘 마무리하셨나요?? ㅎ.ㅎ
이전에 말씀 드린 것과 같이 이번 미션을 진행하면서
Adapter를 추가해주었습니다!!Adapter 를 추가하면서 기존의
DispatcherServlet이 가지고 있던handleManualHandler(),handleAnnotationHandler()와 같은 메소드를 제거할 수 있었습니다.뿐만 아니라
HandlerMappingRegistry를 추가하면서getHandler()와getAdapter()메소드를 제거하면서 "요청을 처리한다" 에만 집중할 수 있게 되었습니다!그리고 이번 미션이 점진적인 리팩터링인 만큼 한 번에 변경하기 보다는 가능을 하나 추가하고, 새롭게 추가된 것으로 대체하는 방식으로 조금씩 리팩터링을 진행해 보았습니다. 꽤나 재밌는 경험이었던 것 같습니다ㅎ.ㅎ (커밋기록으로 남겨두었습니다!!)
처음에
HandlerMappingRegistry 클래스에서 HandlerMapping을 처리하도록 구현했다.체크리스트만 보고는 어떻게 구현해야할지 잘 모르겠어서 힌트에 있는 클래스 다이어그램을 참고하였습니다.그리고 패키지 분리나 클래스 필드 및 메소드를 최대한 클래스 다이어그램과 일치시키려고 노력하였습니다. (기존 구현된 부분들도..)
구현 기능 목록(구현 내용)
1. ControllerScanner
@Controller어노테이션이 붙은 클래스를 찾고, 인스턴스를 생성해 함께 반환해주는 책임을 가지는ControllerScanner를 구현해주었습니다.이후 기존에
AnotationHandlerMapping에서 수행하던 위 작업을ControllerScanner를 이용해줄 수 있도록 리팩터링하는 과정을 진행해보았습니다.2. Adapter
어떤 형식의 핸들러를 지원하는지(supports) 파악가능하고 지원가능한 핸들러를 통해서 요청을 처리해주는 Adapter 를 어노테이션 기반 핸들러에 대해서(AnnotationhandlerMapping), 그리고 Controller 인터페이스를 구현한 핸들러에 대해서(ControllerHandlerMapping) 을 구현해주었습니다.
그리고
DispatcherServlet에서는 이를 이용하여 핸들러에 대한 적절한 어댑터를 찾고, 그 어댑터를 이용해 요청을 처리해줄 수 있도록 리팩터링하는 과정을 진행하였습니다.또한 기존에
DispatcherServlet에서 가지고 있던 뷰 랜더링에 대한 책임을ModelAndView쪽으로 위임해주었습니다.3. Registry
어댑터와 핸들러 매핑에 대해서 각각 Registry 클래스를 구현해주었습니다.
기존에
DispatcherServlet에서 요청에 대한 핸들러를 찾고, 그 핸들러에 맞는 핸들러 어댑터를 가져오는 과정과 핸들러 매핑 및 어댑터 추가에 대한 책임을 위 두 Registry 에 분리해주었습니다.HandlerMappingRegistry는 핸들러매핑을 추가하고 요청에 대해서 적절한 핸들러를 반환해주는 책임을 가지고,HandlerAdapterRegistry는 핸들러어댑터를 추가할 수 있고, 핸들러에 대해 적절한 핸들러 어댑터를 반환해주는 책임을 가지도록 구현하였습니다. 그리고DispatcherServlet에서는 이 둘을 이용하여 요청을 처리하도록 리팩터링을 진행하였습니다.체크 리스트
잘 부탁드립니다!!😃