[톰캣 구현하기 - 1,2단계] 베베(최원용) 미션 제출합니다. #322
Conversation
There was a problem hiding this comment.
안녕하세요, 베베.
우선 리뷰 요청 빠르게 주셨는데, 이렇게 늦게 리뷰하게 되어 죄송합니다. 다음 리뷰부터는 재빠르게 하도록 하겠습니다.
커밋을 작은 단위로 잘 해주신 리뷰하기 정말 편했습니다👍
순서대로 모든 커밋을 보았는데 메서드 분리, 역할에 맞는 클래스 분리 및 위임을 정말 잘하신 것 같습니다. 코드가 깔끔해서 보고 배웠습니다.
전체적인 구조나 흐름은 제 마음에 들기 때문에 구조에 대한 리뷰는 딱히 없습니다.
리뷰 확인하신 뒤에 의견 나누고 싶은 부분은 코멘트 남겨주시면 감사하겠습니다. DM 주셔도 너무 좋습니다.
tomcat/src/main/java/org/apache/coyote/http11/request/line/Protocol.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/auth/AuthService.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/auth/AuthService.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/request/line/vo/DefaultPath.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/request/line/Protocol.java
Show resolved
Hide resolved
| if (session != null) { | ||
| return new ResponseEntity(FOUND, INDEX_PAGE); | ||
| } | ||
| return new ResponseEntity(OK, LOGIN_PAGE); |
There was a problem hiding this comment.
로그인된 상태에서 /login 페이지에 HTTP GET method로 접근하면 이미 로그인한 상태니 index.html 페이지로 리다이렉트 처리한다.
2단계의 4번에 위와 같은 요구사항이 있습니다.
현재는 로그인한 상태에서 로그인 페이지로 이동했을 때 인덱스 페이지로 리다이렉트되지 않는 것 같아요. 확인 부탁드립니다!
There was a problem hiding this comment.
엇.. 저도 몰랐는데 안되는군요..ㅋㅋㅋㅋㅋ 역시 개발 어렵습니다 ㅠ
tomcat/src/main/java/org/apache/coyote/http11/request/line/Path.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/ResponseEntity.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/HttpResponseGenerator.java
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
| if (session == null) { | ||
| return ResponseEntity.getCookieNullResponseEntity(protocol, OK, Location.from(LOGIN_PAGE)); |
There was a problem hiding this comment.
이전 커밋을 보니 세션에 저장된 user 정보를 확인하는 로직을 작성하셨던 것 같네요. 어쩌다 다시 삭제된 건지 알 수 있을까요???
| } | ||
|
|
||
| private ResponseEntity getSuccessLoginResponse(final User user, final Protocol protocol) { | ||
| ResponseEntity response = ResponseEntity.getCookieNullResponseEntity(protocol, FOUND, |
There was a problem hiding this comment.
세션을 생성하고는 있지만, 쿠키를 null로 생성하고 있어서 JSESSIONID가 쿠키에 담겨 있지는 않은 구조인 것 같아요.
브라우저에서 로그인을 성공해도 쿠키가 생성되지 않더라고요.
| private final Cookie cookie; | ||
| private final Location location; | ||
|
|
||
| public ResponseEntity(Protocol protocol, HttpStatus httpStatus, Cookie cookie, Location location) { |
There was a problem hiding this comment.
호출되는 곳이 팩토리 메서드뿐이라 접근제어자 수정하면 좋을 거 같습니다~
|
|
||
| private final SessionRepository sessionRepository = new SessionRepository(); | ||
|
|
||
| public ResponseEntity login(RequestLine requestLine, RequestHeader requestHeader, RequestBody requestBody) { |
There was a problem hiding this comment.
GET 요청을 처리할 때와 POST 요청을 처리할 때의 로직을 각각 메서드로 분리하면 더 깔끔할 거 같네요. 이외에 현재 지원하지 않는 HTTP 메서드에 대해서는 예외 처리하기도 깔끔할 것 같고요~
안녕하세요 민트~ 이번에 톰캣을 구현하면서 WAS의 내부 동작 원리를 약간은 알게된 미션이었네요 ㅎㅎ; 아직까지 추상화랑 코드 정리가 덜 된 부분들이 있어서 보기가 불편하실 수도 있겠어요 ㅠㅠ 바쁘시겠지만 코드리뷰 잘 부탁드립니다!