[Help] 메시지 전송 로직에서 문제가 있는 부분 수정

[lsh1215]

변경 사항

debugging 활용을 통해 ip 등록에 따른 문제 발생 지점 확인

테스트 실행

• 👍 Yes
• 🙅 No, Because they are not neaded
• 🤯 No, Because I needed help with writing tests

테스트 결과

{
  "account": "5513cjy",
  "message_type": "SMS",
  "content": "안녕",
  "from": "01093541711",
  "duplicate_flag": "N",
  "target_count": 1,
  "targets": [
    {
      "to": "01045175145",
      "change_word": {
        "var1": "치환변수1",
        "var2": "치환변수2",
        "var3": "치환변수3",
        "var4": "치환변수4",
        "var5": "치환변수5",
        "var6": "치환변수6",
        "var7": "치환변수7"
      },
      "name": "이름"
    }
  ],
  "ref_key": "refKey"
}

+α Checklist

• 자바 코드 컨벤션을 지키면서 프로그래밍했는가?
• 한 메서드에 오직 한 단계의 들여쓰기(indent)만 허용했는가?
• else 예약어를 쓰지 않았는가?
• 3개 이상의 인스턴스 변수를 가진 클래스를 구현하지 않았는가? (가능하면 인스턴스 변수의 수를 줄이기 위해 노력한다.)
• setter 없이 구현했는가?
• 메소드의 인자 수를 제한했는가? (4개 이상의 인자는 허용하지 않는다. 3개도 가능하면 줄이기 위해 노력해 본다.)
• 코드 한 줄에 점(.)을 하나만 허용했는가?
• 메소드가 한가지 일만 담당하도록 구현했는가?
• 클래스를 작게 유지하기 위해 노력했는가?

[github-actions]

Code review by ChatGPT

[github-actions]

코드를 전반적으로 살펴보았을 때 몇 가지 개선 사항을 제안할 수 있습니다.

1. RestTemplate의 사용자 정의 설정: RestTemplate을 사용할 때 일반적으로 커스터마이징이 필요할 수 있습니다. 예를 들어, 타임아웃 설정이나 메시지 컨버터를 추가하는 방법을 고려할 수 있습니다.

   @Bean
   public RestTemplate restTemplate() {
       RestTemplate restTemplate = new RestTemplate();
       restTemplate.setRequestFactory(new SimpleClientHttpRequestFactory() {{
           setConnectTimeout(5000); // 연결 타임아웃 설정
           setReadTimeout(5000); // 읽기 타임아웃 설정
       }});
       return restTemplate;
   }

2. RestTemplate의 단일 빈 사용: RestTemplate은 스레드 세이프(Thread-safe)하므로 애플리케이션 내에서 단일 인스턴스를 공유하는 것이 좋습니다. 현재 코드에서는 이 부분이 적절하게 처리되고 있습니다.

3. 상수 사용: 타임아웃과 같은 상수 값을 사용할 때, 하드코딩 대신 상수로 선언하고 사용하는 것이 좋습니다. 이렇게 하면 유지보수성이 높아집니다.

   private static final int TIMEOUT = 5000;
   
   @Bean
   public RestTemplate restTemplate() {
       SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
       factory.setConnectTimeout(TIMEOUT);
       factory.setReadTimeout(TIMEOUT);
       return new RestTemplate(factory);
   }

4. Exception Handling: API 호출 시 예외 처리를 좀 더 구체화할 수 있습니다. 특정 예외 발생 시 어떻게 처리할지에 대한 로직을 추가하는 것도 좋습니다.

5. 로깅: RestTemplate을 사용할 때 요청과 응답에 대한 로깅을 추가하면 디버깅에 유용합니다. 이를 위해 ClientHttpRequestInterceptor를 구현할 수 있습니다.

   public class LoggingInterceptor implements ClientHttpRequestInterceptor {
       @Override
       public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
           // 요청 로깅
           System.out.println("Request: " + request.getURI());
           return execution.execute(request, body);
       }
   }
   
   @Bean
   public RestTemplate restTemplate() {
       RestTemplate restTemplate = new RestTemplate();
       restTemplate.getInterceptors().add(new LoggingInterceptor());
       return restTemplate;
   }

