프로젝트 코드리뷰 (2)

11. DTO 의존성 분리

현재 toEntity 메서드가 DTO에 의존적이다. 

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();                                                                                      
}

 

content, methodType는 ApiRequestDTO에 있는 필드의 값을 그대로 사용한다. 즉 어차피 dto의 값이 필요한 건 동일하기 때문에 나는 해당 로직은 유지하기로 결정했다. 물론 static으로 바꾸면 재사용성이나 SRP를 지킬 수 있다는 장점이 있지만 이 로직을 재사용할 곳이 없다고 판단했다.

 

12. Record 클래스 검토

내가 예전에 한번 Record를 조사했던 적이 있었다.

https://qkrqkrrlrl.tistory.com/79

자바 Record (자바 16에서 채택된 새로운 클래스)

 

이때 Record 클래스를 조사했는데 조사만 하고 여태 사용하지는 않았었다. DTO의 경우 한번 값을 할당하고 전달 용도로 사용하다 보니 변경이 될 일이 없으니 Record로 변경하는 것도 되게 좋은 선택이라고 생각했다.

 

총 23개의 requestDTO, responseDTO를 전부 Record로 변경했다.

 

13. HashCode() 불필요

 

이전에는 Equals와 Hashcode에 대해 재정의를 진행하지 않았으나, 예전에 기억으로 혹시 모르니 걸어두는 것이 좋다는 얘기를 들었다. 근데 사실상 hash 컬렉션을 사용하는 것 아니면 필요가 없지 않나??

 

그러나 아래처럼 멤버를 직접 비교하는 로직이 있다. 이런 Entity는 구현을 해줘야 한다.

즉 현재 로직에서 equals를 사용하지 않는 부분에서는 삭제를 진행했다.

 

14. MethodType은 이미 스프링에서 존재

 

찾아보니 HttpMethod에 다양한 메서드가 정의되어있다. 그러나 내가 받으려는 메서드는 5개로 정해져 있고, 또 판단하는 로직을 static으로 만들고 해야 해서 그냥 내버려두기로 결정.(추후에 변경 가능)

https://docs.spring.io/spring-framework/docs/4.0.0.M1_to_4.2.0.M2/Spring%20Framework%204.2.0.M2/org/springframework/http/HttpMethod.html

HttpMethod

 

 

15. offset-based paging 문제

 

우선 내 경우는 offset 페이징을 쓸 수밖에 없었다. 페이지를 구해야 하고 url에 페이지를 넣어서 검색도 가능하게 만들었기 때문.

그런데 offset 페이지네이션은 아무래도 성능 저하의 문제가 있어서 개선 내용은 아래 글에 따로 올려놨다.

https://qkrqkrrlrl.tistory.com/152

OFFSET 페이지네이션 쿼리 시간 단축과정

 

 

16. 관리자의 api 기록 검사

현재는 유저의 email로만 검색하게 만들어뒀는데 생각해 보니 id로 검색하는 게 당연히 있어야 한다고 생각.

유저 번호로도 검색이 가능하도록 기능을 추가.

 

 

17. 조건 쿼리 비교 -> 상수화 및 enum 고려

저런 문자열은 직접 적는 것보다 static final을 써서 상수화를 시키는 것이 좋다고 들었다.(혹시 모를 사고를 방지)

그래서 아래처럼 enum으로 만들어서 관리를 진행했다. 

또한 향상된 switch를 통해 로직을 가독성 좋게 변경했다.

 

 

 

18. String.format() 성능 문제

String.format()은 성능이 떨어진다는 문제가 있다.

아래 표를 보면 String.format()이 월등하게 성능이 나오지 않는다.

해당 게시글에서도 StringBuilder 사용을 권장하고 있다. 그래서 StringBuilder를 이용해서 코드 변경 완료.

https://www.happycoders.eu/java/how-to-convert-int-to-string-fastest/

How to Convert int to String in Java - The Fastest Way

 

 

19. 세마포어로 인한 동시 요청 문제

gpt에 요청을 날리는 api를 호출할 때 멀티스레드로 동작하는데 너무 많은 요청이 몰리는 것을 대비해서 세마포어를 5로 할당해 놨었다.

 

우선 세마포어를 5개로 진행하는데 문제는 동일한 사용자의 여러 요청이 문제인 것.

해당 부분은 비관적 쓰기락으로 해결하기로 결정했다. 애초에 member 데이터를 가져올 때 락을 걸어서 가져오면 동일한 유저가 여러 번 요청을 보내도 순차대로 처리하다가 토큰이 0이면 막힐 것이라 판단했다.

 

우선 현재 상태로 테스트를 진행했다. (스레드 : 10개, 요청 횟수 : 10번)

즉 동일 유저 10명이 10번씩 요청을 보냈다고 가정. 토큰은 10개 가지고 있는 상태

 

그 결과 72번의 수행으로 무려 62번이나 더 수행되는 문제가 발생.

 

이번엔 Member를 가져올 때 비관적 쓰기락을 걸어서 테스트를 진행.

정확히 10번 수행되는 것을 확인했다.

 

멀티 스레드를 고려하지 못했던 설계였기에 빠르게 수정을 진행했다.