[UPLUS-139] message 조립 및 전송 개선#4
Conversation
- UsageNotificationEvent 구조 변경 (templateGroupId, subscriptionInfo, variables) - EmailSendRequest, SmsSendRequest, SendResponse DTO 추가 - RestClient 설정 및 mock-server 엔드포인트 연동 - EmailSender/SmsSender HTTP POST 호출로 변경 - MessageSendService 추가 (EMAIL 우선, SMS 폴백) - 불필요한 파일 삭제 (NotificationService, Customer/Subscription 관련)
Summary of ChangesHello @swthewhite, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 풀 리퀘스트는 알림 서비스의 메시지 조립 및 전송 시스템을 전반적으로 개선하는 데 중점을 둡니다. 기존의 단일 메시지 처리 방식에서 벗어나 배치 처리와 외부 API 연동을 통해 시스템의 효율성과 확장성을 향상시키고, 메시지 전송 로직을 재구성하여 유지보수성을 높였습니다. 또한, 데이터 중복 제거 및 템플릿 관리 방식을 최적화하여 안정적인 메시지 전달을 목표로 합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
안녕하세요. 이번 PR은 알림 서비스의 메시지 조립 및 전송 로직을 대폭 개선하는 중요한 변경 사항을 담고 있네요. 카프카 메시지 처리를 개별에서 배치 방식으로 전환하고, 외부 서비스와의 결합도를 낮추는 등 전반적인 아키텍처가 크게 향상되었습니다. 특히 Redis Lua 스크립트를 이용한 중복 제거 최적화는 인상적입니다. 코드 품질을 더욱 높이기 위해 몇 가지 제안 사항을 남깁니다. 주로 HTTP 클라이언트 설정, 상수 관리, 예외 처리, 메트릭 일관성에 대한 내용입니다. 좋은 변경사항에 감사드립니다.
I am having trouble creating individual review comments. Click here to see my feedback.
src/main/java/com/project/notification/service/MessageSendService.java (48-49)
tryEmailSend와 trySmsFallback 메소드 내에서 템플릿 렌더링(templateEngine.render()) 시 발생하는 예외가 처리되지 않고 있습니다. 이 예외는 processEvent의 @Transactional 범위 밖인 NotificationConsumer까지 전파되어 처리되는데, 이 경우 MessageSendService의 트랜잭션이 롤백되어 실패에 대한 MessageLog가 기록되지 않는 문제가 있습니다. 발송 실패에 대한 모든 기록을 남기기 위해, tryEmailSend와 trySmsFallback 메소드 내에서 템플릿 렌더링을 포함한 모든 과정에 대해 try-catch 블록을 적용하여 예외 발생 시 MessageLog를 저장하도록 로직을 수정해야 합니다.
src/main/java/com/project/global/config/RestClientConfig.java (24-33)
현재 SimpleClientHttpRequestFactory를 사용하고 계십니다. 이 팩토리는 커넥션 풀링을 지원하지 않아, 요청마다 새로운 TCP 연결을 생성하므로 높은 처리량이 요구되는 환경에서는 성능 저하의 원인이 될 수 있습니다. HttpComponentsClientHttpRequestFactory (Apache HttpClient 기반)와 같은 커넥션 풀링을 지원하는 팩토리를 사용하시는 것을 강력히 권장합니다. 이를 통해 커넥션을 재사용하여 성능을 향상시키고 리소스를 효율적으로 관리할 수 있습니다.
src/main/java/com/project/notification/consumer/NotificationConsumer.java (55-68)
현재 이벤트 처리 로직에서 dedupResults와 events 리스트의 크기가 일치하지 않을 가능성이 있습니다. records 리스트를 순회하며 역직렬화에 실패하는 경우, events와 eventIds 리스트에는 해당 이벤트가 추가되지 않습니다. 하지만 dedupResults는 성공적으로 역직렬화된 eventIds를 기반으로 생성됩니다. 이후 for (int i = 0; i < events.size(); i++) 루프에서 dedupResults.get(i)를 호출하는 것은 문제가 없어 보이지만, 만약 dedupResults와 events의 인덱스가 어긋나는 엣지 케이스가 발생하면 IndexOutOfBoundsException이 발생하거나 잘못된 중복 체크를 할 수 있습니다.
보다 안전한 처리를 위해, 역직렬화된 이벤트와 그에 해당하는 원본 레코드 정보를 함께 관리하는 객체를 사용하거나, Map<String, UsageNotificationEvent>을 사용하여 eventId로 이벤트를 관리하는 방식을 고려해볼 수 있습니다. 하지만 현재 구조도 대부분의 경우 잘 동작할 것으로 보이며, 로직을 조금 더 명확하게 하기 위해 주석을 추가하는 것도 좋은 방법입니다.
src/main/java/com/project/notification/consumer/NotificationSendDedupService.java (47)
Redis 키의 접두사 "notification:event:"가 tryAcquire 메소드와 tryAcquireBatch 메소드 양쪽에서 하드코딩되어 사용되고 있습니다. 이 값을 private static final String 상수로 추출하면 코드 중복을 줄이고, 향후 키 구조 변경 시 한 곳만 수정하면 되므로 유지보수성이 향상됩니다.
클래스 상단에 상수를 추가하고, 두 메소드에서 이를 사용하도록 수정하는 것을 권장합니다.
private static final String DEDUP_KEY_PREFIX = "notification:event:";
// ...
public boolean tryAcquire(String eventId) {
Boolean success =
redisTemplate.opsForValue().setIfAbsent(DEDUP_KEY_PREFIX + eventId, "1", TTL);
return Boolean.TRUE.equals(success);
}
public List<Boolean> tryAcquireBatch(List<String> eventIds) {
// ...
List<String> keys = eventIds.stream()
.map(id -> DEDUP_KEY_PREFIX + id)
.toList();
// ...
}src/main/java/com/project/notification/service/MessageSendService.java (65)
tryEmailSend 메소드 내에서 처리 시간을 계산하는 방식에 일관성이 부족합니다. 성공 케이스와 일부 실패 케이스에서는 processEvent에서 전달받은 startTime을 사용하고, emailSender.send()에서 예외가 발생한 경우에만 emailStartTime을 사용합니다. 이로 인해 기록되는 처리 시간이 경우에 따라 달라져 메트릭 분석에 혼란을 줄 수 있습니다. 모든 경우에 tryEmailSend 메소드 시작 시점인 emailStartTime을 기준으로 처리 시간을 계산하여 일관성을 확보하는 것이 좋습니다.
SonarQube Quality Summary (Community)❌ Quality Gate FAILED Branch: Issues
Measures
🔗 Dashboard: https://sonarqube.swthewhite.store/dashboard?id=api-message&branch=feat/UPLUS-139 Generated automatically by GitHub Actions. |
SonarQube Quality Summary (Community)✅ Quality Gate PASSED Branch: Issues
Measures
🔗 Dashboard: https://sonarqube.swthewhite.store/dashboard?id=api-message&branch=feat/UPLUS-139 Generated automatically by GitHub Actions. |
1 participant
🍀 이슈 번호
UPLUS-139
✅ 작업 사항
📋 체크리스트
⌨ 기타