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단계] 로이 미션 제출합니다. #494

Merged
merged 9 commits into from
Sep 13, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ protected HttpResponse doPost(HttpRequest request) throws Exception {
private HttpResponse defaultInternalServerErrorPage() throws URISyntaxException, IOException {
final Path path = PathFinder.findPath("/500.html");
final String responseBody = new String(Files.readAllBytes(path));
return new HttpResponse(HttpStatus.INTERNAL_SERVER_ERROR, responseBody, ContentType.HTML);

return new HttpResponse.Builder(HttpStatus.INTERNAL_SERVER_ERROR, responseBody, ContentType.HTML).build();

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 객체를 나중에 만드는 방식은 조오금 어려워질 것 같기도 합니다..😂
고치기에는 변경점이 많아서 가볍게 어떻게 바뀔지 고민만 해보시면 좋을 것 같아서요~

Copy link
Author

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) 로 구성하는 게 올바른 리팩터링 방향이라고 생각합니다!

기한 내 적용은 못했지만.. 나중에 여유를 좀 가지고 천천히 구조 변경을 해보겠습니다!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ protected HttpResponse doGet(HttpRequest request) throws Exception {
var responseBody = new String(Files.readAllBytes(path));
ContentType contentType = ContentType.findContentType(extension);

return new HttpResponse(HttpStatus.OK, responseBody, contentType);
return new HttpResponse.Builder(HttpStatus.OK, responseBody, contentType).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ public class HelloController extends AbstractController {
@Override
protected HttpResponse doGet(HttpRequest request) {
String responseBody = "Hello world!";
return new HttpResponse(HttpStatus.OK, responseBody, ContentType.HTML);
return new HttpResponse.Builder(HttpStatus.OK, responseBody, ContentType.HTML).build();
}
}
24 changes: 18 additions & 6 deletions tomcat/src/main/java/nextstep/jwp/controller/LoginController.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ protected HttpResponse doGet(HttpRequest request) throws Exception {
if (loginUser.isPresent()) {
return redirectByAlreadyLogin(responseBody);
}
return new HttpResponse(HttpStatus.OK, responseBody, ContentType.HTML);
return new HttpResponse.Builder(HttpStatus.OK, responseBody, ContentType.HTML)
.build();

}

private HttpResponse redirectByAlreadyLogin(String responseBody) {
return new HttpResponse(HttpStatus.FOUND, responseBody, ContentType.HTML, "/index.html");
return new HttpResponse.Builder(HttpStatus.FOUND, responseBody, ContentType.HTML)
.redirect("/index.html")
.build();

}

@Override
Expand All @@ -58,18 +63,25 @@ private HttpResponse makeLoginResponse(Map<String, String> loginData, final Stri
}

private HttpResponse successLoginResponse(String responseBody, User user) {
HttpResponse httpResponse =
new HttpResponse(HttpStatus.FOUND, responseBody, ContentType.HTML, "/index.html");
Session session = new Session();
session.setAttribute("user", user);
SessionManager.add(session);
httpResponse.addJSessionId(session);
HttpResponse httpResponse =
new HttpResponse.Builder(HttpStatus.FOUND, responseBody, ContentType.HTML)
.redirect("index.html")
.addJSessionId(session)
.build();


return httpResponse;
}

private HttpResponse failLoginResponse(String responseBody) {
log.info("로그인 계정 정보가 이상합니다. responseBody={}", responseBody);
return new HttpResponse(HttpStatus.FOUND, responseBody, ContentType.HTML, "/401.html");
return new HttpResponse.Builder(HttpStatus.FOUND, responseBody, ContentType.HTML)
.redirect("/401.html")
.build();

}

private boolean isSuccessLogin(String account, String password) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -56,7 +62,10 @@ private boolean checkInputForm(final HashMap<String, String> registerData) {
private HttpResponse generateBadRequestResponse() throws URISyntaxException, IOException {
Path path = PathFinder.findPath("/400.html");

Choose a reason for hiding this comment

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

혹시 400.html은 별도로 존재했다가 삭제된 정적 파일일까요?

Copy link
Author

Choose a reason for hiding this comment

The 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) {
Expand Down
26 changes: 23 additions & 3 deletions tomcat/src/main/java/org/apache/catalina/connector/Connector.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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

Choose a reason for hiding this comment

The 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();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@
public class Http11Processor implements Runnable, Processor {

private static final Logger log = LoggerFactory.getLogger(Http11Processor.class);
private static final String ACCOUNT_KEY = "account";
private static final String PASSWORD_KEY = "password";
private static final String EMAIL_KEY = "email";
private static final String INDEX_PAGE = "/index.html";
private static final String LOGIN_PAGE = "/login.html";
private static final String UNAUTHORIZED_PAGE = "/401.html";
private static final String REGISTER_PAGE = "/register.html";

private final Socket connection;

Expand Down Expand Up @@ -64,7 +57,10 @@ private HttpResponse makeResponse(HttpRequest httpRequest) throws Exception {
} catch (IllegalArgumentException e) {
Path path = PathFinder.findPath("/400.html");
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();
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,41 @@ public class HttpResponse {
private final Map<String, String> headers;
private final String responseBody;

public HttpResponse(HttpStatus httpStatus, String responseBody,
ContentType contentType, String redirectUrl) {
this.startLine = new ResponseLine(httpStatus);
this.headers = new LinkedHashMap<>();
initHeader(contentType, responseBody, redirectUrl);
this.responseBody = responseBody;
private HttpResponse(Builder builder) {
this.startLine = builder.startLine;
this.headers = builder.headers;
this.responseBody = builder.responseBody;
}

public static class Builder {
private ResponseLine startLine;
private Map<String, String> headers = new LinkedHashMap<>();
private String responseBody;

public HttpResponse(final HttpStatus httpStatus, final String responseBody, final ContentType contentType) {
this.startLine = new ResponseLine(httpStatus);
this.headers = new LinkedHashMap<>();
initHeader(contentType, responseBody);
this.responseBody = responseBody;
}
public Builder(HttpStatus httpStatus, String responseBody, ContentType contentType) {
this.startLine = new ResponseLine(httpStatus);
this.responseBody = responseBody;
initHeader(contentType, responseBody);
}

private void initHeader(final ContentType contentType, final String responseBody, final String redirectUrl) {
initHeader(contentType, responseBody);
headers.put("Location", redirectUrl);
}
public Builder redirect(String redirectUrl) {
headers.put("Location", redirectUrl);
return this;
}

private void initHeader(ContentType contentType, String responseBody) {
headers.put("Content-Type", contentType.getContentType() + ";charset=utf-8");
headers.put("Content-Length", String.valueOf(responseBody.getBytes(StandardCharsets.UTF_8).length));
}
public Builder addJSessionId(Session session) {
headers.put("Set-Cookie", "JSESSIONID=" + session.getId());
return this;
}

public HttpResponse build() {
return new HttpResponse(this);
}

public void addJSessionId(final Session session) {
headers.put("Set-Cookie", "JSESSIONID=" + session.getId());
private void initHeader(ContentType contentType, String responseBody) {
headers.put("Content-Type", contentType.getContentType() + ";charset=utf-8");
headers.put("Content-Length", String.valueOf(responseBody.getBytes(StandardCharsets.UTF_8).length));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package org.apache.coyote.http11.session;

import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;

public class Session {

private final String id;
private final Map<String, Object> values = new HashMap<>();
private final Map<String, Object> values = new ConcurrentHashMap<>();

public Session() {
this.id = UUID.randomUUID().toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();

Choose a reason for hiding this comment

The 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());
Expand Down
16 changes: 12 additions & 4 deletions tomcat/src/main/java/org/apache/coyote/http11/util/PathFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,17 @@ public class PathFinder {

private static final String RESOURCE_ROOT_DIRECTORY_PATH = "static";

public static Path findPath(String resourceName) throws URISyntaxException {
final URL resource =
PathFinder.class.getClassLoader().getResource(RESOURCE_ROOT_DIRECTORY_PATH + resourceName);
return Paths.get(resource.toURI());
public static Path findPath(String resourceName) {
try {
URL resource =
PathFinder.class.getClassLoader().getResource(RESOURCE_ROOT_DIRECTORY_PATH + resourceName);

if (resource == null) {
throw new IllegalArgumentException("Resource Not Found: " + resourceName);
}
return Paths.get(resource.toURI());
} catch (URISyntaxException e) {
throw new RuntimeException("URI Syntax Exception : " + resourceName);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ public static HashMap<String, String> parseByBody(final String requestBody) {

private static void initData(final Map<String, String> data, final String[] params) {
for (final String param : params) {
final String paramInfo = param.split("=")[PARAM_INFO_INDEX];
final String paramValue = param.split("=")[PARAM_VALUE_INDEX];
String[] split = param.split("=");
final String paramInfo = split[PARAM_INFO_INDEX];
final String paramValue = split[PARAM_VALUE_INDEX];
data.put(paramInfo, paramValue);
}
}
Expand Down
39 changes: 39 additions & 0 deletions tomcat/src/main/resources/static/400.html
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>