[톰캣 구현하기 - 1, 2단계] 켬미(김경미) 미션 제출합니다. #555

kyum-q · 2024-09-06T08:33:22Z

안녕하세요 폭포! 😁 이렇게 리뷰이 리뷰어로 만나니 반갑네요 !

이번 PR에서는 1-2단계를 구현하였습니다.
기능은 어찌저찌 동작하지만 코드는 ... 만족스럽지 못하네요 🥲
그러니 마구마구 리뷰 남겨주세요~!

kyum-q

폭포 친절하고 자세한 리뷰 감사해요 !
그리고 폭포가 자신이 배운 점까지 적어줘서 저도 다시 공부해서 좋았어요 ! 학습, 질문, 제안 형식으로 나눠주니 리뷰를 보기도 더 편하네요 😊

리뷰 반영하고 나니 코드가 꽤 많이 개선된 것 같아요 ! 확인 부탁드려요 !

추가적으로 부탁하나 해도 될까요? 다음부터 리뷰 해줄 때 single 리뷰 말고 한번에 보내주시면 더 감사할 것 같아요 !

리뷰 시작 시 start review 클릭

리뷰 추가시 add review comment 클릭

리뷰 완료 시 Finish your review 클릭

kyum-q · 2024-09-08T10:12:35Z

README.md

 1. [File, I/O Stream](study/src/test/java/study)
 2. [HTTP Cache](study/src/test/java/cache)
 3. [Thread](study/src/test/java/thread)
+
+## 기능 요구 사항


레벨 2에서 놓쳤다가 리뷰어에게 잊지말라는 조언을 듣고 나서 기능요구사항을 작게라도 적는 편입니다 ~! 좋은 말 고마워용 !

kyum-q · 2024-09-08T10:17:12Z

study/src/main/java/cache/com/example/cachecontrol/CacheWebConfig.java

+        WebContentInterceptor interceptor = new WebContentInterceptor();
+        interceptor.addCacheMapping(CacheControl.noCache().cachePrivate(), "/**");
+        registry.addInterceptor(interceptor);


제 리뷰이 폴라도 폭포와 동일한 방식으로 했더라고요 ! 해당 방식도 너무 좋은 것 같고 장점이 명확한 것 같아요 !
그래서 제가 간단하게 찾아본 두 방식의 장단점입니다. ! (ChatGPT 많이 이용한 정리)

1. Custom HandlerInterceptor 방식

장점:

세밀한 제어 가능: HandlerInterceptor를 직접 구현하기 때문에 요청과 응답에 대해 더 구체적인 로직을 구현할 수 있음. 예를 들어 특정 경로에 따라 다르게 처리하거나, 조건에 따라 다른 헤더를 추가할 수 있음.
응답 헤더가 이미 설정된 경우 중복 방지: response.containsHeader를 사용해, 이미 Cache-Control 헤더가 있는지 확인한 후 처리하기 때문에 불필요한 덮어쓰기를 방지할 수 있음.

단점:

복잡한 관리: 모든 인터셉터 로직을 직접 작성해야 하기 때문에 코드가 길어질 수 있으며, 다른 설정과 결합하려면 더 많은 코드 작성이 필요.
일관성 유지 어려움: 여러 요청 유형에 대해 동일한 캐시 로직을 유지하려면 코드가 더 복잡해질 수 있음.

2. WebContentInterceptor 방식

장점:

설정의 간결함: Spring에서 제공하는 WebContentInterceptor와 CacheControl API를 사용하여 캐시 전략을 간단하게 설정할 수 있음. 여러 경로에 대한 캐시 제어를 손쉽게 매핑 가능.
일관된 설정: Spring의 설정 기반이기 때문에, 전체 애플리케이션에 걸쳐 동일한 캐시 제어 설정을 쉽게 적용 가능.
유지보수 용이: Spring의 설정 방식을 따르므로 다른 개발자들도 직관적으로 이해할 수 있음. 또한 캐시 전략을 변경할 때 코드가 간결하게 유지됨.

