[ MVC 구현하기 - 2 단계 ] 필립(양재필) 미션 제출합니다. #453

pilyang · 2023-09-17T07:18:33Z

안녕하세요 엔초!! 😄

2단계 미션 제출합니다!!

2단계 진행 내용에 대한 이야기 전에, 1단계에서 코멘트 주신 부분들에 대한 이야기입니다!

request method enum에서 valueOf 사용 에 대한 질문을 듣고 어,, 그러네 하면서 한번 rfc 문서를 찾아보았습니다.
찾아보니 rfc 9110에서 아래와 같은 문구가 있음을 확인할 수 있었습니다.

The method token is case-sensitive because it might be used as a gateway to object-based systems with case-sensitive method names. By convention, standardized methods are defined in all-uppercase US-ASCII letters.

확인할 수 있듯이 표준이 all-uppercase라고 하여서, RequestMethod enum에서는 소문자를 대문자로 변경해주는 처리는 안해도 된다고 판단하였습니다. 그래도 혹시나 클라이언트에서 표준을 안지키고 소문자로 보내는것을 처리해주고싶다 하면 RequestMethod 내부에서는 표준을 지키고, 필요한 경우 외부에서 request method string을 .toUpperCase()로 대문자로 치환해준 후 그대로 valueOf를 사용하게 구현하게 될 것 같습니다. (이번 코드에서는 적용하지는 않았습니다...)

두번째 코멘트에서 뎁스 관련도 이번엔 완성된 코드에서 최대한 코드뎁스나 가독성적인 측면을 지켜보려 노력했습니다!!

2단계 진행은

일단 인터페이스와 어노테이션 기반으로 동작은 하게 만들기
HandlerMapping을 추상화해서 DispatcherServlet에서 각 handlerMapping 의 종류를 모르게 하기
HandlerAdapter를 적용하여 Handler에서 handle 한 결과가 ModelAndView로 통일시키기

순으로 진행을 했고, 리펙터링 한 흐름에 맞게 커밋도 나뉘어져 있습니다.

아래는 이번단계에서 새롭게 생성한 클래스에 대한 내용을 흐름 순서대로 간략하게 작성한 것입니다.

HandlerMappings

Dispatcher Servlet에서 request를 받아 해당 요청을 처리할 handler반환 (Controller inteface를 구현한 Controller 객체 또는 Controller 어노테이션과 RequestMapping을 통해 생성된 HandlerExcution 객체)

HandlerAdapterFactory

HandlerMappings를 통해 반환된 handler를 전달받아 HandlerAdapter 생성
이전에 처리된 handler에 대해서는 adapter를 캐싱해두었다 반환

HandlerAdapter

handler (Controller 객체 또는 HandlerExcution 객체)를 가지고 hanlde에 대한 결과물을 ModelAndView로 변환하여 반환
ManualControllerHandlerAdapter : Controller interface를 implement해 구현한 기존 컨트롤러 객체용 어댑터
HandlerExcutionAdapter : 어노테이션 기반으로 AnnotaionHandlerMapping을 통해 생성된 HandlerExcution용 어댑터

기본적으로 mvc관련된 인터페이스와 구현체들은 mvc모듈에 작성하였지만, 기존 어플리케이션 모듈(com.techcourse)에 있던 ManualHandlerMapping을 위한 클래스는 기존 어플리케이션 모듈에 같이 두었습니다.

이번단계 리뷰도 잘부탁드립니다!!

+) 프로젝트와 병행하니라 테스트는 작성하지 않고 진행하였습니다.. 😭

- 기존 RegisterController를 controller implement기반에서annotation기반으로 변경 - RegisterViewController를 RegisterController로 통합

- DispatcherServlet에서 각각의 HandlerMapping을 가지지 않도록 HanlderMappings 추가 - AnnotationHanlderMapping, ManualHandlerMapping의 HandlerMapping interface 추가 - HandlerMapping추가 위치 이동 : DispatcheServlet -> DispatcheServletInitalizer

- ManualControllerHandlerAdatper - 기존 Controller inteface를 위한 어댑터 - Controller 객체를 필드로 가지고있음 - HandlerExcutionAdapter - Annotation 기반으로 생성된 HanlderExcution을 위한 어댑터 - HandlerExcution을 가지고 있음

kwonyj1022

안녕하세요 필립! 리뷰가 많이 늦었네요..

추상화도 잘해주셨고 pr에 자세히 설명해 주셔서 코드를 수월하게 볼 수 있었던 것 같아요.
커밋대로 코드를 보니 필립이 어떤 흐름으로 구현해 나가셨는지 알 수 있었습니다.

리뷰는 거의 궁금한 점 위주여서 바로 approve 하겠습니다!

kwonyj1022 · 2023-09-18T23:29:41Z

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

+    private final List<HandlerMapping> handlerMappings;
+
+    public HandlerMappings() {
+        this.handlerMappings = new ArrayList<>();
+    }
+
+    public void add(final HandlerMapping handlerMapping) {
+        handlerMappings.add(handlerMapping);
+    }


public HandlerMappings(List<HandlerMapping> handlerMappings) { this.handlerMappings = handlerMappings; }

이런 식으로 생성자에서 리스트를 받아서 초기화하는 방법도 있는데, 빈 생성자에서 빈 리스트로 초기화한 후 add 메서드를 통해 하나하나 add 해주는 방식을 채택한 이유가 궁금합니다.

새로운 핸들러맵핑이 생기고 추가될때에 유연성이 좀 더 좋을것 같다는 생각과,
생성자에서 생성을 하게 되면 HandlerMappings를 가지고 있는 DispatcherServlet이 생성될때 인자로 handlerMappings list또한 같이 넘겨줘야 해서, 디스페쳐서블릿을 생성을 먼저 하고, 후에 핸들러맵핑을 추가해주려면 위처럼 add를 통해 추가해줘야기 때문에 이런식으로 했습니다.

DispatcherServlet을 생성할때 추가를 해주게 되면,, 디스페쳐서블릿 생성 전에 핸들러맵핑과 핸들러어댑터를 초기화 해야한다라는 점과, 이러한식으로 무언가 추가될때마다 디스페쳐서블릿의 생성자 파라미터가 늘어나게 될 것 같아서 좀 꺼려지는것 같습니다...
명확하고 깔끔하게는 이유가 없는것같기는 하네요ㅠ

kwonyj1022 · 2023-09-18T23:35:53Z

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

+    private final HandlerMappings handlerMappings;
+    private final HandlerAdapterFactory handlerAdapterFactory;

    public DispatcherServlet() {
+        this.handlerMappings = new HandlerMappings();
+        this.handlerAdapterFactory = new HandlerAdapterFactory();
+    }
+
+    public void addHandlerMapping(final HandlerMapping handlerMapping) {
+        handlerMappings.add(handlerMapping);
+    }
+
+    public void addHandlerAdapterType(final Class<?> handlerType, final Class<? extends HandlerAdapter> adapterType) {
+        handlerAdapterFactory.addAdapterType(handlerType, adapterType);


이 부분도 마찬가지로 생성자에서 초기화하지 않고 메서드를 통해 값을 넣어주는 이유가 궁금합니다.

관련 내용 HandlerMappings 코멘트에 남겼습니다!

kwonyj1022 · 2023-09-18T23:42:06Z

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

+            final HandlerAdapter adapter = handlerAdapterFactory.getAdapter(handler);
+            final ModelAndView modelAndView = adapter.handle(request, response);
+            render(modelAndView, request, response);
+        } catch (Exception e) {


주어진 코드에서 Throwable을 사용했는데 필립은 Exception으로 바꾸셨군요! catch에 Throwable을 사용하는 것과 Exception을 사용하는 것은 어떤 점에서 다를까요? 필립이 생각하신 Exception으로 바꾸었을 때의 이점이 무엇인지 궁금합니다.

리팩터링을 하며 인텔리제이 자동완성을 쓰다보니 생각없이 Exception으로 변경이 되게 되었네요...

엔초의 코멘트 보고나서 다시 생각을 해보고 찾아보다가 stackoverflow 글을 찾을 수 있었습니다.

해당 내용처럼 Throwable로 하게되면 Error또한 캐치를 하게 되고 에러를 핸들링을 해주게 되어서 문제가 생길 수 있다고 생각합니다.
만약 Throwable로 사용을 하면 해당 디스패쳐 서블릿에서 error또한 ServletException으로 예외로 변환해서 던져주게 됩니다. 그러고 디스페쳐 서블릿 입장에서는 서블렛익셉션이 밖에서 어떤 방식으로 핸들링 되는지 (무시하거나 처리 후 프로그램이 진행되는지, 어플리케이션을 종료시키는지) 알 수 없습니다.

하지만 Error의 경우 스택오버플로우, 메모리초과등 시스템적으로 치명적이고, 개발자가 핸들링 하지 않아야 하는, - 어플리케이션을 종료해야하는 - 경우에 해당함으로 그냥 종료가 될 수 있도록 추가적인 핸들링을 안해주는것이 좋지 않을까 라는 생각이 듭니다.

kwonyj1022 · 2023-09-18T23:55:20Z

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

+public interface HandlerAdapter {
+
+    boolean isSupport(Object handler);
+
+    ModelAndView handle(HttpServletRequest request, HttpServletResponse response) throws Exception;
+}


추상화 👍

kwonyj1022 · 2023-09-18T23:56:59Z

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

            }
