[톰캣 구현하기 - 3,4단계] 도이(유도영) 미션 제출합니다. #453

yoondgu · 2023-09-10T14:26:43Z

안녕하세요 홍실 ,, 🧶
지난 단계 잘 확인해주셔서 일찍이 이번 단계를 시작할 수 있었음에도 리뷰 요청이 많이 늦었습니다 🥲
욕심을 부렸는지 삽질..+ 전체 구조에 변화가 여러 번 있어서, 시간이 오래 걸렸어요.
인터페이스, 클래스명, 패키지 구조 모두 전체적으로 바뀌었고 커밋 당 변경 사항도 많아서 ㅜㅜ
커밋 단위로 코드를 보시기에 좀 힘들 것 같아 죄송스럽네요.

변경된 흐름으로 다시 설명 드리자면, 아래와 같습니다.

톰캣을 사용하는 어플리케이션(jwp 패키지)에서 필요한 객체를 담은 Container를 전달해 Connector를 생성
- 어플리케이션 개발자가 요청에 대해 매핑한 내용을 가진 HttpServlet 객체들
- SessionManager 객체
요청마다 스레드 생성 -> Http11Processor에서 Container를 통해 응답 반환
Container는 RequestHandlerAdaptor를 통해 적절한 HttpServlet을 찾아 요청 처리
- 이 때 Container가 HttpServlet이 사용할 HttpServletRequest, HttpServletResponse 객체를 service() 메서드의 인자로 전달
- 두 객체는 http11 패키지의 Request, Response와는 별도로 관리됨 (어플리케이션 코드와의 의존성 제거를 위함)

참고로 3단계 요구사항인 Controller 인터페이스와 추상클래스의 경우,

톰캣 Controller 인터페이스 -> RequestHandler
톰캣 AbstractController -> HttpServlet
이라는 네이밍으로 바꾸어 구현했습니다.
저는 어플리케이션 코드 개발자가 작성하는 Controller와 톰캣의 Controller 역할을 분리해서 구현하는 데 신경썼는데, 그러다보니 서로 다른 클래스가 같은 네이밍을 쓰면 헷갈릴 것 같아서요!

설계 삽질에 빠져버려서 스레드 관련 고민은 깊게 하지 못한 것 같네요..
동시성 문제 관련 테스트를 짜보려고 했는데 어떻게 해야 할지, 혹시 좋은 의견 있으시면 부탁드립니다 ㅎㅎㅎ

너무 늦게.. 너무 바뀐 코드로 요청드리지만 🥹🥹 잘 부탁드려요!
질문 있으시면 편하게 말씀해주세용

- isNew 메서드로 인해 Session SessionManager 양방향 의존 해결이 필요했음 - Session이 항상 SessionManager에서 만들어지도록 보장하면 로그인 여부 확인 시 isNew 메서드가 필요없는 상황 - 따라서 SessionManager에서 항상 Session을 만들도록 함 - 외부의 Session 생성 방지를 위해 내부 클래스로 변경

- ContentType 관련, 응답 출력 관련 리팩터링 필요

- 하나의 요청 프로세스에 대응하는 객체라는 점에서 기존 RequestHandler -> FrontController 네이밍 변경에서 DispatcherServlet 으로 변경 - 요구사항의 Controller 클래스는 jwp 패키지와의 구분을 위해 Handler로 명명 - 현재는 static하게 사용하지만, 추후 동시성 처리하며 인스턴스를 서블릿 컨테이너에서 관리하도록 하기

- DispatcherServlet -> RequestHandlerAdaptor - RequestHandlerMapping -> RequestMapping - 정적 파일 핸들러는 RequestHandler만 구현하도록 함 - AbstractRequestHandler는 동적 파일 핸들러만 상속하도록 하고 HttpServlet으로 변경 - StaticResourceRequestHandler 는 주입받는 대신 톰캣에서 관리

- 중간 계층 역할로서 ServletResponse 를 Response 클래스에서 분리 - 프로덕션 코드는 톰캣에 의존할 수 있게 톰캣 패키지로 이동

- StaticResourceRequestHandler -> DefaultServlet - 프로덕션 코드의 XXXHandler -> XXXServlet

- Response와 마찬가지로 서블릿에서 사용하는 Request의 추상화 계층 정의 - 프로덕션 코드에서 http11 패키지에 의존하지 않도록 하기 위함

- SessionManager를 Tomcat 생애주기에 따라 관리하므로 패키지 이동

- Response 객체의 생성과 소멸 주기를 같은 곳에서 관리하도록 함