단점:

세밀한 제어 부족: 경로를 기반으로 캐시 전략을 적용하지만, 개별 요청에 대해 세밀하게 제어하기는 어려움. 특정 조건이나 상황에 따라 캐시 설정을 다르게 적용하고 싶다면, 추가적인 커스텀 로직을 작성해야 함.

요약

따라서, 애플리케이션 요구사항에 따라 선택하면 되지만, 간단한 설정을 원하는 경우에는 WebContentInterceptor 방식이 더 적합, 특정 요구사항이 있다면 Custom HandlerInterceptor 방식이 적합.

kyum-q · 2024-09-08T10:25:24Z

study/src/test/java/study/IOStreamTest.java

             * inputStream에서 바이트로 반환한 값을 문자열로 어떻게 바꿀까?
             */
-            final String actual = "";
+
+            final String actual = new String(inputStream.readAllBytes());


미션에 충실하게 read를 사용한 것도 좋네요~! 공유해줘서 고마워요 ! 폭포 덕분에 read 메서드에 대해 더 잘 알게 되었어요 👍🏻

kyum-q · 2024-09-08T10:26:18Z

study/src/test/java/study/IOStreamTest.java

+            String s = bufferedReader.readLine();
+            while (s != null && !s.isBlank()) {
+                actual.append(s).append("\r\n");
+                s = bufferedReader.readLine();
+            }


폭포가 한 번 더 정리해줘서 저도 다시 공부하네요 👍🏻

kyum-q · 2024-09-08T10:40:21Z

study/src/test/java/study/IOStreamTest.java

+            String s = bufferedReader.readLine();
+            while (s != null && !s.isBlank()) {
+                actual.append(s).append("\r\n");
+                s = bufferedReader.readLine();
+            }


무조건적으로 한 줄에 넣는 것은 안좋을 수 있지만 해당 방법은 가독성이 나쁘지 않으니 좋운 것 같아요! 반영하겠습니다 👍🏻

kyum-q · 2024-09-08T11:03:20Z

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

+        if (request.getQueryParam().size() < 2) {
+            return new ResponseContent(HttpStatus.BAD_REQUEST, accept, FileReader.loadFileContent(BAD_REQUEST_PAGE));
+        }
+        if (queryParams.get(ACCOUNT) == null || queryParams.get(PASSWORD) == null) {
+            return new ResponseContent(HttpStatus.BAD_REQUEST, accept, FileReader.loadFileContent(BAD_REQUEST_PAGE));
+        }


좋은 것 같아요~ 대신 if문이 너무 길어지니 따로 메서드 분리하였습니다 😁

kyum-q · 2024-09-08T11:25:02Z

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

+    private Optional<User> authenticateUser(String account, String password) {
+        Optional<User> user = InMemoryUserRepository.findByAccount(account);
+        if (user.isPresent() && user.get().checkPassword(password)) {
+            log.info("인증된 사용자: {}", user.get());
+        }
+        return user;
+    }


폭포의 의견대로 존재하는 아이디에 잘못된 비밀번호를 입력하면 통과되네요 ..!
해당부분은 체크하지 못했습니다. 해당 메서드에서 비밀번호까지 체크된 경우 user을 넘기고 아닌 경우 empty를 넘기도록 변경하겠습니다 ~!

kyum-q · 2024-09-08T11:29:32Z

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

