[MVC 구현하기 - 3단계] 밀리(김미성) 미션 제출합니다. #630

miseongk · 2023-09-25T08:48:47Z

안녕하세요 케로!!

이번 단계에서는 View를 처리할 ViewResolver를 중점적으로 구현했습니다!
Legacy 코드를 최대한 변경하지 않기 위해 컨트롤러에서 String으로 반환하는 것과 ModelAndView를 반환하는 것 모두 호환되도록 구현했습니다. 제대로 했는지는 모르겠네용..

그럼 이번 단계도 잘 부탁드립니다 😊

jyeost

안녕하세요 밀리!!
역시 3단계도 잘해주셨군요...!!!

몇가지 리뷰 남겼으니 확인 후 리뷰 요청 바랍니다 ><

jyeost · 2023-09-26T05:44:56Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

-
-    public DispatcherServlet() {
-    }
+    private final List<ViewResolver> viewResolvers = new ArrayList<>();


이 필드들 일급 컬렉션으로 관리하면 DispatcherServlet에서 service 메서드에 집중할 수 있을 것 같아요 !

수정했습니다! 확실히 코드가 깔끔해졌네요 ㅎㅎ

jyeost · 2023-09-26T05:48:50Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

            final Map<String, Object> model = modelAndView.getModel();
            view.render(model, request, response);
