[1 - 2단계 Tomcat 구현하기] 조조(조은별) 미션 제출합니다 by eun-byeol · Pull Request #518 · woowacourse/java-http

eun-byeol · 2024-09-06T05:55:31Z

도라 반가워요😄

1~2.2단계까지 구현했습니다.
자세한 기능 목록은 README에 적어놓았으니, 확인해주시면 좋을 것 같아요!

아직 코드가 많이 깔끔하지 못해요. 더 많이 리팩터링을 하고싶었는데 시간상 하지 못했네요..! 아래 TODO는 추후 반영해놓겠습니다.

TODO:
- response 중복 코드 제거
- 회원가입시 중복 account 예외처리
- Content-Type or Accept로 분기처리(현재는 path의 확장자로 확인하고 있음)

코드에서 읽기 힘든 부분이 있다면 주저없이 바로 DM 주세요!
감사합니다:)

ChooSeoyeon

안녕하세요 조조!

미션 요구사항을 잘 만족해주신 거 같아요!
리팩토링은 다음 단계에 집중적으로 리뷰할 예정이라,
구조적인 부분보단 기능적인 부분 위주로 리뷰 남겼습니다:)

학습테스트 관련해선 조조의 생각 위주로 답변 남겨주시면 감사하겠습니다!

다음 리뷰 요청 시 머지할 예정입니다! 화이팅입니다~

ChooSeoyeon · 2024-09-08T06:50:06Z

-        // todo
-        final String actual = "";
+        URL url = getClass().getClassLoader().getResource(fileName);
+        final String actual = Objects.requireNonNull(url).getFile();


null 처리 명시적으로 해주는 거 좋네요~
사소한 부분이긴 한데, getFile과 getPath 간에 어떤 차이가 있는지 알아보셔도 좋을 거 같아요!

getPath: 경로만 반환

getFile: 경로 + 쿼리 문자열 반환

덕분에 새로운 내용 학습하고 갑니다👍

ChooSeoyeon · 2024-09-08T07:04:33Z


-        assertThat(actual).containsOnly("nextstep");
+        try (BufferedReader bufferedReader = Files.newBufferedReader(path)) {
+            List<String> actual = bufferedReader.lines().toList();


바꾸자는 건 아니고 궁금한 점입니다:)
해당 테스트에서 Files.lines(path)를 사용하는 대신,
Files.newBufferedReader(path)와 bufferedReader.lines()를 조합해 사용하는 방식을 선택하신 이유가 궁금합니다!

큰 이유는 없었어요. 테스트가 돌아가게 하는게 먼저였습니다.
언급해주신 Files.lines(path)으로 바로 Stream을 만드는게 더 간단하네요.
현재 로직에서는 BufferedReader를 Stream으로 바꾸고 다시 List로 만들고 있으니 불필요하구요👍

ChooSeoyeon · 2024-09-08T07:18:46Z


+            try (inputStream) {
+            } catch (Exception e) {
+            }


자원해제를 위해 반드시 catch가 필요한 건 아니기에,
지금처럼 예외를 별도로 처리하지 않고 있다면 catch문을 빼보는 것도 고려해볼 수 있을 거 같습니다!
어떻게 생각하시는지 궁금합니다!

자원해제시에 catch문이 꼭 필요하지 않다는 것에 동의해요!
다만, catch 문을 둔 이유는 자원해제시 에러가 발생했을 때 예외를 던지지 않게 하기 위함이었어요. 코드는 없지만 예외를 처리하고 있다고 생각해요. 다만, 로그를 남겼으면 더 좋을 것 같네요!

ChooSeoyeon · 2024-09-08T08:57:46Z

+            String responseBody = new String(Files.readAllBytes(new File(resource.getFile()).toPath()));
+            return String.join("\r\n",
+                    "HTTP/1.1 200 OK ",
+                    "Content-Type: text/css;charset=utf-8 ",


Content-Type css 의도하신거 맞나용?

실수한 부분이 있었네요😅 javascript로 수정했습니다! 예리하시네요👍

ChooSeoyeon · 2024-09-08T10:30:21Z

+                    email = tokenizer.nextToken();
+                }
+            }
+            InMemoryUserRepository.save(new User(account, password, email));


account, password, email 값이 없을 때 "", "", "" 값이 user에 저장되는거 의도하신 거 맞나용??

해피케이스만 고려를 해서, 아직 예외처리를 하지 못했어요.

