[MVC 구현하기 - 2, 3단계] 포츈(정윤성) 미션 제출합니다. #97

unluckyjung · 2021-09-16T18:01:23Z

이번 데모데이 발표자라서, 미션이 조금 후순위로 밀렸던지라 이제야 PR을 보내네요!

2단계 요구사항에서 조금 부족했던
HandlerAdapter 인터페이스 를 이용해 추상화를 하는 작업을 진행했구요.
~~하지만 ControllerAdapter는 step3 하면서 삭제 당해버린..~~

3단계 요구사항인 JoinView를 구현했어요.

model에 데이터가 1개면 값을 그대로 반환하고 2개 이상이면 Map 형태 그대로 JSON으로 변환해서 반환한다.
를 확인할 수 있는 url은 아래에 적어두었습니다.

이번에도 역시, 요구사항을 충족하는것을 최우선적인 목표로 잡았습니다.

인비도 추석 잘보내요!

unluckyjung · 2021-09-16T18:02:39Z

app/src/main/java/com/techcourse/controller/LoginController.java

+    @RequestMapping(value = "/login/view", method = RequestMethod.GET)
+    public ModelAndView view(HttpServletRequest req, HttpServletResponse res) {
+        return UserSession.getUserFrom(req.getSession())
+            .map(user -> {
+                LOGGER.info("logged in {}", user.getAccount());
+                View view = new JspView("redirect:/index.jsp");
+                return new ModelAndView(view);
+            })
+            .orElse(new ModelAndView(new JspView("/login.jsp")));
+    }
+
+    @RequestMapping(value = "/login", method = RequestMethod.POST)
+    public ModelAndView login(HttpServletRequest req, HttpServletResponse res) {
        if (UserSession.isLoggedIn(req.getSession())) {
-            return "redirect:/index.jsp";
+            View view = new JspView("redirect:/index.jsp");
+            return new ModelAndView(view);
        }


/login 과 /login/view 는
prefix url이 같으니, 하나의 컨트롤러로 합쳤어요.

unluckyjung · 2021-09-16T18:05:26Z

app/src/main/java/com/techcourse/controller/MultiInputController.java

+@Controller
+public class MultiInputController {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(MultiInputController.class);
+
+    @RequestMapping(value = "/api/multi", method = RequestMethod.GET)
+    public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {
+        String input1 = request.getParameter("input1");
+        String input2 = request.getParameter("input2");
+
+        LOGGER.debug("input1 : {}", input1);
+        LOGGER.debug("input2 : {}", input2);
+
+        ModelAndView modelAndView = new ModelAndView(new JsonView());
+
+        modelAndView.addObject("input1", input1);
+        modelAndView.addObject("input2", input2);
+        return modelAndView;
+    }
+}


2개 입력에 대한 테스트가 용이하도록, 컨트롤러를 추가해주었어요.

unluckyjung · 2021-09-16T18:06:36Z

mvc/src/main/java/nextstep/mvc/view/JsonView.java

+    ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+
    @Override
    public void render(Map<String, ?> model, HttpServletRequest request, HttpServletResponse response) throws Exception {
+        response.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);
+        PrintWriter writer = response.getWriter();
+
+        if (model.size() == 1) {
+            writer.print(OBJECT_MAPPER.writeValueAsString(model.values().toArray()[0]));
+            return;
+        }
+        writer.print(OBJECT_MAPPER.writeValueAsString(model));
    }
 }


1개 입력, 2개 이상의 입력을 따로 처리하도록 JsonView를 구현했어요.

unluckyjung · 2021-09-16T18:08:54Z

mvc/src/main/java/nextstep/mvc/controller/adapter/HandlerExecutionHandlerAdapter.java

+public class HandlerExecutionHandlerAdapter implements HandlerAdapter {
+
+    @Override
+    public boolean supports(final Object handler) {
+        return handler instanceof HandlerExecution;
+    }
+
+    @Override
+    public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response, final Object handler) throws Exception {
+        return ((HandlerExecution) handler).handle(request, response);
+    }
+}


기존 if / else 로 분기 처리되던것을 어뎁터패턴을 이용해 개선했어요.

~~이전 레거시 컨트롤러는 삭제당해버렸지만...~~

unluckyjung · 2021-09-16T18:09:50Z

mvc/src/test/java/nextstep/mvc/controller/ControllerScannerTest.java

-}
+}


인비가 이야기 한것이 요것인것을 나중에서야 알았습니다 ㅋ_ㅋ..

taehee-kim-dev

안녕하세요, 포츈!!

2, 3단계 구현 잘 해주셨네요. 💯

몇 가지 리뷰 남겼으니 참고 부탁드릴게요.

저도 이따가 데모발표를 하는데 포츈도 하시는군요.. ㅎㅎ
같이 화이팅 해요!! 💪 💪

테스트코드를 더 추가하면 좋을 것 같은데요, 이 부분은 포츈의 선택으로 남겨둘게요. 😉

바쁘신 와중에 미션 하시느라 고생 많으셨습니다!!
데모발표 화이팅 하시고, 추석 잘 보내세요!!!

taehee-kim-dev · 2021-09-16T21:37:59Z

app/src/main/java/com/techcourse/controller/ForwardController.java

+
+    @RequestMapping(value = "/", method = RequestMethod.GET)
+    public ModelAndView execute(HttpServletRequest req, HttpServletResponse res) throws Exception {
+        return new ModelAndView(new JspView("/index.jsp"));


"/index.jsp" 등의 View name들이 여러 컨트롤러에 반복적으로 쓰이고 있어요.

mvc 모듈의 nextstep.web.support 패키지의 MediaType.class처럼,
별도의 클래스 상수용 클래스를 만들어 View name을 관리하면 어떨까요?

taehee-kim-dev · 2021-09-16T21:47:29Z

app/src/main/java/com/techcourse/controller/LoginController.java

+    @RequestMapping(value = "/login/view", method = RequestMethod.GET)
+    public ModelAndView view(HttpServletRequest req, HttpServletResponse res) {
+        return UserSession.getUserFrom(req.getSession())
+            .map(user -> {
+                LOGGER.info("logged in {}", user.getAccount());
+                View view = new JspView("redirect:/index.jsp");
+                return new ModelAndView(view);
+            })
+            .orElse(new ModelAndView(new JspView("/login.jsp")));
+    }


아래의 POST /login요청에서 쓰고있는

if (UserSession.isLoggedIn(req.getSession())) { View view = new JspView("redirect:/index.jsp"); return new ModelAndView(view); }

를 사용한다면, 가독성이 좋아지고 통일성이 생길 것 같아요!

taehee-kim-dev · 2021-09-16T21:52:52Z

app/src/main/java/com/techcourse/controller/RegisterController.java

@@ -13,13 +13,14 @@

 @Controller
 public class RegisterController {
-


이미 로그인이 된 사용자라면, GET /register/ivew, POST /register 요청을 했을 때 /index.jsp 로 redirect 하도록 할 수도 있을 것 같아요.

LoginController와 TestController는 메서드가 GET, POST 순으로 있는데, 여기에는 POST, GET 순으로 되어있네요.
여기도 GET, POST 순으로 통일시키면 어떨까요?

taehee-kim-dev · 2021-09-16T21:58:17Z

app/src/main/java/com/techcourse/controller/UserController.java

+@Controller
+public class UserController {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(UserController.class);
+
+    @RequestMapping(value = "/api/user", method = RequestMethod.GET)
+    public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {
+        final String account = request.getParameter("account");
+        LOGGER.debug("user id : {}", account);
+
+        final ModelAndView modelAndView = new ModelAndView(new JsonView());
+        final User user = InMemoryUserRepository.findByAccount(account)
+            .orElseThrow();
+
+        modelAndView.addObject("user", user);
+        return modelAndView;
+    }
+}


final ModelAndView modelAndView = new ModelAndView(new JsonView()); modelAndView.addObject("user", user); return modelAndView;

이렇게 ModelAndView 사용 부분을 모아두면 가독성이 더 좋아질 것 같아요.

final User user = InMemoryUserRepository.findByAccount(account) .orElseThrow();

의 orElseThrow();에 특정 Exception을 메세지를 포함해 던지도록 하면 에러 핸들링, 디버깅 등에 더 유리할 것 같아요. :)

제가 작성한 코드가 아니고, 요구조건에서 주어진 class라서 그냥 곧이 그대로 가져왔었네요. ㅎㅎ;;

수정 하도록할게용

taehee-kim-dev · 2021-09-16T22:05:31Z

app/src/main/java/com/techcourse/domain/UserSession.java

+    private UserSession() {
+    }


private 생성자의 접근제어자가 private 이다 보니까
자동포맷팅 시에 public 메서드를 위에, private 메서드를 아래에 두면서 이렇게 밑으로 내려가더라구요.

생성자니까 멤버 변수 아래, 메서드 위에 두는게 어떨까요?

이부분은 자동 포매팅 때문에 종종 놓치게 되네요 ㅋㅋ;

taehee-kim-dev · 2021-09-16T22:09:32Z

mvc/src/main/java/nextstep/mvc/view/JsonView.java

 public class JsonView implements View {

+    ObjectMapper OBJECT_MAPPER = new ObjectMapper();


앞에 private static final이 필요할 것 같아요!!

헛, 넣은줄알고 있었는데 새벽에 졸면서 코딩하다보니 뺴먹었나 봅니다 ㅠㅠ

taehee-kim-dev · 2021-09-16T22:15:25Z

mvc/src/main/java/nextstep/mvc/DispatcherServlet.java

    private final List<HandlerMapping> handlerMappings;
+    private final List<HandlerAdapter> handlerAdapters;

    public DispatcherServlet() {
-        this.handlerMappings = new ArrayList<>();
+        handlerMappings = new ArrayList<>();
+        handlerAdapters = new ArrayList<>();
    }


handlerMappings와 handlerAdapters를 각각 일급컬렉션으로 빼면 어떨까요?

그러면 객체의 역할과 책임이 분리되면서
service() 메서드의 indent 3을 indent 1까지 없앨 수 있고,
getHandlerMapping() 메서드도 HandlerMappings 일급 컬렉션 안으로 숨길 수 있을 것 같아요!

sonarqubecloud · 2021-09-20T09:59:54Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

unluckyjung

디엠으로 이야기한내용 제외 + 인비가 준 피드백을 적용해보았어요.
휴일이시니 천천히 리뷰해주셔도 정말 괜찮습니다~

명절 잘보내세요 즐추!

unluckyjung · 2021-09-20T10:02:27Z

mvc/src/main/java/nextstep/web/support/JspPage.java

+package nextstep.web.support;
+
+public enum JspPage {
+    INDEX("/index.jsp"),
+    LOGIN("/login.jsp"),
+    REGISTER("/register.jsp"),
+    UNAUTHORIZED("/401.jsp"),
+    GET_TEST("/get-test.jsp"),
+    POST_TEST("/post-test.jsp");
+
+    private final String page;
+
+    JspPage(final String page) {
+        this.page = page;
+    }
+
+    public String value() {
+        return page;
+    }
+
+    public String redirect() {
+        return String.format("redirect:%s", page);
+    }
+}


#97 (comment)

반복적으로 쓰이고 있는, 값들을 모아 Enum에서 처리하도록 했어요.

unluckyjung · 2021-09-20T10:03:15Z

app/src/main/java/com/techcourse/domain/UserSession.java

+    private UserSession() {
+    }
+


생성자 위치 수정완료!

unluckyjung · 2021-09-20T10:03:52Z

mvc/src/main/java/nextstep/mvc/view/JsonView.java

+    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+


이런 기본적인 실수를 했었다니 ㅎㅎ;

unluckyjung · 2021-09-20T10:05:24Z

mvc/src/main/java/nextstep/mvc/controller/adapter/HandlerAdapters.java

+    public HandlerAdapter getHandlerAdapter(final Object handler) {
+        return handlerAdapters.stream()
+            .filter(handlerAdapter -> handlerAdapter.supports(handler))
+            .findFirst()
+            .orElseThrow(() -> new IllegalArgumentException("맞는 핸들러가 없습니다."));
+    }


HandlerAdapters 를 일급컬렉션화하고, DisPatcherServlet에서 하고 있던 많은 작업을 이쪽으로 이관시켰어요.

unluckyjung · 2021-09-20T10:05:39Z

mvc/src/main/java/nextstep/mvc/controller/handler/HandlerMappings.java

+import java.util.List;
+import java.util.Objects;
+
+public class HandlerMappings {


HandlerMappings역시 일급컬렉션화 했습니다.

taehee-kim-dev

안녕하세요, 포츈!!!

피드백 반영해주신 부분 확인했습니다 ㅎㅎ
명절에도 빠르게 반영해주셔서 감사해요!!
미션 요구사항을 모두 만족시키셨다고 판단되어 이만 머지 할게요!!

남은 미션도 화이팅하시고, 명절 잘 보내세요!!
고생 많으셨습니다. 🙇‍♂️

unluckyjung added 6 commits September 16, 2021 23:53

refactor: 레거시 Controller에서 String을 반환하던것을 ModelView를 반환하도록 수정

2fa612d

refactor: 어댑터 패턴을 이용해 프레임워크 지원 기능 리팩토링

db67d7b

chore: jackson 라이브러리 버전업

98ca581

feat: jsonView 구현

10580b1

refactor: 모든 컨트롤러를 어노테이션 기반 MVC로 변경

3cafedb

refactor: Legacy MVC 제거

50f304b

unluckyjung requested a review from taehee-kim-dev September 16, 2021 18:01

unluckyjung self-assigned this Sep 16, 2021

refactor: 코드 포매팅

834708f

unluckyjung force-pushed the step2,3 branch from 98bbd3d to 834708f Compare September 16, 2021 18:04

unluckyjung commented Sep 16, 2021

View reviewed changes

taehee-kim-dev suggested changes Sep 16, 2021

View reviewed changes

unluckyjung added 8 commits September 20, 2021 17:36

refactor: 중복되는 Viewname을 Enum에서 관리하도록 수정

14eb85c

refactor: GET, POST 순으로 메서드 위치 수정

5faa6c5

refactor: 생성자 위치 변경

a847513

refactor: 빠져있던 접근제어자 추가

53ead64

refactor: HandlerMappings 일급컬렉션화

5ffc626

refactor: HandlerAdapters 일급 컬렉션화

9a1dd20

refactor: 예외 메시지 추가

e1c19b3

refactor: 리다이랙팅도 Enum에서 처리하도록 수정

a4caf77

woowacourse deleted a comment from sonarqubecloud bot Sep 20, 2021

unluckyjung commented Sep 20, 2021

View reviewed changes

taehee-kim-dev approved these changes Sep 20, 2021

View reviewed changes

taehee-kim-dev merged commit 4c3d56d into woowacourse:unluckyjung Sep 20, 2021

		@@ -13,13 +13,14 @@

		@Controller
		public class RegisterController {

		public class JsonView implements View {

		ObjectMapper OBJECT_MAPPER = new ObjectMapper();

		private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

		} No newline at end of file
		}

[MVC 구현하기 - 2, 3단계] 포츈(정윤성) 미션 제출합니다. #97

[MVC 구현하기 - 2, 3단계] 포츈(정윤성) 미션 제출합니다. #97

Uh oh!

Conversation

unluckyjung commented Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

taehee-kim-dev left a comment

Choose a reason for hiding this comment

안녕하세요, 포츈!!

Uh oh!

taehee-kim-dev Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

unluckyjung Sep 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unluckyjung Sep 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taehee-kim-dev Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

sonarqubecloud bot commented Sep 20, 2021

Uh oh!

unluckyjung 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!

taehee-kim-dev left a comment

Choose a reason for hiding this comment

안녕하세요, 포츈!!!

Uh oh!

Uh oh!

unluckyjung commented Sep 16, 2021 •

edited

Loading

taehee-kim-dev Sep 16, 2021 •

edited

Loading

unluckyjung Sep 17, 2021 •

edited

Loading

unluckyjung Sep 17, 2021 •

edited

Loading

taehee-kim-dev Sep 16, 2021 •

edited

Loading