Merge/noti kafak into develop - #84
Conversation
![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
This pull request introduces Kafka integration for notifications, including infrastructure setup in Docker, configuration in Spring, and updating event DTOs for JSON serialization. Feedback focuses on maintaining architectural consistency by using ApplicationEventPublisher instead of direct KafkaTemplate calls to preserve transactional integrity. Additionally, the reviewer recommends externalizing hardcoded configurations via @value, separating Kafka settings into a dedicated configuration class, and removing legacy listener code that was left commented out.
| import com.catchtable.reservation.entity.ReservationStatus; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.context.ApplicationEventPublisher; | ||
| import org.springframework.kafka.core.KafkaTemplate; |
There was a problem hiding this comment.
KafkaTemplate을 직접 사용하는 대신 기존의 ApplicationEventPublisher를 사용하는 것이 좋습니다. NotificationKafkaPublisher가 이미 ApplicationEvent를 구독하여 트랜잭션 커밋 후 Kafka 메시지를 발행하는 역할을 수행하고 있으므로, 이를 활용하여 일관성을 유지하십시오.
| import org.springframework.kafka.core.KafkaTemplate; | |
| import org.springframework.context.ApplicationEventPublisher; |
|
|
||
| private final PaymentRepository paymentRepository; | ||
| private final ApplicationEventPublisher eventPublisher; | ||
| private final KafkaTemplate<String, Object> kafkaTemplate; |
There was a problem hiding this comment.
KafkaTemplate 대신 ApplicationEventPublisher를 주입받도록 수정하십시오. 이는 트랜잭션 정합성을 보장하기 위함입니다.
| private final KafkaTemplate<String, Object> kafkaTemplate; | |
| private final ApplicationEventPublisher eventPublisher; |
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.kafka.annotation.EnableKafka; | ||
| import org.springframework.kafka.config.ConcurrentKafkaListenerContainerFactory; | ||
| import org.springframework.kafka.core.*; |
There was a problem hiding this comment.
@value 어노테이션을 사용하여 설정을 외부화하기 위해 필요한 임포트를 추가하십시오.
| import org.springframework.kafka.core.*; | |
| import org.springframework.beans.factory.annotation.Value; | |
| import org.springframework.kafka.core.*; |
|
|
||
| @EnableKafka | ||
| @Configuration | ||
| public static class KafkaConfig { |
There was a problem hiding this comment.
KafkaConfig가 SchedulerConfig의 내부 클래스로 정의되어 있습니다. 카프카 설정과 스케줄러 설정은 서로 다른 관심사이므로, 별도의 설정 클래스로 분리하여 관리하는 것이 유지보수 측면에서 바람직합니다.
| @Configuration | ||
| public static class KafkaConfig { | ||
|
|
||
| private final String bootstrapServers = "localhost:9092"; |
There was a problem hiding this comment.
bootstrapServers 주소가 하드코딩되어 있습니다. 환경별로 유연하게 관리할 수 있도록 @value를 사용하여 application.yml 설정 파일에서 값을 가져오도록 수정하십시오.
| private final String bootstrapServers = "localhost:9092"; | |
| @Value("${spring.kafka.bootstrap-servers:localhost:9092}") | |
| private String bootstrapServers; |
| log.info("[예약 취소 알림] userId: {}, reservationId: {} 알림 생성 완료", event.getUserId(), event.getReservationId()); | ||
| } | ||
| } | ||
| //package com.catchtable.notification.listener; |
There was a problem hiding this comment.
카프카 도입으로 인해 더 이상 사용하지 않는 기존 리스너 코드가 주석 처리된 채로 남아 있습니다. 코드 베이스의 청결함을 위해 주석 처리된 파일은 삭제하거나 내용을 제거하는 것을 권장합니다. (다른 리스너 파일들도 동일하게 적용됩니다.)
2 participants
📢 기능 설명
필요시 실행결과 스크린샷 첨부
연결된 issue
연결된 issue를 자동을 닫기 위해 아래 {이슈넘버}를 입력해주세요.
close #{이슈넘버}
✅ 체크리스트