[MVC 구현하기 - 2단계] 메리(최승원) 미션 제출합니다. #518

swonny · 2023-09-20T17:08:40Z

안녕하세요 에단!
메리입니다.
남겨주신 리뷰 반영하고 2단계 구현하고 리뷰 요청합니다!
리뷰 잘 부탁드리겠습니다.

감사합니다!!

- study 추가

* test: 파일 테스트 통과 * test: IOStream 학습 테스트 * feat: index.html 응답하기 구현 * feat: CSS 지원하기 구현 * feat: login.html 응답 구현 * refactor: 로그인 쿼리 스트링 구분 * feat: 로그인 후 리다이렉트 기능 구현 * refactor: 로그인 컨트롤러, view resolver 생성 * feat: register html 응답 구현

* feat: index.html 응답하기 구현 * feat: CSS 지원하기 구현 * feat: login.html 응답 구현 * refactor: 로그인 쿼리 스트링 구분 * feat: 로그인 후 리다이렉트 기능 구현 * refactor: 로그인 컨트롤러, view resolver 생성 * feat: register html 응답 구현 * fix: body 전송 구현 * refactor: HttpRequest 객체 생성 및 파싱 기능 분리 * feat: 유저 등록 기능 구현 * feat: 로그인 메소드 POST로 변경 * refactor: 회원가입 성공 시 redirect로 응답하도록 변경 * feat: 쿠키 생성 및 로그인 시 쿠키에 세션 저장 * feat: session을 통해 로그인 페이지 redirect하는 기능 구현 * refactor: 패키지 이동 * refactor: 불필요한 필드 삭제 * refactor: 로그 포맷 변경 * refactor: 컨트롤러 핸들러 생성 * refactor: 불필요한 enum 필드 삭제 및 메소드 분리 * refactor: 예외 처리 분기문 추가 * refactor: 파일 경로 조회 메서드 분리 * refactor: GET, POST 요청이 아닌 경우 예외 반환 * fix: 잘못된 유저 정보 입력 시 401 리다이렉트 되지 않는 문제 해결 * refactor: session을 포함한 redirect 응답값 생성 메소드 분리 * refactor: 확장자 검증 메서드 Extension으로 이동 * refactor: 메서드 순서 변경 * refactor: cookie 존재 여부 확인 분기문 getOrDefault로 변경 * refactor: FileController 정팩메 ControllerHandler에 등록 * refactor: HttpRequest에 해당하는 request line, headers, body 클래스 생성 * refactor: controller 인스턴스 저장 방식 변경 * fix: CRLF 읽는 불필요한 로직 삭제 * style: 메서드 순서 재정렬 및 공백 제거 및 네이밍 변경 * refactor: HttpResponse 클래스 생성 및 response 생성 로직 분리 * refactor: 불필요한 메서드, 필드 삭제 * fix: 실패하는 테스트 수정 * fix: 잘못된 타입 수정 * refactor: Controller 인터페이스로 변경

* feat: index.html 응답하기 구현 * feat: CSS 지원하기 구현 * feat: login.html 응답 구현 * refactor: 로그인 쿼리 스트링 구분 * feat: 로그인 후 리다이렉트 기능 구현 * refactor: 로그인 컨트롤러, view resolver 생성 * feat: register html 응답 구현 * fix: body 전송 구현 * refactor: HttpRequest 객체 생성 및 파싱 기능 분리 * feat: 유저 등록 기능 구현 * feat: 로그인 메소드 POST로 변경 * refactor: 회원가입 성공 시 redirect로 응답하도록 변경 * feat: 쿠키 생성 및 로그인 시 쿠키에 세션 저장 * feat: session을 통해 로그인 페이지 redirect하는 기능 구현 * refactor: 패키지 이동 * refactor: 불필요한 필드 삭제 * refactor: 로그 포맷 변경 * refactor: 컨트롤러 핸들러 생성 * refactor: 불필요한 enum 필드 삭제 및 메소드 분리 * refactor: 예외 처리 분기문 추가 * refactor: 파일 경로 조회 메서드 분리 * refactor: GET, POST 요청이 아닌 경우 예외 반환 * fix: 잘못된 유저 정보 입력 시 401 리다이렉트 되지 않는 문제 해결 * refactor: session을 포함한 redirect 응답값 생성 메소드 분리 * refactor: 확장자 검증 메서드 Extension으로 이동 * refactor: 메서드 순서 변경 * refactor: cookie 존재 여부 확인 분기문 getOrDefault로 변경 * refactor: FileController 정팩메 ControllerHandler에 등록 * refactor: HttpRequest에 해당하는 request line, headers, body 클래스 생성 * refactor: controller 인스턴스 저장 방식 변경 * fix: CRLF 읽는 불필요한 로직 삭제 * style: 메서드 순서 재정렬 및 공백 제거 및 네이밍 변경 * refactor: HttpResponse 클래스 생성 및 response 생성 로직 분리 * refactor: 불필요한 메서드, 필드 삭제 * fix: 실패하는 테스트 수정 * fix: 잘못된 타입 수정 * refactor: Controller 인터페이스로 변경 * refactor: Controller 인터페이스 파라미터 변경, AbstractController 생성 * feat: ThreadPool 적용 및 동시성 컬렉션 사용 * style: 불필요한 개행 제거 * refactor: 상속한 클래스에서 재정의하는 메서드 abstract로 변경 * fix: cookie가 아닌 SESSION이 존재할 때 redirect 하도록 수정 * test: Response 문자열 순서가 다른 문제 해결을 위해 테스트 수정 * refactor: HttpResponse 필드 raw 타입에서 객체로 변경 * fix: 누락된 ResponseHeaders 커밋 추가

