[MVC 구현하기 - 1단계] 디우(김동규) 미션 제출합니다. #145
Conversation
|
|
||
| model.keySet().forEach(key -> { | ||
| log.debug("attribute name : {}, value : {}", key, model.get(key)); | ||
| request.setAttribute(key, model.get(key)); | ||
| }); | ||
|
|
||
| // todo | ||
| final var requestDispatcher = request.getRequestDispatcher(viewName); |
There was a problem hiding this comment.
DispatcherServlet 에 있던 로직을 JspView 로 이동시키셨군요! 👍🏻
해당 로직이 어떤 역할을 하는지, 왜 이동시켰는지 디우가 이해한 내용을 듣고 싶어요!
There was a problem hiding this comment.
우선 JspView 의 경우에는 요청을 Redirect 하는 책임과 파라미터로 넘어온 model을 request의 attribute에 추가해주는 책임, 그리고 적절한 뷰로 forward 해주는 책임과 같이 뷰 처리에 대한 책임을 가지고 있다고 생각합니다!!
(todo에 채운 두가지 로직은 redirect와 forward 처리에 대한 역할을 수행합니다!😃)
DispatcherServlet 의 역할은 받은 요청을 적절한 컨트롤러에게 작업을 위임하는 것이라고 생각합니다..!!
그리고 컨트롤러에서 처리한 작업 결과에 따라서(응답에 따라서) 적절한 뷰를 호출해주는 것이라고 생각하였습니다!
기존 구조에서는 DispatcherServlet 에서 어떻게 뷰를 처리해야할지에 대해서도 알고 있지만, 앞서 언급한 DispatcherServlet의 책임만을 수행하기에도 충분하고, 더불어서 (아직 정확하지는 않지만..) 향후에 JSON View 와 같은 것들이 추가될 때마다 뷰에 대한 처리를 위한 로직이 추가되면 너무 많은 책임을 가지게 될 것 같다고 생각하여 분리해주었습니다!!
| private final Map<HandlerKey, HandlerExecution> handlerExecutions; | ||
|
|
||
| public AnnotationHandlerMapping(final Object... basePackage) { | ||
| public AnnotationHandlerMapping(final String... basePackage) { |
There was a problem hiding this comment.
여러개의 베이스 패키지에 있는 어노테이션을 기반으로
핸들러를 등록시키게 된다면, 가변 파라미터로 받게 해두어
로직 변경 없이 수행가능하겠네요. 디우 센스 👍🏻
| public void initialize() { | ||
| log.info("Initialized AnnotationHandlerMapping!"); | ||
| initHandlerExecution(basePackage); | ||
| } | ||
|
|
||
| public Object getHandler(final HttpServletRequest request) { | ||
| return null; | ||
| return handlerExecutions.get( | ||
| new HandlerKey(request.getRequestURI(), RequestMethod.valueOf(request.getMethod()))); | ||
| } | ||
|
|
There was a problem hiding this comment.
메서드 정렬
코드를 살펴보면서, 적절한 단어와 기능 분리를 해주셔서
편안했어요. ㅎㅎ 저는 네이밍 짓는게 너무 어렵던데 디우 대단하네요.
바쁜 시기라서 당연히 디우가 잘하시는 부분이겠지만,
메서드 정렬이 필요해 보이는 로직이 조금 있네요! ㅠㅠ
저는 주로 public -> 내에서 호출되는 private 순으로 메서드를 선언하고 있어요.
공식 라이브러리나 기능 위주의 클래스같은 경우는 public 이 전부 상단에 위치하고, 그 이후로 private 가 오더라구요.
디우는 어떤 기준으로 메서드를 선언해두는지 궁금합니다. ㅎㅎ
There was a problem hiding this comment.
저도 네이밍은 참 어려운 것 같아요..ㅠ.ㅠ
그래도 네이밍에 신경을 썼는데 수달이 알아봐주시고 보기 편하셧다니 기쁘네요!ㅎ.ㅎ😃
수달에게 질문을 받고 나니, 직접적으로 관련있는 내용은 아니지만..1단계 블랙잭 미션 때 로운에게 받은 피드백이 기억이 나네요!! ㅎ.ㅎ 로운 피드백
항상 기능 분리를 하면서 private 메소드가 늘어남에 따라서 메소드의 순서를 어떻게 해야할지 막막할 때가 있었는데, 저는 당시에 로운 피드백을 받고 나서는 어느정도 저만의 기준이 세워졌고, 일반(public, protected, default) 메소드 -> private 메소드 -> getter & setter -> eq&hc .. 순으로 메소드를 정렬하고 있어요..!!
그리고 지금과 같이 private 메소드가 많아지는 경우에는 위에 먼저 선언되어 있는 일반 메소드쪽에서 호출되는 순서로 정렬을 하고 있습니다!! 따라서 AnnotationHandlerMapping 가 이와 같은 순서를 가지게 되었습니다.ㅎㅎ
| private static HandlerKey getHandlerKey(final RequestMapping requestMapping) { | ||
| return new HandlerKey(requestMapping.value(), | ||
| RequestMethod.valueOf(requestMapping.method()[METHOD_INDEX].name())); | ||
| } | ||
|
|
There was a problem hiding this comment.
같은 url 에 메서드가 여러 개인 경우
디우, Annotation 기반으로 Handler init 해두는 부분이 메서드명도 센스있고
책임도 적절히 분배되어서 보기 좋았어요. ㅎㅎ
이 부분에서 조금 더 생각해보면 좋을 점이 메서드 시그니처가 1개 이상일 이후에 들어오는 메서드는 Handler 로 등록이 안될 것 같아요.
실제로 사용하던 코드를 떠올려보면 같은 url 로 GET 을 하기도 하고 POST 를 하기도 하니까 위 점도 고려해보면 재밌을 것 같아요 !!!
ps. 바쁘니까 .. 선택 반영 해주세요.
@RequestMapping(value = "/register", method = {GET, POST})
public String execute(final HttpServletRequest req, final HttpServletResponse res) throws Exception{
...
}There was a problem hiding this comment.
아하,,,그러네요...!! 수달이 예시로 올려주신 코드라면 GET 메소드에 대해서 밖에 등록이 안되겠네요..고려하지 못했던 사항이었던 것 같아요!! ㅠ.ㅠ
꼼꼼하게 리뷰해주셔서 감사합니다..!!😃
수달이 예시로 들어주신 코드에서 POST 메서드에 대해서도 지원해줄 수 있도록 수정해주었으며, 아래와 같은 코드에서 POST 메소드에 대해서도 핸들러 매핑이 되는지를 확인하는 테스트 코드를 추가해주었습니다!!😄
@RequestMapping(value = "/multiple-method-test", method = {GET, POST})
public ModelAndView supportMultipleMethod(final HttpServletRequest request, final HttpServletResponse response) {
log.info("test support multiple method");
final var modelAndView = new ModelAndView(new JspView(""));
modelAndView.addObject("id", request.getAttribute("id"));
return modelAndView;
}| modelAndView.addObject("id", request.getAttribute("id")); | ||
| return modelAndView; | ||
| } | ||
|
|
||
| @RequestMapping(value = "/post-test", method = RequestMethod.POST) | ||
| public ModelAndView save(final HttpServletRequest request, final HttpServletResponse response) { | ||
| log.info("test controller post method"); | ||
| final var modelAndView = new ModelAndView(new JspView("")); | ||
| final var modelAndView = new ModelAndView(new JspView("redirect:/")); |
There was a problem hiding this comment.
AnnotationHandlerMappingTest 에는 ModelAndView 데이터 검증에 대한 로직만 있어요.
해당 부분에서 redirect:/ 를 하게 된다면 forward 가 되지 않을 것 같은데요!
redirect 와 forward 의 차이를 살펴보면 좋을 것 같아요.
저도 이 부분을 자세히 몰라서 이번 기회에 학습했었는데. 디우와 공유하고 싶네요 ㅎㅎ
수달 학습 내용
There was a problem hiding this comment.
오!! 수달, 학습 하신 내용 공유해주셔서 정말 감사합니다!!!ㅎ.ㅎ👍👍
AnnotationHandlerMappingTest 의 목적에 대해서 저도 수달과 동일하게 생각합니다!! AnnotationHandlerMappingTest 의 handlerMapping.getHandler() 에서 어노테이션 기반으로 핸들러를 찾을 수 있는지, 그리고 그렇게 찾은 HandlerExecution 의 handle() 메소드를 통해서 요청을 처리하고 적절한 ModelAndView 를 반환해줄 수 있는지를 확인해주고 있다고 생각해요!!
그런데..AnnotationhandlerMappingTest 에서 handle() 메소드를 통해서 findUserId() 가 수행되고 redirect 와 forward 에 관계없이 ModelAndView 는 적절히 담겨 반환되어 원하는 검증을 수행할 수 있을 것 같은데요..ㅠ.ㅠ
혹시 제가 놓치고 있는 부분이 있을가요...ㅠ.ㅠ
p.s. 이와는 별개로 제가 결정적으로 redirect 했던 이유가 DispatcherServletTest 의 getHandler() 메소드를 통과시키기 위해서 였는데요..!! 어떤 하나의 테스트를 통과시키기 위해서 여러 테스트에서 사용하고 있는 코드 혹은 Fixture 를 그 테스트에 맞게 변화시킨다는 게 적절치 않다고 판단되어 수정하였습니다!! (이렇게 다시 보니, 사실 기존 테스트가 제대로된 테스트였다고도 말 못하겠네요..;;ㅎㅎ)
안녕하세요, 수달!!🦦 디우입니다.😃
이번 미션을 진행하면서 이전에 공부하였던, 스프링 MVC 구조를 다시 한 번 상기시켜보는 기회가 되었습니다!!
특히, 스프링을 사용하면서 항상 궁금하였던 어노테이션 기반의 매핑 과정을, 리플렉션을 이용해서 직접 구현해보는 경험은 매우 새로웠던 것 같아요!! ㅎ.ㅎ
이외에도
JspViewTest에서 mocking 을 잘 몰라서 발생하는 문제도 해결하며 다시 한 번 학습하는 기회가 되었습니다!이번 미션에서는 Adapter 를 사용하지 않고, 분기문을 사용해서 Annotation 기반의 Handler 를 지원해주도록 구현하였습니다!
(2단계 미션(점진적인 리팩터링)을 진행하면서 기존 코드에서 Adapter 를 사용하는 방향으로 개선할 기회가 있을 것 같아 이와 같이 구현하였습니다..!!)
구현 기능 목록(구현 내용)
1. AnnotationHandlerMapping
리플렉션을 사용해서 basePackage 하위에 있는
Controller어노테이션 클래스들을 찾아오도록 하였습니다.이후 해당 클래스들의 메소드들 중
RequestMapping어노테이션이 붙은 메소드들로HandlerExecution을 생성해 등록함으로써 어노테이션 기반의 핸들러를 지원해주도록 구현하였습니다!2. AnnotationhandlerMapping & ManualHandlerMapping 지원
DispatcherServlet에서 등록된handlerMapping에 대해서instanceof키워드로Controller인터페이스를 구현하고 있는지,HandlerExecution인지에 따라 분기해줌으로써 기존ManualHandlerMapping이외에도smaplesbasePackage 하위에 있는 어노테이션 기반의 핸들러를 매핑해줄 수 있도록 구현하였습니다. 별도의 컨트롤러를 프로덕션 코드에 추가하지는 않고, 테스트 코드를 통해서DispatcherServlet이 어노테이션 기반의 핸들러를 지원해주는지 확인해주었습니다.3. JspView
체크리스트에 있는 사항은 아니었지만, 힌트에
JspView는 어떻게 구현 할 수 있을까?와 같은 내용이 있어서 JspView 도 함께 구현하고 테스트도 진행해주었습니다! JspView 로 redirect와 forward에 대한 책임이 분리되면서DispatcherServlet에서는 기존의move()메소드를 제거해주었습니다!체크 리스트
잘 부탁드립니다!!🙂