[1 -2단계 - Tomcat 구현하기] 아톰(이하늘) 미션 제출합니다. #531

le2sky · 2024-09-06T07:32:38Z

초코칩 안녕하세요. 아톰입니다.
Tomcat 구현하기 1-2단계를 구현했어요. AbstractHandler에 핸들러 공통 코드를 작성했고, 각 핸들러들이 상속받도록 처리했어요.
어색한 부분이 있다면, 편하게 말씀 주세요. 리뷰 잘 부탁 드립니다. 🙇🏻‍♂️

Chocochip101

안녕하세요, 아톰! 리뷰를 맡게 된 초코칩입니다 :)

구현을 잘해주셨고, 코드도 인상적이었습니다. 👍🏻

사소한 리뷰를 남겼으니 확인 부탁드립니다.

Chocochip101 · 2024-09-06T09:53:39Z

tomcat/src/main/java/org/apache/coyote/http11/HttpRequest.java

+
+import java.net.URI;
+
+public record HttpRequest(String startLine, Header header, QueryParameter body) {


body라는 인자명에 QueryParameter 자료형인 것이 어색하게 느껴지네요.

public record HttpRequest(String startLine, Header header, char[] body) { public HttpMethod getMethod() { return HttpMethod.from(startLine.split(" ")[0]); } public URI getUri() { return URI.create(startLine.split(" ")[1]); } public HttpVersion getHttpVersion() { return HttpVersion.from(startLine.split(" ")[2]); } public QueryParameter getQueryParameter() { return new QueryParameter(getUri().getQuery()); } public boolean hasNotApplicationXW3FormUrlEncodedBody() { String requestContentType = header.get(HttpHeaderKey.CONTENT_TYPE) .orElse(ContentType.PLAIN.getName()); ContentType contentType = ContentType.from(requestContentType); return !contentType.isApplicationXW3FormUrlEncoded(); } }

현재는 위와 같이 바꿔봤어요. body를 char[]로 변경하고 hasNotApplicationXW3FormUrlEncodedBody 메서드를 제공하여
클라이언트 개발자가 hasNotApplicationXW3FormUrlEncodedBody가 true인 경우에 QueryParamter를 직접 생성하여 사용하는 방식으로 바꿨어요. 아직 어색한 부분이 있다면, 추가 피드백 부탁드립니다. 🙇🏻‍♂️

tomcat/src/main/java/org/apache/coyote/http11/HttpResponse.java

tomcat/src/main/java/org/apache/coyote/http11/HttpMethod.java

Chocochip101 · 2024-09-06T10:10:34Z

tomcat/src/main/java/org/apache/coyote/http11/Http11Processor.java

+    private HttpResponse respondResource(HttpRequest httpRequest) throws IOException {
+        AbstractHandler helloHandler = new HelloHandler();
+        AbstractHandler staticResourceHandler = new StaticResourceHandler();
+        AbstractHandler postLoginHandler = new PostLoginHandler();
+        AbstractHandler postRegisterHandler = new PostRegisterHandler();
+        AbstractHandler getLoginHandler = new GetLoginHandler();
+        AbstractHandler getRegisterHandler = new GetRegisterHandler();
+        AbstractHandler notFoundHandler = new NotFoundHandler();
+
+        List<AbstractHandler> handlers = List.of(
+                helloHandler,
+                staticResourceHandler,
+                postLoginHandler,
+                postRegisterHandler,
+                getLoginHandler,
+                getRegisterHandler
+        );
+        AbstractHandler targetHandler = handlers.stream()
+                .filter(it -> it.canHandle(httpRequest))
+                .findFirst()
+                .orElse(notFoundHandler);
+
+        return targetHandler.handle(httpRequest, sessionManager);
+    }


[질문]
톰캣 코드를 확인해보면 HttpResponse 객체를 미리 생성해서 참조를 끊지 않고 setter로 값을 주입하고 있어요.
아래 코드와 비슷하게요.

private void respondResource(HttpRequest httpRequest, HttpReponse httpResponse) throws IOException { // ... targetHandler.handle(httpRequest, httpResponse, sessionManager); return; }

저도 아톰과 비슷한 형태로 HttpResponse를 반환하도록 코드를 구현했는데, 해당 방식의 장점은 무엇이라고 생각하시나요?

호오.. 한번도 생각해보지는 못했는데 재밌는 주제네요. 좋은 질문 주셔서 감사합니다.
말씀 주신 해당 방식은 톰캣의 방식을 말씀하시는걸까요? was에 대한 깊은 조예가 있지 않지만 추측 정도는 해볼 수 있을 것 같아요.

톰캣은 다양한 사용자에게 서비스를 합니다. 하지만, 범용적인 HttpResponse 응답을 만들기 어렵다고 생각해요. 사용자들마다 http를 해석하고 사용하는 양상이 다르기 때문이라고 생각했어요. 예를 들어, 특정 서비스에서 내려주는 파일은 index.html인데 모종의 이유로 클라이언트에게는 content-type: text/plain로 내려줘야하는 상황이 있을 있을 수 있습니다. 즉, 범용적으로 사용할 수 있도록 사용자가 HttpResponse를 직접 다루도록하여 자유도를 높인것 같아요.

HttpResponse의 참조를 끊지 않는 이유는 추측이 어려워서 HttpServletResponse를 봤는데요. 구현체가 많더라구요.
핸들러 반환값이 void이면 구현체를 결정하는 주체는 톰캣이고, 핸들러의 반환값이 HttpResponse라면 구현체를 결정하는 주체는 클라이언트 개발자가 될 것 같아요. 이를 미루어봤을때, 상황에 따라 적절한 구현체를 핸들러에 주입해줄 수 있는 것이 장점일 것 같아요.
초코칩의 생각은 어떠세요? 제가 놓치고 있는 부분이 있으면 편하게 말씀 주세요.

말씀 주신 해당 방식은 톰캣의 방식을 말씀하시는걸까요?

맞습니다.

초코칩의 생각은 어떠세요?

저는 두 가지 관점에서 참조를 끊지 않는다고 생각했어요.

API 요청에 단 하나의 HttpResponse 객체만이 존재해야 한다.

유연함을 가질 수 있다.

첫 번째로, 요청에 대한 응답은 단 하나의 객체로 관리되어야 일관된 상태를 유지할 수 있어요. 응답 객체를 하나로 유지하면 상태의 변화가 일관적이며, 다른 핸들러나 필터들이 접근해도 동일한 상태를 참조할 수 있기 때문에 복잡한 흐름에서도 안정성을 유지할 수 있습니다.

만약 더 복잡한 환경에서 targetHandler.handle(httpRequest, sessionManager); 내부에 의존하고 있는 다른 메서드들이 의도치 않게 HttpResponse 객체를 2~3개를 만들어낸다면 어떤 것을 반환해야 할지 애매해요. 그 중에서 하나를 반환한다 하더라도 다른 로직이 빠질 가능성이 있다고 생각해요.

두 번째로는 유연성입니다. 이 부분은 아톰의 생각과 동일합니다.

그럼에도 불구하고 제가 HttpResponse를 반환하도록 구현한 이유는, 해당 방식이 자연스럽게 느껴졌기 때문입니다. 객체 지향적으로 생각했을 때 핸들러가 요청을 받아서 다른 객체에 메시지를 전달하고, 그 결과로 응답을 생성하는 흐름이 직관적이었어요. (하지만 저도 나중에 바꿀지도...?)

Chocochip101 · 2024-09-06T10:36:16Z

tomcat/src/main/java/org/apache/coyote/http11/AbstractHandler.java

+        for (ContentType contentType : ContentType.values()) {
+            if (resourcePath.endsWith(contentType.getExtension())) {
+                return String.format(encodedContentTypeFormat, contentType.getName());
+            }
+        }


ContentType에 책임을 더 부여할 수 있겠네요.

Suggested change

for (ContentType contentType : ContentType.values()) {

if (resourcePath.endsWith(contentType.getExtension())) {

return String.format(encodedContentTypeFormat, contentType.getName());

}

}

for (ContentType contentType : ContentType.values()) {

if (contentType.matchesExtension(resourcePath)) {

return String.format(encodedContentTypeFormat, contentType.getName());

}

}

ContentType에 책임을 부여하도록 변경했어요! 어색한 부분이 있다면, 추가 피드백 부탁드립니다. 🙇🏻‍♂️

tomcat/src/main/java/org/apache/coyote/http11/Header.java

tomcat/src/main/java/com/techcourse/handler/PostLoginHandler.java

Chocochip101

수고하셨습니다, 아톰 😄

관련된 저의 의견은 리뷰에 작성했습니다.

3, 4 단계도 화이팅입니다!

제안)
3, 4단계의 미션 성격이 많이 다르더라고요.
괜찮으시면 3단계와 4단계를 나눠서 리뷰 보내주심을 제안드립니다!

Chocochip101 · 2024-09-08T12:39:31Z

tomcat/src/main/java/org/apache/coyote/http11/HttpRequest.java

+
+import java.net.URI;
+
+public record HttpRequest(String startLine, Header header, char[] body) {


👍🏻

그럼 마찬가지로 body도 header와 마찬가지로 객체로 다뤄봐도 좋을 것 같네요.

Chocochip101 · 2024-09-08T12:42:10Z

tomcat/src/main/java/org/apache/catalina/session/SimpleSession.java

+    @Override
+    public void invalidate() {
+
+    }


추후에 세션 만료도 구현해봐도 좋을 것 같아요.

Chocochip101 · 2024-09-08T12:50:39Z

tomcat/src/main/java/org/apache/coyote/http11/AbstractHandler.java

+    protected String getSessionKey() {
+        return "JSESSIONID";
+    }


쿠키와 세션은 Header와 관련된 속성이라고 느껴져요. 추상 클래스에서 getter로 제공하는 것보다 Header 객체 내부에서 처리하는 것은 어떤가요?

Chocochip101 · 2024-09-08T13:11:25Z

tomcat/src/main/java/org/apache/coyote/http11/Http11Processor.java

+    private HttpResponse respondResource(HttpRequest httpRequest) throws IOException {
+        AbstractHandler helloHandler = new HelloHandler();
+        AbstractHandler staticResourceHandler = new StaticResourceHandler();
+        AbstractHandler postLoginHandler = new PostLoginHandler();
+        AbstractHandler postRegisterHandler = new PostRegisterHandler();
+        AbstractHandler getLoginHandler = new GetLoginHandler();
+        AbstractHandler getRegisterHandler = new GetRegisterHandler();
+        AbstractHandler notFoundHandler = new NotFoundHandler();
+
+        List<AbstractHandler> handlers = List.of(
+                helloHandler,
+                staticResourceHandler,
+                postLoginHandler,
+                postRegisterHandler,
+                getLoginHandler,
+                getRegisterHandler
+        );
+        AbstractHandler targetHandler = handlers.stream()
+                .filter(it -> it.canHandle(httpRequest))
+                .findFirst()
+                .orElse(notFoundHandler);
+
+        return targetHandler.handle(httpRequest, sessionManager);
+    }


말씀 주신 해당 방식은 톰캣의 방식을 말씀하시는걸까요?

맞습니다.

초코칩의 생각은 어떠세요?

저는 두 가지 관점에서 참조를 끊지 않는다고 생각했어요.

API 요청에 단 하나의 HttpResponse 객체만이 존재해야 한다.

유연함을 가질 수 있다.

첫 번째로, 요청에 대한 응답은 단 하나의 객체로 관리되어야 일관된 상태를 유지할 수 있어요. 응답 객체를 하나로 유지하면 상태의 변화가 일관적이며, 다른 핸들러나 필터들이 접근해도 동일한 상태를 참조할 수 있기 때문에 복잡한 흐름에서도 안정성을 유지할 수 있습니다.

만약 더 복잡한 환경에서 targetHandler.handle(httpRequest, sessionManager); 내부에 의존하고 있는 다른 메서드들이 의도치 않게 HttpResponse 객체를 2~3개를 만들어낸다면 어떤 것을 반환해야 할지 애매해요. 그 중에서 하나를 반환한다 하더라도 다른 로직이 빠질 가능성이 있다고 생각해요.

두 번째로는 유연성입니다. 이 부분은 아톰의 생각과 동일합니다.

그럼에도 불구하고 제가 HttpResponse를 반환하도록 구현한 이유는, 해당 방식이 자연스럽게 느껴졌기 때문입니다. 객체 지향적으로 생각했을 때 핸들러가 요청을 받아서 다른 객체에 메시지를 전달하고, 그 결과로 응답을 생성하는 흐름이 직관적이었어요. (하지만 저도 나중에 바꿀지도...?)

geoje and others added 30 commits September 5, 2024 11:11

fix: remove implementation logback-classic on gradle (woowacourse#501)

fed02f6

fix: add threads min-spare configuration on properties (woowacourse#502)

7e91356

feat: index.html 응답 구현

44661a3

refactor: 응답값과 contentType 자료구조로 포장

d11ca3b

feat: css, javascript 파일 지원

7a15bf7

feat: login.html 응답 구현

436a244

docs: todo list 작성

bd9e29a

refactor: query parameter 책임 분리

6d95afc

refactor: process 메서드 가독성 개선

172d7c5

fix: javascript contentType 변경

3a92560

test: 누락된 쿼리 파라미터 테스트 추가

314b36a

refactor: header 책임 분리

8107ba5

refactor: 패키지 이동

8c99626

fix: header pair 구분자 변경

281d9d3

refactor: 정적 파일 핸들링 책임 분리

ccecee0

refactor: login 책임 분리

2506893

refactor: handler 추상화

c0b8dfc

refactor: 추상화된 핸들러 사용하도록 변경

8780caf

fix: */* 타입 처리하도록 변경

b7acd82

fix: hello handler hello.html 반환하도록 변경

f852512

refactor: 불필요한 로그 제거

704823c

chore: import 최적화

3b55376

refactor: response 내부로 메시지 생성 책임 위임

23b3098

feat: 로그인 redirection 처리

e36a27b

refactor: 메서드 이름 변경

a43ebae

refactor: /login get post 분리

6cc0cc5

feat: register 페이지 조회 기능 구현

45c7fd7

feat: not found 핸들러 구현

9a84cde

refactor: content-type 분리

2614f38

refactor: forward 결과 포장

0a33eda

le2sky added 12 commits September 6, 2024 09:20

feat: 회원가입 기능 구현

f3f408c

refactor: http header 상수화

236a94b

feat: header 추가 메서드 구현

421ddb6

fix: null 검증 제거

e24a8ef

chore: import문 최적화

7eb2844

refactor: forward 내부에서 status, header 다룰 수 있도록 변경

cce15be

feat: cookie 클래스 구현

5668186

refactor: 패키지 구조 변경

fc664f5

feat: session 구현

c3be469

feat: 로그인 세션 적용

48bec18

style: enum 개행 제거

da8a395

refactor: 불필요한 throws 제거

b11b42a

le2sky changed the title ~~아톰 미션 제출합니다.~~ [1 -2단계 - Tomcat 구현하기] 아톰(이하늘) 미션 제출합니다. Sep 6, 2024

le2sky added 2 commits September 6, 2024 16:57

chore: 학습 테스트 흔적 제거

2e0f82d

chore: p 태그 제거

5837dfd

Chocochip101 requested changes Sep 6, 2024

View reviewed changes

le2sky added 5 commits September 8, 2024 16:59

refactor: content-type 내부로 content-type 결정 책임 이동

d3b1cda

refactor: header.empty 정적 팩터리 메서드로 변경

8bead94

refactor: http version 상수화

8e83204

refactor: http request body를 char[]로 변경

1f1e5f3

fix: from 메서드에서 case 무시하지 않도록 변경

3aab4c8

Chocochip101 approved these changes Sep 8, 2024

View reviewed changes

Chocochip101 merged commit 20e36b8 into woowacourse:le2sky Sep 8, 2024


		import java.net.URI;

		public record HttpRequest(String startLine, Header header, QueryParameter body) {

[1 -2단계 - Tomcat 구현하기] 아톰(이하늘) 미션 제출합니다. #531

[1 -2단계 - Tomcat 구현하기] 아톰(이하늘) 미션 제출합니다. #531

Uh oh!

Conversation

le2sky commented Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Chocochip101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Chocochip101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

le2sky commented Sep 6, 2024 •

edited

Loading