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

[제이] 3단계 - HTTP 웹 서버 구현 미션 제출합니다. #214

Merged
merged 28 commits into from
Nov 26, 2020

Conversation

jjj0611
Copy link

@jjj0611 jjj0611 commented Nov 24, 2020

안녕하세요 하비
HTTP 웹 서버 구현 3단계 제출합니다!
더불어 2단계 리뷰 반영도 함께 하였습니다!

감사합니다!

jnsorn and others added 23 commits September 12, 2020 22:13
* docs : 요구사항1의 구현 기능 목록 작성

* feat : 모든 Request Header 출력하기

* feat : Request에서 path 분리하기

* feat : path에 해당하는 파일 읽어 응답하기

* docs : 요구사항2의 구현 기능 목록 작성

* feat : Request Parameter 추출 및 User 객체 생성

* docs : 요구사항3의 구현 기능 목록 작성

* feat : form.html 파일의 form 태그 method를 get에서 post로 수정

* refactor : HttpRequest를 RequestHeader와 RequestBody로 분리

* refactor : HttpRequestParser 유틸 클래스 생성

* docs : 요구사항4의 구현 기능 목록 작성

* feat : 요청에 따라 다른 HttpResponse를 내려준다.

* docs : 요구사항5의 구현 기능 목록 작성

* refactor : RequestHeader 값의 자료형을 List에서 Map으로 변경

* refactor : HeaderProperty 생성

* feat : 응답에 따라 Content-Type을 변경하여 Stylesheet 파일을 지원하도록 구현

* feat : status code를 302로 변경한 후, Location 값에 리다이렉션 할 페이지를 넣어 응답

* refactor : 변수 정리 및 파일 끝 개행 추가
* docs: README.md에 요구사항 1 추가

* feat: Request Handler에서 Header 추출 기능 구현

* feat: request의 path로 파일을 읽어서 응답하는 기능 추가

* feat: Http Request 생성

	- Http Method를 위한 클래스 생성
	- Http Status를 위한 클래스 생성
	- Http Header를 위한 클래스 생성

* feat: 요구사항1 완료

* docs: 두번째 요구사항 작성

* refactoring: http spec 정의

* refactoring: http request spec 추가 및 테스트 추가

* refactor: http request spec 변경

* feat: 요구사항2 완료

* docs: 요구사항3을 위한 README.md 작성

* feat: 회원 가입 기능이 post 요청에서 정상적으로 동작하도록 구현

* docs: 요구사항4 작성

* feat: 회원가입 완료 후 index.html로 이동하도록 변경

* docs: 요구사항 5 README.md에 작성

* feat: 요청 uri이 static resource인지를 확인하는 기능 생성

* feat: stylesheet 등 다양한 형식의 파일을 지원하도록 변경

* feat: Http Response 분리

* test: SimpleHttRequest에 대한 Test 작성

* test: HttpResponse에 대한 Test 작성

* refactor: HttpServlet 및 UserController 생성

* feat: DispatcherServlet 생성

* feat: ThreadPool 사용

* docs: 구현 내용 정리

* refactor: 사용하지 않는 파일 삭제

