[MVC 구현하기 - 2단계] 망고(고재철) 미션 제출합니다. #531
Conversation
There was a problem hiding this comment.
안녕하세요 망고~~ 코드 잘 보았습니다!
3단계 또한 진행해야 해서 빠른 머지를 할까 했으나 요구사항 컨트롤러 중 하나의 URI 매핑이 잘못 되어있어..!
이 부분은 꼭 수정되어야 할 것 같아 Request Change를...! 합니다! 😂
이것만 수정하신 뒤 요구사항은 만족하므로 다시 리뷰 요청 주시면 바로 머지하도록 하겠습니다!
사실 이번 미션에서는 2단계가 가장 중요하다고 생각해 오래 고민해보셨으면 하는 마음도 있어요~
3단계는 좀 간단히 하더라도...
리뷰에서 일방적인 내용 전달은 지양되어야 함은 알지만 기간이 얼마 남지 않은 것에 비해 중요한 2단계에서 얻어갔으면 하는 내용들이 있어서 제가 파악한 미션의 의도만을 전달드리고, 이에 맞게 고민해보심은 주말에 시간이 있으시다면 진행하시고 아니라면 간단히 확인만 하시면 좋을 것 같아요!!😂
아래는 구구가 오픈해주신 이번 2단계의 힌트와 관계 없이 주신 질문에 대한 제 사견을 적어보니
필요하다고 생각하시면 참고하시고 아니라고 생각이 들면 고민에 시간 쏟지 않으셔도 괜찮습니다~
2단계에서 요구하는 바와 패키지 구조
미션의 이름이 MVC 구현하기지만 제 생각에 절반은 Spring MVC에 대한 구현, 절반은 레거시에 대한 효율적인 리팩터링에 대한 고민이 목표라고 느껴졌습니다.
그렇기 때문에 저는 app 패키지의 작업과 mvc패키지의 작업을 완전히 구분해서 고민해야 한다고 생각합니다.
app패키지의 기존 코드를 보면 톰캣을 통한 서블릿 컨테이너 구현부, 거기에 상속 기반의 컨트롤러를 사용하는 DispatcherServlet라는 이름의 Servlet이 포함됩니다. 이외에는 레거시에 해당하는 컨트롤러 구현체이고요.
미션을 재해석하면 우리 회사에 이미 jetty 혹은 tomcat servlet container를 쓰는 레거시 프로젝트가 있는데 여기에 어노테이션 기반으로 컨트롤러를 작성할 수 있는 servlet 라이브러리를 찾아서 코드좀 바꿔 봐라는 업무를 상사한테 받았다고 생각하면 될 것 같아요.
여기서 servlet 라이브러리는 아예 다른 모듈인 mvc쪽에 있는 코드 전체를 얘기한다고 볼 수 있겠네요.
이제 망고가 주신 다음 질문에 대해 제 대답을 할 수 있을 것 같아요
구현 과정에서 제일 의문이 들었던 점은 아무래도 패키지 구조인 것 같아요..
왜 DispatcherServlet이 app에 위치하는지, 이 위치를 그대로 가져가는게 요구사항인지, 이 위치가 레거시여서 바꾸는 걸 원하는지, 그리고 전체적으로 정확하게 app과 mvc 모듈의 차이가 무엇인지... 고민해봤는데 답이 없는 문제인 것 같고, 잘 모르겠네요..
우린 이제 외부 라이브러리를 가져다 쓰는 상황이므로 app쪽 코드는
- tomcat + legacy dispatcherServlet + legacy controller
의 세 가지 구성에서
- tomcat + legacy controller & annotated controller
의 구성으로만 이루어지고 annotated controller는 새로운 라이브러리에 있을 '어떤' 서블릿에서 알아서 처리를 해줘야 하는 거죠. 이 어떤 서블릿이 mvc 패키지에 있어야 하고 이것을 app에서 톰캣 기반으로 실행되도록 등록하면 되겠네요. 물론 꼭 서블릿을 위한 라이브러리일 필욘 없으나 이렇게 가정하는 편이 가장 구현 코드와 라이브러리의(외부 모듈) 코드를 완벽하게 나눌 수 있는 방향이라는 생각이 들어서요 ㅎㅎ;
언뜻 보면 패키지 구조를 어떻게 가져가냐가 취향의 문제라고 보일 수도 있겠지만
모듈을 나눠놓은 순간 두 모듈의 역할은 완벽히 구분되어야 함을 암시하고 이것을 어느 지점에서 구분할지가 중요한 문제라고 보면 될 것 같아요.
MVC 모듈에는 어떤 내용들이 있어야 할까요? 위의 제 생각이 맞았다고 가정하고,
망고가 오픈소스 MVC 라이브러리를 만들었는데 이거를 자신있게 회사 코드에서 build.gradle에 추가하고 도입한다고 생각하면 될 것 같아요 ㅋㅋㅋㅋㅋㅋ
그렇다면 만들어주신 DispatcherServlet, HandlerAdapter, HandlerMapping...와 같이 라이브러리에서 제공하는 서블릿과 그것을 보조하는 클래스들은 모두 mvc쪽에 있어야 할 것 같습니다.
물론 어디까지나 제 생각입니다 ㅎㅎㅎ
mapping url과 관련된 부분만 수정해주시고 나머지는 시간 되시는대로 필요하다면 고민해주세요~
3단계는 하루면 하실 것 같거든요 😄
| return new ModelAndView(new JspView("redirect:/index.jsp")); | ||
| } | ||
|
|
||
| @RequestMapping(value = "/register", method = RequestMethod.GET) |
There was a problem hiding this comment.
구현은 다 하셨는데...!
요구사항에 맞게 아래처럼 바꿔야 동작합니다...!! ㅋㅋㅋㅋㅋㅋ
| @RequestMapping(value = "/register", method = RequestMethod.GET) | |
| @RequestMapping(value = "/register/view", method = RequestMethod.GET) |
이 부분만 필수적으로 수정되어야 할 부분인 듯 하여 여기만 고치신 뒤 리뷰 요청 주셔도 괜찮습니다!
There was a problem hiding this comment.
2단계 요구 사항에 있는 RegisterController 변경 코드를 보고 이번에도 코드 자체가 요구 사항이라고 생각해서 그대로 적용했었습니다..!
하지만 사람마다 해석이 달라질 수 있겠네요.
저처럼 어노테이션 기반 컨트롤러로 점진적으로 리팩토링 하면서 HTTP Method만 다르고 URI는 같도록 API 명세도 점진적으로 변경한다라고 해석할 수도 있고
요구 사항 자체가 인터페이스 기반 MVC 프레임워크와 어노테이션 기반 MVC 프레임워크가 공존해야 한다라고 생각해서 모디 의견처럼 URI는 아직 기존 그대로 가져가도록 해석할 수도 있을 것 같아요.
이건 사실 리팩토링을 요구한 주체만 정답을 알 것 같네요 ㅋㅋㅋㅋㅋ
그래도 모디 의견을 듣고 보니 요구 사항에 있던 예시 코드보다는 인터페이스 기반 MVC 프레임워크와 어노테이션 기반 MVC 프레임워크가 공존해야 한다라는 말에 더 집중해야 할 것 같네요.
수정했습니다!
There was a problem hiding this comment.
망고 말씀도 맞는 말씀이네요 ㅎㅎㅎ
LMS에서 명시한 방향을 따라가는게 우선이니 망고처럼 /register로 두고 login.jsp쪽에서 /register에 POST로 요청을 보내도록 하는 방향이 맞았을 것 같습니다!
| private final transient HandlerMappings handlerMappings; | ||
| private final transient HandlerAdapters handlerAdapters; |
There was a problem hiding this comment.
transient를 선택하신 이유는 무엇일까요~?
혹시 Servlet이 Serializable한 것과 관계가 있을까요?
There was a problem hiding this comment.
넵! 이 부분은 Serializable 클래스는 필드들도 Serializable or transient 해야한다는 SonarLint의 조언을 보고 적용했습니다.
HandlerMappings와 HandlerAdapters는 직렬화 해줄 필요성을 못느껴서 transient로 두었습니다!
| void initialize(); | ||
|
|
||
| Object getHandler(final HttpServletRequest request); |
There was a problem hiding this comment.
아마 Spring의 HandlerMapping에는 getHandler만이 존재하지만
원래 존재하던 ManualHandlerMapping에는 initialize()가 존재해 이것을 맞춰주고자 initialize()를 인터페이스에도 추가하신게 아닐까 싶어요~
이것을 없애기 위해서는 어댑터 패턴을 활용하면 해결하실 수 있을거에요~!
기존의 ManualHandlerMapping의 handle()메서드 시그니처를 바꿀 필요도 없구요!
There was a problem hiding this comment.
ManualHandlerMapping과 기존 AnnotationHandlerMapping의 메서드 구성이 같아서 해당 클래스들을 추상화 한 HandlerMapping 인터페이스를 만들고 두 메서드를 다 가지도록 했습니다!
이 과정에서 오는 ManualHandlerMapping의 handle() 메서드 시그니쳐 변경은 오히려 이렇게 변경하는 편이 더 자연스럽다고 판단해서 변경했습니다..!
There was a problem hiding this comment.
스프링과 유사하게 initialize()를 생성자에서 호출하면 어떨까 하여 여쭌 것인데
외부에서 초기화를 수행하는 것도 괜찮으시다면 그대로 가셔도 될 듯 합니다 😁
|
아이고.. 정말 엄청난 리뷰를 남겨주셨군요 모디 코멘트를 참고해서 2단계에 조금 더 집중해서 고민해봤습니다! 다시 한 번 리뷰 정말 감사합니다 🙇♂️ |
|
바쁜 일정 속에서 2단계 진행하시느라 고생하셨습니다~~ |
안녕하세요 모디!
이번 2단계는 근로랑 데모데이 준비하느라 생각보다 늦어졌네요 🥲
우선은 2단계 요구 사항에 맞게, RegisterController를 어노테이션 방식으로 변경해도 정상 동작하는지에 집중해서 해당 요구사항까지만 충족하도록 구현했습니다.
구현 과정에서 제일 의문이 들었던 점은 아무래도 패키지 구조인 것 같아요..
왜 DispatcherServlet이 app에 위치하는지, 이 위치를 그대로 가져가는게 요구사항인지, 이 위치가 레거시여서 바꾸는 걸 원하는지, 그리고 전체적으로 정확하게 app과 mvc 모듈의 차이가 무엇인지... 고민해봤는데 답이 없는 문제인 것 같고, 잘 모르겠네요..
이 부분에 대한 모디의 의견도 궁금합니다..!
부족한 부분 많지만 리뷰 잘 부탁드립니다 🙇♂️