[1 - 2 단계 Tomcat 구현하기] 마크(김승현) 미션 제출합니다. #580
Conversation
- MimeType 열거형 추가 - 정적 URI를 못찾을 시 404 반환
- getStaticResource(HttpRequest request) -> getStaticResourceResponse(String requestPath)
- CacheControlInterceptor 추가
- ShallowEtagHeaderFilter 사용
- 캐시 max-age 1년 설정 - ETag 적용 - url에 버전 적용
- 요청 본문 파싱 로직 수정
- HttpCookie 객체 구현 및 요청, 응답 객체에 추가
jongmee
left a comment
jongmee
left a comment
There was a problem hiding this comment.
안녕하세요 마크 !! 깔끔하게 구현해주셔서 코드를 이해하기 쉬웠어요 👍 몇 가지만 리뷰 달아 놓았습니다 😊
| if (isInvalidJSession(jSession)) { | ||
| return HttpResponse.found("/login.html") | ||
| .setCookie(JSESSIONID, jSession).setCookie("Max-Age", "0") | ||
| .build(); |
There was a problem hiding this comment.
아이디를 임의로 바꿔도 접속이 가능한게 신경쓰이더라고요😄
| private HttpResponse register(HttpRequest request) { | ||
| Map<String, String> parameters = new HashMap<>(); | ||
| String body = request.getBody(); | ||
| for (String data : body.split("&")) { |
There was a problem hiding this comment.
미디어 타입이 application/x-www-form-urlencoded이면(html form 태그) body가 쿼리 파라미터와 같은 형식으로 가는데 현재 HttpRequest에 존재하는 parameter 필드에 담아둬도 좋을 것 같아요 !
There was a problem hiding this comment.
스프링도 저 MIME 타입을 쿼리 파라미터로 처리하는 것 같네요! 저도 사용하기도 쉽게 parameter로 관리해봐야겠어요. 놓치고있던 URL디코드도 같이 적용해볼게요.
| return new HttpResponse(500, "Internal Server Error", ""); | ||
| } | ||
|
|
||
| public String build() { |
There was a problem hiding this comment.
(의견) HttpRequestParser처럼 응답에 대해서도 parser가 있으면 파싱 로직과 기타 로직들을 분리할 수 있을 것 같아요 !
There was a problem hiding this comment.
어떤 값을 파싱 해야할지 잘 모르겠어요...! 우선 리팩토링 해보고 다시 제출해볼게요.
There was a problem hiding this comment.
빌더 패턴이 생각나서 build()메소드로 HttpResponse를 만들고 이 객체를 String으로 변환하는 util 클래스(HttpRequestParser의 역버전)를 만들면 어떨까 생각했습니다 ㅎㅎ! 개인적인 의견이니 다음 단계에서 선택적으로 반영해주세요
There was a problem hiding this comment.
확실히 OutStream.write 할 값을 반환하는 것은 HttpResponse 책임이�라 하기에 애매하네요! 방법� 잘 생각해서 적용해볼게요👍
| private static final String HEADER_SEPARATOR = ": "; | ||
|
|
||
| private int statusCode; | ||
| private String statusMessage; |
There was a problem hiding this comment.
상태 코드도 MediaType 처럼 enum으로 관리하면 모아서 보기 편할 것 같아요 !
| return type.mimeType; | ||
| } | ||
| } | ||
| return DEFAULT.mimeType; |
There was a problem hiding this comment.
디폴트도 지정해주셨군요 ! 배워갑니다
application/octet-stream is the default value for all other cases. An unknown file type should use this type.
- enum HttpStatusCode
|
리뷰 반영해주신거 모두 잘 확인했습니다 :) 더 디테일하고 깔끔해졌네요 ! 다음 단계에서 봬요 😊 |
3 participants
안녕하세요 미아~ 이번 Tomcat 구현하기 미션 구현하기 리뷰이 마크입니다.
HttpRequest, HttpResponse 클래스는 먼저 구현하여 사용하고 있었어요.
제목대로 2단계 까지 구현이라 Controller 인터페이스는 추가하지 못했어요🥲 잘부탁드려요~!