sonarcloud · 2023-09-10T14:28:03Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

0.0% Coverage
0.0% Duplication

The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

hong-sile

전체적으로 많이 배울 수 있는 코드였습니다.

이번 미션에서 학습해야 할 것과 본인이 구현하려고 노력한 방식이 신선해서 재밌었어요.

동시성 문제 관련한 테스트는 저도 코드로 구현해보려다가 실패했어요.. ㅠ
학습테스트에 있는 내용을 응용하는게 제가 지식이 모자라서 그런지 어렵더라고요.
그래서 테스트코드 만으론 동시성을 확인하기 어려워서, postMan을 이용해서 동시성에 대해서 학습을 해봤어요.
Postman에 동시성 테스트를 위한 기능들이 많으니, 한 번 이용해보시는 것도 추천드립니다.

궁금한 부분에 대해서 질문 남겼는데 확인해주시면 감사하겠습니다.
이만 Approve Merge 할게요!! 이번 미션 수고하셨습니다.

hong-sile · 2023-09-12T08:36:33Z

tomcat/src/main/java/org/apache/catalina/core/servlet/HttpServlet.java

+import org.apache.catalina.core.RequestHandler;
+
+public abstract class HttpServlet implements RequestHandler {
+


HTTP Servlet(abstract class)은 catalina에 두고, 실 구현체는 jwp에 둔게 인상적이네요. 패키지를 잘 고려해서 분리하신 것 같아요.

hong-sile · 2023-09-12T08:37:43Z

tomcat/src/main/java/org/apache/catalina/core/session/Manager.java

     */
-    HttpSession findSession(String id) throws IOException;
+    Optional<Session> findSession(String id) throws IOException;


어떻게 이걸 활용했나 했는데, 기존에 주어진 코드를 수정하셨군요!
좋은 방법인 것 같아요

hong-sile · 2023-09-12T08:39:50Z

tomcat/src/main/java/org/apache/catalina/core/RequestHandler.java

+
+public interface RequestHandler {
+
+    void service(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) throws Exception;


쭉 보다 보니 HttpServlet abstract class가 구현되고 나서,
RequestHandler를 사용하는 곳은 없는 것? 같아요.
HttpServlet을 제외한 다른 클래스에서 이 RequestHandler를 구현받진 않더라고요.
실제로 쓰는 곳도 반환하는 용도? 였고요.(이 부분도 반환타입을 HttpServlet으로 둘 수 있을 것 같아요.)

RequetHandler를 남겨두신 이유가 있으신가요?

공감합니다 !! 요구사항에 주어진 구조라 그대로 유지한 게 큰 이유이긴 했어요.
요구사항에서는 왜 추상 클래스 상위에 인터페이스를 두는 방식이 주어졌을까요?
HttpServlet이 아니라, 다른 종류의 구현체가 올 수 있도록 하기 위함일까요...? 홍실의 생각도 궁금하네용

hong-sile · 2023-09-12T08:42:02Z

tomcat/src/main/java/nextstep/Application.java

        final var tomcat = new Tomcat();
-        tomcat.start();
+        tomcat.start(Set.of(new LoginRequestServlet(), new RegisterRequestServlet()));


Application에서 Servlet을 주입받는군요.

진짜 코드가 맛있네요. 패키지에 신경을 쓰셨다는게 눈에 보여요.

정말 단순한 궁금증인데, Servlet들을 start로 넘기신 이유가 있을까요?

Tomcat 생성자에 넣을 수도 있을 것 같아서요.
(별 의미 없을 수도 있을 것 같은데, 혹시 도이라면 이유가 있을까? 궁금해서 코멘트 남깁니다.)

생성자로 받게 된다면, 서블릿들을 Tomcat의 멤버변수로 가지게 될텐데 (서블릿의 상태를 관리하는 다른 기능이 없기 때문에) 지금으로서는 그럴 이유가 없다고 생각했어요! 그래서 start 메서드를 통해 Http11Processor까지 전달해주기만 했습니다.

hong-sile · 2023-09-12T08:43:38Z

tomcat/src/main/java/nextstep/jwp/controller/Controller.java

@@ -0,0 +1,5 @@
+package nextstep.jwp.controller;
+
+public interface Controller {


혹시 Controller를 별도의 인터페이스로 분리하신 이유가 있으신가요?

각각의 Controller에 Implement를 하고 별도로 사용하진 않는 것 같아서요.
(따로 추상 메서드가 있는 것도 아니고요.)

지금은 catalina에서 도메인 컨트롤러의 static 메서드를 호출하는 방식을 사용하고 있는데,
이 컨트롤러들도 인스턴스화해서 주입하는 방식으로 바꿀 필요가 있다고 생각하면서 일단 인터페이스만 만들어놨는데요. 해당 부분까지 진행하면 너무 커질 것 같아서 내버려뒀습니다... ㅎㅎㅎ

여기에 쪼금 더 합리화하자면 Spring에서 @Service 어노테이션이 지금은 특별한 기능이 없어도 사용되는 것처럼, 기능이 없는 인터페이스도 추후 변경 시 같이 변경되어야 할 레이어를 지정하는 역할을 할 수 있다고 생각했어요.

hong-sile · 2023-09-12T10:31:22Z

tomcat/src/main/java/org/apache/catalina/core/servlet/HttpServletResponse.java

+    }
+
+    public static HttpServletResponse internalSeverError() {
+        return new HttpServletResponse(INTERNAL_SERVER_ERROR);


InternalServeError나 notFound의 경우에는 사용하지 않으니,
삭제해도 될것 같아요!

hong-sile · 2023-09-12T10:32:22Z

tomcat/src/main/java/org/apache/coyote/http11/request/Request.java

-    private final Method method;
-    private final String uri;
-    private final Headers headers;
+    private final RequestLine requestLine;


coyote 패키지엔 Request를 두고,
catalina 패키지엔 HttpServletRequest로 두셨더라고요.

혹시 그렇게 분리하신 이유가 있을까요?

패키지 별 Request를 분리한 이유를 물어보시는 게 맞을까요?
아니면 네이밍에 대한 질문이실까요?

전자의 경우,
jwp 패키지에서는 통신 방법을 몰라야 한다고 생각해 http 패키지를 참조하지 않게 하기 위해서 분리했습니다. 이를 통해 SessionManager와 Request 의 의존도 끊을 수 있었어요.

Hoxy 후자의 질문이라면,
coyote 패키지는 이미 http11 패키지에 속해있으므로 이름에 Http~ 를 붙일 필요 없다고 생각했고
catalina 패키지에서는 HttpServlet에서 사용하는 Request라는 의미로 구체화했습니다.

hong-sile · 2023-09-12T10:39:43Z

tomcat/src/main/java/org/apache/coyote/http11/common/header/HeaderName.java

+    /**
+     * Entity header
+     */
+    CONTENT_ENCODING("Content-Encoding"),


헤더도 역할별로 분리한것도 야무지네요. 이를 이용해서 클래스를 분리한 것도 인상적이었습니다.

hong-sile · 2023-09-12T10:41:49Z

tomcat/src/main/java/org/apache/coyote/http11/common/header/ResponseHeaders.java

+
+public class ResponseHeaders extends Headers {
+
+    public ResponseHeaders() {


얘는 별도로 안쓰면 private으로 빼도 될 것 같아요.

hong-sile · 2023-09-12T10:48:11Z

tomcat/src/main/java/org/apache/coyote/http11/common/header/Headers.java

+    }
+
+    private Map<String, String> filterByHeaderType(final Map<String, String> headers) {
+        return headers.entrySet().stream()


이게 어떤식으로 동작하나 했는데, 각각의 헤더를 분리하기 위한 용도군요.
재밌는 구현방식인 것 같아요.

yoondgu and others added 30 commits September 7, 2023 23:08

test: Cache 학습 테스트 작성

92b35c9

style: final 키워드 추가

cd9a2f7

style: 타입 추론으로 코드 통일

d7c55aa

refactor: 적절하지 않은 정적 팩터리 메서드 네이밍 수정

1e30f69

refactor: 일부 메서드 분리, Optional 로직 보완, 가독성 개선

3dea82f

refactor: 쿼리 스트링 파싱 클래스 네이밍 변경 및 범용성 있게 수정

fd68631

rename: 요청 핸들러 분기 클래스명 FirstController로 변경

edfeb63

refactor: 헤더 유형 별로 관리하도록 클래스 정의

cc56b3a

- ContentType 관련, 응답 출력 관련 리팩터링 필요

refactor: Response Builder 적용

8bb0991

fix: Enum에 없는 헤더 못읽는 오류 수정

1ba138f

refactor: RequestLine 객체 포장

eac7d14

refactor: StatusLine 객체 포장

a86e454

chore: Servlet 관련 클래스들 catalina 패키지로 이동 (3단계 완료)

a9dc6d0

refactor: SessionManager가 Manger 인터페이스 구현하도록 변경

0133924

test: Thread 테스트 작성

f7942be

rename: MimeType 클래스명 변경

dd3e648

test: Thread 학습 테스트 2단계 작성

589135d

feat: 스레드 풀 적, 세션 매니저 동시성 컬렉션 적용

026e4dd

chore: Session 패키지 coyote/http11로 이동

b0bda83

refactor: HandlerMapping 구조 변경, 구현체 의존성 주입 구현

3fe2930

refactor: 3단계 요구사항에 맞게 컨트롤러(Servlet) 인터페이스 수정

73535c5

refactor: ServletResponse 클래스 분리, 패키지 이동

54a9418

- 중간 계층 역할로서 ServletResponse 를 Response 클래스에서 분리 - 프로덕션 코드는 톰캣에 의존할 수 있게 톰캣 패키지로 이동

refactor: 서블릿 패키지명 및 구조 수정

5bb6dab

rename: ServletXXX -> HttpServletXXX 으로 네이밍 수정

de048a2

rename: 정적 파일에 대한 핸들러도 서블릿으로 취급

ee2a088

- StaticResourceRequestHandler -> DefaultServlet - 프로덕션 코드의 XXXHandler -> XXXServlet

rename: 톰캣에 필요한 객체들 Container로 전달

85a3522

refactor: HttpServletRequest 클래스 정의

2da62d8

- Response와 마찬가지로 서블릿에서 사용하는 Request의 추상화 계층 정의 - 프로덕션 코드에서 http11 패키지에 의존하지 않도록 하기 위함

yoondgu added 6 commits September 10, 2023 22:28

refactor: session 패키지 catalina로 이동

3ff6f80

- SessionManager를 Tomcat 생애주기에 따라 관리하므로 패키지 이동

refactor: 예외에 따른 응답 반환 로직 Container 로 이동

031c166

- Response 객체의 생성과 소멸 주기를 같은 곳에서 관리하도록 함

refactor: 메서드 순서 수정

2745e8c

test: 테스트 패키지 수정

4e64504

fix: 존재하지 않는 정적 파일 요청 시 404 페이지로 리다이렉트, SVG mimeType 추가

a474269

fix: 500 에러 발생 시 500.html 페이지로 리다이렉트하도록 수정

48f7d39

yoondgu requested a review from hong-sile September 10, 2023 14:26

yoondgu self-assigned this Sep 10, 2023

yoondgu marked this pull request as ready for review September 10, 2023 14:53

hong-sile approved these changes Sep 12, 2023

View reviewed changes

hong-sile merged commit d638346 into woowacourse:yoondgu Sep 12, 2023
2 checks passed

yoondgu changed the title ~~[톰캣 구현하기 - 3, 4단계] 도이(유도영) 미션 제출합니다.~~ [톰캣 구현하기 - 3,4단계] 도이(유도영) 미션 제출합니다. Sep 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 - 3,4단계] 도이(유도영) 미션 제출합니다. #453

[톰캣 구현하기 - 3,4단계] 도이(유도영) 미션 제출합니다. #453

yoondgu commented Sep 10, 2023 •

edited

Loading

sonarcloud bot commented Sep 10, 2023

hong-sile left a comment •

edited

Loading

hong-sile Sep 12, 2023

hong-sile Sep 12, 2023

hong-sile Sep 12, 2023

yoondgu Sep 13, 2023

hong-sile Sep 12, 2023

yoondgu Sep 12, 2023

hong-sile Sep 12, 2023

yoondgu Sep 13, 2023 •

edited

Loading

hong-sile Sep 12, 2023

hong-sile Sep 12, 2023

yoondgu Sep 13, 2023

hong-sile Sep 12, 2023

hong-sile Sep 12, 2023

hong-sile Sep 12, 2023

		import org.apache.catalina.core.RequestHandler;

		public abstract class HttpServlet implements RequestHandler {


		public interface RequestHandler {

		void service(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) throws Exception;

		@@ -0,0 +1,5 @@
		package nextstep.jwp.controller;

		public interface Controller {


		public class ResponseHeaders extends Headers {

		public ResponseHeaders() {

[톰캣 구현하기 - 3,4단계] 도이(유도영) 미션 제출합니다. #453

[톰캣 구현하기 - 3,4단계] 도이(유도영) 미션 제출합니다. #453

Conversation

yoondgu commented Sep 10, 2023 • edited Loading

sonarcloud bot commented Sep 10, 2023

hong-sile left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu Sep 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu commented Sep 10, 2023 •

edited

Loading

hong-sile left a comment •

edited

Loading

yoondgu Sep 13, 2023 •

edited

Loading