[톰캣 구현하기 3, 4단계] 루쿠(백경환) 미션 제출합니다. #488

aiaiaiai1 · 2023-09-11T08:05:58Z

안녕하세요 오잉
브랜치작업을 잘못 나누어서 커밋이 1,2단계 까지 같이 들어가버렸네요
따로 3,4단계에서 진행했던 커밋으로만 따로 지정했습니다!
커밋

hanueleee

안녕하세요 루쿠!!
미션 3,4단계 수고하셨습니다~~
저번에 말씀해주신대로 리팩토링을 야무지게 해주셨네요 😄

코드 읽으면서 궁금한 부분들 질문 남겨놨습니다ㅎㅎㅎ
전체적으로 상수 추출, 공백 정리, 테스트 작성만 좀 더 해주면 좋을 것 같아요!

다음 요청 주시면 바로 머지하겠습니다!

hanueleee · 2023-09-11T14:53:20Z

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

-            String requestFileName = requestPath.substring(requestPath.lastIndexOf('/') + 1);
-            String requestBody = httpRequest.getBody();
-            Map<String, String> headers = httpRequest.getHeaders();
+            RequestMapping requestMapping = new RequestMapping();


모든 요청마다 RequestMapping 객체를 새로 생성해주는 이유가 있나용??

엇 그러네요 필드로 옮겨두겠습니다~

hanueleee · 2023-09-11T14:54:23Z

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

+    public Controller getController(HttpRequest request) {
+        if (request.getPath().equals("/login")) {
+            return new LoginController();
+        }
+
+        if (request.getPath().equals("/register")) {
+            return new RegisterController();
+        }
+
+        if (request.getPath().equals("/")) {
+            return new RootController();
+        }
+
+        return new ResourceController();
+    }


마찬가지로 모든 요청마다 LoginController, RegisterController 등을 새로 만들어 주는 이유가 있나요?!

예리하시군요,.. map에다가 담아 사용하도록 수정하였습니다

hanueleee · 2023-09-12T00:33:33Z

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

