[MVC 구현하기 - 3단계] 아벨(신준혁) 미션 제출합니다. #608
Conversation
There was a problem hiding this comment.
안녕하세요 아벨!! 수고 많으셨습니다 🙂
이번에도 깔끔하게 잘 구현해주셔서 코멘트할 부분은 많지 않았는데요.
요구사항에 있는 REST API가 동작하지 않는 것을 확인해 부득이하게 변경 요청 드립니다!
다른 부분은 잘 구현해주신 것 같아 해당 부분 해결하여 리뷰 요청 주시면 approve 하도록 할게요~!
🙋♀️ 질문은 사소한 부분 하나 남겼는데, 여유가 되실 때 답해주셔도 좋습니다.
http://localhost:8080/api/user?account=gugu로 요청을 보냈을 때의 화면입니다.
| if (model == null || model.isEmpty()) { | ||
| response.setStatus(SC_NOT_FOUND); | ||
| return; | ||
| } |
There was a problem hiding this comment.
model이 null이거나 빈 경우도 고려해주셨군요! 👍
그런데 이에 대해 404 상태코드를 보내주는 방식으로 처리하신 이유가 궁금합니다 😀
🙋♀️ 이에 대한 제 생각은 아래와 같은데, 아벨은 어떻게 생각하시는지 말씀해주실 수 있을까요?
- 저는 model에 null을 전달하는 경우라면 사용자의 잘못된 요청보다는 프레임워크 단의 오류일 것이라 생각해 500번대 상태코드를 보낼 것 같아요.
- Spring Boot의
@RestController동작을 기준으로 삼자면, responseBody를 비워두고 응답을 할 때, ContentType을 application/json 으로 설정해주는 것 같은데요.
용례로는 DELETE에 대한 응답을 하는 경우를 생각해볼 수 있을 것 같아요.
model.isEmpty를 검증하면 이런 경우를 지원하지 못하는 것 아닐까? 라는 생각이 들었어요.
비어 있는 responseBody의 ContentType을 application/json으로 해도 되느냐가 관건인 것 같네요. (RFC나 mdn에서는 딱히 언급되지 않는 듯합니다)
There was a problem hiding this comment.
도이가 말씀해주신 2번의 경우 null로 오는지 비어서 오는지 확실하게는 모르겠지만, null인 경우와 빈 값인 경우를 구분해서 상태코드를 반환해주는 것으로 개선해보았습니다 :)
There was a problem hiding this comment.
앞선 질문은 JsonView에서 model이 비어있으면 무조건 404 에러를 줘야 하는 상황인가? 에 대해서 이야기해보고자 남겨본 질문이었습니다! 🙂
Spring Boot의
@RestController동작을 기준으로 삼자면, responseBody를 비워두고 응답을 할 때, ContentType을 application/json 으로 설정해주는 것 같은데요.
이 부분에 대해 추가로 찾아보니 무조건 그런 게 아니라, 여러 경우의 수가 있어서 정정합니다. (객체로 반환 / accept 헤더 / mediaType 지정)
| @GetMapping(value = "/api/user") | ||
| public ModelAndView show(HttpServletRequest request, HttpServletResponse response) { | ||
| final String account = request.getParameter("account"); | ||
| log.debug("user id : {}", account); | ||
|
|
||
| final ModelAndView modelAndView = new ModelAndView(new JsonView()); | ||
| final User user = InMemoryUserRepository.findByAccount(account) | ||
| .orElseThrow(); | ||
|
|
||
| modelAndView.addObject("user", user); | ||
| return modelAndView; | ||
| } |
| return "/register.jsp"; | ||
| @GetMapping("/register/view") | ||
| public ModelAndView execute(final HttpServletRequest req, final HttpServletResponse res) { | ||
| return new ModelAndView(new JspView("/register.jsp")); |
There was a problem hiding this comment.
이제 하나의 컨트롤러에서 동일한 URI에 Http 메서드로 api를 구분할 수 있게 되어서
RegisterController에서 @GetMapping("/register")로 매핑하는 방법도 있지만,
레가시 코드이기 때문에 기존에 이미 사용되고 있는 API의 명세를 바꾸지 않는 방향으로 적용해주신 것 같군요. 👍 맞나요?!?
There was a problem hiding this comment.
맞습니다. 일단 jsp 파일은 그대로 둔다고 가정하고 진행했기 때문에 호환성을 위해서라도 api명세를 일단 유지하는 것을 택했습니다.
| final View view = modelAndView.getView(); | ||
|
|
||
| move(view.getViewName(), request, response); | ||
| } | ||
|
|
||
| private void move( | ||
| final String viewName, | ||
| final HttpServletRequest request, | ||
| final HttpServletResponse response | ||
| ) throws Exception { | ||
| if (viewName.startsWith(JspView.REDIRECT_PREFIX)) { | ||
| response.sendRedirect(viewName.substring(JspView.REDIRECT_PREFIX.length())); | ||
| return; | ||
| } | ||
|
|
||
| final var requestDispatcher = request.getRequestDispatcher(viewName); | ||
| requestDispatcher.forward(request, response); | ||
| view.render(modelAndView.getModel(), request, response); |
There was a problem hiding this comment.
ModelAndView가 model과 view를 모두 알고 있으니,
ModelAndView.render(request, response)로 묶어주는 것도 괜찮아보여요!
| @@ -36,7 +37,7 @@ void register() throws Exception { | |||
| final var handlerExecution = (HandlerExecution) handlerMapping.getHandler(request); | |||
| final var modelAndView = (ModelAndView) handlerExecution.handle(request, response); | |||
|
|
|||
| assertThat(modelAndView.getView().getViewName()).isEqualTo("redirect:/index.jsp"); | |||
| assertThat(modelAndView.getView()).usingRecursiveComparison().isEqualTo(new JspView("redirect:/index.jsp")); | |||
| return "/register.jsp"; | ||
| @GetMapping("/register/view") | ||
| public ModelAndView execute(final HttpServletRequest req, final HttpServletResponse res) { | ||
| return new ModelAndView(new JspView("/register.jsp")); |
안녕하세요 도이! 이번 3단계에서 클래스의 위치와 어노테이션 기반으로의 컨트롤러들로 개선해보았습니다.
리뷰 잘 부탁드립니다! :)