[톰캣 구현하기 - 3, 4단계] 망고(고재철) 미션 제출합니다. #460
Conversation
tomcat/src/main/java/org/apache/coyote/http11/response/HttpResponse.java
Show resolved
Hide resolved
| OK(200, "OK"), | ||
| FOUND(302, "Found"), |
There was a problem hiding this comment.
저희 요구사항을 이 두 응답 코드로 모두 정의할 수 있을까요??
There was a problem hiding this comment.
로그인에 실패하면 401.html로 리다이렉트한다.
라고 되어있어서 401 코드를 반환하기보다는 401.html이라는 페이지로 리다이렉트 하는 게 요구사항이라고 생각해서, 302 코드를 사용해 리다이렉트했습니다.
현재 요구사항에 없는 응답 코드까지 전부 추가하기에는 너무 많다고 생각해서 실제 사용하는 코드만 추가해두었습니다!
| private static final String CONTENT_TYPE = "Content-Type"; | ||
| private static final String FORM_CONTENT_TYPE = "application/x-www-form-urlencoded"; |
There was a problem hiding this comment.
HttpHeader는 앞으로 더 많아질 것 같은데요.
혹시 그렇다면 어떻게 많은 Header를 관리할 수 있을까요?
저는 일단은 enum으로 관리하고 있답니다!
그리고 혹시 Header에도 Reuqest에만 사용되는 Header, Response에만 사용되는 Header 그리고 둘 다 사용되고 바디의 메타데이터를 나타내는 Header가 있다고 하더라구요!
https://developer.mozilla.org/ko/docs/Glossary/HTTP_header
RequestHeader와 ResponseHeader에서 적절한 Header타입을 키값으로 갖는 자료구조를 어떻게 만들 수 있을까요??
저도 한번 생각해보겠습니다
There was a problem hiding this comment.
지금은 상수로 사용하고 있지만 많아지면 enum으로 관리하는 것도 한 가지 방법인 것 같아요!
Request용 Header인지 Response용인지는 둘 다 사용되는 경우가 있기 때문에, 지금 떠오르는 방식들은 오히려 더 복잡할 수 있을 것 같다는 생각이 들어요...! 🥲
|
|
||
| public class RequestLine { | ||
|
|
||
| private final String method; |
There was a problem hiding this comment.
이 값은 enum과 같은 값으로 상수로서 관리되는 것이 좋지 않을까요?
| public static ResponseBody fromUri(final String uri) throws IOException { | ||
| final URL fileUrl = findResourceUrl(uri); | ||
| final String filePath = fileUrl.getPath(); | ||
| final String body = Files.readString(new File(filePath).toPath()); | ||
| return new ResponseBody(body); | ||
| } |
There was a problem hiding this comment.
IO가 되게 깊은 곳까지 침투했네요!
이 과정이 단순히 어떤 정적 파일을 리스폰스 바디에 쓰는 과정일 수도 있을 것 같고 조금 찾아보니까 forward라는 것이 있더라구요.
https://youngjinmo.github.io/2020/08/servlet-redirect-forward/
정적 파일을 리스폰스에 쓴다는 과정으로 볼수도 있지만, StaticController로 forward 한다고 볼 수도 있을 것 같아요!!
저도 현재 망고 같은 구조로 정적 자료를 Response에서 직접 쓰는 구조를 가지고 있지만, 정적 컨트롤러로 요청 응답을 전달해서 파일을 쓰는 과정을 그곳에서 타게하면 어떨까 싶습니다!!
망고 생각은 어떠세요??
There was a problem hiding this comment.
ResponseBody라는 객체가 실제 Response Body에 쓰여야 하는 내용을 가져야한다고 생각해서 위와 같이 구현했습니다!
그렇기 때문에 text가 body에 쓰여지거나 정적 페이지 내용이 쓰여지거나 전부 ResponseBody 객체의 역할이라고 생각해서 ResponseBody에게 책임을 넘겼습니다.
| protected Map<String, String> getRequestParameters(final HttpRequest request) { | ||
| final String[] uri = request.getRequestLine().getPath().split("\\?"); | ||
| final String requestBody = request.getRequestBody(); | ||
| if (requestBody == null) { | ||
| return parseRequestBody(uri[1]); | ||
| } | ||
| return parseRequestBody(requestBody); | ||
| } | ||
|
|
||
| protected Map<String, String> parseRequestBody(final String requestBody) { | ||
| final var requestBodyValues = new HashMap<String, String>(); | ||
| final String[] splitRequestBody = requestBody.split("&"); | ||
| for (var value : splitRequestBody) { | ||
| final String[] splitValue = value.split("="); | ||
| requestBodyValues.put(splitValue[0], splitValue[1]); | ||
| } | ||
| return requestBodyValues; | ||
| } |
There was a problem hiding this comment.
QueryString이나 RequestBody에 담겨오는 RequestParameter를 유저가 구현하는 컨트롤러에서 요청을 인지하고 부모 추상 클래스에 있는 메서드를 호출해서 사용하는 것이 약간은 사용적인 측면에서 어려울 수 있을 것 같다는 생각이 듭니다!!
그리고 또한 이러한 HTTP요청이 어떤 데이터를 담고있는지를 분석하는 것이 HttpRequest 깩체의 역할이라고 생각해볼수도 있지 않을까요?
There was a problem hiding this comment.
루카 의견을 듣고나니 HttpRequest 객체의 역할로 두는 게 더 어울리는 것 같아요!
수정했슴다~~
| @Override | ||
| protected void doPost(final HttpRequest request, final HttpResponse response) { | ||
| throw new HttpRequestException(HTTP_METHOD_EXCEPTION_MESSAGE); | ||
| } | ||
|
|
There was a problem hiding this comment.
커스텀 예외 👍
저희 요구사항에서는 나오지는 않았지만,
보통 /의 요청을 지원하지 않는 Http Method로 요청이 오게되면 405 Method Not Allowed 응답이 나가게 되는데,
그것에 대해서는 어떻게 생각하세요?
There was a problem hiding this comment.
HomeController와 ResourceController에서는 POST 요청을 허용하지 않기 때문에 405 코드를 response로 반환하도록 수정했습니다!
| response.addResponseHeader("Content-Type", TEXT_HTML); | ||
| response.addResponseHeader("Content-Length", String.valueOf(responseBody.getBody().getBytes().length)); |
There was a problem hiding this comment.
위에 헤더에 대해서 말했던 것에 이어서,
Content-Type과 Content-Length는 HTTP 바디에 메타 데이터를 나타내는 헤더라고 할 수 있을 것 같아요!!
그럼 위 두 해더는 따로 유저가 임의적 판단에 의해서 넣어줄 수도 있겠지만, 바디가 세팅된다거나 Request가 와서 Accept 헤더를 분석하는 과정에서 이 헤더들이 세팅될 수 있지 않을까요? 그 일은 유저가 직접 로직을 구현하는 이 영역보다는 다른 영역에서 이루어질 수 있지 않을까요?
There was a problem hiding this comment.
각 컨트롤러의 메서드마다 응답하는 헤더가 달라질 수 있을 것 같다고 생각해서 이 부분은 어떻게 해야할지 아직은 잘 모르겠어요..
어떤 상황에는 어떤 헤더가 무조건 있다..! 라는 걸 생각해서 템플릿같은 걸 만들 수 있나요....??
그래서 이 부분은 직접 필요한 헤더를 추가하는 방식으로 구현해두었습니다//...!! 🥲
There was a problem hiding this comment.
저같은 경우에는 Content-Length는 body가 결정되는 순간 결정되는 헤더이므로 바디를 셋 해주는 메서드에 포함시켰습니다.
그리구 Content-Type은 어떤 service로 들어가기 전에 전처리하는 과정에서 요청의 Accept 헤더를 분석해서 셋해주는 과정으로 처리했습니다! 그 뒤에 유저가 알아서 변경은 할 수도 안할수도 있겠지요?
특정 메타 데이터는 유저가 억지로 변경하지 않는 이상 약간은 자동으로(?) 세팅되는 과정이 Response 객체에 포함되어있으면 좋을 것 같습니다!!
| final String cookieHeader = request.getRequestHeader().getValue(HEADER_COOKIE); | ||
| final var cookie = HttpCookie.from(cookieHeader); |
There was a problem hiding this comment.
https://developer.mozilla.org/ko/docs/Web/HTTP/Cookies
Cookie는 HTTP 통신을 하면서 서버가 브라우저에 작은 스트링 데이터를 저장하기 위해서 사용하는 방식이라고 알고 있습니다.
또 만약 브라우저에 저장되어있는 쿠키가 있다면, 그것을 헤더에 담아서 오는 것이구요.
그렇다면 꽤나 일반적으로 Request에 있을 내용일 수 있을 것 같습니다.
또한 Response로 브라우저에게 저장을 원하는 정보를 담아서 넘겨줄 수 있을 것 같아요!!
이렇게 빈번하고 일반적으로 일어나는 일에 대해서 각각의 객체에게 책임을 부여할 수 있지 않을까요?
There was a problem hiding this comment.
Cookie 값을 반환하는 책임을 HttpRequest 객체한테 넘겨주도록 수정했습니다!
| final Session session = SESSION_MANAGER.findSession(sessionId) | ||
| .orElseGet(Session::create); | ||
| return (User) session.getAttribute("user"); |
There was a problem hiding this comment.
Session은 위에서 언급한 Cookie와 다른 역할을 하고 있지만, 무상태성인 HTTP 통신을 하다가 서버가 특정 브라우저(클라이언트) 임을 알기 위해서 저장하고 그에 대한 아이디를 쿠키에 넘겨주는 형식으로 사용이 되는데요.
Cookie와 마찬가지로 어떤 요청이 오면 그 클라이언트가 Session이 이미 저장이 되어있는 유저라면 Cookie 헤더의 SessionId를 통해서 알 수 있을거에요.
그렇다면 이 책임 또한 Request같은 객체로 옮길 수 있지 않을까요?
만약 그렇다면 어떤 방식으로 Response에게 Set-Cookie 헤더를 명시하라고 지시할 수 있을까요?
There was a problem hiding this comment.
사실 유저 입장에서는 Session Manager가 무엇인지 인지하거나 그것을 통해서 세션을 저장하는 행위가 매우 어려울 수 있다고 생각합니다.
우리 톰캣을 사용하는 유저는
- 요청과 매핑된 컨트롤러를 구현하고
- 그 컨트롤러의 로직을 구현하고
- 그 때 파라미터로 넘어온 request와 response를 조작해서 요청을 분석하고 응답을 설정한다
위에서 나온 객체들로 거의 대부분의 일을 해내야될 수 있다고 생각합니다.
There was a problem hiding this comment.
Session이 이미 저장이 되어있는 유저라면 Cookie 헤더의 SessionId를 통해서 알 수 있을거에요.
이 부분은 위 피드백을 반영하면서 Cookie 값을 가져오는 책임을 HttpRequest 객체로 옮겨주는 방식으로 수정했습니다.
상위 객체가 너무 많은 책임을 가지기보다는 각각 객체마다 가장 어울리는 책임을 담당하도록 하는게 좋다고 생각해서, JSESSIONID를 받아오는 책임은 HttpCookie에게 그대로 두도록 했습니다.
이 부분도 HttpRequest 객체에서 바로 받아오도록 수정하는 게 더 어울릴까요??
There was a problem hiding this comment.
제 생각은 Session과 Cookie의 값들은 어떤 요청에 의한 값이므로 그에 대한 접근 정보 권한도 Request에 있어야된다고 생각했습니다!!
그래서 유저가 Request의 어떤 값으로 Session 저장소에서 Session이 어떤 컬렉션에 담겨있을테니 어떻게 가져와야지 이런식으로 복잡한 과정을 생각하는 것 보다는
Session Cookie RequestParameter같은 값은 손쉽게 Request에서 값을 뽑아낼 수 있어야된다고 생각했고 Request가 저 위에 값들을 필드로 가지고 있어야한다고 생각했습니다!
| private static final Map<String, Controller> mappers = new HashMap<>(); | ||
|
|
||
| static { | ||
| mappers.put("/", new HomeController()); | ||
| mappers.put("/login", new LoginController()); | ||
| mappers.put("/register", new RegisterController()); | ||
| } |
There was a problem hiding this comment.
저는 이 부분이 망고가 구현한 영역에서 의존성 순환이 생긴 상황처럼 보여지는데요.
이에 대한 인터페이스(Controller)와 그에 대한 구현체들이 전부 jwp (유저의 영역)에 구현이되어있는데요.
전체 의종성 흐름도를 그려보지 않고 이 부분만 살펴 봐도 Controller(jwp에 있는 인터페이스)에선 Request와 Resposne(koyote)를 의존하고 있는데요.
이 매핑(koyote에 있는 객체)는 그대로 Controller(jwp)와 연관관계를 형성하고 있습니다.
이렇다는것은 jwp와 koyote와의 의존성이 순환이되고 있다는 뜻인데요.
의미적으로 봤을 대 이 리퀘스트 매핑 정보를 저장하는 객체는 koyote같이 우리가 톰캣을 만드는데 필요한 객체를 구성하는 역할을 하는 곳인데,
그곳에 직접적으로 유저가 구현한 컨트롤러와 그에 대한 요청 매핑을 하는 스태틱 블럭이 있는 것이 적절할까요?
만약 jwp에서 구현을 하고 있던 유저가 또 다른 요청을 추가하려면 이 koyote까지 와서 정보를 바꿔야 하는 것인데요.
이를 어떻게 해결할 수 있을까요?
There was a problem hiding this comment.
의존성... 어렵네요...
제가 이해한 바로는 그렇다면 Controller 인터페이스는 coyote에 두고, Controller 구현체는 jwp에 두고, RequestMapping도 jwp에 두도록 수정하는 게 맞을까요...?
의존성.. 패키지 구조.. 국어 지문처럼 사람마다 해석하는 게 다를 수 있다고 생각해서 너무 어려워요 😢😢😢
There was a problem hiding this comment.
이거는 내일이나 언제 캠퍼스에서 같이 한번 이야기 해보아요!!
There was a problem hiding this comment.
제가 그린 구조는 이렇습니다!!
RequestMapping은 coyote 부분에 있어야 Processor 같은 객체가 사용하기 좋을 것 같습니다!
하지만 그것을 등록하는 과정을 coyote에서 해버리면 톰캣이 톰캣 사용자의 구현체를 알게되는 것이니까 약간 이상함이 생기겠죵?
|
|
||
| public interface Controller { | ||
|
|
||
| void service(final HttpRequest request, final HttpResponse httpResponse) throws IOException; |
There was a problem hiding this comment.
파라미터 네이밍을 httpRequest나 response로 통일 시키면 좋을 것 가타요!
There was a problem hiding this comment.
요구사항 템플릿 그대로 했더니..!
수정했습니당~
| @@ -0,0 +1,11 @@ | |||
| package nextstep.jwp.controller; | |||
There was a problem hiding this comment.
위 의존성 순환에 대한 언급을 했던 것에 연장선일 수 있는데요.
이 Controller는 인터페이스인데요.
이 인터페이스는 Tomcat을 개발하는 우리가 이 톰캣을 사용해서 웹 개발을 할 유저에게 어떤 요청을 핸들링할 때는 이 컨트롤러와 Abstract컨트롤러를 구현해서 사용하라는 일종의 설명서 같은 것일텐데요.
그렇다는 것은 jwp(유저의 영역)와 koyote(톰캣 영역) 둘중 어디에 있는 것이 적절할까요?
There was a problem hiding this comment.
의존성... 어렵네요...
제가 이해한 바로는 그렇다면 Controller 인터페이스는 coyote에 두고, Controller 구현체는 jwp에 두고, RequestMapping도 jwp에 두도록 수정하는 게 맞을까요...?
의존성.. 패키지 구조.. 국어 지문처럼 사람마다 해석하는 게 다를 수 있다고 생각해서 너무 어려워요 😢😢😢
|
Kudos, SonarCloud Quality Gate passed!
|
안녕하세요 루카!!
1, 2단계에 이어서 3, 4단계도 제출합니다 😎
이전 단계 때 하고 싶었던 리팩터링도 하고 테스트들도 만들고 하니까 변경사항이 많아졌네요 하하,,
리팩터링을 거치면서 전보다는 코드가 깔끔해진 것 같은데 패키지 구조는 어떻게 하는 게 좋을지 잘 모르겠어요..
그리고 스레드 관련 테스트는 어떻게 작성해야 할지 고민하다가 잘 모르겠어서 아직 없습니다 🥲
위 두 가지 내용에 대한 루카의 의견을 들어볼 수 있을까요..!
이번에도 리뷰 잘 부탁드립니다 🙇♂️