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단계] 에단(김석호) 미션 제출합니다. #426

Merged
merged 16 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from 12 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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
run: ./gradlew clean build codeCoverageReport --info -x :study:build
run: ./gradlew clean build codeCoverageReport sonar --info -x :study:build
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
# @MVC 구현하기

## 학습테스트
- [x] ReflectionTest
- [x] ServletTest
- [x] FilterTest

## 요구 사항
- [x] AnnotationHandlerMappingTest 성공 테스트로 변경
- [x] get() 성공 테스트로 변경
- [x] post() 성공 테스트로 변경
1 change: 1 addition & 0 deletions mvc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ dependencies {

testImplementation "org.assertj:assertj-core:3.24.2"
testImplementation "org.junit.jupiter:junit-jupiter-api:5.7.2"
testImplementation "org.junit.jupiter:junit-jupiter-params:5.9.2"
testImplementation "org.mockito:mockito-core:5.4.0"
testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:5.7.2"
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
package web.org.springframework.web.bind.annotation;

import java.util.Arrays;

public enum RequestMethod {
GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE
GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE;

public static RequestMethod from(final String method) {
Copy link
Member

Choose a reason for hiding this comment

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

오호 valueOf 안쓰시고 정팩메를 정의하셨네요 ! 👍

Copy link
Author

Choose a reason for hiding this comment

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

예외 메세지를 custom 하려고 이렇게 했습니다. 다만 모디 글을 보니 switch 문도 나쁘지 않을 것 같아서 다음단계에서 바꿔보려고 합니다!

return Arrays.stream(values())
.filter(value -> value.name().equalsIgnoreCase(method))
.findAny()
.orElseThrow(() -> new IllegalArgumentException("지원하지 않는 RequestMethod 입니다."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import jakarta.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import web.org.springframework.web.bind.annotation.RequestMethod;

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

public class AnnotationHandlerMapping {

Expand All @@ -21,9 +23,19 @@ public AnnotationHandlerMapping(final Object... basePackage) {

public void initialize() {
log.info("Initialized AnnotationHandlerMapping!");
final Map<HandlerKey, HandlerExecution> handlerExecutions = Objects.requireNonNull(HandlerExecutionFactory.create(basePackage));
Copy link
Member

Choose a reason for hiding this comment

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

생성 책임을 분리하셨군요 👍

this.handlerExecutions.putAll(handlerExecutions);
}

public Object getHandler(final HttpServletRequest request) {
return null;
final String requestURI = request.getRequestURI();
final String method = request.getMethod();
final HandlerKey targetKey = new HandlerKey(requestURI, RequestMethod.from(method));

if (handlerExecutions.containsKey(targetKey)) {
return handlerExecutions.get(targetKey);
}

return new IllegalArgumentException("처리 할 수 없는 요청 입니다.");
Copy link
Member

Choose a reason for hiding this comment

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

HandlerExecutions에서 해당 메서드와 uri가 없으면 예외를 반환하네요?? 와우.. Optional, Null을 사용하지 않고 해당 방식으로 구현하신 이유가 무엇일까요?

Copy link
Author

@cookienc cookienc Sep 15, 2023

Choose a reason for hiding this comment

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

처리할 수 없는 메서드면 예외를 반환하는게 좋다고 생각해서 이렇게 구현했습니다.
2단계 미션을 보니 여러 HandlerMapping이 등장하는데요. 이럴 경우에는 각각의 handlerMapping에서 예외처리가 분산된다는 문제점이 발생할 것 같습니다! 이런 문제는 2단계하면서 직접 겪어보고 null이나 Optional을 리턴하도록 변경해보겠습니다~

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,22 @@
import jakarta.servlet.http.HttpServletResponse;
import webmvc.org.springframework.web.servlet.ModelAndView;

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;

public class HandlerExecution {

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

public HandlerExecution(final Class<?> controller, final Method method) {
this.controller = controller;
this.method = method;
}

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

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,42 @@
package webmvc.org.springframework.web.servlet.mvc.tobe;

import context.org.springframework.stereotype.Controller;
import org.reflections.Reflections;
import web.org.springframework.web.bind.annotation.RequestMapping;
import web.org.springframework.web.bind.annotation.RequestMethod;

import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

public class HandlerExecutionFactory {

public static Map<HandlerKey, HandlerExecution> create(final Object... basePackage) {
final Map<HandlerKey, HandlerExecution> handlerExecutions = new HashMap<>();

final Reflections reflections = new Reflections(basePackage);

final Set<Class<?>> controllers = reflections.getTypesAnnotatedWith(Controller.class);
for (final Class<?> controller : controllers) {

for (final Method method : controller.getDeclaredMethods()) {
if (method.isAnnotationPresent(RequestMapping.class)) {

final RequestMapping requestMapping = method.getAnnotation(RequestMapping.class);
final String requestUrl = requestMapping.value();
final RequestMethod[] requestMethods = requestMapping.method();

for (final RequestMethod requestMethod : requestMethods) {
final HandlerKey handlerKey = new HandlerKey(requestUrl, requestMethod);
final HandlerExecution handlerExecution = new HandlerExecution(controller, method);
handlerExecutions.put(handlerKey, handlerExecution);
}
}
}
}

return handlerExecutions;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package web.org.springframework.web.bind.annotation;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static org.assertj.core.api.Assertions.assertThatCode;

class RequestMethodTest {

@ParameterizedTest
@ValueSource(strings = {"GET", "POST", "PUT", "DELETE", "HEAD", "PATCH", "OPTIONS", "TRACE"})
void String을_알맞는_RequestMethod로_변환_할_수_있다(final String method) {
// when, then
assertThatCode(() -> RequestMethod.from(method))
.doesNotThrowAnyException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -48,4 +49,15 @@ void post() throws Exception {

assertThat(modelAndView.getObject("id")).isEqualTo("gugu");
}

@Test
void handlerKey가_맞지_않으면_오류가_발생한다() {
final var request = mock(HttpServletRequest.class);

when(request.getRequestURI()).thenReturn("/not-valid");
when(request.getMethod()).thenReturn("NOT");

assertThatThrownBy(() -> handlerMapping.getHandler(request))
.isInstanceOf(IllegalArgumentException.class);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package servlet.com.example;

import jakarta.servlet.*;
import jakarta.servlet.Filter;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse;
import jakarta.servlet.annotation.WebFilter;

import java.io.IOException;
Expand All @@ -11,6 +15,13 @@ public class CharacterEncodingFilter implements Filter {
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
request.getServletContext().log("doFilter() 호출");

final String body = "인코딩";
response.setContentType("text/plain");
response.setCharacterEncoding("UTF-8");
response.setContentLength(body.getBytes().length);
response.getWriter().write(body);

chain.doFilter(request, response);
}
}
10 changes: 9 additions & 1 deletion study/src/test/java/reflection/Junit3TestRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@

import org.junit.jupiter.api.Test;

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;

class Junit3TestRunner {

@Test
void run() throws Exception {
Class<Junit3Test> clazz = Junit3Test.class;

// TODO Junit3Test에서 test로 시작하는 메소드 실행
final Constructor<?> constructor = clazz.getDeclaredConstructor();
final Object junit3Test = constructor.newInstance();

for (final Method method : clazz.getDeclaredMethods()) {
method.invoke(junit3Test);
}
}
}
13 changes: 12 additions & 1 deletion study/src/test/java/reflection/Junit4TestRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,23 @@

import org.junit.jupiter.api.Test;

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;

class Junit4TestRunner {

@Test
void run() throws Exception {
Class<Junit4Test> clazz = Junit4Test.class;

// TODO Junit4Test에서 @MyTest 애노테이션이 있는 메소드 실행
final Method[] methods = clazz.getDeclaredMethods();
final Constructor<Junit4Test> constructor = clazz.getConstructor();
final Junit4Test junit4Test = constructor.newInstance();

for (final Method method : clazz.getDeclaredMethods()) {
if (method.isAnnotationPresent(MyTest.class)) {
method.invoke(junit4Test);
}
}
}
}
Loading
Loading