프로젝트 코드리뷰 (1)

이번에 정말 좋은 기회를 통해 보초님의 코드 리뷰를 받을 수 있었습니다. 해당 리뷰를 바탕으로 코드 개선을 진행하고 그 과정을 공유하려고 합니다.(조금 내용이 길어질 수 있다는 점)

 

1.generated 이그노어 처리

QueryDSL을 사용하면서 큐클래스 파일이 그대로 깃에 올라가있는 문제 발생.

깃 명령으로 캐시 삭제를 진행하고, 명시적으로 이그노어에 추가해서 반영 완료.

 

2. gradle 형식 통일

기존에는 gradle을 추가하면서 형식이 제각각이었다. 어떤 건 short, 어떤건 그냥 gradle 등등..그래서 이번에 하나로 통일을 진행했다.

 

 

 

3. 사용하지 않는 import 제거

 

사실 크게 신경쓰지 않았던 부분이지만 이번에 코드 리뷰를 통해 optimize import 기능을 이용해서 사용하지 않는 임포트를전부 제거해주었습니다.

 

4. 빈 이름 명시적 작성 제거

 

이번에 이메일 전송 로직을 비동기로 처리하면서 스레드풀을 하나 만들었는데 이름을 명시해놨었다. 그런데 빈을 생성하면 디폴트로 메서드의 이름을 따라가기 때문에 불필요한 코드였기에 삭제를 진행했다.

 

 

5. 비동기 스레드 풀 개수 조절

 

기존에는 현재 스레드풀 10, 최대 스레드 수 50, 작업 큐 20으로 설정했다.

해당 스레드풀은 이메일 전송 로직에서만 사용한다. 즉 관리자가 1:1 문의 답변을 남기는데 이메일 전송을 승인해놓은 유저한테만 해당 스레드가 할당되는 것인데, 많은 스레드가 필요하지 않을 것이라 판단했다.

그래서 스레드풀 10, 최대 스레드 수 50 -> 30, 작업 큐 20으로 조절해서 약간의 여유를 두고 설정했다.

 

그러나 지금만큼 스레드가 사실 필요 없을 것 같다는 생각도 들었다.

 

 

6. 코드 컨벤션 사용

 

코드의 형식이 제각각인 곳이 많았다. 하나의 형식으로 맞춰 통일성을 지키기 위해 이번에 checkStyle을 적용했고 formatter를 통해 네이버 컨벤션을 적용했다.

 

checkStyle를 통해 컨벤션에 다른 곳을 파악할 수 있어서 되게 편리한 기능이라고 생각한다.

 

7. 불필요한 출력문 삭제

 

예전에 디버깅을 진행하면서 출력을 찍어놨다. 그 흔적이 남아있어서 불필요한 코드를 제거해주었다.

 

8. API PATH 재정의 진행

 

ApiRequestController에서 두 개의 컨트롤러 path가 상이한 것을 확인했다.

 

admin의 기능이 추가돼서 그런 것인데 통일성을 해치는 느낌을 받을 수 있었다. 그래서 조금 길어지는 느낌이 들더라도 통일성을 지키기 위해 gpt/admin/requests로 경로 수정을 진행했다.

 

또한 시큐리티 패스에 gpt/admin/** 경로는 ADMIN만 잡을 수 있도록 추가를 진행했다.

 

9. 로그 레벨 설정

이것 또한 디버깅 과정에서 생긴 불필요한 로그라고 판단했다. 실제 운영 환경에서는 디버깅이 번거로워지지 않게 필요한 로그만 찍는 것이 좋다고 판단했다. 그래서 해당 로그는 삭제를 진행.

 

10. NPE 가능성 판단

해당 로직이 어떤 부분인지 먼저 살펴보았다.

결과를 받아서 기록을 저장하기 위한 Entity를 생성하는 메서드였다.

 

아래 코드를 보면 GPT한테 응답이 오면 ResponseDTO로 받아서 Entity를 생성하는데 이 때 response의 content가 null일 경우가 발생할 수 있다는 것.

ChatGPTResponseDTO chatGPTResponseDTO = restTemplate.postForObject(URL, requestDTO,            
        ChatGPTResponseDTO.class);                                                             
                                                                                               
// 응답이 왔다면                                                                                     
ApiRequestHistory apiRequestHistory = apiRequestDTO.toEntity(chatGPTResponseDTO, member, true);
apiRequestHistoryRepository.save(apiRequestHistory);

 

왜냐면 응답이 오지 않을 경우 예외가 발생하게 되는데 그 때는 ResponseDTO를 null로 넣어줘서 Entity를 생성한다.

catch (HttpClientErrorException.TooManyRequests e) {                         
  ApiRequestHistory entity = apiRequestDTO.toEntity(null, member, false);    
  apiRequestHistoryRepository.save(entity);                                  
  throw new GPTException(GPTExceptionInfo.FAIL_REQUEST_GPT, e.getMessage());
}

 

결국 로직에는 문제가 없다 판단했지만 자세히 살펴보면 리스트를 꺼내주는 부분에서 null이 혹시나 있을 수도 있다고 생각했다.

.responseContent(responseDTO != null ? responseDTO.getChoices().get(0).getMessage().getContent() : null)

그래서 위에 String 변수를 하나 둬서 null 판단을 진행했다.

 

public ApiRequestHistory toEntity(ChatGPTResponseDTO responseDTO, Member member, boolean access) {       
    String responseContent = null;                                                                       
    if (responseDTO != null && responseDTO.getChoices() != null && !responseDTO.getChoices().isEmpty()){ 
		responseContent = responseDTO.getChoices().get(0).getMessage().getContent();                     
	}                                                                                                    
        return ApiRequestHistory.builder()                                                               
                .member(member)                                                                          
                .requestStatus(access)                                                                   
                .requestContent(content)                                                                 
                .methodType(methodType)                                                                  
                .responseContent(responseContent)                                                        
                .build();                                                                                
}