cookienc

안녕하세요 메리~ 코드를 깔끔하게 짜주셔서 이해하기 편했습니다.
이번에는 같이 얘기해보고 싶은게 있어서 Request Changes 남깁니다.
확인해주시고 다시 요청주세요~ 주말 잘 보내세요🙇‍♂️🙇

cookienc · 2023-09-22T02:18:52Z

app/src/main/java/com/techcourse/DispatcherServlet.java

-        manualHandlerMapping = new ManualHandlerMapping();
-        manualHandlerMapping.initialize();
+        handlerMappers.add(new ManualHandlerMapping());
+        handlerMappers.add(new AnnotationHandlerMapping("com.techcourse.controller"));


이건 사소한 부분인데요. 스프링에서 @controller가 Controller 패키지에 없어도 동작하던데, 패키지 경로를 자세히 쓰면 스프링처럼 동작하지 않을것 같아요!

반영하여 수정했습니다!

cookienc · 2023-09-22T03:34:43Z

app/src/main/java/com/techcourse/DispatcherServlet.java

+                      .orElseThrow(() -> new IllegalArgumentException("존재하지 않는 handler입니다."));
+    }
+
+    private Method findControllerExecution(final Class<?> controllerType) {


findControllerExecution 인데 HandlerExecution이 아니라 Method를 반환하고 있네요

이 부분도 메서드 네이밍과 반환값이 일치하도록 변경했습니다.

cookienc · 2023-09-22T04:20:59Z

app/src/main/java/com/techcourse/DispatcherServlet.java

+                     .filter(DispatcherServlet::isExecuteMethod)
+                     .findFirst()
+                     .orElseThrow(() -> new IllegalArgumentException("컨트롤러 실행 메서드가 존재하지 않습니다."));
+    }


controller가 object 클래스여서 HandlerExecution 과 Controller가 둘 다 반환될 수 있어 메서드를 직접 찾아서 반환하는걸로 이해했습니다. 이렇게 되면 요청이 들어올때마다 리플랙션을 사용해서 Method를 반환할것 같아요.
controllerType이 둘 중 하나니까 이 부분만 확실하면 타입 캐스팅을 통해서 바로 메서드 호출 할 수 있을 것 같은데 이 부분에 대해서는 어댑터 패턴을 학습하시면 해결하실 수 있을것 같습니다👍

감사합니다 에단!
남겨주신 리뷰 보고 힌트를 보고 어댑터 패턴을 적용해보았는데 확실히 구조가 훨씬 깔끔해진 것 같습니다!!
기존에 disapatcherServlet에서 리플렉션으로 실행 메서드를 찾는 로직이 HandlerMapping 구현체에 따라 찾는 로직으로 변경되면서 코드도 훨씬 깔끔해지고 dispatcherServlet의 책임도 적절히 분리가 된 것 같아요!🙇🏻‍♀️

이 부분도 반영하여 리팩토링 완료헀습니다.

cookienc · 2023-09-22T04:21:43Z

