[톰캣 구현하기 - 3, 4단계] 우르(김현우) 미션 제출합니다. #481

java-saeng · 2023-09-11T07:27:17Z

안녕하세요 땡칠 !!!

저번 트러블 슈팅한 문서 공유해주셔서 재밌게 잘 읽었습니다~~

기존 코드와 다르게 많이 리팩터링 되었습니다,,,!!

괜찮으시다면 코드 보면서 이해가 잘 되시는지 궁금해요~~!!
감사합니다.

0chil

안녕하세요 우르! 3, 4단계도 잘 구현해주셨습니다!
2단계를 나중에 진행하신게 오히려 좋은 선택이었던 것 같아요 ㅎㅎ

코드 부분은 말씀대로 리팩터링이 많이 되어서 훨씬 읽기가 수월했습니다 👍
책임 분리도 더 명확해져서 설계가 잘 되어있다고 느꼈고 개인적으로 완성도 높다고 생각했습니다.

질문 몇 개 달아두었으니 확인해주시고 더 궁금하신 부분은 DM이나 커멘트 중 편한 방식으로 남겨주세요!
미션 고생많으셨습니다 🫡

0chil · 2023-09-12T00:49:47Z

tomcat/src/main/java/org/apache/catalina/connector/Connector.java

        this.serverSocket = createServerSocket(port, acceptCount);
        this.stopped = false;
        this.handlers = handlers;
+        executor = Executors.newFixedThreadPool(maxThreads);


학습 질문 중 '250개의 쓰레드가 모두 사용중일 때, 최대 100개의 연결을 대기하게 만드려면 어떻게 해야 하는가?' 라는 질문이 있었어요.

우르는 이걸 어떻게 풀어내야 한다고 생각하시나요?

스레드 풀의 maxThread 의 개수를 250개로 설정하고 acceptCount를 100으로 하면 되지 않을까 싶습니다!!!

땡칠은 어떻게 생각하시나요?

저도 같게 생각했습니다.
'100개 연결 대기' 부분이 OS가 관리하는 backlog 부분이고, acceptCount로 조절 가능하다고 생각했어요!

0chil · 2023-09-12T00:51:25Z

tomcat/src/main/java/org/apache/coyote/handler/Handler.java


 public interface Handler {

  boolean canHandle(final HttpRequest httpRequest);

-  String handle(final HttpRequest httpRequest) throws IOException;
+  void handle(final HttpRequest httpRequest, final HttpResponse httpResponse) throws IOException;


저도 주말에 알게되었는데 Request, Response가 각각 인자로 들어가는게 요구사항이었더라구요!

우르는 왜 이런 디자인을 가져야 한다고 생각하시나요?

request에 맞는 controller를 찾기 전에 response에 관한 전처리를 해줘야하는 경우가 있을까? 에 대해서 생각해봤습니다.

대표적인게 Accept라고 생각이 드는데, 서버에서 지원하는 Content-Type이 없으면 바로 response 406 Not Acceptable 를 응답하는 경우도 있지 않을까라는 생각이 들었습니다.

이게 맞는건지는 모르겠지만,, 톰켓이 설계한 이유 중에 아주 사소한 것 중 하나이지 않을까 싶습니다,,

그러게요 ㅎㅎ 컨트롤러에서 정한 응답 외에 오류 응답등이 있을 수 있겠네요
이런 응답 수정을 언제든지, 쉽게 하려고 이렇게 만들지 않았을까 싶어요.
원래 의도는 톰캣을 만든 본인만 알겠죠!

0chil · 2023-09-12T01:03:03Z

tomcat/src/main/java/org/apache/coyote/handler/HandlerComposite.java

+  public void handle(final HttpRequest httpRequest, final HttpResponse httpResponse) {
+    handlers.stream()
        .filter(it -> it.canHandle(httpRequest))
-        .map(it -> it.safeHandle(httpRequest))
-        .findAny()
-        .orElseThrow(() -> new IllegalArgumentException("handler가 존재하지 않습니다."));
+        .forEach(it -> it.safeHandle(httpRequest, httpResponse));


인터페이스가 바뀌면서 깔끔해졌네요!
그런데 적절한 핸들러를 찾지 못하면 어떻게 되나요?
기존에는 예외를 반환했었는데, 해당 기능이 사라진 것 같네요

와,, 섬세한 리뷰 감사합니다 👍👍👍

바로 수정했습니다 !

0chil · 2023-09-12T01:19:16Z

tomcat/src/main/java/org/apache/coyote/handler/LoginHandler.java

+  private static final String SUCCESS_REDIRECT_URL = "http://localhost:8080/index.html";
+  private static final String FAIL_REDIRECT_URL = "http://localhost:8080/401.html";



접속자가 localhost가 아닌 다른 호스트를 통해 접속한다면 리다이렉션이 잘못 이뤄질 것 같아요.

HTTP Semantics 문서에 따르면 상대 주소로 표기할수도 있다고 하니, /index.html, /401.html 이런식으로 나타낼 수 있을 것 같습니다.

RFC 9110, Location in HTTP Semantics

오.. 링크까지 너무 스윗하시네요

감사합니다!! 바로 반영했습니다

0chil · 2023-09-12T01:20:56Z

tomcat/src/main/java/org/apache/coyote/handler/LoginHandler.java

+            user -> {
+              if (user.checkPassword(password)) {
+                redirectOnSuccessAuthenticate(httpRequest, httpResponse, user);
+                return;
+              }
+              redirectOnFailure(httpResponse);
+            },
+            () -> redirectOnFailure(httpResponse)
+        );
+  }


