[2단계 - Tomcat 구현하기] 짱수(장혁수) 미션 제출합니다. #602
Conversation
- BufferedReader.readLine() 사용부 제거
| package org.apache.coyote.http11.exception; | ||
|
|
||
| public class NotCompleteResponseException extends RuntimeException { | ||
| public NotCompleteResponseException(String message) { |
There was a problem hiding this comment.
클래스명이 되게 상세한데, 이렇게 여러 Exception을 분리한 이유, 기준이 궁금합니다.
There was a problem hiding this comment.
Exception 을 하나로 두지 않고 지금처럼 여러개의 Exception 을 둔 것은 단순한 취향이었던 것 같아요.
아직까지 필요한 Exception 이 많지 않기 때문에 하나의 Exception 을 두고 예외 메시지로 구분하는 것 보다 각각의 명확한 Exception 을 사용하고 싶었어요.
| if (request.getTarget().equals("/")) { | ||
| return Response.builder() | ||
| .versionOf(request.getHttpVersion()) | ||
| .ofStaticResource(new StaticResource("/index.html")) |
There was a problem hiding this comment.
root 요청이 여기오기도전에 에러터지네요. 한번 확인 해봐야할 것 같아요
There was a problem hiding this comment.
static resource 요청을 판단할 때 문제가 있었네용..ㅎㅎ
수정 했습니다!
| .toHttpMessage(); | ||
| } | ||
| if (request.getTarget().contains("login")) { | ||
| return loginResponse(request); |
There was a problem hiding this comment.
contains로 분기해도 괜찮을까요?
localhost:8080/registerlogin 해도 login 페이지가 나오고있어요!
There was a problem hiding this comment.
오! 이젠 로그인 요청이 POST 로 변경되어서 쿼리 파라미터가 포함된 요청이 존재하지 않는군요!
equals() 로 비교하도록 변경하였습니다!
| } | ||
|
|
||
| private Response login(Request request) throws NoSuchUserException { | ||
| QueryParameters queryParams = QueryParameters.parseFrom(request.getBody()); |
There was a problem hiding this comment.
request가 Post일 경우 해당 메서드로 들어오네요!
Post인데 아직 queryParameter를 사용하고있나? 라는 생각이드네요. QueryParameters라는 표현은 URL에서 오는 표현이라고 생각합니다.
There was a problem hiding this comment.
오, 그렇네요!
"Query" -> "Request" 로 변경해서 url 에서 오는 표현을 지웠습니다!
| assertThat(socket.output()).contains( | ||
| "HTTP/1.1 200 OK \r\n", | ||
| "Content-Type: text/html;charset=utf-8 \r\n", | ||
| "Content-Length: 5564 \r\n", |
There was a problem hiding this comment.
맥북에서는 해당 테스트가 성공으로 뜨는 것으로 알고있어요.
하지만 윈도우에서는 Cotent-Length가 다르답니다. 다른 테스트에서 한것처럼 운영체제와 관계없이 성공하도록 바꿔주세요!
There was a problem hiding this comment.
실제로 파일을 읽어서 Content-Length 의 값을 넣어주도록 변경했어요
| private final File file; | ||
|
|
||
| public StaticResource(String resourceName) throws IOException { | ||
| String path = "static" + resourceName; |
There was a problem hiding this comment.
resourceName은 기본적으로 "/" 앞에 붙이고 올 것 같아요.
이 변수는 Name보다는 경로를 의미하는 것 같은데, 어떻게 생각하시나요?
There was a problem hiding this comment.
앗 그렇네요!
resourcePath 라는 이름이 더 적절할 것 같아 수정했습니다!
| @@ -20,7 +20,7 @@ | |||
| <div class="card shadow-lg border-0 rounded-lg mt-5"> | |||
| <div class="card-header"><h3 class="text-center font-weight-light my-4">로그인</h3></div> | |||
| <div class="card-body"> | |||
| <form method="get" action="login"> | |||
| <form method="POST" action="login"> | |||
There was a problem hiding this comment.
index.html에서도 로그인 기능 생기면서 수정해야하는 부분이 생겼는데, 확인해주세요!
There was a problem hiding this comment.
index.html 의 주석처리된 부분을 찾긴 했는데요.
이 부분의 주석을 해제해도 로그인 버튼과 로그아웃 버튼이 한 번에 보인다는 점, 로그아웃 기능이 구현되어 있지 않아 의미 없는 버튼이라는 점 때문에 오히려 혼란을 가중시킬 것 같아요...ㅎㅎ
우선은 이 부분의 수정은 보류하고 넘어가겠습니다!
반가워요 도도!
이전 리뷰의 코멘트들 답변 달아두었습니다!
이번에도 리뷰 잘 부탁드립니다!
참고 사항
ResponseBuilderResponse객체를 가변객체로 두었을 때 예상치 못한 변경을 감지하기 어렵다는 것이 마음에 걸렸어요.그래서 이번 단계에서
Response객체에 빌더 패턴을 적용해 보았습니다.MethodHandler클래스 제거이전 단계에서 요구사항을 잘못 파악했어요. 그래서 API 응답을 위한
MethodHandler를 구현했었는데요.이 핸들러를 사용했을때의 장점도 존재하겠지만, 너무 생각할 것이 많아지는 것 같아 우선 제거하였습니다.
대신 응답 생성의 책임을
Response에게 위임하여 Handle 의 역할을 줄여 보았어요.세션 관련 객체 구현
세션 관련 객체에는 사용되지 않는 메서드도 존재합니다.
이는 LMS 에서 가이드 한 클래스를 최대한 그대로 구현하려고 했기 때문이에요.
이를 참고하여 리뷰 해 주시면 감사하겠습니다!!
3단계 리뷰에 대해
확인해 보니 제가 구현해 두었던
Request,Response객체가 3단계 요구사항이더라고요!딱히 3단계 요구사항을 신경쓰면서 구현한 것은 아닌데요.
그래서 두 객체와 관련된 리뷰는 해 주셔도, 안해주셔도 좋습니다!
이번 단계 완료 후 3단계를 다시 분리하여 리뷰 요청 드릴 계획이니 참고해 주시면 감사하겠습니다!