[톰캣 구현하기 - 3, 4단계] 테오(최우성) 미션 제출합니다. #410
Conversation
`Request` -> `HttpRequest`, `Response` -> `HttpResponse`
서블릿은 비즈니스 영역의 객체이므로 jwp 하위로 이동
| public void service(HttpRequest httpRequest, HttpResponse httpResponse) throws IOException { | ||
| HttpMethod httpMethod = httpRequest.getMethod(); | ||
| if (httpMethod.isEqualTo(HttpMethod.GET)) { | ||
| doGet(httpRequest, httpResponse); | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 ~Servlet 클래스들이 Servlet 클래스를 상속받아 GET, POST 맵핑해주는 해당 메서드가 계속 반복되는 걸로 보여요 !
컨트롤러 인터페이스를 추가하고 각 분기에 있는 로직마다
AbstractController를 상속한 구현체로 만들어보자.
LMS에 나와 있는 것처럼 테오의 코드로 말하면 AbstractServlet 클래스를 구현하고 각 Servlet들이 AbstractServlet 클래스를 상속하도록 한다면 해당 로직의 중복을 제거할 수 있을 것 같은데 어떻게 생각하시나요 ?!
There was a problem hiding this comment.
맞습니다!
각 서블릿마다 공통부분(HTTP Request Method에 따른 분기 로직)이 있는데,
요걸 제가 인지하지 못했었네요 🥲
빠르게 수정하도록 하겠습니다 감사합니다!
| httpResponse.setHttpVersion(httpRequest.getHttpVersion()) | ||
| .setHttpStatus(HttpStatus.OK) | ||
| .addHeader(CONTENT_TYPE, ContentType.parse(absolutePath)) | ||
| .addHeader(CONTENT_LENGTH, String.valueOf(content.getBytes().length)) | ||
| .setResponseBody(responseBody); |
| Headers requestHeaders = httpRequest.getHeaders(); | ||
| Cookie cookie = requestHeaders.getCookie(); | ||
|
|
||
| if (cookie.hasKey("JSESSIONID")) { |
There was a problem hiding this comment.
JSESSIONID 도 상수화 할 수 있을 것 같네요 ˙ᵕ˙
| responseForLoggedIn(httpRequest, httpResponse); | ||
| return; | ||
| } | ||
| responseForNotLoggedIn(httpRequest, httpResponse); |
|
|
||
| private static final int MIN_PORT = 1; | ||
| private static final int MAX_PORT = 65535; | ||
| private static final int DEFAULT_PORT = 8080; | ||
|
|
||
| private static final int DEFAULT_ACCEPT_COUNT = 100; |
There was a problem hiding this comment.
중간중간 불필요한 개행은 제거해도 괜찮을 것 같아요 ˙ᵕ˙
| try (final InputStream inputStream = connection.getInputStream(); | ||
| final OutputStream outputStream = connection.getOutputStream()) { |
There was a problem hiding this comment.
요기는 final이 붙은 이유가 있을까요 ?!
테오는 어떤 기준으로 final을 사용하고 계시나요 ?!
There was a problem hiding this comment.
원래 파라미터나 변수에 final을 사용하지 않는 편이라 코드 컨벤션 맞춘다고 final을 다 제거하긴 했었는데,
저 부분만 빼먹었던 것 같네요..! 🥲 감사합니다!
| LoggingFilter.logUserInfoIfExists(request); | ||
| Response response = Handlers.handle(request); | ||
| HttpRequest httpRequest = RequestExtractor.extract(inputStream); | ||
| LoggingFilter.logUserInfoIfExists(httpRequest); |
| .map(each -> each.split(": ")) | ||
| .forEach(each -> headers.put(each[0], each[1])); | ||
|
|
||
| return new Headers(headers); | ||
| } | ||
|
|
||
| private static RequestBody extractBodyIfExists(BufferedReader reader, |
There was a problem hiding this comment.
여기 밑에 "Content-Length" 부분 HttpHeaders.CONTENT_LENGTH로 바꿀 수 있을 거 같네요 !
| return Arrays.stream(content.split("&")) | ||
| .map(each -> each.split("=")) | ||
| .map(each -> Map.entry(each[0], each[1])) |
There was a problem hiding this comment.
다른 부분은 다 상수화되어 있는데 이 부분만 안 되어 있어서요 ! 이유가 있을까요 ?!
There was a problem hiding this comment.
요거는 RequestBody가 여러 Content-Type에 대해 재사용될 수 있기를 의도한건데요!
현재는 application/x-www-form-urlencoded 의 경우만 Content-Type으로 들어오므로 상관이 없지만..
추후 여러 Content-Type이 들어오는 경우 메소드만 추가하면 되도록 위처럼 구현했는데요!
따라서 상수처리가 애매했던 것 같아요 🥲 공통부분이 아니니 밖으로 빼기가 그렇더라구요.
| public HttpResponse setHttpVersion(HttpVersion httpVersion) { | ||
| this.httpVersion = httpVersion; | ||
| return this; | ||
| } | ||
|
|
||
| public HttpResponse setHttpStatus(HttpStatus httpStatus) { | ||
| this.httpStatus = httpStatus; | ||
| return this; | ||
| } | ||
|
|
||
| public HttpResponse addHeader(HttpHeaders key, String value) { | ||
| headers.addHeader(key, value); | ||
| return this; | ||
| } | ||
|
|
||
| public HttpResponse setResponseBody(ResponseBody responseBody) { | ||
| this.responseBody = responseBody; | ||
| return this; |
|
Kudos, SonarCloud Quality Gate passed!
|
| private void shutdownAndAwaitTermination(ExecutorService executorService) { | ||
| executorService.shutdown(); | ||
| try { | ||
| if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) { | ||
| executorService.shutdownNow(); | ||
| if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) { | ||
| log.error("Pool did not terminate"); | ||
| } | ||
| } | ||
| } catch (InterruptedException ie) { | ||
| executorService.shutdownNow(); | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| } |
There was a problem hiding this comment.
오 저도 덕분에 graceful shutdown에 대해 알게되었어요 !! 👍
안녕하세요 토리 반갑습니다~
step3, step4 모두 완료하고 제출합니다!
이왕 만드는김에 Tomcat의 구조를 모방하고 싶어서 이것저것 시도하느라 시간을 다 보낸 것 같습니다 🥲
그리고 이번에는 미션 내용 이외에도 개인적으로 추가한 기능이 있는데요!
찾아보니 Tomcat의 경우 server.xml 파일을 통해 대부분의 설정을 해주더라구요.
현재 저희가 학습하고 있는 내용인 acceptCount, maxThreads, maxConnections 모두 server.xml 파일을 통해 설정이 되는 것 같아요. (참고)
그래서 비슷하게 server.xml에 설정 값을 입력하면 자동으로 파싱해서 반영하도록 기능을 구현해봤는데, 처음 보시면 난해하실 수도 있을 것 같아 이 부분은 리뷰하시기 전에 미리 설명드립니다!