-
Notifications
You must be signed in to change notification settings - Fork 0
[Tomcat 구현하기] 2-4단계 - HTTP 서버 구현하기 #2
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
base: siso-again
Are you sure you want to change the base?
Conversation
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| public class LoginController implements Controller { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 유저가 작성한다고 생각한 부분을 아예 com 쪽으로 분리해 주었어요!
| private final Map<String, Controller> controllers = new HashMap<>(); | ||
|
|
||
| public RequestMapping() { | ||
| controllers.put("/", new HomeController()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분도 URI는 유저가 생성하는 부분이니 com에 배치하였지만 역할적으로는 catalina에 위치하는 게 맞다고 생각해요 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 RequestMapping 자체는 catalina에 위치하는게 맞다고 생각해요
그렇다면 외부에서 유저가 추가할 수 있도록 구현하는 방법을 고민해보는게 좋을 것 같아요 어떤 방법이 있을 수 있을까요?
그리고 WAS에서는 이를 어떻게 처리할까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
실제 WAS에서는 초기 설정 시 자동으로 컨트롤러를 검색하여 매핑하거나 필요 시 동적으로 핸들러를 등록하는 방식을 사용하고 있어요!
저의 경우에는 어노테이션 방식은 추후 구현 부분인 것 같아서 동적 처리 방법의 일환으로 ControllerRegistry를 도입해 유저 구현 부분을 분리해 주었어요
하지만 이것은 ControllerRegistry를 유저측에서 구현해 두지 않으면 오류가 발생하는 구조라 톰캣이 프레임워크처럼 틀을 두고 동작하게 됩니다! 자동 스캔 방식을 사용하지 않는 한 어쩔 수 없는 문제라는 판단하에 추후 미션에서 어노테이션을 도입해 볼 것 같네요 🙄
| private Http11Response handleRequest(Http11Request request) throws Exception { | ||
| Controller controller = requestMapping.getController(request); | ||
| if (controller == null) { | ||
| controller = new FileController(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fileController는 유저가 관리하는 코드는 아니라고 생각해서 catalina 부분에 배치한 뒤
RequestMapping에서도 완전히 배제시켰습니다!
모든 컨트롤러에 해당되지 않을 때 FileController를 기본으로 들고 나오게 돼요
해당하는 파일이 없다면 예외가 발생합니다
| this.cookie = cookie; | ||
| } | ||
|
|
||
| public void addLocation(String location) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Location까지 들어오며 상당히 멘붕이더라고요
기본 생성자에 포함시키지 않은 이유는 redirect할 때만 사용하는 특수 헤더이기 때문이에요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나머지는 필수 헤더일까요?? 그게 아니라면 다른 필드도 location처럼 외부에서 add하는 방식이 더 맞지 않을까용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 기본 생성자로 주입 중인 헤더는
ContentType contentType, Long contentLength, HttpCookie cookie 세 가지의 헤더인데 셋 다 선택적 헤더라고 볼 수 있습니다
하지만 제가 지원하는 HTTP 서버가 HTTP/1.1버전이며 톰캣의 구현으로 바라 볼 때 일반적으로 톰캣은 Content-Type과 Content-Length를 필요 시 자동으로 설정해 주는 구현이 있어 해당 부분은 필수로 구현하였습니다
Cookie의 경우에는 특수 헤더이기 때문에 따로 구현하였습니다!
Location 부분부터는 Map의 형태로 헤더를 저장하려 했으나 현재 필요성을 느끼지 못해 추가적으로 필요한 헤더가 생긴다면 해당 부분과 함께 리팩토링 작업을 진행할 예정이에요
JINU-CHANG
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔하게 구현해주셨군용 깨달음을 얻고 가는 부분도 많았습니다 👍
궁금했던 부분 코멘트 남겨두었으니 확인해주세요 ~~
고생많으셨습니다
| } | ||
| } | ||
|
|
||
| protected void doGet(Http11Request request, Http11Response response) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AbstractController를 활용하지 않고 doGet, doPost를 구현하신 이유가 따로 있나요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분을 추상화할 생각을 하지 않았는데 추상화한다면 좀 더 복잡한 로직이 줄어들겠네요!
| @Override | ||
| public void service(Http11Request request, Http11Response response) throws Exception { | ||
| HttpMethod method = request.getHttpMethod(); | ||
| if (method.equals(HttpMethod.GET)) { | ||
| doGet(request, response); | ||
| } | ||
|
|
||
| if (method.equals(HttpMethod.POST)) { | ||
| doPost(request, response); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 Controller마다 동일하게 반복되는 로직이네요! 이를 분리해볼 수 있을까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AbstractController를 구현하여 중복 코드를 제거하였습니다!
| private final Map<String, Controller> controllers = new HashMap<>(); | ||
|
|
||
| public RequestMapping() { | ||
| controllers.put("/", new HomeController()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 RequestMapping 자체는 catalina에 위치하는게 맞다고 생각해요
그렇다면 외부에서 유저가 추가할 수 있도록 구현하는 방법을 고민해보는게 좋을 것 같아요 어떤 방법이 있을 수 있을까요?
그리고 WAS에서는 이를 어떻게 처리할까요??
| String account = request.getBodyValue("account"); | ||
| String password = request.getBodyValue("password"); | ||
|
|
||
| if (account == null || password == null) { | ||
| Http11ResponseBuilder.buildFile(response, StatusCode.OK, ContentType.TEXT_HTML_UTF8, "login.html"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
세션값이 존재할 때 리다이렉트되어야하는데 해당 코드가 보이지 않는 것 같아요 어떻게 처리되고 있는걸까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로 구현하였습니다!
| public static void buildRedirect(Http11Response response, HttpCookie cookie, String redirectUri) { | ||
| ResponseStatusLine statusLine = new ResponseStatusLine(HTTP_11, StatusCode.FOUND); | ||
| ResponseHeader header = new ResponseHeader(null, null, cookie); | ||
| header.addLocation(redirectUri); | ||
| response.add(statusLine, header, null); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Response를 생성하는 코드를 어떻게 한 곳에 관리해야하는지 고민하고 있었는데 깨달음을 얻고 갑니당
| this.cookie = cookie; | ||
| } | ||
|
|
||
| public void addLocation(String location) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나머지는 필수 헤더일까요?? 그게 아니라면 다른 필드도 location처럼 외부에서 add하는 방식이 더 맞지 않을까용
|
안녕하세요 제제! |
|
답변 모두 확인했습니다 ~ 길고 길었던 미션 드디어 마무리하네요 |
안녕하세요 제제!
지독한 독감과 코드 대공사를 거치니 좀 늦었어요 😅
상세한 코멘트는 코드 아래에 달도록 하겠습니다 ㅎㅎ