Skip to content

Commit e0fcf91

Browse files
jhan0121jhan0121
authored
이메일 재전송 포기 기준을 maxRetry 횟수 -> deadline 기반으로 전환 (#84)
* refactor: 전송 실패 이메일 재전송 기능 정리 * test: 테스트 회귀 문제 수정
1 parent 31cd44e commit e0fcf91

10 files changed

Lines changed: 180 additions & 62 deletions

File tree

‎src/main/java/com/recyclestudy/email/EmailRetryScheduler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ public class EmailRetryScheduler {
1010

1111
private final EmailRetryService emailRetryService;
1212

13-
@Scheduled(fixedDelay = 60_000)
13+
@Scheduled(fixedDelay = 300_000)
1414
public void runRetry() {
1515
emailRetryService.retryFailedEmails();
1616
}

‎src/main/java/com/recyclestudy/email/EmailRetryService.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,15 @@
2121
@RequiredArgsConstructor
2222
public class EmailRetryService {
2323

24-
// 단기 주기(1일 미만) 재시도 제외: review_cycle에 주기 컬럼이 없으므로 scheduledAt 경과 시간으로 단기/장기 여부를 역산
25-
private static final long SHORT_CYCLE_THRESHOLD_DAYS = 1;
26-
private static final int MAX_RETRY_COUNT = 3;
27-
2824
private final ReviewCycleRepository reviewCycleRepository;
2925
private final SingleReviewEmailSender singleReviewEmailSender;
3026
private final Clock clock;
3127

3228
@Transactional(readOnly = true)
3329
public void retryFailedEmails() {
34-
final LocalDateTime cutoffDateTime = LocalDateTime.now(clock).minusDays(SHORT_CYCLE_THRESHOLD_DAYS);
30+
final LocalDateTime now = LocalDateTime.now(clock);
3531
final List<ReviewCycle> failedCycles = reviewCycleRepository.findAllRetryableCycles(
36-
NotificationStatus.FAILED, MAX_RETRY_COUNT, cutoffDateTime);
32+
NotificationStatus.FAILED, now);
3733

3834
if (failedCycles.isEmpty()) {
3935
return;

‎src/main/java/com/recyclestudy/review/domain/NotificationHistory.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,15 @@ public class NotificationHistory extends BaseEntity {
3939
@Column(name = "last_attempted_at")
4040
private LocalDateTime lastAttemptedAt;
4141

42+
@Column(name = "deadline")
43+
private LocalDateTime deadline;
44+
4245
public static NotificationHistory withoutId(
4346
final ReviewCycle reviewCycle,
4447
final NotificationStatus status
4548
) {
4649
validateNotNull(reviewCycle, status);
47-
return new NotificationHistory(reviewCycle, status, 0, null);
50+
return new NotificationHistory(reviewCycle, status, 0, null, null);
4851
}
4952

5053
private static void validateNotNull(

‎src/main/java/com/recyclestudy/review/repository/NotificationHistoryRepository.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,15 @@ int updateStatus(
2626
@Modifying(clearAutomatically = true)
2727
@Query("""
2828
UPDATE NotificationHistory nh
29-
SET nh.status = :status, nh.failCount = nh.failCount + 1, nh.lastAttemptedAt = :now
30-
WHERE nh.reviewCycle.id IN :reviewCycleIds
29+
SET nh.status = :status, nh.failCount = nh.failCount + 1,
30+
nh.lastAttemptedAt = :now, nh.deadline = :deadline
31+
WHERE nh.reviewCycle.id = :reviewCycleId
3132
""")
3233
int updateStatusWithIncrementFailCount(
33-
@Param("reviewCycleIds") List<Long> reviewCycleIds,
34+
@Param("reviewCycleId") Long reviewCycleId,
3435
@Param("status") NotificationStatus status,
35-
@Param("now") LocalDateTime now
36+
@Param("now") LocalDateTime now,
37+
@Param("deadline") LocalDateTime deadline
3638
);
3739

3840
@Query("""

‎src/main/java/com/recyclestudy/review/repository/ReviewCycleRepository.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
import com.recyclestudy.review.domain.ReviewCycle;
55
import java.time.LocalDateTime;
66
import java.util.List;
7+
import java.util.Optional;
78
import org.springframework.data.jpa.repository.JpaRepository;
89
import org.springframework.data.jpa.repository.Query;
910
import org.springframework.data.repository.query.Param;
1011

12+
1113
public interface ReviewCycleRepository extends JpaRepository<ReviewCycle, Long> {
1214

1315
@Query("""
@@ -29,12 +31,15 @@ List<ReviewCycle> findAllByScheduledAt(
2931
JOIN FETCH r.member
3032
JOIN NotificationHistory nh ON rc.id = nh.reviewCycle.id
3133
WHERE nh.status = :status
32-
AND nh.failCount < :maxRetryCount
33-
AND rc.scheduledAt <= :cutoffDateTime
34+
AND nh.deadline > :now
3435
""")
3536
List<ReviewCycle> findAllRetryableCycles(
3637
@Param("status") NotificationStatus status,
37-
@Param("maxRetryCount") int maxRetryCount,
38-
@Param("cutoffDateTime") LocalDateTime cutoffDateTime
38+
@Param("now") LocalDateTime now
39+
);
40+
41+
Optional<ReviewCycle> findFirstByReview_IdAndScheduledAtGreaterThanOrderByScheduledAtAsc(
42+
Long reviewId,
43+
LocalDateTime currentScheduledAt
3944
);
4045
}

‎src/main/java/com/recyclestudy/review/service/NotificationHistoryService.java‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package com.recyclestudy.review.service;
22

33
import com.recyclestudy.review.domain.NotificationStatus;
4+
import com.recyclestudy.review.domain.ReviewCycle;
45
import com.recyclestudy.review.repository.NotificationHistoryRepository;
6+
import com.recyclestudy.review.repository.ReviewCycleRepository;
57
import java.time.Clock;
68
import java.time.LocalDateTime;
79
import java.util.List;
@@ -15,7 +17,10 @@
1517
@Slf4j
1618
public class NotificationHistoryService {
1719

20+
private static final long LAST_CYCLE_DEADLINE_HOURS = 24;
21+
1822
private final NotificationHistoryRepository notificationHistoryRepository;
23+
private final ReviewCycleRepository reviewCycleRepository;
1924
private final Clock clock;
2025

2126
@Transactional
@@ -26,14 +31,29 @@ public void updateStatus(final List<Long> reviewCycleIds, final NotificationStat
2631
final LocalDateTime now = LocalDateTime.now(clock);
2732
int updated;
2833
if (status == NotificationStatus.FAILED) {
29-
updated = notificationHistoryRepository.updateStatusWithIncrementFailCount(reviewCycleIds, status, now);
34+
updated = updateToFailed(reviewCycleIds, now);
3035
} else {
3136
updated = notificationHistoryRepository.updateStatus(reviewCycleIds, status, now);
3237
}
3338
if (updated != reviewCycleIds.size()) {
3439
log.warn("[NOTIFY_HIST_MISMATCH] 기대={}, 실제={}", reviewCycleIds.size(), updated);
3540
}
36-
3741
log.info("[NOTIFY_HIST_UPDATED] 알림 이력 상태 변경: status={}, count={}", status, updated);
3842
}
43+
44+
private int updateToFailed(final List<Long> reviewCycleIds, final LocalDateTime now) {
45+
int updated = 0;
46+
for (final Long reviewCycleId : reviewCycleIds) {
47+
final ReviewCycle reviewCycle = reviewCycleRepository.findById(reviewCycleId)
48+
.orElseThrow(() -> new IllegalStateException("ReviewCycle을 찾을 수 없습니다: " + reviewCycleId));
49+
final LocalDateTime deadline = reviewCycleRepository
50+
.findFirstByReview_IdAndScheduledAtGreaterThanOrderByScheduledAtAsc(
51+
reviewCycle.getReview().getId(), reviewCycle.getScheduledAt())
52+
.map(ReviewCycle::getScheduledAt)
53+
.orElse(reviewCycle.getScheduledAt().plusHours(LAST_CYCLE_DEADLINE_HOURS));
54+
updated += notificationHistoryRepository.updateStatusWithIncrementFailCount(
55+
reviewCycleId, NotificationStatus.FAILED, now, deadline);
56+
}
57+
return updated;
58+
}
3959
}

‎src/main/resources/db/migration/V20260307_1__add_deadline_to_notification_history.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE notification_history
2+
ADD COLUMN deadline DATETIME NULL;

‎src/test/java/com/recyclestudy/email/EmailRetryServiceTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ void retryFailedEmails_noData() {
5151
given(clock.instant()).willReturn(Instant.parse("2026-01-01T00:00:00Z"));
5252
given(clock.getZone()).willReturn(ZoneId.of("UTC"));
5353
given(reviewCycleRepository.findAllRetryableCycles(
54-
eq(NotificationStatus.FAILED), any(Integer.class), any(LocalDateTime.class)))
54+
eq(NotificationStatus.FAILED), any(LocalDateTime.class)))
5555
.willReturn(Collections.emptyList());
5656

5757
// when
@@ -85,7 +85,7 @@ void retryFailedEmails_success() {
8585
given(cycle2.getReview()).willReturn(review1);
8686

8787
given(reviewCycleRepository.findAllRetryableCycles(
88-
eq(NotificationStatus.FAILED), any(Integer.class), any(LocalDateTime.class)))
88+
eq(NotificationStatus.FAILED), any(LocalDateTime.class)))
8989
.willReturn(List.of(cycle1, cycle2));
9090

9191
// when

‎src/test/java/com/recyclestudy/review/repository/ReviewCycleRepositoryTest.java‎

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.recyclestudy.review.domain.ReviewURL;
1111
import java.time.LocalDateTime;
1212
import java.util.List;
13+
import java.util.Optional;
1314
import org.junit.jupiter.api.DisplayName;
1415
import org.junit.jupiter.api.Test;
1516
import org.springframework.beans.factory.annotation.Autowired;
@@ -32,23 +33,23 @@ class ReviewCycleRepositoryTest {
3233
@Autowired
3334
private ReviewRepository reviewRepository;
3435

35-
private static final LocalDateTime CUTOFF = LocalDateTime.now().minusDays(1);
36-
private static final LocalDateTime LONG_AGO = LocalDateTime.now().minusDays(2);
37-
private static final LocalDateTime RECENT = LocalDateTime.now().minusHours(1);
36+
private static final LocalDateTime NOW = LocalDateTime.of(2026, 1, 1, 0, 0, 0);
37+
private static final LocalDateTime FUTURE_DEADLINE = NOW.plusHours(23);
38+
private static final LocalDateTime PAST_DEADLINE = NOW.minusHours(1);
39+
private static final LocalDateTime SCHEDULED_AT = NOW.minusDays(1);
3840

3941
@Test
40-
@DisplayName("FAILED 상태이고 failCount가 최대 횟수 미만이면 재시도 대상에 포함된다")
42+
@DisplayName("FAILED 상태이고 deadline이 현재보다 미래이면 재시도 대상에 포함된다")
4143
void findAllRetryableCycles_success() {
4244
// given
43-
final ReviewCycle cycle = saveCycle("retry@email.com", LONG_AGO);
45+
final ReviewCycle cycle = saveCycle("retry@email.com", SCHEDULED_AT);
4446
notificationHistoryRepository.save(NotificationHistory.withoutId(cycle, NotificationStatus.PENDING));
45-
4647
notificationHistoryRepository.updateStatusWithIncrementFailCount(
47-
List.of(cycle.getId()), NotificationStatus.FAILED, LocalDateTime.now());
48+
cycle.getId(), NotificationStatus.FAILED, NOW, FUTURE_DEADLINE);
4849

4950
// when
5051
final List<ReviewCycle> results = reviewCycleRepository.findAllRetryableCycles(
51-
NotificationStatus.FAILED, 3, CUTOFF);
52+
NotificationStatus.FAILED, NOW);
5253

5354
// then
5455
assertThat(results).hasSize(1);
@@ -59,12 +60,12 @@ void findAllRetryableCycles_success() {
5960
@DisplayName("PENDING 상태만 있는 경우는 재시도 대상이 아니다")
6061
void findAllRetryableCycles_pendingOnly() {
6162
// given
62-
final ReviewCycle cycle = saveCycle("pending@email.com", LONG_AGO);
63+
final ReviewCycle cycle = saveCycle("pending@email.com", SCHEDULED_AT);
6364
notificationHistoryRepository.save(NotificationHistory.withoutId(cycle, NotificationStatus.PENDING));
6465

6566
// when
6667
final List<ReviewCycle> results = reviewCycleRepository.findAllRetryableCycles(
67-
NotificationStatus.FAILED, 3, CUTOFF);
68+
NotificationStatus.FAILED, NOW);
6869

6970
// then
7071
assertThat(results).isEmpty();
@@ -74,36 +75,31 @@ void findAllRetryableCycles_pendingOnly() {
7475
@DisplayName("이미 SENT 상태이면 재시도 대상이 아니다")
7576
void findAllRetryableCycles_alreadySent() {
7677
// given
77-
final ReviewCycle cycle = saveCycle("sent@email.com", LONG_AGO);
78+
final ReviewCycle cycle = saveCycle("sent@email.com", SCHEDULED_AT);
7879
notificationHistoryRepository.save(NotificationHistory.withoutId(cycle, NotificationStatus.PENDING));
79-
8080
notificationHistoryRepository.updateStatus(
81-
List.of(cycle.getId()), NotificationStatus.SENT, LocalDateTime.now());
81+
List.of(cycle.getId()), NotificationStatus.SENT, NOW);
8282

8383
// when
8484
final List<ReviewCycle> results = reviewCycleRepository.findAllRetryableCycles(
85-
NotificationStatus.FAILED, 3, CUTOFF);
85+
NotificationStatus.FAILED, NOW);
8686

8787
// then
8888
assertThat(results).isEmpty();
8989
}
9090

9191
@Test
92-
@DisplayName("failCount가 최대 재시도 횟수에 도달하면 재시도 대상이 아니다")
93-
void findAllRetryableCycles_maxRetryReached() {
92+
@DisplayName("deadline이 현재보다 과거이면 재시도 대상이 아니다")
93+
void findAllRetryableCycles_deadlineExpired() {
9494
// given
95-
final ReviewCycle cycle = saveCycle("max@email.com", LONG_AGO);
95+
final ReviewCycle cycle = saveCycle("expired@email.com", SCHEDULED_AT);
9696
notificationHistoryRepository.save(NotificationHistory.withoutId(cycle, NotificationStatus.PENDING));
97-
98-
// failCount를 3으로 만들기 위해 3번 increment
99-
for (int i = 0; i < 3; i++) {
100-
notificationHistoryRepository.updateStatusWithIncrementFailCount(
101-
List.of(cycle.getId()), NotificationStatus.FAILED, LocalDateTime.now());
102-
}
97+
notificationHistoryRepository.updateStatusWithIncrementFailCount(
98+
cycle.getId(), NotificationStatus.FAILED, NOW, PAST_DEADLINE);
10399

104100
// when
105101
final List<ReviewCycle> results = reviewCycleRepository.findAllRetryableCycles(
106-
NotificationStatus.FAILED, 3, CUTOFF);
102+
NotificationStatus.FAILED, NOW);
107103

108104
// then
109105
assertThat(results).isEmpty();
@@ -113,32 +109,71 @@ void findAllRetryableCycles_maxRetryReached() {
113109
@DisplayName("notification_history가 없는 경우 재시도 대상이 아니다")
114110
void findAllRetryableCycles_noHistory() {
115111
// given
116-
saveCycle("new@email.com", LONG_AGO);
112+
saveCycle("new@email.com", SCHEDULED_AT);
117113

118114
// when
119115
final List<ReviewCycle> results = reviewCycleRepository.findAllRetryableCycles(
120-
NotificationStatus.FAILED, 3, CUTOFF);
116+
NotificationStatus.FAILED, NOW);
121117

122118
// then
123119
assertThat(results).isEmpty();
124120
}
125121

126122
@Test
127-
@DisplayName("scheduledAt이 cutoffDateTime보다 최근인 단기 주기는 재시도 대상이 아니다")
128-
void findAllRetryableCycles_shortCycle() {
123+
@DisplayName("failCount가 아무리 높아도 deadline이 미래이면 재시도 대상에 포함된다")
124+
void findAllRetryableCycles_failCountDoesNotAffectEligibility() {
129125
// given
130-
final ReviewCycle cycle = saveCycle("short@email.com", RECENT);
126+
final ReviewCycle cycle = saveCycle("many-fails@email.com", SCHEDULED_AT);
131127
notificationHistoryRepository.save(NotificationHistory.withoutId(cycle, NotificationStatus.PENDING));
132-
133-
notificationHistoryRepository.updateStatusWithIncrementFailCount(
134-
List.of(cycle.getId()), NotificationStatus.FAILED, LocalDateTime.now());
128+
// failCount를 10으로 설정해도 deadline이 미래이면 재시도 대상
129+
for (int i = 0; i < 10; i++) {
130+
notificationHistoryRepository.updateStatusWithIncrementFailCount(
131+
cycle.getId(), NotificationStatus.FAILED, NOW, FUTURE_DEADLINE);
132+
}
135133

136134
// when
137135
final List<ReviewCycle> results = reviewCycleRepository.findAllRetryableCycles(
138-
NotificationStatus.FAILED, 3, CUTOFF);
136+
NotificationStatus.FAILED, NOW);
139137

140138
// then
141-
assertThat(results).isEmpty();
139+
assertThat(results).hasSize(1);
140+
}
141+
142+
@Test
143+
@DisplayName("다음 주기가 있으면 scheduledAt이 가장 가까운 ReviewCycle을 반환한다")
144+
void findNextScheduledAt_success() {
145+
// given
146+
final Member member = memberRepository.save(Member.withoutId(Email.from("next@email.com")));
147+
final Review review = reviewRepository.save(Review.withoutId(member, ReviewURL.from("url")));
148+
final ReviewCycle current = reviewCycleRepository.save(ReviewCycle.withoutId(review, NOW.plusHours(1)));
149+
final ReviewCycle next = reviewCycleRepository.save(ReviewCycle.withoutId(review, NOW.plusDays(1)));
150+
reviewCycleRepository.save(ReviewCycle.withoutId(review, NOW.plusDays(7)));
151+
152+
// when
153+
final Optional<ReviewCycle> result = reviewCycleRepository
154+
.findFirstByReview_IdAndScheduledAtGreaterThanOrderByScheduledAtAsc(
155+
review.getId(), current.getScheduledAt());
156+
157+
// then
158+
assertThat(result).isPresent();
159+
assertThat(result.get().getId()).isEqualTo(next.getId());
160+
}
161+
162+
@Test
163+
@DisplayName("마지막 주기이면 Optional.empty()를 반환한다")
164+
void findNextScheduledAt_lastCycle() {
165+
// given
166+
final Member member = memberRepository.save(Member.withoutId(Email.from("last@email.com")));
167+
final Review review = reviewRepository.save(Review.withoutId(member, ReviewURL.from("url")));
168+
final ReviewCycle last = reviewCycleRepository.save(ReviewCycle.withoutId(review, NOW.plusDays(30)));
169+
170+
// when
171+
final Optional<ReviewCycle> result = reviewCycleRepository
172+
.findFirstByReview_IdAndScheduledAtGreaterThanOrderByScheduledAtAsc(
173+
review.getId(), last.getScheduledAt());
174+
175+
// then
176+
assertThat(result).isEmpty();
142177
}
143178

144179
private ReviewCycle saveCycle(final String email, final LocalDateTime scheduledAt) {

0 commit comments

Comments
 (0)