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단계] 썬샷(오진택) 미션 제출합니다. #479

Merged
merged 13 commits into from
Sep 15, 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
27 changes: 27 additions & 0 deletions tomcat/src/main/java/nextstep/jwp/RequestMapping.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package nextstep.jwp;

import java.util.ArrayList;
import java.util.List;
import nextstep.jwp.commandcontroller.Controller;
import nextstep.jwp.request.HttpRequest;

public class RequestMapping {

private final List<Controller> controllers = new ArrayList<>();
private Controller defaultController;

public RequestMapping(final List<Controller> controllers) {
this.controllers.addAll(controllers);
}

public void setDefaultController(final Controller controller) {
this.defaultController = controller;
}

public Controller getController(final HttpRequest request) {
return controllers.stream()
.filter(controller -> controller.canHandle(request))
.findAny()
.orElse(defaultController);
}
}
159 changes: 0 additions & 159 deletions tomcat/src/main/java/nextstep/jwp/RequestProcessor.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package nextstep.jwp.commandcontroller;
Ohjintaek marked this conversation as resolved.
Show resolved Hide resolved

import java.io.IOException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Paths;
import nextstep.jwp.request.HttpRequest;
import nextstep.jwp.response.HttpResponse;

public class AbstractController implements Controller {
Copy link

Choose a reason for hiding this comment

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

미션에서 준 AbstractController는 Java EE의 HttpServlet을 투영한 것 같아요.
따라서 이번 미션에서는 doPost, doGet 등도 추가한 구조로 구현해보면 어떨까 싶습니다!

Copy link
Author

Choose a reason for hiding this comment

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

doPost()와 doGet()을 사용하는 방식으로 바꾸어주었습니다!!


protected static final String DEFAULT_FILE_LOCATION = "static/";

@Override
public boolean canHandle(HttpRequest request) {
return false;
}

@Override
public void service(HttpRequest request, HttpResponse response) throws Exception {
// http method 분기문
}

protected String readResponseBody(final String requestUri) throws IOException, URISyntaxException {
final URL url = getClass().getClassLoader().getResource(DEFAULT_FILE_LOCATION + requestUri);
if (url == null) {
URL notFoundUrl = getClass().getClassLoader().getResource(DEFAULT_FILE_LOCATION + "404.html");
final var path = Paths.get(notFoundUrl.toURI());
return Files.readString(path);
}
final var path = Paths.get(url.toURI());
return Files.readString(path);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package nextstep.jwp.commandcontroller;

import nextstep.jwp.request.HttpRequest;
import nextstep.jwp.response.HttpResponse;

public interface Controller {

boolean canHandle(HttpRequest httpRequest);
void service(HttpRequest httpRequest, HttpResponse httpResponse) throws Exception;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package nextstep.jwp.commandcontroller;

import nextstep.jwp.common.ContentType;
import nextstep.jwp.common.HttpStatus;
import nextstep.jwp.request.HttpRequest;
import nextstep.jwp.response.HttpResponse;
import nextstep.jwp.response.StatusLine;

public class ErrorPageController extends AbstractController {

@Override
public boolean canHandle(HttpRequest request) {
return super.canHandle(request);
}

@Override
public void service(HttpRequest request, HttpResponse response) throws Exception {
response.setStatusLine(StatusLine.of(request.getHttpVersion(), HttpStatus.NOT_FOUND));
response.addHeader("Content-Type", ContentType.HTML.getType());
Copy link

Choose a reason for hiding this comment

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

헤더 값을 각 컨트롤러에서 하드코딩해주고 있는데 휴먼 에러의 위험이 있지 않을까요??!

Copy link
Author

Choose a reason for hiding this comment

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

Http Header도 메서드나 상태 코드와 같이 enum으로 관리해주도록 변경해주었습니다

Copy link

Choose a reason for hiding this comment

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

비즈니스적인 커스텀 헤더를 추가하려고 하면 ENUM 타입의 KEY를 사용할 때 문제가 생길 수 있을 것 같아요.
Enum 제공 측은 톰캣 같은 범용적인 모듈이기 때문에 개발자가 그 Enum에 추가할 수도 없구요!

이러한 경우는 어떻게 하는 것이 좋을까요??!

final String content = readResponseBody("500.html");
response.setResponseBody(content);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package nextstep.jwp.commandcontroller;

import java.util.Arrays;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;
import nextstep.jwp.common.HttpMethod;
import nextstep.jwp.common.HttpStatus;
import nextstep.jwp.db.InMemoryUserRepository;
import nextstep.jwp.model.Session;
import nextstep.jwp.model.User;
import nextstep.jwp.request.HttpRequest;
import nextstep.jwp.response.HttpResponse;
import nextstep.jwp.response.StatusLine;
import org.apache.catalina.SessionManager;

public class LoginController extends AbstractController {

@Override
public boolean canHandle(HttpRequest request) {
final HttpMethod method = request.getHttpMethod();
final String uri = request.getRequestUri();
return method.equals(HttpMethod.POST) && uri.equals("login");
}

@Override
public void service(HttpRequest request, HttpResponse response) {
final Map<String, String> logInfo = Arrays.stream(request.getRequestBody().split("&"))
.map(input -> input.split("="))
.collect(Collectors.toMap(info -> info[0], info -> info[1]));

response.setStatusLine(StatusLine.of(request.getHttpVersion(), HttpStatus.FOUND));
final Optional<User> savedUser = InMemoryUserRepository.findByAccount(logInfo.get("account"));
if (savedUser.isPresent()) {
final User user = savedUser.get();
if (user.checkPassword(logInfo.get("password"))) {
response.addHeader("Location", "index.html");
final Session session = new Session(UUID.randomUUID().toString());
session.setAttribute("user", user);
SessionManager.add(session);
response.addCookie("JSESSIONID", session.getId());
return;
}
}
response.addHeader("Location", "401.html");
}
}
Copy link

Choose a reason for hiding this comment

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

이 부분도 휴먼 에러를 줄이고, 분기를 줄이는 식으로 리팩토링할 수 있을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

메서드 분리하고 상수화 좀 해주긴 했는데 만족스럽게 된 것 같진 않네요

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package nextstep.jwp.commandcontroller;

import nextstep.jwp.common.ContentType;
import nextstep.jwp.common.HttpMethod;
import nextstep.jwp.common.HttpStatus;
import nextstep.jwp.model.Session;
import nextstep.jwp.request.HttpRequest;
import nextstep.jwp.response.HttpResponse;
import nextstep.jwp.response.StatusLine;
import org.apache.catalina.SessionManager;

public class LoginPageController extends AbstractController {

@Override
public boolean canHandle(HttpRequest request) {
final HttpMethod method = request.getHttpMethod();
final String uri = request.getRequestUri();
return method.equals(HttpMethod.GET) && uri.equals("login");
}

@Override
public void service(HttpRequest request, HttpResponse response) throws Exception {
final String sessionId = request.getCookies().ofSessionId("JSESSIONID");
final Session session = SessionManager.findSession(sessionId);
if (session != null) {
response.setStatusLine(StatusLine.of(request.getHttpVersion(), HttpStatus.FOUND));
response.addHeader("Location", "index.html");
return;
}
response.setStatusLine(StatusLine.of(request.getHttpVersion(), HttpStatus.OK));
response.addHeader("Content-Type", ContentType.HTML.getType());
final String content = readResponseBody(request.getRequestUri() + ".html");
response.setResponseBody(content);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package nextstep.jwp.commandcontroller;

import nextstep.jwp.common.ContentType;
import nextstep.jwp.common.HttpMethod;
import nextstep.jwp.common.HttpStatus;
import nextstep.jwp.request.HttpRequest;
import nextstep.jwp.response.HttpResponse;
import nextstep.jwp.response.StatusLine;

public class MainPageController extends AbstractController {

@Override
public boolean canHandle(HttpRequest request) {
final HttpMethod method = request.getHttpMethod();
final String uri = request.getRequestUri();
return method.equals(HttpMethod.GET) && uri.isEmpty();
}

@Override
public void service(HttpRequest request, HttpResponse response) {
response.setStatusLine(StatusLine.of(request.getHttpVersion(), HttpStatus.OK));
response.addHeader("Content-Type", ContentType.HTML.getType());
response.setResponseBody("Hello world!");
}
}
Loading