[HTTP 서버 구현하기 - 1단계] 포츈(정윤성) 미션 제출합니다. #32

unluckyjung · 2021-08-29T19:34:57Z

반갑습니다 파피.

어디 내놓기에도 부끄러운 하드코딩이 범벅인 코드인데
리팩토링은 이 후 단계에 예정되어 있으니, 일단 기능구현에 초점을 맞춰서 미션을 진행했습니다. ㅋㅋ; (이번주 목요일 테코톡인건 안비밀)

학습테스트는 스프링을 까보면서 했는데,
막상 미션 구현은 스프링, RFC 문서도 안보고, 열심히 뇌피셜 돌리면서 그냥 쌩 헤딩 + 구글링으로 구현했네요.

아직 많이 부족한 코드인데, 보시다가 이해가 힘드신부분 있으시면 언제든 편하게 DM주세요!

참고할점

유저 정보에 관한 아이디는 일부러 로깅을 하지않았어요. (민감한 내용이라고 생각)
User 도메인에서 getPassword 같은것을 만들지 말까 고민하다가, 실 서비스에서는 password 자체를 그대로 저장하지 않을뿐더러, 이미 httpRequest에서 body를 파싱할때 password는 노출되니, 이부분은 신경쓰지 않고 userId auto-increment 부분만 신경쓰고 구현했어요.
테스트는 EtoE 테스트위주로 했습니다.

kixtaxwax

안녕하세요, 포츈~ 리뷰가 늦어 죄송합니다!

급하게 무지성으로 짠다고 하셔놓고 깔끔하게 잘 구현해주셨네요 ㅎㅎ 특히 HttpRequest 객체가 깔끔해서 보기 좋았어요!

if-else문 사용은 취향 차이일 수도 있기 때문에 한 번만 언급했습니다.

그리고 몇 가지 의견 남겨보았는데 참고해주세요 ㅎㅎ 제가 틀린 부분이 있으면 알려주세요~

kixtaxwax · 2021-08-30T14:04:51Z

app/src/main/java/nextstep/jwp/RequestHandler.java

+    private static final Logger LOG = LoggerFactory.getLogger(RequestHandler.class);

    private final Socket connection;

