[MVC 구현하기 - 1단계] 후디(조동현) 미션 제출합니다. #163

devHudi · 2022-09-19T17:10:01Z

안녕하세요, 찬 🙌 후디입니다!
쉽지 않은 미션이었어요. 특히 처음에 실수로 Reflections 클래스를 사용하지 않고 구현하다가 후반부에 많이 갈아 엎었습니다 😓
HandlerAdapter 를 사용하는 것은 2단계 요구사항이므로 지금은 일반 Controller와 어노테이션 기반의 컨트롤러를 타입체크로 구분하였습니다. 이 부분은 2단계에서 개선해볼게요.
또, 테스트코드를 하나도 작성하지 못했는데요 ㅠㅠ 2단계 진행하면서 작성하지 못한 테스트 코드 열심히 채워보겠습니다. 자세한 내용은 코멘트로 달아둘게요!

…od 구현

- JspView 구현

devHudi · 2022-09-19T17:11:31Z

mvc/src/main/java/nextstep/mvc/DispatcherServlet.java

+            // TODO: 다형성을 통해 해결해야함
+            if (controller instanceof HandlerExecution) {
+                HandlerExecution handlerExecution = (HandlerExecution) controller;
+                ModelAndView modelAndView = handlerExecution.handle(request, response);
+                Map<String, Object> model = modelAndView.getModel();
+                modelAndView.getView().render(model, request, response);
+                return;
+            }
+
+            final var viewName = ((Controller) controller).execute(request, response);


나중에 HandlerAdapter가 구현이되면, 자연스럽게 해결이 되지 않을까 하는 부분입니다. 사실 아직 HandlerAdapter 의 역할을 잘 알고있지는 않아서 아닐수도 있어요... 찬의 의견이 궁금합니다!

나중에 각 HandlerMapping을 처리할 수 있는 HandlerAdapter를 구현하면 자연스럽게 해결될 것 같아요~!

devHudi · 2022-09-19T17:14:01Z

mvc/src/main/java/nextstep/mvc/controller/tobe/AnnotationHandlerMapping.java

+
+    private void addHandlerExecutions(final Method method) {
+        RequestMapping requestMapping = method.getAnnotation(RequestMapping.class);
+        List<HandlerKey> handlerKeys = HandlerKey.from(requestMapping);


HandlerKey의 from 에 requestMapping 어노테이션 객체(?)를 전달합니다. 내부에서 value() (URL)와 method() (RequestMethod[]) 를 requestMapping 에서 꺼내와서 List<HandlerKey> 를 반환합니다. 리스트를 반환하는 이유는 method() 의 반환값이 RequestMethod 의 배열이기 때문입니다. URL 하나에 여러 HTTP Method를 매핑하는 것이 의도된 것 같아서 이렇게 정적 팩토리 메소드로 만들었어요.

devHudi · 2022-09-19T17:15:09Z

mvc/src/main/java/nextstep/mvc/controller/tobe/HandlerExecution.java

+    public HandlerExecution(final Method method) {
+        this.controller = method.getDeclaringClass();
+        this.method = method;
+    }


Method 만 넣어주면, 알아서 컨트롤러의 Class 도 가져옵니다.

이건 저도 변경해야 할 부분인데 나중에 controller에 필드들이 추가가 된다면 지금과 같은 방법을 사용하면 대응을 할 수 없을 것 같아요! 추후 DI를 통해 생성된 Object를 필드로 갖고 있는 것이 좋을 것 같은데 후디의 생각은 어떠신가요?!

devHudi · 2022-09-19T17:16:35Z

mvc/src/main/java/nextstep/mvc/controller/tobe/HandlerExecution.java

    public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response) throws Exception {
-        return null;
+        Constructor<?> constructor = controller.getConstructor();
+        Object instance = constructor.newInstance();
+
+        return (ModelAndView) method.invoke(instance, request, response);


handle() 이 호출될 때 컨트롤러의 Class 를 사용해서 컨트롤러 인스턴스를 생성합니다.
HandlerExecution 이 초기화 되는 시점에서 인스턴스를 필드에 넣어주지 않은 이유는 handle 에서 throws 를 사용해 외부로 예외 처리를 미루는게 코드가 깔끔해지기 때문입니다.

kimchan123

안녕하세요 후디! 블로그 포스팅 정말 잘 보고 있어요! 👍 리뷰하게 되어 영광입니다..!
전체적으로 코드를 깔끔하게 작성해주셔서 이해하기 정말 쉬웠어요 ㅎㅎ
궁금한 것 몇가지 리뷰로 남겼어요. 이해가 잘 안되거나 다른 의견있으시면 언제든 댓글이나 DM 주세요!
고생하셨습니다 ㅎㅎ

kimchan123 · 2022-09-20T01:55:16Z

mvc/src/main/java/nextstep/mvc/DispatcherServlet.java

+            // TODO: 다형성을 통해 해결해야함
+            if (controller instanceof HandlerExecution) {
+                HandlerExecution handlerExecution = (HandlerExecution) controller;
+                ModelAndView modelAndView = handlerExecution.handle(request, response);
+                Map<String, Object> model = modelAndView.getModel();
+                modelAndView.getView().render(model, request, response);
+                return;
+            }
+
+            final var viewName = ((Controller) controller).execute(request, response);


나중에 각 HandlerMapping을 처리할 수 있는 HandlerAdapter를 구현하면 자연스럽게 해결될 것 같아요~!

kimchan123 · 2022-09-20T01:56:44Z

mvc/src/main/java/nextstep/mvc/DispatcherServlet.java

                .findFirst()
                .orElseThrow();
    }

-    private void move(final String viewName, final HttpServletRequest request, final HttpServletResponse response) throws Exception {
+    private void move(final String viewName, final HttpServletRequest request, final HttpServletResponse response)


나중에 이부분도 DispatcherServlet이 아닌 View 쪽으로 로직을 넘기면 좋을 것 같아요!

HandlerAdapter 를 구현하며 자연스럽게 ModelAndView가 처리하도록 분리하였습니다!

kimchan123 · 2022-09-20T03:21:30Z

mvc/src/main/java/nextstep/mvc/controller/tobe/HandlerKey.java

+        String url = request.getRequestURI();
+        RequestMethod requestMethod = RequestMethod.valueOf(request.getMethod());
+
+        this.url = url;
+        this.requestMethod = requestMethod;


이 부분 inline 처리하면 어떨까요?

this.url = request.getRequestURI(); this.requestMethod = RequestMethod.valueOf(request.getMethod());

이렇게요!

엇 ㅋㅋㅋ 제가 왜 이렇게 했을까요! 수정하겠습니다 ㅎㅎ

kimchan123 · 2022-09-20T03:34:30Z

mvc/src/main/java/nextstep/mvc/controller/tobe/AnnotationHandlerMapping.java

+        if (handlerExecution == null) {
+            throw new ControllerNotFoundException();
+        }


request가 들어오면 DispatcherServlet에 있는 모든 HandlerMapping을 돌며 handle이 가능한지 여부를 확인할텐데 이렇게 특정 HandlerMapping에서 NotFoundException을 터트려버리면 다른 HandlerMapping에서 해당 request를 처리할 수 있는 여지를 남길 수 없을 것 같아요. 모든 HandlerMapping을 전부 순회했는데도 불구하고 찾지 못한다면 그 때 ControllerNotFoundException을 터트리는 것이 어떨까요?

kimchan123 · 2022-09-20T03:42:45Z

mvc/src/main/java/nextstep/mvc/view/JspView.java

+        final var requestDispatcher = request.getRequestDispatcher(viewName);
+        requestDispatcher.forward(request, response);


model에 저장되어 있는 값을 request에 setting 한 후에 forward가 일어나야 할 것 같아요!

kimchan123 · 2022-09-20T03:49:37Z

mvc/src/main/java/nextstep/mvc/controller/tobe/HandlerExecution.java

+    public HandlerExecution(final Method method) {
+        this.controller = method.getDeclaringClass();
+        this.method = method;
+    }


이건 저도 변경해야 할 부분인데 나중에 controller에 필드들이 추가가 된다면 지금과 같은 방법을 사용하면 대응을 할 수 없을 것 같아요! 추후 DI를 통해 생성된 Object를 필드로 갖고 있는 것이 좋을 것 같은데 후디의 생각은 어떠신가요?!

kimchan123 · 2022-09-20T03:50:58Z

mvc/src/main/java/nextstep/mvc/controller/tobe/HandlerKey.java

+        List<HandlerKey> handlerKeys = new ArrayList<>();
+        for (RequestMethod requestMethod : requestMethods) {
+            HandlerKey handlerKey = new HandlerKey(url, requestMethod);
+            handlerKeys.add(handlerKey);
+        }
+
+        return handlerKeys;


사소하지만 이부분을 메서드로 추출하면 더 깔끔할 것 같아요 ㅎㅎ!

kimchan123 · 2022-09-22T16:13:58Z

후디 첫 PR 머지가 오늘까지라 머지하겠습니다! 2단계 화이팅이에요 ㅎㅎ

devHudi added 8 commits September 18, 2022 17:15

feat: 어노테이션 기반으로 컨트롤러를 사용할 수 있도록 개선

9afe4b6

refactor: 패키지에 대한 기능을 PackageUtil로 이동

b4b6372

refactor: for depth 얕게 개선

7fd6fe7

refactor: 반복 구조 개선

e31cbb2

refactor: Class와 Method를 한단계 추상화하는 객체 ControllerClass, ControllerMeth…

17b7677

…od 구현

feat: 커스텀 예외 객체 생성

1f245b9

refactor: HandlerKey의 생성자 추가

ed7c9ef

refactor: Reflections를 사용하도록 개선

921359d

- JspView 구현

devHudi requested a review from kimchan123 September 19, 2022 17:10

devHudi self-assigned this Sep 19, 2022

devHudi commented Sep 19, 2022

View reviewed changes

kimchan123 suggested changes Sep 20, 2022

View reviewed changes

kimchan123 merged commit b04bd9d into woowacourse:devhudi Sep 22, 2022

devHudi mentioned this pull request Sep 26, 2022

[MVC 구현하기 - 2단계] 후디(조동현) 미션 제출합니다. #256

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MVC 구현하기 - 1단계] 후디(조동현) 미션 제출합니다. #163

[MVC 구현하기 - 1단계] 후디(조동현) 미션 제출합니다. #163

devHudi commented Sep 19, 2022

devHudi Sep 19, 2022

kimchan123 Sep 20, 2022

devHudi Sep 19, 2022

devHudi Sep 19, 2022

kimchan123 Sep 20, 2022

devHudi Sep 19, 2022

kimchan123 left a comment

kimchan123 Sep 20, 2022

kimchan123 Sep 20, 2022

devHudi Sep 26, 2022

kimchan123 Sep 20, 2022

devHudi Sep 21, 2022

kimchan123 Sep 20, 2022

kimchan123 Sep 20, 2022

kimchan123 Sep 20, 2022

kimchan123 Sep 20, 2022

kimchan123 commented Sep 22, 2022

		final var requestDispatcher = request.getRequestDispatcher(viewName);
		requestDispatcher.forward(request, response);

[MVC 구현하기 - 1단계] 후디(조동현) 미션 제출합니다. #163

[MVC 구현하기 - 1단계] 후디(조동현) 미션 제출합니다. #163

Conversation

devHudi commented Sep 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimchan123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimchan123 commented Sep 22, 2022