이와 같이 수정하면 RestTemplate의 기능성을 높이고 향후 확장성 및 유지보수성을 고려할 수 있습니다.

[github-actions]

코드를 검토한 결과, 아래와 같은 부분에서 수정이나 개선이 필요합니다.

1. 주석의 가독성:

   • // saveAndFlush 사용으로 @Transactional은 없어도 될 듯 주석에서 불필요한 공백이 존재합니다. 일관된 스타일로 수정하면 가독성이 향상됩니다. 예를 들어, // saveAndFlush 사용으로 @Transactional은 없어도 될 듯와 같이 수정하는 것이 좋습니다.

2. 함수의 반환값 사용:

   • Optional.empty()를 반환하는 부분에서, 주석 등을 통해 왜 빈 값을 반환하는지에 대한 설명이나 문서화가 필요할 수 있습니다. 이 방법은 이 메서드를 호출하는 개발자가 반환값의 의미를 이해하는 데 도움이 됩니다.

3. 메서드 이름:

   • reIssueRefreshToken이라는 메서드 이름이 반환되는 값이 무엇인지 명확하게 나타내지 않을 수 있습니다. 예를 들어, generateNewRefreshTokenForUser와 같은 이름으로 변경하면 이 메서드가 어떤 동작을 수행하는지 더 잘 이해할 수 있습니다.

4. 트랜잭션 주석:

   • 코드에서 @Transactional이 필요하지 않을 수 있다고 언급하고 있습니다. 이 경우, 해당 메서드의 비즈니스 로직을 확인하여 실제로 트랜잭션이 필요한지 확인하는 것이 좋습니다. 만약 필요 없다면, 주석 대신 이 메서드에서 트랜잭션에 대한 정보를 명확히 하는 것이 좋습니다.

5. 파일 끝에 개행 추가:

   • 마지막 줄에 newline 문자(\n)가 없는 것은 일반적으로 좋지 않은 관례입니다. 코드의 가독성과 일관성을 위해 마지막 줄에 개행을 추가하는 것이 좋습니다.

이러한 사항들을 개선하면 코드의 가독성, 유지보수성 및 명확성이 향상될 것입니다.

[github-actions]

코드 리뷰 결과, 다음과 같은 수정 및 개선 사항을 제안합니다:

1. 메소드 명확성 향상:

   • getReceiversAsString 메소드를 MessageController 클래스 내에 정의하여 사용하고 있으나, 이 메소드는 따로 클래스에서 분리하여 유틸리티 클래스로 만들면 재사용성이 높아질 것입니다.
   • sendMessage 메소드 내에서 MessageResponseDto.SendMessageResponse를 사용하여 명시적으로 타입을 작성한 점은 좋지만, 이와 같은 패턴이 반복된다면 일반적인 DTO를 리턴하는 메소드를 별도로 만들어 유연성을 높일 수 있습니다.

2. 코드 중복 제거:

   • getMessages 및 getMessage 메소드에서 메시지 이미지 URL을 추출하는 로직이 중복되어 있습니다. 이 것을 getImageUrls 메소드로 추상화하여 중복을 줄일 수 있습니다.

3. 예외 처리 개선:

   • 서비스 메소드를 호출할 때, 예외 상황을 처리하는 로직이 보이지 않습니다. 예외를 발생시킬 수 있는 부분에 대해 적절한 예외 처리를 추가함으로써 안정성을 높일 수 있습니다.

4. 리턴 타입의 일반화:

   • CommonResponse의 리턴 타입으로 항상 onSuccess 메소드를 호출하기 전에 결과가 널인지 확인하고, 널인 경우 적절한 에러 응답을 리턴하는 것이 좋습니다.