+            log.info("Load Controller - {}", controllerClass.getSimpleName());


정상 흐름에도 로그를 찍는다면 어떤 이점이 있을까요?

기존 코드에서도 (ManualHandlerMapping) 등록된 컨트롤러들을 로그를 찍어서 확인할 수 있게 해주고있어서 통일성을 위해서라도 작성해주는것이 좋다고 생각했습니다.

통일성을 제외한 있어서 좋은점은 서버를 띄우면서 컨트롤러들이 잘 올라가는지 어떤컨트롤러들이 올라가는지를 확인하기 용이해서 좋다고 생각합니다.

pilyang

엔초가 남겨주신 코멘트들 덕분에 그냥 진행했던 부분도 좀 더 생각해보고 찾아보며 이유 정리해볼 수 있었습니다!!

관련 내용들 코멘트 남겼습니다!

pilyang · 2023-09-19T03:13:59Z

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

+            final HandlerAdapter adapter = handlerAdapterFactory.getAdapter(handler);
+            final ModelAndView modelAndView = adapter.handle(request, response);
+            render(modelAndView, request, response);
+        } catch (Exception e) {


리팩터링을 하며 인텔리제이 자동완성을 쓰다보니 생각없이 Exception으로 변경이 되게 되었네요...

엔초의 코멘트 보고나서 다시 생각을 해보고 찾아보다가 stackoverflow 글을 찾을 수 있었습니다.

해당 내용처럼 Throwable로 하게되면 Error또한 캐치를 하게 되고 에러를 핸들링을 해주게 되어서 문제가 생길 수 있다고 생각합니다.
만약 Throwable로 사용을 하면 해당 디스패쳐 서블릿에서 error또한 ServletException으로 예외로 변환해서 던져주게 됩니다. 그러고 디스페쳐 서블릿 입장에서는 서블렛익셉션이 밖에서 어떤 방식으로 핸들링 되는지 (무시하거나 처리 후 프로그램이 진행되는지, 어플리케이션을 종료시키는지) 알 수 없습니다.

하지만 Error의 경우 스택오버플로우, 메모리초과등 시스템적으로 치명적이고, 개발자가 핸들링 하지 않아야 하는, - 어플리케이션을 종료해야하는 - 경우에 해당함으로 그냥 종료가 될 수 있도록 추가적인 핸들링을 안해주는것이 좋지 않을까 라는 생각이 듭니다.

pilyang · 2023-09-19T03:22:23Z

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

            }
+            log.info("Load Controller - {}", controllerClass.getSimpleName());


기존 코드에서도 (ManualHandlerMapping) 등록된 컨트롤러들을 로그를 찍어서 확인할 수 있게 해주고있어서 통일성을 위해서라도 작성해주는것이 좋다고 생각했습니다.

통일성을 제외한 있어서 좋은점은 서버를 띄우면서 컨트롤러들이 잘 올라가는지 어떤컨트롤러들이 올라가는지를 확인하기 용이해서 좋다고 생각합니다.

