-
Notifications
You must be signed in to change notification settings - Fork 90
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
[디디] 1단계 - HTTP 웹 서버 구현 미션 제출합니다. #129
Changes from all commits
739ec97
8d3b963
d48f363
45b3d57
f0d37fd
c1c589d
bac52fd
876865b
71d1777
b5a9165
4eb599a
7d5cfbc
3e01533
b9cc2a1
5276016
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package exception; | ||
|
||
public class InvalidRequestBodyException extends RuntimeException { | ||
public InvalidRequestBodyException() { | ||
super("지원하지 않는 request body 형식입니다."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package exception; | ||
|
||
public class InvalidRequestHeaderException extends RuntimeException { | ||
public InvalidRequestHeaderException() { | ||
super("지원하지 않는 request header 형식입니다."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package exception; | ||
|
||
public class MethodNotAllowedException extends RuntimeException { | ||
public MethodNotAllowedException() { | ||
super("올바른 핸들러 메서드를 발견하지 못했습니다."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package exception; | ||
|
||
public class MethodParameterBindException extends RuntimeException { | ||
public MethodParameterBindException() { | ||
super("파라미터 매핑 예외"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package exception; | ||
|
||
public class NoDefaultConstructorException extends RuntimeException { | ||
public <T> NoDefaultConstructorException(Class<T> clazz) { | ||
super(clazz.getName() + "클래스에 기본생성자가 필요합니다."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package exception; | ||
|
||
public class UnsupportedMethodTypeException extends RuntimeException { | ||
public UnsupportedMethodTypeException(String input) { | ||
super(String.format("%s는 지원하지 않는 메소드 타입입니다.", input)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,10 +5,40 @@ | |
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Objects; | ||
|
||
public class FileIoUtils { | ||
public static byte[] loadFileFromClasspath(String filePath) throws IOException, URISyntaxException { | ||
Path path = Paths.get(FileIoUtils.class.getClassLoader().getResource(filePath).toURI()); | ||
private static final List<String> BASE_PATH = Arrays.asList("templates", "static"); | ||
public static final String NOT_FOUND = "404 NOT FOUND 잘 부탁드립니다."; | ||
public static final String INDEX_PAGE = "/index.html"; | ||
|
||
public static byte[] loadFileFromClasspath(String filePath) throws IOException { | ||
if (filePath.equals("/")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이런 값들도 상수로 뺴는 걸 고민해봐도 좋겠어요. |
||
return loadFileFromClasspath("/index.html"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 상수로 뺴셨는데 그대로 사용하셨네요. :) |
||
} | ||
|
||
if(!filePath.contains(".")) { | ||
return "".getBytes(); | ||
} | ||
|
||
Path path = BASE_PATH.stream() | ||
.map(base -> getPath(base + filePath)) | ||
.filter(Objects::nonNull) | ||
.findAny() | ||
.orElseGet(() -> getPath("static/notFound.html")); | ||
|
||
return Files.readAllBytes(path); | ||
} | ||
|
||
private static Path getPath(String path) { | ||
try { | ||
return Paths.get(FileIoUtils.class.getClassLoader().getResource(path).toURI()); | ||
} catch (NullPointerException e) { | ||
return null; | ||
} catch (URISyntaxException e) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package webserver; | ||
|
||
import java.lang.reflect.Field; | ||
import java.lang.reflect.InvocationTargetException; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
import exception.MethodParameterBindException; | ||
import exception.NoDefaultConstructorException; | ||
|
||
public class DefaultHttpMessageConverter implements HttpMessageConverter { | ||
@Override | ||
public <T> T convert(Class<T> clazz, Map<String, String> attributes) { | ||
try { | ||
T instance = clazz.getConstructor().newInstance(); | ||
attributes.forEach((key, value) -> { | ||
try { | ||
Field field = clazz.getDeclaredField(key); | ||
field.setAccessible(true); | ||
field.set(instance, value); | ||
} catch (NoSuchFieldException | IllegalAccessException e) { | ||
e.printStackTrace(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.printStackTrace(); 는 안티패턴이라 별도 로거를 통해서 로그를 찍는게 좋을 것 같네요. :) |
||
} | ||
}); | ||
return instance; | ||
} catch (InstantiationException | InvocationTargetException | IllegalAccessException e) { | ||
throw new MethodParameterBindException(); | ||
} catch (NoSuchMethodException e) { | ||
throw new NoDefaultConstructorException(clazz); | ||
} | ||
} | ||
|
||
@Override | ||
public boolean isSupport(Class<?> parameterType, Map<String, String> body) { | ||
List<String> collect = Arrays.stream(parameterType.getDeclaredFields()).map(Field::getName) | ||
.collect(Collectors.toList()); | ||
return collect.stream() | ||
.anyMatch(body::containsKey); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
package webserver; | ||
|
||
import java.io.BufferedReader; | ||
import java.io.DataOutputStream; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.io.InputStreamReader; | ||
import java.io.OutputStream; | ||
import java.lang.reflect.Method; | ||
import java.net.Socket; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import utils.FileIoUtils; | ||
import webserver.controller.Handlers; | ||
import webserver.controller.IndexController; | ||
import webserver.controller.UserController; | ||
|
||
public class DispatcherServlet implements Runnable { | ||
private static final Logger logger = LoggerFactory.getLogger(DispatcherServlet.class); | ||
|
||
private Socket connection; | ||
|
||
public DispatcherServlet(Socket connectionSocket) { | ||
this.connection = connectionSocket; | ||
} | ||
|
||
public void run() { | ||
logger.debug("New Client Connect! Connected IP : {}, Port : {}", connection.getInetAddress(), | ||
connection.getPort()); | ||
try (InputStream in = connection.getInputStream(); OutputStream out = connection.getOutputStream(); | ||
InputStreamReader ir = new InputStreamReader(in); BufferedReader br = new BufferedReader(ir)) { | ||
DataOutputStream dos = new DataOutputStream(out); | ||
|
||
ServletRequest servletRequest = new ServletRequest(br); | ||
|
||
String path = servletRequest.getPath(); | ||
|
||
if (path.contains(".") && !path.endsWith(".html")) { | ||
byte[] bytes = FileIoUtils.loadFileFromClasspath(path); | ||
|
||
try { | ||
dos.writeBytes("HTTP/1.1 200 OK \r\n"); | ||
dos.writeBytes("Content-Length: " + bytes.length + "\r\n"); | ||
dos.writeBytes( | ||
"Content-Type: " + servletRequest.getHeader("Accept").split(",")[0] + ";charset=utf-8\r\n"); | ||
dos.writeBytes("\r\n"); | ||
dos.write(bytes, 0, bytes.length); | ||
dos.flush(); | ||
} catch (IOException e) { | ||
logger.error(e.getMessage()); | ||
} | ||
return; | ||
} | ||
Comment on lines
+42
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 정적 자원에 대한 부분도 별도 핸들러로 분리하고, 응답도 SerlvetResposne로 함께 추상화 할 수 있지 않을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 감사합니다! 나름대로 주신 피드백에 맞도록 반영했습니다! |
||
|
||
List<Class<? extends Handlers>> controllers = Arrays.asList(UserController.class, IndexController.class); | ||
HandlerMapping handlerMapping = new HandlerMapping(controllers); | ||
Method handler = handlerMapping.mapping(servletRequest); | ||
HandlerAdaptor handlerAdaptor = new HandlerAdaptor(); | ||
HttpMessageConverter converter = new DefaultHttpMessageConverter(); | ||
ServletResponse servletResponse = handlerAdaptor.invoke(handler, servletRequest, converter); | ||
servletResponse.createResponse(dos, servletRequest); | ||
} catch (IOException e) { | ||
logger.error(e.getMessage()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 예외 발생 시 500이라는 응답이라도 클라이언트에 전달해줘야 하지 않을까요? |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package webserver; | ||
|
||
import java.lang.reflect.InvocationTargetException; | ||
import java.lang.reflect.Method; | ||
|
||
import model.User; | ||
|
||
public class HandlerAdaptor { | ||
|
||
public ServletResponse invoke(Method handler, ServletRequest servletRequest, | ||
HttpMessageConverter converter) { | ||
|
||
Class<?>[] parameterTypes = handler.getParameterTypes(); | ||
User user = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HandlerAdaptor가 도메인 모델에 있는 User에 대한 의존을 가지고 있는게 맞을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분이 많이 헷갈렸는데, Cast를 통해 해결했습니다! |
||
|
||
try { | ||
Object handlers = handler.getDeclaringClass().newInstance(); | ||
if (parameterTypes.length == 0) { | ||
return (ServletResponse)handler.invoke(handlers); | ||
} | ||
if (converter.isSupport(parameterTypes[0], servletRequest.getBody())) { | ||
user = (User)converter.convert(parameterTypes[0], servletRequest.getBody()); | ||
} | ||
return (ServletResponse)handler.invoke(handlers, user); | ||
Comment on lines
+21
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 파라미터 메소드가 다른 컨트롤러가 추가될 때마다 상당히 복잡해질 우려가 있네요. httpRequest, httpResposne 위주로 의존하고 도메인과는 최대한 격리되도록이요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Map<String, String> 형태로 들어온 Body나 Params를 컨트롤러의 파라미터로 바인딩 하는 과정에 Object로 만든 걸 해당 타입으로 바꾸는 과정에서 여러개의 파라미터 바인딩을 하는게 어렵더라구요. 그런 역할은 Converter가 아니라 Argument Resolver에서 해주는 게 맞나요? 이 질문에 대한 내용이 이 부분때문 인 것 같은데요. |
||
} catch (IllegalAccessException | InvocationTargetException | InstantiationException e) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package webserver; | ||
|
||
import java.lang.reflect.Method; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
import exception.MethodNotAllowedException; | ||
import webserver.controller.Handlers; | ||
import webserver.controller.annotation.Controller; | ||
import webserver.controller.annotation.RequestMapping; | ||
|
||
public class HandlerMapping { | ||
private final List<Class<? extends Handlers>> handlers; | ||
|
||
public HandlerMapping(List<Class<? extends Handlers>> handlers) { | ||
this.handlers = handlers.stream() | ||
.filter(handler -> Objects.nonNull(handler.getAnnotation(Controller.class))) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
public Method mapping(ServletRequest request) { | ||
RequestHeader.MethodType methodType = request.getMethod(); | ||
String origin = request.getPath(); | ||
String findPath = origin.endsWith(".html") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. html이 붙어있다면 제외하고 찾기 보다는 정적 파일로 취급하는게 맞지 않을까요? :) |
||
? origin.substring(0, origin.lastIndexOf(".")) | ||
: origin; | ||
|
||
return handlers.stream() | ||
.flatMap(controller -> Stream.of(controller.getMethods())) | ||
.filter(method -> { | ||
RequestMapping annotation = method.getAnnotation(RequestMapping.class); | ||
return Objects.nonNull(annotation) && Arrays.asList(annotation.value()).contains(findPath) | ||
&& annotation.type().equals(methodType); | ||
}) | ||
.findAny() | ||
.orElseThrow(MethodNotAllowedException::new); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package webserver; | ||
|
||
import java.util.Map; | ||
|
||
public interface HttpMessageConverter { | ||
<T> T convert(Class<T> clazz, Map<String, String> attributes); | ||
|
||
boolean isSupport(Class<?> parameterType, Map<String, String> body); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package webserver; | ||
|
||
import java.util.Collections; | ||
import java.util.LinkedHashMap; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
import exception.InvalidRequestBodyException; | ||
|
||
public class RequestBody { | ||
private final Map<String, String> attribute; | ||
|
||
private RequestBody(Map<String, String> attribute) { | ||
this.attribute = attribute; | ||
} | ||
|
||
public static RequestBody of(String line) { | ||
Map<String, String> attributes = parseBodyAttributes(line); | ||
|
||
return new RequestBody(attributes); | ||
} | ||
|
||
private static Map<String, String> parseBodyAttributes(String line) { | ||
Map<String, String> attributes = new LinkedHashMap<>(); | ||
if (Objects.isNull(line) || line.isEmpty()) { | ||
return attributes; | ||
} | ||
try { | ||
String[] params = line.split("&"); | ||
for (String param : params) { | ||
String[] attribute = param.split("="); | ||
attributes.put(attribute[0], attribute[1]); | ||
} | ||
return attributes; | ||
} catch (IndexOutOfBoundsException e) { | ||
throw new InvalidRequestBodyException(); | ||
} | ||
} | ||
|
||
public Map<String, String> getAttribute() { | ||
return Collections.unmodifiableMap(attribute); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예외에 대해서 꼼꼼하게 정의를 해주셨네요 👍
몇가지는 http 메시지에 대해서 정의된 예외로 보이는데 이를 같이 조금 더 추상화해봐도 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
급하게 하느라 신경을 못썼네요 😭 수정했습니다!