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단계 - HTTP 웹 서버 구현 미션 제출합니다. #207

Open
wants to merge 14 commits into
base: jinjumoon
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@
- [x] 추가 요구사항이나 변경이 발생하는 경우를 고려한다.
- [x] HTTP에서 POST 방식으로 데이터를 전달할 때 body를 통한 데이터 전달뿐만 아니라 Query String을 활용한 데이터 전달도 지원하도록 한다.

## 🚀 3단계 - 로그인 및 세션 구현

### 요구사항

- [x] 앞 단계에서 회원가입할 때 생성한 User 객체를 DataBase.addUser() 메서드를 활용해 RAM 메모리에 저장한다.
- [x] 회원가입한 사용자로 로그인할 수 있어야 한다.
- [x] user/login.html에서 로그인이 성공하면 index.html로 이동하고, 로그인이 실패하면 /user/login_failed.html로 이동한다.
- [x] 로그인이 성공하면 cookie를 활용해 로그인 상태를 유지할 수 있어야 한다. 로그인이 성공할 경우 요청 header의 Cookie header 값이 logined=true, 로그인이 실패하면 Cookie header 값이 logined=false로 전달되어야 한다.
- [x] Set-Cookie 설정시 모든 요청에 대해 Cookie 처리가 가능하도록 Path 설정 값을 /(Path=/)로 설정한다.
- [x] 응답 header에 Set-Cookie값을 설정한 후 요청 header에 Cookie이 전달되는지 확인한다.
- [x] 접근하고 있는 사용자가 “로그인” 상태일 경우(Cookie 값이 logined=true) http://localhost:8080/user/list 로 접근했을 때 사용자 목록을 출력한다.
- [x] 만약 로그인하지 않은 상태라면 로그인 페이지(login.html)로 이동한다.
- [x] 동적으로 html을 생성하기 위해 handlebars.java template engine을 활용한다.
- [x] 서블릿에서 지원하는 HttpSession API의 일부를 지원한다.
- [x] HttpSession API 중 getId(), setAttribute(String name, Object value), getAttribute(String name), removeAttribute(String name), invalidate() 5개의 메소드를 구현한다.

## 진행 방법
* 웹 애플리케이션 서버 요구사항을 파악한다.
* 요구사항에 대한 구현을 완료한 후 자신의 github 아이디에 해당하는 브랜치에 Pull Request(이하 PR)를 통해 코드 리뷰 요청을 한다.
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/db/SessionDataBase.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package db;

import java.util.HashMap;
import java.util.Map;

import webserver.http.request.HttpSession;