pilyang · 2023-09-19T03:31:08Z

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

+    private final List<HandlerMapping> handlerMappings;
+
+    public HandlerMappings() {
+        this.handlerMappings = new ArrayList<>();
+    }
+
+    public void add(final HandlerMapping handlerMapping) {
+        handlerMappings.add(handlerMapping);
+    }


새로운 핸들러맵핑이 생기고 추가될때에 유연성이 좀 더 좋을것 같다는 생각과,
생성자에서 생성을 하게 되면 HandlerMappings를 가지고 있는 DispatcherServlet이 생성될때 인자로 handlerMappings list또한 같이 넘겨줘야 해서, 디스페쳐서블릿을 생성을 먼저 하고, 후에 핸들러맵핑을 추가해주려면 위처럼 add를 통해 추가해줘야기 때문에 이런식으로 했습니다.

DispatcherServlet을 생성할때 추가를 해주게 되면,, 디스페쳐서블릿 생성 전에 핸들러맵핑과 핸들러어댑터를 초기화 해야한다라는 점과, 이러한식으로 무언가 추가될때마다 디스페쳐서블릿의 생성자 파라미터가 늘어나게 될 것 같아서 좀 꺼려지는것 같습니다...
명확하고 깔끔하게는 이유가 없는것같기는 하네요ㅠ

pilyang · 2023-09-19T04:37:43Z

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

+    private final HandlerMappings handlerMappings;
+    private final HandlerAdapterFactory handlerAdapterFactory;

    public DispatcherServlet() {
+        this.handlerMappings = new HandlerMappings();
+        this.handlerAdapterFactory = new HandlerAdapterFactory();
+    }
+
+    public void addHandlerMapping(final HandlerMapping handlerMapping) {
+        handlerMappings.add(handlerMapping);
+    }
+
+    public void addHandlerAdapterType(final Class<?> handlerType, final Class<? extends HandlerAdapter> adapterType) {
+        handlerAdapterFactory.addAdapterType(handlerType, adapterType);


관련 내용 HandlerMappings 코멘트에 남겼습니다!

pilyang added 6 commits September 15, 2023 19:44

refactor: 컨트롤러 초기화 메소드 추출

461e019

chore: 컨트롤러 로딩시 로깅 추가

2e8d0a5

feat: 어노테이션 기반 컨트롤러 맵핑 기능 추가

c27da0a

refactor: RegisterController를 어노테이션 기반으로 변경

e8568c1

- 기존 RegisterController를 controller implement기반에서annotation기반으로 변경 - RegisterViewController를 RegisterController로 통합

kwonyj1022 approved these changes Sep 19, 2023

View reviewed changes

kwonyj1022 merged commit 5d8ac5e into woowacourse:pilyang Sep 19, 2023
1 check failed

pilyang commented Sep 19, 2023

View reviewed changes

pilyang mentioned this pull request Sep 24, 2023

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ MVC 구현하기 - 2 단계 ] 필립(양재필) 미션 제출합니다. #453

[ MVC 구현하기 - 2 단계 ] 필립(양재필) 미션 제출합니다. #453

pilyang commented Sep 17, 2023 •

edited

Loading

kwonyj1022 left a comment

kwonyj1022 Sep 18, 2023

pilyang Sep 19, 2023

kwonyj1022 Sep 18, 2023

pilyang Sep 19, 2023

kwonyj1022 Sep 18, 2023

pilyang Sep 19, 2023

kwonyj1022 Sep 18, 2023

kwonyj1022 Sep 18, 2023

pilyang Sep 19, 2023

pilyang left a comment

pilyang Sep 19, 2023

pilyang Sep 19, 2023

pilyang Sep 19, 2023

pilyang Sep 19, 2023

		}
		log.info("Load Controller - {}", controllerClass.getSimpleName());

[ MVC 구현하기 - 2 단계 ] 필립(양재필) 미션 제출합니다. #453

[ MVC 구현하기 - 2 단계 ] 필립(양재필) 미션 제출합니다. #453

Conversation

pilyang commented Sep 17, 2023 • edited Loading

HandlerMappings

HandlerAdapterFactory

HandlerAdapter

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

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

pilyang commented Sep 17, 2023 •

edited

Loading