+    private static ResponseContent generateResponseForPostUrl(Request headers) {
+        String url = headers.getUrl();
+        String accept = headers.getFileType();
+        if (REGISTER_PATH.equals(url)) {
+            return handleRegistration(headers.getBody(), accept);
+        }
+        return new ResponseContent(HttpStatus.BAD_REQUEST, accept, FileReader.loadFileContent(NOT_FOUND_PAGE));
+    }
+
+    private static ResponseContent handleRegistration(Map<String, String> bodyParams, String accept) {


없습니다.. ㅎㅎ 실수입니다 ㅎㅎ
수정하였습니다 !

kyum-q · 2024-09-08T11:35:58Z

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

+    private void handleRequestMethod(Request request) {
+        String httpMethod = request.getHttpMethod();
+        if (httpMethod.equals("GET")) {
+            handleGetRequest(request);
+        } else if (httpMethod.equals("POST")) {
+            handlePostRequest(request);
+        } else {
+            log.warn("지원되지 않는 HTTP 메서드: {}", httpMethod);
+        }
+    }

-            final var response = String.join("\r\n",
-                    "HTTP/1.1 200 OK ",
-                    "Content-Type: text/html;charset=utf-8 ",
-                    "Content-Length: " + responseBody.getBytes().length + " ",
-                    "",
-                    responseBody);
+    private void handleGetRequest(Request headers) {
+        try (final OutputStream outputStream = connection.getOutputStream()) {
+            String response = generateResponseForUrl(headers).responseToString();
+            outputStream.write(response.getBytes());
+            outputStream.flush();
+        } catch (IOException | UncheckedServletException e) {
+            log.error("GET 요청 처리 중 오류 발생: {}", e.getMessage(), e);
+        }
+    }


너무 좋은 방식인 것 같아요 저도 두번째 방법이 마음에 듭니다 ~! 추후 3단계에서 더 리펙토링 하도록 하겠습니다 !

kyum-q · 2024-09-08T11:48:56Z

tomcat/src/main/java/org/apache/catalina/response/ResponseContent.java

@@ -0,0 +1,49 @@
+package org.apache.catalina.response;
+
+public class ResponseContent {


너무 좋은 리펙터링 방식이네요 ~!
폭포의 의견을 너무 이해하기 쉽게 적어줘서 고마워요 ! 바로 이해했고 수정했습니다 !

BurningFalls · 2024-09-08T13:11:55Z

폭포 친절하고 자세한 리뷰 감사해요 ! 그리고 폭포가 자신이 배운 점까지 적어줘서 저도 다시 공부해서 좋았어요 ! 학습, 질문, 제안 형식으로 나눠주니 리뷰를 보기도 더 편하네요 😊

리뷰 반영하고 나니 코드가 꽤 많이 개선된 것 같아요 ! 확인 부탁드려요 !

추가적으로 부탁하나 해도 될까요? 다음부터 리뷰 해줄 때 single 리뷰 말고 한번에 보내주시면 더 감사할 것 같아요 !

리뷰 시작 시 start review 클릭

리뷰 추가시 add review comment 클릭

리뷰 완료 시 Finish your review 클릭

이런 방법이 있는지 몰랐네요! 가르쳐주셔서 감사합니다. 덕분에 배워가요 ㅎㅎ 👍

BurningFalls

제가 제안드린 사항들 빠르게 반영해주셨네요!

저도 켬미 코드를 리뷰하면서, 켬미가 정리해주신 내용들 보면서 많이 배워갈 수 있어서 좋았습니다. 😄
리뷰 방식은 알려주신 방식이 있는지 처음 알게 되었고, 다음부터 적용해보도록 하겠습니다. 감사합니다.

다른 리뷰들은 전부 해결되었고, 아래의 리뷰 하나만 생각해봐도 좋을 것 같습니다.
#555 (comment)

그리고 패스워드 문제는 해결되었지만, 앱을 실행해보니 아직 쿠키 및 세션 관련 문제가 완전히 해결되지 않은 것으로 보입니다. (해당 시나리오는 LMS에 있는 실행 결과 시나리오를 참고했습니다.)

localhost:8080/login 접속
회원가입 하러 가기 클릭
아이디 이메일주소 비밀번호 입력 후 가입하기
화면은 index.html 화면인데, url은 localhost:8080/register로 표시됨
localhost:8080/login으로 url 직접 입력으로 다시 접근
만든 아이디 비밀번호 정확히 입력해도 401. 이때 url을 보면 쿼리파라미터로 접근한 모습이 보여짐

기능 정상 작동을 위해, 이 부분들 한번 더 확인부탁드립니다!

kyum-q · 2024-09-08T15:28:47Z

폭포 리뷰 읽고 리다이렉션이 html 만 바뀌고 url은 바뀌지 않았다는 걸 알았네요..! ㅎㅎ
그리고 회원가입한 id/비밀번호로 로그인이 안됬던 이유는 확인해보니 비밀번호로 email을 저장했더라고요.. 이런 큰 실수를 ! 🤔😊
리뷰 반영해서 수정했습니다 !

BurningFalls

오류가 난 부분이나 변경사항에 대한 수정 속도가 정말 빠르다고 느껴져서 대단하신 것 같아요.

이제 정말_정말_진짜_마지막 최종_최종 수정사항 하나 남았습니다.

로그인된 상태에서 /login 페이지에 HTTP GET method로 접근하면 이미 로그인한 상태니 index.html 페이지로 리다이렉트 처리한다.

이 요구사항이 아직 지켜지지 않은 것 같습니다.

추가로 기능이 잘 작동하는지를 확인하기 위해, 다음 3~4단계에서는 테스트 코드를 작성해서 실수를 줄이는 것도 좋을 것 같아요! 그리고 테스트 코드 뿐만 아니라 웹에서 실제로 기능들이 잘 작동하는지 확인하는 것도 한 스푼 추가되면 더더욱 좋을 것 같습니다. 👍

BurningFalls · 2024-09-08T16:02:39Z

tomcat/src/main/java/org/apache/catalina/response/ResponseContent.java

@@ -9,11 +9,11 @@ public class ResponseContent {

    private final HttpStatus httpStatus;
    private final String body;
-    private final Map<String, String> headers = new HashMap<>();
+    private final Map<String, String> headers = new LinkedHashMap<>();


LinkedHashMap을 사용하면 넣은 순서대로 순서를 유지할 수 있네요! 딱 제가 필요한 자료구조였습니다. 덕분에 알게 되었습니다. 감사합니다! 😄

특징 HashMap LinkedHashMap

순서 보장 순서를 보장하지 않음 삽입된 순서 또는 접근 순서를 보장

내부 구현 해시 테이블 기반으로 구성 해시 테이블 + 이중 연결 리스트로 구성

성능 O(1) 시간 복잡도 (평균적으로 빠름) O(1) 시간 복잡도 (LinkedList로 인해 약간 느림)

메모리 사용 메모리 사용량이 적음 이중 연결 리스트로 인해 메모리 사용량이 더 많음

접근 순서 유지 지원하지 않음 옵션으로 접근 순서 유지 가능 (accessOrder=true 시)

사용 예 순서가 중요하지 않을 때 주로 사용 삽입 순서나 접근 순서가 중요한 경우에 사용

순회 방식 무작위 순서로 키와 값을 순회 삽입된 순서대로 키와 값을 순회

kyum-q · 2024-09-09T07:57:39Z

정말_정말_진짜_마지막 최종_최종 수정하였습니다 😁

테스트 코드 리펙터링 후에 넣으려고 했으나 매번 오류 찾기 힘들어서 최소한의 테스트 코드만 구현해두었습니다.
테스트 코드 및 구조에 관한 리펙터링은 3단계에서 제대로 해올게요 ~!

폭포가 말씀해주신 오류의 이유는 request에서 cookie를 찾을 때 Key가 Set-Cookie인 값이 있는지 확인해서 문제가 되었습니다..! 해당 코드를 Cookie로 변경하니 해결되었어요 !
애플리케이션 테스트도 해보고 구현한 테스트도 돌려봤습니다 ! 마지막으로 확인 부탁드려요 😊

BurningFalls

모든 클래스의 테스트를 엄청 상세하게 구현하셨네요! 저는 Http11ProcessorTest 정도만 작성해서 httpRequest에 대해 의도한 httpResponse가 오는 정도로만 확인했는데, 제가 반성해야 할 것 같습니다. 😓

그래도 Http11ProcessTest를 TDD 하는 것만으로도, 정확한 응답이 오는지 테스트로 체크가 가능해서 1~2단계를 구현할 때 좀더 원활했던 것 같습니다. 켬미가 지금 이렇게 테스트를 자세하게 만든 만큼, 3단계 리팩토링을 하면서 큰 도움이 될 거 같아요!!

모든 기능 정상 작동하는지 확인 완료했습니다. 테스트 코드는 3단계때 리팩토링 한다고 하셔서 따로 자세하게 확인하지 않았습니다. 정말 수고 많으셨습니다! 👍👍👍👍👍👍👍👍

kyum-q and others added 30 commits September 4, 2024 16:02

fix: javax -> jakarta

42e854f

test: 학습 테스트 구현

f9f7f9e

docs: 1단계 기능 요구 사항 작성

1470005

fix: 운영체제 줄바꿈 형식 문제로 테스트 수정

5ed2090

feat(Http11Processor): GET /index.html 응답 구현

6b20fd7

refactor(Http11Processor): header 정보 모두 읽도록 변경 및 response 정보 객체로 구현

a4d974f

feat(Http11Processor): Accept를 분석해서 반환 content-type 설정

99771c5

feat(Http11Processor): Query String을 파싱 후 아이디/비밀번호 일치 확인 및 로그 출력

3e33fe0

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

fed02f6

docs: 2단계 기능 요구 사항 목록 작성

2a5ed97

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

7e91356

refactor(Http11Processor): 헤더 읽는 메서드 분리

d0479d9

refactor(Http11Processor): 상수 처리

27af449

refactor(Http11Processor): 반환 타입 찾는 메서드 개선

7e7c647

refactor(Http11Processor): 에러 메시지 구체화

74e0967

refactor(ResponseContent): response를 문자열로 변환하는 메소드 구현

53fb3f7

feat(ResponseContent): 로그인 성공 시, http status code 302 반환

3cf2d79

feat(ResponseContent): 로그인 성공 시, /index.html로 리다이렉트

a065e6e

feat(ResponseContent): 로그인 실패 시, /401.html 로 리다이렉트

191981c

feat(resource): 400.html 추가 및 html 타이틀 수정

82ce1bd

feat(Http11Processor): 400 에러 오류 페이지 로드

639b9b0

docs: HTTP Status Code 302 : 로그인 페이지에서 로그인 시도에 따른 응답 처리 체크

e06f0a4

feat(resource): 401.html 수정

769d4cf

refactor(Http11Processor): login url 입력 시 로그인 페이지 로드

796e937

refactor(FileResponseReader): 파일 로드 메서드 분리

5ce6943

feat(ResponsePage): url에 따른 페이지 로드 구현

39d9565

feat(ResponsePage): 회원가입 페이지 로드

4984581

refactor(FileReader): 클래스 명 변경

b4414fc

refactor(Http11Processor): 파일 체크와 페이지 로드 메서드 분리

b849401

refactor(Http11Processor): file type 확인 로직 이동

887a2a5

kyum-q added 8 commits September 8, 2024 19:45

refactor(HttpCookie): 접근제어자 변경

26647a8

refactor(Session): 세션에 값이 존재하지 않을 경우 예외 처리

2862b60

refactor(ResponsePage): stream 사용하여 코드 개선

5d4361d

refactor(Http11Processor): 로그인 시 파라미터 검증 메서드 분리

f9d92fb

fix(Http11Processor): 로그인 시 패스워드 검증 로직 수정

2980a0d

refactor(Http11Processor): 필요 없는 static 제거

f7bc1b3

refactor(Http11Processor): 중복된 코드 제거를 위해 코드 출력 로직을 process 메서드로 이동

fac6b90

refactor(ResponseContent): 헤더 Map 사용으로 추가정보를 유동적으로 처리

edd193d

kyum-q commented Sep 8, 2024

View reviewed changes

BurningFalls requested changes Sep 8, 2024

View reviewed changes

kyum-q added 3 commits September 8, 2024 23:29

fix(Http11Processor): 쿠키 유틸 클래스에서 클래스로 변경 및 회원가입 오류 해결

44f05f0

fix(ResponseContent): header 출력에 순서에 따른 테스트 오류 해결

b2f439c

fix(Http11Processor): 페이지 리다이렉션 문제 해결

1ca753e

BurningFalls requested changes Sep 8, 2024

View reviewed changes

kyum-q added 11 commits September 9, 2024 13:22

refactor(HttpCookie): 메서드 명 변경

a7475b9

test: 세션 관련 테스트 구현

84e74d4

test: 파일 Reader 테스트 구현

8c02411

test: Request 테스트 구현

7912f4a

refactor(Response): 클래스 명 변경

c9f2324

test: ResponsePage 테스트 구현

e34499c

fix(Request): 쿠키 얻어오는 로직 문제 해결

3a16f64

test: processor 테스트 코드 구현

4b2510d

test: test 에러 수정

58222c2

test: @DisplayName 추가

383b9d0

refactor: 코드 컨벤션 정리

12acb9c

BurningFalls approved these changes Sep 9, 2024

View reviewed changes

BurningFalls merged commit c9a961e into woowacourse:kyum-q Sep 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 - 1, 2단계] 켬미(김경미) 미션 제출합니다. #555

[톰캣 구현하기 - 1, 2단계] 켬미(김경미) 미션 제출합니다. #555

kyum-q commented Sep 6, 2024

kyum-q left a comment

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

kyum-q Sep 8, 2024

BurningFalls commented Sep 8, 2024 •

edited

Loading

BurningFalls left a comment

kyum-q commented Sep 8, 2024

BurningFalls left a comment

BurningFalls Sep 8, 2024

BurningFalls Sep 9, 2024

kyum-q commented Sep 9, 2024

BurningFalls left a comment

		@@ -0,0 +1,49 @@
		package org.apache.catalina.response;

		public class ResponseContent {

특징	HashMap	LinkedHashMap
순서 보장	순서를 보장하지 않음	삽입된 순서 또는 접근 순서를 보장
내부 구현	해시 테이블 기반으로 구성	해시 테이블 + 이중 연결 리스트로 구성
성능	`O(1)` 시간 복잡도 (평균적으로 빠름)	`O(1)` 시간 복잡도 (LinkedList로 인해 약간 느림)
메모리 사용	메모리 사용량이 적음	이중 연결 리스트로 인해 메모리 사용량이 더 많음
접근 순서 유지	지원하지 않음	옵션으로 접근 순서 유지 가능 (`accessOrder=true` 시)
사용 예	순서가 중요하지 않을 때 주로 사용	삽입 순서나 접근 순서가 중요한 경우에 사용
순회 방식	무작위 순서로 키와 값을 순회	삽입된 순서대로 키와 값을 순회

[톰캣 구현하기 - 1, 2단계] 켬미(김경미) 미션 제출합니다. #555

[톰캣 구현하기 - 1, 2단계] 켬미(김경미) 미션 제출합니다. #555

Conversation

kyum-q commented Sep 6, 2024

kyum-q left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1. Custom HandlerInterceptor 방식

장점:

단점:

2. WebContentInterceptor 방식

장점:

단점:

요약

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

BurningFalls commented Sep 8, 2024 • edited Loading

BurningFalls left a comment

Choose a reason for hiding this comment

kyum-q commented Sep 8, 2024

BurningFalls left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyum-q commented Sep 9, 2024

BurningFalls left a comment

Choose a reason for hiding this comment

BurningFalls commented Sep 8, 2024 •

edited

Loading