-
Notifications
You must be signed in to change notification settings - Fork 309
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단계] 로이 미션 제출합니다. #494
Changes from all commits
e4900af
63b4190
13f8efd
6cb1b9a
c3ecfe8
938aaba
118768f
0cf6cd6
7a95a6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,9 @@ protected HttpResponse doGet(HttpRequest request) throws Exception { | |
String requestUrl = request.getRequestUrl(); | ||
Path path = PathFinder.findPath(requestUrl + ".html"); | ||
var responseBody = new String(Files.readAllBytes(path)); | ||
return new HttpResponse(HttpStatus.OK, responseBody, ContentType.HTML); | ||
return new HttpResponse.Builder(HttpStatus.OK, responseBody, ContentType.HTML) | ||
.build(); | ||
|
||
} | ||
|
||
@Override | ||
|
@@ -44,7 +46,11 @@ protected HttpResponse doPost(HttpRequest request) throws Exception { | |
saveUser(registerData); | ||
Path path = PathFinder.findPath(requestUrl + ".html"); | ||
String responseBody = new String(Files.readAllBytes(path)); | ||
return new HttpResponse(HttpStatus.FOUND, responseBody, ContentType.HTML, "/index.html"); | ||
return new HttpResponse.Builder(HttpStatus.FOUND, responseBody, ContentType.HTML) | ||
.redirect("/index.html") | ||
.build(); | ||
|
||
|
||
} | ||
|
||
private boolean checkInputForm(final HashMap<String, String> registerData) { | ||
|
@@ -56,7 +62,10 @@ private boolean checkInputForm(final HashMap<String, String> registerData) { | |
private HttpResponse generateBadRequestResponse() throws URISyntaxException, IOException { | ||
Path path = PathFinder.findPath("/400.html"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 혹시 400.html은 별도로 존재했다가 삭제된 정적 파일일까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 샘플 코드로 주어진 것으로 잘못 알았네요ㅜ 400에러 페이지를 추가해봤습니다! 세심한 피드백 감사합니다👍 |
||
String responseBody = new String(Files.readAllBytes(path)); | ||
return new HttpResponse(HttpStatus.FOUND, responseBody, ContentType.HTML, "/400.html"); | ||
return new HttpResponse.Builder(HttpStatus.FOUND, responseBody, ContentType.HTML) | ||
.redirect("/400.html") | ||
.build(); | ||
|
||
} | ||
|
||
private void saveUser(HashMap<String, String> registerData) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,24 +8,32 @@ | |
import java.io.UncheckedIOException; | ||
import java.net.ServerSocket; | ||
import java.net.Socket; | ||
import java.util.concurrent.ExecutorService; | ||
import java.util.concurrent.Executors; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
public class Connector implements Runnable { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(Connector.class); | ||
|
||
private static final int DEFAULT_PORT = 8080; | ||
private static final int DEFAULT_ACCEPT_COUNT = 100; | ||
private static final int DEFAULT_MAX_THREADS = 200; | ||
|
||
private static final int TERMINATION_LIMIT_SECONDS = 60; | ||
|
||
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_THREADS); | ||
} | ||
|
||
public Connector(final int port, final int acceptCount) { | ||
public Connector(final int port, final int acceptCount, final int maxThreads) { | ||
this.serverSocket = createServerSocket(port, acceptCount); | ||
this.stopped = false; | ||
this.executorService = Executors.newFixedThreadPool(maxThreads); | ||
} | ||
|
||
private ServerSocket createServerSocket(final int port, final int acceptCount) { | ||
|
@@ -67,15 +75,27 @@ private void process(final Socket connection) { | |
return; | ||
} | ||
var processor = new Http11Processor(connection); | ||
new Thread(processor).start(); | ||
executorService.execute(processor); | ||
} | ||
|
||
public void stop() { | ||
stopped = true; | ||
try { | ||
executorService.shutdown(); | ||
terminateWhenTimeLimitExceed(); | ||
serverSocket.close(); | ||
} catch (IOException e) { | ||
log.error(e.getMessage(), e); | ||
} catch (InterruptedException e) { | ||
executorService.shutdownNow(); | ||
log.error(e.getMessage(), e); | ||
Thread.currentThread().interrupt(); | ||
} | ||
} | ||
Comment on lines
83
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 스레드 풀을 닫는 부분 고민해주신거 너무 좋네요~~ 저도 배웠습니다! |
||
|
||
private void terminateWhenTimeLimitExceed() throws InterruptedException { | ||
if (!executorService.awaitTermination(TERMINATION_LIMIT_SECONDS, TimeUnit.SECONDS)) { | ||
executorService.shutdownNow(); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,14 @@ | |
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
|
||
public class SessionManager { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(SessionManager.class); | ||
|
||
private static final Map<String, Session> SESSIONS = new HashMap<>(); | ||
private static final Map<String, Session> SESSIONS = new ConcurrentHashMap<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기는 ConcurrentHashMap이 필요하겠네요~~ 👍 |
||
|
||
public static void add(Session session) { | ||
log.info("session add 완료: {}", session.getId()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>Error 400 - Bad Request</title> | ||
<style> | ||
body { | ||
font-family: Arial, sans-serif; | ||
text-align: center; | ||
background-color: #f7f7f7; | ||
margin: 0; | ||
padding: 0; | ||
} | ||
.container { | ||
max-width: 600px; | ||
margin: 0 auto; | ||
padding: 20px; | ||
background-color: #fff; | ||
border: 1px solid #ddd; | ||
border-radius: 5px; | ||
box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); | ||
} | ||
h1 { | ||
color: #ff6347; | ||
} | ||
p { | ||
font-size: 18px; | ||
} | ||
|
||
</style> | ||
</head> | ||
<body> | ||
<div class="container"> | ||
<h1>Error 400 - Bad Request</h1> | ||
<p>The request you made was malformed or invalid.</p> | ||
</div> | ||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3단계 요구사항이 Controller의 service()메서드의 시그니처가
service(HttpRequest request, HttpResponse response)
이런 식으로 강제되게 되었네요 ㅠㅠ 이런 방식으로 가게 되면 response를 외부에서 만들어 넣어주는 방식으로 구성해야 하고 Servlet과 HttpServlet의 정확한 표준을 따르게 되면 제안드렸던 builder패턴 사용으로 response 객체를 나중에 만드는 방식은 조오금 어려워질 것 같기도 합니다..😂고치기에는 변경점이 많아서 가볍게 어떻게 바뀔지 고민만 해보시면 좋을 것 같아서요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일단 빌더 패턴을 한번 적용해보는 것 부터가 많은 공부가 되었습니다😀
말씀하신 것 처럼 service(HttpRequest request, HttpResponse response) 로 구성하는 게 올바른 리팩터링 방향이라고 생각합니다!
기한 내 적용은 못했지만.. 나중에 여유를 좀 가지고 천천히 구조 변경을 해보겠습니다!