+) 우선은 간단하게 예외 발생시키는 부분만 추가했어요. 핸들링과 다른 예외처리 부분도 추후 구현하겠습니다👍

if (account.isBlank() || password.isBlank() || email.isBlank()) { // TODO: 예외처리 개선 throw new IllegalArgumentException("올바르지 않은 request body 형식입니다."); }

ChooSeoyeon

안녕하세요 조조!

미션 요구사항을 잘 만족해주신 거 같아요!
리팩토링은 다음 단계에 집중적으로 리뷰할 예정이라,
구조적인 부분보단 기능적인 부분 위주로 리뷰 남겼습니다:)

학습테스트 관련해선 조조의 생각 위주로 답변 남겨주시면 감사하겠습니다!

다음 리뷰 요청 시 머지할 예정입니다! 화이팅입니다~

eun-byeol · 2024-09-10T04:58:02Z

도라, 조조입니다! 코드가 많이 지저분했을텐데, 꼼꼼하게 봐주셔서 감사해요☺️
2.4까지 구현 완료하고 추가 리뷰보내려고 했는데, 더 길어질 것 같아 리뷰만 반영 후에 다시 요청드렸어요.
다음 PR에는 2.4 + 3단계를 구현 후에 PR 보낼 예정입니다.
요즘도 많이 바쁠텐데, 화이팅합시다!💪

ChooSeoyeon · 2024-09-10T08:19:20Z

조조! 도라입니다:)
리뷰 남겼던 부분 꼼꼼히 반영해주셨네요!
이번 단계는 여기까지 진행하고 다음 단계에서 뵙겠습니다:)

eun-byeol added 16 commits September 4, 2024 13:19

test: File, IOStream 학습 테스트 솔루션 추가

d128f4c

docs: 1단계 기능 목록 추가

ed18268

feat: GET /index.html 응답하기

0299545

fix: application 실행 시 InputStream.readAllBytes 오류 해결

1f140bd

feat: CSS 지원하기

0906d54

docs: 생각해보기 내용 추가

d41f5d5

feat: 로그인 페이지 응답하기

fd17035

feat: Query String 파싱 및 회원 조회

ceeb1d0

docs: 2단계 기능 목록 추가

76f109b

feat: 로그인 성공 시 index 페이지 리다이렉트

6c30f87

feat: 로그인 실패 시 404 페이지 리다이렉트

7809d51

test: 회원가입 페이지 응답하기

7807218

feat: POST 방식으로 회원가입

03c9397

refactor: request header, body 파싱 로직 개선

2d7985a

feat: POST 방식으로 로그인

35d4a61

refactor: get, post 요청 분기 로직 개선

7819082

eun-byeol requested a review from ChooSeoyeon September 6, 2024 05:55

3Juhwan deleted the branch woowacourse:eun-byeol September 6, 2024 06:51

3Juhwan closed this Sep 6, 2024

eun-byeol reopened this Sep 6, 2024

eun-byeol changed the title ~~[1 -2단계 - Tomcat 구현하기] 조조(조은별) 미션 제출합니다~~ [1 - 2단계 Tomcat 구현하기] 조조(조은별) 미션 제출합니다 Sep 6, 2024

eun-byeol added 3 commits September 6, 2024 22:49

fix: 로그인 실패 시 401 페이지 반환하도록 수정

4152057

docs: 수정된 기능 문서 반영

d327ee9

test: 사용하지 않는 throws Exception 제거

bc00f19

ChooSeoyeon reviewed Sep 8, 2024

View reviewed changes

ChooSeoyeon requested changes Sep 8, 2024

View reviewed changes

eun-byeol added 3 commits September 10, 2024 13:23

test: 파일 읽기 로직 개선

666f6e2

fix: js 파일 Content-Type 수정

1844a1f

feat: 회원가입 폼 입력이 올바르지 않은 경우 예외 발생

6abd94d

ChooSeoyeon merged commit 2b53964 into woowacourse:eun-byeol Sep 10, 2024

Conversation

eun-byeol commented Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChooSeoyeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eun-byeol Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChooSeoyeon left a comment

Choose a reason for hiding this comment

Uh oh!

eun-byeol commented Sep 10, 2024

Uh oh!

ChooSeoyeon commented Sep 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eun-byeol commented Sep 6, 2024 •

edited

Loading

eun-byeol Sep 10, 2024 •

edited

Loading