Skip to content
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

[톰캣 구현하기 - 3, 4단계] 성하(김성훈) 미션 제출합니다. #414

Merged
merged 23 commits into from
Sep 10, 2023

Conversation

sh111-coder
Copy link

안녕하세요~ 비버! 성하입니다! 😃

이번 3, 4단계에서는 컨트롤러 구현, 동시성 확장 기능을 구현했습니다!
HttpRequest, HttpResponse는 2단계에서 구현을 했었어서 거의 기존 구조 그대로 가져갔습니다!

컨트롤러는 요구사항 및 힌트대로 RequestMapping, Controller 인터페이스를 상속한 AbstaractController를
각각 컨트롤러가 상속받아서 구현했습니다.

Get, Post 요청 이외의 요청 시에는 예외가 발생하게 했고,
Get, Post 요청 처리 로직은 추상 메소드로 각 컨트롤러에서 구현하여 처리하도록 구현했습니다!

1,2 단계 때는 Http Request 요청이 들어 왔을 때 handle을 제대로 해주는지 판단하는 테스트가 없었어서
이번에 컨트롤러를 만들면서 빡세게 ControllerTest를 진행해봤습니다!!

리뷰 잘 부탁드립니다!! 🙇🏻‍♂️

@sh111-coder sh111-coder self-assigned this Sep 8, 2023
Copy link

@ingpyo ingpyo left a comment

Choose a reason for hiding this comment

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

안녕하세요 성하!
3, 4단계도 엄청나네요..

Comment on lines +19 to +26
private static final int DEFAULT_MAX_THREAD_SIZE = 250;

private final ServerSocket serverSocket;
private final ExecutorService executorService;
private boolean stopped;

public Connector() {
this(DEFAULT_PORT, DEFAULT_ACCEPT_COUNT);
this(DEFAULT_PORT, DEFAULT_ACCEPT_COUNT, DEFAULT_MAX_THREAD_SIZE);
Copy link

Choose a reason for hiding this comment

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

👍

public HttpResponse service(final HttpRequest request) throws Exception {
if (request.getHttpMethod() == HttpMethod.GET) {
return doGet(request);
} else if (request.getHttpMethod() == HttpMethod.POST) {
Copy link

Choose a reason for hiding this comment

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

else 없어도 될거같은데 어떻게 생각하시나요?!

Copy link
Author

Choose a reason for hiding this comment

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

수정했습니다!!

Comment on lines 33 to 37
for (String uri : requestMapingMap.keySet()) {
if (uri.equals(mappingUri)) {
return requestMapingMap.get(uri);
}
}
Copy link

Choose a reason for hiding this comment

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

Stream으로 바꿔보는게 어떨까요??

Copy link
Author

Choose a reason for hiding this comment

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

수정했스빈다!!

@sonarcloud
Copy link

sonarcloud bot commented Sep 10, 2023

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 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

Copy link

@ingpyo ingpyo left a comment

Choose a reason for hiding this comment

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

안녕하세요 성하!
코드 잘봤습니다. 리뷰도 잘 반영된거 같아요 👍

이만 머지하겠습니다!!

@ingpyo ingpyo merged commit ed5b467 into woowacourse:sh111-coder Sep 10, 2023
2 checks passed
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.

None yet

2 participants