저는 이 부분이 교통정리가 잘 안되던데, 깔끔하게 풀어주셨네요!
덕분에 로직이 잘 보입니다 👍

영광입니다!

0chil · 2023-09-12T04:21:20Z

tomcat/src/main/java/org/apache/coyote/parser/HttpRequestReader.java

+public class HttpRequestReader {
+
+  private HttpRequestReader() {
+  }
+


과정이 복잡하다보니 별도의 객체로 분리하셨네요!
적절한 분리라고 생각합니다.

0chil · 2023-09-12T04:31:15Z

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

+  private void initializeSession(final Manager sessionManager) {
+    cookie.getJSessionId()
+        .ifPresentOrElse(
+            id -> {
+              try {
+                addSession(sessionManager.findSession(id));
+              } catch (IOException e) {
+                initializeSession(sessionManager);
+              }
+            },
+            () -> {
+              final Session newSession = new Session(UUID.randomUUID().toString());
+              sessionManager.add(newSession);
+              addSession(newSession);
+            }
+        );
  }


Session 생성 및 등록을 request에서 다뤄주고 있네요.
세션이 없는 요청에 세션을 부여해주므로 모든 연결이 세션을 가지게 될 것 같습니다.

세션 부여 여부를 Request에서 강제해도 괜찮을까요?

로그인 외 단순한 요청은 세션을 부여할 필요가 없을 것 같아요. 이게 자원 낭비는 아닐까요?

어떻게 생각하시는지 궁금합니다!

저는 개인적으로 요구사항 차이라고 생각해요

예를 들면 쇼핑몰에서 비로그인이어도 사이드바에 최근 본 상품이 있는데, 이건 어떻게 구현하는거지? 라는 의문이 들었어요
관리자 도구 보니까 세션이 있더라구요. 저는 로그인을 하지 않았는데도요.

이럴때도 세션을 만들 수 있구나 라는 생각이 들어서 한번 구현해봤습니다!!!

자원 낭비가 그만큼 사용자의 편의를 봐주고 있어서 괜찮다고 생각했어요

1번에 대한 답변은 위 기능을 구현할 때, request가 처음 생성될 때 세션 값이 필요하다고 생각이 들어 request에서 강제하도록 했습니다!

초큼 어색한가요,,?

그런 경험이 있으셨군여 ㅎㅎ 저도 접속만 해도 세션이 생기는 서비스들을 본 적 있는 것 같아요 (많이 본 것 같습니다)
세션이 무조건 생성되는 기능이 어색하지는 않지만, 그 기능의 사용 여부를 선택할 수 없다는 점이 아쉽다는 의미였어요.
웹 서버라는 범용적인 코드를 만드는 입장에선 사용자에게 조금 더 선택권을 줘야 좋지 않을까요?

사용자의 편의라는게 코드 사용자의 편의를 말씀하시는 거라면 잘 이해했습니다.

우르 의견 충분히 이해가 되었습니다~ 답변 감사해요 ㅎㅎ

0chil · 2023-09-12T04:42:48Z

tomcat/src/main/java/org/apache/coyote/response/HttpResponseHeader.java

+  public String read() {
+    return values.entrySet()
+        .stream()
+        .map(it -> it.getKey() + ": " + it.getValue() + " ")


RFC 7230, HTTP에서 Header 스펙에 OWS(Optional Whitespace)를 언급하고 있네요.

header-field = field-name ":" OWS field-value OWS

field-value 뒤의 공백은 선택사항이므로 꼭 필요하지 않으면 생략해도 된다고 합니다.

미션을 하다가 테스트 코드에 왜 공백이 있는지 궁금해 찾아본 내용이라서, 공백을 넣는 의도가 따로 있었다면 알려주시면 감사하겠습니다 🫡

와,, 저는 이거 생각도 없이 무조건 있어야하는구나라고 지나쳤는데,,
소중한 정보공유 감사합니다!!!

0chil · 2023-09-12T04:47:48Z

tomcat/src/main/java/org/apache/coyote/response/Protocol.java

+public enum Protocol {
+
+  HTTP_1_1("HTTP","1.1")
+  ;
+
+  private final String protocol;
+  private final String version;
+
+  Protocol(final String protocol, final String version) {
+    this.protocol = protocol;
+    this.version = version;
+  }
+
+  public String read() {
+    return protocol + "/" + version + " ";
+  }
+}


추상화를 꼼꼼하게 해주셨네요!
웹 서버이므로 이 정도까지도 추상화할 수 있을 것 같아요.

0chil · 2023-09-12T04:49:01Z

tomcat/src/test/java/org/apache/coyote/handler/LoginHandlerTest.java

  @DisplayName("canHandle() : URI가 /login으로 시작하고, HTTP 요청이 GET일 경우 true를 반환할 수 있다.")
  void test_canHandle() throws Exception {
    //given
-    final String request = "GET /login";
+    final Cookie cookie = Cookie.from("");
+    final HttpRequestLine httpRequestLine = new HttpRequestLine("GET", "/notLogin");
+    final HttpRequest httpRequest = new HttpRequest(
+        httpRequestLine,
+        null,
+        null,
+        cookie,
+        new SessionManager()
+    );

    //when
-    assertTrue(loginHandler.canHandle(HttpRequest.from(request)));
+    assertFalse(loginHandler.canHandle(httpRequest));


요거 테스트가 제목과 일치하지 않는 것 같아요!

오 이게 전에 고쳤던 것 같아요!

현재 상태는 아래와 같습니다~~

sonarcloud · 2023-09-12T17:06:46Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 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

java-saeng

안녕하세요 땡칠 !!

매번 코멘트와 함께 링크 공유해주셔서 감사합니다 :)

땡칠 질문에 대한 답 남겨보았습니다 !!

java-saeng · 2023-09-12T16:38:10Z

tomcat/src/main/java/org/apache/coyote/handler/LoginHandler.java

+            user -> {
+              if (user.checkPassword(password)) {
+                redirectOnSuccessAuthenticate(httpRequest, httpResponse, user);
+                return;
+              }
+              redirectOnFailure(httpResponse);
+            },
+            () -> redirectOnFailure(httpResponse)
+        );
+  }