-    public RequestHandler(Socket connection) {
+    public RequestHandler(final Socket connection) {
        this.connection = Objects.requireNonNull(connection);
    }


사소한 부분까지 챙기시는 군요!👍

kixtaxwax · 2021-08-30T14:05:57Z

app/src/main/java/nextstep/jwp/RequestHandler.java

+            if (controller == null) {
+                httpResponse.transfer(httpRequest.getUrl());
+            } else if (httpRequest.getHttpMethod().equals(HttpMethod.GET)) {
+                controller.get(httpRequest, httpResponse);
+            } else if (httpRequest.getHttpMethod().equals(HttpMethod.POST)) {
+                controller.post(httpRequest, httpResponse);
+            }


if문들만 있어도 되지 않을까요? 취향 차이긴 하지만요 ㅎㅎ

지금의 경우 if로 문기를 4번 하는 경우에도 문제가 되지 않지만
해당 컨트롤러의 상태에 따라서 4개의 분기가 완전히 분리되는 경우를 의도하는 경우에는 else if를 쓰는게 저는 맞다고 생각해요.

아주 쉬운 문제인데도, 프로그래머들이 생각보다 많이 실수하는 문제중에 fizzbuzz 같은 문제들이 있죠.
이 경우도 if 문만으로 잘못 짜는경우 문제가 발생할수 있구요 ㅎ_ㅎ..

아무튼 저는 분기가 완전히 분리되는 경우를 의도 했기 때문에 else if를 사용했어요.
물론 if + return 조합도 가능했지만. 어차피 나중에 리팩토링 할부분으로 크게 힘을 안준부분이에요!

오오..덕분에 알고리즘 문제 하나 풀고 왔어요!!!!ㅋㅋㅋ

그런데 풀어보고 느낀 점은 fizzbuzz 문제는 공배수 개념 때문에 else if가 필수인 게 와 닿는데,

HttpMethod는 어떤 하나의 HttpMethod가 다른 HttpMethod 두 개를 포함하고 있다든지 하는 경우는 없어서 if만으로도 가능하지 않나요?

이건 제가 궁금해서 하는 질문입니다 ㅋㅋㅋㅋ 태클 아닙니다!!

저는 어느 한상황에 대한, 분기가 여러개로 나뉘고 그것이 완전히 독립적이어야 하면, if문을 N번 반복하지 않으려고 해요.
그런식의 코딩은 혹시나 하는 실수의 여지를 남겨두는것이라고 생각하거든요.

지금의 경우 if로 문기를 4번 하는 경우에도 문제가 되지 않지만
저는 분기가 완전히 분리되는 경우를 의도 했기 때문에 else if를 사용했어요

아아 실수 여지와 의도를 중요시한 케이스군요!!!!!!! 이해했습니다👍👍

kixtaxwax · 2021-08-30T15:06:04Z

app/src/main/java/nextstep/jwp/http/HttpResponse.java

+            if (url.equals("/")) {
+                responseBody = DEFAULT_MESSAGE;


localhost:8080일 때도 index.html을 보여주는 건 어떨까요?!

처음에 저도 index.html를 반환하게 짯었다가,
주어진 기본 테스트 코드에 helloworld를 검사하고 있고, 그것을 유지하는 형태로 가야할꺼 같아
지금의 형태로 재변경했어요!

kixtaxwax · 2021-08-30T15:11:49Z

app/src/main/java/nextstep/jwp/http/HttpResponse.java

+            } else if (url.equals("/login") || url.equals("/register")) {
+                responseBody = getBodyByUrl(DEFAULT_RESOURCE_PATH + url + ".html");


httpResponse.transfer 메서드 발동 조건이 requestUri가 "/login", "/register" 둘 다 아니어서 컨트롤러가 null일 때인 것 같은데, 그러면 이 조건문을 타는 경우가 없지 않나요?

transfer 이 불리는 부분은 requestUri가 "/login", "/register" 인 경우에도 존재합니다.
만약 localhost:8080/register 으로 접속을 시도한다면

ControllerMapper 에서 register관련 컨트롤러를 찾겠죠.
이후. registerController 의 get이 호출될텐데,

이때 transfer(httpRequest.getUrl()) 에서 요청은 /register이었기때문에
transfer(/register) 가 수행되게 됩니다.

이후 httpResponse 에서, /register 에 대한 처리가 필요해요.

실제로 분기를 제외하면 테스트도 실패합니다.

transfer가 RequestHandler에서만 불리는 걸로 착각했었네요 ㅎㅎ 친절한 답변 고마워요!

제가 코드를 알아보기 힘들게 작성한탓입니다 ㅋㅋ;

kixtaxwax · 2021-08-30T15:19:44Z

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

+    @Override
+    public void post(final HttpRequest httpRequest, final HttpResponse httpResponse) {
+
+    }


일단은 get으로 처리하는 건가요?!

마이 미스테잌

ㅋㅋㅋㅋㅋㅋㅋ

sonarqubecloud · 2021-08-30T17:34:15Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
11 Code Smells

94.2% Coverage
0.0% Duplication

unluckyjung · 2021-08-30T17:34:18Z

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

+    public void post(final HttpRequest httpRequest, final HttpResponse httpResponse) throws IOException {
+        loginService.login(httpRequest, httpResponse);


로그인을 post에서 처리하도록 수정했습니다 ㅎㅎ;

unluckyjung · 2021-08-30T17:36:56Z

app/src/main/java/nextstep/jwp/http/Params/BodyParams.java

+    private Map<String, String> parseBodyData(final Map<String, String> bodyType, final String body) {
+        String[] parseRequestBody = parseRequestBody(body);
+
+        for (String bodyData : parseRequestBody) {
+            String[] parseBodyData = bodyData.split("=");
+            bodyType.put(parseBodyData[0], parseBodyData[1]);
+        }
+        return bodyType;
+    }
+
+    private String[] parseRequestBody(final String body) {
+        return body.split("&");
    }


수정하는김에 바디정보를 추출하는 기능도, BodyPrams 객체가 수행하도록했어요.

unluckyjung · 2021-08-30T17:38:02Z

app/src/main/java/nextstep/jwp/http/HttpRequest.java

-    private String body;
+    private BodyParams bodyParams;


기존에 String으로 가지고 있던 body 데이터를 BodyParams로 들고있게 했어요.
HttpRequest가 이전로부터 키값을 던지면 Body 데이터들을 반환하도록 했네요.

kixtaxwax

빠르게 피드백을 반영해주셨네요 ㅎㅎ덕분에 fizzbuzz 문제도 풀어보고 아주 재밌었습니다 다음 단계도 화이팅입니다~~

unluckyjung added 24 commits August 24, 2021 17:24

feat: 파일 학습테스트 진행

3edf85b

feat: IOStream 학습 테스트 진행

e808186

doc(기능목록) : GET /index.html 응답하기 기능목록

fdccca0

doc(기능목록): GET /index.html HTTP/1.1 기능목록 업데이트

545ee8e

feat: HTTP 헤더를 저장, 조회하는 기능 구현

e7b0223

feat: Request URI 를 파싱하는 기능 구현

29deb76

feat: 클라이언트가 보낸 Request 에서 필요한 정보를 추출하는 기능구현

ac3278d

feat: index.html 파일데이터를 클라이언트에게 전송하는 기능구현

27af455

test: 불필요한 공백 제거

9f79e43

feat: Response 처리 기능("/", "*.확장자") 구현

cbad281

doc(기능목록) : Query String 파싱 기능목록 작성

bf3e9b5

feat: 로그인 페이지 (login.html) 를 보여주는 기능

c76d199

feat: url에서 아이디, 비밀번호를 파싱하는 기능

ed94efd

feat: 유저 객체를 만들어서 존재여부를 조회하는 기능

bec8efd

feat: 유저 패스워드를 검증하는 기능

c16ad4f

feat: 유저 아이디어를 검증하는 기능

856ad36

refactor: 컨트롤러, 서비스 분리

95dc340

feat: 회원가입 페이지(register.html)를 보여주는 기능

a7ba5b8

feat: pos시 http 요청에서 body 데이터를 추출하는기능

9001ff4

feat: 회원가입을 처리하는 기능

ec88735

refactor: 상태코드에 따른 리다이렉팅 기능에서, 상태코드를 DI 하도록 변경

6be2d7c

feat: 404페이지 리다이렉팅 기능 구현

6baecd5

chore: jacoco 의존성 추가

55a1e87

refactor: 상수를 사용할수 있는 부분 리팩토링

5d574ba

unluckyjung requested a review from kixtaxwax August 29, 2021 19:34

unluckyjung self-assigned this Aug 30, 2021

kixtaxwax suggested changes Aug 30, 2021

View reviewed changes

unluckyjung added 2 commits August 31, 2021 02:27

refactor: bodyparamas를 HttpRequest에서 관리하도록 수정

a691bbf

refactor: login시 post로 처리하도록 수정

6fe6b4b

unluckyjung commented Aug 30, 2021

View reviewed changes

kixtaxwax approved these changes Aug 31, 2021

View reviewed changes

kixtaxwax merged commit e08cc99 into woowacourse:unluckyjung Aug 31, 2021

		} else if (url.equals("/login") \|\| url.equals("/register")) {
		responseBody = getBodyByUrl(DEFAULT_RESOURCE_PATH + url + ".html");

		public void post(final HttpRequest httpRequest, final HttpResponse httpResponse) throws IOException {
		loginService.login(httpRequest, httpResponse);

[HTTP 서버 구현하기 - 1단계] 포츈(정윤성) 미션 제출합니다. #32

[HTTP 서버 구현하기 - 1단계] 포츈(정윤성) 미션 제출합니다. #32

Uh oh!

Conversation

unluckyjung commented Aug 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

참고할점

Uh oh!

kixtaxwax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unluckyjung Aug 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kixtaxwax Aug 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unluckyjung Aug 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unluckyjung Aug 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 30, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kixtaxwax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

unluckyjung commented Aug 29, 2021 •

edited

Loading

unluckyjung Aug 30, 2021 •

edited

Loading

kixtaxwax Aug 30, 2021 •

edited

Loading

unluckyjung Aug 30, 2021 •

edited

Loading

unluckyjung Aug 30, 2021 •

edited

Loading