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

[엘리] WAS 1단계 미션 제출합니다. #96

Merged
merged 19 commits into from
Sep 11, 2020
Merged
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
32 changes: 24 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
# 웹 애플리케이션 서버
## 진행 방법
* 웹 애플리케이션 서버 요구사항을 파악한다.
* 요구사항에 대한 구현을 완료한 후 자신의 github 아이디에 해당하는 브랜치에 Pull Request(이하 PR)를 통해 코드 리뷰 요청을 한다.
* 코드 리뷰 피드백에 대한 개선 작업을 하고 다시 PUSH한다.
* 모든 피드백을 완료하면 다음 단계를 도전하고 앞의 과정을 반복한다.

## 우아한테크코스 코드리뷰
* [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)

## 기능요구사항
### 1단계
#### 요구사항 1
- [x] http://localhost:8080/index.html 로 접속했을 때 webapp 디렉토리의 index.html 파일을 읽어 클라이언트에 응답한다.

#### 요구사항 2
- [x] “회원가입” 메뉴를 클릭하면 http://localhost:8080/user/form.html 으로 이동하면서 회원가입할 수 있다.

회원가입을 하면 다음과 같은 형태로 사용자가 입력한 값이 서버에 전달된다.

```http request
/create?userId=javajigi&password=password&name=%EB%B0%95%EC%9E%AC%EC%84%B1&email=javajigi%40slipp.net
```
- [x] HTML과 URL을 비교해 보고 사용자가 입력한 값을 파싱해 model.User 클래스에 저장한다.

#### 요구사항 3
- [x] http://localhost:8080/user/form.html 파일의 form 태그 method를 get에서 post로 수정한 후 회원가입 기능이 정상적으로 동작하도록 구현한다.

#### 요구사항 4
- [x] “회원가입”을 완료하면 /index.html 페이지로 이동하고 싶다. 현재는 URL이 /user/create 로 유지되는 상태로 읽어서 전달할 파일이 없다. 따라서 redirect 방식처럼 회원가입을 완료한 후 “index.html”로 이동해야 한다. 즉, 브라우저의 URL이 /index.html로 변경해야 한다.

#### 요구사항 5
- [x] 지금까지 구현한 소스 코드는 stylesheet 파일을 지원하지 못하고 있다. Stylesheet 파일을 지원하도록 구현하도록 한다.
10 changes: 7 additions & 3 deletions src/main/java/utils/FileIoUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
import java.nio.file.Paths;

public class FileIoUtils {
public static byte[] loadFileFromClasspath(String filePath) throws IOException, URISyntaxException {
Path path = Paths.get(FileIoUtils.class.getClassLoader().getResource(filePath).toURI());
return Files.readAllBytes(path);
public static byte[] loadFileFromClasspath(String filePath) throws IOException {
try {
Path path = Paths.get(FileIoUtils.class.getClassLoader().getResource(filePath).toURI());
return Files.readAllBytes(path);
} catch (URISyntaxException e) {
throw new RuntimeException(String.format("%s: URI로 변환할 수 없습니다.", filePath));
Copy link

Choose a reason for hiding this comment

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

RuntimeException 보다 IllegalArgumentException 과 같은 구체적인 예외 클래스를 활용해보면 어떨까요?
예외 메시지와 함께 구체적인 예외 클래스를 통해 발생한 원인을 좀 더 빨리 파악할 수 있게 해줄 수 있을 것 같습니다!

}
}
}
23 changes: 23 additions & 0 deletions src/main/java/web/HandlerMapping.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package web;

import web.servlet.Servlet;
import web.servlet.UserCreateServlet;

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

public class HandlerMapping {
Copy link

Choose a reason for hiding this comment

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

클래스 설계 👍

private static final Map<RequestMapping, Servlet> handlerMapping = new HashMap<>();

static {
handlerMapping.put(new RequestMapping("/user/create", HttpMethod.POST), new UserCreateServlet());
}

public static Servlet find(HttpRequest httpRequest) {
RequestMapping key = handlerMapping.keySet().stream()
Copy link

Choose a reason for hiding this comment

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

RequestMappingeqauls, hashCode 를 활용해보면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

그런 방법도 있군요!
equals, hashCodegetter를 사용하도록 변경했습니다!

.filter(mapping -> mapping.match(httpRequest))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException(String.format("요청(%s: %s)을 처리할 수 없습니다.", httpRequest.getMethod(), httpRequest.getRequestPath())));
return handlerMapping.get(key);
}
}
14 changes: 14 additions & 0 deletions src/main/java/web/HttpMethod.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package web;

public enum HttpMethod {
GET,
POST;

public boolean isGet() {
return this == GET;
}

public boolean isPost() {
return this == POST;
}
}
38 changes: 38 additions & 0 deletions src/main/java/web/HttpRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package web;

import java.io.BufferedReader;
import java.io.IOException;

public class HttpRequest {
Copy link

Choose a reason for hiding this comment

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

책임 분리 💯

private RequestUri requestUri;
private RequestHeader requestHeader;
private RequestBody requestBody;

public HttpRequest(final BufferedReader bufferedReader) throws IOException {
this.requestUri = new RequestUri(bufferedReader);
this.requestHeader = new RequestHeader(bufferedReader);
if (HttpMethod.POST == getMethod()) {
this.requestBody = new RequestBody(bufferedReader, requestHeader.getContentLength());
}
Comment on lines +12 to +16
Copy link

Choose a reason for hiding this comment

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

현재 각 클래스로 BufferedReader 를 전달해서 처리하고 있는데요~
이러한 방식도 괜찮다고 생각하지만, HttpRequest 클래스 입장에선 각 클래스가 BufferedReader 를 얼마나 읽냐에 따라 순서가 보장되지 않는 단점이 있습니다.

HttpRequest 클래스 한 곳에서 요청을 읽어 순서를 보장하면 응집도를 높일 수 있고, 다른 클래스 변경에도 취약하지 않은 구조로 변경할 수 있을 것 같습니다.
뿐만 아니라, RequestUri, RequestHeader 에서도 BufferedReader 와의 의존성을 끊을 수 있어 각 클래스를 테스트하기가 쉬워질 듯 합니다~!

Copy link
Author

Choose a reason for hiding this comment

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

그렇군요! 테스트할 때마다 매번 BufferedReader를 만드는 과정이 번거로웠는데, 알려주셔서 감사합니다!
좀 더 유연한 구조로 바꿔보았어요! 🙂

}

public String getRequestPath() {
return requestUri.getPath().getRequestPath();
Copy link

Choose a reason for hiding this comment

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

규칙 4. 한 줄에 한 점만 사용 을 지키도록 수정해보면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

디미터 법칙을 위반하고 있었네요!
반영했습니다~

}

public String getParam(final String key) {
return requestUri.getParam(key);
}

public int getContentLength() {
return requestHeader.getContentLength();
}

public RequestBody getRequestBody() {
return requestBody;
}

public HttpMethod getMethod() {
return requestUri.getMethod();
}
}
59 changes: 59 additions & 0 deletions src/main/java/web/HttpResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package web;

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

import java.io.DataOutputStream;
import java.io.IOException;

public class HttpResponse {

private static final Logger logger = LoggerFactory.getLogger(HttpResponse.class);

private DataOutputStream dataOutputStream;

public HttpResponse(DataOutputStream dataOutputStream) {
this.dataOutputStream = dataOutputStream;
}

public void response200Header(int lengthOfBodyContent) {
Copy link

Choose a reason for hiding this comment

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

중복 코드를 제거할 수 있지 않을까요?

try {
this.dataOutputStream.writeBytes("HTTP/1.1 200 OK \r\n");
this.dataOutputStream.writeBytes("Content-Type: text/html;charset=utf-8\r\n");
this.dataOutputStream.writeBytes("Content-Length: " + lengthOfBodyContent + "\r\n");
this.dataOutputStream.writeBytes("\r\n");
} catch (IOException e) {
logger.error(e.getMessage());
}
}

public void response200Header(int lengthOfBodyContent, String contentType) {
try {
this.dataOutputStream.writeBytes("HTTP/1.1 200 OK \r\n");
this.dataOutputStream.writeBytes("Content-Type: " + contentType + "\r\n");
this.dataOutputStream.writeBytes("Content-Length: " + lengthOfBodyContent + "\r\n");
this.dataOutputStream.writeBytes("\r\n");
} catch (IOException e) {
logger.error(e.getMessage());
}
}

public void response302Header(String url) {
try {
this.dataOutputStream.writeBytes("HTTP/1.1 302 Found \r\n");
this.dataOutputStream.writeBytes("Location: " + url + "\r\n");
this.dataOutputStream.writeBytes("\r\n");
} catch (IOException e) {
logger.error(e.getMessage());
}
}

public void responseBody(byte[] body) {
try {
this.dataOutputStream.write(body, 0, body.length);
this.dataOutputStream.flush();
} catch (IOException e) {
logger.error(e.getMessage());
}
}
}
35 changes: 35 additions & 0 deletions src/main/java/web/RequestBody.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package web;

import utils.IOUtils;

import java.io.BufferedReader;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

public class RequestBody {

private String body;

public RequestBody(BufferedReader bufferedReader, int contentLength) throws IOException {
if (contentLength == 0) {
return;
}
this.body = IOUtils.readData(bufferedReader, contentLength);
}

public String getBody() {
return body;
}

public Map<String, String> parse() {
String[] splitBody = body.split("&");
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.

👍
아래 로직도 클래스 분리를 해봤습니다!


Map<String, String> bodies = new HashMap<>();
for (String body : splitBody) {
String[] keyValue = body.split("=");
bodies.put(keyValue[0], keyValue[1]);
Copy link

Choose a reason for hiding this comment

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

userId= 와 같이 값을 비워 요청하면 문제가 발생하지 않을까요?
예외 케이스에 대해서도 적절히 고민해보면 좋을 것 같아요~

Copy link
Author

Choose a reason for hiding this comment

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

클래스로 분리해 적용해보았습니다 🙂

}
return bodies;
}
}
32 changes: 32 additions & 0 deletions src/main/java/web/RequestHeader.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package web;