public class SessionDataBase {
private static Map<String, HttpSession> sessions = new HashMap<>();

Choose a reason for hiding this comment

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

Suggested change
private static Map<String, HttpSession> sessions = new HashMap<>();
private static Map<String, HttpSession> sessions = new ConcurrentHashMap<>();

여러 스레드에서 접근할 수 있는 경우 동시성을 보장해주는 것이 좋습니다!


public static void addHttpSession(HttpSession httpSession) {
sessions.put(httpSession.getId(), httpSession);
}

public static HttpSession findHttpSessionById(String id) {
return sessions.get(id);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package db;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;

import com.google.common.collect.Maps;

import model.User;

public class DataBase {
private static Map<String, User> users = Maps.newHashMap();
public class UserDataBase {
private static Map<String, User> users = new HashMap<>();

public static void addUser(User user) {
users.put(user.getUserId(), user);
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/webserver/ClientException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package webserver;

import webserver.http.response.StatusCode;

public abstract class ClientException extends RuntimeException {
public ClientException(String message) {
super(message);
}

public abstract StatusCode getStatus();
}
7 changes: 0 additions & 7 deletions src/main/java/webserver/RequestException.java

This file was deleted.

9 changes: 6 additions & 3 deletions src/main/java/webserver/RequestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,15 @@ public HttpResponse controlRequestAndResponse(HttpRequest httpRequest) {
try {
Controller controller = requestMapping.getController(httpRequest);
return controller.service(httpRequest);
} catch (ApplicationBusinessException | RequestException e) {
} catch (ApplicationBusinessException e) {
logger.info(e.getMessage());
return HttpResponse.badRequest(e.getMessage()).build();
return HttpResponse.businessException(e).build();
} catch (ClientException e) {
logger.info(e.getMessage());
return HttpResponse.clientException(e).build();
} catch (ServerException e) {
logger.error(e.getMessage());
return HttpResponse.internalServer(e.getMessage()).build();
return HttpResponse.internalServerError(e).build();

Choose a reason for hiding this comment

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

꼼꼼한 예외처리 👍

}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/webserver/RequestMapping.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ public Controller getController(HttpRequest httpRequest) {
return staticController;
}

throw new UnsupportedRequestUrlException(httpRequest.getDefaultPath());
throw new UnsupportedClientUrlException(httpRequest.getDefaultPath());
}
}
7 changes: 7 additions & 0 deletions src/main/java/webserver/ResourceCannotLoadException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package webserver;

public class ResourceCannotLoadException extends ServerException {
public ResourceCannotLoadException(String message) {
super(message);
}
}
6 changes: 6 additions & 0 deletions src/main/java/webserver/ServerException.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package webserver;

import webserver.http.response.StatusCode;

public class ServerException extends RuntimeException {
public ServerException(String message) {
super(message);
}

public StatusCode getStatus() {
return StatusCode.INTERNAL_SERVER_ERROR;
}
}
4 changes: 3 additions & 1 deletion src/main/java/webserver/ServletContainer.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ public ServletContainer() {
servletNameMapper = new HashMap<>();
servletContainer = new HashMap<>();
servletNameMapper.put("/user/create", "webserver.controller.UserCreateController");
servletNameMapper.put("/user/login", "webserver.controller.LoginController");
servletNameMapper.put("/user/list", "webserver.controller.UserListController");
logger.info("ServletContainer has loaded.");
}

public boolean hasMappingServlet(HttpRequest httpRequest) {
return servletNameMapper.get(httpRequest.getDefaultPath()) != null;
return servletNameMapper.containsKey(httpRequest.getDefaultPath());
}

public Controller getController(HttpRequest httpRequest) {
Expand Down
24 changes: 24 additions & 0 deletions src/main/java/webserver/TemplateFactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package webserver;

import java.io.IOException;

import com.github.jknack.handlebars.Handlebars;
import com.github.jknack.handlebars.Template;
import com.github.jknack.handlebars.io.ClassPathTemplateLoader;
import com.github.jknack.handlebars.io.TemplateLoader;

public class TemplateFactory {
public static String of(String location, Object context) {
TemplateLoader loader = new ClassPathTemplateLoader();
loader.setPrefix("/templates");
loader.setSuffix(".html");
Handlebars handlebars = new Handlebars(loader);

Choose a reason for hiding this comment

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

Handlebars를 매번 생성하는 것은 어떠한 단점이 있을지 고민해보고 재사용할 수 있다면 어떠한 장점이 있을지 고민해보세요!


try {
Template template = handlebars.compile(location);
return template.apply(context);
} catch (IOException e) {
throw new ResourceCannotLoadException("자원을 로드할 수 없습니다.");
}
}
}
14 changes: 14 additions & 0 deletions src/main/java/webserver/UnsupportedClientUrlException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package webserver;

import webserver.http.response.StatusCode;

public class UnsupportedClientUrlException extends ClientException {
public UnsupportedClientUrlException(String requestUrl) {
super(String.format("해당 URL에 대한 요청은 지원하지 않습니다. {'URL' : %s}", requestUrl));
}

@Override
public StatusCode getStatus() {
return StatusCode.NOT_FOUND;
}
}
7 changes: 0 additions & 7 deletions src/main/java/webserver/UnsupportedRequestUrlException.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package webserver.controller;

import webserver.http.response.StatusCode;

public class ApplicationBusinessException extends RuntimeException {
public ApplicationBusinessException(String message) {
super(message);
}

public StatusCode getStatus() {
return StatusCode.BAD_REQUEST;
}
}
30 changes: 16 additions & 14 deletions src/main/java/webserver/controller/ContentType.java
Original file line number Diff line number Diff line change
@@ -1,30 +1,32 @@
package webserver.controller;

import java.util.Optional;

public enum ContentType {

Choose a reason for hiding this comment

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

ContentType의 옵션(charset, boundary, ...)들은 상황에 따라 추가될 수 있을 것 같아요.
정적인 enum으로 관리하긴 어려워보이는데 어떻게 생각하시나요?

HTML("text/html", Optional.of("charset=utf-8"), Optional.empty()),
CSS("text/css", Optional.empty(), Optional.empty()),
JAVASCRIPT("application/javascript", Optional.empty(), Optional.empty()),
TEXT("text/plain", Optional.empty(), Optional.empty());
HTML("text/html", "charset=utf-8", ""),
CSS("text/css", "", ""),

Choose a reason for hiding this comment

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

위 피드백을 반영하면 필요없을 수 있는 내용이지만 빈 값들을 추가하기 위해서 불필요한 값들이 많이 추가됐는데요.
이 또한 중복으로 본다면 생성자를 통해 해결할 수 있을 것 같습니다!

JAVASCRIPT("application/javascript", "", ""),
TEXT("text/plain", "", ""),
APPLICATION_JSON("application/json", "", ""),
ICO("image/x-icon", "", ""),
TTF("font/ttf", "", ""),
WOFF("font/woff", "", ""),
WOFF2("font/woff2", "", "");

private final String mediaType;
private final Optional<String> charset;
private final Optional<String> boundary;
private final String charset;
private final String boundary;

ContentType(String mediaType, Optional<String> charset, Optional<String> boundary) {
ContentType(String mediaType, String charset, String boundary) {
this.mediaType = mediaType;
this.charset = charset;
this.boundary = boundary;
}

public String value() {
StringBuilder stringBuilder = new StringBuilder();
String charsetToAppend = (charset == "") ? "" : (";" + charset);

Choose a reason for hiding this comment

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

String이 객체라는 사실을 생각하며 객체의 비교에서 왜 equals를 사용하는지 공부해보아요!

String boundaryToAppend = (boundary == "") ? "" : ( ";" + boundary);

stringBuilder.append(mediaType);
charset.ifPresent(charset -> stringBuilder.append(String.format(";%s", charset)));
boundary.ifPresent(boundary -> stringBuilder.append(String.format(";%s", boundary)));
String value = mediaType + charsetToAppend + boundaryToAppend;

return stringBuilder.toString();
return value;
}
}
12 changes: 8 additions & 4 deletions src/main/java/webserver/controller/ContentTypeMapper.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package webserver.controller;

import static webserver.controller.ContentType.*;

import java.util.Arrays;

public enum ContentTypeMapper {
HTML("html", ContentType.HTML),
CSS("css", ContentType.CSS),
JS("js", JAVASCRIPT);
JS("js", ContentType.JAVASCRIPT),
TEXT("txt", ContentType.TEXT),
APPLICATION_JSON("json", ContentType.APPLICATION_JSON),
ICO("ico", ContentType.ICO),
TTF("ttf", ContentType.TTF),
WOFF("woff", ContentType.WOFF),
WOFF2("woff2", ContentType.WOFF2);

private final String extension;
private final ContentType contentType;
Expand All @@ -21,7 +25,7 @@ public static ContentType map(String extension) {
return Arrays.stream(values())
.filter(value -> value.extension.equals(extension))
.findAny()
.orElseThrow(() -> new UndefinedExtension(extension))
.orElseThrow(() -> new UndefinedExtensionException(extension))
.contentType;
}
}
56 changes: 56 additions & 0 deletions src/main/java/webserver/controller/LoginController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package webserver.controller;

import java.util.HashMap;
import java.util.Map;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import db.SessionDataBase;
import db.UserDataBase;
import model.User;
import webserver.http.request.HttpRequest;
import webserver.http.request.HttpSession;
import webserver.http.response.HttpResponse;

public class LoginController extends AbstractController {
private static final Logger logger = LoggerFactory.getLogger(LoginController.class);

@Override
protected HttpResponse post(HttpRequest httpRequest) {
Map<String, String> data = new HashMap<>();

Choose a reason for hiding this comment

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

Body의 정보를 파싱하는 것은 Request를 사용하는 곳에서 자주 사용되는데요.
매번 Controller에서 해당 로직을 반복하기 보다 추상화를 통해 해당 기능을 제공해보면 어떨까요?

String body = httpRequest.getBody();
logger.debug("body : {}", body);
String[] inputs = body.split("&");
for (String input : inputs) {
logger.debug("bodyParam : {}", input);
String[] keyAndValue = input.split("=");
if (keyAndValue.length < 2) {
continue;
}
logger.debug("key : {}", keyAndValue[0]);
logger.debug("value : {}", keyAndValue[1]);
data.put(keyAndValue[0], keyAndValue[1]);
}

String userId = data.get("userId");
String password = data.get("password");

User user = UserDataBase.findUserById(userId);
HttpSession httpSession = httpRequest.getHttpSession();
httpSession.setAttribute("user", user);
SessionDataBase.addHttpSession(httpSession);

Choose a reason for hiding this comment

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

만약 해당 프로젝트의 정보를 잘 몰라 SessionDataBase의 존재를 모른다면 Session은 어떻게 관리될까요?
SessionDataBase의 존재를 몰라도 Session을 관리할 수 있으면 좋을 것 같습니다 :)


if (user != null && password.equals(user.getPassword())) {

Choose a reason for hiding this comment

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

getter를 사용하지 않고 메시지를 던져보면 어떨까요?

return HttpResponse.ok()
.bodyByPath("./templates/index.html")

Choose a reason for hiding this comment

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

./templates의 경로를 매번 입력해주는 것 보다 ViewTemplate에서 공통으로 처리해보면 어떨까요?

.setCookie("JSESSIONID", httpRequest.getSessionId(), "/")

Choose a reason for hiding this comment

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

JSESSIONID는 매우 특별한 의미를 가진만큼 상수로 분리해보면 어떨까요?

.build();
}

return HttpResponse.ok()
.bodyByPath("./templates/user/login_failed.html")
.setCookie("JSESSIONID", httpRequest.getSessionId(), "/")
.build();
}
}
12 changes: 1 addition & 11 deletions src/main/java/webserver/controller/StaticController.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package webserver.controller;

import utils.FileIoUtils;
import webserver.http.request.HttpRequest;
import webserver.http.response.HttpResponse;

public class StaticController extends AbstractController {

private static final String DELIMITER = "\\.";

@Override
protected HttpResponse get(HttpRequest httpRequest) {
String defaultPath = httpRequest.getDefaultPath();
Expand All @@ -20,14 +16,8 @@ protected HttpResponse get(HttpRequest httpRequest) {
path = String.format("./static%s", defaultPath);
}

byte[] body = FileIoUtils.loadFileFromClasspath(path);
String extension = path.split(DELIMITER)[1];
ContentType contentType = ContentTypeMapper.map(extension);

return HttpResponse.ok()
.contentType(contentType.value())
.contentLength(body.length)
.body(body)
.bodyByPath(path)
.build();
}

Expand Down
9 changes: 0 additions & 9 deletions src/main/java/webserver/controller/UndefinedExtension.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package webserver.controller;

import webserver.ClientException;
import webserver.http.response.StatusCode;

public class UndefinedExtensionException extends ClientException {
public UndefinedExtensionException(String extension) {
super(String.format("정의되지 않은 확장자입니다. {'extension' : %s}", extension));
}

@Override
public StatusCode getStatus() {
return StatusCode.BAD_REQUEST;
}
}
Loading