🚀 4단계 - Controller 메서드 인자 매핑 by woo-yu · Pull Request #61 · next-step/java-mvc

woo-yu · 2024-08-19T10:19:12Z

안녕하세요 정완님!
리뷰 요청이 많이 늦어져서 죄송합니다 🙇‍♀️

마지막 스텝도 잘 부탁드리겠습니다!

- build.gradle에 컴파일 인수로 -parameters 추가

ghojeong

오랜만입니다 우혜진님!
끝까지 포기하지 않고 계속 진행해주셔서 감사합니다.
마지막 단계라서 세세하게 파고든 피드백을 남겨보았습니다.
만약 피드백을 반영하기에 지나치게 양이 많다 싶으면 과감하게 여기까지 미션 진행하겠다고 말씀주세요!

ghojeong · 2024-08-19T10:50:24Z

mvc/src/main/java/com/interface21/web/support/PathPatternUtil.java

+public class PathPatternUtil {
+


Suggested change

public class PathPatternUtil {

public final class PathPatternUtil {

private PathPatternUtil() {}

저는 유틸성 클래스의 경우 인스턴스의 생성이 불가능하도록,
생성자를 private 으로 선언해주는 것을 좋아합니다.

ghojeong · 2024-08-19T10:53:55Z

mvc/src/main/java/com/interface21/web/support/TypedParsers.java

+    public static void add(TypedParser... parsers) {
+        parserList.addAll(List.of(parsers));
+    }


요 메서드는 어디에 쓰이는지, 무슨 의도로 만든 것인지 궁금해서 여쭈어봅니다.
런타임 중에 TypedParser 가 추가되는 경우가 상상이 가지를 않아서요.

ghojeong · 2024-08-19T10:56:17Z

mvc/src/main/java/com/interface21/web/support/TypedParser.java

+public class TypedParser {
+    protected static final TypedParser STRING = new TypedParser(String.class, (s) -> s);
+    protected static final TypedParser INT = new TypedParser(int.class, Integer::parseInt);
+    protected static final TypedParser WRAPPER_INTEGER = new TypedParser(Integer.class, Integer::parseInt);
+    protected static final TypedParser LONG = new TypedParser(long.class, Long::parseLong);
+    protected static final TypedParser WRAPPER_LONG = new TypedParser(Long.class, Integer::parseInt);
+    protected static final TypedParser BOOLEAN = new TypedParser(boolean.class, Boolean::parseBoolean);
+    protected static final TypedParser WRAPPER_BOOLEAN = new TypedParser(Boolean.class, Boolean::parseBoolean);
+    protected static final TypedParser SHORT = new TypedParser(short.class, Short::parseShort);
+    protected static final TypedParser WRAPPER_SHORT = new TypedParser(Short.class, Short::parseShort);
+    protected static final TypedParser FLOAT = new TypedParser(float.class, Float::parseFloat);
+    protected static final TypedParser WRAPPER_FLOAT = new TypedParser(Float.class, Float::parseFloat);
+    protected static final TypedParser DOUBLE = new TypedParser(double.class, Double::parseDouble);
+    protected static final TypedParser WRAPPER_DOUBLE = new TypedParser(Double.class, Double::parseDouble);
+    protected static final TypedParser CHAR = new TypedParser(char.class,  s -> s.charAt(0));
+    protected static final TypedParser WRAPPER_CHAR = new TypedParser(Character.class,  s -> s.charAt(0));


TypedParser 내부에 선언된 static 상수들은 TypedParsers 에서만 쓰이는 듯한데,
굳이 상수명까지 하나하나 지어가며 protected 하게 공개를 할 필요가 있을지 궁금합니다.

TypedParsers 클래스 안에서 익명으로 선언할 수도 있을 것 같아서요.

ghojeong · 2024-08-19T10:59:23Z

mvc/src/main/java/com/interface21/web/support/TypedParsers.java

+    public static Object parse(final Class<?> type, final String value) {
+        return parserList.stream().filter(parser -> parser.canParse(type)).findFirst()
+                .orElseThrow(IllegalArgumentException::new)
+                .parse(value);
+    }


현재 TypeParser 를 찾을 때 List 에서 findFirst 를 하는데,
일급컬렉션을 활용하고 싶으셨다고 의도를 이해했습니다.
일급컬렉션 객체에 대한 테스트 코드 작성도 같이 해주시면 어떨까 제안해봅니다.

ghojeong · 2024-08-19T11:01:55Z

mvc/src/main/java/com/interface21/web/support/PathPatternParser.java

+    public boolean matches(String path) {
+        return pattern.matcher(path).matches();
+    }
+
+    public Map<String, String> extractUriVariables(String path) {
+        var matcher = pattern.matcher(path);
+        if (!matcher.matches()) {
+            return Collections.emptyMap();
+        }
+        Map<String, String> variables = new HashMap<>();
+        for (int i = 0; i < variableNames.size(); i++) {
+            variables.put(variableNames.get(i), matcher.group(i + 1));
+        }
+        return variables;
+    }


PathPatternUtil 에 대한 테스트는 작성해주셨지만, 정작 PathPatternParser 객체에 대해서는 테스트 작성해주시지 않았는데 보완 부탁드립니다.

ghojeong · 2024-08-19T12:30:41Z

mvc/src/main/java/com/interface21/web/parameter/ParameterParser.java

+public interface ParameterParser {
+    Object parse(final Method method, final Parameter parameter, final HttpServletRequest request);
+
+}


ParameterParser 를 인터페이스로 정의해주시고,
이에 대한 3가지 구현체를 만들어주신점 좋습니다. 👍
(ParameterTypedParser, PathVariableParser, QueryParamParser)

ghojeong · 2024-08-19T12:31:47Z

mvc/src/main/java/com/interface21/web/parameter/ParameterParsers.java

+public class ParameterParsers {
+
+    private final List<ParameterParser> parsers = new ArrayList<>(Arrays.asList(new PathVariableParser(), new QueryParamParser()));
+
+    public ParameterParsers() {
+
+    }
+
+    public ParameterParsers(ParameterParser... parsers) {
+        this.parsers.addAll(Arrays.asList(parsers));
+    }


ParameterParsers 일급컬렉션은 디폴트로 PathVariableParser 와 QueryParamParser 를 가지고,
파라미터로 넘겨받은 parsers 를 추가로 List 에 추가를 하는 군요

ghojeong · 2024-08-19T12:32:49Z

mvc/src/main/java/com/interface21/webmvc/servlet/DispatcherServlet.java

    public DispatcherServlet() {
-        requestHandlers.addHandlerMapping(new ControllerHandlerMapping(BASE_PACKAGE));
-        exceptionHandlers.addHandlerMapping(new ControllerAdviceHandlerMapping(BASE_PACKAGE));
+        var parameterParsers = new ParameterParsers(new PathVariableParser(), new QueryParamParser());


이미 디폴트로 PathVariableParser 와 QueryParamParser 를
ParameterParsers 가 가지고 있는데,
다시 파라미터로 넘겨주어서 addAll 을 굳이 하는 이유가 무엇인가요?

ghojeong · 2024-08-19T12:37:31Z

mvc/src/test/java/com/interface21/webmvc/servlet/TestUserControllerTest.java

+
+    private ControllerHandlerMapping handlerMapping;
+    private HttpRequestHandlers handlers = new HttpRequestHandlers();
+    private ParameterParsers parsers = new ParameterParsers(new ParameterTypedParser(TestUser.class));


ParameterTypedParser 의 경우 사용법이,
어플리케이션 개발자가 자신이 만든 클래스 정보를 ParameterTypedParser 인스턴스로 만들어서 ParameterParsers 에 넘겨주어야한다고 이해했습니다.

어플리케이션 개발자에게 프레임워크의 조작을 지나치게 많이 맡기는 것은 아닌가 하는 생각이 듭니다.

ghojeong · 2024-08-19T12:41:26Z

mvc/src/test/java/samples/TestUserController.java

+    @RequestMapping(value = "/users/bean", method = RequestMethod.POST)
    public ModelAndView create_javabean(TestUser testUser) {


컨트롤러 메서드의 파라미터 타입 클래스 정보를 직접 읽어와서,
어플리케이션 개발자가 아닌 프레임워크 차원에서 new ParameterTypedParser(TestUser.class) 인스턴스를 만들 수 있도록 부탁드립니다.
현재는 프레임워크에서 ParameterTypedParser 인스턴스가 생성되고 등록되는 로직이 미구현되어 있다는 느낌을 받았습니다.

woo-yu added 15 commits August 5, 2024 22:27

[docs] step3의 todo check

30ef3e5

[docs] step4 todo

36b39f5

[refactor] servlet/mvc/support 에서 web/support로 package 이동

758a99c

[feat] pathPattern util/parser 추가

ef782d2

[feat] 메서드 인자 이름 찾기위한 설정 추가

477d673

- build.gradle에 컴파일 인수로 -parameters 추가

[docs] readme에 todo 정리

320f09d

[feat] query param parser

fa68f4f

[feat] path variable parser

1f515cc

[feat] typed parser

1bf4d48

[feat] parameterTypedParser

6f4c02a

[feat] 기본 parser으로 queryparam, pathvariable parser 등록

6c2ebc9

[fix] HandlerKey에 path 일치가 아닌 uri pattern match로도 검증가능하도록 변경

d1ebe40

[test] TestUserController 검증

6bff222

[fix] null일 경우 TypedParser를 거치지 않도록 변경

3845f67

[feat] IllegalArugmentException 추가

125fbff

ghojeong requested changes Aug 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 4단계 - Controller 메서드 인자 매핑#61

🚀 4단계 - Controller 메서드 인자 매핑#61
woo-yu wants to merge 15 commits intonext-step:woo-yufrom
woo-yu:step4

woo-yu commented Aug 19, 2024

Uh oh!

ghojeong left a comment

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

ghojeong Aug 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	public class PathPatternUtil {
	public final class PathPatternUtil {
	private PathPatternUtil() {}

		@RequestMapping(value = "/users/bean", method = RequestMethod.POST)
		public ModelAndView create_javabean(TestUser testUser) {

Conversation

woo-yu commented Aug 19, 2024

Uh oh!

ghojeong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants