-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/알림 배치처리 적용 #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "feat/\uC54C\uB9BC-\uBC30\uCE58\uCC98\uB9AC-\uC801\uC6A9"
Feat/알림 배치처리 적용 #17
Changes from 11 commits
b5aebff
1f491eb
900553b
67ab83a
0c757cf
6aa26f0
ee0e4b2
ba187da
74d499e
928a668
d345d28
a2a7e58
6a9e4ec
09b6897
8f93b53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package me.pinitnotification.application.notification; | ||
|
|
||
| import me.pinitnotification.domain.notification.UpcomingScheduleNotification; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public record NotificationDispatchItem( | ||
| UpcomingScheduleNotification notification, | ||
| List<String> tokens | ||
| ) { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package me.pinitnotification.application.notification; | ||
|
|
||
| import java.time.Instant; | ||
| import java.util.List; | ||
|
|
||
| public interface NotificationDispatchQueryRepository { | ||
| List<NotificationDispatchItem> findDueNotificationsWithTokens(Instant now); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,39 @@ | ||
| package me.pinitnotification.application.notification; | ||
|
|
||
| import me.pinitnotification.application.push.PushSendResult; | ||
| import me.pinitnotification.application.push.PushService; | ||
| import me.pinitnotification.domain.notification.UpcomingScheduleNotification; | ||
| import me.pinitnotification.domain.notification.UpcomingScheduleNotificationRepository; | ||
| import me.pinitnotification.domain.push.PushSubscription; | ||
| import me.pinitnotification.domain.push.PushSubscriptionRepository; | ||
| import me.pinitnotification.application.push.PushService; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.scheduling.annotation.Scheduled; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.time.Clock; | ||
| import java.time.OffsetDateTime; | ||
| import java.time.format.DateTimeParseException; | ||
| import java.time.Instant; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| @Service | ||
| public class NotificationDispatchScheduler { | ||
| private static final Logger log = LoggerFactory.getLogger(NotificationDispatchScheduler.class); | ||
|
|
||
| private final UpcomingScheduleNotificationRepository notificationRepository; | ||
| private final NotificationDispatchQueryRepository dispatchQueryRepository; | ||
| private final PushSubscriptionRepository pushSubscriptionRepository; | ||
| private final PushService pushService; | ||
| private final Clock clock; | ||
|
|
||
| public NotificationDispatchScheduler(UpcomingScheduleNotificationRepository notificationRepository, | ||
| NotificationDispatchQueryRepository dispatchQueryRepository, | ||
| PushSubscriptionRepository pushSubscriptionRepository, | ||
| PushService pushService, | ||
| Clock clock) { | ||
| this.notificationRepository = notificationRepository; | ||
| this.dispatchQueryRepository = dispatchQueryRepository; | ||
| this.pushSubscriptionRepository = pushSubscriptionRepository; | ||
| this.pushService = pushService; | ||
| this.clock = clock; | ||
|
|
@@ -38,27 +42,37 @@ public NotificationDispatchScheduler(UpcomingScheduleNotificationRepository noti | |
| @Scheduled(cron = "0 */10 * * * *") | ||
| @Transactional | ||
| public void dispatchDueNotifications() { | ||
| OffsetDateTime now = OffsetDateTime.now(clock); | ||
| List<UpcomingScheduleNotification> dueNotifications = notificationRepository.findAll().stream() | ||
| .filter(notification -> notification.isDue(now)) | ||
| .toList(); | ||
| Instant now = Instant.now(clock); | ||
| List<NotificationDispatchItem> dispatchItems = dispatchQueryRepository.findDueNotificationsWithTokens(now); | ||
|
|
||
| if (dueNotifications.isEmpty()) { | ||
| if (dispatchItems.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| dueNotifications.forEach(this::sendNotificationToOwner); | ||
| notificationRepository.deleteAllInBatch(dueNotifications); | ||
| Set<String> tokensToDelete = new LinkedHashSet<>(); | ||
|
||
| dispatchItems.forEach(item -> sendNotificationToOwner(item, tokensToDelete)); | ||
| notificationRepository.deleteAllInBatch(dispatchItems.stream().map(NotificationDispatchItem::notification).toList()); | ||
|
|
||
| if (!tokensToDelete.isEmpty()) { | ||
| pushSubscriptionRepository.deleteByTokens(tokensToDelete); | ||
| } | ||
|
Comment on lines
53
to
57
|
||
| } | ||
|
|
||
|
|
||
| private void sendNotificationToOwner(UpcomingScheduleNotification notification) { | ||
| List<PushSubscription> subscriptions = pushSubscriptionRepository.findAllByMemberId(notification.getOwnerId()); | ||
| if (subscriptions.isEmpty()) { | ||
| private void sendNotificationToOwner(NotificationDispatchItem dispatchItem, Set<String> tokensToDelete) { | ||
| UpcomingScheduleNotification notification = dispatchItem.notification(); | ||
| List<String> tokens = dispatchItem.tokens(); | ||
|
|
||
| if (tokens.isEmpty()) { | ||
| log.info("No push tokens for owner; skip sending. ownerId={}, scheduleId={}", notification.getOwnerId(), notification.getScheduleId()); | ||
| return; | ||
| } | ||
|
|
||
| subscriptions.forEach(subscription -> pushService.sendPushMessage(subscription.getToken(), notification)); | ||
| tokens.forEach(token -> { | ||
| PushSendResult result = pushService.sendPushMessage(token, notification); | ||
| if (result.shouldDeleteToken()) { | ||
| tokensToDelete.add(token); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package me.pinitnotification.application.push; | ||
|
|
||
| public record PushSendResult( | ||
| boolean success, | ||
| boolean invalidToken | ||
| ) { | ||
| public static PushSendResult successResult() { | ||
| return new PushSendResult(true, false); | ||
| } | ||
|
|
||
| public static PushSendResult invalidTokenResult() { | ||
| return new PushSendResult(false, true); | ||
| } | ||
|
|
||
| public static PushSendResult failedResult() { | ||
| return new PushSendResult(false, false); | ||
| } | ||
|
|
||
| public boolean shouldDeleteToken() { | ||
| return invalidToken; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||
| package me.pinitnotification.domain.push; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||
| import java.util.Optional; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
@@ -12,5 +13,7 @@ public interface PushSubscriptionRepository { | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| void deleteByToken(String token); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| /** | |
| * 주어진 푸시 토큰 컬렉션에 해당하는 모든 {@link PushSubscription} 엔티티를 일괄 삭제합니다. | |
| * <p> | |
| * 구현체에 따라 이 메서드는 데이터베이스 트랜잭션 내에서 실행될 수 있으며, | |
| * 전달된 토큰 중 실제로 존재하지 않는 토큰은 무시될 수 있습니다. | |
| * | |
| * @param tokens 삭제 대상이 되는 푸시 토큰들의 컬렉션 | |
| * <ul> | |
| * <li>{@code null} 이면 구현체에 따라 {@link IllegalArgumentException} 등 런타임 예외가 발생할 수 있습니다.</li> | |
| * <li>빈 컬렉션인 경우 일반적으로 아무 삭제도 수행하지 않습니다.</li> | |
| * </ul> | |
| * @throws RuntimeException 구현체에 따라 데이터 액세스 오류, 트랜잭션 오류 등이 발생할 경우 런타임 예외를 던질 수 있습니다. | |
| */ |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,103 @@ | ||||||||
| package me.pinitnotification.infrastructure.persistence.notification; | ||||||||
|
|
||||||||
| import me.pinitnotification.application.notification.NotificationDispatchItem; | ||||||||
| import me.pinitnotification.application.notification.NotificationDispatchQueryRepository; | ||||||||
| import me.pinitnotification.domain.notification.UpcomingScheduleNotification; | ||||||||
| import org.springframework.jdbc.core.simple.JdbcClient; | ||||||||
| import org.springframework.stereotype.Repository; | ||||||||
|
|
||||||||
| import java.time.Instant; | ||||||||
| import java.util.*; | ||||||||
|
|
||||||||
| @Repository | ||||||||
| public class NotificationDispatchQueryRepositoryAdapter implements NotificationDispatchQueryRepository { | ||||||||
| private static final String FIND_DUE_WITH_TOKENS_SQL = """ | ||||||||
| SELECT n.public_id, | ||||||||
| n.owner_id, | ||||||||
| n.schedule_id, | ||||||||
| n.schedule_title, | ||||||||
| n.schedule_start_time, | ||||||||
| n.idempotent_key, | ||||||||
| ps.token | ||||||||
| FROM upcoming_schedule_notification n | ||||||||
| LEFT JOIN push_subscription ps ON ps.member_id = n.owner_id | ||||||||
| WHERE n.schedule_start_time IS NOT NULL | ||||||||
| AND n.schedule_start_time <= ? | ||||||||
|
Comment on lines
+24
to
+25
|
||||||||
| WHERE n.schedule_start_time IS NOT NULL | |
| AND n.schedule_start_time <= ? | |
| WHERE n.schedule_start_time <= ? |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문제점: now.toString()를 SQL 파라미터로 사용하면 Instant가 ISO-8601 형식으로 변환되지만, 데이터베이스의 문자열 비교 방식에 따라 예상치 못한 결과가 발생할 수 있습니다.
영향: schedule_start_time이 VARCHAR로 저장되어 있어 문자열 비교가 수행되는데, 타임존 정보가 포함된 ISO-8601 문자열(예: "2024-06-01T10:00:00Z")과 데이터베이스에 저장된 형식이 다를 경우 정확한 비교가 불가능합니다.
수정 제안:
- 데이터베이스 컬럼을 TIMESTAMP 타입으로 변경하거나
- 파라미터 바인딩 시 적절한 타입 변환을 명시적으로 수행해야 합니다 (예:
.param(Timestamp.from(now))또는 데이터베이스가 지원하는 형식으로 변환)
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문제점: 토큰 중복이 발생할 수 있는 상황에 대한 처리가 없습니다. LEFT JOIN으로 인해 같은 토큰이 여러 번 추가될 수 있습니다.
영향: 동일한 member_id를 가진 push_subscription이 여러 개 있을 경우(같은 사용자가 여러 기기에서 같은 토큰을 사용하는 경우는 드물지만), 중복 토큰이 리스트에 포함되어 동일한 푸시 메시지가 여러 번 전송될 수 있습니다.
수정 제안: 토큰을 List 대신 Set으로 수집하거나, 중복을 방지하는 로직을 추가하세요. 예를 들어 accumulator.tokens가 Set이거나, 추가 전에 contains 체크를 수행할 수 있습니다.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문제점: List.copyOf(accumulator.tokens)를 사용하여 불변 리스트를 생성하는 것은 좋지만, 이미 내부적으로 mutable ArrayList를 사용하고 있어 방어적 복사의 의미가 제한적입니다.
영향: 성능상 약간의 오버헤드가 발생합니다. 만약 tokens가 Set으로 관리된다면 중복 제거와 불변성을 동시에 보장할 수 있습니다.
수정 제안: accumulator.tokens를 Set으로 변경하고 List.copyOf(accumulator.tokens) 또는 Set.copyOf()를 사용하여 일관성을 유지하세요.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,9 @@ | |
| uniqueConstraints = { | ||
| @UniqueConstraint(name = "uk_schedule_owner", columnNames = {"schedule_id", "owner_id"}), | ||
| @UniqueConstraint(name = "uk_idempotent_key", columnNames = {"idempotent_key"}) | ||
| }, | ||
| indexes = { | ||
| @Index(name = "idx_upcoming_start_time", columnList = "schedule_start_time") | ||
| } | ||
|
Comment on lines
+19
to
21
|
||
| ) | ||
| @Getter | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| import me.pinitnotification.domain.push.PushSubscriptionRepository; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import java.util.Collection; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
||
|
|
@@ -39,6 +40,14 @@ public void deleteByToken(String token) { | |
| jpaRepository.deleteByToken(token); | ||
| } | ||
|
|
||
| @Override | ||
| public void deleteByTokens(Collection<String> tokens) { | ||
| if (tokens == null || tokens.isEmpty()) { | ||
| return; | ||
| } | ||
| jpaRepository.deleteByTokenIn(tokens); | ||
| } | ||
|
Comment on lines
+43
to
+49
|
||
|
|
||
| @Override | ||
| public void deleteByMemberIdAndDeviceId(Long memberId, String deviceId) { | ||
| jpaRepository.deleteByMemberIdAndDeviceId(memberId, deviceId); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문제점: 메서드 이름
findDueNotificationsWithTokens가 실제 동작을 정확히 설명하지 못합니다. 이 메서드는 토큰이 있는 알림만 반환하는 것이 아니라, 토큰이 없는 알림도 함께 반환합니다.영향: 메서드 이름이 오해를 불러일으킬 수 있으며, 개발자가 토큰이 있는 알림만 반환될 것으로 잘못 이해할 수 있습니다.
수정 제안: 메서드 이름을
findDueNotificationsWithAggregatedTokens또는findAllDueNotificationsWithTokens로 변경하여 모든 만료된 알림을 반환하되 토큰을 집계한다는 의미를 명확히 하세요.