[ coco,K ] HTTP 웹 서버 2단계 - 리팩토링 #8
Conversation
-접근 제어자 변경 -메소드명 변경 -패키지명 변경
refactor : HttpRequest closing #15
- HttpResponse 테스트를 위한 코드
-response의 값들이 잘 들어갔는지 검사 -static file 리턴 가능
-css 파일 읽기 가능
-테스트 작성
-redirect 동작 확인
-cookie 값이 있는지 test한다.
-cookie값 헤더에 추가
-controller의 중복 제거 -response에서 header를 map으로 관리
Http response close #16
-같은 httpMethod로 만든 controller key는 같아야한다.
-컨트롤러 다형성를 이용해서 리퀘스트 핸들러를 리팩토링
Controller closing #17
There was a problem hiding this comment.
몰랐는데 HTTP 웹서버 미션이 1/2단계 딸랑 두 개의 구성인 것 같네요.
이번 2단계 리팩토링 스텝에서 충분한 경험을 하시는게 더 좋을 것 같습니다.
조금 어려울수도 있는 피드백을 드렸는데, 핵심은 Request Mapping의 자동화 입니다.
스프링이 어떤 방식으로 이 메카니즘을 구현했을지 고민해보시고, 제가 제시해드린 개념들 충분히 학습하시고,
무엇보다 라이브러리 속의 소스 코드들을 까보는 일을 두려워하지 마시기 바랍니다.
그럼 다음 리뷰 요청 기대하겠습니다. 🥇
|
|
||
| import java.io.IOException; | ||
|
|
||
| public interface Controller { |
| public void service(HttpRequest httpRequest, HttpResponse httpResponse) throws IOException { | ||
| if ("true".equals(httpRequest.cookie("logined"))) { | ||
| httpResponse.forward("/user/list.html"); | ||
| } else { |
| } else if (user.checkPassword(httpRequest.data("password"))) { | ||
| httpResponse.addHeader("Set-Cookie", "logined=true; Path=/"); | ||
| httpResponse.redirect("/index.html"); | ||
| } else { |
There was a problem hiding this comment.
이번 미션에서 else if 는 사용하지 않는 것을 원칙으로 삼아 보시면 도전적인 과제가 될 것 같습니다.
충분히 가능하니 한번 숙고해주세요.
| public static HttpRequest of(InputStream in) throws IOException { | ||
| HttpRequest httpRequest = new HttpRequest(); | ||
| BufferedReader br = new BufferedReader(new InputStreamReader(in)); | ||
| String buffer; | ||
|
|
||
| buffer = br.readLine(); | ||
| httpRequest.addStartLine(buffer); | ||
| while (!(buffer = br.readLine()).equals("")) { | ||
| httpRequest.addHeaders(buffer); | ||
| } | ||
|
|
||
| String contentLength = httpRequest.header("Content-Length"); | ||
| if (contentLength != null) { | ||
| httpRequest.addBody(IOUtils.readData(br, Integer.parseInt(contentLength))); | ||
| } | ||
|
|
||
| return httpRequest; | ||
| } |
There was a problem hiding this comment.
스태틱 팩토리 메서드에 이렇게 로직이 많은 것은 크게 추천하고 싶진 않습니다.
InputStream 을 파스해서 적절한 정보를 뽑아낸 후, HttpRequest 로 변환하는 컴포넌트를 따로 한 번 구현해보는 건 어떨까요.
src/main/java/http/HttpResponse.java
Outdated
| if (url.endsWith(".css")) { | ||
| addHeader("Content-Type", "text/css;charset=utf-8"); | ||
| } else { | ||
| addHeader("Content-Type", "text/html;charset=utf-8"); | ||
| } |
There was a problem hiding this comment.
리팩토링 제안 ContentType 이라는 enum 설계해 봅니다.
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class RequestHandler extends Thread { | ||
| private static final Logger log = LoggerFactory.getLogger(RequestHandler.class); | ||
|
|
||
| private Map<ControllerKey, Controller> controllerMap; |
There was a problem hiding this comment.
Map<ControllerKey, Controller> 를 감싸는 클래스를 구현하고,
애플리케이션 구동 초기에 모든 컨트롤러를 탐색해 이 클래스의 객체를 형성한 뒤,
RequestHandler 의 생성자로 넘겨 요청을 처리할 수 있는 구조를 만들어 봅니다.
힌트
- Annotation
- Reflection
| static { | ||
| controllerMap.put(new ControllerKey(HttpMethod.POST, "/user/create"), new CreateUserController()); | ||
| controllerMap.put(new ControllerKey(HttpMethod.POST, "/user/login"), new LoginController()); | ||
| controllerMap.put(new ControllerKey(HttpMethod.GET, "/user/list"), new ListUserController()); | ||
| } |
There was a problem hiding this comment.
지금 구조라면 path 가 추가될때마다 한 땀 한 땀 매핑을 코드로서 관리해줘야하는 구조인데요,
이 구조를 개선하기 위해 어떤 자동화를 추구할 수 있을까요?
|
리뷰 해주신 부분을 조금 반영하였습니다.
HttpRequest static factory 메소드 와 ContentType Enum 리뷰사항은 시간이 모자라 아직 하지 못했습니다. ContentType Enum의 경우, ContentType.CSS / HTML 의 형태로 해서 필드로 헤더를 구성하는데 필요한 값을 가지는 형태로 구현하면 될 것 같습니다. 그리고 HttpRequest static factory의 방법은 아직 잘 모르겠습니다. 리뷰 감사드립니다! |
안녕하세요.
백엔드 반의 coco와 K입니다.
리팩토링 힌트를 참고했습니다.
구현 사항
위 3개를 추가하여 정리를 하였습니다.
HttpRequest
소켓의 inputstream에서 request를 파싱하는 역할
HttpResponse
소켓의 outputStream에 응답을 적어서 보내는 역할
Controller
HttpRequest 를 이용해서 HttpResponse 구성하고 응답을 보냄
감사합니다.