+        } catch (final HandlerMappingException | ViewResolverException e) {
+            throw e;


다시 예외를 던지는 것 보다
응답에 404를 내려 보내주는 것은 어떤가요?!

JspView로 404.jsp를 내려주는 것을 말씀하시는 건가요?!
그렇다면 특정 앱의 jsp 파일명을 디스패쳐 서블릿에 명시해주는 것이라서 고민이 되더라구요..!
만약 그렇게 한다면 이 디스패쳐 서블릿을 사용하는 모든 어플리케이션이 강제로 404.jsp 파일을 만들어야하기 때문에 해당 예외가 터지면 처리는 개발자에게 맡기는게 좋을 것 같다고 생각해서 그냥 예외를 던졌어요!
이 부분은 어떻게 생각하시나요??

response에 뷰를 그려주는것이 아니라,
response.setStatus(CODE); 를 해주는 것을 의미했어요 !
지금은 커스텀 예외여도 500에러로 내려가니까용 !

jyeost · 2023-09-26T05:55:02Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/view/JsonView.java

+
+        final PrintWriter writer = response.getWriter();
+        if (model.size() == 1) {
+            writer.write(mapper.writeValueAsString(model.values().toArray()[0]));


model 을 이용해서 String으로 직렬화를 해주는 로직은
따로 메서드를 분리하는 것이 어떠신가용??

jyeost · 2023-09-26T06:50:34Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/tobe/HandlerExecution.java

@@ -16,6 +17,13 @@ public HandlerExecution(final Object handler, final Method method) {
    }

    public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response) throws Exception {
-        return (ModelAndView) method.invoke(handler, request, response);
+        final Object result = method.invoke(handler, request, response);


컨트롤러에서 다양한 return값을 허용하는 거 좋네요 !!!!
널처리도 꼼꼼 하구... =bbbb

jyeost · 2023-09-26T06:52:37Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/tobe/HandlerExecution.java

+        if (result == null) {
+            return new ModelAndView(new JsonView());
+        }
+        return new ModelAndView(result);


그래도 더 다양한 return값이 있을수 있으니 String일 때라는 걸 명시해주면 좋을 것 같아여ㅎㅅㅎ

근데 여기서 return된 String은 view 라는 가정하에는

new ModelAndView(new JspView((String)result));

이렇게 리턴시켜주면 뷰 리졸버가 없어도 될 것 같은데.....
제가 잘못 이해한 거라면 알려주십쇼...!!

만약 JspView가 아닌 새로운 형태의 View가 생길수도 있다고 생각해서 뷰 이름만 넣어주도록 했어요.
그리고 해당 뷰 생성의 책임은 뷰 리졸버에게 넘겼어요!

다양한 return 값이 있을 수 있기 때문에 String일 때라는 것을 명시해주고, 만약 제공되지 않는 형태의 값이 들어온다면 예외를 던져주어야 겠군요!

새로운 View가 생긴다면 ,이에 맞는 다른 View 클래스를 생성하면 되지 않을까여??
ModelAndView의 View필드가 Object이다 보니까 어떤 객체든지 들어올 수 있는 것이
추후에 예외를 초래할 수도 있을것 같다고 생각 했어요 ㅜㅜ

jyeost · 2023-09-26T06:55:20Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

    }

    private void initHandlerMappings() {
-        handlerMappings.add(new WrappedManualHandlerMapping());
-        handlerMappings.add(new AnnotationHandlerMapping(getClass().getPackageName()));
+        handlerMappings.add(new AnnotationHandlerMapping());


오... AnnotationHandlerMapping 생성할때 basePakage명을 명시해주지 않아도 되는군요...!!

DispatcherServletInitializer 클래스 기준으로 만들어 주는 걸까요???

넵 명시해주지 않아도 AnnotationHandlerMapping 내부의 초기화하는 로직에서 Reflections을 생성할 때 빈 패키지가 들어오면 해당 프로젝트의 클래스를 가져오는 것 같더라구요!

jyeost · 2023-09-26T07:04:27Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

@@ -59,9 +65,11 @@ protected void service(final HttpServletRequest request, final HttpServletRespon
            final Object handler = getHandler(request);
            final HandlerAdapter handlerAdapter = getHandlerAdapter(handler);
            final ModelAndView modelAndView = handlerAdapter.handle(handler, request, response);
-            final View view = modelAndView.getView();
+            final View view = getView(modelAndView);


HandlerExecution 클래스 리뷰에 남긴것처럼 수정 된다면,
view resolver도 없어지고 render도 ModelAndView 내부에서 해줄 수 있을까여...??

저는 뷰 리졸버가 있는게 훨씬 확장성이 좋다고 생각해서 그대로 뒀습니다..!!

miseongk

안녕하세요 케로!
리뷰 반영했습니다 ㅎㅎ
다시한번 확인 부탁드립니다!!

miseongk · 2023-09-26T08:06:57Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

-
-    public DispatcherServlet() {
-    }
+    private final List<ViewResolver> viewResolvers = new ArrayList<>();


수정했습니다! 확실히 코드가 깔끔해졌네요 ㅎㅎ

miseongk · 2023-09-26T08:09:28Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

            final Map<String, Object> model = modelAndView.getModel();
            view.render(model, request, response);
+        } catch (final HandlerMappingException | ViewResolverException e) {
+            throw e;


JspView로 404.jsp를 내려주는 것을 말씀하시는 건가요?!
그렇다면 특정 앱의 jsp 파일명을 디스패쳐 서블릿에 명시해주는 것이라서 고민이 되더라구요..!
만약 그렇게 한다면 이 디스패쳐 서블릿을 사용하는 모든 어플리케이션이 강제로 404.jsp 파일을 만들어야하기 때문에 해당 예외가 터지면 처리는 개발자에게 맡기는게 좋을 것 같다고 생각해서 그냥 예외를 던졌어요!
이 부분은 어떻게 생각하시나요??

miseongk · 2023-09-26T08:19:48Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/tobe/HandlerExecution.java

+        if (result == null) {
+            return new ModelAndView(new JsonView());
+        }
+        return new ModelAndView(result);


만약 JspView가 아닌 새로운 형태의 View가 생길수도 있다고 생각해서 뷰 이름만 넣어주도록 했어요.
그리고 해당 뷰 생성의 책임은 뷰 리졸버에게 넘겼어요!

다양한 return 값이 있을 수 있기 때문에 String일 때라는 것을 명시해주고, 만약 제공되지 않는 형태의 값이 들어온다면 예외를 던져주어야 겠군요!

miseongk · 2023-09-26T08:34:52Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

@@ -59,9 +65,11 @@ protected void service(final HttpServletRequest request, final HttpServletRespon
            final Object handler = getHandler(request);
            final HandlerAdapter handlerAdapter = getHandlerAdapter(handler);
            final ModelAndView modelAndView = handlerAdapter.handle(handler, request, response);
-            final View view = modelAndView.getView();
+            final View view = getView(modelAndView);


저는 뷰 리졸버가 있는게 훨씬 확장성이 좋다고 생각해서 그대로 뒀습니다..!!

miseongk · 2023-09-26T10:54:24Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/DispatcherServlet.java

    }

    private void initHandlerMappings() {
-        handlerMappings.add(new WrappedManualHandlerMapping());
-        handlerMappings.add(new AnnotationHandlerMapping(getClass().getPackageName()));
+        handlerMappings.add(new AnnotationHandlerMapping());


넵 명시해주지 않아도 AnnotationHandlerMapping 내부의 초기화하는 로직에서 Reflections을 생성할 때 빈 패키지가 들어오면 해당 프로젝트의 클래스를 가져오는 것 같더라구요!

jyeost · 2023-09-27T04:21:38Z

...c/main/java/webmvc/org/springframework/web/servlet/mvc/adapter/AnnotationHandlerAdapter.java


 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import webmvc.org.springframework.web.servlet.ModelAndView;
+import webmvc.org.springframework.web.servlet.mvc.handler.HandlerExecution;


패키지 분리 좋와용~~

jyeost

안녕하세요 밀리~~!!
이번 미션 너무 수고많으셨습니다!
같이 이야기하면서 저도 많이 배운것 같아서 재밌었습니다 !!
여기서 이만 머지 할게여 ><
혹시 다른 궁금한점이 있다면 디엠 주쎼용~~

miseongk added 7 commits September 25, 2023 01:09

refactor: 메서드명 변경

5780286

feat: JsonView 구현

31952ac

feat: ViewResolver 구현

839d97c

refactor: Legacy 코드 제거

e2ee628

refactor: test 제거

48b38c9

refactor: DispatcherServlet 패키지 이동

d2a8bbc

refactor: 핸들러 매핑의 getHandler가 Optional을 반환하도록 수정

3ca34d7

miseongk requested a review from jyeost September 25, 2023 08:48

miseongk self-assigned this Sep 25, 2023

miseongk added 2 commits September 25, 2023 21:38

refactor: 핸들러 매핑 실패 시 예외 처리 수정

e510279

refactor: 뷰 리졸버 처리 실패 시 예외 처리 수정

e0ca93f

jyeost requested changes Sep 26, 2023

View reviewed changes

miseongk added 4 commits September 26, 2023 16:39

refactor: 패키지 구조 변경

0c30f71

refactor: 일급컬렉션으로 개선

dca0adb

refactor: 핸들러의 반환타입이 String인 경우도 명시적으로 나타내도록 수정

a3fc8c2

refactor: 메서드 분리

206f941

miseongk commented Sep 26, 2023

View reviewed changes

jyeost reviewed Sep 27, 2023

View reviewed changes

jyeost approved these changes Sep 27, 2023

View reviewed changes

jyeost merged commit 6e9532c into woowacourse:miseongk Sep 27, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MVC 구현하기 - 3단계] 밀리(김미성) 미션 제출합니다. #630

[MVC 구현하기 - 3단계] 밀리(김미성) 미션 제출합니다. #630

miseongk commented Sep 25, 2023

jyeost left a comment

jyeost Sep 26, 2023

miseongk Sep 26, 2023

jyeost Sep 26, 2023

miseongk Sep 26, 2023

jyeost Sep 27, 2023

jyeost Sep 26, 2023

jyeost Sep 26, 2023

jyeost Sep 26, 2023

miseongk Sep 26, 2023

jyeost Sep 27, 2023

jyeost Sep 26, 2023

miseongk Sep 26, 2023

jyeost Sep 26, 2023

miseongk Sep 26, 2023

miseongk left a comment

miseongk Sep 26, 2023

miseongk Sep 26, 2023

miseongk Sep 26, 2023

miseongk Sep 26, 2023

miseongk Sep 26, 2023

jyeost Sep 27, 2023

jyeost left a comment

[MVC 구현하기 - 3단계] 밀리(김미성) 미션 제출합니다. #630

[MVC 구현하기 - 3단계] 밀리(김미성) 미션 제출합니다. #630

Conversation

miseongk commented Sep 25, 2023

jyeost 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

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

miseongk 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

jyeost left a comment

Choose a reason for hiding this comment