import java.io.BufferedReader;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

public class RequestHeader {
private final Map<String, String> headers = new HashMap<>();

public RequestHeader(BufferedReader bufferedReader) throws IOException {
String line = bufferedReader.readLine();
while (!isEmpty(line)) {
String[] tokens = line.split(": ");
headers.put(tokens[0], tokens[1]);
line = bufferedReader.readLine();
}
}

private boolean isEmpty(String line) {
return line == null || "".equals(line);
}

public Map<String, String> getHeaders() {
return headers;
}

public int getContentLength() {
String contentLength = headers.get("Content-Length");
return Integer.parseInt(contentLength);
}
}
15 changes: 15 additions & 0 deletions src/main/java/web/RequestMapping.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package web;

public class RequestMapping {
private final String uri;
private final HttpMethod httpMethod;

public RequestMapping(final String uri, final HttpMethod httpMethod) {
this.uri = uri;
this.httpMethod = httpMethod;
}

public boolean match(HttpRequest httpRequest) {
return this.uri.equals(httpRequest.getRequestPath()) && this.httpMethod == httpRequest.getMethod();
}
}
37 changes: 37 additions & 0 deletions src/main/java/web/RequestPath.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package web;

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

public class RequestPath {
private final String fullPath;
private final String requestPath;
private final Map<String, String> requestParameters = new HashMap<>();
Copy link

Choose a reason for hiding this comment

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

파라미터와 관련된 일급 컬렉션을 만들어 관리해보면 어떨까요?


public RequestPath(String requestPath) {
this.fullPath = requestPath;
String[] splitPath = requestPath.split("\\?");
this.requestPath = splitPath[0];
if (hasNoParameters()) {
return;
}
String parameters = splitPath[1];
String[] splitParameters = parameters.split("&");
for (String parameter : splitParameters) {
String[] key = parameter.split("=");
requestParameters.put(key[0], key[1]);
}
}

private boolean hasNoParameters() {
return this.fullPath.equals(this.requestPath);
}

public String getRequestPath() {
return this.requestPath;
}

public String getRequestParameter(String key) {
return requestParameters.get(key);
}
}
37 changes: 37 additions & 0 deletions src/main/java/web/RequestUri.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package web;

import java.io.BufferedReader;
import java.io.IOException;

public class RequestUri {
private HttpMethod method;
private RequestPath path;
private String protocol;

public RequestUri(BufferedReader bufferedReader) throws IOException {
String uri = bufferedReader.readLine();
if (uri.isEmpty()) {
return;
Copy link

Choose a reason for hiding this comment

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

여기서 return 하면 인스턴스는 정상적으로 생성되지 않나요?
uri가 잘못되어도 인스턴스가 생성되도록 하신건 의도하신 걸까요?

Copy link
Author

Choose a reason for hiding this comment

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

exception을 던지도록 했습니다!

}
String[] splitUri = uri.split(" ");
this.method = HttpMethod.valueOf(splitUri[0]);
this.path = new RequestPath(splitUri[1]);
this.protocol = splitUri[2];
}

public String getParam(final String key) {
return this.path.getRequestParameter(key);
}

public HttpMethod getMethod() {
return method;
}

public RequestPath getPath() {
return path;
}

public String getProtocol() {
return protocol;
}
}
11 changes: 11 additions & 0 deletions src/main/java/web/filter/Filter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package web.filter;

import web.HttpRequest;
import web.HttpResponse;

import java.io.IOException;

public interface Filter {

void doFilter(HttpRequest httpRequest, HttpResponse httpResponse, FilterChain filterChain) throws IOException;
}
32 changes: 32 additions & 0 deletions src/main/java/web/filter/FilterChain.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package web.filter;

import web.HttpRequest;
import web.HttpResponse;
import web.servlet.DispatcherServlet;
import web.servlet.Servlet;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

public class FilterChain {

private static final List<Filter> filters = new ArrayList<>();
private static final Servlet servlet = new DispatcherServlet();

static {
filters.add(new ResourceFilter());
}

private int filterIndex = 0;

public void doFilter(HttpRequest httpRequest, HttpResponse httpResponse) throws IOException {
if (filterIndex < filters.size()) {
Filter filter = filters.get(filterIndex);
filterIndex++;
filter.doFilter(httpRequest, httpResponse, this);
return;
}
servlet.doService(httpRequest, httpResponse);
}
}
Loading