Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[톰캣 구현하기 - 1,2단계] 오잉(이하늘) 미션 제출합니다. #341

Merged
merged 51 commits into from
Sep 7, 2023

Conversation

hanueleee
Copy link

@hanueleee hanueleee commented Sep 4, 2023

안뇽하세요 하디씨?!
제가 리뷰어인줄 알았는데 하디가 리뷰어였군요 😋

이번 미션 참 쉽지 않았는데요...
리팩토링 갈 길이 멀어 보입니다..^^

제가 구현한 로직은 다음과 같습니다!

  1. 사용자 요청
  2. BufferedReader로 input을 쫙 읽고 파싱하여 HttpRequest 생성
  3. Handler들을 Map형태로 관리하는 HandlerFinder를 통해 해당 request를 처리할 수 있는 Handler 찾아오기
  4. 해당 handler에 request를 넘기면, handler가 요청에 따라 작업 후 적절한 HttpResponse를 반환
  5. response를 byte로 바꿔서 outputStream에 보낸다.

@hanueleee hanueleee self-assigned this Sep 4, 2023
@jundonghyuk jundonghyuk self-requested a review September 5, 2023 04:12
Copy link

@jundonghyuk jundonghyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 오잉 코드 정말 잘 봤습니다 !
객체의 책임에 따른 분리가 되게 깔끔하네요 !
특히 Request 클래스내의 책임분리가 돋보여요..!

코드를 보면서 많이 배웠습니다 ㅎㅎ

개별 코드에 관련된 커멘트는 각각 남겨놨습니다 !

차이점을 묻는 질문 같은거는 답변 안달아도 괜찮습니다. 코드 읽다가 저도 궁금해서 찾아보려고 질문한것도 몇개있어서요 ㅋㅋ

다음 리퀘스트 때 머지 하겠습니다 ! 커멘트 보시고 각자 생각하는 중요도의 차이가 있으니 반영은 자유롭게 해주시면 도리 것 같아요 !

고생하셨습니다 ~~

@sonarcloud
Copy link

sonarcloud bot commented Sep 7, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning 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

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

Copy link

@jundonghyuk jundonghyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오잉 코드 잘봤습니다..!

진짜 코드 너무너무 깔끔해서 읽으면서 박수쳤습니다.

몇개 커멘트 남겼으니 확인 부탁드려요 !

이번단계는 머지하겠씀다 고생하셨어요 ~

return HttpResponse.createMethodNotAllowed(List.of(HttpMethod.GET, HttpMethod.POST));
}

private static HttpResponse doGet() throws IOException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static method인 이유가 있을까요 ? 밑에도 마찬가지입니다 !

try {
return valueOf(name);
} catch (IllegalArgumentException e) {
return NONE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOW ! 배우고갑니다

Comment on lines +103 to +105
HttpHeaders headers = new HttpHeaders();
headers.addHeader(HttpHeaderName.LOCATION, Page.UNAUTHORIZED.getUri());
return HttpResponse.create(StatusCode.FOUND, headers);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 여기서 2가지 방법이 있다고 생각했는데용

  1. Unauthorized상태코드를 쓰면서 bodyUnauthorized전용 예외페이지를 넣는다.
  2. Found 상태코드를 쓰면서 Location헤더에 Unauthorized 페이지 접근 uri를 넣는다.

궁금해서 여쭤봅니다. 2번을 쓰신 이유가 있을까용 ? (궁금궁금)

@jundonghyuk jundonghyuk merged commit 656342b into woowacourse:hanueleee Sep 7, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants