[톰캣 구현하기 - 1,2단계] 엔델(김민준) 미션 제출합니다. #331

SproutMJ · 2023-09-04T06:51:21Z

안녕하세요 가비, 엔델입니다.

HTTP에 대한 이해도가 부족하다보니 미션 구현에 집중했던 것 같아요. 마침 3단계가 리팩토링이기도 하니 메서드 분리를 최대한 하는 방향으로 진행했습니다.

흐름은 아래와 같습니다.

Http11Processor클래스의 process 메서드에서 시작합니다.
BufferedReader를 통해 requestLine, header, requestBody를 파싱합니다.
requestLine에서 다시 method, path, queryString을 파싱합니다.
controller에서 요청을 처리하고 response를 생성합니다.

4-1. if 분기를 통해 method와 path에 따라 다르게 처리합니다.
4-2. redirect인 경우, result를 만드는 경우(현재의 경우 파일을 전송하는 경우) generateResult, 단순히 string만 보내는 경우( 현재의 경우 "Hello world!" 전송하는 경우) generateResponseBody에 따라 다른 결과를 만듭니다. 이때 쿠키가 들어가는 경우에 따라 메서드가 오버로딩 되어있습니다.

outputStream.write를 합니다.

현재 가장 큰 문제는 구현에 집중하기 위해 Cookie, Session 등을 제외한 클래스 분리가 되어있지 않은 것입니다.
특히 Header나 Cookie가 Optional하게 들어가는 경우가 많은데 이를 위하다보니 코드가 많이 복잡해진 것 같습니다.
이점 참고해주시면 감사하겠습니다.

1. GET /index.html 응답하기 2. CSS 지원하기 3. Query String 파싱

1. HTTP Status Code 302 2. POST 방식으로 회원가입 3. Cookie에 JSESSIONID 값 저장하기 4. Session 구현하기

iamjooon2

안녕하세요 엔델🙇🏻‍♂️

코드가 일관성있고, 작성해주신 코멘트 덕에 수월하게 읽을 수 있었습니다👍
구조는 이미 언급해주시기도 했고, step3가 있으니 찬찬히 고민하셔도 될 것 같습니다!
덕분에 사소한 부분들에 코멘트를 남기게 된 것 같네요!
여유가 되신다면 테스트를 작성하시면 더욱 좋을 것 같습니다~

미션 고생 많으셨습니다!

iamjooon2 · 2023-09-04T11:40:05Z

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

@@ -28,20 +70,185 @@ public void run() {
    public void process(final Socket connection) {
        try (final var inputStream = connection.getInputStream();
             final var outputStream = connection.getOutputStream()) {
+            final BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));


BufferedReader도 try-with-resource 구문 안으로 넣어서 사용 후 자원종료를 해줄 수 있을 것 같아요!

생각하고 있었는데 미처 적용하지 못했네요 감사합니다~

iamjooon2 · 2023-09-04T13:01:57Z

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

+        return String.join("\r\n",
+                "HTTP/1.1 " + HTTP_STATUS.get(statusCode) + " ",
+                "Set-Cookie: " + cookie.generateCookieHeaderValue() + " ",
+                "Location: " + location + " ");


공백, 개행문자, 헤더 key가 다양한 곳에서 쓰일 수 있을 것 같아서 상수화 할 수 있을 것 같습니다

일단은 헤더의 Key 부분은 HttpHeaders 클래스를 만들어 상수화했습니다.
추가로 캡슐화를 하고 싶은데 이 부분은 ResponseBody를 리팩토링 하면서 같이 하면 좋을 것 같아서 남겨두었습니다.
지금 당장 이부분만 하려니 엄두가 안나네요

iamjooon2 · 2023-09-04T13:54:53Z

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

+        final Map<String, String> contentType = new HashMap<>();
+        contentType.put("html", "text/html;charset=utf-8");
+        contentType.put("css", "text/css");
+        contentType.put("js", "application/javascript");
+        contentType.put("ico", "image/x-icon");
+        CONTENT_TYPE = Collections.unmodifiableMap(contentType);
+        final Map<String, String> httpStatus = new HashMap<>();
+        httpStatus.put("OK", "200 OK");
+        httpStatus.put("CREATED", "201 CREATED");
+        httpStatus.put("FOUND", "302 FOUND");
+        httpStatus.put("NOT_FOUND", "404 NOT FOUND");
+        httpStatus.put("UNAUTHORIZED", "401 UNAUTHORIZED");
+        HTTP_STATUS = Collections.unmodifiableMap(httpStatus);


enum이라는 선택지도 있었을 텐데, Map을 사용하신 이유를 여쭙고 싶습니다!

일단 직관적인 구현을 위해 이렇게한 것 같습니다.
현재는 두 ContentType, HttpStatus를 enum화 했습니다.

iamjooon2 · 2023-09-04T14:07:59Z

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

        } catch (IOException | UncheckedServletException e) {
            log.error(e.getMessage(), e);
        }
    }
