[nathan & jerry] 웹서버 4단계 - 쿠키를 이용한 로그인 구현#63
Open
jeremy0405 wants to merge 13 commits intocodesquad-members-2022:nathan-jerryfrom
Open
[nathan & jerry] 웹서버 4단계 - 쿠키를 이용한 로그인 구현#63jeremy0405 wants to merge 13 commits intocodesquad-members-2022:nathan-jerryfrom
jeremy0405 wants to merge 13 commits intocodesquad-members-2022:nathan-jerryfrom
Conversation
wheejuni
requested changes
Mar 31, 2022
wheejuni
left a comment
wheejuni
left a comment
There was a problem hiding this comment.
수고 많으셨습니다. 💯
그런데 테스트가 너무 없습니다.
특히 Pair 클래스에 대한 테스트는 모든 경우의 수에 대해 진행해 주시면 좋겠습니다.
Comment on lines
+21
to
+23
| map.put(new ControllerMapper(HttpMethod.POST, "/user/create"), UserJoinController.getInstance()); | ||
| map.put(new ControllerMapper(HttpMethod.GET, "/user/logout"), UserLogoutController.getInstance()); | ||
| map.put(new ControllerMapper(HttpMethod.POST, "/user/login"), UserLoginController.getInstance()); |
There was a problem hiding this comment.
이미 여기서 하나의 객체만 보관해서 로직을 처리할때 꺼내는 구조로 되기 때문에, 굳이 하부 컨트롤러들이 싱글턴일 이유가 없어 보이기도 합니다.
|
|
||
| @Override | ||
| public void process(Request request, Response response) throws IOException { | ||
| byte[] body = Files.readAllBytes(new File("./webapp" + request.getPath()).toPath()); |
There was a problem hiding this comment.
FileReader 같은 클래스가 따로 있으면 좋을 것 같은 느낌이네요~
| } | ||
| log.debug("Save Fail: {}", user); | ||
| pairs.add(new Pair("Location", "http://localhost:8080/user/form.html")); | ||
| response.write(HttpStatus.FOUND, pairs); |
| @@ -0,0 +1,61 @@ | |||
| package util; | |||
|
|
|||
| public class Pair { | |||
There was a problem hiding this comment.
이런 종류의 클래스를 설계하실 때는 정말 정말 신중해야 합니다.
- 꼭 필요한가요?
- 이미 있지 않을까요? 다른 라이브러리에서 찾아 보았습니까?
- 테스트가 충분히 되었나요?
이 세가지 질문에 당당하게 대답하실 수 있으셔야 합니다.
Comment on lines
+4
to
+5
| private final String key; | ||
| private final String value; |
| private final String value; | ||
|
|
||
| public Pair(String key, String value) { | ||
| this.key = key.trim(); |
Comment on lines
+20
to
+22
| public boolean isContentLength() { | ||
| return key.equals("Content-Length"); | ||
| } |
There was a problem hiding this comment.
범용적으로 사용될 수 있는 클래스인데... 프레젠테이션 계층에 강하게 결합하고 있군요.
별로 좋은 메소드가 아닌 듯 합니다 ^^;;
Comment on lines
+7
to
+15
| public static HttpMethod create(String httpMethod) { | ||
| if (httpMethod.equals("GET")) { | ||
| return GET; | ||
| } | ||
| if (httpMethod.equals("POST")) { | ||
| return POST; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
앞으로 메서드가 추가될때마다 (PUT, PATCH 등...) 로직도 계속 새로 작성하실 거죠?
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
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.
안녕하세요 nathan과 jerry입니다.
4단계 미션 PR을 보냅니다. 😄
항상 감사드립니다.
기능요구사항
한 일
refactor: Controller 역할을 하던 Response 클래스를 추출해서 Controller Class 작성
feat: 로그인, 로그아웃 기능 구현 (SessionId=uuid 형태로 쿠키 세팅)
docs: 미션 4단계에서 학습한 내용 README 정리