Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.recyclestudy.member.domain.Email;
import com.recyclestudy.review.domain.NotificationStatus;
import com.recyclestudy.review.domain.ReviewURL;
import com.recyclestudy.review.service.NotificationHistoryService;
import com.recyclestudy.review.service.ReviewCycleService;
import com.recyclestudy.review.service.output.ReviewSendOutput.ReviewSendElement;
import java.util.List;
import lombok.RequiredArgsConstructor;
Expand All @@ -20,7 +20,7 @@ public class SingleReviewEmailSender {

private final EmailSender emailSender;
private final TemplateEngine templateEngine;
private final NotificationHistoryService notificationHistoryService;
private final ReviewCycleService reviewCycleService;

@Async
public void sendOne(final ReviewSendElement element) {
Expand All @@ -30,10 +30,10 @@ public void sendOne(final ReviewSendElement element) {
final boolean success = sendToTargetEmail(targetEmail, message);

if (success) {
notificationHistoryService.updateStatus(element.reviewCycleIds(), NotificationStatus.SENT);
reviewCycleService.updateStatus(element.reviewCycleIds(), NotificationStatus.SENT);
return;
}
notificationHistoryService.updateStatus(element.reviewCycleIds(), NotificationStatus.FAILED);
reviewCycleService.updateStatus(element.reviewCycleIds(), NotificationStatus.FAILED);
}

private boolean sendToTargetEmail(final Email targetEmail, final String message) {
Expand Down

This file was deleted.

35 changes: 31 additions & 4 deletions src/main/java/com/recyclestudy/review/domain/ReviewCycle.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import com.recyclestudy.common.NullValidator;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.FetchType;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
Expand All @@ -30,15 +32,40 @@ public class ReviewCycle extends BaseEntity {
@Column(name = "scheduled_at", nullable = false)
private LocalDateTime scheduledAt;

public static ReviewCycle withoutId(final Review review, final LocalDateTime scheduledAt) {
validateNotNull(review, scheduledAt);
return new ReviewCycle(review, scheduledAt);
@Enumerated(EnumType.STRING)
@Column(name = "status", nullable = false)
private NotificationStatus status;

@Column(name = "fail_count", nullable = false)
private int failCount;

@Column(name = "last_attempted_at")
private LocalDateTime lastAttemptedAt;

@Column(name = "deadline", nullable = false)
private LocalDateTime deadline;

public static ReviewCycle withoutId(
final Review review,
final LocalDateTime scheduledAt,
final NotificationStatus status,
final LocalDateTime deadline
) {
validateNotNull(review, scheduledAt, status, deadline);
return new ReviewCycle(review, scheduledAt, status, 0, null, deadline);
}

private static void validateNotNull(final Review review, final LocalDateTime scheduledAt) {
private static void validateNotNull(
final Review review,
final LocalDateTime scheduledAt,
final NotificationStatus status,
final LocalDateTime deadline
) {
NullValidator.builder()
.add(Fields.review, review)
.add(Fields.scheduledAt, scheduledAt)
.add(Fields.status, status)
.add(Fields.deadline, deadline)
.validate();
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.time.LocalDateTime;
import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;

Expand All @@ -15,9 +16,8 @@ public interface ReviewCycleRepository extends JpaRepository<ReviewCycle, Long>
SELECT rc FROM ReviewCycle rc
JOIN FETCH rc.review r
JOIN FETCH r.member
JOIN NotificationHistory nh ON rc.id = nh.reviewCycle.id
WHERE rc.scheduledAt <= :scheduledAt
AND nh.status = :status
AND rc.status = :status
""")
List<ReviewCycle> findAllByScheduledAt(
@Param("scheduledAt") LocalDateTime scheduledAt,
Expand All @@ -28,13 +28,48 @@ List<ReviewCycle> findAllByScheduledAt(
SELECT rc FROM ReviewCycle rc
JOIN FETCH rc.review r
JOIN FETCH r.member
JOIN NotificationHistory nh ON rc.id = nh.reviewCycle.id
WHERE nh.status = :status
AND nh.deadline > :now
WHERE rc.status = :status
AND rc.deadline > :now
""")
List<ReviewCycle> findAllRetryableCycles(
@Param("status") NotificationStatus status,
@Param("now") LocalDateTime now
);

@Query("""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[정보] r.member JOIN FETCH 불일치

findAllByScheduledAt, findAllRetryableCyclesJOIN FETCH r.member까지 함께 로딩하지만, 이 쿼리는 JOIN FETCH rc.review r만 있고 member는 lazy 로딩으로 남습니다.

현재 findNextReview()에서는 getScheduledAt()만 접근하므로 문제없지만, 향후 review.member에 접근하는 코드가 추가되면 N+1이 발생할 수 있습니다. 의도적인 생략이라면 주석으로 명시하거나, 일관성을 위해 JOIN FETCH r.member를 추가하는 것을 고려해주세요.

SELECT rc FROM ReviewCycle rc
JOIN FETCH rc.review r
JOIN FETCH r.member
WHERE r.member.id = :memberId
AND rc.status = :status
ORDER BY rc.scheduledAt ASC
""")
List<ReviewCycle> findAllByMemberAndStatus(
@Param("memberId") Long memberId,
@Param("status") NotificationStatus status
);

@Modifying(clearAutomatically = true, flushAutomatically = true)
@Query("""
UPDATE ReviewCycle rc
SET rc.status = :status, rc.lastAttemptedAt = :now
WHERE rc.id IN :ids
""")
int updateStatus(
@Param("ids") List<Long> ids,
@Param("status") NotificationStatus status,
@Param("now") LocalDateTime now
);

@Modifying(clearAutomatically = true, flushAutomatically = true)
@Query("""
UPDATE ReviewCycle rc
SET rc.status = :status, rc.failCount = rc.failCount + 1, rc.lastAttemptedAt = :now
WHERE rc.id IN :ids
""")
int updateStatusAndIncrementFailCount(
@Param("ids") List<Long> ids,
@Param("status") NotificationStatus status,
@Param("now") LocalDateTime now
);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,29 @@
import com.recyclestudy.exception.UnauthorizedException;
import com.recyclestudy.member.domain.Member;
import com.recyclestudy.member.repository.MemberRepository;
import com.recyclestudy.review.domain.NotificationHistory;
import com.recyclestudy.review.domain.NotificationStatus;
import com.recyclestudy.review.domain.ReviewCycle;
import com.recyclestudy.review.repository.NotificationHistoryRepository;
import com.recyclestudy.review.repository.ReviewCycleRepository;
import com.recyclestudy.review.service.input.NextReviewInput;
import com.recyclestudy.review.service.input.ReviewSendInput;
import com.recyclestudy.review.service.output.NextReviewOutput;
import com.recyclestudy.review.service.output.ReviewSendOutput;
import java.time.Clock;
import java.time.LocalDateTime;
import java.util.List;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
@Slf4j
public class ReviewCycleService {

private final ReviewCycleRepository reviewCycleRepository;
private final MemberRepository memberRepository;
private final NotificationHistoryRepository notificationHistoryRepository;
private final Clock clock;

@Transactional(readOnly = true)
public ReviewSendOutput findTargetReviewCycle(final ReviewSendInput input) {
Expand All @@ -38,18 +39,36 @@ public NextReviewOutput findNextReview(final NextReviewInput input) {
final Member member = memberRepository.findByIdentifier(input.identifier())
.orElseThrow(() -> new UnauthorizedException("유효하지 않은 디바이스입니다"));

final List<NotificationHistory> allPending = notificationHistoryRepository
final List<ReviewCycle> allPending = reviewCycleRepository
.findAllByMemberAndStatus(member.getId(), NotificationStatus.PENDING);

if (allPending.isEmpty()) {
return NextReviewOutput.empty();
}

final LocalDateTime earliest = allPending.getFirst().getReviewCycle().getScheduledAt();
final LocalDateTime earliest = allPending.getFirst().getScheduledAt();
final int count = (int) allPending.stream()
.takeWhile(nh -> nh.getReviewCycle().getScheduledAt().equals(earliest))
.takeWhile(rc -> rc.getScheduledAt().equals(earliest))
.count();

return NextReviewOutput.of(earliest, count);
}

@Transactional
public void updateStatus(final List<Long> reviewCycleIds, final NotificationStatus status) {
if (reviewCycleIds.isEmpty()) {
return;
}
final LocalDateTime now = LocalDateTime.now(clock);
final int updated;
if (status == NotificationStatus.FAILED) {
updated = reviewCycleRepository.updateStatusAndIncrementFailCount(reviewCycleIds, status, now);
} else {
updated = reviewCycleRepository.updateStatus(reviewCycleIds, status, now);
}
if (updated != reviewCycleIds.size()) {
log.warn("[RC_STATUS_MISMATCH] 기대={}, 실제={}", reviewCycleIds.size(), updated);
}
log.info("[RC_STATUS_UPDATED] 상태 변경: status={}, count={}", status, updated);
}
}
Loading
Loading