Skip to content

[HTTP 서버 구현하기 - 2, 3단계] 포츈(정윤성) 미션 제출합니다. #104

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

Merged
merged 13 commits into from
Sep 6, 2021

Conversation

unluckyjung
Copy link

미션 제출이 굉장히 늦었습니다 ㅋㅋ 테코톡 있는 주는 역시 힘드네요 ㅠ

JSESSIONID 을 언제 발급하면 좋을지에 대한 고민을 굉장히 많이했는데요.
구현은 /login외의 요청에서도 Request에 JESSIONID 가 담겨져 있지 않은경우에는, Response에 Set-Cookie를 반환 하도록 했습니다.

이유는 JESSIONID 가 로그인외 다른 요청에서도 쓰일수 있다고 생각했어요.
예를들어 비로그인 상태에서의 팝업 이라던가...

그래서 테스트 코드에서도 try + staticMock을 이용하여,
UUID의 static method인 randomUUID 를 모킹한뒤, 프로덕트에서도 모킹된 값이 나오도록 한뒤 테스트를 진행했어요!

여유가 없어 코드 완성도가 개인적으로 아직 많이 부족하다고 생각하는데 ㅋㅋ
혹시 보시다가 이해가 안가거나, 모호한점 있으시면 언제든지 DM 주세요!

@unluckyjung unluckyjung self-assigned this Sep 4, 2021
Copy link

@kixtaxwax kixtaxwax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 포츈 ㅎㅎ 테코톡 고생 많으셨습니다~

잘 구현해주셔서 그런지 남길 피드백이 별로 없네요 ㅎㅎ

비로그인 상태 세션 발급에 대해서는 조금 얘기해보고 싶습니다!

고생 많으셨습니다 ㅎㅎ

@@ -37,14 +38,13 @@ public void run() {
);

HttpResponse httpResponse = new HttpResponse(outputStream);
cookieCheck(httpRequest, httpResponse);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비로그인 상태에서의 팝업같은 경우는 쿠키만으로도 해결할 수 있지 않을까요?

팝업 관련해서만 아니라 사실상 웹사이트에는 비로그인 상태의 접속이 많을 텐데, 모든 접속자에게 세션을 발급하면 사용자가 많아질수록 서버 메모리를 많이 차지하게 될 거라고 생각합니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일단 Set-Cookie : JSESSIONID 를 넣어라 하는것과, 실제 세션발급은 다르다는 이야기를 먼저 하고싶네요.

지금 정확히는, 비로그인시에는 세션을 발급하지 않고,
request 헤더필드에 JSESSIONID 필드가 없으면, Response에 Set-Cookie를 반환 하기만 합니다! 세션을 만들지 않아요.
세션은 로그인이 된경우에만 만들어지고있어요.

저 쿠키 체크는 서버가 클라이언트에게, 너 JSESSIONID 없이 요청 오고있네?, JSESSIONID 해서 다시보내! 라고 하고 있어요.

근데 이부분은 ㅋㅋ 저도 좀 고민을했고 다시보니 이상한 부분이라, 아예 JESSIONID 해서 다시보내! 하는 요청자체도 세션발급이랑 묶어서 리팩토링 할까 고민중이에요.

Comment on lines 12 to 16
if (httpRequest.getHttpMethod().equals(HttpMethod.GET)) {
doGet(httpRequest, httpResponse);
} else if (httpRequest.getHttpMethod().equals(HttpMethod.POST)) {
doPost(httpRequest, httpResponse);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RequestLine 클래스의 isGet, isPost 메서드를 활용해서 httpRequest.isPost도 될 것 같은데 어떻게 생각하시나요? ㅎ ㅎ

private HttpSession getHttpSessionByCookie(final HttpCookie cookie) {
return HttpSessions.getSession(cookie.getCookieValueByKey(JSESSIONID));
}

private boolean isValidateUser(final HttpRequest httpRequest) {
User user = getUser(httpRequest);
return user != null && isCollectPassword(httpRequest, user);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이걸 의도하신 게 맞겠죠?! 저도 속으로 발음하면서 코딩하느라 오타가 종종 나는데 왠지 반갑네요😂

Suggested change
return user != null && isCollectPassword(httpRequest, user);
return user != null && isCorrectPassword(httpRequest, user);

Comment on lines 27 to 28
try (MockedStatic<UUID> uuidMockedStatic = mockStatic(UUID.class)) {
UUID uuid = new UUID(1L, 2L);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MockedStatic 배워갑니다 👍👍👍👍👍

for (String cookieData : cookieDataSet) {
cookieData = cookieData.trim();
String[] cookieDatas = cookieData.split("=");
cookieMap.put(cookieDatas[COOKIE_KEY], cookieDatas[COOKIE_VALUE]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 keyValue[0] 이런 식으로 구현했었는데 인덱스 상수화를 하니까 읽기가 훨씬 편하네요👍

Copy link

@kixtaxwax kixtaxwax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

알림용 Request change..!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 5, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

86.7% 86.7% Coverage
0.0% 0.0% Duplication

@unluckyjung
Copy link
Author

#104 (comment)

여기에도 적어놨지만, 세션을 발급하는 작업이랑, 쿠키에 Set-Cookie 을 넣는 작업은 다르지만,
그냥 통일성있게, JESSIONID 넣어서 해서 다시보내! 하는 요청자체도 로그인에 성공한 경우에 세션발급이랑 묶어서 리팩토링 했습니다!

Copy link

@kixtaxwax kixtaxwax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

피드백 반영 수고하셨습니다!! 바쁘신 와중에 빠르게 구현하시는 모습이 인상적이었어요! 코드리뷰하면서 많이 배웠습니다 ㅎㅎ 다음 미션도 화이팅입니다!

@kixtaxwax kixtaxwax merged commit 80d6255 into woowacourse:unluckyjung Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants