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단계] 디노(신종화) 미션 제출합니다. #441

Merged
merged 16 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions tomcat/src/main/java/nextstep/jwp/DefaultController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package nextstep.jwp;

Choose a reason for hiding this comment

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

jwp -> catalina -> coyote 의 의존성 방향에서 어느 클래스를 어느 패키지에 위치시켜야 하는가

이 부분은 현재 어디가 마음에 안 드시는 걸까요??
저는 의존성 방향은 생각조차 못 했는데,, 대단하십니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

패키지 간 양방향 의존이 일어나지 않는 방향으로 구현하고 싶었습니다!

51ca60f


import java.io.IOException;
import org.apache.catalina.AbstractController;
import org.apache.coyote.http11.FileExtractor;
import org.apache.coyote.http11.HttpStatusCode;
import org.apache.coyote.http11.request.HttpRequest;
import org.apache.coyote.http11.response.HttpResponse;
import org.apache.coyote.http11.response.ResponseBody;

public class DefaultController extends AbstractController {

@Override
protected HttpResponse doGet(final HttpRequest request) throws IOException {

Choose a reason for hiding this comment

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

왜 빈 response를 만든 뒤 Controller 거치며 요소 추가하는 식으로 만들어야 하는가

저는 컨트롤러 로직에 따라서 Response가 달라지기 때문에
컨트롤러 로직 안에서 Response의 요소가 만들어져서 반환된다고 생각했어요!

일단 저의 생각은 이런데, 이런 의미로 질문하신게 아닐까요??

final String resource = request.getRequestPath().getResource();
final ResponseBody responseBody = FileExtractor.extractFile(resource);
return HttpResponse.of(HttpStatusCode.OK, responseBody);
}
}
20 changes: 20 additions & 0 deletions tomcat/src/main/java/nextstep/jwp/HomeController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package nextstep.jwp;

import java.io.IOException;
import org.apache.catalina.AbstractController;
import org.apache.coyote.http11.ExtensionType;
import org.apache.coyote.http11.HttpStatusCode;
import org.apache.coyote.http11.request.HttpRequest;
import org.apache.coyote.http11.response.HttpResponse;
import org.apache.coyote.http11.response.ResponseBody;

public class HomeController extends AbstractController {

private static final String DEFAULT_CONTENT = "Hello world!";

@Override
protected HttpResponse doGet(final HttpRequest request) throws IOException {
final ResponseBody responseBody = ResponseBody.of(ExtensionType.HTML.getExtension(), DEFAULT_CONTENT);
return HttpResponse.of(HttpStatusCode.OK, responseBody);
}
}
80 changes: 80 additions & 0 deletions tomcat/src/main/java/nextstep/jwp/LoginController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package nextstep.jwp;

import static org.reflections.Reflections.log;

import java.io.IOException;
import java.util.UUID;
import nextstep.jwp.db.InMemoryUserRepository;
import nextstep.jwp.exception.LoginException;
import nextstep.jwp.model.User;
import org.apache.catalina.AbstractController;
import org.apache.catalina.SessionManager;
import org.apache.coyote.http11.FileExtractor;
import org.apache.coyote.http11.HttpCookie;
import org.apache.coyote.http11.HttpStatusCode;
import org.apache.coyote.http11.Session;
import org.apache.coyote.http11.request.HttpRequest;
import org.apache.coyote.http11.response.HttpResponse;
import org.apache.coyote.http11.response.ResponseBody;

public class LoginController extends AbstractController {

private static final SessionManager sessionManager = new SessionManager();

private static final String JSESSIONID = "JSESSIONID";
private static final String INDEX = "/index";
private static final String ACCOUNT = "account";
private static final String PASSWORD = "password";
private static final String COOKIE = "Cookie";
private static final String USER = "user";
private static final String UNAUTHORIZED = "/401";

@Override
protected HttpResponse doPost(final HttpRequest request) throws IOException {
try {
final String userName = request.getRequestBody().get(ACCOUNT);
final String password = request.getRequestBody().get(PASSWORD);

final User user = InMemoryUserRepository.findByAccount(userName)
.orElseThrow(LoginException::new);

if (user.checkPassword(password)) {
log.info("user : " + user);
final HttpCookie cookie = HttpCookie.from(request.getRequestHeaders().geHeaderValue(COOKIE));
checkSession(user, cookie);
final ResponseBody responseBody = FileExtractor.extractFile(INDEX);
final HttpResponse httpResponse = HttpResponse.of(HttpStatusCode.FOUND, responseBody);
httpResponse.addCookie(cookie);
return httpResponse;
}
throw new LoginException();
} catch (LoginException exception) {
final ResponseBody responseBody = FileExtractor.extractFile(UNAUTHORIZED);
return HttpResponse.of(HttpStatusCode.UNAUTHORIZED, responseBody);
}
}

private void checkSession(final User user, final HttpCookie cookie) {
if (!cookie.contains(JSESSIONID)) {
final Session session = new Session(UUID.randomUUID().toString());
session.setAttribute(USER, user);
sessionManager.add(session);
cookie.setCookie(JSESSIONID, session.getId());
}
}

@Override
protected HttpResponse doGet(final HttpRequest request) throws IOException {
final HttpCookie cookie = HttpCookie.from(request.getRequestHeaders().geHeaderValue(COOKIE));
final String requestResource = request.getRequestPath().getResource();

if (cookie.contains(JSESSIONID)) {
final ResponseBody responseBody = FileExtractor.extractFile(INDEX);
final HttpResponse httpResponse = HttpResponse.of(HttpStatusCode.FOUND, responseBody);
httpResponse.addCookie(cookie);
return httpResponse;
}
final ResponseBody responseBody = FileExtractor.extractFile(requestResource);
return HttpResponse.of(HttpStatusCode.OK, responseBody);
}
}
52 changes: 52 additions & 0 deletions tomcat/src/main/java/nextstep/jwp/RegisterController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package nextstep.jwp;

import java.io.IOException;
import java.util.Map;
import nextstep.jwp.db.InMemoryUserRepository;
import nextstep.jwp.model.User;
import org.apache.catalina.AbstractController;
import org.apache.coyote.http11.FileExtractor;
import org.apache.coyote.http11.HttpStatusCode;
import org.apache.coyote.http11.request.HttpRequest;
import org.apache.coyote.http11.response.HttpResponse;
import org.apache.coyote.http11.response.ResponseBody;

