[MVC 구현하기 2단계] 히이로(문제웅) 미션 제출합니다. #464
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
parkmuhyeun
requested changes
Sep 19, 2023
| import webmvc.org.springframework.web.servlet.mvc.tobe.HandlerExecution; | ||
| import webmvc.org.springframework.web.servlet.mvc.tobe.HandlerMapping; | ||
|
|
||
| public class HandlerMappingAdapter implements HandlerMapping { |
|
|
||
| public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response) | ||
| throws Exception { | ||
| return (ModelAndView) method.invoke(clazz.getDeclaredConstructor().newInstance(), request, |
There was a problem hiding this comment.
요청이 handle 될 때마다 컨트롤러를 새로 인스턴스화 할꺼 같은데 처음에 AnnotationHandlerMapping 클래스의 getAnnotatedControllerClasses() 메서드에서 미리 컨트롤러들을 인스턴스화 시켜놓는 건 어떤가요!?
There was a problem hiding this comment.
처음 구현할 때는 생각치 못한 부분이었는데 확실히 매 요청마다 컨트롤러가 초기화 되는 것은 이상하죠 ㅎㅎ... 일러주신 방향대로 수정했습니다! 👍
| import org.reflections.Reflections; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import web.org.springframework.web.bind.annotation.RequestMapping; | ||
| import web.org.springframework.web.bind.annotation.RequestMethod; | ||
|
|
||
| public class AnnotationHandlerMapping { | ||
| public class AnnotationHandlerMapping implements HandlerMapping{ |
There was a problem hiding this comment.
Controller를 찾고 인스턴스화 하는 역활을 하는 객체를 한번 분리해보면 어떨까요!?
There was a problem hiding this comment.
처음에는 이걸 어떤 이유로 분리해야 할까? 라고 생각이 들었는데 막상 분리해보니 annotation 기반 컨트롤러를 스캔해서 인스턴스화 하는 책임까지 부여하니까 위에서 무민이 언급해주신 요청마다 컨트롤러를 인스턴스화 해줘야 하는 문제도 함께 해결할 수 있었네요 ㅎㅎ... 👍
| private final List<HandlerMapping> handlerMappings = new ArrayList<>(); | ||
|
|
||
| public void init() { | ||
| handlerMappings.add(new HandlerMappingAdapter(new ManualHandlerMapping())); |
MoonJeWoong
commented
Sep 24, 2023
parkmuhyeun
approved these changes
Sep 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 무민! 히이로입니다.
이번 step2에서는 기존 controller들과 ManualHandlerMapping 클래스를 수정하지 않고 새로운 annotation 기반 controller와 공존하도록 할 수 있을지를 포커싱해서 진행했습니다. 그 결과 Adapter 패턴을 사용해서 기존 ManualHandlerMapping과 AnnotationHandlerMApping이 함께 동작할 수 있도록 다형성을 부여했습니다. 요구사항 동작 여부를 테스트하실 수 있도록 app 패키지 내 techcourse/controller 경로에 AnnotatedLogoutController를 작성해뒀습니다. HandlerMapping 패키지 내 ManualHandlerMapping 클래스에서 logoutController를 put해주는 부분을 주석처리 후 프로젝트 정상 로그아웃 플로우를 테스트해주시면 될 것 같습니다.
구현완료한 뒤 패키지 정리를 해보려 했는데 techcourse 패키지 내 기존 ManualHandlerMapping을 의존하는 코드들 때문에 DispatcherServlet이나 HandlerMapping 패키지 내 클래스들 등... MVC 관련 코드들을 app 패키지에서 분리하기가 어려웠습니다. 해당 부분은 이후 기존 controller 로직을 제거하면서 리팩토링을 진행하고 나면 자연스럽게 MVC 패키지로 분리할 수 있지 않을까 생각하고 있습니다.
리뷰하시는데 조금이라도 도움이 될까 싶어 이번 미션에서 작성한 클래스 다이어그램을 첨부합니다.
따로 스프링 코드를 참고하며 만들지는 않았어서 원래 설계되었던 의도와는 다르게 구현된 부분도 있을 수 있을 것 같은데 이런 부분은 함께 말씀해주시면 좋을 것 같습니다 😃
혹시 애매하거나 궁금한 부분이 있다면 편하게 DM 주세요. 리뷰 잘 부탁드립니다!! 🙇