SCRUM-117 식당 주인은 사용자가 예약을 하면 해당 알림을 받을 수 있다#6
SCRUM-117 식당 주인은 사용자가 예약을 하면 해당 알림을 받을 수 있다#6unifolio0 merged 33 commits intoTeam-soft-macaron:developfrom unifolio0:SCRUM-117
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughKafka 기반 알림 소비·전달 흐름과 WebPush 패키지 재구성, 알림 도메인/레포지토리/엔티티/DTO 추가, 메일·카프카 의존성 및 환경설정·스키마 확장, dev용 CI/CD 워크플로우와 배포 스크립트 및 관련 테스트/임포트 경로 갱신을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Kafka as Kafka Broker
participant NC as NotificationConsumer
participant NER as NotificationEnabledRepository
participant NS as NotificationSender
participant S as Sender (WebPushSender 등)
participant PSR as PushSubscriptionRepository
participant Push as Web Push Provider
rect rgba(215,235,255,0.4)
Kafka->>NC: NotificationMessage
NC->>NER: findByUserIdAndType(userId, type)
NER-->>NC: List<NotificationEnabled>
NC->>NS: send(message, enables)
loop 각 enabled 채널
NS->>S: 채널에 맞는 Sender 선택
alt WEB_PUSH
S->>PSR: findAllByUserId(userId)
PSR-->>S: 구독 목록
S->>Push: push payload 전송 (구독별)
else 기타 채널
S-->>NS: 채널별 전송 수행
end
end
end
sequenceDiagram
autonumber
actor Dev as Developer (push to develop)
participant CI as dev-ci Workflow
participant CD as dev-cd Workflow
participant Art as GitHub Artifacts
participant VM as Deploy Server
participant Sh as replace-new-version.sh
Dev->>CI: push / PR to develop
CI-->>CI: Build & Test (MySQL/ZooKeeper/Kafka)
CI-->>CD: workflow_call (test job)
CD->>CD: Build bootJar (JDK21)
CD->>Art: Upload app.jar & deploy scripts
CD->>VM: Download artifacts
CD->>Sh: Execute replace-new-version.sh
Sh-->>VM: stop old process, start new app.jar (dev profile)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (12)
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 |
Summary of ChangesHello @unifolio0, 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! 이 Pull Request는 사용자가 예약을 생성할 때 식당 주인에게 알림을 전송하는 기능을 구현하기 위해 포괄적인 알림 시스템을 도입합니다. 주요 변경 사항으로는 Kafka를 활용한 메시지 처리, WebPush 모듈의 리팩토링 및 확장성 개선, 알림 설정 및 이력 관리를 위한 새로운 데이터 모델 추가, 그리고 개발 환경 배포 스크립트 도입 등이 있습니다. 이로써 시스템은 다양한 알림 채널을 유연하게 지원하고, 알림 전송 과정을 효율적으로 관리할 수 있게 됩니다. Highlights
Ignored Files
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
|
There was a problem hiding this comment.
Code Review
이 PR은 Kafka를 사용하여 알림 시스템을 도입하는 중요한 변경 사항을 포함하고 있습니다. 전반적인 아키텍처는 좋지만, 병합하기 전에 반드시 수정해야 할 몇 가지 심각한(critical) 및 높은(high) 심각도의 문제가 발견되었습니다. 주요 문제점으로는 Kafka 역직렬화 설정 오류, NotificationEnabled 엔티티의 기본 키 설계 결함, 그리고 Kafka 컨슈머가 중단될 수 있는 잠재적 위험이 있습니다. 또한 보안, 안정성, 코드 중복성 측면에서 개선할 수 있는 부분들도 함께 제안했습니다.
| props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); | ||
|
|
||
| props.put(JsonDeserializer.USE_TYPE_INFO_HEADERS, false); | ||
| props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.consumer.dto"); |
There was a problem hiding this comment.
TRUSTED_PACKAGES에 설정된 패키지 경로가 잘못되었습니다. com.wellmeet.consumer.dto가 아닌 com.wellmeet.notification.consumer.dto가 올바른 경로입니다. 이 오류로 인해 Kafka 메시지 역직렬화에 실패하여 애플리케이션이 정상적으로 동작하지 않을 것입니다.
| props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.consumer.dto"); | |
| props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.notification.consumer.dto"); |
| @Id | ||
| private String userId; |
src/main/resources/schema.sql
Outdated
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS notification_enabled ( | ||
| user_id VARCHAR(255) PRIMARY KEY, |
| sudo nohup java \ | ||
| -Dspring.profiles.active=dev \ | ||
| -Duser.timezone=Asia/Seoul \ | ||
| -Dserver.port=8080 \ | ||
| -Ddd.service=wellmeet-notification \ | ||
| -Ddd.env=dev \ | ||
| -jar "$JAR_FILE" > "$APP_LOG_FILE" 2>&1 & |
There was a problem hiding this comment.
Java 애플리케이션을 실행하기 위해 sudo를 사용하는 것은 보안상 위험합니다. 애플리케이션은 최소한의 권한으로 실행되어야 합니다. 8080 포트는 1024보다 크므로 루트 권한이 필요하지 않습니다. 특별한 이유가 없다면 sudo 없이 ubuntu와 같은 일반 사용자 권한으로 애플리케이션을 실행하는 것이 좋습니다.
| sudo nohup java \ | |
| -Dspring.profiles.active=dev \ | |
| -Duser.timezone=Asia/Seoul \ | |
| -Dserver.port=8080 \ | |
| -Ddd.service=wellmeet-notification \ | |
| -Ddd.env=dev \ | |
| -jar "$JAR_FILE" > "$APP_LOG_FILE" 2>&1 & | |
| nohup java \ | |
| -Dspring.profiles.active=dev \ | |
| -Duser.timezone=Asia/Seoul \ | |
| -Dserver.port=8080 \ | |
| -Ddd.service=wellmeet-notification \ | |
| -Ddd.env=dev \ | |
| -jar "$JAR_FILE" > "$APP_LOG_FILE" 2>&1 & |
| public void send(NotificationMessage message, List<NotificationEnabled> enables) { | ||
| for (NotificationEnabled enabled : enables) { | ||
| notificationHistoryRepository.save(new NotificationHistory(message.getNotification().getRecipient())); | ||
| Sender sender = senders.stream() | ||
| .filter(low -> low.isEnabled(enabled.getChannel())) | ||
| .findFirst() | ||
| .orElseThrow(); | ||
| sender.send(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
send 메서드에 몇 가지 개선이 필요합니다:
notificationHistoryRepository.save()가 루프 내에 있어 사용자의 각 활성화된 알림 채널에 대해 중복된 히스토리 레코드를 생성합니다. 알림 메시지당 하나의 히스토리만 기록하는 것이 의도라면 이 코드는 루프 밖으로 이동해야 합니다..orElseThrow()를 인수 없이 사용하면NoSuchElementException이 발생하여 Kafka 컨슈머가 중단되고 동일한 메시지를 계속 재시도할 수 있습니다. 해당 채널에 대한Sender가 없는 경우를 우아하게 처리하는 것이 좋습니다(예: 경고 로깅).filter의 람다 파라미터 이름low는 의미가 명확하지 않습니다.sender와 같이 더 설명적인 이름을 사용하는 것이 좋습니다.
public void send(NotificationMessage message, List<NotificationEnabled> enables) {
if (enables.isEmpty()) {
return;
}
notificationHistoryRepository.save(new NotificationHistory(message.getNotification().getRecipient()));
for (NotificationEnabled enabled : enables) {
senders.stream()
.filter(sender -> sender.isEnabled(enabled.getChannel()))
.findFirst()
.ifPresent(sender -> sender.send(message));
}
}| @Getter | ||
| @Entity | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class NotificationHistory { |
There was a problem hiding this comment.
| public void send(NotificationMessage message) { | ||
| List<PushSubscription> subscriptions = pushSubscriptionRepository.findByUserId( | ||
| message.getNotification().getRecipient()); | ||
| if (subscriptions.isEmpty()) { | ||
| throw new WellMeetNotificationException(ErrorCode.SUBSCRIPTION_NOT_FOUND); | ||
| } | ||
|
|
||
| subscriptions.forEach(subscription -> { | ||
| Keys keys = new Keys(subscription.getP256dh(), subscription.getAuth()); | ||
| Subscription sub = new Subscription(subscription.getEndpoint(), keys); | ||
| Map<String, Object> notificationPayload = getNotificationPayload(message); | ||
| webPushSend(notificationPayload, sub); | ||
| }); | ||
| } |
There was a problem hiding this comment.
사용자가 웹 푸시 구독을 하지 않은 경우 WellMeetNotificationException을 발생시키는 것은 과도한 처리입니다. 이는 오류 상황이 아니라 정상적인 시나리오일 수 있습니다. 이 예외로 인해 Kafka 컨슈머가 메시지 처리에 실패하고 재시도를 반복하며 멈출 수 있습니다. 구독이 없는 경우 경고 로그를 남기고 푸시 알림을 보내지 않는 것이 더 안정적인 방법입니다. (클래스에 @Slf4j 어노테이션 추가가 필요할 수 있습니다.)
| public void send(NotificationMessage message) { | |
| List<PushSubscription> subscriptions = pushSubscriptionRepository.findByUserId( | |
| message.getNotification().getRecipient()); | |
| if (subscriptions.isEmpty()) { | |
| throw new WellMeetNotificationException(ErrorCode.SUBSCRIPTION_NOT_FOUND); | |
| } | |
| subscriptions.forEach(subscription -> { | |
| Keys keys = new Keys(subscription.getP256dh(), subscription.getAuth()); | |
| Subscription sub = new Subscription(subscription.getEndpoint(), keys); | |
| Map<String, Object> notificationPayload = getNotificationPayload(message); | |
| webPushSend(notificationPayload, sub); | |
| }); | |
| } | |
| public void send(NotificationMessage message) { | |
| List<PushSubscription> subscriptions = pushSubscriptionRepository.findByUserId( | |
| message.getNotification().getRecipient()); | |
| if (subscriptions.isEmpty()) { | |
| // log.warn("No web push subscription found for user: {}", message.getNotification().getRecipient()); | |
| return; | |
| } | |
| subscriptions.forEach(subscription -> { | |
| Keys keys = new Keys(subscription.getP256dh(), subscription.getAuth()); | |
| Subscription sub = new Subscription(subscription.getEndpoint(), keys); | |
| Map<String, Object> notificationPayload = getNotificationPayload(message); | |
| webPushSend(notificationPayload, sub); | |
| }); | |
| } |
| private final NotificationEnabledRepository notificationEnabledRepository; | ||
| private final NotificationSender notificationSender; | ||
|
|
||
| @KafkaListener(topics = "notification", groupId = "notification-group") |
There was a problem hiding this comment.
Kafka groupId가 하드코딩되어 있습니다. 설정 관리를 중앙화하기 위해 application.yml 파일의 값을 프로퍼티 플레이스홀더(${spring.kafka.consumer.group-id})를 통해 주입받는 것이 좋습니다.
| @KafkaListener(topics = "notification", groupId = "notification-group") | |
| @KafkaListener(topics = "notification", groupId = "${spring.kafka.consumer.group-id}") |
|
|
||
| @KafkaListener(topics = "notification", groupId = "notification-group") | ||
| public void consume(NotificationMessage message) { | ||
| log.info("Received message: {}", message); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/wellmeet/notification/webpush/infrastructure/WebPushSender.java (2)
39-50: 초기화 실패 시 구체적인 예외 정보를 보존하세요.파이프라인 실패 로그에서 48번 라인의 예외가 보고되었습니다. 현재 모든 예외를
INTERNAL_SERVER_ERROR로 래핑하면 근본 원인(VapidConfig 문제, 키 형식 오류 등)을 파악하기 어렵습니다.@PostConstruct public void init() { Security.addProvider(new BouncyCastleProvider()); try { pushService = new PushService(); pushService.setPublicKey(vapidConfig.getPublicKey()); pushService.setPrivateKey(vapidConfig.getPrivateKey()); pushService.setSubject(vapidConfig.getSubject()); - } catch (Exception e) { + } catch (GeneralSecurityException | IllegalArgumentException e) { - throw new WellMeetNotificationException(ErrorCode.INTERNAL_SERVER_ERROR); + throw new WellMeetNotificationException(ErrorCode.WEB_PUSH_INITIALIZATION_FAILED, e); } }
ErrorCode.WEB_PUSH_INITIALIZATION_FAILED를 추가하고, 예외 체인을 유지하도록WellMeetNotificationException생성자에cause를 전달하세요.
73-110: 중복된 페이로드 생성 로직을 통합하세요.
getNotificationPayload(NotificationMessage)(73-87줄)와getNotificationPayload(TestPushRequest)(96-110줄)에 동일한 구조의 코드가 반복됩니다.- private Map<String, Object> getNotificationPayload(NotificationMessage message) { + private Map<String, Object> getNotificationPayload(String title, Object body) { Map<String, Object> notificationPayload = new HashMap<>(); - notificationPayload.put("title", "WellMeet 알림"); + notificationPayload.put("title", title); - notificationPayload.put("body", message.getPayload()); + notificationPayload.put("body", body); notificationPayload.put("icon", "/icon-192x192.png"); notificationPayload.put("badge", "/badge-72x72.png"); notificationPayload.put("vibrate", new int[]{100, 50, 100}); notificationPayload.put("requireInteraction", false); Map<String, Object> defaultData = new HashMap<>(); defaultData.put("url", "/notifications"); defaultData.put("timestamp", System.currentTimeMillis()); notificationPayload.put("data", defaultData); return notificationPayload; } + + private Map<String, Object> getNotificationPayload(NotificationMessage message) { + return getNotificationPayload("WellMeet 알림", message.getPayload()); + } private Map<String, Object> getNotificationPayload(TestPushRequest request) { - Map<String, Object> notificationPayload = new HashMap<>(); - notificationPayload.put("title", request.title()); - notificationPayload.put("body", request.body()); - notificationPayload.put("icon", "/icon-192x192.png"); - notificationPayload.put("badge", "/badge-72x72.png"); - notificationPayload.put("vibrate", new int[]{100, 50, 100}); - notificationPayload.put("requireInteraction", false); - - Map<String, Object> defaultData = new HashMap<>(); - defaultData.put("url", "/notifications"); - defaultData.put("timestamp", System.currentTimeMillis()); - notificationPayload.put("data", defaultData); - return notificationPayload; + return getNotificationPayload(request.title(), request.body()); }
🧹 Nitpick comments (10)
build.gradle (1)
30-31: 테스트 의존성 추가를 고려하세요.Kafka와 메일 스타터 의존성 추가가 적절합니다. 다만 Kafka 통합 테스트를 위해
spring-kafka-test의존성 추가를 권장합니다.테스트 의존성을 추가하려면 다음 diff를 적용하세요:
testImplementation 'org.springframework.boot:spring-boot-starter-test' + testImplementation 'org.springframework.kafka:spring-kafka-test' testRuntimeOnly 'org.junit.platform:junit-platform-launcher'src/main/resources/schema.sql (1)
22-29: notification_history 테이블에 추가 컨텍스트 필드를 고려하세요.알림 이력 테이블이
user_id와 타임스탬프만 저장하고 있어 실제 알림 내역을 추적하기에 정보가 부족합니다. 디버깅과 감사(audit) 목적으로 다음 필드들의 추가를 권장합니다:
notification_type: 어떤 타입의 알림이 발송되었는지channel: 어떤 채널로 발송되었는지 (WEB_PUSH, EMAIL 등)status: 발송 성공/실패 여부message또는title: 발송된 메시지 내용 (선택적)error_message: 실패 시 오류 메시지 (선택적)이러한 정보는 알림 발송 문제를 진단하고 사용자 알림 이력을 조회할 때 유용합니다.
CREATE TABLE IF NOT EXISTS notification_history ( id BIGINT AUTO_INCREMENT PRIMARY KEY, user_id VARCHAR(255) NOT NULL, + notification_type VARCHAR(50) NOT NULL, + channel VARCHAR(50) NOT NULL, + status VARCHAR(20) NOT NULL, + title VARCHAR(255), + message TEXT, + error_message TEXT, created_at DATETIME(6), updated_at DATETIME(6), INDEX idx_user_id (user_id), INDEX idx_created_at (created_at) );src/main/java/com/wellmeet/notification/repository/NotificationHistoryRepository.java (1)
7-9: 향후 확장을 위한 쿼리 메서드 추가 고려기본 CRUD 작업만 제공하는 현재 구현은 정상적으로 작동합니다. 하지만 알림 이력 조회를 위한 다음과 같은 쿼리 메서드들이 필요할 수 있습니다:
findByUserId(String userId)- 사용자별 알림 이력 조회findByNotificationType(NotificationType type)- 알림 유형별 조회findByUserIdAndCreatedAtBetween(String userId, LocalDateTime start, LocalDateTime end)- 기간별 조회향후 필요 시 다음과 같이 확장할 수 있습니다:
public interface NotificationHistoryRepository extends JpaRepository<NotificationHistory, Long> { List<NotificationHistory> findByUserId(String userId); List<NotificationHistory> findByNotificationType(NotificationType type); List<NotificationHistory> findByUserIdAndCreatedAtBetween( String userId, LocalDateTime start, LocalDateTime end ); }src/main/java/com/wellmeet/notification/consumer/dto/NotificationType.java (1)
5-9: enum에서 @Getter 애노테이션은 불필요합니다enum 상수는 기본적으로 public static final이므로 Lombok @Getter가 필요하지 않습니다. 현재 enum에는 추가 필드가 없어서 getter를 생성할 대상이 없습니다.
다음과 같이 @Getter를 제거할 수 있습니다:
-@Getter public enum NotificationType { RESERVATION_CREATED }또는 향후 확장을 위해 설명 필드를 추가하고 @Getter를 활용할 수 있습니다:
+@Getter +@RequiredArgsConstructor public enum NotificationType { - RESERVATION_CREATED + RESERVATION_CREATED("예약 생성"), + RESERVATION_CANCELLED("예약 취소"), + RESERVATION_CONFIRMED("예약 확인"); + + private final String description; }src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (1)
6-12: 불변성 강화 및 빌더 패턴 고려DTO는 불변 객체로 만드는 것이 좋습니다. 현재 구현은 @NoArgsConstructor만 있어 필드를 설정할 방법이 없습니다.
다음과 같이 개선할 수 있습니다:
@Getter -@NoArgsConstructor +@AllArgsConstructor +@Builder public class NotificationInfo { private NotificationType type; private String recipient; }또는 record 타입 사용을 고려할 수 있습니다 (Java 17+):
public record NotificationInfo( NotificationType type, String recipient ) {}src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java (1)
7-14: DTO 불변성 및 빌더 패턴 적용 권장Kafka 메시지 DTO는 불변 객체로 만드는 것이 좋습니다. 현재는 @NoArgsConstructor만 있어 객체 생성 후 필드를 설정할 방법이 없습니다.
다음과 같이 개선할 수 있습니다:
@Getter -@NoArgsConstructor +@AllArgsConstructor +@Builder public class NotificationMessage { private MessageHeader header; private NotificationInfo notification; private Map<String, Object> payload; }Java 17+를 사용 중이라면 record 타입 사용을 고려할 수 있습니다:
public record NotificationMessage( MessageHeader header, NotificationInfo notification, Map<String, Object> payload ) {}src/main/java/com/wellmeet/notification/Sender.java (1)
6-11: 예외 처리 및 반환 타입 개선 고려현재 인터페이스는 깔끔하지만 몇 가지 개선 사항을 고려할 수 있습니다:
send()메서드가 void를 반환하여 전송 성공/실패를 확인할 수 없습니다- 예외 처리 전략이 문서화되어 있지 않습니다
- Javadoc이 없어 구현체가 따라야 할 계약이 불명확합니다
다음과 같이 개선할 수 있습니다:
/** * 알림을 전송하는 공통 인터페이스입니다. * 구현체는 특정 채널(WebPush, Email, SMS 등)을 통해 알림을 전송합니다. */ public interface Sender { /** * 지정된 채널이 활성화되어 있는지 확인합니다. * * @param channel 확인할 알림 채널 * @return 채널이 활성화되어 있으면 true */ boolean isEnabled(NotificationChannel channel); /** * 알림 메시지를 전송합니다. * * @param message 전송할 알림 메시지 * @throws NotificationSendException 전송 실패 시 */ void send(NotificationMessage message); }또는 전송 결과를 반환하도록 변경:
public interface Sender { boolean isEnabled(NotificationChannel channel); /** * @return 전송 성공 여부 */ boolean send(NotificationMessage message); }src/main/java/com/wellmeet/notification/consumer/NotificationConsumer.java (1)
20-26: 환경 설정과 동일한 값으로 주입하세요.Line 20의
@KafkaListener가 토픽과 그룹을 문자열로 하드코딩하고 있어application-dev.yml등 외부 설정과 불일치할 여지가 큽니다.${spring.kafka.*}프로퍼티를 그대로 참조하도록 변경하면 환경별 설정을 맞추기 쉬워집니다.src/main/java/com/wellmeet/notification/domain/NotificationEnabled.java (1)
12-27: 엔티티 생성을 위한 빌더 패턴을 추가하세요.현재 protected 기본 생성자만 있어 엔티티 인스턴스를 생성하기 어렵습니다. Lombok의
@Builder어노테이션을 추가하여 가독성 높은 빌더 패턴을 제공하세요.@Getter @Entity @NoArgsConstructor(access = AccessLevel.PROTECTED) +@Builder +@AllArgsConstructor(access = AccessLevel.PRIVATE) public class NotificationEnabled {src/main/java/com/wellmeet/notification/webpush/infrastructure/WebPushSender.java (1)
59-63: 구독이 없는 경우 예외 대신 로깅 후 조용히 반환하는 것을 고려하세요.사용자가 구독을 취소했거나 아직 구독하지 않은 경우 예외를 던지면 Kafka 메시지 처리가 실패하여 재시도가 발생할 수 있습니다. 알림 구독은 선택사항이므로, 구독이 없을 때 로그만 남기고 정상 처리하는 것이 더 적합할 수 있습니다.
List<PushSubscription> subscriptions = pushSubscriptionRepository.findByUserId( message.getNotification().getRecipient()); if (subscriptions.isEmpty()) { - throw new WellMeetNotificationException(ErrorCode.SUBSCRIPTION_NOT_FOUND); + log.warn("No web push subscriptions found for user: {}", message.getNotification().getRecipient()); + return; }단, 비즈니스 요구사항에 따라 예외가 필요하다면 이 제안은 무시하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.github/workflows/Dev_CD.yml(1 hunks).github/workflows/Dev_CI.yml(1 hunks).gitignore(1 hunks)build.gradle(1 hunks)scripts/dev/replace-new-version.sh(1 hunks)src/main/java/com/wellmeet/config/KafkaConfig.java(1 hunks)src/main/java/com/wellmeet/notification/Sender.java(1 hunks)src/main/java/com/wellmeet/notification/consumer/NotificationConsumer.java(1 hunks)src/main/java/com/wellmeet/notification/consumer/NotificationSender.java(1 hunks)src/main/java/com/wellmeet/notification/consumer/dto/MessageHeader.java(1 hunks)src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java(1 hunks)src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java(1 hunks)src/main/java/com/wellmeet/notification/consumer/dto/NotificationType.java(1 hunks)src/main/java/com/wellmeet/notification/domain/NotificationChannel.java(1 hunks)src/main/java/com/wellmeet/notification/domain/NotificationEnabled.java(1 hunks)src/main/java/com/wellmeet/notification/domain/NotificationHistory.java(1 hunks)src/main/java/com/wellmeet/notification/repository/NotificationEnabledRepository.java(1 hunks)src/main/java/com/wellmeet/notification/repository/NotificationHistoryRepository.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/WebPushController.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/WebPushService.java(3 hunks)src/main/java/com/wellmeet/notification/webpush/domain/PushSubscription.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/dto/SubscribeRequest.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/dto/SubscribeResponse.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/dto/TestPushRequest.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/dto/UnsubscribeRequest.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/infrastructure/WebPushSender.java(3 hunks)src/main/java/com/wellmeet/notification/webpush/repository/PushSubscriptionRepository.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(2 hunks)src/main/resources/application-test.yml(1 hunks)src/main/resources/local-secret.yml(1 hunks)src/main/resources/schema.sql(1 hunks)src/test/java/com/wellmeet/BaseControllerTest.java(1 hunks)src/test/java/com/wellmeet/BaseServiceTest.java(1 hunks)src/test/java/com/wellmeet/webpush/WebPushControllerTest.java(1 hunks)src/test/java/com/wellmeet/webpush/WebPushServiceTest.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationType.java (2)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (1)
Getter(6-12)src/main/java/com/wellmeet/notification/domain/NotificationEnabled.java (1)
Getter(12-27)
src/main/java/com/wellmeet/notification/consumer/dto/MessageHeader.java (2)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (1)
Getter(6-12)src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java (1)
Getter(7-14)
src/main/java/com/wellmeet/notification/domain/NotificationEnabled.java (2)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (1)
Getter(6-12)src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java (1)
Getter(7-14)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java (2)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (1)
Getter(6-12)src/main/java/com/wellmeet/notification/consumer/dto/MessageHeader.java (1)
Getter(7-14)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (2)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java (1)
Getter(7-14)src/main/java/com/wellmeet/notification/domain/NotificationEnabled.java (1)
Getter(12-27)
src/main/java/com/wellmeet/notification/domain/NotificationHistory.java (1)
src/main/java/com/wellmeet/notification/consumer/dto/NotificationInfo.java (1)
Getter(6-12)
src/main/java/com/wellmeet/notification/consumer/NotificationConsumer.java (1)
src/main/java/com/wellmeet/notification/webpush/infrastructure/WebPushSender.java (1)
Service(30-121)
🪛 actionlint (1.7.7)
.github/workflows/Dev_CD.yml
62-62: label "dev" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 GitHub Actions: dev-ci
src/test/java/com/wellmeet/webpush/WebPushControllerTest.java
[error] 1-1: Test failures due to cascading UnsatisfiedDependencyException and BeanCreationException in WebPush-related tests. 26 tests failed.
src/main/java/com/wellmeet/notification/webpush/infrastructure/WebPushSender.java
[error] 48-48: WellMeetNotificationException encountered during test execution at WebPushSender.java:48 (dependency/initialization issue cascading from test context).
🪛 Gitleaks (8.28.0)
src/main/resources/local-secret.yml
[high] 4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Shellcheck (0.11.0)
scripts/dev/replace-new-version.sh
[warning] 36-36: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
⏰ 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: build-and-push
🔇 Additional comments (27)
.gitignore (1)
39-39: 추가 무시 규칙 확인 완료
.serena/디렉터리를 무시하도록 한 설정 적절합니다. 다른 항목과도 일관성 있습니다..github/workflows/Dev_CD.yml (1)
60-63: ラン너 라벨 확인이 필요합니다.
runs-on: dev는 커스텀 라벨일 가능성이 높습니다. 해당 라벨이 실제 셀프 호스트 러너에 적용돼 있지 않으면 잡이 대기 상태로 멈춥니다. 한번 더 확인 부탁드립니다.src/main/java/com/wellmeet/notification/webpush/dto/UnsubscribeRequest.java (1)
1-9: LGTM!패키지 리팩토링이 올바르게 적용되었습니다. 검증 로직도 적절합니다.
src/main/java/com/wellmeet/notification/webpush/domain/PushSubscription.java (1)
1-52: LGTM!패키지 마이그레이션이 일관되게 적용되었습니다. 엔티티 구조와 Lombok 사용이 적절합니다.
src/main/java/com/wellmeet/notification/webpush/dto/TestPushRequest.java (1)
1-17: LGTM!패키지 마이그레이션과 검증 로직이 올바릅니다.
src/main/java/com/wellmeet/notification/webpush/WebPushController.java (1)
1-50: LGTM!패키지와 import 경로가 일관되게 업데이트되었습니다. 컨트롤러 로직은 변경되지 않았습니다.
src/main/resources/application-local.yml (3)
23-37: LGTM!메일 SMTP 설정이 적절하게 구성되었습니다. Gmail SMTP를 사용하며 인증 및 TLS가 올바르게 활성화되었습니다.
53-54: LGTM!로컬 개발 환경을 위한 포트 오버라이드가 적절합니다.
38-44: application-local.yml 변경 불필요: KafkaConfig에서 JsonDeserializer로 오버라이드됩니다KafkaConfig.java에서 이미 VALUE_DESERIALIZER_CLASS_CONFIG에 JsonDeserializer.class를 설정하고 있어 YAML의 value-deserializer 설정은 적용되지 않습니다.
Likely an incorrect or invalid review comment.
src/main/resources/schema.sql (2)
4-6: VARCHAR 길이 선택이 적절합니다.Web Push 엔드포인트는 긴 URL이 될 수 있으므로
VARCHAR(500)이 적절하며,p256dh와auth는 base64로 인코딩된 32바이트 키(약 44자)를 저장하므로VARCHAR(255)로 충분합니다.
10-11: 인덱스 전략이 적절합니다.
user_id와endpoint에 대한 인덱스는 구독 조회 및 중복 체크 쿼리 성능을 개선합니다.src/main/java/com/wellmeet/notification/webpush/dto/SubscribeRequest.java (1)
1-3: 패키지 리팩토링이 일관성 있게 적용되었습니다.WebPush 관련 클래스들을
com.wellmeet.notification.webpush하위로 이동하는 리팩토링이 올바르게 적용되었습니다. 코드 로직에는 변경이 없습니다.src/main/java/com/wellmeet/notification/webpush/repository/PushSubscriptionRepository.java (1)
1-16: 패키지 이동이 정확하게 처리되었습니다.Repository의 패키지 경로가
com.wellmeet.notification.webpush.repository로 올바르게 업데이트되었으며, 쿼리 메서드들은 변경 없이 유지되었습니다.src/main/java/com/wellmeet/notification/webpush/dto/SubscribeResponse.java (1)
1-22: 패키지 구조 변경이 올바르게 반영되었습니다.DTO의 패키지 경로와 import가 새로운 구조에 맞게 업데이트되었으며, 응답 레코드의 구조는 그대로 유지되었습니다.
src/main/java/com/wellmeet/notification/domain/NotificationChannel.java (1)
1-7: 알림 채널 enum이 적절하게 설계되었습니다.
WEB_PUSH와src/main/resources/application-test.yml (1)
23-24: Kafka bootstrap 서버 설정이 적절합니다.테스트 환경에서 로컬 Kafka 인스턴스(
localhost:9092)를 사용하는 것은 표준적인 접근입니다.src/test/java/com/wellmeet/BaseServiceTest.java (1)
3-3: 테스트 베이스 클래스의 import가 올바르게 업데이트되었습니다.
PushSubscriptionRepository의 새로운 패키지 경로가 정확하게 반영되었습니다.src/test/java/com/wellmeet/BaseControllerTest.java (1)
3-3: 패키지 경로 업데이트 확인 완료
PushSubscriptionRepository의 패키지 경로가com.wellmeet.notification.webpush.repository로 정확히 업데이트되었습니다.src/main/java/com/wellmeet/notification/webpush/WebPushService.java (3)
1-11: 패키지 재구성 및 네이밍 개선 완료WebPush 관련 컴포넌트들이
com.wellmeet.notification.webpush패키지로 이동되었고, 모든 import가 올바르게 업데이트되었습니다.
23-23: 필드명 개선으로 명확성 향상
pushService에서webPushSender로 필드명을 변경하여 역할이 더 명확해졌습니다. 특히 새로운Sender인터페이스 구현과 일관성을 유지합니다.
31-36: 가독성 향상을 위한 포매팅 개선조건문 주변에 빈 줄을 추가하여 코드 흐름이 더 읽기 쉬워졌습니다.
src/main/java/com/wellmeet/notification/consumer/dto/NotificationMessage.java (1)
13-13: payload의 타입 안정성 검토 필요
Map<String, Object>는 유연하지만 타입 안정성이 없어 런타임 에러의 위험이 있습니다.다음 사항을 확인해주세요:
- payload에 어떤 데이터가 담기는지 명확히 정의되어 있는가?
- 각 NotificationType별로 예상되는 payload 구조가 문서화되어 있는가?
- 향후 타입 안전성을 위해 sealed interface나 별도의 payload 클래스 계층을 고려할 수 있는가?
예시:
// 타입 안전한 payload 접근 방식 public sealed interface NotificationPayload permits ReservationPayload, CancellationPayload {} public record ReservationPayload( String restaurantId, String reservationId, LocalDateTime reservationTime ) implements NotificationPayload {}src/test/java/com/wellmeet/webpush/WebPushServiceTest.java (2)
10-14: 패키지 경로 업데이트 확인 완료WebPush 관련 클래스들의 import 경로가
com.wellmeet.notification.webpush패키지로 정확히 업데이트되었습니다.
46-62: 테스트 개선: 업데이트 동작 검증 강화테스트 메서드명과 검증 로직이 개선되었습니다:
- 메서드명 개선: "기존_구독을_반환한다" → "기존_구독을_업데이트한다" - 실제 동작을 더 정확히 표현
- 검증 로직 개선:
- 기존: 저장된 인스턴스와 비교
- 변경 후: 요청값(
request.p256dh(),request.auth())과 비교이 변경으로 구독 정보가 실제로 업데이트되는지 명확히 검증합니다.
src/main/java/com/wellmeet/notification/repository/NotificationEnabledRepository.java (2)
10-10: 엔티티의 복합 키 수정에 맞춰 레포지토리 제네릭 타입을 업데이트하세요.
NotificationEnabled엔티티가 복합 키를 사용하도록 수정되면, 레포지토리의 ID 타입도String에서 복합 키 클래스로 변경해야 합니다.
NotificationEnabled.java의 복합 키 적용이 완료되면 다음과 같이 수정하세요:-public interface NotificationEnabledRepository extends JpaRepository<NotificationEnabled, String> { +public interface NotificationEnabledRepository extends JpaRepository<NotificationEnabled, NotificationEnabled.NotificationEnabledId> {(또는
@EmbeddedId방식을 사용한다면 해당 임베디드 ID 클래스명으로 변경)
12-12: 쿼리 메서드 네이밍이 Spring Data JPA 규칙을 따릅니다.
findByUserIdAndType메서드는 Spring Data JPA의 메서드 네이밍 규칙을 올바르게 따르고 있으며, 의도한 쿼리(WHERE userId = ? AND type = ?)를 정확히 생성합니다.참고: 엔티티에 복합 키가 적용되면 이 쿼리 메서드는 여러 채널의 설정을 반환하게 되므로, 호출하는 측에서 채널별 필터링이나 추가 처리를 고려하세요.
src/main/java/com/wellmeet/notification/webpush/infrastructure/WebPushSender.java (1)
76-76: NotificationMessage.payload를 Web Push body에 맞게 문자열로 변환하세요.
- 현재
message.getPayload()는Map<String, Object>전체를body에 넣고 있어, 런타임 오류 또는 예상치 못한 값 전송 위험이 있습니다.- payload에서 실제 텍스트 메시지 필드(예:
"message")를 추출하거나, 필요 시 JSON 문자열로 직렬화하여body에 할당해야 합니다.
| - name: Setting local-secret.yml | ||
| run: | | ||
| echo "${{ secrets.LOCAL_SECRET_YML }}" > ./src/main/resources/local-secret.yml | ||
|
|
There was a problem hiding this comment.
포크 PR에서 시크릿 미전달로 인한 CI 실패 가능성
pull_request 이벤트로 실행될 때 외부 포크에서 올라온 PR에는 GitHub 시크릿이 전달되지 않습니다. 이 단계가 그대로 실행되면 빈 local-secret.yml만 생성되고 이후 빌드/테스트가 비정상 종료되어 외부 기여자의 CI가 항상 실패하게 됩니다. 포크 PR을 고려해 시크릿이 비어 있을 때는 단계를 건너뛰거나, 대체 설정(예: 더미 설정·if 조건으로 전체 잡 스킵)을 두는 식의 방어 로직을 추가해 주세요.
🤖 Prompt for AI Agents
.github/workflows/Dev_CI.yml lines 71-74: the workflow writes
secrets.LOCAL_SECRET_YML unconditionally which causes empty local-secret.yml and
CI failures for forked PRs (secrets are not available). Fix by guarding the step
with an if conditional that only runs when the secret is present (e.g. if: ${{
secrets.LOCAL_SECRET_YML != '' }}) or replace the step with a small script that
checks if the secret is set and either writes the real secret or writes a
harmless dummy/local-default file and logs that a fallback was used; ensure the
job downstream expects the dummy or skip downstream jobs with an appropriate if
if you choose to skip generation.
| JAR_FILE=$(ls /home/ubuntu/app/*.jar | head -n 1) | ||
|
|
||
| echo "JAR 파일 실행: $JAR_FILE" | ||
|
|
There was a problem hiding this comment.
JAR 파일 존재 여부를 먼저 확인해야 합니다.
ls /home/ubuntu/app/*.jar가 빈 결과를 반환하면 JAR_FILE이 비어 있는 상태로 java -jar가 실행되어 배포가 실패합니다. JAR 미존재 시 명확히 종료하도록 가드가 필요합니다.
다음과 같이 방어 로직을 추가해 주세요:
JAR_FILE=$(ls /home/ubuntu/app/*.jar | head -n 1)
echo "JAR 파일 실행: $JAR_FILE"
+
+if [ -z "$JAR_FILE" ]; then
+ echo "실행 가능한 JAR 파일을 찾을 수 없습니다."
+ exit 1
+fi🤖 Prompt for AI Agents
In scripts/dev/replace-new-version.sh around lines 20 to 23, the script assigns
JAR_FILE=$(ls /home/ubuntu/app/*.jar | head -n 1) but does not verify the
result; add a guard that checks whether JAR_FILE is non-empty and points to an
existing file, and if not, print a clear error message to stderr and exit with a
non-zero status; implement this by testing [ -z "$JAR_FILE" ] || [ ! -f
"$JAR_FILE" ] and then echo the error and exit 1 before attempting to run java
-jar.
| # 애플리케이션 로그 파일 설정 | ||
| APP_LOG_DIR="/home/ubuntu/app/logs" | ||
| APP_LOG_FILE="$APP_LOG_DIR/application-$(date +%Y%m%d-%H%M%S).log" | ||
|
|
||
| echo "애플리케이션 로그 파일: $APP_LOG_FILE" | ||
|
|
||
| sudo nohup java \ | ||
| -Dspring.profiles.active=dev \ | ||
| -Duser.timezone=Asia/Seoul \ | ||
| -Dserver.port=8080 \ | ||
| -Ddd.service=wellmeet-notification \ | ||
| -Ddd.env=dev \ | ||
| -jar "$JAR_FILE" > "$APP_LOG_FILE" 2>&1 & |
There was a problem hiding this comment.
로그 디렉터리를 먼저 생성하지 않으면 재실행이 실패합니다.
APP_LOG_DIR가 존재하지 않는 상태에서 리디렉션이 수행되면 nohup 실행이 바로 실패합니다. 로그 디렉터리를 선행 생성해 주세요.
아래와 같이 mkdir -p를 추가하면 안전합니다:
APP_LOG_DIR="/home/ubuntu/app/logs"
APP_LOG_FILE="$APP_LOG_DIR/application-$(date +%Y%m%d-%H%M%S).log"
echo "애플리케이션 로그 파일: $APP_LOG_FILE"
+sudo mkdir -p "$APP_LOG_DIR"
+
sudo nohup java \📝 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.
| # 애플리케이션 로그 파일 설정 | |
| APP_LOG_DIR="/home/ubuntu/app/logs" | |
| APP_LOG_FILE="$APP_LOG_DIR/application-$(date +%Y%m%d-%H%M%S).log" | |
| echo "애플리케이션 로그 파일: $APP_LOG_FILE" | |
| sudo nohup java \ | |
| -Dspring.profiles.active=dev \ | |
| -Duser.timezone=Asia/Seoul \ | |
| -Dserver.port=8080 \ | |
| -Ddd.service=wellmeet-notification \ | |
| -Ddd.env=dev \ | |
| -jar "$JAR_FILE" > "$APP_LOG_FILE" 2>&1 & | |
| # 애플리케이션 로그 파일 설정 | |
| APP_LOG_DIR="/home/ubuntu/app/logs" | |
| APP_LOG_FILE="$APP_LOG_DIR/application-$(date +%Y%m%d-%H%M%S).log" | |
| echo "애플리케이션 로그 파일: $APP_LOG_FILE" | |
| sudo mkdir -p "$APP_LOG_DIR" | |
| sudo nohup java \ | |
| -Dspring.profiles.active=dev \ | |
| -Duser.timezone=Asia/Seoul \ | |
| -Dserver.port=8080 \ | |
| -Ddd.service=wellmeet-notification \ | |
| -Ddd.env=dev \ | |
| -jar "$JAR_FILE" > "$APP_LOG_FILE" 2>&1 & |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 36-36: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
🤖 Prompt for AI Agents
In scripts/dev/replace-new-version.sh around lines 24 to 36, the script
redirects Java output to a file inside APP_LOG_DIR but never ensures that
APP_LOG_DIR exists, causing nohup to fail if the directory is missing; before
starting the Java process, create the log directory (e.g., run mkdir -p
"$APP_LOG_DIR") and optionally set ownership/permissions as needed so the
redirection can succeed, then proceed with the nohup java command to write to
the log file.
| props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); | ||
|
|
||
| props.put(JsonDeserializer.USE_TYPE_INFO_HEADERS, false); | ||
| props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.consumer.dto"); |
There was a problem hiding this comment.
잘못된 trusted package 설정으로 역직렬화가 실패합니다.
Line 37에서 JsonDeserializer.TRUSTED_PACKAGES 값을 com.wellmeet.consumer.dto로 지정했지만 실제 DTO 패키지는 com.wellmeet.notification.consumer.dto입니다. 이 상태로는 NotificationMessage를 역직렬화할 때 “Trusted packages [com.wellmeet.consumer.dto] don't allow class com.wellmeet.notification.consumer.dto.NotificationMessage” 예외가 발생합니다. 아래처럼 올바른 패키지명을 지정해주세요.
- props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.consumer.dto");
+ props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.notification.consumer.dto");📝 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.
| props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.consumer.dto"); | |
| props.put(JsonDeserializer.TRUSTED_PACKAGES, "com.wellmeet.notification.consumer.dto"); |
🤖 Prompt for AI Agents
In src/main/java/com/wellmeet/config/KafkaConfig.java around line 37, the
JsonDeserializer.TRUSTED_PACKAGES is set to "com.wellmeet.consumer.dto" which
doesn't include the actual DTO package; update that property to the correct
package "com.wellmeet.notification.consumer.dto" (or include both
packages/com.wellmeet.* if you want broader coverage) by changing the props.put
call to reference the proper package name so NotificationMessage can be
deserialized.
| @Id | ||
| private String userId; |
There was a problem hiding this comment.
복합 키 설계 오류: userId만으로는 알림 설정을 표현할 수 없습니다.
현재 userId만 @Id로 지정되어 있어 한 사용자당 하나의 NotificationEnabled 레코드만 저장할 수 있습니다. 그러나 레포지토리의 findByUserIdAndType 메서드와 스키마의 (type, channel) 인덱스를 보면, 사용자는 여러 타입과 채널 조합으로 알림 설정을 가져야 합니다.
다음과 같이 복합 키를 적용하세요:
+import jakarta.persistence.IdClass;
+import java.io.Serializable;
+import java.util.Objects;
+
+@IdClass(NotificationEnabled.NotificationEnabledId.class)
@Getter
@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class NotificationEnabled {
@Id
private String userId;
+ @Id
@Enumerated(value = EnumType.STRING)
private NotificationType type;
+ @Id
@Enumerated(value = EnumType.STRING)
private NotificationChannel channel;
private boolean enabled;
+
+ @Getter
+ @NoArgsConstructor
+ @AllArgsConstructor
+ public static class NotificationEnabledId implements Serializable {
+ private String userId;
+ private NotificationType type;
+ private NotificationChannel channel;
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (!(o instanceof NotificationEnabledId)) return false;
+ NotificationEnabledId that = (NotificationEnabledId) o;
+ return Objects.equals(userId, that.userId) &&
+ type == that.type &&
+ channel == that.channel;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(userId, type, channel);
+ }
+ }
}또는 @EmbeddedId를 사용한 대안도 고려할 수 있습니다.
| kafka: | ||
| bootstrap-servers: localhost:9092 | ||
| consumer: | ||
| group-id: notification-group | ||
| auto-offset-reset: earliest | ||
| key-deserializer: org.apache.kafka.common.serialization.StringDeserializer | ||
| value-deserializer: org.apache.kafka.common.serialization.StringDeserializer |
There was a problem hiding this comment.
Kafka 역직렬화 설정이 프로덕션 설정과 불일치합니다.
테스트 프로필에서 value-deserializer가 StringDeserializer로 설정되어 있지만, 실제 Kafka 소비자는 NotificationMessage 객체를 JSON으로 역직렬화해야 합니다.
KafkaConfig.java의 프로덕션 설정에 따르면 JsonDeserializer를 사용하고 있으므로, 테스트 환경에서도 동일한 역직렬화 설정을 사용해야 일관성 있는 테스트가 가능합니다.
consumer:
group-id: notification-group
auto-offset-reset: earliest
key-deserializer: org.apache.kafka.common.serialization.StringDeserializer
- value-deserializer: org.apache.kafka.common.serialization.StringDeserializer
+ value-deserializer: org.springframework.kafka.support.serializer.JsonDeserializer
+ properties:
+ spring.json.trusted.packages: com.wellmeet.notification.consumer.dto
+ spring.json.value.default.type: com.wellmeet.notification.consumer.dto.NotificationMessage📝 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.
| kafka: | |
| bootstrap-servers: localhost:9092 | |
| consumer: | |
| group-id: notification-group | |
| auto-offset-reset: earliest | |
| key-deserializer: org.apache.kafka.common.serialization.StringDeserializer | |
| value-deserializer: org.apache.kafka.common.serialization.StringDeserializer | |
| kafka: | |
| bootstrap-servers: localhost:9092 | |
| consumer: | |
| group-id: notification-group | |
| auto-offset-reset: earliest | |
| key-deserializer: org.apache.kafka.common.serialization.StringDeserializer | |
| value-deserializer: org.springframework.kafka.support.serializer.JsonDeserializer | |
| properties: | |
| spring.json.trusted.packages: com.wellmeet.notification.consumer.dto | |
| spring.json.value.default.type: com.wellmeet.notification.consumer.dto.NotificationMessage |
🤖 Prompt for AI Agents
In src/main/resources/application-test.yml around lines 23 to 29, the test
profile sets Kafka value-deserializer to StringDeserializer which mismatches
production where JsonDeserializer is used; update the test config to use Spring
Kafka's JsonDeserializer (and configure any needed trusted packages or type
mapping consistent with KafkaConfig.java) so NotificationMessage JSON payloads
are deserialized the same way in tests as in production.
src/main/resources/local-secret.yml
Outdated
| secret: | ||
| vapid: | ||
| public-key: BCjLRdYi3EapfKAjZlIONNWb7PgUGnSo9-HDedbcd02o0zwriW-93jZ35Ufqu_C4jFtcKuHCdsGA_3TYyAHXqxs | ||
| private-key: LjC3sekYvWtxxtN6R4qEEUunAI592EcpK8bc1Ggy8tU | ||
| subject: mailto:admin@wellmeet.com |
There was a problem hiding this comment.
보안 위험: 시크릿이 버전 관리에 커밋되었습니다.
VAPID 키와 같은 시크릿 정보를 Git에 커밋하는 것은 심각한 보안 위험입니다. 개발용 키라 하더라도 공개 저장소에 노출되면 악용될 수 있습니다.
다음 조치를 취하세요:
- 즉시 조치: 이 파일을
.gitignore에 추가하고 Git 히스토리에서 제거 - 키 로테이션: 노출된 VAPID 키를 새로운 키로 교체
- 환경 변수 사용: 시크릿을 환경 변수로 로드하도록 변경
다음 diff를 적용하여 .gitignore를 업데이트하세요:
+# Local secrets - do not commit
+src/main/resources/local-secret.yml
+src/main/resources/*-secret.yml그리고 application-local.yml에서 환경 변수를 사용하도록 수정하세요:
vapid:
- public-key: ${secret.vapid.public-key}
- private-key: ${secret.vapid.private-key}
- subject: ${secret.vapid.subject}
+ public-key: ${VAPID_PUBLIC_KEY:}
+ private-key: ${VAPID_PRIVATE_KEY:}
+ subject: ${VAPID_SUBJECT:mailto:admin@wellmeet.com}Git 히스토리에서 시크릿을 제거하려면:
#!/bin/bash
# Description: Git 히스토리에서 시크릿 파일 제거 (주의: 강제 푸시 필요)
# 1. BFG Repo-Cleaner 또는 git-filter-repo 사용 권장
echo "경고: 이 작업은 Git 히스토리를 다시 작성합니다."
echo "팀원들과 협의 후 실행하세요."
# git-filter-repo 사용 예시 (설치 필요)
# git filter-repo --path src/main/resources/local-secret.yml --invert-paths
# 2. 새로운 VAPID 키 생성
echo -e "\n새로운 VAPID 키를 생성하세요:"
echo "https://vapidkeys.com/ 또는 web-push CLI 도구 사용"참고: 정적 분석 도구가 Line 4의 private-key를 API 키로 탐지했습니다.
🧰 Tools
🪛 Gitleaks (8.28.0)
[high] 4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In src/main/resources/local-secret.yml lines 1-5 the VAPID public/private keys
are committed to Git; remove this secret from the repository, stop committing
it, and replace usage with environment variables: (1) add
src/main/resources/local-secret.yml to .gitignore and remove the file from the
repo history using git-filter-repo or BFG (coordinate with team and force-push
as needed), (2) rotate the exposed VAPID keys by creating new keys and revoke
the old ones, (3) change configuration to read VAPID_PUBLIC_KEY,
VAPID_PRIVATE_KEY and VAPID_SUBJECT from environment variables (e.g.,
application-local.yml should reference env vars instead of hardcoded values),
and (4) ensure local development uses a secure mechanism (local .env not
committed or a secrets manager) and update docs for key rotation and secret
handling.
| CREATE TABLE IF NOT EXISTS notification_enabled ( | ||
| user_id VARCHAR(255) PRIMARY KEY, | ||
| type VARCHAR(50) NOT NULL, | ||
| channel VARCHAR(50) NOT NULL, | ||
| enabled BOOLEAN NOT NULL DEFAULT TRUE, | ||
| INDEX idx_type_channel (type, channel) | ||
| ); |
There was a problem hiding this comment.
notification_enabled 테이블에 유니크 제약 조건 추가를 고려하세요.
현재 스키마는 user_id가 PRIMARY KEY이므로 사용자당 하나의 설정만 저장할 수 있습니다. 하지만 type과 channel 필드가 존재하는 것으로 보아, 사용자별로 여러 알림 타입과 채널 조합을 저장하려는 의도로 보입니다.
만약 사용자가 여러 (type, channel) 조합에 대해 설정을 가질 수 있어야 한다면:
- PRIMARY KEY를 복합키
(user_id, type, channel)로 변경하거나 user_id를 일반 컬럼으로 변경하고id를 PRIMARY KEY로 추가한 뒤,UNIQUE(user_id, type, channel)제약 조건을 추가해야 합니다.
현재 구조에서는 한 사용자가 하나의 설정만 가질 수 있어 type과 channel 필드의 목적이 불명확합니다.
CREATE TABLE IF NOT EXISTS notification_enabled (
- user_id VARCHAR(255) PRIMARY KEY,
+ id BIGINT AUTO_INCREMENT PRIMARY KEY,
+ user_id VARCHAR(255) NOT NULL,
type VARCHAR(50) NOT NULL,
channel VARCHAR(50) NOT NULL,
enabled BOOLEAN NOT NULL DEFAULT TRUE,
+ UNIQUE KEY uk_user_type_channel (user_id, type, channel),
INDEX idx_type_channel (type, channel)
);📝 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.
| CREATE TABLE IF NOT EXISTS notification_enabled ( | |
| user_id VARCHAR(255) PRIMARY KEY, | |
| type VARCHAR(50) NOT NULL, | |
| channel VARCHAR(50) NOT NULL, | |
| enabled BOOLEAN NOT NULL DEFAULT TRUE, | |
| INDEX idx_type_channel (type, channel) | |
| ); | |
| CREATE TABLE IF NOT EXISTS notification_enabled ( | |
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | |
| user_id VARCHAR(255) NOT NULL, | |
| type VARCHAR(50) NOT NULL, | |
| channel VARCHAR(50) NOT NULL, | |
| enabled BOOLEAN NOT NULL DEFAULT TRUE, | |
| UNIQUE KEY uk_user_type_channel (user_id, type, channel), | |
| INDEX idx_type_channel (type, channel) | |
| ); |
🤖 Prompt for AI Agents
In src/main/resources/schema.sql around lines 14 to 20, the current schema uses
user_id as PRIMARY KEY which prevents storing multiple (type, channel) settings
per user; change the schema to either make the PRIMARY KEY a composite key
(user_id, type, channel) or add a separate surrogate id as the PRIMARY KEY and
add a UNIQUE(user_id, type, channel) constraint; update the CREATE TABLE
accordingly and adjust any application code/queries that assume user_id is the
sole primary key.
| import com.wellmeet.notification.webpush.domain.PushSubscription; | ||
| import com.wellmeet.notification.webpush.dto.SubscribeRequest; | ||
| import com.wellmeet.notification.webpush.dto.SubscribeResponse; | ||
| import com.wellmeet.notification.webpush.dto.TestPushRequest; | ||
| import com.wellmeet.notification.webpush.dto.UnsubscribeRequest; |
There was a problem hiding this comment.
🧩 Analysis chain
파이프라인 실패를 해결하세요.
import 경로 업데이트는 올바르지만, 26개 테스트가 BeanCreationException과 UnsatisfiedDependencyException으로 실패했습니다. 이는 Spring 컨텍스트가 WebPush 관련 빈을 제대로 생성하지 못하고 있음을 나타냅니다.
다음 사항을 확인하세요:
- 컴포넌트 스캔이
com.wellmeet.notification.webpush패키지를 포함하는지 확인 WebPushService와 관련 빈들이 올바르게 설정되었는지 확인- 테스트 컨텍스트 설정에서 필요한 빈 정의나 mock이 누락되지 않았는지 확인
다음 스크립트로 관련 설정 파일을 검증하세요:
🏁 Script executed:
#!/bin/bash
# Description: Spring 설정 및 컴포넌트 스캔 경로 확인
# 1. Application 클래스에서 컴포넌트 스캔 범위 확인
echo "=== Checking component scan configuration ==="
rg -n "@SpringBootApplication|@ComponentScan" --type java -A 3
# 2. WebPushService 빈 정의 확인
echo -e "\n=== Checking WebPushService bean definitions ==="
rg -n "class WebPushService|@Service.*WebPushService" --type java -B 2 -A 5
# 3. 테스트 설정 파일 확인
echo -e "\n=== Checking test configuration ==="
rg -n "@SpringBootTest|@TestConfiguration" --type java -B 2 -A 5 src/test/Length of output: 4095
🏁 Script executed:
#!/bin/bash
echo "=== Locating WebPushControllerTest contents ==="
rg -n "class WebPushControllerTest" -A50 src/test/java
echo -e "\n=== Locating WebPushSender definition(s) ==="
rg -n "WebPushSender" --type java -A5 src/main/javaLength of output: 7704
WebPushSender 및 구성 빈 등록 누락
- WebPushService 의존성인 WebPushSender가 빈으로 등록되지 않아 BeanCreationException이 발생합니다. WebPushSender 클래스에 @Component/@service를 추가하거나 Configuration 클래스에서 @bean으로 정의하세요.
- VapidConfig(@ConfigurationProperties)와 PushService 빈 등록 여부도 함께 검토해 주세요.
🤖 Prompt for AI Agents
In src/test/java/com/wellmeet/webpush/WebPushControllerTest.java around lines 8
to 12 the test fails due to missing beans: WebPushSender (a dependency of
WebPushService) and potentially VapidConfig/@ConfigurationProperties and
PushService are not registered. Fix by either annotating WebPushSender with
@Component or @Service (or create a @Bean factory method in an existing
@Configuration class) so Spring can auto-detect it; ensure VapidConfig is
annotated with @ConfigurationProperties and is enabled (via
@EnableConfigurationProperties(VapidConfig.class) or
@ConfigurationPropertiesScan or a @Bean providing the populated config); and
register PushService as a bean (annotate with @Service/@Component or declare a
@Bean). After adding these annotations or bean declarations, re-run the test to
confirm BeanCreationException is resolved.
🚩 Jira Ticket
SCRUM-117
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
테스트
작업