[톰캣 구현하기 - 3, 4단계] 케로(김지현) 미션 제출합니다. #480

jyeost · 2023-09-11T06:52:00Z

바론 -- !
늘 꼼꼼한 리뷰 감사합니다람쥐 ◠‿◠
1,2 단계에서 리팩 빡시게 해서 3단계에 큰 구조적 변화는 없습니다만...
그래도 파일 체인지는 많네유 ㅠㅠ

4단계 스레드 부분은 아직 잘 몰라서...
점차 알아나가면서 개선해가겠습니다....

(로컬에서는 잘 돌아가는데 ci시 STANDARD_OUT이 발생하여 주석 처리 된 테스트가 존재합니다)

somsom13

안녕하세요 케로! 바론입니다 🙇 🙇‍♀️

1, 2 단계 코드도 구조가 워낙 잘 잡혀있었어서 3단계에서 구조적으로 크게 변한점은 없고 코드가 더 깔끔해졌네용! 👍 👍

몇 가지 질문만 남겨두었습니당 ㅎ.ㅎ
너무 고생하셨습니다~~!! ❤️ 😍

somsom13 · 2023-09-11T10:11:39Z

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

+            httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
+            httpResponse.setRedirect("/index.html");
+            httpResponse.addCookie(Cookie.ofJSessionId(session.getId()));
+            return;


로그인에 성공하면 302 헤더와 함께 index.html 로 redirect 하도록 응답을 보내는 것 같아요!
요 상황에서 serViewPathAsBody 를 따로 설정하신 이유가 있나용?.?

헉 그러게요...
필요없이 response에 login.html을 담고 있었네여...
수정했습니다

somsom13 · 2023-09-11T10:12:32Z

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