5. Stream API 사용 최적화:

   • map 및 collect의 남용으로 인해 가독성이 떨어질 수 있습니다. 예를 들어, toList() 호출하는 부분은 Java 16 이상에서만 사용할 수 있으며, 하위 버전에서는 collect(Collectors.toList())로 변환해주어야 합니다. 모든 경우를 고려하여 코드를 작성할 필요가 있습니다.

6. 주석:

   • 기능에 대한 설명과 함께 주석을 추가하면 더 많은 이해를 돕는 데 도움이 됩니다. 특히 API 메소드에서 어떤 데이터를 수신하고 어떤 데이터가 반환되는지에 대한 주석이 유용합니다.

이러한 개선점을 적용하면 코드의 가독성과 유지보수성이 높아질 것입니다. 추가적으로, 모든 메소드에 대한 단위 테스트를 작성하면 코드의 신뢰성을 더욱 강화할 수 있습니다.

[github-actions]

코드를 살펴본 결과, 다음과 같은 부분에서 수정이나 개선이 필요합니다:

1. 필드 주석 처리 및 불필요한 필드 제거:

   • createdAt, updatedAt, deleted 필드가 주석 처리되어 있습니다. 이들이 필요하지 않다면 완전히 제거해도 됩니다. 만약 사용될 예정이라면, 해당 필드의 로직을 복원하는 것이 좋습니다.

2. 메서드 이름:

   • addReceivers와 addReceiver 메서드가 혼동을 줄 수 있습니다. 여러 수신자를 추가하는 메서드와 단일 수신자를 추가하는 메서드를 구분하기 위해 맞춤형 이름을 고려해보세요. 예를 들어, addReceivers는 addMultipleReceivers로, addReceiver는 addSingleReceiver로 변경할 수 있습니다.

3. softDelete 메서드:

   • softDelete 메서드가 코드에 없어진 것 같습니다. 이 메서드가 꼭 필요한 기능이라면 추가해야 합니다. 삭제 상태를 관리할 로직이 필요합니다.

4. Builder 패턴:

   • withExternalMessageId 메서드는 설계상으로 좋지만, Builder 패턴을 사용할 때는 Builder를 활용하여 객체를 생성하는 것이 더 일관성 있고 직관적일 수 있습니다. 여기에 externalMessageId를 Builder에 추가함으로써 객체 생성 시 해당 값을 설정할 수 있도록 개선할 수 있습니다.

5. 메서드의 일관성:

   • addReceivers 메서드와 addReceiver 메서드 간에 associateWithMessage 호출이 모두 포함되어 있는데, 만약 이 로직이 필요하다면 두 메서드 모두에서 호출하도록 유지해야 합니다. 논리적인 일관성을 유지하는 것이 중요합니다.

6. Optional 사용 고려:

   • externalMessageId와 같은 필드는 유효성을 검사하고 필요에 따라 Optional을 사용함으로써 NPE(널 포인터 예외)를 방지하는 것도 좋은 방법입니다.

수정 후 코드는 다음과 같이 개선될 수 있습니다:

public class Message extends BaseEntity {
    private String content;

    @Column(name = "external_message_id")
    private String externalMessageId;

    private LocalDateTime sentAt;
    private List<Receiver> receivers = new ArrayList<>();

    public void addMultipleReceivers(List<Receiver> newReceivers) {
        this.receivers.addAll(newReceivers);
        newReceivers.forEach(receiver -> receiver.associateWithMessage(this));
    }

    public void addSingleReceiver(Receiver receiver) {
        this.receivers.add(receiver);
        receiver.associateWithMessage(this);
    }

    public void cancel() {
        // Cancellation logic
    }

    public Message withExternalMessageId(String externalMessageId) {
        this.externalMessageId = externalMessageId;
        return this;
    }
}

위의 제안들을 통해 코드의 가독성과 유지 보수성을 높일 수 있습니다.

[github-actions]

코드에서 수정이 필요한 부분과 개선할 점은 다음과 같습니다.

1. ENUM의 정의: 현재 MessageStatus 열거형에서 CANCELLED 후에 쉼표가 추가되었습니다. 일반적으로 마지막 Enum 상수에는 쉼표를 붙이지 않는 것이 관례입니다. 따라서, CANCELLED 뒤의 쉼표를 제거하는 것이 좋습니다.