영광입니다!

java-saeng · 2023-09-12T16:41:48Z

tomcat/src/main/java/org/apache/coyote/handler/LoginHandler.java

+  private static final String SUCCESS_REDIRECT_URL = "http://localhost:8080/index.html";
+  private static final String FAIL_REDIRECT_URL = "http://localhost:8080/401.html";



오.. 링크까지 너무 스윗하시네요

감사합니다!! 바로 반영했습니다

java-saeng · 2023-09-12T16:51:25Z

tomcat/src/main/java/org/apache/coyote/handler/RegisterHandler.java

+    final HttpResponseHeader header = new HttpResponseHeader().sendRedirect(
+        REDIRECT_URL);
+
+    final HttpResponseStatusLine statusLine = HttpResponseStatusLine.redirect();
+
+    httpResponse.setHttpResponseHeader(header);
+    httpResponse.setHttpResponseStatusLine(statusLine);


오,, 이렇게 하니까 중복된 코드가 제거되네요 감사합니다~~!

java-saeng · 2023-09-12T16:52:29Z

tomcat/src/main/java/org/apache/coyote/handler/RegisterPageHandler.java

+      final HttpRequest httpRequest,
+      final HttpResponse httpResponse
+  ) throws IOException {
+    final String bodyData = FileDetector.detect(RENDERING_FILE_NAME);


그러면 땡칠이 생각하시기에는 detect는 찾는거니까 boolean 이 적절하다고 생각하시나요??

java-saeng · 2023-09-12T16:55:41Z

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

+  private void initializeSession(final Manager sessionManager) {
+    cookie.getJSessionId()
+        .ifPresentOrElse(
+            id -> {
+              try {
+                addSession(sessionManager.findSession(id));
+              } catch (IOException e) {
+                initializeSession(sessionManager);
+              }
+            },
+            () -> {
+              final Session newSession = new Session(UUID.randomUUID().toString());
+              sessionManager.add(newSession);
+              addSession(newSession);
+            }
+        );
  }


저는 개인적으로 요구사항 차이라고 생각해요

예를 들면 쇼핑몰에서 비로그인이어도 사이드바에 최근 본 상품이 있는데, 이건 어떻게 구현하는거지? 라는 의문이 들었어요
관리자 도구 보니까 세션이 있더라구요. 저는 로그인을 하지 않았는데도요.

이럴때도 세션을 만들 수 있구나 라는 생각이 들어서 한번 구현해봤습니다!!!

자원 낭비가 그만큼 사용자의 편의를 봐주고 있어서 괜찮다고 생각했어요

1번에 대한 답변은 위 기능을 구현할 때, request가 처음 생성될 때 세션 값이 필요하다고 생각이 들어 request에서 강제하도록 했습니다!

초큼 어색한가요,,?

java-saeng · 2023-09-12T16:56:36Z

tomcat/src/main/java/org/apache/coyote/response/HttpResponseHeader.java

+  public String read() {
+    return values.entrySet()
+        .stream()
+        .map(it -> it.getKey() + ": " + it.getValue() + " ")


와,, 저는 이거 생각도 없이 무조건 있어야하는구나라고 지나쳤는데,,
소중한 정보공유 감사합니다!!!

java-saeng · 2023-09-12T16:57:59Z

tomcat/src/test/java/org/apache/coyote/handler/LoginHandlerTest.java

  @DisplayName("canHandle() : URI가 /login으로 시작하고, HTTP 요청이 GET일 경우 true를 반환할 수 있다.")
  void test_canHandle() throws Exception {
    //given
-    final String request = "GET /login";
+    final Cookie cookie = Cookie.from("");
+    final HttpRequestLine httpRequestLine = new HttpRequestLine("GET", "/notLogin");
+    final HttpRequest httpRequest = new HttpRequest(
+        httpRequestLine,
+        null,
+        null,
+        cookie,
+        new SessionManager()
+    );

    //when
-    assertTrue(loginHandler.canHandle(HttpRequest.from(request)));
+    assertFalse(loginHandler.canHandle(httpRequest));


오 이게 전에 고쳤던 것 같아요!

현재 상태는 아래와 같습니다~~

java-saeng · 2023-09-12T17:04:55Z

tomcat/src/main/java/org/apache/coyote/handler/HandlerComposite.java

+  public void handle(final HttpRequest httpRequest, final HttpResponse httpResponse) {
+    handlers.stream()
        .filter(it -> it.canHandle(httpRequest))
-        .map(it -> it.safeHandle(httpRequest))
-        .findAny()
-        .orElseThrow(() -> new IllegalArgumentException("handler가 존재하지 않습니다."));
+        .forEach(it -> it.safeHandle(httpRequest, httpResponse));


와,, 섬세한 리뷰 감사합니다 👍👍👍

바로 수정했습니다 !

0chil

안녕하세요 우르~
새벽에 리뷰 반영해주셨네요! 퇴근 후에도 열공하시는 모습 너무 멋있습니다.

리뷰하면서 우르덕에 새로운 생각들을 많이 얻고 학습도 하게 되었네요.

Optional 사용이 꺼려졌는데 그래도 가독성에서 차이가 크다는 점
Composite 패턴 활용
canHandle() 메서드를 활용해 Controller 객체에 자율성을 부여하는 아이디어
HTTP 표준 스펙
등등...

앞으로도 많이 배워가겠습니다 ㅎㅎ 미션 진행하시느라 고생 많으셨어요!

java-saeng added 25 commits September 6, 2023 15:18

feat : 학습 테스트

3af2ce9

feat : Cookie 클래스 생성

1b3c576

feat : Session 클래스 생성

f15ec86

feat : HTTP 요청을 파싱하는 유틸리티 클래스 생성

865345e

feat : HttpRequestLine 클래스 추가

c3d4faa

feat : RequestBody 클래스 추가

67473e0

feat : QueryString 클래스 추가

a04d297

refactor : queryString, body, cookie, httpRequestLine 추가

1aa6444

feat : 회원 가입 및 로그인 기능 추가

066f291

refactor : path -> uri 로 변수 변경

da41b0d

feat : handler 추가

d1d8c11

refactor : handle 반환값 void 로 변경

91dceb5

refactor : handler composite 반환값 변경

728620e

feat : Http 응답 관련 header, body, status line 객체 추가

d096920

feat : Handler 반환값을 void로 변경하여 response에 값을 넣도록 수정

ecd6ddd

feat : HttpRequest, HttpResponse 공통적으로 Cookie 사용하도록 수정

f24fd29

feat : 매 요청마다 세션이 있으면 세션 매니저에 넣어주고, 없으면 생성하여 넣어주기

e7b66dd

feat : 로그인 관련 로직 수정

8e403ca

refactor : 세션 추가할 때 기존 세션이 가지고 있는 id로 저장

c9ebbb1

refactor : LoginPageHandler는 정적 Handler 로 수정

2184f8d

refactor : Http11Processor의 HandlerComposite 타입을 Handler 인터페이스로 수정

fb55927

refactor : 동시성 보장을 위한 ConcurrentHashMap 사용

d792f64

refactor : max thread 설정

fa4598e

test : 테스트를 위한 getter 추가

656e694

feat : 학습 테스트

843184c

java-saeng requested a review from 0chil September 11, 2023 07:27

java-saeng added 2 commits September 11, 2023 16:37

refactor : 공백 추가

22ce601

refactor : 코드 스멜 제거

84c9d52

0chil requested changes Sep 12, 2023

View reviewed changes

refactor : redirect URL 상대 경로로 설정

d60c205

java-saeng added 2 commits September 13, 2023 01:59

refactor : HttpResponse에 redirect 행위 캡술화

e8ec3a5

refactor : 핸들러가 존재하지 않을 때 exception 추가

c8f3e61

java-saeng commented Sep 12, 2023

View reviewed changes

0chil approved these changes Sep 13, 2023

View reviewed changes

0chil merged commit 92f744a into woowacourse:java-saeng Sep 13, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 - 3, 4단계] 우르(김현우) 미션 제출합니다. #481

[톰캣 구현하기 - 3, 4단계] 우르(김현우) 미션 제출합니다. #481

java-saeng commented Sep 11, 2023

0chil left a comment

0chil Sep 12, 2023

java-saeng Sep 12, 2023

0chil Sep 13, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023 •

edited

Loading

0chil Sep 13, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023

0chil Sep 12, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023

0chil Sep 13, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023

0chil Sep 12, 2023

0chil Sep 12, 2023

java-saeng Sep 12, 2023

sonarcloud bot commented Sep 12, 2023

java-saeng left a comment

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

java-saeng Sep 12, 2023

0chil left a comment

		private static final String SUCCESS_REDIRECT_URL = "http://localhost:8080/index.html";
		private static final String FAIL_REDIRECT_URL = "http://localhost:8080/401.html";

[톰캣 구현하기 - 3, 4단계] 우르(김현우) 미션 제출합니다. #481

[톰캣 구현하기 - 3, 4단계] 우르(김현우) 미션 제출합니다. #481

Conversation

java-saeng commented Sep 11, 2023

0chil left a 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

java-saeng Sep 12, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 12, 2023

java-saeng left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0chil left a comment

Choose a reason for hiding this comment

java-saeng Sep 12, 2023 •

edited

Loading