public class RegisterController extends AbstractController {

private static final String BAD_REQUEST = "/400";
private static final String CONFLICT = "/409";
private static final String INDEX = "/index";
private static final String REGISTER = "/register";
private static final String ACCOUNT = "account";
private static final String PASSWORD = "password";
private static final String EMAIL = "email";

@Override
protected HttpResponse doPost(final HttpRequest request) throws IOException {
final Map<String, String> requestBody = request.getRequestBody();

final String account = requestBody.get(ACCOUNT);
final String password = requestBody.get(PASSWORD);
final String email = requestBody.get(EMAIL);

if (account.isBlank() || password.isBlank() || email.isBlank()) {
final ResponseBody responseBody = FileExtractor.extractFile(BAD_REQUEST);
return HttpResponse.of(HttpStatusCode.BAD_REQUEST, responseBody);
}

if (InMemoryUserRepository.checkExistingId(account)) {
final ResponseBody responseBody = FileExtractor.extractFile(CONFLICT);
return HttpResponse.of(HttpStatusCode.OK, responseBody);
}
final User user = new User(account, password, email);
InMemoryUserRepository.save(user);
final ResponseBody responseBody = FileExtractor.extractFile(INDEX);
return HttpResponse.of(HttpStatusCode.OK, responseBody);
}

@Override
protected HttpResponse doGet(final HttpRequest request) throws IOException {
final ResponseBody responseBody = FileExtractor.extractFile(REGISTER);
return HttpResponse.of(HttpStatusCode.OK, responseBody);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,23 @@
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import nextstep.jwp.model.User;

public class InMemoryUserRepository {

private static final AtomicLong id = new AtomicLong(1L);

private static final Map<String, User> database = new ConcurrentHashMap<>();

static {
final User user = new User(1L, "gugu", "password", "[email protected]");
final User user = new User(id.getAndIncrement(), "gugu", "password", "[email protected]");
database.put(user.getAccount(), user);
}

public static void save(User user) {
database.put(user.getAccount(), user);
final User userWithId = new User(id.getAndIncrement(), user.getAccount(), user.getPassword(), user.getEmail());
database.put(user.getAccount(), userWithId);
}

public static Optional<User> findByAccount(String account) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
public class NotAllowedMethodException extends RuntimeException {

public NotAllowedMethodException() {
super("해당하는 HttpMethod가 존재하지 않습니다.");
super("해당하는 Method가 존재하지 않습니다.");
}
}
8 changes: 8 additions & 0 deletions tomcat/src/main/java/nextstep/jwp/model/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ public String getAccount() {
return account;
}

public String getPassword() {
return password;
}

public String getEmail() {
return email;
}

@Override
public String toString() {
return "User{" +
Expand Down
29 changes: 29 additions & 0 deletions tomcat/src/main/java/org/apache/catalina/AbstractController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.apache.catalina;

import java.io.IOException;
import org.apache.coyote.http11.request.HttpMethod;
import org.apache.coyote.http11.request.HttpRequest;
import org.apache.coyote.http11.response.HttpResponse;

public class AbstractController implements Controller {

@Override
public HttpResponse service(final HttpRequest request) throws IOException {
if (request.getHttpMethod().equals(HttpMethod.GET)) {
return doGet(request);
}
if (request.getHttpMethod().equals(HttpMethod.POST)) {
return doPost(request);
}
throw new UnsupportedOperationException();

Choose a reason for hiding this comment

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

제가 제안드린 방향을 고려해서 해주신 것 같아요!! 👍🏻
다만 실제 동작은 Mapping된 컨트롤러에서 지원하지 않는 메소드면
405 method not allowed가 응답되기 때문에 405가 뜨도록 리팩토링해보는 것을 제안했던 것이었습니다!!

그래도 잘 고려해주셔서 감사합니다 ㅎㅎ
미션이니 이 부분은 언급만 하고 머지하도록 하겠습니다!! 👍🏻👍🏻👍🏻

}


protected HttpResponse doPost(final HttpRequest request) throws IOException {
return null;
}

protected HttpResponse doGet(final HttpRequest request) throws IOException {
return null;
}

Choose a reason for hiding this comment

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

이 부분 각 구체 컨트롤러에서 @OverRide해서 사용하고 있는데,
return null 보다는 추상 메소드로 만들어서
나머지 클래스에서 Override하는게 좋을 것 같아요!

그리고 POST를 사용하지 않는 컨트롤러에서 POST 요청이 온다면 실제는 어떻게 동작하는지 생각해보고
해당 방향으로 리팩토링 하는게 좋아 보입니다!! 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

아 너무 좋습니다.

7566751 반영 완!

}
10 changes: 10 additions & 0 deletions tomcat/src/main/java/org/apache/catalina/Controller.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.apache.catalina;

import java.io.IOException;
import org.apache.coyote.http11.request.HttpRequest;
import org.apache.coyote.http11.response.HttpResponse;

public interface Controller {

HttpResponse service(final HttpRequest request) throws IOException;
}

Choose a reason for hiding this comment

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

이 부분 오늘 구구가 슬랙에 남긴 거긴 한데,
컨트롤러 인터페이스에서 파라미터를 줄이거나 더 추가하지 마시고 그대로 사용하라고 하네요...
저도 사실 HttpRequest만 받고 있어서 이유는 강의에서 들어봐야 할 것 같아요!
일단 언급만 해놓겠습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

넵넵 저도 확인하고 구조 변경했습니다.!

46d938a

26 changes: 26 additions & 0 deletions tomcat/src/main/java/org/apache/catalina/RequestMapping.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.apache.catalina;

import nextstep.jwp.DefaultController;
import nextstep.jwp.HomeController;
import nextstep.jwp.LoginController;
import nextstep.jwp.RegisterController;
import org.apache.coyote.http11.request.HttpRequest;

public class RequestMapping {

public Controller getController(final HttpRequest request) {
final String resource = request.getRequestPath().getResource();

if (resource.equals("/")) {
return new HomeController();
}
if (resource.equals("/login")) {
return new LoginController();
}
if (resource.equals("/register")) {
return new RegisterController();
}

return new DefaultController();
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
package org.apache.coyote.http11;
package org.apache.catalina;

Choose a reason for hiding this comment

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

sessionManager 어디에서 선언해야 하는가?

이 부분에 대해서 Apache의 coyote, catalina 패키지 중에 어떤 패키지에 둘지 고민하신 것 같아요!

찾아보니 톰캣의 catalina에서 Session 관리를 한다고 하더라구요!
그래서 catalina 패키지가 맞는 것 같습니다!!

실제 코드에서 구구가 만든 Manager를 구현하고 있는데 catalina 패키지에 만들어 놓으셨네요!

톰캣의 세션 관리 기능을 구현한 Manager 구성 요소 공식문서


import java.util.HashMap;
import java.util.Map;
import org.apache.catalina.Manager;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.coyote.http11.Session;

public class SessionManager implements Manager{
public class SessionManager implements Manager {

private static final Map<String, Session> SESSIONS = new HashMap<>();
private static final Map<String, Session> SESSIONS = new ConcurrentHashMap<>();

@Override
public void add(final Session session) {
SESSIONS.put(session.getId(), session);
}

@Override
public Session findSession(final String id) {
return SESSIONS.get(id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.io.UncheckedIOException;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import org.apache.coyote.http11.Http11Processor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -14,17 +16,20 @@ public class Connector implements Runnable {

private static final int DEFAULT_PORT = 8080;
private static final int DEFAULT_ACCEPT_COUNT = 100;
private static final int DEFAULT_THREAD_COUNT = 200;

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_THREAD_COUNT);
}

public Connector(final int port, final int acceptCount) {
public Connector(final int port, final int acceptCount, final int threadCount) {
this.serverSocket = createServerSocket(port, acceptCount);
this.stopped = false;
this.executorService = Executors.newFixedThreadPool(threadCount);
}

private ServerSocket createServerSocket(final int port, final int acceptCount) {
Expand Down Expand Up @@ -66,7 +71,7 @@ private void process(final Socket connection) {
return;
}
var processor = new Http11Processor(connection);
new Thread(processor).start();
executorService.execute(processor);
}

public void stop() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.apache.coyote.http11;

import java.util.Arrays;
import org.apache.coyote.http11.request.RequestLine;
import nextstep.jwp.exception.NotAllowedMethodException;

public enum HttpExtensionType {
public enum ExtensionType {

HTML(".html", "text/html;charset=utf-8"),
CSS(".css", "text/css"),
Expand All @@ -14,23 +14,16 @@ public enum HttpExtensionType {
private final String extension;
private final String contentType;

HttpExtensionType(final String extension, final String contentType) {
ExtensionType(final String extension, final String contentType) {
this.extension = extension;
this.contentType = contentType;
}

public static HttpExtensionType from(final String extension) {
public static ExtensionType from(final String extension) {
return Arrays.stream(values())
.filter(it -> extension.contains(it.extension))
.findAny()
.orElse(HTML);
}

public static HttpExtensionType from(final RequestLine requestLine) {
return Arrays.stream(values())
.filter(it -> requestLine.hasFileExtension(it.getExtension()))
.findAny()
.orElse(HTML);
.orElseThrow(NotAllowedMethodException::new);

Choose a reason for hiding this comment

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

여기서 Extension을 찾지 못하면 NotAllowedMethodException을 발생시키고 있는데,
왜 해당 예외를 발생시키는 걸까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

NotAllowedExtenstionException 만들어 반환하도록 변경했습니다!

c5d20eb

}

public String getExtension() {
Expand Down
Loading