2. 주석의 일관성: 각 Enum 상수에 대한 주석을 추가하는 것은 좋지만, 주석이 일관되게 작성되도록 확인해야 합니다. 모든 상수에 대한 주석이 포함되어 있는지 확인하고, 필요하다면 추가적으로 다양한 상태에 대한 설명을 덧붙이는 것이 좋습니다.

3. 네이밍: TEMP라는 이름이 명확하지 않을 수 있습니다. 이 변수의 의미나 사용 용도에 대한 설명이 있는 주석을 추가하거나 좀 더 명확한 이름으로 변경하는 것이 좋습니다.

개선된 코드는 다음과 같이 될 수 있습니다:

public enum MessageStatus {
    SCHEDULED, // 예약됨
    SENT,      // 전송됨
    CANCELLED, // 예약 취소됨
    TEMP;      // 임시 저장됨
}

이와 같은 방식으로 Enum의 사용 및 정의를 개선할 수 있습니다.

[github-actions]

코드에서 수정이 필요하거나 리팩토링할 수 있는 부분에 대해 다음과 같은 피드백을 드립니다:

1. 주석 처리된 필드:

   • 현재 rejectType, sendTime, subject, 및 files 필드가 주석 처리되어 있습니다. 해당 필드들이 필요하지 않다면 삭제하고, 필요하다면 재활용할 수 있도록 재정의하는 것이 좋습니다. 주석보다는 코드에서 명확한 의도를 드러내는 것이 중요합니다.

2. 접근 제어자:

   • SendMessageRequest 및 다른 내부 클래스에 대해 기본적으로 private 필드가 사용되고 있습니다. 클래스 외부에서 접근할 필요가 있는 경우 public 혹은 적절한 접근 제어자를 사용하는 것이 좋습니다.

3. Getter와 Setter의 통일성:

   • @Getter 애너테이션을 사용하고 있지만, 필요하다면 @Setter 애너테이션도 추가하는 것이 좋습니다. 데이터 클래스의 필드 변경이 필요한 경우를 대비하여 Setter를 추가하는 것도 좋은 방법입니다.

4. 예외 처리:

   • 요청을 처리하는 과정에서 발생할 수 있는 예외에 대한 처리 로직이 없습니다. 예외 상황을 적절히 처리하여 안정성을 높이는 것이 필요합니다.

5. 지속적인 객체의 불변성:

   • FileDto 클래스 같은 경우 상태가 변하지 않도록 final 키워드를 추가하여 불변 객체로 만드는 것이 좋습니다. 이로 인해 객체의 상태 변화에 따른 버그를 예방할 수 있습니다.

6. 코드 주석 개선:

   • 클래스와 메서드에 대한 설명 주석을 추가하여 다른 개발자들이 코드를 이해하기 쉽게 만드는 것이 좋습니다. 주석은 코드의 목적과 사용법을 설명하는 데 유용합니다.

7. 끝부분 빈 줄:

   • 코드 끝부분에 빈 줄을 추가하여 코드 스타일을 맞추고 가독성을 높이는 것도 좋은 관행입니다.

이 점들을 고려하여 코드를 수정하고 개선해보세요.

[github-actions]

코드를 검토한 결과, 다음과 같은 부분들이 개선될 수 있습니다.

1. 일관성 있는 접근 제어자 사용: TempMessageResponse 클래스의 접근 제어자가 생략되어 있습니다. 만약 이 클래스가 다른 클래스에서도 사용될 예정이라면 public 접근 제어자를 명시하는 것이 좋습니다.

   수정 예시:

   @Getter
   @Builder
   public static class TempMessageResponse {
       private Long messageId;
       private String status;
   }

2. 클래스 주석 추가: 각 응답 클래스를 설명하는 주석을 추가하면 나중에 코드를 유지보수하거나 다른 개발자가 이해하는 데 도움이 됩니다.

   수정 예시:

   /** 
    * 임시 메시지 응답 클래스 
    */
   @Getter
   @Builder
   public static class TempMessageResponse {
       private Long messageId;
       private String status;
   }

