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

[MVC 구현하기 - 1단계] 후디(조동현) 미션 제출합니다. #163

Merged
merged 8 commits into from
Sep 22, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import jakarta.servlet.ServletContext;
import nextstep.mvc.DispatcherServlet;
import nextstep.mvc.controller.tobe.AnnotationHandlerMapping;
import nextstep.web.WebApplicationInitializer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -14,6 +15,7 @@ public class AppWebApplicationInitializer implements WebApplicationInitializer {
public void onStartup(final ServletContext servletContext) {
final var dispatcherServlet = new DispatcherServlet();
dispatcherServlet.addHandlerMapping(new ManualHandlerMapping());
dispatcherServlet.addHandlerMapping(new AnnotationHandlerMapping("com.techcourse.controller"));

final var dispatcher = servletContext.addServlet("dispatcher", dispatcherServlet);
dispatcher.setLoadOnStartup(1);
Expand Down
33 changes: 23 additions & 10 deletions mvc/src/main/java/nextstep/mvc/DispatcherServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import nextstep.mvc.controller.asis.Controller;
import nextstep.mvc.controller.tobe.HandlerExecution;
import nextstep.mvc.view.JspView;
import nextstep.mvc.view.ModelAndView;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

public class DispatcherServlet extends HttpServlet {

private static final long serialVersionUID = 1L;
Expand All @@ -34,29 +36,40 @@ public void addHandlerMapping(final HandlerMapping handlerMapping) {
}

@Override
protected void service(final HttpServletRequest request, final HttpServletResponse response) throws ServletException {
protected void service(final HttpServletRequest request, final HttpServletResponse response)
throws ServletException {
log.debug("Method : {}, Request URI : {}", request.getMethod(), request.getRequestURI());

try {
final var controller = getController(request);
final var viewName = controller.execute(request, response);
final Object controller = getController(request);

// TODO: 다형성을 통해 해결해야함
if (controller instanceof HandlerExecution) {
HandlerExecution handlerExecution = (HandlerExecution) controller;
ModelAndView modelAndView = handlerExecution.handle(request, response);
Map<String, Object> model = modelAndView.getModel();
modelAndView.getView().render(model, request, response);
return;
}

final var viewName = ((Controller) controller).execute(request, response);
Comment on lines +46 to +55
Copy link
Author

Choose a reason for hiding this comment

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

나중에 HandlerAdapter가 구현이되면, 자연스럽게 해결이 되지 않을까 하는 부분입니다. 사실 아직 HandlerAdapter 의 역할을 잘 알고있지는 않아서 아닐수도 있어요... 찬의 의견이 궁금합니다!

Choose a reason for hiding this comment

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

나중에 각 HandlerMapping을 처리할 수 있는 HandlerAdapter를 구현하면 자연스럽게 해결될 것 같아요~!

move(viewName, request, response);
} catch (Throwable e) {
log.error("Exception : {}", e.getMessage(), e);
throw new ServletException(e.getMessage());
}
}

private Controller getController(final HttpServletRequest request) {
private Object getController(final HttpServletRequest request) {
return handlerMappings.stream()
.map(handlerMapping -> handlerMapping.getHandler(request))
.filter(Objects::nonNull)
.map(Controller.class::cast)
.findFirst()
.orElseThrow();
}

private void move(final String viewName, final HttpServletRequest request, final HttpServletResponse response) throws Exception {
private void move(final String viewName, final HttpServletRequest request, final HttpServletResponse response)

Choose a reason for hiding this comment

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

나중에 이부분도 DispatcherServlet이 아닌 View 쪽으로 로직을 넘기면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

HandlerAdapter 를 구현하며 자연스럽게 ModelAndView가 처리하도록 분리하였습니다!

throws Exception {
if (viewName.startsWith(JspView.REDIRECT_PREFIX)) {
response.sendRedirect(viewName.substring(JspView.REDIRECT_PREFIX.length()));
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
package nextstep.mvc.controller.tobe;

import jakarta.servlet.http.HttpServletRequest;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import nextstep.mvc.HandlerMapping;
import nextstep.mvc.controller.tobe.exception.ControllerNotFoundException;
import nextstep.web.annotation.Controller;
import nextstep.web.annotation.RequestMapping;
import org.reflections.Reflections;
import org.reflections.scanners.Scanners;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

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

public class AnnotationHandlerMapping implements HandlerMapping {

private static final Logger log = LoggerFactory.getLogger(AnnotationHandlerMapping.class);
Expand All @@ -21,10 +30,46 @@ public AnnotationHandlerMapping(final Object... basePackage) {
}

public void initialize() {
Set<Class<?>> controllerClasses = extractClasses();
List<Method> methods = extractMethods(controllerClasses);

for (Method method : methods) {
addHandlerExecutions(method);
}

log.info("Initialized AnnotationHandlerMapping!");
}

private Set<Class<?>> extractClasses() {
Reflections classReflections = new Reflections(basePackage, Scanners.TypesAnnotated);
return classReflections.getTypesAnnotatedWith(Controller.class);
}

private List<Method> extractMethods(final Set<Class<?>> controllers) {
return controllers.stream()
.flatMap(it -> Arrays.stream(it.getMethods()))
.filter(it -> it.isAnnotationPresent(RequestMapping.class))
.collect(Collectors.toList());
}

private void addHandlerExecutions(final Method method) {
RequestMapping requestMapping = method.getAnnotation(RequestMapping.class);
List<HandlerKey> handlerKeys = HandlerKey.from(requestMapping);
Copy link
Author

Choose a reason for hiding this comment

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

HandlerKeyfromrequestMapping 어노테이션 객체(?)를 전달합니다. 내부에서 value() (URL)와 method() (RequestMethod[]) 를 requestMapping 에서 꺼내와서 List<HandlerKey> 를 반환합니다. 리스트를 반환하는 이유는 method() 의 반환값이 RequestMethod 의 배열이기 때문입니다. URL 하나에 여러 HTTP Method를 매핑하는 것이 의도된 것 같아서 이렇게 정적 팩토리 메소드로 만들었어요.

HandlerExecution handlerExecution = new HandlerExecution(method);

for (HandlerKey handlerKey : handlerKeys) {
handlerExecutions.put(handlerKey, handlerExecution);
}
}

public Object getHandler(final HttpServletRequest request) {
return null;
HandlerKey handlerKey = new HandlerKey(request);
HandlerExecution handlerExecution = handlerExecutions.get(handlerKey);

if (handlerExecution == null) {
throw new ControllerNotFoundException();
}
Comment on lines +69 to +71

Choose a reason for hiding this comment

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

request가 들어오면 DispatcherServlet에 있는 모든 HandlerMapping을 돌며 handle이 가능한지 여부를 확인할텐데 이렇게 특정 HandlerMapping에서 NotFoundException을 터트려버리면 다른 HandlerMapping에서 해당 request를 처리할 수 있는 여지를 남길 수 없을 것 같아요. 모든 HandlerMapping을 전부 순회했는데도 불구하고 찾지 못한다면 그 때 ControllerNotFoundException을 터트리는 것이 어떨까요?


return handlerExecution;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,24 @@

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import nextstep.mvc.view.ModelAndView;

public class HandlerExecution {

private final Class<?> controller;
private final Method method;

public HandlerExecution(final Method method) {
this.controller = method.getDeclaringClass();
this.method = method;
}
Comment on lines +14 to +17
Copy link
Author

Choose a reason for hiding this comment

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

Method 만 넣어주면, 알아서 컨트롤러의 Class 도 가져옵니다.

Choose a reason for hiding this comment

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

이건 저도 변경해야 할 부분인데 나중에 controller에 필드들이 추가가 된다면 지금과 같은 방법을 사용하면 대응을 할 수 없을 것 같아요! 추후 DI를 통해 생성된 Object를 필드로 갖고 있는 것이 좋을 것 같은데 후디의 생각은 어떠신가요?!


public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response) throws Exception {
return null;
Constructor<?> constructor = controller.getConstructor();
Object instance = constructor.newInstance();

return (ModelAndView) method.invoke(instance, request, response);
Comment on lines 19 to +23
Copy link
Author

Choose a reason for hiding this comment

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

handle() 이 호출될 때 컨트롤러의 Class 를 사용해서 컨트롤러 인스턴스를 생성합니다.
HandlerExecution 이 초기화 되는 시점에서 인스턴스를 필드에 넣어주지 않은 이유는 handle 에서 throws 를 사용해 외부로 예외 처리를 미루는게 코드가 깔끔해지기 때문입니다.

}
}
39 changes: 35 additions & 4 deletions mvc/src/main/java/nextstep/mvc/controller/tobe/HandlerKey.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package nextstep.mvc.controller.tobe;

import nextstep.web.support.RequestMethod;

import jakarta.servlet.http.HttpServletRequest;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import nextstep.web.annotation.RequestMapping;
import nextstep.web.support.RequestMethod;

public class HandlerKey {

Expand All @@ -14,6 +19,28 @@ public HandlerKey(final String url, final RequestMethod requestMethod) {
this.requestMethod = requestMethod;
}

public HandlerKey(final HttpServletRequest request) {
String url = request.getRequestURI();
RequestMethod requestMethod = RequestMethod.valueOf(request.getMethod());

this.url = url;
this.requestMethod = requestMethod;
Comment on lines +23 to +27

Choose a reason for hiding this comment

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

이 부분 inline 처리하면 어떨까요?

this.url = request.getRequestURI();
this.requestMethod = RequestMethod.valueOf(request.getMethod()); 

이렇게요!

Copy link
Author

Choose a reason for hiding this comment

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

엇 ㅋㅋㅋ 제가 왜 이렇게 했을까요! 수정하겠습니다 ㅎㅎ

}

public static List<HandlerKey> from(final RequestMapping requestMapping) {
String url = requestMapping.value();
List<RequestMethod> requestMethods = Arrays.stream(requestMapping.method())
.collect(Collectors.toList());

List<HandlerKey> handlerKeys = new ArrayList<>();
for (RequestMethod requestMethod : requestMethods) {
HandlerKey handlerKey = new HandlerKey(url, requestMethod);
handlerKeys.add(handlerKey);
}

return handlerKeys;
Comment on lines +35 to +41

Choose a reason for hiding this comment

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

사소하지만 이부분을 메서드로 추출하면 더 깔끔할 것 같아요 ㅎㅎ!

}

@Override
public String toString() {
return "HandlerKey{" +
Expand All @@ -24,8 +51,12 @@ public String toString() {

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof HandlerKey)) return false;
if (this == o) {
return true;
}
if (!(o instanceof HandlerKey)) {
return false;
}
HandlerKey that = (HandlerKey) o;
return Objects.equals(url, that.url) && requestMethod == that.requestMethod;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package nextstep.mvc.controller.tobe.exception;

public class ControllerNotFoundException extends RuntimeException {

public ControllerNotFoundException() {
super("요청에 대한 적절한 컨트롤러를 찾을 수 없습니다.");
}
}
21 changes: 13 additions & 8 deletions mvc/src/main/java/nextstep/mvc/view/JspView.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,34 @@

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Map;

public class JspView implements View {

private static final Logger log = LoggerFactory.getLogger(JspView.class);

public static final String REDIRECT_PREFIX = "redirect:";
private static final Logger log = LoggerFactory.getLogger(JspView.class);
private final String viewName;

public JspView(final String viewName) {
this.viewName = viewName;
}

@Override
public void render(final Map<String, ?> model, final HttpServletRequest request, final HttpServletResponse response) throws Exception {
// todo
public void render(final Map<String, ?> model, final HttpServletRequest request, final HttpServletResponse response)
throws Exception {
if (viewName.startsWith(JspView.REDIRECT_PREFIX)) {
response.sendRedirect(viewName.substring(JspView.REDIRECT_PREFIX.length()));
return;
}

final var requestDispatcher = request.getRequestDispatcher(viewName);
requestDispatcher.forward(request, response);
Comment on lines +27 to +28

Choose a reason for hiding this comment

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

model에 저장되어 있는 값을 request에 setting 한 후에 forward가 일어나야 할 것 같아요!


model.keySet().forEach(key -> {
log.debug("attribute name : {}, value : {}", key, model.get(key));
request.setAttribute(key, model.get(key));
});

// todo
}
}