@@ -4,13 +4,15 @@
 import java.util.Map;

 public class HttpResponse {
-    private final StringBuilder status = new StringBuilder();
+    private HttpStatus status;
    private final Map<String, String> headers = new HashMap<>();
    private final StringBuilder body = new StringBuilder();

    public String createResponse() {


뭔가 createResponse라는 이름을 보면 HttpResponse를 만드는 정적팩토리메소드인 느낌이 드는데,
실제로 반환하는 값은 Http 응답 포맷에 맞춘 String이네용!!
메소드명을 살짝 수정해보는 것은 어떨까요?!

그렇네요! 좋은의견감사합니다~

hanueleee · 2023-09-12T00:34:23Z

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

    private final Map<String, String> headers = new HashMap<>();
    private final StringBuilder body = new StringBuilder();

    public String createResponse() {
        StringBuilder responseBuilder = new StringBuilder();
-        responseBuilder.append("HTTP/1.1 ").append(status).append("\r\n");
+        responseBuilder.append("HTTP/1.1 ")


builder 👍

hanueleee · 2023-09-12T00:35:05Z

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

 import org.apache.catalina.Manager;

 public class SessionManager implements Manager {

-    private static final Map<String, Session> SESSIONS = new HashMap<>();
+    private static final Map<String, Session> SESSIONS = new ConcurrentHashMap<>();


HashMap 대신 ConcurrentHashMap을 사용한 이유가 뭔가요??

그리고 SESSIONS는 상수일까요?! 루쿠의 의견이 궁금해요~~

해시맵은 멀티 쓰레드시로 사용시 동기화가 이루어지지 않으므로 내부적으로 동기화 처리를하는ConcurrentHashMap을 사용하였습니다~
SESSIONS은 map의 값이 고정되지 않고 세션이 추가되거나 제거될수 있으니 상수는 아닌거같아요! 그래서 변수명도 소문자로 수정하였습니다

hanueleee · 2023-09-12T00:38:33Z

tomcat/src/test/java/org/apache/coyote/controller/RegisterControllerTest.java

+
+

hanueleee · 2023-09-12T00:38:49Z

tomcat/src/test/java/org/apache/coyote/controller/RequestMappingTest.java

+
+        assertThat(controller).isInstanceOf(RegisterController.class);
+    }
+


hanueleee · 2023-09-12T00:39:28Z

tomcat/src/test/java/org/apache/coyote/controller/RequestMappingTest.java

+    @Test
+    @DisplayName("올바른 매핑인지 확인1")
+    void getController1() {
+        RequestMapping requestMapping = new RequestMapping();
+        HttpRequest httpRequest = new HttpRequest(null, "/login", null, null);
+        Controller controller = requestMapping.getController(httpRequest);
+
+        assertThat(controller).isInstanceOf(LoginController.class);
+    }
+
+    @Test
+    @DisplayName("올바른 매핑인지 확인2")
+    void getController2() {
+        RequestMapping requestMapping = new RequestMapping();
+        HttpRequest httpRequest = new HttpRequest(null, "/register", null, null);
+        Controller controller = requestMapping.getController(httpRequest);
+
+        assertThat(controller).isInstanceOf(RegisterController.class);
+    }


ParameterizedTest를 활용해 보는 것은 어떨까요??

hanueleee · 2023-09-12T00:46:59Z

tomcat/src/main/java/org/apache/coyote/controller/RegisterController.java

+        User user = new User(queryParms.get("account"), queryParms.get("password"),
+                queryParms.get("email"));


queryParams에 account, password, email 중 하나라도 없으면 User에 잘못된 값이 들어갈 것 같은데,
살짝의 방어로직을 추가해보는 것은 어떨까용??

추가하였습니다!

hanueleee · 2023-09-12T00:49:33Z

tomcat/src/main/java/org/apache/coyote/controller/ResourceController.java

+        if (fileName.endsWith(".html")) {
+            response.setContentType("text/html");
+            response.setBody(Utils.readFile("static", fileName));
+            return;
+        }
+
+        if (fileName.equals("styles.css")) {
+            response.setContentType("text/css");
+            response.setBody(Utils.readFile("static/css", fileName));
+            return;
+        }
+
+        if (fileName.endsWith(".js") && !fileName.equals("scripts.js")) {
+            response.setContentType("text/javascript");
+            response.setBody(Utils.readFile("static/assets", fileName));
+            return;
+        }
+
+        if (fileName.equals("scripts.js")) {
+            response.setContentType("text/javascript");
+            response.setBody(Utils.readFile("static/js", fileName));
+            return;
+        }
+
+        if (fileName.equals("favicon.ico")) {
+            response.setContentType("text/javascript");
+            response.setBody("Hello world!");
+            return;
+        }


지금 구조에서는
새로운 정적파일이 추가될 때마다 if 분기문이 계속해서 추가되어야 할 것 같은데
어떻게 생각하시나용??

맞습니다 오잉이 말씀해주신것처럼 정적파일이 추가될때마다 If문이 추가될거같아요..
이부분에 대해서는 좀더 근본적인 방법이 잘 떠오르지 않아서 좀더 고민하고 생각해봐야할거같습니다 😭

aiaiaiai1 · 2023-09-14T09:39:02Z

안녕하세요 오잉~
리뷰해주신부분 수정하였습니다!
분기문관련해서는 좀더 생각할 시간이 필요할거같아서 나중에 추가적으로 학습해보도록 할께요~
리뷰하시느라 고생하셨어요!

hanueleee

안녕하세요 루쿠!
3,4단계 너무너무 수고 많으셨어요~~
톰캣 미션은 여기서 마무리 하도록 하겠습니다!
다음 미션도 화이팅팅😋

aiaiaiai1 added 29 commits September 2, 2023 14:56

train : 학습 테스트 학습

69e0cae

feat : GET /index.html 응답하기 + CSS 지원하기

eaf05ed

feat : QueryString 파싱

4983f19

feat : HTTP Status Code 302 구현

6e9a13d

feat : reqeustHeader 파싱 기능 추가 + POST 방식으로 회원가입 기능 추가

dfc5091

feat : 로그인 페이지도 버튼을 눌렀을 때 GET 방식에서 POST 방식으로 전송하도록 변경

21fa23a

feat : 세션, 쿠키, 세션매니저 클래스 구현

0cad6ff

feat : HttpResponse 클래스 구현

c8c49b3

feat : 로그인시 쿠키 설정, 로그인 페이지 접속시 쿠키 헤더 확인 후 응답 처리 추가

09f7da5

refactor: 불필요한 메서드 삭제

b97149c

feat: HttpRequest 클래스 구현

cf030ce

feat: HttpRequest 클래스 적용

7f37022

fix: 컨텐트 타입 text/html로 응답 수정

8df0133

fix: favicon.ico 응답 추가

2dae997

fix: 로그 삭제

be6dc82

refactor: 접근 제어자 수정

a199ec5

refactor: 기존 메서드 활용

b367bc5

fix: 기존 메서드 내용 구현

e71dd0a

refactor: HttpStatus enum 추가

bc337a1

refactor: HttpMethod enum 추가

031bdf6

feat: Controller 인터페이스 추가

84c25a5

refactor: login, register Controller 도입

0086b13

refactor: Request url 매핑 컨트롤러 추가

959a333

test: RequestMapping 테스트 코드 추가

72532ba

refactor: RootController 분리 및 메서드 삭제

9b278bd

test: 테스트코드 추가

85917d6

feat: 쓰레드 풀 설정 + 세션 동시성 해결

df9ef9c

feat: 쓰레드 풀 설정 + 세션 동시성 해결

0d3e1f0

충돌해결

176a1ef

aiaiaiai1 requested a review from hanueleee September 11, 2023 08:46

hanueleee requested changes Sep 12, 2023

View reviewed changes

aiaiaiai1 added 6 commits September 14, 2023 17:37

fix: 로그인 페이지 불러오기 버그 수정

d8b6ab8

refactor: 새로운 인스턴스 생기지 않도록 수정

c7cb1a0

refactor: 메서드명 수정

39abdb4

refactor: 필드 이름 수정

833eaea

refactor: 개행 제거

8df60ee

refactor: 방어 로직 추가

147d760

hanueleee approved these changes Sep 15, 2023

View reviewed changes

hanueleee merged commit 56a9141 into woowacourse:aiaiaiai1 Sep 15, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[톰캣 구현하기 3, 4단계] 루쿠(백경환) 미션 제출합니다. #488

[톰캣 구현하기 3, 4단계] 루쿠(백경환) 미션 제출합니다. #488

aiaiaiai1 commented Sep 11, 2023 •

edited

Loading

hanueleee left a comment

hanueleee Sep 11, 2023

aiaiaiai1 Sep 14, 2023

hanueleee Sep 11, 2023

aiaiaiai1 Sep 14, 2023

hanueleee Sep 12, 2023

aiaiaiai1 Sep 14, 2023

hanueleee Sep 12, 2023

hanueleee Sep 12, 2023

hanueleee Sep 12, 2023

aiaiaiai1 Sep 14, 2023

hanueleee Sep 12, 2023

hanueleee Sep 12, 2023

hanueleee Sep 12, 2023

aiaiaiai1 Sep 14, 2023

hanueleee Sep 12, 2023

aiaiaiai1 Sep 14, 2023 •

edited

Loading

hanueleee Sep 12, 2023

aiaiaiai1 Sep 14, 2023 •

edited

Loading

aiaiaiai1 commented Sep 14, 2023

hanueleee left a comment


		assertThat(controller).isInstanceOf(RegisterController.class);
		}

		User user = new User(queryParms.get("account"), queryParms.get("password"),
		queryParms.get("email"));

[톰캣 구현하기 3, 4단계] 루쿠(백경환) 미션 제출합니다. #488

[톰캣 구현하기 3, 4단계] 루쿠(백경환) 미션 제출합니다. #488

Conversation

aiaiaiai1 commented Sep 11, 2023 • edited Loading

hanueleee 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aiaiaiai1 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aiaiaiai1 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

aiaiaiai1 commented Sep 14, 2023

hanueleee left a comment

Choose a reason for hiding this comment

aiaiaiai1 commented Sep 11, 2023 •

edited

Loading

aiaiaiai1 Sep 14, 2023 •

edited

Loading

aiaiaiai1 Sep 14, 2023 •

edited

Loading