3. 필드에 대한 추가 설명: status 필드의 경우 그 의미나 가능 값을 설명하는 주석을 추가하면 코드의 가독성을 높일 수 있습니다.

   수정 예시:

   @Getter
   @Builder
   public static class TempMessageResponse {
       private Long messageId;
   
       /** 
        * 메시지 상태 (예: SUCCESS, FAILURE) 
        */
       private String status;
   }

4. 타입 안전성을 위한 Enum 사용: status 필드가 특정 값만 가질 수 있다면 String 대신 enum 타입을 사용하는 것이 좋습니다. 이를 통해 잘못된 값이 들어가는 것을 방지할 수 있습니다.

   예시:

   public enum MessageStatus {
       SUCCESS,
       FAILURE
   }
   
   @Getter
   @Builder
   public static class TempMessageResponse {
       private Long messageId;
       private MessageStatus status;
   }

이러한 개선사항을 통해 코드의 가독성과 유지보수성을 높이고, 잠재적인 오류를 줄일 수 있습니다.

[github-actions]

코드에서 수정이 필요하거나 리팩토링할 수 있는 부분에 대해 제안하겠습니다.

1. 삭제 상태 관련 메소드의 의미 변경:

   • findAllByDeletedFalseAndSenderUser(User user) 메소드를 findAllBySenderUser(User user)로 변경하셨는데, 삭제 여부를 검사하는 로직이 사라지면서 중요한 정보가 손실되었습니다. 삭제되지 않은 메시지를 조회하려면 삭제 여부에 대한 조건을 유지해야 합니다. 조건을 삭제하지 말고, 원래의 의미를 유지하는 것이 좋습니다.

   // 수정된 코드
   List<Message> findAllByDeletedFalseAndSenderUser(User user);

2. 삭제된 메시지 처리:

   • 카스쿼리에서 삭제된 메시지를 어떻게 처리할지에 대한 방침을 정해야 합니다. 만약 삭제된 메시지를 무시해야 한다면, 항상 삭제 여부를 체크하는 메소드를 사용하거나 애플리케이션 전반에 걸쳐 삭제 여부를 어떻게 처리할지를 명확히 해야 합니다.

3. 메소드 명세:

   • 구체적으로 메소드의 역할을 구분하기 위해 명확한 네이밍을 사용하는 것이 좋습니다. 예를 들어, 삭제된 메시지와 관련된 메소드를 구분 지을 수 있는 방법을 고려해보세요.

4. Optional 처리:

   • findByIdAndSenderUser(Long id, User user) 메소드는 특정 사용자에게 특정 메시지가 있는지 여부를 확인하는 것과 관련이 있습니다. 이 메소드는 결과가 없을 수 있기 때문에 결과를 처리할 때 Optional을 적절히 활용하는 것이 중요합니다. 호출하는 쪽에서 Optional을 안전하게 처리하도록 문서화하는 것도 고려해보세요.

// Optional을 사용하여 안전하게 처리하도록 작업
Optional<Message> message = messageRepository.findByIdAndSenderUser(id, user);
message.ifPresentOrElse(
    m -> {/* 메시지가 있을 경우 처리 */},
    () -> {/* 메시지가 없을 경우 처리 */}
);

5. 테스트:
   • 리팩토링 후 코드가 의도한 대로 작동하는지 검증하기 위해 단위 테스트를 작성하는 것이 좋습니다. JPA 메소드를 직접 호출할 수 있는 테스트를 구성하여 삭제 여부 확인 로직이 제대로 작동하는지 확인하십시오.

위의 고려 사항을 통해 코드의 가독성과 유지보수성을 높일 수 있습니다.

[github-actions]

코드의 일부 수정이 필요한 부분과 개선할 수 있는 점들을 다음과 같이 제안합니다:

