[톰캣 구현하기 - 1,2단계] 제이(이재윤) 미션 제출합니다. #324
Conversation
|
|
||
| User user; | ||
| try { | ||
| user = userService.login(params.get("account"), params.get("password")); |
There was a problem hiding this comment.
params 에 account 와 password 가 없고 다른 값이 들어오면 예상치 못한 상황이 발생할 것 같아요
There was a problem hiding this comment.
Map이라서 null 값이 나오겠네요!
해당 경우를 처리하기 위해서 if문 분기를 하나 두고 badRequest를 내려주도록 변경했습니다.
(추후에 3단계 리팩토링하면서 모두 나누겠습니다 ㅠ.ㅠ)
| } | ||
| } | ||
|
|
||
| private HttpResponse getResponse(final HttpRequest httpRequest) throws URISyntaxException, IOException, NotFoundException { |
There was a problem hiding this comment.
HttpResponse 객체로 분리를 너무 깔끔하게 해주신 것 같아요!!! 👍👍👍
이 부분에 제이에 대한 생각이 궁금해서 코멘트를 남깁니다.
HttpResponse 를 정적팩토리 메소드를 통해서 생성하고 index.html, login.html 등등... 를 직접 Processor 에서 넣어주고 있습니다.
이때 해당 "xxx.html" 를 관리하는 역할은 어떤 객체가 가져야 된다고 생각하시나요?
There was a problem hiding this comment.
일단 자원을 반환하는 것은 HttpResponse가 하는 것이 맞다고 생각합니다!
하지만 지금 저 상태에서는 요청이 늘어난다면 if문이 더 범벅이 되고 말씀하신 반환 부분을 모두 지저분하게 한 곳에서 관리할 것 같습니다.
따라서 handler를 추가하고, 추후에 여기서 요청 별 반환 자원을 관리를 해주는 것이 좋다고 생각합니다.
tomcat/src/main/java/org/apache/coyote/http11/Http11Processor.java
Outdated
Show resolved
Hide resolved
|
|
||
| Session session = httpRequest.createSession(); | ||
| session.setAttribute("user", user); | ||
| HttpResponse response = HttpResponse.ok(ContentType.HTML, Status.FOUND, Map.of(Header.LOCATION, "/index.html"), ""); |
There was a problem hiding this comment.
HttpResponse.ok()
라서 ok 인가 라고 생각했지만, Status.FOUND 를 보고 조금 혼란이 생겼던 것 같아요.
ok 말고 좀 더 명확한 메소드명은 어떠신가요?
There was a problem hiding this comment.
추가하겠습니다!
이 부분도 리팩토링하면서 Builder 패턴으로 변경해보도록 하겠습니다 ㅠㅠ 보시느라 고생하셨습니다
tomcat/src/main/java/org/apache/coyote/http11/cookie/Cookie.java
Outdated
Show resolved
Hide resolved
| public static HttpRequest from(final BufferedReader reader) throws IOException { | ||
| String rawRequestLine = Objects.requireNonNull(reader.readLine()); | ||
| Uri uri = Uri.from(rawRequestLine); | ||
|
|
||
| List<String> headersRead = readHeader(reader); | ||
| Headers headers = Headers.from(getHeaders(headersRead)); | ||
|
|
||
| Optional<String> contentLength = headers.getHeader(Header.CONTENT_LENGTH); | ||
| if (contentLength.isPresent()) { | ||
| String rawContentLength = contentLength.get(); | ||
| String body = getBody(reader, Integer.parseInt(rawContentLength.trim())); | ||
| return getHttpRequest(uri, headers, Params.from(body)); | ||
| } | ||
|
|
||
| return getHttpRequest(uri, headers, Params.createEmpty()); | ||
| } |
There was a problem hiding this comment.
제이의 생각이 궁금한 부분입니다!!
HttpRequest 객체를 생성하기 위해 BufferedReader 를 받아와서 사용하고 있는 모습을 볼 수 있는데요?
이 부분이 궁금합니다!!
There was a problem hiding this comment.
InputStream은 1byte씩 읽고, 아스키 코드로 반환됩니다!
반면에 InputStream은 char로 받아서 문자 데이터를 읽는데 특화 됐고, 인코딩을 적용할 수 있어서 사용했습니다!
마찬가지의 이유로 OutputStream도 BufferedWriter로 변경하였습니다 :)
tomcat/src/main/java/org/apache/coyote/http11/request/HttpRequest.java
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
우가 안녕하세요?
이번 미션은 개인적으로 난해하고 너무 어려웠습니다.
아직 Handler를 추가하지 않아서 코드가 if문 범벅인데요.
마음 같아서는 싹 고치고 싶은데 3단계 미션에서 리팩토링을 진행하기에 일단 이대로 제출합니다.
구조를 나누지 않아서 코드 보시기 힘드실텐데 미리 사과의 말씀 드리고 시작하겠습니다 ㅠㅠ 이점 유의해서 리뷰해주세요 :)
오늘도 좋은 하루 보내세요!