Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
Walkthrough이 변경사항은 RabbitMQ 기반의 메시지 큐 시스템을 도입하여 FCM 알림 전송을 비동기로 처리하는 이벤트 기반 아키텍처로 전환합니다. 새로운 설정 클래스들(RabbitMQConfig, RabbitWorkerConfig, WebAsyncConfig)과 메시지 처리 컴포넌트들(NotificationConsumer, NotificationEventHandler, NotificationMQProducer)을 추가하며, Worker 프로필을 통해 독립적인 메시지 처리 서비스를 구현합니다. 기존의 AsyncConfig는 제거되고, 환경별 YAML 설정이 확장됩니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 상세 검토 의견1. 아키텍처 전환의 복잡성새로운 메시지 큐 기반 아키텍처가 상당히 많은 컴포넌트를 한번에 도입하고 있습니다. RabbitMQConfig → RabbitWorkerConfig → NotificationConsumer → NotificationMQProducer → NotificationEventHandler으로 이어지는 의존성 체인이 있는데, 각 계층 간의 관계가 명확히 문서화되지 않아 있습니다. 구체적 한계:
개선 방향: 2. 에러 처리 전략 부재가장 주목할 점은 NotificationMQProducer와 NotificationConsumer에서 명시적인 에러 처리가 없다는 것입니다. 문제점:
권장 개선 방안: 3. 스레드 풀 설정의 타당성 검토RabbitWorkerConfig에서 검토 포인트:
권장 검토 항목:
# application-worker.yml에 추가 가능
executor:
core-pool-size: 4
max-pool-size: 8
queue-capacity: 2004. NotificationMQProducer의 배치 로직publishTokens: req -> Lists.partition(tokens, 500) -> publish each확인해야 할 사항:
개선 제안: private static final int TOKEN_BATCH_SIZE = 500; // FCM Multicast API 제한이렇게 상수화하면, 향후 정책 변경 시 한 곳만 수정하면 됩니다. 5. 프로필 관리의 복잡성현재 구조:
우려사항:
권장 대응:
6. 데이터 정합성 문제NotificationEventHandler는 확인 사항:
7. application-worker.yml의 설정server.port: 0 # 웹 서버 미실행
spring.main.web-application-type: none
task.scheduling.enabled: false
flyway.enabled: false좋은 점:
검토 포인트:
최종 평가이 PR은 확장성 있는 메시지 기반 아키텍처로의 전환이라는 긍정적인 방향입니다. 다만 다음을 보완하면 더욱 견고해질 것입니다:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmEventListener.java (1)
28-38: 에러 처리 부재: FCM 전송 실패 시 재시도 메커니즘 필요세 메서드 모두
fcmClient호출 시 예외 처리가 없습니다. FCM 전송이 실패하면 이벤트는 소실되고, 사용자는 알림을 받지 못하게 됩니다. 특히onTokens메서드의 배치 처리에서는 일부 배치만 실패할 경우 어떤 배치가 실패했는지 추적 불가능합니다.권장하는 개선 방안:
선택지 1 (재시도 + 로깅, 현재 권장):
@Transactional @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void onTokens(SendMessageByFcmTokensRequest request) { Lists.partition(request.allTokens(), FCM_BATCH_SIZE) .forEach(tokens -> { try { fcmClient.sendMulticast(request.withTokens(tokens)); } catch (FcmException e) { log.error("FCM 배치 전송 실패. 토큰: {}, 원인: {}", tokens, e.getMessage()); // 실패한 토큰을 재시도 큐에 저장하거나 // 모니터링 대시보드에 기록 } }); }
- 장점: 에러 격리, 부분 성공 처리 가능
- 단점: 개별 배치별 실패 추적 필요
선택지 2 (Resilience4j + Circuit Breaker, 더 견고함):
- RabbitMQ로 이미 전환 중이라면, 불필요한 추가 복잡도
- 기존 시스템이 오래 유지된다면 고려 가치
현재 RabbitMQ 전환 과정이므로, 최소한 선택지 1 수준의 에러 핸들링을 추가하여 데이터 손실을 방지하시는 걸 강력히 권장합니다.
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmClient.java (1)
67-67: 로그 레벨을 재검토하세요.
log.warn은 경고나 문제 상황에 사용하는 레벨인데, 정상적인 multicast 배치 응답을 로깅하는 데 사용되고 있습니다. BatchResponse에는 성공/실패 정보가 모두 포함되므로, 로그 레벨을 상황에 맞게 조정해야 합니다.개선 방안:
- 성공 케이스는
log.info또는log.debug- 실패가 포함된 경우만
log.warn또는log.error- 실패 건에 대해서만 별도 로깅
- log.warn("multicast batchResponse : {}", batchResponse.toString()); + if (batchResponse.getFailureCount() > 0) { + log.warn("Multicast 전송 중 실패 발생 - 성공: {}, 실패: {}, 응답: {}", + batchResponse.getSuccessCount(), + batchResponse.getFailureCount(), + batchResponse); + } else { + log.info("Multicast 전송 완료 - 성공: {}", batchResponse.getSuccessCount()); + }
🧹 Nitpick comments (11)
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmEventListener.java (2)
14-17: @async 실행기 설정 확인 필요: 기본 스레드 풀로 인한 병목 가능성현재
@Async어노테이션이 있지만, 전용 Executor가 지정되지 않았습니다. 기본값(SimpleAsyncTaskExecutor)을 사용하면 요청마다 새로운 스레드가 생성되어 다음 문제가 발생할 수 있습니다:
- 높은 FCM 요청 트래픽 시 스레드 고갈
- 메모리 누수 위험 (스레드 재사용 안 됨)
- 컨텍스트 스위칭 오버헤드
PR 설명에서
WebAsyncConfig와producerExecutor언급이 있으니, 다음과 같이 명시적으로 지정하는 것을 권장합니다:@Async("fcmWorkerExecutor") // 또는 워커 전용 executor 지정또는 별도로 FcmEventListener 전용 executor 생성:
@Bean(name = "fcmListenerExecutor") public Executor fcmListenerExecutor() { ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); executor.setCorePoolSize(5); executor.setMaxPoolSize(20); executor.setQueueCapacity(100); executor.setThreadNamePrefix("fcm-listener-"); executor.initialize(); return executor; }현재 설정이 어떻게 되어 있는지
WebAsyncConfig와 함께 검토해주시면, 이 부분이 예상대로 동작하는지 확인할 수 있을 것 같습니다.
33-37: 배치 처리 로직의 시점 검토: 리스너 vs MQ 컨슈머현재 리스너 내부에서 배치를 나누어 처리하는 방식인데, MQ 기반 아키텍처로 전환할 때 이 로직의 위치를 재검토하는 것이 좋습니다.
현재 방식의 한계:
- 리스너에서 배치 처리 → 하나의 리스너 스레드에서 순차 처리
- 높은 토큰 수(예: 10,000개) 시 리스너 블로킹 가능
- 배치 간 재시도 전략이 불명확함
MQ 전환 후 권장 방식:
배치 처리를 라우팅 로직으로 이동 (MQ Producer 단계에서 처리):// NotificationMQProducer에서 처리 public void publishTokensEvent(SendMessageByFcmTokensRequest request) { Lists.partition(request.allTokens(), FCM_BATCH_SIZE) .forEach(tokens -> { MQMessage msg = new MQMessage(request.withTokens(tokens)); rabbitTemplate.convertAndSend(msg); }); }
- 장점: 리스너는 단일 배치만 처리, 확장성 좋음, 병렬 처리 용이
- 단점: Producer 쪽 로직 증가
현재 이 클래스는 임시 상태이므로, 새로운 NotificationConsumer에서는 이 배치 로직을 어디에 배치할 예정인지 확인해주면 좋을 것 같습니다.
backend/src/main/java/backend/mulkkam/common/config/WebAsyncConfig.java (1)
13-22: ThreadPoolTaskExecutor 설정에 대한 검토가 필요합니다.현재 설정(core=2, max=4, queue=50)이 상대적으로 작아 보입니다. 특히
RabbitWorkerConfig의fcmWorkerExecutor(core=4, max=8, queue=200)와 비교했을 때 차이가 큽니다.잠재적 문제점:
- 큐 용량 부족: 큐 크기 50은 부하가 높을 때 금방 차서
RejectedExecutionException이 발생할 수 있습니다 (기본 정책이AbortPolicy).- 처리량 제한: 최대 4개 스레드로는 많은 메시지 발행 요청을 처리하기 어려울 수 있습니다.
개선 방안:
- 큐 용량을 최소 100 이상으로 증가시키거나, 예상 부하에 맞춰 조정하세요.
- Rejection policy를 명시적으로 설정하세요 (예:
CallerRunsPolicy- 호출 스레드에서 실행, 또는DiscardOldestPolicy- 가장 오래된 작업 제거).- 운영 환경에서 스레드 풀 메트릭을 모니터링하고 튜닝하세요.
@Bean("producerExecutor") public ThreadPoolTaskExecutor producerExecutor() { var ex = new ThreadPoolTaskExecutor(); ex.setCorePoolSize(2); ex.setMaxPoolSize(4); - ex.setQueueCapacity(50); + ex.setQueueCapacity(100); ex.setThreadNamePrefix("producer-async-"); + ex.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); ex.initialize(); return ex; }추가 import가 필요합니다:
import java.util.concurrent.ThreadPoolExecutor;backend/src/main/resources/application-prod.yml (1)
33-37: 운영 환경에 RabbitMQ 고급 설정 추가를 검토하세요.현재는 기본 연결 정보만 설정되어 있습니다. 운영 환경의 안정성과 성능을 위해 다음 설정들을 추가로 고려해보세요:
권장 추가 설정:
- Connection pool: 연결 재사용 및 관리
- Timeout 설정: 네트워크 장애 시 빠른 실패
- Retry 정책: 일시적 장애 대응
- TLS/SSL: 보안 통신 (운영 환경 필수)
rabbitmq: host: ${MQ_HOST} port: 5672 username: ${MQ_USER} password: ${MQ_PASSWORD} + connection-timeout: 30000 # 30초 + requested-heartbeat: 60 # 60초마다 heartbeat + cache: + connection: + mode: connection # 연결 풀 모드 + size: 5 # 연결 풀 크기 + channel: + size: 50 # 채널 풀 크기 + checkout-timeout: 5000 # 채널 대기 타임아웃 + template: + retry: + enabled: true + initial-interval: 1000 + max-attempts: 3 + multiplier: 2backend/src/main/resources/application-worker.yml (2)
1-9: Worker 프로필 기본 설정이 적절합니다.웹 애플리케이션을 띄우지 않고 순수 메시지 컨슈머로 동작하도록 설정되어 있어 의도에 맞습니다.
lazy-initialization: true는 시작 시간을 단축시키지만, 몇 가지 주의사항이 있습니다.Lazy Initialization 주의사항:
- 첫 요청 시 지연: 첫 메시지 처리 시 빈 초기화로 인한 지연 발생
- 초기화 오류 발견 지연: 애플리케이션 시작 시가 아닌 런타임에 설정 오류 발견
- Listener 활성화:
@RabbitListener는 eager 초기화가 필요할 수 있음필요시 특정 빈만 eager 로딩하도록 설정할 수 있습니다:
@Component @Lazy(false) // 이 컴포넌트는 즉시 로딩 public class NotificationConsumer { // ... }
18-23: 스케줄러와 Flyway 비활성화가 적절합니다.Worker는 메시지 소비에만 집중하므로 스케줄링이나 마이그레이션이 불필요합니다. 이는 리소스 절약과 책임 분리 측면에서 올바른 설계입니다.
다만, 로깅 설정이 누락되어 있습니다. Worker의 메시지 처리 과정을 모니터링하려면 로그 레벨과 패턴 설정이 필요합니다.
다음 로깅 설정 추가를 권장합니다:
logging: level: backend.mulkkam.common.infrastructure.fcm: INFO org.springframework.amqp: INFO # RabbitMQ 관련 로그 pattern: console: "%d{yyyy-MM-dd HH:mm:ss} [%thread] %-5level %logger{36} - %msg%n"backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationEventHandler.java (1)
12-15: @async 실행기 지정 + 프로필 범위 축소 제안현재 클래스 레벨 @async는 기본 실행기(SimpleAsyncTaskExecutor)로 동작할 수 있습니다. prod/dev에서만 비동기 발행이 필요하고, 이미 WebAsyncConfig에 "producerExecutor"가 있으므로 해당 풀을 명시적으로 사용하고, worker 프로필에서는 빈이 생성되지 않도록 제한하는 편이 안전합니다.
-@Async -@RequiredArgsConstructor -@Component +@RequiredArgsConstructor +@Component +@Profile({"prod","dev"}) public class NotificationEventHandler {그리고 각 메서드에 실행기 지정:
-@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) +@Async("producerExecutor") +@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void onTopic(...) -@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) +@Async("producerExecutor") +@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void onToken(...) -@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) +@Async("producerExecutor") +@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void onTokens(...)장점: 요청 쓰레드 비차단 + 일관된 풀 사용 + worker 기동 시 불필요한 빈 생성 방지. 단점: prod/dev 외 프로필에서 이벤트 발행이 필요하다면 추가 프로필 고려 필요. (As per coding guidelines)
backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java (1)
17-26: 워커 풀 종료 시 그레이스풀 셧다운 보강현재 ThreadPoolTaskExecutor는 즉시 종료될 수 있어 처리 중 작업이 중단될 위험이 있습니다. 셧다운 대기 설정을 권장합니다.
public ThreadPoolTaskExecutor fcmWorkerExecutor() { var ex = new ThreadPoolTaskExecutor(); ex.setCorePoolSize(4); ex.setMaxPoolSize(8); ex.setQueueCapacity(200); ex.setThreadNamePrefix("fcm-worker-"); + ex.setWaitForTasksToCompleteOnShutdown(true); + ex.setAwaitTerminationSeconds(30); ex.initialize(); return ex; }장점: 배포/스케일인 시 메시지 유실 방지. 단점: 종료 시간이 늘 수 있음.
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java (1)
17-21: 퍼블리셔 확인/리턴 처리(Confirm/Return) 여부 확인
RabbitTemplate사용 시 라우팅 실패나 브로커 비가용을 조기에 감지하려면 Publisher Confirm/Returns 활성화가 필요합니다. 현 클래스에서는 설정할 수 없고 ConnectionFactory/Template 설정에서 켜야 합니다. RabbitMQConfig 코멘트를 참고해 주세요.backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationConsumer.java (1)
27-43: 멀티캐스트/토픽 소비: 부분 실패/중복 방지 전략
- 멀티캐스트는 일부 토큰 실패가 흔합니다. 실패 토큰 추출 후 재큐(지연 큐) 또는 별도 보정 파이프라인 권장.
- 재처리 시 중복 발송 방지를 위해 메시지에 idempotency 키(예: eventId) 포함 후 중복 차단(캐시/DB) 또는 FCM collapseKey 사용을 고려하세요.
원하시면 실패 토큰만 재게시하는 보정 로직과 DTO 확장(eventId/collapseKey) 패치 제안 가능합니다.
backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java (1)
53-66: 라우팅키 하드코딩 → 프로퍼티화 + 상수 중복 제거현재 바인딩 키 "notify.*"가 프로듀서와 컨피그 양쪽에 하드코딩되어 있어 변경 시 드리프트 위험이 큽니다. 프로퍼티로 외부화하고 동일 키를 사용하도록 일원화하세요.
- public Binding bindToken() { - return BindingBuilder.bind(tokenQueue()).to(notificationsExchange()).with("notify.token"); - } + @Value("${mulkkam.mq.routing.token:notify.token}") + private String rkToken; + @Bean + public Binding bindToken() { + return BindingBuilder.bind(tokenQueue()).to(notificationsExchange()).with(rkToken); + } - public Binding bindTokens() { - return BindingBuilder.bind(tokensQueue()).to(notificationsExchange()).with("notify.tokens"); - } + @Value("${mulkkam.mq.routing.tokens:notify.tokens}") + private String rkTokens; + @Bean + public Binding bindTokens() { + return BindingBuilder.bind(tokensQueue()).to(notificationsExchange()).with(rkTokens); + } - public Binding bindTopic() { - return BindingBuilder.bind(topicQueue()).to(notificationsExchange()).with("notify.topic"); - } + @Value("${mulkkam.mq.routing.topic:notify.topic}") + private String rkTopic; + @Bean + public Binding bindTopic() { + return BindingBuilder.bind(topicQueue()).to(notificationsExchange()).with(rkTopic); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/backend-dev-cd.yml(1 hunks)backend/build.gradle(1 hunks)backend/src/main/java/backend/mulkkam/common/config/AsyncConfig.java(0 hunks)backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java(1 hunks)backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java(1 hunks)backend/src/main/java/backend/mulkkam/common/config/WebAsyncConfig.java(1 hunks)backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmClient.java(1 hunks)backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmEventListener.java(1 hunks)backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationConsumer.java(1 hunks)backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationEventHandler.java(1 hunks)backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java(1 hunks)backend/src/main/resources/application-dev.yml(2 hunks)backend/src/main/resources/application-local.yml(1 hunks)backend/src/main/resources/application-prod.yml(1 hunks)backend/src/main/resources/application-worker.yml(1 hunks)backend/src/main/resources/data.sql(0 hunks)backend/src/test/resources/application-ci.yml(2 hunks)
💤 Files with no reviewable changes (2)
- backend/src/main/resources/data.sql
- backend/src/main/java/backend/mulkkam/common/config/AsyncConfig.java
🧰 Additional context used
📓 Path-based instructions (1)
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/main/resources/application-dev.ymlbackend/src/main/resources/application-local.ymlbackend/src/main/resources/application-prod.ymlbackend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmEventListener.javabackend/src/main/resources/application-worker.ymlbackend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.javabackend/build.gradlebackend/src/main/java/backend/mulkkam/common/config/WebAsyncConfig.javabackend/src/test/resources/application-ci.ymlbackend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmClient.javabackend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.javabackend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationEventHandler.javabackend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationConsumer.javabackend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java
🧬 Code graph analysis (5)
backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java (3)
backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java (1)
Configuration(13-44)backend/src/main/java/backend/mulkkam/common/config/WebAsyncConfig.java (1)
Configuration(9-23)backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationConsumer.java (1)
Profile(11-44)
backend/src/main/java/backend/mulkkam/common/config/WebAsyncConfig.java (1)
backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java (1)
Configuration(13-44)
backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java (1)
backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java (1)
Profile(17-80)
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationEventHandler.java (1)
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java (1)
RequiredArgsConstructor(13-37)
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationConsumer.java (2)
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java (1)
RequiredArgsConstructor(13-37)backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java (1)
Profile(17-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (8)
.github/workflows/backend-dev-cd.yml (2)
1-132: PR 체크리스트 미완료 항목 확인PR 설명에서 다음 항목들이 아직 미완료로 표시되어 있습니다:
- 알림 전송 여부 확인 테스트
- 패키지 위치 수정
이 워크플로우 파일은 병합 전에 이 항목들이 완료되어야 합니다. 특히 "패키지 위치 수정"은 코드 구조와 관련이 있을 가능성이 높으므로, 완료 상태를 확인하고 필요시 추가 파일 리뷰가 있어야 합니다.
5-5: 기능 브랜치를 자동 배포에 포함시키는 패턴은 신중히 검토가 필요합니다.현재 구조의 맥락을 먼저 인정하겠습니다. Blue-Green 배포 방식과
backend/**경로 필터링, 그리고self-hosted, dev환경으로 제한한 점들이 위험을 상당히 완화하고 있습니다. 다만 몇 가지 점검할 사항들이 있습니다:현재 구조에서 고려할 점:
임시 설정의 영구화 위험: 기능 브랜치는 본질적으로 일시적입니다. PR 머지 후
feat/977을 워크플로우에서 제거하지 않으면, 다시 푸시할 때마다 불필요하게 배포 파이프라인이 돌아갑니다. 이는 CI/CD 자원 낭비와 예기치 않은 배포를 초래할 수 있습니다.명확한 정책의 부재: 다른 워크플로우 파일들이 푸시 브랜치를 명시하지 않은 반면, 이 파일만 명시적으로 지정한 것을 보면, 팀의 배포 정책이 명확하게 문서화되지 않은 상태로 보입니다.
스케일 문제는 아직 미래형: 지금은 한 브랜치지만, 다음 번엔
feat/978,feat/979…가 생길 텐데, 매번 워크플로우를 수정하는 것은 번거롭습니다.제안:
- 단기 (현재): 현재 설정을 유지하되, 이 변경의 의도를 PR 설명이나 체크리스트에 명시하세요. 예: "feat/977 개발 완료 후 삭제 예정" 같은 메모가 도움이 됩니다.
- 중기 (PR 완료 후): feat/977 브랜치가 develop-be로 머지되면, 곧바로 이 워크플로우에서 제거하는 것을 체크리스트에 추가하세요.
- 장기 (팀 정책): 모든 feature 브랜치를 자동 배포하거나, 명시적으로 지정하는 것 중 어느 정책을 택할지 팀과 논의해 README나 CONTRIBUTING.md에 기록하는 것을 권장합니다.
현재 PR 체크리스트의 "알림 전송 여부 확인 테스트"와 "패키지 위치 수정"이 완료되면, 이 워크플로우 설정도 함께 검토하시기 바랍니다.
backend/build.gradle (1)
44-44: AMQP 의존성 추가가 적절합니다.Spring Boot의 dependency management를 통해 버전이 자동 관리되므로 안전하게 추가되었습니다. RabbitMQ 메시징 기능 구현에 필요한 적절한 의존성입니다.
backend/src/test/resources/application-ci.yml (1)
24-28: CI 환경 RabbitMQ 설정이 적절하게 추가되었습니다.다른 환경(dev, prod)과 동일한 토폴로지 구조를 유지하여 일관성이 확보되었습니다. CI 테스트 시 RabbitMQ 기반 메시징 로직을 검증할 수 있는 환경이 마련되었습니다.
Also applies to: 46-57
backend/src/main/resources/application-prod.yml (1)
39-50: MQ 토폴로지 네이밍이 명확합니다.Exchange와 Queue, Routing Key의 네이밍이 일관성 있고 의도가 명확하게 드러납니다.
notifications.exexchange에 세 가지 용도별 큐(token, tokens, topic)를 바인딩하는 구조가 합리적입니다.backend/src/main/resources/application-dev.yml (1)
11-15: 개발 환경 RabbitMQ 설정이 운영 환경과 일관성 있게 구성되었습니다.토폴로지가 prod와 동일하여 개발/스테이징에서 운영과 동일한 메시징 흐름을 테스트할 수 있습니다.
다만,
application-prod.yml에 남긴 리뷰처럼, 개발 환경에서도 connection timeout, retry 정책 등의 고급 설정을 추가하면 실제 운영 환경과 더 유사한 조건에서 테스트할 수 있습니다.Also applies to: 83-94
backend/src/main/resources/application-worker.yml (1)
11-41: 현재 Worker 설정에는 데이터베이스가 필요하지 않습니다.코드 검증 결과, NotificationConsumer와 FcmClient 모두 DB 접근 없이 순수 메시지 수신 및 FCM 전송 기능만 수행하고 있습니다. 원본 리뷰에서 제시한 시나리오들(무효 토큰 삭제, 전송 이력 저장, 재시도 정책, 유저 설정 조회)은 현재 코드에 구현되지 않았으므로, 지금 단계에서 DB 설정을 미리 추가하는 것은 YAGNI 원칙에 어긋납니다. 향후 해당 기능이 필요해질 때 그때 설정하는 것이 더 적절합니다.
다만 한 가지 고려할 점은,
FcmClient.sendMulticast()메서드에서BatchResponse를 단순 로그로만 기록하고 있다는 부분입니다. 이 응답에는 실패한 토큰 정보가 포함되어 있는데, 이를 버려버리면 나중에 "무효 토큰 삭제" 기능을 추가하려 할 때 대응이 어려워집니다. 지금 당장 DB는 필요 없지만, 구조적으로 실패 정보를 추적할 수 있도록 설계해두면 향후 확장이 수월해질 것입니다.backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationEventHandler.java (1)
19-32: 이벤트 발행의 트랜잭션 안전성 확인 결과현재 코드를 검증한 결과,
publishEvent()호출 시점들이 모두@Transactional메서드 내에서 이루어지고 있습니다:
SuggestionNotificationService.createAndSendSuggestionNotification()(라인 96-97): @transactionalNotificationService.createAndSendTokenNotification()(라인 127-128): @transactionalNotificationService.processReminderNotifications()(라인 53-54): @transactional따라서 AFTER_COMMIT에서 이벤트가 무시되는 현재 상황은 발생하지 않습니다. 다만, 향후 새로운 코드 경로에서 비트랜잭션 컨텍스트에서 이벤트를 발행할 가능성을 대비한 방어적 개선을 고려할 수 있습니다:
선택 1 (현재 상태 유지): 데이터 정합성이 보장되는 대신, 트랜잭션 없는 컨텍스트에서는 이벤트 미발행.
선택 2 (fallbackExecution 추가):
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT, fallbackExecution = true)로 설정하여 비트랜잭션 컨텍스트에서도 즉시 발행. 장점은 포용성, 단점은 커밋 보장 없음.팀의 이벤트 발행 정책과 FCM 신뢰성 요구사항을 고려하여 선택하시면 됩니다.
| @Bean | ||
| public Queue tokenQueue() { | ||
| return QueueBuilder.durable(qToken).build(); | ||
| } | ||
|
|
||
| @Bean | ||
| public Queue tokensQueue() { | ||
| return QueueBuilder.durable(qTokens).build(); | ||
| } | ||
|
|
||
| @Bean | ||
| public Queue topicQueue() { | ||
| return QueueBuilder.durable(qTopic).build(); | ||
| } |
There was a problem hiding this comment.
DLQ/DLX 미구성 → 영구 실패 격리 불가
현재 큐는 DLX/DLQ가 없습니다. 소비 실패 시 분석/재처리가 어렵습니다.
선택지:
- 큐에 DLX 지정 + DLQ 선언(+ 선택적 지연 재시도용 x-message-ttl).
- 운영에서 중앙 토폴로지로 선언 시, 이 클래스에서는 선언 생략하고 문서화.
예시(간단형):
public Queue tokenQueue() {
- return QueueBuilder.durable(qToken).build();
+ return QueueBuilder.durable(qToken)
+ .withArgument("x-dead-letter-exchange", ex + ".dlx")
+ .withArgument("x-dead-letter-routing-key", qToken + ".dlq")
+ .build();
}토픽/토큰즈 큐도 동일하게 적용하고, DLX/DLQ Declarables 추가 필요. 장점: 실패 격리/재처리 용이. 단점: 브로커 리소스 증가/운영 복잡도. (As per coding guidelines)
Committable suggestion skipped: line range outside the PR's diff.
| @Bean | ||
| public RabbitTemplate rabbitTemplate(ConnectionFactory cf, | ||
| Jackson2JsonMessageConverter converter) { | ||
| RabbitTemplate t = new RabbitTemplate(cf); | ||
| t.setMessageConverter(converter); | ||
| return t; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Publisher Confirm/Returns 활성화로 라우팅/브로커 이슈 조기 감지
현재 RabbitTemplate에 Confirm/Returns 설정이 없습니다. 운영 장애 시 조기 탐지가 어렵습니다.
예시:
@Bean
public RabbitTemplate rabbitTemplate(ConnectionFactory cf,
Jackson2JsonMessageConverter converter) {
RabbitTemplate t = new RabbitTemplate(cf);
t.setMessageConverter(converter);
+ t.setMandatory(true); // unroutable returns 수신
+ // t.setReturnsCallback(returned -> log.warn("Returned: {}", returned));
+ // ConfirmType 설정은 CachingConnectionFactory에서 publisherConfirmType=CORRELATED 로
return t;
}추가로 spring.rabbitmq.publisher-confirm-type=correlated, spring.rabbitmq.publisher-returns=true 설정과 함께 사용하세요.
검증:
🏁 Script executed:
#!/bin/bash
rg -n 'publisher-returns|publisher-confirm' backend/src/main/resources -g '*application*.yml'Length of output: 0
🏁 Script executed:
#!/bin/bash
# 1. Check full RabbitMQConfig.java content
echo "=== RabbitMQConfig.java Content ==="
cat backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java
echo -e "\n=== Search for ConfirmCallback/ReturnsCallback ==="
rg -n 'ConfirmCallback|ReturnsCallback' backend/src --type=java
echo -e "\n=== Search for setMandatory or setReturns ==="
rg -n 'setMandatory|setReturns|setConfirm' backend/src --type=java
echo -e "\n=== Search for RabbitMQ properties in application configs ==="
find backend/src/main/resources -name '*application*' -type f | xargs grep -l 'rabbitmq' 2>/dev/null || echo "No files found"
echo -e "\n=== Show RabbitMQ property contents ==="
find backend/src/main/resources -name '*application*.yml' -o -name '*application*.yaml' -o -name '*application*.properties' | xargs cat 2>/dev/null | grep -A5 -B5 'rabbitmq' || echo "No rabbitmq config found"Length of output: 3699
Publisher Confirm/Returns 설정으로 메시지 전달 보증 추가
검증 결과, 리뷰 코멘트의 우려가 타당함을 확인했습니다.
확인된 현황:
- RabbitMQConfig.java의
rabbitTemplate빈에 Confirm/Returns 설정이 없음 - 모든 application 설정 파일(dev/local/prod/worker)에서
publisher-confirm-type,publisher-returns설정 부재 - 코드베이스 어디에도 ConfirmCallback/ReturnsCallback 구현 없음
왜 필요한가:
토큰 관련 중요 메시지를 취급하면서 현재는 메시지 손실이나 라우팅 실패를 감지할 방법이 없습니다. 운영 중 메시지가 유실되어도 애플리케이션은 알 수 없어 데이터 무결성 문제로 이어질 수 있습니다.
제시된 예시 코드와 추가 설정이 정확하고 권장되는 방식이므로, 해당 내용을 참고하여 아래 두 단계로 구현하세요:
- RabbitTemplate 설정 추가 (예시 코드 참조)
- application 설정에 다음 속성 추가:
spring: rabbitmq: publisher-confirm-type: correlated publisher-returns: true
🤖 Prompt for AI Agents
In backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java
around lines 73-79, the RabbitTemplate bean lacks publisher confirm and returns
handling; update the bean to enable publisher confirms/returns by configuring
the template to use correlating confirms and returns (setMandatory(true)),
register a ConfirmCallback and a ReturnsCallback (or a ConfirmCorrelationData
handler) to surface publish acknowledgements and undeliverable messages, and
ensure application.yml/properties includes
spring.rabbitmq.publisher-confirm-type=correlated and
spring.rabbitmq.publisher-returns=true so broker-level confirms and return
routing are enabled.
| @Bean | ||
| public SimpleRabbitListenerContainerFactory rabbitListenerContainerFactory( | ||
| ConnectionFactory cf, | ||
| @Qualifier("fcmWorkerExecutor") TaskExecutor exec, | ||
| Jackson2JsonMessageConverter converter | ||
| ) { | ||
| var f = new SimpleRabbitListenerContainerFactory(); | ||
| f.setConnectionFactory(cf); | ||
| f.setMessageConverter(converter); | ||
| f.setConcurrentConsumers(4); | ||
| f.setMaxConcurrentConsumers(8); | ||
| f.setPrefetchCount(100); | ||
| f.setTaskExecutor(exec); | ||
| f.setDefaultRequeueRejected(false); | ||
| return f; | ||
| } |
There was a problem hiding this comment.
재시도/격리 경로(DLQ) 없음 → 소비 실패 시 메시지 유실 위험
setDefaultRequeueRejected(false)로 예외가 발생하면 즉시 버려집니다. 외부 FCM 호출은 일시 오류가 잦아 재시도/격리 전략이 필요합니다.
권장(둘 중 택1 또는 병행):
- 컨테이너 레벨 재시도 + 복구기(DLQ 퍼블리셔)
public SimpleRabbitListenerContainerFactory rabbitListenerContainerFactory(...){
var f = new SimpleRabbitListenerContainerFactory();
...
- f.setDefaultRequeueRejected(false);
+ f.setDefaultRequeueRejected(false); // 고의 비재큐
+ f.setAdviceChain(
+ org.springframework.amqp.rabbit.config.RetryInterceptorBuilder
+ .stateless()
+ .maxAttempts(5)
+ .backOffOptions(1000, 2.0, 15000) // 1s → 15s
+ .recoverer(new org.springframework.amqp.rabbit.retry.RejectAndDontRequeueRecoverer())
+ .build()
+ );
return f;
}- 토폴로지에 DLX/DLQ 추가하고, 실패 시 DLQ로 라우팅(아래 RabbitMQConfig 코멘트 참고).
보조 권장:
+ f.setMissingQueuesFatal(false); // 초기 구동 시 큐 미존재 허용장단점:
- 재시도: 일시 오류에 강함, 지연 증가 가능.
- DLQ: 영구 실패 분석 용이, 재처리 파이프라인 필요.
원하시면 RetryAdvice + DLQ 선언 전체 패치 제안 드립니다. (As per coding guidelines)
🤖 Prompt for AI Agents
In backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java
around lines 28-43, the listener factory sets f.setDefaultRequeueRejected(false)
which immediately drops messages on exception; change to enable retries and DLQ
handling by (a) enabling container-level retry (e.g., configure a
RetryOperationsInterceptor or RetryTemplate on the factory) or (b) keep requeue
false but declare and bind a DLX/DLQ for the queue and publish failed messages
to the DLQ via a Recoverer/RepublishMessageRecoverer so transient FCM errors are
retried and permanent failures are routed to a DLQ for analysis; ensure
TaskExecutor and concurrency settings remain and wire in the chosen
retry/recoverer beans and DLX/DLQ declarations in RabbitMQConfig.
| f.setConcurrentConsumers(4); | ||
| f.setMaxConcurrentConsumers(8); | ||
| f.setPrefetchCount(100); | ||
| f.setTaskExecutor(exec); | ||
| f.setDefaultRequeueRejected(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Prefetch/동시성 튜닝 가이드
prefetch=100, concurrent 48은 단일 인스턴스 기준 최대 800개까지 워커에 적재될 수 있습니다. FCM 쿼터/속도제한과 메모리 상황에 맞춰 2050 수준으로 낮추고, 실제 처리량에 따라 상향 조정하는 방식을 권장합니다.
검증:
🏁 Script executed:
#!/bin/bash
# 운영/스테이징 YAML에서 prefetch, concurrency 설정 여부 확인
rg -n 'prefetch|concurrent' backend/src/main/resources -g '*application*.yml'Length of output: 0
더 정확한 검증을 위해 추가 조회가 필요합니다. YAML 파일이 없다는 것은 해당 설정이 하드코딩되어 있거나 다른 위치에 있을 가능성을 시사합니다.
🏁 Script executed:
#!/bin/bash
# 1. resources 디렉토리의 모든 설정 파일 확인
echo "=== All config files in resources ==="
find backend/src/main/resources -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.properties" \) 2>/dev/null | head -20
# 2. RabbitWorkerConfig에서 @Value 또는 @ConfigurationProperties 사용 여부 확인
echo -e "\n=== RabbitWorkerConfig 전체 코드 ==="
cat backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java
# 3. application 설정 파일 기본 경로 확인
echo -e "\n=== Content of application.yml/properties (if exists) ==="
if [ -f backend/src/main/resources/application.yml ]; then
cat backend/src/main/resources/application.yml
elif [ -f backend/src/main/resources/application.properties ]; then
cat backend/src/main/resources/application.properties
else
echo "No default application.yml/properties found"
fiLength of output: 2383
더 정확한 검증을 위해 application-worker.yml 내용을 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
echo "=== Content of application-worker.yml ==="
cat backend/src/main/resources/application-worker.yml
echo -e "\n=== Checking for any RabbitMQ configuration across all profiles ==="
rg -n 'rabbit:|spring\.rabbitmq' backend/src/main/resources
echo -e "\n=== Checking for hardcoded RabbitMQ config patterns in Java ==="
rg -n 'setPrefetchCount|setConcurrentConsumers|setMaxConcurrentConsumers' backend/src/main/javaLength of output: 1253
설정값 하드코딩 제거 및 Prefetch/동시성 튜닝 필요
현재 prefetch, concurrency 설정이 Java 코드에 완전히 하드코딩되어 있어 운영 환경별 튜닝이 불가능합니다.
문제점:
- prefetch=100과 max concurrent=8의 조합으로 최대 800개 메시지까지 워커에 동시 적재 가능
- FCM 쿼터(초당 처리량 제한)와 메모리 부담을 고려하면 과도한 값
- YAML 기반 환경별 조정이 불가능하므로 튜닝마다 재배포 필요
권장 개선 방안:
① @ConfigurationProperties를 이용해 설정 외부화 (가장 권장)
- 장점: 운영/스테이징에서 YAML로 유연하게 조정, 코드 변경 불필요
- 단점: 초기 설정 추가 작업
② 하드코딩 값만 즉시 낮추기
- prefetch: 20~50으로 설정
- 장점: 빠른 적용
- 단점: 나중에 또 조정해야 할 경우 재배포 필요
이유: 메시지 버스 시스템에서 prefetch는 메모리 부담과 처리 효율의 균형점입니다. 높은 값은 일시적 부하에서 메모리 압박을 유발하고 FCM 속도제한에 걸릴 위험이 있습니다. 환경별로 실제 처리량에 맞춰 동적으로 조정하는 것이 운영 안정성의 핵심입니다.
🤖 Prompt for AI Agents
In backend/src/main/java/backend/mulkkam/common/config/RabbitWorkerConfig.java
around lines 37 to 41, the prefetch and concurrency values are hardcoded;
replace them with configurable properties and lower safe defaults: create a
@ConfigurationProperties class (e.g., rabbit.worker.*) with fields for prefetch,
concurrentConsumers, maxConcurrentConsumers and default values (prefetch 20-50,
concurrentConsumers 4, maxConcurrentConsumers 8), inject that config into
RabbitWorkerConfig and use
config.getPrefetch()/getConcurrentConsumers()/getMaxConcurrentConsumers()
instead of hardcoded literals; if you need a quick fix without adding
@ConfigurationProperties, immediately reduce the hardcoded prefetch to ~20–50
and lower max concurrent to reduce memory/FCM quota risk, but prefer the
property-based approach for environment tuning.
| @Slf4j | ||
| @RequiredArgsConstructor | ||
| @Component | ||
| // TODO: 구현 모두 끝내면 @Profile("worker") 추가하기 |
There was a problem hiding this comment.
TODO 주석에 대한 명확한 계획이 필요합니다.
현재 @Profile("worker") 없이 모든 프로필에서 FcmClient가 활성화되는데, 이는 의도하지 않은 동작을 유발할 수 있습니다.
잠재적 문제:
- Worker 외 프로필(web, dev 등)에서도 FCM 클라이언트가 로드되어 불필요한 리소스 사용
NotificationConsumer는 worker 프로필 전용인데, 이를 호출하는FcmClient는 모든 프로필에서 활성화되는 불일치
해결 방안:
PR 체크리스트에 "패키지 위치 수정"이 포함되어 있으므로, 다음 중 하나를 선택하세요:
- 즉시
@Profile("worker")를 추가하고 TODO 제거 FcmClient를 worker 전용 패키지로 이동- Web과 Worker에서 모두 사용해야 한다면, 호출 계층을 분리하여 각 프로필별로 다른 컴포넌트가 사용하도록 설계
이 부분을 구현하는 데 도움이 필요하시면 말씀해주세요. 프로필 분리 전략에 대한 구체적인 코드를 제안해드릴 수 있습니다.
🤖 Prompt for AI Agents
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmClient.java
around line 20: The TODO notes adding @Profile("worker") but currently the
FcmClient is loaded in all profiles causing mismatch with worker-only
NotificationConsumer; fix by adding @Profile("worker") to the FcmClient class
and remove the TODO comment so it only registers in the worker profile, and
update the PR checklist/commit message to reflect this change; alternatively if
you prefer package-based separation, move FcmClient to the worker-only package
and ensure no other profiles import it, or implement an interface with two
profile-specific implementations (worker annotated with @Profile("worker") and a
no-op or web implementation for other profiles) — pick one approach and apply it
consistently, updating tests and DI wiring accordingly.
| import org.springframework.transaction.event.TransactionPhase; | ||
| import org.springframework.transaction.event.TransactionalEventListener; | ||
|
|
||
| // TODO: MQ 구현 완료되는 시점에 제거 |
There was a problem hiding this comment.
TODO 주석: 전환 전략을 더 구체적으로 명문화해주세요.
MQ 구현 완료 시점까지 이 클래스가 병행 운영된다는 뜻인데, 기존의 TransactionalEventListener 방식과 새로운 RabbitMQ 기반 방식이 동시에 작동하면서 동일한 FCM 메시지가 중복 발송될 위험이 있습니다.
특히 NotificationMQProducer와 FcmEventListener가 같은 이벤트를 구독하고 있다면, 같은 요청에 대해 메시지가 두 번 처리될 가능성을 배제할 수 없습니다. 제안하는 전환 전략:
선택지 1 (점진적 마이그레이션):
- Feature flag나 환경 설정으로 어떤 방식을 사용할지 제어
- 장점: 문제 발생 시 빠른 롤백 가능
- 단점: 코드 복잡도 증가
선택지 2 (이벤트 분리):
- RabbitMQ로 이동할 이벤트와 유지할 이벤트를 명확히 구분
- 예: SendMessageByFcmTopicRequest → RabbitMQ (NonBlockingFcmEvent로 변경)
- 장점: 명확한 책임 분리
- 단점: 이벤트 타입 증가
TODO 주석에 담당자, 예상 완료 일시, 선택한 전환 전략을 명시하면 향후 리뷰와 관리가 훨씬 수월할 것 같습니다.
🤖 Prompt for AI Agents
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/FcmEventListener.java
around line 13: replace the vague TODO with a concrete migration plan that
states the chosen transition strategy (either "feature-flag controlled gradual
migration" or "event-type separation into RabbitMQ-only events"), the
responsible owner, an expected completion date, and the exact
rollback/mitigation for duplicate sends (e.g., enable/disable via env flag
and/or enforce idempotency token or disable old listener when feature flag ON).
Also update the comment to mention which event types will remain on
TransactionalEventListener vs move to RabbitMQ, and add a reference to any
config key or feature-flag name to be used to toggle behavior during rollout.
| // 단일 토큰 기반 발송 | ||
| @RabbitListener( | ||
| queues = "${mulkkam.mq.queue.token}", | ||
| containerFactory = "rabbitListenerContainerFactory" | ||
| ) | ||
| public void onToken(SendMessageByFcmTokenRequest req) { | ||
| fcmClient.sendMessageByToken(req); | ||
| } | ||
|
|
There was a problem hiding this comment.
단일 토큰 소비: 실패 처리/관찰성 보강
예외가 발생하면 컨테이너 설정상 재큐 없이 폐기됩니다. 일시 오류(Firebase 500/UNAVAILABLE 등) 분류 재시도 또는 DLQ 격리가 필요하고, 최소한의 구조적 로그가 있으면 운영 가시성이 좋아집니다.
예시(재시도 어드바이스 전제, 실패 시 예외 전파):
@RabbitListener(queues = "${mulkkam.mq.queue.token}", containerFactory = "rabbitListenerContainerFactory")
public void onToken(SendMessageByFcmTokenRequest req) {
- fcmClient.sendMessageByToken(req);
+ try {
+ fcmClient.sendMessageByToken(req);
+ // log.info("FCM token sent: token={}, title={}", mask(req.token()), req.title());
+ } catch (TransientFcmException e) {
+ throw e; // 재시도 유도
+ } catch (Exception e) {
+ // 영구 실패라면 AmqpRejectAndDontRequeueException 고려
+ throw e;
+ }
}참고: 토큰은 PII 성격이 있으니 로그 시 마스킹 필수. (As per coding guidelines)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationConsumer.java
around lines 18 to 26, the token consumer currently calls
fcmClient.sendMessageByToken(req) with no error handling or observability; wrap
the send in a try/catch that (1) classifies errors into transient (e.g.,
Firebase 5xx, UNAVAILABLE, INTERNAL/UNKNOWN network issues) and permanent
(invalid token, 4xx) and rethrows transient errors so the container/broker can
retry or route to DLQ, (2) for permanent failures swallow the exception but emit
a structured log containing operation, maskedToken (mask all but last 4 chars),
messageId or request id, and the error type/message, and (3) use the existing
logging framework (no PII in logs) and ensure any rethrown exception is a
runtime exception so container retry semantics apply.
| private static final int FCM_BATCH_SIZE = 500; | ||
|
|
||
| public void publishToken(SendMessageByFcmTokenRequest req) { | ||
| rabbitTemplate.convertAndSend(exchange, "notify.token", req); | ||
| } | ||
|
|
||
| public void publishTokens(SendMessageByFcmTokensRequest req) { | ||
| for (List<String> part : Lists.partition(req.allTokens(), FCM_BATCH_SIZE)) { | ||
| rabbitTemplate.convertAndSend(exchange, "notify.tokens", req.withTokens(part)); | ||
| } | ||
| } | ||
|
|
||
| public void publishTopic(SendMessageByFcmTopicRequest req) { | ||
| rabbitTemplate.convertAndSend(exchange, "notify.topic", req); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
라우팅키/배치 크기 하드코딩 → 설정화 및 가드 필요
문자열 상수("notify.*")가 여기와 RabbitMQConfig 양쪽에 중복돼 드리프트 위험이 있습니다. 또한 토큰 리스트가 비어있을 때 불필요한 MQ 송신을 피하는 가드가 있으면 좋습니다.
@RequiredArgsConstructor
@Component
public class NotificationMQProducer {
...
- private static final int FCM_BATCH_SIZE = 500;
+ @Value("${mulkkam.fcm.batch-size:500}")
+ private int batchSize;
+
+ @Value("${mulkkam.mq.routing.token:notify.token}")
+ private String rkToken;
+ @Value("${mulkkam.mq.routing.tokens:notify.tokens}")
+ private String rkTokens;
+ @Value("${mulkkam.mq.routing.topic:notify.topic}")
+ private String rkTopic;
public void publishToken(SendMessageByFcmTokenRequest req) {
- rabbitTemplate.convertAndSend(exchange, "notify.token", req);
+ rabbitTemplate.convertAndSend(exchange, rkToken, req);
}
public void publishTokens(SendMessageByFcmTokensRequest req) {
- for (List<String> part : Lists.partition(req.allTokens(), FCM_BATCH_SIZE)) {
- rabbitTemplate.convertAndSend(exchange, "notify.tokens", req.withTokens(part));
+ var all = req.allTokens();
+ if (all == null || all.isEmpty()) return;
+ for (List<String> part : Lists.partition(all, batchSize)) {
+ rabbitTemplate.convertAndSend(exchange, rkTokens, req.withTokens(part));
}
}
public void publishTopic(SendMessageByFcmTopicRequest req) {
- rabbitTemplate.convertAndSend(exchange, "notify.topic", req);
+ rabbitTemplate.convertAndSend(exchange, rkTopic, req);
}
}추가 검증 스크립트(하드코딩된 "notify." 사용 위치 파악):
🏁 Script executed:
#!/bin/bash
rg -n '"notify\.(token|tokens|topic)"' --type=javaLength of output: 1064
</shell_scripts>
🏁 Script executed:
#!/bin/bash
# Verify RabbitMQConfig.java structure around the routing key bindings
cat backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.java | sed -n '50,70p'Length of output: 643
🏁 Script executed:
#!/bin/bash
# Find SendMessageByFcmTokensRequest class to verify methods
ast-grep --pattern 'class SendMessageByFcmTokensRequest {
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for any other usages of these routing keys beyond these two files
rg '"notify\.' --type=java -g '!RabbitMQConfig.java' -g '!NotificationMQProducer.java' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify if there are any tests or listeners using these routing keys
rg 'notify\.(token|tokens|topic)' --type=java | grep -i -E '(listener|consumer|queue|binding)' | head -10Length of output: 514
🏁 Script executed:
#!/bin/bash
# Check if there are constants defined elsewhere for these routing keys
rg -n 'static final.*notify' --type=javaLength of output: 0
🏁 Script executed:
#!/bin/bash
# Verify NotificationMQProducer full context for method signatures
cat backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.javaLength of output: 1394
🏁 Script executed:
#!/bin/bash
# Find SendMessageByFcmTokensRequest definition
fd -e java -x grep -l 'class SendMessageByFcmTokensRequest' {}Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the SendMessageByFcmTokensRequest source
find . -name '*.java' -exec grep -l 'class SendMessageByFcmTokensRequest' {} \;Length of output: 0
🏁 Script executed:
#!/bin/bash
# More targeted search for the request class
rg -n 'class SendMessageByFcmTokensRequest' --type=java -A 20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if configuration properties are already defined somewhere
rg 'mulkkam\.(fcm|mq)' --type=javaLength of output: 1027
🏁 Script executed:
#!/bin/bash
# Look for properties files with configuration
fd -e properties -o -e yml -o -e yaml | xargs grep -l 'mulkkam' 2>/dev/null | head -5Length of output: 116
🏁 Script executed:
#!/bin/bash
# Check application properties/yaml files
find . -type f \( -name 'application*.properties' -o -name 'application*.yml' -o -name 'application*.yaml' \) -exec echo "File: {}" \; -exec head -50 {} \;Length of output: 5827
🏁 Script executed:
#!/bin/bash
# Check if there are any guards already in place
rg -n 'allTokens.*isEmpty\|isEmpty.*allTokens' --type=javaLength of output: 0
🏁 Script executed:
#!/bin/bash
# Verify import statements and dependencies in NotificationMQProducer
head -20 backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.javaLength of output: 795
🏁 Script executed:
#!/bin/bash
# Check Lists usage (from Guava)
rg -n 'import.*Lists' --type=java backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/Length of output: 277
🏁 Script executed:
#!/bin/bash
# Verify complete structure of request classes
rg -n 'class SendMessage.*Request' --type=java -A 10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are other MQ producers following different patterns
find backend -name '*Producer.java' -o -name '*Publisher.java' | head -5Length of output: 99
🏁 Script executed:
#!/bin/bash
# Final check: confirm routing key strings are ONLY in these two files
rg -l '"notify\.(token|tokens|topic)"' --type=javaLength of output: 171
🏁 Script executed:
#!/bin/bash
# Check if batchSize or similar config already exists
rg -n 'FCM_BATCH|fcm\.batch|batch-size' --type=java --type=properties --type=yamlLength of output: 34
🏁 Script executed:
#!/bin/bash
# Inspect test files for routing key usage
find . -path '*test*' -name '*.java' | xargs rg '"notify\.(token|tokens|topic)"' 2>/dev/null | head -5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the full NotificationMQProducer to understand the current implementation
rg -n 'class NotificationMQProducer' --type=java -A 50Length of output: 3122
🏁 Script executed:
#!/bin/bash
# Check RabbitTemplate convertAndSend signatures to ensure the suggested changes are compatible
rg -n 'rabbitTemplate' --type=java -B2 -A2 backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.javaLength of output: 735
🏁 Script executed:
#!/bin/bash
# Verify the complete RabbitMQConfig to understand queue/binding patterns
cat backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.javaLength of output: 2348
🏁 Script executed:
#!/bin/bash
# Check if there are integration tests that could be affected
find . -path '*test*' -name '*RabbitMQ*' -o -path '*test*' -name '*Notification*Producer*'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check environment-specific configuration files
find . -name 'application*.properties' -o -name 'application*.yml' | xargs ls -la 2>/dev/nullLength of output: 682
🏁 Script executed:
#!/bin/bash
# Look for @ConfigurationProperties usage in the codebase
rg -n '@ConfigurationProperties\|@Value.*notify\|@Value.*fcm' --type=java | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check common configuration patterns used in backend
rg -n '@Value.*{.*}' --type=java | head -10Length of output: 103
🏁 Script executed:
#!/bin/bash
# Verify if @Value with default values is already used elsewhere
rg -n '@Value.*:' --type=java | head -15Length of output: 471
🏁 Script executed:
#!/bin/bash
# Final confirmation: list all Java files that reference routing keys
rg -l 'notify\.' --type=javaLength of output: 171
🏁 Script executed:
#!/bin/bash
# Check the actual structure and methods in SendMessageByFcmTokensRequest
find . -name '*.java' -type f | xargs grep -l 'SendMessageByFcmTokensRequest' | xargs cat | grep -A 30 'class SendMessageByFcmTokensRequest'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Ensure the suggested empty check makes sense - verify request object nullability
rg -n 'SendMessageByFcmTokensRequest\|allTokens\|withTokens' --type=java -B2 -A2 | head -40Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check Lists.partition behavior with empty/null
rg -n 'Lists\.partition' --type=java -B3 -A3Length of output: 2064
🏁 Script executed:
#!/bin/bash
# Find where these request DTOs are instantiated
rg -n 'new SendMessageByFcmTokens\|new SendMessageByFcmToken' --type=java | head -10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Summary check - list all references to these routing keys one more time with context
rg -n 'notify\.(token|tokens|topic)' --type=java -B2 -A2 | head -50Length of output: 3669
🏁 Script executed:
#!/bin/bash
# Check if there are any existing constants for magic strings
rg -n 'private static final.*String.*=' --type=java backend/src/main/java/backend/mulkkam/common/ | head -15Length of output: 923
🏁 Script executed:
#!/bin/bash
# Verify @Value injection style matches team convention
rg -n '@Value' --type=java -B1 -A1 backend/src/main/java/backend/mulkkam/common/ | head -30Length of output: 2560
🏁 Script executed:
#!/bin/bash
# Check if application configuration already includes FCM or MQ settings
find . \( -name '*.properties' -o -name '*.yml' -o -name '*.yaml' \) | xargs grep -l 'fcm\|mq\|rabbit' 2>/dev/nullLength of output: 249
🏁 Script executed:
#!/bin/bash
# Look at team patterns for externalized configuration
rg -n 'spring.rabbitmq\|amqp\|queue' --type=properties --type=yaml | head -20Length of output: 34
🏁 Script executed:
#!/bin/bash
# Validate that the suggested property names follow conventions
rg -n 'mulkkam\.' --type=yaml --type=properties 2>/dev/null | head -15Length of output: 0
🏁 Script executed:
#!/bin/bash
# Final: ensure our understanding of the issue is correct
echo "=== NotificationMQProducer routing keys ===" && \
rg -n '"notify\.(token|tokens|topic)"' --type=java backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java && \
echo "" && \
echo "=== RabbitMQConfig routing keys ===" && \
rg -n '"notify\.(token|tokens|topic)"' --type=java backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.javaLength of output: 629
🏁 Script executed:
#!/bin/bash
# Check if there are any existing listener/consumer classes using these routing keys
find . -name '*Listener.java' -o -name '*Consumer.java' | xargs grep -l 'notify\.' 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Verify the team uses @Component for MQ producers
rg -n '@Component.*Producer\|@Component' --type=java backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/ -B2 -A2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check how rabbitTemplate is injected
rg -n 'rabbitTemplate' --type=java -B5 backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/ | head -20Length of output: 2568
🏁 Script executed:
#!/bin/bash
# Final validation: look at the complete diff context and confirm it's viable
cat backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java | head -50Length of output: 1394
🏁 Script executed:
#!/bin/bash
# Check if batch size is used anywhere else
rg -n 'FCM_BATCH_SIZE\|500' --type=java backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify no other references to FCM_BATCH_SIZE or similar constant
rg -n 'BATCH_SIZE' --type=java backend/Length of output: 669
🏁 Script executed:
#!/bin/bash
# Double check: is the empty guard necessary? Look at calling code
rg -n 'publishTokens\(' --type=java -B3 -A3Length of output: 1859
🏁 Script executed:
#!/bin/bash
# Check consumer/listener implementation
find . -path '*fcm*' -name '*.java' | xargs grep -l 'Listener\|Consumer' 2>/dev/null | head -5Length of output: 299
🏁 Script executed:
#!/bin/bash
# Final comprehensive check to ensure we haven't missed anything
echo "Total files with notify routing keys:" && \
rg '"notify\.' --type=java | cut -d: -f1 | sort -u | wc -l && \
echo "" && \
echo "Files:" && \
rg '"notify\.' --type=java | cut -d: -f1 | sort -uLength of output: 219
🏁 Script executed:
#!/bin/bash
# Check if there's documentation about these constants
rg -n 'notify\.token\|notify\.tokens\|notify\.topic' --type=markdown -o -name '*.md'Length of output: 422
🏁 Script executed:
#!/bin/bash
# Ensure we can actually find and view both files in full
echo "=== File 1: NotificationMQProducer.java ===" && \
wc -l backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java && \
echo "" && \
echo "=== File 2: RabbitMQConfig.java ===" && \
wc -l backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.javaLength of output: 258
🏁 Script executed:
#!/bin/bash
# Get complete NotificationMQProducer
cat backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.javaLength of output: 1394
🏁 Script executed:
#!/bin/bash
# Get complete RabbitMQConfig
cat backend/src/main/java/backend/mulkkam/common/config/RabbitMQConfig.javaLength of output: 2348
🏁 Script executed:
#!/bin/bash
# Find SendMessageByFcmTokensRequest and related request classes
find . -name '*.java' -type f | xargs grep -l 'SendMessageByFcm.*Request' | head -10Length of output: 1004
🏁 Script executed:
#!/bin/bash
# Get SendMessageByFcmTokensRequest definition
find . -name '*SendMessage*.java' | xargs cat | head -100Length of output: 2006
🏁 Script executed:
#!/bin/bash
# Check for any record/dto pattern files
fd -e java | xargs grep -l 'SendMessageByFcmTokensRequest' | head -3Length of output: 294
🏁 Script executed:
#!/bin/bash
# Look for request DTO files in common infrastructure
find backend/src/main/java/backend/mulkkam/common -name '*Request*.java' | head -10Length of output: 420
🏁 Script executed:
#!/bin/bash
# Find FCM-related DTOs
find backend/src/main/java/backend/mulkkam -path '*fcm*' -name '*.java' -type fLength of output: 1161
🏁 Script executed:
#!/bin/bash
# Get all FCM infrastructure files to understand the structure
ls -la backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/Length of output: 326
🏁 Script executed:
#!/bin/bash
# Check subdirectories
find backend/src/main/java/backend/mulkkam/common/infrastructure/fcm -name '*.java' -type fLength of output: 1161
🏁 Script executed:
#!/bin/bash
# Get content of all FCM-related files
find backend/src/main/java/backend/mulkkam/common/infrastructure/fcm -name '*.java' -type f -exec echo "=== {} ===" \; -exec cat {} \;Length of output: 14410
라우팅키/배치 크기 하드코딩 → 설정화 필수이며 RabbitMQConfig도 함께 수정 필요
리뷰 의도는 정확하나 일부 중요한 점을 보완해야 합니다:
-
라우팅키 드리프트 위험 실제 확인됨: 문자열 상수가 NotificationMQProducer.java와 RabbitMQConfig.java에 중복되어 있어, 한쪽만 변경하면 바인딩 실패가 발생합니다. 특히 RabbitMQConfig.java의 Binding 메서드들(lines 55, 60, 65)도 동일하게 외부화해야 합니다.
-
설정 파일에 키가 이미 존재: application-prod.yml과 application-worker.yml에
mulkkam.mq.routing하위에 해당 키들이 이미 정의되어 있으므로, 제안된 설정명을 그대로 사용하면 됩니다. 팀 컨벤션과도 일치합니다. -
배치 크기 중복: FCM_BATCH_SIZE = 500이 NotificationMQProducer.java(line 22)와 FcmEventListener.java(line 19) 두 곳에 중복 정의되어 있습니다. 이 역시 외부화하여 통일해야 변경 시 일관성이 유지됩니다.
-
공수 측면의 제안:
- RabbitMQConfig.java의 세 Binding 메서드도 함께 수정
- 배치 크기를
${mulkkam.fcm.batch-size:500}(또는${mulkkam.mq.batch-size:500})로 통합 - 제안된 diff에 RabbitMQConfig 수정분도 추가 필요
-
빈 리스트 가드의 의의: Lists.partition()이 공 리스트를 잘 처리하지만, 불필요한 MQ 송신을 명시적으로 방지하는 것이 좋은 방어 로직입니다.
// RabbitMQConfig.java도 함께 수정
@Bean
public Binding bindToken() {
- return BindingBuilder.bind(tokenQueue()).to(notificationsExchange()).with("notify.token");
+ return BindingBuilder.bind(tokenQueue()).to(notificationsExchange()).with(rkToken);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
backend/src/main/java/backend/mulkkam/common/infrastructure/fcm/service/NotificationMQProducer.java
around lines 22 to 36, routing keys and batch size are hardcoded and duplicated
elsewhere; change to read routing keys from the existing application config path
(mulkkam.mq.routing.<token|tokens|topic>) and read batch size from a shared
property (e.g. mulkkam.fcm.batch-size or mulkkam.mq.batch-size with default
500), add a guard to skip sending when partitioned token list is empty, and
remove the FCM_BATCH_SIZE constant; concurrently update RabbitMQConfig.java
binding methods (around lines 55, 60, 65) to use the same config keys instead of
string literals so bindings and producers stay in sync, and remove the duplicate
batch-size constant in FcmEventListener.java so both classes use the shared
property.
| rabbitmq: | ||
| host: ${MQ_HOST} | ||
| port: 5672 | ||
| username: ${MQ_USER} | ||
| password: ${MQ_PASSWORD} |
There was a problem hiding this comment.
로컬 환경에 MQ 토폴로지 설정이 누락되었습니다.
다른 환경 설정 파일들(dev, prod, worker, ci)에는 mulkkam.mq 블록이 정의되어 있지만, local 프로필에는 해당 설정이 없습니다. 이로 인해 로컬 환경에서 RabbitMQ 관련 기능을 테스트할 때 exchange, queue, routing 정보를 찾지 못해 런타임 에러가 발생할 수 있습니다.
다음과 같이 토폴로지 설정을 추가하세요:
rabbitmq:
host: ${MQ_HOST}
port: 5672
username: ${MQ_USER}
password: ${MQ_PASSWORD}
+
+# MQ 토폴로지
+mulkkam:
+ mq:
+ exchange: notifications.ex
+ queue:
+ token: notifications.token.q
+ tokens: notifications.tokens.q
+ topic: notifications.topic.q
+ routing:
+ token: notify.token
+ tokens: notify.tokens
+ topic: notify.topic
app:
logging:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rabbitmq: | |
| host: ${MQ_HOST} | |
| port: 5672 | |
| username: ${MQ_USER} | |
| password: ${MQ_PASSWORD} | |
| rabbitmq: | |
| host: ${MQ_HOST} | |
| port: 5672 | |
| username: ${MQ_USER} | |
| password: ${MQ_PASSWORD} | |
| # MQ 토폴로지 | |
| mulkkam: | |
| mq: | |
| exchange: notifications.ex | |
| queue: | |
| token: notifications.token.q | |
| tokens: notifications.tokens.q | |
| topic: notifications.topic.q | |
| routing: | |
| token: notify.token | |
| tokens: notify.tokens | |
| topic: notify.topic |
🤖 Prompt for AI Agents
In backend/src/main/resources/application-local.yml around lines 29 to 33, the
local profile is missing the mulkkam.mq topology block present in other
profiles; add a mulkkam.mq section that defines the same exchanges, queues, and
bindings used in dev/prod/worker/ci so local RabbitMQ setup mirrors other
environments (include exchange names, queue names, routing keys and any
durable/auto-delete properties), and ensure the values reference the same
property names or literals as in the other files so local runs can create and
discover the required topology.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
2 participants
🔗 관련 이슈
체크리스트
Summary by CodeRabbit