[톰캣 구현하기 1, 2단계] 트레 미션 제출합니다. #510
Conversation
| private String handle(HttpRequest request) { | ||
| HttpCookie cookie = new HttpCookie(request.getHeader(COOKIE)); | ||
| RequestBody requestBody = request.getRequestBody(); | ||
|
|
||
| if (request.pointsTo(GET, "/")) { | ||
| return HttpResponse.ofContent("Hello world!") | ||
| .build(); | ||
| } | ||
|
|
||
| if (request.pointsTo(GET, "/login")) { | ||
| return getLoginPage(cookie); | ||
| } | ||
|
|
||
| if (request.pointsTo(POST, "/login")) { | ||
| return login(cookie, requestBody); | ||
| } | ||
|
|
||
| if (request.pointsTo(POST, "/register")) { | ||
| return saveUser(cookie, requestBody); | ||
| } | ||
|
|
||
| return HttpResponse.ofStaticFile(request.getPath().substring(1), HttpStatusCode.OK) | ||
| .cookie(cookie) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
HttpRequest에 handle을 수행할 수 있는 모든 정보를 가지고 있는데 객체에게 물어보도록 변경해 보는 건 어떻게 생각하시나용? 😄 아래의 의견 주신 것에 대해 힌트도 될 것 같아요!
JSESSIONID가 없으면
Set-Cookie를 응답한다. 라는 요구사항이 있었는데, 이러한 분기처리를 어떤 클래스에 구현해야 할지 감이 잘 안 잡혔던 것 같아요. 더 좋은 방법 없을까요?!
HttpResponse response = request.handle();
response.cookie(request.getSession());There was a problem hiding this comment.
HttpCookie랑 HttpRequest에게 물어보는 방식으로 바꿔봐야겠네요!
같이 고민해줘서 고마워요 😄
| public Object getAttribute(String name) { | ||
| return values.get(name); | ||
| } | ||
|
|
||
| public void removeAttribute(String name) { | ||
| values.remove(name); | ||
| } |
| private static final SessionManager INSTANCE = new SessionManager(new HashMap<>()); | ||
|
|
||
| private final Map<String, Session> sessions; | ||
|
|
||
| private SessionManager(Map<String, Session> sessions) { | ||
| this.sessions = sessions; | ||
| } | ||
|
|
||
| public static SessionManager getInstance() { | ||
| return INSTANCE; |
| return HttpResponse.redirectTo("/index.html") | ||
| .cookie(cookie) | ||
| .build(); |
There was a problem hiding this comment.
세션을 설정하기 위해서 cookie 메서드를 중복으로 사용하는 느낌이 들어요 😄 코멘트처럼 build를 각 메서드 안에서 처리하기보단 HttpResponse를 반환하는 방향으로 가는건 어떻게 생각하시나요?
There was a problem hiding this comment.
Set-Cookie가 필요 없는 곳까지 의미 없게 호출하고 있었어요.
필요한 부분에서만 헤더를 설정하도록 바꿨어요!
| if (requestBody.containsAll("account", "password")) { | ||
| String account = requestBody.get("account"); | ||
| String password = requestBody.get("password"); | ||
|
|
|
|
||
| private static final FileReader FILE_READER = FileReader.getInstance(); | ||
|
|
||
| private static final String JSESSIONID = "JSESSIONID"; |
There was a problem hiding this comment.
JSESSIONID 상수가 여기저기 있는게 보이네요 😃
세션 존재, 검증, 생성 부분은 cookie 객체에서 해주는 건 어떻게 생각하시나요?
// 세션id 얻기
String sessionId = cookie.getSession();
// 세션 검증
if(cookie.hasSession()) {
// ...
}
// 세션 생성
cookie.makeSession();There was a problem hiding this comment.
Cookie 밖에서는 모르도록 수정하였습니다!!!
There was a problem hiding this comment.
안녕하세요 트레 🏃
레벨 4 리뷰어 포케 🥗 입니다. 😄
필요한 기능을 적재적소에 배치하고, 확신이 서지 않는 부분에 대해 테스트 코드 작성하는 등 정말 담백하게 작성하셔서 리뷰 할 부분이 많이 없네요 😅
제가 오히려 트레의 코드를 보면서 성장한 느낌이 들어서 좋았습니다. 잘해주셨어요 ㅎㅎ
몇 가지 코멘트 남겼으니 확인 부탁드려요 ㅎㅎ 🙏
리뷰에 대해 코멘트 혹은 반영 후 Approve & Squash Merge 드릴 예정이니 좀 더 고치고 싶은 부분이 있거나 고민되는 부분이 있다면 공유 바랍니다 😄
감사합니다
|
안녕하세요 포케! |
| public static Session ofUser(User user) { | ||
| Session session = new Session(); | ||
| session.values.put(USER_SESSION_NAME, user); | ||
| return session; |
|
|
||
| public class HttpHeader { | ||
|
|
||
| private static final String DELIMITER = ": "; |
There was a problem hiding this comment.
RFC 2616 4.2를 참고하면, DELIMITER 기준으로 field-name은 공백이 없으면 안되고, field-value는 공백을 허용합니다. 관련 내용을 적용해주시고 테스트 수정 부탁드려요~
| public String get(String name) { | ||
| return headers.get(name); | ||
| } | ||
|
|
||
| public boolean contains(String name) { | ||
| return headers.containsKey(name); | ||
| } |
There was a problem hiding this comment.
위의 코멘트와 동일하게 field-name은 case-insensitive 합니다 😃
There was a problem hiding this comment.
문서 보면서 전체적으로 점검해볼게요~~!
|
|
||
| public class SessionManager implements Manager { | ||
|
|
||
| private static final SessionManager INSTANCE = new SessionManager(new HashMap<>()); |
There was a problem hiding this comment.
현재는 INSTANCE가 상태 값이 변경되는 값을 저장하고 있어요.
상수 네이밍은 upper snake-case를 사용하는게 관례인데, INSTANCE가 변하지 않는 상수가 맞을까요?
There was a problem hiding this comment.
내부 값이 변하고 있기는 하지만, 인스턴스 자체는 변하지 않고 있어서 upper-case로 두었어요!
|
|
||
| @Configuration | ||
| public class CacheWebConfig implements WebMvcConfigurer { | ||
|
|
||
| @Override | ||
| public void addInterceptors(final InterceptorRegistry registry) { | ||
| WebContentInterceptor noCacheIntercepter = new WebContentInterceptor(); | ||
| noCacheIntercepter.addCacheMapping(CacheControl.noCache().cachePrivate(), "/**"); | ||
| registry.addInterceptor(noCacheIntercepter) |
안녕하세요 포케!
월요일이 되기 전까지만 리뷰해주시면 될 것 같아요. 잘 부탁드립니다!
단계별 커밋 목록
2단계까지는 코드가 많이 지저분합니다 😢
학습 테스트는 전체 files changed에서 봐주세요!
1단계
2단계
리팩터링
참고 사항
Set-Cookie를 응답한다. 라는 요구사항이 있었는데, 이러한 분기처리를 어떤 클래스에 구현해야 할지 감이 잘 안 잡혔던 것 같아요. 더 좋은 방법 없을까요?!