Skip to content

Conversation

geoje
Copy link
Member

@geoje geoje commented Sep 25, 2024

안녕하세요 미아,

데모데이가 얼마 남지 않아 미션에 시간을 많이 쏟지 못하여 이제야 제출하게 되었어요! 😥
이번 구현 사항에 대한 주요 항목들은 아래와 같습니다!

0. 학습 테스트

LocalCounterServletSharedCounterServlet 이 모두 @WebServlet 어노테이션을 달고 있는데요,
현재 상황의 서블릿 컨테이너의 Tomcat 은 어플리케이션 실행 시점에 Reflection 으로 스캔하여 서블릿들을 생성하여 컨테이너에 담아 둡니다.
이로써, 특정 Endpoint 에 대한 요청에 대해 생성해둔 서블릿 인스턴스가 작업을 처리하게 됩니다.

이러한 특성으로 인해 SharedCounterServlet 에 필드 값을 스레드간 공유하여 값을 올리게 되고, LocalCounterServlet 은 지역변수를 사용하기 때문에 메서드 호출 시점에 새롭게 0 으로 초기화 되고 +1 하므로 항상 1을 반환하게 됩니다.

1. Legacy MVC와 @MVC 통합하기

appManualHandlerMappingmvcAnnotationHandlerMapping 을 추상화 하기 위해 HandlerMapping 을 두었고,
각각에 대하여 핸들러를 연결하여 서비싱하기 위해 HandlerAdapter 또한 생성하였습니다.

🤗🎊

Copy link

@jongmee jongmee left a comment

Choose a reason for hiding this comment

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

안녕하세요 새양!

저도 팀 프로젝트로 바빠서 이제 리뷰 드리네요.. 그런데 추상화나 역할 분리가 잘 되어 있고 코드가 깔끔해서 리뷰할 게 많지 않습니다... 🥲💯 학습 테스트도 잘 해주신 것 같고요!

테스트 관련해서 리뷰 남긴게 있어 RC 일단 드려요 ✉️

.map(handlerMapping -> handlerMapping.getHandler(request))
.filter(Objects::nonNull)
.findAny();
}
Copy link

Choose a reason for hiding this comment

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

깔끔하고 좋은데요?! 아이디어 💯

// then
assertThat(modelAndView.getModel()).isEmpty();
assertThat(modelAndView.getView()).isInstanceOf(JspView.class);
}
Copy link

Choose a reason for hiding this comment

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

레거시 MVC와 애너테이션 기반 MVC가 공존해야 되는게 미션이었으니 공존에 대한 테스트 코드도 필요하지 않을까요?

애너테이션 기반 MVC가 잘 동작하는 지만 확인하면 공존을 확인할 수 있을까요? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

네!
테스트 코드쪽에 ManualController 를 만든 뒤 2개의 핸들러 매핑을 모두 넣고 Registry 가 각각 잘 선택하는지 테스트 2개를 작성하였습니다!

048cf66

Copy link

Choose a reason for hiding this comment

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

와우 최고입니다 💯

Copy link

@jongmee jongmee left a comment

Choose a reason for hiding this comment

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

새양 리뷰 반영해주신 것까지 잘 봤습니다!

스프링의 HandlerAdapter부터 꼼꼼한 테스트까지 👍 💯 다음 단계에서 봐요!

@jongmee jongmee merged commit 7ba4403 into woowacourse:geoje Sep 29, 2024
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.

2 participants