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

[BE] Refactor/#563 RestDocs 응답 오류(500) 해결 및 상태코드 검증 추가 #565

Merged
merged 10 commits into from
Oct 13, 2023

Conversation

yoondgu
Copy link
Collaborator

@yoondgu yoondgu commented Oct 9, 2023

작업 대상

모든 RestDocs 테스트 코드

📄 작업 내용

기존 명세서에 올라와있던 500 응답이 나오는 API들

그 외

  • 모든 테스트 코드에 상태 코드 검증 조건을 추가했습니다. (.andExpect()) 기존에 이 조건이 없었어서 상태코드가 500이어도 테스트가 통과하는 문제가 있었습니다. RestDocs 테스트 추가 작성 시 참고 부탁드립니다.
  • 이 과정에서 명세서에는 올려놓지 않았더라도 실패가 확인된 테스트들을 모두 수정했습니다.
    • 이제 이미지 명세도 RestDocs 명세서에서 확인 가능합니다.
    • LoginControllerTest의 실패 원인은 MockMvcTest 상의 쿠키 설정이었습니다. (해당 커밋 바디 참조)
  • ControllerTest에서 관리자 인증 방식 내용을 반영하여, 관리자 API 명세서를 다시 활성화했습니다.
  • 내부 로직이 불완전해 사용되면 안되는 API들을 adoc만 삭제했습니다. (Topic 삭제, Pin 삭제)

🙋🏻 주의 사항

스크린샷

📎 관련 이슈

closed #563

레퍼런스

@yoondgu yoondgu self-assigned this Oct 9, 2023
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

Unit Test Results

  68 files    68 suites   25s ⏱️
301 tests 301 ✔️ 0 💤 0
310 runs  310 ✔️ 0 💤 0

Results for commit 5336989.

♻️ This comment has been updated with latest results.

- MockMvcTest에서는 Cookie를 .header로 담을 수 없다
- Cookie는 US-ASCII chars만 허용한다
@yoondgu yoondgu marked this pull request as ready for review October 9, 2023 17:15
@yoondgu yoondgu added bug Something isn't working BE 백엔드 관련 이슈 우선순위 : 중 refactor 리팩토링 관련 이슈 labels Oct 9, 2023
Copy link
Collaborator

@kpeel5839 kpeel5839 left a comment

Choose a reason for hiding this comment

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

도이 너무 고생하셨습니다! 해결사 또이또이 멋집니다.

그리고, 코멘트는 궁금증에 질문 하나 남겼습니다! 대답해주시면 너무 감사할 것 같습니다.

Comment on lines +149 to +151
MockMultipartFile requestJson = new MockMultipartFile("pinId", "pinId", APPLICATION_JSON_VALUE,
objectMapper.writeValueAsBytes(1));
MockMultipartFile image = new MockMultipartFile("image", "test.png", IMAGE_PNG_VALUE, "data".getBytes());
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게나 깔끔하게 해결해주시다니 멋집니다!

@@ -65,6 +65,7 @@ public ResponseEntity<Void> update(
.build();
}

@Deprecated(since = "2023.10.10 (이미지 삭제 로직 불완전, 사용되지 않는 API)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

최근들어 Deprecated 를 아주 야무지게 쓰시는군요 멋집니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

코드 나중에 보면 힘들거가타서여.. 근데 이것도 부채네요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

그래도 이렇게 붙여주신 덕분에 추후에 볼 때 편할 것 같네요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

🪭

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🪭 이거 무슨 이모지져? 저는 안보임..
image

).andDo(restDocs.document());
mockMvc.perform(MockMvcRequestBuilders.get("/admin/members/1")
.header(AUTHORIZATION, "testKey"))
.andExpect(MockMvcResultMatchers.status().isOk())
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

// === 핀 이미지 삭제
//
// operation::admin-controller-test/delete-pin-image[snippets='http-request,http-response']
== 관리자 기능
Copy link
Collaborator

Choose a reason for hiding this comment

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

관리자 기능 rest docs 를 재활성화 시키신 이유는 어짜피 Authorization 이 외부로 드러나 있지 않으니 안전하다고 판단하셔서인가요?

그냥 궁금해서 여쭤봅니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 맞습니다, 그리고 그 외에 지웠던 이유가 잘 기억이 안나서 편의상 다시 추가해보았는데 그럼에도 노출하지 않는게 좋다고 생각하시면 다시 되돌리겠습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

근데 그 때 잠깐 주석처리 했던 이유가, 굳이 admin api 명세를 모두가 볼 수 있도록 공개해야하나? 라는 가벼운 이유 였던 것 같아서 괜찮을 것 같긴합니다.

무엇보다 secret key 가 없다면, admin api 를 사용할 수 없기도 하구요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

근데... 집에 벌써 가셨나요? 전광석화 또또이..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저는 깃허브 모바일 애용자입니다 ^_^

Copy link
Collaborator

Choose a reason for hiding this comment

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

킹쩔TV

Copy link
Collaborator

@cpot5620 cpot5620 left a comment

Choose a reason for hiding this comment

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

확인이 늦었네요 ! 고생하셨습니다 도이

@@ -65,6 +65,7 @@ public ResponseEntity<Void> update(
.build();
}

@Deprecated(since = "2023.10.10 (이미지 삭제 로직 불완전, 사용되지 않는 API)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

🪭

@@ -74,7 +75,7 @@ class AdminControllerTest extends RestDocsIntegration {
private AdminAuthInterceptor adminAuthInterceptor;

@BeforeEach
void setAll() throws Exception {
void setAll() {
given(adminAuthInterceptor.preHandle(any(), any(), any())).willReturn(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿

Copy link
Collaborator

@junpakPark junpakPark left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@yoondgu yoondgu merged commit 03742b7 into develop-BE Oct 13, 2023
3 checks passed
@semnil5202 semnil5202 deleted the refactor/#563 branch February 9, 2024 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 관련 이슈 bug Something isn't working refactor 리팩토링 관련 이슈 우선순위 : 중
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants