4단계 - Interceptor 구현 - 리뷰 요청#186
Conversation
ghojeong
left a comment
ghojeong
left a comment
There was a problem hiding this comment.
고산하님!
Thread-Safe 하게 속도를 측정할 수 있도록 매우 훌륭하게 구현해주셨습니다.
워낙 잘해주셔서 해당 부분은 크게 드릴 말씀은 없고,
Registry 와 관련된 아이디어가 굉장히 재밌어서 개인적으로 질문을 몇개 남겨보았습니다.
답변 주시면 감사하겠습니다.
즐거운 추석되세요. 🌔
| import java.sql.ResultSet; | ||
| import java.sql.SQLException; | ||
|
|
||
| public class ObjectMapper<T> implements RowMapper { |
There was a problem hiding this comment.
ObjectMapper 와 RowMapper 구분해주신 점 멋집니다. 👍
| public interface HandlerInterceptor { | ||
|
|
||
| boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler); | ||
| void postHandle(HttpServletRequest request, HttpServletResponse response, Object handler, ModelAndView modelAndView); | ||
| void afterCompletion(HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex); | ||
| } |
There was a problem hiding this comment.
RowMapper 때와 마찬가지로, HandlerInterceptor 를 인터페이스로 정의해주신 점 멋집니다. 👍
프레임워크를 구현하면서 인터페이스를 매우 능숙하게 사용해주시는 군요.
| public class HandlerInterceptorRegistry implements HandlerInterceptor { | ||
|
|
||
| private static final List<HandlerInterceptor> INTERCEPTORS = List.of(new HandlerInterceptorImpl()); | ||
|
|
||
| private final Collection<HandlerInterceptor> interceptors; | ||
|
|
||
| private HandlerInterceptorRegistry(Collection<HandlerInterceptor> interceptors) { | ||
| Assert.notEmpty(interceptors, ""); | ||
| this.interceptors = interceptors; | ||
| } | ||
|
|
||
| public static HandlerInterceptorRegistry defaults() { | ||
| return new HandlerInterceptorRegistry(INTERCEPTORS); | ||
| } |
There was a problem hiding this comment.
제가 읽다가 감탄했던 부분이 이 부분인데,
등록된 인터셉터들을 private static final List 로 가지고 있는 Registry 를,
다시 또 HandlerInterceptor 로 implements 했다는 점입니다.
| handlerInterceptor.preHandle(request, response, maybeHandler); | ||
| ModelAndView mav = handlerExecutor.handle(request, response, maybeHandler.get()); | ||
| render(mav, request, response); | ||
| handlerInterceptor.postHandle(request, response, maybeHandler, mav); |
There was a problem hiding this comment.
DispatcherServlet 에 오직 InterceptorRegistry 만을 갖고 있도록 하고,
그에대한 처리를 일반 Interceptor 처럼 하고 있는거 매우 신선하고 멋진 아이디어라고 생각합니다.
| public class HandlerInterceptorImpl implements HandlerInterceptor { | ||
| private static final Logger logger = LoggerFactory.getLogger(HandlerInterceptorImpl.class); | ||
|
|
||
| private final ThreadLocal<StopWatch> stopWatchThreadLocal = new ThreadLocal<>(); |
There was a problem hiding this comment.
다만 HandlerInterceptorRegistry 가 있다는 점에서, 추후에 더 많은 Interceptor 를 붙이려고 계획중이신것 같은데,
그렇다면 그냥 HandlerInterceptorImpl 이 아니라, 이 인터셉터가 무슨 역할을 하는 인터셉터인지 클래스명만 보고서도,
바로 알 수 있도록 이름을 지어주시면 좋겠습니다
저는 개인적으로 인터페이스 이름 + Impl 의 조합은 DI 프레임워크를 통한 유일무이한 싱글톤 객체 주입만이 있을 때만 사용합니다.
지금처럼 여러개의 Interceptor 가 등록될 수 있는 상황에서는, 이름을 더 구체적으로 지어주는게 관리가 가능할 것 같습니다.
There was a problem hiding this comment.
피드백 감사합니다!
인터셉터의 역할에 맞게 시간 측정과 관련된 네이밍으로 변경하겠습니다
| private static final Logger logger = LoggerFactory.getLogger(DispatcherServlet.class); | ||
|
|
||
| private HandlerMappingRegistry handlerMappingRegistry; | ||
| private final HandlerInterceptor handlerInterceptor = HandlerInterceptorRegistry.defaults(); |
There was a problem hiding this comment.
마지막으로 이건 Registry 에 대해 논의를 해보고 싶은 제 개인적인 궁금증인데,
고산하 님은 Registry 를 일급 컬렉션으로 보고 HandlerInterceptor 와는 개념 혹은 인터페이스를 분리하는 것에 대해서 어떻게 생각하시나요?
HandlerInterceptorRegistry 또한 HandlerInterceptor 로 취급하는 것은 매우 훌륭한 아이디어이지만,
훗날에 어떤 바보 동료가 Registry 와 일반 HandlerInterceptor 개념을 헷갈려서,
Registry 안에 인터셉터로 다시 Registry 를 등록해버리는 일이 일어날 수도 있지 않을까라는 걱정이 순간 생겨서요.
그러면 무한참조 버그가 생길 수도 있지 않을까 라는 괜한 걱정이 문득 생겼습니다.
괜한 걱정일 수도 있으니깐, 그냥 본인 생각 말씀해주시면 됩니다. 😄
저는 동료가 할 수 있는 최대한의 멍청한 실수를 컴파일 타임에 잡아내는 걸 고민하고는 해서요.
참고로 구조와 관련해서는 저는 지금과 같은 구조가 훨씬 간결하고 멋지다고 생각합니다.
Composite 디자인 패턴이 떠오르네요.
There was a problem hiding this comment.
네 맞습니다!
Registry를 Composite으로 보고 Component를
HandlerInterceptor로 생각하여 컴포짓 패턴을 적용해 보았는데요.
정완님의 의견에 동의합니다
동료의 실수가 아무리 바보같은 실수이더라도 그러한 실수의 빌미를 제공한 사람의 책임도 크다는 생각이 드네요.
정완님의 피드백대로 굳이 Interceptor 구현체들과 Registry를 동일하게 취급해야할까 라는 생각도 듭니다.
학습목적으로 다양한 디자인 패턴을 적용 해보고자 해서 컴포짓 패턴을 사용해 보았는데 다소 적합하지 않은 상황임에도 불구하고 특정한 디자인패턴에 종속적으로 코드를 작성하였던건 아닌지 생각해보았습니다.
Registry를 일급 컬렉션으로 보고 HandlerInterceptor와는 분리하는 방식으로 수정해 보도록 하겠습니다!
ghojeong
left a comment
ghojeong
left a comment
There was a problem hiding this comment.
마지막까지 훌륭하게 피드백을 반영해주셔서 감사합니다.
함께 jdbc 미션을 진행할 수 있어서 저도 즐거웠습니다. 🙇
2 participants
안녕하세요 정완님!
스프링의
HandlerInterceptor의 동작 과정에 대해서 이해하고 이를 비슷하게 구현하고자 노력했습니다.Interceptor를 구현하는것도 어려웠지만 멀티 쓰레드 환경에서 어떻게 Thread-safe 하게 속도를 측정을 할 수 있을지 고민을 많이 해볼 수 있었던 미션이었던것 같습니다.
마지막 단계도 다양한 의견 주시면 감사하겠습니다! 🙇♂️
그럼 즐거운 추석연휴 보내시길 바라겠습니다 🌕 🐇