[1,2단계 - Tomcat 구현하기] 로키(정해성) 미션 제출합니다. #564

HaiSeong · 2024-09-06T08:43:38Z

초롱 안녕하세요! 🙇‍♂️

어쩌다 보니 조금씩 리펙토링 하며 코드를 짜서 3단계를 진행해버린것 같아요. 🔨
코드량이 좀 많습니다. 😂

리뷰 잘 부탁드립니다! 🤗

HoeSeong123

안녕하세요 로키!! 초롱입니다.

리뷰를 하러 왔는데 의도치 않게 공부만 잔뜩 하고 가버리게 됐네요..
각 객체별로 역할에 맞게 구분이 잘 되어 있는 것 같고, 전반적으로 코드가 너무 깔끔해서 실제 사용되는 코드인 줄 알았습니다.

따끔한 리뷰를 해드리고 싶었지만 역량 부족으로 간단한 리뷰밖에 남기지 못했네요.
한 번 읽어보시고 반영할지는 로키가 정해주시면 될 것 같아요!!

한 가지 궁금한 점이 있는데, 코드가 너무 잘 짜여있는 것 같아서 어떤 식으로 공부를 하신 건지 너무 궁금합니다.
LMS에 있는 정보만으로는 이정도로 세세하게 각 도메인들의 역할을 적절하게 나누기 힘들었을 것 같은데 이전에 해본 적이 있는 건지, 아니라면 어떤 방식으로 공부하신 건지 너무나도 궁금합니다

HoeSeong123 · 2024-09-07T08:02:31Z

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

@@ -96,6 +109,7 @@ class OutputStream_학습_테스트 {
             * try-with-resources를 사용한다.
             * java 9 이상에서는 변수를 try-with-resources로 처리할 수 있다.
             */
+            outputStream.close();


주석에 안내된대로 try-with-resources를 사용하는 방법도 생각해보시면 좋을 것 같습니다!

그렇네요 주석에 애초에 try-with-resources 를 사용하라고 명시되어 있었군요! 수정하겠습니다!

HoeSeong123 · 2024-09-07T08:08:23Z

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

+            byte[] readBytes = new byte[4];
+
+            int i = 0;
+            while (true) {
+                int read = inputStream.read();
+                if (read == -1) {
+                    break;
+                }
+                readBytes[i] = (byte) read;
+                i++;
+            }
+            final String actual = new String(readBytes);


저는 inputStream.readAllBytes()를 사용했는데 코드가 굉장히 단순해졌습니다!! 강추드립니다ㅎㅎ

이 코드는 read 메서드가 -1 을 리턴하는걸 눈으로 확인해보고 싶어서 의도적으로 반복문으로 작성하게 되었습니다!

처음에 "왜 read가 byte가 아닌 int를 리턴하는거지?" 라는 궁금증에서 코드의 구현을 찾아보게 되었습니다.

The value byte is returned as an int in the range 0 to 255. If no byte is available because the end of the stream has been reached, the value -1 is returned.

기본적으로 read는 스트림에서 바이트를 읽어 0 ~ 255의 값을 리턴해주고 더이상 읽을게 없을때는 -1을 리턴해주도록 구현되었습니다. 이때 -1 ~ 255를 담기에는 byte는 부족하기 때문에 read가 byte가 아닌 int를 리턴하고 불편하게 형변환을 해줘야 하는것을 알게되었습니다!

그래서 이 코드는 유지하고 싶습니다!

좋은 정보 감사합니다!! 로키의 학구열이 정말 대단하네요 👍

학습용 코드인만큼 로키가 원하는 대로 두어도 좋을 것 같습니다!

HoeSeong123 · 2024-09-08T12:45:22Z

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

-            final var responseBody = "Hello world!";
+        try (final var inputStreamReader = new InputStreamReader(connection.getInputStream());
+             final var outputStreamWriter = new OutputStreamWriter(connection.getOutputStream())) {
+            InputView inputView = new InputView(new BufferedReader(inputStreamReader));


BufferedReader도 close를 시켜줘야 하는 클래스인데, 자원이 낭비되지 않도록 try-with-resource 안에 포함시켜보는 것은 어떤가요?

BufferedReader와 InputStreamReader 모두 Closeable 인터페이스를 구현하고 있기 때문에, InputStreamReader가 닫히면 그와 연결된 BufferedReader도 자동으로 닫히게 된다고 들어 명시적으로 try-with-resource에 추가하지는 않았습니다. 😀

초롱이 보기에는 명시적으로 추가해주는게 좋아보이실까요!!?

와 연결된 것까지 자동으로 닫히는 것은 처음 알았네요!!

흠.. 어려운 문제네요. 저라면 명시적으로 추가해줄 것 같기는 합니다. 저처럼 이런 사실을 모르는 사람들도 있을 것 같고, try-with-resource에 추가하지 못 할 만한 코드로는 보이지 않아서요!!
하지만 반대로 성능적인 문제가 있는 것도 아니고, 반드시 닫아야 하는 자원들만 try-with-resource에 추가한다는 관점으로 본다면 오히려 추가하는 것이 어색하게 느껴질 수도 있을 것 같네요.

이 부분은 로키의 선택에 맡기겠습니다!!

HoeSeong123 · 2024-09-08T13:19:26Z

tomcat/src/test/java/com/techcourse/service/UserServiceTest.java

+    @Test
+    @DisplayName("로그인을 한다.")
+    void login() {
+        UserService userService = new UserService();
+
+        UserResponse userResponse = userService.login("gugu", "password");
+
+        assertThat(userResponse.email()).isEqualTo("[email protected]");
+    }


UserServiceTest에서 컴파일 오류가 발생합니다!! UserResponse가 없어진 클래스인 것 같네요.
확인 부탁드립니다!

이부분은 제가 잘 확인하고 제출했어야하는데 죄송해요!!
🙇‍♂️🙇‍♂️🙇‍♂️🙇‍♂️

HoeSeong123 · 2024-09-08T13:43:18Z

tomcat/src/main/java/org/apache/coyote/http11/domain/request/RequestHeaders.java

+    private Map<String, String> parseHeaders(List<String> headerLines) {
+        return headerLines.stream()
+                .filter(header -> header.contains(":"))
+                .collect(Collectors.toMap(
+                        header -> header.substring(0, header.indexOf(":")).trim(),
+                        header -> header.substring(header.indexOf(":") + 1).trim())
+                );
+    }


해당 코드는 :를 포함하는 것들만 필터링하여 파싱을 진행하는 것으로 보이는데 만약 :가 없는 문자열이 포함된다면 어떻게 되는지 궁금합니다.

이외에도 거의 모든 파싱 과정에서 필터링을 진행하셨던데 예외처리 대신 이렇게 구현하신 이유가 있을까요??

오 제가 고민했었던 부분인데 정확히 질문주셔서 기쁘네요!

처음 구현을 할때는 실제 톰캣의 동작과 최대한 비슷하게 구현하고 싶은 생각이 들어 실제 톰캣에 여러가지 이상한 요청을 보내보며 어떤식으로 구현할지 고민해봤습니다.

그러나 그렇게 구현하기는 쉽지 않다는것을 느꼈고 그럴 필요도 없다고 생각되었습니다. 왜냐하면 우리 과제의 목표는 http header를 파싱하는 연습을 하는게 아니라 톰켓의 동작 방식을 학습하는것이니까요!

당연히 Header가 형식에 맞춰서 :으로 잘 구분되어서 오는 경우가 당연히 베스트라고 생각합니다! 만약 클라이언트에서 Http 규약에 맞지 않게 보냈을때 예상한 결과가 나오지 않는것은 당연한일입니다.

그러나 경험상 제가 이용한 웹서비스에서는 제가 형식을 잘못준 요청에 대해서 예외를 던지기 보다는 예상하지 않은 결과로 자연스럽게 처리되는 것 같아 가장 쉬우면서 자연스럽게 처리되는식으로 구현했습니다.

아래는 톰켓이 내장된 스프링 서버를 띄우고 curl로 요청을 보낸 경우입니다.

curl -H "Header-Name" http://localhost:8080

헤더에 : 없이 요청을 날려도 예외를 던지기 보다 정상적인 요청을 보냅니다.

<!DOCTYPE html> <html lang="ko"> <head> <meta charset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <title>방탈출 예약 페이지</title> ...

만약 클라이언트에서 Http 규약에 맞지 않게 보냈을때 예상한 결과가 나오지 않는것은 당연한일입니다.

이 부분이 굉장히 공감이 많이 가는 말이네요. 로키의 말대로 오히려 예외처리 되는 것이 어색한 것 같아요. 저도 잘못된 요청에 대한 처리를 어떻게 해야 할 지 고민이 많았는데 많은 도움이 됐습니다!!
로키의 고민이 많이 담긴 코드인 것 같아서 좋네요 😁

HoeSeong123 · 2024-09-08T15:06:01Z

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

+    }
+
+    private List<String> readHttpHeaders(InputView inputView) throws IOException {
+        ArrayList<String> headerLines = new ArrayList<>();


사소한 거긴 한데 List가 아닌 ArrayList로 선언하신 이유가 있나요?

앗 그렇네요! List로 선언해도 될것 같아요! 👍👍

HoeSeong123 · 2024-09-08T15:11:04Z

tomcat/src/main/java/org/apache/coyote/http11/domain/controller/AbstractController.java

+    public AbstractController() {
+        methods.put(HttpMethod.GET, this::doGet);
+        methods.put(HttpMethod.POST, this::doPost);
+        methods.put(HttpMethod.HEAD, this::doHead);
+        methods.put(HttpMethod.OPTIONS, this::doOptions);
+        methods.put(HttpMethod.PUT, this::doPut);
+        methods.put(HttpMethod.DELETE, this::doDelete);
+        methods.put(HttpMethod.TRACE, this::doTrace);
+        methods.put(HttpMethod.CONNECT, this::doConnect);
+    }


👍👍👍👍👍👍

HoeSeong123 · 2024-09-08T15:21:30Z

tomcat/src/main/java/org/apache/coyote/http11/domain/controller/RequestMapping.java

+    public boolean canHandle(HttpRequest request) {
+        return controllers.containsKey(request.getPath());
+    }


해당 메서드는 사용되지 않는 것으로 보이는데 추후에 활용될 가능성을 염두에 두고 일부러 남겨두신 건가요??

그렇네요! 당장 불필요한 메서드인데 삭제하겠습니다!

HoeSeong123 · 2024-09-08T15:32:26Z

tomcat/src/main/java/org/apache/coyote/http11/domain/request/RequestURI.java

+
+public class RequestURI {
+
+    private static final String QUESTION_MARK = "?";


다른 상수들은 해당 문자가 쓰인 의미로 네이밍을 진행했는데 해당 상수만 문자의 이름으로 네이밍을 한 것 같네요. 그렇게 하신 이유가 있을까요?

없으시다면 QUERY_DELIMITER와 같이 쓰인 의미로 이름을 변경해보면 어떤가요?

QUERY_DELIMITER 이름 좋네요!
반영하겠습니다!

HoeSeong123

고생하셨습니다 로키!! 피드백 반영도 굉장히 빠르고 좋네요.
그리고 코드 하나하나에 많은 고민이 담겨있는 것 같아서 너무 좋았어요.
특히 톰캣에 이런저런 요청을 직접 보내보면서 어떻게 구현되어있는지 확인하는 점도 정말 놀라웠는데, 그 결과를 그대로 따르지 않고 여기에 자신의 생각을 입혀서 재해석하는 과정이 굉장히 인상적이었습니다.

피드백 반영 및 테스트 코드 정상 동작 모두 확인했으니 이만 merge 하도록 하겠습니다!!
다음 단계도 화이팅입니다.

HaiSeong and others added 30 commits September 3, 2024 20:31

feat: FileTest, IOStreamTest 학습

2fdaaed

fix: replace javax with jakarta on imports (woowacourse#500)

1bc0585

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

86e3553

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

568473a

feat: 학습테스트 2 완료

a9a8d1c

feat: HttpMethod 구현

a406857

feat: RequestLine 구현

15630b9

feat: HttpRequest 구현

ffb752f

refactor: RequestLine split 스페이스 상수 사용

8f04610

package: HttpMethod package 변경

ce29ef8

refactor: HttpRequest I/O 리소스 커넥션 유지

aa9f230

fear: HttpStatus 구현

2c2cc55

feat: HttpResponse 구현

9cd43dc

feat: HttpResponseDto 생성

8ce1de9

feat: OutputView 구현

02b2ffe

feat: GetMethodHandler 구현

4c961d4

refactor: response의 header 순서가 변경되지 않도록 변경

b51bfd0

feat: Http11Processor static 페이지 get 요청 구현

c1a51ac

feat: ContentTypeResolver 구현

fa8c853

refactor: GetMethodHandler contentType 관련 로직 개선

52d4a30

refactor: InputView 로직 분리

cdc20e4

package: 패키지 구조 변경

7a790b1

refactor: StaticResourceHandler 으로 이름 변경

77d7291

package: 패키지 구조 변경

477e546

feat: queryParameter 파싱 구현

654133a

feat: queryParameter 안맞는 형식 파싱

267ae7c

test: RequestURITest 순서 보장 상관없게 변경

d832ac5

feat: AbstractController 구현

ccce58e

feat: RequestMapping 구현

935d338

test: 필요없는 테스트 삭제

3afa906

HaiSeong added 17 commits September 6, 2024 14:54

feat: RequestMapping 연결

4cbebe8

package: 패키지 구조 변경

dc706a8

refactor: Http11Processor에 StaticResourceHandler 주입

76db2ee

refactor: AbstractController 에 StaticResourceHandler 주입

84130db

feat: redirect 기능 구현

d3a3d4b

fix: 대문자 오타 수정

b6811e4

feat: RequestHeaders 구현

802a570

feat: RequestMessage 구현

f689338

refactor: OutputStreamWriter 사용

4d74cc6

refactor: 루트로 접근시 index.html로 리다이렉트

7d95bb1

feat: LoginService 구현

59782a6

feat: LoginController 구현

5590b54

feat: RegisterController GET 요청 구현

e48845c

feat: LoginService 을 UserService 으로 이름 변경

90bb930

feat: 회원가입 기능 구현

f13ed6a

feat: 쿠키 로직 구현

4a8f414

feat: Session 구현

7963457

HaiSeong requested a review from HoeSeong123 September 6, 2024 08:43

HaiSeong self-assigned this Sep 6, 2024

HaiSeong changed the title ~~[1,2단계 - Tomcat 구현하기] 커비(이현지) 미션 제출합니다.~~ [1,2단계 - Tomcat 구현하기] 로키(정해성) 미션 제출합니다. Sep 6, 2024

fix: 회원가입 버그 수정

16cee37

HoeSeong123 requested changes Sep 8, 2024

View reviewed changes

HaiSeong added 5 commits September 9, 2024 13:31

test: 연습 테스트 try-with-resources 사용

93174fc

bug: 지워진 클래스 테스트 수정

e64d9cb

refactor: ArrayList -> List

f20ddad

refactor: 필요없는 메서드 삭제

aa76238

refactor: QUERY_DELIMITER로 이름 변경

7778742

HoeSeong123 approved these changes Sep 9, 2024

View reviewed changes

HoeSeong123 merged commit 7962ef4 into woowacourse:haiseong Sep 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1,2단계 - Tomcat 구현하기] 로키(정해성) 미션 제출합니다. #564

[1,2단계 - Tomcat 구현하기] 로키(정해성) 미션 제출합니다. #564

HaiSeong commented Sep 6, 2024

HoeSeong123 left a comment

HoeSeong123 Sep 7, 2024

HaiSeong Sep 9, 2024

HoeSeong123 Sep 7, 2024

HaiSeong Sep 9, 2024 •

edited

Loading

HoeSeong123 Sep 9, 2024

HoeSeong123 Sep 8, 2024

HaiSeong Sep 9, 2024

HoeSeong123 Sep 9, 2024 •

edited

Loading

HoeSeong123 Sep 8, 2024

HaiSeong Sep 9, 2024

HoeSeong123 Sep 8, 2024

HaiSeong Sep 9, 2024

HoeSeong123 Sep 9, 2024

HoeSeong123 Sep 8, 2024

HaiSeong Sep 9, 2024

HoeSeong123 Sep 8, 2024

HoeSeong123 Sep 8, 2024

HaiSeong Sep 9, 2024

HoeSeong123 Sep 8, 2024

HaiSeong Sep 9, 2024

HoeSeong123 left a comment


		public class RequestURI {

		private static final String QUESTION_MARK = "?";

[1,2단계 - Tomcat 구현하기] 로키(정해성) 미션 제출합니다. #564

[1,2단계 - Tomcat 구현하기] 로키(정해성) 미션 제출합니다. #564

Conversation

HaiSeong commented Sep 6, 2024

HoeSeong123 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

HaiSeong Sep 9, 2024 • 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

HoeSeong123 Sep 9, 2024 • 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

HoeSeong123 left a comment

Choose a reason for hiding this comment

HaiSeong Sep 9, 2024 •

edited

Loading

HoeSeong123 Sep 9, 2024 •

edited

Loading