-    private ResponseEntity loginPage(final Request request) {
-        final Session session = request.getSession(false);
+    private void loginPage(final HttpRequest httpRequest, final HttpResponse httpResponse) {
+        final Session session = httpRequest.getSession(false);


getSession(boolean) 에 들어가는 파라미터가 어떤 의미를 가지는 값인지 알 수 있을까요?.?

getSession의 메서드 파라미터 명을 수정하여 다시 작성했습니다.
도 실제 request session에서 처럼 true 일 때 기존 세션이 존재하지 않으면 새로 생성하고, false에서 존재하지 않으면 null을 반환하도록 수정했습니다

somsom13 · 2023-09-11T10:13:02Z

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

+            httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
+            httpResponse.setRedirect("/index.html");
+            return;


59번 라인의 코멘트와 동일합니다!

somsom13 · 2023-09-11T10:14:47Z

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

+        httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
+        httpResponse.setRedirect("/index.html");


위의 코멘트와 동일합니다 😄

somsom13 · 2023-09-11T10:16:00Z

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

+    private static final int DEFAULT_ACCEPT_COUNT = 30;
+    private static final int DEFAULT_MAX_THREAD = 30;


이렇게 제한을 두신 이유가 궁금하네용 ㅎㅎ 케로가 코멘트에 말씀해주신 것 처럼 스레드에 대해서 더 공부하시게 된다면... 저도 알려주세용 (@^0^@)/

일단 로컬에서 돌리고 있기 때문에 많은 스레드를 둘 필요가 없다고 판단해서 처음에 10개를 사용하였는데, ci에서 테스트가 깨져서 스레드 풀 개수 때문일가?? 하고 30까지 수정했던것 입니다....ㅎㅅㅎ

아직도 잘 모르겠어요 ...ㅠㅠ

사실 저도 잘 모르겠어용 ㅋㅋㅋㅋ 근데 지금 미션 상황에서는 케로 말씀대로 많은 accept count와 thread가 필요하지 않을 것 같아요!
(공부한 내용 공유해달라고 꼬드기는 코멘트 였습니당 ㅎㅋㅎㅎ)

somsom13 · 2023-09-11T10:21:27Z

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

+
+public class HttpResponse {
+
+    private final ResponseStartLine responseStartLine;


TMI) RFC 2616에 따르면 Response Message의 시작 라인은 StatusLine 이라고 부른다고 합니당

요건 수정 요청이 아니라 정말 그냥 TMI입니당 (ﾉ◕ヮ◕)ﾉ*:･ﾟ✧

somsom13 · 2023-09-11T10:25:26Z

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

        this.httpVersion = httpVersion;
-        this.status = status;
+        this.status = ResponseStatus.OK;


궁금!!! Response Status를 기본적으로 OK로 설정해두신 이유가 궁금해요~~~

Response의 기본 설정들이 setter로 바뀌면서 널 값을 방지하기 위하여 기본 값으로 Http 응답에 가장 많이 쓰이는 ok 상태코드로 지정해주었습니다 ㅎㅅㅎ

somsom13 · 2023-09-11T10:28:45Z

tomcat/src/main/java/org/apache/front/Proxy.java

+    private final StaticMapping staticMapping;

-    private final DynamicController dynamicController;
+    private final RequestMapping requestMapping;

    public Proxy() {
-        this.staticController = StaticController.singleTone();
-        this.dynamicController = DynamicController.singleTone();
+        this.staticMapping = new StaticMapping();
+        this.requestMapping = new RequestMapping();


궁금해서 남기는 질문입니당! Static Mapping과 RequestMapping 필드를 static이 아닌, Proxy 객체 생성 시에 함께 생성하는 이유가 무엇인지 궁금해요~!

1 Proxy - 1 Mapping으로 관리가 되어야 하는건가요?.?

1 Proxy - 1 Mapping 로 관리되지 않아도 될 것 같아서 처음에는 싱글톤 패턴으로 구현했다가,
리팩하는 과정에서 실수한 것 같습니다... ㅠㅠ

somsom13 · 2023-09-11T10:47:47Z

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

    @Override
-    public ResponseEntity handle(final Request request) {
-        return ResponseEntity.fromString(request, "Hello world!", ResponseStatus.OK);
+    protected void doGet(final HttpRequest request, final HttpResponse response) {
+        response.setStringAsBody("Hello world!");
    }


HelloWorldController에 doPost() 요청이 들어오면 어떻게 되나요?.?

피알 날리고 궁금해서 해봤는데 NPE 가 발생하더라고여...
이부분도 처리 처리되도록 수정하였습니다 !

somsom13

안녕하세요 케로~~~ 바론입니다~~~!!!!
빠른 코멘트 답변과 완벽한 수정,,, 박수갈채 보냅니다 👏 👏 👏

이번 미션의 요구 사항을 모두 만족하신 것 같아서 merge합니다 💯 👍

너무 고생하셨습니다~~ 다음 미션도 화이팅이에요!

somsom13 · 2023-09-12T08:11:41Z

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

+    @Override
+    protected void doPost(final HttpRequest request, final HttpResponse response) {
+        throw new MethodMappingFailException();
+    }
+


따봉 👍

somsom13 · 2023-09-12T08:13:12Z

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

-            final Session session = Session.create();
-            SessionManager.add(session);
-            return session;
+    public Session getSession(final boolean create) {


따따봉 👍

somsom13 · 2023-09-12T08:20:26Z

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

@@ -65,10 +65,10 @@ private void login(final HttpRequest httpRequest, final HttpResponse httpRespons
    }

    private void loginPage(final HttpRequest httpRequest, final HttpResponse httpResponse) {
-        final Session session = httpRequest.getSession(false);
+        final Session session = httpRequest.getSession(true);


케로의 getSession(true) 는 다음과 같은 흐름인 것 같은데,

이미 저장되어 있던 세션이라면 user 를 담은 세션이 반환되고

새롭게 생성된 세션이라면 user를 담고 있지 않는 세션을 반환한다.

loginPage 에서 로그인 여부에 따라 반환할 페이지를 다르게 설정할 때 getSession(false) 를 한 후, session == null 여부로 판단하지 않고, 항상 Session이 만들어지고 나서 그 내부의 Optional<User> 여부로 판단하도록 구성하신 이유가 궁금해용!!

sonarcloud · 2023-09-12T08:31:48Z

Kudos, SonarCloud Quality Gate passed!

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

jyeost added 5 commits September 11, 2023 00:49

study: 스레드 스터디 완료

895f85d

refactor: 3단계 리팩터링

7303cd6

refactor: http 개행을 CRLF로 수정

6f175a6

feat: Connector에서 매번 스레드를 생성하지 않고 스레드풀을 사용하도록 하였다.

b6350b8

feat: Connector에서 매번 스레드를 생성하지 않고 스레드풀을 사용하도록 하였다.

0e84058

jyeost requested a review from somsom13 September 11, 2023 06:52

jyeost self-assigned this Sep 11, 2023

jyeost added 3 commits September 11, 2023 16:07

refactor: 최대 스레드 개수 수정

1803462

refactor: 빌드 깨지는 것 수정

54ec71d

refactor: 테스트 깨지는 것 수정

7ce8966

somsom13 requested changes Sep 11, 2023

View reviewed changes

jyeost added 4 commits September 12, 2023 15:43

refactor: redirect시 view를 body로 지정하지 않도록 수정

4187e31

refactor: getSession 파라미터의 값에 따른 로직 수정

1ca0c3a

refactor: url에 맞지 않는 method로 요청이 들어오면 해당하는 예외를 던지도록 한다

8011ced

refactor: 각 매핑 클래스를 매번 새롭게 만들지 않도록 수정함

b79b279

somsom13 approved these changes Sep 12, 2023

View reviewed changes

refactor: 테스트 깨지는 것 수정

ec5fa7a

somsom13 merged commit b111f95 into woowacourse:jyeost Sep 12, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 - 3, 4단계] 케로(김지현) 미션 제출합니다. #480

[톰캣 구현하기 - 3, 4단계] 케로(김지현) 미션 제출합니다. #480

jyeost commented Sep 11, 2023 •

edited

Loading

somsom13 left a comment •

edited

Loading

somsom13 Sep 11, 2023

jyeost Sep 12, 2023

somsom13 Sep 11, 2023

jyeost Sep 12, 2023

somsom13 Sep 11, 2023

somsom13 Sep 11, 2023

somsom13 Sep 11, 2023

jyeost Sep 12, 2023

somsom13 Sep 12, 2023

somsom13 Sep 11, 2023

somsom13 Sep 11, 2023

jyeost Sep 12, 2023

somsom13 Sep 11, 2023

jyeost Sep 12, 2023

somsom13 Sep 11, 2023

jyeost Sep 12, 2023

somsom13 left a comment

somsom13 Sep 12, 2023

somsom13 Sep 12, 2023

somsom13 Sep 12, 2023

sonarcloud bot commented Sep 12, 2023

		httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
		httpResponse.setRedirect("/index.html");

		private static final int DEFAULT_ACCEPT_COUNT = 30;
		private static final int DEFAULT_MAX_THREAD = 30;


		public class HttpResponse {

		private final ResponseStartLine responseStartLine;

[톰캣 구현하기 - 3, 4단계] 케로(김지현) 미션 제출합니다. #480

[톰캣 구현하기 - 3, 4단계] 케로(김지현) 미션 제출합니다. #480

Conversation

jyeost commented Sep 11, 2023 • edited Loading

somsom13 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

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

somsom13 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

sonarcloud bot commented Sep 12, 2023

jyeost commented Sep 11, 2023 •

edited

Loading

somsom13 left a comment •

edited

Loading