1. DDL Auto 설정:

   • ddl-auto: create-drop로 변경하는 것은 개발 환경에서는 유용할 수 있지만, 프로덕션 환경에서는 데이터 손실의 위험이 있습니다. 따라서 환경에 따라 다른 설정을 두는 것이 좋습니다. 예를 들어, 프로덕션 환경에서는 ddl-auto: none 또는 ddl-auto: validate를 사용하는 것이 더 안전할 수 있습니다.

2. 환경변수 사용:

   • ppurio 부분에서 API URL과 키 등을 환경변수로 설정하는 것은 좋은 접근입니다. 하지만 해당 환경변수가 존재하지 않을 경우 애플리케이션이 어떻게 동작할지에 대한 명확한 핸들링이 필요합니다. 예를 들어, 환경변수가 비어있을 때는 예외를 발생시키거나 기본값을 제공하는 방식으로 처리하는 것이 좋습니다.

3. 주석 및 문서화:

   • 설정 파일에 대한 설명이나 주석을 추가하면 다른 개발자가 코드의 의도를 이해하는 데 도움이 됩니다. 특히 중요한 설정(예: ddl-auto의 의미)에 대한 설명을 추가하는 것이 좋습니다.

4. 일관성 있는 형식:

   • cloud와 ppurio 설정의 배열 형식 일관성을 유지하는 것이 좋습니다. 예를 들어, 모든 설정이 동일한 키 워드 스타일을 사용하는지 확인하고, 필요한 경우 정렬을 맞추어 가독성을 높여주세요.

5. 종속성 관리:

   • 만약 코드의 다른 부분에서 AWS S3와 관련된 설정이 필요하다면, 해당 부분을 별도로 관리하여 설정을 모듈화하는 것이 좋습니다. 예를 들어, S3와 관련된 특정 설정을 별도의 파일로 분리하여 관리할 수 있습니다.

이러한 부분을 개선하면 코드의 유지 보수성이 높아질 것입니다.

[github-actions]

코드의 특정 부분에서 개선할 수 있는 점에 대해 말씀드리겠습니다.

1. 빈 줄 추가: 파일 끝에 줄바꿈이 누락되었습니다. 코드의 가독성을 높이기 위해 파일 끝에 최소한 하나의 줄바꿈을 추가하는 것이 좋습니다.

2. 키 이름 개선: ppurio라는 키를 사용하는 부분에서 해당 키의 의미가 명확하지 않습니다. 다른 팀원이 코드를 이해하기 쉽게 하기 위해 보다 구체적이고 직관적인 이름을 사용하는 것이 좋습니다.

3. 중복 키 줄 정리: api 내의 키들이 Mock으로 시작하는데, 실제 사용 시 프로덕션 코드와의 혼동을 피하기 위해 Mock 변수 이름에 Mock이라는 접두사를 넣는 것이 좋습니다. 이렇게 하면 실제 데이터가 아닌 테스트 데이터임을 명확히 할 수 있습니다. 예를 들어, url, key, account를 mock_url, mock_key, mock_account로 변경할 수 있습니다.

4. 구성 관리: YAML 파일의 가독성을 위해 중첩 구조에 주석을 달아서 각 변수의 의미를 설명해주는 것이 좋습니다. 이는 추후 유지보수 시 도움이 됩니다.

5. 형식 통일성: 코드의 일관성을 위해 false와 같은 불리언 값도 다른 데이터 유형과 동일하게 형식을 유지해주는 것이 좋습니다. 예를 들어 false 대신 False로 대문자 스타일을 통일할 수 있습니다(만약 이 스타일이 코드 전반에서 사용된다면).

아래는 코드 개선 예시입니다:

cloud:
  region:
    static: MockAWSS3Region
  stack:
    auto: false  # 자동 스택 생성 여부 설정

ppurio:  # PPURIO 서비스 관련 정보
  api:
    mock_url: MockPPURIOUrl  # PPURIO API URL
    mock_key: MockPPURIOKey  # PPURIO API 키
    mock_account: MockPPURIOAccount  # PPURIO 계정 정보

이러한 변경 사항들을 반영하면 코드의 가독성과 유지보수성이 더욱 향상될 것입니다.