[Tomcat 구현하기 - 3단계] 짱수(장혁수) 미션 제출합니다. #673
Conversation
ehtjsv2
left a comment
ehtjsv2
left a comment
There was a problem hiding this comment.
짱수~ 어플리케이션 실행 시, 잘 수행되지 않는 것 같아요. 실행해서 확인 부탁드립니다!
| var tomcat = new Tomcat(); | ||
| RequestMapping requestMapping = new RequestMapping( | ||
| Map.of("/login", new LoginController()) | ||
| ); |
There was a problem hiding this comment.
이부분에서 논리적인 오류가 있는 것 같은데, 어플리케이션 실행 후 확인부탁드립니다~!
There was a problem hiding this comment.
앗, 설정을 위한 클래스를 따로 만들어 두고 그 설정을 안썼네요 ㅠ
반영했습니다!
| } | ||
|
|
||
| public boolean isStaticResourceRequest() { | ||
| return target.contains(".css") || |
There was a problem hiding this comment.
이 메서드 단위테스트는 잘 작성해주셨는데, 실제로는 application에는 사용되지 않는 메서드인 것 같네요.
| private final Socket connection; | ||
|
|
||
| public Http11Processor(Socket connection) { | ||
| public Http11Processor(ServletContainer servletContainer, Socket connection) { |
There was a problem hiding this comment.
ServeltContainer를 외부에서 주입받은 것이 흥미롭네요. Servelt Container만 갈아끼우면 제가 만든 것도 돌아가겠네요!?
There was a problem hiding this comment.
맞아요!
결국 ServletContainer 가 각 요청들에 대한 서블릿 매핑 정보를 알고 있을 것이라 생각했어요.
Http11Processor 은 Http 요청, 응답에 대한 큰 흐름에서의 처리를 담당할 뿐이지, 세부적인 로직은 servletContainer 에게 위임해야 할 것 같았습니다.
사용자가 어플리케이션 레벨에서 요청과 서블릿의 매핑 정보를 적절히 담은 ServletContainer 를 구성하여 Tomcat 에게 주입해 주게 됩니다!
| servletContainer.service(httpRequest, httpResponse); | ||
| } catch (Exception e) { | ||
| log.error("request 처리 실패 : ", e); | ||
| httpResponse.setStatusBadRequest(); |
There was a problem hiding this comment.
catch해서 response설정해준 것이 인상적이네요~ 지금은 모든 에러에 대해서 bad request인 건가요?
There was a problem hiding this comment.
그렇습니다!
애초에 학습에서 요구했던 요청들도 GET, POST 밖에 없고 요청 URL 도 매우 한정적인 상황이었어요.
Tomcat 의 역할을 학습하는 것에 집중하기 위해 주어진 요청 이외에는 모두 잘못된 요청으로 간주하였습니다.
| public class RequestMappingConfig { | ||
| public static RequestMapping getRequestMapping() { | ||
| var staticResourceController = new StaticResourceController(); | ||
| return new RequestMapping( |
There was a problem hiding this comment.
테스트용 메서드인가요? staticResource들 매핑하는 것이 확장성이 떨어져보입니다. /static/assets 애들은 부를 수가 없는 것 같아요
There was a problem hiding this comment.
앗, 테스트용 메서드는 아니었습니다!!
정규식 매칭으로 컨트롤러 조회 방법을 변경하였어요. 이제는 문제가 해결되었습니다!!
|
역할분리와 확장성 있는 코드에서 많이 배우고 갑니다!! 테스트도 충분히 있고 이제 4단계 진행해봅시다! |
3 participants
안녕하세요, 도도!
리팩토링을 끝내고 돌아왔습니다!!
테스트 코드가 조금 부족한 상황이에요.
전체 시나리오에 대한 테스트는 존재하는데, 각 객체들의 동작을 확인하는 테스트가 존재하지 않는데요.
해당 테스트 코드는 RC 반영과 함께 올려 두어도 괜찮을까요?
기타
HttpResponse객체가 가변 객체로 변경 되었습니다.