Skip to content

Conversation

geoje
Copy link
Member

@geoje geoje commented Sep 30, 2024

안녕하세요 미아,

드디어 마지막 미션 제출이네요~
이번 구현 사항에 대한 주요 항목들은 아래와 같습니다! 🤩

1. JsonView 클래스를 구현한다.

  • JsonMapper 사용
    다른 기능들을 많이 사용하지 않으니 ObjectMapper 를 상속하고 있는 구체화 클래스인 JsonMapper 를 사용하여 객체와 JSON 간의 매핑만 딱 처리할 수 있도록 하였습니다.

  • 브라우저로 API 요청
    /api/user 접속 시 아래와 같이 JSON 을 보여주게 됩니다.
    image

  • ArgumentHandlerExceptionResolver 생성
    DispatcherServlet.service() 동작 중 예외가 발생할 경우 기본적으로 예외를 해결할 수 있는 클래스를 생성하였습니다.
    UserControllerTest 에 전부 작성해두었지만 존재하지 않는 계정 정보로 브라우저에서 요청 시 아래와 같이 보여지게 됩니다.
    image
    어라? API 요청이니 응답도 JSON으로 했어야 했나...

2. Legacy MVC 제거하기

  • 구조화
    기존 미션 진행을 위해 필요했던 asistobe 패키지를 정리하여 아래와 같은 구조로 변경하였습니다.
    또한 DispatcherServletapp 패키지에서 mvc 패키지로 넘어오게 되어 더욱 짜임새 있는 구조가 되었네요!
    image

  • 공존 그리고 점진적 제거
    LegacyFeature 가 공존하는 코드에서 점직적으로 Legacy의 비중으로 낮춰가며 완전 삭제까지 완료하였습니다.
    또한 각 커밋 시점 마다 API 요청 테스트(app 패키지에도 test 생성)를 계속 진행하였습니다.

  1. geoje@f30adee ManualHandlerMapping 에서 Mapput 코드 제거와 동시에 각 컨트롤러에 @Controller@RequestMapping 추가
  2. geoje@6ea97f2 ManualHandlerMapping 제거
  3. geoje@b245f11 프레임워크의 legacy asis 제거
  4. geoje@995c6ad 패키지 대거 변경

🥳🎉

Copy link

@jongmee jongmee left a comment

Choose a reason for hiding this comment

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

안녕하세요 새양! pr description과 코드 모두 너무 꼼꼼하네요 ☺️ 덕분에 리뷰하기 수월했어요! 이번에는 argument resolver까지 만들어주셨군요 💯

아주 작은 리뷰 하나 때문에 일단 다시 RC 드려요!! 고생하셨습니다 😊


@BeforeEach
void setUp() {
tomcat = new TomcatStarter("src/main/webapp/", 8080);
Copy link

Choose a reason for hiding this comment

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

저는 익숙한 Rest assured를 사용했는데 추가 의존성을 사용하지 않고 TomcatStarter와 HttpClient를 사용해주셨네요! 💯 배워갑니다 👍

}

@Test
@DisplayName("존재하지 않는 계정으로 요청 시 401 페이지로 리디렉션을 응답한다.")
Copy link

Choose a reason for hiding this comment

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

역시 디테일한 테스트케이스 👍

String json = jsonMapper.writeValueAsString(model);

response.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);
response.getWriter().write(json);
Copy link

Choose a reason for hiding this comment

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

model에 데이터가 1개면 값을 그대로 반환하고 2개 이상이면 Map 형태 그대로 JSON으로 변환해서 반환한다.

이 부분만 추가되면 완벽할 것 같네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

geoje@ac51700

넵! key 를 반환하지 않는게 마음에 들지 않아 요구사항을 무시했는데 딱 걸렸군요 😅
해당 사항 반영 하였습니다!!

Copy link

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋ 그런거였군요 저도 키가 왜 없어야 하는지는.. 모르겠네요 🤔 굿 👍

Copy link

@jongmee jongmee left a comment

Choose a reason for hiding this comment

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

고생하셨어요 새양!!! 새양의 코드를 리뷰할 수 있어서 즐거웠습니다 😊 😊

다음 미션도 화이팅👍

String json = jsonMapper.writeValueAsString(model);

response.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);
response.getWriter().write(json);
Copy link

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋ 그런거였군요 저도 키가 왜 없어야 하는지는.. 모르겠네요 🤔 굿 👍

@jongmee jongmee merged commit 5d622fb into woowacourse:geoje Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants