[톰캣 구현하기 1 & 2단계] 파워(송재백) 미션 제출합니다 #348

thdwoqor · 2023-09-04T08:07:14Z

안녕하세요 주드! 잘부탁드려요
step1에서 step2까지 작업해버렸어요,,

전체적인 흐름은 아래와 같습니다.

사용자 요청
process 메서드 에서 HttpRequest, httpResponse 생성
RequestURI Path로 처리가능한 컨트롤러 선택 (HttpServletMapper)
HttpServlet 로직 수행

kevstevie

안녕하세요 파워
전체적인 코드 흐름이나 역할 분리가 잘 되어있는 것 같아요.
리뷰 한번만 확인해주시고 수정하고 싶으신거 수정하시고 다시 요청주시면 될 것 같습니다.

kevstevie · 2023-09-04T10:00:18Z

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

-        }
-    }
-}
+package org.apache.coyote.http11;import java.io.IOException;import java.net.Socket;import java.net.URISyntaxException;import nextstep.jwp.exception.UncheckedServletException;import org.apache.coyote.Processor;import org.apache.coyote.http11.request.HttpRequest;import org.apache.coyote.http11.response.HttpResponse;import org.slf4j.Logger;import org.slf4j.LoggerFactory;public class Http11Processor implements Runnable, Processor {    private static final Logger log = LoggerFactory.getLogger(Http11Processor.class);    private final Socket connection;    public Http11Processor(final Socket connection) {        this.connection = connection;    }    @Override    public void run() {        log.info("connect host: {}, port: {}", connection.getInetAddress(), connection.getPort());        process(connection);    }    @Override    public void process(final Socket connection) {        try (final var inputStream = connection.getInputStream();             final var outputStream = connection.getOutputStream()) {            HttpRequest httpRequest = HttpRequest.of(inputStream);            HttpResponse httpResponse = new HttpResponse();            HttpServletMapper httpServletMapper = new HttpServletMapper();            HttpServlet httpServlet = httpServletMapper.get(httpRequest.getPath());            httpServlet.service(httpRequest, httpResponse);            outputStream.write(httpResponse.generateResponse());            outputStream.flush();        } catch (IOException | UncheckedServletException e) {            log.error(e.getMessage(), e);        } catch (URISyntaxException e) {            throw new RuntimeException(e);        }    }}


이거 왜 이럴까요 fetch 해서 보니 제대로 나오긴 하네요

개행이 해당 클래스만 CR로 되어있었네요

kevstevie · 2023-09-04T10:02:24Z

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

+        String[] pairs = cookies.split(COOKIE_DELIMITER);
+        for (String pair : pairs) {
+            String[] cookie = pair.split(KEY_VALUE_DELIMITER);
+            httpCookies.put(cookie[KEY_INDEX].strip(), cookie[VALUE_INDEX].strip());


cookie delimiter를 "; "로 하면 strip을 안해도 될 것 같아요

kevstevie · 2023-09-04T10:05:02Z

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

+            doGet(req, res);
+            if(res.isNotRedirect()){
+                //FIXME: 임시로 요청을 구별하기 위한 코드
+                if(!req.getPath().contains(".")){


url을 파일형식으로 바꾸는건 다른 객체의 역할로 바꿀 수 있을 것 같아요

kevstevie · 2023-09-04T10:07:17Z

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

+    public void doGet(final HttpRequest req, final HttpResponse resp) throws IOException {
+        throw new UnsupportedOperationException();
+    }
+
+    public void doPost(final HttpRequest req, final HttpResponse resp) {
+        throw new UnsupportedOperationException();
+    }


abstract method가 아닌 이유가 뭔가요 (진짜 모름)

HttpServlet에 HttpMethod를 모두 abstract method만든다면

public enum HttpMethod { GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE }

Get 요청만 받는 ResourceController 경우 나머지 HttpMethod메서드를 모두 오버라이드후
throw new UnsupportedOperationException(); 작성해줘야해서

abstract method를 사용하지않았어요

취약한 기반 클래스 문제에 대해 들어보셨나요?
사실 이건 원래 구현도 이런 구조일 것 같아서 넘어가셔도 될 것 같습니다

kevstevie · 2023-09-04T10:14:46Z

tomcat/src/main/java/org/apache/coyote/http11/request/HttpRequestHeaders.java

+        final Map<String, String> requestHeaders = new LinkedHashMap<>();
+
+        String line;
+        while (!"".equals(line = bufferedReader.readLine())) {


isBlank 를 쓰는 건 어떤가요

kevstevie · 2023-09-04T10:15:48Z

tomcat/src/main/java/org/apache/coyote/http11/request/HttpRequestHeaders.java

+
+    public HttpRequestHeaders add(String key, String value) {
+        handlerMap.put(key.toLowerCase(), value);
+        return new HttpRequestHeaders(handlerMap);


return this 대신 새로운 객체를 만드는 이유가 뭔가요? (진짜 모름)

아마 방어적복사를 할려고했던것같아요,,,
HttpRequestHeaders에는 아직까지는 add가 필요가 없었네요,, 지웠습니다,, 😭

kevstevie · 2023-09-04T10:17:20Z

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

+
+    //FIXME: 쿠키를 여러개 설정할 수 있도록 수정
+    public void addCookie(final String key, final String value) {
+        headers.add("Set-Cookie", key + KEY_VALUE_DELIMITER + value);


Set-Cookie를 enum이나 상수화하는 것 어떤가요

enum으로 표현하기에는 쿠키 key에는 다양한값이 들어갈수있어서 상수클래스를 만들었습니다!

kevstevie · 2023-09-04T10:20:51Z

tomcat/src/test/java/nextstep/org/apache/coyote/http11/Http11ProcessorTest.java

-                "Content-Type: text/html;charset=utf-8 ",
-                "Content-Length: 12 ",
-                "",
-                "Hello world!");


홈화면 테스트 어디갔나요

그리고 테스트 추가로 더 있으면 좋을 것 같아요

Hello world!가 홈화면이였군요,,,

sonarcloud · 2023-09-05T12:29:34Z

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

kevstevie

안녕하세요 power, jude입니다.
1,2단계 요구사항은 충분히 만족한 것 같습니다.
아니 어쩌면 3단계까지도..?
코멘트 한번씩만 확인해주시고 다음 단계에서 만나시죠
잘 배우고 갑니다

kevstevie · 2023-09-05T12:50:39Z

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

+        if (findAccount.isPresent()) {
+            User user = findAccount.get();
+            if (user.checkPassword(requestParam.get("password"))) {


Optional의 ifPresent같은 메서드를 활용한다면 옵셔널을 더 의미있게, 그리고 가독성 좋은 코드를 만들 수 있을 것 같습니다.

kevstevie · 2023-09-05T12:52:47Z

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

+    private static final String PREFIX = "static";
+    private static final String SUFFIX = ".html";


중복이 발생하는 것 같아요 다른 좋은 방법은 없을까요? 스프링에선 ViewResolver가 이런 역할을 하고 있는 걸로 알고 있습니다

kevstevie · 2023-09-05T12:54:35Z

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

+    public void doGet(final HttpRequest req, final HttpResponse resp) throws IOException {
+        throw new UnsupportedOperationException();
+    }
+
+    public void doPost(final HttpRequest req, final HttpResponse resp) {
+        throw new UnsupportedOperationException();
+    }


취약한 기반 클래스 문제에 대해 들어보셨나요?
사실 이건 원래 구현도 이런 구조일 것 같아서 넘어가셔도 될 것 같습니다

thdwoqor added 14 commits September 3, 2023 18:32

feat: httpRequest 구현

006ea83

feat: httpResponse 구현

b31469e

feat: HttpProcessor 구현

09fb6e9

chore: 학습 테스트

028583d

feat: html 파일을 읽어오기위한 FileIOUtils 생성

20c5afc

feat: servlet mapper를 통해서 컨트롤러를 선택할 수 있게 수정

7c7a522

feat: post요청으로 회원가입 로그인을 수행하도록 구현

dec42e7

feat: 세션 구현 및 로그인시 세션에 유저정보를 저장하게 구현

650307d

feat: 모든 요청에 세션을 생성하지않고 특정 요청에만 세션을 생성할 수 있게 수정

7fee65e

feat: 컨트롤러 중복 코드 제거 및 리펙터링

c7f0c5e

feat: HttpRequest 생성자 분리 및 리팩터링

8a6f86f

feat: HttpResponse 순서대로 출력하기위해 LinkedHashMap사용

2a7b16d

feat: 접근제어자 등 리팩터링

5aaee35

feat: 컨트롤러 중복코드 제거

0973577

kevstevie requested changes Sep 4, 2023

View reviewed changes

thdwoqor added 10 commits September 4, 2023 23:47

feat: isNotRedirect에서 NPE이 발생하는 문제 해결

1f2d989

error: Http11Processor 클래스에 개행이 제대로 되지않는 문제 해결

cf6099a

refactor: HttpHeaders 상수 클래스 생성

a514895

feat: HttpServlet안에 로직을 넣기에는 경우에 수가 많아서 각 컨트롤러에서 뷰를 렌더링 하도록 수정

8c1fe9b

refactor: sonarqube java:S2583 반영

4e291ac

fix: session을 제대로 불러오지 못하는 문제 해결

52e9572

fix: 페이지를 제대로 랜더링 하지못하는 문제 해결

9002d93

feat: 테스트 추가 작성 및 리팩터링

138984d

chore: 캐시 학습 테스트

c83587a

feat: RequestParam이 인코딩된 문자열을 읽을수있게 수정

b88e866

kevstevie approved these changes Sep 5, 2023

View reviewed changes

kevstevie merged commit 39596a2 into woowacourse:thdwoqor Sep 5, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 1 & 2단계] 파워(송재백) 미션 제출합니다 #348

[톰캣 구현하기 1 & 2단계] 파워(송재백) 미션 제출합니다 #348

thdwoqor commented Sep 4, 2023 •

edited

Loading

kevstevie left a comment

kevstevie Sep 4, 2023

thdwoqor Sep 4, 2023

kevstevie Sep 4, 2023

kevstevie Sep 4, 2023

kevstevie Sep 4, 2023

thdwoqor Sep 4, 2023

kevstevie Sep 5, 2023

kevstevie Sep 4, 2023

kevstevie Sep 4, 2023

thdwoqor Sep 4, 2023

kevstevie Sep 4, 2023

thdwoqor Sep 4, 2023

kevstevie Sep 4, 2023

kevstevie Sep 4, 2023 •

edited

Loading

thdwoqor Sep 4, 2023

sonarcloud bot commented Sep 5, 2023

kevstevie left a comment

kevstevie Sep 5, 2023

kevstevie Sep 5, 2023

kevstevie Sep 5, 2023

		private static final String PREFIX = "static";
		private static final String SUFFIX = ".html";

[톰캣 구현하기 1 & 2단계] 파워(송재백) 미션 제출합니다 #348

[톰캣 구현하기 1 & 2단계] 파워(송재백) 미션 제출합니다 #348

Conversation

thdwoqor commented Sep 4, 2023 • edited Loading

kevstevie 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

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

kevstevie Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 5, 2023

kevstevie 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

thdwoqor commented Sep 4, 2023 •

edited

Loading

kevstevie Sep 4, 2023 •

edited

Loading