* [제이] 1단계 - HTTP 웹 서버 리팩터링 미션 제출합니다. (woowacourse#198)

* docs: README.md에 요구사항 1 추가

* feat: Request Handler에서 Header 추출 기능 구현

* feat: request의 path로 파일을 읽어서 응답하는 기능 추가

* feat: Http Request 생성

	- Http Method를 위한 클래스 생성
	- Http Status를 위한 클래스 생성
	- Http Header를 위한 클래스 생성

* feat: 요구사항1 완료

* docs: 두번째 요구사항 작성

* refactoring: http spec 정의

* refactoring: http request spec 추가 및 테스트 추가

* refactor: http request spec 변경

* feat: 요구사항2 완료

* docs: 요구사항3을 위한 README.md 작성

* feat: 회원 가입 기능이 post 요청에서 정상적으로 동작하도록 구현

* docs: 요구사항4 작성

* feat: 회원가입 완료 후 index.html로 이동하도록 변경

* docs: 요구사항 5 README.md에 작성

* feat: 요청 uri이 static resource인지를 확인하는 기능 생성

* feat: stylesheet 등 다양한 형식의 파일을 지원하도록 변경

* feat: Http Response 분리

* test: SimpleHttRequest에 대한 Test 작성

* test: HttpResponse에 대한 Test 작성

* refactor: HttpServlet 및 UserController 생성

* feat: DispatcherServlet 생성

* feat: ThreadPool 사용

* docs: 구현 내용 정리

* docs: 2단계 리팩토링 목록 작성

* refactor: AbstractController 수정
- 지원하지 않는 메서드에 대해서는 MethodNotAllowed 응답 반환

* refactor: resource 처리 영역과 아닌 영역을 구분

* test: 다양한 resource postfix 형식에 대한 테스트 작성

Co-authored-by: jaeju.jang <jjj0611@gmail.com>
Co-authored-by: Jang Jaeju <44603719+jjj0611@users.noreply.github.com>
Copy link

@young891221 young891221 left a comment

Choose a reason for hiding this comment

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

안녕하세요 제이!
기능 구현들 잘 해주셨네요ㅎㅎ
그냥 머지할까 하다가..몇 가지 아쉬운 부분들만 적어보았습니다.
확인 부탁드려요~!

try {
return Objects.isNull(pair[VALUE_INDEX]) ? null : URLDecoder.decode(pair[VALUE_INDEX], "UTF-8");
} catch (UnsupportedEncodingException e) {
e.printStackTrace();
Copy link

@young891221 young891221 Nov 25, 2020

Choose a reason for hiding this comment

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

e.printStackTrace();는 불필요한 비용이 드는 메서드라 해당하는 로그 타이틀정도만 호출 or 로깅처리 도구를 사용하는게 좋아 보입니다.
이걸 사용하면 안되는 이유를 조금 자세하게 설명하자면... 이걸 사용하는 쓰레드 관점에서 쓰레드는 응답을 빨리 내보내야 더 많은 일들을 처리할 수 있는데 e.printStackTrace();의 모든 스택들을 탐색하며 찍어내야 하기에 로직상에서 불필요한 비용이 발생합니다. 해당 쓰레드가 에러가 났는데 로그출력까지 처리해야 하는 상황인거죠. 로직만 수행하고 빠르게 응답해야 하는데...
logback과 같은 도구를 사용하면 이러한 스택들을 별도의 쓰레드로 찍어낼 수 있는데 async하게 동작하기에 현재 쓰레드가 응답을 내보내는데 영향을 주지 않습니다.
그래서 logback, log4j와 같은 로깅 도구를 사용해야 합니다ㅎ

Choose a reason for hiding this comment

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

여기서는 연습이니 굳이 사용하지 않더라도 그냥 에러 타이틀정도만 반환시켜버려도 무방할 듯 합니다.

} catch (UnsupportedEncodingException e) {
e.printStackTrace();
}
return null;

Choose a reason for hiding this comment

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

null을 리턴하는건 올드한 방식이라 비어있는 인스턴스라던지 다른 방식으로 리턴하는게 어떨까요?

Comment on lines 45 to 50
HttpSession session = httpRequest.getSession();
String logined = String.valueOf(session.getAttribute("logined"));
if (Objects.isNull(logined) || !"true".equals(logined)) {
unauthorized(httpResponse);
return;
}

Choose a reason for hiding this comment

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

validation1

Comment on lines 53 to 60
TemplateLoader loader = new ClassPathTemplateLoader();
loader.setPrefix("/templates");
loader.setSuffix(".html");
Handlebars handlebars = new Handlebars(loader);

try {
Template template = handlebars.compile("user/list");
String listPage = template.apply(users);

Choose a reason for hiding this comment

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

make template3

unauthorized(httpResponse);
return;
}
List<UserResponseDto> users = userService.findAll();

Choose a reason for hiding this comment

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

get Data2

Comment on lines 61 to 62
httpResponse.setStatus(HttpStatus.OK);
httpResponse.setBody(listPage.getBytes(), ContentType.HTML);

Choose a reason for hiding this comment

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

http response4

httpResponse.setStatus(HttpStatus.FOUND);
httpResponse.addHeader("Location", "/index.html");
}

@Override
protected void doGet(HttpRequest httpRequest, HttpResponse httpResponse) {

Choose a reason for hiding this comment

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

로직이 아래와 같은 순서로 이뤄집니다.

  1. validation
  2. get Data
  3. make template
  4. http response

이걸 템플릿화? 혹은 모듈화하여 재사용 가능한 구조를 만들어 보는것도 좋을것 같습니다. 혹은, 리펙토링 관점에서 재사용 가능한 모듈들로만이라도 분리해 보는것도 좋을 것 같습니다.

Copy link

@young891221 young891221 left a comment

Choose a reason for hiding this comment

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

리뷰 반영 감사합니다. 제이!
머지할게요ㅎ

@young891221 young891221 merged commit c02a51b into woowacourse:jjj0611 Nov 26, 2020
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.

3 participants