[@MVC 구현하기 - step 3] 필립(양재필) 미션 제출합니다. #565

pilyang · 2023-09-24T10:07:57Z

안녕하세요 엔초!!

3단계도 잘 부탁 드립니다!

지난 단계에서 질문주셨던 내용들은 지난 pr에 코멘트로 남겨두었습니다.

그냥 하다보니 작동이 되게만 어케 만들게 된 것 같습니다...

놓친 부분이나 이상한 부분들, 궁금한 부분들 편하게 지적 부탁드립니다.

이번에도 잘 부탁 드립니다!!!

DispatcerServlet -> ModelAndView

kwonyj1022

안녕하세요 필립!
깔끔하게 잘 구현해 주셨네요!!
궁금한 부분과 사소한 제안 사항이 있어서 리뷰 남겼습니다.
수고하셨어요~

kwonyj1022 · 2023-09-24T14:55:16Z

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

+    public void render(final HttpServletRequest request, HttpServletResponse response) throws Exception {
+        view.render(model, request, response);
+    }
+


DispatcherServlet에 있는 render()를 ModelAndView로 이동시켜 주었군요! 👍

kwonyj1022 · 2023-09-24T15:00:31Z

app/src/main/java/com/techcourse/DispatcherServletInitializer.java


    @Override
    public void onStartup(final ServletContext servletContext) {
        final var dispatcherServlet = new DispatcherServlet();
-        dispatcherServlet.addHandlerMapping(new ManualHandlerMapping());
-        dispatcherServlet.addHandlerMapping(new AnnotationHandlerMapping(PACKAGE_TO_SEARCH_CONTROLLER));
+        dispatcherServlet.addHandlerMapping(new AnnotationHandlerMapping(getClass().getPackageName()));


패키지를 좀 더 유연하게 작성하셨군요! 저는 Application.java 파일이 위치한 패키지를 기준으로 했는데 이런 방법도 있었네요

kwonyj1022 · 2023-09-24T15:03:27Z

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

+    static {
+        objectMapper = new ObjectMapper();
+        objectMapper.setVisibility(objectMapper.getSerializationConfig()
+                .getDefaultVisibilityChecker()
+                .withFieldVisibility(JsonAutoDetect.Visibility.ANY)
+                .withGetterVisibility(JsonAutoDetect.Visibility.NONE)
+                .withSetterVisibility(JsonAutoDetect.Visibility.NONE)
+                .withCreatorVisibility(JsonAutoDetect.Visibility.NONE)
+                .withIsGetterVisibility(JsonAutoDetect.Visibility.NONE));
+    }


이 부분은 어떤 설정인지 궁금합니다!

기본적으로 object mapper가 public getter, public field를 기반으로 json을 생성해주는데, 모든 필드에 대해서 생성을 해주도록 하기위해서 설정했습니다!

해당 부분은 withFieldVisibility(JsonAutoDetect.Visibility.ANY) 부분까지이고, 나머지 부분은 작성 당시에는 큰 생각 없이 나머지는 그럼 none으로 해야지 하고 작성하긴 했습니다...

아직 나머지부분에 대한 공부나 차이에 대해서는 공부를 하지 못하였는데, 간단히 확인해보니 위에 withFieldVisibility만 있어도 되어서 나머지 부분은 지웠습니다. 시간이 없어서 나머지에 대해서 공부는 못하였습니다ㅠㅜ

객체를 직렬화 할 때 getter를 사용하는것과, 필드들을 보일 수 있도록 설정해주는것 사이에서 고민을 하다가 이렇게 한 이유는 json으로 변환하기 위해서만 도메인코드에 getter가 들어가는것이 좋지 못하다고 생각을 해서입니다..

하지만 다시생각해보니 getter를 통해서 보일정보와 숨길정보를 명시할 수 있다는 점이 장점일 수 있나라는 생각도 들었습니다.
하지만 또 다시생각해보면 만약 숨겨야하는 정보가 있다면 도메인을 그대로 넘기는게 아니라 ResponseDto를 만들어서 보일 정보만 jsonView에 전달해줘야하는것이 아닌가라는 생각도 듭니다..

혹시 엔초는 public getter기반으로 직렬화하는것과, 모든 필드를 기반으로 직렬화하는것에 대한 어떤것이 좋다고 생각되는지 궁금합니다!!

흠 정말 어려운 고민이네요. 저는 public getter 기반으로 직렬화하는 것이 좋을 것 같습니다! 요즘 프로젝트에서 롬복을 사용하다 보니 애플리케이션 단에서 사용하지 않는 getter에 대한 거부감이 줄어들어서 그런 것일 수도 있어요 ㅎㅎ public getter 기반으로 직렬화하도록 하고 ResponseDto를 사용할 것 같습니다.

kwonyj1022 · 2023-09-24T15:09:13Z

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

 public class DispatcherServlet extends HttpServlet {



어디에 작성할지 모르겠어서 여기에 작성합니다!
현재 컨트롤러가 없는 요청이 올 경우(예를 들어 localhost:8080/abcdefg) HTTP Status 500 – Internal Server Error 화면을 보여주는데요, 의도하신 걸까요?
제 생각에는 404 페이지를 보여주는 것이 좀 더 적절할 것 같은데 필립의 의견이 궁금합니다!

의도한건 아니고, 그냥 하다보니 404에 대한 고민을 안하게 된 것 같습니다.

저또한 404페이지를 보여주는것이 좋다고 생각을 해서 404페이지를 보여줄 수 있도록 기능추가했습니다.

구현 방식에 대해서 고민하다가 HandlerExecution을 상속받는 ForwardingExecution을 구현하고, DispatcherServletIntializer에서 404페이지 ModelAndView를 주입하여서 없는 url에 한해서는 404를 보여줄 수 있도록 구현하였습니다.

처음 구현을 하였을때는 AnnotationHandlerMapping에 해당 404 ModelAndView를 주입하는 방식으로 구현을 하였는데, 이렇게 되면 지금은 제거된 legacy ManualHandlerMapping이 아직 남아있고, 해당 핸들러맵핑에서 핸들러 맵핑이 되더라도 AnnotationHandlerMapping에 의해 404페이지가 뜰 수 있을 것 같다는 생각이 들었습니다.
마찬가지로 지금의 AnnotationHandlerMapping이 레거시가 되고 다른 핸들러맵핑이 추가되어도 비슷한 상황이 발생할 수 있을 것 같았습니다.

그래서 SimpleForwardingHandlerMapping을 추가로 구현하여 HanlderMappings에서 맵핑되는 핸들러를 찾지 못하였을때 404페이지를 보이는 핸들러를 내보낼 수 있도록 변경하였습니다.

위처럼 404페이지가 보이는 부분을 DispatcherServlet에서 다 구현하기보다는 404페이지 정보를 DispatcherServletIntializer에서 받도록 한 이유는 이 부분은 mvc보다는 application에서 지정해줘야하는 역할이라 생각해서입니다. 애당초 404.jsp도 mvc모듈이 아닌 app모듈에 있어서 app모듈에서 404를 주입해줄 수 있는 방식으로 구현했습니다.

필립의 의견을 듣고 생각을 해봤을 때 저는 DispatcherServlet과 함께 DispatcherServletInitializer, TomcatStarter 모두 mvc 모듈로 옮겨야 하지 않나 하는 생각이 들었습니다. (하지만 컴파일이 안되더군요..) DispatcherServletInitializer, TomcatStarter도 mvc 모듈에 있어야 한다는 제 생각에 대한 필립의 의견이 궁금합니다!
또, 만약 mvc 모듈로 옮기는 게 맞다면, 404.jsp는 mvc 모듈에 있는데 DispatcherServletInitializer는 mvc 모듈에 위치하게 됩니다. 그렇다면 mvc 모듈에 있는 DispatcherServletInitializer에 404.jsp를 직접 입력하지 않고 application에서 주입해 주려면 어떻게 할 수 있을까요?
(저도 잘 몰라서 질문드리는 거라 편하게 말씀해 주시면 감사하겠습니다!)

kwonyj1022 · 2023-09-24T15:10:24Z

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

+    private static void writeResponseBody(final HttpServletResponse response, final String body) throws IOException {
+        final PrintWriter out = response.getWriter();
+        out.write(body);
+        out.flush();
    }


이 부분은 static 메서드일 필요가 없을 것 같습니다!

인텔리제이에서 메소드 자동추출하다보니 static이 들어갔네요..!

인텔리제이에서는 메소드 자동추출을 하면 객체의 상태에 영향이 없는 경우에는 자동으로 static을 붙여주는 것 같습니다.

저도 평소에는 이 static들을 제거해주는걸 선호하기는 합니다. static이 붙어있으면 객체가 생성이 되지 않아도 사용이 가능하고, oop적으로 안맞다고 생각을 해서이긴 합니다.

하지만 만약 이런 static 메소드가 이것처럼 private이면 사용될일이 없고, 초기 메모리 static영역에 단 한번만 올라가고 객체생성시마다 heap영역에 올라가지 않아 성능상의 이점이 있어서 좋다는 의견도 있는것같습니다.
반대로 static영역에 올라가기에 프로그램 시작과 끝까지 계속 남아있기에 성능상 안좋다고 하는것같기도 합니다..ㅠㅜ

이부분은 전 static을 안붙이는것을 선호했는데 (이번엔 실수로 못지운겁니다ㅠㅜㅜ) 혹시 엔초는 이런 의견들에대해서 어떻게 생각하는지 궁금합니다..!!!

private static이면 확실히 외부에서 사용될 여지가 없겠군요. 애플리케이션 실행 시에 한 번만 올라간다면 성능상 이점도 분명히 있을 것 같아요. 그렇지만 그 의도를 모르는 다른 개발자를 위해서라도 저는 쓰지 않을 것 같습니다. static이라는 키워드가 붙어있으면 뭔가 유틸리티 같은 느낌이 들거나 static 메서드에서 호출하는 메서드라는 생각이 들 것 같아서요! 뭔가 명확하지는 않지만 static을 안 붙이는 것을 선호하다 보니 그렇게 느껴지는 것 같습니다 ㅠㅠ

kwonyj1022 · 2023-09-24T15:19:41Z

app/src/main/java/com/techcourse/controller/UserController.java

+    @RequestMapping(value = "/api/user", method = RequestMethod.GET)
+    public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {


이 부분도 다른 컨트롤러에서처럼 파라미터에 final 붙여주면 통일성 있고 좋을 것 같습니다! 추가적으로 여기 file changes에는 없지만 RegisterController의 메서드에도 적용해 주시면 좋을 것 같아요~

헉.. 기존코드 재사용하고, lms코드 복붙한부분들에서 놓쳤네요...
감사합니다!!

- 지정된 ModelAndView를 표시하는 ForwardingExcution 생성 - AnnotationHandlerMapping에 notfound용 모델앤뷰 내용 추가 - DispatcherServletInitializer 에서 404 페이지 추가

pilyang

안녕하세요 엔초!!

남겨주신 코멘트들 덕분에 놓치고 있던 부분들 생각해볼 수 있었습니다!!

코멘트 내용들 반영하고 반영한 내용도 같이 남겨두었고, 궁금한 내용들도 같이 남겨두었습니다.

pilyang · 2023-09-25T13:00:55Z

app/src/main/java/com/techcourse/controller/UserController.java

+    @RequestMapping(value = "/api/user", method = RequestMethod.GET)
+    public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {


헉.. 기존코드 재사용하고, lms코드 복붙한부분들에서 놓쳤네요...
감사합니다!!

pilyang · 2023-09-25T13:09:27Z

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

+    private static void writeResponseBody(final HttpServletResponse response, final String body) throws IOException {
+        final PrintWriter out = response.getWriter();
+        out.write(body);
+        out.flush();
    }


인텔리제이에서 메소드 자동추출하다보니 static이 들어갔네요..!

인텔리제이에서는 메소드 자동추출을 하면 객체의 상태에 영향이 없는 경우에는 자동으로 static을 붙여주는 것 같습니다.

저도 평소에는 이 static들을 제거해주는걸 선호하기는 합니다. static이 붙어있으면 객체가 생성이 되지 않아도 사용이 가능하고, oop적으로 안맞다고 생각을 해서이긴 합니다.

하지만 만약 이런 static 메소드가 이것처럼 private이면 사용될일이 없고, 초기 메모리 static영역에 단 한번만 올라가고 객체생성시마다 heap영역에 올라가지 않아 성능상의 이점이 있어서 좋다는 의견도 있는것같습니다.
반대로 static영역에 올라가기에 프로그램 시작과 끝까지 계속 남아있기에 성능상 안좋다고 하는것같기도 합니다..ㅠㅜ

이부분은 전 static을 안붙이는것을 선호했는데 (이번엔 실수로 못지운겁니다ㅠㅜㅜ) 혹시 엔초는 이런 의견들에대해서 어떻게 생각하는지 궁금합니다..!!!

pilyang · 2023-09-25T13:22:37Z

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

+    static {
+        objectMapper = new ObjectMapper();
+        objectMapper.setVisibility(objectMapper.getSerializationConfig()
+                .getDefaultVisibilityChecker()
+                .withFieldVisibility(JsonAutoDetect.Visibility.ANY)
+                .withGetterVisibility(JsonAutoDetect.Visibility.NONE)
+                .withSetterVisibility(JsonAutoDetect.Visibility.NONE)
+                .withCreatorVisibility(JsonAutoDetect.Visibility.NONE)
+                .withIsGetterVisibility(JsonAutoDetect.Visibility.NONE));
+    }


기본적으로 object mapper가 public getter, public field를 기반으로 json을 생성해주는데, 모든 필드에 대해서 생성을 해주도록 하기위해서 설정했습니다!

해당 부분은 withFieldVisibility(JsonAutoDetect.Visibility.ANY) 부분까지이고, 나머지 부분은 작성 당시에는 큰 생각 없이 나머지는 그럼 none으로 해야지 하고 작성하긴 했습니다...

아직 나머지부분에 대한 공부나 차이에 대해서는 공부를 하지 못하였는데, 간단히 확인해보니 위에 withFieldVisibility만 있어도 되어서 나머지 부분은 지웠습니다. 시간이 없어서 나머지에 대해서 공부는 못하였습니다ㅠㅜ

객체를 직렬화 할 때 getter를 사용하는것과, 필드들을 보일 수 있도록 설정해주는것 사이에서 고민을 하다가 이렇게 한 이유는 json으로 변환하기 위해서만 도메인코드에 getter가 들어가는것이 좋지 못하다고 생각을 해서입니다..

하지만 다시생각해보니 getter를 통해서 보일정보와 숨길정보를 명시할 수 있다는 점이 장점일 수 있나라는 생각도 들었습니다.
하지만 또 다시생각해보면 만약 숨겨야하는 정보가 있다면 도메인을 그대로 넘기는게 아니라 ResponseDto를 만들어서 보일 정보만 jsonView에 전달해줘야하는것이 아닌가라는 생각도 듭니다..

혹시 엔초는 public getter기반으로 직렬화하는것과, 모든 필드를 기반으로 직렬화하는것에 대한 어떤것이 좋다고 생각되는지 궁금합니다!!

pilyang · 2023-09-25T13:51:25Z

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

 public class DispatcherServlet extends HttpServlet {



의도한건 아니고, 그냥 하다보니 404에 대한 고민을 안하게 된 것 같습니다.

저또한 404페이지를 보여주는것이 좋다고 생각을 해서 404페이지를 보여줄 수 있도록 기능추가했습니다.

구현 방식에 대해서 고민하다가 HandlerExecution을 상속받는 ForwardingExecution을 구현하고, DispatcherServletIntializer에서 404페이지 ModelAndView를 주입하여서 없는 url에 한해서는 404를 보여줄 수 있도록 구현하였습니다.

처음 구현을 하였을때는 AnnotationHandlerMapping에 해당 404 ModelAndView를 주입하는 방식으로 구현을 하였는데, 이렇게 되면 지금은 제거된 legacy ManualHandlerMapping이 아직 남아있고, 해당 핸들러맵핑에서 핸들러 맵핑이 되더라도 AnnotationHandlerMapping에 의해 404페이지가 뜰 수 있을 것 같다는 생각이 들었습니다.
마찬가지로 지금의 AnnotationHandlerMapping이 레거시가 되고 다른 핸들러맵핑이 추가되어도 비슷한 상황이 발생할 수 있을 것 같았습니다.

그래서 SimpleForwardingHandlerMapping을 추가로 구현하여 HanlderMappings에서 맵핑되는 핸들러를 찾지 못하였을때 404페이지를 보이는 핸들러를 내보낼 수 있도록 변경하였습니다.

위처럼 404페이지가 보이는 부분을 DispatcherServlet에서 다 구현하기보다는 404페이지 정보를 DispatcherServletIntializer에서 받도록 한 이유는 이 부분은 mvc보다는 application에서 지정해줘야하는 역할이라 생각해서입니다. 애당초 404.jsp도 mvc모듈이 아닌 app모듈에 있어서 app모듈에서 404를 주입해줄 수 있는 방식으로 구현했습니다.

kwonyj1022

안녕하세요 필립! 리뷰 답변 잘 봤습니다. 생각할 거리도 같이 공유해 주셔서 도움이 많이 됐어요 👍
수정 잘해주셔서 머지하도록 하겠습니다~
필립이 답변 달아주신 부분에 개인적인 궁금증을 하나 남겨놓았는데 시간 되실 때 확인하고 답변 달아주면 감사하겠습니다.
다음 미션도 화이팅하세요~!

pilyang added 8 commits September 24, 2023 18:28

refactor: render메서드 위치 이동

ecb1612

DispatcerServlet -> ModelAndView

feat: JsonView render 구현

7b0e7fe

feat: UserController 추가

11ab96c

refactor: Login Conotroller 어노테이션 기반으로 변경

0a36e25

refactor: logout 컨트롤러 어노테이션 기반으로 변경

c0b4197

refactor: 포워딩 컨트롤러 어노테이션 기반으로 변경

cced0e7

refactor: legacy 컨트롤러 제거

72d054a

refactor: DispatcherSErvlet 위치 변경

54c25c6

pilyang changed the title ~~[@MVC 만들기 - step 3] 필립(양재필) 미션 제출합니다.~~ [@MVC 구현하기 - step 3] 필립(양재필) 미션 제출합니다. Sep 24, 2023

kwonyj1022 requested changes Sep 24, 2023

View reviewed changes

pilyang added 6 commits September 25, 2023 21:20

refactor: 누락된 final추가

725f326

refactor: static 제거

749cce8

refactor: 필요한 설정 외 제거

9386b7b

feat: 핸들러를 찾을 수 없을때 404패이지 응답 구현

5291dc2

- 지정된 ModelAndView를 표시하는 ForwardingExcution 생성 - AnnotationHandlerMapping에 notfound용 모델앤뷰 내용 추가 - DispatcherServletInitializer 에서 404 페이지 추가

refactor: 누락된 final추가

3a73658

fix: 여러개의 HandlerMapping추가시 NotFound가 맵핑될 수 있는 오류 가능성 제거

6d5ecd2

pilyang commented Sep 25, 2023

View reviewed changes

kwonyj1022 approved these changes Sep 26, 2023

View reviewed changes

kwonyj1022 merged commit 760f056 into woowacourse:pilyang Sep 26, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[@MVC 구현하기 - step 3] 필립(양재필) 미션 제출합니다. #565

[@MVC 구현하기 - step 3] 필립(양재필) 미션 제출합니다. #565

pilyang commented Sep 24, 2023

kwonyj1022 left a comment

kwonyj1022 Sep 24, 2023

kwonyj1022 Sep 24, 2023

kwonyj1022 Sep 24, 2023

pilyang Sep 25, 2023

kwonyj1022 Sep 25, 2023

kwonyj1022 Sep 24, 2023

pilyang Sep 25, 2023

kwonyj1022 Sep 25, 2023

kwonyj1022 Sep 24, 2023

pilyang Sep 25, 2023

kwonyj1022 Sep 26, 2023

kwonyj1022 Sep 24, 2023

pilyang Sep 25, 2023

pilyang left a comment

pilyang Sep 25, 2023

pilyang Sep 25, 2023

pilyang Sep 25, 2023

pilyang Sep 25, 2023

kwonyj1022 left a comment

		@RequestMapping(value = "/api/user", method = RequestMethod.GET)
		public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {

[@MVC 구현하기 - step 3] 필립(양재필) 미션 제출합니다. #565

[@MVC 구현하기 - step 3] 필립(양재필) 미션 제출합니다. #565

Conversation

pilyang commented Sep 24, 2023

kwonyj1022 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

pilyang 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

kwonyj1022 left a comment

Choose a reason for hiding this comment