app/src/main/java/com/techcourse/ManualHandlerMapping.java

@@ -21,15 +26,15 @@ public void initialize() {
        controllers.put("/login/view", new LoginViewController());
        controllers.put("/logout", new LogoutController());
        controllers.put("/register/view", new RegisterViewController());
-        controllers.put("/register", new RegisterController());
+//        controllers.put("/register", new RegisterController());


주석이 들어갔어요~

cookienc · 2023-09-22T04:29:08Z

app/src/main/java/com/techcourse/ManualHandlerMapping.java

+    public Controller getHandler(final HttpServletRequest request) {
+        log.debug("Request Mapping Uri : {}", request);
+        return controllers.getOrDefault(request.getRequestURI(), null);


제가 저번 리뷰때 언급해서 바꾸신것 같은데 제가 답글을 너무 늦게 달았나보네용
Map에서 없는 key로 조회를 하면 예외가 아니라 null을 반환하더라구요~ 그래서 getOrDefalut는 따로 안써도 될 것 같습니다
다음 부터는 빨리 답변하도록 노력할게요..🙇‍♂️

그렇군요..!
저는 getOrDefault()를 말씀해주셔서
해당 메서드에서 null을 반환했을 때의 null 처리에 대해 조금 고민해보게 되었습니다!
말씀하신 것처럼 getOrDefault()를 하지 않아도 null을 반환하는 부분이 걸려서 리팩토링 과정에서 Optional.ofNullable()을 반환하도록 변경했습니다.

그러다가 HandlerMapping의 일급컬렉션인 HandlerMappingRegistry를 생성하면서 한 번 더 변경이 있었습니다.
HandlerMappingRegistry에서 HandlerMapping을 찾을 때 HandlerMapping 객체에게 요청에 매핑되는 HandlerMapping이 존재하는지 묻고, 존재한다면 해당 핸들러를 반환하는 검증 로직이 추가되어 getHandler()를 호출하는 순간에는 null이 반환될 일이 없어졌습니다. 따라서 반환값이었던 Optional을 Object로 다시 한번 변경해주었습니다!

남겨주신 리뷰 덕분에 놓치고 있던 부분에 대해 한 번 더 생각해 볼 수 있어 좋았어요 감사합니다!!

cookienc · 2023-09-22T04:29:25Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/tobe/AnnotationHandlerMapping.java

-    public Object getHandler(final HttpServletRequest request) {
-        return handlerExecutions.get(
+    public HandlerExecution getHandler(final HttpServletRequest request) {
+        return handlerExecutions.getOrDefault(


여기도 위와 마찬가지입니다!

cookienc · 2023-09-22T04:39:30Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/tobe/AnnotationHandlerMapping.java

+            Arrays.stream(controller.getMethods())
+                  .filter(method -> method.isAnnotationPresent(RequestMapping.class))
+                  .forEach(methods::add);


저는 foreach를 사용해서 외부에 영향을 끼치게 하는 편은 지양하는 편입니다. 왜냐하면 스트림은 함수형 프로그래밍을 위해 나왔는데 이를 위반한다고 생각해요. 주변 크루들을 보면 그럼에도 사용하시는 분들도 있고 나름의 이유도 존재하는데요. 메리는 어떤 입장이신가요?

저도 forEach의 사용이 외부로 영향을 미치면 안된다는 것에 동의합니다.
그리고 해당 로직이 프로그램이 복잡해지면 관리가 어려워질 것 같다고 생각했습니다!
따라서 초반에는 해당 로직을 메서드로 분리하여 리스트를 반환하는 메서드로 리팩토링 했다가,
현재는 해당 로직이 HandlerMappingRegistry로 이동한 상태입니다!

cookienc

메리~ 고생하셨습니다! 다음 단계로 넘어가시죠!

cookienc · 2023-09-24T06:49:07Z

app/src/main/java/com/techcourse/DispatcherServlet.java

+            final HandlerMapping handlerMapping = handlerMappingRegistry.getHandlerMapping(request);
+            final HandlerAdapter handlerAdapter = handlerAdapterRegistry.getHandlerAdapter(request, handlerMapping);
+            final ModelAndView view = handlerAdapter.handle(handlerMapping, request, response);
+            move(view, request, response);


Dispatcher 서블릿이 역할이 많이 줄었네요👍👍

cookienc · 2023-09-24T06:50:04Z

app/src/test/java/com/techcourse/HandlerAdapterRegistryTest.java

+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class HandlerAdapterRegistryTest {


오 테스트까지.. 대단하시네요!

cookienc · 2023-09-24T06:51:57Z

mvc/src/main/java/webmvc/org/springframework/web/servlet/mvc/tobe/AnnotationHandlerMapping.java

+            final List<Method> methods,
+            final Class<?> controller
+    ) {
+        final HashMap<HandlerKey, HandlerExecution> handlerExecutions = new HashMap<>();


Map으로 받을 수 있을거 같아요!

kang-hyungu and others added 30 commits September 11, 2023 15:29

패키지 위치 변경 및 코드 정리

52ee778

서블릿 학습 테스트 코드 개선

e7c21ab

test: Reflection API 학습 테스트 구현

69c80dd

test: 서블릿 학습테스트 구현

e3bfa93

test: 필터 학습테스트 구현

1a838a7

feat: 요청 값에 해당하는 handler 반환하는 기능 구현

1d36bb2

chore: 소나클라우드 관련 설정 추가

5b00504

first commit

79c9c4c

톰캣 구현하기

df51648

- 라이브러리 버전 업데이트

da5c772

- study 추가

thread 실습 추가

6ff649d

sonarcloud 설정 추가

04cb1b2

fix: 깃 충돌 해결

ac68d54

@MVC 구현하기

bd62f65

라이브러리 버전 정리 및 실습 코드 추가

3b9532d

패키지 위치 변경 및 코드 정리

a67ac5f

서블릿 학습 테스트 코드 개선

9fcf553

refactor: 여러 basePackage를 Reflections에 저장할 수 있도록 변경

38ad1ed

feat: handlerMapping 인터페이스 생성

79db98c

feat: HanlderMapping 구현체 초기화 기능 추가

5b1f8b4

style: 개행 추가

9458cbf

refactor: handlerMapping 인터페이스 구현 메서드 시그니처 수정

272696a

refactor: 분기문을 포함한 for문 stream으로 변경

5d940e5

feat: 3단계에서 구현할 부분 주석 추가 및 실행 시 예외 발생하지 않도록 처리

ddd80d5

refactor: 이중 for문 메서드 분리

aa50659

feat: 어노테이션 컨트롤러 생성

08017cf

Merge remote-tracking branch 'upstream/swonny' into step2

f284a79

cookienc requested changes Sep 22, 2023

View reviewed changes

swonny added 8 commits September 23, 2023 16:03

refactor: 스트림 내부에서 외부에 영향이 없도록 메서드 분리

a2ed81b

refactor: 메서드 분리

300e2b6

refactor: null이 포함될 수 있는 반환값 Optional로 포장

9630bd9

refactor: 반환값과 일치하는 네이밍으로 변경

3969b52

style: 주석 제거

7571c7a

refactor: 컨트롤러 패키지 범위 변경

dcf1ab3

refactor: HandlerMapping에서 Handler 존재 여부 판단 메서드 추가

2d4ac84

feat: HandlerMappings 일급컬렉션과 HandlerAdapter 추가

aa40d02

cookienc approved these changes Sep 24, 2023

View reviewed changes

cookienc merged commit 0fc2310 into woowacourse:swonny Sep 24, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MVC 구현하기 - 2단계] 메리(최승원) 미션 제출합니다. #518

[MVC 구현하기 - 2단계] 메리(최승원) 미션 제출합니다. #518

swonny commented Sep 20, 2023

cookienc left a comment

cookienc Sep 22, 2023

swonny Sep 23, 2023

cookienc Sep 22, 2023

swonny Sep 23, 2023

cookienc Sep 22, 2023

swonny Sep 23, 2023

cookienc Sep 22, 2023

cookienc Sep 22, 2023

swonny Sep 23, 2023

cookienc Sep 22, 2023

cookienc Sep 22, 2023

swonny Sep 23, 2023

cookienc left a comment

cookienc Sep 24, 2023

cookienc Sep 24, 2023

cookienc Sep 24, 2023

[MVC 구현하기 - 2단계] 메리(최승원) 미션 제출합니다. #518

[MVC 구현하기 - 2단계] 메리(최승원) 미션 제출합니다. #518

Conversation

swonny commented Sep 20, 2023

cookienc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cookienc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment