[톰캣 구현하기 - 3단계] 안나(김민겸) 미션 제출합니다. #652
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* test: 학습 테스트 진행 * feat: `/index.html`에 접근했을 때 static/index.html 파일이 보여지도록 구현 * feat: 자원에 따라 다른 Conent-Type으로 응답을 보내도록 수정 * fix: remove implementation logback-classic on gradle (woowacourse#501) * fix: 공백 처리 때문에 index.html이 불러와지지 않는 현상 해결 * feat: 로그인 요청 시 쿼리 파라미터를 파싱하는 기능 구현 * fix: add threads min-spare configuration on properties (woowacourse#502) * feat: 로그인에 성공하거나 실패한 경우 다른 페이지로 리다이렉트하는 * feat: POST 요청으로 회원 가입 시 요청 본문에서 정보를 가져와 사용자 저장하는 기능 구현 * feat: 로그인 시 POST 요청을 보내도록 수정 * feat: 로그인 시 세션 아이디를 쿠키에 등록하는 기능 구현 * feat: 쿠키에 JSESSIONID가 없을 때 Set-Cookie를 발급하는 기능 구현 * feat: 이미 로그인한 회원의 경우 로그인 페이지에 접근 불가하도록 구현 * refactor: 쿠키가 없을 때 일반 페이지에 접근하면 세션 아이디를 주지 않음 * feat: HTTP 활용 학습 테스트 진행 * refactor: 뭉친 코드를 클래스 분리하여 리팩토링 * fix: JSESSIONID를 추가하지 않아 로그인되지 않는 문제 해결 * fix: 세션 아이디가 달라서 로그인 시 로그인 페이지가 접속되는 현상 해결 * refactor: 기존 코드 주석 제거 * style: 주석 제거 및 컨벤션 맞추기 * refactor: 미구현된 파일 제거 * refactor: 필터를 사용하여 ETag를 지정하는 것으로 변경 * refactor: 인터셉터에서 CacheControl을 적용할 수 있도록 수정 * refactor: Content-Length 헤더 이름을 상수화 * refactor: 정적 파일을 한 번에 처리하도록 리팩토링 * fix: Json 파일을 보내는 경우를 고려하여 MediaType 수정 * refactor: 로그인 시 쿠키 세션을 꺼내오는 코드 가독성 개선 * refactor: 요청한 사용자 정보가 없는 경우 401 페이지를 보여주도록 수정 * refactor: 쿠키의 키값을 상수화하여 리팩토링 --------- Co-authored-by: Gyeongho Yang <chchch1213@naver.com>
jinchiim
requested changes
Sep 12, 2024
Member
jinchiim
left a comment
jinchiim
left a comment
There was a problem hiding this comment.
안녕하세요 안나! 🤭
우선 테스트를 제외하고 리뷰를 작성해보았습니다!
테스트 보강해주시면 추가 리뷰 작성해볼게요! 😋
그리고 현재, RequestHandler 의 httpRequest 쪽에서 반환해주는 타입이 달라 빌드가 되지 않고 있습니다!
이부분 한번만 확인해주세요! 이후 미션 요구사항도 같이 확인해볼게요!
미션과 프로젝트 둘 다 하느라 바쁘실텐데 화이팅입니다! 🥲
tomcat/src/main/java/org/apache/coyote/http11/common/ContentType.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/servlet/LoginServlet.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/servlet/RegisterServlet.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/servlet/RegisterServlet.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/session/Session.java
Outdated
Show resolved
Hide resolved
Mingyum-Kim
commented
Sep 13, 2024
Author
Mingyum-Kim
left a comment
Mingyum-Kim
left a comment
There was a problem hiding this comment.
안녕하세요 폴라.
꼼꼼하게 리뷰 남겨주어 감사합니다!
테스트는 모든 클래스에 대해서 작성하지는 못헀지만, 비즈니스 로직에 해당하는 유저 관리 부분에서 테스트를 작성하고 올려요.
확인 후 이상 없다면 Approve 남겨주시면 감사하겠습니다 😎
tomcat/src/main/java/org/apache/coyote/http11/servlet/LoginServlet.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/servlet/LoginServlet.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/servlet/RegisterServlet.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/servlet/RegisterServlet.java
Outdated
Show resolved
Hide resolved
jinchiim
reviewed
Sep 13, 2024
Comment on lines
43
to
45
| public void setVersionOfProtocol(String value) { | ||
| this.responseLine.setVersionOfProtocol(new VersionOfProtocol(value)); | ||
| } |
Member
There was a problem hiding this comment.
사용하지 않는 것으로 보이는데 삭제해도 좋을 것 같습니다!
tomcat/src/main/java/org/apache/coyote/http11/request/HttpRequest.java
Outdated
Show resolved
Hide resolved
jinchiim
requested changes
Sep 13, 2024
Member
jinchiim
left a comment
jinchiim
left a comment
There was a problem hiding this comment.
안녕하세요 안나! 리뷰 확인했습니다!!
최대한 4단계 제출을 위해 빠르게 달려보고 있는데, 아직 빌드가 되지 않는 것 같습니다! 🥲
검증과 같은 테스트와 빌드 확인 후 재요청 주시면 빠르게 확인해볼게요!
거의 다왔어요! 😆
jinchiim
approved these changes
Sep 13, 2024
Member
jinchiim
left a comment
jinchiim
left a comment
There was a problem hiding this comment.
안녕하세요 안나!
요구사항 전부 확인했습니다! 😋 3단계 고생 많으셨어요!!
4단계에서는 비교적 지금보다 볼륨이 작으니 천천히 상세 테스트와 추가적인 리팩토링 하여 요청주세요!
고생하셨습니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 폴라 😀
3단계 요구사항 반영 후 PR 드립니다.
아직 테스트를 다 구현하지 못했는데, 빠른 리뷰를 위해서 우선 올리고 오늘 내로 테스트 코드 만들어서 올리겠습니다.
잘 부탁드립니다!