+
+    private String controller(final Map<String, String> httpRequestHeader, final String method, final String path, final Map<String, String> queryString, Map<String, String> requestBody) throws IOException {


마지막 requestBody만 final이 빠져있네요! ( 달게 없을 때 적는 비겁한 리뷰입니다 )

붙였습니다!

iamjooon2 · 2023-09-04T14:26:13Z

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

+    private String generateResult(final String path, final String statusCode) throws IOException {
+        return generateResult(path, statusCode, Cookie.newInstance());
+    }
+
+    private String generateResult(final String path, final String statusCode, final Cookie cookie) throws IOException {


쿠키를 담아 보낼 경우 generateResult가 아닌 generateRedirect를 사용중이군요!
두 메서드를 합칠 수 있을 것 같습니다!

현재 구현에 어려움이 있어서
쿠키를 보내면서 보내는 경우 / 쿠키를 보내지 않으면서 보내는 경우 / 쿠키를 보내면서 리다이렉트하는 경우 / 쿠키를 보내지 않으면서 리다이렉트하는 경우
총 네가지의 메서드로 response 결과를 만듭니다.
이 과정이 복잡해서 역시 리펙토링 대상인데요, 현재는 Header를 최종적으로 만들어서 response에 전달하는 방식으로 해볼까 생각중입니다. 다만, 3단계때 한꺼번에 하는 것은 어떤지 여쭤봐도 될까요?

넵 좋습니다!!

iamjooon2 · 2023-09-04T14:27:44Z

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

+            return "";
+        }
+        final Path staticFilePath = new File(resource.getPath()).toPath();
+        final int lastDotIndex = staticFilePath.toString().lastIndexOf(".");


lastIndexOf()...! 배워갑니다👀

iamjooon2 · 2023-09-04T14:38:32Z

tomcat/src/main/resources/static/register.html

@@ -20,7 +20,7 @@
                                <div class="card shadow-lg border-0 rounded-lg mt-5">
                                    <div class="card-header"><h3 class="text-center font-weight-light my-4">회원 가입</h3></div>
                                    <div class="card-body">
-                                        <form method="post" action="register">
+                                        <form method="post" action="/register">


남길 곳이 마땅치 않아서 이곳에 남깁니다

2단계 요구사항 중 로그인 페이지도 버튼을 눌렀을 때 GET 방식에서 POST 방식으로 전송하도록 변경하자. 가 있었는데요
요 부분을 깜빡 하신 것 같습니다. 한번 확인부탁드립니다!

어쩐지 뭔가 이상하다고 생각했었는데 미처 적용하지 못했네요
페이지를 불러오는 경우에는 GET, 정보를 입력하고 버튼을 누르면 POST를 사용해 요청하도록 구현했습니다.

iamjooon2 · 2023-09-04T14:48:20Z

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

+            final String requestBody = new String(buffer);
+            Arrays.stream(requestBody.split("&")).forEach(query -> {
+                final String[] keyValue = query.split("=");
+                final String decodedValue = URLDecoder.decode(keyValue[1], StandardCharsets.UTF_8);


인코딩까지 꼼꼼하게 고려해주셨군요👍

iamjooon2 · 2023-09-04T14:58:14Z

tomcat/src/main/java/org/apache/cookie/Cookie.java

+    public static Cookie newInstance() {
+        return new Cookie(new HashMap<>());
+    }


미리 생성해둔 객체를 사용하면 좀 더 효율적이지 않을까라는 생각이 들었습니다! (사실 저도 놓친 부분입니다😂)
엔델은 어떻게 생각하시나요?

제가 미처 이해를 정확히 하지 못했는데요, 개별적인 빈 캐시를 만들기 위해 만든 메서드입니다.
일단 직관적인 네이밍이 아니어서 empty()로 변경해보았습니다.

이 부분은 제가 잘못봤네요!
해당 메서드를 통해 빈 쿠키를 반환하는 데만 사용하는 것으로 보고 단 코멘트입니다
혼란을 드려서 죄송합니다🙇🏻‍♂️

sonarcloud · 2023-09-05T11:45:27Z

Kudos, SonarCloud Quality Gate passed!

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

iamjooon2

엔델 고생 많으셨습니다!
다음 단계에서 만나시죠🚀

SproutMJ added 2 commits September 2, 2023 18:58

feat: 1단계 - HTTP 서버 구현하기

cba06e0

1. GET /index.html 응답하기 2. CSS 지원하기 3. Query String 파싱

feat: 2단계 - 로그인 구현하기

9ee956c

1. HTTP Status Code 302 2. POST 방식으로 회원가입 3. Cookie에 JSESSIONID 값 저장하기 4. Session 구현하기

SproutMJ requested a review from iamjooon2 September 4, 2023 06:51

SproutMJ self-assigned this Sep 4, 2023

test: 테스트를 통과하도록 수정

2b2558a

iamjooon2 requested changes Sep 4, 2023

View reviewed changes

SproutMJ added 8 commits September 5, 2023 15:29

test: 학습 테스트 작성

d52ab59

refactor: BufferedReader를 try with resource에 넣기

a75ccc1

refactor: HttpHeaders를 클래스로 변경

3925657

refactor: HttpStatus를 열거로 변경

f3cfc6f

style: final 추가

3e77dcb

feat: 로그인을 post 방식으로 변경

d9ffa6d

refactor: enum의 valueOf를 of로 네이밍 변경, ContentType을 enum으로 변경

9d87621

refactor: 빈 Cookie인 경우 .empty() 메서드를 사용해 생성하도록 메서드 네이밍 변경

e99c5fe

iamjooon2 approved these changes Sep 5, 2023

View reviewed changes

iamjooon2 merged commit 5b5ee3f into woowacourse:sproutmj Sep 5, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 - 1,2단계] 엔델(김민준) 미션 제출합니다. #331

[톰캣 구현하기 - 1,2단계] 엔델(김민준) 미션 제출합니다. #331

SproutMJ commented Sep 4, 2023

iamjooon2 left a comment

iamjooon2 Sep 4, 2023

SproutMJ Sep 5, 2023

iamjooon2 Sep 4, 2023

SproutMJ Sep 5, 2023

iamjooon2 Sep 4, 2023

SproutMJ Sep 5, 2023

iamjooon2 Sep 4, 2023 •

edited

Loading

SproutMJ Sep 5, 2023

iamjooon2 Sep 4, 2023

SproutMJ Sep 5, 2023

iamjooon2 Sep 5, 2023

iamjooon2 Sep 4, 2023

iamjooon2 Sep 4, 2023

SproutMJ Sep 5, 2023

iamjooon2 Sep 4, 2023

iamjooon2 Sep 4, 2023 •

edited

Loading

SproutMJ Sep 5, 2023

iamjooon2 Sep 5, 2023 •

edited

Loading

sonarcloud bot commented Sep 5, 2023

iamjooon2 left a comment

[톰캣 구현하기 - 1,2단계] 엔델(김민준) 미션 제출합니다. #331

[톰캣 구현하기 - 1,2단계] 엔델(김민준) 미션 제출합니다. #331

Conversation

SproutMJ commented Sep 4, 2023

iamjooon2 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

iamjooon2 Sep 4, 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

iamjooon2 Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamjooon2 Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Sep 5, 2023

iamjooon2 left a comment

Choose a reason for hiding this comment

iamjooon2 Sep 4, 2023 •

edited

Loading

iamjooon2 Sep 4, 2023 •

edited

Loading

iamjooon2 Sep 5, 2023 •

edited

Loading