Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MVC 구현하기 - 3단계] 제이미(임정수) 미션 제출합니다. #593

Merged
merged 11 commits into from
Sep 25, 2023

Conversation

JJ503
Copy link
Member

@JJ503 JJ503 commented Sep 25, 2023

안녕하세요 민트!
드디어 마지막 3단계네요!

2단계에서 리뷰해 주신 내용에 대한 답변은 해당 pr의 코멘트에서 작성했습니다.
확인해 주시면 감사하겠습니다! (2단계 pr 바로가기)

예외 처리

예외와 같은 경우엔 민트가 말씀해 주신 내용과 함께 전체적으로 고민이 있었던 상황이었습니다.
어느 위치에서 해야 하는 것이 적절한지에 대해서도 고민이 되었고, 예외를 어떻게 처리해줘야 할지에 대해서도 고민되었기 때문이었습니다.
민트의 답변 이후로 DispatcherServlet에서 예외 처리를 해보았는데 어떤 지 확인해 주시면 감사하겠습니다!
추가로 예외 처리와 관련해 여우가 좋은 아이디어로 해결했다고 생각하는데, 해당 pr도 한번 궁금하시다면 확인해 보셔도 좋을 것 같습니다. (여우 pr 바로가기)

이번 요구 사항 의문점

이번 요구 사항 중에선 아래와 같은 요구 사항이 존재했습니다.

Legacy MVC를 제거하고 나서 DispatcherServlet도 app 패키지가 아닌 mvc 패키지로 옮겨보자.

해당 요구 사항으로 DispatcherServlet을 옮긴 후 DispatcherServletInitializer도 옮겨야 하지 않을까? 라고 생각했습니다.
하지만, DispatcherServletInitializer의 경우 mvc 패키지로 옮겼을 때 applicatioin이 제대로 동작하지 않는 문제가 있었습니다.
시간 관계상 해당 문제의 원인을 아직 못 찾기도 했고, 옮기지 않는 것이 맞는 거일까? 라는 의문에 일단 보류해 두었습니다.
해당 고민과 관련해 민트의 의견도 궁금하네요!

이번 리뷰도 잘 부탁드립니다.

@JJ503 JJ503 self-assigned this Sep 25, 2023
Copy link
Member

@yujamint yujamint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 제이미 3단계 잘 진행하셨네요. 고생하셨습니다.
예외처리에 대해 좋은 예시 확인할 수 있도록 레퍼런스 남겨 주셔서 감사합니다!
MVC 미션은 이만 머지하도록 하겠습니다. 코멘트를 좀 남겼으니 이건 확인해 주세요~
고생 많으셨고, JDBC 구현 미션도 파이팅하십쇼~

Comment on lines +39 to +50
} catch (IllegalArgumentException e) {
log.error("Exception : {}", e.getMessage(), e);

final var modelAndView = new ModelAndView(new JspView("/404.jsp"));
response.setStatus(404);
modelAndView.render(request, response);
} catch (Throwable e) {
log.error("Exception : {}", e.getMessage(), e);
throw new ServletException(e.getMessage());

final var modelAndView = new ModelAndView(new JspView("/500.jsp"));
response.setStatus(500);
modelAndView.render(request, response);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외처리 꼼꼼하시네요👍

Comment on lines +17 to +24
response.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);

if (model.isEmpty()) {
return;
}

final String json = objectMapper.writeValueAsString(model);
response.getWriter().write(json);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이게 요구사항인진 모르겠지만.. 힌트에서 다음과 같이 설명하고 있습니다.

model에 데이터가 1개면 값을 그대로 반환하고 2개 이상이면 Map 형태 그대로 JSON으로 변환해서 반환한다.

저는 이 내용이 model에 데이터가 1개면 value만 반환하고, 2개 이상이면 key-value 형태로 반환하는 것이라고 이해했습니다.

근데 요구사항은 아니니까 꼭 고치실 필요는 없는 것 같습니다~

@@ -4,9 +4,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import web.org.springframework.web.WebApplicationInitializer;
import webmvc.org.springframework.web.servlet.mvc.DispatcherServlet;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DispatcherServletInitializer에 대한 이야기는 이 코멘트로 남기겠습니다.
(이니셜라이저라고 칭하겠습니다.)

우선 저도 이니셜라이저를 mvc 모듈 안으로 옮기려고 시도하다가 말았는데요.
현재는 이니셜라이저가 app 모듈에 존재하는 것이 적절하다고 생각하고 있습니다.
그 이유는 이니셜라이저의 역할이 DispatcherServlet을 내 애플리케이션에 맞게 사용하도록 설정하는 것이라고 생각하기 때문입니다.

DispatcherServlet은 현재 다양한 핸들러를 처리할 수 있도록 인터페이스를 제공하고 있습니다. 우리는 이니셜라이저를 통해서 애플리케이션 내의 핸들러를 DispatcherServlet의 인터페이스에 맞게 등록하고 있습니다.

즉, 이니셜라이저는 스프링을 사용하며 우리의 애플리케이션에 맞게 DispatcherServlet을 사용하기 위해 필요한 것이므로 app 모듈 내에 존재하는 것이 적절해 보입니다.

하지만 우리가 지금까지 스프링으로 개발을 하며 애플리케이션 내에 이니셜라이저를 구현한 경험이 없는 것은, 스프링 부트가 이를 자동으로 해주기 때문이라고 하네요..

정확한 레퍼런스와 함께 의견을 제시하면 좋았겠지만,, 저도 아직 제대로 찾아보지 않아서 틀린 정보가 있을 수도 있다는 점.. 양해 부탁드립니다..
(그런가보다~ 하고 넘어가 주셔도 좋습니다)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스프링의 DispatcherServlet 공식 문서를 보다가 다음과 같은 내용을 찾았습니다.

Spring Boot follows a different initialization sequence. Rather than hooking into the lifecycle of the Servlet container, Spring Boot uses Spring configuration to bootstrap itself and the embedded Servlet container. Filter and Servlet declarations are detected in Spring configuration and registered with the Servlet container. For more details, see the Spring Boot documentation.

번역을 해보니.. 스프링 부트에서는 내장된 서블릿 컨테이너를 사용한다고 합니다. 아마도 이러한 내장 설정 덕분에 우리는 서블릿을 설정한 적이 없는 것으로 보입니다!

@yujamint yujamint merged commit 42f1a06 into woowacourse:jj503 Sep 25, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants