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

[refactor] : Chatbot 관련 리팩토링을 진행한다 #85

Merged
merged 9 commits into from
Nov 23, 2024

Conversation

bbbang105
Copy link
Member

@bbbang105 bbbang105 commented Nov 23, 2024

✅ PR 유형

어떤 변경 사항이 있었나요?

  • 새로운 기능 추가
  • 버그 수정
  • 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
  • 코드 리팩토링
  • 주석 추가 및 수정
  • 문서 수정
  • 빌드 부분 혹은 패키지 매니저 수정
  • 파일 혹은 폴더명 수정
  • 파일 혹은 폴더 삭제

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • 아래의 코드 리뷰 코멘트를 해결하고자 하였습니다.
  1. [A팀] 백엔드 파트 코드리뷰용 PR #44 (comment)
java 진영에서는 enum을 대문자로 표기하는게 컨벤션입니다!
  1. [A팀] 백엔드 파트 코드리뷰용 PR #44 (comment)
Java의 stream을 사용해봐도 좋을거 같아요
익숙해지면 매우 편합니다.
  1. [A팀] 백엔드 파트 코드리뷰용 PR #44 (comment)
Clova를 사용한다는게 chatbot 도메인에 중요한 정보일까요?
추후, 더 싸고 더 좋은 기술이 등장해서 clova 말고 다른것을 사용하려고 했을때 어디를 변경해야할까요?

위와 같은 상황일때 적절한 추상화로 infra layer만 변경할 수 있도록 만들면 더 좋을거 같아요.
  1. [A팀] 백엔드 파트 코드리뷰용 PR #44 (comment)
reactive 기술을 사용하셨는데요.
여기서 block을 하게되면 reactive의 장점을 누리지 못할 가능성이 큽니다.
(관련해서 reactive 방식의 프로그래밍을 했을때 왜 빨라지는지 알아보셔도 좋을거 같아요)

하지만, webClient는 편하기 때문에 비슷한 대안으로 RestClient라는게 있으며 FeignClient를 사용하시는것도 매우 편하기 때문에 추천드려요.

기존 코드

@Override
    public String requestChatbot(ChatbotRequest request) {
        if (!(request instanceof ClovaRequest clovaRequest)) {
            throw new CustomException(ChatbotErrorStatus._INVALID_CHATBOT_REQUEST);
        }

        try {
            ClovaChatbotAnswer clovaChatbotAnswer = webClient.post()
                    .bodyValue(clovaRequest)
                    .retrieve()
                    .bodyToMono(ClovaChatbotAnswer.class)
                    .block();

            if (clovaChatbotAnswer == null || clovaChatbotAnswer.result() == null || clovaChatbotAnswer.result().message() == null) {
                throw new CustomException(ChatbotErrorStatus._NOT_FOUND_GUIDE_CHATBOT_ANSWER);
            }

            return clovaChatbotAnswer.result().message().content();

        } catch (WebClientResponseException e) {
            // 외부 API 응답 처리 중 발생한 예외 처리
            throw new CustomException(ChatbotErrorStatus._CHATBOT_API_COMMUNICATION_ERROR);
        } catch (Exception e) {
            // 기타 예외 처리
            throw new CustomException(ChatbotErrorStatus._CHATBOT_API_COMMUNICATION_ERROR);
        }
    }

변경 코드

@Override
    public Mono<String> requestChatbot(ChatbotRequest request) {
        if (!(request instanceof ClovaRequest clovaRequest)) {
            return Mono.error(new CustomException(ChatbotErrorStatus._INVALID_CHATBOT_REQUEST));
        }

        return webClient.post()
                .bodyValue(clovaRequest)
                .retrieve()
                .bodyToMono(ClovaChatbotAnswer.class)
                .flatMap(clovaChatbotAnswer -> {
                    if (clovaChatbotAnswer == null || clovaChatbotAnswer.result() == null || clovaChatbotAnswer.result().message() == null) {
                        return Mono.error(new CustomException(ChatbotErrorStatus._NOT_FOUND_GUIDE_CHATBOT_ANSWER));
                    }
                    return Mono.just(clovaChatbotAnswer.result().message().content());
                })
                .onErrorMap(WebClientResponseException.class, e -> new CustomException(ChatbotErrorStatus._CHATBOT_API_COMMUNICATION_ERROR))
                .onErrorMap(Exception.class, e -> new CustomException(ChatbotErrorStatus._CHATBOT_API_COMMUNICATION_ERROR));
    }

기존에는 block을 사용했는데, 이는 비동기가 아닌 동기 처리로 응답이 올 때까지 기다린다고 합니다.
만약 1,000명의 유저가 챗봇을 동시 사용할 때, 1명의 유저를 처리하는 과정에서 처리가 늦어지면 비동기 처리에 비해 월등하게 속도가 느려진다는 단점이 존재합니다. 때문에 block을 제거하고 reactive 기술의 흐름을 살리는 방향으로 변경하였습니다.

참고 블로그

이 과정에서 테스트 코드 또한 일부 변경해야 했는데요.

기존 코드

        // when
        ResultActions resultActions = this.mockMvc.perform(RestDocumentationRequestBuilders.post("/api/v1/chatbot/clova")
                .content(clovaChatbotAnswerJsonRequest)
                .contentType(MediaType.APPLICATION_JSON)
                .accept(MediaType.APPLICATION_JSON));

        // then
        resultActions
                .andExpect(status().isOk())
                .andExpect(jsonPath("$.isSuccess").value(true))
                .andExpect(jsonPath("$.code").value("200"))
                .andExpect(jsonPath("$.message").value("네이버 클로바 챗봇 답변을 가져오는 데 성공했습니다."))
                .andExpect(jsonPath("$.payload.answer").value("안녕하세요! 저는 야구 가이드 챗봇 '루키'에요! 야구에 대한 궁금한 점이 있다면 언제든지 물어봐 주세요!"))

변경 코드

        // when
        MvcResult mvcResult = this.mockMvc.perform(
                        RestDocumentationRequestBuilders.post("/api/v1/chatbot/clova")
                                .content(clovaChatbotAnswerJsonRequest)
                                .contentType(MediaType.APPLICATION_JSON)
                                .accept(MediaType.APPLICATION_JSON))
                .andExpect(request().asyncStarted()) // 비동기 처리 시작 확인
                .andReturn();

        this.mockMvc.perform(asyncDispatch(mvcResult))
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(jsonPath("$.isSuccess").value(true))
                .andExpect(jsonPath("$.code").value("200"))
                .andExpect(jsonPath("$.message").value("네이버 클로바 챗봇 답변을 가져오는 데 성공했습니다."))
                .andExpect(jsonPath("$.payload.answer").value(
                        "안녕하세요! 저는 야구 가이드 챗봇 '루키'에요! 야구에 대한 궁금한 점이 있다면 언제든지 물어봐 주세요!"
                ))

중요 변경점은 아래와 같습니다.

  1. .andExpect(request().asyncStarted()) : 비동기 처리를 시작합니다.
  2. asyncDispatch(mvcResult) : 비동기 작업이 종료될 때까지 기다리고 최종 응답을 가져옵니다.
  3. .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON)) : application/json 이 아닌 application/json;charset=UTF-8 의 경우에도 호환 허용합니다.
  1. [A팀] 백엔드 파트 코드리뷰용 PR #44 (comment)
Object보다는, ResponseEntity<ApiResponse> 와 같이 명시해주는것이 더 좋은것 같습니다!

✏️ 관련 이슈

본인이 작업한 내용이 어떤 Issue Number와 관련이 있는지만 작성해주세요

#78


🎸 기타 사항 or 추가 코멘트

비동기 작업에 대한 테스트 코드 부분은 나중에 필요 시 참고해봐도 좋을 것 같습니다!
참고 블로그

@bbbang105 bbbang105 added 🚀 feat 새로운 기능 추가 / 일부 코드 추가 / 일부 코드 수정 (리팩토링과 구분) / 디자인 요소 수정 🔄 refactor 코드 리팩토링 🤯 SANGHO 상호 Issue or PR labels Nov 23, 2024
@bbbang105 bbbang105 requested a review from juuuunny November 23, 2024 15:03
@bbbang105 bbbang105 self-assigned this Nov 23, 2024
Copy link
Contributor

@juuuunny juuuunny left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! LGTM!!

)
.map(answer -> GetGuideChatbotAnswerResponse.of(answer.getAnswers(), answer.getImgUrl()))
// 3. 아무 답변도 없으면 예외 처리
.orElseThrow(() -> new CustomException(ChatbotErrorStatus._NOT_FOUND_GUIDE_CHATBOT_ANSWER));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

저도 키워드에 따른 결과 찾을 때 stream 많이 사용했는데 사용하기 간편하고 좋더라구요
filter, and, or 조건으로 쉽게 조건 붙일 수 있어서 사용하는 데 익숙해지면 좋을 것 같아요!!

request.messages().add(messageFactory.createUserMessage(message));
String answer = clovaApiClient.requestClova(request);
public Mono<GetClovaChatbotAnswerResponse> getClovaChatbotAnswer(String message) {
ChatbotRequest request = clovaRequestFactory.createClovaRequest();
Copy link
Contributor

Choose a reason for hiding this comment

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

GetClovaChatbotAnswerResponseDto로 바꿔주세요!

Copy link
Member Author

Choose a reason for hiding this comment

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

변경 완료!

}

// 실패 응답 - 오버라이드 메서드용
public static <T> ResponseEntity<Object> onFailureForOverrideMethod(BaseErrorCode code, String message) {
ApiResponse<T> response = new ApiResponse<>(false, code.getReasonHttpStatus().getCode(), message, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

public static <T> ResponseEntity<ApiResponse<T>> onFailureForOverrideMethod(BaseErrorCode code, String message) {

여기 onFailureForOverrideMethod메서드도 ApiResponse로 바꾸면 좋을 것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

아 이부분은

    // HttpMessageNotReadableException 처리 (잘못된 JSON 형식)
    @Override
    public ResponseEntity<Object> handleHttpMessageNotReadable(HttpMessageNotReadableException ex,
                                                               HttpHeaders headers,
                                                               HttpStatusCode status,
                                                               WebRequest request) {
        String errorMessage = "요청 본문을 읽을 수 없습니다. 올바른 JSON 형식이어야 합니다.";
        logError("HttpMessageNotReadableException", ex);
        return ApiResponse.onFailureForOverrideMethod(ErrorStatus._BAD_REQUEST, errorMessage);
    }

위처럼 ResponseEntityExceptionHandler 내에 기본적으로 있는 메서드를 오버라이드하는 경우에 쓰는 거라서 타입을 ResponseEntity<Object>로 고정해야 할 것 같아요!!

@bbbang105 bbbang105 merged commit 8edac89 into refactor-v1 Nov 23, 2024
@bbbang105 bbbang105 deleted the feature/#78/refactor-chatbot branch November 23, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 feat 새로운 기능 추가 / 일부 코드 추가 / 일부 코드 수정 (리팩토링과 구분) / 디자인 요소 수정 🔄 refactor 코드 리팩토링 🤯 SANGHO 상호 Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants